Files
rdbms-playground/docs/handoff/20260528-handoff-48.md

13 KiB
Raw Permalink Blame History

Session handoff — 2026-05-28 (48)

Forty-eighth handover. ADR-0038 is feature-complete except for the §4 styled-runs rendering polish. Buckets A + B and the §6 category-3 prose all ship, every catalogue row round-trips per line, docs are in sync, and a /runda Devil's-Advocate pass landed cleanup. The session built the renderer out from the handoff-46 walking skeleton through Phases 1, 2, and 3 in one go; if you continue, the polish is the only open item. If you start a different feature, ADR-0038 won't surprise you.

§1. State at handoff

Branch: main. HEAD 5cb105b. Tests: 2015 passing, 0 failing, 0 skipped, 1 ignored. Clippy: clean (--all-targets -D warnings, nursery).

Commits since handoff-47's c60026c:

5cb105b docs(adr): /runda DA cleanup — reflect Phases 1-3 done, pin Bucket C
e6ad1ae feat: DSL→SQL teaching echo — Phase 3 cat-3 caveat (ADR-0038)
275c726 feat: DSL→SQL teaching echo — Bucket B renderer (ADR-0038 Phase 2)
558cfae docs(adr): promote ADR-0037 + ADR-0038 to Accepted (Phase 1 shipped)

(Plus handoff-47 itself, c60026c, immediately prior.)

§2. ADR status

ADR What Build status
ADR-0035 Am2 standard-first dialect + ALTER COLUMN gap-fill done (handoff-46 338dc8a)
ADR-0033 Am4 update … --all-rows falls back to DSL done (handoff-46 9a23e28)
ADR-0037 EffectiveMode execution-time side-channel Accepted — channel done (handoff-46 04c8e42), validated end-to-end by all four ADR-0038 commits
ADR-0038 the echo + full catalogue Accepted — Phases 1-3 done (90479cb / 275c726 / e6ad1ae); §4 styled-runs polish remains
ADR-0039 EXPLAIN over advanced SQL ⏸ deferred follow-up — separate feature

ADR-0037 and ADR-0038 were promoted to Accepted with implementation notes in 558cfae; the /runda cleanup (5cb105b) refreshed both entries to reflect every shipping commit.

§3. What landed since handoff-47

§3a. Phase 2 — Bucket B renderers (275c726, +1049 / 53 across 4 files)

