feat: stream advisor tool output by ThomasK33 · Pull Request #25032 · coder/coder

github-actions

Well-structured cross-cutting streaming feature. The callback chain from nested chatloop through runner/tool/chatd to the websocket is clean, each layer adds exactly one piece of context, and the persistence boundary is correctly maintained (deltas are transient, final result is authoritative). Test coverage spans every layer: unit, integration, visual, and a negative persistence assertion. The generic result_delta / isStreaming infrastructure is reusable if other tools adopt streaming later.

2 P3, 2 P4, 5 Nit. The P3s are about non-happy-path streaming behavior: retry-induced duplicate deltas and missing test coverage for the error-after-partial-deltas path. Everything else is cosmetic or observational.

"'RunAdvisorOptions configures one advisor invocation.' is the kind of comment that would make me flag '// GetUser gets a user' on a hospital chart." (Leorio)

codersdk/chats.go:321-323

Nit [DEREM-6] The StripInternal comment says ResultDelta is produced by "processStepStream in chatloop," but this PR now produces ResultDelta via PublishAdviceDelta in chatd.go:7063. Before this PR the field was never populated so the reference was harmlessly wrong; now it will send someone looking in the wrong file.

Something like: (see processStepStream in chatloop for ArgsDelta, PublishAdviceDelta in chatd for ResultDelta). (Gon)

🤖

🤖 This review was automatically generated with Coder Agents.