feat(reads): wire intended evidence modules - #3809
Conversation
Problem Three modules deleted in PR #3390 were unreachable despite current architecture and project intent still describing their capabilities. The holdout guard also lacked a production enforcement seam. What changed Restore and wire the holdout policy into DurableRefResolver before retained result-set members are read, expose private Fable packet regeneration through the async archive facade, and resolve content-hash block anchors through Polylogue.resolve_ref. Add real-route tests and restore the empty delegation predicate path needed by cold Fable regeneration. Update API parity and generated documentation for the new facade operation. Disposition - storage/sqlite/holdout_cohorts.py: restore and wire. Bead polylogue-rxdo.9.4, PR #2888, and current planner architecture establish this as an adopted analysis-integrity policy. The production MCP reference query route now fails closed for exploratory reads of marked holdouts. - insights/fable_packet.py: restore and wire. Bead polylogue-212.9.1 and PR #2814 establish the private descriptive packet as intended capability. The async archive facade supplies the missing operator-facing production route. - storage/block_anchor.py: restore and wire. Bead polylogue-svfj and PR #2588 establish content-hash citation anchors as intended capability. Polylogue.resolve_ref now provides the existing public read surface; deferred lineage relocation states remain unchanged. Alternatives rejected Deleting all three based only on their former lack of callers would discard capabilities still claimed by the architecture and tracking history. Archive migration and lifecycle work was intentionally excluded. Compatibility/migration No schema or migration changes. The holdout guard is default-deny for exploratory retained-result reads; confirmation callers may explicitly declare confirmation through DurableRefResolver. The Fable route remains fail-closed when structural or annotation evidence is insufficient. Co-Authored-By: Claude <noreply@anthropic.com>
Problem The initial Fable facade test covered only an empty archive and therefore did not prove that the restored adapter could compose canonical delegations with durable annotation evidence. What changed Add a real archive-backed facade test that writes a delegation through the production archive writer, initializes the durable built-in schema, persists an active evidence-backed annotation, and asserts a complete packet with the selected delegation and distribution. Adversarial review The independent review identified the empty-only coverage as a legitimate gap. This test closes it. Removing the delegation query, durable schema lookup, or assertion query changes the complete result to not_supported or an empty population. Compatibility/migration Test-only change. No production schema, migration, or lifecycle behavior changed. Co-Authored-By: Claude <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 24 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 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (15)
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. Comment |
Summary
Restore and wire the three modules identified by polylogue-ic5i. Git history, Bead intent, and the current architecture all show these as intended capabilities rather than positively unwanted code. No archive migration or lifecycle work is included.
Problem
PR #3390 removed
storage/sqlite/holdout_cohorts.py,insights/fable_packet.py, andstorage/block_anchor.pybecause they had no production callers. Their prior design records show incomplete integration or an adopted contract, so deletion would remove desired behavior. The holdout policy was especially unsafe as an uncalled guard could not protect any route.Solution
storage/sqlite/holdout_cohorts.py: RESTORE + WIRE. Beadpolylogue-rxdo.9.4and PR feat(insights): rxdo.9 measurement substrate primitives #2888 establish the analysis-integrity policy.DurableRefResolver._retained_resultnow applies the guard before reading result-set members, and the registered MCP reference-query route reaches that resolver. Exploratory reads fail closed for held-out result sets; explicit confirmation remains available to confirmation callers.insights/fable_packet.py: RESTORE + WIRE. Beadpolylogue-212.9.1and PR feat(insights): regenerate Fable packets from archive #2814 establish the archive-backed cold-regeneration adapter.Polylogue.regenerate_private_fable_packetexposes it through the existing async API facade and operation-parity surface, including real delegation and annotation reads.storage/block_anchor.py: RESTORE + WIRE. Beadpolylogue-svfjand PR feat(storage): block content-hash citation anchors with typed resolver #2588 establish the content-hash citation-anchor contract.Polylogue.resolve_refnow recognizes the textual anchor form and resolves it through the existing archive read route, returning a typed block-anchor payload.No module was deleted. The deferred
relocated_lineageandquarantinedanchor states remain unchanged because this task does not establish their production semantics.Holdout enforcement and anti-vacuity
The guard is enforced in the production evaluation path, before the sole
get_result_set_members()call.tests/unit/archive/query/test_evaluator.py::test_retained_holdout_result_set_fails_closed_before_members_are_readproves the resolver ordering.tests/unit/mcp/test_reference_query_pipeline.py::test_mcp_query_refuses_an_exploratory_read_of_a_holdout_result_setseeds durable result-set data and exercises the registered MCP query handler. Removingrequire_non_holdout_accesschanges that route from a typed refusal to a successful member read, so the tests do not merely assert a helper or a test-only registry.The Fable facade test uses a real temporary archive with persisted delegations and labels. The block-anchor test resolves an actual stored block through
run_archive_readand checks its content hash.Verification
Focused real-route tests:
Quick gate on the rebased head:
devtools verify --alland the integration suite were not run. The focused suite covers the changed production routes and the quick gate covers formatting, typing, generated surfaces, layering, and policy checks.Review notes
An independent adversarial review traced the MCP route to
DurableRefResolverand found one legitimate gap: the initial Fable facade test covered only an empty archive. Commit0813d4415adds a non-empty persisted delegation and label case, then the focused suite and quick gate were rerun on the rebased head. A nested cold review process later hit the checkout guard because it bypasseddirenv; it emitted no additional actionable finding.