refactor(spec): make missed SpecCommand fields a compile error, and fix the four that were already missed

[jdx]

Follow-up to the review on #739, where effect was dropped by SpecCommand::merge and by the docs model. Neither omission could have been caught by anything but a reviewer noticing.

The problem

Adding a field to SpecCommand means updating seven places — the struct, Default, the prop-parse arm, the child-node-parse arm, the KDL serializer, merge, the builder, and docs::models::SpecCommand + its From. Nothing enforces any of them, and every omission fails silently.

Auditing every existing field against the three impls turned up four fields already falling through, all on main:

field	gap	effect
deprecated	not carried by merge	an included spec silently un-deprecates a command
restart_token	never reaches the docs model	no template can render it
examples	not written by the KDL serializer	spec.to_string() drops every example
help_md, before_help_md, after_help_md	accepted only as props, serialized as child nodes	any spec with markdown help fails to reparse: Error: unsupported cmd key help_md

The last one is the worst and I only found it because of the test in the final commit.

The fix

merge and both From impls now destructure their source with no ... Add a field and you get three compile errors pointing at exactly the places that owe it a decision. Runtime-derived fields are bound to _ with a comment so the reason is visible rather than implied. No macros, no new machinery, no runtime cost.

A round-trip test covers what the compiler can't. The parser is a match on node names, not a struct pattern, so a field the serializer writes and the parser rejects still compiles — which is exactly how the help_md bug survived. The test parses a spec exercising every field, serializes it, reparses, and compares the two serde representations. Comparing serialized values rather than named fields means a field added later is covered without anyone remembering to extend the test.

Commits

Each is reviewable on its own, and the refactor is behavior-neutral:

1. refactor(spec): exhaustive destructuring — no behavior change; the four gaps are bound with NOTE comments so they're visible before being fixed
2. fix(spec): carry deprecated through merge
3. fix(docs): pass restart_token to the docs model
4. fix(spec): serialize examples to KDL
5. fix(spec): accept help_md / before_help_md / after_help_md as child nodes
6. test(spec): assert a spec survives a KDL round-trip

Considered and skipped

A #[derive(SpecNode)] macro generating parse/serialize/merge from field attributes would cover all of it, but the irregular fields are a real tax — deprecated accepts bool-or-string, alias splits into visible and hidden, the collection children each parse differently. Destructuring plus the round-trip test gets nearly the same coverage for a fraction of the machinery. Worth revisiting if the struct keeps growing.

Notes

• Branched off main rather than #739, so it's independently mergeable. Whichever lands second gets a trivial conflict in the destructuring lists (one added binding for effect).
• subcommand_lookup became pub(crate) so other modules can name it in a pattern. Still private outside the crate.
• serde_json added as a dev-dependency for the round-trip comparison.

Verified

cargo test -p usage-lib passes, 271 + doctests. cargo test --workspace has the same two pre-existing failures in usage-cli --test examples (test_usage_double_slash_execution, ..._old) present on a clean main. cargo clippy has one pre-existing error in choices.rs under a feature-gated path, also present on main and untouched here.

This PR was generated by an AI coding assistant.

---

Note

Medium Risk
Changes core spec merge, KDL parse/serialize, and docs mapping used when composing specs (includes/mounts), but fixes are narrow and guarded by new round-trip and merge tests.

Overview
SpecCommand maintenance is enforced at compile time by replacing .. skips with exhaustive destructuring in merge, KDL serialization (From<&SpecCommand> for KdlNode), and the docs From impl—new struct fields must be handled explicitly in each path.

Behavior fixes close four silent gaps: deprecated is merged like other optional metadata; restart_token is copied into the docs model; examples are emitted when serializing to KDL; and help_md / before_help_md / after_help_md are accepted as child nodes on reparse (matching how they are written), so round-tripped specs no longer fail with unsupported help_md keys.

Tests add deprecated merge coverage and a KDL parse → to_string() → reparse test that compares full serde_json snapshots, plus fixture guards so the round-trip exercise stays meaningful. subcommand_lookup is pub(crate) only so other modules can name it in destructuring patterns.

^{Reviewed by Cursor Bugbot for commit 53055a0. Bugbot is set up for automated code reviews on this repo. Configure here.}

[coderabbitai]

Warning

Review limit reached

@jdx, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 30 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: f5c0d5d6-3861-42ed-bb40-5955e7ca16ce

📥 Commits

Reviewing files that changed from the base of the PR and between 405cebf and 53055a0.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (3)

• lib/Cargo.toml
• lib/src/docs/models.rs
• lib/src/spec/cmd.rs

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Greptile Summary

The PR makes SpecCommand field propagation exhaustive and repairs previously omitted merge, documentation-model, and KDL round-trip behavior.

• Preserves deprecated during command merges and exposes restart_token to documentation renderers.
• Serializes examples and accepts markdown-help child nodes during KDL parsing.
• Adds a comprehensive round-trip fixture, including the fields identified in the previous review thread.
• Adds serde_json as a test-only dependency for structural comparisons.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
lib/src/spec/cmd.rs	Exhaustive destructuring now guards command merging and KDL serialization, while the expanded round-trip fixture populates all fields identified in the previous review.
lib/src/docs/models.rs	The documentation model now carries restart_token, and exhaustive source destructuring makes future omitted-field decisions compile-time visible.
lib/Cargo.toml	Adds serde_json as a development dependency for full-model round-trip assertions.
Cargo.lock	Records the usage-lib development dependency on the already-resolved serde_json package.

_{Reviews (3): Last reviewed commit: "test(spec): exercise every round-trippab..." | Re-trigger Greptile}

[jdx]

Good catch, and it mattered — before_help_md and after_help_md are parser paths this branch adds, so the fix was shipping untested. Comparing two default values is always equal, which is exactly how a round-trip test degrades into passing for free.

Fixed by populating before_help_long, after_help_long, before_help_md, after_help_md, command-level complete, and mounts in the fixture.

I also added a guard so this can't silently regress: after the equality check, the test asserts each of those fields is actually non-default in the parsed spec, and fails with fixture does not exercise \`` otherwise. Verified it isn't vacuous by adding a bogus field name and watching it fail.

This comment was generated by an AI coding assistant.