Skip to content

fix(server): bound getThreadProjection history with explicit paging - #11514

Closed
saphid wants to merge 624 commits into
pingdotgg:t3code/codex-turn-mappingfrom
saphid:work/ov2-20260913-06
Closed

saphid wants to merge 624 commits into
pingdotgg:t3code/codex-turn-mappingfrom
saphid:work/ov2-20260913-06

Conversation

@saphid

@saphid saphid commented Sep 13, 2026 •

Copy link
Copy Markdown
Contributor

Summary

orchestration.getThreadProjection materialized an entire long-lived thread — every
turn item, payload, and checkpoint — into one response. Simply capping the reply is
not acceptable: a response silently cut to N rows leaves clients with no path to
older history. This PR bounds snapshot rows, encoded bytes, and decode work while
keeping the complete raw history in storage and adding cursor-based paging to reach
it, with an explicit opt-in so legacy clients are never silently truncated.

Behavior

  • getThreadProjection stays opt-in: only requests with
    acceptBoundedSnapshot: true receive the bounded recent-turn window plus paging
    metadata (snapshotSequence, historyCursor, hasMoreHistory,
    latestLocalTurnOrdinal, payloadBudgetExceeded). Pre-PR clients never send the
    flag and keep the full projection — complete history and out-of-window checkpoint
    rewind included.
  • Bounded reads bound work inside the query: bounded_json write-time previews
    (migration 054 backfills them) mean oversized raw payloads never cross
    SQLite→JS; the window enforces row and byte budgets, keeps the newest rows,
    drops a partial oldest turn at its exact (ordinal, turn_item_id) boundary, and
    shares the budget across fork ancestry. Rows bigger than any budget stay
    reachable through at-least-one-row semantics.
  • Older history pages through orchestration.getThreadHistoryPage (WS) or the
    existing HTTP history endpoint with the same opaque cursor. Cursor v2 anchors on
    (thread digest, ordinal, item digest) — fixed size regardless of id length and
    stable across driver id mangling; v1 cursors still decode. A deleted or
    superseded anchor returns typed invalid_history_cursor; the client reseeds
    progressive history from a fresh bounded snapshot (guarded by applyLock and
    the snapshot sequence so it cannot clobber in-flight socket events), and a
    structured not-found marks the thread deleted.
  • orchestration.getThreadCheckpointContext returns whole-history checkpoint
    metadata (run/scope/checkpoint ids, ordinals, status, refs — no payloads) so
    bounded clients resolve rewind ordinals without the full projection. The
    threadCheckpointContext capability advertises it; revertThreadCheckpoint
    prefers it and falls back to an unbounded projection read on older servers.
    stopThreadSession also reads the unbounded projection so ready provider
    sessions outside the retained cohort are still detached.
  • Migration 055 adds item_id_digest (+ index) so v2 anchor resolution is an
    indexed lookup, not a per-page payload decode of every equal-ordinal sibling.
  • Both new RPCs require the existing orchestration.read scope.

Limitations

  • Interactive arrays (approval_request.options, todo_list.steps,
    user_input_request.questions/options) preserve all members in previews —
    dropping them would silently change what the user is asked — so a preview can
    exceed the per-row byte cap; emitted items are still billed at their true size,
    so window budgets hold.
  • Bounded snapshots retain only the timeline cohort's dependent collections; live
    control state outside the cohort is reachable via getThreadCheckpointContext
    or an unbounded read, not via visibleTurnItems.
  • Memory reduction is established by row/byte-budget tests, not a measured
    heap-profile run.

Test plan

  • 183 focused tests green on the head: ProjectionStore.test.ts,
    threadHistoryPaging.test.ts, ws.test.ts, commands.test.ts,
    threads-sync.test.ts, 054/055 migration tests — huge histories, oversized single
    rows, pagination to the true beginning (v1+v2 cursors, equal-ordinal siblings,
    lone-surrogate ids, fork markers), reconnect/reseed, legacy decode, and
    authorization.
  • Regressions mutation-verified (wrapper-vs-item billing, ordinal-only
    alignment, exempt-all collision groups all detected).
  • Per-package typecheck clean; fmt clean.

Supersedes the approach explored in #10512 — credited for the earlier proposal.


Built with SWE-2 Max via Devin/T3 Code.
Coordination: T3 thread c1c50bb3-6c48-44ec-a174-25715c55a302; campaign issue
saphid/t3code-personal#298.

@cursor

cursor Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Sep 13, 2026
Comment thread apps/server/src/orchestration-v2/threadHistoryPaging.ts
@macroscopeapp

