fix(autocomplete): suggest interactive option names by autocarl · Pull Request #50 · yllibed/repl

autocarl · 2026-07-08T21:30:08Z

Summary

Add interactive autocomplete option candidates for --/partial --flag tokens.
Reuse the same option sources as shell completion: static/global output options, custom global options, and route OptionSchema.KnownTokens.
Add regression coverage for route/global option suggestions and partial option-prefix filtering.

Fixes #43

Test Plan

dotnet test src/Repl.Tests/Repl.Tests.csproj -c Release --filter 'FullyQualifiedName~Given_InteractiveAutocomplete_OptionCandidates'
dotnet test src/Repl.Tests/Repl.Tests.csproj -c Release --filter 'FullyQualifiedName~Given_InteractiveAutocomplete'
dotnet test src/Repl.IntegrationTests/Repl.IntegrationTests.csproj -c Release --filter 'FullyQualifiedName~Given_Completions'
dotnet test src/Repl.slnx -c Release

Add interactive autocomplete option candidates from global and route option sources so --flag prefixes behave like shell completion.\n\nFixes yllibed#43

carldebilly · 2026-07-08T21:33:49Z

@codex please review

…parser semantics Review-panel round on the initial commit — all three field repros fixed red-first, plus the structural findings: - One option-candidate source (OptionTokenCompletionSource) now feeds both the interactive engine and the shell-completion engine: the previous copy-pasted lists could only drift, and a new global option had to be added twice. Composed tokens (--alias, --output:format) only allocate when they can match the typed prefix. - Prior tokens are normalized through InvocationOptionParser exactly like the shell path: option VALUES are consumed ("--channel beta" no longer leaves a stray positional that hides route options) and the bare "--" end-of-options separator suppresses option suggestions past it. - The committed command prefix goes through the router''s own ResolveUniquePrefixes expansion, and route-prefix matching mirrors RouteResolver''s terminal-segment alias rule: alias and unique-prefix invocations ("i bib-overalls") now surface the route''s options. - Route options are offered as soon as the command WORDS are typed (only dynamic argument segments may be missing): "install --" suggests --force, matching the parser''s options-anywhere acceptance. - IsOptionPrefixToken gates suggestions: a single dash surfaces short aliases (-f) while signed numeric literals (-42) stay positional, mirroring the invocation parser; token classification uses the same predicate so short flags no longer paint as invalid. - Dynamic completion providers are skipped when the current token is an option prefix — provider values are parameter values, not option names. - OptionSchema.KnownTokens is materialized once (was a recomputed LINQ projection hit per keystroke per route). - Tests: regression coverage for every repro above (valued option, alias, single dash, missing positionals, end-of-options, negative number, provider pollution, cross-source dedup); neutral test data.

carldebilly · 2026-07-09T04:22:29Z

Review round on the initial commit (local multi-perspective review + field repros) — fixes pushed in dc6997b, every behavioral fix red-first. Full suite green (1091 tests).

Fixed

Copy-paste vs reuse: the option-candidate enumeration was a line-for-line clone of the shell-completion engine's. Both engines now consume one shared source (OptionTokenCompletionSource), so the two surfaces cannot drift and a new global option is added once. Composed tokens (--alias, --output:format) only allocate when they can match the typed prefix.
Valued options broke route matching (install x --channel beta -- lost --force): prior tokens are now normalized through InvocationOptionParser exactly like the shell path — option values are consumed, and a bare -- (POSIX end-of-options) suppresses option suggestions past it.
Aliases/unique prefixes (i x --): the committed prefix goes through the router's own ResolveUniquePrefixes expansion, and route-prefix matching mirrors RouteResolver's terminal-segment alias rule — autocomplete now agrees with what would execute.
Options before positionals: install -- suggests --force; route options appear as soon as the command words are typed (only dynamic segments may be missing), matching the parser's options-anywhere acceptance.
Single-dash aliases: a new IsOptionPrefixToken gate surfaces -f-style aliases from -, while signed numeric literals (-42) stay positional (same rule as the parser). Token classification uses the same predicate, so short flags no longer paint as invalid.
Provider pollution: dynamic completion providers are skipped when the current token is an option prefix.
Perf: OptionSchema.KnownTokens materialized once (was a recomputed LINQ projection per keystroke per route); hot-path LINQ/interpolations removed; dead token.Length >= 2 dropped.
Tests: regression coverage for each case above + cross-source dedup pin; neutral test data (public repo).

Considered, deliberately unchanged

