feat(sdk): persist compaction and injected context through the transcript storage state - #4894
Conversation
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe change adds in-memory transcript storage and persisted runtime-state helpers. Merge Risk: 🟡 Moderate · up to Conversational context injected during tool-result flows can affect the current model step but be missing from later transcript processing and continuation runs. This breaks the new persistence guarantee for a reachable injection path and should be fixed before merge. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 36.36% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 2 files. (1 skipped: 1 too large.) ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
b986d19 to
92d04a8
Compare
@trigger.dev/build
trigger.dev
@trigger.dev/core
@trigger.dev/python
@trigger.dev/react-hooks
@trigger.dev/redis-worker
@trigger.dev/rsc
@trigger.dev/schema-to-json
@trigger.dev/sdk
commit: |
…ript storage state The model lane after a compaction cannot be rebuilt from the transcript, so every continuation used to re-read the whole conversation and summarise it again. The runtime now records the compacted lane in the storage's state slot, with the transcript id it covers and a fingerprint of that prefix, and rebuilds from it at boot when the prefix is unchanged. A rollback or edit that reconverts the lane clears the state in the same changeset as the truncate. Conversational messages added with chat.inject are recorded the same way, anchored to the transcript message they followed, so they survive a continuation instead of living only in the worker that received them. Adds an in-memory storage that logs the changesets it receives, and a test-only override for the storage the runtime persists through, so the exact changesets for a turn, a mid-turn steer, a compaction, a rollback and an injection are asserted.
92d04a8 to
1677c26
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Team
Run ID: 1ca0ff6a-b54c-4e6f-90a9-42e719cf45df
📒 Files selected for processing (3)
packages/trigger-sdk/src/v3/ai.tspackages/trigger-sdk/src/v3/transcriptStorage.tspackages/trigger-sdk/test/transcript-changesets.test.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.
📜 Review details
🧰 Additional context used
📓 Path-based instructions (9)
Always import from `@trigger.dev/sdk`.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/trigger-sdk/test/transcript-changesets.test.tspackages/trigger-sdk/src/v3/ai.tspackages/trigger-sdk/src/v3/transcriptStorage.ts
We use vitest exclusively.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/trigger-sdk/test/transcript-changesets.test.ts
**Prefer static imports over dynamic imports.**
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/trigger-sdk/test/transcript-changesets.test.tspackages/trigger-sdk/src/v3/ai.tspackages/trigger-sdk/src/v3/transcriptStorage.ts
Add crumbs as you write code — not just when debugging.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/trigger-sdk/test/transcript-changesets.test.tspackages/trigger-sdk/src/v3/ai.tspackages/trigger-sdk/src/v3/transcriptStorage.ts
In the Trigger.dev SDK (packages/trigger-sdk), prefer isomorphic code like fetch and ReadableStream instead of Node.js-specific code
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/trigger-sdk/test/transcript-changesets.test.tspackages/trigger-sdk/src/v3/ai.tspackages/trigger-sdk/src/v3/transcriptStorage.ts
Use vitest for all tests in the Trigger.dev repository
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/trigger-sdk/test/transcript-changesets.test.ts
Use function declarations instead of default exports
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/trigger-sdk/test/transcript-changesets.test.tspackages/trigger-sdk/src/v3/ai.tspackages/trigger-sdk/src/v3/transcriptStorage.ts
Use types over interfaces for TypeScript Avoid using enums; prefer string unions or const objects instead
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/trigger-sdk/test/transcript-changesets.test.tspackages/trigger-sdk/src/v3/ai.tspackages/trigger-sdk/src/v3/transcriptStorage.ts
When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs Do not use high-cardinality attributes in OTEL metr...
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
packages/trigger-sdk/test/transcript-changesets.test.tspackages/trigger-sdk/src/v3/ai.tspackages/trigger-sdk/src/v3/transcriptStorage.ts
🧠 Learnings (2)
📚 Learning: 2026-08-16T18:36:58.179Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 4537
File: packages/trigger-sdk/test/normalizeKeyString.test.ts:1-2
Timestamp: 2026-08-16T18:36:58.179Z
Learning: For related SDK `chat.agent` tests in the Trigger.dev repository—including chat channels, handover, snapshot, and transport-event coverage—keep new test files under `packages/trigger-sdk/test/` rather than colocating them with the `packages/trigger-sdk/src/v3/` source files.
Applied to files:
packages/trigger-sdk/test/transcript-changesets.test.ts
📚 Learning: 2026-06-04T18:16:35.386Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 3836
File: apps/supervisor/src/backpressure/backpressureMonitor.ts:3-5
Timestamp: 2026-06-04T18:16:35.386Z
Learning: When reviewing TypeScript in this repo, apply the rule “prefer type aliases over interfaces” only to data/object shapes and union/intersection type modeling. If an interface is being used as a behavioral contract for collaborators to implement (e.g., method-shape interfaces that define required behavior, such as `BackpressureLogger` / `BackpressureSignalSource` in `apps/supervisor/src/backpressure/backpressureMonitor.ts`), keep it as an `interface` and do not flag it as a type-alias-vs-interface violation.
Applied to files:
packages/trigger-sdk/src/v3/transcriptStorage.ts
🔇 Additional comments (12)
packages/trigger-sdk/src/v3/transcriptStorage.ts (4)
237-237: LGTM!Also applies to: 263-279
281-324: LGTM!
326-373: LGTM!
380-447: LGTM!packages/trigger-sdk/src/v3/ai.ts (7)
89-101: LGTM!
6892-6903: LGTM!
6951-6967: LGTM!Also applies to: 6988-6988
7073-7074: LGTM!
7419-7426: LGTM!
8098-8099: LGTM!Also applies to: 8137-8138, 8271-8272, 8322-8323, 8377-8378, 8560-8561, 8825-8826, 8946-8947, 9174-9175, 9235-9236, 9601-9602
8852-8853: LGTM!Also applies to: 9067-9068
packages/trigger-sdk/test/transcript-changesets.test.ts (1)
1-333: LGTM!Source: Learnings
| const injected = bgQueue.splice(0); | ||
| accumulatedMessages.push(...injected); | ||
| laneInjections.push({ | ||
| afterId: accumulatedUIMessages.at(-1)?.id ?? "", | ||
| messages: injected, | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Background injections drained through the inner-loop prepareStep path are never tracked, so they are silently lost from accumulatedMessages and never persisted.
This drain only runs when the last accumulated model message is not a tool message. The comment on this block states the fallback: when the tail is a tool message, the queued messages are picked up instead by toStreamTextOptions()'s own auto-injected prepareStep (the background-context-injection step further up in this file). That sibling drain returns the injected content only inside resultMessages for the current streamText step. It does not push into accumulatedMessages and does not push an entry into laneInjections.
Two consequences follow from the missing sync:
- Within the same run,
accumulatedMessagesno longer reflects what the model actually saw for that step. Outer-loop compaction,onTurnComplete.messages/newMessages, and any later reconversion checkpoint all operate on a lane that is missing the injected content. - On a continuation boot,
restoreModelLanehas no record of that injection (it was never added tolaneInjections, and it is not part of the transcript, so it cannot be recovered by reconverting UI messages either). This directly contradicts the PR's stated goal that "Injected messages are stored with anchors to the transcript messages they followed and reinserted on continuation" — for this specific, already-documented fallback case, they are not.
This is reachable whenever a chat.inject() call queues conversational content while the accumulator's last model message is a tool message (for example, right after a HITL tool-result exchange), and the agent's turn involves multiple streamText steps (tool calls).
Fix this by reconciling the inner-loop background-injection drain the same way drainSteeringQueue/reconcilePendingSteer reconcile steering messages: record what was actually drained and its anchor, then merge it back into accumulatedMessages and laneInjections once the turn's userRun/streamText call returns (before the turn's snapshot is written). For example, in the toStreamTextOptions() prepareStep step handling background context (around the block that does const injected = bgQueue.splice(0); resultMessages = [...(resultMessages ?? messages), ...injected];), also stash injected alongside the current tail id so the caller can merge it back:
// toStreamTextOptions()'s prepareStep, background-context step:
const bgQueue = locals.get(chatBackgroundQueueKey);
if (bgQueue && bgQueue.length > 0) {
const injected = bgQueue.splice(0);
resultMessages = [...(resultMessages ?? messages), ...injected];
const pendingBg = locals.get(chatPendingBackgroundInjectionKey) ?? [];
pendingBg.push({ afterId: <current UI tail id>, messages: injected });
locals.set(chatPendingBackgroundInjectionKey, pendingBg);
}Then, after userRun/streamText returns, merge chatPendingBackgroundInjectionKey into accumulatedMessages and laneInjections the same way the existing pre-run drain does at lines 8627-8632.
None of the new tests exercise this fallback (they only cover the pre-run drain, triggered from onTurnComplete). Consider adding a test that forces the accumulator's tail to be a tool message before queuing a chat.inject() call, to lock in the fix.
1 participant
Summary
Makes a compaction summary and
chat.injectcontext survive a continuation run, for every storage including the default.Until now the model lane after a compaction lived only in the running worker. When the next run booted it rebuilt the lane from the transcript, so every continuation re-read the whole conversation and summarised it again. The same applied to conversational messages added with
chat.inject: they lived for the worker's life and vanished on a continuation.Design
The runtime records what it cannot rebuild from the transcript in the storage's
stateslot: after a compaction, the compacted model lane together with the transcript id it covers and a fingerprint of that prefix; for injections, the messages anchored to the transcript message they followed. At boot the compacted lane is used when the covered prefix is unchanged, otherwise the lane is converted from the transcript as before, and injections are re-inserted after their anchors.A rollback or edit that reconverts the lane clears the state in the same changeset as the
truncateAfter, so a storage never holds a summary for a transcript it no longer matches. A mid-turn steering message reaches the storage as aputin that turn's changeset.An in-memory storage that logs the changesets it receives, and a test-only override for the storage the runtime persists through, let the tests assert the exact changesets for a turn, a steer, a compaction, a rollback and an injection.