Distinguish corrupt subgraph records from absence

[thedavidmeister]

Closes #213. Closes #234.

What was wrong

process_meta_query and process_deployer_query returned Error::NoRecordFound
for every failure short of a transport error, so "this subgraph has no such
record" and "this subgraph has the record but cannot serve it" were the same
value to every caller. Under select_ok, a systemically corrupt record —
or a subgraph rejecting the query outright — surfaced to search,
search_deployer and Store::update as found no matching record, and was
treated as a cache miss.

What changed

Two new Error variants, and each failure now routes to the one that describes
it:

• NoRecordFound — and only this — for genuine absence: data.meta is null,
  or expressionDeployers is empty.
• CorruptRecord(String) for a record that was found but is unusable: a
  missing required field, more or fewer than one meta, or a hex field that
  does not decode. The message names the field (bytecode is missing,
  constructorMetaHash: <hex error>, expected exactly one meta, got 2), so
  which of the eight deployer fields is bad is visible without a repro.
• SubgraphError(String) for a query the subgraph itself failed: the
  previously-unread top level graphql errors member, with every message
  carried, or a response with neither data nor errors. An empty errors
  array is the wire form for "no errors" and still succeeds.

select_ok semantics are unchanged: every one of these is still a non-fatal
per-subgraph Err, so a healthier subgraph still wins the race. What changes is
what the caller is told when no subgraph wins.

Why both issues

#234 is the same finding against process_meta_query, including its point that
the graphql errors member is never inspected. The two functions share the
response-shape handling and the single error enum, so splitting this into two
PRs would have meant either a near-duplicate diff or one half of a collapse
fixed and the other left in place. Both are closed here; the errors member is
now inspected in both queries, not only the meta one.

Tests

The round-1/round-2 mutation-test suites pinned the old collapse byte-for-byte;
#234 anticipated that a fix updates them. Every test that asserted
NoRecordFound for corruption now asserts CorruptRecord and its message,
which is strictly more discriminating than the matches! it replaces — a
mutation that swapped one field's name or dropped the decode would have survived
the old assertion. The two tests that assert genuine absence
(test_process_meta_query_missing_meta_is_no_record_found,
test_process_deployer_query_empty_no_record) are untouched and still assert
NoRecordFound.

New tests cover the paths that had none: graphql errors on both queries, a
response carrying neither data nor errors, and an empty errors array
alongside good data.

cargo test -p rain-metadata --lib: 311 passed, 0 failed.
cargo clippy --workspace --all-targets -- -D warnings and
cargo fmt --all --check: clean.

QA

• Discriminating tests: the 15 test_process_deployer_query_*_is_corrupt_record,
  test_process_meta_query_bad_hex_is_corrupt_record,
  test_process_meta_query_missing_data_is_subgraph_error,
  test_process_meta_query_graphql_errors_is_subgraph_error,
  test_process_deployer_query_graphql_errors_is_subgraph_error,
  test_process_meta_query_paths — each fails on base (verified by reverting
  the fix's three mapping sites in place to base's .or(Err(Error::NoRecordFound)),
  return Err(Error::NoRecordFound) and un-inspected errors, keeping the new
  tests: 20 failed; 13 passed in meta::query::, and those 20 are exactly the
  tests listed here). test_process_meta_query_empty_errors_array_is_success is
  deliberately NOT in that set — it passes on base and guards the fix against
  over-rejecting; M3 below is what kills it.
