perf(orchestration): per-thread shell deltas, visit throttling, event compaction - #4971
All clear
All clear
Details
Note
Your check run agent prompt is: .macroscope/check-run-agents/effect-service-conventions.md
More information about how Check Run Agents work can be found in our Help Center.
🔍 Agent activity — reasoning, no tool calls
💭 Looking at the diff for Effect service convention violations in the orchestration files, I'm checking the existing patterns like
ProjectionMaintenanceV2Shape,ProjectionStoreV2Shape, andOrchestratorV2Shapeto understand what's already established in the codebase. These are pre-existing conventions, so I'm not going to push for unrelated repository-wide changes. I'm examining the actual files now to verify import styles and check for concrete violations introduced in this pull request. I noticeEffect.catchis being used appropriately for intentional error handling, andEffect.yieldNowis fine for Effect 4. I need to verify whethermapErroris a pre-existing helper and check if the imports are properly consolidated from the "effect" module. ShellStream is just utility functions, so that's straightforward. For ProjectionMaintenance, the newcompactEventStorewrapper follows the existing pattern with themapErrorhelper—it's consistent with what's already there, even though the convention discourages curried helpers that only wrap error construction. The error itself uses a multi-value discriminator on the operation field with a generic type. Now I'm checking ws.ts and serverRuntimeStartup for how they useEffect.catch, and verifying the import statements across the touched files to make sure everything's pulling from the consolidated "effect" module. I should also confirm that Duration is properly imported in ws.ts since it's used in the new code. The ws.ts file looks good stylistically. Now I'm checking ThreadManagementService and noticing it uses the correct type reference pattern with["Service"]. ThreadManagementServiceShape is already defined as a standalone interface, which aligns with the repo's convention of pairing Shape interfaces withContext.Servicetypes, though there seems to be some inconsistency about whether these should be defined inline or separately. Since this PR only adds methods to an existing shape rather than creating a new one, and the instructions say not to flag retained legacy patterns or demand unrelated cleanup, I won't flag this. Moving on to LegacyV1ThreadImporter, I see the newconfirmedTranscriptThreadIdsSet is scoped inside themakeEffect generator, so it's per-instance state in a closure rather than a module global. It's mutable state used as a cache, which doesn't violate the listed conventions about hiding dependencies in globals or singleton services, even though Effect conventions might prefer using a Ref instead. Now I'm checking whether the behavior changes are properly tested. The caching logic in LegacyV1ThreadImporter and the queued turnItem null change in ThreadManagementService don't appear to have corresponding tests, though the visit and mark-unread hydration skip is covered. I'm also verifying the new ProjectionStore method and serverRuntimeStartup code look fine, but I need to check the Effect v4 API usage — specifically whetherEffect.yieldNowis being called correctly as a value rather than a function in this version. The ThreadManagementService correctly referencesgetThreadShellfrom OrchestratorV2["Service"], and the Shape members consistently use the same pattern throughout. Let me verify the serverRuntimeStartup block context as well. Looking at the conventions: themapErrorhelper is pre-existing and used multiple times, so that's fine. ProjectionMaintenanceError's schema with unbounded operation string and optional defect is pre-existing. The newcompactEventStorein the Shape uses an inline anonymous return object, which is acceptable. The Layer.succeed pattern in Orchestrator's layerUnavailable is pre-existing, and the error handling in serverRuntimeStartup intentionally catches the entire error channel with logging — that's fine too. ChatView.tsx is a React component using setTimeout in an effect, which is standard. Now I need to verify that the compaction fiber is properly scoped and thatprojectionMaintenanceis acquired via yield*. I need to verify that ProjectionStore's implementation is consistent, particularly checking how layerMemory's getThreadShell usesservice.getThreadProjectionand making sure there aren't any issues with error handling or type widening in the surrounding context. Let me also review the ThreadManagementService imports and namespace to confirm everything aligns. The repo follows its conventions consistently, and whileProjectionMaintenanceaddscompactEventStoreto the Shape interface (which technically violates the pattern of not retaining shape interfaces), this mirrors an existing repo-wide pattern withContext.Service<Foo, FooShape>, so it's not worth demanding a broader cleanup here. Since there are no actual findings, I need to return success with "All clear" as the summary and leave the details empty.
Agent Credits: 93 credits