Skip to content

fix(retrieval): close beta.5 fallback gaps - #637

Merged
mohanagy merged 2 commits into
nextfrom
agent/issue-631-beta5-followup
Aug 2, 2026
Merged

fix(retrieval): close beta.5 fallback gaps#637
mohanagy merged 2 commits into
nextfrom
agent/issue-631-beta5-followup

Conversation

@mohanagy

@mohanagy mohanagy commented Aug 2, 2026

Copy link
Copy Markdown
Owner

Summary

  • parse the exact beta.5 manual workflow prompt as idea report with explicit generate-request, pipeline, and saved-report bounds
  • make ready MCP dossiers terminal for clients and constrain non-ready recovery to named gaps
  • fail no-fallback attribution on repository or unclassified tools while allowing neutral ToolSearch discovery
  • preserve immutable beta.5 and issue 630 receipts while recording the corrective package and source measurements

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

  • real GoValidate graph: incident, control, and three saved-report variants are ready 7/7 with identical flow and evidence
  • 914/914 full tests passed with coverage thresholds
  • frozen 14-prompt issue 630 benchmark passed; negative mutations rejected
  • typecheck, production build, evaluator build, registry validation, release hygiene, audit, isolation, and packed parity passed
  • package: 102 files / 155253 packed / 653988 unpacked bytes
  • replacement: 1424 source LOC / 60983 emitted bytes
  • three independent exact-head reviews found no blocker

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

    • MCP guidance now clearly explains readiness, dossier-only answers, repository-tool restrictions, and named-gap handling.
    • Workflow planning better recognizes varied request phrasing, optional verbs, saved-report commands, and compound terms.
    • Repository tool attribution now covers additional read-only and discovery actions.
  • Bug Fixes

    • Improved consistency when interpreting paraphrased report-flow requests.
    • Package and replacement validation now use the active corrective receipt, reducing stale-result and artifact mismatch errors.
  • Tests

    • Expanded coverage for query planning, MCP instructions, attribution rules, and release qualification checks.

@coderabbitai

coderabbitai Bot commented Aug 2, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f8b7932b-f7f6-4e53-8795-340a21accddf

📥 Commits

Reviewing files that changed from the base of the PR and between 2b34513 and e9557bc.

📒 Files selected for processing (1)
  • tools/eval/core-reset/verify-isolation.mjs
🚧 Files skipped from review as they are similar to previous changes (1)
  • tools/eval/core-reset/verify-isolation.mjs

📝 Walkthrough

Walkthrough

The 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.

Changes

Core Reset corrective phase

Layer / File(s) Summary
Corrective phase governance contract
docs/core-reset/removal-manifest.yml, tests/unit/core-reset-governance.test.ts
The manifest records beta.5 publication evidence and corrective qualification data. Governance tests validate ancestry, source scope, measurements, budgets, package state, and diff hashes.
MCP and workflow query contracts
src/adapters/mcp/protocol.ts, src/domain/query/plan.ts, tests/unit/mcp-schema-budget.test.ts, tests/unit/query-plan.test.ts, tests/unit/retrieve-context.test.ts, tests/unit/stdio-server.test.ts
MCP instructions define one question per retrieval, dossier-only ready responses, and named-gap handling. Workflow parsing and report-flow tests cover optional subjects, saved reports, compound terminals, and paraphrased queries.
Repository tool attribution validation
tools/eval/lib/infrastructure/compare.ts, tests/unit/compare.test.ts
Attribution classifies repository, neutral, and unclassified tools. Madar verification rejects repository or unclassified calls and tests cover discovery and post-retrieval exploration.
Active corrective receipt validation
.github/scripts/verify-packed-retrieval-parity.mjs, tools/eval/core-reset/verify-isolation.mjs
Package and replacement measurements use active corrective receipts, with fallback candidates. Artifact and receipt mismatch messages identify the active receipt or artifact.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

  • mohanagy/madar#635: Shares the package-parity, isolation, MCP, query-planning, retrieval, and governance areas.
  • mohanagy/madar#636: Extends beta.5 release governance with active corrective package receipt validation.
  • mohanagy/madar#624: Overlaps in package-receipt validation and expected artifact measurement.

Suggested labels: core-reset

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the changes and validation, but it omits the required Testing, Core Reset contract, checklist, and related-issue sections. Add the required template sections and complete their fields, including test checkboxes, Core Reset contract values, scope checks, checklist items, and related issues.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the retrieval fallback fixes and the beta.5 scope.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ 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 agent/issue-631-beta5-followup

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

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

📥 Commits

Reviewing files that changed from the base of the PR and between 81045cc and 2b34513.

📒 Files selected for processing (12)
  • .github/scripts/verify-packed-retrieval-parity.mjs
  • docs/core-reset/removal-manifest.yml
  • src/adapters/mcp/protocol.ts
  • src/domain/query/plan.ts
  • tests/unit/compare.test.ts
  • tests/unit/core-reset-governance.test.ts
  • tests/unit/mcp-schema-budget.test.ts
  • tests/unit/query-plan.test.ts
  • tests/unit/retrieve-context.test.ts
  • tests/unit/stdio-server.test.ts
  • tools/eval/core-reset/verify-isolation.mjs
  • tools/eval/lib/infrastructure/compare.ts

Comment thread .github/scripts/verify-packed-retrieval-parity.mjs
Comment thread tests/unit/retrieve-context.test.ts
Comment thread tools/eval/core-reset/verify-isolation.mjs
@mohanagy

mohanagy commented Aug 2, 2026

Copy link
Copy Markdown
Owner Author

Exact-head merge gate receipt

  • Base: protected next at 81045cc (tree 9d3d90fe7feb3e55042c4f38ac8925abca6e83d1)
  • Reviewed head: e9557bc
  • Reviewed tree: 4500d416d0e890b1dc67c9228a2cbb49cf4220a3
  • Required CI: all six jobs green on run 30744121576
  • CodeRabbit: passed on the exact head; three findings reviewed, replies recorded, zero unresolved threads
  • Independent review: no blocker on the exact head/tree
  • Mergeability: CLEAN / MERGEABLE
  • Target: next only
  • Publication: not authorized and not performed

The reviewed head is unchanged and the protected base remains its ancestor. Squash merge is authorized by the satisfied gates.

@mohanagy
mohanagy merged commit 6872916 into next Aug 2, 2026
7 checks passed
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