• Mutations applied (each run alone against the whole meta::query:: suite):
  • decode_field both arms -> Error::NoRecordFound (base's mapping) -> 15
    failed, killed by every *_is_corrupt_record hex/missing test and
    test_process_meta_query_paths.
  • response_data drops the errors inspection -> 2 failed, killed by
    test_process_meta_query_graphql_errors_is_subgraph_error and
    test_process_deployer_query_graphql_errors_is_subgraph_error.
  • response_data drops .filter(|errors| !errors.is_empty()) -> 1 failed,
    killed by test_process_meta_query_empty_errors_array_is_success.
  • [meta] -> [meta, ..] (accept more than one meta) -> 1 failed, killed by
    test_process_deployer_query_two_metas_is_corrupt_record.
  • the parser field labelled "store" -> 2 failed, killed by
    test_process_deployer_query_invalid_parser_hex_is_corrupt_record and
    test_process_deployer_query_null_parser_is_corrupt_record — the message
    assertions, not the variant, are what catch this.
    No surviving mutants.
• Oracle: the graphql over HTTP spec and the functions' own doc comments, not
  the implementation. The doc comment says NoRecordFound models "nothing
  found", so absence is the only input that may produce it; the graphql
  response spec says errors is carried independently of data and that an
  empty errors array means no errors, which fixes what
  test_process_meta_query_empty_errors_array_is_success and the two
  graphql_errors tests must assert. Expected messages are literal strings
  written from the field names in deployer.graphql / meta.graphql, so a
  wrong label in the implementation cannot satisfy them.
• Category check: process_deployer_query reports corrupt subgraph payloads as NoRecordFound, indistinguishable from absence #213 asks for the eight deployer hex-decode collapses plus
  the missing-field collapses; covered, all eight named individually plus the
  five null-field cases and both meta-count cases. process_meta_query reports corrupt payloads, bad hex and GraphQL errors all as NoRecordFound, indistinguishable from absence #234 asks for four items —
  (1) absent data, (2) null data.meta staying absence, (3) corrupt
  rawBytes, (4) the never-inspected errors member; covered 1, 2, 3, 4, with
  (4) applied to the deployer query as well, which process_meta_query reports corrupt payloads, bad hex and GraphQL errors all as NoRecordFound, indistinguishable from absence #234 did not ask for but
  process_deployer_query reports corrupt subgraph payloads as NoRecordFound, indistinguishable from absence #213's data path needs.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

Next included review available in 18 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: 9d0fd797-348e-49e9-a3d6-b42cccb7090d

📥 Commits

Reviewing files that changed from the base of the PR and between 3a2e0cf and ccd0437.

📒 Files selected for processing (2)

• crates/cli/src/error/mod.rs
• crates/cli/src/meta/query/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

• X
• Mastodon
• Reddit
• LinkedIn

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

[github-actions]

@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment:

S/M/L PR Classification Guidelines:

This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed.

Small (S)

Characteristics:

• Simple bug fixes, typos, or minor refactoring
• Single-purpose changes affecting 1-2 files
• Documentation updates
• Configuration tweaks
• Changes that require minimal context to review

Review Effort: Would have taken 5-10 minutes

Examples:

• Fix typo in variable name
• Update README with new instructions
• Adjust configuration values
• Simple one-line bug fixes
• Import statement cleanup

Medium (M)

Characteristics:

• Feature additions or enhancements
• Refactoring that touches multiple files but maintains existing behavior
• Breaking changes with backward compatibility
• Changes requiring some domain knowledge to review

Review Effort: Would have taken 15-30 minutes

Examples:

• Add new feature or component
• Refactor common utility functions
• Update dependencies with minor breaking changes
• Add new component with tests
• Performance optimizations
• More complex bug fixes

Large (L)

Characteristics:

• Major feature implementations
• Breaking changes or API redesigns
• Complex refactoring across multiple modules
• New architectural patterns or significant design changes
• Changes requiring deep context and multiple review rounds

Review Effort: Would have taken 45+ minutes

Examples:

• Complete new feature with frontend/backend changes
• Protocol upgrades or breaking changes
• Major architectural refactoring
• Framework or technology upgrades

Additional Factors to Consider

When deciding between sizes, also consider:

• Test coverage impact: More comprehensive test changes lean toward larger classification
• Risk level: Changes to critical systems bump up a size category
• Team familiarity: Novel patterns or technologies increase complexity

Notes:

• the assessment must be for the totality of the PR, that means comparing the base branch to the last commit of the PR
• the assessment output must be exactly one of: S, M or L (single-line comment) in format of: SIZE={S/M/L}
• do not include any additional text, only the size classification
• your assessment comment must not include tips or additional sections
• do NOT tag me or anyone else on your comment