Add batch_id to explode parameters - #451
Merged
Merged
Conversation
`explode` built its params inline and did not serialize `batch_id`, so a client-supplied value never reached the wire. The server accepts it on this endpoint (`before_action :capture_notification_batch_id` covers every UploadController action) and needs it to partition the completion notification into a pollable batch: the poll destination reads the key from ThreadContext, which is populated only from a client value. Without one the notification is enqueued unpartitioned and cannot be polled, even though the response and the payload both echo a server-minted `batch_id`. Co-Authored-By: Claude Opus 5 <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
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.
Problem
uploader.explodebuilds its params inline and does not serializebatch_id, so a client-supplied value never reaches the wire:This matters because
batch_idis what makes the completion notification pollable.explodeis unconditionally async — it always enqueues a job and returns{"status": "processing", …}— so polling is the only way to observe completion.Why the response
batch_idisn't a substituteThe response and the notification payload both echo a
batch_ideven when the client sends none, but that value is not a poll partition key. The poll destination reads the key fromThreadContext, which is populated only from a client-supplied value; the server-minted fallback feeds the response body and payload via a different path. With no client value the notification is enqueued unpartitioned and cannot be polled.Verification
Live against a real product environment, with a
poll://*/explodetrigger in place.Before — three attempts, including one with
notification_url='poll://*'passed explicitly, all timed out with an empty buffer despite the job succeeding (3 derived pages produced):After:
upload --waiton the same cloud succeeded throughout, confirming the poll transport itself was healthy and the gap was specific toexplode.Tests
Adds
test_explode_parameters— there was noexplodecoverage previously. It assertspublic_id,format,notification_urlandbatch_idall reach the request params, and fails withKeyError: 'batch_id'without this change.Only
batch_idis added: the server ignoresasyncon this endpoint sinceexplodeis always asynchronous.🤖 Generated with Claude Code