perf(server): index active thread sweep expressions#21777

open3
server-persistence-03/startup-scheduling
server-persistence-03/sweep-indexes
opened8d agobyJJulius Marminge
View on GitHubTwo-way sync enabledPR last imported 4h ago

Problem

Settlement and PR sweeps parse thread JSON while scanning and sorting active threads. A partial expression index matches those reads; strict migration identity checks remain in place.

Change

Migration 065 adds an active-thread expression index; sweep filters use the maintained archive column without changing eligibility.

Scope and approval

Maintainer takeover of @maria-rcks’s investigation and proof of concept in #17609, claimed in the takeover comment. Each stack layer owns one concern. Custom chunk/precompression/reload machinery and unrelated runtime changes were omitted. This records the authorized takeover; Macroscope’s required human review remains outstanding for startup, compact events, web, and desktop.

Verification

Validation: migration coverage and all 43 ProjectionStore tests passed; server typecheck and scoped lint passed. In a synthetic in-memory fixture with 2,000 active threads and 16 KB PR metadata each (seven-sample medians), settlement read fell from 30.451 to 1.166 ms and PR read from 108.003 to 9.854 ms. Index construction took 18.401 ms and added 80 KiB. These are query measurements, not whole-app startup timings; update-batch timing was noisy (53.533 vs 45.840 ms).

Six independent whole-stack review rounds are complete. Final round-6 Opus and Sol reviews found no confirmed code defects in the complete pinned-base-to-945a89a diff. Confirmed earlier findings are fixed: bounded early admission, provider subscription ordering, legacy event replay and service ownership, sibling PR refreshes, literal HTML loading/failure fallback, desktop unreadiness errors, migration expectations, memory fixtures, and browser test readiness/library compatibility. The final existing Markdown file passes 68/68 on owner and top; scoped web typecheck, lint and formatting pass. No new case was added for those browser readiness or compatibility repairs.

Current web CI passed, including Test Web, Build and Typecheck, and Check. Current desktop CI passed, with Test Web skipped by its scope gate. A duplicate desktop workflow was cancelled; its aggregate Check failed on cancelled input jobs and remains visible in the rollup alongside the successful same-head run. The earlier attachment-preview failure predates this stack’s web changes, passed isolated current/base comparisons, and did not recur in current web CI; its exact timing cause remains unproven and no unrelated patch was added.

Required human approvals remain outstanding for startup, compact events, web and desktop. CodeRabbit’s latest desktop incremental review was skipped because it could not recover its checkpoint; the independent full-diff round-6 reviews cover the current head. No bot approval is inferred from a skipped review. Native/platform proof limits remain as stated above. This stack is unmerged and is not declared merge-ready.

Models: GPT-6-Astra (tests), GPT-6.1-Sol (implementation), GPT-6-Sol and Claude Opus 5.5 (review); Codex and Claude harnesses in T3 Code.

Checks

22 successful · 8 skipped
All checks passed
e1e8523
Synced from GitHub

Approvability

Verdict: Approved at e1e8523

Macroscope's review found this PR approvable — This is a focused performance optimization that adds a partial expression index and avoids repeated JSON parsing on existing active-thread queries. Migration coverage verifies index behavior and preserves existing paths without changing product defaults or introducing new user-facing capability.

You can add or adjust custom eligibility rules. Learn more.

Synced from GitHub

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ No successful main baseline artifact is available yet. This run establishes the initial measurement.

ProviderMetricMain baselineThis PRImpactPR ceiling
CodexTotal thread wire—4.9 KiB—6.8 KiB✅
CodexThread snapshot wire—3.8 KiB—4.9 KiB✅
CodexLive turn WebSocket wire—1.2 KiB—2.0 KiB✅
CodexLive turn WebSocket decoded—20.8 KiB—29.3 KiB✅
CodexLive turn messages—1—8✅
ClaudeTotal thread wire—5.0 KiB—6.8 KiB✅
ClaudeThread snapshot wire—3.8 KiB—4.9 KiB✅
ClaudeLive turn WebSocket wire—1.2 KiB—2.0 KiB✅
ClaudeLive turn WebSocket decoded—21.2 KiB—29.3 KiB✅
ClaudeLive turn messages—2—8✅

Baseline: unavailable · PR result: e1e8523 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

Added 2 commits

  • refactor(server): limit indexed reads to startup sweep queries64bf54a
  • perf(server): index active thread sweep expressions79e254f
Synced from GitHub

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Team
  • Run ID: 8d51a3ec-ec30-4b3f-be01-faa5902b210f
📥 Commits

Reviewing files that changed from the base of the PR and between 64bf54a31659dd8038203f3550d550eef0615241 and e1e852385850be20ab6740fa0d4608495027b7d3.

📒 Files selected for processing: 3
  • apps/server/src/persistence/Migrations/055_OrchestrationV2.test.ts
  • apps/server/src/persistence/Migrations/064_CanonicalizeThreadPullRequestLinks.test.ts
  • apps/server/src/persistence/reconcileV2PreviewMigration.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

📝 Walkthrough

Migration 65 adds a partial index for active projection threads. Three queries now use archived_at to filter archived threads. Migration tests cover the query behavior, migration registration, and rerun expectations.

Changes: Active projection thread sweep

Layer / File(s)Summary
Register the active-thread index
apps/server/src/persistence/Migrations/065_ProjectionThreadSweepIndexes.ts, apps/server/src/persistence/Migrations.ts, apps/server/src/persistence/Migrations/055_OrchestrationV2.test.ts, apps/server/src/persistence/Migrations/064_CanonicalizeThreadPullRequestLinks.test.ts, apps/server/src/persistence/reconcileV2PreviewMigration.test.ts
Migration 65 creates a composite index for rows where deleted_at and archived_at are NULL. The migration registry and migration tests include migration 65.
Use the archive column in active-thread queries
apps/server/src/orchestration-v2/ProjectionStore.ts, apps/server/src/persistence/Migrations.test.ts
Three queries use archived_at to exclude archived threads. The migration test checks sweep query results, JSON function opcodes, archive and override changes, and migration idempotency.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Suggested reviewers: yash-singh1

Merge Risk: ⚪ Minimal · up to e1e85

The change preserves sweep eligibility for supported data paths and updates migration expectations. No merge-blocking behavior risk was identified; normal CI checks and required approvals remain outstanding.

Pre-merge checks: Passed 4
Check nameStatusExplanation
Title checkPassedThe title is concise, conventional, and clearly identifies the primary change: adding an index for active thread sweep expressions.
Linked Issues checkPassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes checkPassedCheck skipped because no linked issues were found for this pull request.
Description checkPassedThe description includes all required sections: Problem, Change, Scope and approval, and Verification. It explains the issue, implementation, scope, linked investigation and takeover approval, focused…
Full details: Description check

Explanation

The description includes all required sections: Problem, Change, Scope and approval, and Verification. It explains the issue, implementation, scope, linked investigation and takeover approval, focused validation, performance measurements, outstanding approvals, and agent details. It also clearly states that the pull request is not merge-ready.

✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

Added 1 commit

  • test(server): update migration ledger expectations for sweep indexese1e8523