-
Notifications
You must be signed in to change notification settings - Fork 92
fix(#6806): emit fallback ResultEvent when stream ends without result line #6808
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -108,6 +108,13 @@ type resultEvent struct { | |
| // emits normalized AgentEvent values via the onEvent callback. It processes | ||
| // system events, stream_event deltas (thinking, text, tool input JSON), | ||
| // result events, errors, and assistant message fallback. | ||
| // | ||
| // When the stream ends (EOF or read error) without a "result" event and at | ||
| // least one conversation turn was observed, a fallback ResultEvent is emitted | ||
| // with the best-available token counts accumulated from message_start and | ||
| // message_delta events. This ensures metrics are non-zero when the agent | ||
| // process terminates abnormally after doing work but before emitting its | ||
| // final result line. See #6806. | ||
| func parseClaudeStream(r io.Reader, onEvent func(AgentEvent)) error { | ||
| br := bufio.NewReaderSize(r, streamBufSize) | ||
|
|
||
|
|
@@ -121,14 +128,44 @@ func parseClaudeStream(r io.Reader, onEvent func(AgentEvent)) error { | |
| totalCacheRead int | ||
| totalCacheWrite int | ||
| lastEmittedTotal int | ||
| // Cumulative tracking for fallback ResultEvent when the stream | ||
| // ends without a "result" line (agent crash / signal kill). | ||
| numTurns int | ||
| sawResult bool | ||
| cumulInput int | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] naming-convention The new cumulative token-tracking variables use a cumul prefix (cumulInput, cumulOutput, cumulCacheRead, cumulCacheWrite) that does not appear elsewhere in the codebase. However, the prefix is semantically meaningful — it disambiguates session-wide accumulators from the per-turn totalInput/totalOutput variables in the same scope that get reset at each message_start. The naming is defensible as-is. |
||
| cumulOutput int | ||
| cumulCacheRead int | ||
| cumulCacheWrite int | ||
| ) | ||
|
|
||
| // emitFallbackResult sends a synthetic ResultEvent from accumulated | ||
| // per-turn data when the real result event is missing. The last | ||
| // turn's output tokens must be folded in before calling. | ||
| emitFallbackResult := func() { | ||
| if sawResult || numTurns == 0 { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] missing-error-message The fallback ResultEvent does not set ErrorMessage, unlike the real result handler (which sets ErrorMessage from re.Result) and sibling parsers (parsePiStream, parseOpenCodeStream) which always set ErrorMessage on error results. Downstream consumers that check ErrorMessage would see an empty string for fallback results. Suggested fix: Set ErrorMessage on the fallback ResultEvent to a descriptive string such as "stream ended without result event". There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] edge-case The fallback ResultEvent sets TotalCostUSD to zero because cost information is only available in the real result event, not from intermediate message_start/message_delta events. The PR limitations section acknowledges this known constraint. |
||
| return | ||
| } | ||
| cumulOutput += totalOutput | ||
| onEvent(ResultEvent{ | ||
| NumTurns: numTurns, | ||
| IsError: true, | ||
| ErrorMessage: "stream ended without result event", | ||
| Subtype: "stream_incomplete", | ||
| InputTokens: cumulInput, | ||
| OutputTokens: cumulOutput, | ||
| CacheCreationInputTokens: cumulCacheWrite, | ||
| CacheReadInputTokens: cumulCacheRead, | ||
| }) | ||
| } | ||
|
|
||
| for { | ||
| line, isPrefix, err := br.ReadLine() | ||
| if err == io.EOF { | ||
| emitFallbackResult() | ||
| return nil | ||
| } | ||
| if err != nil { | ||
| emitFallbackResult() | ||
| return err | ||
| } | ||
| if isPrefix { | ||
|
|
@@ -236,10 +273,19 @@ func parseClaudeStream(r io.Reader, onEvent func(AgentEvent)) error { | |
| } `json:"message"` | ||
| } | ||
| if err := json.Unmarshal(wrapper.Event, &msg); err == nil { | ||
| // Accumulate previous turn's output before resetting. | ||
| cumulOutput += totalOutput | ||
|
|
||
| totalInput = msg.Message.Usage.InputTokens | ||
| totalOutput = 0 | ||
| totalCacheRead = msg.Message.Usage.CacheReadInputTokens | ||
| totalCacheWrite = msg.Message.Usage.CacheCreationInputTokens | ||
|
|
||
| // Accumulate this turn's input and cache tokens. | ||
| cumulInput += totalInput | ||
| cumulCacheRead += totalCacheRead | ||
| cumulCacheWrite += totalCacheWrite | ||
| numTurns++ | ||
| } | ||
|
|
||
| case "message_delta": | ||
|
|
@@ -264,6 +310,7 @@ func parseClaudeStream(r io.Reader, onEvent func(AgentEvent)) error { | |
| } | ||
|
|
||
| case "result": | ||
| sawResult = true | ||
| var re resultEvent | ||
| if err := json.Unmarshal(line, &re); err != nil { | ||
| continue | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[low] scope-alignment
The change is tightly scoped to the Claude Code stream parser and does not touch sibling parsers or other subsystems. Appropriate given the bug was observed in Claude Code agent runs.