macroscopeapp Bot commented Sep 13, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR substantially changes production history retrieval, persistence schemas, RPC contracts, authorization, and client defaults while introducing new paging and recovery capabilities. Its cross-cutting behavior and an unresolved high-severity missing-thread error-classification concern require human review.

Not approved because:

  • Per-PR cost limit exceeded (workspace setting). Approvability relies on correctness review in order to determine eligibility

Review your spending limits in Billing settings, or comment @macroscope-app review this PR to bypass the limit and review now. You can add or adjust custom eligibility rules. Learn more.

@saphid
saphid force-pushed the work/ov2-20260913-06 branch 2 times, most recently from 8019ff8 to 38666c4 Compare September 13, 2026 10:12
@github-actions github-actions Bot added size:XXL 1,000+ changed lines (additions + deletions). and removed size:L 100-499 changed lines (additions + deletions). labels Sep 14, 2026
Comment thread apps/server/src/orchestration-v2/ProjectionStore.ts Outdated
Comment thread apps/server/src/orchestration-v2/ProjectionStore.ts
Comment thread apps/server/src/orchestration-v2/ProjectionStore.ts Outdated
Comment thread apps/server/src/orchestration-v2/ProjectionStore.ts Outdated
Comment thread packages/client-runtime/src/operations/commands.ts
Comment thread packages/client-runtime/src/state/threads.ts
Comment thread apps/server/src/orchestration-v2/ProjectionStore.ts Outdated
Comment thread apps/server/src/orchestration-v2/boundedPayloadPreview.ts Outdated
@juliusmarminge
juliusmarminge force-pushed the work/ov2-20260913-06 branch 2 times, most recently from d914d92 to 027d8d0 Compare September 14, 2026 23:29
@cursor

cursor Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@macroscopeapp

macroscopeapp Bot commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

Macroscope skipped reviewing this pull request. Per-PR cost limit exceeded (workspace setting).

Reviews on this PR have cost $49.04 so far. This review would add an estimated $13.60, bringing the total to $62.63 — above your per-PR limit of $50.00.

Tip

To get this pull request reviewed, you can:

  1. Comment @macroscope-app on this PR to request a manual review (monthly spend limits still apply).
  2. Exclude large or generated files from review by adding a pattern to your .macroscope/ignore.md — note that creating this file replaces Macroscope's built-in default ignores rather than extending them.
  3. Raise your cost limit in your workspace billing settings.

Turn off this reminder going forward

@saphid
saphid force-pushed the work/ov2-20260913-06 branch from 75e77b7 to 619294f Compare September 15, 2026 01:55
@cursor

cursor Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

Comment thread apps/server/src/orchestration-v2/ProjectionStore.ts Outdated
@juliusmarminge
juliusmarminge force-pushed the t3code/codex-turn-mapping branch from 163e8e6 to c2dbc35 Compare September 15, 2026 05:51
@saphid
saphid force-pushed the work/ov2-20260913-06 branch from 5f516ec to 614afb8 Compare September 15, 2026 06:00
@cursor

cursor Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

visibleTurnItems: bounded,
},
};
}).pipe(Effect.mapError((cause) => new ProjectionStoreReadError({ threadId, cause }))),

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.

🟠 High orchestration-v2/ProjectionStore.ts:6428

Missing threads from getThreadSnapshot are reclassified as ProjectionStoreReadError, so callers and the HTTP isThreadNotFound mapper receive an internal read error instead of thread_not_found. Preserve ProjectionStoreThreadNotFoundError before wrapping other failures, matching the SQL implementation.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ProjectionStore.ts around line 6428:

Missing threads from `getThreadSnapshot` are reclassified as `ProjectionStoreReadError`, so callers and the HTTP `isThreadNotFound` mapper receive an internal read error instead of `thread_not_found`. Preserve `ProjectionStoreThreadNotFoundError` before wrapping other failures, matching the SQL implementation.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

False positive — the mapError at :6428 is scoped inside the flatMap callback and wraps only the decode/bounding gen. service.getThreadSnapshot(threadId) is the pipe source: its ProjectionStoreThreadNotFoundError (from getThreadProjection at :5889) propagates past flatMap unwrapped, so callers see the bare error — error.cause._tag === "ProjectionStoreThreadNotFoundError" still matches isThreadNotFound → thread_not_found, identical to the SQL impl. Pinned by regression tests on both drivers in fc07d990b: "surfaces missing threads as not-found, not read errors" (SQL) and "memory snapshot windows surface missing threads as not-found, not read errors" (memory).

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.

