Skip to content

test(conformance): let accepted_rejection disposition an empty-projection Err (#903)#904

Merged
nh13 merged 1 commit into
mainfrom
test/issue-903-empty-projection-divergence
Jul 2, 2026
Merged

test(conformance): let accepted_rejection disposition an empty-projection Err (#903)#904
nh13 merged 1 commit into
mainfrom
test/issue-903-empty-projection-divergence

Conversation

@nh13

@nh13 nh13 commented Jul 1, 2026

Copy link
Copy Markdown
Member

Closes #903.

Problem

AxisTally::record could give no typed disposition to an empty-projection Err (ferro produced no output on an axis): accepted_divergence/known_bug/spec_citation are gated behind actual.is_ok(), and accepted_rejection explicitly excluded the EMPTY_PROJECTION_SENTINEL. So a row where ferro spec-correctly declines but the comparator emits a value — e.g. #891's NM_000143.3:c.-1_1insCAT (ferro declines protein per the #857 non-CDS policy; mutalyzer emits p.(=)) — could only land in the raw baseline-failures/<axis>.txt ledger, never a typed bucket. This is why ~18 protein_description rows can't leave that ledger, blocking #326's "every ledger empty" close condition.

Fix

An empty projection is a decline, and accepted_rejection is already the Err-on-projection-axis disposition — its only blocker was the sentinel exclusion. So (adversarial review picked this home over grafting a second Err-exception onto accepted_divergence):

  • Add RejectionReason::NonCdsNoProjection857 + a closed RejectionReason::disposition_empty_projection() predicate (_ => false, so opting in is a deliberate reviewed choice, never an ad-hoc flag).
  • Relax the record() accepted_rejection condition to admit the sentinel only when the reason opts in.

Safety — no masked regressions (verified)

  • The empty_got increment is unconditional at the top of record(), above every disposition/return, so the #764-pinned empty-projection count gate in finish still catches any rise regardless of bucketing.
  • Disjoint by construction: the opt-in block requires the sentinel; every non-opt-in reason keeps the pre-test(conformance): let accepted_divergence disposition an empty-projection Err (unblocks #326 close-path) #903 hard-FAIL.
  • XPASS / Ok-transition honest: accepted_rejection buckets only Err, so if ferro starts emitting a value it is not masked — an Ok that matches XPASS-FAILs (stale annotation caught), an Ok that mismatches hard-FAILs (ferro stopped declining).
  • A genuine panic still hard-FAILs.

Payoff

Gives the ~18 empty-projection protein_description rows a typed, XPASS-guarded disposition instead of raw ledger entries — advancing #326's close-path (option (b) in the #326 close-condition caveat) and unblocking their conversion in the #861 sweep.

Scope

Mechanism + hermetic tests only — no src/ production code (projector/normalizer), no corpus/ledger change. Converting the real rows (#891's row + the ~17 NG_ init-codon rows) to this typed disposition is a follow-up re-bless (#861 / an amend to #900), kept out here so the mechanism lands conflict-free with the in-flight cases.json PRs.

Verification

Full suite 7683 passed; new hermetic tests cover: opt-in buckets an empty Err (+ count still increments), non-opt-in still FAILs, opt-in still buckets a non-empty Err, Ok-mismatch still FAILs, and the predicate/serde round-trip. clippy -D warnings + fmt clean.

Summary by CodeRabbit

  • New Features

    • Added support for a new accepted-rejection reason that can treat empty-projection results as accepted in specific cases.
  • Bug Fixes

    • Improved rejection handling so opted-in cases no longer hard-fail on empty-projection sentinel errors.
    • Preserved existing failure behavior for non-opted-in cases and for normal mismatches.
    • Updated matching behavior and coverage to confirm the new routing and serialization work correctly.

@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 23ecd004-5b1d-4b6e-9373-981c5a8c313a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Walkthrough

Adds a RejectionReason::NonCdsNoProjection857 variant with a disposition_empty_projection() opt-in method, updates as_str() and docs accordingly, and modifies AxisTally::record in the conformance test harness so empty-projection sentinel Errs bucket into divergence_accepted only when the reason opts in, adding corresponding unit tests.

Changes

Empty-projection opt-in disposition

Layer / File(s) Summary
RejectionReason variant and opt-in method
src/conformance/mutalyzer.rs
Adds NonCdsNoProjection857 variant, disposition_empty_projection() method returning true only for it, updates as_str() mapping, revises AcceptedRejection docs, and adds unit tests for opt-in behavior and serde round-trip.
AxisTally bucketing logic and comparator tests
tests/it/mutalyzer_normalize_tests.rs
Changes accepted_rejection match condition to permit sentinel Err bucketing into divergence_accepted only when disposition_empty_projection() is true, and adds tests for opt-in, non-opt-in, and Ok-mismatch scenarios.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

Poem

  • A sentinel Err once forced a FAIL,
  • now a reason can bend that tale.
  • disposition_empty_projection true,
  • lets divergence_accepted pull it through.
  • The count gate still stands watch nearby—
  • no silent value-to-empty slips by.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and matches the main conformance change around empty-projection Err disposition.
Linked Issues check ✅ Passed PR #903 adds the opt-in empty-projection Err path and keeps the count gate and non-opt-in hard fails intact.
Out of Scope Changes check ✅ Passed The diff stays within the conformance harness mechanism and tests, with no unrelated production or corpus updates.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test/issue-903-empty-projection-divergence

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

@nh13

nh13 commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai resume

@nh13

nh13 commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown
✅ Action performed

Reviews resumed.

@nh13

nh13 commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai resume

@nh13

nh13 commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown
✅ Action performed

Reviews resumed.

@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@nh13

nh13 commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@nh13

nh13 commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@nh13

nh13 commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@nh13

nh13 commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/it/mutalyzer_normalize_tests.rs`:
- Around line 573-590: The contract comment for record() is outdated because it
still says accepted_rejection only buckets non-empty-projection Errs and that
the empty-projection sentinel always hard-fails, which no longer matches the
logic in the accepted_rejection check. Update the comment near record() to
reflect the opt-in exception via disposition_empty_projection(), using the
accepted_rejection, EMPTY_PROJECTION_SENTINEL, and
disposition_empty_projection() symbols so the documented behavior matches the
implemented branch.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 30adc43a-427b-479f-a6db-a2af54dfe956

📥 Commits

Reviewing files that changed from the base of the PR and between ef475e2 and fb66d2e.

📒 Files selected for processing (2)
  • src/conformance/mutalyzer.rs
  • tests/it/mutalyzer_normalize_tests.rs

Comment thread tests/it/mutalyzer_normalize_tests.rs Outdated
@nh13 nh13 force-pushed the test/issue-903-empty-projection-divergence branch from fb66d2e to b275cb1 Compare July 2, 2026 14:27
…tion Err (#903)

An empty-projection Err (ferro produces no output on an axis) could get no typed
disposition: accepted_divergence/known_bug/spec_citation are gated behind
actual.is_ok(), and accepted_rejection explicitly excluded the
EMPTY_PROJECTION_SENTINEL — so a row where ferro spec-correctly declines but the
comparator emits a value (e.g. #891's c.-1_1insCAT: ferro declines protein per
the #857 non-CDS policy, mutalyzer emits p.(=)) could only land in the raw
baseline-failures ledger, never a typed bucket. This is why ~18
protein_description rows can't leave that ledger, blocking #326's close condition.

An empty projection is a *decline*, and accepted_rejection is already the
Err-on-projection-axis disposition — its only blocker was the sentinel exclusion.
So: add a RejectionReason::NonCdsNoProjection857 variant + a closed
disposition_empty_projection() predicate, and relax the record() exclusion to
admit the sentinel only for an opt-in reason. This reuses the existing decline
home rather than adding a second Err-exception to accepted_divergence.

Safety: the empty_got increment is unconditional at the top of record() (above
all returns), so the #764-pinned empty-projection COUNT gate still catches any
rise regardless of bucketing; the XPASS guard still fires if ferro starts
matching; a genuine panic still hard-FAILs; a non-opt-in reason keeps the
pre-#903 hard-FAIL (disjoint by construction).

Mechanism + hermetic tests only — no src production code, no corpus/ledger
change. Converting the real rows (#891 + the ~17 init-codon rows) to this typed
disposition is a follow-up re-bless (#861 / amend #900). Full suite 7683 passed;
clippy -D warnings + fmt clean.
@nh13 nh13 force-pushed the test/issue-903-empty-projection-divergence branch from b275cb1 to a26f367 Compare July 2, 2026 14:42
@nh13 nh13 merged commit 0af029f into main Jul 2, 2026
9 checks passed
@nh13 nh13 deleted the test/issue-903-empty-projection-divergence branch July 2, 2026 14:45
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.

test(conformance): let accepted_divergence disposition an empty-projection Err (unblocks #326 close-path)

1 participant