Skip to content

fix(sdk): chat.agent transcript follow-ups - #4910

Merged
ericallam merged 4 commits into
mainfrom
fix/transcript-storage-followups
Sep 9, 2026
Merged

fix(sdk): chat.agent transcript follow-ups#4910
ericallam merged 4 commits into
mainfrom
fix/transcript-storage-followups

Conversation

@ericallam

@ericallam ericallam commented Sep 9, 2026

Copy link
Copy Markdown
Member

Follow-ups to the transcript storage feature (#4896), surfaced by dogfooding it in the durable-chat example.

Changes

  • Drop the compaction prefix fingerprint. A compacted lane is reused while its throughId still exists in the transcript. A rollback that crosses the compaction point rebuilds from the transcript; an in-place edit of a summarized message is tolerated (unsupported by design). This removes the implicit requirement that a custom TranscriptStorage preserve exact message JSON — no more "store as TEXT not JSONB / canonicalize keys" trap.
  • Skip persisting a content-less assistant response. A turn that errors before the model writes anything no longer leaves an empty assistant bubble in the transcript or the next turn's context.
  • Non-empty error fallback. A thrown error with no message now surfaces a generic message instead of a blank error to the client.
  • Test harness: seedSessionInTail stamps the harness clientData onto seeded records (matching how production carries each record's metadata), so recovery re-dispatch is testable for clientDataSchema agents.

Verification

  • @trigger.dev/sdk + @trigger.dev/core typecheck clean
  • Full @trigger.dev/sdk suite: 633 passing (includes new tests for the throughId-boundary compaction behavior, the empty-response skip, and clientData-scoped recovery re-dispatch)
  • oxfmt + oxlint clean

- Drop the compaction prefix fingerprint. A compacted lane is reused while
  its throughId still exists in the transcript, so a rollback that crosses
  the compaction point rebuilds from the transcript and an in-place edit of
  a summarized message is tolerated (unsupported by design). A custom
  TranscriptStorage no longer has to preserve exact message JSON for a
  compaction to survive.
- Skip persisting a content-less assistant response, so a turn that errors
  before the model writes anything no longer leaves an empty assistant
  bubble in the transcript or the next turn's context.
- Fall back to a generic error message when a thrown error has no message,
  instead of surfacing a blank error to the client.
- The test harness seedSessionInTail stamps the harness clientData onto
  seeded records, matching how production preserves each record's metadata,
  so recovery re-dispatch is testable for clientDataSchema agents.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VG39FXXkFFU24U5EtJMwPi
@changeset-bot

changeset-bot Bot commented Sep 9, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 1ee0952

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 27 packages
Name Type
@trigger.dev/sdk Patch
@trigger.dev/python Patch
@internal/dashboard-agent Patch
@trigger.dev/build Patch
trigger.dev Patch
@trigger.dev/core Patch
@trigger.dev/react-hooks Patch
@trigger.dev/redis-worker Patch
@trigger.dev/rsc Patch
@trigger.dev/schema-to-json Patch
@trigger.dev/database Patch
@trigger.dev/otlp-importer Patch
@trigger.dev/rbac Patch
@trigger.dev/sso Patch
@internal/clickhouse Patch
@internal/llm-model-catalog Patch
@internal/metrics-pipeline Patch
@internal/redis Patch
@internal/replication Patch
@internal/run-engine Patch
@internal/run-store Patch
@internal/schedule-engine Patch
@internal/tracing Patch
@internal/webhook-engine Patch
@internal/webhook-sources Patch
@internal/testcontainers Patch
@internal/cache Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 0a12860a-13d0-4378-b6ae-ce031dddaf1f

📥 Commits

Reviewing files that changed from the base of the PR and between 298e405 and 1ee0952.

📒 Files selected for processing (2)
  • packages/trigger-sdk/src/v3/ai.ts
  • packages/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.

📜 Recent review details
⏰ Context from checks skipped due to timeout. (44)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (21, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (20, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (19, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (22, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (17, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (13, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (24, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (12, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (23, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (14, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (16, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (15, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (18, 24)
  • GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 24)
  • GitHub Check: sdk-compat / Node.js 20.20 (warp-ubuntu-latest-x64-4x)
  • GitHub Check: sdk-compat / Node.js 24.18 (warp-ubuntu-latest-x64-4x)
  • GitHub Check: sdk-compat / Node.js 26.4 (warp-ubuntu-latest-x64-4x)
  • GitHub Check: sdk-compat / Deno Runtime
  • GitHub Check: sdk-compat / Node.js 22.23 (warp-ubuntu-latest-x64-4x)
  • GitHub Check: sdk-compat / Cloudflare Workers
  • GitHub Check: packages / 🧪 Unit Tests: Packages (3, 3)
  • GitHub Check: fk-cascade-guard / fk-cascade-guard
  • GitHub Check: typecheck / typecheck
  • GitHub Check: packages / 🧪 Unit Tests: Packages (2, 3)
  • GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (1, 2)
  • GitHub Check: sdk-compat / Bun Runtime
  • GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp (2, 2)
  • GitHub Check: packages / 🧪 Unit Tests: Packages (1, 3)
  • GitHub Check: internal / 🧪 Unit Tests: Internal
  • GitHub Check: runops-guard / runops-guard
  • GitHub Check: code-quality / code-quality
  • GitHub Check: audit
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Build and publish previews
🧰 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.ts
  • packages/trigger-sdk/src/v3/ai.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.ts
  • packages/trigger-sdk/src/v3/ai.ts
Add crumbs as you write code — not just when debugging.

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • packages/trigger-sdk/test/transcript-changesets.test.ts
  • packages/trigger-sdk/src/v3/ai.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.ts
  • packages/trigger-sdk/src/v3/ai.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.ts
  • packages/trigger-sdk/src/v3/ai.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.ts
  • packages/trigger-sdk/src/v3/ai.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.ts
  • packages/trigger-sdk/src/v3/ai.ts
🧠 Learnings (1)
📚 Learning: 2026-06-16T09:19:47.637Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3960
File: apps/webapp/test/prismaInfrastructureErrorCapture.test.ts:0-0
Timestamp: 2026-06-16T09:19:47.637Z
Learning: In this repo’s Vitest setup, `vitest.config.ts` uses `globals: true`, so identifiers like `vi`, `describe`, `it`, and `expect` are available as globals in Vitest test files. During code review, do not flag missing `vi`/`describe`/`it`/`expect` imports as a runtime error or correctness issue when they’re used in `*.test.ts/tsx` or `*.spec.ts/tsx` files. Explicit imports are still preferred for consistency, but they’re not required for runtime behavior.

Applied to files:

  • packages/trigger-sdk/test/transcript-changesets.test.ts
🔇 Additional comments (5)
packages/trigger-sdk/src/v3/ai.ts (4)

1352-1353: LGTM!


9152-9236: Correctly tracks whether the response was skipped due to lack of content.

responseWasSkipped is set only when capturedResponseMessage had no parts besides step-start, which is exactly the case where the response was never pushed into accumulatedUIMessages. This flag lets the later late-parts drain (Lines 9475-9506) distinguish a content-less response from one a hook deliberately removed.


9488-9502: Fixes the previously flagged data-loss case without resurrecting hook-removed responses.

This correctly separates two cases:

  • The response id is still present in accumulatedUIMessages — merge the late parts in place (unchanged behavior).
  • The id is absent because the response was content-less and never accumulated (responseWasSkipped) — push it now so late data parts are not lost.
  • The id is absent because a hook (e.g. chat.history.slice(0, -1)) deliberately removed it — leave it removed.

This matches the test at packages/trigger-sdk/test/transcript-changesets.test.ts Lines 561-582, which verifies a hook-removed response is not resurrected.


9781-9783: LGTM!

packages/trigger-sdk/test/transcript-changesets.test.ts (1)

561-582: LGTM!


Walkthrough

The change updates chat.agent transcript handling. Compaction state now uses throughId instead of a prefix fingerprint. Contentless assistant responses are not persisted unless late parts are added. Empty error messages use generic fallback text. Recovery fixtures preserve client data metadata. Tests cover client-data recovery, response persistence, and compaction restoration.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 1ee09

Chat-agent transcripts now omit empty assistant responses, retain late response parts when applicable, preserve hook removals, and restore compaction by transcript boundary. The covered behavior changes are ready to merge.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 4 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the main change: follow-up fixes for chat.agent transcript handling in the SDK.
Description check ✅ Passed The description is detailed and directly explains the transcript, error, recovery, and test-harness changes. It includes verification results, but it does not use all template sections, including the …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 4 files. (1 skipped: 1 too large.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/transcript-storage-followups

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@ericallam
ericallam marked this pull request as ready for review September 9, 2026 10:10
devin-ai-integration[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

ericallam and others added 2 commits September 9, 2026 11:25
When the model returns no content the response is no longer accumulated,
but a non-transient data part written during onBeforeTurnComplete still
has to be persisted. The late-part drain now commits the previously
skipped response into the UI transcript (kept out of the model lane,
since the parts are UI-only) instead of clearing the parts without
saving them.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VG39FXXkFFU24U5EtJMwPi
The comment described metadata: undefined; seeded records now carry the
harness clientData. The code is self-explanatory, so remove the comment
rather than reword it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VG39FXXkFFU24U5EtJMwPi
devin-ai-integration[bot]

This comment was marked as resolved.

@pkg-pr-new

pkg-pr-new Bot commented Sep 9, 2026

Copy link
Copy Markdown

Open in StackBlitz

@trigger.dev/build

npm i https://pkg.pr.new/@trigger.dev/build@1ee0952

trigger.dev

npm i https://pkg.pr.new/trigger.dev@1ee0952

@trigger.dev/core

npm i https://pkg.pr.new/@trigger.dev/core@1ee0952

@trigger.dev/python

npm i https://pkg.pr.new/@trigger.dev/python@1ee0952

@trigger.dev/react-hooks

npm i https://pkg.pr.new/@trigger.dev/react-hooks@1ee0952

@trigger.dev/redis-worker

npm i https://pkg.pr.new/@trigger.dev/redis-worker@1ee0952

@trigger.dev/rsc

npm i https://pkg.pr.new/@trigger.dev/rsc@1ee0952

@trigger.dev/schema-to-json

npm i https://pkg.pr.new/@trigger.dev/schema-to-json@1ee0952

@trigger.dev/sdk

npm i https://pkg.pr.new/@trigger.dev/sdk@1ee0952

commit: 1ee0952

The late-parts drain distinguished nothing between a content-less
response skipped before onBeforeTurnComplete and a real response the hook
removed via chat.history/setMessages: both reached the drain with
findIndex === -1, so the drain re-added a deliberately-removed reply and
double-pushed it into turnNewUIMessages. Track whether the response was
skipped as content-less and only re-insert in that case; a hook-removed
response stays removed and its late parts are dropped.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VG39FXXkFFU24U5EtJMwPi
@ericallam
ericallam added this pull request to the merge queue Sep 9, 2026
Merged via the queue into main with commit 6f5c49c Sep 9, 2026
64 checks passed
@ericallam
ericallam deleted the fix/transcript-storage-followups branch September 9, 2026 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants