Completion: per-group separator modes instead of conflict filtering#2169
Merged
curtisman merged 11 commits intomicrosoft:mainfrom Apr 9, 2026
Merged
Completion: per-group separator modes instead of conflict filtering#2169curtisman merged 11 commits intomicrosoft:mainfrom
curtisman merged 11 commits intomicrosoft:mainfrom
Conversation
Move separatorMode from result-level to per-CompletionGroup across the entire completion pipeline (actionGrammar → cache → dispatcher → shell). Problem: A single separatorMode per completion result cannot express 'some completions need a separator, others don't'. When Latin (spacePunctuation) and CJK (optionalSpace) rules produce completions at the same position, the merged mode either hides CJK completions behind a required separator or lets users select completions that produce invalid input. The advance-1 workaround (filterSepConflicts) was complex, lossy, and created artificial re-fetch cycles. Fix: Each CompletionGroup now carries its own separatorMode. The shell partitions groups into SepLevels and shows/hides them independently based on the user's trailing separator state. actionGrammar: - Replace result-level completions/separatorMode with groups[] array of GrammarCompletionGroup (completions + separatorMode per group) - Remove filterSepConflicts, mergeSepMode, computeRangeNeedsSep, PROPERTY_SENTINEL_CHAR, and hasSepConflict/hasTrailingSep/ effectiveTrailingSep fields from CompletionContext - Introduce per-candidate separatorMode grouping in materializeCandidates agentSdk: - Move separatorMode from CompletionResult to CompletionGroup - Remove mergeSeparatorMode two-arg overload from commandHelpers cache: - grammarStore: emit CompletionGroup[] instead of flat completions[]; remove cross-grammar conflict detection/filtering - constructionCache: propagate per-group structure through merge dispatcher: - completion.ts: pass through per-group structure; remove result-level separatorMode handling - Remove separatorMode from DispatcherCompletionResult type shell (partialCompletionSession): - Replace single separatorMode with ItemPartition[] and SepLevel model - Two-anchor system: data-validity anchor + menuSepLevel for trie - loadLevel()/positionMenu()/stripAtLevel() for level-aware trie ops - Decision table in reuseSession: A (validity) → B (sep narrowing) → C (trie matching) → D (exhaustion cascade with widen) Tests updated across all layers; new separatorMode.spec.ts tests for per-group display logic. Adds plan-perGroupSeparatorMode.prompt.md.
…el counts Thread grammar-level spacingMode from GrammarCompletionProperty through the cache layer (CompletionProperty) into the dispatcher, so entity and parameter completions receive context-sensitive separator modes instead of a hardcoded "space". - actionGrammar: add spacingMode to GrammarCompletionProperty; pass it through getGrammarCompletionProperty; export candidateSeparatorMode, requiresSeparator, and GrammarCompletionProperty; remove unused mergeSeparatorMode, hasTrailingSeparator, and isRequiringSepMode. - cache: propagate spacingMode in grammarStore and CompletionProperty. - dispatcher: add completionSeparatorMode() helper; partition parameter completions by computed SeparatorMode per (property, mode) pair. - shell: precompute levelCounts array via setPartitions() to avoid rebuilding arrays on every keystroke; refactor targetLevel and lowestLevelWithItems to use LevelCounts. - Update tests for new separator mode defaults and property assertions.
Instead of the grammar/cache layers eagerly resolving auto-spacing into concrete SeparatorMode values (spacePunctuation or optionalSpacePunctuation) at match time, pass 'autoSpacePunctuation' through as a new SeparatorMode value. The shell renderer resolves it per-item based on the character pair at startIndex. - Add 'autoSpacePunctuation' to SeparatorMode in agentSdk and actionGrammar - Replace computeCandidateSeparatorMode/computeNeedsSep with spacingModeToSeparatorMode in grammarCompletion - Change GrammarCompletionProperty.spacingMode to separatorMode - Update CompletionProperty in cache to use separatorMode instead of spacingMode (removes CompiledSpacingMode dependency) - Simplify collectActionCompletions in dispatcher — pass separatorMode through instead of partitioning per-completion - Inline needsSeparatorInAutoMode in partialCompletionSession.ts to avoid importing action-grammar into the Vite browser bundle - Update toPartitions to resolve autoSpacePunctuation per-item - Update all test expectations accordingly
… guard, docs, import order - Add ResolvedSeparatorMode (excludes autoSpacePunctuation) for processed partitions; toPartitions resolves undefined→space explicitly - targetLevel/lowestLevelWithItems return undefined when no items exist; callers handle the empty case - Add defensive loop guard (menuSepLevel < 2) in reuseSession D1 WIDEN - Move inline needsSeparatorInAutoMode below imports; add SYNC comments cross-referencing grammarMatcher.ts and partialCompletionSession.ts - Update completion.md: remove stale separatorMode from CommandCompletionResult/CompletionGroups, add separatorMode to CompletionGroup table
Remove candidateSeparatorMode() from grammarMatcher and the per-character needsSeparatorInAutoMode logic from constructionCache completion. Both now emit 'autoSpacePunctuation' uniformly, deferring resolution to consumers with richer input context (dispatcher and shell). In the dispatcher, resolve autoSpacePunctuation/spacePunctuation/ optionalSpacePunctuation based on target.separatorMode when grammar matchedPrefixLength is zero or undefined. Also change subcommand completion separatorMode from 'none' to 'optionalSpace' for consistency.
Replace the merged separator-mode model with per-group separator modes. Each CompletionGroup now carries its own separatorMode; the shell's SepLevel model shows/hides groups based on trailing separator state instead of the grammar layer filtering conflicting candidates. - Refactor hasWhitespaceBefore() to inferSeparatorMode(), handling index=0 and returning SeparatorMode directly - NFA completions use autoSpacePunctuation (consumer resolves per-item) - Remove conflict-filtering docs; update invariant microsoft#13 and separator mode table - Simplify toPartitions() — empty partitions are harmless - Add debugCompletion channel to grammarStore
…AutoMode Fill testing gaps for the per-group separator mode pipeline: - Dispatcher: separatorMode override logic when matchedPrefixLength=0, inferSeparatorMode at position 0, new automodetest agent fixture - Shell: autoSpacePunctuation resolution (Latin/CJK/digit pairs), toPartitions multi-group bucketing across SepLevels - Cache: cross-layer separatorMode preservation through merge (all 6 modes) - ActionGrammar: spacingModeToSeparatorMode unit tests Re-export needsSeparatorInAutoMode from action-grammar through agent-dispatcher/helpers/completion so the shell renderer imports the canonical implementation instead of maintaining an inlined copy. - action-grammar: add ./completion subpath export (lightweight, no Node deps) - agent-dispatcher: add ./helpers/completion re-export - shell: replace inlined copy with import from dispatcher helper
…p casts, fix docs, narrow inferSeparatorMode type - Remove dead needsSeparatorInAutoMode + helpers from cache/utils/language.ts (no longer imported after constructionCache switched to action-grammar) - Remove unnecessary 'as unknown as CompiledSpacingMode' casts in spacingModeToSeparatorMode.spec.ts (undefined is a valid union member) - Fix actionGrammar.md auto-spacing doc table: auto mode produces 'autoSpacePunctuation', resolved per-item to spacePunctuation or optionalSpacePunctuation (was incorrectly showing optionalSpace) - Narrow inferSeparatorMode return type to 'optionalSpace' | undefined with clarified comment explaining undefined semantics
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
Refactors the separator mode system across the completion pipeline: moves from a single merged
separatorModeonCommandCompletionResultto per-group separator modes on eachCompletionGroup, with deferred resolution in the shell.Key changes
SeparatorModetype expanded (agentSdk)Added
optionalSpacePunctuation,optionalSpace, andautoSpacePunctuationmodes. EachCompletionGroupnow carries its ownseparatorModefield (optional, defaults to"space").Per-group separator propagation
GrammarCompletionResultreturnsgroups: GrammarCompletionGroup[]instead of flatcompletions: string[]. Each group carries its ownseparatorMode, derived fromspacingModeToSeparatorMode().CompletionResultusesgroups: CompletionGroup[].GrammarStoreandConstructionCacheemit per-mode groups. Property completions carryseparatorMode: "autoSpacePunctuation".CommandCompletionResultdrops the top-levelseparatorModefield.getCommandCompletion()sets per-group modes.requestCompletion()passes separator modes through to agent property completions.inferSeparatorMode()replaceshasWhitespaceBefore().PartialCompletionSessionpartitions items into 3 SepLevels (0=none, 1=space, 2=spacePunc) and shows/hides groups based on what the user typed. Two-anchor model (data validity + trie matching level).connect.tsandinteractive.tscheck per-group separator modes.Removed
mergeSeparatorMode()from agentSdk helpers and grammarMatcherfilterSepConflicts()— ~160 lines of within-grammar conflict detection/filteringgrammarStorecandidateSeparatorMode(),computeRangeNeedsSep(),computeNeedsSep(),mergeSepMode()needsSeparatorInAutoModefromcache/utils/language.ts(moved to actionGrammar, re-exported through dispatcher)New
spacingModeToSeparatorMode()— deterministic mapping from compiled spacing modesaction-grammar/completionsubpath export — lightweight re-export for browser bundlesdispatcher/helpers/completion— re-exportsneedsSeparatorInAutoModespacingModeToSeparatorMode.spec.ts— unit tests for the new mappingArchitecture docs
actionGrammar.mdandcompletion.mdto reflect per-group modelTesting
Extensive test updates across all layers — grammar, cache, dispatcher, and shell completion tests updated or added to validate per-group separator behavior.