This is a comment left during a code review.
Path: src/agents/pi-embedded-runner/extra-params.ts
Line: 184:185

Comment:
**Dead `enabled` parameter**
The `enabled` parameter is always `true` at the only call site (line 251), since the caller already checks `toolStreamEnabled` before invoking the wrapper. The `if (!enabled)` branch (lines 191-193) is unreachable dead code. Consider removing the parameter and the early-return branch to simplify the function, or alternatively move the `toolStreamEnabled` check inside the wrapper (similar to how `createOpenAIResponsesStoreWrapper` handles its `shouldForceResponsesStore` check internally).

```suggestion
function createZaiToolStreamWrapper(
  baseStreamFn: StreamFn | undefined,
): StreamFn {
```

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

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

This is a comment left during a code review.
Path: src/agents/pi-embedded-runner/extra-params.zai-tool-stream.test.ts
Line: 15:44

Comment:
**Tests don't verify payload mutation**
This test declares `capturedPayloads` (line 16) but never uses it. More importantly, none of the three tests actually invoke the wrapped `streamFn` and verify that `tool_stream: true` gets injected into the payload. Compare with the existing e2e tests in `pi-embedded-runner-extraparams.e2e.test.ts`, which call `agent.streamFn(...)` and then assert on the mutated payload (e.g., `expect(payload.store).toBe(true)`).

As written, the first test only checks `agent.streamFn !== mockStreamFn` (which is true simply because `createOpenAIResponsesStoreWrapper` always wraps at the end), and the third test only checks `agent.streamFn` is defined. Consider invoking the wrapped function and verifying the payload, e.g.:

```ts
const payload: Record<string, unknown> = { model: "glm-5" };
const baseStreamFn: StreamFn = (_model, _context, options) => {
  options?.onPayload?.(payload);
  return new AssistantMessageEventStream();
};
const agent = { streamFn: baseStreamFn };
applyExtraParamsToAgent(agent, cfg as any, "zai", "glm-5");
void agent.streamFn?.(model, { messages: [] }, {});
expect(payload.tool_stream).toBe(true);
```

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

This is a comment left during a code review.
Path: src/agents/pi-embedded-runner/extra-params.zai-tool-stream.test.ts
Line: 3:3

Comment:
**Unused type imports**
`Context` and `Model` are imported but never used in any of the test cases.

```suggestion
import type { StreamFn } from "@mariozechner/pi-agent-core";
```

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

This is a comment left during a code review.
Path: src/agents/pi-embedded-runner/extra-params.zai-tool-stream.test.ts
Line: 84:86

Comment:
**Misleading test comment**
The comment says "The tool_stream wrapper should be applied but with enabled=false" but this is incorrect. When `tool_stream: false` is in the config, the wrapper is never applied at all (the `if (toolStreamEnabled)` check on line 249 of `extra-params.ts` prevents it). The test should verify that `tool_stream` is *not* injected into the payload when disabled, not just that `streamFn` is defined (which it always will be due to the OpenAI Responses wrapper).

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