Quoted option-like tokens ("--channel") are treated as options — identical to what execution does after tokenization, so autocomplete stays consistent with the parser rather than diverging.
Interactive shows parameter placeholders on an empty token (options appear from the first -), while shell completion lists options on empty input after a terminal route — an intentional UX difference between a live menu and a dump-style completion list.
The token-classification pass predates this PR and rebuilds prefixes per token (quadratic on very long pasted lines, hosted sessions included), and suggestion rendering does not strip control characters from the raw prefix; both are pre-existing surfaces worth their own issue rather than a rider on this PR.

Option suggestions share AutocompleteSuggestionKind.Parameter, so the live hint could shortcut to "Param skillName" while the menu listed option names. In option-prefix mode the parameter shortcut is skipped and the hint renders the option alternatives.

…eview round 2) - SECURITY: both completion engines parsed prior tokens with the legacy parser profile, whose AllowResponseFiles default meant a @file token triggered server-side file reads (UNC probes included) — per keystroke on remote-controlled lines in hosted sessions. A shared CompletionParsingOptions profile (response files off) now feeds both engines; the execution pipeline already gated expansion on !isInteractiveSession for the same reason. - Single-dash short flags are dropped from the prefix only BEFORE the POSIX "--" separator; after it they are positional arguments and keep their place in the route prefix (exact-route providers fire again). - The shell gate is now the same IsOptionPrefixToken as the interactive menu: "-" surfaces short aliases in shell completion too, and two parity tests pin interactive == shell candidates for "--" and "-" inputs — a divergent reimplementation on either side now fails. - Unique-prefix/alias expansion is bounded by the deepest route/context template: the per-index candidate derivation no longer scales with the token count of an adversarial line; the misleading "menu shows the competing alternatives" comment now states what really happens to an ambiguous committed shorthand. - The autocomplete literal/alias rule delegates to RouteResolver.IsLiteralMatch (comparison parameterized) instead of mirroring it; ReplCaseSensitivity.ToStringComparison() replaces the ternary re-derived at every call site; GlobalOptionParser reuses the shared IsSignedNumericLiteral. - docs/shell-completion.md documents the two deliberate divergences that remain (empty-token options, enum values).

carldebilly · 2026-07-09T05:03:36Z

Second review round on a1eaafa (multi-perspective panel) — fixes pushed in 168d644, red-first throughout. Full suite green (1096 tests).

Fixed

Security (the round's blocker): both completion engines normalized prior tokens with the legacy parser profile, whose AllowResponseFiles default meant a @file prior token triggered server-side File.Exists/File.ReadAllText — per keystroke on remote-controlled lines in hosted sessions (UNC probes / NTLM leak vector on Windows), the very exposure the execution pipeline gates with !isInteractiveSession. A shared CompletionParsingOptions (response files off) now feeds both engines; pinned by a test proving a real @file prior token is never expanded.
Post--- positionals: single-dash flags are dropped from the route prefix only before the POSIX separator; after it they are positionals and keep their segment position (red observed by guard inversion).
Parity pinned: two tests run BOTH engines on the same app and assert identical candidates for -- and - inputs — the shared token source was only half the guarantee, the gates now agree too (shell adopts IsOptionPrefixToken, so -f works in shell completion as well).
Bounded expansion: unique-prefix/alias expansion is capped at the deepest route/context template — per-keystroke cost no longer scales with the token count of an adversarial line; the misleading "menu shows the competing alternatives" comment now states the real behavior of an ambiguous committed shorthand.
No more mirrors: the autocomplete literal/alias rule delegates to RouteResolver.IsLiteralMatch (comparison parameterized); ReplCaseSensitivity.ToStringComparison() replaces five hand-rolled ternaries; GlobalOptionParser reuses the shared IsSignedNumericLiteral.
docs/shell-completion.md documents the two deliberate divergences that remain (empty-token options, enum values).

Analyzed, deliberately unchanged

The permissive parser's greedy value consumption (e.g. config --verbose set swallowing set) is more permissive than execution, not less: at execution time an undeclared option interposed before the end of the command words breaks route resolution too (only GlobalOptionParser knows arities, and only for declared global options). Completion stays consistent with both the shell path and execution's outcome.
The three case-sensitivity regimes (router hardcodes OrdinalIgnoreCase, Autocomplete.CaseSensitive, Parsing.OptionCaseSensitivity) predate this PR and span the whole routing surface — worth a dedicated issue rather than a rider here.

…guess (review round 3) Prior-token normalization no longer guesses option arities with the generic parser; it mirrors execution's resolve-then-parse order: - The command prefix is built by stripping global options with the parser that knows their arities (GlobalOptionParser: --no-logo takes no value, --output: does), then resolving the route. A matched route contributes exactly its own segments and leaves every route option in its trailing tokens — so a valued short alias (-c beta) or a valueless global flag before the command can never corrupt the prefix. - Route options are sourced from the single route RouteResolver would actually select (fixes overloaded routes offering the wrong overload's options) and only when every required positional is filled (a resolved match with no leftover positional) — accepting a suggestion always yields a command that runs cleanly. Per maintainer decision, options wait until required arguments are satisfied. - The dynamic completion provider is skipped for an option-prefix token only before the POSIX "--" separator; after it a dash token is a positional and the provider runs. - Shell completion mirrors all of the above (route-first prefix, "--" termination state, valued short-alias handling), keeping the two surfaces consistent. Every behavioral fix is covered by a red-first regression test; full suite green (1102).

