CF-7 Remediation Phase 3: exhaustive-category heuristic hardening - #61
Conversation
Implements review item #4: BuildExhaustiveAnswer's entity-scoped vs. category-wide classification is a heuristic (minDocumentFrequency < 50% of cards), not a proof, and the Grading Spec has flagged since 2026-07-04 that a genuinely category-wide question with no hyphenated identifier could be misclassified as entity-scoped by corpus coincidence -- with no regression test proving it. Investigation finding: this risk is currently LATENT, not live. All 15 Exhaustive questions in the actual 150-question suite name a hyphenated identifier and are fully handled by Tier 1c's verbatim anchor match -- the fallback heuristic this hardening targets is never reached by any question in the current corpus. - Added a regression test reproducing the exact failure with a synthetic 10-card corpus: a category-wide question where only 3 of 10 relevant cards happen to use the literal word being scored (the other 7 use different phrasing), causing the heuristic to hard- require that word and silently drop the 7 differently-phrased cards. Confirms the documented risk is real, not just theoretical. - Added FabricBenchmarkQuestion.ExhaustiveIsEntityScopedOverride (nullable bool): an optional authored ground-truth annotation. BuildExhaustiveAnswer uses it directly when set, bypassing the heuristic entirely -- the "pre-compute ground-truth classification" resolution the Grading Spec had tracked as not-yet-implemented. Proven by a second test using the identical fixture with the override set, confirming all 10 cards match correctly. - Deliberately NOT backfilled onto the existing 150-question suite -- every current question is already correctly handled by Tier 1c, so setting the override on them would be a no-op. It's there for future question authors to set explicitly on any new hyphen-free Exhaustive question, not retrofitted onto questions that don't need it. - Docs: Grading Spec §5.3 known-limitation note updated to reflect the mitigation and its scope; Bug History §7d records the investigation.
|
Warning Review limit reached
Next review available in: 51 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a nullable exhaustive-question classification override, applies it in ChangesExhaustive classification remediation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
Both docs claimed the fallback unigram classification path was never
reached by any live question. Wrong -- DeterministicFabricCorpus.cs's
own exhaustive-archive-tokens question ("List every archive token in
section order", no hyphenated identifier) exercises it on every
cf7-gate run. It currently classifies correctly only because every
segment's text literally contains "archive token", giving that term
100% document frequency in that specific corpus -- nowhere near the
<50% threshold that triggers misclassification. Corrected both docs:
the heuristic path is live and currently correct by fortunate corpus
construction, not unreached; the boundary-case FAILURE MODE (not the
heuristic path itself) is what remains untriggered by any current
question, which is what the new regression test reproduces
synthetically.
The two doc fixes corrected the "never reached by any live question" claim; this code comment made the identical overclaim and was missed.
…eview) §9 still said the boundary case had no regression test, contradicted by the two Phase 3 tests and §5.3's rewrite earlier in the same commit range.
Fourth and hopefully final instance of the "fallback heuristic isn't reached by any live question" overclaim, this time in FabricBenchmarkQuestion's own XML doc comment. Did a repo-wide grep for the phrase pattern afterward to confirm no more instances remain.
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
Summary
Implements review item #4:
BuildExhaustiveAnswer's entity-scoped vs. category-wide classification is a heuristic (minDocumentFrequency < 50% of cards), not a proof, and the Grading Spec has flagged since 2026-07-04 that a genuinely category-wide question with no hyphenated identifier could be misclassified — with no regression test proving it.Investigation finding: this risk is currently latent, not live. All 15 Exhaustive questions in the actual 150-question suite name a hyphenated identifier and are fully handled by Tier 1c's verbatim anchor match — the fallback heuristic this hardening targets is never reached by any question in the current corpus.
FabricBenchmarkQuestion.ExhaustiveIsEntityScopedOverride(nullable bool): an optional authored ground-truth annotation.BuildExhaustiveAnsweruses it directly when set, bypassing the heuristic — the "pre-compute ground-truth classification" resolution the Grading Spec had tracked as not-yet-implemented. Proven by a second test with the identical fixture, confirming all 10 cards match once set.Test plan
dotnet buildclean acrossContextFabricBenchandOrchestratorIDE.Avaloniadotnet test— 561/565 pass (4 pre-existing GPU-conditional skips), including both new tests, which passed on first try (confirms the fixture accurately reproduces the bug and the fix genuinely resolves it, not just asserting a tautology)ContextFabricExhaustiveAnswerTestspass unmodified🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests
Documentation