Skip to content

Add timeout to explain summary generation - #876

Merged
Soph merged 5 commits into
mainfrom
fix/explain-summary-timeout
Apr 10, 2026
Merged

Add timeout to explain summary generation#876
Soph merged 5 commits into
mainfrom
fix/explain-summary-timeout

Conversation

@peyton-alt

@peyton-alt peyton-alt commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a timeout to checkpoint summary generation for entire explain --generate so the command cannot hang indefinitely when the Claude CLI stalls.

What Changed

  • scopes the 30 second timeout to explain --generate in explain.go
  • keeps summarize.GenerateFromTranscript generic for other call sites
  • preserves shorter parent deadlines and clamps longer parent deadlines to the 30 second safety timeout
  • improves Claude CLI cancellation and deadline handling so context cancellation surfaces cleanly
  • prints Generating checkpoint summary... while explain --generate is waiting
  • adds unit coverage for default timeout, shorter parent deadline, longer parent deadline, and cancellation behavior

Why

A stalled Claude summary generation path currently leaves users waiting without a bounded failure mode. This change keeps the fix narrowly scoped to explain --generate instead of changing shared summarize behavior for other callers.

Current state before this fix:
entire explain --generate
^Cfailed to generate summary: failed to generate summary: claude CLI failed (exit -1):

Verification

  • go test ./cmd/entire/cli -run 'TestGenerateCheckpointAISummary|TestExplainCmd_RejectsPositionalArgs'
  • go test ./cmd/entire/cli/summarize
  • mise run lint
  • mise run test

Copilot AI review requested due to automatic review settings April 8, 2026 19:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a default timeout around AI summary generation to prevent the explain --generate (and other shared summarization call sites) from hanging indefinitely.

Changes:

  • Add a 30s context timeout wrapper inside summarize.GenerateFromTranscript.
  • Improve Claude CLI generator error handling to surface context cancellation/timeouts consistently.
  • Update explain summary generation to emit a user-facing “Generating…” message and add a unit test for timeout behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
cmd/entire/cli/summarize/summarize.go Adds a default timeout and timeout-specific error handling around generator execution.
cmd/entire/cli/summarize/generator_config_test.go Adds a new unit test intended to validate timeout behavior.
cmd/entire/cli/summarize/claude.go Returns context.DeadlineExceeded / context.Canceled when the context ends during Claude CLI execution.
cmd/entire/cli/explain.go Wires through an error writer and prints progress text while generating a checkpoint summary.

Comment thread cmd/entire/cli/summarize/summarize.go Outdated
Comment thread cmd/entire/cli/summarize/summarize.go Outdated
Comment thread cmd/entire/cli/summarize/generator_config_test.go Outdated
Comment thread cmd/entire/cli/summarize/generator_config_test.go Outdated
Comment thread cmd/entire/cli/summarize/summarize.go Outdated
@peyton-alt

Copy link
Copy Markdown
Contributor Author

@BugBot review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 1b2851e. Configure here.

Comment thread cmd/entire/cli/explain.go
@peyton-alt
peyton-alt marked this pull request as ready for review April 8, 2026 21:57
@peyton-alt
peyton-alt requested a review from a team as a code owner April 8, 2026 21:57
@peyton-alt
peyton-alt force-pushed the fix/explain-summary-timeout branch from 66c2071 to 5c18bb6 Compare April 10, 2026 04:46
@Soph
Soph enabled auto-merge April 10, 2026 11:31
@Soph
Soph merged commit 314282a into main Apr 10, 2026
9 checks passed
@Soph
Soph deleted the fix/explain-summary-timeout branch April 10, 2026 11:34
peyton-alt added a commit that referenced this pull request May 14, 2026
Two paired user-facing improvements to `entire explain --generate`:

1. Live progress on TTY. summaryProgressWriter renders agent.GenerationProgress
   events from the streaming generator as provider-neutral phase lines:
   "Sending request to provider...", "Provider responded (TTFT Xs, Nk
   cached input tokens)", "Writing summary... (~Nk tokens)", "Summary
   generated (Ds, N output tokens)". In-place line updates on TTY; one
   line per event in non-TTY/CI. ACCESSIBLE=1 swaps the styled glyphs
   (→ / ✓) for ASCII (-> / *) and suppresses ANSI escapes.

2. Observable-state timeout diagnostic. When --summary-timeout-seconds
   fires, the error message is built from captured subprocess state, not
   a generic three-cause list. Streaming providers (Claude) get phase-
   specific attribution: "never sent its request" / "received no response"
   / "responded but did not finish". Non-streaming providers (Codex/
   Cursor/Copilot/Gemini) surface captured stderr and distinguish
   "produced no output" from "was generating output when killed".

