Completion suppression gate uses schemaless parse (input_parses_complete) — sibling of the #2 fallback bug #18

New Issue

2026-05-29T11:21:17+01:00

oliversturm commented

2026-05-29 11:21:17 +01:00

(Migrated from github.com)

Observed

src/completion.rs:336:

let input_parses_complete = parse_command_in_mode(input, mode).is_ok();

This gate uses the schemaless parse_command_in_mode, the same function whose schemaless-ness caused #2 (the ambient-hint between-values bug). A schemaless parse can report a wrong-arity / type-mismatched insert as Ok (the type-blind grammar accepts any value count ≥ 1), while the schema-aware parse rejects it. The gate may therefore consider an input "complete" when the schema-aware view says it is incomplete or wrong.

Risk

If input_parses_complete is used to suppress or alter completion candidates, completions may be wrongly suppressed for inputs that only look complete to the type-blind grammar (e.g. insert into Customers values ('Alice') where the table needs more values).

Suggested fix

Thread the schema cache through this gate as #2 did for the ambient-hint fallback — use parse_command_with_schema_in_mode(input, schema, mode) so the completeness judgement matches the schema-aware grammar that the rest of the typing surface uses. Confirm via a test what the gate actually drives before changing (it may be benign, but it is the same latent inconsistency #2 fixed in the hint path).

Context

Spotted during the #2 DA review while auditing all schemaless parse_command_in_mode call sites in the render/completion path. The ambient-hint site (input_render.rs:751) was the #2 fix; this is the remaining sibling.

### Observed `src/completion.rs:336`: ```rust let input_parses_complete = parse_command_in_mode(input, mode).is_ok(); ``` This gate uses the **schemaless** `parse_command_in_mode`, the same function whose schemaless-ness caused #2 (the ambient-hint between-values bug). A schemaless parse can report a wrong-arity / type-mismatched insert as `Ok` (the type-blind grammar accepts any value count ≥ 1), while the schema-aware parse rejects it. The gate may therefore consider an input "complete" when the schema-aware view says it is incomplete or wrong. ### Risk If `input_parses_complete` is used to suppress or alter completion candidates, completions may be wrongly suppressed for inputs that only *look* complete to the type-blind grammar (e.g. `insert into Customers values ('Alice')` where the table needs more values). ### Suggested fix Thread the schema cache through this gate as #2 did for the ambient-hint fallback — use `parse_command_with_schema_in_mode(input, schema, mode)` so the completeness judgement matches the schema-aware grammar that the rest of the typing surface uses. Confirm via a test what the gate actually drives before changing (it may be benign, but it is the same latent inconsistency #2 fixed in the hint path). ### Context Spotted during the #2 DA review while auditing all schemaless `parse_command_in_mode` call sites in the render/completion path. The ambient-hint site (`input_render.rs:751`) was the #2 fix; this is the remaining sibling.

oliversturm commented

2026-05-29 11:38:55 +01:00

(Migrated from github.com)

Closed via 7cccf4e as benign-but-hardened.

Investigation (the ticket asked to confirm what the gate drives before changing):

input_parses_complete (completion.rs:336) drives exactly one thing (completion.rs:780): when the input parses complete and there is a non-empty trailing partial, it drops candidates whose text exactly equals that partial — i.e. don't re-suggest pk at the end of create table T with pk.

The schemaless vs schema-aware parse does diverge (e.g. insert into T values (1, 1, 'x') — int into a bool column: schemaless reports complete, schema-aware rejects). But the divergence can never reach the drop: the drop only ever removes a re-offered keyword equal to the partial, while schema-divergence is confined to value type/arity errors — and those positions never coincide. Probed every plausible co-occurrence (bool/null ident-shaped values; a value type-error followed by a where / RETURNING column partial) and the candidate output was identical with and without the change in every case.

So this is not an observable bug with the current grammar — there is nothing to reproduce, hence no failing-first test.

Disposition: applied the one-line consistency fix anyway (schema-aware gate), matching #2's pattern. It is strictly safe — it can only ever retain a candidate the schemaless gate would have dropped, never the reverse — and future-proofs against the grammar evolving so the divergence becomes reachable. The full suite stays green (2043/0/1, clippy clean). Rationale recorded in a code comment at the call site.

Closed via `7cccf4e` as **benign-but-hardened**. **Investigation (the ticket asked to confirm what the gate drives before changing):** `input_parses_complete` (`completion.rs:336`) drives exactly one thing (`completion.rs:780`): when the input parses complete **and** there is a non-empty trailing partial, it drops candidates whose text *exactly equals* that partial — i.e. don't re-suggest `pk` at the end of `create table T with pk`. The schemaless vs schema-aware parse **does** diverge (e.g. `insert into T values (1, 1, 'x')` — int into a bool column: schemaless reports complete, schema-aware rejects). **But the divergence can never reach the drop:** the drop only ever removes a *re-offered keyword* equal to the partial, while schema-divergence is confined to **value type/arity** errors — and those positions never coincide. Probed every plausible co-occurrence (bool/null ident-shaped values; a value type-error followed by a `where` / `RETURNING` column partial) and the candidate output was identical with and without the change in every case. So this is **not an observable bug** with the current grammar — there is nothing to reproduce, hence no failing-first test. **Disposition:** applied the one-line consistency fix anyway (schema-aware gate), matching #2's pattern. It is strictly safe — it can only ever *retain* a candidate the schemaless gate would have dropped, never the reverse — and future-proofs against the grammar evolving so the divergence becomes reachable. The full suite stays green (2043/0/1, clippy clean). Rationale recorded in a code comment at the call site.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: oli/rdbms-playground#18