feat(sdk): route chat.agent transcript persistence through a TranscriptStorage seam - #4893
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 transcript storage with reducers, shadow-based diffs, pagination, cursors, and version 2 snapshot persistence. It adds injectable and production snapshot I/O paths with failure handling. Chat agent persistence now uses transcript changesets for loading, completed turns, actions, and errors. Mocks and integration tests support version 1 compatibility and version 2 message envelopes. Merge Risk: 🟠 High · up to Transcript state can remain incorrectly partial, long-lived workers can accumulate chat histories, and snapshot data may traverse an unsafe destination. These issues should be resolved before merge. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description gives a detailed and relevant summary of the TranscriptStorage design, runtime behavior, and default snapshot implementation, but it omits the required issue reference, checklist, testing steps, changelog, and screenshots sections from the repository template. ✨ 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 |
4c91a95 to
a7e6626
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: |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| * blob from that, exactly as before this storage existed. | ||
| */ | ||
| export function snapshotTranscriptStorage(): TranscriptStorage<unknown> { | ||
| const transcripts = new Map<string, TranscriptState>(); |
There was a problem hiding this comment.
🔴 Transcript cache grows without bound
Each loaded or saved chat leaves its full transcript in transcripts, which has no eviction or run cleanup. Long-lived workers can exhaust memory.
Prompt for agents
The built-in default storage is created once at module scope, and snapshotTranscriptStorage keeps complete transcripts in a private Map forever. The runtime only needs that mutable state for one chat.agent run. Make the built-in storage state run-scoped, or explicitly release each chat's state when its run ends. Preserve the ability to reduce multiple saves during one run without an extra GET, and avoid sharing mutable transcript state across concurrent runs.
Was this helpful? React with 👍 or 👎 to provide feedback.
| await saveTranscript({ | ||
| reason: "turn-complete", | ||
| messages: accumulatedUIMessages, | ||
| turn, | ||
| trigger: storageTrigger(currentWirePayload.trigger), | ||
| clientData, | ||
| lastOutEventId: lastSnapshotOutEventId, |
There was a problem hiding this comment.
🟡 Stopped responses are marked final
When a user stops generation, saveTranscript omits the partial response ID from nonFinalIds. Snapshot consumers then treat the interrupted response as complete.
Prompt for agents
The normal completion path also handles user-stopped generation through wasStopped and persists the captured partial assistant response. Pass that response ID to saveTranscript as non-final when wasStopped is true, matching the TranscriptChange and TranscriptSnapshotEntry contract. Add coverage asserting that a stopped response is written with final: false while ordinary completed responses remain final.
Was this helpful? React with 👍 or 👎 to provide feedback.
| await transcriptStorage.save( | ||
| { | ||
| chatId: payload.chatId, |
There was a problem hiding this comment.
🔍 Storage identity contract changed
Storage now receives payload.chatId, while the replaced path preferred payload.sessionId. Define the intended persistence key before exposing custom adapters.
Was this helpful? React with 👍 or 👎 to provide feedback.
| export async function writeChatSnapshot<TUIMessage extends UIMessage>( | ||
| sessionId: string, | ||
| snapshot: TranscriptSnapshotV2<TUIMessage> | ||
| ): Promise<void> { | ||
| if (writeChatSnapshotImpl) { | ||
| await writeChatSnapshotImpl<TUIMessage>(sessionId, snapshot); | ||
| return; | ||
| } | ||
| const apiClient = apiClientManager.clientOrThrow(); | ||
| let presignedUrl: string; | ||
| try { | ||
| const resp = await apiClient.createChatSnapshotUploadUrl(sessionId); | ||
| presignedUrl = resp.presignedUrl; | ||
| } catch (error) { | ||
| logger.warn("chat.agent: snapshot presign (write) failed; next run will replay further", { | ||
| error: error instanceof Error ? error.message : String(error), | ||
| sessionId, | ||
| }); | ||
| return; | ||
| } | ||
| let response: Response; | ||
| try { | ||
| response = await fetch(presignedUrl, { | ||
| method: "PUT", | ||
| headers: { "content-type": "application/json" }, | ||
| body: JSON.stringify(snapshot), | ||
| }); | ||
| } catch (error) { | ||
| logger.warn("chat.agent: snapshot upload failed; next run will replay further", { | ||
| error: error instanceof Error ? error.message : String(error), | ||
| sessionId, | ||
| }); | ||
| return; | ||
| } | ||
| if (!response.ok) { | ||
| logger.warn("chat.agent: snapshot upload returned non-OK; next run will replay further", { | ||
| status: response.status, | ||
| sessionId, | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🔍 Upload failures resolve as successful saves
writeChatSnapshot swallows upload failures, so the runtime advances its shadow after an unsuccessful default save. Later changesets omit those failed changes.
Was this helpful? React with 👍 or 👎 to provide feedback.
| function install(initial: unknown) { | ||
| stored = undefined; | ||
| reads = 0; | ||
| writes = []; | ||
| __setReadChatSnapshotImplForTests(() => { | ||
| reads++; | ||
| return initial; | ||
| }); | ||
| __setWriteChatSnapshotImplForTests((_id, snapshot) => { | ||
| stored = snapshot as TranscriptSnapshotV2; | ||
| writes.push(stored); | ||
| }); |
There was a problem hiding this comment.
🔍 Tests bypass the no-mocking rule
New storage tests replace global snapshot I/O implementations. The repository mandates testcontainers instead of mocks, so maintainers must approve this exception.
Was this helpful? React with 👍 or 👎 to provide feedback.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Team
Run ID: 66ef3764-47c3-4405-92bd-7b3cfb661d6b
📒 Files selected for processing (12)
apps/webapp/test/chat-snapshot-integration.test.tsapps/webapp/test/replay-after-crash.test.tspackages/trigger-sdk/src/v3/ai.tspackages/trigger-sdk/src/v3/chatSnapshotIo.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.tspackages/trigger-sdk/src/v3/transcriptStorage.tspackages/trigger-sdk/test/action-snapshot.test.tspackages/trigger-sdk/test/action-stream-accumulator.test.tspackages/trigger-sdk/test/action-turn.test.tspackages/trigger-sdk/test/chat-snapshot.test.tspackages/trigger-sdk/test/mockChatAgent.test.tspackages/trigger-sdk/test/transcript-storage.test.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
📜 Review details
🧰 Additional context used
📓 Path-based instructions (13)
Always import from `@trigger.dev/sdk`.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/trigger-sdk/test/action-snapshot.test.tspackages/trigger-sdk/test/chat-snapshot.test.tspackages/trigger-sdk/test/action-stream-accumulator.test.tspackages/trigger-sdk/test/transcript-storage.test.tspackages/trigger-sdk/test/action-turn.test.tspackages/trigger-sdk/test/mockChatAgent.test.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.tspackages/trigger-sdk/src/v3/transcriptStorage.tspackages/trigger-sdk/src/v3/ai.tspackages/trigger-sdk/src/v3/chatSnapshotIo.ts
We use vitest exclusively.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/trigger-sdk/test/action-snapshot.test.tspackages/trigger-sdk/test/chat-snapshot.test.tspackages/trigger-sdk/test/action-stream-accumulator.test.tspackages/trigger-sdk/test/transcript-storage.test.tspackages/trigger-sdk/test/action-turn.test.tspackages/trigger-sdk/test/mockChatAgent.test.tsapps/webapp/test/chat-snapshot-integration.test.tsapps/webapp/test/replay-after-crash.test.ts
Test files must not import `app/env.server.ts`; pass configuration as options instead.
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
apps/webapp/test/chat-snapshot-integration.test.tsapps/webapp/test/replay-after-crash.test.ts
**Prefer static imports over dynamic imports.**
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/trigger-sdk/test/action-snapshot.test.tspackages/trigger-sdk/test/chat-snapshot.test.tspackages/trigger-sdk/test/action-stream-accumulator.test.tspackages/trigger-sdk/test/transcript-storage.test.tspackages/trigger-sdk/test/action-turn.test.tspackages/trigger-sdk/test/mockChatAgent.test.tsapps/webapp/test/chat-snapshot-integration.test.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.tspackages/trigger-sdk/src/v3/transcriptStorage.tspackages/trigger-sdk/src/v3/ai.tspackages/trigger-sdk/src/v3/chatSnapshotIo.tsapps/webapp/test/replay-after-crash.test.ts
Add crumbs as you write code — not just when debugging.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/trigger-sdk/test/action-snapshot.test.tspackages/trigger-sdk/test/chat-snapshot.test.tspackages/trigger-sdk/test/action-stream-accumulator.test.tspackages/trigger-sdk/test/transcript-storage.test.tspackages/trigger-sdk/test/action-turn.test.tspackages/trigger-sdk/test/mockChatAgent.test.tsapps/webapp/test/chat-snapshot-integration.test.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.tspackages/trigger-sdk/src/v3/transcriptStorage.tspackages/trigger-sdk/src/v3/ai.tspackages/trigger-sdk/src/v3/chatSnapshotIo.tsapps/webapp/test/replay-after-crash.test.ts
Use zod for validation in packages/core and apps/webapp
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/test/chat-snapshot-integration.test.tsapps/webapp/test/replay-after-crash.test.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/action-snapshot.test.tspackages/trigger-sdk/test/chat-snapshot.test.tspackages/trigger-sdk/test/action-stream-accumulator.test.tspackages/trigger-sdk/test/transcript-storage.test.tspackages/trigger-sdk/test/action-turn.test.tspackages/trigger-sdk/test/mockChatAgent.test.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.tspackages/trigger-sdk/src/v3/transcriptStorage.tspackages/trigger-sdk/src/v3/ai.tspackages/trigger-sdk/src/v3/chatSnapshotIo.ts
Do not import `env.server.ts` directly or indirectly into test files; instead pass environment-dependent values through options/parameters to make code testable
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/webapp/test/chat-snapshot-integration.test.tsapps/webapp/test/replay-after-crash.test.ts
Access environment variables through the `env` export of `env.server.ts` instead of directly accessing `process.env` Use subpath exports from `@trigger.dev/core` package instead of importing from the root `@trigger.dev/core` path
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/webapp/test/chat-snapshot-integration.test.tsapps/webapp/test/replay-after-crash.test.ts
Use vitest for all tests in the Trigger.dev repository
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/trigger-sdk/test/action-snapshot.test.tspackages/trigger-sdk/test/chat-snapshot.test.tspackages/trigger-sdk/test/action-stream-accumulator.test.tspackages/trigger-sdk/test/transcript-storage.test.tspackages/trigger-sdk/test/action-turn.test.tspackages/trigger-sdk/test/mockChatAgent.test.tsapps/webapp/test/chat-snapshot-integration.test.tsapps/webapp/test/replay-after-crash.test.ts
Use function declarations instead of default exports
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/trigger-sdk/test/action-snapshot.test.tspackages/trigger-sdk/test/chat-snapshot.test.tspackages/trigger-sdk/test/action-stream-accumulator.test.tspackages/trigger-sdk/test/transcript-storage.test.tspackages/trigger-sdk/test/action-turn.test.tspackages/trigger-sdk/test/mockChatAgent.test.tsapps/webapp/test/chat-snapshot-integration.test.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.tspackages/trigger-sdk/src/v3/transcriptStorage.tspackages/trigger-sdk/src/v3/ai.tspackages/trigger-sdk/src/v3/chatSnapshotIo.tsapps/webapp/test/replay-after-crash.test.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/action-snapshot.test.tspackages/trigger-sdk/test/chat-snapshot.test.tspackages/trigger-sdk/test/action-stream-accumulator.test.tspackages/trigger-sdk/test/transcript-storage.test.tspackages/trigger-sdk/test/action-turn.test.tspackages/trigger-sdk/test/mockChatAgent.test.tsapps/webapp/test/chat-snapshot-integration.test.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.tspackages/trigger-sdk/src/v3/transcriptStorage.tspackages/trigger-sdk/src/v3/ai.tspackages/trigger-sdk/src/v3/chatSnapshotIo.tsapps/webapp/test/replay-after-crash.test.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/action-snapshot.test.tspackages/trigger-sdk/test/chat-snapshot.test.tspackages/trigger-sdk/test/action-stream-accumulator.test.tspackages/trigger-sdk/test/transcript-storage.test.tspackages/trigger-sdk/test/action-turn.test.tspackages/trigger-sdk/test/mockChatAgent.test.tsapps/webapp/test/chat-snapshot-integration.test.tspackages/trigger-sdk/src/v3/test/mock-chat-agent.tspackages/trigger-sdk/src/v3/transcriptStorage.tspackages/trigger-sdk/src/v3/ai.tspackages/trigger-sdk/src/v3/chatSnapshotIo.tsapps/webapp/test/replay-after-crash.test.ts
🧠 Learnings (4)
📚 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-storage.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/test/action-turn.test.ts
📚 Learning: 2026-05-19T22:37:47.286Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3671
File: packages/trigger-sdk/test/recovery-boot.test.ts:456-457
Timestamp: 2026-05-19T22:37:47.286Z
Learning: In `packages/trigger-sdk` (Trigger.dev SDK), `logger.warn` (and other SDK logger methods) should route to the Trigger.dev structured logger sink, not to `console.warn`. In SDK tests, `vi.spyOn(console, "warn")` (or similar console spies) should only be used to suppress stray console output; reviewers should not suggest asserting on `console.warn` spies to verify SDK-internal warning/fallback log behavior. Use the SDK’s structured-logger outputs/capture approach instead of console spies.
Applied to files:
packages/trigger-sdk/src/v3/chatSnapshotIo.ts
📚 Learning: 2026-06-09T17:58:04.699Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 3879
File: apps/webapp/app/models/vercelIntegration.server.ts:619-630
Timestamp: 2026-06-09T17:58:04.699Z
Learning: In this codebase, outbound raw `fetch` calls should typically rely on Node/undici’s default request timeout (about ~300s) rather than adding a per-call `AbortController` + `setTimeout` wrapper inside individual functions (e.g. in files like `apps/webapp/app/models/vercelIntegration.server.ts`). During code review, do not flag the absence of a per-call timeout on a single `fetch` as an issue; if per-call timeouts are needed, they should be implemented via a codebase-wide convention (e.g., a shared fetch wrapper or documented pattern) rather than ad-hoc per-function changes.
Applied to files:
packages/trigger-sdk/src/v3/chatSnapshotIo.ts
🪛 ast-grep (0.45.2)
packages/trigger-sdk/src/v3/ai.ts
[warning] 7031-7034: Avoid logging sensitive data
Context: logger.warn("chat.agent: transcript load failed; continuing from the stream tail", {
error: error instanceof Error ? error.message : String(error),
sessionId: sessionIdForSnapshot,
})
Note: [CWE-532] Insertion of Sensitive Information into Log File.
(log-sensitive-data-typescript)
🔇 Additional comments (7)
packages/trigger-sdk/test/mockChatAgent.test.ts (1)
1878-1880: LGTM!apps/webapp/test/chat-snapshot-integration.test.ts (1)
2-2: LGTM!Also applies to: 19-36
apps/webapp/test/replay-after-crash.test.ts (1)
27-27: LGTM!Also applies to: 267-286
packages/trigger-sdk/src/v3/test/mock-chat-agent.ts (1)
3-3: LGTM!Also applies to: 100-100, 236-236, 275-275, 399-410
packages/trigger-sdk/test/action-snapshot.test.ts (1)
66-69: LGTM!packages/trigger-sdk/test/action-stream-accumulator.test.ts (1)
67-70: LGTM!Also applies to: 129-133
packages/trigger-sdk/test/action-turn.test.ts (1)
83-96: LGTM!Also applies to: 130-133
| * next boot resumes from. The shadow only advances when the save | ||
| * succeeds, so a failed save is folded into the next changeset. | ||
| */ | ||
| const saveTranscript = async (opts: { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add required crumbs to the transcript save path.
The new transcript persistence flow has no // @Crumbs marker or `// `#region` `@crumbs block. Add crumbs with a permitted namespace before merge.
As per coding guidelines: “Add crumbs as you write code” and “Do not invent new namespaces — pick from this table or ask first.”
Source: Coding guidelines
| } | ||
| let response: Response; | ||
| try { | ||
| response = await fetch(presignedUrl, { method: "GET" }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- target file ---'
cat -n packages/trigger-sdk/src/v3/chatSnapshotIo.ts
printf '%s\n' '--- direct API and presign definitions ---'
rg -n -g '*.ts' 'getChatSnapshotUrl|createChatSnapshotUploadUrl|generatePresignedUrl|presignedUrl|redirect:' packages apps | head -n 240Repository: triggerdotdev/trigger.dev
Length of output: 21640
🤖 get_repo_knowledge executed:
get_repo_knowledge triggerdotdev/trigger.dev /tmp/coderabbit-repo-knowledge/triggerdotdev-trigger-dev-0bdd0019/learnings /tmp/coderabbit-repo-knowledge/triggerdotdev-trigger-dev-0bdd0019/conventions
Length of output: 27883
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- API client methods and response schema ---'
sed -n '660,715p' packages/core/src/v3/apiClient/index.ts
sed -n '980,1015p' packages/core/src/v3/schemas/api.ts
printf '%s\n' '--- presigned URL generation ---'
sed -n '380,500p' apps/webapp/app/v3/objectStore.server.ts
printf '%s\n' '--- snapshot URL route ---'
cat -n 'apps/webapp/app/routes/api.v1.sessions.$sessionId.snapshot-url.ts' | sed -n '1,125p'Repository: triggerdotdev/trigger.dev
Length of output: 8418
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- object-store client selection and presign implementation ---'
rg -n -g '*.ts' 'function getObjectStoreClient|const getObjectStoreClient|class .*ObjectStore|presign\(|OBJECT_STORE_DEFAULT_PROTOCOL|storeProtocol' apps/webapp/app/v3 apps/webapp/app/services packages | head -n 220
printf '%s\n' '--- surrounding object-store implementation ---'
sed -n '1,180p' apps/webapp/app/v3/objectStore.server.ts
sed -n '180,430p' apps/webapp/app/v3/objectStore.server.tsRepository: triggerdotdev/trigger.dev
Length of output: 16953
🏁 Script executed:
#!/bin/bash
set -e
cat -n apps/webapp/app/v3/objectStoreClient.server.ts | sed -n '1,235p'Repository: triggerdotdev/trigger.dev
Length of output: 8431
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- object-store URL configuration contracts ---'
rg -n -g '*.ts' -g '*.tsx' 'OBJECT_STORE_BASE_URL|OBJECT_STORE_.*BASE_URL|z\.string\(\).*BASE_URL|baseUrl:.*URL|baseUrl.*url' apps/webapp packages | head -n 220Repository: triggerdotdev/trigger.dev
Length of output: 5186
Security Misconfiguration (CWE-319): Cleartext Transmission of Sensitive Information
Reachability: External · Exploitability: Difficult
Enforce HTTPS and reject unvalidated redirects for presigned snapshot I/O.
The server creates presigned URLs from a configurable object-store endpoint, and both snapshot paths pass them directly to fetch. Require https: before each request and set redirect: "error" unless the storage contract validates redirects.
| if (shadow.fingerprints.get(message.id) !== nextShadow.fingerprints.get(message.id)) { | ||
| put(message); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Persist a transition from partial to final.
TranscriptShadow does not retain final state. A first diff can write { final: false }. A later diff with the same message and no nonFinalIds emits no put. The stored entry then remains partial.
Track finality in the shadow and seed it from loaded TranscriptSnapshotEntry values. Emit a put when finality changes. Add a regression test for partial-to-final with unchanged message content.
| export function snapshotTranscriptStorage(): TranscriptStorage<unknown> { | ||
| const transcripts = new Map<string, TranscriptState>(); | ||
|
|
||
| return { | ||
| async load<TUIMessage extends UIMessage = UIMessage>( | ||
| scope: TranscriptScope<unknown>, | ||
| opts?: TranscriptLoadOptions | ||
| ): Promise<TranscriptLoadResult<TUIMessage>> { | ||
| const snapshot = await readChatSnapshot<TUIMessage>(scope.chatId); | ||
| const full: TranscriptState<TUIMessage> = snapshot | ||
| ? { entries: snapshot.messages, state: snapshot.state } | ||
| : emptyTranscriptState<TUIMessage>(); | ||
| transcripts.set(scope.chatId, full as TranscriptState); | ||
|
|
||
| let entries = full.entries; | ||
| if (opts?.before !== undefined) { | ||
| const idx = entries.findIndex((e) => e.id === opts.before); | ||
| if (idx !== -1) entries = entries.slice(0, idx); | ||
| } | ||
| let nextCursor: string | undefined; | ||
| if (opts?.limit !== undefined && entries.length > opts.limit) { | ||
| entries = entries.slice(entries.length - opts.limit); | ||
| nextCursor = entries[0]?.id; | ||
| } | ||
| return { | ||
| messages: entries.map((e) => e.message), | ||
| state: full.state, | ||
| cursors: snapshot | ||
| ? { lastOutEventId: snapshot.lastOutEventId, lastInEventId: snapshot.lastInEventId } | ||
| : undefined, | ||
| nextCursor, | ||
| }; | ||
| }, | ||
|
|
||
| async save(ctx, changeset) { | ||
| const prev = transcripts.get(ctx.chatId) ?? emptyTranscriptState(); | ||
| const next = reduceTranscriptChanges(prev, changeset.changes); | ||
| transcripts.set(ctx.chatId, next); | ||
| await writeChatSnapshot(ctx.chatId, { | ||
| version: 2, | ||
| savedAt: Date.now(), | ||
| messages: next.entries, | ||
| state: next.state, | ||
| lastOutEventId: changeset.cursors?.lastOutEventId, | ||
| lastInEventId: changeset.cursors?.lastInEventId, | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add approved @crumbs instrumentation to the new code.
packages/trigger-sdk/src/v3/transcriptStorage.ts#L222-L267: add crumbs around snapshot load and save state transitions.packages/trigger-sdk/test/transcript-storage.test.ts#L44-L309: add crumbs for the new transcript test flows.
No approved namespace table is included here. Ask for an approved namespace before adding markers.
As per coding guidelines, “Add crumbs as you write code” and “Do not invent new namespaces — pick from this table or ask first.”
📍 Affects 2 files
packages/trigger-sdk/src/v3/transcriptStorage.ts#L222-L267(this comment)packages/trigger-sdk/test/transcript-storage.test.ts#L44-L309
Source: Coding guidelines
| * blob from that, exactly as before this storage existed. | ||
| */ | ||
| export function snapshotTranscriptStorage(): TranscriptStorage<unknown> { | ||
| const transcripts = new Map<string, TranscriptState>(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Bound the process-wide transcript cache.
defaultStorage is a singleton. Each distinct chatId adds a full transcript to transcripts, and no code removes or bounds that entry. A long-lived worker that handles many chats retains all message histories until the process exits.
Scope this cache to a transcript lifecycle, or add eviction that reloads persisted state before a cache-miss save.
| stubApiClient({}); | ||
| const snapshot = buildSnapshotV2(2); | ||
| stubFetch( |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Replace this mock-based regression test with a testcontainer-backed test.
This new test calls helpers that use vi.spyOn, vi.fn, and vi.stubGlobal. Use the real presigned object-store path through testcontainers instead.
As per coding guidelines: “We use vitest exclusively. Never mock anything - use testcontainers instead.”
Source: Coding guidelines
| __setReadChatSnapshotImplForTests(() => { | ||
| reads++; | ||
| return initial; | ||
| }); | ||
| __setWriteChatSnapshotImplForTests((_id, snapshot) => { | ||
| stored = snapshot as TranscriptSnapshotV2; | ||
| writes.push(stored); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Replace snapshot I/O mocks with a testcontainers fixture.
These setters replace the snapshot reader and writer with test doubles. The tests can pass while the production storage transport behaves differently. Use a testcontainers-backed snapshot fixture and assert the persisted blob through that fixture.
As per coding guidelines, “We use vitest exclusively. Never mock anything - use testcontainers instead.”
Source: Coding guidelines
…ptStorage seam Introduces the TranscriptStorage interface (load/save over id-addressed changes: put, remove, truncateAfter, state) and makes the built-in snapshot writer its default implementation. The runtime keeps a shadow of the transcript it last saved and hands the storage the diff after every turn, failed turn and history-changing action, together with the stream cursors the next boot resumes from. The default storage reduces each changeset onto an in-memory copy and rewrites the blob as version 2, so a turn still costs one PUT and no GET, and the boot read goes through load. The snapshot read/write helpers move to their own module so the storage can import them without a cycle; the test seams keep their import path. The mock chat agent harness now reports version 2 snapshots and accepts either version as a seed.
a7e6626 to
49acce9
Compare
1 participant
Summary
Introduces the
TranscriptStorageseam insidechat.agentand makes the built-in snapshot writer its default implementation. No public option yet; the default path behaves as before, apart from the blob now being written as version 2.Design
A storage has
load(called once when a run boots to continue a conversation) andsave(called after every turn, failed turn and history-changing action).savereceives a changeset of id-addressed operations:put(upsert by message id),remove,truncateAfterandstate, plus the stream cursors the next boot resumes from.The runtime keeps a shadow of the transcript it last handed to
save(ids in order plus a fingerprint per message) and diffs the accumulator against it. A changed message in the common prefix is an in-placeput; anything past the prefix is onetruncateAfteron the last common id followed byputs in order. Applying the result reproduces the accumulator exactly for any edit, and the common cases come out as the natural operations: a turn is twoputs, an undo is onetruncateAfter, a regenerate is atruncateAfterand aput. The shadow only advances whensaveresolves, so a failed save is folded into the next changeset; every operation is idempotent, so a retried changeset converges.The default storage reduces each changeset onto an in-memory copy and rewrites the blob, so a turn still costs one PUT and no GET. The snapshot read and write helpers move to their own module so the storage can import them without a cycle; the test seams keep their import path.