Fix/use current_retrievability_seconds in SQL to keep consistent with card info (#4231)

* Feat/use current_retrievability_seconds in SQL

* replace `days_since_last_review` with `seconds_since_last_review`

* Update is_due_in_days logic to include original or current due date check
This commit is contained in:
Jarrett Ye 2025-07-28 17:06:20 +08:00 committed by GitHub
parent 3dc6b6b3ca
commit 4dc00556c1
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 78 additions and 57 deletions

View file

@ -105,7 +105,8 @@ impl Card {
/// Returns true if the card has a due date in terms of days.
fn is_due_in_days(&self) -> bool {
matches!(self.queue, CardQueue::DayLearn | CardQueue::Review)
self.original_or_current_due() <= 365_000 // keep consistent with SQL
|| matches!(self.queue, CardQueue::DayLearn | CardQueue::Review)
|| (self.ctype == CardType::Review && self.is_undue_queue())
}
@ -127,19 +128,17 @@ impl Card {
/// This uses card.due and card.ivl to infer the elapsed time. If 'set due
/// date' or an add-on has changed the due date, this won't be accurate.
pub(crate) fn days_since_last_review(&self, timing: &SchedTimingToday) -> Option<u32> {
pub(crate) fn seconds_since_last_review(&self, timing: &SchedTimingToday) -> Option<u32> {
if let Some(last_review_time) = self.last_review_time {
Some(timing.next_day_at.elapsed_days_since(last_review_time) as u32)
Some(timing.now.elapsed_secs_since(last_review_time) as u32)
} else if !self.is_due_in_days() {
Some(
(timing.next_day_at.0 as u32).saturating_sub(self.original_or_current_due() as u32)
/ 86_400,
)
let last_review_time =
TimestampSecs(self.original_or_current_due() as i64 - self.interval as i64);
Some(timing.now.elapsed_secs_since(last_review_time) as u32)
} else {
self.due_time(timing).map(|due| {
(due.adding_secs(-86_400 * self.interval as i64)
.elapsed_secs()
/ 86_400) as u32
.elapsed_secs()) as u32
})
}
}
@ -543,12 +542,12 @@ impl RowContext {
self.cards[0]
.memory_state
.as_ref()
.zip(self.cards[0].days_since_last_review(&self.timing))
.zip(self.cards[0].seconds_since_last_review(&self.timing))
.zip(Some(self.cards[0].decay.unwrap_or(FSRS5_DEFAULT_DECAY)))
.map(|((state, days_elapsed), decay)| {
let r = FSRS::new(None).unwrap().current_retrievability(
.map(|((state, seconds), decay)| {
let r = FSRS::new(None).unwrap().current_retrievability_seconds(
(*state).into(),
days_elapsed,
seconds,
decay,
);
format!("{:.0}%", r * 100.)

View file

@ -378,9 +378,10 @@ fn card_order_from_sort_column(column: Column, timing: SchedTimingToday) -> Cow<
Column::Stability => "extract_fsrs_variable(c.data, 's') asc".into(),
Column::Difficulty => "extract_fsrs_variable(c.data, 'd') asc".into(),
Column::Retrievability => format!(
"extract_fsrs_retrievability(c.data, case when c.odue !=0 then c.odue else c.due end, c.ivl, {}, {}) asc",
"extract_fsrs_retrievability(c.data, case when c.odue !=0 then c.odue else c.due end, c.ivl, {}, {}, {}) asc",
timing.days_elapsed,
timing.next_day_at.0
timing.next_day_at.0,
timing.now.0,
)
.into(),
}

View file

@ -418,13 +418,13 @@ impl SqlWriter<'_> {
write!(self.sql, "extract_fsrs_variable(c.data, 'd') {op} {d}").unwrap()
}
PropertyKind::Retrievability(r) => {
let (elap, next_day_at) = {
let (elap, next_day_at, now) = {
let timing = self.col.timing_today()?;
(timing.days_elapsed, timing.next_day_at)
(timing.days_elapsed, timing.next_day_at, timing.now)
};
write!(
self.sql,
"extract_fsrs_retrievability(c.data, case when c.odue !=0 then c.odue else c.due end, c.ivl, {elap}, {next_day_at}) {op} {r}"
"extract_fsrs_retrievability(c.data, case when c.odue !=0 then c.odue else c.due end, c.ivl, {elap}, {next_day_at}, {now}) {op} {r}"
)
.unwrap()
}

View file

@ -30,10 +30,10 @@ impl GraphsContext {
.or_insert((0.0, 0));
entry.1 += 1;
if let Some(state) = card.memory_state {
let elapsed_days = card.days_since_last_review(&timing).unwrap_or_default();
let r = fsrs.current_retrievability(
let elapsed_seconds = card.seconds_since_last_review(&timing).unwrap_or_default();
let r = fsrs.current_retrievability_seconds(
state.into(),
elapsed_days,
elapsed_seconds,
card.decay.unwrap_or(FSRS5_DEFAULT_DECAY),
);

View file

@ -14,6 +14,8 @@ pub(crate) fn order_and_limit_for_search(
) -> String {
let temp_string;
let today = timing.days_elapsed;
let next_day_at = timing.next_day_at.0;
let now = timing.now.0;
let order = match term.order() {
FilteredSearchOrder::OldestReviewedFirst => "(select max(id) from revlog where cid=c.id)",
FilteredSearchOrder::Random => "random()",
@ -29,15 +31,13 @@ pub(crate) fn order_and_limit_for_search(
&temp_string
}
FilteredSearchOrder::RetrievabilityAscending => {
let next_day_at = timing.next_day_at.0;
temp_string =
build_retrievability_query(fsrs, today, next_day_at, SqlSortOrder::Ascending);
build_retrievability_query(fsrs, today, next_day_at, now, SqlSortOrder::Ascending);
&temp_string
}
FilteredSearchOrder::RetrievabilityDescending => {
let next_day_at = timing.next_day_at.0;
temp_string =
build_retrievability_query(fsrs, today, next_day_at, SqlSortOrder::Descending);
build_retrievability_query(fsrs, today, next_day_at, now, SqlSortOrder::Descending);
&temp_string
}
};
@ -49,11 +49,12 @@ fn build_retrievability_query(
fsrs: bool,
today: u32,
next_day_at: i64,
now: i64,
order: SqlSortOrder,
) -> String {
if fsrs {
format!(
"extract_fsrs_relative_retrievability(c.data, case when c.odue !=0 then c.odue else c.due end, {today}, ivl, {next_day_at}) {order}"
"extract_fsrs_relative_retrievability(c.data, case when c.odue !=0 then c.odue else c.due end, {today}, ivl, {next_day_at}, {now}) {order}"
)
} else {
format!(

View file

@ -808,8 +808,9 @@ impl fmt::Display for ReviewOrderSubclause {
ReviewOrderSubclause::RetrievabilityFsrs { timing, order } => {
let today = timing.days_elapsed;
let next_day_at = timing.next_day_at.0;
let now = timing.now.0;
temp_string =
format!("extract_fsrs_relative_retrievability(data, case when odue !=0 then odue else due end, {today}, ivl, {next_day_at}) {order}");
format!("extract_fsrs_relative_retrievability(data, case when odue !=0 then odue else due end, {today}, ivl, {next_day_at}, {now}) {order}");
&temp_string
}
ReviewOrderSubclause::Added => "nid asc, ord asc",

View file

@ -310,14 +310,14 @@ fn add_extract_fsrs_variable(db: &Connection) -> rusqlite::Result<()> {
}
/// eg. extract_fsrs_retrievability(card.data, card.due, card.ivl,
/// timing.days_elapsed, timing.next_day_at) -> float | null
/// timing.days_elapsed, timing.next_day_at, timing.now) -> float | null
fn add_extract_fsrs_retrievability(db: &Connection) -> rusqlite::Result<()> {
db.create_scalar_function(
"extract_fsrs_retrievability",
5,
6,
FunctionFlags::SQLITE_DETERMINISTIC,
move |ctx| {
assert_eq!(ctx.len(), 5, "called with unexpected number of arguments");
assert_eq!(ctx.len(), 6, "called with unexpected number of arguments");
let Ok(card_data) = ctx.get_raw(0).as_str() else {
return Ok(None);
};
@ -328,18 +328,18 @@ fn add_extract_fsrs_retrievability(db: &Connection) -> rusqlite::Result<()> {
let Ok(due) = ctx.get_raw(1).as_i64() else {
return Ok(None);
};
let days_elapsed = if let Some(last_review_time) = card_data.last_review_time {
// Use last_review_time to calculate days_elapsed
let Ok(next_day_at) = ctx.get_raw(4).as_i64() else {
return Ok(None);
};
(next_day_at as u32).saturating_sub(last_review_time.0 as u32) / 86_400
let Ok(now) = ctx.get_raw(5).as_i64() else {
return Ok(None);
};
let seconds_elapsed = if let Some(last_review_time) = card_data.last_review_time {
now.saturating_sub(last_review_time.0) as u32
} else if due > 365_000 {
// (re)learning card in seconds
let Ok(next_day_at) = ctx.get_raw(4).as_i64() else {
let Ok(ivl) = ctx.get_raw(2).as_i64() else {
return Ok(None);
};
(next_day_at as u32).saturating_sub(due as u32) / 86_400
let last_review_time = due.saturating_sub(ivl);
now.saturating_sub(last_review_time) as u32
} else {
let Ok(ivl) = ctx.get_raw(2).as_i64() else {
return Ok(None);
@ -348,29 +348,32 @@ fn add_extract_fsrs_retrievability(db: &Connection) -> rusqlite::Result<()> {
return Ok(None);
};
let review_day = due.saturating_sub(ivl);
(days_elapsed as u32).saturating_sub(review_day as u32)
days_elapsed.saturating_sub(review_day) as u32 * 86_400
};
let decay = card_data.decay.unwrap_or(FSRS5_DEFAULT_DECAY);
Ok(card_data.memory_state().map(|state| {
FSRS::new(None)
.unwrap()
.current_retrievability(state.into(), days_elapsed, decay)
}))
let retrievability = card_data.memory_state().map(|state| {
FSRS::new(None).unwrap().current_retrievability_seconds(
state.into(),
seconds_elapsed,
decay,
)
});
Ok(retrievability)
},
)
}
/// eg. extract_fsrs_relative_retrievability(card.data, card.due,
/// timing.days_elapsed, card.ivl, timing.next_day_at) -> float | null. The
/// higher the number, the higher the card's retrievability relative to the
/// configured desired retention.
/// timing.days_elapsed, card.ivl, timing.next_day_at, timing.now) -> float |
/// null. The higher the number, the higher the card's retrievability relative
/// to the configured desired retention.
fn add_extract_fsrs_relative_retrievability(db: &Connection) -> rusqlite::Result<()> {
db.create_scalar_function(
"extract_fsrs_relative_retrievability",
5,
6,
FunctionFlags::SQLITE_DETERMINISTIC,
move |ctx| {
assert_eq!(ctx.len(), 5, "called with unexpected number of arguments");
assert_eq!(ctx.len(), 6, "called with unexpected number of arguments");
let Ok(due) = ctx.get_raw(1).as_i64() else {
return Ok(None);
@ -381,6 +384,9 @@ fn add_extract_fsrs_relative_retrievability(db: &Connection) -> rusqlite::Result
let Ok(next_day_at) = ctx.get_raw(4).as_i64() else {
return Ok(None);
};
let Ok(now) = ctx.get_raw(5).as_i64() else {
return Ok(None);
};
let days_elapsed = if due > 365_000 {
// (re)learning
(next_day_at as u32).saturating_sub(due as u32) / 86_400
@ -402,17 +408,30 @@ fn add_extract_fsrs_relative_retrievability(db: &Connection) -> rusqlite::Result
desired_retrievability = desired_retrievability.max(0.0001);
let decay = card_data.decay.unwrap_or(FSRS5_DEFAULT_DECAY);
let days_elapsed = if let Some(last_review_time) =
card_data.last_review_time
{
TimestampSecs(next_day_at).elapsed_days_since(last_review_time) as u32
} else {
days_elapsed
};
let seconds_elapsed =
if let Some(last_review_time) = card_data.last_review_time {
now.saturating_sub(last_review_time.0) as u32
} else if due > 365_000 {
// (re)learning card in seconds
let Ok(ivl) = ctx.get_raw(2).as_i64() else {
return Ok(None);
};
let last_review_time = due.saturating_sub(ivl);
now.saturating_sub(last_review_time) as u32
} else {
let Ok(ivl) = ctx.get_raw(2).as_i64() else {
return Ok(None);
};
let Ok(days_elapsed) = ctx.get_raw(3).as_i64() else {
return Ok(None);
};
let review_day = due.saturating_sub(ivl);
days_elapsed.saturating_sub(review_day) as u32 * 86_400
};
let current_retrievability = FSRS::new(None)
.unwrap()
.current_retrievability(state.into(), days_elapsed, decay)
.current_retrievability_seconds(state.into(), seconds_elapsed, decay)
.max(0.0001);
return Ok(Some(