Skip to content

Split the empty-result error into by-hash and by-subject variants - #262

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

Split the empty-result error into by-hash and by-subject variants#262
thedavidmeister wants to merge 1 commit into
mainfrom
2026-08-25-issue-179

Conversation

@thedavidmeister

@thedavidmeister thedavidmeister commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Closes #179

The defect

MetaboardSubgraphClientError::Empty(String) carried one message for two different query keys:

#[error("Subgraph query returned no data for metahash {0}")]
Empty(String),

get_metabytes_by_hash fills it with the metahash, but get_metabytes_by_subject fills it with subject.0, so an empty subject query rendered Subgraph query returned no data for metahash 0x315 where 0x315 was the subject argument.

The fix

Split it the way RequestErrorByHash / RequestErrorBySubject in the same enum already are:

#[error("Subgraph query returned no data for metahash {metahash}")]
EmptyByHash { metahash: String },
#[error("Subgraph query returned no data for subject {subject}")]
EmptyBySubject { subject: String },

Each construction site now names the key it actually queried on. The one consumer, DotrainSourceV1::fetch_by_subject, matched Empty(_) to map an empty result to Ok(None); it now matches EmptyBySubject { .. }, the only empty variant that method can produce.

Public API change: Empty(String) is gone from rain-metaboard-subgraph's public error enum, replaced by the two variants above. That is the "per-key variant" option the issue's triage framing named; the alternative it named — neutral wording (for query key {0}) — keeps the variant but gives up what the intent oracle asks for, a message that identifies what was queried. No shim is kept for the old variant. Version bumps are the release workflow's job, so none here.

QA

  • Discriminating tests: metaboard_client::tests::test_get_metabytes_by_subject_empty — it now asserts the rendered Display string, which is where the defect lived (the payload was already the subject on base, so the previous payload-only assertion could not see it). It cannot be run verbatim on base, because EmptyBySubject does not exist there; base behaviour was reproduced in-tree by mutation, below, and the test fails on it with the issue's exact reported string. test_get_metabytes_by_hash_empty also gained a Display assertion but is NOT discriminating for this issue — by-hash rendering was already correct on base; it is a regression guard for the split.
  • Mutations applied:
    • crates/metaboard/src/metaboard_client.rs:27 #[error("...no data for subject {subject}")] -> base's wording #[error("...no data for metahash {subject}")] -> killed by test_get_metabytes_by_subject_empty: left: "Subgraph query returned no data for metahash 0x315", right: "Subgraph query returned no data for subject 0x315".
    • crates/metaboard/src/metaboard_client.rs:115 construction EmptyBySubject { subject: subject.0.clone() } -> EmptyByHash { metahash: subject.0.clone() } -> killed by test_get_metabytes_by_subject_empty, same left/right.
    • crates/cli/src/meta/types/dotrain/source_v1.rs:59 arm EmptyBySubject { .. } -> EmptyByHash { .. } -> killed by the pre-existing source_v1::tests::test_fetch_by_subject_not_found (assertion failed: result.is_ok()), so the retargeted arm is covered.
  • Oracle: the issue's intent oracle — an error message identifies what was queried — plus this enum's own established wording, where RequestErrorByHash says "metahash" and RequestErrorBySubject says "subject" for the same two keys. The expected strings were written from that pair, not read back off the #[error] attributes under test.
  • Category check: the issue asks one thing — the Empty Display labels a subject value as a metahash — and it is covered. Its open triage note ("the variant shape is a public API") is a design call, ruled here in favour of the per-key split with the reasoning in the Public API change paragraph above, for a reviewer to override.

Run locally in the flake shell: cargo fmt --check, cargo clippy -p rain-metaboard-subgraph -p rain-metadata --all-targets -- -D warnings, cargo test -p rain-metaboard-subgraph --lib (12 passed) and cargo test -p rain-metadata --lib source_v1 (15 passed). The full suite was not run locally; CI covers it.

Closes #179

MetaboardSubgraphClientError::Empty(String) rendered "no data for
metahash {0}" for both queries, so an empty subject query labelled the
subject value as a metahash. Split it the way RequestErrorByHash /
RequestErrorBySubject already are, so each message names the key it
actually queried on.

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 49 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: 88dc154c-47f3-422f-9551-f7b4fea91eba

📥 Commits

Reviewing files that changed from the base of the PR and between 45ca96c and 7ec94e4.

📒 Files selected for processing (2)
  • crates/cli/src/meta/types/dotrain/source_v1.rs
  • crates/metaboard/src/metaboard_client.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.

Empty-result error Display labels a subject query value as a metahash

1 participant