add handoff-13: typing-surface matrix complete + 5 matrix-found bug fixes

2026-05-15 20:53:25 +00:00
parent c7ecc64757
commit 42cf851f7e
1 changed files with 299 additions and 0 deletions
@@ -0,0 +1,299 @@
+# Session handoff — 2026-05-15 (13)
+
+Thirteenth handover. This session executed handoff-12 §1: the
+systematic typing-surface test matrix. The matrix was built
+across all thirteen command families and, in the process,
+surfaced and fixed five production bugs that the 859-test suite
+had missed — exactly the class of issue §1 was designed to
+catch.
+
+**The headline: the matrix works. It found real bugs on its
+first three Form A cells and kept finding them through the DDL
+families. Every bug is now fixed and pinned by a regression
+cell.** See §2 for the bug list and §4 for what's still open.
+
+## State at handoff
+
+**Branch:** `main`. Working tree clean. Local HEAD is ahead of
+`origin/main` by twenty-four commits across four sessions (the
+user pushes asynchronously; nothing here is pushed).
+
+Commits since handoff-12 (`3b1955c`):
+
+```
+0b15ce0 Walker + parser: surface mid-typing after separators and Form C/A ambiguity
+24e641b Matrix: typing-surface infrastructure + insert Form A coverage
+fffb44f input_render: schema-aware classify_input for wrong-count value lists
+a9a04cf Matrix: insert Form B coverage
+37db2f5 Matrix: insert Form C + update + delete coverage
+216e7ba DDL grammar: writes_table on table-name slots for column narrowing
+c7ecc64 Matrix: create table, DDL, and app-command coverage
+```
+
+(Plus `3b1955c` itself — the handoff-12 commit — which was the
+prior HEAD.)
+
+**Tests:** **989 passing, 0 failing, 1 ignored** (up from 859).
+The 130 new tests are the typing-surface matrix
+(`tests/typing_surface_matrix.rs` + `tests/typing_surface/`).
+The ignored test is the long-standing `` ```ignore `` doc-test
+in `src/friendly/mod.rs`.
+
+**Clippy:** clean with `nursery` lints + `-D warnings`.
+
+## §1. What shipped — the typing-surface matrix
+
+Per handoff-12 §1. The matrix is a folder of per-command test
+files under `tests/typing_surface/`, driven by a single test
+binary (`tests/typing_surface_matrix.rs`).
+
+### Infrastructure (`tests/typing_surface/mod.rs`)
+
+- **Five canonical schema shapes** (handoff-12 §1
+  CANONICAL_SCHEMA_SHAPES): `schema_empty`, `schema_serial_pk`
+  (Customers: id:serial, Name:text, Email:text), `schema_text_pk`
+  (Items: Code:text, Title:text), `schema_multi_table`
+  (Customers + Orders, disjoint column names — the
+  leakage-detection shape), `schema_every_type` (Things: one
+  column per `Type` variant).
+- **`assess(input, cursor, schema) -> Assessment`** — runs the
+  four typing-surface APIs at one cell: `classify_input_with_schema`,
+  `ambient_hint`, `candidates_at_cursor`, `parse_command_with_schema`.
+- **Property-assertion helpers** — `assert_no_candidate_named`
+  (leakage detection), `assert_candidate_present`,
+  `hint_prose_contains`, etc.
+- **`snap!` macro** — wraps `insta::assert_debug_snapshot!` with a
+  stable per-cell suffix. Every cell has both a snapshot (drift
+  detection) and explicit property assertions (bug-class
+  regressions). This dual approach is deliberate: the four
+  handoff-12 bugs were *properties* (no leakage, X offered, not
+  red), not snapshots.
+
+### Coverage — all 13 command families
+
+| File | Tests | Notes |
+|---|---|---|
+| `insert_form_a.rs` | 23 | explicit column list |
+| `insert_form_b.rs` | 12 | bare `values` keyword |
+| `insert_form_c.rs` | 8 | bare value list |
+| `update_with_where.rs` | 12 | |
+| `update_all_rows.rs` | 4 | |
+| `delete_with_where.rs` | 7 | |
+| `delete_all_rows.rs` | 3 | |
+| `create_table.rs` | 11 | |
+| `drop_column.rs` | 6 | drove the §2.2 fix |
+| `drop_relationship.rs` | 7 | endpoints + by-name |
+| `add_relationship.rs` | 9 | |
+| `rename_change_column.rs` | 8 | |
+| `app_commands.rs` | 17 | quit/help/save/… |
+| (infrastructure smoke) | 3 | in `mod.rs` |
+
+## §2. Bugs the matrix found — all fixed
+
+Five production bugs, each now pinned by a matrix cell plus (for
+the parser/walker ones) the regression survives in the matrix.
+
+**Bug 1 — `walk_repeated` rolled back mid-typing after a
+separator.** `insert into T (a, ` at EOF rolled the matched `,`
+back, the outer `(`-list then expected `)`, saw the `,`, and
+reported `DefiniteErrorAt` at the comma. The input pane flashed
+red on every comma. Fix (`src/dsl/walker/driver.rs`): when the
+separator consumed and the inner failed at post-separator EOF,
+propagate `Incomplete` instead of rolling back. Commit `0b15ce0`.
+
+**Bug 2 — Form C accepted column-shaped items.** `insert into T
+(Name)` (no `values` keyword) walked to a complete match and
+produced `Insert { columns: None, values: [] }` — Form C's value
+collector silently drops ident-shaped items. The user meant
+Form A. Fix (`src/dsl/grammar/data.rs` `build_insert`): reject
+with a `ValidationError` (`parse.custom.insert_form_a_missing_values`)
+naming the Form-A continuation; the input now classifies as
+`IncompleteAtEof`. Commit `0b15ce0`.
+
+**Bug 3 — `ValidationFailed` lost the expected set.**
+`completion_probe` / `expected_at_input` returned an empty
+expected set for `ValidationFailed` outcomes, so Tab had nothing
+to offer at the new Form-A flag point. Fix
+(`src/dsl/walker/mod.rs`): surface `result.tail_expected` (the
+skipped-Optional expectations captured before validation fired).
+Commit `0b15ce0`.
+
+**Bug 4 — `classify_input` was schemaless.** Wrong-count Form B
+value lists (`insert into Customers values ('Alice')` against a
+3-column table) showed as `Valid` until submit. Fix
+(`src/input_render.rs`): added `classify_input_with_schema`,
+wired `render_input_runs` to it. Schemaless `classify_input`
+kept public for schema-independent regression tests. Commit
+`fffb44f`.
+
+**Bug 5 — DDL table-name slots didn't `writes_table`** (this is
+handoff-12 §2.2). `drop column from Customers: ` offered every
+table's columns. Same for `rename`/`change column` and the
+relationship qualified-column slots (`from T.col`). Fix
+(`src/dsl/grammar/ddl.rs`): `writes_table: true` on
+`TABLE_NAME_EXISTING` and on `DR_PARENT`/`DR_CHILD`/`AR_PARENT`/
+`AR_CHILD` table idents. The deliberately-documenting test
+`completion::tests::drop_column_from_offers_only_current_table_columns`
+now asserts per-table narrowing. Commit `216e7ba`.
+
+## §3. CRITICAL: open finding needing a user decision
+
+**Partial entry words classify as `DefiniteErrorAt`.** While
+typing the first word of a command — `q`, `qu`, `qui` — the
+input is classified `DefiniteErrorAt(0)`, identical to a
+genuinely unknown command (`frobulate`). Cause: the walker only
+engages once a *complete* registered entry word is present
+(`try_walker_route` returns `None` for `qu`, and the router
+emits the synthetic unknown-command error). So the input pane
+shows `qu` in error-red even though it is a valid prefix of
+`quit`.
+
+Tab-completion still works (`qu` → `quit` is offered), so the
+user can recover. But the red overlay while typing the very
+first word is jarring.
+
+This is documented — not silently asserted as correct — in
+`app_commands::partial_entry_word_classifies_as_definite_error_but_completes`.
+**The next agent (or the user) should decide: is partial-entry
+red overlay acceptable, or should a partial that prefixes a
+registered entry word classify as `IncompleteAtEof`?** A fix
+would need the router to recognise "this token is a prefix of
+≥1 entry word, no other token typed yet" and classify
+accordingly. Not done this session — flagged per CLAUDE.md's
+"escalate, don't decide" rule.
+
+## §4. Honest scope disclosures
+
+Two places where this session's matrix is lighter than
+handoff-12 §1's literal text. Neither was confirmed with the
+user as a deferral, so treat them as open:
+
+**4.1 Cursor positions: meaningful transitions, not every byte
+offset.** Handoff-12 §1's pseudo-code said
+`EVERY_TOKEN_BOUNDARY_AND_MID_TOKEN`. The matrix covers
+end-of-input positions at each meaningful typing transition,
+plus partial-keyword mid-token cases (`insert`, `qu`,
+`create table Customers with pk`). It does *not* exhaustively
+walk every interior byte offset with the cursor. The judgment
+call: end-of-input is where the user actually pauses and reads
+the hint; interior-cursor behaviour is mostly the cursor-split
+rendering logic, already covered by `input_render` tests. If the
+user wants literal every-offset coverage, that is a follow-up.
+
+**4.2 Assertion (5) is parse-layer, not a live dispatch
+differential.** Handoff-12 §1 assertion (5) wanted
+"`parse_command_with_schema` matches dispatch behavior — what
+the grammar accepts equals what `do_insert`/`do_update` accept."
+The matrix records `parse_result` (Ok/Err + command kind) in
+every cell's snapshot, so grammar-acceptance is pinned. It does
+*not* run the actual `db::do_insert` / `do_update` against the
+same input and diff the two. A true differential would need a
+test DB per cell (testcontainers-grade) — heavier. The
+Phase-D-aligned grammar (Form B skips auto-gen, typed slots)
+already mirrors `do_insert`'s `user_cols` contract by
+construction, so the risk is low, but this is a genuine gap
+versus the literal spec.
+
+## §5. Deferred items — status vs. handoff-12 §2
+
+### §2.2 items — addressed this session
+
+- **DDL `writes_table`** — DONE (Bug 5 above).
+- **Add/drop-relationship qualified-column narrowing** — DONE
+  (`writes_table` on the endpoint table idents; the `.col`
+  slots now narrow). Handoff-12 called this "low priority"; it
+  fell out for free once the pattern was established.
+
+### §2.2 items — still open
+
+- **Form C is type-unaware.** Confirmed and pinned by
+  `insert_form_c::form_c_type_unaware_grammar_accepts_decimal_for_int_column`
+  — `insert into Customers (3.14, …)` parses Valid even though
+  `3.14` lands in a non-real column slot; bind time catches it.
+  Handoff-12 said "defer until users care." Still deferred —
+  **the next agent should confirm with the user** whether to
+  make Form C typed (requires Form A/C lookahead disambiguation
+  in the walker).
+- **`value_slot_serial`/`shortid` Form-A-only wording.** The
+  pedagogical "Skipping `id` (auto-generated)…" Prose hint at
+  `insert into T values (` for an auto-gen table is still not
+  added. Unchanged.
+
+### §2.1 carry-forwards — all still open, untouched this session
+
+- `CommandNode.help_id` not consumed.
+- `Box::leak` memory growth per dynamic walk (the matrix's
+  schema-aware walks exercise this more than before, but it's
+  still bounded per walk; no arena yet).
+- **Replay path is schemaless** — note `classify_input` is now
+  schema-aware (Bug 4), but `runtime::run_replay` still calls
+  the schemaless `parse_command`. Per-line schema re-fetch
+  during replay is still unbuilt.
+- HintMode is detection-based, not node-attached.
+- Ranker is scaffolding-only.
+- Dead `parse.token.*` catalog entries.
+
+## §6. New API surface (delta vs. handoff-12)
+
+- `input_render::classify_input_with_schema(input, &SchemaCache)
+  -> InputState` — schema-aware classification.
+  `render_input_runs` uses it. Schemaless `classify_input`
+  retained.
+- `parse.custom.insert_form_a_missing_values` catalog key
+  (placeholder `columns`).
+- Walker `walk_repeated` Incomplete-on-mid-typing semantics
+  (driver.rs) — see Bug 1.
+- `completion_probe` / `expected_at_input` surface
+  `tail_expected` for `ValidationFailed` outcomes — see Bug 3.
+
+## §7. How to take over
+
+1. **Read this file.**
+2. **Read handoff-12, then 11** if you lack prior context.
+3. **Read `CLAUDE.md`** — especially "Out of scope and
+   non-blocking require user confirmation" and "Test-first
+   debugging." This session followed both: every bug was a
+   failing matrix cell first.
+4. **Run `cargo test`** — 989 passing, 0 failing, 1 ignored.
+5. **Run `cargo clippy --all-targets -- -D warnings`** — clean.
+6. **Decide §3** (partial-entry-word red overlay) with the
+   user — it is the one open finding from this session.
+7. **Confirm the §4 scope disclosures** with the user — do they
+   want exhaustive cursor coverage and/or a live dispatch
+   differential, or is the current matrix sufficient?
+8. **Then pick from §5** — Form C typing, the §2.1
+   carry-forwards (`help_id`, arena, schemaless replay,
+   node-attached HintMode).
+
+### Working with the matrix
+
+- The matrix is one test binary. Run a single family with
+  `cargo test --test typing_surface_matrix <family>`.
+- New cells: add a `#[test]` to the relevant
+  `tests/typing_surface/<family>.rs`, call `assess`/`assess_at_end`,
+  assert properties, end with `crate::snap!("<name>", a)`.
+- First run of a new snapshot fails (no baseline). Generate
+  baselines with `INSTA_UPDATE=always cargo test --test
+  typing_surface_matrix <family>`, then review the `.snap`
+  files before committing.
+- When a matrix cell fails after a grammar change: if the new
+  behaviour is correct, update the snapshot; if it's a
+  regression, the snapshot just earned its keep.
+
+## §8. Commit narrative
+
+1. **0b15ce0** — three coupled fixes (walk_repeated rollback,
+   Form C/A disambiguation, ValidationFailed expected-set) that
+   the Form A matrix surfaced.
+2. **24e641b** — matrix infrastructure + Form A (23 cells).
+3. **fffb44f** — schema-aware `classify_input` (Bug 4),
+   surfaced by Form B.
+4. **a9a04cf** — Form B (12 cells).
+5. **37db2f5** — Form C + update + delete (34 cells, no new
+   bugs — the data-mutation family was already well-shaped).
+6. **216e7ba** — DDL `writes_table` fix (Bug 5 / §2.2).
+7. **c7ecc64** — create table + DDL + app coverage (55 cells).
+
+The pattern, again: the bugs all came from the matrix, not the
+pre-existing suite — which is the whole point handoff-12 §1 was
+making.