Sorry, I'm unable to act on this request because you do not have permissions within this repository.

juliusmarminge and others added 29 commits September 20, 2026 18:40
Co-authored-by: Vitalii Yehorov <vitalyiegorov@gmail.com>
The legacy projection RPC materialized entire long-lived threads. Serve
the bounded recent-turn window instead, with additive paging metadata
(historyCursor, hasMoreHistory, snapshotSequence) old clients safely
ignore, plus a cursor-paginated getThreadHistoryPage RPC sharing the
HTTP cursor format so newer clients can reach older history. Add a
capability-gated getThreadCheckpointContext RPC so ordinal-based rewind
still resolves checkpoints outside the bounded window, and reject
unresolvable cursor anchors with a typed invalid_history_cursor error
rather than guessing from window-relative positions.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
… rejected

An unresolvable history anchor now returns a typed invalid_history_cursor
error, which would leave loadEarlier retrying the same dead cursor. On
that error, refetch the bounded snapshot and install its fresh projection
and history meta so paging resumes from a live anchor.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Bounded snapshot/history reads still transferred and decoded full raw
payloads, and decoded-payload identifiers compared against bound id
columns miss whenever the driver's UTF-8 binding diverges (node:sqlite
folds lone surrogates to U+FFFD, bun:sqlite stores WTF-8 verbatim).

Store a schema-safe bounded_json preview beside every raw payload
(migration 053) and read COALESCE(bounded_json, payload_json); resolve
cursor anchors to rowids with a persisted item_id_digest index
(migration 054); compare all bounded-path cohort identities in decoded
JSON space or as hex of the decoded bytes; keep hex(NULL)='' out of the
node cohort so thread-unscoped hydration arms cannot match ownerless
foreign rows; and guard unscoped json_extract arms with json_valid so a
malformed foreign row cannot abort another thread's bounded read.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Two remaining O(history) gaps in the bounded snapshot path. The billed
CTE ran the suffix byte sum over all eligible rows; the byte filter is
prefix-monotonic in newest-first order, so pre-limiting to the newest
maxWindowRows is output-identical and bounds the scan to O(window).

The in-memory getThreadSnapshotWindow bounded payload bytes but passed
every historical collection through boundRows unfiltered; it now derives
the retained cohort from windowed turn items and filters all 13
collections arm-for-arm with the SQL hydration predicates, including a
global providerThreadsById replay index so cross-projection updates are
latest-write-wins like SQL's ON CONFLICT(provider_thread_id).

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Greedy first-come budgeting let head array members spend the whole member
budget, so a pending user_input_request lost tail questions and options its
compacted skeleton could have afforded. Measure each member's floor size
first: when all floors fit the budget every member survives and splits the
slack; members only drop when floors genuinely overflow. Interactive arrays
(user_input_request questions/options, approval_request options, todo_list
steps) always keep their members — dropping them silently changes what the
user is asked, and their stored input already bounds the floor.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Each fork-ancestor readProjection previously received the full window
budget, so a fork chain could decode and merge depth x window rows before
the wire page trimmed them, and the boundary anchor row was billed as new
content (stranding ancestors behind oversized anchors). Ancestor reads now
spend from a shared remaining budget and report hasOlderHistory when it is
exhausted; billing covers exactly the rows each segment contributes to the
merged timeline (including rolled-back runs restored through a required
run), exempts only the delivered boundary row, and identifies that row by
stored identity plus decoded size so cross-runtime id collisions cannot
exempt a whole group. Equal-ordinal siblings below the anchor stay billed
in stored UTF-8 order via the runtime-aware comparator.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…review backfill

Turn-mode alignment dropped the partial oldest turn at an ordinal-only boundary, so an equal-ordinal sibling sorting before the retained turn start leaked into the initial window and could page again later. The cut now carries the boundary row's (ordinal, turn_item_id) position. The 053 backfill also staged up to 200 unbounded payload_json values per page; it now fetches one row at a time so startup memory stays bounded by a single payload.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…04 threads

