feat(sdk): run tail recovery for every chat.agent and let a transcript storage own the model's context - #4895
Conversation
|
WalkthroughThe SDK adds an optional Merge Risk: 🟡 Moderate · up to Recovered agents can resume without persisted injected context at a compaction boundary. Resolve this context-loss risk before merging. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description explains the design and behavior changes in detail, but it does not follow most required template sections. It omits the issue reference, checklist, Testing section, Changelog section, and Screenshots section. Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 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 |
c5e1f5d to
bac2a16
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: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
bac2a16 to
bb1acb8
Compare
bb1acb8 to
8bb1a1c
Compare
8bb1a1c to
10a1d68
Compare
10a1d68 to
f2a1179
Compare
860969d to
0b4aa77
Compare
0b4aa77 to
b12a0b7
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
b12a0b7 to
dce7b9b
Compare
dce7b9b to
7ea075f
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/trigger-sdk/src/v3/transcriptStorage.ts (2)
486-486: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRestore injections at the compaction boundary.
If an injection follows the message identified by
throughId,idxequalsstart - 1. This filter removes that injection. A recovered model lane can then omit persistedchat.injectcontext.Track whether compaction already contains a boundary injection, or invalidate compaction when a new boundary injection is added. Do not exclude the injection only because its anchor is the final compacted message.
142-173: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd crumb markers to the new transcript state paths.
Add
//@Crumbsmarkers or approved `// `#region` `@crumbsblocks around state reduction, persistence, runtime-state parsing, and model-lane restoration. Do not invent a namespace before approval.As per coding guidelines, “Add crumbs as you write code” and “Do not invent new namespaces — pick from this table or ask first.”
Also applies to: 296-304, 365-369, 395-425, 458-501
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Team
Run ID: 382fdf9d-0477-4ab4-8c00-1a86837b708e
📒 Files selected for processing (3)
packages/trigger-sdk/src/v3/ai.tspackages/trigger-sdk/src/v3/transcriptStorage.tspackages/trigger-sdk/test/transcript-gate-split.test.ts
💤 Files with no reviewable changes (1)
- packages/trigger-sdk/src/v3/ai.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 4 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
- GitHub Check: All PR Checks
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: audit
- GitHub Check: audit
🧰 Additional context used
📓 Path-based instructions (9)
Always import from `@trigger.dev/sdk`.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/trigger-sdk/test/transcript-gate-split.test.tspackages/trigger-sdk/src/v3/transcriptStorage.ts
We use vitest exclusively.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/trigger-sdk/test/transcript-gate-split.test.ts
**Prefer static imports over dynamic imports.**
📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/trigger-sdk/test/transcript-gate-split.test.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-gate-split.test.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-gate-split.test.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-gate-split.test.ts
Use function declarations instead of default exports
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
packages/trigger-sdk/test/transcript-gate-split.test.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-gate-split.test.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-gate-split.test.tspackages/trigger-sdk/src/v3/transcriptStorage.ts
🧠 Learnings (3)
📚 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/test/transcript-gate-split.test.ts
📚 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-gate-split.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
7ea075f to
1e23c34
Compare
…t storage own the model's context One condition used to decide three things at boot: whether to read the persisted transcript, whether to replay the session's output tail, and whether to replay unacknowledged input. Registering hydrateMessages switched all three off, so an app that owned its own context also lost crash recovery, and no application can rebuild the tail its dead run had already emitted. The replays and onRecoveryBoot now run for every agent; only the transcript read is skipped for hydrateMessages. The storage can now declare loadContext, which the runtime calls on every turn and action in place of the accumulated transcript, the role hydrateMessages played, while save keeps receiving every change. hydrateMessages is deprecated with a one-time warning, and configuring it together with a storage that has loadContext is an error.
1e23c34 to
d627683
Compare
2 participants
Summary
Crash recovery now runs for every
chat.agent, and a transcript storage can own the model's context throughloadContext, which replaces the deprecatedhydrateMessageshook.One condition used to decide three things at boot: whether to read the persisted transcript, whether to replay the session's output tail, and whether to replay unacknowledged input. Registering
hydrateMessagesswitched all three off, so an app that owned its own context also lost recovery of a half-written answer and of the messages the dead run never acknowledged, which no application can rebuild from its own rows because the tail is exactly what never got persisted. The replays andonRecoveryBootnow run for every agent; only the transcript read is skipped forhydrateMessages, whose store is the source of truth.Design
A storage may declare
loadContext(scope, event). When present, the runtime calls it on every turn and action, with the same eventhydrateMessagesreceived, and uses what it returns as the model's context, whilesavekeeps receiving every change.hydrateMessageskeeps working with a one-time warning at definition. Setting it together with a storage that hasloadContextis an error.For
hydrateMessagesagents this is the behaviour-changing PR: they regain tail recovery. Runtime writes stay off for them, as documented on the actions page.