Fix PR body sync scope and root-portable handoff test - #1830
Merged
Conversation
…-workflow-safety-230-296
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
7 tasks
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe PR updates CI policy-change detection, prevents generation-fallback answers from cache use, updates related tests, revises issue records, and changes handoff-stop failure coverage for directory marker paths. ChangesCI policy routing
RAG fallback cache handling
Repository maintenance updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: dependency version conflict. Check your lock file or package.json. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/ci-change-scope.mjs (1)
365-365: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the obsolete test-only option.
classifyno longer readsprPolicyBodyPresent, butassertScopestill passesprPolicyBodyPresent: falseat Line 577. Remove the stale property and any overrides. This keeps self-test inputs aligned with the diff-based contract.Proposed cleanup
function assertScope(name, files, expected, options = {}) { - const result = classify(files, { readLedger: emptyLedger, prPolicyBodyPresent: false, ...options }); + const result = classify(files, { readLedger: emptyLedger, ...options });This finding is based on the supplied
classifyandassertScopeimplementations.🤖 Prompt for 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. In `@scripts/ci-change-scope.mjs` at line 365, Remove the obsolete prPolicyBodyPresent option from classify-related test inputs, including the prPolicyBodyPresent: false property passed by assertScope and any other overrides. Keep classify’s existing readLedger option and diff-based behavior unchanged.
🤖 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.
Nitpick comments:
In `@scripts/ci-change-scope.mjs`:
- Line 365: Remove the obsolete prPolicyBodyPresent option from classify-related
test inputs, including the prPolicyBodyPresent: false property passed by
assertScope and any other overrides. Keep classify’s existing readLedger option
and diff-based behavior unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: aa131f36-0b2b-4433-a394-b2ceda2905e7
📒 Files selected for processing (5)
.github/workflows/ci.ymldocs/outstanding-issues.mdscripts/ci-change-scope.mjstests/ci-cache-safety.test.tstests/pr-handoff-stop.test.ts
Owner
Author
Babysit PR summary
|
Owner
Author
|
Review sweep complete for PR #1830 (HEAD fb71ce3).
|
Owner
Author
Codex babysit summary (scope babysit)
No merge blockers were surfaced by this sweep. |
Owner
Author
Babysit review summary
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens CI/workflow behavior around PR-policy body syncing, makes the PR handoff “marker write failure” contract test portable across root/non-root Linux runs, and hardens RAG caching so provider-generation fallback answers are not cached/served.
Changes:
- Gate the “Sync PR policy body” workflow on
PR_POLICY_BODY.mdbeing present in the current PR diff (not merely present on the branch filesystem). - Make the handoff-stop hook test portable by forcing marker write failure via a directory at the marker path (works for both root and non-root).
- Prevent caching/serving provider-generation fallback answers by updating route-budget cache eligibility and shared-cache eviction behavior, with focused test coverage.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/rag-shared-cache.test.ts | Adds a regression test asserting shared cache does not serve generation-fallback answers and deletes the shared row. |
| tests/rag-route-budget.test.ts | Updates cache-eligibility helper signature and adds a test ensuring generation-fallback answers are never cacheable even before route deadlines. |
| tests/rag-answer-fallback.test.ts | Replaces direct fallback-cacheability assertions with route-budget cache eligibility checks for generation-fallback answers. |
| tests/pr-handoff-stop.test.ts | Makes marker-write failure test root-portable by using a directory at the marker path and validating fail-open behavior. |
| tests/ci-cache-safety.test.ts | Updates assertions to match the new CI scope output name (pr_policy_body_changed). |
| src/lib/rag/rag.ts | Routes all cache writes through the updated cache-eligibility predicate (including excluding generation-fallback answers). |
| src/lib/rag/rag-route-budget.ts | Extends cache-eligibility logic to reject generation-fallback results (deadline-aware + answer-aware). |
| src/lib/rag/rag-cache.ts | Detects and evicts generation-fallback answers from shared cache reads (delete row + treat as miss). |
| scripts/ci-change-scope.mjs | Changes CI scope output from pr_policy_body_present to pr_policy_body_changed based on the PR diff file list. |
| docs/outstanding-issues.md | Archives/resolves the referenced ledger items and updates the outstanding issues tables accordingly. |
| .github/workflows/ci.yml | Updates workflow wiring/condition to use pr_policy_body_changed for the PR-policy body sync job. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Owner
Author
Babysit unblock summary — PR #1830Tip: Sync / merge-tree: Merged What blocked
What I fixed / dispositioned
Required CI: Change scope, PR policy, PR mergeability (and security jobs) in progress on this tip after the push. Auto-merge is not armed. Residual risks
|
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
BigSimmo
enabled auto-merge
August 12, 2026 06:35
Contributor
There was a problem hiding this comment.
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 `@src/lib/rag/rag-route-budget.ts`:
- Around line 61-62: Update the routing checks in the rag-route budget logic to
recognize bare, case-insensitive, delimiter-aware generation_fallback markers in
both routingReason and optional degradedReason, matching the behavior used by
rag-cache.ts. Ensure routingReason alone cannot bypass the deadline when
degradedMode is absent, and add coverage for that case alongside existing tests.
🪄 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: dfc3fe5a-1bc1-4580-beaf-9a346b23e06f
📒 Files selected for processing (7)
docs/outstanding-issues.mdsrc/lib/rag/rag-cache.tssrc/lib/rag/rag-route-budget.tssrc/lib/rag/rag.tstests/rag-answer-fallback.test.tstests/rag-route-budget.test.tstests/rag-shared-cache.test.ts
Align answerRouteResultCanBeCached with isProviderGenerationDegraded so routingReason markers without a :reason suffix (and without degradedMode) cannot be cached. Addresses CodeRabbit on PR #1830. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Owner
Author
CodeRabbit thread resolvedTip: Fixed the open CodeRabbit finding on
|
Contributor
CI triageCI failed on this PR. Automated classification of the 3 failed job(s):
Compared with main CI run #9819 (cancelled). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
Inline the generation_fallback marker in answerRouteResultCanBeCached so importing rag-route-budget from tests does not freeze the offline vitest env snapshot via rag-answer-support → deep-memory → owner-scope. Also resetModules after stubEnv in the fallback helper so provider mocks always re-parse env. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Owner
Author
Babysit CI fix — PR #1830Tip: Merge-tree: clean vs What blocked required CI
Cause
Fix
Proof
Merge left to auto-merge once required checks go green. |
Merged
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR_POLICY_BODY.mdto appear in the current PR diff before body sync can run, so inherited scratch bodies cannot overwrite unrelated PR descriptionsgeneration_fallbackanswers fromrag_response_cachewrites and hits so stale source-only fallbacks cannot stick after the provider recovers (fix(ci): bump checkout/setup-node/cache actions to v5 #231 partial)RAG impact: no retrieval behaviour change — answer-cache write/hit exclusion for generation_fallback only; retrieval selection and ranking are untouched.
Verification
npm run verify:pr-local— partial: runtime, installed-lock parity, formatting, documentation, ledger, workflow/policy contracts, lint, and related static gates passed; typecheck initially could not acquire the repository coordinator and then passed separatelynpm run typecheck— passednode scripts/run-vitest.mjs run tests/ci-cache-safety.test.ts— 31 passed, 11 skippednpm run check:ci-scope— passednpm run check:github-actions— passednpm run check:pr-policy— passednpm run check:outstanding-issues— passed after main merge conflict resolutionROOT_PORTABLE_MARKER_FAILURE_PASS status=0 marker_type=directory stdout=emptyfor UID 1000 and UID 0UI verification not run: no UI, routing, styling, or browser behavior changed by this PR's own commits.
Risk and rollout
Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes
origin/mainto clear the outstanding-issues conflict before mergeability/required CI can proceed.Summary by CodeRabbit
Bug Fixes
Documentation
Chores