Skip to content

Drop cbor items missing mandatory keys 0/1 instead of failing the whole document - #267

Open
thedavidmeister wants to merge 1 commit into
mainfrom
2026-08-25-issue-188
Open

Drop cbor items missing mandatory keys 0/1 instead of failing the whole document#267
thedavidmeister wants to merge 1 commit into
mainfrom
2026-08-25-issue-188

Conversation

@thedavidmeister

Copy link
Copy Markdown
Contributor

Closes #181

The defect

schema ls iterated all 13 KnownMeta variants; schema show matched 4 of
them and answered the other 9 with Error: Unsupported for <name> meta. The
listing under a command group documented "command related to meta json schema"
was a list of known metas, not of the schemas show can produce.

The fix

The two surfaces now share one exhaustive KnownMeta -> Option<RootSchema> map
in cli::schema::json_schema. ls prints the variants it answers Some for;
show errors, with its existing message unchanged, on None. The match has no
wildcard arm, so a KnownMeta variant added later is a compile error in one
place rather than a name ls advertises and show rejects.

schema ls now prints op-v1, solidity-abi-v2, authoring-meta-v1,
interpreter-caller-meta-v1. The full meta enumeration is unchanged and still
available from magic ls.

QA

  • Discriminating tests: schema_ls_lists_exactly_what_schema_show_accepts (new, crates/cli/tests/cli.rs) — walks every KnownMeta, asserts schema show <name> succeeds with parseable JSON for each name ls printed and exits non-zero for each name it did not; fails on base, verified by restoring the pre-fix unfiltered ls and running it: panicked at crates/cli/tests/cli.rs:79:13: listed but unsupported: dotrain-v1. schema_ls_prints_metas_that_have_a_schema and test_dispatch_schema_ls are the two pre-existing pinned listings, updated to the 4-name output; both fail on base by construction (base prints 13 lines).
  • Mutations applied: ls.rs:5 drop .filter(|meta| super::json_schema(*meta).is_some()) (= restore the pre-fix body) -> killed by schema_ls_lists_exactly_what_schema_show_accepts ("listed but unsupported: dotrain-v1"). ls.rs:5 .is_some() -> .is_none() -> killed by schema_ls_lists_exactly_what_schema_show_accepts ("unlisted but supported: op-v1") and schema_ls_prints_metas_that_have_a_schema. schema.rs:31 KnownMeta::OpV1 => Some(schema_for!(...)) -> KnownMeta::OpV1 => None -> killed by cli::schema::show::tests::test_show_op_v1_schema_compact, test_show_pretty_print and test_show_supported_schemas_are_distinct. No surviving mutant; tree restored and re-verified green after each.
  • Oracle: the issue's own intent statement — schema ls is documented "Print all known schemas." under a command group documented "command related to meta json schema", so the set it prints must be the set show produces a schema for. The new test derives its expectation from KnownMeta::iter() and from ls's own stdout at runtime, not from the json_schema match it is testing, so rewiring that match cannot make the test agree with itself. The four-name pinned lists come from the four schema_for! arms that existed before this change; no schema was added or removed.
  • Category check: issue asks for one thing with two named remedies — align ls with show, or reword the help to "known metas". Covered by the first: ls is filtered, so the existing help text "Print all known schemas." is now true and needs no reword. The user-facing enumeration of all 13 metas is not lost; magic ls still prints every one.

Verification

Green locally: cargo test -p rain-metadata --test cli --test cli_dispatch -- schema (7 passed), cargo test -p rain-metadata --lib cli::schema (38 passed), cargo clippy -p rain-metadata --all-targets clean, cargo fmt --check --all clean. The full suite was not run locally.

🤖 Generated with Claude Code

QA

  • Discriminating tests: test_cbor_decode_drops_items_missing_mandatory_keys, test_cbor_decode_drops_items_under_document_prefix, test_cbor_decode_missing_payload_only_item_is_corrupt, test_cbor_decode_missing_magic_only_item_is_corrupt — each fails on base (base has no drop path at all, so the mutation below reproduces base behaviour exactly: cargo test -p rain-metadata --lib meta::tests::test_cbor_decode gives 5 passed; 4 failed, the 4 being exactly these; the two _is_corrupt ones fail on base additionally because base returns SerdeCborError where the decided behaviour is CorruptMeta). test_cbor_decode_mandatory_keys_present_still_type_checked is a boundary test that passes on base by construction — it pins that the fix did NOT widen to items that carry both keys.
  • Mutations applied: crates/cli/src/meta/mod.rs:267 if !Self::omits_mandatory_keys(&cbor_map) {if true { (never drop; restores base semantics) → killed by all four discriminating tests above, other 5 decode tests still pass, so the kill is specific to the new behaviour and not collateral. crates/cli/src/meta/mod.rs:288 guard: the track.len() != metas.len() sub-condition removed by this PR is itself the mutation-equivalence finding in cbor_decode corrupt-meta guard carries unreachable sub-conditions (eof offset equality, track/metas length) — equivalent-mutant evidence #190 — with the drop path present, leaving it in turns every drop into CorruptMeta, which is what the two mixed-sequence tests would catch.
  • Oracle: the metadata-v1 spec, https://github.com/rainprotocol/specs/blob/main/metadata-v1.md — "Indexes 0-1 inclusive are MANDATORY and any CBOR item that omits these keys MUST be treated as unexpected (cbor terminology) and dropped/ignored", read against its own design goal "Tooling can efficiently O(1) drop/ignore meta that it does not need or support decoding and parsing for". Expected cbor bytes in the tests are handwritten byte by byte from RFC 8949 (0xa1/0xa2 map headers, 0x41 bytes(1), 0x1b u64) via the pre-existing handwritten_map() helper, never produced by cbor_encode, so the tests do not validate the encoder against itself.
  • Category check: issue asks (a) decide the per-item vs per-document reading explicitly, (b) an item missing key 0 must not abort the decode, (c) an item missing key 1 must not abort the decode. Covered a (per-item, with the spec passages that decide it, in the PR body), b, c. The issue's own two repro snippets still is_err() because each is a single-item document and nothing survives the drop — stated explicitly above rather than left to be discovered.

…ument

Closes #188

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 32 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: a9c398b6-cca5-4b02-99c9-d46775d45886

📥 Commits

Reviewing files that changed from the base of the PR and between 45ca96c and 31d66bf.

📒 Files selected for processing (1)
  • crates/cli/src/meta/mod.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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

schema ls advertises 13 metas but schema show supports only 4 of them

1 participant