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

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.

src/completion.rs

@@ -133,6 +133,13 @@ pub enum CandidateKind {
     /// A `--name`-style flag. Coloured with `tok_flag` so the
     /// hint matches the way it'll render in the input pane.
     Flag,
+    /// A single-char punctuation token the walker expects next
+    /// (e.g. `(` at the start of `insert into T (cols)`). Used
+    /// to surface branching alternatives the user might not
+    /// otherwise discover — at `insert into Orders ` the walker
+    /// expects either `values` or `(`, and surfacing both makes
+    /// the Form A path discoverable.
+    Punct,
 }
 
 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -203,6 +210,18 @@ pub fn candidates_at_cursor_with(
     let partial_prefix = input[start..cursor].to_string();
     let leading = &input[..start];
 
+    // When the full input already parses, the cursor is at
+    // the end of a "complete enough" command — but the leading
+    // slice still reports an expected set (the words the user
+    // just finished typing, or optional-suffix continuations).
+    // We let the completion engine run normally, then below
+    // filter out candidates that exactly equal the partial
+    // prefix the user already typed: it's not useful to suggest
+    // `pk` at the end of `create table T with pk`. The
+    // optional-suffix case (`save ` → `as`) is preserved
+    // because there `partial_prefix` is empty.
+    let input_parses_complete = parse_command(input).is_ok();
+
     let expected = expected_at(leading);
     if expected.is_empty() {
         return None;
@@ -310,6 +329,27 @@ pub fn candidates_at_cursor_with(
         .filter(|s| matches_prefix(s))
         .collect();
 
+    // Source 1.7: branching-punct candidates. At positions
+    // where the walker expects a punct character that opens a
+    // sub-shape (notably `(` opening Form A or C of insert),
+    // surface the punct as a Tab candidate so the user
+    // discovers the option. Closing punct (`)`, `,`, etc.) and
+    // expected-after-content punct (`:`, `=`, `.`) are not
+    // surfaced — they're trailing terminals the user types
+    // naturally, not "shape branches" worth advertising in the
+    // hint panel.
+    let punct_candidates: Vec<String> = if partial_prefix.is_empty() {
+        expected
+            .iter()
+            .filter_map(|e| match e {
+                Expectation::Punct('(') => Some("(".to_string()),
+                _ => None,
+            })
+            .collect()
+    } else {
+        Vec::new()
+    };
+
     // Source 2: schema identifiers — accumulated across every
     // matching schema-listable `Ident { source }` expectation.
     // `NewName` / `Types` / `Free` sources don't query the
@@ -337,11 +377,13 @@ pub fn candidates_at_cursor_with(
     // Keywords first (grammar parts read before content),
     // then type names (closed-set grammar — coloured as
     // keywords), then composite literals (`1:n`, …), then
-    // flags (own colour), then schema identifiers.
+    // branching punct (`(` opening a sub-shape), then flags
+    // (own colour), then schema identifiers.
     let mut candidates: Vec<Candidate> = Vec::with_capacity(
         keywords.len()
             + type_names.len()
             + composites.len()
+            + punct_candidates.len()
             + flags.len()
             + identifiers.len(),
     );
@@ -357,6 +399,10 @@ pub fn candidates_at_cursor_with(
         text,
         kind: CandidateKind::Keyword,
     }));
+    candidates.extend(punct_candidates.into_iter().map(|text| Candidate {
+        text,
+        kind: CandidateKind::Punct,
+    }));
     candidates.extend(flags.into_iter().map(|text| Candidate {
         text,
         kind: CandidateKind::Flag,
@@ -370,6 +416,17 @@ pub fn candidates_at_cursor_with(
         return None;
     }
 
+    // When the input is already a valid complete command, drop
+    // candidates that exactly match the partial prefix — those
+    // are the words the user just finished typing (e.g. `pk` in
+    // `create table T with pk`), not useful suggestions. Keeps
+    // schema-narrowing intact (`show data Cu` → `Customers` is
+    // not an exact match; preserved).
+    if input_parses_complete && !partial_prefix.is_empty() {
+        let lowered_partial = partial_prefix.to_lowercase();
+        candidates.retain(|c| c.text.to_lowercase() != lowered_partial);
+    }
+
     let candidates = ranker(candidates);
     if candidates.is_empty() {
         return None;
@@ -959,12 +1016,41 @@ mod tests {
     }
 
     #[test]
-    fn punctuation_expected_does_not_produce_candidates() {
+    fn non_branching_punctuation_is_not_surfaced_as_candidate() {
-        // After `add column to table T` parser expects `:`.
+        // After `add column to table T` the walker expects `:`.
-        // Tab should NOT offer the colon character.
+        // `:` is a "trailing-content" punct — the user types it
+        // naturally as they continue the command, so the hint
+        // panel doesn't surface it. Only branching punct (`(`
+        // opening a sub-shape) becomes a Tab candidate.
         let input = "add column to table T";
         let cs = cands(input, input.len());
-        assert!(cs.is_empty(), "punctuation should not be offered: {cs:?}");
+        assert!(cs.is_empty(), "trailing-content punct should not surface: {cs:?}");
+    }
+
+    #[test]
+    fn open_paren_branching_punct_surfaces_after_insert_into_table() {
+        // After `insert into Orders ` the walker expects either
+        // `values` (Form B) or `(` (Forms A / C). Both surface
+        // as Tab candidates so the user discovers the column-
+        // list form.
+        let cs = cands("insert into Orders ", 19);
+        assert!(cs.contains(&"values".to_string()), "got {cs:?}");
+        assert!(cs.contains(&"(".to_string()), "got {cs:?}");
+    }
+
+    #[test]
+    fn open_paren_candidate_is_classified_as_punct_kind() {
+        // The `(` candidate gets its own kind so the hint
+        // renderer can colour it as punctuation rather than
+        // mis-classifying it as a keyword.
+        let comp = candidates_at_cursor("insert into Orders ", 19, &SchemaCache::default())
+            .expect("some completion");
+        let paren = comp
+            .candidates
+            .iter()
+            .find(|c| c.text == "(")
+            .expect("( present");
+        assert_eq!(paren.kind, CandidateKind::Punct);
     }
 
     #[test]

src/dsl/walker/driver.rs

@@ -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
+            // Inner didn't engage at all — skip the Optional
-            // so the caller's expected-set sees them.
+            // 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
+        NodeWalkResult::Incomplete { position: p, expected } if !inner_committed => {
-        // structural Mismatch). Match chumsky's `or_not`
+            // Inner reported Incomplete without consuming
-        // semantics: roll back any state the partial match
+            // anything — same as NoMatch from the user's
-        // pushed and treat the optional as skipped, carrying
+            // perspective. Roll back and skip.
-        // 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, .. } => {
             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
+        }
     }
 }
 

src/input_render.rs

@@ -673,6 +673,29 @@ mod tests {
         }
     }
 
+    #[test]
+    fn in_progress_form_a_values_list_classifies_as_incomplete_not_definite_error() {
+        // Regression: typing `insert into T (a, b, c) values
+        // (1, 2, 3` (no closing paren yet) used to classify as
+        // DefiniteErrorAt(<values position>) because the
+        // walker's Optional rolled back the partial values
+        // list, leaving the rest of the input as trailing junk
+        // — the renderer then overlaid `values` in red. After
+        // the walk_optional fix (only roll back when the inner
+        // hasn't committed), the Optional propagates Incomplete
+        // and the user sees no error overlay until they submit.
+        assert_eq!(
+            classify_input(
+                "insert into Orders (id, CustId, Total) values (42, 89, 17.59"
+            ),
+            InputState::IncompleteAtEof,
+        );
+        assert_eq!(
+            classify_input("insert into Orders (id, CustId, Total) values (42, 89"),
+            InputState::IncompleteAtEof,
+        );
+    }
+
     #[test]
     fn ambient_hint_at_insert_first_value_mentions_column_name() {
         use crate::dsl::types::Type;

src/ui.rs

@@ -766,6 +766,7 @@ fn render_candidate_line(
             crate::completion::CandidateKind::Keyword => theme.tok_keyword,
             crate::completion::CandidateKind::Identifier => theme.tok_identifier,
             crate::completion::CandidateKind::Flag => theme.tok_flag,
+            crate::completion::CandidateKind::Punct => theme.tok_punct,
         };
         let mut s = Style::default().fg(base_fg);
         if Some(i) == selected {