Skip to content

Harden the eval-canary safeguards: per-case rr diff, dispatch failure issues, contract pins - #1843

Merged
BigSimmo merged 5 commits into
mainfrom
claude/rag-canary-test-review-seprbt
Aug 12, 2026
Merged

Harden the eval-canary safeguards: per-case rr diff, dispatch failure issues, contract pins#1843
BigSimmo merged 5 commits into
mainfrom
claude/rag-canary-test-review-seprbt

Conversation

@BigSimmo

@BigSimmoBigSimmo commented Aug 12, 2026

Copy link
Copy Markdown
Owner

Summary

  • Reviewed the RAG eval-canary infrastructure end-to-end (workflow, golden eval, snapshot/tuning tooling, docs/rag-behaviour/) and found the mechanism sound but with manual and silent gaps; this PR mechanizes and pins them without changing any retrieval behaviour.
  • scripts/compare-retrieval-eval.ts: new per-case rr@10/contentRR@10 comparison with a --fail-on-regression flag, mechanizing the canary-pair protocol's "zero per-case rr regressions" gate that previously depended on eyeballing two tee'd logs. It fails closed on summary-only artifacts and on non-identical case sets, and prints each regression as REGRESSION <case> <metric>: <baseline> -> <candidate>.
  • .github/workflows/eval-canary.yml: the failure issue now opens for failed repository_dispatch runs too (a red dispatch is usually half of a canary pair gating a behaviour change and previously failed silently); the issue body now also includes answer-targeting.log and the validate_override/answer_targeting step outcomes, and distinguishes weekly from dispatched runs.
  • scripts/build-ranking-snapshot.ts: fails loudly when a hardNegativeTemplatescaseId matches no artifact case — previously a renamed golden case silently dropped its hard negatives. The CLI entry is now guarded so tests can import convertArtifact.
  • New contract tests: tests/build-ranking-snapshot.test.ts (hard-negative attachment + orphan guard) and tests/clinical-aliases-tiering.test.ts (pins the strict alias-tier key sets so a bulk merge of the wide tier in src/lib/eval-document-matching.ts into scripts/lib/clinical-aliases.ts can no longer pass CI — previously comment-enforced only).
  • docs/rag-behaviour/README.md + safeguards.md: corrected stale claims that the canary runs via workflow_dispatch or a ref dispatch input (both are deliberately absent and asserted absent by tests/eval-canary-workflow.test.ts); the pair protocol now documents the per-case compare command and that post runs are post-merge only.
  • docs/outstanding-issues.md: captured the snapshot-freshness handoff (Remove Evidence-based and All sources chips from answer footer #304 — the ranking snapshot dated 2026-07-20 trips the 30-day gate ~2026-08-19 and needs an operator-sent eval-canary repository dispatch to regenerate) and deferred canary observations (fix(db): repair Supabase Preview migration replay failures #305 — no latency-mode coverage, cost readout is a lower bound, concurrency-group pair straddling).

RAG impact: no retrieval behaviour change — canary tooling, workflow failure reporting, documentation corrections, and contract-test hardening only; the golden fixture, ranking code, comparator chains, and alias tables are byte-identical.

Verification

  • npm run verify:pr-local — routed fail-closed to the heavy path; all static gates, lint, and typecheck passed; unit suite 1 failed | 6082 passed | 4 skipped where the single failure is the pre-existing environmental root-container case tracked as ledger #296 (tests/pr-handoff-stop.test.ts, reproduced on a clean checkout previously; CI runners are non-root and unaffected).
  • npm run eval:rag:offlineOffline RAG fixture and production-contract checks passed. (23 files, 574 tests).
  • Focused suites for every changed surface: tests/compare-retrieval-eval.test.ts, tests/eval-canary-workflow.test.ts, tests/build-ranking-snapshot.test.ts, tests/clinical-aliases-tiering.test.ts, plus untouched guards tests/ranking-tuning.test.ts and tests/rag-imputation-contract.test.ts — 6 files, 40 tests, all passed.
  • CLI proof of the new gate on synthetic artifacts: a regressed pair prints REGRESSION a reciprocalRankAt10: 1.0000 -> 0.3300 and exits 1; an identical pair prints zero per-case rr regressions and exits 0.
  • UI verification not run: no UI, routing, styling, or browser behaviour changed.
  • npm run eval:retrieval:quality not run: retrieval, ranking, selection, chunking, and scoring behaviour are untouched (tooling/docs/tests only); the source-pin contract test and offline RAG fixture checks above are green and unchanged.

Risk and rollout

  • Risk: low — offline tooling, workflow failure-reporting, documentation, and tests; no runtime, ranking, or clinical answer path changes. The strictest new behaviour is fail-closed tooling (the compare gate and the snapshot orphan guard) that can only turn silent passes into loud failures.
  • Rollback: single-commit revert; nothing depends on the new compare flag or guards yet.
  • Provider or production effects: None in this diff. The follow-up snapshot refresh (ledger Remove Evidence-based and All sources chips from answer footer #304) is provider-backed (~$1–2 eval-canary dispatch) and was explicitly user-approved on 2026-08-12, but must be sent by an operator identity since the workflow accepts only repository_dispatch.

Clinical Governance Preflight

  • Source-backed claims still require linked source verification before clinical use
  • No patient-identifiable document workflow was introduced or expanded without explicit governance approval
  • Supabase target remains Clinical KB Database (sjrfecxgysukkwxsowpy)
  • Service-role keys and private document access remain server-only
  • Demo/synthetic content remains clearly separated from real clinical sources
  • Source metadata, review status, and outdated/unknown-source behavior remain conservative
  • Deployment classification/TGA SaMD impact was checked when clinical decision-support behavior changed

Notes

  • The snapshot freshness gate (tests/ranking-tuning.test.ts) starts failing the whole unit suite around 2026-08-19. The refresh is approved but blocked on an operator-sent dispatch: gh api repos/bigsimmo/database/dispatches -f event_type=eval-canary, then regenerate with npm run build:ranking-snapshot -- --input <golden-retrieval.json> --output scripts/fixtures/rag-ranking-candidate-snapshot.v1.json --source-run-id <run-id>. Full handoff recorded as ledger #304.

🤖 Generated with Claude Code

https://claude.ai/code/session_01UoHPMZ1FinSYzH7rJ42xTo


Generated by Claude Code

Summary by CodeRabbit

  • New Features

    • Added per-case retrieval quality comparisons for document and content rankings.
    • Validation can now fail when regressions, missing cases, or incomplete results are detected.
    • Ranking snapshot generation now reports missing hard-negative cases clearly.
  • Bug Fixes

    • Improved evaluation failure reports with trigger details, diagnostic logs, and step outcomes.
    • Corrected evaluation issue wording for scheduled and dispatched runs.
  • Documentation

    • Updated canary procedures, safeguards, review records, and outstanding issue tracking to reflect the revised validation process.
  • Tests

    • Expanded coverage for ranking snapshots, retrieval comparisons, workflow failures, and clinical alias tiering.

… issues, contract pins
Review of the RAG canary infrastructure (2026-08-12) found the mechanism sound
but with manual/silent gaps. This change mechanizes and pins them; no retrieval
behaviour changes.
- compare-retrieval-eval: new per-case rr@10/contentRR@10 diff with
--fail-on-regression, mechanizing the canary-pair protocol's 'zero per-case
rr regressions' gate that was previously eyeballed from tee'd logs; fails
closed on summary-only artifacts and non-identical case sets.
- eval-canary.yml: open the failure issue on failed dispatches too (a red
dispatch is usually half a canary pair), and feed answer-targeting.log,
validate_override and answer_targeting outcomes into the issue body.
- build-ranking-snapshot: fail loudly when a hardNegativeTemplates caseId
matches no artifact case (a renamed golden case previously dropped its hard
negatives silently); CLI guarded so tests can import convertArtifact.
- New contract tests: hard-negative attachment guard, and a strict/wide
alias-tier pin so a bulk merge of src/lib/eval-document-matching.ts into
scripts/lib/clinical-aliases.ts can no longer pass CI.
- docs/rag-behaviour: fix stale claims that the canary has workflow_dispatch /
a ref input (both deliberately absent); document the per-case compare command.
- outstanding-issues: capture the snapshot-freshness dispatch handoff (#304)
and deferred canary observations (#305).
RAG impact: no retrieval behaviour change — canary tooling, docs, and
contract-test hardening only; golden fixture and ranking code untouched.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UoHPMZ1FinSYzH7rJ42xTo
@supabase

supabaseBot commented Aug 12, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@coderabbitai

coderabbitaiBot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in:7 minutes

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 802a5e31-b2b6-4797-ac1b-acf7d6c40d56

📥 Commits

Reviewing files that changed from the base of the PR and between a462ab5 and ea357ca.

📒 Files selected for processing (6)
  • .github/workflows/eval-canary.yml
  • docs/branch-review-ledger.md
  • docs/outstanding-issues.md
  • docs/rag-behaviour/safeguards.md
  • scripts/compare-retrieval-eval.ts
  • tests/compare-retrieval-eval.test.ts
📝 Walkthrough

Walkthrough

The PR changes eval-canary failure reporting and dispatch rules. It adds hard-negative snapshot validation and per-case retrieval regression checks. Tests cover workflow diagnostics, snapshot cases, ranking comparisons, and clinical alias tiering. Documentation records safeguards and follow-up issues.

Changes

Eval canary validation

Layer / File(s)Summary
Canary triggers and failure diagnostics
.github/workflows/eval-canary.yml, docs/rag-behaviour/*, tests/eval-canary-workflow.test.ts, docs/branch-review-ledger.md, docs/outstanding-issues.md
The canary uses repository dispatch and default-branch execution. Failure issues include trigger-specific wording, evaluation logs, answer-targeting logs, and step outcomes. Documentation and ledgers record the safeguards and follow-up items.
Ranking snapshot integrity
scripts/build-ranking-snapshot.ts, tests/build-ranking-snapshot.test.ts
convertArtifact is exportable for tests. It rejects orphaned hard-negative templates and only runs the CLI entry point during direct execution.
Per-case regression enforcement
scripts/compare-retrieval-eval.ts, tests/compare-retrieval-eval.test.ts, docs/rag-behaviour/safeguards.md, tests/clinical-aliases-tiering.test.ts
The comparator checks document and content rr@10 values, missing cases, and missing result arrays. The CLI can fail on regressions. Tests cover comparisons, case mismatches, and strict alias-tier contracts.

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

Sequence Diagram(s)

sequenceDiagram
participant RepositoryDispatch
participant EvalCanaryWorkflow
participant CompareRetrievalEval
participant FailureIssue
RepositoryDispatch->>EvalCanaryWorkflow: start default-branch canary
EvalCanaryWorkflow->>CompareRetrievalEval: compare JSON evaluation artifacts
CompareRetrievalEval-->>EvalCanaryWorkflow: report regressions and case-set differences
EvalCanaryWorkflow->>FailureIssue: create or update failure issue with diagnostics
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (3 warnings)

Check nameStatusExplanationResolution
Linked Issues check⚠️ WarningThe PR does not address the directly linked issue's UI objectives for removing answer-footer chips and preserving scope-dismiss focus behavior [#304].Link the PR to the correct eval-canary issue, or implement the answer-footer chip removal and related focus-restoration objectives from #304.
Out of Scope Changes check⚠️ WarningThe PR changes eval-canary tooling, workflows, tests, and documentation, while the linked issue requires answer-footer UI changes [#304].Retarget the PR to an issue covering eval-canary hardening, or remove these changes and implement the UI scope defined in #304.
Docstring Coverage⚠️ WarningDocstring coverage is 27.27% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly summarizes the main safeguards added for eval-canary comparison, failure reporting, and contract enforcement.
Description check✅ PassedThe description follows the template and documents scope, verification, risk, rollout, governance, and known limitations.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/rag-canary-test-review-seprbt

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

Comment threadscripts/compare-retrieval-eval.ts Outdated

@chatgpt-codex-connectorchatgpt-codex-connectorBot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:bcf357a96f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment threadscripts/compare-retrieval-eval.ts Outdated
@BigSimmo

Copy link
Copy Markdown
OwnerAuthor

@codex resolve actionable Codex review findings for this pull request and current head using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. The only repository destination is BigSimmo/Database, and the only branch destination is the pull request head branch claude/rag-canary-test-review-seprbt at starting commit a462ab5; never publish fixes to a detached or synthetic work branch and never create a stacked pull request. Use the authenticated GitHub connector to commit each approved fix to BigSimmo/Database:claude/rag-canary-test-review-seprbt, then verify that the pull request head contains the published commit before reporting success. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. For a fixed thread, reply with as the first line and as the second line. For a no-code disposition, use followed by . These result markers authorize the workflow to close that exact thread only after it verifies a fixed commit is the pull request head; a local-only commit is not a fix. If publication or verification fails, do not use either result marker, do not claim success, and leave the thread open with the blocker. If human input or new authorization is required, do the same. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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 @.github/workflows/eval-canary.yml:
- Around line 200-203: Update the “Open or update canary failure issue” step to
guard the dynamic import of productivity-core.mjs, falling back to a fixed
diagnosis derived from recorded step outcomes when checkout or the analyzer is
unavailable. Preserve the existing analyzer-based diagnosis when the import
succeeds, and ensure the fallback still reaches the issue creation or comment
update calls.
In `@scripts/compare-retrieval-eval.ts`:
- Around line 155-165: Update the per-case comparison around readCaseRank so
absent or non-finite baseline/candidate metrics are recorded as non-comparable
instead of skipped, allowing the gate to fail closed rather than reporting zero
regressions. Update the --fail-on-regression condition around the final result
handling to fail when either missingInCandidate or missingInBaseline is
non-empty, and add tests covering both incomplete metric comparisons and
candidate-only case IDs.
🪄 Autofix

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: CHILL

Plan: Pro

Run ID: ef92f7c1-4cb6-4624-ab6e-05feee95b074

📥 Commits

Reviewing files that changed from the base of the PR and between 6727930 and a462ab5.

📒 Files selected for processing (11)
  • .github/workflows/eval-canary.yml
  • docs/branch-review-ledger.md
  • docs/outstanding-issues.md
  • docs/rag-behaviour/README.md
  • docs/rag-behaviour/safeguards.md
  • scripts/build-ranking-snapshot.ts
  • scripts/compare-retrieval-eval.ts
  • tests/build-ranking-snapshot.test.ts
  • tests/clinical-aliases-tiering.test.ts
  • tests/compare-retrieval-eval.test.ts
  • tests/eval-canary-workflow.test.ts

Comment thread.github/workflows/eval-canary.yml
Comment threadscripts/compare-retrieval-eval.ts Outdated
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🎉

Reviewed commit:a462ab52e2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens the repo’s eval-canary safeguards and related RAG evaluation tooling by mechanizing previously manual checks (per-case RR diffs), improving failure reporting for canary runs (including dispatch runs), and adding contract tests/docs to prevent silent drift—without changing retrieval/ranking behavior.

Changes:

  • Add per-case rr@10 / contentRR@10 comparisons (with a fail-closed mode) to enforce “zero per-case regressions” mechanically.
  • Improve eval-canary failure issue creation/update to include dispatched runs, additional logs, and step outcome context.
  • Add contract tests and snapshot-building guards to prevent silent fixture/template drift; update docs and ledgers accordingly.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
FileDescription
tests/eval-canary-workflow.test.tsExtends workflow contract coverage to assert dispatch failures also open issues and include more diagnostic context.
tests/compare-retrieval-eval.test.tsAdds unit coverage for the new per-case rank comparison helper.
tests/clinical-aliases-tiering.test.tsPins strict alias-tier keysets to prevent accidental widening of golden-eval ground truth.
tests/build-ranking-snapshot.test.tsAdds tests ensuring hard-negative templates attach and that orphan templates fail loudly.
scripts/compare-retrieval-eval.tsImplements per-case RR comparison and --fail-on-regression gate behavior.
scripts/build-ranking-snapshot.tsExports convertArtifact, adds orphan hard-negative template guard, and prevents CLI execution on import.
docs/rag-behaviour/safeguards.mdUpdates canary-pair protocol to reflect dispatch-only triggering and the new per-case comparison command.
docs/rag-behaviour/README.mdCorrects stale claims about how eval-canary is triggered and what it can run against.
docs/outstanding-issues.mdCaptures new operational follow-ups (#304/#305) and bumps issues:next-id.
docs/branch-review-ledger.mdRecords this review/work as a ledger entry.
.github/workflows/eval-canary.ymlOpens/updates failure issues for scheduled and dispatched runs and includes additional logs + step outcomes.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment threadscripts/compare-retrieval-eval.ts
Comment threaddocs/rag-behaviour/safeguards.md Outdated
@BigSimmo
BigSimmo enabled auto-merge August 12, 2026 10:25
…t-review-seprbt
# Conflicts:
#	docs/outstanding-issues.md
…e analyzer import
Review fixes for PR #1843 (sentry, codex, coderabbit, copilot all converged on
the first item):
- compare-retrieval-eval --fail-on-regression now fails on candidate-only
cases (missingInBaseline) and on absent/non-finite per-case rank metrics,
not just baseline-only cases — a superset candidate could previously pass
the identical-case-set gate. CLI-level exit-code tests added.
- eval-canary.yml failure-issue step falls back to a fixed diagnosis when the
analyzer import fails (e.g. checkout failed), so a red dispatch still opens
an issue.
- safeguards.md: canary compare command moved to a fenced block so it
copy/pastes intact.
RAG impact: no retrieval behaviour change — eval tooling and workflow only.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UoHPMZ1FinSYzH7rJ42xTo
@BigSimmo
BigSimmo merged commit 34a2f9f into mainAug 12, 2026
88 checks passed
@BigSimmo
BigSimmo deleted the claude/rag-canary-test-review-seprbt branch August 12, 2026 10:39
BigSimmo pushed a commit that referenced this pull request Aug 13, 2026
The Codex auto-resolve pass pushed its own version of the PR #1843 review
fixes after the PR had already merged with the same fixes in ea357ca (which
main now carries in stronger form: separate unavailableMetrics channel and
CLI exit-code tests). Content is taken wholly from the current branch;
5ef02f7 is kept in history only so the branch can push without a rewrite.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UoHPMZ1FinSYzH7rJ42xTo
Sign up for freeto 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.

3 participants

@BigSimmo@claude