Skip to content

perf(server): Codex item maps no longer grow and copy for the whole session - #13876

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

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

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Sep 26, 2026 •

Copy link
Copy Markdown
Member

The Codex V2 adapter kept every item position of the session in one Map. It copied the whole map on each new item (new Map(current) inside Ref.update), along with a per-turn counter map that was also copied per item and never pruned. Streamed plan text worked the same way: planDeltas held the full markdown of every plan item for the whole session and copied the map on every delta. A long session paid O(items) memory and O(items²) copying. 50k items took 8 minutes of adapter time.

This is the same fix as #13864 (Claude), #13866 (Cursor) and #13868 (ACP).

What changed

  • Item positions now live on the turn context (ActiveCodexTurnContext.itemPositions) and are mutated in place. The map's size is the per-turn counter, so nextItemOrdinalsByTurn is gone. The map goes away with the context once the turn settles, or once a settled turn retained for background commands is released.
  • Plan deltas are a plain map with no per-delta copy. A plan item's entry is deleted when item/completed arrives for that item.

Why per-turn is safe

Every resolveItemPosition call passes the context that owns the item:

  • Late background command and dynamic-tool events resolve their settled turn's own context through resolveItemEventContext.
  • Interrupt and fail terminalization passes that same context.
  • A subagent's approvals allocate on the owning root turn (approvalOwnerCodexTurn), which is still active while its subagent runs.
  • A resumed subagent never looks its spawn item up again, because registerSubagentThreads returns early for an already-registered thread.

To confirm this, I instrumented resolveItemPosition before the fix. It shadowed a per-context map next to the session map and flagged two cases: a hit on an entry some other context had allocated, and any ordinal that differed from the per-context result. I ran the Codex replay suite, the orchestrator replay, recovery and contract suites, the provider-switch, fork, merge-back, steering and selection-restart integration suites, and the CodexAdapterV2 and CodexDriver tests: 11 files, 328 tests. That covered 260 allocations and 425 lookups, with 0 cross-turn hits and 0 ordinal mismatches. For plans, no item/plan/delta arrived after its plan item completed. The one plan completion in the fixtures carried its full text and also had streamed deltas. The instrumentation was not committed.

Benchmark

A scratch test (not committed) drove the real adapter through a replay transcript: 10 turns, N/10 webSearch items each (item/started + item/completed), then turn/completed. The last row also streams 1,000 plan deltas per turn and completes the plan item with empty text. Each size ran in a fresh process.

N items before after
1,000 0.34 s 0.29 s
10,000 11.8 s 1.5 s
50,000 478.3 s 6.2 s
10,000 + 10×1,000 plan deltas 13.7 s 1.8 s

Heap after gc() was within noise (e.g. 131.3 vs 130.2 MiB at 50k), because the map's keys are strings the emitted events share. Once all turns settle, the maps retain nothing.

Verification

  • vp test run in apps/server with the fix, on the same 11 files: CodexAdapterV2.test.ts, CodexReplayFixtures.integration.test.ts, OrchestratorReplayFixtures.integration.test.ts, OrchestratorReplayRecovery.integration.test.ts, OrchestratorReplayFixtures.contract.test.ts, ProviderSwitch.integration.test.ts, ThreadFork.integration.test.ts, ThreadMergeBack.integration.test.ts, SteeringCompletion.integration.test.ts, SelectionRestart.integration.test.ts, CodexDriver.test.ts. All 328 tests pass, and no tests or fixtures changed.
  • vp exec tsc --noEmit -p . in apps/server: no errors.
  • vp exec knip --workspace apps/server: nothing for the touched file. The only finding is the existing unlisted cc binary in an ACP test.
  • vp lint on the touched file: only the existing makeCodexAppServerClientFactoryCommandLayer and layer unused-variable warnings.
  • Not run: repo-wide checks, live Codex.

Model: Claude Opus 5.5 (Claude Code)

🤖 Generated with Claude Code


Devin Review

…ession

Item positions move onto the turn context and are mutated in place, so
they live only as long as the turn (or its retained settled context).
Streamed plan text is kept in a plain map without per-delta copies and
dropped when the plan item completes.

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
@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: 03e22c1 · 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 03e22c1

Macroscope's review found this PR approvable — This is a contained one-file server performance refactor that preserves item ordinals and streamed plan assembly while scoping temporary maps to their owning turns and releasing completed plan buffers. It adds no user-facing capability, schema change, product-default change, or static-analysis override.

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

@juliusmarminge
juliusmarminge merged commit 93dbb55 into t3code/codex-turn-mapping Sep 26, 2026
24 of 25 checks passed
@juliusmarminge
juliusmarminge deleted the v2/codex-item-maps branch September 26, 2026 22:03
dillonc-dev added a commit to exarch-run/t3code that referenced this pull request Sep 27, 2026
…ession (pingdotgg#13876)

Adapted: the completed-plan hunk conflicted on a comment from upstream
0481b76, which the fork doesn't have. The resolution keeps the fork's
lines and applies upstream's change to them.

(cherry picked from commit 93dbb55)

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