The three-cause shrug message from PR #876 is removed. Adds
summaryAttempt as the diagnostic state value type (separate from the UI
writer), providerCLIName for binary-name lookup, and *agent.TextGenerationError
for the non-streaming provider stderr-capture path through
RunIsolatedTextGeneratorCLI.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
peyton-alt added a commit that referenced this pull request May 20, 2026
Two paired user-facing improvements to `entire explain --generate`:

1. Live progress on TTY. summaryProgressWriter renders agent.GenerationProgress
   events from the streaming generator as provider-neutral phase lines:
   "Sending request to provider...", "Provider responded (TTFT Xs, Nk
   cached input tokens)", "Writing summary... (~Nk tokens)", "Summary
   generated (Ds, N output tokens)". In-place line updates on TTY; one
   line per event in non-TTY/CI. ACCESSIBLE=1 swaps the styled glyphs
   (→ / ✓) for ASCII (-> / *) and suppresses ANSI escapes.

2. Observable-state timeout diagnostic. When --summary-timeout-seconds
   fires, the error message is built from captured subprocess state, not
   a generic three-cause list. Streaming providers (Claude) get phase-
   specific attribution: "never sent its request" / "received no response"
   / "responded but did not finish". Non-streaming providers (Codex/
   Cursor/Copilot/Gemini) surface captured stderr and distinguish
   "produced no output" from "was generating output when killed".

The three-cause shrug message from PR #876 is removed. Adds
summaryAttempt as the diagnostic state value type (separate from the UI
writer), providerCLIName for binary-name lookup, and *agent.TextGenerationError
for the non-streaming provider stderr-capture path through
RunIsolatedTextGeneratorCLI.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
peyton-alt added a commit that referenced this pull request May 20, 2026
Two paired user-facing improvements to `entire explain --generate`:

1. Live progress on TTY. summaryProgressWriter renders agent.GenerationProgress
   events from the streaming generator as provider-neutral phase lines:
   "Sending request to provider...", "Provider responded (TTFT Xs, Nk
   cached input tokens)", "Writing summary... (~Nk tokens)", "Summary
   generated (Ds, N output tokens)". In-place line updates on TTY; one
   line per event in non-TTY/CI. ACCESSIBLE=1 swaps the styled glyphs
   (→ / ✓) for ASCII (-> / *) and suppresses ANSI escapes.

2. Observable-state timeout diagnostic. When --summary-timeout-seconds
   fires, the error message is built from captured subprocess state, not
   a generic three-cause list. Streaming providers (Claude) get phase-
   specific attribution: "never sent its request" / "received no response"
   / "responded but did not finish". Non-streaming providers (Codex/
   Cursor/Copilot/Gemini) surface captured stderr and distinguish
   "produced no output" from "was generating output when killed".

The three-cause shrug message from PR #876 is removed. Adds
summaryAttempt as the diagnostic state value type (separate from the UI
writer), providerCLIName for binary-name lookup, and *agent.TextGenerationError
for the non-streaming provider stderr-capture path through
RunIsolatedTextGeneratorCLI.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
peyton-alt added a commit that referenced this pull request Jul 21, 2026
Two paired user-facing improvements to `entire explain --generate`:

1. Live progress on TTY. summaryProgressWriter renders agent.GenerationProgress
   events from the streaming generator as provider-neutral phase lines:
   "Sending request to provider...", "Provider responded (TTFT Xs, Nk
   cached input tokens)", "Writing summary... (~Nk tokens)", "Summary
   generated (Ds, N output tokens)". In-place line updates on TTY; one
   line per event in non-TTY/CI. ACCESSIBLE=1 swaps the styled glyphs
   (→ / ✓) for ASCII (-> / *) and suppresses ANSI escapes.

2. Observable-state timeout diagnostic. When --summary-timeout-seconds
   fires, the error message is built from captured subprocess state, not
   a generic three-cause list. Streaming providers (Claude) get phase-
   specific attribution: "never sent its request" / "received no response"
   / "responded but did not finish". Non-streaming providers (Codex/
   Cursor/Copilot/Gemini) surface captured stderr and distinguish
   "produced no output" from "was generating output when killed".

The three-cause shrug message from PR #876 is removed. Adds
summaryAttempt as the diagnostic state value type (separate from the UI
writer), providerCLIName for binary-name lookup, and *agent.TextGenerationError
for the non-streaming provider stderr-capture path through
RunIsolatedTextGeneratorCLI.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants