diff --git a/docs/handoff/20260602-handoff-54.md b/docs/handoff/20260602-handoff-54.md new file mode 100644 index 0000000..70e6a84 --- /dev/null +++ b/docs/handoff/20260602-handoff-54.md @@ -0,0 +1,272 @@ +# Session handoff — 2026-06-02 (54) + +Fifty-fourth handover. **Two issues off handoff-53's open list, +resolved in sequence.** Closed **#10** (output `[error]`/`[system]` +tag-colour collision) and **#14** (per-project input-mode persist & +restore). Two ADR amendments authored: **ADR-0037 Amendment 1** and +**ADR-0015 Amendment 1**. + +The session is worth reading for the **same process lesson, paid twice**: +the `/runda` (DA-hat) pass on the **implementation** found real, +ship-blocking gaps that the green suite did not — a semantics +discrepancy on #14 (which the user then re-decided), and a missing +integration test for the #14 unload wiring (added red-first afterwards). +See §2. + +## §1. State at handoff + +**Branch:** `main`. **HEAD `516848f`.** **Ahead of `origin/main` by +5** — handoff-52's `8311de4` (#9) and handoff-53's `6d8c9ee` (#15/#16) +were already unpushed at the start, and this session adds three on top. +**Tests: 2134 passing / 0 failing / 1 ignored** (full `cargo test`; +lib-only is **1565**). The 1 ignored is the same long-standing +`` ```ignore `` doctest in `src/friendly/mod.rs` as every prior baseline. +**Clippy: clean** (nursery, all targets, `-D warnings`). Push is the +user's step. + +Commits since handoff-53's `6d8c9ee`: + +``` +516848f test: integration-test the mode persist-on-unload wiring (#14) +4cd574b feat: persist & restore per-project input mode (#14) +ae57c6f feat: colour output tags by status, not mode — readable error bodies (#10) +``` + +**The five unpushed commits resolve four issues — close them on GitHub +once pushed: #15, #16, #10, #14.** (#15/#16 are from handoff-53; +#10/#14 are this session.) + +## §2. The process saves that paid off (read this) + +Both issues followed the now-standard arc: read the issue + the ADRs it +names → trace the exact symbols → **escalate the genuine design choices +to the user** → test-first build → **`/runda` on the implementation** → +fix findings red-first → commit. The DA pass earned its keep on both, +and twice on #14. + +1. **#14, round one (during impl): a semantics discrepancy.** My first + build persisted the mode *on change* (the `mode` command + any DDL). + I had told the user it would persist "at quit." The DA pass caught + the divergence and I escalated it rather than shipping. The user + re-decided: **persist on *unload*** (quit + project switch), rejecting + both persist-on-change (confusingly selective — whether a `--mode` + override "stuck" depended on whether you happened to run a DDL) and + persist-on-*any*-command (too heavy — would rewrite `project.yaml` + and bump its mtime, which orders the load picker, on every read-only + `select`/`show data`). The shipped design is the user's. + +2. **#14, round two (post-commit `/runda`, user-requested): a missing + test.** The persist-on-unload code was written *after* round one's DA + pass, so it had never had its own adversarial review. The second pass + verified every exit path (the `quit` command, **Ctrl-C**, and the + **fatal-persistence quit** all route through `Action::Quit` → + `set_mode`; only an abnormal channel-close skips it, acceptably) and + the temp-cleanup interaction (writing `mode:` keeps `tables: []`, so + `is_unmodified_temp` still deletes empty temps). It found one real + gap: the unload *wiring* had no integration test. Added + `runtime::switch_persists_the_outgoing_projects_mode`, driving the + real (private) `handle_project_switch` end-to-end — **red-first + verified** (disable the `set_mode` call → `None` vs `Some(Advanced)`). + Commit `516848f`. + +3. **#14 design: the user simplified my over-engineering — twice.** My + first instinct mirrored the mode in the db metadata table (like + `created_at`). The user pushed back: mode is *live UI state, known at + any time* — just write the current value on every `project.yaml` + write; there is nothing to "preserve." That deleted a whole layer. + +Lesson reinforced (held since handoff-52 §2.2 / handoff-53 §2): +**`/runda` on the implementation, not just the design.** A green suite +is necessary, not sufficient. + +## §3. Resolved issues + +### #10 — `[error]`/`[system]` tag-colour collision (`ae57c6f`, ADR-0037 Amendment 1) + +**Problem.** `render_output_line` (`src/ui.rs`) coloured the output tag +by `mode_at_submission` for *every* line kind, so a `[system]` line and +an `[error]` line had an identical leftmost tag (blue in simple, orange +in advanced) — distinguishable only by body colour. And flooding the +whole error body in red made long messages hard to read. + +**Change — the status-coloured-tag model.** The tag is coloured by the +message's **status** (`OutputKind`): `[system]` → green, `[error]` → +red, **echo keeps the mode tint** (the *sole* exception — that is +ADR-0037's actual stated purpose; per-command success rides the ✓/✗ +marker). Bodies go neutral `theme.fg`; the **error body is bold** +(rustc-style: severity-coloured label, readable bold message). Yields a +status traffic-light (green = ok, red = error) matching the ADR-0040 +✓/✗ palette. This *narrows* ADR-0037's mode side-channel to the echo +line it was always for, and closes the tag-colour gap ADR-0040 had +flagged as orthogonal (issue #10). + +**Two autonomous decisions flagged + user-confirmed:** `TeachingEcho` +tag → green (it is a `[system]`-tagged line); echo tag stays +mode-tinted. + +**Files:** `src/ui.rs` (the single rendering site — confirmed no other +call site colours tags); ADR-0037 Amendment 1 + README. **Coverage:** 4 +`ui.rs` tests incl. `error_and_system_tags_are_distinguishable_in_both_themes` +(the direct regression guard, dark + light). The insta snapshots only +capture glyphs (`symbol()`), not colour, so no snapshot churn. + +### #14 — per-project input-mode persist & restore (`4cd574b` + `516848f`, ADR-0015 Amendment 1) + +**Decision (user's).** The input mode is **per-project state stored in +`project.yaml`** (`project.mode:`, optional, default `simple`), restored +on every open and persisted on **unload**. A teacher can ship a project +that opens in advanced mode and hand/export it to students; a learner's +mode is restored per project ("loading triggers the mode switch each +time"). The export carries the mode (it is part of `project.yaml`) — +that is the intended teacher→student behaviour, not a leak. + +**Mechanism (deliberately *not* the db-meta round-trip used for +`created_at`).** Mode is live UI state, not schema, and is **not stored +in the database**. The persistence handle carries the current mode and +the worker **stamps it into `project.yaml` on every write** — so a later +schema-mutating command rewrites the live value rather than clobbering +it; there is nothing to preserve. `rebuild` ignores the field. Optional +field with a `simple` default → pre-#14 files parse unchanged, no +version bump, no migrator (same pattern as the `unique` index flag). + +**Restore precedence: `--mode` > stored > `simple`.** New CLI flag +`--mode simple|advanced` overrides at startup; combines with `--resume` +and a positional path (not mutually exclusive — flag wins on collision). +`Mode::resolve_startup(flag, stored)` is the extracted, tested precedence +helper. The `--mode` override applies **only at boot**; a later switch +restores that project's own stored mode. + +**Persist on unload (the deciding rule).** The mode is written whenever +the current project is unloaded — on quit and on a project switch +(load/new/save-as/import), the runtime calls `set_mode(App.mode)` on the +outgoing database before it is dropped. By the time you leave a project, +the mode you were in is always recorded — including a bare `--mode` +override or a read-only session. The `mode` command also persists +immediately (`Action::PersistMode` → `Database::set_mode`, crash-safe). +All unload persistence is **best-effort** (a write failure must never +escalate a UI action into a fatal). A switch saves the outgoing mode, +then restores the incoming project's stored mode via the +`ProjectSwitched` event. + +**Files:** `src/mode.rs` (keyword parse/round-trip + `resolve_startup`), +`src/cli.rs` (`--mode`), `src/persistence/mod.rs` (`Persistence` +current-mode + `read_stored_mode`), `src/persistence/yaml.rs` +(serialize/deserialize + `parse_stored_mode` — `None` distinguishes +"no preference" from explicit `simple`), `src/db.rs` +(`SchemaSnapshot.mode`, `Request::SetMode`, `Database::set_mode`, +`finalize_persistence` stamp), `src/action.rs` (`PersistMode`), +`src/app.rs` (mode command emits it; `ProjectSwitched` restores), +`src/event.rs` (`ProjectSwitched { mode }`), `src/runtime.rs` (boot +resolve + the two unload call sites), `src/archive.rs` (export +round-trip test) + ADR-0015 Amendment 1, ADR-0003 note, README, +`requirements.md` L1b, `--help` banner. + +**Coverage (23 tests):** `mode.rs` parse/round-trip + `resolve_startup`; +`yaml.rs` mode round-trip / default-when-absent / +`parse_stored_mode` absent-vs-explicit / unknown-value; +`persistence` read_stored_mode round-trip + missing-file; `db` +`set_mode_persists_and_survives_a_later_ddl_command` (no-clobber) + +`set_mode_persists_even_with_no_prior_command` (unload-with-no-work); +`archive::export_carries_the_stored_input_mode`; `cli` `--mode` +parse/precedence/combines-with-resume; `app` mode-command-emits-persist +/ one-shot / `ProjectSwitched`-restores; +`runtime::switch_persists_the_outgoing_projects_mode` (the real-path +integration test, `516848f`). + +## §4. ADR / docs work + +- **ADR-0037 Amendment 1 — new** (`ae57c6f`): the status-coloured-tag + model (table + rationale + the rustc-precedent for bold-neutral error + bodies + coverage). README ADR-0037 entry gains the amendment note; + ADR-0040's "issue #10 is orthogonal" gap is now closed (cross-noted). +- **ADR-0015 Amendment 1 — new** (`4cd574b`, coverage note extended in + `516848f`): per-project mode in `project.yaml`; mode-is-live-state / + worker-stamps-on-write; `--mode` + precedence; persist-on-unload (with + the two rejected alternatives recorded — persist-on-change and + persist-on-any-command — and *why*). README ADR-0015 + ADR-0003 + entries updated; `requirements.md` gains **L1b** (marked done). +- **ADR-0003 note**: the startup mode is no longer always `simple`. + +## §5. What's open + +### Bug reports still open (filed handoff-50) + +| # | Title | Label | +|---|-------|-------| +| [11](https://github.com/oliversturm/rdbms-playground/issues/11) | Copy output panel contents to the system clipboard | enhancement | + +**Closed this session:** #10, #14 (commits `ae57c6f`, `4cd574b`, +`516848f` — **close on GitHub once pushed**, along with the still-open- +on-GitHub #15/#16 from handoff-53). + +### Categorisation / notes for pickup — #11 + +- **#11 is the only one left** from the handoff-50 bug-list trio. It is + a *different shape* from the recent run: it pulls in a **new + cross-platform clipboard crate** — the first new third-party + dependency in a while. +- **Security-Reviewer hat applies** (per the global standards: new + dependency → include it). Vet the crate before adopting: run the + mandated scanners (`trivy fs`, `grype`, `osv-scanner`, `cargo audit`/ + `cargo deny` if available), check maintenance/popularity, and prefer a + well-maintained option (e.g. `arboard` is the common choice — confirm + current status, ~3–6 month freshness, and Wayland/X11/macOS/Windows + support). Escalate the crate choice to the user. +- Likely a new app-level command (`copy` / `yank`?) + a key binding — + **escalate the command surface** (name, sigil-free per ADR-0009, which + modes) before building. There is no ADR for clipboard yet; decide + whether it needs one (probably a short ADR, given it adds a dep and a + command). + +### Other tracks (from `requirements.md`) + +- Track 2 project storage Iter 5/6 — export/import shipped; `--resume` + shipped; **mode restore (L1b) shipped this session**; remaining Iter 6: + `history.log`-based persistent input-history hydration (§12), + migration-framework exercise. +- C3a (modify relationship), C4 (m:n convenience). +- H1 friendly DB-error layer (partial); tutorial/lesson system (needs + its own ADR); V4 session log + Markdown export; I1/I1b multi-line + + readline; I3 tab-completion polish; I4 highlighting beyond input echo. + +## §6. Process pins (carried forward + this session's reinforcement) + +- **`/runda` on the implementation, not just the design.** This session, + twice on #14 (semantics discrepancy → re-decided by user; missing + integration test → added red-first). Held from handoff-52/-53. +- **Red-first proof for every fix *and* every load-bearing test.** The + #14 integration test was proven by disabling the `set_mode` call and + watching it fail (`None` vs `Some(Advanced)`) before restoring. +- **Escalate genuine design choices — do not decide for the user.** This + session: #10's error-tag treatment + the two autonomous tag decisions; + #14's storage location (project.yaml vs db-meta vs new file), the + persist timing (the big one), and the `--mode` precedence. The user + re-decided the persist semantics *after* I had built the wrong one — + surfacing it beat shipping it. +- **Let the user simplify.** Twice on #14 the user removed complexity I + had added (the db-meta mirror; the over-selective persist rule). Lead + with the simplest correct design; flag low-confidence improvisation. +- **Commits on `main`, append-only, user-confirmed, no AI attribution.** + Each commit message was proposed and approved before `git commit`. + Push remains the user's step; five unpushed commits is a normal + working state. +- **insta snapshots capture glyphs, not colour** (handoff-aid for future + output-rendering work): colour-only changes to `render_output_line` + do not churn snapshots. + +## §7. How to take over + +1. Read this note, then `CLAUDE.md`, then `docs/requirements.md` (L1b is + now done), then `docs/adr/README.md` and any ADR you'll touch + (ADR-0037 + ADR-0015 each gained an Amendment 1 this session). +2. The codebase is on `main` at `516848f`, working tree clean, **5 + commits ahead of origin** (all unpushed — see §1). Closing #15, #16, + #10, #14 on GitHub is pending the push (the user's step). +3. If continuing the bug-list run: **#11 is the only one left.** It is a + new-dependency task — engage the Security-Reviewer hat for the + clipboard crate, and escalate both the crate choice and the command + surface before building (§5). Consider whether it warrants a short + ADR (new dep + new command suggests yes). +4. Honour the process pins in §6 — `/runda` on implementation + + red-first + escalate-don't-decide are the three that earned their + keep this session.