feat(sdk): run tail recovery for every chat.agent and let a transcript storage own the model's context - #4895
Conversation
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
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. (45)
WalkthroughThe SDK adds an optional Merge Risk: 🟡 Moderate · up to The transcript-context changes still have open concerns around recovery retaining persisted injections and warning verification using the SDK logger path. These could affect recovered agent context and deprecation-warning behavior, so they should be resolved before merge. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
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.) Full details: Description checkExplanation The description clearly explains the recovery and context-loading changes, but it omits the required issue reference, checklist, testing steps, changelog, and screenshots sections from the repository template. ✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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
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.
92d04a8 to
1677c26
Compare
bac2a16 to
bb1acb8
Compare
1677c26 to
1bf36ef
Compare
bb1acb8 to
8bb1a1c
Compare
1bf36ef to
7a020c4
Compare
8bb1a1c to
10a1d68
Compare
7a020c4 to
516c8de
Compare
10a1d68 to
f2a1179
Compare
516c8de to
192cc15
Compare
860969d to
0b4aa77
Compare
192cc15 to
fc7e243
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
fc7e243 to
322d000
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
322d000 to
e3e6324
Compare
1e23c34 to
d627683
Compare
e3e6324 to
4d2129d
Compare
d627683 to
8292d10
Compare
4d2129d to
2d6f77d
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.
2d6f77d to
ac12025
Compare
8292d10 to
70a6e1d
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.