CF-7: close the honest-corpus gate GO (open-extraction recall + exhaustive-leaf-coverage fixes) - #65
Merged
Conversation
…tly missed facts The last two live CF-7 gate runs on the honest (un-marked) expanded corpus both came back NO-GO on segment_terminal_coverage (5-11 of 128 segments rejected), and root-causing the rejected segments plus two Exhaustive-question failures traced to the same cause: dense segments pack a real planted fact among 15+ near-identical filler lines, and the open-extraction reader silently drops it. Unlike marked-mode reading, open extraction had no recall safety net at all -- worse, a segment returning zero claims short-circuited straight to rejection before the (marked-mode-only) repair logic could even run. ContextFabricFeasibilityRunner.ReadSegmentAsync now runs a parallel completeness check for open-extraction reads: GetCandidateFactSentences flags lines carrying a hyphenated alphanumeric code (BR-540, RPT-013, CASE-12-1, grade-3) -- the generic shape of a citable fact (case numbers, invoice IDs, ratings) that this corpus's planted facts all share and its filler/gap/adversarial lines never do. Any candidate not already covered by the model's claims triggers one repair call (RepairOpenExtractionSegmentAsync) that, unlike the forced marked-mode repair, explicitly lets the model decline a false-positive candidate rather than fabricate a claim -- the detector is a heuristic signal, not ground truth. Live-validated on Qwen3.5-9B-Q8_0: segment_terminal_coverage went from 123/128 to 128/128 on the exact corpus/model combination that was failing. Also fixes a misplaced code comment in ContextFabricBenchmarkGateEvaluator that described question_pass_rate's non-blocking rationale while sitting under segment_terminal_coverage (which stays fully blocking by design). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…egment set
Live-validating the open-extraction recall fix (previous commit) surfaced a
second, independent bug: exhaustive-leaf-coverage required the found
Exhaustive question's IncludedSegmentIds to equal the ENTIRE corpus segment
count. That was correct for DeterministicFabricCorpus's frozen fixture, whose
one exhaustive category ("archive tokens") is genuinely defined over every
segment -- but DeterministicExpandedFabricCorpus's 15 per-ledger exhaustive
categories each scope to their own 3-5 segments by design, so this equality
was unsatisfiable there regardless of answer quality. The live gate showed
included=4/128 with Verification.Passed=true (a flawless answer to "list
every case-file ID under ledger case-ledger-01" correctly cites its 4
segments, never all 128) and sank the gate anyway.
Now checks exhaustive.Question.ExpectedSegmentIds against IncludedSegmentIds
directly, which is correct for both fixtures.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CURRENT_STATE.yaml previously claimed CF-7 was closed GO from the 2026-07-03 run, which the adversarial pass later found was against an overclaimed (marked/leaked) corpus -- this updates it to the real result: GO against the un-marked cf-expanded-book-v1 corpus with 120 real held-out questions, Qwen3.5-9B-Q8_0 at 128/128 segments and 104/120 questions. Also marks the §7e NoKvSlot handoff doc superseded (PR #63 it describes as open merged the same day it was generated) rather than deleting it -- its root-cause narrative for the NoKvSlot crash stays useful, it just isn't a live status document anymore. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughContext Fabric adds open-extraction completeness repair and regression tests, corrects exhaustive coverage validation, and updates CF-7 benchmark and NoKvSlot handoff documentation. ChangesOpen Extraction Repair
CF-7 Benchmark Handoff Documentation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ContextFabricFeasibilityRunner
participant OpenRepairRuntime
participant SegmentValidation
ContextFabricFeasibilityRunner->>SegmentValidation: detect uncovered candidate facts
ContextFabricFeasibilityRunner->>OpenRepairRuntime: invoke read-repair-open
OpenRepairRuntime-->>ContextFabricFeasibilityRunner: return repair claims
ContextFabricFeasibilityRunner->>SegmentValidation: normalize merged evidence
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
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 `@OrchestratorIDE/Services/ContextFabric/ContextFabricFeasibilityRunner.cs`:
- Around line 352-359: Validate model-produced claims and citations before
dereferencing or merging them in the open extraction and repair paths of the
feasibility runner. Ensure null claims, null citations, and invalid
quote/segment IDs are rejected safely; when open-path repair is unusable, retain
the original draft, while valid repairs continue to merge and renumber claims.
Add a regression test covering a parseable but invalid repair response.
- Around line 1388-1391: Update the exhaustive-leaf-coverage evaluation in the
feasibility runner to inspect every exhaustive question result rather than only
the FirstOrDefault result. Collect all exhaustive results, require each to have
Verification.Passed and to contain every corresponding
Question.ExpectedSegmentIds value, and preserve the coverage detail counts using
the complete exhaustive result set.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3413afcb-4a4e-418f-8997-326cd71c25e1
📒 Files selected for processing (5)
OrchestratorIDE.UnitTests/ContextFabricOpenExtractionTests.csOrchestratorIDE/Services/ContextFabric/ContextFabricBenchmarkGateEvaluator.csOrchestratorIDE/Services/ContextFabric/ContextFabricFeasibilityRunner.csdocs/CF7_NOKVSLOT_7E_CLAUDE_HANDOFF.mddocs/CURRENT_STATE.yaml
Comment on lines
+352
to
+359
| var repairDraft = FabricJson.ParseModelObject<FabricEvidenceCard>(repair.Output); | ||
| draft = draft with | ||
| { | ||
| Claims = draft.Claims | ||
| .Concat(repairDraft.Claims) | ||
| .Select((claim, index) => claim with { ClaimId = $"c{index + 1}" }) | ||
| .ToList(), | ||
| }; |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Validate model-produced claims before traversing or merging them.
The open path dereferences raw claims/citations before validation, while both repair branches concatenate repairDraft.Claims directly. A parseable response such as claims: null, null citations, or an invalid quote/segment ID can throw outside the current catch filters or poison an otherwise valid initial draft. Validate repair output before merging and, for open extraction, retain the original draft whenever the repair is unusable. Add a parseable-invalid-repair regression test.
Also applies to: 373-403
🤖 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 `@OrchestratorIDE/Services/ContextFabric/ContextFabricFeasibilityRunner.cs`
around lines 352 - 359, Validate model-produced claims and citations before
dereferencing or merging them in the open extraction and repair paths of the
feasibility runner. Ensure null claims, null citations, and invalid
quote/segment IDs are rejected safely; when open-path repair is unusable, retain
the original draft, while valid repairs continue to merge and renumber claims.
Add a regression test covering a parseable but invalid repair response.
Comment on lines
1388
to
+1391
| new("exhaustive-leaf-coverage", | ||
| exhaustive is not null && exhaustive.Verification.Passed && | ||
| exhaustive.IncludedSegmentIds.Count == fixture.Corpus.Segments.Count, | ||
| $"included={exhaustive?.IncludedSegmentIds.Count ?? 0}/{fixture.Corpus.Segments.Count}"), | ||
| exhaustive.Question.ExpectedSegmentIds.All(exhaustive.IncludedSegmentIds.Contains), | ||
| $"included={exhaustive?.IncludedSegmentIds.Count ?? 0}/{exhaustive?.Question.ExpectedSegmentIds.Count ?? 0}"), |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Evaluate coverage for every exhaustive question.
exhaustive is obtained with FirstOrDefault, so this gate can pass when only the first of the expanded corpus’s exhaustive questions covers its expected segments. Collect all exhaustive results and require each one to pass verification and include all of its own ExpectedSegmentIds.
Proposed fix
- var exhaustive = questions.FirstOrDefault(result => result.Question.Kind == FabricQuestionKind.Exhaustive);
+ var exhaustives = questions
+ .Where(result => result.Question.Kind == FabricQuestionKind.Exhaustive)
+ .ToArray();
new("exhaustive-leaf-coverage",
- exhaustive is not null && exhaustive.Verification.Passed &&
- exhaustive.Question.ExpectedSegmentIds.All(exhaustive.IncludedSegmentIds.Contains),
- $"included={exhaustive?.IncludedSegmentIds.Count ?? 0}/{exhaustive?.Question.ExpectedSegmentIds.Count ?? 0}"),
+ exhaustives.Length > 0 && exhaustives.All(result =>
+ result.Verification.Passed &&
+ result.Question.ExpectedSegmentIds.All(result.IncludedSegmentIds.Contains)),
+ $"covered={exhaustives.Count(result => result.Verification.Passed &&
+ result.Question.ExpectedSegmentIds.All(result.IncludedSegmentIds.Contains))}/{exhaustives.Length}"),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| new("exhaustive-leaf-coverage", | |
| exhaustive is not null && exhaustive.Verification.Passed && | |
| exhaustive.IncludedSegmentIds.Count == fixture.Corpus.Segments.Count, | |
| $"included={exhaustive?.IncludedSegmentIds.Count ?? 0}/{fixture.Corpus.Segments.Count}"), | |
| exhaustive.Question.ExpectedSegmentIds.All(exhaustive.IncludedSegmentIds.Contains), | |
| $"included={exhaustive?.IncludedSegmentIds.Count ?? 0}/{exhaustive?.Question.ExpectedSegmentIds.Count ?? 0}"), | |
| var exhaustives = questions | |
| .Where(result => result.Question.Kind == FabricQuestionKind.Exhaustive) | |
| .ToArray(); | |
| new("exhaustive-leaf-coverage", | |
| - exhaustive is not null && exhaustive.Verification.Passed && | |
| - exhaustive.Question.ExpectedSegmentIds.All(exhaustive.IncludedSegmentIds.Contains), | |
| exhaustives.Length > 0 && exhaustives.All(result => | |
| result.Verification.Passed && | |
| result.Question.ExpectedSegmentIds.All(result.IncludedSegmentIds.Contains)), | |
| $"covered={exhaustives.Count(result => result.Verification.Passed && | |
| result.Question.ExpectedSegmentIds.All(result.IncludedSegmentIds.Contains))}/{exhaustives.Length}"), |
🤖 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 `@OrchestratorIDE/Services/ContextFabric/ContextFabricFeasibilityRunner.cs`
around lines 1388 - 1391, Update the exhaustive-leaf-coverage evaluation in the
feasibility runner to inspect every exhaustive question result rather than only
the FirstOrDefault result. Collect all exhaustive results, require each to have
Verification.Passed and to contain every corresponding
Question.ExpectedSegmentIds value, and preserve the coverage detail counts using
the complete exhaustive result set.
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
1 participant
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
CF-7's benchmark gate has been NO-GO on the real, un-marked expanded corpus
(
cf-expanded-book-v1, 128 segments, 120 real held-out questions) since theadversarial pass found the original 2026-07-03 GO was against an overclaimed
corpus. This closes it GO on Qwen3.5-9B-Q8_0 by fixing two independent bugs
found while live-validating the honest corpus, neither related to the §7e
NoKvSlot crash (PR #63, already merged and holding clean).
Bug 1 — open-extraction reader silently drops facts on dense segments.
ContextFabricFeasibilityRunner.ReadSegmentAsync: marked-mode reading has arepair pass that catches missed evidence lines against ground truth; open
extraction (the honest, un-marked corpus) had none. Worse, a segment that
came back with zero claims short-circuited straight to rejection before
any repair logic could run. Root cause traced to real rejected/incomplete
segments: a genuine fact (
RPT-278,BR-342, a ledger row, ...) sittingamong 15+ near-identical filler lines, silently dropped by the reader.
Fix: a heuristic completeness check (
GetCandidateFactSentences) flags linescarrying a hyphenated alphanumeric code — the generic shape every planted
fact in this corpus shares (case numbers, report IDs, ratings) and filler/gap/
adversarial lines never do. Any candidate not covered by the model's claims
triggers one repair call (
RepairOpenExtractionSegmentAsync) that — unlikethe forced marked-mode repair — explicitly lets the model decline a
false-positive candidate instead of fabricating a claim.
Bug 2 —
exhaustive-leaf-coveragegate checks a stale whole-corpusassumption. It required the found Exhaustive question's cited segments to
equal the entire 128-segment corpus. That was correct for the frozen
16-segment fixture (whose one exhaustive category genuinely spans every
segment), but the expanded corpus has 15 per-ledger exhaustive categories
each scoped to their own 3-5 segments — so a flawless answer (
included=4/128,Verification.Passed=true) sank this gate on every live run regardless ofquality. Fixed to check the question's own
ExpectedSegmentIdsinstead.Live validation
Q8_0 is the flagship-tracked quant for this benchmark; Q4_K_M's residual gap
is real but narrow and does not block this PR's primary claim.
Test plan
dotnet test OrchestratorIDE.UnitTests— 596 passed, 0 failed (3 newtests added: repair recovers a fact the first pass missed entirely,
repair can correctly decline every candidate without force-accepting,
no repair call when everything's already covered)
(
.orc/adversarial/cf7_gate_20260717_162108...json/.md)docs/CURRENT_STATE.yamland the stale §7e handoff doc updated toreflect the real current state (see the docs commit)
Do not merge
Per this session's ground rules, this PR is left open for explicit human
approval — I have not merged it.
Co-Authored-By: Claude Fable 5 noreply@anthropic.com
Summary by CodeRabbit
New Features
Bug Fixes
Documentation