Skip to content

fix(#6806): emit fallback ResultEvent when stream ends without result line - #6808

Open
fullsend-ai-coder[bot] wants to merge 2 commits into
mainfrom
agent/6806-metrics-fallback-stream-incomplete
Open

fix(#6806): emit fallback ResultEvent when stream ends without result line#6808
fullsend-ai-coder[bot] wants to merge 2 commits into
mainfrom
agent/6806-metrics-fallback-stream-incomplete

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Summary

When the Claude Code agent process terminates abnormally after doing work (e.g., killed by signal, exit code -1), parseClaudeStream now emits a fallback ResultEvent with accumulated per-turn token counts and turn count. Previously, metrics.json reported all-zero values because metrics relied entirely on the result NDJSON line emitted at the very end of the stream — if the process died before writing it, everything was lost.

Changes

  • internal/runtime/claude_progress.go: Track cumulative token counts across turns (input, output, cache read, cache creation) and turn count from message_start events. At EOF or read error, if no result event was seen and ≥1 turn occurred, emit a synthetic ResultEvent with Subtype: "stream_incomplete" and IsError: true.
  • internal/runtime/claude_progress_test.go: Added 4 test cases covering the fallback on incomplete stream (multi-turn), suppression when real result is present, progressParser metrics population, and fallback on read error (broken pipe).

Testing

  • All new tests pass (TestParseClaudeStreamFallbackResultOnIncompleteStream, TestParseClaudeStreamNoFallbackWhenResultPresent, TestProgressParserFallbackMetrics, TestParseClaudeStreamFallbackOnReadError)
  • Existing TestProgressParserNoResultEvent still passes (no message_start events → no fallback, consistent with legacy behavior)
  • Full internal/runtime/... test suite passes
  • go vet and go build pass
  • Patch coverage: parseClaudeStream at 90.4% (above 80% threshold)

Limitations

TotalCostUSD remains zero in the fallback — it cannot be derived from token counts without pricing data. Non-zero num_turns and token counts are the primary improvement.


Closes #6806

Post-script verification

  • Branch is not main/master (agent/6806-metrics-fallback-stream-incomplete)
  • Secret scan passed (gitleaks — 32ad401f884b3fe989d689bab02be30657dce380..HEAD)
  • PR body secret scan passed (gitleaks — no-git)

… line

When the Claude Code agent process terminates abnormally (e.g., killed
by a signal after writing output but before emitting its final result
NDJSON line), parseClaudeStream now emits a synthetic ResultEvent with
the best-available metrics accumulated from message_start and
message_delta events during the stream.

Previously, metrics relied entirely on the "result" event emitted at
the very end of the Claude Code NDJSON stream. If the process was
killed before that line was written, RunMetrics stayed at zero values
(num_turns: 0, all token counts: 0), producing a metrics.json with
zeroed data despite the agent having done substantial work.

The fix tracks per-turn token counts incrementally: each message_start
accumulates input and cache tokens, and each subsequent message_start
folds in the previous turn's output tokens. At EOF or read error, if
no real result event was seen and at least one turn was observed, a
fallback ResultEvent is emitted with subtype "stream_incomplete" and
IsError: true. When a real result event IS present, the fallback is
suppressed and the definitive values are used as before.

TotalCostUSD remains zero in the fallback because it cannot be derived
from token counts alone without pricing data. This is an acceptable
limitation — non-zero token counts and num_turns are the primary
improvement over the previous all-zeros behavior.

Closes #6806
@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner August 31, 2026 16:04
@fullsend-ai-coder fullsend-ai-coder Bot added the ready-for-review Agent PR ready for human review label Aug 31, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 31, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 4:05 PM UTC · Completed 4:22 PM UTC

Commit: 8a41828 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.72

@codecov

codecov Bot commented Aug 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@fullsend-ai-review fullsend-ai-review Bot added the risk/moderate PR risk: moderate label Aug 31, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 31, 2026

Copy link
Copy Markdown

Risk Assessment: moderate (2/5)

Details

Small, well-tested bot PR with good test coverage, but elevated git history signals (10 fix/revert commits in 90d and 4-author contention on these files) offset the otherwise low-risk metadata, yielding a moderate composite score consistent with the prior assessment.

Previous run

Risk Assessment: moderate (2/5)

Details

Small, well-tested bot PR with good test coverage, but elevated git history signals (high fix/revert churn and multi-author contention on these files) offset the otherwise low-risk metadata, yielding a moderate composite score.

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 31, 2026

Copy link
Copy Markdown

Review

Findings

Low

  • [edge-case] internal/runtime/claude_progress.go:145 — The fallback ResultEvent sets TotalCostUSD to zero because cost information is only available in the real result event. This is an inherent limitation acknowledged by the PR: when the agent crashes, the server-side cost tally is unavailable. Downstream consumers will show $0.00 for these runs, which is preferable to the prior behavior of showing nothing.

  • [naming-convention] internal/runtime/claude_progress.go:135 — The new cumulative token-tracking variables use a cumul prefix (cumulInput, cumulOutput, cumulCacheRead, cumulCacheWrite) that does not appear elsewhere in the codebase. The prefix is semantically meaningful — it disambiguates session-wide accumulators from the per-turn totalInput/totalOutput variables that get reset at each message_start. Defensible as-is.

  • [scope-alignment] internal/runtime/claude_progress.go — The change is tightly scoped to the Claude Code stream parser's EOF/error handling path. It does not touch sibling parsers or other subsystems. The scope directly matches the authorization in issue Review agent metrics zeroed when agent process exits with error after writing valid output #6806.

Previous run

Review

Findings

Low

  • [missing-error-message] internal/runtime/claude_progress.go:145 — 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.
    Remediation: Set ErrorMessage on the fallback ResultEvent to a descriptive string such as "stream ended without result event".

  • [naming-convention] internal/runtime/claude_progress.go:135 — 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.

  • [scope-alignment] internal/runtime/claude_progress.go:131 — 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.

  • [edge-case] internal/runtime/claude_progress.go:145 — 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.


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 4:24 PM UTC · Completed 4:30 PM UTC

Commit: 8a41828 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.23

Add ErrorMessage field to the synthetic ResultEvent emitted when the
Claude stream ends without a result line, matching the pattern used by
the real result handler and sibling parsers. Update fallback tests to
assert the new field.

Addresses review feedback on #6808
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 1 (bot-triggered)

Addressed the one actionable finding (missing-error-message) by adding ErrorMessage to the fallback ResultEvent. The other three findings were informational or positive observations requiring no code changes.

Fixed (1):

  1. missing-error-message: fallback ResultEvent does not set ErrorMessage (internal/runtime/claude_progress.go): Added ErrorMessage field set to "stream ended without result event" on the fallback ResultEvent, matching the pattern used by the real result handler and sibling parsers. Updated fallback tests to assert the new field.

Disagreed (3):

  1. naming-convention: cumul prefix not used elsewhere in codebase: The reviewer explicitly noted the naming is defensible as-is, stating the prefix disambiguates session-wide accumulators from per-turn variables. No change needed.
  2. scope-alignment: change is tightly scoped to Claude Code stream parser: This is a positive observation, not a request for change. The reviewer confirmed the scope is appropriate.
  3. edge-case: fallback ResultEvent sets TotalCostUSD to zero: The reviewer acknowledged this is a known constraint documented in the PR limitations section. Cost cannot be derived from token counts without pricing data.

Tests: passed

Next steps:

  • /fs-review — request a re-review of the changes
  • /fs-fix <your instruction> — run another fix pass with specific guidance
  • Push commits directly — review re-runs automatically on push
    Updated by fullsend fix agent

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 31, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 4:32 PM UTC · Completed 4:49 PM UTC

Commit: 3ad460e · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.30

@fullsend-ai-review
fullsend-ai-review Bot dismissed their stale review August 31, 2026 16:49

Superseded by updated review

@fullsend-ai-review fullsend-ai-review Bot added the ready-for-merge All reviewers approved — ready to merge label Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-merge All reviewers approved — ready to merge ready-for-review Agent PR ready for human review risk/moderate PR risk: moderate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Review agent metrics zeroed when agent process exits with error after writing valid output

0 participants