Skip to content

fix(review): publish the diagnostic contract on stage failures and coverage gaps - #43

Open
Kizunad wants to merge 2 commits into
mainfrom
fix/diagnostic-contract
Open

Kizunad wants to merge 2 commits into
mainfrom
fix/diagnostic-contract

Conversation

@Kizunad

@Kizunad Kizunad commented Aug 10, 2026

Copy link
Copy Markdown
Owner

Summary

Every distinct failure tonight collapsed to the same string — after 3 attempts: claude exited 1. This PR records what a stage failure and a coverage gap actually were, end to end:

  • runner annotates each terminal result with model, attempts, retryable (schema-error exhaustion is retryable: false — the runner misreading the model, not the model failing)
  • cli adds a structural terminalReason tier (closed enum) that surfaces even for CLI-level errors, without opening the restricted event.result envelope unless it is a confirmed api_error with integer status 100–599
  • orchestrator passes apiErrorStatus / apiErrorMessage / terminalReason / model / attempts / retryable through stageFailure and coverageGap, and the finder failure census now collapses distinct statuses (apiErrorStatuses) and models (models) across the dead batches
  • review-entry shape-checks all six fields at the public boundary and drops malformed values instead of publishing them; coverage gaps now carry the bounded stage diagnostic so a gapped lens is nameable
  • final-review.schema.json extends failures.items and coverageGaps.items so the artifact contract validates the additive fields (additionalProperties: false would otherwise reject them)

Test plan

  • 223 pass / 0 fail / 6 skip (merged main baseline 220/0/6; briefed baseline 205/0/6)
  • New: aggregate census test (distinct 402+503 → apiErrorStatuses: [402, 503]), public-boundary junk-dropping test, coverage-gap structured-field passthrough tests, schema shape assertions

🤖 Generated with Claude Code

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Aug 10, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

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

Next review available in: 23 minutes

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

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: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d50bfafe-bc92-43ca-95f5-883eba365386

📥 Commits

Reviewing files that changed from the base of the PR and between ae90091 and 24cb49e.

📒 Files selected for processing (11)
  • schemas/final-review.schema.json
  • src/claude-cli.mjs
  • src/claude-runner.mjs
  • src/orchestrator.mjs
  • src/review-entry.mjs
  • src/review-limits.mjs
  • test/claude-cli.test.mjs
  • test/claude-runner.test.mjs
  • test/orchestrator.test.mjs
  • test/review-entry.test.mjs
  • test/schemas.test.mjs

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.

@Kizunad

Kizunad commented Aug 10, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 10, 2026 •

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

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.

server_kizuna and others added 2 commits August 10, 2026 15:37
…verage gaps

Every distinct failure tonight collapsed to "after 3 attempts: claude exited 1".
This records what a stage failure and a coverage gap actually were, at the
runner, through the orchestrator, and at the public boundary:

- runner annotates each terminal result with model, attempts, and retryable
- cli extracts a structural terminalReason tier (closed enum) without touching
  the restricted event.result envelope unless it is a confirmed api_error
- orchestrator passes apiErrorStatus/apiErrorMessage/terminalReason/model/
  attempts/retryable through stageFailure and coverageGap, and collapses
  distinct statuses and models into the finder failure census
- review-entry shape-checks all six fields at the public boundary and drops
  malformed values instead of publishing them; coverage gaps now carry the
  stage diagnostic so a gapped lens is nameable
- final-review.schema.json extends failures and coverageGaps items so the
  artifact contract validates the additive fields

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: ENGINE-DIAGNOSTIC-CONTRACT
The contract's whole point is that an exhausted record names its own
cause - 503 vs 402 vs 400 all collapsed to "after N attempts: claude
exited 1". Assert the fields surface on the surviving record for each
infra class: status, model, attempts, retryable. The 402 balance class
was the only named cause with no explicit test; it now has one.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: w13-diagnostic-contract
@Kizunad
Kizunad force-pushed the fix/diagnostic-contract branch from c136e96 to 24cb49e Compare August 10, 2026 07:58
@Kizunad

Kizunad commented Aug 10, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 10, 2026 •

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

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.

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.

1 participant