…case (review round 4) Removes the last divergences from execution's resolve-then-parse order: - BuildCommandPrefix no longer strips the bare "--" separator or drops dash-prefixed tokens before resolving. Route resolution runs before option parsing (as CommittedInputResolver does), so "--" and short tokens like "-prod" bind to route segments as positional values — "remote -prod status" / "remote -- status" now keep the terminal literal suggestion. The no-match fallback keeps the tokens as typed instead of second-guessing which are options. - Shell completion mirrors the same: "--" only records the end-of-options state, tokens stay as typed for route resolution. - Route-option candidates are filtered against OptionSchema.Entries with each entry's own effective case sensitivity, not the flattened KnownTokens under one global comparison — a case-insensitive option is offered for a differently-cased prefix even under a case-sensitive global default (pinned via the shared source, since a per-option override is otherwise not expressible through the attribute). - Shell enum-value completion recognizes short option aliases: a pending option is detected with the single-dash gate, so "-m <TAB>" offers the enum values like "--mode" does. All red-first; full suite green (1106).

carldebilly · 2026-07-10T04:20:38Z

@codex please review again

chatgpt-codex-connector · 2026-07-10T04:27:05Z

Codex Review: Didn't find any major issues. 🎉

Reviewed commit: ae605dbb7d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

…ion, and shell (review round 5) Replaces the per-site token normalization — each of which diverged from the executor differently — with a single ResolveCommitted pass computed once against the captured graph. Completion, token classification, and shell completion all derive the command prefix, terminal route, option boundary, and end-of-options state from that one result, so they cannot drift from each other or from execution. Findings addressed: - Route options wait until ALL positionals (required AND optional) are filled: "run --" on "run {profile?}" no longer offers --force (it would bind to {profile}); "run x --" does. - The bare "--" only terminates options when it lands among the route's trailing option tokens; a "--" consumed as a positional segment value ("remote -- status --") no longer suppresses option candidates. - Token classification is driven by the same resolution: a dash token bound to a segment ("remote -prod status") classifies as its positional and the later literal keeps its segment index. - Output-format aliases are matched case-insensitively (OutputOptions. Aliases is a case-insensitive dictionary), so "--J" offers "--json" even under a case-sensitive global default. - Shell completion expands unique command prefixes ("i" -> "install") and derives the pending option from the matched route's trailing tokens, so enum values are not offered for a dash token routing bound as a positional ("deploy -m"). - Route resolution uses the captured graph everywhere (app.Resolve(..., activeGraph.Routes)) — no mid-pass graph swap. All red-first; full suite green (1112).

carldebilly · 2026-07-10T13:47:25Z

@codex please review again

chatgpt-codex-connector · 2026-07-10T13:53:03Z

Codex Review: Didn't find any major issues. 👍

Reviewed commit: f82f495ec9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

…al/case edges (review round 6) - PERF (P1 regression from round 5): token classification resolved the whole prefix once per token. It now resolves ONCE per pass and derives every token's role from that single result — flat per-keystroke cost instead of O(tokens) resolves (the hosted remote-controlled path). - Cache poisoning (P1): completion evaluated the routing graph and wrote it into the durable cache. Because module presence can depend on the invocation's globals (a `--env` gate) that are not part of the cache key, a committed valid command could then read the completion-time, module-absent graph and fail as Unknown. Completion now resolves with useDurableCache:false — it neither reads nor writes the shared cache. - Leading globals (P2 regression from round 5): a valid global option before the command (`--no-logo show`) was classified Invalid. Global tokens are now identified up front (via GlobalOptionParser) and always classified as options, independent of the route-option region. - Output transformer names are matched case-insensitively too (`--output:J` -> `--output:json`), matching the case-insensitive transformer dictionary — completing the round-4 alias fix. - Case-distinct executable aliases are preserved under case-sensitive option parsing: shell dedupes option candidates with the parser's case semantics, and the interactive UI dedupe keeps option tokens case-sensitively, so `-m` and `-M` both survive on both surfaces. All red-first (the cache-poisoning red observed by re-enabling the cache); full suite green (1117).

