diff options
Diffstat (limited to 'docs/design')
| -rw-r--r-- | docs/design/2026-07-21-code-scan-findings.org | 141 | ||||
| -rw-r--r-- | docs/design/fsrs-spec.org | 209 | ||||
| -rw-r--r-- | docs/design/stats-dashboard.org | 118 |
3 files changed, 378 insertions, 90 deletions
diff --git a/docs/design/2026-07-21-code-scan-findings.org b/docs/design/2026-07-21-code-scan-findings.org new file mode 100644 index 0000000..85b7ea6 --- /dev/null +++ b/docs/design/2026-07-21-code-scan-findings.org @@ -0,0 +1,141 @@ +#+TITLE: Code Scan Findings — org-drill.el +#+AUTHOR: Craig Jennings +#+DATE: 2026-07-21 + +* About + +Full-file bug and refactoring scan of =org-drill.el= (6003 lines), run 2026-07-21 during the overnight sentry session. Method: mechanical metrics in the main thread (function lengths via sexp parsing, parameter counts, literal duplication, dead-code grep), then four scoped read-only review agents over line regions 1-1380, 1380-2440, 2440-3660, 3660-6003, each required to quote verbatim code for every claim. The highest-impact claims were re-verified against the source by hand before this report was written. Line numbers refer to the tree at commit =26091e7= (sentry/2026-07-21-ratio). + +Severity grading uses the severity × frequency matrix in the global todo-format rule. Both ERT suites (unit + integration) pass at scan time; the bugs below live on paths the suites don't exercise (interactive flows, legacy data formats, default-config live use). + +* Bugs + +** [A] BUG-1 — Statistics dashboard scans its own buffer, not the deck (VERIFIED) +=org-drill.el:5730-5735= (=org-drill-statistics--render=). The dashboard body — overview counts, attention tables, forecast, cards CSV rows, all of which call =org-drill-map-entries= — is computed inside =with-current-buffer buffer= (the =*Org Drill Statistics*= buffer). With the default =org-drill-scope= of ='file=, =org-drill-current-scope= returns nil ("current buffer"), so every card scan traverses the empty dashboard buffer. Result with default config: Total cards 0, all attention tables empty, forecast all zeros, empty cards.csv — regardless of deck contents. Bonus crash: cycling scope to ='directory= calls =(file-name-directory (buffer-file-name))= with nil in the file-less dashboard buffer → wrong-type-argument. +Fix direction: capture the originating deck buffer (or resolve the scope to a concrete file list) in =org-drill-statistics= before rendering, and run all card traversals in that context. +Grading: Major severity (new feature non-functional in default config, no data loss) × every dashboard user, every time = P1 = [#A]. + +** [B] BUG-2 — copy-entry fallback pastes a destination subtree instead of the source card (VERIFIED) +=org-drill.el:3888-3901= (=org-drill-copy-entry-to-other-buffer=). =org-copy-subtree= runs before =switch-to-buffer dest=; when =org-find-olp= fails (path only partially exists in DEST), the recursive fallback re-executes the whole function while DEST is current — re-copying whatever subtree point sits on in DEST. DEST gains a stripped "imported" copy of one of its own subtrees; the source card is silently lost. +Trigger: =org-drill-merge-buffers= where SRC has a new item whose outline path only partially exists in DEST. +Fix direction: copy once up front and pass the text (or keep SRC current); use =with-current-buffer= instead of =switch-to-buffer=. +Grading: Critical severity (deck corruption + silent card loss) × rare edge = P2 = [#B]. + +** [B] BUG-3 — Dashboard card links use an unregistered link type (VERIFIED) +=org-drill.el:5280-5281= emits =[[org-drill-card:POS][...]]= links, but no =org-link-set-parameters= call exists anywhere in the package (grep confirmed). RET (bound to =org-open-at-point=) treats the type as a fuzzy link and signals "No match" — dashboard navigation never works. A bare POS also cannot identify the buffer under multi-file scopes. +Fix direction: register the type with a :follow handler and encode buffer/file identity alongside POS. +Grading: Major × every dashboard user attempting navigation = P2 = [#B]. Fix alongside BUG-1. + +** [B] BUG-4 — Leitner rating 1 on a box-0 card writes box -1; card vanishes (VERIFIED) +=org-drill.el:4495-4501= (=org-drill-leitner-rebox=). Box 0 is a valid state (capture accepts 0-5; rebox defaults an absent property to 0), but the demotion floors at box 1 only when current-box = 1, so box 0 yields "-1", which matches neither the unboxed nor the boxed branch of =org-drill-map-leitner-capture= — the card silently leaves the system. Related: the prompt says "0 - Completely forgot. (Back to Zero)" while the ?0 branch writes box "1". +Fix direction: clamp the demotion at the minimum box; reconcile the ?0 branch with its prompt. +Grading: Critical (silent card loss) × rare edge = P2 = [#B]. + +** [C] BUG-5 — Tags key at the answer prompt reveals the answer (VERIFIED) +=org-drill.el:2096=: =(while (memq ch '(nil org-drill--tags-key)))= quotes the symbol, but =ch= is a character, so the memq never matches the tags key; after =org-set-tags-command= runs, control falls through to =(t t)= and reveals the answer instead of re-prompting. Contrast the correct =(list ...)= form at 1800-1804. +Fix: =(memq ch (list nil org-drill--tags-key))=. +Grading: Major (tag-editing at prompt broken, card prematurely revealed) × some users sometimes = P3 = [#C]. + +** [C] BUG-6 — "No longer due" skip leaves current-item set; session ends in error (VERIFIED shape) +=org-drill.el:3163=: the not-due skip branch (unlike the 'skip route at 3120) leaves =(oref session current-item)= pointing at the skipped marker, so =org-drill-entries-pending-p= stays true with empty queues; the next iteration pops nil and hits "Unexpectedly ran out of pending drill items". Trigger: last card of a session fails for the 16th time → tagged leech → under default =org-drill-leech-method= 'skip it is no longer due → session aborts with an error; the stats record and SM5 persist are skipped. +Fix: clear current-item in the skip branch (or treat a nil pop as clean exit). +Grading: Major (session record lost, error instead of report) × rare edge = P3 = [#C]. + +** [C] BUG-7 — store-item-data crashes on nil meanq/ease from legacy cards (VERIFIED shape) +=org-drill.el:1373-1378=: =org-drill-round-float= is applied to =meanq= / =ease= slots that =org-drill-get-item-data= can legitimately leave nil (legacy LEARN_DATA path at 1331; property path at 1352-1353 when DRILL_AVERAGE_QUALITY / DRILL_EASE are absent) → wrong-type-argument. Reachable via =org-drill--copy-scheduling-to-marker= (get → store directly). +Fix: guard nil in the store (skip or default), or never emit nil for rated cards in get. +Grading: Major (crash) × rare (legacy-format cards) = P3 = [#C]. + +** [C] BUG-8 — Empty-string org-drill-match crashes every session (VERIFIED) +=org-drill.el:999-1003=: =(elt org-drill-match 0)= on ="\"\""= signals args-out-of-range; the defcustom's string widget makes "" easy to set via customize. Every command routed through =org-drill-map-entries= then errors. +Fix: require non-zero length before the elt (treat "" as nil). +Grading: Major (session can't start) × rare = P3 = [#C]. + +** [C] BUG-9 — SM2 e-factor floor clamps input, not output (VERIFIED) +=org-drill.el:1441-1443=: canonical SM2 clamps after the update; here the check runs before, so returned EF can dip below 1.3, and at exactly 1.3 the quality argument is ignored next call — a hard card's ratings stop mattering near the floor. +Fix: compute the update, then =(max 1.3 result)=. +Grading: Minor (scheduling drift near floor) × some users sometimes = P3 = [#C]. + +** [C] BUG-10 — Weight clamping diverges between smart-reschedule and the hypothetical path +=org-drill.el:1685-1689= vs =1734-1738=: smart-reschedule clamps the whole weighted sum at 1.0; the hypothetical-next-review-date form clamps only the delta/weight quotient (forcing ≥1 day growth). Since =org-drill-reschedule= always passes numeric days-ahead, the hypothetical formula is what actually schedules interactively — the two entry points schedule the same card differently (e.g. last 10, computed 13, weight 4 → 10.75 vs 11). +Fix: extract one shared weight-adjust helper; pick one clamp semantics. +Grading: Minor × some (weighted cards) = P3 = [#C]. + +** [C] BUG-11 — Early/late-repetition adjustment is inert in the shipped call graph +=org-drill.el:1674-1680= (+ 1726-1730): sm5's delta-days argument is never passed by either caller, and simple8's delta-days-adjusted interval is discarded whenever days-ahead is numeric — which it is for both in-tree callers. Setting =org-drill-adjust-intervals-for-early-and-late-repetitions-p= changes nothing interactively. +Fix: thread delta-days through the hypothetical path (or compute the adjustment where days-ahead is derived). +Grading: Major (documented option does nothing) × some users sometimes = P3 = [#C]. + +** [C] BUG-12 — org-drill-leitner-completed never resets (VERIFIED) +Defvar at 4316, incremented at 4525, read at 4335/4387 — no reset anywhere. It accumulates for the life of the Emacs process, so =org-drill-sm-or-leitner='s dispatch (pending > completed) increasingly favors Leitner over due SM sessions. +Fix: reset to 0 at the start of each Leitner session. +Grading: Major (wrong session dispatch) × some (Leitner users, long-lived daemon) = P3 = [#C]. + +** [C] BUG-13 — Cloze overlays leak on C-g in hide-n / hide-nth +=org-drill.el:2759= (and 2782-2784): cleanup sits in a =prog1= tail, skipped on nonlocal exit; the surrounding macros' unwind handlers delete only the hidden-text overlay category, not cloze overlays. C-g at the prompt of any multicloze card leaves [...] masks in the org buffer permanently. +Fix: unwind-protect around prompt + cleanup — falls out of the shared presenter helper (REF-2). +Grading: Minor × some = P3 = [#C]. + +** [C] BUG-14 — replace-entry-text-multi assert too weak; overlays can run past point-max +=org-drill.el:2494=: layout consumes 2 chars per replacement (needs ≥ 2N-1) but the assert requires only N; a short body at buffer end → args-out-of-range mid-presentation; mid-buffer, the overlay spills past the next heading. Odd-offset characters between one-char overlays also show through. +Fix: strengthen the assert to 2N-1 and clamp overlay ends to p-max. +Grading: Major (crash) × rare = P3 = [#C]. + +** [C] BUG-15 — show1-lastmore's uncommon branch never reveals the first piece +=org-drill.el:2849=: passes force-hide-first, guaranteeing the first piece stays hidden — but the comment says "show any item except the last" and the docstring says "any random piece". Behavior matches neither. +Fix: plain =(org-drill-present-multicloze-hide-n session -1)= for "any random piece" (or implement except-last); fix the comment. +Grading: Minor × some = P3 = [#C]. + +** [C] BUG-16 — "All time" CSV export truncates daily.csv to 90 days +=org-drill.el:5983-5984= (clamp at 5927): the "all time" preset maps days to nil, which --daily-rows replaces with =org-drill-statistics-trend-days= (90); daily.csv silently drops older days while sessions.csv covers the whole log. +Fix: derive days from the oldest record when the range is unbounded (or document the floor). +Grading: Minor × some = P3 = [#C]. + +** [C] BUG-17 — Trends renders as level-1 heading, swallowing later sections +=org-drill.el:5527-5529=: "* Trends" among "** ..." siblings — TAB-collapsing Trends hides Distribution, Attention, and Forecast. +Fix: "** Trends". +Grading: Cosmetic × every dashboard user = P3 = [#C]. + +** [D] BUG-18 — Merge leaves point moved in DEST +=org-drill.el:3977-3981=: the strip maphash has no save-excursion; after merge, point in DEST sits on the last unmatched entry. Fix: wrap in save-excursion. Cosmetic × some = P4 = [#D]. + +** [D] BUG-19 — free-markers call in prepare-fresh-session is a no-op +=org-drill.el:3511= runs on the brand-new empty session (constructed at 3634); the previous session's queue markers are never explicitly freed (GC-bounded impact). Fix: free the old session's markers before reassigning, or drop the call. Minor internal × every = [#D]. + +** [D] BUG-20 — "Drill session finished!" on quit, doubled on completion +=org-drill.el:3653=: unconditional after =org-drill-entries=, including quit/edit exits; on normal completion the same message + sit-for repeats in --show-end-message. Fix: delete; let show-end-message own it. Cosmetic × most = P4 = [#D]. + +** [D] BUG-21 — Hidden-cloze display string baked at load time +=org-drill.el:279-284=: the "[...]" display string is computed once at load from the delimiter defcustoms, while the cloze regexp is recomputed per buffer — customized delimiters mismatch the mask. Fix: compute at overlay creation. Minor × rare = P4 = [#D]. + +** [D] BUG-22 — card-type/card-tags alist :type specs don't match value shape +=org-drill.el:414-416= (and 446-447): values are lists, spec says =function= — customize edits produce malformed entries. Fix: correct the :type. Minor × rare = P4 = [#D]. + +* Refactoring opportunities + +** [C] REF-1 — Scheduler duplication: meanq update triplicated, normalize/assert prologue duplicated +=org-drill.el:1503-1506= (verbatim at 1402-1405, 1608-1610); n/ef defaulting + quality asserts duplicated across sm2/sm5. Extract =org-drill--update-meanq= + a shared state-normalize prologue. Pairs naturally with fixing BUG-9/10/11. Also: SM5 failure path writes an OF-matrix entry keyed by a next-ef the card never adopts (1520-1528) — decide and document, or move the write into the success branch. SM2's second-repetition noise table (1420-1427) carries unreachable arms given the failure boundary. + +** [C] REF-2 — Presenter boilerplate: one shared present-and-prompt helper +The latex/inline-images/hide-drawers/prompt/re-hide closing sequence is near-verbatim in six presenters (2573-2578, 2615-2620, 2643-2648, 2660-2665, 2755-2761, 2778-2784). Extract one helper with an unwind-protect — which also fixes BUG-13. The weight-validation cond is additionally triplicated across firstmore/lastmore/firstless (2817-2831, 2841-2855, 2866-2880) — extract a 'fallback/'uncommon/'common dispatcher. + +** [C] REF-3 — Statistics attention pipeline duplicated +=org-drill-statistics--render-attention= (5316-5399) re-implements the filter+sort+cap+row pipeline of the three selectors (5131-5195), which production code never calls (tests only); comparators exist in two copies each. Drive both from one (predicate, comparator) table. Also delete: dead =count= in org-drill-leitner (4354-4364), vestigial let-bindings in --render-all (5679-5681), inert same-file declare-function block (5596-5600). + +** [C] REF-4 — org-drill-get-item-data duplication +The seven-line property-path constructor appears verbatim twice (1335-1341, 1347-1353) and the virgin-state constructor twice (1343-1345, 1355-1357); the inner if re-implements the outer cond's arms. Extract =org-drill--card-state-from-properties= + a virgin-state helper. Pairs with fixing BUG-7. + +** [D] REF-5 — Small cleanups batch +- =org-drill-pop-random= macro evaluates PLACE five times (967-972) — bind once via macroexp-let2. +- =(or default nil)= → =default= in the DRILL_AVERAGE_QUALITY getter (1201-1204). +- Dead =again-entries= arm in entries-pending-p (3003) — outer or already covers it. +- DRILL_* property-name literals repeat 4-9× each — consider named constants. +- =org-drill-with-replaced-entry-text-multi= has zero in-tree callers but is public API for custom card-type authors — keep, do not remove. + +* Metrics summary + +Longest defuns (true sexp lengths): render-attention 75, org-drill 71, simple8 69, --read-rating-key 66 — all under the 100-line critical band; ten functions in the 51-100 "split recommended" band, most either schedulers (REF-1) or renderers (REF-3). No functions with 5+ positional parameters remain (card-state refactor). Byte-compile is warning-free as of =26091e7=. + +* Verification status + +VERIFIED = re-checked by hand against the source during consolidation. VERIFIED shape = quoted code confirmed present and mechanism plausible; the full trigger chain still wants a reproduction test. Unmarked = agent-reported with verbatim quotes that matched spot-checks; write the failing ERT reproduction first per the project's TDD convention before fixing any of them. diff --git a/docs/design/fsrs-spec.org b/docs/design/fsrs-spec.org index 76ec4db..0b0d9eb 100644 --- a/docs/design/fsrs-spec.org +++ b/docs/design/fsrs-spec.org @@ -78,20 +78,40 @@ Locked product decisions for v1. These resolve the design questions, but implementation is still blocked on the research prerequisites in =Status= above (source pin, equation cross-check, reference vectors). -| # | Decision | Value | -|---+----------+-------| -| 1 | FSRS version pin | FSRS-4.5 (17 parameters), with the v4.5 default tuple from the fsrs4anki algorithm wiki | -| 2 | Default parameters | See =Version pin= below — the exact v4.5 tuple | -| 3 | Forgetting curve | v4.5: =DECAY = -0.5=, =FACTOR = 19/81= | -| 4 | Quality mapping | Again iff quality ≤ =org-drill-failure-quality=; otherwise Hard (≤3), Good (4), Easy (5). At the default threshold of 2 this is 0/1/2 → Again, 3 → Hard, 4 → Good, 5 → Easy | -| 5 | State shape | =cl-defstruct org-drill-fsrs-state= | -| 6 | Result shape | =cl-defstruct org-drill-fsrs-result= | -| 7 | DRILL_CARD_WEIGHT applies to FSRS | Yes — same delta interpolation as SM/Simple8: =next = max(1.0, last + (computed − last) / weight)= on success intervals; weight 2 = more frequent review. The =Again= same-day path (interval 0) bypasses weight | -| 8 | Failure semantics | =Again= schedules same-day (=next-interval = 0=); D/S/R state still updates | -| 9 | First-FSRS-review cold-start message | No message (silent transition) | -| 10 | Malformed =DRILL_FSRS_*= behavior | User-facing error; scheduling untouched (data-loss safety) | -| 11 | =org-drill-fsrs-desired-retention= as file-local | Yes — safe-local, bounded 0 < x < 1 | -| 12 | DRILL_FSRS_* property ownership | Add to =org-drill-scheduling-properties=; undo/strip/copy/migration cover them via the existing list | +| # | Decision | Value | +|----+--------------------------------------------------+--------------------------------------------------------------| +| 1 | FSRS version pin | FSRS-4.5 (17 parameters), with the v4.5 default tuple from | +| | | the fsrs4anki algorithm wiki | +|----+--------------------------------------------------+--------------------------------------------------------------| +| 2 | Default parameters | See =Version pin= below — the exact v4.5 tuple | +|----+--------------------------------------------------+--------------------------------------------------------------| +| 3 | Forgetting curve | v4.5: =DECAY = -0.5=, =FACTOR = 19/81= | +|----+--------------------------------------------------+--------------------------------------------------------------| +| 4 | Quality mapping | Again iff quality ≤ =org-drill-failure-quality=; otherwise | +| | | Hard (≤3), Good (4), Easy (5). At the default threshold of 2 | +| | | this is 0/1/2 → Again, 3 → Hard, 4 → Good, 5 → Easy | +|----+--------------------------------------------------+--------------------------------------------------------------| +| 5 | State shape | =cl-defstruct org-drill-fsrs-state= | +|----+--------------------------------------------------+--------------------------------------------------------------| +| 6 | Result shape | =cl-defstruct org-drill-fsrs-result= | +|----+--------------------------------------------------+--------------------------------------------------------------| +| 7 | DRILL_CARD_WEIGHT applies to FSRS | Yes — same delta interpolation as SM/Simple8: =next = | +| | | max(1.0, last + (computed − last) / weight)= on success | +| | | intervals; weight 2 = more frequent review. The =Again= | +| | | same-day path (interval 0) bypasses weight | +|----+--------------------------------------------------+--------------------------------------------------------------| +| 8 | Failure semantics | =Again= schedules same-day (=next-interval = 0=); D/S/R | +| | | state still updates | +|----+--------------------------------------------------+--------------------------------------------------------------| +| 9 | First-FSRS-review cold-start message | No message (silent transition) | +|----+--------------------------------------------------+--------------------------------------------------------------| +| 10 | Malformed =DRILL_FSRS_*= behavior | User-facing error; scheduling untouched (data-loss safety) | +|----+--------------------------------------------------+--------------------------------------------------------------| +| 11 | =org-drill-fsrs-desired-retention= as file-local | Yes — safe-local, bounded 0 < x < 1 | +|----+--------------------------------------------------+--------------------------------------------------------------| +| 12 | DRILL_FSRS_* property ownership | Add to =org-drill-scheduling-properties=; | +| | | undo/strip/copy/migration cover them via the existing list | +|----+--------------------------------------------------+--------------------------------------------------------------| * Open questions @@ -145,10 +165,14 @@ The four-rating scale FSRS uses internally: | Rating | Name | Meaning | |--------+-------+------------------------| -| 1 | Again | failed recall | -| 2 | Hard | recalled with struggle | -| 3 | Good | recalled normally | -| 4 | Easy | trivial recall | +| 1 | Again | failed recall | +|--------+-------+------------------------| +| 2 | Hard | recalled with struggle | +|--------+-------+------------------------| +| 3 | Good | recalled normally | +|--------+-------+------------------------| +| 4 | Easy | trivial recall | +|--------+-------+------------------------| org-drill uses a 0–5 quality scale. The mapping (locked, see Agreed decisions #4) honors =org-drill-failure-quality= for the Again boundary, @@ -167,9 +191,13 @@ At the default threshold of 2 this is the familiar table: | org-drill quality | FSRS rating | |-------------------+-------------| | 0, 1, 2 | Again (1) | +|-------------------+-------------| | 3 | Hard (2) | +|-------------------+-------------| | 4 | Good (3) | +|-------------------+-------------| | 5 | Easy (4) | +|-------------------+-------------| A custom =org-drill-failure-quality= shifts the Again boundary the same way it does for the SM-family schedulers: a threshold of 3 sends quality 3 to @@ -183,13 +211,18 @@ could let users remap the success grades; v1 keeps them fixed. Per-card persisted state lives in four properties plus the existing =DRILL_LAST_REVIEWED=: -| Property | Type | Meaning | -|-------------------------+--------+----------------------------------------| -| =DRILL_FSRS_STABILITY= | float | =S=, the memory-stability estimate | -| =DRILL_FSRS_DIFFICULTY= | float | =D=, the difficulty estimate (1..10) | -| =DRILL_FSRS_REVIEWS= | int | review count under FSRS (≥ 0) | -| =DRILL_FSRS_LAPSES= | int | failures (rating = Again) under FSRS | -| =DRILL_LAST_REVIEWED= | time | reused from the existing SM property | +| Property | Type | Meaning | +|-------------------------+-------+--------------------------------------| +| =DRILL_FSRS_STABILITY= | float | =S=, the memory-stability estimate | +|-------------------------+-------+--------------------------------------| +| =DRILL_FSRS_DIFFICULTY= | float | =D=, the difficulty estimate (1..10) | +|-------------------------+-------+--------------------------------------| +| =DRILL_FSRS_REVIEWS= | int | review count under FSRS (≥ 0) | +|-------------------------+-------+--------------------------------------| +| =DRILL_FSRS_LAPSES= | int | failures (rating = Again) under FSRS | +|-------------------------+-------+--------------------------------------| +| =DRILL_LAST_REVIEWED= | time | reused from the existing SM property | +|-------------------------+-------+--------------------------------------| =DRILL_LAST_REVIEWED= is written by the shared reschedule flow (=org-drill.el=:1930, unconditional after the algorithm dispatch), not by @@ -291,14 +324,22 @@ participate in every workflow that already handles SM scheduling properties. V1 adds them to =org-drill-scheduling-properties= so the existing helpers cover them without per-call plumbing: -| Workflow | Helper / call site | Behavior | -|----------+---------------------+----------| -| Rating snapshot for undo | =org-drill--snapshot-entry-data= | Captures =DRILL_FSRS_*= alongside SM properties | -| Undo last rating | =org-drill-undo-last-rating= → =org-drill--restore-entry-data= | Restores =DRILL_FSRS_*= verbatim | -| Strip single entry | =org-drill-strip-entry-data= | Removes =DRILL_FSRS_*= | -| Strip all entries | =org-drill-strip-all-data= | Removes =DRILL_FSRS_*= across the scope | -| Copy / share to marker | =org-drill--copy-scheduling-to-marker= | Migrates =DRILL_FSRS_*= | -| Org property completion | =org-drill-scheduling-properties= consumers | =DRILL_FSRS_*= appear in completion | +| Workflow | Helper / call site | Behavior | +|--------------------------+---------------------------------------------+---------------------------------------------| +| Rating snapshot for undo | =org-drill--snapshot-entry-data= | Captures =DRILL_FSRS_*= alongside SM | +| | | properties | +|--------------------------+---------------------------------------------+---------------------------------------------| +| Undo last rating | =org-drill-undo-last-rating= → | Restores =DRILL_FSRS_*= verbatim | +| | =org-drill--restore-entry-data= | | +|--------------------------+---------------------------------------------+---------------------------------------------| +| Strip single entry | =org-drill-strip-entry-data= | Removes =DRILL_FSRS_*= | +|--------------------------+---------------------------------------------+---------------------------------------------| +| Strip all entries | =org-drill-strip-all-data= | Removes =DRILL_FSRS_*= across the scope | +|--------------------------+---------------------------------------------+---------------------------------------------| +| Copy / share to marker | =org-drill--copy-scheduling-to-marker= | Migrates =DRILL_FSRS_*= | +|--------------------------+---------------------------------------------+---------------------------------------------| +| Org property completion | =org-drill-scheduling-properties= consumers | =DRILL_FSRS_*= appear in completion | +|--------------------------+---------------------------------------------+---------------------------------------------| *Implementation.* Add the four FSRS property names to the =org-drill-scheduling-properties= defvar list. No new branching; @@ -414,15 +455,22 @@ Two new helpers parallel the existing item-data round-trip: The implementation separates pure functions from property IO so each layer is independently testable: -| Layer | Functions | Tested in | -|-------+-----------+-----------| -| Pure quality mapping | =org-drill--fsrs-rating-from-quality= | mapping tests | -| Pure retrievability | =org-drill--fsrs-retrievability= | reference-vector tests | -| Pure initial state | =org-drill--fsrs-initial-state= | reference-vector tests | -| Pure update step | =org-drill--fsrs-update= | reference-vector tests | -| Pure interval calc | =org-drill--fsrs-interval= | reference-vector tests | -| Property IO | =org-drill-get-fsrs-state=, =org-drill-store-fsrs-result= | round-trip tests | -| Orchestration | =org-drill-determine-next-interval-fsrs= | integration tests | +| Layer | Functions | Tested in | +|----------------------+-----------------------------------------------------------+------------------------| +| Pure quality mapping | =org-drill--fsrs-rating-from-quality= | mapping tests | +|----------------------+-----------------------------------------------------------+------------------------| +| Pure retrievability | =org-drill--fsrs-retrievability= | reference-vector tests | +|----------------------+-----------------------------------------------------------+------------------------| +| Pure initial state | =org-drill--fsrs-initial-state= | reference-vector tests | +|----------------------+-----------------------------------------------------------+------------------------| +| Pure update step | =org-drill--fsrs-update= | reference-vector tests | +|----------------------+-----------------------------------------------------------+------------------------| +| Pure interval calc | =org-drill--fsrs-interval= | reference-vector tests | +|----------------------+-----------------------------------------------------------+------------------------| +| Property IO | =org-drill-get-fsrs-state=, =org-drill-store-fsrs-result= | round-trip tests | +|----------------------+-----------------------------------------------------------+------------------------| +| Orchestration | =org-drill-determine-next-interval-fsrs= | integration tests | +|----------------------+-----------------------------------------------------------+------------------------| The =cl-case= branch in =org-drill-smart-reschedule= is the orchestration layer. Everything below the branch is pure or @@ -648,13 +696,76 @@ updated to "not implementation-ready as of Review 2" before this response. * Review and iteration history -| Iteration | Date | Contributor | Role | What changed or was recommended | Why | -|------------+------------+---------------+-----------+----------------------------------+-----| -| Draft v0 | 2026-05-27 | Craig Jennings | Author | Initial draft with 6 DECIDE markers and recommendations baked in for each. Same shape as the stats-dashboard spec. | Establish a written design before any FSRS code lands. | -| Review 1 | 2026-05-28 | External reviewer | Reviewer | Four blocking findings (version/tuple/equation mismatch, scheduler/state result shape, property ownership across strip/undo/copy/migration, learning/relearning scope) plus two medium, plus UX/Architecture/Robustness observations and full test-strategy expansion. | An external read of the spec against the existing codebase caught the v4-vs-v4.5 drift and several missing secondary-workflow paths. | -| Response 1 | 2026-05-28 | Claude Code | Responder | All four blocking findings resolved; medium-priority items accepted; new sections added (Agreed decisions, Scheduling property ownership, Validation and malformed-state behavior); test strategy expanded to four files; v4 → v4.5 tuple and equations corrected; struct shapes locked; DRILL_CARD_WEIGHT confirmed to apply (Craig's call). | Convert the spec from "not implementation-ready" to "implementation-ready" via the spec-response workflow. Removes invented-product-behavior risk for the implementer. | -| Review 2 | 2026-05-28 | Codex | Reviewer | Marked the spec as needs-research before implementation; identified unresolved =py-fsrs= source pin/formula/vector prerequisites, =DRILL_CARD_WEIGHT= semantics reversed against current org-drill, and a quality-mapping conflict with =org-drill-failure-quality=. | Applying the updated spec-review workflow to the moved =docs/design/= spec caught contradictions introduced while preserving Review 1 decisions. | -| Response 2 | 2026-05-31 | Claude Code | Responder | Corrected =DRILL_CARD_WEIGHT= to the real SM/Simple8 delta-interpolation (Craig's call to match existing semantics); resolved the quality mapping to honor =org-drill-failure-quality= for the Again boundary with a fixed success sub-mapping (Craig's call); reframed status/open-questions to "Needs research" with the three blocking prerequisites stated honestly; fixed =DRILL_LAST_REVIEWED= ownership (M1) and the algorithm-symbol coverage (M2); flagged the update equations as paraphrased-and-unverified. | Fix the three blockers and two mediums from Review 2. The B2 reversal traced to a wrong verbal description of SM weight handling in Response 1; this response grounds every claim in the cited =org-drill.el= line. | +| Iteration | Date | Contributor | Role | What changed or was | Why | +| | | | | recommended | | +|------------+------------+-------------------+-----------+------------------------------+-----------------------------| +| Draft v0 | 2026-05-27 | Craig Jennings | Author | Initial draft with 6 DECIDE | Establish a written design | +| | | | | markers and recommendations | before any FSRS code lands. | +| | | | | baked in for each. Same | | +| | | | | shape as the stats-dashboard | | +| | | | | spec. | | +|------------+------------+-------------------+-----------+------------------------------+-----------------------------| +| Review 1 | 2026-05-28 | External reviewer | Reviewer | Four blocking findings | An external read of the | +| | | | | (version/tuple/equation | spec against the existing | +| | | | | mismatch, scheduler/state | codebase caught the | +| | | | | result shape, property | v4-vs-v4.5 drift and | +| | | | | ownership across | several missing | +| | | | | strip/undo/copy/migration, | secondary-workflow paths. | +| | | | | learning/relearning scope) | | +| | | | | plus two medium, plus | | +| | | | | UX/Architecture/Robustness | | +| | | | | observations and full | | +| | | | | test-strategy expansion. | | +|------------+------------+-------------------+-----------+------------------------------+-----------------------------| +| Response 1 | 2026-05-28 | Claude Code | Responder | All four blocking findings | Convert the spec from "not | +| | | | | resolved; medium-priority | implementation-ready" to | +| | | | | items accepted; new sections | "implementation-ready" via | +| | | | | added (Agreed decisions, | the spec-response workflow. | +| | | | | Scheduling property | Removes | +| | | | | ownership, Validation and | invented-product-behavior | +| | | | | malformed-state behavior); | risk for the implementer. | +| | | | | test strategy expanded to | | +| | | | | four files; v4 → v4.5 tuple | | +| | | | | and equations corrected; | | +| | | | | struct shapes locked; | | +| | | | | DRILL_CARD_WEIGHT confirmed | | +| | | | | to apply (Craig's call). | | +|------------+------------+-------------------+-----------+------------------------------+-----------------------------| +| Review 2 | 2026-05-28 | Codex | Reviewer | Marked the spec as | Applying the updated | +| | | | | needs-research before | spec-review workflow to the | +| | | | | implementation; identified | moved =docs/design/= spec | +| | | | | unresolved =py-fsrs= source | caught contradictions | +| | | | | pin/formula/vector | introduced while preserving | +| | | | | prerequisites, | Review 1 decisions. | +| | | | | =DRILL_CARD_WEIGHT= | | +| | | | | semantics reversed against | | +| | | | | current org-drill, and a | | +| | | | | quality-mapping conflict | | +| | | | | with | | +| | | | | =org-drill-failure-quality=. | | +|------------+------------+-------------------+-----------+------------------------------+-----------------------------| +| Response 2 | 2026-05-31 | Claude Code | Responder | Corrected | Fix the three blockers and | +| | | | | =DRILL_CARD_WEIGHT= to the | two mediums from Review 2. | +| | | | | real SM/Simple8 | The B2 reversal traced to a | +| | | | | delta-interpolation (Craig's | wrong verbal description of | +| | | | | call to match existing | SM weight handling in | +| | | | | semantics); resolved the | Response 1; this response | +| | | | | quality mapping to honor | grounds every claim in the | +| | | | | =org-drill-failure-quality= | cited =org-drill.el= line. | +| | | | | for the Again boundary with | | +| | | | | a fixed success sub-mapping | | +| | | | | (Craig's call); reframed | | +| | | | | status/open-questions to | | +| | | | | "Needs research" with the | | +| | | | | three blocking prerequisites | | +| | | | | stated honestly; fixed | | +| | | | | =DRILL_LAST_REVIEWED= | | +| | | | | ownership (M1) and the | | +| | | | | algorithm-symbol coverage | | +| | | | | (M2); flagged the update | | +| | | | | equations as | | +| | | | | paraphrased-and-unverified. | | +|------------+------------+-------------------+-----------+------------------------------+-----------------------------| * References diff --git a/docs/design/stats-dashboard.org b/docs/design/stats-dashboard.org index 6bb18d9..ba01ae3 100644 --- a/docs/design/stats-dashboard.org +++ b/docs/design/stats-dashboard.org @@ -24,17 +24,26 @@ what each card carries in its own =DRILL_*= properties. The currently-retained per-card data is real and usable: -| Property | Source | -|----------+--------| -| =DRILL_LAST_INTERVAL= | scheduler output, updated every review | -| =DRILL_REPEATS_SINCE_FAIL= | review counter resetting on a lapse | -| =DRILL_TOTAL_REPEATS= | lifetime review count for the card | -| =DRILL_FAILURE_COUNT= | lifetime lapses | -| =DRILL_AVERAGE_QUALITY= | running mean of the 0–5 quality scale | -| =DRILL_EASE= | SM-family easiness factor | -| =DRILL_LAST_QUALITY= | quality at the most recent review | -| =DRILL_LAST_REVIEWED= | inactive timestamp of the most recent review | -| =DATE_ADDED= | inactive timestamp of card creation | +| Property | Source | +|----------------------------+----------------------------------------------| +| =DRILL_LAST_INTERVAL= | scheduler output, updated every review | +|----------------------------+----------------------------------------------| +| =DRILL_REPEATS_SINCE_FAIL= | review counter resetting on a lapse | +|----------------------------+----------------------------------------------| +| =DRILL_TOTAL_REPEATS= | lifetime review count for the card | +|----------------------------+----------------------------------------------| +| =DRILL_FAILURE_COUNT= | lifetime lapses | +|----------------------------+----------------------------------------------| +| =DRILL_AVERAGE_QUALITY= | running mean of the 0–5 quality scale | +|----------------------------+----------------------------------------------| +| =DRILL_EASE= | SM-family easiness factor | +|----------------------------+----------------------------------------------| +| =DRILL_LAST_QUALITY= | quality at the most recent review | +|----------------------------+----------------------------------------------| +| =DRILL_LAST_REVIEWED= | inactive timestamp of the most recent review | +|----------------------------+----------------------------------------------| +| =DATE_ADDED= | inactive timestamp of card creation | +|----------------------------+----------------------------------------------| What's missing is the *temporal axis*: there's no record that the user reviewed N cards on a given day, or that the pass percentage trended @@ -116,18 +125,28 @@ Each entry is an `org-drill-session-record' struct.") =org-drill-session-record= is a =cl-defstruct=: -| Slot | Type | Meaning | -|------+------+---------| -| =start-time= | float | =float-time= at session start | -| =end-time= | float | =float-time= at session end | -| =scope= | symbol or list | =org-drill-scope= value at session start | -| =algorithm= | symbol | =org-drill-spaced-repetition-algorithm= at start | -| =qualities= | vector of int | every quality 0–5 entered, in order | -| =pass-percent= | int | qualities > =org-drill-failure-quality=, / total | -| =new-count= | int | size of =(oref session new-entries)= at end | -| =mature-count= | int | =young-mature= + =old-mature= entries at end | -| =failed-count= | int | =failed-entries= at end | -| =cram-mode= | bool | =cram-mode= at session start | +| Slot | Type | Meaning | +|----------------+----------------+--------------------------------------------------| +| =start-time= | float | =float-time= at session start | +|----------------+----------------+--------------------------------------------------| +| =end-time= | float | =float-time= at session end | +|----------------+----------------+--------------------------------------------------| +| =scope= | symbol or list | =org-drill-scope= value at session start | +|----------------+----------------+--------------------------------------------------| +| =algorithm= | symbol | =org-drill-spaced-repetition-algorithm= at start | +|----------------+----------------+--------------------------------------------------| +| =qualities= | vector of int | every quality 0–5 entered, in order | +|----------------+----------------+--------------------------------------------------| +| =pass-percent= | int | qualities > =org-drill-failure-quality=, / total | +|----------------+----------------+--------------------------------------------------| +| =new-count= | int | size of =(oref session new-entries)= at end | +|----------------+----------------+--------------------------------------------------| +| =mature-count= | int | =young-mature= + =old-mature= entries at end | +|----------------+----------------+--------------------------------------------------| +| =failed-count= | int | =failed-entries= at end | +|----------------+----------------+--------------------------------------------------| +| =cram-mode= | bool | =cram-mode= at session start | +|----------------+----------------+--------------------------------------------------| A single record is small (a few hundred bytes). At one session per day, the log holds a year of history in well under 100 KB. At three @@ -275,12 +294,19 @@ helper. Users who want TSV can run a one-line sed pipe. The expensive paths and their bounds: -| Path | Cost | Mitigation | -|------+------+------------| -| Session log save | one =persist-save= per session | already wrapped in =condition-case=; cost is a few KB write | -| Scope walk on dashboard open | =org-map-entries= over scope | same cost as a session open; cached for the dashboard's lifetime, refreshes on =g= | -| CSV export | one walk + one file write | one-off; user-triggered | -| Sparkline rendering | bucket the in-memory log by day | log is bounded; bucket-by-day is linear in log length | +| Path | Cost | Mitigation | +|------------------------------+---------------------------------+-----------------------------------------------------| +| Session log save | one =persist-save= per session | already wrapped in =condition-case=; cost is a few | +| | | KB write | +|------------------------------+---------------------------------+-----------------------------------------------------| +| Scope walk on dashboard open | =org-map-entries= over scope | same cost as a session open; cached for the | +| | | dashboard's lifetime, refreshes on =g= | +|------------------------------+---------------------------------+-----------------------------------------------------| +| CSV export | one walk + one file write | one-off; user-triggered | +|------------------------------+---------------------------------+-----------------------------------------------------| +| Sparkline rendering | bucket the in-memory log by day | log is bounded; bucket-by-day is linear in log | +| | | length | +|------------------------------+---------------------------------+-----------------------------------------------------| The dashboard does *not* run at session open or close — only when the user invokes =M-x org-drill-statistics=. Session-end pays one =persist-save= @@ -393,18 +419,28 @@ the export + docs + polish. All 10 open questions resolved as recommended. Implementation can proceed against this spec. -| # | Question | Resolution | -|---+----------+------------| -| 1 | Persistence shape | =persist-defvar=, mirroring the SM5 matrix | -| 2 | Corrupted-load recovery | warn, fresh-start, rename to =.corrupt-YYYY-MM-DD= | -| 3 | Sparkline character set | quadrant blocks (▁▂▃▄▅▆▇█) | -| 4 | Filter scope | single buffer-wide filter | -| 5 | CSV delimiter | =,= with proper quoting | -| 6 | Dashboard-open mode | sync | -| 7 | Aborted-session recording | record nothing; =unwind-protect= deferred | -| 8 | Dashboard keymap | =q g e s r a RET= | -| 9 | Leech-quality threshold default | 2.5 | -| 10 | Defcustom group placement | sibling group =org-drill-statistics= | +| # | Question | Resolution | +|----+---------------------------------+----------------------------------------------------| +| 1 | Persistence shape | =persist-defvar=, mirroring the SM5 matrix | +|----+---------------------------------+----------------------------------------------------| +| 2 | Corrupted-load recovery | warn, fresh-start, rename to =.corrupt-YYYY-MM-DD= | +|----+---------------------------------+----------------------------------------------------| +| 3 | Sparkline character set | quadrant blocks (▁▂▃▄▅▆▇█) | +|----+---------------------------------+----------------------------------------------------| +| 4 | Filter scope | single buffer-wide filter | +|----+---------------------------------+----------------------------------------------------| +| 5 | CSV delimiter | =,= with proper quoting | +|----+---------------------------------+----------------------------------------------------| +| 6 | Dashboard-open mode | sync | +|----+---------------------------------+----------------------------------------------------| +| 7 | Aborted-session recording | record nothing; =unwind-protect= deferred | +|----+---------------------------------+----------------------------------------------------| +| 8 | Dashboard keymap | =q g e s r a RET= | +|----+---------------------------------+----------------------------------------------------| +| 9 | Leech-quality threshold default | 2.5 | +|----+---------------------------------+----------------------------------------------------| +| 10 | Defcustom group placement | sibling group =org-drill-statistics= | +|----+---------------------------------+----------------------------------------------------| * References |