Greptile Summary

This PR adds the missing !existingReasoning branch to createOpenAIThinkingLevelWrapper so that thinkingLevel/thinkingDefault is actually forwarded to openai-codex and openai-responses endpoints when the pi-ai client never populated body.reasoning in the first place. The fix is correct and mirrors the existing normalizeProxyReasoningPayload pattern in proxy-stream-wrappers.ts.

• The existing test "does not add reasoning for non-reasoning models without existing reasoning payload" (line 82) uses openaiModel (api: "openai-responses", provider: "openai", no baseUrl), which resolves to supportsOpenAIReasoningCompatPayload: true — the same gate the fix passes through. This test will assert undefined but the fixed code will now produce { effort: "medium" }, causing a test failure.

Confidence Score: 4/5

The runtime fix is correct, but a pre-existing test contradicts the new behavior and will fail in CI.

One P1 finding: the test at line 82–88 is rendered incorrect by the new branch and will fail, since openaiModel without a baseUrl is reasoning-capable. The production code change itself is sound.

src/agents/pi-embedded-runner/openai-stream-wrappers.test.ts — the stale 'non-reasoning models' test at line 82 needs updating.

This is a comment left during a code review.
Path: src/agents/pi-embedded-runner/openai-stream-wrappers.test.ts
Line: 82-88

Comment:
**Stale test will fail after the fix**

`openaiModel` (`api: "openai-responses"`, `provider: "openai"`, no `baseUrl`) is a reasoning-capable model: `endpointClass` resolves to `"default"` (no configured base URL), so `usesExplicitProxyLikeEndpoint` is `false`, and `supportsOpenAIReasoningCompatPayload` evaluates to `true` in `provider-attribution.ts`. The new `if (existingReasoning === undefined || existingReasoning === null)` branch will now fire for it, injecting `{ effort: "medium" }` — so the assertion `expect(payloads[0]?.reasoning).toBeUndefined()` fails.

If this test is meant to cover a truly non-reasoning model (e.g. a proxy route with a custom `baseUrl`), the fixture needs to match. If it's meant to confirm that `openaiModel` without pre-existing reasoning also gets injection after this fix, it should be updated to `toEqual({ effort: "medium" })` with a descriptive rename.

```suggestion
  it("does not add reasoning for proxy-routed models without existing reasoning payload", () => {
    const { baseStreamFn, payloads } = createPayloadCapture();
    const wrapped = createOpenAIThinkingLevelWrapper(baseStreamFn, "medium");
    void wrapped(
      {
        api: "openai-responses",
        provider: "openai",
        id: "gpt-5.2",
        baseUrl: "https://proxy.example.com/v1",
      } as Model<"openai-responses">,
      { messages: [] },
      {},
    );

    expect(payloads[0]?.reasoning).toBeUndefined();
  });
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix(pi-embedded): inject body.reasoning ..." | Re-trigger Greptile}

Reporter here. Confirming the runtime change in openai-stream-wrappers.ts is exactly the fix — mirrors normalizeProxyReasoningPayload at proxy-stream-wrappers.ts:319 with the same shouldApplyOpenAIReasoningCompatibility gate.

Have had the equivalent manual patch active on the bundled proxy-stream-wrappers-*.js since v2026.4.21 (re-applied cleanly through the v2026.4.22 upgrade). Three agents running openai-codex/gpt-5.4 family with thinkingDefault high/medium/high: A/B on identical prompts confirms body.reasoning reaches chatgpt.com/backend-api/codex as { effort: "<level>" } with the branch and is absent without it. No regressions observed.

On the 3 checks-node-core-* failures: Greptile's diagnosis on openai-stream-wrappers.test.ts:82 is right. openaiModel has no baseUrl → endpointClass === "default" → supportsOpenAIReasoningCompatPayload === true, so the new branch now injects { effort: "medium" } and breaks toBeUndefined(). The test was passing only because the bug suppressed injection. Minimal fix: rename it to something like "proxy-routed models without prior reasoning" and add baseUrl: "https://proxy.example.com/v1" to the fixture so it covers a genuinely non-compat route. Happy to push the test update if that unblocks.

Thanks for the runtime A/B confirmation and the Greptile diagnosis — both spot-on. Pushed your suggested test fix in 4dbf49a: renamed to "proxy-routed models without prior reasoning" and added baseUrl: "https://proxy.example.com/v1" so the fixture exercises a genuine non-compat route. CI should go green on the next run.

Thanks for the follow-up and the runtime A/B notes. I dug through the production path and the upstream Pi layers before deciding this.

The normal embedded OpenAI/Codex path is already covered by the session-level reasoning path:

• src/agents/pi-embedded-runner/run/attempt.ts passes the selected thinkLevel into createAgentSession as thinkingLevel.
• @mariozechner/pi-agent-core converts non-off session thinking into options.reasoning for the stream call.
• @mariozechner/pi-ai then serializes that options.reasoning into the outgoing OpenAI/Codex Responses payload as reasoning.effort.

I added a maintainer-side regression contract on main in 0211280 (test: cover OpenAI thinking payload contract) that proves the path directly:

• OpenAI + Codex sessions with thinkingLevel: high forward options.reasoning: "high".
• streamSimpleOpenAIResponses serializes that to reasoning: { effort: "high", summary: "auto" }.
• streamSimpleOpenAICodexResponses serializes that to the same payload shape.
• thinkingLevel: off is pinned too: Codex leaves reasoning absent, OpenAI Responses emits the existing explicit disabled payload.

Verification on current main:

• pnpm test src/agents/openai-thinking-contract.test.ts => 8 passed
• pnpm check:changed => green before commit

Given that, I do not want to land this wrapper change as-is. It makes createOpenAIThinkingLevelWrapper compensate for a missing options.reasoning path that we still cannot reproduce in the normal embedded runtime, and it would make the wrapper responsible for papering over a lower-level contract break instead of exposing it.

If you can show the concrete production code path where a selected/default thinking level exists but pi-ai receives no options.reasoning, please open a fresh PR with that repro/trace or a failing test. That would be the right thing to fix. Closing this PR for now to avoid a speculative payload mutation.

Thanks for the detailed path trace and the regression contract in 0211280 — that's the right place to guarantee the behavior. Agreed the wrapper change would have hidden a lower-level contract break. Not reopening; will leave the wrapper alone and file a fresh PR only if I can produce a concrete repro on the embedded runtime path that reaches the wrapper with missing options.reasoning.