Uh oh!
There was an error while loading. Please reload this page.
perf(server): add targeted projection queries - #1646
Conversation
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: New query excludes deleted threads unlike original code
- Removed the unintentional
AND threads.deleted_at IS NULLfilter fromgetThreadCheckpointContextThreadRowso soft-deleted threads are still found during checkpoint diff lookups, matching the original behavior.
- Removed the unintentional
Or push these changes by commenting:
@cursor push 1a06fbeaff
Preview (1a06fbeaff)
diff --git a/apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.ts b/apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.ts--- a/apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.ts+++ b/apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.ts@@ -407,7 +407,6 @@
INNER JOIN projection_projects AS projects
ON projects.project_id = threads.project_id
WHERE threads.thread_id = ${threadId}
- AND threads.deleted_at IS NULL
LIMIT 1
`,
});You can send follow-ups to this agent here.
| INNER JOIN projection_projects AS projects | ||
| ON projects.project_id = threads.project_id | ||
| WHERE threads.thread_id = ${threadId} | ||
| AND threads.deleted_at IS NULL |
There was a problem hiding this comment.
New query excludes deleted threads unlike original code
Medium Severity
The getThreadCheckpointContext SQL query adds AND threads.deleted_at IS NULL, but the original code path via getSnapshot() returned all threads (including deleted ones) and CheckpointDiffQuery looked up the thread by ID without any deleted_at filter. This behavioral change means checkpoint diff requests for soft-deleted threads now fail with a "Thread not found" error instead of returning the diff data.
Additional Locations (1)
0e0bad4 to
dbbf25cCompareCo-authored-by: codex <codex@users.noreply.github.com>
ApprovabilityVerdict: Needs human review An open review comment identifies a potential behavioral regression: the new targeted query filters out deleted threads while the original code did not, which could cause checkpoint diff requests for soft-deleted threads to fail. This warrants human review to determine if this behavioral change is intentional. You can customize Macroscope's approvability policy. Learn more. |
Co-authored-by: codex <codex@users.noreply.github.com>
dbbf25c to
03d24d6CompareCo-authored-by: codex <codex@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Summary
Validation
bun run test src/orchestration/Layers/ProjectionSnapshotQuery.test.ts src/checkpointing/Layers/CheckpointDiffQuery.test.tsbun run test src/orchestration/Layers/ProjectionSnapshotQuery.test.ts src/checkpointing/Layers/CheckpointDiffQuery.test.ts src/persistence/Layers/ProjectionThreadMessages.test.ts src/orchestration/Layers/ProjectionPipeline.test.ts src/orchestration/Layers/OrchestrationEngine.test.ts src/orchestration/Layers/CheckpointReactor.test.ts src/orchestration/Layers/ProviderRuntimeIngestion.test.ts src/orchestration/Layers/ProviderCommandReactor.test.tsbun fmtbun lintbun typecheck(currently fails inapps/webdue pre-existing@effect/atom-react/Layer.suspenderrors)Note
Medium Risk
Moderate risk because it changes the
ProjectionSnapshotQueryservice contract, adds new SQL query paths plus indexes/migration, and alters startup + diff flows to depend on the new targeted lookups.Overview
Reduces server startup and diff-query overhead by avoiding full orchestration snapshot hydration.
ProjectionSnapshotQuerynow exposes targeted read APIs (getCounts,getActiveProjectByWorkspaceRoot,getFirstActiveThreadIdByProjectId,getThreadCheckpointContext) backed by narrow SQL queries.CheckpointDiffQueryswitches togetThreadCheckpointContext(and derivescwdfromworktreePath ?? workspaceRoot) instead of loading the full snapshot, and startup bootstrap/telemetry paths inserverRuntimeStartup.tsare updated to use the new targeted queries; the startup heartbeat is now launched asynchronously vialaunchStartupHeartbeat.Adds migration
019_ProjectionSnapshotLookupIndexesto create supporting indexes onprojection_projects(workspace_root, deleted_at)andprojection_threads(project_id, deleted_at, created_at), plus new/updated tests covering the targeted query behavior and ensuring full-snapshot reads are not triggered.Written by Cursor Bugbot for commit 711a3e0. This will update automatically on new commits. Configure here.
Note
Add targeted projection queries to avoid full snapshot hydration on server startup
ProjectionSnapshotQueryShape:getCounts,getActiveProjectByWorkspaceRoot,getFirstActiveThreadIdByProjectId, andgetThreadCheckpointContext, each fetching only the data needed rather than loading the full read model.autoBootstrapWelcomeandrecordStartupHeartbeatin serverRuntimeStartup.ts to use these targeted queries instead ofgetSnapshot.CheckpointDiffQuery.getTurnDiffin CheckpointDiffQuery.ts to fetch only the relevant thread checkpoint context viagetThreadCheckpointContext.projection_projects(workspace_root, deleted_at)andprojection_threads(project_id, deleted_at, created_at)to support the new queries.launchStartupHeartbeat(forked), so it no longer blocks the server ready event.Macroscope summarized 711a3e0.