…y, pending values (review round 7) - Classification separator is per-position, not whole-input: a later "--" no longer retroactively classifies an earlier "--force" as Invalid (round-6 regression). A running separatorSeen flag flips only at the "--" in the trailing option region. - Global-option tokens are marked from GlobalOptionParser's exact consumed INDICES (new RemainingTokenIndices), not string-value matching — so "--tenant show show" marks the second "show" as the command and the consumed value "show" as the option, instead of mixing them up (round-6 limitation). - Pending valued options suppress misleading completions: when the last committed token is a valued route option ("run --channel -") or a valued custom global ("--tenant "), the current token is that option's value, so no command or option-name candidates are offered (accepting one would misparse). All red-first; full suite green (1121). The deeper completion/execution graph-under-globals parity (module presence evaluated under the line's parsed globals, and durable-cache read consistency) is tracked separately — it is a routing-cache design concern beyond option-name completion.

carldebilly · 2026-07-10T21:08:54Z

@codex review

…sult-flow edges (review round 8) - Pending valued option no longer blanks the menu: it keeps the option's own value provider (WithCompletion / enum values) and suppresses only command and option-NAME candidates (regression from round 7's return []). - Result-flow options are offered ("--res" -> --result:page-size|cursor| pager|all) and their valued suboptions are treated as pending. - Global-pending only when the current token is not option-like (the parser skips a dash-prefixed follower): "--tenant --" still offers options; and it resolves through the parser's own token map, so a colliding alias picks the same definition the parser would (respecting bool arity). - Route-option pending honors the POSIX "--" separator: after it a token is positional, not a value awaiting option. - No subcommand is offered once a terminal route carries trailing option tokens (a bool flag's zero-or-one arity would otherwise swallow the suggested word). - Shell completion mirrors all of the above (pending route/global, no-subcommand-after-option). - Strengthened the valued-global pending test to use a partial command token (the previous one passed vacuously with a fully-typed word). All red-first; full suite green (1131). The classification/global test setups were corrected after confirming custom globals are consumed in completion (empty results were cursor-at-end-of-complete-word, not a bug).

autocarl

Final re-review — verdict: ship

Reviewed on d8aec52. The material completion/execution parity regressions reported in the previous rounds are fixed, including pending route/global values across interactive and shell completion, preservation of option-target value completion, result-flow options, POSIX --, Boolean/subcommand handling, and parser-consistent global resolution.

Validation:

Full solution: 1,131 passed / 1 external-toolchain skip / 0 failed
Focused round-8 regressions: 5/5 passed
GitHub CI is green on Ubuntu, macOS, build/test/pack, documentation, and real-shell smoke tests
git diff --check passed; worktree clean

The remaining open autocomplete observations are edge cases or follow-up design work rather than blockers for issue #43. In particular, selecting a completion provider by its exact target should replace the pre-existing single-provider heuristic in a focused follow-up; routing-graph/cache semantics remain tracked separately in #53.

I would merge this PR.

fix(autocomplete): suggest interactive option names

f710d7c

Add interactive autocomplete option candidates from global and route option sources so --flag prefixes behave like shell completion.\n\nFixes yllibed#43

autocarl marked this pull request as ready for review July 8, 2026 21:32

This comment was marked as resolved.

Sign in to view

carldebilly approved these changes Jul 9, 2026

View reviewed changes

This comment was marked as resolved.

Sign in to view

carldebilly mentioned this pull request Jul 10, 2026

Completion evaluates the routing graph inconsistently with committed execution (globals + durable-cache read) #53

Open

This comment was marked as resolved.

Sign in to view

carldebilly mentioned this pull request Jul 11, 2026

Completion suggestion vs execution parity: option-value & option-region edges (follow-ups from #50) #54

Closed

autocarl commented Jul 11, 2026

View reviewed changes

carldebilly approved these changes Jul 11, 2026

View reviewed changes

carldebilly merged commit 41202c6 into yllibed:main Jul 11, 2026
6 checks passed

carldebilly mentioned this pull request Jul 11, 2026

fix(autocomplete): option-value & option-region completion parity edges #55

Merged

Uh oh!

Conversation

autocarl commented Jul 8, 2026

Summary

Test Plan

Uh oh!

carldebilly commented Jul 8, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

carldebilly commented Jul 9, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

carldebilly commented Jul 9, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

carldebilly commented Jul 10, 2026

Uh oh!

chatgpt-codex-connector Bot commented Jul 10, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

carldebilly commented Jul 10, 2026

Uh oh!

chatgpt-codex-connector Bot commented Jul 10, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

carldebilly commented Jul 10, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

autocarl left a comment

Choose a reason for hiding this comment

Final re-review — verdict: ship

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants