feat(spec): the config vocabulary - #835
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 |
Greptile SummaryThe PR defines and documents a comprehensive configuration vocabulary backed by a richer parsed model.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Reviews (7): Last reviewed commit: "fix(spec): stop the config block accepti..." | Re-trigger Greptile |
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
|
9ab438a to
b41d239
Compare
|
All three findings were real, and there was a fourth hiding behind the third. Fixed in b41d239. Config merge drops new fields — confirmed, and worse than described: Malformed types normalize silently — confirmed for all three inputs, and Non-string bindings become strings — confirmed. The one place where "convert rather than refuse" was actually wrong in the other direction was the list default: Five tests, seven mutations, each one verified to fail with its fix reverted. The three descendant PRs are rebased; the docs model needed one follow-on change for the typed list, folded into #837. AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: unavailable. |
b41d239 to
87418b9
Compare
|
Also fixed the builder inconsistency Bugbot flagged (
The AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: unavailable. |
87418b9 to
d587ccf
Compare
|
Three more findings from the later review round, all real, all fixed in the amended head. A union claimed a legacy type it does not have.
The two env spellings could disagree. The sync ran only when one side was empty, so a prop writing both The builder inconsistency from the earlier round is fixed here too. AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: unavailable. |
What a `config` block can say, so that documentation, a JSON schema, completions and a resolver can all be generated from one declaration instead of five hand-written copies. Three node kinds. `source` declares a kind usage knows nothing about — a git config, a pkl file, an `.npmrc` — with the display metadata docs need to say "settable with `git config hk.check`" without usage having any idea what git is. `file` lists config file locations in ascending precedence, with `findup` and a `scope`, which is the rc-style chain documentation has to describe. `prop` is the setting. Props gain what the fleet`s registries actually declare: a type in a small expression grammar (`list<string>`, `map<string, url>`, `option<path>`, `bool|string`, with unknown names kept verbatim as the escape hatch), typed defaults including list defaults, several env var spellings in precedence order, the flags that set it, its keys in each custom source kind, choices with per-value help, a `merge` policy, a `scope` that keeps a setting out of files a repository can carry, the deprecation triple plus `renamed_to`, `hide`, `since`, a named value parser, `writes_to`, examples, and `help_heading`. And `x "ns.key" value` for everything usage should not interpret: `mise.rust_type`, `aube.npm_shared`. Preserved in order, written back out, present in `usage g json`, read by nothing here. It is the seam that lets a CLI with special rules describe its settings without usage modelling those rules. The old spellings still work: `data_type=` is read as `type=`, and `boolean`/`integer`/ `number`/`usize`/`array<>`/`optional<>` are accepted and normalized. Unknown vocabulary is refused rather than half-read, which is what `min_usage_version` is for. The reference page is rewritten, because the one it replaces documented a syntax that had never parsed — `file`, `findup`, `default "k" "v"`, `alias`, `config_file`, none of which the parser has ever accepted, on a page in the site nav. A test now parses every KDL example on it, so it cannot drift again. Running that test against the other reference pages reports five failures; four are catalogue blocks listing alternative spellings, but one is real — `flag "flag1"` in cmd.md is missing its dashes and the parser rejects it. Left for its own change rather than smuggled in here. The whole vocabulary is round-tripped in a test, field by field, and the parsed model is committed as a snapshot: that is the artifact `usage g json` hands downstream and what an implementation in another language can diff against.
d587ccf to
2769a49
Compare
|
Two more, both real.
A second AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: unavailable. |
2769a49 to
39a8d87
Compare
|
Right, and it is the case I missed when I made 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 39a8d87. Configure here.
Five holes Greptile found in the vocabulary, each the same shape as the bugs this stack opened with: input that changes meaning instead of being refused. `SpecConfig::merge` copied only props, and `is_empty` counted only props. So the case `include` exists for — a spec with many settings keeping them in their own file — silently dropped that file's `source` and `file` declarations, and a block that declared just where config files live was reported empty and left out by the writer entirely. The type grammar dropped empty members and kept stray delimiters, so four typos loaded as something else: `bool|` as plain `bool`, `bool||string` as the two-member union, `map<string,>` as a map to strings, and `int>` as a *custom type named* `int>` that every consumer then treats as a string. `Base::Custom` is the escape hatch for names a newer usage understands; it must not swallow an expression that failed to parse. A name is now refused if it is empty or holds one of `< > , |`. `env`, `cli`, `source` and `example` hold names, and a non-string argument was rendered with `to_string()` rather than refused — `env #true` became a variable called `#true`, written back out quoted as though somebody had meant it. And a list default is now typed like the scalar one, so `default 80 443` for a `list<int>` stays two numbers through the writer and the JSON schema instead of becoming two strings. Five tests, seven mutations, each verified to fail without its fix.
39a8d87 to
4ae32fe
Compare
|
Right — and it is the same class as AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: unavailable. |

Stacked on #832. What a
configblock can say — so docs, a JSON schema, completions and a resolver come from one declaration instead of the five hand-written copies the fleet has now.Three node kinds
sourceis what lets docs say "settable withgit config hk.check" without usage having any idea what git is.scope="global"is not decoration — mise treats "a checked-in file must not be able to change this" as a security property, so it is declared rather than left to each tool.Types
An unknown name is kept verbatim rather than refused — that's the type-level escape hatch, so a spec naming a type only its own tool understands still loads everywhere. Old spellings (
data_type=,boolean,integer,number,usize,array<…>,optional<…>) are accepted and normalized.xextensionsx "ns.key" value: preserved in order, written back out, inusage g json, interpreted by nothing here. Wheremise.rust_type,mise.parse_envandaube.npm_sharedlive. This is the seam that lets a CLI with special rules describe its settings without usage having to model those rules.Verification
usage g jsonhands downstream, and what a port in another language diffs against.file,findup,default "k" "v",aliasandconfig_file— none of which the parser has ever accepted — while never mentioningprop, the one thing it did. It's in the site nav. It cannot drift like that again.min_usage_versionis for.One finding for elsewhere: running that docs test against the other reference pages reports five failures. Four are catalogue blocks — lists of alternative spellings that aren't specs (one even puts an argument after a variadic, which the parser rightly refuses). The fifth is real:
flag "flag1"incmd.mdis missing its dashes and the parser rejects it. Left for its own change rather than smuggled in here.AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: unavailable.
Note
Medium Risk
Large expansion of the spec parser and serialized config shape; behavior changes (strict errors, merge rules for files/sources) could break hand-written specs that relied on old permissive or undocumented behavior, though compatibility aliases and tests aim to limit that.
Overview
Adds a full config vocabulary to usage specs so settings, file chains, and external source kinds can be declared once for docs, JSON export, and resolvers.
configblock now supportssource(opaque kinds like git/pkl with doc hints),file(precedence chain withfindup,scope,format), and richerpropnodes:typegrammar,cli/envaliases, bindings,choices,merge/scope, deprecation/renames,xextensions, and list defaults.Introduces
SpecConfigType(list<>,map<>, unions, legacy synonym normalization) alongside the old five-valuedata_type, with stricter parsing (unknown keys refused, no silent child drops, union defaults not coerced via legacy type).docs/spec/reference/config.mdis rewritten to match the parser;lib/tests/docs_examples.rsparses every KDL example on that page. Generated CLI JSON gains emptysources/filesonconfig.Reviewed by Cursor Bugbot for commit 4ae32fe. Bugbot is set up for automated code reviews on this repo. Configure here.