Skip to content

perf(server): Claude item ordinals no longer copy a session-wide map per item - #13864

Merged
juliusmarminge merged 1 commit into
t3code/codex-turn-mappingfrom
v2/claude-item-ordinals
Sep 26, 2026
Merged

juliusmarminge merged 1 commit into
t3code/codex-turn-mappingfrom
v2/claude-item-ordinals

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Sep 26, 2026 •

Copy link
Copy Markdown
Member

The Claude V2 adapter kept every item ordinal of the session in one Map and copied the whole map on each new item (new Map(current) inside Ref.update). Nothing ever pruned it. A long session paid O(items) memory and O(items²) copying: 50k items took 5.5 minutes of adapter time, almost all of it spent copying the map.

What changed

The ordinal map now lives on the turn context (ActiveClaudeTurnContext.itemOrdinals). It is mutated in place, and it goes away when the turn settles and the context is dropped. The per-turn counter map (nextItemOrdinalsByTurn, also copied per item and never pruned) is gone, because the map's size is the counter. OpenCode and Pi already allocate ordinals this way.

Why per-turn is safe

Every resolveItemOrdinal call passes the active turn's context, and keys are either turn-scoped (terminal-failure:${providerTurnId}, usage-limit:${providerTurnId}:…, assistant:${runId}) or native ids first seen in that turn (message uuids, tool_use ids, thinking blocks). The one long-lived reference is a resumed subagent (SendMessage, wake replay, #13668/#13735 restart recovery). It keeps its turnItemOrdinal on the session subagent registry (existingSubagent?.turnItemOrdinal ?? resolveItemOrdinal(...)) and never looks it up again. Its child items use subagent.nextChildItemOrdinal, not this map.

To confirm, before the fix I instrumented resolveItemOrdinal to flag any hit on an ordinal a different turn had allocated. Then I ran the full replay suites (all providers: background tasks, wakes, subagent resume, resume after restart, forks, rollback) plus the Claude, Cursor, ACP, Codex, Grok and registry adapter tests: 566 tests, 314 Claude allocations, 0 cross-turn hits. The instrumentation was not committed.

Benchmark

A scratch, uncommitted test drove the real adapter through a fake query runner: 10 turns, N/10 root assistant messages each, then a result per turn. Each size ran in a fresh process. Heap is measured after gc() once all turns settled.

N items before: time after: time before: heap retained after: heap retained
1,000 0.42 s 0.28 s 2.43 MiB 2.34 MiB
10,000 8.9 s 0.75 s 3.24 MiB 2.48 MiB
50,000 325.6 s 2.1 s 2.73 MiB 2.49 MiB

Time is the real win. Heap retention is noisy at this scale because the map's keys are strings the emitted events share. Measured alone, a 50k-entry map holds about 1.8 MiB for the life of the session. After the fix, that memory is bounded by the largest single turn.

Verification

  • vp test run with ClaudeAdapterV2.test.ts, ClaudeReplayFixtures.integration.test.ts, OrchestratorReplayFixtures.integration.test.ts, OrchestratorReplayRecovery.integration.test.ts and OrchestratorReplayFixtures.contract.test.ts: 5 files, 223 tests pass. The replay suites are unchanged.
  • Before the fix, the same replay suites plus the Codex replay suite and the Cursor, ACP, Codex, Grok and registry adapter tests ran with the cross-turn instrumentation above: 14 files, 566 tests pass, 0 cross-turn ordinal hits for Claude.
  • vp exec tsc --noEmit -p . in apps/server: no errors.
  • vp exec knip: nothing new for the touched file.
  • vp lint on the touched file: only the existing layer unused-variable warning.
  • Not run: repo-wide checks, live Claude.

Model: Claude Opus 5.5 (Claude Code)

🤖 Generated with Claude Code


Devin Review

…per item

The Claude V2 adapter kept every item ordinal of the session in one Map
that it copied on each new item and never pruned, so a long session paid
O(items) memory and O(items^2) copying. Ordinals are only ever looked up
by the turn that allocated them (a resumed subagent keeps its ordinal on
the session subagent registry), so the map now lives on the turn context,
is mutated in place, and goes away when the turn settles.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Sep 26, 2026
providerTurnId,
providerTurnOrdinal,
startedAt,
itemOrdinals: new Map(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes ordinal allocation for native item IDs reused across turns: the fresh map now assigns an ordinal in the new turn rather than retaining the previous turn's value. Could you add focused adapter tests covering repeated IDs across turns and resumed subagents, asserting the emitted ordinals and ordering? The existing resume test checks routing but not ordinals.

Posted via Macroscope — Effect Service Conventions

@github-actions

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

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

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire — 4.9 KiB — 6.8 KiB ✅
Codex Thread snapshot wire — 3.7 KiB — 4.9 KiB ✅
Codex Live turn WebSocket wire — 1.2 KiB — 2.0 KiB ✅
Codex Live turn WebSocket decoded — 20.4 KiB — 29.3 KiB ✅
Codex Live turn messages — 2 — 8 ✅
Claude Total thread wire — 4.9 KiB — 6.8 KiB ✅
Claude Thread snapshot wire — 3.7 KiB — 4.9 KiB ✅
Claude Live turn WebSocket wire — 1.2 KiB — 2.0 KiB ✅
Claude Live turn WebSocket decoded — 20.8 KiB — 29.3 KiB ✅
Claude Live turn messages — 2 — 8 ✅

Baseline: unavailable · PR result: ef5f0c0 · 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: 106.1 KiB
  • Claude decoded thread snapshot: 106.4 KiB

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

@macroscopeapp

macroscopeapp Bot commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at ef5f0c0

Macroscope's review found this PR approvable — This is a focused server-side performance refactor that scopes Claude item ordinal allocation to each provider turn while preserving resumed subagent ordinals. It changes only ordering metadata, introduces no schema, deployment, security, billing, or configuration impact, and the open comment identifies test coverage rather than a concrete defect.

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

@juliusmarminge
juliusmarminge merged commit 2e68ee5 into t3code/codex-turn-mapping Sep 26, 2026
24 of 25 checks passed
@juliusmarminge
juliusmarminge deleted the v2/claude-item-ordinals branch September 26, 2026 22:02
dillonc-dev added a commit to exarch-run/t3code that referenced this pull request Sep 27, 2026
…per item (pingdotgg#13864)

Applied with reduced patch context: upstream's claudeSubagentIds helper,
from subagent commits the fork doesn't have yet, sits next to the change.
The changed lines are upstream's.

(cherry picked from commit 2e68ee5)

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant