CF-6: distributed Context Fabric readers (first slice) - #15
Merged
Conversation
Stage segments as content-addressed corpus artifacts and dispatch them to
HIVE workers as Context Fabric reader work units.
- WorkUnit.DependsOn + HiveTaskQueue lease/claim dependency barrier so a
reduce unit can wait on its reader units (AreDependenciesSatisfied).
- Register theorc.context-fabric@1.0.0 pack (NativeAgent execution kind, but
dispatch bypasses the generic agent loop).
- HiveWorkerAgent: PackId-gated reader branch that fetches the staged corpus
from /hive/artifacts/{digest} (digest re-verified locally, fail-closed),
rebuilds a single-segment FabricCorpus, and runs
ContextFabricFeasibilityRunner.ReadCorpusAsync via the native role adapter.
- CampaignTemplates.StageReaderCorpusAsync + ContextFabricReaders: split a
corpus into single-segment artifacts and build one reader unit per segment.
- ContentAddressedStore.ComputeSha256(bytes) for download verification.
Tests: staging round-trip/content-addressing, template wiring, dependency
barrier, and reader execution end-to-end via the scripted runtime.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds dependency-barrier handling for campaign work units and expands CF-6 Context Fabric across contracts, staging, execution, persistence, acceptance tooling, and tests. ChangesCF-6 Context Fabric Reader, Reducer, and Dependency Barrier
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
OrchestratorIDE/Services/Hive/HiveTaskQueue.cs (1)
600-635: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReject invalid dependency IDs during
SubmitCampaign().
SubmitCampaign()copiesunit.DependsOnstraight onto the bundle without checking that each referenced work unit actually exists in the same campaign. A typoed dependency then becomes permanently unsatisfied inAreDependenciesSatisfied(), so the campaign can never finish.🤖 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/Hive/HiveTaskQueue.cs` around lines 600 - 635, SubmitCampaign currently copies each work unit’s DependsOn values directly into the HiveTaskBundle without validating that every referenced work unit exists in the same CampaignDefinition. Update SubmitCampaign (and the bundle creation path around HiveTaskBundle/AreDependenciesSatisfied) to verify all dependency IDs are present among definition.WorkUnits before registering the campaign, and reject the submission with a clear exception if any dependency is missing or invalid.
🧹 Nitpick comments (1)
OrchestratorIDE.UnitTests/ContextFabricReaderWorkUnitTests.cs (1)
53-61: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThis test doesn't actually prove the ordinal sort.
TwoSegmentCorpus()already returns[seg-a, seg-b]in ordinal order, so these assertions still pass ifStageReaderCorpusAsyncdropsOrderBy(s => s.Ordinal). Make the fixture intentionally unsorted to lock in the staging-order contract.Test hardening
private static FabricCorpus TwoSegmentCorpus() { var fixture = DeterministicFabricCorpus.Create(); const string textA = "EVIDENCE: The reactor core runs at nine hundred kelvin."; const string textB = "EVIDENCE: The archive ledger is sealed in cabinet forty-two."; var a = new FabricSegment("seg-a", 1, "Reactor", textA, FabricHashing.Sha256(textA), 12); var b = new FabricSegment("seg-b", 2, "Archive", textB, FabricHashing.Sha256(textB), 14); - return fixture.Corpus with { Segments = [a, b], EstimatedSourceTokens = 26 }; + return fixture.Corpus with { Segments = [b, a], EstimatedSourceTokens = 26 }; }Also applies to: 156-163
🤖 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.UnitTests/ContextFabricReaderWorkUnitTests.cs` around lines 53 - 61, The StageReaderCorpusAsync ordering test is currently using an already-sorted TwoSegmentCorpus fixture, so it does not verify the Ordinal-based sort contract. Update the relevant test setup in ContextFabricReaderWorkUnitTests to build an intentionally unsorted corpus and keep the assertions on the staged names, so the test fails if CampaignTemplates.StageReaderCorpusAsync stops ordering by Ordinal.
🤖 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/Hive/HiveNativeRoleExecutorAdapter.cs`:
- Around line 94-106: `ExecuteContextFabricReaderAsync` should reject corpora
with anything other than exactly one segment before any work starts. Add an
upfront guard using `corpus.Segments.Count == 1` near the beginning of this
method, and throw a clear `InvalidOperationException` (or equivalent) if the
contract is violated; keep the existing `ReadCorpusAsync` and
`SingleOrDefault()` flow only for the valid single-segment case.
In `@OrchestratorIDE/Services/Hive/HiveTaskQueue.cs`:
- Around line 438-455: The AreDependenciesSatisfied helper in HiveTaskQueue only
treats dependency tasks as satisfied when they are "completed", which can wedge
dependents forever if an upstream task ends in "failed", "cancelled", or
"timeout". Update the dependency-check flow around AreDependenciesSatisfied and
the campaign terminal handling in UpdateCampaignAfterTerminal() so terminal
dependency failures are recognized as blockers and cause the dependent work unit
to be cascaded to a terminal state instead of remaining pending.
In `@OrchestratorIDE/Services/Hive/HiveWorkerAgent.cs`:
- Around line 665-677: The Context Fabric pack path in ExecuteTaskAsync is not
failing closed when NativeRoleExecutor is unavailable, allowing the task to fall
back to the generic LLM path. Add an early guard in ExecuteTaskAsync before the
pack-specific branch for CampaignPackCatalog.ContextFabricPackId that checks
NativeRoleExecutor and rejects the task if it is missing, rather than proceeding
to the normal agent/tool-call flow. Keep the existing
ExecuteContextFabricReaderAsync branch unchanged, but ensure the new guard
forces a clear failure for CF reader tasks on hosts without native-role support.
---
Outside diff comments:
In `@OrchestratorIDE/Services/Hive/HiveTaskQueue.cs`:
- Around line 600-635: SubmitCampaign currently copies each work unit’s
DependsOn values directly into the HiveTaskBundle without validating that every
referenced work unit exists in the same CampaignDefinition. Update
SubmitCampaign (and the bundle creation path around
HiveTaskBundle/AreDependenciesSatisfied) to verify all dependency IDs are
present among definition.WorkUnits before registering the campaign, and reject
the submission with a clear exception if any dependency is missing or invalid.
---
Nitpick comments:
In `@OrchestratorIDE.UnitTests/ContextFabricReaderWorkUnitTests.cs`:
- Around line 53-61: The StageReaderCorpusAsync ordering test is currently using
an already-sorted TwoSegmentCorpus fixture, so it does not verify the
Ordinal-based sort contract. Update the relevant test setup in
ContextFabricReaderWorkUnitTests to build an intentionally unsorted corpus and
keep the assertions on the staged names, so the test fails if
CampaignTemplates.StageReaderCorpusAsync stops ordering by Ordinal.
🪄 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: 09fd8a5a-c17f-4677-8a48-4225f01b4e71
📒 Files selected for processing (11)
OrchestratorIDE.UnitTests/CampaignDependencyBarrierTests.csOrchestratorIDE.UnitTests/ContextFabricReaderWorkUnitTests.csOrchestratorIDE/Services/Hive/CampaignContracts.csOrchestratorIDE/Services/Hive/CampaignPackCatalog.csOrchestratorIDE/Services/Hive/CampaignTemplates.csOrchestratorIDE/Services/Hive/ContentAddressedStore.csOrchestratorIDE/Services/Hive/HiveNativeRoleExecutorAdapter.csOrchestratorIDE/Services/Hive/HiveTaskBundle.csOrchestratorIDE/Services/Hive/HiveTaskQueue.csOrchestratorIDE/Services/Hive/HiveWorkerAgent.csOrchestratorIDE/Services/Hive/IHiveNativeRoleExecutor.cs
- HiveTaskQueue.SubmitCampaign: validate all DependsOn IDs exist in the same campaign at submission time; unknown IDs were permanently unresolvable. - AreDependenciesSatisfied: cascade dependent to 'failed' when any dependency reaches a terminal failure state (failed/timeout/cancelled), so campaigns can reach a terminal state rather than wedging in pending forever. - HiveWorkerAgent.ExecuteTaskAsync: fail closed with a clear error when the Context Fabric reader pack is dispatched on a host with no NativeRoleExecutor; previously the task would silently fall through to the generic LLM path. - HiveNativeRoleExecutorAdapter.ExecuteContextFabricReaderAsync: upfront guard that the corpus contains exactly one segment before invoking the runner. - ContextFabricReaderWorkUnitTests: TwoSegmentCorpus now supplied reversed so the ordinal sort in StageReaderCorpusAsync is actually exercised. - Tests for both new safety paths (invalid dependency ID + terminal cascade). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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/Hive/HiveTaskQueue.cs`:
- Around line 445-471: The issue is that AreDependenciesSatisfied mutates
entry.Status and triggers CampaignRepository?.UpdateWorkUnit(...) plus
UpdateCampaignAfterTerminal(...) even when it is called from HandleGetNextAsync
without _claimLock, which can race the lock-protected claim/lease/timeout
writers. Fix this by either making the dependency check side-effect-free on the
read path and deferring the cascade to a locked path, or by serializing the
HandleGetNextAsync scan under _claimLock so the status change and
campaign-terminal updates happen only once. Keep the change aligned with the
existing locking pattern used by HandleClaimAsync, HandleLeaseAsync, and
CheckTimeouts.
- Around line 621-635: The dependency validation in SubmitCampaign/HiveTaskQueue
only rejects missing WorkUnitIds, but it still allows self-references and cycles
that can leave the campaign pending forever. Extend the existing
WorkUnitId/DependsOn validation in the campaign submission path to detect any
unit depending on itself and any cyclic dependency chain among
definition.WorkUnits, using a DFS or topological-sort based check before queuing
the campaign. Keep the failure behavior consistent by throwing an
ArgumentException that clearly identifies the offending work unit(s) and
dependency chain.
🪄 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: bcf55ed0-c0c3-4e9b-a4aa-eff07850bd56
📒 Files selected for processing (5)
OrchestratorIDE.UnitTests/CampaignDependencyBarrierTests.csOrchestratorIDE.UnitTests/ContextFabricReaderWorkUnitTests.csOrchestratorIDE/Services/Hive/HiveNativeRoleExecutorAdapter.csOrchestratorIDE/Services/Hive/HiveTaskQueue.csOrchestratorIDE/Services/Hive/HiveWorkerAgent.cs
🚧 Files skipped from review as they are similar to previous changes (3)
- OrchestratorIDE/Services/Hive/HiveNativeRoleExecutorAdapter.cs
- OrchestratorIDE/Services/Hive/HiveWorkerAgent.cs
- OrchestratorIDE.UnitTests/ContextFabricReaderWorkUnitTests.cs
Comment on lines
+445
to
+471
| private bool AreDependenciesSatisfied(QueuedTask entry) | ||
| { | ||
| if (entry.Bundle.DependsOnWorkUnitIds.Length == 0) return true; | ||
| foreach (var depWorkUnitId in entry.Bundle.DependsOnWorkUnitIds) | ||
| { | ||
| var depTaskId = $"{entry.Bundle.CampaignId}-{depWorkUnitId}"; | ||
| if (!_tasks.TryGetValue(depTaskId, out var dep)) return false; | ||
| if (dep.Status == "completed") continue; | ||
|
|
||
| // Cascade a terminal failure: a dependency that can never complete should not | ||
| // leave this entry pending forever. | ||
| if (dep.Status is "failed" or "timeout" or "cancelled") | ||
| { | ||
| if (entry.Status != "failed" && entry.Status != "cancelled") | ||
| { | ||
| entry.Status = "failed"; | ||
| CampaignRepository?.UpdateWorkUnit( | ||
| entry.Bundle.CampaignId, entry.Bundle.WorkUnitId, | ||
| "failed", entry.Bundle.Attempt, | ||
| error: $"Dependency '{depWorkUnitId}' reached terminal state '{dep.Status}'."); | ||
| UpdateCampaignAfterTerminal(entry.Bundle.CampaignId); | ||
| } | ||
| } | ||
| return false; | ||
| } | ||
| return true; | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
AreDependenciesSatisfied performs locked-invariant writes but is also invoked off the lock.
This method mutates entry.Status and fires CampaignRepository?.UpdateWorkUnit(...) + UpdateCampaignAfterTerminal(...) as a side effect. Every other writer of entry.Status (claim, lease, complete, fail, CheckTimeouts) holds _claimLock, and the cascade is correctly serialized when called from HandleClaimAsync (Line 507) and HandleLeaseAsync (Line 564). But HandleGetNextAsync (Line 486) calls it without _claimLock, so the failure-cascade write races concurrent lock-protected writers, and two concurrent GET /tasks/next scans can both pass the entry.Status != "failed" guard and double-fire the campaign-terminal/repository updates.
Either make the cascade side-effect-free on the read path (only flag, cascade under lock), or wrap the HandleGetNextAsync scan in _claimLock so the mutation is serialized like the claim/lease paths.
🤖 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/Hive/HiveTaskQueue.cs` around lines 445 - 471, The
issue is that AreDependenciesSatisfied mutates entry.Status and triggers
CampaignRepository?.UpdateWorkUnit(...) plus UpdateCampaignAfterTerminal(...)
even when it is called from HandleGetNextAsync without _claimLock, which can
race the lock-protected claim/lease/timeout writers. Fix this by either making
the dependency check side-effect-free on the read path and deferring the cascade
to a locked path, or by serializing the HandleGetNextAsync scan under _claimLock
so the status change and campaign-terminal updates happen only once. Keep the
change aligned with the existing locking pattern used by HandleClaimAsync,
HandleLeaseAsync, and CheckTimeouts.
Comment on lines
+621
to
+635
|
|
||
| // Reject any unit that references a DependsOn ID not present in this campaign — an unresolvable | ||
| // dependency would silently wedge the campaign forever in AreDependenciesSatisfied. | ||
| var unitIds = definition.WorkUnits.Select(u => u.WorkUnitId).ToHashSet(StringComparer.Ordinal); | ||
| foreach (var unit in definition.WorkUnits) | ||
| { | ||
| foreach (var dep in unit.DependsOn) | ||
| { | ||
| if (!unitIds.Contains(dep)) | ||
| throw new ArgumentException( | ||
| $"Work unit '{unit.WorkUnitId}' depends on '{dep}' which does not exist in the campaign.", | ||
| nameof(definition)); | ||
| } | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP -C3 '(cycle|topolog|DependsOn|self.?depend)' --type=cs OrchestratorIDE/Services/Hive/Repository: hardcoreerik/TheOrc
Length of output: 14580
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant files first.
ast-grep outline OrchestratorIDE/Services/Hive/HiveTaskQueue.cs --view expanded
printf '\n---\n'
ast-grep outline OrchestratorIDE/Services/Hive/CampaignContracts.cs --view expanded
printf '\n=== searches ===\n'
# Look for any explicit cycle/self-dependency validation or graph checks in the Hive service.
rg -n -C 3 -i '(self.?depend|cycle|topolog|acyclic|dag|dfs|depth.?first|indegree|Kahn|depends on itself|circular)' OrchestratorIDE/Services/Hive/
printf '\n=== submit path ===\n'
# Show the submission method around the validation block.
sed -n '600,690p' OrchestratorIDE/Services/Hive/HiveTaskQueue.cs
printf '\n=== dependency barrier ===\n'
sed -n '438,470p' OrchestratorIDE/Services/Hive/HiveTaskQueue.csRepository: hardcoreerik/TheOrc
Length of output: 16401
Reject cyclic and self-referential dependencies here too. The current check only blocks missing IDs; a cycle (a→b→a) or self-dependency (a→a) still passes, and AreDependenciesSatisfied has no path to make either task terminal, so the campaign can stay pending forever. Add a cycle/self-reference guard during SubmitCampaign (DFS or topological sort).
🤖 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/Hive/HiveTaskQueue.cs` around lines 621 - 635, The
dependency validation in SubmitCampaign/HiveTaskQueue only rejects missing
WorkUnitIds, but it still allows self-references and cycles that can leave the
campaign pending forever. Extend the existing WorkUnitId/DependsOn validation in
the campaign submission path to detect any unit depending on itself and any
cyclic dependency chain among definition.WorkUnits, using a DFS or
topological-sort based check before queuing the campaign. Keep the failure
behavior consistent by throwing an ArgumentException that clearly identifies the
offending work unit(s) and dependency chain.
…r-node telemetry - WorkUnit.NativeRole discriminator routes CF pack bundles to reader vs reducer - CampaignTemplates.ContextFabricReducer: builds reduce work unit with DependsOn barrier - CampaignTemplates.StageReducerCorpusMetaAsync: strips segment text for reducer input - HiveNativeRoleExecutorAdapter.ExecuteContextFabricReducerAsync: runs ReduceEvidenceCardsAsync and writes reduction-nodes.json for artifact upload - FabricHiveCampaignImporter: generation-safe, idempotent evidence import via ReplaceSegmentEvidenceCard; skips unknown-segment cards gracefully - Migration v12: fabric_claims.generation_id column + index - HiveWorkerAgent: evidence_count propagated to HiveTaskResult.Metrics after reader execution - HiveTaskQueue: DependsOn validation on submit; terminal cascade fails dependents immediately - 8 new unit tests in ContextFabricCf6Slice2Tests covering staging, reducer dispatch, importer idempotency, and unknown-segment skip Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
OrchestratorIDE/Services/Hive/HiveTaskQueue.cs (1)
624-634: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReject duplicate work-unit IDs before registering the campaign.
ToHashSetsilently collapses duplicateWorkUnitIds; the later enqueue loop can then fail after_campaigns.TryAddandCampaignRepository.Create, leaving a partially registered campaign.Proposed fix
// Reject any unit that references a DependsOn ID not present in this campaign — an unresolvable // dependency would silently wedge the campaign forever in AreDependenciesSatisfied. + var duplicateUnitId = definition.WorkUnits + .GroupBy(u => u.WorkUnitId, StringComparer.Ordinal) + .FirstOrDefault(g => g.Count() > 1)?.Key; + if (!string.IsNullOrEmpty(duplicateUnitId)) + throw new ArgumentException( + $"Campaign contains duplicate work unit id '{duplicateUnitId}'.", + nameof(definition)); + var unitIds = definition.WorkUnits.Select(u => u.WorkUnitId).ToHashSet(StringComparer.Ordinal);🤖 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/Hive/HiveTaskQueue.cs` around lines 624 - 634, Reject duplicate work-unit IDs in HiveTaskQueue before campaign registration, because the current ToHashSet-based validation silently hides duplicates. In the campaign validation path that iterates definition.WorkUnits, add an explicit duplicate check on WorkUnitId before any _campaigns.TryAdd or CampaignRepository.Create work, and throw an ArgumentException if the same ID appears more than once. Keep the existing dependency validation for unit.DependsOn, but ensure uniqueness is verified first so the campaign is never partially registered.OrchestratorIDE/Services/ContextFabric/DocumentGraphRepository.cs (1)
387-397: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRead back the
generation_idcolumn you now persist.Line 496 writes
claim.GenerationId, butMapClaimstill drops the column. Any caller usingListClaims/ListClaimsForDocumentgets entries with null generation and can later clear the stored value on upsert.Proposed fix
private static FabricClaimEntry MapClaim(SqliteDataReader reader) => new( reader.GetString(reader.GetOrdinal("claim_id")), reader.GetString(reader.GetOrdinal("corpus_id")), reader.GetString(reader.GetOrdinal("document_id")), reader.GetString(reader.GetOrdinal("segment_id")), reader.GetString(reader.GetOrdinal("claim_type")), reader.GetString(reader.GetOrdinal("claim_text")), reader.GetString(reader.GetOrdinal("verification_status")), GetReal(reader, "confidence"), DateTimeOffset.Parse(reader.GetString(reader.GetOrdinal("created_at"))), - DateTimeOffset.Parse(reader.GetString(reader.GetOrdinal("updated_at")))); + DateTimeOffset.Parse(reader.GetString(reader.GetOrdinal("updated_at"))), + GenerationId: GetStr(reader, "generation_id"));Also applies to: 471-496
🤖 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/DocumentGraphRepository.cs` around lines 387 - 397, The claim mapping is missing the persisted generation identifier, so callers of ListClaims and ListClaimsForDocument receive FabricClaimEntry objects with no GenerationId and can overwrite it later. Update MapClaim in DocumentGraphRepository to read the generation_id column and pass it into the FabricClaimEntry constructor, and make sure the claim read/write paths stay aligned wherever claims are projected from SQLite.
🧹 Nitpick comments (1)
OrchestratorIDE.UnitTests/ContextFabricCf6Slice2Tests.cs (1)
153-196: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a non-null
expectedGenerationIdcoverage path.Lines 160, 174-175, and 192 all pass
expectedGenerationId: null, so the new generation-aware branch never runs. A regression in theexpectedGenerationIdflow intoReplaceSegmentEvidenceCard(...)would still pass this fixture. Please add one matching-generation case and one mismatched-generation case that asserts stale evidence is rejected and existing claims stay unchanged.🤖 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.UnitTests/ContextFabricCf6Slice2Tests.cs` around lines 153 - 196, The current tests in FabricHiveCampaignImporter_ImportCards only exercise the null expectedGenerationId path, so the generation-aware branch in ImportCards and ReplaceSegmentEvidenceCard is untested. Add one test that passes a matching non-null expectedGenerationId and verifies cards/claims import normally, and another that passes a mismatched generation id and asserts stale evidence is rejected while existing claims remain unchanged; use FabricHiveCampaignImporter, ImportCards, and ReplaceSegmentEvidenceCard to locate the flow.
🤖 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/ContextFabricCf6Slice2Tests.cs`:
- Around line 189-190: The synthetic unknown-segment test card is inconsistent
because only the top-level SegmentId is changed in ContextFabricCf6Slice2Tests,
while the nested citation still references the original segment and claim
metadata. Update the derived card in the test so all nested segment-related
fields on the card and citation use the same unknown segment id, keeping the
“missing from library” scenario internally consistent.
In `@OrchestratorIDE/Services/ContextFabric/FabricEvidenceGraphImporter.cs`:
- Around line 36-56: Update the segment replacement flow so claims are scoped by
generation instead of replacing all claims for a segment. In
FabricEvidenceGraphImporter.ReplaceSegmentEvidenceCard, make sure the claim IDs
produced by BuildClaimImports/BuildScopedClaimId include generationId, and
change graphRepository.ReplaceClaimsForSegment to only delete/update claims for
the same document, segment, and generation scope rather than all segment claims.
Keep the generation value attached to the claim and preserve separate rows so
later sweeps can distinguish re-indexed generations.
In `@OrchestratorIDE/Services/ContextFabric/FabricHiveCampaignImporter.cs`:
- Around line 46-60: The stale-generation check in
FabricHiveCampaignImporter.ImportCampaignArtifacts is currently a no-op, so
unverified cards can still be imported and tagged with expectedGenerationId.
Update this method to take the artifact/corpus generation metadata explicitly,
compare it against the expected generation before entering the try block, and
skip calling ReplaceSegmentEvidenceCard when they do not match. Keep the current
card-level fields like card.CorpusId and card.PromptVersion as supporting
context, but rely on the passed generation metadata for the actual gate.
In `@OrchestratorIDE/Services/Hive/HiveNativeRoleExecutorAdapter.cs`:
- Around line 138-153: The reducer flow in HiveNativeRoleExecutorAdapter should
not treat an empty result from ReduceEvidenceCardsAsync as success. After the
call to ReduceEvidenceCardsAsync and before serializing ReducerOutput in the
execution path, detect when nodes.Count is zero and fail the execution instead
of writing reduction-nodes.json or returning HiveNativeAgentExecution. Use the
existing symbols ReduceEvidenceCardsAsync, ReducerOutput, and
HiveNativeAgentExecution to locate the check and raise the appropriate failure
for the work unit.
In `@OrchestratorIDE/Services/Hive/HiveWorkerAgent.cs`:
- Around line 530-545: The reducer input validation in HiveWorkerAgent’s
artifact handling is mismatched with the produced Context Fabric files: update
the evidence-card matching in the bundle.InputArtifacts scan so it accepts the
actual reader output name used by the reader executor, and relax the corpus
validation in FetchAndVerifyJsonAsync/FabricCorpus processing so the staged
reducer corpus’s stripped segment metadata is accepted instead of rejecting any
non-empty Segments. Keep the existing InvalidOperationException paths in
HiveWorkerAgent, but make the checks align with the CF artifact shapes produced
by the pipeline.
---
Outside diff comments:
In `@OrchestratorIDE/Services/ContextFabric/DocumentGraphRepository.cs`:
- Around line 387-397: The claim mapping is missing the persisted generation
identifier, so callers of ListClaims and ListClaimsForDocument receive
FabricClaimEntry objects with no GenerationId and can overwrite it later. Update
MapClaim in DocumentGraphRepository to read the generation_id column and pass it
into the FabricClaimEntry constructor, and make sure the claim read/write paths
stay aligned wherever claims are projected from SQLite.
In `@OrchestratorIDE/Services/Hive/HiveTaskQueue.cs`:
- Around line 624-634: Reject duplicate work-unit IDs in HiveTaskQueue before
campaign registration, because the current ToHashSet-based validation silently
hides duplicates. In the campaign validation path that iterates
definition.WorkUnits, add an explicit duplicate check on WorkUnitId before any
_campaigns.TryAdd or CampaignRepository.Create work, and throw an
ArgumentException if the same ID appears more than once. Keep the existing
dependency validation for unit.DependsOn, but ensure uniqueness is verified
first so the campaign is never partially registered.
---
Nitpick comments:
In `@OrchestratorIDE.UnitTests/ContextFabricCf6Slice2Tests.cs`:
- Around line 153-196: The current tests in
FabricHiveCampaignImporter_ImportCards only exercise the null
expectedGenerationId path, so the generation-aware branch in ImportCards and
ReplaceSegmentEvidenceCard is untested. Add one test that passes a matching
non-null expectedGenerationId and verifies cards/claims import normally, and
another that passes a mismatched generation id and asserts stale evidence is
rejected while existing claims remain unchanged; use FabricHiveCampaignImporter,
ImportCards, and ReplaceSegmentEvidenceCard to locate the flow.
🪄 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: fef29bfd-9d23-44e9-8fec-4619c1283900
📒 Files selected for processing (15)
OrchestratorIDE.Avalonia/OrchestratorIDE.Avalonia.csprojOrchestratorIDE.UnitTests/ContextFabricCf6Slice2Tests.csOrchestratorIDE/Services/ContextFabric/ContextFabricFeasibilityRunner.csOrchestratorIDE/Services/ContextFabric/ContextFabricIngestionContracts.csOrchestratorIDE/Services/ContextFabric/DocumentGraphRepository.csOrchestratorIDE/Services/ContextFabric/FabricEvidenceGraphImporter.csOrchestratorIDE/Services/ContextFabric/FabricHiveCampaignImporter.csOrchestratorIDE/Services/Data/Migrations.csOrchestratorIDE/Services/Hive/CampaignContracts.csOrchestratorIDE/Services/Hive/CampaignPackCatalog.csOrchestratorIDE/Services/Hive/CampaignTemplates.csOrchestratorIDE/Services/Hive/HiveNativeRoleExecutorAdapter.csOrchestratorIDE/Services/Hive/HiveTaskQueue.csOrchestratorIDE/Services/Hive/HiveWorkerAgent.csOrchestratorIDE/Services/Hive/IHiveNativeRoleExecutor.cs
🚧 Files skipped from review as they are similar to previous changes (3)
- OrchestratorIDE/Services/Hive/CampaignPackCatalog.cs
- OrchestratorIDE/Services/Hive/CampaignContracts.cs
- OrchestratorIDE/Services/Hive/IHiveNativeRoleExecutor.cs
Comment on lines
+189
to
+190
| var unknownCard = cards[0] with { SegmentId = "seg-does-not-exist" }; | ||
| var mixed = new[] { cards[0], unknownCard }; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep the synthetic “unknown segment” card internally consistent.
Line 189 only rewrites the card-level SegmentId; the nested citation still points at seg-imp-a (and the claim id still names that segment). That means this test can pass or fail because of mixed identifiers instead of the intended “segment missing from library” path. Update the derived card so its nested segment references also use the unknown segment id.
🤖 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.UnitTests/ContextFabricCf6Slice2Tests.cs` around lines 189 -
190, The synthetic unknown-segment test card is inconsistent because only the
top-level SegmentId is changed in ContextFabricCf6Slice2Tests, while the nested
citation still references the original segment and claim metadata. Update the
derived card in the test so all nested segment-related fields on the card and
citation use the same unknown segment id, keeping the “missing from library”
scenario internally consistent.
Comment on lines
+36
to
+56
| public int ReplaceSegmentEvidenceCard( | ||
| FabricEvidenceCard card, | ||
| string verificationStatus, | ||
| string? generationId) | ||
| { | ||
| ArgumentNullException.ThrowIfNull(card); | ||
| var imports = BuildClaimImports(card, verificationStatus, generationId); | ||
|
|
||
| graphRepository.ReplaceClaimsForSegment( | ||
| card.DocumentId, | ||
| card.SegmentId, | ||
| imports.Select(item => item.Claim).ToArray(), | ||
| imports.ToDictionary( | ||
| item => item.Claim.ClaimId, | ||
| item => (IReadOnlyList<FabricClaimCitationEntry>)item.Citations, | ||
| StringComparer.Ordinal)); | ||
|
|
||
| if (imports.Count > 0) | ||
| UpsertEntities(imports[0].Document, verificationStatus, imports[0].Card.Entities); | ||
| return imports.Count; | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Scope replacement and claim identity by generation.
generationId is written onto the claim, but ReplaceClaimsForSegment still deletes every claim for the segment, and BuildScopedClaimId still omits the generation. This means a re-index cannot retain distinct generation rows for later sweeping.
Sketch of the needed contract change
- graphRepository.ReplaceClaimsForSegment(
+ graphRepository.ReplaceClaimsForSegment(
card.DocumentId,
card.SegmentId,
+ generationId,
imports.Select(item => item.Claim).ToArray(),
imports.ToDictionary( var claimId = BuildScopedClaimId(
document.CorpusId,
document.DocumentId,
segment.SegmentId,
- BuildLocalClaimId(claim, claimIndex));
+ BuildLocalClaimId(claim, claimIndex),
+ generationId);- private static string BuildScopedClaimId(string corpusId, string documentId, string segmentId, string claimId) =>
- $"claim-{FabricHashing.Sha256($"{corpusId}|{documentId}|{segmentId}|{claimId}")[..24]}";
+ private static string BuildScopedClaimId(
+ string corpusId,
+ string documentId,
+ string segmentId,
+ string claimId,
+ string? generationId = null)
+ {
+ var generationScope = string.IsNullOrWhiteSpace(generationId)
+ ? ""
+ : $"|generation:{generationId.Trim()}";
+ return $"claim-{FabricHashing.Sha256($"{corpusId}|{documentId}|{segmentId}{generationScope}|{claimId}")[..24]}";
+ }Also applies to: 136-153
🤖 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/FabricEvidenceGraphImporter.cs` around
lines 36 - 56, Update the segment replacement flow so claims are scoped by
generation instead of replacing all claims for a segment. In
FabricEvidenceGraphImporter.ReplaceSegmentEvidenceCard, make sure the claim IDs
produced by BuildClaimImports/BuildScopedClaimId include generationId, and
change graphRepository.ReplaceClaimsForSegment to only delete/update claims for
the same document, segment, and generation scope rather than all segment claims.
Keep the generation value attached to the claim and preserve separate rows so
later sweeps can distinguish re-indexed generations.
Comment on lines
+46
to
+60
| // Reject stale-generation cards to avoid overwriting current-generation evidence. | ||
| if (expectedGenerationId is not null && | ||
| !string.IsNullOrWhiteSpace(card.CorpusId) && | ||
| !string.Equals(card.PromptVersion, FabricSchemaVersions.ReaderPrompt, StringComparison.Ordinal)) | ||
| { | ||
| // PromptVersion is not generationId — the generationId comes from the corpus, not the card. | ||
| // We rely on the caller to only pass cards that came from the correct generation's artifacts. | ||
| // No per-card generationId field exists on FabricEvidenceCard; filtering is done by the | ||
| // caller selecting artifacts from the target generation's campaign. | ||
| } | ||
|
|
||
| try | ||
| { | ||
| imported += importer.ReplaceSegmentEvidenceCard(card, verificationStatus, expectedGenerationId); | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Do not tag unverified cards as the expected generation.
This stale-generation check is a no-op, so expectedGenerationId only re-labels imported claims at Line 59. A stale artifact can therefore overwrite segment evidence while being stored as the current generation. Pass artifact/corpus generation metadata into this method and skip when it differs before calling ReplaceSegmentEvidenceCard.
🤖 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/FabricHiveCampaignImporter.cs` around
lines 46 - 60, The stale-generation check in
FabricHiveCampaignImporter.ImportCampaignArtifacts is currently a no-op, so
unverified cards can still be imported and tagged with expectedGenerationId.
Update this method to take the artifact/corpus generation metadata explicitly,
compare it against the expected generation before entering the try block, and
skip calling ReplaceSegmentEvidenceCard when they do not match. Keep the current
card-level fields like card.CorpusId and card.PromptVersion as supporting
context, but rely on the passed generation metadata for the actual gate.
Comment on lines
+138
to
+153
| var nodes = await new ContextFabricFeasibilityRunner(inner) | ||
| .ReduceEvidenceCardsAsync(corpusMeta, cards, ct).ConfigureAwait(false); | ||
|
|
||
| var json = FabricJson.Serialize(new ReducerOutput( | ||
| corpusMeta.CorpusId, | ||
| corpusMeta.DocumentId, | ||
| corpusMeta.GenerationId, | ||
| nodes.Count, | ||
| nodes)); | ||
| await File.WriteAllTextAsync(Path.Combine(outputDirectory, "reduction-nodes.json"), json, ct) | ||
| .ConfigureAwait(false); | ||
|
|
||
| var promptTokens = 0; | ||
| var completionTokens = 0; | ||
| return new HiveNativeAgentExecution(json, outputDirectory, Steps: nodes.Count, | ||
| promptTokens, completionTokens, FabricHashing.Sha256(json)); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fail reducer execution when no reduction nodes are produced.
ReduceEvidenceCardsAsync can return an empty list when every reduce call fails; this path currently serializes nodeCount: 0 and reports the work unit as completed.
Proposed fix
var nodes = await new ContextFabricFeasibilityRunner(inner)
.ReduceEvidenceCardsAsync(corpusMeta, cards, ct).ConfigureAwait(false);
+ if (nodes.Count == 0)
+ throw new InvalidOperationException("Context Fabric reducer produced no reduction nodes.");
var json = FabricJson.Serialize(new ReducerOutput(📝 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
| var nodes = await new ContextFabricFeasibilityRunner(inner) | |
| .ReduceEvidenceCardsAsync(corpusMeta, cards, ct).ConfigureAwait(false); | |
| var json = FabricJson.Serialize(new ReducerOutput( | |
| corpusMeta.CorpusId, | |
| corpusMeta.DocumentId, | |
| corpusMeta.GenerationId, | |
| nodes.Count, | |
| nodes)); | |
| await File.WriteAllTextAsync(Path.Combine(outputDirectory, "reduction-nodes.json"), json, ct) | |
| .ConfigureAwait(false); | |
| var promptTokens = 0; | |
| var completionTokens = 0; | |
| return new HiveNativeAgentExecution(json, outputDirectory, Steps: nodes.Count, | |
| promptTokens, completionTokens, FabricHashing.Sha256(json)); | |
| var nodes = await new ContextFabricFeasibilityRunner(inner) | |
| .ReduceEvidenceCardsAsync(corpusMeta, cards, ct).ConfigureAwait(false); | |
| if (nodes.Count == 0) | |
| throw new InvalidOperationException("Context Fabric reducer produced no reduction nodes."); | |
| var json = FabricJson.Serialize(new ReducerOutput( | |
| corpusMeta.CorpusId, | |
| corpusMeta.DocumentId, | |
| corpusMeta.GenerationId, | |
| nodes.Count, | |
| nodes)); | |
| await File.WriteAllTextAsync(Path.Combine(outputDirectory, "reduction-nodes.json"), json, ct) | |
| .ConfigureAwait(false); | |
| var promptTokens = 0; | |
| var completionTokens = 0; | |
| return new HiveNativeAgentExecution(json, outputDirectory, Steps: nodes.Count, | |
| promptTokens, completionTokens, FabricHashing.Sha256(json)); |
🤖 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/Hive/HiveNativeRoleExecutorAdapter.cs` around lines
138 - 153, The reducer flow in HiveNativeRoleExecutorAdapter should not treat an
empty result from ReduceEvidenceCardsAsync as success. After the call to
ReduceEvidenceCardsAsync and before serializing ReducerOutput in the execution
path, detect when nodes.Count is zero and fail the execution instead of writing
reduction-nodes.json or returning HiveNativeAgentExecution. Use the existing
symbols ReduceEvidenceCardsAsync, ReducerOutput, and HiveNativeAgentExecution to
locate the check and raise the appropriate failure for the work unit.
Comment on lines
+530
to
+545
| var metaArtifact = bundle.InputArtifacts.FirstOrDefault(a => | ||
| a.Name.EndsWith("corpus-meta.json", StringComparison.OrdinalIgnoreCase)) | ||
| ?? throw new InvalidOperationException( | ||
| "Context Fabric reducer task has no 'corpus-meta.json' input artifact."); | ||
| var cardArtifacts = bundle.InputArtifacts | ||
| .Where(a => a.Name.EndsWith(".evidence-card.json", StringComparison.OrdinalIgnoreCase)) | ||
| .ToArray(); | ||
| if (cardArtifacts.Length == 0) | ||
| throw new InvalidOperationException( | ||
| "Context Fabric reducer task has no evidence-card input artifacts."); | ||
|
|
||
| using var http = new HttpClient { Timeout = TimeSpan.FromMinutes(5) }; | ||
| var corpusMeta = await FetchAndVerifyJsonAsync<FabricCorpus>(http, metaArtifact, ct).ConfigureAwait(false); | ||
| if (corpusMeta.Segments.Count != 0) | ||
| throw new InvalidOperationException( | ||
| "Reducer corpus-meta artifact must have empty Segments; include only structural metadata."); |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Align reducer input validation with produced CF artifacts.
The staged reducer corpus keeps stripped segment metadata, but Line 543 rejects any segments. The reader executor writes evidence-card.json, but Line 535 only matches names ending in .evidence-card.json, so actual reader outputs can be missed.
Proposed fix
var cardArtifacts = bundle.InputArtifacts
- .Where(a => a.Name.EndsWith(".evidence-card.json", StringComparison.OrdinalIgnoreCase))
+ .Where(a =>
+ string.Equals(a.Name, "evidence-card.json", StringComparison.OrdinalIgnoreCase) ||
+ a.Name.EndsWith(".evidence-card.json", StringComparison.OrdinalIgnoreCase))
.ToArray();
@@
var corpusMeta = await FetchAndVerifyJsonAsync<FabricCorpus>(http, metaArtifact, ct).ConfigureAwait(false);
- if (corpusMeta.Segments.Count != 0)
+ if (corpusMeta.Segments.Any(s =>
+ !string.IsNullOrEmpty(s.Text) ||
+ !string.IsNullOrEmpty(s.TextDigest) ||
+ s.EstimatedTokens != 0))
throw new InvalidOperationException(
- "Reducer corpus-meta artifact must have empty Segments; include only structural metadata.");
+ "Reducer corpus-meta artifact must strip segment text, text digests, and token counts.");📝 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
| var metaArtifact = bundle.InputArtifacts.FirstOrDefault(a => | |
| a.Name.EndsWith("corpus-meta.json", StringComparison.OrdinalIgnoreCase)) | |
| ?? throw new InvalidOperationException( | |
| "Context Fabric reducer task has no 'corpus-meta.json' input artifact."); | |
| var cardArtifacts = bundle.InputArtifacts | |
| .Where(a => a.Name.EndsWith(".evidence-card.json", StringComparison.OrdinalIgnoreCase)) | |
| .ToArray(); | |
| if (cardArtifacts.Length == 0) | |
| throw new InvalidOperationException( | |
| "Context Fabric reducer task has no evidence-card input artifacts."); | |
| using var http = new HttpClient { Timeout = TimeSpan.FromMinutes(5) }; | |
| var corpusMeta = await FetchAndVerifyJsonAsync<FabricCorpus>(http, metaArtifact, ct).ConfigureAwait(false); | |
| if (corpusMeta.Segments.Count != 0) | |
| throw new InvalidOperationException( | |
| "Reducer corpus-meta artifact must have empty Segments; include only structural metadata."); | |
| var metaArtifact = bundle.InputArtifacts.FirstOrDefault(a => | |
| a.Name.EndsWith("corpus-meta.json", StringComparison.OrdinalIgnoreCase)) | |
| ?? throw new InvalidOperationException( | |
| "Context Fabric reducer task has no 'corpus-meta.json' input artifact."); | |
| var cardArtifacts = bundle.InputArtifacts | |
| .Where(a => | |
| string.Equals(a.Name, "evidence-card.json", StringComparison.OrdinalIgnoreCase) || | |
| a.Name.EndsWith(".evidence-card.json", StringComparison.OrdinalIgnoreCase)) | |
| .ToArray(); | |
| if (cardArtifacts.Length == 0) | |
| throw new InvalidOperationException( | |
| "Context Fabric reducer task has no evidence-card input artifacts."); | |
| using var http = new HttpClient { Timeout = TimeSpan.FromMinutes(5) }; | |
| var corpusMeta = await FetchAndVerifyJsonAsync<FabricCorpus>(http, metaArtifact, ct).ConfigureAwait(false); | |
| if (corpusMeta.Segments.Any(s => | |
| !string.IsNullOrEmpty(s.Text) || | |
| !string.IsNullOrEmpty(s.TextDigest) || | |
| s.EstimatedTokens != 0)) | |
| throw new InvalidOperationException( | |
| "Reducer corpus-meta artifact must strip segment text, text digests, and token counts."); |
🤖 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/Hive/HiveWorkerAgent.cs` around lines 530 - 545, The
reducer input validation in HiveWorkerAgent’s artifact handling is mismatched
with the produced Context Fabric files: update the evidence-card matching in the
bundle.InputArtifacts scan so it accepts the actual reader output name used by
the reader executor, and relax the corpus validation in
FetchAndVerifyJsonAsync/FabricCorpus processing so the staged reducer corpus’s
stripped segment metadata is accepted instead of rejecting any non-empty
Segments. Keep the existing InvalidOperationException paths in HiveWorkerAgent,
but make the checks align with the CF artifact shapes produced by the pipeline.
Adds work-unit builders and execution dispatch for the boundary stitcher, CPU-bound citation verifier, and per-segment exhaustive query roles, completing the HIVE-native CF-6 role set alongside the existing reader and reducer dispatch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
OrchestratorIDE/Services/Hive/HiveWorkerAgent.cs (2)
518-522: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the malformed XML docs around the new fetch helpers.
The reducer summary starts before
FetchStitcherInputsAsyncand closes nearFetchReducerInputsAsync, so the XML docs are malformed and attached to the wrong helper.Suggested fix
- /// <summary> - /// CF-6: downloads and deserializes the reducer's inputs from the Warchief artifact store. - /// The first artifact named "corpus-meta.json" supplies structural metadata (CorpusId, DocumentId, - /// GenerationId); every remaining artifact named "*.evidence-card.json" is a reader output card. + /// <summary> + /// CF-6: downloads and deserializes the stitcher's left/right single-segment corpora. + /// All digests are re-verified locally before deserialization. + /// </summary> private async Task<(FabricCorpus Left, FabricCorpus Right)> FetchStitcherInputsAsync(HiveTaskBundle bundle, CancellationToken ct)- /// All digests are re-verified locally before deserialization. + /// <summary> + /// CF-6: downloads and deserializes the reducer's inputs from the Warchief artifact store. + /// The first artifact named "corpus-meta.json" supplies structural metadata (CorpusId, DocumentId, + /// GenerationId); every remaining artifact named "*.evidence-card.json" is a reader output card. + /// All digests are re-verified locally before deserialization. /// </summary> private async Task<(FabricCorpus CorpusMeta, IReadOnlyList<FabricEvidenceCard> Cards)>Also applies to: 569-571
🤖 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/Hive/HiveWorkerAgent.cs` around lines 518 - 522, The XML documentation around the new fetch helpers is malformed because the summary/comment block is starting before FetchStitcherInputsAsync and closing near FetchReducerInputsAsync, causing the docs to attach to the wrong method. Move and rebalance the <summary> blocks so each helper, especially FetchStitcherInputsAsync and FetchReducerInputsAsync, has its own complete XML doc comment immediately above the correct signature.
774-822: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFail closed on unknown Context Fabric native roles.
Any unrecognized non-empty
NativeRolefalls through to the reader path. A typo in reducer/stitcher/verifier/query role wiring should fail clearly instead of fetching reader inputs and running the wrong branch.Suggested fix
if (string.Equals(bundle.NativeRole, CampaignPackCatalog.ContextFabricQueryRole, StringComparison.OrdinalIgnoreCase)) { var (questionId, questionText, queryCorpus) = await FetchQueryInputsAsync(bundle, ct).ConfigureAwait(false); _lastAgentExecution = await NativeRoleExecutor.ExecuteContextFabricQueryAsync( bundle, questionId, questionText, queryCorpus, ct).ConfigureAwait(false); Log($"🐝 [{bundle.Role}] '{bundle.Title}' — Context Fabric query {(_lastAgentExecution.Steps > 0 ? "found evidence" : "no evidence")}{DescribeNativeRuntimeTelemetry(bundle.Role)}"); return _lastAgentExecution.Output; } + if (!string.IsNullOrWhiteSpace(bundle.NativeRole)) + throw new InvalidOperationException( + $"Unsupported Context Fabric native role '{bundle.NativeRole}'."); + var corpus = await FetchReaderCorpusAsync(bundle, ct).ConfigureAwait(false);🤖 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/Hive/HiveWorkerAgent.cs` around lines 774 - 822, The Context Fabric dispatch in HiveWorkerAgent.Execute should not fall through to the reader branch for an unrecognized non-empty NativeRole. Update the NativeRole checks under the CampaignPackCatalog.ContextFabricPackId path to explicitly handle only the known roles (ContextFabricReducerRole, ContextFabricStitcherRole, ContextFabricVerifierRole, ContextFabricQueryRole) and throw a clear exception for any other non-empty NativeRole before calling FetchReaderCorpusAsync. Keep the reader path only for the intended empty/default role case so wiring typos fail fast instead of executing the wrong branch.OrchestratorIDE/Services/Hive/IHiveNativeRoleExecutor.cs (1)
38-47: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAlign the reducer contract docs with the actual artifact.
Line 41 says reducer execution outputs a
FabricCorpusReadReport, but the adapter writesreduction-nodes.jsonwith reduction nodes. Update the interface comment so new implementations follow the same contract.Suggested fix
- /// over pre-read evidence cards supplied as input artifacts -- the distributed fan-in step that follows - /// the reader fan-out. Outputs a serialized FabricCorpusReadReport to the output directory. + /// over pre-read evidence cards supplied as input artifacts -- the distributed fan-in step that follows + /// the reader fan-out. Outputs reduction-nodes.json with serialized reduction nodes to the output directory.🤖 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/Hive/IHiveNativeRoleExecutor.cs` around lines 38 - 47, The XML doc on ExecuteContextFabricReducerAsync describes the wrong output artifact, so update the summary/comments to match what the adapter actually produces. In IHiveNativeRoleExecutor and the reducer contract around ContextFabricFeasibilityRunner.ReduceEvidenceCardsAsync, replace the FabricCorpusReadReport wording with the reduction-nodes.json / reduction nodes output so future implementations follow the same contract and artifact name.OrchestratorIDE/Services/Hive/CampaignTemplates.cs (1)
66-72: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAlign reducer metadata with the worker’s accepted shape.
This stages stripped segment records, but the worker currently rejects any reducer meta artifact with non-empty
Segments; reducers built from this helper will fail beforeReduceEvidenceCardsAsync. With the current worker contract, emit an empty segment list here or relax the worker validation.Suggested fix if the worker contract remains empty-segments
var stripped = corpus with { - Segments = corpus.Segments - .Select(s => s with { Text = "", TextDigest = "", EstimatedTokens = 0 }) - .ToArray(), + Segments = [], EstimatedSourceTokens = 0, };🤖 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/Hive/CampaignTemplates.cs` around lines 66 - 72, The reducer payload built in CampaignTemplates should match the worker contract used by the reducer path, since the current staged metadata still carries non-empty Segments and gets rejected before ReduceEvidenceCardsAsync. Update the helper that constructs the stripped corpus to emit an empty segment list (or otherwise align it with the worker’s accepted shape), and keep the metadata fields on the corpus record consistent with the reducer’s expectations.
♻️ Duplicate comments (1)
OrchestratorIDE/Services/Hive/HiveWorkerAgent.cs (1)
541-543: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAccept the actual reader output artifact name for verifier inputs.
Reader execution writes and uploads
evidence-card.json; this matcher only accepts names ending in.evidence-card.json, so verifier work units wired from reader outputs can fail to find their card.Suggested fix
var cardArtifact = bundle.InputArtifacts.FirstOrDefault(a => - a.Name.EndsWith(".evidence-card.json", StringComparison.OrdinalIgnoreCase)) + string.Equals(a.Name, "evidence-card.json", StringComparison.OrdinalIgnoreCase) || + a.Name.EndsWith(".evidence-card.json", StringComparison.OrdinalIgnoreCase)) ?? throw new InvalidOperationException("Verifier task has no '.evidence-card.json' input artifact.");🤖 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/Hive/HiveWorkerAgent.cs` around lines 541 - 543, The verifier input lookup in HiveWorkerAgent should accept the reader’s actual uploaded artifact name, not only names ending in “.evidence-card.json”. Update the cardArtifact selection logic in the verifier path to match “evidence-card.json” as produced by the reader output, while keeping the existing fallback error in place if no matching artifact is found. Use the existing bundle.InputArtifacts filtering around the cardArtifact assignment to locate and adjust the matcher.
🤖 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/ContextFabricScriptedRuntime.cs`:
- Around line 196-201: The generic stitch fallback in
ContextFabricScriptedRuntime should only apply to adapter-generated stitch-...
case IDs, not every unknown caseId. Update the switch/fallback around
FabricBoundaryStitchDraft creation so supported named cases still throw, and
only dynamic stitch IDs return the stitched draft with FabricJson.Serialize.
In `@OrchestratorIDE/Services/Hive/HiveNativeRoleExecutorAdapter.cs`:
- Around line 219-235: The citation validation in
HiveNativeRoleExecutorAdapter’s claim/citation loop currently checks quote
presence, CharStart, and QuoteDigest, but it still allows mismatched SegmentId
or CharEnd to pass. Update the verification logic in the same citation iteration
to compare citation.SegmentId against the current segment identifier and
validate citation.CharEnd against the matched quote end offset in segment.Text,
adding errors when either differs. Keep the existing checks in place and extend
them so the verifier rejects citations with the wrong segment or end boundary.
In `@OrchestratorIDE/Services/Hive/HiveWorkerAgent.cs`:
- Around line 525-534: The Context Fabric stitcher selection in HiveWorkerAgent
should preserve the original left/right input order instead of re-sorting
artifacts by name, and it should reject any case other than exactly two corpora.
Update the corpus collection logic in the stitcher path to use the work unit’s
Inputs order, then keep the first item as left and second as right when calling
FetchAndVerifyJsonAsync<FabricCorpus>; also change the validation so
corpora.Length must equal 2 rather than only checking for fewer than 2.
---
Outside diff comments:
In `@OrchestratorIDE/Services/Hive/CampaignTemplates.cs`:
- Around line 66-72: The reducer payload built in CampaignTemplates should match
the worker contract used by the reducer path, since the current staged metadata
still carries non-empty Segments and gets rejected before
ReduceEvidenceCardsAsync. Update the helper that constructs the stripped corpus
to emit an empty segment list (or otherwise align it with the worker’s accepted
shape), and keep the metadata fields on the corpus record consistent with the
reducer’s expectations.
In `@OrchestratorIDE/Services/Hive/HiveWorkerAgent.cs`:
- Around line 518-522: The XML documentation around the new fetch helpers is
malformed because the summary/comment block is starting before
FetchStitcherInputsAsync and closing near FetchReducerInputsAsync, causing the
docs to attach to the wrong method. Move and rebalance the <summary> blocks so
each helper, especially FetchStitcherInputsAsync and FetchReducerInputsAsync,
has its own complete XML doc comment immediately above the correct signature.
- Around line 774-822: The Context Fabric dispatch in HiveWorkerAgent.Execute
should not fall through to the reader branch for an unrecognized non-empty
NativeRole. Update the NativeRole checks under the
CampaignPackCatalog.ContextFabricPackId path to explicitly handle only the known
roles (ContextFabricReducerRole, ContextFabricStitcherRole,
ContextFabricVerifierRole, ContextFabricQueryRole) and throw a clear exception
for any other non-empty NativeRole before calling FetchReaderCorpusAsync. Keep
the reader path only for the intended empty/default role case so wiring typos
fail fast instead of executing the wrong branch.
In `@OrchestratorIDE/Services/Hive/IHiveNativeRoleExecutor.cs`:
- Around line 38-47: The XML doc on ExecuteContextFabricReducerAsync describes
the wrong output artifact, so update the summary/comments to match what the
adapter actually produces. In IHiveNativeRoleExecutor and the reducer contract
around ContextFabricFeasibilityRunner.ReduceEvidenceCardsAsync, replace the
FabricCorpusReadReport wording with the reduction-nodes.json / reduction nodes
output so future implementations follow the same contract and artifact name.
---
Duplicate comments:
In `@OrchestratorIDE/Services/Hive/HiveWorkerAgent.cs`:
- Around line 541-543: The verifier input lookup in HiveWorkerAgent should
accept the reader’s actual uploaded artifact name, not only names ending in
“.evidence-card.json”. Update the cardArtifact selection logic in the verifier
path to match “evidence-card.json” as produced by the reader output, while
keeping the existing fallback error in place if no matching artifact is found.
Use the existing bundle.InputArtifacts filtering around the cardArtifact
assignment to locate and adjust the matcher.
🪄 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: bf5b7ec1-4821-47eb-8d8d-85455d92c346
📒 Files selected for processing (9)
OrchestratorIDE.UnitTests/ContextFabricCf6Slice3Tests.csOrchestratorIDE.UnitTests/ContextFabricScriptedRuntime.csOrchestratorIDE/Services/ContextFabric/ContextFabricContracts.csOrchestratorIDE/Services/ContextFabric/ContextFabricFeasibilityRunner.csOrchestratorIDE/Services/Hive/CampaignPackCatalog.csOrchestratorIDE/Services/Hive/CampaignTemplates.csOrchestratorIDE/Services/Hive/HiveNativeRoleExecutorAdapter.csOrchestratorIDE/Services/Hive/HiveWorkerAgent.csOrchestratorIDE/Services/Hive/IHiveNativeRoleExecutor.cs
✅ Files skipped from review due to trivial changes (1)
- OrchestratorIDE.UnitTests/ContextFabricCf6Slice3Tests.cs
🚧 Files skipped from review as they are similar to previous changes (2)
- OrchestratorIDE/Services/Hive/CampaignPackCatalog.cs
- OrchestratorIDE/Services/ContextFabric/ContextFabricFeasibilityRunner.cs
Comment on lines
+196
to
+201
| _ => FabricJson.Serialize(new FabricBoundaryStitchDraft | ||
| { | ||
| CaseId = caseId, | ||
| Summary = $"Stitched boundary for case '{caseId}'.", | ||
| LinkedFacts = [], | ||
| }), |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep the generic stitch fallback scoped to adapter-generated cases.
Returning a valid draft for every unknown caseId can mask broken scripted test fixtures. Gate the fallback to dynamic stitch-... IDs and keep throwing for unsupported named cases.
Suggested fix
- _ => FabricJson.Serialize(new FabricBoundaryStitchDraft
+ _ when caseId.StartsWith("stitch-", StringComparison.Ordinal) => FabricJson.Serialize(new FabricBoundaryStitchDraft
{
CaseId = caseId,
Summary = $"Stitched boundary for case '{caseId}'.",
LinkedFacts = [],
}),
+ _ => throw new InvalidOperationException($"Unsupported scripted stitch caseId '{caseId}'."),📝 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
| _ => FabricJson.Serialize(new FabricBoundaryStitchDraft | |
| { | |
| CaseId = caseId, | |
| Summary = $"Stitched boundary for case '{caseId}'.", | |
| LinkedFacts = [], | |
| }), | |
| _ when caseId.StartsWith("stitch-", StringComparison.Ordinal) => FabricJson.Serialize(new FabricBoundaryStitchDraft | |
| { | |
| CaseId = caseId, | |
| Summary = $"Stitched boundary for case '{caseId}'.", | |
| LinkedFacts = [], | |
| }), | |
| _ => throw new InvalidOperationException($"Unsupported scripted stitch caseId '{caseId}'."), |
🤖 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.UnitTests/ContextFabricScriptedRuntime.cs` around lines 196 -
201, The generic stitch fallback in ContextFabricScriptedRuntime should only
apply to adapter-generated stitch-... case IDs, not every unknown caseId. Update
the switch/fallback around FabricBoundaryStitchDraft creation so supported named
cases still throw, and only dynamic stitch IDs return the stitched draft with
FabricJson.Serialize.
Comment on lines
+219
to
+235
| foreach (var citation in claim.Citations ?? []) | ||
| { | ||
| if (citation is null) continue; | ||
| if (!string.IsNullOrWhiteSpace(citation.Quote)) | ||
| { | ||
| var pos = segment.Text.IndexOf(citation.Quote, StringComparison.Ordinal); | ||
| if (pos < 0) | ||
| errors.Add($"Quote not found in source: '{citation.Quote[..Math.Min(80, citation.Quote.Length)]}'"); | ||
| else if (citation.CharStart >= 0 && citation.CharStart != pos) | ||
| errors.Add($"CharStart mismatch: expected {pos}, got {citation.CharStart}"); | ||
| if (!string.IsNullOrWhiteSpace(citation.QuoteDigest)) | ||
| { | ||
| var expectedDigest = FabricHashing.Sha256(citation.Quote); | ||
| if (!string.Equals(expectedDigest, citation.QuoteDigest, StringComparison.OrdinalIgnoreCase)) | ||
| errors.Add($"QuoteDigest mismatch for claim {claim.ClaimId}"); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Verify citation segment IDs and end offsets too.
A citation with the wrong SegmentId or CharEnd can still pass as long as the quote text is present. That weakens the verifier artifact used downstream.
Suggested fix
foreach (var citation in claim.Citations ?? [])
{
if (citation is null) continue;
+ if (!string.Equals(citation.SegmentId, segment.SegmentId, StringComparison.Ordinal))
+ errors.Add($"SegmentId mismatch: expected {segment.SegmentId}, got {citation.SegmentId}");
if (!string.IsNullOrWhiteSpace(citation.Quote))
{
var pos = segment.Text.IndexOf(citation.Quote, StringComparison.Ordinal);
if (pos < 0)
errors.Add($"Quote not found in source: '{citation.Quote[..Math.Min(80, citation.Quote.Length)]}'");
- else if (citation.CharStart >= 0 && citation.CharStart != pos)
- errors.Add($"CharStart mismatch: expected {pos}, got {citation.CharStart}");
+ else
+ {
+ if (citation.CharStart >= 0 && citation.CharStart != pos)
+ errors.Add($"CharStart mismatch: expected {pos}, got {citation.CharStart}");
+ var expectedEnd = pos + citation.Quote.Length;
+ if (citation.CharEnd >= 0 && citation.CharEnd != expectedEnd)
+ errors.Add($"CharEnd mismatch: expected {expectedEnd}, got {citation.CharEnd}");
+ }
if (!string.IsNullOrWhiteSpace(citation.QuoteDigest))📝 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
| foreach (var citation in claim.Citations ?? []) | |
| { | |
| if (citation is null) continue; | |
| if (!string.IsNullOrWhiteSpace(citation.Quote)) | |
| { | |
| var pos = segment.Text.IndexOf(citation.Quote, StringComparison.Ordinal); | |
| if (pos < 0) | |
| errors.Add($"Quote not found in source: '{citation.Quote[..Math.Min(80, citation.Quote.Length)]}'"); | |
| else if (citation.CharStart >= 0 && citation.CharStart != pos) | |
| errors.Add($"CharStart mismatch: expected {pos}, got {citation.CharStart}"); | |
| if (!string.IsNullOrWhiteSpace(citation.QuoteDigest)) | |
| { | |
| var expectedDigest = FabricHashing.Sha256(citation.Quote); | |
| if (!string.Equals(expectedDigest, citation.QuoteDigest, StringComparison.OrdinalIgnoreCase)) | |
| errors.Add($"QuoteDigest mismatch for claim {claim.ClaimId}"); | |
| } | |
| } | |
| foreach (var citation in claim.Citations ?? []) | |
| { | |
| if (citation is null) continue; | |
| if (!string.Equals(citation.SegmentId, segment.SegmentId, StringComparison.Ordinal)) | |
| errors.Add($"SegmentId mismatch: expected {segment.SegmentId}, got {citation.SegmentId}"); | |
| if (!string.IsNullOrWhiteSpace(citation.Quote)) | |
| { | |
| var pos = segment.Text.IndexOf(citation.Quote, StringComparison.Ordinal); | |
| if (pos < 0) | |
| errors.Add($"Quote not found in source: '{citation.Quote[..Math.Min(80, citation.Quote.Length)]}'"); | |
| else | |
| { | |
| if (citation.CharStart >= 0 && citation.CharStart != pos) | |
| errors.Add($"CharStart mismatch: expected {pos}, got {citation.CharStart}"); | |
| var expectedEnd = pos + citation.Quote.Length; | |
| if (citation.CharEnd >= 0 && citation.CharEnd != expectedEnd) | |
| errors.Add($"CharEnd mismatch: expected {expectedEnd}, got {citation.CharEnd}"); | |
| } | |
| if (!string.IsNullOrWhiteSpace(citation.QuoteDigest)) | |
| { | |
| var expectedDigest = FabricHashing.Sha256(citation.Quote); | |
| if (!string.Equals(expectedDigest, citation.QuoteDigest, StringComparison.OrdinalIgnoreCase)) | |
| errors.Add($"QuoteDigest mismatch for claim {claim.ClaimId}"); | |
| } | |
| } |
🤖 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/Hive/HiveNativeRoleExecutorAdapter.cs` around lines
219 - 235, The citation validation in HiveNativeRoleExecutorAdapter’s
claim/citation loop currently checks quote presence, CharStart, and QuoteDigest,
but it still allows mismatched SegmentId or CharEnd to pass. Update the
verification logic in the same citation iteration to compare citation.SegmentId
against the current segment identifier and validate citation.CharEnd against the
matched quote end offset in segment.Text, adding errors when either differs.
Keep the existing checks in place and extend them so the verifier rejects
citations with the wrong segment or end boundary.
Comment on lines
+525
to
+534
| var corpora = bundle.InputArtifacts | ||
| .Where(a => a.Name.EndsWith(".corpus.json", StringComparison.OrdinalIgnoreCase)) | ||
| .OrderBy(a => a.Name, StringComparer.OrdinalIgnoreCase) | ||
| .ToArray(); | ||
| if (corpora.Length < 2) | ||
| throw new InvalidOperationException( | ||
| $"Context Fabric stitcher task requires exactly 2 corpus artifacts, got {corpora.Length}."); | ||
| using var http = new HttpClient { Timeout = TimeSpan.FromMinutes(5) }; | ||
| var left = await FetchAndVerifyJsonAsync<FabricCorpus>(http, corpora[0], ct).ConfigureAwait(false); | ||
| var right = await FetchAndVerifyJsonAsync<FabricCorpus>(http, corpora[1], ct).ConfigureAwait(false); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve stitcher input order and reject extra corpora.
Sorting by artifact name can swap the logical left/right boundary, and corpora.Length > 2 is silently accepted despite the error text saying exactly two. Use the Inputs order produced by the work unit and enforce exactly two.
Suggested fix
var corpora = bundle.InputArtifacts
.Where(a => a.Name.EndsWith(".corpus.json", StringComparison.OrdinalIgnoreCase))
- .OrderBy(a => a.Name, StringComparer.OrdinalIgnoreCase)
.ToArray();
- if (corpora.Length < 2)
+ if (corpora.Length != 2)
throw new InvalidOperationException(
$"Context Fabric stitcher task requires exactly 2 corpus artifacts, got {corpora.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
| var corpora = bundle.InputArtifacts | |
| .Where(a => a.Name.EndsWith(".corpus.json", StringComparison.OrdinalIgnoreCase)) | |
| .OrderBy(a => a.Name, StringComparer.OrdinalIgnoreCase) | |
| .ToArray(); | |
| if (corpora.Length < 2) | |
| throw new InvalidOperationException( | |
| $"Context Fabric stitcher task requires exactly 2 corpus artifacts, got {corpora.Length}."); | |
| using var http = new HttpClient { Timeout = TimeSpan.FromMinutes(5) }; | |
| var left = await FetchAndVerifyJsonAsync<FabricCorpus>(http, corpora[0], ct).ConfigureAwait(false); | |
| var right = await FetchAndVerifyJsonAsync<FabricCorpus>(http, corpora[1], ct).ConfigureAwait(false); | |
| var corpora = bundle.InputArtifacts | |
| .Where(a => a.Name.EndsWith(".corpus.json", StringComparison.OrdinalIgnoreCase)) | |
| .ToArray(); | |
| if (corpora.Length != 2) | |
| throw new InvalidOperationException( | |
| $"Context Fabric stitcher task requires exactly 2 corpus artifacts, got {corpora.Length}."); | |
| using var http = new HttpClient { Timeout = TimeSpan.FromMinutes(5) }; | |
| var left = await FetchAndVerifyJsonAsync<FabricCorpus>(http, corpora[0], ct).ConfigureAwait(false); | |
| var right = await FetchAndVerifyJsonAsync<FabricCorpus>(http, corpora[1], ct).ConfigureAwait(false); |
🤖 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/Hive/HiveWorkerAgent.cs` around lines 525 - 534, The
Context Fabric stitcher selection in HiveWorkerAgent should preserve the
original left/right input order instead of re-sorting artifacts by name, and it
should reject any case other than exactly two corpora. Update the corpus
collection logic in the stitcher path to use the work unit’s Inputs order, then
keep the first item as left and second as right when calling
FetchAndVerifyJsonAsync<FabricCorpus>; also change the validation so
corpora.Length must equal 2 rather than only checking for fewer than 2.
HiveTaskQueue's GET /hive/tasks/{id} now returns OutputArtifacts for
completed work units, closing the gap where an external orchestrator
couldn't chain CF-6 campaign stages (reader -> reducer/stitcher/verifier
-> query) by digest without in-process access to the queue.
Tools/Cf6AcceptanceRunner is a new CLI that stages the deterministic
synthetic-book corpus on a live Warchief over HTTP, dispatches the full
CF-6 pipeline across whatever real worker nodes are enrolled, and
records which WorkerId claimed each reader unit -- the actual evidence
for the 2-node/3-node distribution exit-gate requirement. It validates
exhaustive-query answers against the benchmark's expected terms,
segments, and abstention, and writes a JSON evidence report. Verifier/
stitcher/reducer output is checked for completion only, not semantic
correctness (tracked separately, task_3c6f8eab).
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@Tools/Cf6AcceptanceRunner/Program.cs`:
- Around line 206-208: The noFalsePositives check in Program.Main is too
permissive because it only calls MatchesUnit for non-expected units and can miss
a non-expected finding that still reports Relevant=true. Update the acceptance
logic around findingByUnit, expectedUnitIds, and MatchesUnit so any unit not in
expectedUnitIds fails if it claims relevance, regardless of SegmentId or
QuestionId matching. Keep the fix localized to the noFalsePositives predicate
and preserve the existing expected-unit 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9192834f-7fc1-41ad-ab6a-cb8803d4996b
📒 Files selected for processing (4)
OrchestratorIDE/Services/Hive/HiveTaskBundle.csOrchestratorIDE/Services/Hive/HiveTaskQueue.csTools/Cf6AcceptanceRunner/Cf6AcceptanceRunner.csprojTools/Cf6AcceptanceRunner/Program.cs
🚧 Files skipped from review as they are similar to previous changes (2)
- OrchestratorIDE/Services/Hive/HiveTaskBundle.cs
- OrchestratorIDE/Services/Hive/HiveTaskQueue.cs
Live multi-node testing surfaced a chain of real CF-6 production bugs that
made native worker execution fail for any real run, plus gaps in the
acceptance runner's validation:
- HiveNativeRoleExecutorAdapter: reader wrote a bare 'evidence-card.json',
but FetchVerifier/ReducerInputsAsync match on Name.EndsWith('.evidence-
card.json') -- the missing segment-id prefix meant every verifier and the
reducer failed. Also dispatch now uses a 4096-token reader budget
(HiveDispatchOptions) so reasoning-model <think> traces don't truncate the
JSON, instead of the silent 1024 default.
- CampaignTemplates.StageReducerCorpusMetaAsync: emit genuinely empty
Segments (FetchReducerInputsAsync hard-rejects a non-empty list), not
text-blanked entries.
- HiveService: wire the worker agent's OnLog into the daemon logger (native
failures were previously masked behind a generic message), and pre-resolve
the Researcher role with RuntimeWorkloadKind.ContextFabricReader so
ModelAdmissionGate deprioritizes reasoning-tuned models for CF-6.
- ModelDepot: check IsReasoningTuned ahead of the role-tag tie-break.
- HiveTaskQueue: measure in-memory task eviction from completion headroom
(30 min) so an external poller can still read a slow multi-stage
campaign's later units.
- Cf6AcceptanceRunner: validate verifier verdicts against recomputed ground
truth, stitch output against the boundary fixture, reducer segment/claim
coverage, and exhaustive-query answers (terms + segment identity + no
false positives); gate PASS on reader-stage node count; handle the
post-eviction 404 instead of hanging.
- Test: assert the segment-id-prefixed evidence-card filename.
Verified end-to-end: a full single-node CF-6 pipeline (16 readers, 16
verifiers, 15 stitchers, stitch fixture, reducer, exhaustive queries) runs
green with a real model.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- HiveNodeServer: new OnLog event, wired into the app's Activity panel. On an 'already_paired' pairing rejection it now logs the in-memory peer list -- added while diagnosing a stuck pairing where the on-disk hive-peers.json showed no matching entry yet the trust check still fired. The write path that produced the in-memory-only entry wasn't conclusively pinned down, so the log stays in to make any recurrence diagnosable instead of opaque (the working reset is: clear the stale peer on BOTH machines, restart both, re-pair via right-click 'Pair with this node' + fingerprint check). - MainWindow status bar now shows the machine name next to the build stamp (v<ver> . <sha> . <MACHINE>), so screenshots and computer-use sessions across the fleet are immediately distinguishable. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A fully-synced HARDCOREPC still couldn't run any CF-6 native work -- every model load died with TypeInitializationException on LLama.Native.NativeApi. Root cause: the single-file publish used IncludeNativeLibrariesForSelfExtract, which leaves AppContext.BaseDirectory pointing at the exe dir while the native backend DLLs self-extract to a temp dir, so LLamaSharp's './'-relative backend probing finds nothing. - release.yml + sync-theorc-fleet skill: switch the app and daemon single-file publishes to IncludeAllContentForSelfExtract=true, so the bundle runs FROM the extraction dir and BaseDirectory == where the natives are. (Installer left as native-only -- it doesn't load LLamaSharp.) - OrchestratorIDE.Avalonia.csproj: UndefineProperties='AssemblyName;OutputType' on the NativeRuntime reference so the publish's -p:AssemblyName=OrchestratorIDE override doesn't cascade and cause an ambiguous-project-name restore error. - Tools/NativeProbe: a minimal LLamaSharp native-load probe (DryRun selection + native log + optional real model load) that reproduces the app's exact native path in isolation -- this is what pinned the root cause. Verified with the probe (DryRun success: True, real model loads) AND proven on HARDCOREPC: it leased a CF-6 reader task and executed it natively, returning a valid evidence card -- first successful native CF-6 work on that machine. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The daemon's HiveService already pre-binds the Researcher role with
RuntimeWorkloadKind.ContextFabricReader so ModelDepot deprioritizes
reasoning-tuned models, but the Avalonia app builds its HIVE worker's
NativeRoleRuntime via a separate path (BuildExperimentalNativeRoleRuntime)
that never got that binding. As a result a GUI node with multiple models
could pick a reasoning model (observed live: NEWCOREPC selecting
DeepSeek-R1-Distill-Qwen-7B) whose <think> trace breaks the reader's
structured evidence-card output ('summary is required' / unterminated JSON),
failing ~60% of its CF-6 reads.
Thread a forHiveWorker flag through BuildExperimentalNativeRoleRuntime and,
for the worker, pre-resolve Researcher with the ContextFabricReader workload
and pass it as a roleBinding -- mirroring the daemon. Main chat keeps the
workload-agnostic default (a reasoning model is fine there).
Verified across a full 2-node CF-6 pipeline: with the fix, NEWCOREPC selects
gemma-12B and completes all its readers/verifiers/stitchers/reducer/queries
with zero failures (readerNodeCount=2, distribution proven).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The system prompt's 'direct evidence' wording caused models to return relevant=false when a segment only holds PART of the answer — one hop of a multihop chain, or one item in an exhaustive list. Clarified that any segment containing ANY fact that contributes toward the question should be marked relevant, even if it alone cannot answer the question completely. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…index lookup Query work units now receive the segment's pre-extracted evidence card as an input artifact. When the card is present the worker calls the new static QueryEvidenceCard() on ContextFabricFeasibilityRunner, which scores each claim's token set against the question's token set (BM25-style) and returns relevant=true deterministically — no LLM call, no model-size sensitivity, no prompt ambiguity. Falls back to the LLM QuerySegmentAsync path only when no card artifact is supplied, preserving backward compatibility with older campaign definitions. This fixes the multihop, exhaustive, and contradiction query failures that stemmed from small models (qwen2.5-coder-7b) misinterpreting "partial evidence" even after the reader-prompt fix, because the query prompt had the same ambiguity. Retrieval is now host-deterministic; only synthesis (the final answer step) still uses the LLM. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
BM25 claim matching used >=3-char ANY-overlap tokens, so stop-words ("the"
in every "The archive token..." claim) and single-word coincidences
("approved") routed unrelated questions to all 16 segments -- failing the
multihop, contradiction, and unanswerable benchmarks. Require >=4-char
tokens AND >=3 distinct matching tokens per claim.
Add --resume-from <prior-report.json> to iterate on query logic without
re-running the expensive reader stage: it replays readers/verifiers/
stitchers/reducer evidence from a prior run and jumps straight to the query
campaigns. Fail closed when the prior run's corpus generation or
--model-hash don't match, and stamp resumed runs (Resumed=true,
GateMode="resumed-query-only", dedicated exit code 3) so a replay can never
be mistaken for a clean multi-node acceptance pass. Also record the actual
reader-claimant set (ReaderWorkerIds) separately from all claimants so the
fan-out proof isn't overstated.
Reviewed-by: Codex (Tools/codex-review.ps1)
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…stamps Recovers a worker whose Warchief trust went stale (repeated HTTP 401 after an identity rotation) without hand-editing hive-peers.json or restarting both apps -- the manual dance this replaces. - HivePeerStore.PruneSuperseded drops same-name/different-NodeId duplicates on every successful pair (both the responder and initiator sides), killing the stale-secret accumulation that produces the 401s. - HiveWorkerAgent.TryResyncWithWarchiefAsync discovers the Warchief's live NodeId via the unauthenticated /hive/update/version endpoint and re-pairs when trust is stale; auto-fires on a persistent 401 when opted in. - HiveNodeServer time-boxed dev auto-approve window (10 min) completes the re-pair headlessly -- gated on the initiator already being trusted, so it only fast-tracks re-pairs of known nodes and never a brand-new peer. - Settings UI: Start/Stop worker buttons (no restart needed), "Accept re-sync"/"Re-sync now" buttons + an auto-resync toggle; async-void handlers catch faults instead of crashing the UI thread. Activity-log rows now show an HH:mm:ss timestamp column. - T17: PruneSuperseded unit tests. Reviewed-by: Codex (Tools/codex-review.ps1) Co-Authored-By: Codex <noreply@openai.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…sed center
The native ModelDepot auto-selects the SMALLEST admitted quant for a role
(ranks by VRAM headroom first), so a fleet with both q2_k and q5_k_m
qwen2.5-coder builds landed the boundary stitcher on q2_k. That 2-bit model
dropped an entire fact on the cross-pronoun-reference case ("sealed the
blue ledger in cabinet forty-two") while resolving the pronoun to the
other segment's fact, failing summaryPreserved/linkedFactsCovered.
Fix is prompt-only, not a heavier model (a heavier model wedges the
fleet's weaker nodes): the stitcher system message now explicitly requires
keeping BOTH the referring fact and the fact a pronoun/clause points back
to, while still merging ONLY cross-boundary facts (not restating unrelated
segment content, which would bloat real production stitches). Verified live
on an isolated q2_k run (both stitch fixture cases pass) and end-to-end in
a 3-node CF-6 acceptance run.
Also, two HIVE window observability/UX changes:
- Constellation shows a directional work-flow pulse per worker (amber out
on claim, blue back on complete/fail) instead of only ambient particles,
and the Warchief's activity log lines lead with the PC and direction
(`sent to X` / `returned from X`).
- The constellation now centers/crowns whichever machine the elected
Warchief actually is, not unconditionally "This PC" -- a worker or
observer machine sees the Warchief in the middle and itself on the ring.
Required splitting the overloaded IsCenter (now: crowned/centered) from a
new IsLocal (this machine -- drives the self-action menu and "this
machine" label independently of where it's drawn).
Reviewed-by: Codex (Tools/codex-review.ps1) -- round 1 caught a stitcher
prompt overreach (restating all facts, not just cross-boundary ones, would
have regressed production stitches) and a worker-name prefix-match false
positive; both fixed and re-verified clean.
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Add --death-test mode to Cf6AcceptanceRunner: submits a single reader unit, drives suspend/resume via the fault script, and auto-captures the full requeue -> re-claim -> stale-token-409 -> single task_complete timeline (incl. durable claim-token rows read from the Warchief db). - Add death-fault.ps1 helper (SSH NtSuspendProcess/resume; refuses worker ids not in worker-map.json so the Warchief host can never be targeted; pure ASCII for PS 5.1 compatibility). - Harden HiveWorkerAgent timeout handling: OperationCanceledException is only treated as shutdown when ct.IsCancellationRequested, so a single timed-out lease poll no longer silently kills the worker loop forever. - Add RunLoop_Survives_A_Timed_Out_Lease_Poll test covering the timeout-vs-cancellation distinction. - Ignore _deploy/ fleet-sync publish output. - Preserves the worker-death evidence path (.orc/cf6-acceptance/, ignored by design); no CF-5/CF-7 scope changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
5 tasks
3 tasks
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
First slice of CF-6 (HIVE stage engine and distributed readers). Stages document segments as content-addressed corpus artifacts and dispatches them to HIVE workers as Context Fabric reader work units, with a dependency barrier so downstream reduce units can wait on their readers.
What's in this slice
WorkUnit.DependsOnpropagated ontoHiveTaskBundle.DependsOnWorkUnitIds;HiveTaskQueue.AreDependenciesSatisfiedgates lease/claim/get-next until each dependency reachescompleted.theorc.context-fabric@1.0.0(NativeAgentexecution kind for capability matching; dispatch bypasses the generic agent/tool-call loop).HiveWorkerAgentroutesPackId == theorc.context-fabrictasks to a reader branch that fetches the staged corpus from/hive/artifacts/{digest}(digest re-verified locally, fails closed), rebuilds a single-segmentFabricCorpus, and runsContextFabricFeasibilityRunner.ReadCorpusAsyncviaHiveNativeRoleExecutorAdapter.ExecuteContextFabricReaderAsync. The resultingevidence-card.jsonflows through the existing output-artifact upload + attestation path.CampaignTemplates.StageReaderCorpusAsyncsplits a corpus into single-segment corpus artifacts (chunked writes into the content store) andContextFabricReadersbuilds one reader unit per segment.ContentAddressedStore.ComputeSha256(ReadOnlySpan<byte>)for download verification, matching the on-disk lowercase-hex digest format.Artifact staging
Reader input/output is fully artifact-based (content-addressed digests), not inline parameter passing.
Tests
8 new unit tests (
ContextFabricReaderWorkUnitTests,CampaignDependencyBarrierTests): staging round-trip + content-addressed dedup, template wiring, dependency-barrier blocking/unblocking, and reader execution end-to-end via the scripted runtime.Verification
🤖 Generated with Claude Code
Summary by CodeRabbit