chore: replace ResponsesNewParamsWrapper with ResponsesRequestPayload in responses interceptor by pawbana · Pull Request #213 · coder/aibridge

pawbana · 2026-03-12T12:25:27Z

Refactored the responses interceptor replacing ResponsesNewParamsWrapper with a new ResponsesRequestPayload type that wraps raw JSON bytes and consolidates operations made on request payload.

pawbana · 2026-03-12T12:25:42Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

dannykopping · 2026-03-16T09:09:31Z

intercept/responses/reqpayload.go

+
+	existing, err := p.existingToolItems()
+	if err != nil {
+		return p, err


Please wrap errors.

dannykopping · 2026-03-16T09:13:29Z

intercept/responses/reqpayload_test.go

+			raw:    []byte(`{"model":123,"stream":"yes","input":42}`),
+			want:   `{"model":123,"stream":"yes","input":42}`,
+			model:  "123",
+			stream: false,


I understand that this is a consequence of not unmarshaling the payload, but this feels like an odd behaviour to test for without explicitly documenting this shortcoming of the new approach.

It won't matter once we move towards a reverse-proxy model, though.

Where should it be documented?

Even just documenting it here is fine. I just don't want us to mistake "this is how it works" for "this is how it should work".

dannykopping · 2026-03-16T09:15:51Z

intercept/responses/reqpayload_test.go

+			if tc.wantSame {
+				require.Equal(t, p, updated)
+			}


Nit: you could drop this and just specify wantNames.

I've left it as is since it is a bit more stronger check as it checks whole JSON instead of just ".tools" field.

dannykopping · 2026-03-16T09:17:30Z

intercept/responses/reqpayload_test.go

+				assert.Equal(t, p, updated)
+			}
+			if tc.wantFlagSet {
+				assert.False(t, gjson.GetBytes(updated, "parallel_tool_calls").Bool())


We should have a helper for this in the ResponsesRequestPayload type rather than scattering business logic around like this.

dannykopping · 2026-03-16T09:18:19Z

intercept/responses/reqpayload_test.go

+		wantFlagSet bool
+	}{
+		{
+			name:        "sets flag when tools exist",


It's more complicated than this now.

We only disable parallel tool calls when injected tools are defined, regardless of whether any client-issued tools exist.

dannykopping · 2026-03-16T09:20:49Z

intercept/responses/reqpayload.go

+	return p.set(reqPathParallelToolCalls, false)
+}
+
+func (p ResponsesRequestPayload) withAppendedInputItems(items []responses.ResponseInputItemUnionParam) (ResponsesRequestPayload, error) {


Nit: I'd prefer this were named as setters, which is what they are.

The "withXXX" pattern is usually used with the options pattern.

dannykopping · 2026-03-16T09:22:07Z

intercept/responses/reqpayload_test.go

+			name:     "unsupported input shape errors during rewrite",
+			raw:      []byte(`{"model":"gpt-4o","input":123}`),
+			items:    []responses.ResponseInputItemUnionParam{responses.ResponseInputItemParamOfFunctionCallOutput("call_123", "done")},
+			wantErr:  "unsupported input type",


This error is not going to be useful to us or operators. We should include the field name and the type of the field plus the type of the incoming value.

Updated error messages and the tests to check full error message instead of just a fragment of it.

dannykopping · 2026-03-16T09:23:17Z

intercept/responses/reqpayload_test.go

+				require.NoError(t, err)
+			}
+
+			require.True(t, json.Valid(updated))


These are not necessary in the tests since NewResponsesRequestPayload already rejects invalid input.

dannykopping · 2026-03-16T09:26:34Z

intercept/responses/injected_tools.go

+
+	// Disable parallel tool calls after injecting tools, so the check for
+	// existing tools in the payload sees the newly injected ones.
+	i.disableParallelToolCalls()


I didn't notice this when I refactored the parallel tool calls logic, but disableParallelToolCalls is now being called twice in this func.

Thanks, I was supposed to fix it but it got lost. I'll need to double check if it works after rebase.

dannykopping · 2026-03-16T09:28:58Z

intercept/responses/reqpayload.go

+		return nil, fmt.Errorf("responses empty request body")
+	}
+	if !json.Valid(raw) {
+		return nil, fmt.Errorf("responses invalid JSON request body")


Nit: this reads a bit strangely.

Suggested change

return nil, fmt.Errorf("responses invalid JSON request body")

return nil, fmt.Errorf("invalid JSON payload")

I think I changed it to match copilot tests

aibridge/provider/copilot_test.go

Line 129 in b3cd38b

assert.Contains(t, err.Error(), "unmarshal chat completions request body")

aibridge/provider/copilot_test.go

Line 220 in b3cd38b

assert.Contains(t, err.Error(), "unmarshal responses request body")

but yeah, it sound a bit weird. Changed.

… in responses interceptor

…oad_test.go

ssncferreira

LGTM 🧹 nice refactor! Just a few nits

intercept/responses/base.go

intercept/responses/reqpayload.go

intercept/responses/reqpayload_test.go

pawbana · 2026-03-18T18:29:43Z

Merge activity

Mar 18, 6:29 PM UTC: A user started a stack merge that includes this pull request via Graphite.
Mar 18, 6:29 PM UTC: @pawbana merged this pull request with Graphite.

pawbana force-pushed the pb/responses-remove-paramswrap-blocking branch 2 times, most recently from 5c4e7b0 to 7465ac6 Compare March 12, 2026 16:34

pawbana mentioned this pull request Mar 12, 2026

chore: move lastUserPrompt and correlatingToolCallID to ResponsesRequestPayload #215

Merged

pawbana force-pushed the pb/responses-remove-paramswrap-blocking branch 5 times, most recently from 11aeeb0 to be0adac Compare March 13, 2026 08:53

pawbana marked this pull request as ready for review March 13, 2026 08:53

pawbana requested review from dannykopping and ssncferreira March 13, 2026 08:54

pawbana force-pushed the pb/responses-remove-paramswrap-blocking branch from be0adac to 809aae2 Compare March 13, 2026 15:25

dannykopping reviewed Mar 16, 2026

View reviewed changes

pawbana force-pushed the pb/responses-remove-paramswrap-blocking branch from 809aae2 to 93d2b9c Compare March 16, 2026 14:51

pawbana added 3 commits March 16, 2026 15:18

chore: replace ResponsesNewParamsWrapper with ResponsesRequestPayload…

306119d

… in responses interceptor

review 1

248963e

rebase + review: fix disableParallelToolCalls + test names in reqpayl…

5f14de7

…oad_test.go

pawbana force-pushed the pb/responses-remove-paramswrap-blocking branch from 93d2b9c to 5f14de7 Compare March 16, 2026 15:29

pawbana mentioned this pull request Mar 16, 2026

chore: remove SDK struct usage from requests in messages API #218

Open

ssncferreira approved these changes Mar 17, 2026

View reviewed changes

review 2

f7cd9d1

pawbana merged commit 1937653 into main Mar 18, 2026
5 checks passed

This was referenced Mar 19, 2026

provider/openai: migrate responses route from json.Unmarshal to json.NewDecoder #204

Closed

fix: remove unsupported fields from requests to Bedrock #225

Open

fix: filter unsupported Anthropic-Beta headers from Bedrock #226

Open

	return nil, fmt.Errorf("responses invalid JSON request body")
	return nil, fmt.Errorf("invalid JSON payload")

Conversation

pawbana commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pawbana commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ssncferreira left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pawbana commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pawbana commented Mar 12, 2026 •

edited

Loading

pawbana commented Mar 12, 2026 •

edited

Loading

pawbana commented Mar 18, 2026 •

edited

Loading