Two new echo-build paths the handoff-46 ⚠️ gotcha foreshadowed:

  • collect_echo_lookups (pre-execution, runtime) — resolves names the dropped thing or not-yet-created column would erase post- execution: positional drop index (describe the table, find the index by exact column-set match), drop relationship in both selector forms (Endpoints via child describe + matching by endpoints; Named via a small list_tables scan acceptable for teaching-playground schemas), and the --create-fk pre-state (whether the child column already existed + the parent's PK type to derive the new column type via Type::fk_target_typeserial → int, shortid → text).
  • build_schema_echo (post-execution, runtime) — subsumes the Bucket A pure-Command schema cases (create table, rename column, add/drop constraint — delegates to command_to_sql) and renders Bucket B from the description (add index resolved from desc.indexes by column set; add relationship from the parent's inbound_relationships by endpoints) and the lookups.

Plus a payload refactorOption<String>Option<Vec<String>> on all 7 success events + arms + render path, motivated by the category-2 multi-statement echoes (drop column --cascade, add relationship --create-fk). Each catalogue row's pure renderer (and its round-trip) lives in src/echo.rs; the runtime composes them.

The DropColumn arm gained build_drop_column_cascade_echo, which reads DropColumnResult::dropped_indexes to emit the multi-line cascade echo; non-cascade falls through to the pre-execution Bucket A echo unchanged.

§3b. Phase 3 — category-3 prose (e6ad1ae, +130 / 8 across 5 files)

Phase 3 turned out to be small: the existing client_side.* notes already covered shortid generation (auto_fill_add_shortid, auto_fill_transition) and type-conversion transforms (transformed, transformed_lossy), and they already render in the right position (between the echo and the structure). Only the change column … --dont-convert caveat was actually missing — the one Bucket A caveat (every other category-3 line is illuminating).

Wiring: a new dont_convert_caveat: bool on DslChangeColumnSucceeded, set by the runtime when submission_mode.is_advanced() && matches!(cmd, ChangeColumnType { mode: DontConvert, .. }), rendered by handle_dsl_change_column_success between the client-side notes and the structure. Gated on advanced mode because the caveat references "the line above" — the echo, which only fires in advanced mode. New i18n key client_side.dont_convert_caveat (registered in keys::KEYS_AND_PLACEHOLDERS).

§3c. /runda DA cleanup (5cb105b, +128 / 32 across 4 files)

Devil's-Advocate audit at the end of the session caught three doc-drift bugs the per-phase commits left behind (requirements.md M4 and both ADR-0038 Status block + README index entry still said "Phase 2 / Phase 3 remain") and one test gap — Bucket C cases (show table, explain, replay, every Command::App variant) flowed through the catch-all _ => None without explicit no-echo coverage. The new bucket_c_no_echo_commands_all_return_none test pins all four; a future renderer arm at the wrong place can no longer silently leak.

§4. The architecture in shape now

Two echo construction paths now run in spawn_dsl_dispatch, side by side:

  • Pre-execution (echo::echo_forecho::command_to_sql) — the outer echo var, used by the non-Schema arms (Update, Delete, AddColumn, DropColumn non-cascade, ChangeColumn). Pure function of the Command. Returns Option<Vec<String>> (single-element for the Bucket A single-line cases).
  • Post-execution — three builders, each in the arm matching its outcome:
    • build_schema_echo(command, mode, description, lookups) for Schema outcomes (subsumes Bucket A pure-Command schema + Bucket B add index / drop index / add/drop relationship).
    • build_show_data_echo(database, command, mode) for Query outcomes (only show data echoes; the runtime describes the table when the query is limited to source the ORDER BY PK).
    • build_drop_column_cascade_echo(command, mode, result) for the DropColumn arm when cascade: true (reads DropColumnResult::dropped_indexes); non-cascade falls through to the pre-execution echo.

The pre-execution lookup helper collect_echo_lookups runs once at the top of the dispatch, captures whatever drops or --create-fk pre-state will need post-execution, and passes through to build_schema_echo. Replay still bypasses the whole spawn (run_replay calls execute_command_typed directly) so no echo fires there — per ADR-0038 §3.

Rendering: note_ok_summary loops over pending_echo: Vec<String>, emitting one Executing SQL: <line> per entry via note_system. Plain [system] lines today — the polish replaces this with ADR-0028 styled runs.

§5. The one open item — §4 styled-runs polish

ADR-0038 §4: "The App renders it as one or more de-emphasised OutputLines beneath the [ok] summary, using the ADR-0028 styled- runs mechanism (a dimmed Executing SQL: prefix; the SQL in a code-ish run). One statement per line (§6 category 2)."

Today the echo + the DontConvert caveat surface as plain note_system lines (a single style). The polish needs:

  • Read ADR-0028 for the styled-runs OutputLine shape — there is already a mechanism (used by the explain plan tree rendering — see output_render::render_explain_plan for the established pattern of pushing pre-built OutputLines via push_output).
  • Replace note_system(crate::t!("echo.executing_sql", sql = line)) in note_ok_summary with a styled OutputLine: dimmed Executing SQL: prefix run + a code-ish run for the SQL itself. Decide the exact style (italic? dim? distinct fg colour?) consistent with ADR-0028's existing styled-runs surface; check both theme::light() and theme::dark() for legibility.
  • The DontConvert caveat is also a category-3 prose line — apply the same de-emphasised treatment (or a complementary one — caveat could warrant a WARNING/yellow accent, since it flags non-equivalence).
  • Multi-statement echoes: each line in the Vec becomes its own styled OutputLine. Today the rendering already emits one line per entry; the polish is per-line styling, not per-line splitting.

Tests will need adjustment: the App-level tests (bucket_a_success_*, bucket_b_multi_line_*, change_column_dont_convert_renders_*) currently assert texts[…].contains("Executing SQL: …") from note_system output. After the polish, the same content will live in styled-runs — the tests should still find the substring within the output line text but may need to look at OutputLine styled-run content rather than the flat text field. Decide style-vs-text assertion shape early.

The work is rendering-only — no Command-level changes, no event-shape changes, no worker changes. Estimated scope: small. The biggest care point is consistency with ADR-0028's existing styled-runs usage.

§6. Catalogue cheat-sheet (authoritative: ADR-0038 §7)

Unchanged since handoff-47. Every Bucket A and Bucket B row is now implemented and round-tripped (echo::tests::* + the runtime end-to-end tests bucket_b_resolved_name_echoes_against_real_worker + bucket_b_multi_statement_echoes_against_real_worker). Bucket C (no-echo) is explicitly pinned by bucket_c_no_echo_commands_all_return_none. Category-3 lines flow through the existing client_side.* infrastructure plus the new dont_convert_caveat.

§7. Known follow-ups outside the ADR-0038 scope

These are not the styled-runs polish — they are separate items the /runda surfaced or that emerged during the session, captured here so they aren't lost.

  • Unquoted-identifier round-trip caveat. The echo style (render_* in src/echo.rs) emits bare identifiers — matching the existing render_create_table aesthetic. A user-created column or table named after a SQL keyword (e.g. ORDER, SELECT, WHERE) would produce an echo whose round-trip is fragile. Pre-existing limitation (the create-table echo had it from handoff-46); the §1 contract is only formally violated for this edge case. If it ever bites a learner, the fix is to add identifier quoting (probably backticks or double-quotes per the advanced grammar's accepted spelling, plus a conservative "needs quoting" predicate).
  • No full-spawn integration test for the echo pipeline. Coverage today is layered: pure renderers + runtime-helper unit tests (with real Database) + App-level rendering tests. The unwired glue is small (~3 lines per arm in spawn_dsl_dispatch). A Tier-3 test that drives the full submit → runtime → App pipeline would complete the picture; not blocking.
  • Pure-Command Schema commands double-compute echo. echo_for is called for every command, and the Schema arm then computes build_schema_echo (whose catch-all redelegates to command_to_sql). Minor inefficiency, no behaviour bug. Could be optimized later; not worth the code complexity now.
  • ADR-0039 (EXPLAIN over advanced SQL) — separate deferred follow-up, not part of ADR-0038. Recorded 2026-05-27 (handoff-45/46); pick up when prioritised.

§8. Process pins (unchanged)

  • Confirm every commit (propose message, wait). No AI attribution.
  • Test-first; green + clippy-clean is the only acceptable end state; current baseline 2015 / 0 / 1.
  • Keep docs lockstep: ADR + README.md index + requirements.md. This session's 5cb105b was the corrective pass when the prior per-phase commits left M4 / ADR-0038 status drifted.
  • Round-trip every catalogue row (the §1 copy-paste contract; for multi-statement, per line).
  • Pre-execution lookups for any future Bucket-B-style additions whose echo needs info the dropped/created thing erases. Mirror the collect_echo_lookups shape (best-effort, fail-soft) and consume in the post-execution arm.

§9. How to take over

  1. Read, in order: this file → ADR-0038 (§4 styled-runs reference is the relevant bit for the polish) → ADR-0028 (OutputLine styled-runs mechanism; output_render::render_* for established patterns) → src/echo.rs (the renderer surface itself is settled — but read to understand the catalogue shape if you'll touch rendering) → src/app.rs's note_ok_summary and handle_dsl_change_column_success (the two consumers of the echo
    • caveat strings — these are what the polish replaces).
  2. Baseline: cargo test (2015 / 0 / 1) + cargo clippy --all-targets -- -D warnings (clean).
  3. If continuing on ADR-0038: the polish is the only open item. See §5. Decide the App-test assertion shape early (style-vs-text).
  4. If picking up something else: ADR-0038 is done modulo the polish; the §4 line of the ADR remains the formal outstanding item but does not block other work. ADR-0039 (EXPLAIN over advanced SQL) is a self-contained deferred follow-up if you want the next feature.