test: end-to-end proxy test for chunked save → chain serve - #90
Merged
Conversation
Closes a real test gap surfaced after PR #88 merged: nothing proved that a response saved via the chunked staging protocol (POST /staging → PUT /staging/{sid}/chunks/{k} → PUT /staging/{sid}/complete) was correctly reassembled into a chain entry that the proxy could serve back as part of a subsequent turn's flat context. Existing coverage: - cmd/proxy/chunk_test.go: unit-level chunk→chunked byte round-trip via recordingBackend. - internal/chainstore/streaming_test.go: server-side chunk reassembly via AppendChunk/Complete/Retrieve. - cmd/proxy/backend_routing_test.go: wire-pattern pinning via routingRecorder (counts paths, doesn't inspect bytes). - cmd/proxy/handler_test.go::TestStoreTrueContinuation: continuation succeeds but doesn't verify the inference call received the prior turn's output. What this adds: - cmd/proxy/inference/mock.go: MockServer gains a RequestBodies() accessor that snapshots every POST /responses body in arrival order. Tests use this to inspect what the proxy actually forwarded to the inference backend. - cmd/proxy/chunked_roundtrip_test.go::TestChunkedRoundtripReassemblesIntoChain: saves an anchor turn with maxChunkBytes=64 (forces ≥2 AppendChunk), asserts the wire pattern (≥2 chunks, 1 complete, 0 abort), issues a continuation via previous_response_id, and verifies the continuation's inference call received the anchor's response output (msg_ok, role=assistant) in its input array — the strongest end-to-end proof that chunks → chain → served-context is intact.
Address the 3 Important findings from the PR #90 review: 1. recordBody call-order dependency — added a comment in handle() above recordAndReplaceBody explaining that it MUST run before any JSON decode of r.Body, and what would break if a future change decodes first. 2. Silent io.ReadAll error → r.Body left in unknown state. Reworked readAndRecord into recordAndReplaceBody that always closes the original r.Body and re-installs io.NopCloser(bytes.NewReader(body)) with what was read. On a read error the body may be partial or empty — but the downstream decoder now sees a deterministic payload rather than re-draining an unknown state. 3. RequestBodies() shallow-copied slice headers. Now deep-copies each body via append([]byte(nil), b...) so callers may mutate the returned slices without affecting each other or future snapshots. Also renamed readAndRecord → recordAndReplaceBody to match the new contract (it now always replaces r.Body, not just records).
Address the 4 Minor findings from the PR #90 review: 4. Wire-pattern assertions (steps 1+2) duplicated TestBufferedProxyMultipleChunks. Dropped. The wire pattern is pinned by that existing test — this test focuses on the unique chain-reassembly property (anchor's response output appears in the continuation's inference input). 5. With the wire-pattern assertions gone, no need for routingRecorder. Switched from newRoutingStack to newTestStack — drops the recorder import and aligns with how backend_routing_test.go and other proxy tests set up the inference-side stack. 6. decodeJSON closes the body, so calling it before require.Equal on StatusCode swallows the status assertion on non-200 responses. Swapped the order in both anchor and continuation turns — status check now runs first, with a self-diagnosing message. 7. Loose []map[string]interface{} replaced with a local inputItem struct that decodes only the fields the assertions check. No equivalent type exists in inference/types.go (Response / SSEEvent / UsageInfo are server-side, not input items), so the local definition is justified; typed access also documents the shape under test.
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.
Closes a test gap surfaced after PR #88 merged: nothing proved that a response saved via the chunked staging protocol (POST /staging → PUT /staging/{sid}/chunks/{k} → PUT /staging/{sid}/complete) was correctly reassembled into a chain entry that the proxy could serve back as part of a subsequent turn's flat context.
What this PR adds
cmd/proxy/inference/mock.go—MockServergains aRequestBodies()accessor that snapshots every POST /responses body in arrival order. Tests use this to inspect what the proxy actually forwarded to the inference backend.cmd/proxy/chunked_roundtrip_test.go—TestChunkedRoundtripReassemblesIntoChainsaves an anchor turn withmaxChunkBytes=64(forces ≥2AppendChunk), asserts the wire pattern (≥2 chunks, 1 complete, 0 abort), issues a continuation viaprevious_response_id, and verifies the continuation's inference call received the anchor's response output (msg_ok, role=assistant) in its input array. That is the strongest end-to-end proof that chunks → chain → served-context is intact.Existing coverage this complements
chunk_test.gointernal/chainstore/streaming_test.gobackend_routing_test.gohandler_test.go::TestStoreTrueContinuationmake presubmit is green.