stopThreadSession iterated the bounded projection's providerSessions, so a ready session outside the retained timeline cohort was never detached and kept running. It now requests the unbounded projection. And when a dead history cursor's bounded reseed 404s, the stale projection and cursor are dropped via setDeleted instead of rendering the deleted thread forever.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
The in-memory window billed the projected row wrapper against maxWindowBytes while SQL bills LENGTH(bounded_json) — the item payload only. Wrapper overhead made the memory store drop rows the SQL store kept, diverging the two implementations' windows on identical history.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Single-value reason literal duplicated a distinction the error union
already expresses; getThreadHistoryPage now carries
OrchestrationV2InvalidHistoryCursorError and callers branch on its tag.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Base repair, separable: the release_replay_gate steps added in 20a410d
burn the 10k iteration budget in ~180ms while the replay feed is still
awaiting an event-loop turn on loaded runners, failing
queued_cancelled_while_active/codex in CI. Adopts the file's existing
scenarioWaitExhausted pattern (attempts AND 60s deadline) already used by
the sibling waits.
…after rebase

The codex-turn-mapping rewrite relocated subscribeOrchestrationV2Thread,
which silently misapplied three ws.ts hunks during the --onto rebase:
the acceptBoundedSnapshot gate on getThreadProjection (legacy clients
would have received silently bounded history), the v2 cursor anchor
decode (ordinal + thread/item digests), and hasOlderHistory on the
history-page selector. Restores all three to match the pre-rebase head.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…removal

The rebased base dropped NodeSqliteClient.layerMemory; match the base's
layer({ filename: ":memory:" }) call form in the bounded-history
migration tests.
…mark

A partial timeline used a bare ordinal watermark to decide whether a
turn-item.updated for a row outside the window was stale history or new
work. Item ordinals interleave non-monotonically across runs (user items
at runOrdinal*100, provider items at providerTurnOrdinal*100+index), so a
genuinely new item from a later run could sort below the watermark and be
dropped with no replay path to recover it.

shouldDropMissingPartialTurnItem now checks run/provider-turn lineage
first: an item whose run is missing or still live, or whose provider turn
is missing or pending/running, is treated as new regardless of ordinal.
Only items with fully terminal lineage fall back to the watermark check,
and lineage-less items keep the prior behavior so stale-history drops
still work.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…ecovery

Three bounded-history gaps found in review:

Resolved runtime requests hydrated by provider-turn membership, so one
retained turn could pull every resolved approval/question it ever owned
past the window caps. Pending requests still hydrate unconditionally, but
resolved ones now hydrate only when a retained item references them by
requestId/runtimeRequestId, and the resolved cohort shares a 1 MiB
aggregate stored-byte budget (newest first) in both the SQL and memory
paths.

Pending requests could also lose their display item entirely when it
paged out, hiding the question from derivePendingThreadRequests. Items
referenced by pending requests are now retained as hidden dependencies
(in_window = 0, never pageable, reserved in the byte budget) alongside
the existing interrupt-request dependency handling.

Compacted items were indistinguishable from complete ones with no way to
recover dropped content. Turn-item previews now carry payloadTruncated,
migration 57 backfills the flag onto existing previews, read-time
compaction stamps it too, and the raw row is retrievable through the new
authenticated orchestration.getThreadTurnItem RPC (read scope, single
bounded row, null when absent).

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…pages

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…hots

Bounded snapshot hydration already capped resolved runtime requests, but
completed subagents, run attempts, provider threads/turns, checkpoint
scopes, checkpoints, and context transfers still hydrated every record
matching a retained run/node cohort. A retained run with thousands of
completed subagents could fan out the snapshot unbounded.

Live rows and rows referenced by retained items still hydrate
unconditionally; cohort-only completed records now share one aggregate
1 MiB stored-byte budget per collection, newest first. SQL bills the
fetched bounded_json preview; the in-memory path bills bytesOfJson, so
both paths bound the bytes actually decoded.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@juliusmarminge

Copy link
Copy Markdown
Member

Closing as superseded. v2 landed its own bounded-projection model on Sep 11 (12fb985, e2c5227, 6076240, c3e2920):

  • getThreadProjection is served from getThreadSnapshotWindow through buildBoundedThreadProjection and is bounded unconditionally, so stale clients cannot materialize a full transcript.
  • subscribeThread accepts acceptBoundedSnapshot and returns historyCursor / hasMoreHistory; older history pages through the HTTP endpoint in orchestration-v2/http.ts, and client-runtime already opts in.
  • Control-plane arrays (checkpoints, runs, provider sessions) stay whole; only messages / turnItems are windowed, which removes the need for a separate checkpoint-context RPC.

That solves the unbounded read with a much smaller surface than this branch (three migrations, write-time previews, a new cursor format), so I don't want to carry both models. If there is a measured hot spot that the current paging does not cover, for example oversized single-row payloads, a small PR with the measurement attached would be welcome.

Thanks for the effort on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XXL 1,000+ 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.

9 participants