CFM-18: lower attachment threshold so large saves succeed (backport of #428) - #432
Merged
Conversation
Large interactive states near 480 KiB produced ~1.06 MB Firestore answer documents that exceeded the 1 MB limit and failed to save. Activity Player stores the state twice as escaped JSON strings inside a report wrapper, expanding it to ~2.2 bytes of document per character of state. Lower kDynamicAttachmentSizeThreshold from 480 KiB to 400 KiB so oversized states are offloaded to an S3 attachment before the document grows too large. (cherry picked from commit 41cd3cd)
- Clarify that state at or above the threshold is offloaded, matching the inclusive >= check in shouldSaveAsAttachment. - Use consistent KiB/MiB units in the size math. (cherry picked from commit de0d63a)
shouldSaveAsAttachment() measured String.prototype.length (UTF-16 code units), which undercounts non-ASCII content relative to Firestore's byte-based 1 MiB document limit. Switch to UTF-8 byte counting via TextEncoder so the size check matches the byte-based threshold. Adds a jsdom test-environment shim for TextEncoder and a regression test covering multi-byte content. (spec R6) (cherry picked from commit ddc46e9)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Backports the bug fix from #428 onto the
v1.9.xrelease branch.Summary
Large interactive states near 480 KiB produced ~1.06 MB Firestore answer documents that exceeded the 1 MB limit and failed to save. Activity Player stores the state twice as escaped JSON strings inside a report wrapper, expanding it to ~2.2 bytes of document per character of state. This lowers
kDynamicAttachmentSizeThresholdfrom 480 KiB to 400 KiB so oversized states are offloaded to an S3 attachment before the document grows too large.Additionally,
shouldSaveAsAttachment()now measures the serialized state's UTF-8 byte length (viaTextEncoder) instead ofJSON.stringify(content).length, which counts UTF-16 code units. The previous check undercounted non-ASCII content relative to Firestore's byte-based 1 MB limit, so multi-byte states could exceed the limit even while reported as "below" the threshold. A regression test covers this case, and a jsdom test-environment shim providesTextEncoderfor tests.Details
Cherry-picked (
git cherry-pick -x) the three commits from #428. One conflict insrc/setupTests.ts: the v2.2.x branch has additionaljestSpyConsolesetup that does not exist on v1.9.x. Resolved by keeping only theTextEncoderpolyfill (the part the bug fix needs) and omitting the unrelatedjestSpyConsolelines. Theinteractive-api-providertest suite passes (15/15), including the new multi-byte regression test.See the original investigation: https://gist.github.com/dougmartin/54bafa93a9dc5185a609da8b5b63b4ac
Relates to CFM-18.
Test plan
npx jest src/code/providers/interactive-api-provider.test.tspasses locallyCo-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com