diff --git a/ts/docs/architecture/completion.md b/ts/docs/architecture/completion.md index b9f0570e3..55ecc5b64 100644 --- a/ts/docs/architecture/completion.md +++ b/ts/docs/architecture/completion.md @@ -273,9 +273,8 @@ lifecycle of a completion interaction. | A7 | Direction changed on direction-sensitive result | Invalidation | Re-fetch | | B4 | Unique match (always fires) | Navigation | Re-fetch next level | | B5 | Separator typed after exact match | Navigation | Re-fetch next level | -| C6 | No trie matches + open set | Discovery | Re-fetch (or slide) | +| C6 | No trie matches | Discovery | Per `noMatchPolicy` | | — | Trie has matches | — | Reuse locally | -| — | No matches + closed set | — | Reuse (menu hidden) | **Key concepts:** @@ -284,6 +283,10 @@ lifecycle of a completion interaction. filter the local trie. - **Separator stripping**: when `separatorMode` requires a separator, the leading separator character in the raw prefix is stripped before trie lookup. +- **`noMatchPolicy`**: computed once from the backend's descriptive fields + (`closedSet`, `openWildcard`) when a result arrives (see `NoMatchPolicy` + below). Drives the A3 and C6 decisions as a simple `switch` instead of + checking two booleans independently. - **Session preservation**: `hide()` cancels in-flight fetches but preserves anchor and menu state for quick re-activation on re-focus. @@ -356,18 +359,20 @@ satisfied. ### `closedSet` -A boolean flowing through the entire pipeline: +A boolean flowing through the backend pipeline (grammar → cache → dispatcher): - **`true`** — completions are exhaustive (finite enum, known subcommands). - When the trie empties, the shell does not re-fetch. - **`false`** — completions may be incomplete (entity values, open-ended - text). When the trie empties, the shell re-fetches to discover more. + text). Merge rule: AND across sources (closed only if _all_ sources are closed). +The shell does not store `closedSet` directly; it is folded into +`noMatchPolicy` (see below). + ### `openWildcard` -A boolean flowing through the entire pipeline, signaling that the completions +A boolean flowing through the backend pipeline, signaling that the completions are offered at a position where a wildcard was finalized at end-of-input. - **`true`** — the wildcard's extent is ambiguous (the user may still be @@ -375,27 +380,52 @@ are offered at a position where a wildcard was finalized at end-of-input. offered as a completion, and `closedSet` correctly describes that keyword set as exhaustive. However, the _position_ of that set is uncertain. - The shell handles this with **anchor sliding**: instead of re-fetching - (which would return the same keyword at a shifted position) or giving up - (stuck when `closedSet=true`), the shell slides the anchor forward to the - current input. The trie and metadata stay intact, so the menu re-appears - at the next word boundary when the user types a separator. - - Recovery is automatic: when the user eventually types the keyword and it - uniquely matches in the trie (trigger B4), the session re-fetches for the - next grammar part. - -- **`false`** — no sliding wildcard boundary; normal `closedSet` semantics - apply. +- **`false`** — no sliding wildcard boundary. Merge rule: OR across sources (open wildcard if _any_ source has one). -Affects triggers A3 and C6 in the re-fetch decision tree: - -- **A3** (non-separator after anchor): when `openWildcard=true`, the anchor - slides forward instead of triggering a re-fetch. -- **C6** (trie empty, closed set): when `openWildcard=true`, the anchor - slides forward instead of staying permanently hidden. +The shell does not store `openWildcard` directly; it is folded into +`noMatchPolicy` (see below). + +### `NoMatchPolicy` (shell-internal) + +Computed once from `closedSet` and `openWildcard` when a backend result +arrives. Controls what the shell does when the local trie has no matches +for the user's typed prefix. + +**Why derive a policy?** The backend returns _descriptive_ metadata — +`closedSet` says whether the completion list is exhaustive, `openWildcard` +says whether the anchor position is ambiguous. These are grammar-level +facts that don't depend on the shell's UI. The shell translates them into +a single actionable policy on arrival, keeping the decision points (A3 +and C6) simple: each is a `switch` on one enum rather than reasoning +about two independent booleans. + +**Why `openWildcard` wins over `closedSet`:** When a wildcard is finalized +at end-of-input, `closedSet` correctly describes the _keyword_ set (e.g. +"by" is exhaustive), but the _position_ of that set is uncertain because +the wildcard extent is ambiguous. Re-fetching would return the same +keywords at a shifted position (wasteful), and `"accept"` would leave the +user stuck (no menu, no re-fetch). Sliding is the only useful action, so +`openWildcard=true` maps to `"slide"` regardless of `closedSet`. + +| Policy | Derived from | Shell action at A3 / C6 | +| ----------- | --------------------------------------- | -------------------------------- | +| `"accept"` | `closedSet=true`, `openWildcard=false` | Reuse (menu hidden, no re-fetch) | +| `"refetch"` | `closedSet=false`, `openWildcard=false` | Re-fetch (backend may know more) | +| `"slide"` | `openWildcard=true` (any `closedSet`) | Slide anchor forward | + +This replaces independent checks on two booleans with a single `switch`: + +- **A3** (non-separator after anchor): `"slide"` slides the anchor forward; + `"accept"` / `"refetch"` trigger a re-fetch. +- **C6** (trie empty): `"slide"` slides the anchor forward; `"accept"` + reuses silently; `"refetch"` re-fetches. + +Anchor sliding preserves the trie and metadata so the menu re-appears at +the next word boundary. Recovery is automatic: when the user eventually +types the keyword and it uniquely matches (trigger B4), the session +re-fetches for the next grammar part. --- diff --git a/ts/packages/shell/src/renderer/src/partialCompletionSession.ts b/ts/packages/shell/src/renderer/src/partialCompletionSession.ts index 79943d618..ce4cebdf9 100644 --- a/ts/packages/shell/src/renderer/src/partialCompletionSession.ts +++ b/ts/packages/shell/src/renderer/src/partialCompletionSession.ts @@ -34,6 +34,29 @@ export interface ICompletionDispatcher { ): Promise; } +// Describes what the shell should do when the local trie has no matches +// for the user's typed prefix. Computed once from the backend's +// descriptive fields (closedSet, openWildcard) when a result arrives, +// then used in reuseSession() decisions. +// +// "accept" — the completion set is exhaustive; no re-fetch can help. +// (Derived from closedSet=true, openWildcard=false.) +// "refetch" — the set is open-ended; the backend may know more. +// (Derived from closedSet=false, openWildcard=false.) +// "slide" — the anchor sits at a sliding wildcard boundary; slide +// it forward instead of re-fetching or giving up. +// (Derived from openWildcard=true, any closedSet.) +type NoMatchPolicy = "accept" | "refetch" | "slide"; + +function computeNoMatchPolicy( + closedSet: boolean, + openWildcard: boolean, +): NoMatchPolicy { + if (openWildcard) return "slide"; + if (closedSet) return "accept"; + return "refetch"; +} + // PartialCompletionSession manages the state machine for command completion. // // States: @@ -42,7 +65,7 @@ export interface ICompletionDispatcher { // ACTIVE anchor !== undefined && completionP === undefined // // Design principles: -// - Completion result fields (separatorMode, closedSet) are stored as-is +// - Completion result fields (separatorMode, etc.) are stored as-is // from the backend response and never mutated as the user keeps typing. // reuseSession() reads them to decide whether to show, hide, or re-fetch. // - reuseSession() makes exactly four kinds of decisions: @@ -56,11 +79,13 @@ export interface ICompletionDispatcher { // entry (and it is not a prefix of any other). Always re-fetches // for the NEXT level's completions — the direction to use for the // re-fetch is determined by the caller. -// - The `closedSet` flag controls the no-match fallthrough: when the trie +// - The `noMatchPolicy` controls the no-match fallthrough: when the trie // has zero matches for the typed prefix: -// closedSet=true → reuse (closed set, nothing else exists) -// closedSet=false → re-fetch (set is open, backend may know more) -// - The anchor is never advanced after a result is received. +// "accept" → nothing else exists, stay quiet +// "refetch" → backend may know more, re-fetch +// "slide" → wildcard boundary, slide anchor forward +// - The anchor is never advanced after a result is received (except +// when noMatchPolicy is "slide", which slides the anchor forward). // When `separatorMode` requires a separator, the separator is stripped // from the raw prefix before being passed to the menu, so the trie // still matches. @@ -75,14 +100,11 @@ export class PartialCompletionSession { // Saved as-is from the last completion result. private separatorMode: SeparatorMode = "space"; - private closedSet: boolean = false; + // Computed from the backend's closedSet + openWildcard fields. + // Controls what happens when the local trie has no matches. + private noMatchPolicy: NoMatchPolicy = "refetch"; // True when completions differ between forward and backward. private directionSensitive: boolean = false; - // True when the completions are offered at a sliding wildcard - // boundary. When set, the shell slides the anchor forward on - // further input instead of re-fetching or giving up, and - // re-shows the menu at every word boundary. - private openWildcard: boolean = false; // Direction used for the last fetch. private lastDirection: CompletionDirection = "forward"; @@ -160,10 +182,10 @@ export class PartialCompletionSession { // UNIQUE — prefix exactly matches one entry and is not a prefix of // any other; re-fetch for the NEXT level (return false). // SHOW — constraints satisfied; update the menu. The final - // return is `this.closedSet || this.menu.isActive()`: - // reuse when the trie still has matches, or when the set - // is closed (nothing new to fetch). Re-fetch only - // when the trie is empty AND the set is open. + // decision uses `noMatchPolicy`: + // reuse when the trie still has matches. When the trie + // is empty: "accept" → reuse, "refetch" → re-fetch, + // "slide" → slide anchor forward. // // Re-fetch triggers (returns false → startNewSession): // @@ -196,9 +218,9 @@ export class PartialCompletionSession { // C. Open-set discovery — trie has zero matches and the set is not // exhaustive; the backend may know about completions not yet loaded. // Gated by closedSet === false. - // 6. Open set, no matches — trie has zero matches for the typed prefix - // AND closedSet is false. The backend may know about - // completions not yet loaded. + // 6. No matches + open set — trie has zero matches for the typed prefix + // AND noMatchPolicy is "refetch". The backend may + // know about completions not yet loaded. private reuseSession( input: string, getPosition: (prefix: string) => SearchMenuPosition | undefined, @@ -217,7 +239,7 @@ export class PartialCompletionSession { } // ACTIVE from here. - const { anchor, separatorMode: sepMode, closedSet } = this; + const { anchor, separatorMode: sepMode, noMatchPolicy } = this; // [A7] Direction changed on a direction-sensitive result. // The loaded completions were computed for the opposite direction @@ -284,21 +306,23 @@ export class PartialCompletionSession { return true; // HIDE+KEEP } if (!separatorRegex(sepMode).test(rawPrefix)) { - // [A3] closedSet is not consulted here: it describes whether - // the completion *entries* are exhaustive, not whether - // the anchor token can extend. The grammar may parse - // the longer input on a completely different path. + // [A3] noMatchPolicy is not consulted for "accept" vs + // "refetch" here: it describes whether the completion + // *entries* are exhaustive, not whether the anchor + // token can extend. The grammar may parse the longer + // input on a completely different path. // - // However, when openWildcard is set, the anchor sits at - // a sliding wildcard boundary — the user is still typing - // within the wildcard, and re-fetching would produce the - // same result at a shifted position. Instead, slide the - // anchor forward to the current input: the trie and - // metadata stay intact, so the menu will re-appear at - // the next word boundary when the user types a separator. - if (this.openWildcard) { + // However, when noMatchPolicy is "slide", the anchor + // sits at a sliding wildcard boundary — the user is + // still typing within the wildcard, and re-fetching + // would produce the same result at a shifted position. + // Instead, slide the anchor forward to the current + // input: the trie and metadata stay intact, so the menu + // will re-appear at the next word boundary when the + // user types a separator. + if (noMatchPolicy === "slide") { debug( - `Partial completion anchor slide (A3): '${anchor}' → '${input}' (openWildcard)`, + `Partial completion anchor slide (A3): '${anchor}' → '${input}' (slide)`, ); this.anchor = input; this.menu.hide(); @@ -361,32 +385,41 @@ export class PartialCompletionSession { // [C6] When the menu is still active (trie has matches) we always // reuse — the loaded completions are still useful. When there are - // NO matches, the decision depends on `closedSet`: - // closedSet=true → the set is closed; the user typed past all - // valid continuations, so re-fetching won't help. - // closedSet=false → the set is NOT closed; the user may have - // typed something valid that wasn't loaded, so - // re-fetch with the longer input (open-set discovery). - // - // Special case: when openWildcard is set and the trie is empty, - // the user is still typing within the wildcard. Slide the anchor - // forward instead of re-fetching (wasteful, same result) or - // giving up (stuck). The trie stays intact so the menu will - // re-appear at the next word boundary. + // NO matches, the decision depends on `noMatchPolicy`: + // "accept" → the set is exhaustive; the user typed past all + // valid continuations, so re-fetching won't help. + // "refetch" → the set is open-ended; the user may have typed + // something valid that wasn't loaded, so re-fetch + // with the longer input (open-set discovery). + // "slide" → the anchor sits at a sliding wildcard boundary; + // slide forward instead of re-fetching (wasteful, + // same result) or giving up (stuck). The trie + // stays intact so the menu will re-appear at the + // next word boundary. const active = this.menu.isActive(); - if (!active && this.openWildcard) { - debug( - `Partial completion anchor slide (C6): '${anchor}' → '${input}' (openWildcard)`, - ); - this.anchor = input; - this.menu.hide(); - return true; + if (!active) { + switch (noMatchPolicy) { + case "slide": + debug( + `Partial completion anchor slide (C6): '${anchor}' → '${input}' (slide)`, + ); + this.anchor = input; + this.menu.hide(); + return true; + case "accept": + debug( + `Partial completion reuse: noMatchPolicy=accept, menuActive=false`, + ); + return true; + case "refetch": + debug( + `Partial completion re-fetch: noMatchPolicy=refetch, menuActive=false`, + ); + return false; + } } - const reuse = closedSet || active; - debug( - `Partial completion ${reuse ? "reuse" : "re-fetch"}: closedSet=${closedSet}, menuActive=${active}`, - ); - return reuse; + debug(`Partial completion reuse: menuActive=true`); + return true; } // Start a new completion session: issue backend request and process result. @@ -400,7 +433,7 @@ export class PartialCompletionSession { this.menu.setChoices([]); this.anchor = input; this.separatorMode = "space"; - this.closedSet = false; + this.noMatchPolicy = "refetch"; const completionP = this.dispatcher.getCommandCompletion( input, direction, @@ -417,9 +450,11 @@ export class PartialCompletionSession { debug(`Partial completion result: `, result); this.separatorMode = result.separatorMode ?? "space"; - this.closedSet = result.closedSet; + this.noMatchPolicy = computeNoMatchPolicy( + result.closedSet, + result.openWildcard, + ); this.directionSensitive = result.directionSensitive; - this.openWildcard = result.openWildcard; this.lastDirection = direction; const completions = toMenuItems(result.completions); @@ -430,9 +465,9 @@ export class PartialCompletionSession { ); // Keep anchor at the full input so the anchor // covers the entire typed text. The menu stays empty, - // so reuseSession()'s SHOW path will use `closedSet` to - // decide: closedSet=true → reuse (nothing more exists); - // closedSet=false → re-fetch when new input arrives. + // so reuseSession()'s SHOW path will use noMatchPolicy + // to decide: "accept" → reuse (nothing more exists); + // "refetch" → re-fetch when new input arrives. // // Override separatorMode: with no completions, there is // nothing to separate from, so the separator check in