CF-4 hierarchy and cognitive paging - #13
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughPhase CF-4 adds SQLite schema migration 11 ( ChangesCF-4 Hierarchy and Cognitive Paging
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 8
🧹 Nitpick comments (1)
OrchestratorIDE/Services/ContextFabric/FabricLibraryRepository.cs (1)
85-102: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winBatch
GetSegmentsByIdsinstead of looping throughGetSegment.This helper still does one SQL round trip per unique ID. On the new reopen/evidence path, that turns a bounded segment fetch into avoidable N+1 queries. A single
INquery plus in-memory reordering would keep the same semantics.🤖 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/FabricLibraryRepository.cs` around lines 85 - 102, `FabricLibraryRepository.GetSegmentsByIds` still issues one `GetSegment` lookup per unique id, causing an N+1 query pattern. Update this method to fetch all requested segment ids in a single batch query (for example, via the repository/data access path used by `GetSegment`) and then de-duplicate and preserve the requested semantics in memory, returning only non-null matches from the batched result.
🤖 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.UnitTests/ContextFabricCf4Tests.cs`:
- Around line 87-112: The
EvidencePackBuilder_Respects_8k_Budget_And_Reserves_Response_Tokens test is not
forcing any evidence trimming, so it never validates the excluded-evidence path.
Update the test around FabricQueryPlanner.BuildPlan and
EvidencePackBuilder.Build by either lowering the prompt budget or expanding the
seeded claims so the pack exceeds budget, then assert that pack.Excluded is
non-empty instead of using a tautological count check. Keep the existing budget
and reserve assertions, but make the test explicitly cover the trimming behavior
that EvidencePackBuilder is supposed to record.
In `@OrchestratorIDE/Services/ContextFabric/DocumentGraphRepository.cs`:
- Around line 263-290: Validate and reject invalid hierarchy memberships before
writing them in ReplaceMemoryNodesForDocument and the related
InsertMemoryMembershipOn path. Right now only the node DocumentId is checked,
while memberships can still point to a parent/child pair that crosses documents
or references a missing/dangling child; add explicit validation that each
membership belongs to the same document/corpus as the current replacement set
and that the parent-child relationship is consistent before calling
InsertMemoryNodeOn or InsertMemoryMembershipOn. Reuse the existing symbols
ReplaceMemoryNodesForDocument and InsertMemoryMembershipOn so the checks happen
at the persistence boundary, not by rewriting ParentNodeId during insert.
- Around line 81-95: The claim listing cap in ListClaimsForDocument is too low
for the CF-4 reducer contract, which expects up to 4,096 claims per document.
Update the Math.Clamp limit in DocumentGraphRepository.ListClaimsForDocument to
allow the higher maximum (or otherwise align it with the reducer’s documented
cap), and make sure the query still bounds the request safely while returning
the full claim set needed by the reducer.
In `@OrchestratorIDE/Services/ContextFabric/EvidencePackBuilder.cs`:
- Around line 90-91: The token estimate in
EvidencePackBuilder.EvaluateBudget/EstimateTokens is using a whitespace-based
word count, which can undercount minified JSON, CSV, or code blocks and make
WithinBudget unreliable. Replace the EstimateTokens logic with a real token
counter used by the prompt budget path, and ensure the budget check in the
evidence pack flow uses that same tokenizer so MaxPromptTokens is enforced
accurately.
In `@OrchestratorIDE/Services/ContextFabric/FabricCitationVerifier.cs`:
- Around line 78-85: Negation handling in FabricCitationVerifier currently runs
after overlap scoring, so contradicted claims can be mislabeled as Supported or
PartiallySupported. In FabricCitationVerifier’s verification logic, check for
negation mismatches before computing overlap and before returning on overlap
thresholds, and make the contradiction test symmetric for claimText and
matchedQuotes (not just quotes containing " not " while the claim does not).
In `@OrchestratorIDE/Services/ContextFabric/FabricQueryPlanner.cs`:
- Around line 24-25: Normalize and validate inputs in BuildPlan before any
search or plan assembly: trim and canonicalize query, corpusId, and mode once,
then use the normalized values consistently for Search and the returned plan so
the plan matches what was queried. Update BuildPlan and any related mode
handling to reject or classify unknown explicit mode strings instead of echoing
them through, and ensure only the supported "study" mode preserves reopen
behavior while all other invalid modes fall back to classification or are
rejected.
- Around line 116-127: The ClassifyMode logic in FabricQueryPlanner is matching
study terms as substrings, which incorrectly upgrades unrelated queries to Study
mode. Update ClassifyMode to tokenize the query and compare whole words against
the study keyword set instead of using Contains on the lowercase string, so
terms like “show” or “exchange” do not trigger on embedded fragments. Keep the
existing mode decision in FabricQueryMode, but change the matching strategy
around ClassifyMode so only standalone keywords such as compare, across,
between, change, exception, why, and how are considered.
In `@OrchestratorIDE/Services/ContextFabric/FabricReducer.cs`:
- Around line 46-52: The parent span is being truncated because FabricReducer
only propagates a child’s first ordinal, so generation > 0 nodes build their
range from the last child’s start instead of its full end. Update the reduction
flow in FabricReducer so ReductionChild carries both start and end ordinals,
then use those values when computing the parent node’s DisplayName and NodeId in
the node-building logic. Also adjust CreateLeaf to initialize both ordinals to
the segment index so leaf spans are correct from the start.
---
Nitpick comments:
In `@OrchestratorIDE/Services/ContextFabric/FabricLibraryRepository.cs`:
- Around line 85-102: `FabricLibraryRepository.GetSegmentsByIds` still issues
one `GetSegment` lookup per unique id, causing an N+1 query pattern. Update this
method to fetch all requested segment ids in a single batch query (for example,
via the repository/data access path used by `GetSegment`) and then de-duplicate
and preserve the requested semantics in memory, returning only non-null matches
from the batched result.
🪄 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: b37110c1-8991-4e5b-bee9-fcb6279d1ee9
📒 Files selected for processing (11)
OrchestratorIDE.Avalonia/OrchestratorIDE.Avalonia.csprojOrchestratorIDE.UnitTests/ContextFabricCf4Tests.csOrchestratorIDE/Services/ContextFabric/ContextFabricIngestionContracts.csOrchestratorIDE/Services/ContextFabric/DocumentGraphRepository.csOrchestratorIDE/Services/ContextFabric/EvidencePackBuilder.csOrchestratorIDE/Services/ContextFabric/FabricCitationVerifier.csOrchestratorIDE/Services/ContextFabric/FabricLibraryRepository.csOrchestratorIDE/Services/ContextFabric/FabricQueryPlanner.csOrchestratorIDE/Services/ContextFabric/FabricReducer.csOrchestratorIDE/Services/Data/Migrations.csdocs/The Orc Context Fabric.md
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
OrchestratorIDE/Services/ContextFabric/FabricCitationVerifier.cs (1)
73-78: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDon’t derive contradiction from the union of all matched quotes.
Line 77 treats the entire citation set as contradicted if any matched quote contributes
"not". That produces false negatives for multi-citation claims, or for a long quote that contains an unrelated negated sentence. For example,"The harbor route is open."would be markedContradictedagainst"The ridge route is not safe. The harbor route is open."Proposed fix
var claimTokens = Tokenize(claimText); - var sourceTokens = Tokenize(string.Join(" ", matchedQuotes)); - if (claimTokens.Count == 0 || sourceTokens.Count == 0) + var quoteTokens = matchedQuotes + .Select(Tokenize) + .Where(tokens => tokens.Count > 0) + .ToArray(); + if (claimTokens.Count == 0 || quoteTokens.Length == 0) return FabricCitationVerificationLabel.Interpretive; - if (claimTokens.Contains("not") != sourceTokens.Contains("not")) + + if (quoteTokens.Any(tokens => + claimTokens.Count(tokens.Contains) / (double)claimTokens.Count >= 0.50 && + claimTokens.Contains("not") != tokens.Contains("not"))) return FabricCitationVerificationLabel.Contradicted; + + var sourceTokens = quoteTokens + .SelectMany(tokens => tokens) + .ToHashSet(StringComparer.Ordinal); var overlap = claimTokens.Count(sourceTokens.Contains) / (double)claimTokens.Count;🤖 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/FabricCitationVerifier.cs` around lines 73 - 78, The contradiction check in FabricCitationVerifier’s citation verification logic is too coarse because it tokenizes the joined matchedQuotes set and flags Contradicted whenever any quote contains “not.” Update the logic around Tokenize(...) and the claimTokens/sourceTokens comparison so contradiction is determined per matched quote or per sentence-level match, not from the union of all quotes. Keep the existing Interpretation/Contradicted flow in FabricCitationVerificationLabel, but only return Contradicted when the specific quote supporting the claim actually negates it; otherwise fall back to the non-contradicted result.
🤖 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.
Outside diff comments:
In `@OrchestratorIDE/Services/ContextFabric/FabricCitationVerifier.cs`:
- Around line 73-78: The contradiction check in FabricCitationVerifier’s
citation verification logic is too coarse because it tokenizes the joined
matchedQuotes set and flags Contradicted whenever any quote contains “not.”
Update the logic around Tokenize(...) and the claimTokens/sourceTokens
comparison so contradiction is determined per matched quote or per
sentence-level match, not from the union of all quotes. Keep the existing
Interpretation/Contradicted flow in FabricCitationVerificationLabel, but only
return Contradicted when the specific quote supporting the claim actually
negates it; otherwise fall back to the non-contradicted result.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: de8e583b-a880-42e2-95e4-b0d2d8ea7740
📒 Files selected for processing (6)
OrchestratorIDE.UnitTests/ContextFabricCf4Tests.csOrchestratorIDE/Services/ContextFabric/DocumentGraphRepository.csOrchestratorIDE/Services/ContextFabric/EvidencePackBuilder.csOrchestratorIDE/Services/ContextFabric/FabricCitationVerifier.csOrchestratorIDE/Services/ContextFabric/FabricQueryPlanner.csOrchestratorIDE/Services/ContextFabric/FabricReducer.cs
🚧 Files skipped from review as they are similar to previous changes (3)
- OrchestratorIDE/Services/ContextFabric/DocumentGraphRepository.cs
- OrchestratorIDE/Services/ContextFabric/FabricQueryPlanner.cs
- OrchestratorIDE/Services/ContextFabric/FabricReducer.cs
This was referenced Jun 29, 2026
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
Verification command
Files changed
Scope explicitly excluded
Confirmation
Summary by CodeRabbit
New Features
Bug Fixes
Documentation