perf(server): reuse stored turn item positions

[tannerpolley]

Problem

TurnItemPositionStore.allocate runs on every turn-item update. For an item that already has a position it still reads the run ordinal, runs the INSERT … SELECT MAX(ordinal) … ON CONFLICT DO NOTHING, and then reads the stored position back. A streaming message updates the same item many times, so this repeats on the server's database connection for every delta.

Change

Look up the item's stored position first and return it when one exists. First allocation, the conflict handling, the final read and the error mapping are unchanged. The old path already kept an existing item's position (the insert was discarded by ON CONFLICT), so the result is the same; only the redundant work goes away.

Scope and approval

This is a small, focused performance fix with no behavior change: 8 lines in the existing store plus one test. No schema, contract, ordering or provider change.

#18094 also edits this function (to assign timeline bands when runs start). It doesn't add this shortcut; whichever lands second needs a trivial rebase.

Verification

• New test TurnItemPositionStore.test.ts: a temporary BEFORE INSERT trigger rejects any insert for an item that already has a position. Updating an existing item, after another item was allocated, after a run change, and the same item ID in another thread all return the expected ordinals. The test fails on the current code (the redundant insert hits the trigger) and passes with the change.
• vp test run apps/server/src/orchestration-v2/TurnItemPositionStore.test.ts apps/server/src/orchestration-v2/FoundationPersistence.test.ts: 43 passed. Server tsc-rs --noEmit, targeted vp lint and vp fmt --check pass.

Benchmark on Linux, Node 22: a fresh file-backed SQLite database per sample, built with the project's own persistence layer and migrations (WAL), 2,000 stored positions, 20,000 sequential updates cycling through 100 existing items, 2,000 warmup updates excluded, 7 alternating samples per version.

20,000 existing-item updates	Before	After
Median	901 ms	282 ms
Range	820–969 ms	248–366 ms

That is about 3.2× less time in the allocator. This measures the allocator only, not end-to-end streaming latency.

Built with Codex (GPT-6.1 Sol) and Claude Code (Opus 5.5).

🤖 Generated with Claude Code

[macroscopeapp]

Approvability

Verdict: Approved at f70f891

Macroscope's review found this PR approvable — This is a small, self-contained performance optimization that reuses an existing stored ordinal without changing allocation results or schemas. New coverage verifies the reuse path and leaves first-time allocation behavior unchanged.

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

[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: 9d634d7a-8f5e-4684-906a-11f6351fc572

📥 Commits

Reviewing files that changed from the base of the PR and between 11fd54f and f70f891.

📒 Files selected for processing: 2

• apps/server/src/orchestration-v2/TurnItemPositionStore.test.ts
• apps/server/src/orchestration-v2/TurnItemPositionStore.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

TurnItemPositionStoreV2.allocate now returns an existing ordinal for the same thread and turn item before attempting new allocation. A SQLite test checks position reuse, next-position allocation, and independent numbering across threads.

Changes: Turn item position allocation

Layer / File(s)	Summary
Existing-position reuse and validation apps/server/src/orchestration-v2/TurnItemPositionStore.ts, apps/server/src/orchestration-v2/TurnItemPositionStore.test.ts	allocate returns the existing ordinal for the same thread and turn item. The test verifies reuse with changed run metadata, next-position allocation, and numbering for another thread.

Priority: ➖ Normal

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

Change: Refactor

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to f70f8

No actionable issue remains from this review; the PR is mergeable after normal checks.

Pre-merge checks: 4

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: reusing stored turn item positions in the server.
Description check		The description includes the problem, change, scope and approval rationale, targeted verification, test results, benchmark details, and limitations. It matches the required template and explains the p…
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.

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