oli/rdbms-playground
1
0
Fork 0
Code Issues 4 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

Hint: surface ( as a branching candidate; stop red-flagging in-progress Form A values

Browse Source 
Two related fixes from a user-reported snag:

1. After typing \`insert into Orders \`, the hint suggested only
   \`values\` even though the user could also choose \`(\` to
   open Form A (the explicit-column-list variant). The walker
   reports both \`Expectation::Word("values")\` and
   \`Expectation::Punct('(')\` at that position, but
   \`candidates_at_cursor\` had a blanket "no punctuation as Tab
   candidate" policy.

   Loosened the policy to surface branching punct
   (specifically \`(\` opening a sub-shape). Closing punct
   (\`)\`), separators (\`,\`), and content-trailing punct (\`:\`,
   \`=\`, \`.\`) stay out — the user types those naturally and
   advertising them in the Tab menu is noise. New
   \`CandidateKind::Punct\` so the renderer colors it as punct
   rather than mis-classifying as a keyword.

2. While typing \`insert into Orders (id, CustId, Total) values
   (42, 89, 17.59\` (no closing paren yet), the word \`values\`
   was rendered in \`tok_error\` red. The walker's
   \`Optional(Seq[values, '(', list, ')'])\` was rolling back on
   the partial inner match — treating \`(id, CustId, Total)\` as
   Form C (bare value list) followed by trailing junk starting
   at \`values\`. The classify_input call thus returned
   \`DefiniteErrorAt(<values byte>)\` and the renderer overlaid.

   Tightened \`walk_optional\`: roll back only when the inner
   reports NoMatch (or Incomplete / Mismatch without consuming
   anything). Once the inner has committed to at least one
   terminal (e.g. matched the \`values\` keyword), propagate
   Incomplete / Mismatch up — the user is mid-typing the
   optional's content and rolling back would lose their
   intent.

   The pre-existing chumsky-or_not-style aggressive rollback
   covered cases like \`save Customers\` (Optional(\`as\`)
   inner is a single Word that returns NoMatch without
   consuming, so rollback still fires). Those keep working.

3. Side effect: with \`Optional\` no longer hiding the
   in-progress Form A from the leading slice, the walker on
   \`create table T with \` correctly reports the next-expected
   keyword as \`pk\` — so cursor at the end of the complete
   command \`create table T with pk\` would now re-offer \`pk\`
   as a Tab candidate against the partial \"pk\". Added a final
   filter: when the full input is a valid parse AND the
   partial prefix is non-empty, drop candidates that equal the
   partial exactly. Preserves schema narrowing
   (\`show data Cu\` → \`Customers\` is not an exact match).

Tests:
- New \`in_progress_form_a_values_list_classifies_as_incomplete\`
  asserts the input-state for the user's exact scenario.
- New \`open_paren_branching_punct_surfaces_after_insert_into_table\`
  and \`open_paren_candidate_is_classified_as_punct_kind\` cover
  the punct-as-candidate surface.
- Renamed and rewrote \`punctuation_expected_does_not_produce_candidates\`
  to \`non_branching_punctuation_is_not_surfaced_as_candidate\`
  to document the new finer-grained policy.
- Existing tests for \`save Tab → as\` and the schema-
  narrowing case continue to pass.

Tests: 854 passing, 0 failing, 1 ignored. Clippy clean.
This commit is contained in:
claude@clouddev1
2026-05-15 18:58:28 +00:00
parent b3f1a20652
commit 5815918efb
4 changed files with 142 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/dsl/walker/driver.rs
+27 -14
View File
@@ -693,11 +693,14 @@ fn walk_optional(
) -> NodeWalkResult {
let saved_path_len = path.items.len();
let saved_byte_len = per_byte.len();
match walk_node(source, position, child, ctx, path, per_byte) {
let result = walk_node(source, position, child, ctx, path, per_byte);
let inner_committed = path.items.len() > saved_path_len;
match result {
m @ NodeWalkResult::Matched { .. } => m,
NodeWalkResult::NoMatch { expected, .. } => {
// Skip the optional but carry the inner's expectations
// so the caller's expected-set sees them.
// Inner didn't engage at all — skip the Optional
// but carry the inner's expectations so the caller's
// expected-set sees them.
path.items.truncate(saved_path_len);
per_byte.truncate(saved_byte_len);
NodeWalkResult::Matched {
@@ -705,17 +708,13 @@ fn walk_optional(
skipped: expected,
}
}
// Partial-match failure (mid-shape Incomplete or
// structural Mismatch). Match chumsky's `or_not`
// semantics: roll back any state the partial match
// pushed and treat the optional as skipped, carrying
// the partial's expected-set as `skipped` so callers
// can surface "what would have completed it" at the
// failure point. Validation errors do NOT backtrack —
// those are content failures the user means to fix.
NodeWalkResult::Incomplete { expected, .. } => {
NodeWalkResult::Incomplete { position: p, expected } if !inner_committed => {
// Inner reported Incomplete without consuming
// anything — same as NoMatch from the user's
// perspective. Roll back and skip.
path.items.truncate(saved_path_len);
per_byte.truncate(saved_byte_len);
let _ = p;
NodeWalkResult::Matched {
end: position,
skipped: expected,
@@ -724,7 +723,9 @@ fn walk_optional(
NodeWalkResult::Failed {
kind: FailureKind::Mismatch { expected },
..
} => {
} if !inner_committed => {
// Inner reported Mismatch without consuming
// anything — roll back and skip.
path.items.truncate(saved_path_len);
per_byte.truncate(saved_byte_len);
NodeWalkResult::Matched {
@@ -732,7 +733,19 @@ fn walk_optional(
skipped: expected,
}
}
validation_failure @ NodeWalkResult::Failed { .. } => validation_failure,
// Inner committed (consumed at least one terminal) but
// then ran out / hit a mismatch. Propagate the failure
// up — the user is mid-typing the optional's content and
// we'd lose their intent by rolling back. (Pre-fix
// behavior matched chumsky's `or_not` rollback, but
// that conflates "Form A in progress" with "Form C with
// trailing junk" — see e.g. `insert into T (a, b, c)
// values (1, 2, 3` losing the `values (…)` partial.)
// Validation failures already propagate as a separate
// branch below.
propagated @ (NodeWalkResult::Incomplete { .. } | NodeWalkResult::Failed { .. }) => {
propagated
}
}
}