feat(argv): write the run= a declared completer answers - #890
Conversation
A spec is what every other consumer reads — the usage CLI, another shell's generator, a docs page — so a CLI whose completers live in Rust still has to say what they are. It now writes them: `complete "tool" run="mise __complete_word__ --candidates tool"`, generated from the completer rather than declared beside it, so there is one place a completer is said to exist and no second declaration to keep in step. And the binary answers exactly that command, so what the KDL promises is a thing that works rather than a string that looks plausible. A name nothing declares answers empty rather than failing: a script generated against a newer version of a CLI is stale, and a stale script should complete nothing rather than print into somebody's prompt. Found by the name a spec uses — the lowercased argument name, or a flag's value placeholder — which is the rule the reference resolves a `complete` block by, so both sides agree about which completer a `run=` belongs to. Deduplicated, because six of mise's commands take a `tool` and a spec declares one block for it. The round trip is the test: emit the KDL, parse it back with usage-lib, and check it reads a `run=` for that argument — then ask the binary the same question and get the answer, filtered by what was typed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThe PR emits declared Rust completers as executable KDL
Confidence Score: 4/5The PR is not yet safe to merge because attached flag values can still be answered by a same-named positional completer. The generated Files Needing Attention: argv/src/complete.rs, conformance/tests/spec_roundtrip.rs Important Files Changed
Reviews (6): Last reviewed commit: "test(argv): pin that an attached value i..." | Re-trigger Greptile |
…line Three from review, all in what the emitted `run=` promises. A `complete` block now sits inside the command that declares it, which is where the reference looks for one first. Written at the top level, two sibling commands that take a `TOOL` and mean different things by it — the installed ones against the available ones — collapsed into a single block, and whichever came first in tree order answered for both. `for_name` resolves the same way: the command the line reached, and only then anywhere in the tree. The line goes with the request. A caller running that command is the reference, which interpolates `words` through tera before running it, so a completer reading an earlier flag sees the same line it would have seen natively. Without it the answer is computed against nothing, which for that completer is a wrong answer rather than a missing one. And `descriptions=#true` is gone, because it was not true: it tells the reference to read a description after an unescaped colon, while this answers with values — so a value containing a colon would have been split in half, and mise's task names are full of them. Found by greptile and Cursor Bugbot. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Instruction counts
No instruction-count regression above 1%. Only instruction counts gate. Wall clock is shown for context — on identical hardware it moves 4-20% run to run. Measured by tak — instruction-counted CLI benchmarks, stored in this repository's git notes. Shadow comparisonParsing
|
|
Three findings, all correct, all fixed in the head commit. Completer context never reaches binary — right, and it would have been a wrong answer rather than a missing one: a completer that reads an earlier flag would compute against nothing and answer confidently. The Deduplication conflates distinct callbacks — also right. A Description protocol mismatches output — correct, and the fix is to stop claiming it. AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: unavailable. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 21cf365. Configure here.
The root's own first, then the command the line reached, then the tree. The order matters because the reference reads `spec.complete.get(name).or(cmd.complete .get(name))` — spec-level before command-level — and the root's completers are exactly what a spec writes at its top level. Answering in a different order would mean this binary and the reference disagreed about a spec they both read. The fallback to tree order stays last, which is what keeps two sibling commands meaning different things by one name. Found by greptile. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Two more, one fixed and one that matches the reference. Completer lookup reverses precedence — correct. usage-lib reads Unescaped apostrophes break run line — real, and shared with the reference: its own AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: unavailable. |
`--source=⌶` is a dash-prefixed token, so it is a flag position: the flag branches come first and the positional's completer is never reached. Raised as a case where it would be, and it is not — but it is worth holding, because the word being completed is excluded from the walk, so the flag is not `awaiting_value` either and the position would look like the argument's if that order ever changed. The fixture's flag and argument now have *different* completers, which is what makes which one answered visible at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Checked, and this one does not hold: a dash-prefixed token never reaches the positional branch. The order in — so Pinned anyway, with a fixture whose flag and argument have different completers so that which one answered is visible at all — because your reasoning about the walk is right even though the conclusion is not: the word being completed is excluded, so the flag is not AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: unavailable. |
A spec is what every other consumer reads — the usage CLI, another shell's generator, a docs page — so a CLI whose completers live in Rust still has to say what they are. It now writes them: `complete "tool" run="mise __complete_word__ --candidates tool"`, generated from the completer rather than declared beside it, so there is one place a completer is said to exist and no second declaration to keep in step. And the binary answers exactly that command, so what the KDL promises is a thing that works rather than a string that looks plausible. A name nothing declares answers empty rather than failing: a script generated against a newer version of a CLI is stale, and a stale script should complete nothing rather than print into somebody's prompt. Found by the name a spec uses — the lowercased argument name, or a flag's value placeholder — which is the rule the reference resolves a `complete` block by, so both sides agree about which completer a `run=` belongs to. Deduplicated, because six of mise's commands take a `tool` and a spec declares one block for it. The round trip is the test: emit the KDL, parse it back with usage-lib, and check it reads a `run=` for that argument — then ask the binary the same question and get the answer, filtered by what was typed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --- <sub>Stack created with <a href="https://github.com/github/gh-stack">GitHub Stacks CLI</a> • <a href="https://gh.io/stacks-feedback">Give Feedback 💬</a></sub> <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes completion protocol and spec output that external generators read; behavior is heavily covered by roundtrip and dispatch tests, but wrong resolution order would break mise-style multi-command `tool` completers. > > **Overview** > Declared Rust completers are now **written into emitted KDL** as `complete` blocks whose `run=` invokes the same binary with `__complete_word__ --candidates <name> --line '…'`, instead of leaving completion as a second hand-maintained declaration beside the Rust callbacks. > > The binary **implements that contract** via new `for_name` resolution (aligned with usage-lib: root, then command reached on the line, then tree fallback; cursor-aware when one name maps to both a positional and a flag value) and derive support for **`--candidates`**, returning filtered candidates or an empty list when the name is unknown. Spec emission also adds **`completers_on`**, per-command `write_completers`, and shell-safe line interpolation (apostrophe escaping; no `descriptions=#true` so colon-heavy values are not split). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 968892b. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

