walker+completion: surface list trailing-optionals + identifiers-first ordering (ADR-0022 Amendment 2)
walk_repeated discarded the last matched item's trailing-optional expectations at a clean item boundary, so a comma-separated list offered no continuation after a complete item: `order by Name ` gave no asc/desc, `select Name ` no `as`, `create table … Code(text) ` no not/unique/default/check. Capture the last item's skipped set and surface it when the list ends at an item boundary (the separator `,` itself is deliberately not surfaced). That fix made expression-position candidate lists long, which exposed a visibility problem: the hint panel's candidate line is single-row and window-scrolls on overflow, centring on item 0 when nothing is selected — so with keywords-first, schema identifiers scrolled off behind the `>` marker. Reverse the ordering: schema identifiers (table/column/relationship names) now sort before keywords, since a name the user would have to look up is the highest-value completion and must stay visible (keywords are learned over time; the tok_identifier/tok_keyword colour split marks the boundary). This reverses the handoff-14 keywords-first call, now recorded in ADR-0022 Amendment 2. Tests: walker expected-set + completion-layer regressions for the trailing-optionals and the ordering; candidate_ordering.rs header invariant inverted; ~20 typing-surface snapshots re-baselined; a two-line hint box recorded as a deferred follow-up.
This commit is contained in:
@@ -480,6 +480,75 @@ re-baseline those snapshots. New snapshots cover:
|
||||
The snapshots are the regression net for "did we change the
|
||||
visual output unexpectedly".
|
||||
|
||||
## Amendment 2 — Candidate ordering: schema identifiers before keywords (2026-05-21)
|
||||
|
||||
This amendment **reverses the candidate-ordering call made in the
|
||||
handoff-14 ranker discussion** (keywords before schema
|
||||
identifiers). That call was never recorded in an ADR — it lived
|
||||
only in `tests/typing_surface/candidate_ordering.rs` — so this
|
||||
amendment also gives the ordering a decision record.
|
||||
|
||||
### The obsolete premise
|
||||
|
||||
Handoff-14 ordered command-part keywords before schema
|
||||
identifiers on the rationale that "grammar parts are read before
|
||||
the content that fills them," so `add column to table T` reads in
|
||||
order. That held while candidate lists were short. The SQL surface
|
||||
(ADR-0030/0031/0032/0033) made lists long — an expression position
|
||||
such as `where Name ` or `order by ` legitimately offers the
|
||||
column names *plus* the full expression-continuation keyword run
|
||||
(`is not like between in and or`, plus `asc`/`desc` in ORDER BY).
|
||||
|
||||
The hint panel's candidate line is **single-row and
|
||||
window-scrolled** (`render_candidate_line`): when it overflows it
|
||||
centres on the selected item, or on item 0 when nothing is
|
||||
selected (the ambient, just-typed state). With keywords first, the
|
||||
schema identifiers sat at the tail and scrolled off behind the `>`
|
||||
marker — invisible until the user Tab-cycled to them.
|
||||
|
||||
### The decision
|
||||
|
||||
Schema identifiers (table / column / relationship names) now sort
|
||||
**before** keywords in the candidate list. A name the user would
|
||||
otherwise have to look up is the highest-value completion —
|
||||
valuable even to experts, who come to know the keywords over time —
|
||||
so it must stay visible by default. Within each section the prior
|
||||
rules are unchanged: identifiers alphabetised; keywords in
|
||||
grammar-declaration order (`to` before `table`); then type names,
|
||||
composite literals, branching punct, flags. The existing colour
|
||||
split (`tok_identifier` teal vs `tok_keyword` purple, §colour)
|
||||
makes the section boundary legible once both are on screen.
|
||||
|
||||
### Related fix — Repeated trailing optionals
|
||||
|
||||
This amendment shipped alongside a `walk_repeated` fix: a
|
||||
comma-separated list (`Repeated`) was discarding the last matched
|
||||
item's trailing-optional expectations at a clean item boundary, so
|
||||
`order by Name ` offered no `asc`/`desc`, `select Name ` no `as`,
|
||||
and `create table … Code(text) ` no `not`/`unique`/`default`/
|
||||
`check`. Those now surface (the separator `,` itself is
|
||||
deliberately not surfaced). This is what made identifier
|
||||
visibility pressing — the lists these positions produce are now
|
||||
both correct and long.
|
||||
|
||||
### Deferred — two-line hint box
|
||||
|
||||
As hint lists grow, a **two-line candidate box** (more candidates
|
||||
visible without scrolling) is worth considering. Deferred for now
|
||||
on screen-space grounds; recorded so it is not lost.
|
||||
|
||||
### Coverage
|
||||
|
||||
`tests/typing_surface/candidate_ordering.rs` rewritten to assert
|
||||
identifiers precede keywords (header invariant #2 inverted; the
|
||||
`to`-before-`table` keyword-order invariant #1 retained).
|
||||
`completion::tests::identifiers_come_before_keywords_in_grammar_order`
|
||||
and `identifiers_precede_keywords_at_expression_position` lock the
|
||||
ordering; `order_by_after_sort_item_offers_direction`,
|
||||
`projection_after_item_offers_alias_keyword`, and
|
||||
`create_table_after_column_spec_offers_constraints` lock the
|
||||
trailing-optional fix. ~20 typing-surface snapshots re-baselined.
|
||||
|
||||
## Out of scope
|
||||
|
||||
Deliberately deferred to keep this ADR shippable as a single
|
||||
|
||||
+1
-1
@@ -27,7 +27,7 @@ This directory contains the project's ADRs, recorded per
|
||||
- [ADR-0019 — Friendly error layer (H1) and i18n message catalog](0019-friendly-error-layer-and-i18n.md)
|
||||
- [ADR-0020 — Tokenization layer for the DSL parser](0020-tokenization-layer-for-the-dsl-parser.md)
|
||||
- [ADR-0021 — Parser-as-source-of-truth for H1a (per-command usage in parse errors)](0021-parser-as-source-of-truth-for-h1a.md)
|
||||
- [ADR-0022 — Ambient typing assistance: colour, hint panel, completion (I3 + I4)](0022-ambient-typing-assistance.md) — **Amendment 1 supersedes §12's simple-mode-only carve-out**: the unified mode-aware walker (ADR-0030/0031/0032) now speaks SQL, so advanced-mode ambient assistance is re-enabled. `ambient_hint_in_mode` + `hint_resolution_at_input_in_mode` + `expected_for_hint_snapshot` thread `Mode`; `render_hint_panel` calls ambient for all modes (no more advanced-mode `None`); the one-shot `:` sigil is stripped before the ambient walk. Fixes a live bug where advanced-mode SQL hinting/completion-preview were dead despite Phase 2 marking them green (validated at the engine layer, not the UI). Simple-mode gating, highlighting, and the §13 performance posture are unchanged; covered by an app-level render test plus ambient-layer regression locks
|
||||
- [ADR-0022 — Ambient typing assistance: colour, hint panel, completion (I3 + I4)](0022-ambient-typing-assistance.md) — **Amendment 1 supersedes §12's simple-mode-only carve-out**: the unified mode-aware walker (ADR-0030/0031/0032) now speaks SQL, so advanced-mode ambient assistance is re-enabled. `ambient_hint_in_mode` + `hint_resolution_at_input_in_mode` + `expected_for_hint_snapshot` thread `Mode`; `render_hint_panel` calls ambient for all modes (no more advanced-mode `None`); the one-shot `:` sigil is stripped before the ambient walk. Fixes a live bug where advanced-mode SQL hinting/completion-preview were dead despite Phase 2 marking them green (validated at the engine layer, not the UI). Simple-mode gating, highlighting, and the §13 performance posture are unchanged; covered by an app-level render test plus ambient-layer regression locks; **Amendment 2 reverses the handoff-14 keywords-first candidate ordering** — schema identifiers (table/column/relationship names) now sort *before* keywords so a name the user would have to look up stays visible in the single-row, window-scrolled candidate line (keywords are learned over time; the `tok_identifier`/`tok_keyword` colour split marks the boundary); shipped with a `walk_repeated` fix that surfaces a list item's trailing optionals at a clean boundary (`order by Name ` → `asc`/`desc`, `select Name ` → `as`, `create table … Code(text) ` → `not`/`unique`/`default`/`check`; the `,` separator deliberately not surfaced); records a deferred two-line hint box for growing lists
|
||||
- [ADR-0023 — Unified declarative grammar tree](0023-unified-grammar-tree.md) — direction (superseded for execution detail by ADR-0024)
|
||||
- [ADR-0024 — Unified grammar tree: execution plan](0024-unified-grammar-tree-execution-plan.md) — **Accepted**, the executable spec — implemented (Phases A–F; Phase F shipped "minimal", `parser.rs` retained as the router — see the ADR's Phase F implementation note)
|
||||
- [ADR-0025 — Indexes](0025-indexes.md) — **Accepted**, `add index` / `drop index`, persistence, rebuild-table preservation, and items-list display (`C3` index portion + `S2`)
|
||||
|
||||
Reference in New Issue
Block a user