test(flow): lock the wire envelopes before consolidating the Flow API surface - #46
Merged
Merged
Conversation
The batch builders are about to gain a caller: Omni will submit through the same generate_video that Veo uses, differing only by a model key. That refactor can drop a trailing null and still look fine — Flow accepts a malformed payload and then ignores it, which reads exactly like success. test_flow_batch.py asserts positions, and positions survive that. So assert the whole string instead. Checked by flipping SURFACE_ID 22 -> 23: all three goldens fail, and all 48 position tests pass. Also adds requirements-dev.txt — the test deps were undeclared, so a fresh checkout could not run the suite that guards any of this. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DB27wvPQ195KGiQ6VZfCDf
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
1 participant
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.
Groundwork for consolidating the Flow API surface. Tests and dev deps only — no production code changes.
Why
Several contributors migrated to
flow.google.com/batchexecutein parallel, and the same Flow call now has multiple implementations. Consolidating them means moving who builds the request envelope, and that refactor has a nasty failure mode: Flow accepts a malformed payload and then quietly ignores it, which is indistinguishable from success. You only find out when the render never arrives.tests/unit/test_flow_batch.pyasserts positions — slot 3 is the seed, slot 5 is the model. A dropped trailing null or a changed constant sails straight through. So these assert the whole envelope string.Verified, not assumed
Flipping
SURFACE_ID22 → 23 inflow_batch.py— exactly the kind of silent wire change that breaks Flow:That gap is what this closes.
Contents
tests/unit/test_flow_batch_golden.py— byte-for-byte locks onimage_request,video_request,text_video_request, plus a test thatvideo_requestpasses its model key through untouched (the property that lets one submit serve both Veo and Omni).uuid4is pinned to a counter; nothing else is mocked.requirements-dev.txt— test deps were undeclared, so a fresh checkout could not run the suite that guards any of this.New file rather than additions to
test_flow_batch.py, to stay clear of #40 and #42, which both touch that file.Test
The 3 failures are pre-existing on
main:test_processor.pypatchesagent.worker.processor._retry_state, which no longer exists. Unrelated to this PR, tracked separately.Run with
--ignore=tests/unit/test_result_handler.py --ignore=tests/unit/test_video_reviewer.py; both have pre-existing environmental problems (the former hangs on a real DB handle, the latter needs an ffmpeg built withdrawtext).🤖 Generated with Claude Code
https://claude.ai/code/session_01DB27wvPQ195KGiQ6VZfCDf