A spec is what every other consumer reads — the usage CLI, another shell's
generator, a docs page — so a CLI whose completers live in Rust still has to say
what they are. It now writes them:
complete "tool" run="mise __complete_word__ --candidates tool", generated from the completer rather than declared beside it,so there is one place a completer is said to exist and no second declaration to
keep in step.
And the binary answers exactly that command, so what the KDL promises is a thing
that works rather than a string that looks plausible. A name nothing declares
answers empty rather than failing: a script generated against a newer version of a
CLI is stale, and a stale script should complete nothing rather than print into
somebody's prompt.
Found by the name a spec uses — the lowercased argument name, or a flag's value
placeholder — which is the rule the reference resolves a
completeblock by, soboth sides agree about which completer a
run=belongs to. Deduplicated, becausesix of mise's commands take a
tooland a spec declares one block for it.The round trip is the test: emit the KDL, parse it back with usage-lib, and check
it reads a
run=for that argument — then ask the binary the same question and getthe answer, filtered by what was typed.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Stack created with GitHub Stacks CLI • Give Feedback 💬
Note
Medium Risk
Changes completion protocol and spec output that external generators read; behavior is heavily covered by roundtrip and dispatch tests, but wrong resolution order would break mise-style multi-command
toolcompleters.Overview
Declared Rust completers are now written into emitted KDL as
completeblocks whoserun=invokes the same binary with__complete_word__ --candidates <name> --line '…', instead of leaving completion as a second hand-maintained declaration beside the Rust callbacks.The binary implements that contract via new
for_nameresolution (aligned with usage-lib: root, then command reached on the line, then tree fallback; cursor-aware when one name maps to both a positional and a flag value) and derive support for--candidates, returning filtered candidates or an empty list when the name is unknown. Spec emission also addscompleters_on, per-commandwrite_completers, and shell-safe line interpolation (apostrophe escaping; nodescriptions=#trueso colon-heavy values are not split).Reviewed by Cursor Bugbot for commit 968892b. Bugbot is set up for automated code reviews on this repo. Configure here.