Skip to content

fix: review follow-ups for #76 — logging, wording, usage-shape test#79

Merged
yuanhao merged 1 commit into
mainfrom
fix/76-review-followups
Jul 21, 2026
Merged

fix: review follow-ups for #76 — logging, wording, usage-shape test#79
yuanhao merged 1 commit into
mainfrom
fix/76-review-followups

Conversation

@yuanhao

@yuanhao yuanhao commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Two-agent post-merge review of #77. Verdict: the guard is sound (vendored eventsource surfaces body close ONLY as StreamEnded; network drops go Transport→Network→retryable, so the clean-EOF is not over-broad), and the usage-after-finish_reason shape is captured — now proven by a test. Surviving findings applied:

  1. debug! on the DONE-less break + corrected comment (graceful None is unreachable for body close in this lib)
  2. warn! when non-empty tool-call args fail to parse (pre-existing silent unwrap_or({}) — truncated args no longer vanish traceless)
  3. CHANGELOG retry wording fixed: deliberate closes fail fast (Other, non-retryable); network drops retry
  4. New test: include_usage-shaped stream (usage chunk after finish_reason, no [DONE]) → usage captured

3/3 tests green, clippy clean.

🤖 Generated with Claude Code

- debug! on the DONE-less clean-EOF break (field-debugging aid for
  yoyo-evolve#612-class reports); comment corrected — this eventsource
  surfaces every body close as StreamEnded, never a graceful None
- warn! when a non-empty tool-call argument buffer fails to parse
  (pre-existing silent unwrap_or({}) fallback now leaves a trace)
- CHANGELOG: truncation 'retries' claim corrected (network drops retry;
  deliberate closes fail fast — StreamEnded classifies as Other)
- test: usage chunk after finish_reason + DONE-less close → usage
  captured (confirms include_usage shape survives the guard)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@yuanhao
yuanhao merged commit e5ff06a into main Jul 21, 2026
5 checks passed
@yuanhao
yuanhao deleted the fix/76-review-followups branch July 21, 2026 21:17
@yuanhao yuanhao mentioned this pull request Jul 21, 2026
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