fix(retrieval): close beta.5 fallback gaps - #637
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR records beta.5 corrective-phase governance, validates active package receipts, tightens repository-tool attribution, updates MCP instructions, and expands workflow parsing and retrieval coverage. ChangesCore Reset corrective phase
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/scripts/verify-packed-retrieval-parity.mjs:
- Around line 61-70: The receipt selected by the active package validation must
include npm_artifact_sha256 before SHA-256 comparison. Update the corrective
package_candidate data or receipt-selection logic around
activePhase?.corrective?.package_candidate and the validation loop so the exact
active artifact receipt supplies all required fields, while preserving the
existing manifest.current fallback contract.
In `@tests/unit/retrieve-context.test.ts`:
- Around line 171-201: Add an assertion in the ready-state comparison block to
verify fifth.dossier.query.subject matches first.dossier.query.subject,
alongside the existing fifth flow and evidence comparisons.
In `@tools/eval/core-reset/verify-isolation.mjs`:
- Around line 74-75: Update the drift diagnostic near the replacementReceipt
validation in verify-isolation.mjs so it does not imply the receipt came from
the active corrective measurement. Use a source-neutral “replacement receipt
drifted” message, or derive the label from whichever source selected
replacementReceipt, while preserving the existing validation behavior.
🪄 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: CHILL
Plan: Pro Plus
Run ID: b17ef22b-a585-411d-bf88-cc4b06908d95
📒 Files selected for processing (12)
.github/scripts/verify-packed-retrieval-parity.mjsdocs/core-reset/removal-manifest.ymlsrc/adapters/mcp/protocol.tssrc/domain/query/plan.tstests/unit/compare.test.tstests/unit/core-reset-governance.test.tstests/unit/mcp-schema-budget.test.tstests/unit/query-plan.test.tstests/unit/retrieve-context.test.tstests/unit/stdio-server.test.tstools/eval/core-reset/verify-isolation.mjstools/eval/lib/infrastructure/compare.ts
|
Exact-head merge gate receipt
The reviewed head is unchanged and the protected base remains its ancestor. Squash merge is authorized by the satisfied gates. |
Summary
Root cause
The workflow subject parser required text after work, so the exact owner prompt bound the wrong subject. Separately, ready-state MCP guidance did not explicitly prohibit repository reads, and the evaluator only rejected repository tools before Madar.
Validation
Scope
Production changes are exactly two existing files at +8/-8/net 0. No dependency, lockfile, graph, index, result-schema, retrieval-budget, package-ceiling, publication, release, tag, Registry, or main change.
Refs #631
Refs #629
Summary by CodeRabbit
New Features
Bug Fixes
Tests