diff options
| -rw-r--r-- | .gitignore | 3 | ||||
| -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 | ||||
| -rw-r--r-- | org-drill.el | 161 | ||||
| -rw-r--r-- | tests/test-org-drill-prompt-and-misc.el | 53 | ||||
| -rw-r--r-- | tests/test-org-drill-statistics-render-attention.el | 12 | ||||
| -rw-r--r-- | tests/test-org-drill-statistics-source-buffer.el | 174 |
8 files changed, 736 insertions, 135 deletions
@@ -15,3 +15,6 @@ todo.org /.coverage/ /*-autoloads.el *~ + +# task archive (follows todo file privacy) +/archive/task-archive.org 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 diff --git a/org-drill.el b/org-drill.el index 4a8262c..8c895b6 100644 --- a/org-drill.el +++ b/org-drill.el @@ -202,12 +202,12 @@ Possible values: (defface org-drill-visible-cloze-face '((t (:foreground "darkseagreen"))) - "The face used to hide the contents of cloze phrases." + "The face used for visible cloze text." :group 'org-drill-display) (defface org-drill-visible-cloze-hint-face '((t (:foreground "dark slate blue"))) - "The face used to hide the contents of cloze phrases." + "The face used for hints inside visible cloze text." :group 'org-drill-display) (defface org-drill-hidden-cloze-face @@ -919,7 +919,7 @@ even if answered correctly." :type 'integer) (defvar org-drill--lapse-very-overdue-entries-p nil - "If non-nil, entries more than `org-drill-lapse-threshold-days' overdue are lapsed. + "If non-nil, lapse entries `org-drill-lapse-threshold-days' overdue. This means that when the item is eventually re-tested it will be treated as \\='failed\\=' (quality 2) for rescheduling purposes, regardless of whether the test was successful.") @@ -1305,7 +1305,7 @@ Returns the parsed list or nil if invalid or unsafe." last-interval repetitions ease failures meanq total-repeats) (defun org-drill-get-item-data () - "Return an `org-drill-card-state' with the stored recall data for the item at point. + "Return an `org-drill-card-state' with the item at point's recall data. Slots: - LAST-INTERVAL is the interval in days that was used to schedule the item's @@ -1357,7 +1357,7 @@ Slots: :total-repeats 0 :meanq nil :ease nil))))) (defun org-drill-store-item-data (state) - "Store the recall data in STATE, an `org-drill-card-state', into the item at point. + "Store STATE, an `org-drill-card-state', into the item at point. STATE's LAST-INTERVAL slot holds the interval to schedule going forward (the caller passes the scheduler's next-interval there)." (org-entry-delete (point) "LEARN_DATA") @@ -1986,6 +1986,26 @@ Consider reformulating the item to make it easier to remember.\n" prompt) prompt)) +(defcustom org-drill-show-outline-path-during-drill nil + "When non-nil, show the card's outline path in the drill prompt. +The ancestor headings are joined with \" > \" and bracketed, e.g. +\"[Spanish > Greetings]\", and prepended to the prompt so you can see +where the current card sits in the deck. Off by default." + :group 'org-drill-display + :type 'boolean) + +(put 'org-drill-show-outline-path-during-drill 'safe-local-variable 'booleanp) + +(defun org-drill--outline-path-string () + "Return the outline path of the entry at point, bracketed for the prompt. +Joins the ancestor headings with \" > \" and wraps them in brackets with a +trailing space, e.g. \"[Spanish > Greetings] \". Returns the empty string +when the entry has no ancestors or point is not on a heading." + (let ((path (ignore-errors (org-get-outline-path)))) + (if path + (concat "[" (mapconcat #'identity path " > ") "] ") + ""))) + (defun org-drill--make-minibuffer-prompt (session prompt) "Make a mini-buffer for the SESSION, with PROMPT." (let ((status (cl-first (org-drill-entry-status session))) @@ -2027,7 +2047,9 @@ Consider reformulating the item to make it easier to remember.\n" 'face `(:foreground ,org-drill-new-count-color) 'help-echo (concat "The number of new items that you " "have never reviewed.")) - prompt))) + (if org-drill-show-outline-path-during-drill + (concat (org-drill--outline-path-string) prompt) + prompt)))) (defun org-drill-presentation-prompt (session &optional prompt) "Create a card prompt with a timer and user-specified menu. @@ -4981,9 +5003,11 @@ an integer (0 when absent). AVG-QUALITY is DRILL_AVERAGE_QUALITY as a float, or nil when absent. DAYS-SINCE-REVIEW is the integer day count since DRILL_LAST_REVIEWED, or nil when never reviewed. DAYS-SINCE-ADDED is the integer day count since DATE_ADDED, or nil when absent. -TOTAL-REPEATS is DRILL_TOTAL_REPEATS as an integer (0 when absent)." +TOTAL-REPEATS is DRILL_TOTAL_REPEATS as an integer (0 when absent). +FILE is the file the entry's buffer visits, or nil for a fileless +buffer; card links carry it so the dashboard can jump across files." heading pos failure-count avg-quality - days-since-review days-since-added total-repeats) + days-since-review days-since-added total-repeats file) ;; Short aliases for the long struct accessors, used by the sort ;; comparators below so the comparator lines stay readable. @@ -5035,7 +5059,8 @@ via `org-get-heading', so the point must be on a drill heading." (org-drill-statistics--days-since-org-timestamp last-raw today-day) :days-since-added (org-drill-statistics--days-since-org-timestamp added-raw today-day) - :total-repeats (if repeats-raw (string-to-number repeats-raw) 0)))) + :total-repeats (if repeats-raw (string-to-number repeats-raw) 0) + :file (buffer-file-name)))) (defun org-drill-statistics--days-since-org-timestamp (timestamp today-day) "Return integer days from org TIMESTAMP string to TODAY-DAY. @@ -5244,25 +5269,51 @@ with no SCHEDULED time are ignored." ;; each scan and cap) gives the pre-cap totals needed for the "+N more" ;; footers for free. -(defun org-drill-statistics--card-link (heading pos) +(defun org-drill-statistics--card-link (heading pos &optional file) "Return an org bracket link to a drill card heading. HEADING is the card's outline heading string, used as the link -description. POS is the integer buffer position of the heading, carried -in the link path so the dashboard's RET handler can jump to the card. -The path has the form \"org-drill-card:POS\". Any closing bracket in -HEADING is replaced so a literal \"]]\" cannot terminate the link early. -An empty or nil HEADING falls back to a position-based description." +description. POS is the integer buffer position of the heading. FILE +is the file the card's buffer visits, or nil for a fileless buffer. +The path has the form \"org-drill-card:POS@FILE\" (FILE may be empty), +so the follow handler can jump to the card even across files. Any +closing bracket in HEADING is replaced so a literal \"]]\" cannot +terminate the link early. An empty or nil HEADING falls back to a +position-based description." (let* ((desc (if (and heading (not (string-empty-p heading))) heading (format "card at %d" pos))) (safe (replace-regexp-in-string "]" "}" desc))) - (format "[[org-drill-card:%d][%s]]" pos safe))) + (format "[[org-drill-card:%d@%s][%s]]" pos (or file "") safe))) + +(defun org-drill-statistics--follow-card-link (path &optional _prefix) + "Follow an org-drill-card link to its card. +PATH is \"POS@FILE\" (FILE may be empty) or a legacy bare \"POS\". +With a file, visit it in another window and move point to POS. Without +one, fall back to the dashboard's stored source buffer. Signals a +`user-error' when neither identifies a live target." + (let* ((at (string-match "@" path)) + (pos (string-to-number (if at (substring path 0 at) path))) + (file (and at (substring path (1+ at)))) + (source (and (boundp 'org-drill-statistics--source-buffer) + org-drill-statistics--source-buffer))) + (cond + ((and file (not (string-empty-p file))) + (find-file-other-window file) + (goto-char (min (max 1 pos) (point-max)))) + ((buffer-live-p source) + (pop-to-buffer source) + (goto-char (min (max 1 pos) (point-max)))) + (t (user-error + "Card buffer is gone; refresh the dashboard from a deck buffer"))))) + +(org-link-set-parameters "org-drill-card" + :follow #'org-drill-statistics--follow-card-link) (defun org-drill-statistics--render-attention-table (title rows total empty-note) "Render one needs-attention subsection as an org string. TITLE is the third-level heading text for the subsection. ROWS is a -list of (HEADING . POS) cons cells, already capped at +list of (HEADING POS FILE) lists, already capped at `org-drill-statistics-attention-row-limit' and already sorted. TOTAL is the full count of matching cards before capping, used for the \"+N more\" footer. EMPTY-NOTE is the line shown when ROWS is empty. @@ -5282,7 +5333,8 @@ the number of rows shown." (mapconcat (lambda (row) (format "| %s |" - (org-drill-statistics--card-link (car row) (cdr row)))) + (org-drill-statistics--card-link + (nth 0 row) (nth 1 row) (nth 2 row)))) rows "\n") "\n" @@ -5346,8 +5398,9 @@ pre-cap totals are available for the footers without rescanning." (lambda (structs) (mapcar (lambda (d) - (cons (org-drill-statistics--entry-attention-data-heading d) - (org-drill-statistics--entry-attention-data-pos d))) + (list (org-drill-statistics--entry-attention-data-heading d) + (org-drill-statistics--entry-attention-data-pos d) + (org-drill-statistics--entry-attention-data-file d))) (org-drill-statistics--cap-rows structs))))) (concat "** Needs attention\n" @@ -5612,6 +5665,14 @@ algorithms. Set by `org-drill-statistics' and rotated by `org-drill-statistics-cycle-algorithm'. Read by the render helpers via the filtered log they are handed.") +(defvar-local org-drill-statistics--source-buffer nil + "Buffer-local deck buffer the dashboard's card scans run in. +Set by `org-drill-statistics' to the buffer the command was invoked +from and read by `org-drill-statistics--render' (via refresh and the +cycle commands) so every card traversal sees the deck, not the +dashboard's own text. Also the fallback jump target for card links +whose path carries no file.") + (defun org-drill-statistics--range-cutoff-float (label) "Return the float-time cutoff for range LABEL, or nil for all time. LABEL is a key in `org-drill-statistics-range-presets'. When its DAYS @@ -5698,24 +5759,34 @@ is put in `org-mode'; not intended to be toggled by hand." :lighter " OrgDrillStats" :keymap org-drill-statistics-mode-map) -(defun org-drill-statistics--render (buffer scope range algorithm) +(defun org-drill-statistics--render (buffer scope range algorithm + &optional source) "Render the dashboard for SCOPE, RANGE, ALGORITHM into BUFFER. -BUFFER is the target buffer. Its read-only state is lifted for the -write, the assembled body replaces the contents, point returns to the -top, and the buffer is left read-only. The three filter values are -stored buffer-locally so refresh and the cycle commands can read and -rotate them. Returns BUFFER." - (with-current-buffer buffer - (let ((inhibit-read-only t) - (body (org-drill-statistics--render-all scope range algorithm))) - (erase-buffer) - (insert body) - (goto-char (point-min))) - (setq org-drill-statistics--scope scope - org-drill-statistics--range range - org-drill-statistics--algorithm algorithm) - (setq buffer-read-only t) - buffer)) +BUFFER is the target buffer. SOURCE is the deck buffer the card scans +run in, defaulting to the current buffer, so the traversals see the +deck rather than the dashboard's own text. A dead SOURCE signals a +`user-error'. BUFFER's read-only state is lifted for the write, the +assembled body replaces the contents, point returns to the top, and the +buffer is left read-only. The filter values and SOURCE are stored +buffer-locally so refresh and the cycle commands can read and rotate +them. Returns BUFFER." + (let ((source (or source (current-buffer)))) + (unless (buffer-live-p source) + (user-error + "The dashboard's deck buffer is gone; run org-drill-statistics from a deck")) + (let ((body (with-current-buffer source + (org-drill-statistics--render-all scope range algorithm)))) + (with-current-buffer buffer + (let ((inhibit-read-only t)) + (erase-buffer) + (insert body) + (goto-char (point-min))) + (setq org-drill-statistics--scope scope + org-drill-statistics--range range + org-drill-statistics--algorithm algorithm + org-drill-statistics--source-buffer source) + (setq buffer-read-only t) + buffer)))) ;;;###autoload (defun org-drill-statistics () @@ -5726,13 +5797,14 @@ filter header and the five render sections, using the current to it. Refresh and the s/r/a cycle commands re-render in place." (interactive) (let ((buffer (get-buffer-create org-drill-statistics--buffer-name)) + (source (current-buffer)) (scope org-drill-scope) (range (caar org-drill-statistics-range-presets)) (algorithm nil)) (with-current-buffer buffer (org-mode) (org-drill-statistics-mode 1)) - (org-drill-statistics--render buffer scope range algorithm) + (org-drill-statistics--render buffer scope range algorithm source) (switch-to-buffer buffer) buffer)) @@ -5745,7 +5817,8 @@ every section against the current log and card state. Bound to g." (current-buffer) org-drill-statistics--scope org-drill-statistics--range - org-drill-statistics--algorithm)) + org-drill-statistics--algorithm + org-drill-statistics--source-buffer)) (defun org-drill-statistics-cycle-scope () "Cycle the dashboard scope filter and refresh. @@ -5968,7 +6041,15 @@ do. Interactively, prompts for DIRECTORY." (org-drill-statistics--write-csv (expand-file-name "cards.csv" directory) org-drill-statistics--cards-csv-header - (org-drill-statistics--cards-rows scope)) + (let ((source org-drill-statistics--source-buffer)) + (cond + ((buffer-live-p source) + (with-current-buffer source + (org-drill-statistics--cards-rows scope))) + (source + (user-error + "The dashboard's deck buffer is gone; refresh from a deck first")) + (t (org-drill-statistics--cards-rows scope))))) (org-drill-statistics--write-csv (expand-file-name "daily.csv" directory) org-drill-statistics--daily-csv-header diff --git a/tests/test-org-drill-prompt-and-misc.el b/tests/test-org-drill-prompt-and-misc.el index 968ad57..5b42f6b 100644 --- a/tests/test-org-drill-prompt-and-misc.el +++ b/tests/test-org-drill-prompt-and-misc.el @@ -140,6 +140,59 @@ (org-drill-progress-message 42 50) (should (string-match-p "42" got-message))))) +;;;; org-drill outline path in the drill prompt (m.galimski patch) + +(ert-deftest test-org-drill--outline-path-string-nested () + "A nested entry yields its ancestor path, bracketed and arrow-joined." + (with-temp-buffer + (insert "* Spanish\n** Greetings\n*** Hola :drill:\nhola = hello\n") + (org-mode) + (goto-char (point-min)) + (search-forward "Hola") + (should (equal (org-drill--outline-path-string) "[Spanish > Greetings] ")))) + +(ert-deftest test-org-drill--outline-path-string-top-level-empty () + "A top-level entry has no ancestors, so the path string is empty." + (with-temp-buffer + (insert "* Hola :drill:\nhola = hello\n") + (org-mode) + (goto-char (point-min)) + (should (equal (org-drill--outline-path-string) "")))) + +(ert-deftest test-org-drill--make-minibuffer-prompt-omits-path-by-default () + "With the defcustom off, the outline path is absent from the prompt." + (with-temp-buffer + (let ((org-startup-folded nil) + (org-drill-show-outline-path-during-drill nil)) + (insert "* Spanish\n** Greetings\n*** Hola :drill:\nhola = hello\n") + (org-mode) + (goto-char (point-min)) + (search-forward "Hola") + (with-fixed-now + (let* ((session (org-drill-session)) + (prompt (substring-no-properties + (org-drill--make-minibuffer-prompt session "test")))) + (should-not (string-match-p "Greetings" prompt))))))) + +(ert-deftest test-org-drill--make-minibuffer-prompt-shows-path-when-on () + "With the defcustom on, the prompt carries the bracketed outline path." + (with-temp-buffer + (let ((org-startup-folded nil) + (org-drill-show-outline-path-during-drill t)) + (insert "* Spanish\n** Greetings\n*** Hola :drill:\nhola = hello\n") + (org-mode) + (goto-char (point-min)) + (search-forward "Hola") + (with-fixed-now + (let* ((session (org-drill-session)) + (prompt (substring-no-properties + (org-drill--make-minibuffer-prompt session "test")))) + (should (string-match-p "\\[Spanish > Greetings\\]" prompt))))))) + +(ert-deftest test-org-drill-show-outline-path-defaults-off () + "The outline-path defcustom ships nil so the prompt is unchanged by default." + (should (eq nil (default-value 'org-drill-show-outline-path-during-drill)))) + (provide 'test-org-drill-prompt-and-misc) ;;; test-org-drill-prompt-and-misc.el ends here diff --git a/tests/test-org-drill-statistics-render-attention.el b/tests/test-org-drill-statistics-render-attention.el index b3f2375..4adcfcc 100644 --- a/tests/test-org-drill-statistics-render-attention.el +++ b/tests/test-org-drill-statistics-render-attention.el @@ -65,8 +65,9 @@ current time so the fixture never hardcodes today." (test-org-drill-stats--attn-fixture) (let ((out (org-drill-statistics--render-attention 'file))) (should (string-match-p "| Card |" out)) - (should (string-match-p "\\[\\[org-drill-card:[0-9]+\\]\\[Leech 00\\]\\]" - out))))) + (should (string-match-p + "\\[\\[org-drill-card:[0-9]+@[^]]*\\]\\[Leech 00\\]\\]" + out))))) (ert-deftest test-org-drill-statistics-attention-cap-and-footer () "Twelve leeches over a 10 cap show 10 rows and a +2 more footer." @@ -126,15 +127,16 @@ current time so the fixture never hardcodes today." (ert-deftest test-org-drill-statistics-card-link-sanitizes-brackets () "Closing brackets in a heading cannot terminate the link early." (let ((link (org-drill-statistics--card-link "a]] b" 42))) - (should (string-prefix-p "[[org-drill-card:42][" link)) + (should (string-prefix-p "[[org-drill-card:42@][" link)) (should (string-suffix-p "]]" link)) (should-not (string-match-p "a]] b" link)))) (ert-deftest test-org-drill-statistics-card-link-empty-heading-fallback () "An empty heading falls back to a position-based description." (let ((link (org-drill-statistics--card-link "" 99))) - (should (string-match-p "\\[\\[org-drill-card:99\\]\\[card at 99\\]\\]" - link)))) + (should (string-match-p + "\\[\\[org-drill-card:99@[^]]*\\]\\[card at 99\\]\\]" + link)))) (provide 'test-org-drill-statistics-render-attention) diff --git a/tests/test-org-drill-statistics-source-buffer.el b/tests/test-org-drill-statistics-source-buffer.el new file mode 100644 index 0000000..5732b63 --- /dev/null +++ b/tests/test-org-drill-statistics-source-buffer.el @@ -0,0 +1,174 @@ +;;; test-org-drill-statistics-source-buffer.el --- Dashboard source-buffer tests -*- lexical-binding: t; -*- + +;;; Commentary: +;; ERT tests for the statistics dashboard's source-buffer plumbing and +;; the org-drill-card link type. The dashboard must scan the deck the +;; command was invoked from, not its own buffer, and its card links must +;; be followable. + +;;; Code: + +(require 'ert) +(require 'org-drill) +(require 'org) + +(defmacro org-drill-statistics-source-test--with-deck (var &rest body) + "Run BODY with VAR bound to a live org deck buffer holding two drill cards." + (declare (indent 1)) + `(let ((,var (generate-new-buffer "*source-deck*"))) + (unwind-protect + (with-current-buffer ,var + (org-mode) + (insert "* Card one :drill:\nBody one.\n" + "* Card two :drill:\nBody two.\n") + (goto-char (point-min)) + ,@body) + (when (buffer-live-p ,var) (kill-buffer ,var))))) + +;;; Normal cases + +(ert-deftest test-org-drill-statistics-render-counts-source-deck-cards () + "Rendering into the dashboard counts the cards of the source deck. +With the default file scope, the card scans must run in the source +buffer, not the dashboard buffer being rendered into." + (org-drill-statistics-source-test--with-deck deck + (let ((dash (generate-new-buffer "*dash-test*")) + (org-drill-scope 'file) + (org-drill-session-log nil)) + (unwind-protect + (progn + (with-current-buffer dash (org-mode)) + (org-drill-statistics--render dash 'file "last 90d" nil deck) + (with-current-buffer dash + (goto-char (point-min)) + ;; Overview table data row: | total | new | mature | lapsed | + (should (search-forward "| 2 | " nil t)))) + (kill-buffer dash))))) + +(ert-deftest test-org-drill-statistics-refresh-uses-stored-source () + "Refresh from inside the dashboard re-scans the stored source deck." + (org-drill-statistics-source-test--with-deck deck + (let ((dash (generate-new-buffer "*dash-test*")) + (org-drill-scope 'file) + (org-drill-session-log nil)) + (unwind-protect + (progn + (with-current-buffer dash (org-mode)) + (org-drill-statistics--render dash 'file "last 90d" nil deck) + (with-current-buffer dash + (org-drill-statistics-refresh) + (goto-char (point-min)) + (should (search-forward "| 2 | " nil t)))) + (kill-buffer dash))))) + +(ert-deftest test-org-drill-statistics-card-link-type-is-registered () + "The org-drill-card link type has a follow handler registered." + (should (functionp (org-link-get-parameter "org-drill-card" :follow)))) + +(ert-deftest test-org-drill-statistics-card-link-encodes-file () + "Card links carry the file identity after the position." + (let ((link (org-drill-statistics--card-link "Heading" 42 "/tmp/deck.org"))) + (should (string-prefix-p "[[org-drill-card:42@/tmp/deck.org][" link)))) + +(ert-deftest test-org-drill-statistics-follow-jumps-to-file-position () + "Following a card link visits the file and moves point to the position." + (let* ((file (make-temp-file "org-drill-follow-test" nil ".org" + "* Card one :drill:\nBody.\n")) + (buf nil)) + (unwind-protect + (progn + (org-drill-statistics--follow-card-link (format "1@%s" file)) + (setq buf (current-buffer)) + (should (equal (buffer-file-name) file)) + (should (= (point) 1))) + (when (and buf (buffer-live-p buf)) (kill-buffer buf)) + (delete-file file)))) + +;;; Boundary cases + +(ert-deftest test-org-drill-statistics-render-empty-deck-counts-zero () + "An empty source deck renders a zero-card overview, not an error." + (let ((deck (generate-new-buffer "*empty-deck*")) + (dash (generate-new-buffer "*dash-test*")) + (org-drill-scope 'file) + (org-drill-session-log nil)) + (unwind-protect + (progn + (with-current-buffer deck (org-mode)) + (with-current-buffer dash (org-mode)) + (org-drill-statistics--render dash 'file "last 90d" nil deck) + (with-current-buffer dash + (goto-char (point-min)) + (should (search-forward "| 0 | " nil t)))) + (kill-buffer deck) + (kill-buffer dash)))) + +(ert-deftest test-org-drill-statistics-card-link-empty-file-omits-identity () + "A card link built with no file still carries the position." + (let ((link (org-drill-statistics--card-link "Heading" 7 nil))) + (should (string-prefix-p "[[org-drill-card:7@][" link)))) + +(ert-deftest test-org-drill-statistics-export-cards-from-source-deck () + "CSV export scans the stored source deck, not the dashboard buffer." + (org-drill-statistics-source-test--with-deck deck + (let ((dash (generate-new-buffer "*dash-test*")) + (dir (make-temp-file "org-drill-export-test" t)) + (org-drill-scope 'file) + (org-drill-session-log nil)) + (unwind-protect + (progn + (with-current-buffer dash (org-mode)) + (org-drill-statistics--render dash 'file "last 90d" nil deck) + (with-current-buffer dash + (org-drill-statistics-export-csv dir)) + (with-temp-buffer + (insert-file-contents (expand-file-name "cards.csv" dir)) + ;; Header plus one row per deck card. + (should (= 3 (count-lines (point-min) (point-max)))))) + (kill-buffer dash) + (delete-directory dir t))))) + +(ert-deftest test-org-drill-statistics-follow-clamps-position-floor () + "A zero or garbage position clamps to buffer start instead of erroring." + (let* ((file (make-temp-file "org-drill-follow-clamp" nil ".org" + "* Card :drill:\n")) + (buf nil)) + (unwind-protect + (progn + (org-drill-statistics--follow-card-link (format "0@%s" file)) + (setq buf (current-buffer)) + (should (= (point) 1))) + (when (and buf (buffer-live-p buf)) (kill-buffer buf)) + (delete-file file)))) + +;;; Error cases + +(ert-deftest test-org-drill-statistics-refresh-dead-source-signals-user-error () + "Refresh after the source deck is killed signals a clear user-error." + (let ((dash (generate-new-buffer "*dash-test*")) + (deck (generate-new-buffer "*doomed-deck*")) + (org-drill-scope 'file) + (org-drill-session-log nil)) + (unwind-protect + (progn + (with-current-buffer deck + (org-mode) + (insert "* Card :drill:\n")) + (with-current-buffer dash (org-mode)) + (org-drill-statistics--render dash 'file "last 90d" nil deck) + (kill-buffer deck) + (with-current-buffer dash + (should-error (org-drill-statistics-refresh) + :type 'user-error))) + (when (buffer-live-p deck) (kill-buffer deck)) + (kill-buffer dash)))) + +(ert-deftest test-org-drill-statistics-follow-dead-source-signals-user-error () + "Following a bufferless link with no live source signals a user-error." + (with-temp-buffer + ;; No dashboard source var set in this buffer; empty file identity. + (should-error (org-drill-statistics--follow-card-link "5@") + :type 'user-error))) + +(provide 'test-org-drill-statistics-source-buffer) +;;; test-org-drill-statistics-source-buffer.el ends here |