Skip to content

Chat process follow-ups: a mid-stream error raises, .events survives partial reads, bad show_process chokes first - #470

Merged
rejojer merged 2 commits into
mainfrom
fix/chat-process-followups
Sep 3, 2026
Merged

Chat process follow-ups: a mid-stream error raises, .events survives partial reads, bad show_process chokes first#470
rejojer merged 2 commits into
mainfrom
fix/chat-process-followups

Conversation

@rejojer

@rejojer rejojer commented Sep 3, 2026

Copy link
Copy Markdown
Member

Review fixes for the chat-process feature (v0.2.13), re-applied on main. They sit on the review view #457 as d5110fb and 82b7558; chat() was reworked by #460 since, so the client.py hunks were merged by hand onto the current wording.

A mid-stream error chunk now raises. The managed endpoint reports a server-side failure as a final {"error": ...} chunk after the partial answer (it refunds the credits, then yields it). Neither SSE decoder in cloud_api looked at it, so chat(stream=True) and chat_completions(stream=True) in both modes ended as a short, apparently complete answer with no exception. One guard in each decoder raises PageIndexAPIError; the partial answer is still delivered first.

.events survives partial reads. ChatStream.events delegated with yield from, so a dropped handle (next(stream.events), for ... break) closed the shared run on GC and the rest of the run silently vanished. A plain loop leaves it alone.

Hidden call lines still label results. _weave recorded a call's arguments only past the tool_call visibility guard, so with call lines hidden the standalone [tool_result] lines never carried the arguments they promise.

A bad show_process chokes first. The value is validated before the stream check, so an invalid value is refused as such instead of being told to add stream=True and then refused again; the managed lane's duplicate choke goes with it.

Docs. stream: and the Returns block say show_process is on by default and show_process=False gives the bare answer; the show_process opener no longer says own-model only.

Verification: 475 passed; whole suite with openai-agents blocked 369 passed / 106 skipped; pyright client.py 0, package unchanged (236). The error-chunk test loops all three streaming surfaces and was red before the guard; removing either guard alone fails it.

https://claude.ai/code/session_01PYr9yG1FPQxKCA9m7ECQWY

…sults; bad show_process chokes first

- ChatStream.events delegated with `yield from`, so a dropped handle
  (next(stream.events), for ... break) closed the shared run on GC and
  the rest of the run silently vanished. A plain loop leaves it alone.
- _weave filled call_args only past the tool_call visibility guard, so
  with call lines hidden the standalone result lines never carried the
  arguments they promise.
- show_process is validated before the stream check: an invalid value
  is refused as such instead of being told to add stream=True and then
  refused again; the managed lane's duplicate choke goes with it.
- Docstring: show_process is not own-model-only.

Claude-Session: https://claude.ai/code/session_016M3qaQedSK7L4DwysFRmk2
…r; stream docstring says show_process is on by default

- The managed endpoint reports a server-side failure as a final
  {"error": ...} chunk after the partial answer (api.py refunds the
  credits, then yields it). Neither chunk decoder looked at it, so
  chat(stream=True) and chat_completions(stream=True) in both modes
  ended as an apparently complete short answer with no exception. One
  guard in each decoder raises PageIndexAPIError; the partial answer
  is still delivered first.
- The `stream:` arg and the Returns block still described the pre-PR
  contract (bare text chunks); only the show_process paragraph said it
  is on by default.

Claude-Session: https://claude.ai/code/session_01PYr9yG1FPQxKCA9m7ECQWY
@rejojer
rejojer merged commit 564bc97 into main Sep 3, 2026
9 checks passed
@rejojer
rejojer deleted the fix/chat-process-followups branch September 3, 2026 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant