Skip to content

Simplify directionSensitive computation in grammar and cache completion#2092

Merged
curtisman merged 5 commits intomicrosoft:mainfrom
curtisman:curtism/extract-grammar-completion
Mar 28, 2026
Merged

Simplify directionSensitive computation in grammar and cache completion#2092
curtisman merged 5 commits intomicrosoft:mainfrom
curtisman:curtism/extract-grammar-completion

Conversation

@curtisman
Copy link
Copy Markdown
Member

Summary

Replaces the scattered, mutation-based directionSensitive accumulation in matchGrammarCompletion with a single post-loop decision tree. Also simplifies the cache-layer equivalent in constructionCache.ts.

Key changes

  • constructionCache.ts: Move noTrailingSeparator out of the hot loop into a single post-loop derivation (hasMatchedPart && noTrailingSeparator).
  • tryPartialStringMatch: Rename return field directionSensitivecouldBackUp for clarity (local function shouldn't claim to compute the global property). Add JSDoc.
  • Tests: Update ~20 expected values where openWildcard: true cases now correctly report directionSensitive: true. Remove openWildcard guards from cross-direction invariant checks — invariants are now validated uniformly.
  • Docs: Add forward/backward equivalence analysis tables to actionGrammar.md. Add scope blurbs cross-referencing actionGrammar.mdcompletion.md. Compact wide tables for narrow-context readability.

Design rationale

openWildcard → directionSensitive=true is the correct semantic: wildcard boundaries are genuinely ambiguous. Truncating to input[0..P] removes the wildcard content that established the anchor, so completion(input[0..P], "backward") always diverges. This eliminates partialKeywordAgrees tracking entirely and enables unguarded cross-query invariant checking in tests.

All 419 grammar completion tests pass.

Replace the mutable, incrementally-accumulated directionSensitive flag
with a declarative computation evaluated once after the main loop.

grammarCompletion.ts:
- Remove per-state directionSensitive accumulation and post-loop
  recomputation logic.
- Compute directionSensitive as a single const decision tree based on
  openWildcard, midPosition, partialKeywordAgrees, and
  trailingSepAdvanced.
- Rename tryPartialStringMatch return field from 'directionSensitive'
  to 'couldBackUp' to reflect its local semantics.
- Remove the now-unnecessary partialKeywordAgreesWithForward per-state
  variable (logic moved to Phase B).

testUtils.ts:
- Pass minPrefixLength through to truncated-prefix requeries in
  cross-direction invariant checks instead of hardcoding undefined,
  enabling invariant validation when minPrefixLength is specified.

constructionCache.ts:
- Replace mutable directionSensitive accumulator with a hasMatchedPart
  tracker, computed post-loop as hasMatchedPart && noTrailingSeparator.
…Agrees

Simplify the directionSensitive decision tree: openWildcard positions
are always direction-sensitive because the wildcard boundary is
ambiguous — backward can always reconsider.

Decision tree:
  openWildcard        → true
  P = minPrefixLength → false
  midPosition         → true

This removes partialKeywordAgrees from the codebase (types, candidate
tracking, Phase B accumulation, forward partial keyword block).

Tighten cross-query invariants in testUtils.ts:
- Remove openWildcard guards from #2microsoft#5 (openWildcard → true means
  #2/#3 never fire, microsoft#4/microsoft#5 validate correctly)
- Remove minPrefixLength blanket skip; pass minPrefixLength through
  to cross-queries instead
- 28 test expectations updated: directionSensitive false→true for
  openWildcard cases
- Group trailingSepAdvanced with the directionSensitive decision tree
  under a shared section header for better locality (microsoft#7)
- Add invariant comment: openWildcard requires a preceding keyword
  match so P > 0 whenever openWildcard is true (#1)
- Add JSDoc to couldBackUp return field in tryPartialStringMatch (microsoft#5)
- Compact equivalence analysis tables in actionGrammar.md from 5-column
  wide format to 3-column for readability in narrow contexts (microsoft#6)
@curtisman curtisman enabled auto-merge March 28, 2026 15:59
@curtisman curtisman added this pull request to the merge queue Mar 28, 2026
Merged via the queue into microsoft:main with commit fe3705a Mar 28, 2026
13 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant