[sisodias]

Problem

Projection reads can scan a long thread's historical turn items and build a temporary B-tree to recover message order, even though the query only needs message items grouped by their JSON message ID. Related: #21578, the accepted report of long-thread SQLite stalls.

Change

Migration 067 adds seven missing access paths for projection and application-event reads: thread/type and thread/message-ID turn items, run/status turn items, running runs, runs by user-message ID, provider turns by thread/completion time, and aggregate/version event sequence. The migration and upgrade expectations use the next free number on main at 67c0fc0cda.

I checked the running-only index against the existing runs_recovery_idx before keeping it. On the upstream schema, WHERE status = 'running' scans the runs table; SQLite does not infer the partial recovery index's status IN (...) predicate from that equality. After migration it scans the small covering running-only index. The matching status IN (...) AND thread_id = ? query continues to use runs_recovery_idx.

The port excludes the original update-time and message-prefix indexes: their callers were fork-specific maintenance code.

Scope and approval

This is a focused persistence performance fix for missing query access paths, with unchanged query results and a migration regression test. #21578 establishes the accepted long-thread SQLite performance problem. This does not claim to resolve its unfinished-item lookup: open #21585 addresses that predicate. I also checked #21193 (live snapshot cohort indexes), #21777 (thread sweep expressions), #21491 (wire filtering), and #21315 (checkpoint process batching); these seven index definitions do not duplicate those changes.

Verification

macOS arm64, Node v26.7.0, sqlite_version() 3.54.1. Used a private logical DB copy containing one real thread: 11,911 turn items, 8,143 nodes, 203 runs, 1,915 message groups, about 90 MB before the new indexes. Only that thread's projections were copied; application events were omitted. No private records are included here.

Compared the same copy with upstream's existing indexes, then with migration 067's seven indexes. No planner statistics. Each query ran once to warm up, followed by 20 measured executions; the table gives medians. This measures SQL lookups, not browser paint or the complete snapshot request.

Query	Before (ms)	After (ms)	Observed plan change
Message ID / minimum ordinal, 1,915 groups	27.440	4.007	thread/run scan + temporary GROUP BY B-tree → thread/message expression index, no temporary B-tree
Run by user message ID, absent-key lookup	0.276	0.002	table scan → expression-index seek
Running runs, zero live rows in this copy	0.0153	0.0015	table scan → covering partial-index scan
Existing recovery lookup, zero live rows	0.0039	0.0036	existing recovery index before and after

Building the seven indexes took 60.30 ms on this copy. Full multi-GB migration cost and write-throughput impact were not measured.

vp test run apps/server/src/persistence/Migrations.test.ts apps/server/src/persistence/Migrations/067_OrchestrationSlowQueryIndexes.test.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
# 5 files, 18 tests passed
vp lint apps/server/src/persistence/Migrations.ts apps/server/src/persistence/Migrations/067_OrchestrationSlowQueryIndexes.ts apps/server/src/persistence/Migrations/067_OrchestrationSlowQueryIndexes.test.ts apps/server/src/persistence/Migrations/055_OrchestrationV2.test.ts apps/server/src/persistence/reconcileV2PreviewMigration.test.ts
# exit 0
pnpm exec tsc-rs --noEmit -p apps/server/tsconfig.json
# exit 0

The new test checks the migration, a second no-op migration run, message-result equivalence, and the message, user-message run, running-only, and existing recovery query plans.

Model: GPT-6. Harness: Codex.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-20T14:54:22.078598Z	134b2e6	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[macroscopeapp]

Approvability

Verdict: Approved at 134b2e6

Macroscope's review found this PR approvable — This is a bounded persistence performance fix that adds seven backward-compatible SQLite indexes and verifies migration idempotence, query plans, and unchanged results. Its production impact is limited to index build/storage and maintenance overhead rather than a behavior or API change.

^{You can add or adjust custom eligibility rules. Learn more.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 134b2e6e96

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Guard malformed JSON before building expression indexes

When upgrading a database that contains a malformed run projection, this statement evaluates json_extract for every existing row and SQLite aborts the migration with malformed JSON; Sqlite.ts:39 runs migrations before services start, so the server cannot reach its unreadable-projection handling. This is an explicitly supported recovery case: ProjectionRecovery.test.ts:345-368 stores '{broken' in this table and expects the affected thread to be reported unreadable. Repair or guard invalid payloads before creating this and the analogous turn-item expression index, and cover that upgrade path.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Drop the index exercised only by the synthetic query

This planner query does not occur in production: a repo-wide search finds the only two $.userMessageId SQL reads at ProjectionStore.ts:4234-4235 and 5140-5144, and both restrict the run by run_id before extracting the field, so the primary-key lookup wins. Consequently this test blesses an expression index that every run.created/run.updated upsert must maintain without accelerating a real read; exercise an actual lookup and drop or reshape the index.

AGENTS.md reference: AGENTS.md:L98-L101

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Remove the unmatched provider-turn completion index

For normal provider-turn updates, this adds a second B-tree with the same useful prefix as the existing orchestration_v2_projection_provider_turns_thread_idx, but a repo-wide search of production SQL finds no provider-turn read that filters or orders on completed_at; those reads use identifiers, thread/status, or provider-thread/ordinal instead. It therefore adds maintenance to every provider-turn.updated upsert at ProjectionStore.ts:2379-2413 without improving a current lookup, so remove it or introduce it only alongside a query whose ordering matches it.

AGENTS.md reference: AGENTS.md:L15-L17

Useful? React with 👍 / 👎.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Path: .coderabbit.config.ts
• Review profile: CHILL
• Plan: Advanced
• Run ID: fcdb8f39-c4f5-4269-b216-aa3fc6dceae6

📥 Commits

Reviewing files that changed from the base of the PR and between 67c0fc0 and 134b2e6.

📒 Files selected for processing: 5

• apps/server/src/persistence/Migrations.ts
• apps/server/src/persistence/Migrations/055_OrchestrationV2.test.ts
• apps/server/src/persistence/Migrations/067_OrchestrationSlowQueryIndexes.test.ts
• apps/server/src/persistence/Migrations/067_OrchestrationSlowQueryIndexes.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; 9 remain after this review.

📝 Walkthrough

Migration 67 adds seven SQLite indexes for orchestration lookups and event sequencing. The migration manifest and upgrade expectations now include it. Tests verify index creation, selected query plans, and unchanged grouped message results.

Changes: Orchestration query indexes

Layer / File(s)	Summary
Define and register migration 67 apps/server/src/persistence/Migrations/067_OrchestrationSlowQueryIndexes.ts, apps/server/src/persistence/Migrations.ts	Migration 67 creates seven idempotent indexes for orchestration turn items, runs, provider turns, and events. The migration manifest registers it.
Verify migration and index behavior apps/server/src/persistence/Migrations/067_OrchestrationSlowQueryIndexes.test.ts, apps/server/src/persistence/Migrations/055_OrchestrationV2.test.ts, apps/server/src/persistence/reconcileV2PreviewMigration.test.ts	Tests verify index creation, query plans for selected lookups, unchanged grouped message results, and migration 67 in upgrade expectations.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: shivamhwp

Merge Risk: ⚪ Minimal · up to 134b2

The migration and its tests show no identified issue requiring correction before merge.

Security architecture risk: 🔵 Low · up to 134b2

The change adds query indexes without changing stored records, query permissions, or externally exposed interfaces. Existing transactions protect migration state. Remaining uncertainty concerns index-building costs and failure recovery on production-sized databases, rather than an identified security vulnerability.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The direct affected asset is the server's orchestration SQLite database. Index construction spans the qualifying records in that database, rather than only one thread, but does not grant new read access or change record ownership.

Trust Boundaries and Controls

• observed — The changed production behavior consists of fixed schema statements executed through the existing migration authority. The routed test declarations do not add an attacker-controlled source, identity transition, or privileged production entrypoint.

Resilience and Maintainability Implications

• inferred — Existing writer transactions acquire a connection permit and use BEGIN IMMEDIATE before executing their bodies. Preview reconciliation reads and updates its history inside such a transaction. These controls counter the proposed concurrent history-rewrite race, although concurrent startup may still encounter lock contention and was not dynamically tested.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title clearly identifies a server performance change that matches the migration adding indexes for slow projection lookups.
Description check		The description includes complete Problem, Change, Scope and approval, and Verification sections. It explains the performance issue, implementation, scope boundaries, measured results, tests, and limi…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description includes complete Problem, Change, Scope and approval, and Verification sections. It explains the performance issue, implementation, scope boundaries, measured results, tests, and limitations.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• 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.}