Strip trailing separators from matchedPrefixLength; simplify directionSensitive#2100
Merged
curtisman merged 7 commits intomicrosoft:mainfrom Mar 30, 2026
Merged
Strip trailing separators from matchedPrefixLength; simplify directionSensitive#2100curtisman merged 7 commits intomicrosoft:mainfrom
curtisman merged 7 commits intomicrosoft:mainfrom
Conversation
Place P before flex-space (trailing whitespace/punctuation) rather than after it, making wildcard→keyword transitions consistent with keyword→keyword transitions where P stays at the last matched token boundary. - Add stripTrailingSeparators() utility that scans backward from a position to the first non-separator character - Phase B1 (backward): strip trailing separators from partial keyword positions before collecting as fixed candidates - Phase B2 (forward): strip trailing separators from EOI anchor; simplify displace/merge logic since separator-only gaps now collapse naturally (no explicit isSeparatorOnlyGap check needed) - Simplify directionSensitive: remove redundant openWildcard check (every openWildcard path already advances P past minPrefixLength) - Update architecture docs to describe separator stripping behavior - Update test expectations: P decreases by 1, separatorMode changes from 'optional' to 'spacePunctuation' where separator is no longer pre-consumed
- Trie narrowing after wildcard: 'play unknown b' narrows to 'by' - Separator round-trip: 'play unknown ' → backspace → retype space - Double space: 'play unknown ' shows keyword menu - Full wildcard→keyword→entity chain: 'play unknown by' shows artists
directionSensitive means 'would backward at P give different results than forward?' — a property of the position P, not relative to any caller-supplied floor. - Simplify computation: P > 0 (was P != minPrefixLength) - Fix invariant cross-queries (microsoft#5-microsoft#8): pass undefined instead of minPrefixLength to re-invocations — the definition is about unconstrained calls at input[0..P] - Update decision tree in actionGrammar.md and completion.md - Update test expectation: minPrefixLength=5 with P=5 is now directionSensitive=true (P > 0)
Consolidate the repeated computeNeedsSep + mergeSeparatorMode two-step pattern (6 call sites) into a single mergeSepMode function that computes separator need and merges it in one call.
- Fix misleading debug log: 'prefix.length=' → 'P=' in Phase B merge - Add comment clarifying stripTrailingSeparators separator-detection idiom - Add comment explaining why candidate push after updateMaxPrefixLength is valid (completionWord is position-independent) - Hoist WildcardStringRangeCandidate to module scope; compose RangeCandidate from it instead of using inline Extract<> - Fix completion.md: 'multi-word keywords' → 'keywords whose content ends with a separator character' - DRY E2E tests: extract primeWildcard() helper for shared preamble - Add content assertion verifying trie narrows to 'by' in E2E test
- Replace 13 stale 'not direction-sensitive' comments with
'P > 0 → direction-sensitive (backward can back up)' to match
the actual directionSensitive: true assertions
- Clarify position=4 comments as 'raw position=4, stripped to P=3'
to explain the gap between findPartialKeywordInWildcard's raw
result and the final matchedPrefixLength after separator stripping
- Remove stale requiresSeparator trace comments that referenced the
wrong character (prefix at old position, not stripped position)
- Simplify 'Merge at anchor' Phase B2 comment: stripping collapses
separator-only gaps, so the 'OR' clause is not a distinct case
- Replace quoted '"separator already consumed"' override with plain
description ('no position-based override needed')
- Fix 'nothing was consumed' → 'nothing was matched' for consistency
with surrounding terminology
- Clarify 'bounded by maxPrefixLength' → 'stripping stops at
maxPrefixLength to avoid discarding previously matched content'
- Add bridge sentence for openWildcard → directionSensitive: 'Since
P > 0 whenever openWildcard=true, the simplified decision tree
already covers this case'
- Remove unused minPrefixLength parameter from
assertCrossDirectionInvariants; rewrite cross-query comment to
explain the real reason (property of position P, not caller floor)
… separator stripping
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Strip trailing separator characters from
matchedPrefixLength(P) so it alwayslands before the flex-space, aligning wildcard→keyword transitions with
keyword→keyword transitions. Simplify
directionSensitivetoP > 0.Changes
Core:
grammarCompletion.tsstripTrailingSeparatorshelper — scans backward from a position,stripping separator characters down to a lower bound.
advancing
maxPrefixLength, so the backward anchor sits at the lastnon-separator boundary.
decision. The old
isSeparatorOnlyGapcheck in Phase B2 is replaced:stripping collapses separator-only gaps naturally (anchor = maxPrefixLength
→ merge), while non-separator gaps remain above maxPrefixLength (→ displace).
mergeSepModehelper — combines the oldcomputeNeedsSep+mergeSeparatorModetwo-step into a single function, reducing boilerplateat every call site.
directionSensitivesimplified — fromopenWildcard || P != minPrefixLengthtoP > 0.minPrefixLengthis a caller-supplied floor, not a result property.WildcardStringRangeCandidatereplaces inlineExtract<RangeCandidate, {kind: "wildcardString"}>.Tests
matchedPrefixLengthexpectations (−1 for trailing separator nolonger counted) and corresponding
separatorModechanges(
"optional"→"spacePunctuation"where separator is now required).directionSensitive: false→truefor the minPrefixLength floor case.undefinedforminPrefixLength.grammarE2E.spec.ts: wildcard→keyword trienarrowing, backspace round-trip, double-space, wildcard→keyword→entity
pipeline. Extracted
primeWildcard()helper for DRY setup.Documentation
actionGrammar.md— updated Phase B1/B2 descriptions,directionSensitivedecision tree, and design-choice rationale.
completion.md— updatedseparatorMode,directionSensitive, anddecision tree sections.