feat(persistence): generation run persistence (client + server) - #1011
Open
tombeckenham wants to merge 48 commits into
Open
feat(persistence): generation run persistence (client + server)#1011tombeckenham wants to merge 48 commits into
tombeckenham wants to merge 48 commits into
Conversation
Layer a lightweight, read-only resume snapshot onto media generation. As a run streams, the client builds a GenerationResumeSnapshot (run identity, status, errors, result metadata + artifact refs — never media bytes) and writes it to an optional GenerationServerPersistence store. - ai-client: GenerationResumeSnapshot types + updateGenerationResumeSnapshot reducer; GenerationClient/VideoGenerationClient observe chunks, persist snapshots (serialized queue, warn-not-throw), expose getResumeSnapshot(); disposed guard. No resume() action (stream re-attach is PR #955). - ai-event-client: optional threadId/runId on generation events. - react/solid/vue/svelte/angular hooks: persistence + initialResumeSnapshot options; expose resumeSnapshot/resumeState (+ pending/result artifacts). - example: Persisted mode on the image generation route. - docs: persistence/generation-persistence.md + nav entry. Pairs with the existing withGenerationPersistence server middleware.
Drop the bespoke `GenerationServerPersistence` type and the `{ server }`
option wrapper. The `persistence` option is now a bare storage adapter
reusing the shared `ChatStorageAdapter` contract (aliased as
`GenerationPersistence`), so `localStoragePersistence` /
`sessionStoragePersistence` / `indexedDBPersistence` work for generations
exactly as they do for chat — matching main's ergonomics.
…istence (no call-site generic)
Default `localStoragePersistence` / `sessionStoragePersistence` /
`indexedDBPersistence` to a value-agnostic `TValue` so a bare, unannotated
call works for BOTH chat and generation persistence — the consuming
`persistence` option constrains the stored value. Generation docs/example now
use `localStoragePersistence({ keyPrefix })` with no type declaration.
PR #955 (resumable streams) is merged, so delivery durability is available today — it was wrongly described as an unlanded future feature. Rewrite the generation-persistence doc: the server example now wires a durability adapter + GET handler, and the delivery section explains that a dropped mid-generation connection re-attaches through the same adapters useChat uses. Clarify that the read-only snapshot carries run state (incl. runId) across reloads, while hooks do not auto-resume on mount.
…e revival - kiira: replace phantom @tanstack/ai-persistence-drizzle import with the hand-rolled adapter from build-your-own-adapter (CI was red on this) - hydrate the resume snapshot from persistence.getItem on construction, validated via new parseGenerationResumeSnapshot(unknown) export; initialResumeSnapshot seed takes precedence - namespace storage keys as generation:<id> so chat and generation clients sharing an id and adapter no longer collide - write terminal snapshots on stop() (idle) and transport-level errors (error); reset() clears memory + removeItem; RUN_STARTED drops stale result/error/pendingArtifacts from the previous run; plain-fetcher runs now record a complete snapshot built from the fetcher result - capture video jobId into the snapshot from video:job:created - add schemaVersion: 1 to persisted snapshots - gate persistence writes on material change (ignore lastEvent-only churn), warn once per failure transition, clear resumePersistenceError on success - mountDevtools() revives a disposed client (React StrictMode replay); generate() checks disposed before mounting devtools - onResumeSnapshotChange now receives undefined when reset() clears - fix mojibake em dashes in 12 hook files
…t coverage - rewrite docs/persistence/generation-persistence.md around the implemented behavior: hydration on mount, generation:<id> keys, resumeState vs resumeSnapshot semantics, honest reconnect story, no media-URL claim; drop the inert threadId/runId spreads from the server sample - fix the example's Persisted panel: distinguish in-flight run from last-run outcome; reload now actually shows the persisted record - revert ai-event-client: BaseEventContext already carries threadId/runId, the 36 added lines were redundant redeclarations; changeset no longer bumps that package and now describes hydration + lifecycle accurately - normalize wrong hook JSDoc (Server-side → client-side storage; read-only seed claims; run/cursor wording) and mark artifact fields dormant - React hooks: post-dispose guards on callbacks/setters, StrictMode revive via mount effect, stable empty artifact arrays, re-export persistence types (+ PersistedArtifactRef) - tests: replace the two vacuous reducer tests with real externalUrl positive/negative and stop coverage; add reducer seed-merge, RUN_STARTED stale-field-drop, video jobId capture, parseGenerationResumeSnapshot suite; add client lifecycle suite (hydration, seed precedence, corrupt storage, stop/reset/transport-error, write gating, StrictMode revive); add React hydration/StrictMode/artifact-exposure hook tests
…hots Provider-free harness (api.generation-persistence streams a fixed AG-UI sequence; aimock-exempt) + page using useGenerateImage with localStoragePersistence. Proves: snapshot written under tanstack-ai:generation:<id> with no media bytes, hydrated after reload with no auto-run, and removed by reset().
… + media-generation skills
…ration ordering - solid: build the client outside reactive tracking (untrack) — the old createMemo second-arg was a seed, not deps, so option reads were tracked and a change orphaned an undisposed client; stable empty artifact arrays - svelte: explicit generate() now revives a disposed client (mountDevtools) since Svelte has no remount effect; reactive bindings revive with it - vue: stable empty artifact array constants (shallowRef identity) - angular: JSDoc for persistence/initialResumeSnapshot on inject-generate-video - all four: re-export GenerationPersistence/GenerationResumeSnapshot/ GenerationResumeState/GenerationResumeStatus/GenerationPendingArtifact + PersistedArtifactRef from package index; hydration + reset()/removeItem tests against Map-backed adapters - ai-client: kick off snapshot hydration only after callbacksRef is assigned (removes a sync-adapter ordering hazard)
…enerate casts UseGenerationReturn gains a defaulted second generic (TInput extends Record<string, any> = Record<string, any>) so generate is typed (input: TInput) => Promise<void>. useGeneration returns the type it actually builds — the unsound internal narrow-to-wide cast and the five wrapper-level casts back down to the concrete input type all disappear, and direct useGeneration consumers get a precisely typed generate. Existing UseGenerationReturn<MyOutput> references keep compiling via the default.
…svelte/angular Same fix as fbc3dc3 for the remaining four frameworks: the base return interface (UseGenerationReturn / CreateGenerationReturn / InjectGenerationResult) gains a defaulted second generic (TInput extends Record<string, any> = Record<string, any>) so generate is typed (input: TInput) => Promise<void>. The base hooks return the type they actually build and the internal narrow-to-wide casts plus every wrapper-level 'generate as' cast are deleted. Video hooks were already cast-free (they build their own client). Defaults keep existing single-generic references compiling.
…Value = any) Revert the web-storage factory defaults to TValue = ChatPersistedState, as shipped in #984. The any default erased type safety on every direct adapter use (getItem returned any; a store built for one domain assigned silently to the other's hook) and carried three oxlint suppressions — while buying nothing for inline usage, where contextual typing infers the value type from the persistence option regardless of the default. The one affected pattern, a standalone store for generations, now states its type: localStoragePersistence<GenerationResumeSnapshot>(). Doc, example, and e2e call sites updated; runtime behavior unchanged.
Layer server-side artifact + blob storage onto the client generation snapshot. When the persistence backend provides both an artifacts (ArtifactStore) and a blobs (BlobStore) store, withGenerationPersistence writes each generated file's bytes to the blob store (key artifacts/<runId>/<artifactId>), records an ArtifactRecord, attaches PersistedArtifactRefs to the result, and emits generation:artifacts (which the client reducer already consumes). - @tanstack/ai: result-transform machinery (resultTransforms/artifactInputs on GenerationMiddlewareContext, applyGenerationResultTransforms), threadId/runId on the image/audio/speech/transcription activities, generation:artifacts emission from streamGenerationResult. - @tanstack/ai-utils: base64ToUint8Array. - @tanstack/ai-persistence: ArtifactStore + BlobStore contracts + in-memory impls in memoryPersistence(); byte persistence in withGenerationPersistence (extractArtifacts/nameArtifact); retrieveArtifact/retrieveBlob/artifactBlobKey serve helpers. - @tanstack/ai-event-client: optional threadId/runId on generation events. - docs + changeset updated for byte storage.
Give media generation the same two persistence modes useChat has, driven by the `persistence` option: - server-driven (`persistence: true` + a stable `threadId`): the client keeps no local store and hydrates the last generation job from the server on mount via a read-only `hydrateGeneration` GET, answered by the new `reconstructGeneration` helper. - client-driven (a storage adapter): unchanged. Server: reshape `withGenerationPersistence` off the flagged stopgap that faked `threadId = requestId` on the chat RunStore onto a dedicated `GenerationJobStore` keyed by `jobId` (threadId only an optional link). Add `defineGenerationJobStore` / `defineArtifactStore` / `defineBlobStore` and `reconstructGeneration`; durable byte storage (artifacts + blobs) stays an optional layer on top. Client: widen `persistence` to `boolean | adapter`, add `threadId`, and thread both through every generation hook across react/solid/vue/svelte/ angular. `hydrateFromServer` validates the untrusted server snapshot and only adopts it when nothing was observed locally first; a live generate() always wins and no run is ever auto-started. Docs (two modes + BYO job/artifact/blob stores), a Cloudflare R2 artifact/blob skill, unit tests, and a server-driven e2e spec included.
…te storage Generation persistence shipped, but nothing pointed readers to it. Fix the discovery paths: - Split "keep the generated files" out of generation-persistence into its own Keep Generated Files page (server-only byte storage is a distinct journey). - Point the media docs at it: a callout on the generation-hooks hub and video-generation (minutes-long runs), lighter pointers on image/audio/ transcription. - Give the persistence overview a Generation persistence sibling section, add the jobs/artifacts/blobs stores to the store-contract table, and link the generation pages from "Where to go next". - Note in client-persistence that generation hooks share the same true/adapter modes.
…al hook fields
Generation persistence exposed a bolt-on client surface: `resumeSnapshot`,
`resumeState`, `pendingArtifacts`, `resultArtifacts`, and on restore it
repainted only `resumeSnapshot`, leaving `result`/`status`/`error` idle. Make
it invisible like chat, which restores straight into `messages`.
Client (@tanstack/ai-client + 5 frameworks):
- Hooks now return only `generate`, `result`, `isLoading`, `error`, `status`,
`stop`, `reset`, `resumeState`. `resumeSnapshot` / `pendingArtifacts` /
`resultArtifacts` are gone; final artifact refs live on `result.artifacts`,
in-flight ones on `resumeState.pendingArtifacts`.
- On restore (client store or server hydrate) the client repaints
`result` / `status` / `error` and emits `resumeState`, so a reload looks like
a just-finished run. A per-activity `reconstructResult` mapper (image / audio
/ transcription / summarize; video built into the video client) rebuilds a
typed result, with media resolved to the durable serve URL. Live `generate()`
still wins over a slow restore; no run is auto-started.
- `localStoragePersistence()` / `sessionStoragePersistence()` /
`indexedDBPersistence()` now work on a generation hook with no type argument.
Server (@tanstack/ai + @tanstack/ai-persistence):
- `PersistedArtifactRef.url` (durable app-origin serve URL). New
`withGenerationPersistence({ artifactUrl })` stamps it onto each ref and
rewrites the live result's media URL to it, so live and restored results both
render media from your own origin, not the provider's expiring link.
- Text results (transcription / summarize) persist their text + usage so they
restore too.
Docs, skills, the example, and both e2e specs updated to the transparent
surface; the e2e now asserts the restored image renders from the durable URL.
…o its own page The generation-persistence page had grown to cover everything: the two modes, reconnecting a live stream, resumeState semantics, seeding state, securing the hydration endpoint, and the record internals. Keep the main page a focused two-mode quickstart (choose a mode, server-driven, client-driven) and move the deeper material to a new "Generation Persistence: Advanced" page.
…at parity) When a generation run was still streaming at reload, the client only repainted the record; it did not re-attach to the live stream. Now it does, mirroring useChat: on mount, when hydration reports a run still generating, the client tails it through the durability log and finishes it in place. - Expose the connection's `joinRun` on the generation `ConnectConnectionAdapter` (the SSE/HTTP adapters already implement it for chat). - `rejoinInFlight(runId)` in the generation + video clients, reusing `processStream`. Triggered from the server hydrate's `activeRun` and from a client-driven `running` snapshot's `resumeState.runId`. A live `generate()` wins; each run rejoins once; the loading/abort reset is guarded so a stop-then-generate race can't clear a fresh run's loading flag. - Docs: drop the "cannot re-attach on reload" caveat; the main page now states a dropped connection or reload rejoins automatically.
Its reconnect section became false once in-flight runs rejoin automatically, and the rest (resumeState, seeding, record internals) is already covered on the main page. Fold the one load-bearing bit — the reconstructGeneration `authorize` tenancy note — inline into the server example and drop the page + its nav entry.
…persistence docs Example app: every generation route now wires its hook through `generationRunPersistence()`, which delegates to `localStoragePersistence()` and layers a shared run-history list on top of the storage-adapter seam. The new `GenerationRunHistory` component renders that list, so each page shows its previous runs — run history is an app concern, and the adapter seam is where you build it. Docs/comments: correct three stale claims that predate the dedicated `GenerationJobStore`. - `internals.md` still said generation "reuses chat `RunStore` and dual-keys `(runId, threadId)` both to `requestId`" as a stopgap, and called artifact persistence a follow-up. Both shipped; replaced with what the middleware actually does and how the optional `threadId` link works. - `controls.md` and `internals.md` both listed `withGenerationPersistence` as requiring `runs`; it requires `jobs`. - `RunRecord`'s JSDoc glossed a run as "one agent turn within a conversation", contradicting every other use of "turn" in the package. A run is one AG-UI `RUN_STARTED` → `RUN_FINISHED` cycle: it contains many agent-loop turns, and one user turn may span several runs across interrupt-resume.
…re, runId, providerJobId) One generation id previously wore three names: minted as runId on the wire (AG-UI), stored as jobId in the generation store, and handed back as runId on hydration. 'jobId' also collided with the provider's async video job handle sitting one field away in the same snapshot. Converge on 'run' for the AG-UI id and reserve 'job' for provider async jobs: - GenerationJobStore/Record/Status -> GenerationRunStore/Record/Status; defineGenerationJobStore -> defineGenerationRunStore; record field jobId -> runId (matches chat's RunStore/RunRecord.runId) - stores.jobs -> stores.generationRuns (bundle key, validators, memory store) - reconstructGeneration reads ?runId= (option jobParam -> runParam) - GenerationResultSnapshot.jobId -> providerJobId (ditto GenerationRestoredResult); parser accepts both spellings since live provider results still carry jobId - provider surfaces unchanged: VideoGenerateResult.jobId, getVideoJobStatus, useGenerateVideo jobId state, PersistedArtifactRef.source.jobId, video:job:created payload - docs (6 persistence pages + config dates), 4 skills, changeset updated All unreleased surface (none of it is on main), so no migration needed.
…and persistence Add a 'Threads, runs, and turns' section to the streaming guide defining threadId vs runId and why a turn can span multiple runs, then cross-link it from interrupts, resumable streams, and the persistence docs. Add mermaid diagrams for the run/interrupt/generation state lifecycles, the persistence ER schema, and the reconnect sequences.
…sistence Rename the streaming section to 'Threads and runs': just the two id definitions, a note that tool calls stream inside the same run, and a mermaid diagram of one thread with three runs. Update the inbound links from interrupts, resumable streams, and the persistence docs to the new anchor.
…o Grok Imagine Each generation page now shows only its last run, restored from the shared localStorage snapshot adapter (lib/generation-persistence.ts) — the shared history list, GenerationRunHistory component, and label/preview recording are gone. Image and video generation move from OpenAI (gpt-image-1, sora-2) to xAI Grok Imagine (grok-imagine-image, grok-imagine-video) in the API routes and server functions.
This was referenced Jul 29, 2026
Contributor
🚀 Changeset Version Preview17 package(s) bumped directly, 34 bumped as dependents. 🟥 Major bumps
🟨 Minor bumps
🟩 Patch bumps
|
|
View your CI Pipeline Execution ↗ for commit 02d9ad1
☁️ Nx Cloud last updated this comment at |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughGeneration persistence adds resumable generation snapshots, server-side generation-run and artifact/blob stores, durable media URLs, secure artifact fetching, reconstruction endpoints, framework hook support, documentation, examples, and end-to-end tests. ChangesGeneration persistence
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
@tanstack/ai
@tanstack/ai-acp
@tanstack/ai-angular
@tanstack/ai-anthropic
@tanstack/ai-bedrock
@tanstack/ai-claude-code
@tanstack/ai-client
@tanstack/ai-code-mode
@tanstack/ai-code-mode-skills
@tanstack/ai-codex
@tanstack/ai-devtools-core
@tanstack/ai-durable-stream
@tanstack/ai-elevenlabs
@tanstack/ai-event-client
@tanstack/ai-fal
@tanstack/ai-gemini
@tanstack/ai-grok
@tanstack/ai-grok-build
@tanstack/ai-groq
@tanstack/ai-isolate-cloudflare
@tanstack/ai-isolate-node
@tanstack/ai-isolate-quickjs
@tanstack/ai-mcp
@tanstack/ai-memory
@tanstack/ai-mistral
@tanstack/ai-ollama
@tanstack/ai-openai
@tanstack/ai-opencode
@tanstack/ai-openrouter
@tanstack/ai-persistence
@tanstack/ai-preact
@tanstack/ai-react
@tanstack/ai-react-ui
@tanstack/ai-sandbox
@tanstack/ai-sandbox-cloudflare
@tanstack/ai-sandbox-daytona
@tanstack/ai-sandbox-docker
@tanstack/ai-sandbox-local-process
@tanstack/ai-sandbox-sprites
@tanstack/ai-sandbox-vercel
@tanstack/ai-solid
@tanstack/ai-solid-ui
@tanstack/ai-svelte
@tanstack/ai-utils
@tanstack/ai-vue
@tanstack/ai-vue-ui
@tanstack/openai-base
@tanstack/preact-ai-devtools
@tanstack/react-ai-devtools
@tanstack/solid-ai-devtools
commit: |
Byte storage had one fetch path serving two purposes: `descriptorBody`
branched on `descriptor.url` alone and never looked at `descriptor.role`,
so a prompt part with `source: { type: 'url' }` was fetched server-side and
stored, readable back through the artifact GET route. Fetching an expiring
provider result URL is the point of the feature; mirroring a caller-supplied
URL is not, and the bytes are redundant since the client already had them.
Input URLs are no longer fetched. Opting back in is `allowInputUrl`, a
predicate rather than a boolean so the check can't be skipped. Every artifact
fetch is now http/https-only, timed out (`artifactFetchTimeoutMs`) and
size-capped during the drain (`maxArtifactBytes`); input fetches also block
loopback/private/link-local hosts and refuse redirects. Output fetches skip
the host block on purpose — a self-hosted provider legitimately returns a
localhost URL. `artifactFetch` injects the fetch for egress-proxy routing.
Also from review:
- gate `emitResumeState` on a signature, so a per-chunk snapshot rebuild no
longer re-renders every framework hook on every stream event
- guard an invalid Date before `toISOString()` in the resume snapshot reducer
- fall back to the literal payload when a data URL has a bad percent escape
- treat a non-object hydration body as a miss instead of reading `.activeRun`
off null
- docs: authorize artifact reads by `ArtifactRecord.threadId` (404, not 403),
drop auto-resume language for snapshot hydration, add the Mode B server
snippet, honour `limit: 0` in the R2 sample
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ai-svelte/src/create-generate-speech.svelte.ts (1)
111-114: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDocument TTS persistence returning
result === null.
createGenerateSpeechsupportspersistence/threadId/initialResumeSnapshot, but it passes noreconstructResulttocreateGenerationand there is no speech reconstruct mapper. SinceTTSResult.audiois base64 bytes plusformat, persisted speech metadata reloads with status/restoration state, but the audio cannot be rebuilt from storage — document this inCreateGenerateSpeechOptionsto avoid leaving users with a persisting run that never plays.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-svelte/src/create-generate-speech.svelte.ts` around lines 111 - 114, Document in CreateGenerateSpeechOptions that persistence, threadId, and initialResumeSnapshot restore speech state but do not reconstruct TTSResult.audio; persisted runs may therefore return result === null and cannot play audio after reload. Keep the existing createGeneration call unchanged unless needed to align the option documentation.
🟡 Minor comments (16)
packages/ai/skills/ai-core/media-generation/SKILL.md-628-632 (1)
628-632: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winScope the no-byte-storage limitation to media payloads.
Transcription and summarize rebuild
resultfrom persistedtext/metadata (reconstructTranscriptionResultandreconstructSummarizeResult), while only image/audio/video refs require the durableartifactUrlblob route. Change this paragraph to say the media refs/media results need byte storage, not thatresultstaysnullgenerally.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai/skills/ai-core/media-generation/SKILL.md` around lines 628 - 632, Revise the no-byte-storage paragraph to scope the limitation specifically to image, audio, and video media refs/results that depend on durable artifact storage. Preserve that transcription and summarize restore their results from persisted text/metadata via reconstructTranscriptionResult and reconstructSummarizeResult, and remove the blanket claim that a reload leaves result null.packages/ai/skills/ai-core/client-persistence/SKILL.md-156-158 (1)
156-158: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winClarify which results need byte storage on reload.
The client snapshot already preserves text results (
transcription.textandsummary) and artifact refs, so this should say media-bearing results need server byte storage /artifactUrlwhile text-output results restore without it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai/skills/ai-core/client-persistence/SKILL.md` around lines 156 - 158, The client snapshot preserves text outputs such as transcription.text and summary, so revise the result persistence guidance to distinguish them from media-bearing results. State that only media-bearing results require server byte storage or artifactUrl for reload, while text-output results restore without it, and update the surrounding examples accordingly.packages/ai-persistence/skills/ai-persistence/SKILL.md-45-63 (1)
45-63: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winNew store keys contradict "Critical rule 5".
This section documents
generationRuns/artifacts/blobs, but rule 5 (Line 197) still statesstoresaccepts onlymessages,runs,interrupts,metadata. An agent reading the rules section will reject valid configs — update that line alongside this table.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-persistence/skills/ai-persistence/SKILL.md` around lines 45 - 63, The “Critical rule 5” store-key list must include the generation persistence stores documented in the table. Update the rule’s `stores` description to accept `generationRuns`, `artifacts`, and `blobs` in addition to the existing chat stores, preserving the requirement that `artifacts` and `blobs` are provided together.packages/ai-persistence/src/middleware.ts-454-455 (1)
454-455: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFallback mime
${type}/mpegyields bogusimage/mpeg/video/mpegfor prompt media.When a prompt part omits
source.mimeType, images get stored (and later served) asimage/mpeg. A per-type default is cheap:🐛 Proposed fix
- const mimeType = stringField(source, 'mimeType') ?? `${type}/mpeg` + const defaultMime = + type === 'image' ? 'image/png' : type === 'video' ? 'video/mp4' : 'audio/mpeg' + const mimeType = stringField(source, 'mimeType') ?? defaultMime🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-persistence/src/middleware.ts` around lines 454 - 455, Update the mimeType fallback in the source-processing logic near sourceType to use a valid per-type default instead of always constructing `${type}/mpeg`; preserve an explicitly provided source.mimeType and ensure image and video prompt media receive appropriate media types.packages/ai-persistence/src/middleware.ts-383-405 (1)
383-405: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
parseDataUrlrejects data URLs carrying media-type parameters.
data:image/png;charset=utf-8;base64,…doesn't match the regex (only a bare;base64may follow the mime), sodescriptorBodyfalls through to the URL branch and throwsRefusing to fetch artifact over data:— failing the whole run instead of storing the inline bytes. Allowing arbitrary;param=valuesegments before the optional;base64fixes it.🐛 Proposed fix
- const match = /^data:([^;,]+)?(;base64)?,(.*)$/s.exec(value) + const match = /^data:([^;,]+)?((?:;[^;,]*)*?)(;base64)?,(.*)$/s.exec(value) if (!match) return undefined const mimeType = match[1] || 'application/octet-stream' - const raw = match[3] ?? '' + const raw = match[4] ?? ''and switch the base64 check to
match[3].🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-persistence/src/middleware.ts` around lines 383 - 405, Update parseDataUrl’s data-URL regex to accept arbitrary media-type parameter segments before the optional ;base64 marker, while preserving the existing MIME type and payload captures. Adjust the base64 detection to use the new capture index (match[3]) so parameterized URLs such as data:image/png;charset=utf-8;base64,... are decoded as inline bytes.packages/ai-persistence/skills/ai-persistence/build-cloudflare-artifact-store/SKILL.md-345-355 (1)
345-355: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winThe serve-route sample tells readers to authorize but shows no check.
Agents copy this handler as-is, producing an endpoint where any guessed
artifactIdstreams bytes. Show the actual check against the session-derived owner andrecord.threadId, returning 404.🛡️ Proposed fix
const record = await retrieveArtifact(persistence, artifactId) if (!record) return new Response('Not found', { status: 404 }) + + // Ownership comes from server-side session state, never from the caller's id. + const session = await getSession(request, env) + if (!session || !(await ownsThread(env, session.userId, record.threadId))) { + return new Response('Not found', { status: 404 }) + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-persistence/skills/ai-persistence/build-cloudflare-artifact-store/SKILL.md` around lines 345 - 355, Update the GET handler sample to derive the authenticated owner from the session and validate it against the retrieved record’s record.threadId before calling retrieveBlob. Return a 404 response when authorization fails, ensuring artifact bytes are never served for an unauthorized artifactId.Source: Learnings
packages/ai-client/src/connection-adapters.ts-781-791 (1)
781-791: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
resumeState.threadIdis typed optional but is required by the client parser.
parseGenerationResumeSnapshot(packages/ai-client/src/generation-types.ts, Lines 463-465) rejects the entire snapshot whenresumeStatelacks athreadId, so a server response matching this declared type is silently dropped rather than partially adopted. Either makethreadIdrequired here or have the parser tolerate its absence.🛠️ Proposed fix (tighten the type)
- resumeState: { threadId?: string; runId: string } | null + resumeState: { threadId: string; runId: string } | null🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-client/src/connection-adapters.ts` around lines 781 - 791, Make resumeState.threadId required in the GenerationHydrationResult interface, matching the validation behavior of parseGenerationResumeSnapshot. Update the nested resumeState type while preserving its nullable state and existing runId requirement.packages/ai-client/src/generation-client.ts-851-885 (1)
851-885: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winFailed rejoin leaves the visible state stuck on
generating.The catch only records the snapshot error;
status/errorare never repainted, so a rejoin that throws (network drop,joinRun404 after the log expired) leavesstatus: 'generating'withisLoading: falseand no error surfaced — the exact stuck staterecordResumeSnapshotErrorexists to avoid.generate()'s catch sets both.🛠️ Proposed fix
} catch (error) { if (!controller.signal.aborted) { - this.recordResumeSnapshotError( - error instanceof Error ? error : new Error(String(error)), - ) + const err = error instanceof Error ? error : new Error(String(error)) + this.setError(err) + this.setStatus('error') + this.recordResumeSnapshotError(err) + this.callbacksRef.onError?.(err) } } finally {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-client/src/generation-client.ts` around lines 851 - 885, Update the catch block in rejoinInFlight to surface failed rejoin errors through the same status and error state updates used by generate(), while preserving the existing abort check and snapshot error recording. Ensure a non-aborted joinRun/processStream failure repaints the visible error state instead of leaving status as generating.packages/ai-react/src/use-generate-speech.ts-125-134 (1)
125-134: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDocument the missing speech snapshot restore path.
Speech result contains base64
audiobytes, and client-driven persistence only stores the lightweightGenerationResultSnapshotartifacts/ref metadata. BecauseuseGenerateSpeechhas noreconstructResult, a restored completed run will repaintresult = nulleven whenstatus/resumeStateare restored — update the speech options docs to say restored result data is unavailable unless anonResulttransform provides durable restore data.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-react/src/use-generate-speech.ts` around lines 125 - 134, Update the options documentation for useGenerateSpeech and its useGeneration configuration to document that completed speech results cannot be reconstructed from restored snapshots because audio bytes are not persisted; restored runs may have status/resumeState without result data unless an onResult transform supplies durable restore data..changeset/generation-persistence.md-22-22 (1)
22-22: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winClarify the
result.artifactswording.“Refs on
result.artifacts” is awkward; use “refs inresult.artifacts” for clearer documentation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.changeset/generation-persistence.md at line 22, Update the generation persistence documentation sentence describing restored media references, replacing “refs on result.artifacts” with “refs in result.artifacts” while preserving the surrounding behavior and wording.Source: Linters/SAST tools
docs/interrupts/overview.md-44-49 (1)
44-49: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAvoid implying that every continuation immediately finishes.
A continuation can hit another interrupt, so “final answer” and “two run lifecycles” are only guaranteed for a single interrupt/continuation pair. Say “continues the agent” and “at least two runs” instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/interrupts/overview.md` around lines 44 - 49, Update the interrupt sequence description in the overview diagram and accompanying note to say the continuation continues the agent rather than producing a final answer, and describe the lifecycle as at least two runs. Preserve the explanation that the interrupted run ends and continuation starts a new run, while allowing for additional interrupts and runs.docs/config.json-392-392 (1)
392-392: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRefresh the media documentation timestamps to July 29, 2026.
The corresponding documentation files were changed in this PR, but these entries still use
2026-07-28. Update theirupdatedAtvalues without changingaddedAt.As per coding guidelines, substantive documentation changes must refresh
updatedAtto today’s date.Also applies to: 404-422
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/config.json` at line 392, Update the updatedAt values for the affected media documentation entries in docs/config.json, including entries 392 and 404-422, from 2026-07-28 to 2026-07-29. Leave each corresponding addedAt value unchanged.Source: Coding guidelines
docs/media/generation-hooks.md-22-25 (1)
22-25: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winQualify the persistence guarantees across the documentation.
The callouts overpromise what
persistencealone provides: client snapshots are lightweight, expired media requires durable server-side artifact/blob storage, and result reconstruction is conditional.
docs/media/generation-hooks.md#L22-L25: qualify that restoredresultdepends on the configured persistence mode.docs/media/audio-generation.md#L164-L166: state that retaining audio after provider URL expiry requires durable artifact/blob storage.docs/media/image-generation.md#L511-L512: state that retaining images after provider URL expiry requires durable artifact/blob storage.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/media/generation-hooks.md` around lines 22 - 25, Qualify the persistence documentation across all three sites: in docs/media/generation-hooks.md lines 22-25, state that restored result availability depends on the configured persistence mode; in docs/media/audio-generation.md lines 164-166 and docs/media/image-generation.md lines 511-512, clarify that retaining media after provider URL expiry requires durable server-side artifact/blob storage.examples/ts-react-chat/src/routes/generations.image.tsx-21-24 (1)
21-24: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMake reset available after persisted failures.
Both examples enable persistence that can restore an error or status without a result, while their shared UIs only expose Clear when
resultexists.
examples/ts-react-chat/src/routes/generations.image.tsx#L21-L24: update the streaming hook’s UI path to expose reset for restored error/non-idle states.examples/ts-react-chat/src/routes/generations.image.tsx#L42-L49: apply the same reset condition to the direct image variant.examples/ts-react-chat/src/routes/generations.image.tsx#L66-L73: apply the same reset condition to the server-function image variant.examples/ts-react-chat/src/routes/generations.video.tsx#L17-L20: update the streaming video variant.examples/ts-react-chat/src/routes/generations.video.tsx#L31-L38: update the direct video variant.examples/ts-react-chat/src/routes/generations.video.tsx#L48-L55: update the server-function video variant.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/ts-react-chat/src/routes/generations.image.tsx` around lines 21 - 24, Update the UI reset/Clear visibility condition for the generation hook results: examples/ts-react-chat/src/routes/generations.image.tsx lines 21-24, 42-49, and 66-73, and examples/ts-react-chat/src/routes/generations.video.tsx lines 17-20, 31-38, and 48-55. Expose reset when persisted state has an error or any non-idle status, even without a result, while preserving the existing result-based behavior.docs/persistence/build-your-own-adapter.md-938-971 (1)
938-971: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject negative page limits.
With
limit: -1, this code executesLIMIT 0, then returnstruncated: truewithout a cursor. Validate that limits are non-negative integers before constructing the query.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/persistence/build-your-own-adapter.md` around lines 938 - 971, Validate options.limit in list before constructing the SQL query, rejecting values that are negative or not integers; allow undefined and non-negative integers, including zero. Preserve the existing zero-limit response and pagination behavior for valid limits.docs/persistence/keep-generated-files.md-94-95 (1)
94-95: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winCorrect the 404/403 explanation.
Returning the same
404for missing and unauthorized artifacts avoids confirming whether an ID exists; a403would be the distinguishable response. The current prose says the opposite.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/persistence/keep-generated-files.md` around lines 94 - 95, Correct the explanation associated with the owned check so it states that returning 404 for both missing and unauthorized artifacts prevents confirming whether an ID exists, while 403 would distinguish an existing but forbidden artifact. Keep the `if (!owned) return new Response('not found', { status: 404 })` behavior unchanged.
🧹 Nitpick comments (15)
packages/ai/skills/ai-core/media-generation/SKILL.md (1)
593-593: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse one verified latest provider model across both examples.
packages/ai/skills/ai-core/media-generation/SKILL.md#L593-L593: verify whethergpt-image-1is current.packages/ai/skills/ai-core/client-persistence/SKILL.md#L199-L199: verify whethergpt-image-2is current.As per coding guidelines, use the latest model from the adapter’s
model-meta.tsand keep edited examples consistent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai/skills/ai-core/media-generation/SKILL.md` at line 593, The OpenAI image model examples are inconsistent and may not use the latest verified provider model. Check the adapter’s model-meta.ts, update the adapter configuration at packages/ai/skills/ai-core/media-generation/SKILL.md:593-593 and the corresponding example at packages/ai/skills/ai-core/client-persistence/SKILL.md:199-199 to the latest model, and keep both examples consistent.Source: Coding guidelines
packages/ai-persistence/tests/persistence-validation.test.ts (1)
76-78: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe artifacts/blobs both-or-neither invariant is enforced twice but tested nowhere.
validateGenerationPersistenceStoresenforces it at runtime andInvalidGenerationPersistenceenforces it at the type level; neither branch has a test, so a regression in either would pass CI.
packages/ai-persistence/tests/persistence-validation.test.ts#L76-L78: add cases with{ generationRuns, artifacts }and{ generationRuns, blobs }expecting/requires both stores\.artifacts and stores\.blobs/i.packages/ai-persistence/tests/persistence-types.test-d.ts#L155-L160: add@ts-expect-errorcalls towithGenerationPersistencefor the same two half-configured store sets.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-persistence/tests/persistence-validation.test.ts` around lines 76 - 78, The artifacts/blobs both-or-neither invariant lacks runtime and type-level coverage. In packages/ai-persistence/tests/persistence-validation.test.ts:76-78, add validation cases for { generationRuns, artifacts } and { generationRuns, blobs }, expecting /requires both stores\.artifacts and stores\.blobs/i. In packages/ai-persistence/tests/persistence-types.test-d.ts:155-160, add `@ts-expect-error` calls to withGenerationPersistence for those same half-configured store sets.packages/ai-persistence/src/memory.ts (1)
72-116: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider returning copies of run records.
createOrResume/gethand back the live map entry, so a caller mutating the returned object silently edits store state —MemoryArtifactStore.save(Line 229) already copies. A{ ...record }on return keeps the reference backend honest as a conformance target for real backends.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-persistence/src/memory.ts` around lines 72 - 116, The MemoryGenerationRunStore currently exposes live GenerationRunRecord objects, allowing callers to mutate stored state. Update createOrResume, get, and findLatestForThread to return shallow copies of records while preserving the map’s internal references; align this behavior with MemoryArtifactStore.save and keep update’s merge semantics unchanged.packages/ai-persistence/tests/reconstruct-generation.test.ts (1)
119-137: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider covering
authorizereturning aResponseand customparam/runParam. Thedecision instanceof Responsebranch and the option-renamed query params are unexercised; both are one-liner tests.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-persistence/tests/reconstruct-generation.test.ts` around lines 119 - 137, Add focused tests alongside the existing authorization test for reconstructGeneration: verify an authorize callback returning a Response is returned unchanged, and verify custom param/runParam option names are used to locate the generation run. Keep each case minimal and assert the expected response behavior.packages/ai-persistence/src/reconstruct-generation.ts (1)
42-51: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
authorizecannot tell arunIdfrom athreadId. The callback gets a bareid, so an ownership check has to guess which lookup to run (or check both, widening what it accepts). Consider passing the kind alongside the id, e.g.authorize({ runId, threadId }, request)or a secondkind: 'run' | 'thread'argument, so tenancy checks stay precise.Authorization ordering itself is correct — it runs before any store read.
Also applies to: 157-171
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-persistence/src/reconstruct-generation.ts` around lines 42 - 51, The authorize callback currently receives only a bare id, preventing precise distinction between run and thread authorization. Update the authorize contract and its invocation in the reconstruction flow to pass the identifier kind alongside the id, such as an explicit runId/threadId object or run/thread kind argument, while preserving authorization before store reads and existing boolean/Response handling.Source: Learnings
packages/ai-persistence/tests/error-abort.test.ts (1)
265-265: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTest titles still say "job" after the jobs → runs rename. Store, records, and ids are all
run-flavoured now; renaming these titles to "run" keeps failure output consistent with the API.Also applies to: 289-289, 313-313, 338-338
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-persistence/tests/error-abort.test.ts` at line 265, Rename the test titles at the cases around lines 265, 289, 313, and 338 from “job” terminology to “run” terminology, preserving each test’s existing behavior and wording aside from the renamed entity.packages/ai-persistence/src/middleware.ts (1)
1404-1408: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocblock contradicts the artifact fallback.
persistGenerationArtifacts(Line 872) does setthreadId = ctx.threadId ?? ctx.requestIdon everyArtifactRecord/PersistedArtifactRef(required field), so "never faked from the request id" only holds for the run record. Worth wording it as "the run record never fakes a threadId; artifact records fall back to the request id".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-persistence/src/middleware.ts` around lines 1404 - 1408, The docblock above persistGenerationArtifacts incorrectly claims threadId is never faked from the request id. Clarify that only the generation run record preserves an optional caller-supplied threadId without fallback, while artifact records use ctx.threadId ?? ctx.requestId as their required threadId.packages/ai-persistence/tests/generation-artifacts.test.ts (1)
28-30: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThese
void (undefined as unknown as T)lines look like leftover scaffolding.The three types are only imported to be discarded here; either drop them (and the imports) or add real assertions, e.g. a
satisfies GenerationArtifactDescriptoron the literal passed toextractArtifactsat Line 388.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-persistence/tests/generation-artifacts.test.ts` around lines 28 - 30, Remove the unused GenerationArtifactDescriptor, GenerationArtifactExtractionInput, and GenerationArtifactNameInput imports and their corresponding void-cast scaffolding in generation-artifacts.test.ts. If type coverage is needed, replace the scaffolding with a real satisfies assertion on the extractArtifacts input literal, but do not retain discarded type references.packages/ai-client/src/generation-reconstruct.ts (1)
23-35: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: drop the
as stringcast withflatMap.- return restored.artifacts - .filter( - (a) => - a.role === 'output' && - a.source.mediaType === mediaType && - a.url != null, - ) - .map((a) => a.url as string) + return restored.artifacts.flatMap((a) => + a.role === 'output' && a.source.mediaType === mediaType && a.url != null + ? [a.url] + : [], + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-client/src/generation-reconstruct.ts` around lines 23 - 35, Update mediaUrls to use flatMap so it returns each matching artifact URL only when present, allowing TypeScript to infer string values and removing the `as string` cast while preserving the existing output-role and media-type filters.packages/ai-angular/tests/inject-generation.test.ts (1)
50-80: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: collapse the three near-identical render helpers.
renderInjectGenerateVideo/renderInjectGenerateImagediffer fromrenderInjectGenerationonly in which injectable is called; a small factory taking the injectable would remove two copies.♻️ Sketch
function renderWith<T>(inject: (options: any) => T) { return (options: any) => { `@Component`({ standalone: true, template: '' }) class Host { gen = inject(options) } const fixture = TestBed.createComponent(Host) fixture.detectChanges() return { get result() { return fixture.componentInstance.gen }, flush: () => fixture.detectChanges(), destroy: () => fixture.destroy(), } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-angular/tests/inject-generation.test.ts` around lines 50 - 80, Optionally replace the duplicated renderInjectGenerateVideo and renderInjectGenerateImage helpers, along with renderInjectGeneration, with a shared renderWith factory that accepts the injectable function and returns the options-based renderer. Preserve the existing Host component setup, fixture lifecycle methods, and result access behavior for each injectable.packages/ai-client/src/video-generation-client.ts (1)
529-554: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a single clone helper instead of hand-rolled field spreads.
This copies each nested field explicitly, so any new field added to
GenerationResumeSnapshotsilently starts leaking an internal reference.structuredClone(this.resumeSnapshot)(or a sharedcloneResumeSnapshothelper ingeneration-types.ts, reusable by the other generation clients) keeps this correct as the shape grows. Note the current copy is also shallow at the element level —artifacts/pendingArtifactsentries are still shared references.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-client/src/video-generation-client.ts` around lines 529 - 554, Replace the hand-rolled cloning in getResumeSnapshot with structuredClone or a shared cloneResumeSnapshot helper, ensuring the entire GenerationResumeSnapshot—including nested objects and pendingArtifacts/artifacts entries—is deeply copied. If introducing the helper, place it with the generation types and reuse the established helper across generation clients.packages/ai-angular/src/inject-generate-video.ts (1)
38-52: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInconsistent with the sibling injectables: consider
Pick<InjectGenerationOptions, …>.
InjectGenerateAudioOptions(and the image/speech variants) inherit these three fields viaextends Pick<InjectGenerationOptions<…>, 'persistence' | 'threadId' | 'initialResumeSnapshot'>, while this file re-declares them plus the doc comment inline. Same for the Svelte/React/Vue video hooks. Inheriting keeps the docs and types in one place as the persistence surface evolves.Video's client is a separate class (
VideoGenerationClient, notGenerationClient), so this may be deliberate — if so, no change needed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-angular/src/inject-generate-video.ts` around lines 38 - 52, Align the video injectable options with the sibling audio, image, and speech variants by inheriting persistence, threadId, and initialResumeSnapshot through Pick<InjectGenerationOptions<…>, 'persistence' | 'threadId' | 'initialResumeSnapshot'> instead of redeclaring them inline. Verify VideoGenerationClient supports the same option types; if it does not, preserve the current declarations and make no change.packages/ai-react/tests/use-generation.test.ts (1)
143-160: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNit: the spy and the generator share the name
connect, andrunContextsis unused in the added tests.The inner
async *connectand the outerconst connect = vi.fn()reading the same identifier makes the helper harder to follow; considerconnectSpy. Also droprunContextsunless a test asserts on it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-react/tests/use-generation.test.ts` around lines 143 - 160, In createRunContextCaptureAdapter, rename the outer vi.fn() variable from connect to connectSpy and update the returned property and all references accordingly, while retaining the adapter’s connect method. Remove the unused runContexts array, its push call, and its return property unless the added tests assert on it.packages/ai-vue/src/use-generation.ts (1)
165-170: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueComment contradicts the code.
The comment says the
bodykey must be omitted when absent under EOPT, but Line 170 assignsbody: options.bodyunconditionally (possiblyundefined). Either drop/reword the comment or apply the conditional spread used for the neighbouring options.♻️ Suggested change
- // Conditional spread on `body`: `GenerationClientOptions.body` is a strict - // optional (`body?: Record<string, any>`), and under EOPT we must omit the - // key when absent rather than assign `undefined`. const clientOptions: GenerationClientOptions<TInput, TResult, TOutput> = { id: clientId, - body: options.body, + ...(options.body !== undefined && { body: options.body }),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-vue/src/use-generation.ts` around lines 165 - 170, Update the clientOptions construction near GenerationClientOptions so body is conditionally spread only when options.body is present, matching the comment and strict optional-property requirements; preserve the existing id and other option handling.packages/ai-vue/src/use-generate-speech.ts (1)
114-123: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSpeech has no
reconstructResult, soresultnever repaints on restore — document it.Every other Vue generation composable injects a reconstructor (
reconstructImageResult,reconstructTranscriptionResult,reconstructSummarizeResult). Speech can't rebuild aTTSResultfrom a durable URL becauseaudiois base64 plus a requiredformat, so omitting it looks deliberate — but the inheritedpersistenceoption at Lines 22-25 now implies restore support. Add a short note on the option docs (or the hook JSDoc) that a restored speech snapshot repaintsstatus/erroronly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-vue/src/use-generate-speech.ts` around lines 114 - 123, Add a concise documentation note near the inherited persistence option or the use-generate-speech hook JSDoc explaining that restored speech snapshots repaint only status and error; result is not reconstructed because TTS audio requires base64 data and its format. Keep the existing useGeneration configuration unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/persistence/build-your-own-adapter.md`:
- Around line 893-923: The put method’s selectCreated-before-upsert flow is
race-prone and can return values that differ from the stored row. Update put to
atomically upsert the record and then re-read the persisted row, or use an
atomic RETURNING-supported upsert, deriving createdAt and returned metadata from
the record actually stored.
- Around line 617-640: Validate and narrow persisted database values before
hydrating records in mapGenerationRun, mapArtifact, and mapBlobRecord. Replace
direct coercions and unguarded JSON.parse calls with schema validation or type
guards, rejecting or safely handling malformed and partially migrated rows
before constructing domain objects. Preserve valid-row hydration while
preventing invalid state from reaching get and findLatestForThread.
- Around line 655-676: Update createOrResume so it returns the persisted record
after insert rather than reconstructing and returning the attempted input.
Re-read the run using select.get(input.runId) after insert, including when ON
CONFLICT DO NOTHING means another caller created it first, and map that stored
record with mapGenerationRun.
In `@docs/persistence/generation-persistence.md`:
- Around line 29-35: Update the generation persistence documentation around
withGenerationPersistence and generationRuns to distinguish client-side adapters
from server persistence. State that a server run record is created only when the
server route opts into withGenerationPersistence, and that server persistence is
required for server-driven hydration and/or durable artifacts rather than for
every persistence mode.
- Around line 37-39: The persistence documentation should distinguish result
metadata from durable media bytes. In docs/persistence/generation-persistence.md
lines 37-39, remove the claim that result always remains null without byte
storage; in lines 224-228, state that artifacts/blobs are required to restore
durable media, while metadata and non-binary results can still be restored.
- Around line 113-116: Make both persistence examples fail closed: in
docs/persistence/generation-persistence.md lines 113-116, update the
reconstructGeneration call to provide a real server-side authorize callback that
validates the authenticated user’s ownership before using the client-supplied
threadId; in docs/persistence/keep-generated-files.md lines 77-95, replace the
const owned = true placeholder with session-derived artifact ownership
validation. Ensure neither example permits client-provided identifiers alone to
read another user’s generation.
In `@docs/persistence/keep-generated-files.md`:
- Around line 151-167: Add a concise client-side hook or component example to
the “Wire the durable URL through to the client” section, showing consumption of
durable media URLs from the result fields (such as result.images, result.url, or
result.audio.url). Keep the snippet focused on rendering the persisted URL and
complement the existing server-side explanation without changing the documented
artifact flow.
In `@examples/ts-react-chat/src/routes/generations.audio.tsx`:
- Around line 81-94: Update the audio persistence configuration in
examples/ts-react-chat/src/routes/generations.audio.tsx:81-94 and the speech
persistence configuration in
examples/ts-react-chat/src/routes/generations.speech.tsx:55-79 so persisted
AudioOutput.url and SpeechOutput.audioUrl values are not stored as
document-scoped object URLs; omit those fields or reconstruct them during
hydration from a durable source while preserving the existing generation and
playback behavior.
- Around line 81-94: Update the audio generation persistence ids in
examples/ts-react-chat/src/routes/generations.audio.tsx (lines 81-94) to include
selectedModel. Update the speech generation persistence id and its useMemo
dependencies in examples/ts-react-chat/src/routes/generations.speech.tsx (lines
55-79) to include voice, ensuring distinct models or voices cannot hydrate stale
persisted output.
In `@packages/ai-client/src/video-generation-client.ts`:
- Around line 850-860: Update maybeHydrateResumeSnapshot and hydrateFromServer
to check this.disposed immediately after their awaited storage/server hydration
calls and return before applying state or rejoining work. Ensure disposed
clients cannot call repaintFromSnapshot or start rejoinInFlight/joinRun, while
preserving the existing live-state guards for active clients.
In `@packages/ai/skills/ai-core/client-persistence/SKILL.md`:
- Around line 209-219: Authorize all caller-supplied persistence identifiers
using server-side session ownership checks. In
packages/ai/skills/ai-core/client-persistence/SKILL.md lines 209-219, replace
the unconditional authorization in GET with a real ownership check or
fail-closed behavior; in packages/ai/skills/ai-core/media-generation/SKILL.md
lines 590-595, authorize the submitted thread before persisting the run; and in
lines 609-618, authorize record.threadId before retrieving or returning blob
bytes.
- Around line 191-203: Update the POST handler’s generation flow around
generationParamsFromRequest and generateImage to ensure every request carries a
stable threadId into withGenerationPersistence, including bare prompt requests.
Use the request envelope or an equivalent propagated identifier so the persisted
run identity matches the threadId used by hydration, and cover the
POST-to-hydration path.
In `@packages/ai/src/activities/generateAudio/index.ts`:
- Around line 163-164: Propagate threadId and runId from the activity context
into every aiEventClient.emit lifecycle payload. Update started, completed,
error, and usage events in packages/ai/src/activities/generateAudio/index.ts
(163-164), packages/ai/src/activities/generateImage/index.ts (256-257),
packages/ai/src/activities/generateSpeech/index.ts (171-172), and
packages/ai/src/activities/generateTranscription/index.ts (206-207); ensure all
emitted payloads include both IDs.
In `@testing/e2e/src/routes/api.generation-persistence-server.ts`:
- Around line 36-39: Add a test-only reset path for the completedByThread record
keyed by generation-server-thread, either through a DELETE handler or by
clearing it in the persistence spec before the first page.goto. Ensure retries
start with no prior completed generation while preserving the existing
server-authoritative record behavior.
---
Outside diff comments:
In `@packages/ai-svelte/src/create-generate-speech.svelte.ts`:
- Around line 111-114: Document in CreateGenerateSpeechOptions that persistence,
threadId, and initialResumeSnapshot restore speech state but do not reconstruct
TTSResult.audio; persisted runs may therefore return result === null and cannot
play audio after reload. Keep the existing createGeneration call unchanged
unless needed to align the option documentation.
---
Minor comments:
In @.changeset/generation-persistence.md:
- Line 22: Update the generation persistence documentation sentence describing
restored media references, replacing “refs on result.artifacts” with “refs in
result.artifacts” while preserving the surrounding behavior and wording.
In `@docs/config.json`:
- Line 392: Update the updatedAt values for the affected media documentation
entries in docs/config.json, including entries 392 and 404-422, from 2026-07-28
to 2026-07-29. Leave each corresponding addedAt value unchanged.
In `@docs/interrupts/overview.md`:
- Around line 44-49: Update the interrupt sequence description in the overview
diagram and accompanying note to say the continuation continues the agent rather
than producing a final answer, and describe the lifecycle as at least two runs.
Preserve the explanation that the interrupted run ends and continuation starts a
new run, while allowing for additional interrupts and runs.
In `@docs/media/generation-hooks.md`:
- Around line 22-25: Qualify the persistence documentation across all three
sites: in docs/media/generation-hooks.md lines 22-25, state that restored result
availability depends on the configured persistence mode; in
docs/media/audio-generation.md lines 164-166 and docs/media/image-generation.md
lines 511-512, clarify that retaining media after provider URL expiry requires
durable server-side artifact/blob storage.
In `@docs/persistence/build-your-own-adapter.md`:
- Around line 938-971: Validate options.limit in list before constructing the
SQL query, rejecting values that are negative or not integers; allow undefined
and non-negative integers, including zero. Preserve the existing zero-limit
response and pagination behavior for valid limits.
In `@docs/persistence/keep-generated-files.md`:
- Around line 94-95: Correct the explanation associated with the owned check so
it states that returning 404 for both missing and unauthorized artifacts
prevents confirming whether an ID exists, while 403 would distinguish an
existing but forbidden artifact. Keep the `if (!owned) return new Response('not
found', { status: 404 })` behavior unchanged.
In `@examples/ts-react-chat/src/routes/generations.image.tsx`:
- Around line 21-24: Update the UI reset/Clear visibility condition for the
generation hook results: examples/ts-react-chat/src/routes/generations.image.tsx
lines 21-24, 42-49, and 66-73, and
examples/ts-react-chat/src/routes/generations.video.tsx lines 17-20, 31-38, and
48-55. Expose reset when persisted state has an error or any non-idle status,
even without a result, while preserving the existing result-based behavior.
In `@packages/ai-client/src/connection-adapters.ts`:
- Around line 781-791: Make resumeState.threadId required in the
GenerationHydrationResult interface, matching the validation behavior of
parseGenerationResumeSnapshot. Update the nested resumeState type while
preserving its nullable state and existing runId requirement.
In `@packages/ai-client/src/generation-client.ts`:
- Around line 851-885: Update the catch block in rejoinInFlight to surface
failed rejoin errors through the same status and error state updates used by
generate(), while preserving the existing abort check and snapshot error
recording. Ensure a non-aborted joinRun/processStream failure repaints the
visible error state instead of leaving status as generating.
In
`@packages/ai-persistence/skills/ai-persistence/build-cloudflare-artifact-store/SKILL.md`:
- Around line 345-355: Update the GET handler sample to derive the authenticated
owner from the session and validate it against the retrieved record’s
record.threadId before calling retrieveBlob. Return a 404 response when
authorization fails, ensuring artifact bytes are never served for an
unauthorized artifactId.
In `@packages/ai-persistence/skills/ai-persistence/SKILL.md`:
- Around line 45-63: The “Critical rule 5” store-key list must include the
generation persistence stores documented in the table. Update the rule’s
`stores` description to accept `generationRuns`, `artifacts`, and `blobs` in
addition to the existing chat stores, preserving the requirement that
`artifacts` and `blobs` are provided together.
In `@packages/ai-persistence/src/middleware.ts`:
- Around line 454-455: Update the mimeType fallback in the source-processing
logic near sourceType to use a valid per-type default instead of always
constructing `${type}/mpeg`; preserve an explicitly provided source.mimeType and
ensure image and video prompt media receive appropriate media types.
- Around line 383-405: Update parseDataUrl’s data-URL regex to accept arbitrary
media-type parameter segments before the optional ;base64 marker, while
preserving the existing MIME type and payload captures. Adjust the base64
detection to use the new capture index (match[3]) so parameterized URLs such as
data:image/png;charset=utf-8;base64,... are decoded as inline bytes.
In `@packages/ai-react/src/use-generate-speech.ts`:
- Around line 125-134: Update the options documentation for useGenerateSpeech
and its useGeneration configuration to document that completed speech results
cannot be reconstructed from restored snapshots because audio bytes are not
persisted; restored runs may have status/resumeState without result data unless
an onResult transform supplies durable restore data.
In `@packages/ai/skills/ai-core/client-persistence/SKILL.md`:
- Around line 156-158: The client snapshot preserves text outputs such as
transcription.text and summary, so revise the result persistence guidance to
distinguish them from media-bearing results. State that only media-bearing
results require server byte storage or artifactUrl for reload, while text-output
results restore without it, and update the surrounding examples accordingly.
In `@packages/ai/skills/ai-core/media-generation/SKILL.md`:
- Around line 628-632: Revise the no-byte-storage paragraph to scope the
limitation specifically to image, audio, and video media refs/results that
depend on durable artifact storage. Preserve that transcription and summarize
restore their results from persisted text/metadata via
reconstructTranscriptionResult and reconstructSummarizeResult, and remove the
blanket claim that a reload leaves result null.
---
Nitpick comments:
In `@packages/ai-angular/src/inject-generate-video.ts`:
- Around line 38-52: Align the video injectable options with the sibling audio,
image, and speech variants by inheriting persistence, threadId, and
initialResumeSnapshot through Pick<InjectGenerationOptions<…>, 'persistence' |
'threadId' | 'initialResumeSnapshot'> instead of redeclaring them inline. Verify
VideoGenerationClient supports the same option types; if it does not, preserve
the current declarations and make no change.
In `@packages/ai-angular/tests/inject-generation.test.ts`:
- Around line 50-80: Optionally replace the duplicated renderInjectGenerateVideo
and renderInjectGenerateImage helpers, along with renderInjectGeneration, with a
shared renderWith factory that accepts the injectable function and returns the
options-based renderer. Preserve the existing Host component setup, fixture
lifecycle methods, and result access behavior for each injectable.
In `@packages/ai-client/src/generation-reconstruct.ts`:
- Around line 23-35: Update mediaUrls to use flatMap so it returns each matching
artifact URL only when present, allowing TypeScript to infer string values and
removing the `as string` cast while preserving the existing output-role and
media-type filters.
In `@packages/ai-client/src/video-generation-client.ts`:
- Around line 529-554: Replace the hand-rolled cloning in getResumeSnapshot with
structuredClone or a shared cloneResumeSnapshot helper, ensuring the entire
GenerationResumeSnapshot—including nested objects and pendingArtifacts/artifacts
entries—is deeply copied. If introducing the helper, place it with the
generation types and reuse the established helper across generation clients.
In `@packages/ai-persistence/src/memory.ts`:
- Around line 72-116: The MemoryGenerationRunStore currently exposes live
GenerationRunRecord objects, allowing callers to mutate stored state. Update
createOrResume, get, and findLatestForThread to return shallow copies of records
while preserving the map’s internal references; align this behavior with
MemoryArtifactStore.save and keep update’s merge semantics unchanged.
In `@packages/ai-persistence/src/middleware.ts`:
- Around line 1404-1408: The docblock above persistGenerationArtifacts
incorrectly claims threadId is never faked from the request id. Clarify that
only the generation run record preserves an optional caller-supplied threadId
without fallback, while artifact records use ctx.threadId ?? ctx.requestId as
their required threadId.
In `@packages/ai-persistence/src/reconstruct-generation.ts`:
- Around line 42-51: The authorize callback currently receives only a bare id,
preventing precise distinction between run and thread authorization. Update the
authorize contract and its invocation in the reconstruction flow to pass the
identifier kind alongside the id, such as an explicit runId/threadId object or
run/thread kind argument, while preserving authorization before store reads and
existing boolean/Response handling.
In `@packages/ai-persistence/tests/error-abort.test.ts`:
- Line 265: Rename the test titles at the cases around lines 265, 289, 313, and
338 from “job” terminology to “run” terminology, preserving each test’s existing
behavior and wording aside from the renamed entity.
In `@packages/ai-persistence/tests/generation-artifacts.test.ts`:
- Around line 28-30: Remove the unused GenerationArtifactDescriptor,
GenerationArtifactExtractionInput, and GenerationArtifactNameInput imports and
their corresponding void-cast scaffolding in generation-artifacts.test.ts. If
type coverage is needed, replace the scaffolding with a real satisfies assertion
on the extractArtifacts input literal, but do not retain discarded type
references.
In `@packages/ai-persistence/tests/persistence-validation.test.ts`:
- Around line 76-78: The artifacts/blobs both-or-neither invariant lacks runtime
and type-level coverage. In
packages/ai-persistence/tests/persistence-validation.test.ts:76-78, add
validation cases for { generationRuns, artifacts } and { generationRuns, blobs
}, expecting /requires both stores\.artifacts and stores\.blobs/i. In
packages/ai-persistence/tests/persistence-types.test-d.ts:155-160, add
`@ts-expect-error` calls to withGenerationPersistence for those same
half-configured store sets.
In `@packages/ai-persistence/tests/reconstruct-generation.test.ts`:
- Around line 119-137: Add focused tests alongside the existing authorization
test for reconstructGeneration: verify an authorize callback returning a
Response is returned unchanged, and verify custom param/runParam option names
are used to locate the generation run. Keep each case minimal and assert the
expected response behavior.
In `@packages/ai-react/tests/use-generation.test.ts`:
- Around line 143-160: In createRunContextCaptureAdapter, rename the outer
vi.fn() variable from connect to connectSpy and update the returned property and
all references accordingly, while retaining the adapter’s connect method. Remove
the unused runContexts array, its push call, and its return property unless the
added tests assert on it.
In `@packages/ai-vue/src/use-generate-speech.ts`:
- Around line 114-123: Add a concise documentation note near the inherited
persistence option or the use-generate-speech hook JSDoc explaining that
restored speech snapshots repaint only status and error; result is not
reconstructed because TTS audio requires base64 data and its format. Keep the
existing useGeneration configuration unchanged.
In `@packages/ai-vue/src/use-generation.ts`:
- Around line 165-170: Update the clientOptions construction near
GenerationClientOptions so body is conditionally spread only when options.body
is present, matching the comment and strict optional-property requirements;
preserve the existing id and other option handling.
In `@packages/ai/skills/ai-core/media-generation/SKILL.md`:
- Line 593: The OpenAI image model examples are inconsistent and may not use the
latest verified provider model. Check the adapter’s model-meta.ts, update the
adapter configuration at
packages/ai/skills/ai-core/media-generation/SKILL.md:593-593 and the
corresponding example at
packages/ai/skills/ai-core/client-persistence/SKILL.md:199-199 to the latest
model, and keep both examples consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+617
to
+640
| function mapGenerationRun(row: Record<string, unknown>): GenerationRunRecord { | ||
| return { | ||
| runId: String(row.run_id), | ||
| ...(typeof row.thread_id === 'string' ? { threadId: row.thread_id } : {}), | ||
| activity: String(row.activity), | ||
| provider: String(row.provider), | ||
| model: String(row.model), | ||
| status: toGenerationRunStatus(row.status), | ||
| startedAt: Number(row.started_at), | ||
| ...(row.finished_at != null ? { finishedAt: Number(row.finished_at) } : {}), | ||
| ...(typeof row.error_json === 'string' | ||
| ? { error: JSON.parse(row.error_json) } | ||
| : {}), | ||
| ...(typeof row.result_json === 'string' | ||
| ? { result: JSON.parse(row.result_json) } | ||
| : {}), | ||
| ...(typeof row.artifacts_json === 'string' | ||
| ? { artifacts: JSON.parse(row.artifacts_json) } | ||
| : {}), | ||
| ...(typeof row.usage_json === 'string' | ||
| ? { usage: JSON.parse(row.usage_json) } | ||
| : {}), | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Validate persisted rows before hydration.
mapGenerationRun directly coerces database values and calls JSON.parse. A corrupt or partially migrated row can throw during get/findLatestForThread or hydrate invalid state. Apply schema validation/type guards here and to mapArtifact (Lines 739-751) and mapBlobRecord (Lines 845-858).
Based on learnings, hand-rolled persistence adapters must narrow and validate parsed values before use.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/persistence/build-your-own-adapter.md` around lines 617 - 640, Validate
and narrow persisted database values before hydrating records in
mapGenerationRun, mapArtifact, and mapBlobRecord. Replace direct coercions and
unguarded JSON.parse calls with schema validation or type guards, rejecting or
safely handling malformed and partially migrated rows before constructing domain
objects. Preserve valid-row hydration while preventing invalid state from
reaching get and findLatestForThread.
Source: Learnings
Comment on lines
+655
to
+676
| async createOrResume(input) { | ||
| const existing = select.get(input.runId) | ||
| if (existing) return mapGenerationRun(existing) | ||
| const status: GenerationRunStatus = input.status ?? 'running' | ||
| insert.run( | ||
| input.runId, | ||
| input.threadId ?? null, | ||
| input.activity, | ||
| input.provider, | ||
| input.model, | ||
| status, | ||
| input.startedAt, | ||
| ) | ||
| return { | ||
| runId: input.runId, | ||
| activity: input.activity, | ||
| provider: input.provider, | ||
| model: input.model, | ||
| status, | ||
| startedAt: input.startedAt, | ||
| ...(input.threadId !== undefined ? { threadId: input.threadId } : {}), | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Return the record after the insert.
Two concurrent callers can both miss select.get; the loser’s ON CONFLICT DO NOTHING is ignored, and it returns its attempted input instead of the stored winner. That violates the documented idempotency contract and can report incorrect run metadata.
Proposed fix
insert.run(
input.runId,
input.threadId ?? null,
input.activity,
input.provider,
input.model,
status,
input.startedAt,
)
- return {
- runId: input.runId,
- activity: input.activity,
- provider: input.provider,
- model: input.model,
- status,
- startedAt: input.startedAt,
- ...(input.threadId !== undefined ? { threadId: input.threadId } : {}),
- }
+ const created = select.get(input.runId)
+ if (!created) throw new Error('Generation run was not created')
+ return mapGenerationRun(created)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async createOrResume(input) { | |
| const existing = select.get(input.runId) | |
| if (existing) return mapGenerationRun(existing) | |
| const status: GenerationRunStatus = input.status ?? 'running' | |
| insert.run( | |
| input.runId, | |
| input.threadId ?? null, | |
| input.activity, | |
| input.provider, | |
| input.model, | |
| status, | |
| input.startedAt, | |
| ) | |
| return { | |
| runId: input.runId, | |
| activity: input.activity, | |
| provider: input.provider, | |
| model: input.model, | |
| status, | |
| startedAt: input.startedAt, | |
| ...(input.threadId !== undefined ? { threadId: input.threadId } : {}), | |
| } | |
| async createOrResume(input) { | |
| const existing = select.get(input.runId) | |
| if (existing) return mapGenerationRun(existing) | |
| const status: GenerationRunStatus = input.status ?? 'running' | |
| insert.run( | |
| input.runId, | |
| input.threadId ?? null, | |
| input.activity, | |
| input.provider, | |
| input.model, | |
| status, | |
| input.startedAt, | |
| ) | |
| const created = select.get(input.runId) | |
| if (!created) throw new Error('Generation run was not created') | |
| return mapGenerationRun(created) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/persistence/build-your-own-adapter.md` around lines 655 - 676, Update
createOrResume so it returns the persisted record after insert rather than
reconstructing and returning the attempted input. Re-read the run using
select.get(input.runId) after insert, including when ON CONFLICT DO NOTHING
means another caller created it first, and map that stored record with
mapGenerationRun.
Comment on lines
+37
to
+39
| The record never holds the generated bytes, so on its own a reload restores | ||
| `status` and `error` while `result` stays `null`. To bring the media back too, | ||
| add server byte storage: see [Keep generated files](./keep-generated-files). |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Separate result metadata from durable media bytes. The persistence contract can restore terminal result metadata without storing generated bytes; artifacts/blobs are required for durable media restoration.
docs/persistence/generation-persistence.md#L37-L39: remove the claim thatresultstays null whenever byte storage is absent.docs/persistence/generation-persistence.md#L224-L228: state that byte storage is needed for durable media, while metadata and non-binary results can still be restored.
📍 Affects 1 file
docs/persistence/generation-persistence.md#L37-L39(this comment)docs/persistence/generation-persistence.md#L224-L228
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/persistence/generation-persistence.md` around lines 37 - 39, The
persistence documentation should distinguish result metadata from durable media
bytes. In docs/persistence/generation-persistence.md lines 37-39, remove the
claim that result always remains null without byte storage; in lines 224-228,
state that artifacts/blobs are required to restore durable media, while metadata
and non-binary results can still be restored.
Comment on lines
+850
to
+860
| if (stored === null || stored === undefined) return | ||
| const snapshot = parseGenerationResumeSnapshot(stored) | ||
| if (!snapshot) return | ||
| // Live state wins: adopt the stored snapshot only if nothing has been | ||
| // observed since construction. | ||
| if (this.resumeSnapshot || this.isLoading || this.status !== 'idle') return | ||
| this.repaintFromSnapshot(snapshot) | ||
| if (snapshot.status === 'running' && snapshot.resumeState?.runId) { | ||
| this.rejoinInFlight(snapshot.resumeState.runId) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Async hydration can resume work on a disposed client.
maybeHydrateResumeSnapshot() starts in the constructor, but nothing re-checks this.disposed after the await on line 840. If the client is disposed while the storage read is in flight (unmount during hydration, or React StrictMode's mount → dispose → remount replay), this continues to repaintFromSnapshot and then rejoinInFlight, which opens a fresh joinRun stream on a client that has already been torn down. dispose() has already run, so nothing will abort that controller. hydrateFromServer() (Line 875-892) has the same gap after awaiting hydrate(...).
The existing this.resumeSnapshot || this.isLoading || this.status !== 'idle' guard doesn't cover this — a disposed-and-never-restarted client is still idle.
🔒️ Proposed guard
if (stored === null || stored === undefined) return
const snapshot = parseGenerationResumeSnapshot(stored)
if (!snapshot) return
// Live state wins: adopt the stored snapshot only if nothing has been
// observed since construction.
if (this.resumeSnapshot || this.isLoading || this.status !== 'idle') return
+ // The read is async: the client may have been disposed while it was in
+ // flight. Never repaint or rejoin onto a torn-down client.
+ if (this.disposed) return
this.repaintFromSnapshot(snapshot)And mirror it in hydrateFromServer():
// Re-check: a send may have started while the fetch was in flight.
if (this.resumeSnapshot || this.isLoading || this.status !== 'idle')
return
+ if (this.disposed) return
this.repaintFromSnapshot(snapshot)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (stored === null || stored === undefined) return | |
| const snapshot = parseGenerationResumeSnapshot(stored) | |
| if (!snapshot) return | |
| // Live state wins: adopt the stored snapshot only if nothing has been | |
| // observed since construction. | |
| if (this.resumeSnapshot || this.isLoading || this.status !== 'idle') return | |
| this.repaintFromSnapshot(snapshot) | |
| if (snapshot.status === 'running' && snapshot.resumeState?.runId) { | |
| this.rejoinInFlight(snapshot.resumeState.runId) | |
| } | |
| } | |
| if (stored === null || stored === undefined) return | |
| const snapshot = parseGenerationResumeSnapshot(stored) | |
| if (!snapshot) return | |
| // Live state wins: adopt the stored snapshot only if nothing has been | |
| // observed since construction. | |
| if (this.resumeSnapshot || this.isLoading || this.status !== 'idle') return | |
| // The read is async: the client may have been disposed while it was in | |
| // flight. Never repaint or rejoin onto a torn-down client. | |
| if (this.disposed) return | |
| this.repaintFromSnapshot(snapshot) | |
| if (snapshot.status === 'running' && snapshot.resumeState?.runId) { | |
| this.rejoinInFlight(snapshot.resumeState.runId) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ai-client/src/video-generation-client.ts` around lines 850 - 860,
Update maybeHydrateResumeSnapshot and hydrateFromServer to check this.disposed
immediately after their awaited storage/server hydration calls and return before
applying state or rejoining work. Ensure disposed clients cannot call
repaintFromSnapshot or start rejoinInFlight/joinRun, while preserving the
existing live-state guards for active clients.
Comment on lines
+209
to
+219
| export function GET(request: Request) { | ||
| return reconstructGeneration(persistence, request, { | ||
| // Multi-user routes MUST authorize: the ids come from the caller. Derive | ||
| // identity from server-side session state, then check ownership. | ||
| authorize: async (id, req) => { | ||
| // const user = await auth(req) | ||
| // return user != null && (await db.threadOwnedBy(user.id, id)) | ||
| void id | ||
| void req | ||
| return true | ||
| }, |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Authorize every persistence identifier in the documentation examples.
The examples accept caller-controlled run/thread/artifact identifiers without consistently enforcing server-side ownership.
packages/ai/skills/ai-core/client-persistence/SKILL.md#L209-L219: replacereturn truewith a real session ownership check or fail closed.packages/ai/skills/ai-core/media-generation/SKILL.md#L590-L595: authorize the submitted thread before persisting the run.packages/ai/skills/ai-core/media-generation/SKILL.md#L609-L618: authorizerecord.threadIdbefore retrieving and returning blob bytes.
Based on learnings: persistence identifiers must be authorized against server-side session ownership before reads or writes.
🧰 Tools
🪛 SkillSpector (2.4.4)
[warning] 52: [MP2] Context Window Stuffing: Skill attempts to fill the context window with filler content, displacing legitimate instructions and safety constraints. This can degrade agent performance or bypass safety boundaries.
Remediation: Implement context-window management that detects and rejects padding or stuffing attempts. Prioritize system instructions over user-injected content.
(Memory Poisoning (MP2))
📍 Affects 2 files
packages/ai/skills/ai-core/client-persistence/SKILL.md#L209-L219(this comment)packages/ai/skills/ai-core/media-generation/SKILL.md#L590-L595packages/ai/skills/ai-core/media-generation/SKILL.md#L609-L618
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ai/skills/ai-core/client-persistence/SKILL.md` around lines 209 -
219, Authorize all caller-supplied persistence identifiers using server-side
session ownership checks. In
packages/ai/skills/ai-core/client-persistence/SKILL.md lines 209-219, replace
the unconditional authorization in GET with a real ownership check or
fail-closed behavior; in packages/ai/skills/ai-core/media-generation/SKILL.md
lines 590-595, authorize the submitted thread before persisting the run; and in
lines 609-618, authorize record.threadId before retrieving or returning blob
bytes.
Source: Learnings
Comment on lines
+163
to
+164
| threadId, | ||
| runId, |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Propagate correlation IDs into lifecycle events.
The IDs are only placed in middleware context; every aiEventClient.emit payload still omits them. Event subscribers therefore receive no threadId/runId, despite the newly expanded event contracts.
packages/ai/src/activities/generateAudio/index.ts#L163-L164: add both IDs to each started, completed, error, and usage payload.packages/ai/src/activities/generateImage/index.ts#L256-L257: add both IDs to each emitted lifecycle payload.packages/ai/src/activities/generateSpeech/index.ts#L171-L172: add both IDs to each started, completed, error, and usage payload.packages/ai/src/activities/generateTranscription/index.ts#L206-L207: add both IDs to each started, completed, error, and usage payload.
📍 Affects 4 files
packages/ai/src/activities/generateAudio/index.ts#L163-L164(this comment)packages/ai/src/activities/generateImage/index.ts#L256-L257packages/ai/src/activities/generateSpeech/index.ts#L171-L172packages/ai/src/activities/generateTranscription/index.ts#L206-L207
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ai/src/activities/generateAudio/index.ts` around lines 163 - 164,
Propagate threadId and runId from the activity context into every
aiEventClient.emit lifecycle payload. Update started, completed, error, and
usage events in packages/ai/src/activities/generateAudio/index.ts (163-164),
packages/ai/src/activities/generateImage/index.ts (256-257),
packages/ai/src/activities/generateSpeech/index.ts (171-172), and
packages/ai/src/activities/generateTranscription/index.ts (206-207); ensure all
emitted payloads include both IDs.
Comment on lines
+36
to
+39
| // Server-authoritative record of the last completed generation per thread. In | ||
| // production this is a `GenerationRunStore` row; here a process-lifetime map is | ||
| // enough for the reload round-trip (the e2e server stays up across reloads). | ||
| const completedByThread = new Map<string, Record<string, unknown>>() |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
git ls-files | rg 'generation-persistence-server|playwright.config|package.json' | sed -n '1,120p'
echo
echo "== target route outline =="
ast-grep outline testing/e2e/src/routes/api.generation-persistence-server.ts --view expanded || true
echo
echo "== target route relevant lines =="
cat -n testing/e2e/src/routes/api.generation-persistence-server.ts | sed -n '1,220p'
echo
echo "== companion files relevant lines =="
fd 'generation-persistence-server' testing/e2e -t f -x sh -c 'echo "--- $1"; wc -l "$1"; cat -n "$1" | sed -n "1,220p"' sh {}
echo
echo "== playwirght config candidates summary =="
for f in $(git ls-files | rg '(^|/)(playwright\.config|.*playwright.*\.config).*\.(ts|js|mjs)$' | sed -n '1,50p'); do
echo "--- $f"
wc -l "$f"
rg -n "webServer|timeout|retries|workers|projects|testDir|reporters|use:|useWebServer|web_servers|server" "$f" || true
done
echo
echo "== relevant searches =="
rg -n "completedByThread|persistedResult|generation-persistence-server|THREAD_ID|generation-server-thread|DELETE|reset|webServer|retries" testing/e2e | sed -n '1,240p'Repository: TanStack/ai
Length of output: 31542
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== e2e playwright config relevant lines =="
cat -n testing/e2e/playwright.config.ts | sed -n '1,90p'
echo
echo "== e2e global setup/teardown =="
fd 'global|teardown|setup' testing/e2e -t f | sed -n '1,120p'
for f in $(fd 'global|teardown|setup' testing/e2e -t f | sed -n '1,40p'); do
echo "--- $f"
rg -n "export default|testDir|globalSetup|globalTeardown|process|port|webServer|retries|shard|environmentVariables|playwright" "$f" || true
done
echo
echo "== all shared server routes with DELETE/reset endpoints nearby generation-persistence server =="
cat -n testing/e2e/src/routes/api.middleware-test.ts | sed -n '360,455p'
rg -n "DELETE:|reset|completedByThread|generation-server-thread|generation-persistence-server|threadId" testing/e2e/src/routes testing/e2e/tests/generation-persistence-server.spec.ts | sed -n '1,220p'
echo
echo "== deterministic probe: route behavior after first completed job without reset =="
python3 - <<'PY'
completed_by_thread = {}
thread_id = 'generation-server-thread'
def persisted_result(thread_id, run_id):
return {'id': 'image-1', 'model': 'mock-image-model', 'artifacts': [{'threadId': thread_id, 'runId': run_id}]}
# Simulate the fixed-thread POST completing once
completed_by_thread[thread_id] = persisted_result(thread_id, 'run-1')
for i in range(3):
result = completed_by_thread.get(thread_id)
body = (
{
'resumeSnapshot': {'schemaVersion': 1, 'resumeState': None, 'status': 'complete', 'activity': 'image', 'result': result},
'activeRun': None,
}
if result else {'resumeSnapshot': None, 'activeRun': None}
)
print(f'request {i}: status={body["resumeSnapshot"]["status"] if body["resumeSnapshot"] else None}')
print("no_route_deletion_found", all(
'completedByThread.delete' in code or "completedByThread.delete" in code
for _ in []
))
PYRepository: TanStack/ai
Length of output: 20753
Add a reset/cleanup path for the server-driven persistence record.
completedByThread is keyed by the hardcoded generation-server-thread ID and is never cleared, while the e2e setup has no global teardown and Playwright reuses the dev server across retries. After the run completes once, the initial status → 'idle' assertion can fail deterministically on retry. Add a test-only DELETE/reset handler for the thread record, or clear it from the spec before the first page.goto.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@testing/e2e/src/routes/api.generation-persistence-server.ts` around lines 36
- 39, Add a test-only reset path for the completedByThread record keyed by
generation-server-thread, either through a DELETE handler or by clearing it in
the persistence spec before the first page.goto. Ensure retries start with no
prior completed generation while preserving the existing server-authoritative
record behavior.
`externalUrl` sat directly above `url` on `PersistedArtifactRef` and read backwards: `externalUrl` is the provider's original expiring link, kept for provenance, while the plain `url` is the durable app-origin URL that actually serves the bytes publicly. The field named "external" was the internal one. `sourceUrl` says what it is — where the bytes came from. It also covers the case `providerUrl` would miss: with `allowInputUrl`, an input artifact's source is a caller-supplied URL, not a provider's. Straight rename, no alias: `PersistedArtifactRef` is not in the published @tanstack/ai@0.42.0, so nothing downstream can be depending on the old name.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/ai-react/tests/use-generation.test.ts (1)
1-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPlace this test alongside its source module.
Move
packages/ai-react/tests/use-generation.test.tsnext topackages/ai-react/src/use-generation.ts(for example,packages/ai-react/src/use-generation.test.ts) to comply with the repository rule for**/*.test.ts.As per coding guidelines, unit tests must be placed in
*.test.tsfiles alongside the source they cover.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-react/tests/use-generation.test.ts` around lines 1 - 24, Move the use-generation test file from the package-level tests directory to sit alongside the useGeneration source module, preserving its existing contents and imports. Place it as use-generation.test.ts next to use-generation.ts so the repository’s test placement rule is satisfied.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/ai-react/tests/use-generation.test.ts`:
- Around line 1-24: Move the use-generation test file from the package-level
tests directory to sit alongside the useGeneration source module, preserving its
existing contents and imports. Place it as use-generation.test.ts next to
use-generation.ts so the repository’s test placement rule is satisfied.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d175f848-bc98-48ce-9a08-7dd8c82bf67a
📒 Files selected for processing (9)
docs/persistence/build-your-own-adapter.mdpackages/ai-client/src/generation-types.tspackages/ai-client/tests/generation-resume-state.test.tspackages/ai-persistence/skills/ai-persistence/build-cloudflare-artifact-store/SKILL.mdpackages/ai-persistence/src/middleware.tspackages/ai-persistence/src/types.tspackages/ai-persistence/tests/generation-artifacts.test.tspackages/ai-react/tests/use-generation.test.tspackages/ai/src/types.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/ai-client/tests/generation-resume-state.test.ts
- docs/persistence/build-your-own-adapter.md
- packages/ai-persistence/tests/generation-artifacts.test.ts
- packages/ai-persistence/src/middleware.ts
- packages/ai-client/src/generation-types.ts
- packages/ai-persistence/src/types.ts
`threadId` was introduced as an optional "link to the chat conversation that triggered this generation". It is not that — it is the generation's own scope, the stable slot successive runs are filed under, and a workflow generating (say) a video's start frame has no conversation anywhere near it. Presenting it as optional produced three concrete defects: - The fallback chain `threadId ?? id ?? generated` ends in Date.now()+random, rebuilt on every construction. With neither supplied, client-driven wrote a new localStorage key every reload (restoring nothing, orphaning the last one) and server-driven asked for a threadId that had never existed. Both failed silently. - The two modes keyed on DIFFERENT values — client-driven on `id`, server-driven on `threadId` — so `id: 'a'` + `threadId: 'b'` wrote slot a and read slot b. - `id` did double duty as devtools label and persistence key, so relabelling in devtools silently relocated persisted data. `threadId` is now required whenever `persistence` is set, via a union (`GenerationPersistenceOptions`) intersected onto each hook's parameter. It stays optional for ephemeral generations, so the published no-persistence signature is untouched — adding an unconditional required option would have broken every existing call site. Persistence now keys on the explicit `threadId` in both modes. The `?? id` fallback survives only for the AG-UI wire thread id, which the protocol requires even when nothing is persisted; a runtime warning covers JS callers who bypass the type. The union is fragile in one specific way — a plain `Omit` over it collapses the union and the requirement silently disappears — so the options interfaces stay non-union (keeping Pick/Omit composition working in vue/solid/svelte/angular) and `use-generation-persistence-types.test.ts` pins the behaviour.
…ForThread `findLatestForThread` is optional on GenerationRunStore and was called through `?.`, so an adapter that does not implement it produced `undefined ?? null` — indistinguishable from an ordinary 'no run found'. A server-driven client would therefore restore nothing, forever, with no error anywhere to explain why. Throw instead, and only on the path that actually needs the method: an explicit `?runId=` lookup never calls it and keeps working on a minimal adapter.
Generated bytes were written to a hardcoded `artifacts/<runId>/<artifactId>` with no way to influence it, so "keep my generated files in my own R2 folder structure" was not expressible. `withGenerationPersistence` now takes a `storageKey` mapper receiving the artifact's identity, role, activity, mime type and resolved name. Server-side only, deliberately: a key supplied by the browser would be a path-traversal and cross-tenant-write vector, the same class as the two issues already fixed on this branch. This forces a companion change. `retrieveBlob` RECOMPUTED the path from runId + artifactId, which only works while the derivation is a fixed constant — the moment it is user-supplied the read looks in the wrong place. The resolved key is therefore recorded on the new `ArtifactRecord.blobKey`, and reads go through `resolveArtifactBlobKey`, which falls back to the old convention for records written before the field existed. That fallback is what makes this a non-breaking addition, and also why the default convention can never be changed retroactively. Worth having independently of `storageKey`: with the key recomputed rather than remembered, the default convention was effectively frozen forever — changing `artifactBlobKey` would have orphaned every blob already written. Also threads the required `threadId` through the docs, skills, E2E harness and example call sites, and documents both new capabilities in the changeset.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/ai-solid/src/use-generation.ts (2)
259-265: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClear the previous body when the option becomes undefined.
The conditional spread on Line 263 skips the update when
currentBodyis removed, so the client keeps sending the previous body on later generations. Explicitly clear it using the supported empty/unset representation.Proposed fix
createEffect(() => { const currentBody = options.body - client.updateOptions({ - ...(currentBody !== undefined && { body: currentBody }), - }) + client.updateOptions({ body: currentBody ?? {} }) })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-solid/src/use-generation.ts` around lines 259 - 265, Update the body synchronization effect around currentBody and client.updateOptions so it explicitly clears the client’s previous body when options.body becomes undefined, using the client’s supported empty or unset representation; preserve passing the current body when it is defined.
186-201: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winUse the conditional body spread described by this implementation comment.
options.bodymay be absent whileGenerationClientOptions.bodyis a strict optional, so this direct assignment can fail underexactOptionalPropertyTypes. Match the similar Svelte/React use-generation handling and omit the key when it is undefined.Proposed fix
- body: options.body, + ...(options.body !== undefined && { body: options.body }),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-solid/src/use-generation.ts` around lines 186 - 201, Update the clientOptions construction in useGeneration to remove the direct body assignment and conditionally spread body only when options.body is defined. Preserve the existing body value when present and omit the property when absent, matching the handling in the Svelte/React use-generation implementations.
♻️ Duplicate comments (1)
packages/ai/skills/ai-core/client-persistence/SKILL.md (1)
198-210: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMake every persisted generation route require and propagate
threadId.
packages/ai/skills/ai-core/client-persistence/SKILL.md#L198-L210: pass the validated clientthreadIdintogenerateImage.packages/ai/skills/ai-core/media-generation/SKILL.md#L589-L595: reject missingthreadIdinstead of documenting it as optional.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai/skills/ai-core/client-persistence/SKILL.md` around lines 198 - 210, Make every persisted generation route require a validated client threadId and propagate it through the generation call. In packages/ai/skills/ai-core/client-persistence/SKILL.md lines 198-210, update the POST handler and generateImage invocation to pass threadId; in packages/ai/skills/ai-core/media-generation/SKILL.md lines 589-595, change the documented contract to reject missing threadId rather than treating it as optional.
🧹 Nitpick comments (1)
packages/ai-react/tests/use-generation-persistence-types.test.ts (1)
18-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffPlace these tests alongside their covered hooks.
This shared test is under
tests/while it covers modules insrc/. Split or relocate the assertions into colocated*.test.tsfiles. As per coding guidelines,**/*.test.ts: “Place unit tests in*.test.tsfiles alongside the source they cover.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-react/tests/use-generation-persistence-types.test.ts` around lines 18 - 24, The shared persistence type tests in use-generation-persistence-types.test.ts should be colocated with the hooks they cover. Split the assertions into *.test.ts files alongside useGenerateImage, useGenerateVideo, useGeneration, useSummarize, and useTranscription, preserving the existing coverage and removing the centralized tests/placement.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/ai-solid/src/use-generation.ts`:
- Around line 259-265: Update the body synchronization effect around currentBody
and client.updateOptions so it explicitly clears the client’s previous body when
options.body becomes undefined, using the client’s supported empty or unset
representation; preserve passing the current body when it is defined.
- Around line 186-201: Update the clientOptions construction in useGeneration to
remove the direct body assignment and conditionally spread body only when
options.body is defined. Preserve the existing body value when present and omit
the property when absent, matching the handling in the Svelte/React
use-generation implementations.
---
Duplicate comments:
In `@packages/ai/skills/ai-core/client-persistence/SKILL.md`:
- Around line 198-210: Make every persisted generation route require a validated
client threadId and propagate it through the generation call. In
packages/ai/skills/ai-core/client-persistence/SKILL.md lines 198-210, update the
POST handler and generateImage invocation to pass threadId; in
packages/ai/skills/ai-core/media-generation/SKILL.md lines 589-595, change the
documented contract to reject missing threadId rather than treating it as
optional.
---
Nitpick comments:
In `@packages/ai-react/tests/use-generation-persistence-types.test.ts`:
- Around line 18-24: The shared persistence type tests in
use-generation-persistence-types.test.ts should be colocated with the hooks they
cover. Split the assertions into *.test.ts files alongside useGenerateImage,
useGenerateVideo, useGeneration, useSummarize, and useTranscription, preserving
the existing coverage and removing the centralized tests/placement.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c3d1d0d8-9105-4d15-9bf4-26d4ae4a53f1
📒 Files selected for processing (65)
.changeset/generation-persistence.mddocs/persistence/generation-persistence.mddocs/persistence/keep-generated-files.mdexamples/ts-react-chat/src/routes/generation-hooks.tsxexamples/ts-react-chat/src/routes/generations.audio.tsxexamples/ts-react-chat/src/routes/generations.image.tsxexamples/ts-react-chat/src/routes/generations.speech.tsxexamples/ts-react-chat/src/routes/generations.summarize.tsxexamples/ts-react-chat/src/routes/generations.transcription.tsxexamples/ts-react-chat/src/routes/generations.video.tsxpackages/ai-angular/src/inject-generate-audio.tspackages/ai-angular/src/inject-generate-image.tspackages/ai-angular/src/inject-generate-speech.tspackages/ai-angular/src/inject-generate-video.tspackages/ai-angular/src/inject-generation.tspackages/ai-angular/src/inject-summarize.tspackages/ai-angular/src/inject-transcription.tspackages/ai-angular/tests/inject-generation.test.tspackages/ai-client/src/generation-client.tspackages/ai-client/src/generation-types.tspackages/ai-client/src/index.tspackages/ai-persistence/skills/ai-persistence/SKILL.mdpackages/ai-persistence/src/index.tspackages/ai-persistence/src/middleware.tspackages/ai-persistence/src/reconstruct-generation.tspackages/ai-persistence/src/retrieve.tspackages/ai-persistence/src/types.tspackages/ai-persistence/tests/generation-artifacts.test.tspackages/ai-persistence/tests/reconstruct-generation.test.tspackages/ai-react/src/use-generate-audio.tspackages/ai-react/src/use-generate-image.tspackages/ai-react/src/use-generate-speech.tspackages/ai-react/src/use-generate-video.tspackages/ai-react/src/use-generation.tspackages/ai-react/src/use-summarize.tspackages/ai-react/src/use-transcription.tspackages/ai-react/tests/use-generation-persistence-types.test.tspackages/ai-react/tests/use-generation.test.tspackages/ai-solid/src/use-generate-audio.tspackages/ai-solid/src/use-generate-image.tspackages/ai-solid/src/use-generate-speech.tspackages/ai-solid/src/use-generate-video.tspackages/ai-solid/src/use-generation.tspackages/ai-solid/src/use-summarize.tspackages/ai-solid/src/use-transcription.tspackages/ai-solid/tests/use-generation.test.tspackages/ai-svelte/src/create-generate-audio.svelte.tspackages/ai-svelte/src/create-generate-image.svelte.tspackages/ai-svelte/src/create-generate-speech.svelte.tspackages/ai-svelte/src/create-generate-video.svelte.tspackages/ai-svelte/src/create-generation.svelte.tspackages/ai-svelte/src/create-summarize.svelte.tspackages/ai-svelte/src/create-transcription.svelte.tspackages/ai-svelte/tests/create-generation.test.tspackages/ai-vue/src/use-generate-audio.tspackages/ai-vue/src/use-generate-image.tspackages/ai-vue/src/use-generate-speech.tspackages/ai-vue/src/use-generate-video.tspackages/ai-vue/src/use-generation.tspackages/ai-vue/src/use-summarize.tspackages/ai-vue/src/use-transcription.tspackages/ai-vue/tests/use-generation.test.tspackages/ai/skills/ai-core/client-persistence/SKILL.mdpackages/ai/skills/ai-core/media-generation/SKILL.mdtesting/e2e/src/routes/generation-persistence.tsx
🚧 Files skipped from review as they are similar to previous changes (55)
- examples/ts-react-chat/src/routes/generations.audio.tsx
- packages/ai-persistence/src/retrieve.ts
- examples/ts-react-chat/src/routes/generations.speech.tsx
- packages/ai-client/src/index.ts
- examples/ts-react-chat/src/routes/generation-hooks.tsx
- docs/persistence/generation-persistence.md
- examples/ts-react-chat/src/routes/generations.image.tsx
- examples/ts-react-chat/src/routes/generations.summarize.tsx
- packages/ai-persistence/tests/reconstruct-generation.test.ts
- packages/ai-persistence/src/index.ts
- examples/ts-react-chat/src/routes/generations.video.tsx
- packages/ai-react/src/use-generate-speech.ts
- packages/ai-react/src/use-summarize.ts
- packages/ai-react/src/use-transcription.ts
- packages/ai-persistence/src/reconstruct-generation.ts
- packages/ai-solid/src/use-generate-audio.ts
- packages/ai-vue/src/use-generate-video.ts
- packages/ai-angular/src/inject-generate-video.ts
- packages/ai-vue/src/use-transcription.ts
- examples/ts-react-chat/src/routes/generations.transcription.tsx
- docs/persistence/keep-generated-files.md
- packages/ai-react/src/use-generate-audio.ts
- testing/e2e/src/routes/generation-persistence.tsx
- packages/ai-angular/src/inject-generate-image.ts
- packages/ai-react/src/use-generate-image.ts
- packages/ai-vue/src/use-generate-speech.ts
- packages/ai-solid/src/use-summarize.ts
- packages/ai-solid/src/use-generate-image.ts
- packages/ai-vue/src/use-summarize.ts
- packages/ai-svelte/src/create-generate-audio.svelte.ts
- packages/ai-solid/src/use-generate-video.ts
- packages/ai-persistence/tests/generation-artifacts.test.ts
- packages/ai-svelte/src/create-generate-video.svelte.ts
- packages/ai-angular/src/inject-generation.ts
- packages/ai-vue/src/use-generate-audio.ts
- packages/ai-angular/src/inject-generate-speech.ts
- packages/ai-solid/src/use-generate-speech.ts
- packages/ai-angular/src/inject-summarize.ts
- packages/ai-svelte/src/create-summarize.svelte.ts
- packages/ai-vue/src/use-generate-image.ts
- packages/ai-angular/src/inject-transcription.ts
- packages/ai-react/src/use-generation.ts
- packages/ai-vue/tests/use-generation.test.ts
- packages/ai-svelte/tests/create-generation.test.ts
- packages/ai-solid/src/use-transcription.ts
- packages/ai-solid/tests/use-generation.test.ts
- packages/ai-react/tests/use-generation.test.ts
- packages/ai-svelte/src/create-generation.svelte.ts
- packages/ai-client/src/generation-types.ts
- packages/ai-react/src/use-generate-video.ts
- packages/ai-svelte/src/create-transcription.svelte.ts
- packages/ai-persistence/src/middleware.ts
- packages/ai-client/src/generation-client.ts
- packages/ai-vue/src/use-generation.ts
- packages/ai-persistence/src/types.ts
… require store methods The example demonstrated only the client-driven half of generation persistence. `withGenerationPersistence` and `reconstructGeneration` had never been run against each other over HTTP anywhere — each was unit-tested in isolation, and the e2e harness deliberately hand-builds the hydration JSON rather than pull in `@tanstack/ai-persistence`. That left the join between them, which this branch just changed the key of, as the least-covered part of the feature. `/api/generate/image` now runs the real thing: `withGenerationPersistence` with byte storage and an `artifactUrl`, plus a GET that serves artifact bytes by id or answers mount hydration. The Streaming variant switches to `persistence: true`; Direct and Server Fn keep the client adapter because server functions have no GET path for server-driven restore to use. Make three store methods required, per this file's own evolution policy: - `GenerationRunStore.findLatestForThread` was optional and feature-detected — the exact anti-pattern the policy documents, and the exact bug it records `findActiveRun` causing for a release cycle. Server-driven hydration calls it on every mount, so an adapter without it was indistinguishable from a thread with no runs: `persistence: true` silently restored nothing, forever. The runtime guard added earlier on this branch is deleted — the compiler enforces it now, and the cases those tests covered are unrepresentable. - `ArtifactStore.delete` / `deleteForRun` were optional while their pair `BlobStore.delete` is required, so a backend could drop the bytes but keep the record. An app calling `stores.artifacts.delete?.(id)` for an erasure request would silently no-op. Also documents `blobKey` in the adapter guide's reference record and ER diagram, where `ARTIFACT ||--|| BLOB` was a derived convention and is now a real key, and deletes `api.interrupts.test.ts` — 19 assertions no runner has ever executed (the example's vitest config scopes to `src/lib/**`), which also cost a route-scanner warning on every dev start.
The module-level `memoryPersistence()` was rebuilt every time Vite re-evaluated this module, so any artifact URL already stamped into a rendered result 404'd on the next file save — the image broke even though its b64Json was still present, because the UI prefers `img.url`. Stash the instance on globalThis so one dev session keeps one store.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/ts-react-chat/src/routes/generations.image.tsx (1)
186-195: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAvoid rendering a broken data URL after client-side restoration.
The documented direct/server-function restore path has no image bytes. When
img.urlandimg.b64Jsonare both absent, this rendersdata:image/png;base64,undefinedinstead of a meaningful unavailable state.Proposed fix
{result && ( <div className="space-y-4"> - {result.images.map((img, i) => ( + {result.images + .filter((img) => img.url || img.b64Json) + .map((img, i) => ( <img key={i} src={img.url || `data:image/png;base64,${img.b64Json}`} alt={img.revisedPrompt || prompt} className="w-full rounded-lg border border-gray-700" /> - ))} + ))} </div> )}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/ts-react-chat/src/routes/generations.image.tsx` around lines 186 - 195, Update the image rendering in the result.images map to handle entries where both img.url and img.b64Json are absent: avoid constructing a data URL with undefined and render a meaningful unavailable state instead. Preserve the existing URL and base64 image rendering for entries that contain image data.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/ts-react-chat/src/routes/api.generate.image.ts`:
- Around line 41-64: Authorize image-generation persistence using a
server-derived per-session identity rather than trusting supplied opaque IDs. In
examples/ts-react-chat/src/routes/api.generate.image.ts:41-64, validate
ownership of supplied threadId/runId before passing them to generateImage and
withGenerationPersistence; in
examples/ts-react-chat/src/routes/api.generate.image.ts:71-96, provide the
authorization callback during reconstruction and verify the artifact record’s
threadId before returning bytes. In
examples/ts-react-chat/src/routes/generations.image.tsx:29-38, replace the
shared "image:streaming" identifier with a per-user/per-thread identifier, while
continuing to authorize it at the route boundary.
---
Outside diff comments:
In `@examples/ts-react-chat/src/routes/generations.image.tsx`:
- Around line 186-195: Update the image rendering in the result.images map to
handle entries where both img.url and img.b64Json are absent: avoid constructing
a data URL with undefined and render a meaningful unavailable state instead.
Preserve the existing URL and base64 image rendering for entries that contain
image data.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: be02f6ef-0654-4d85-b07c-48aa8fc75311
📒 Files selected for processing (11)
docs/persistence/build-your-own-adapter.mdexamples/ts-react-chat/src/lib/generation-persistence.tsexamples/ts-react-chat/src/lib/generation-server-store.tsexamples/ts-react-chat/src/routes/api.generate.image.tsexamples/ts-react-chat/src/routes/api.interrupts.test.tsexamples/ts-react-chat/src/routes/generations.image.tsxpackages/ai-persistence/skills/ai-persistence/build-cloudflare-artifact-store/SKILL.mdpackages/ai-persistence/src/reconstruct-generation.tspackages/ai-persistence/src/types.tspackages/ai-persistence/tests/persistence-fixtures.tspackages/ai-persistence/tests/reconstruct-generation.test.ts
💤 Files with no reviewable changes (2)
- examples/ts-react-chat/src/routes/api.interrupts.test.ts
- packages/ai-persistence/tests/reconstruct-generation.test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- examples/ts-react-chat/src/lib/generation-persistence.ts
- packages/ai-persistence/tests/persistence-fixtures.ts
- packages/ai-persistence/src/reconstruct-generation.ts
- packages/ai-persistence/src/types.ts
- docs/persistence/build-your-own-adapter.md
Comment on lines
+41
to
+64
| const { input, threadId, runId } = await generationParamsFromRequest( | ||
| 'image', | ||
| request, | ||
| ) | ||
| if (typeof input.prompt !== 'string') { | ||
| throw new Error('This endpoint accepts text image prompts only.') | ||
| } | ||
|
|
||
| const stream = generateImage({ | ||
| adapter: openaiImage(model ?? 'gpt-image-1'), | ||
| prompt, | ||
| size, | ||
| numberOfImages, | ||
| adapter: grokImage('grok-imagine-image'), | ||
| prompt: input.prompt, | ||
| // `size` is deliberately not forwarded: the generic image input types | ||
| // it as `string`, while each adapter narrows it to its own union, and | ||
| // this page's UI never sends one. | ||
| ...(input.numberOfImages | ||
| ? { numberOfImages: input.numberOfImages } | ||
| : {}), | ||
| ...(threadId ? { threadId } : {}), | ||
| ...(runId ? { runId } : {}), | ||
| stream: true, | ||
| middleware: [ | ||
| withGenerationPersistence(persistence, { | ||
| artifactUrl: (ref) => artifactServeUrl(ref.artifactId), | ||
| }), |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Authorize generation persistence per session instead of using the shared thread.
Any visitor can request ?threadId=image:streaming, obtain the latest restored result and its artifact URL, then fetch the artifact bytes. The POST also accepts arbitrary run/thread IDs, allowing untrusted clients to write into that shared scope.
examples/ts-react-chat/src/routes/api.generate.image.ts#L41-L64: derive the effective identity server-side and authorize supplied run/thread ownership before persisting.examples/ts-react-chat/src/routes/api.generate.image.ts#L71-L96: pass an authorization callback to reconstruction and verify the artifact record’sthreadIdbefore returning its blob.examples/ts-react-chat/src/routes/generations.image.tsx#L29-L38: replace the globally shared literal with a per-user/per-thread identifier; do not treat it as authorization.
Based on learnings, threadId must be treated only as an opaque lookup key and authorized at the route boundary before persistence access.
📍 Affects 2 files
examples/ts-react-chat/src/routes/api.generate.image.ts#L41-L64(this comment)examples/ts-react-chat/src/routes/api.generate.image.ts#L71-L96examples/ts-react-chat/src/routes/generations.image.tsx#L29-L38
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/ts-react-chat/src/routes/api.generate.image.ts` around lines 41 -
64, Authorize image-generation persistence using a server-derived per-session
identity rather than trusting supplied opaque IDs. In
examples/ts-react-chat/src/routes/api.generate.image.ts:41-64, validate
ownership of supplied threadId/runId before passing them to generateImage and
withGenerationPersistence; in
examples/ts-react-chat/src/routes/api.generate.image.ts:71-96, provide the
authorization callback during reconstruction and verify the artifact record’s
threadId before returning bytes. In
examples/ts-react-chat/src/routes/generations.image.tsx:29-38, replace the
shared "image:streaming" identifier with a per-user/per-thread identifier, while
continuing to authorize it at the route boundary.
Source: Learnings
Finish the example's `node:sqlite` adapter for generations: the schema and row types were in place, the store implementations were not. - `GenerationRunStore`: idempotent `createOrResume` via ON CONFLICT DO NOTHING, dynamic-SET `update` over the JSON columns, `findLatestForThread` on the (thread_id, started_at DESC) index. - `ArtifactStore`: upsert `save` persisting `blobKey`/`sourceUrl`, run-scoped `list` / `deleteForRun`. - `BlobStore`: bytes in a BLOB column, keyset-cursor `list`. Prefix matching uses `substr(key, 1, length(?)) = ?` rather than LIKE — SQLite's LIKE is case-insensitive for ASCII and treats %/_ as wildcards, both of which break the contract's literal, case-sensitive prefix rule. The factory returns a fully-spelled seven-store `AIPersistence`, so one instance backs both `withPersistence` and `withGenerationPersistence`, and the example's generation route now runs on it instead of `memoryPersistence()` — generated images survive a dev-server restart, which is what the reverted HMR workaround was standing in for. Extend `runPersistenceConformance` to `generationRuns` / `artifacts` / `blobs` so the generation half is held to the same gate as chat. Because the suite fails loudly on an undeclared missing store, a chat-only adapter now passes `skip: ['generationRuns', 'artifacts', 'blobs']`; the adapter-building skills and the build-your-own-adapter guide are updated to match. Also fixes the pre-`blobKey` artifact schema still shown in the docs and the Cloudflare artifact-store skill (`external_url`, no `blob_key`) — copying it made any artifact written with a custom `storageKey` unreadable, since the key can no longer be recomputed.
Image was the only route running `withGenerationPersistence`; video, audio, speech and transcription streamed straight through, so their media lived only at the provider's expiring URL and a restored run had nothing to render. All five now persist. Bytes are served by ONE shared route — `/api/artifacts` — instead of a per-route `?artifact=` branch: artifacts are addressed by id and carry their own `mimeType`, so nothing about serving them is activity-specific, and the authorization check a real deployment needs lives in one place. `artifactServeUrl` points there and every route passes it as `artifactUrl`, so results are rewritten to our origin. The image route's GET is now purely `reconstructGeneration` mount hydration. Audio/speech/transcription keep their zod validation and typed 400s; they gain `generationParamsFromBody` to lift `threadId` / `runId` off the AG-UI envelope so runs are filed under the scope the client hydrates by. Video reads its adapter arguments off `data` as before — `size`/`model` are adapter-specific unions the provider-agnostic video input widens to `string` — and uses the helper for identity only. Transcription produces text, not media: what it persists is the run record plus the input audio artifact.
Refreshing mid-generation surfaced "Stream response body read failed". Resumability is automatic on the CLIENT and opt-in on the SERVER. On mount the client re-attaches to a run it believes is still going by issuing `GET <route>?offset=-1&runId=…`. None of the generation routes had a GET, so Start's catch-all answered with the SPA's HTML shell, which the client then failed to parse as SSE — surfacing a raw transport error (StreamReadError) in place of anything actionable. Every streaming generation route now opts in, per the resumable-streams guide: chunks are logged and id-tagged through `memoryStream` on the response, and a GET replays the log. An unknown or aged-out run now answers with a RUN_ERROR event on a real `text/event-stream` instead of HTML. Video additionally detaches its run from the request (`startDetachedGeneration` in the new lib/generation-durability), so a reload cannot kill a multi-minute job — the producer keeps going and the reader is what gets cancelled. That is the persistent-chat route's policy and it is deliberately NOT applied to the short activities: a detached run keeps billing after the user leaves, and an image or a speech clip is cheaper to re-run than to keep alive. The image GET now serves two jobs in order, like the chat route: delivery replay when the request carries a resume offset, otherwise `reconstructGeneration` mount hydration.
`nitro/dist/_build/vite.dev.mjs` classifies a request as a static asset from `Sec-Fetch-Dest`: anything that isn't `document`/`iframe`/`frame` falls through to vite's static middleware, which has no file and 404s with connect's `Cannot GET` page. The extension branch only applies when the header is absent or `empty`, so renaming the route doesn't help. That makes every artifact URL unloadable in dev: `<img src="/api/artifacts?id=…">` sends `Sec-Fetch-Dest: image` and 404s, while the same URL fetched from JS (`empty`) returns the bytes. It only bites routes served under Start's catch-all `/**`, which is all of them here. A pre-plugin presents `empty` for our own `/api/` paths, routing them back to the server without changing what the browser sends. Dev-only — this middleware does not exist in a production build.
…unctions
Server-driven persistence (`persistence: true`) previously required an HTTP
endpoint, because the hydrate/rejoin handlers lived on the connection adapter
and only the fetch/XHR adapters implemented them. A TanStack Start server
function had no way to participate, so `persistence: true` silently restored
nothing there.
Persistence handlers are now supplied independently of the transport:
- `stream()` takes an optional second argument of `{ hydrate,
hydrateGeneration, joinRun }`, spread onto the adapter.
- The generation client accepts `hydrateGeneration` / `joinRun` as options,
used when the connection carries none. The connection's handlers win when
both exist, and `persistence: true` with no handler from either source warns
instead of silently no-opping.
- `memoryStream` accepts an explicit `{ runId, offset }` alongside a `Request`,
and the new `replayRunStream` replays a run's delivery log as a bare chunk
stream — what a server function needs to serve `joinRun` without an HTTP
`Response`.
A restored snapshot that reports a run still in flight is now repainted through
one path: tail it via `joinRun` when a handler exists, otherwise repaint it as
an interrupted error rather than a `generating` status that would never settle.
The generation hooks across React, Solid, Vue, Svelte and Angular forward the
new options.
tombeckenham
force-pushed
the
feat/generation-persistence-full
branch
from
July 29, 2026 11:09
0fa9d8a to
420a9da
Compare
…rateVideo A persisted video restored as nothing on reload. The run record showed `status: 'complete'` and nothing else — no result metadata, no artifact refs, no stored bytes, and `thread_id` NULL. Streaming video was the only media activity that never called `applyGenerationResultTransforms`, and never put the caller's `threadId` / `runId` on the middleware context. `withGenerationPersistence` registers BOTH its artifact capture and its run-record `result` write as result transforms, pushed onto an OPTIONAL `ctx.resultTransforms` — so both silently no-opped, and the run was filed under the internal `requestId` with no thread link. The client rebuilds a restored video from an output artifact carrying a durable url, found none, and restored nothing. Video now applies the transforms to its terminal result before yielding it, so the `generation:result` chunk and the stored record carry the same urls (including the app-origin one `artifactUrl` stamps), and passes `threadId` / `runId` / `artifactInputs` into the context like `generateImage`. `threadId` is now a documented option on `generateVideo`. It previously had none, so callers passing one through an object spread type-checked and were silently ignored — which is how the example's route looked correct while recording NULL. When omitted, an id is still minted for the RUN_* wire chunks, but the middleware context gets `undefined` instead: a fabricated thread id is a slot no client can hydrate by, which is worse than no link at all. Both regression tests fail against the previous behaviour.
The client hooks require `threadId` whenever `persistence` is set; the server
middleware did not. That asymmetry hid a class of silent failure: a run filed
under no scope cannot be hydrated by one, so `persistence: true` restored
nothing, forever, with no error to explain why. The example's video route hit
exactly this — its runs recorded `thread_id: NULL`.
`withGenerationPersistence(persistence, { threadId, ... })` now takes a
required `threadId` via the new `WithGenerationPersistenceOptions`, mirroring
the client's discriminated union.
The option is also the AUTHORITY for the run record's and artifacts' scope, in
preference to `ctx.threadId`. An activity mints a throwaway thread id for its
RUN_* wire chunks when the caller passes none, and persisting that fabricated
id filed runs in a slot nothing could look up — worse than recording no link,
because it looks like one. A test that asserted the old fallback (wire id ==
persisted id) now asserts they deliberately diverge.
Call sites updated across the example routes, docs and skills. The example
routes reject a request carrying no `threadId` with a 400 rather than inventing
one, which is the pattern the docs now show.
Note: `docs/persistence/generation-persistence.md` has one remaining kiira
failure in the `getImageHydrationFn` snippet (a `ReconstructedGeneration` /
Start `ServerFn` return-type mismatch). It predates this commit — verified by
stashing these changes — and is left alone.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎯 Changes
Generation persistence — the media-generation parallel of chat persistence. A media run (image, video, audio, TTS, transcription, summarize) now survives a page reload or a dropped connection, restoring transparently into the normal hook fields, with optional durable storage of the generated bytes.
Split out of #987 and rebuilt on the current
feat/persistence-core. The previously-stacked byte-storage half has been folded in, so this is now the whole feature in one PR.Supersedes #997 (same change, renamed from
feat/generation-persistence-client— the branch always contained the server half too) and folds in #998.The shape
A generation run is recorded in its own store, keyed by its
runId(the same AG-UI run id the client sends), withthreadIdan optional link — it no longer overloads the chatRunStore. On the client, the hook picks a mode exactly the wayuseChatdoes:Restore is invisible. There is no
resumeSnapshot/pendingArtifacts/resultArtifactshook field — a reload repaintsresult/status/erroras if the run had just finished.resumeStatecarries the in-flight run identity only.Crucially, this restores a record, not provider work: generation still only begins when
generate(...)is called, and nothing is ever restarted. A run that is still streaming is re-attached and finished in place, via the connection'sjoinRundurability replay — the same mechanismuseChatuses.Server
withGenerationPersistencerecords each run in a dedicatedgenerationRuns(GenerationRunStore) store: activity/provider/model, lifecycle status, result metadata, and — when byte storage is on — the durable artifact refs.reconstructGeneration(persistence, request, options?)is the generation parallel ofreconstructChat: it reads?runId=(preferred) or the latest run linked to?threadId=, authorizes viaauthorize, and returns{ resumeSnapshot, activeRun }JSON for a server-authoritative client to hydrate from.artifacts(ArtifactStore) and ablobs(BlobStore) store and the middleware writes each generated file's bytes underartifacts/<runId>/<artifactId>, records anArtifactRecord, and attachesPersistedArtifactRefs to the result and the run record. The newartifactUrloption stamps a durable app-origin serve URL onto each ref and rewrites the live result's media URL to it, so live and restored results both render from your origin instead of the provider's expiring link.retrieveArtifact/retrieveBlob(and the sharedartifactBlobKey) serve the bytes back; extraction is customizable viaextractArtifacts/nameArtifact.memoryPersistence()ships in-memorygenerationRuns/artifacts/blobs;defineGenerationRunStore/defineArtifactStore/defineBlobStoretype a custom store inline the waydefineMessageStore/defineRunStorealready do.Client
Generation hooks (
useGenerateImage,useGenerateVideo,useGenerateAudio,useGenerateSpeech,useGeneration,useSummarize,useTranscription, and the Solid/Vue/Svelte/Angular equivalents) take the samepersistenceoption chat does. The option reuses theChatStorageAdaptercontract, solocalStoragePersistence/sessionStoragePersistence/indexedDBPersistencework for generations too with no type argument — a bare call is correct on either side. Untrusted snapshots are validated on read with the newparseGenerationResumeSnapshot.With byte storage configured, a restored
resultis rebuilt whole, its media resolved to the durable serve URL and its refs onresult.artifacts. Without it,status/errorrestore andresultstaysnull— a client snapshot never holds bytes.The base generation return types (
UseGenerationReturn/CreateGenerationReturn/InjectGenerationResult) also gained a defaultedTInputgeneric across all five frameworks, sogenerateis precisely typed(input: TInput) => Promise<void>. That deleted the unsound internal narrow-to-wide casts and every wrapper-levelgenerate ascast — twenty-five in total — with existing single-generic references unaffected.Packages
@tanstack/ai-persistence—GenerationRunStore,reconstructGeneration, the byte-storage half (ArtifactStore/BlobStore/retrieveArtifact/retrieveBlob/artifactUrl), and thedefine*store helpers.@tanstack/ai-client— the resume-snapshot reducer,parseGenerationResumeSnapshot, mount hydration (client store and serverGET), result reconstruction from refs.@tanstack/ai-utils—base64ToUint8Array.@tanstack/ai— the generation activities gainedthreadId/runIdoptions.Docs, skills, examples, tests
docs/persistence/generation-persistence.mdanddocs/persistence/keep-generated-files.md, plus updates acrossclient-persistence,overview,controls,build-your-own-adapter,internals, thedocs/media/*pages,docs/chat/streaming.mdanddocs/interrupts/overview.md(threads / runs / turns are now explained consistently). All kiira-verified.ai-core/client-persistence,ai-core/media-generation,ai-persistence, and a newai-persistence/build-cloudflare-artifact-store.examples/ts-react-chatwires generation persistence end to end (image + video via Grok Imagine).generation-persistence.spec.ts(client-driven: stream → reload → transparent restore → reset) andgeneration-persistence-server.spec.ts(server-driven:persistence: true, restore from a?threadId=GET,localStoragestays empty, no run auto-starts). Plus hydration/restore unit coverage in all five framework packages and the persistence package.✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
threadId+ ephemeralrunId.resumeStateand correctly repaintstatus/result/errorwhen persistence is enabled.threadId, plus secure URL input fetching protections.threadId/runIdfor better correlation.