test(go): check in mise's generated tables, so the emitter cannot drift unnoticed - #932
Conversation
|
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 |
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 70fc4f2. Configure here.
Greptile SummaryThe PR checks in generated Go parser tables for mise and adds CI regeneration, semantic parsing, key-integrity, and allocation checks.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Reviews (3): Last reviewed commit: "fix(go): regenerate mise's tables, and p..." | Re-trigger Greptile |
Instruction countsNothing was compared, and so nothing was gated. No series appears on both sides: either the base has no measurements recorded, or the two were measured on different runner classes, which are deliberately not comparable — counts shift between machine types by more than a real regression does. New, nothing to compare against: 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
|
…ft unnoticed The generator landed in the commit before this one with snapshot tests over small fixtures, which prove it emits what it meant to and nothing about whether the result works. This is the other half: mise's committed spec, generated into `go/internal/shadow/mise`, checked in, and parsed against. Checked in rather than built by the test, for the two reasons the Rust shadows are: a reviewer can read the diff when the emitter's vocabulary changes, and CI runs `mise run gen-go` and fails if regenerating produces one. A change to the generator that nobody meant now has to be committed rather than discovered. mise is the fixture because it is the largest usage CLI there is — 211 commands, 711 flags, 128 positionals, four deep — and because every shape that has been awkward to express came from it. The cases are invocations out of its own docs, hand-written on purpose: what the generator produces is only worth checking if it parses the words users actually type. They cover the `[ARGS]… [-- ARGS_LAST]…` split that made the Rust derive's validation wrong, a hidden alias selecting a command, and a root global reaching a command two levels down. Two properties are measured here rather than at fixture scale, because scale is what would break them: Keys are unique and dense. Generated code dispatches on a Key, so two entries sharing one would bind the wrong field. The Rust derive hashes its way around this because two macro expansions cannot see each other; a generator sees the whole spec and can count, so a collision would be inexcusable rather than unlucky — and this checks it across all 989 entries. A parse still allocates nothing. A scope lookup that collected flags into a slice, or a walk that built one per token, is invisible on a spec with four flags and obvious on one with 711. 110ns and 0 allocations for `mise use -g node@20`. One case pins a hole rather than a property: `run --wat` is `unexpected_arg`, because mise's spec gives `run` no positional at all — it clears them and adds `mount run="mise tasks --usage"`, so task names come from running that, and binding does not resolve mounts. When mounts are answered that case changes, and pinning it means it changes loudly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…t wrong The one-line diff is the whole point of checking this file in: `DefaultSubcommand` went from `cmdOciRun` to `cmdRun`, which is what the fix in the commit before this one produces. A reviewer reading the generated diff is how the bug was found, and regenerating is how the fix is shown to be real. Also pins it as a test. Nothing in the parse of an ordinary command line shows the difference — `mise build` reports `unexpected_arg` either way, because mise's spec gives `run` no positional and task names come from a mount — so the pointer is asserted directly, against `cmdRun` and specifically not against `cmdOciRun`, the one a whole-tree search used to win with. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ead of a derive (#931) Go has no macros. What a Rust CLI gets from `#[derive(Cli)]` at compile time, a Go CLI has to get from a generator at build time — so this is the same milestone for usage-go that usage-derive is for the Rust side: the point at which a spec, rather than a hand-written table, is what a CLI is declared in. ```go //go:generate usage generate go -f mycli.usage.kdl -o tables.go ``` Follows #921 and #922, which landed the binder and the conformance suite. ## Binding tables only Help text, choices, defaults and `env` are absent, for the same reason they are absent from the Rust hot path: a successful parse never reads them, and mise's several hundred kilobytes of help strings do not belong in front of a parser. A cold table is separate work. ## The output is gofmt-clean as it comes out This took some care and is worth it — the alternative is every adopter running a formatter before they can commit, and this repo's own CI failing `gofmt -l` on the table #932 checks in. gofmt pads within runs of consecutive single-line entries and starts a new run after anything spanning lines, so the emitter models a literal as a list of fields and blocks rather than printing strings. Verified against mise's spec, usage's own, and the examples spec: `gofmt -w` changes nothing in any of them. ## What the author gets beyond speed The key constants. An event carries a `Key`, so generated code dispatches on `mise.FlagUseGlobal` rather than comparing strings — and a flag renamed in the spec then fails to compile instead of silently never matching. Three things are resolved at generation time so the parser reads one field per command instead of walking: `unknown_flags` inheritance, `default_subcommand` into a pointer at the node it names, and hidden aliases folded in beside visible ones (hiding is a help-output concern; binding never reads it). Identifier collisions are ordinary rather than exotic, and are handled: mise declares both a `macos-defaults` command and a `macos defaults` path, and both want to be spelled `CmdMacosDefaults`. ## Checked end to end The fixture that keeps this honest in CI is #932, but before committing I confirmed the generated mise tables compile, parse `mise use -g node@20`, resolve `x` to `exec` through its hidden alias, split `tasks run build extra --dry-run -- --verbose` across `ARGS` and `ARGS_LAST`, and produce a package with **no init function** whose `Root` is a type `D` symbol — 211 commands and 711 flags that cost nothing before `main`. --- <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> 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Large new code generator with many spec edge cases, but it is additive build-time output; mistakes would break adopters' generated Go compile/parse behavior rather than runtime security. > > **Overview** > Introduces **`usage generate go`**, the Go analogue of compile-time Rust derives: specs become check-in-ready Go source for **`github.com/jdx/usage/go/argv`**, typically via `//go:generate usage generate go -f mycli.usage.kdl -o tables.go`. > > The new **`usage::go`** emitter writes **binding-only** tables (commands, flags, args, keys, aliases)—not help, choices, or defaults—using package-level `var` trees consumable by the linker without `init`. Output is **gofmt-aligned** (const blocks and struct field runs). Generation resolves **`unknown_flags` inheritance**, **`default_subcommand`** against root direct children only (avoids wrong deep matches), folds **hidden aliases** into bindings, and deduplicates **Go identifier collisions** with suffixed names. > > The CLI accepts **`-f`/`--spec`**, **`-o`**, and **`-p`/`--package`**; invalid explicit package names error instead of silent mangling. Spec metadata, man page, Fig completion, docs, and command **read/write** effects are updated for the new subcommand. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 858445b. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added `generate go` to generate Go source code from a CLI usage specification. * Supports specification files or inline input, custom output files, and Go package names. * Generated output includes commands, flags, arguments, aliases, subcommands, and related parsing behavior. * Validates package names and safely handles generated identifiers and special characters. * **Documentation** * Added CLI help, reference documentation, usage examples, and shell completion support for the new command. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Binding says which token becomes which flag or argument. This says
whether what landed is acceptable, and fills in what the command line
left empty.
**The corpus goes from 122 answered to 145 of 152.**
```
$ mise run test:go
152 vectors: 145 answered, 7 not yet
```
Follows #931 and #932. `required`, `choices`, the `env`-then-`default`
fallback, `var_min` and `var_max` all need to know something no single
token can tell you, which is why they were left out of the parser rather
than overlooked.
## Shape
They read a second, cold table — `Meta`, indexed by the same key the
parse table carries, so the two cannot drift on identity — and a program
that never applies them never touches it. The zero-allocation test now
covers a parse with metadata present, since the property that matters is
that binding does not reach for any of this.
Deliberately **not a framework**. `Fill` and `Check` are pure functions
over what binding produced, because the caller is the one that knows how
it accumulated: generated code assigns to a field, a harness with no
target type appends to a slice. Inventing a value model here would force
both through it.
## Three things the corpus settled that guessing would have got wrong
**An environment variable set to the empty string is set.** `EX_JOBS=`
is a value — treating empty as unset would make it mean something no
other empty value in the grammar means. The value is one token, never
re-split: quoting is the shell's job and there was no shell here at all.
**A flag that holds no value reads its variable as a yes or a no**, by
an allow-list — `1`, `true`, `True`, `TRUE` — matching usage-lib
exactly. So `yes`, `on` and `TrUe` are all false. Worth pinning rather
than discovering, since `EX_VERBOSE=0` meaning verbose would be a trap.
The corpus pins four cases; `TestEnvTruth` records the rest.
**`var_max` counts occurrences here, never values.** A variadic's
per-occurrence bound is a limit binding applies, so judging the total
again afterwards would fail an invocation that never broke it. (This is
the same distinction as the one discussed on #931.)
## What is left
Seven vectors, all relationships *between* flags — `conflicts`,
`overrides`, `required_unless` — which need a name resolved to the entry
it refers to. Listed by id with a reason rather than inferred from the
spec, because `overrides-loser-is-not-refilled-from-env` is as much an
env question as an overrides one and inference would have exempted
vectors nobody meant to exempt. The skip count is asserted against the
list, so it cannot become a way of hiding failures.
Also still open, and noted in the README: `usage generate go` emits the
parse tables but not the `Meta` ones, so these rules are reachable today
from a spec lowered at run time rather than from a generated package.
Proving them against the corpus came first, the way the binder did
before the generator.
---
<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>
🤖 Generated with [Claude Code](https://claude.com/claude-code)
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Behavior changes CLI validation semantics (env, defaults, required,
choices, variadic bounds) and expands the conformance harness; parser
hot path is explicitly tested to stay zero-alloc, but adopters must call
the new APIs correctly.
>
> **Overview**
> Adds a **cold `Meta` / `Metadata` table** alongside the parse tables
and **`Fill` / `Check`** in `post.go` so binding stays allocation-free
while **required**, **choices**, **env → default** fallback,
**`var_min`**, and **flag `var_max` (occurrences)** are judged after the
last token. Post-binding failures reuse **`argv.Error`** with new codes
and fields (`Name`, `Choices`, `Bound`/`Got`).
>
> **`spec.Build()`** now emits parse tables and metadata in one pass
(shared keys); the JSON spec builder picks up the extra declaration
fields and usage-lib-aligned rules (defaults on nested arg values, no
nested `env`, choices on the value).
>
> **Conformance** runs post-binding vectors with per-vector env,
accumulates by flag/arg key along the command path, and documents **7
remaining** inter-flag vectors (`conflicts`, `overrides`,
`required_unless`) in an asserted `notYet` list. README updates the pass
count (**145 / 152**).
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
de6de50. 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>

The generator landed in #931 with snapshot tests over small fixtures, which prove it emits what it meant to and nothing about whether the result works. This is the other half: mise's committed spec, generated into
go/internal/shadow/mise, checked in, and parsed against.Checked in rather than built by the test, for the two reasons the Rust shadows are — a reviewer can read the diff when the emitter's vocabulary changes, and CI runs
mise run gen-goand fails if regenerating produces one. A change to the generator that nobody meant now has to be committed rather than discovered later.mise is the fixture because it is the largest usage CLI there is (211 commands, 711 flags, 128 positionals, four deep) and because every shape that has been awkward to express came from it. The cases are invocations out of its own docs, hand-written on purpose: what the generator produces is only worth checking if it parses the words users actually type.
Two properties measured at scale rather than at fixture size
Keys are unique and dense. Generated code dispatches on a
Key, so two entries sharing one would bind the wrong field. The Rust derive hashes its way around this because two macro expansions cannot see each other; a generator sees the whole spec and can count, so a collision would be inexcusable rather than unlucky. Checked across all 989 entries.A parse still allocates nothing. A scope lookup that collected flags into a slice, or a walk that built one per token, is invisible on a spec with four flags and obvious on one with 711. 110 ns and 0 allocations for
mise use -g node@20.One case pins a hole rather than a property
run --watisunexpected_arg, because mise's spec givesrunno positional at all — it clears them and addsmount run="mise tasks --usage", so task names come from running that, and binding does not resolve mounts. When mounts are answered that case changes, and pinning it means it changes loudly.Also here
The README's conformance count was stale on
main: #926 imported the argv questions clap's suite answers, taking the corpus from 123 vectors to 152. The Go parser answers all 122 binding vectors, up from 101, without a change — which is a better advertisement for the corpus than the original number was.Stack created with GitHub Stacks CLI • Give Feedback 💬
🤖 Generated with Claude Code
Note
Low Risk
Mostly generated data, docs, CI, and tests; no runtime product behavior changes beyond guarding the Go emitter output.
Overview
Adds checked-in Go binding tables for mise’s full spec under
go/internal/shadow/mise, regenerated viamise run gen-go(usage generate goonbenches/mise.usage.kdl), mirroring the Rust shadow workflow so emitter changes show up in review and CI.CI now runs
gen-goand fails ifrender,gen-shadow, orgen-goleave a dirty tree.Tests on the shadow tables cover real mise invocations, root
default_subcommand run(notoci run), unique denseKeys across ~1000 entries, and zero allocations at full CLI scale (plus a benchmark).go/README.mddocuments//go:generate, the shadow package, updated corpus counts (122 binding / 30 post-binding skipped), and drops the generator from “what is missing.”Reviewed by Cursor Bugbot for commit dcd3446. Bugbot is set up for automated code reviews on this repo. Configure here.