Skip to content

fix(cli): show request progress while streaming-capable operations wait for a response - #138

Open
TristanSpeakEasy wants to merge 3 commits into
mainfrom
fix/cli-progress-on-streaming-operations
Open

TristanSpeakEasy wants to merge 3 commits into
mainfrom
fix/cli-progress-on-streaming-operations

Conversation

@TristanSpeakEasy

@TristanSpeakEasy TristanSpeakEasy commented Oct 23, 2026 •

Copy link
Copy Markdown
Member

Why

Generated CLIs show a short-delay spinner on stderr while a request is in flight on an interactive terminal. Operations that declare a streaming response (text/event-stream or JSONL) never started it, because the operation command takes a separate streaming branch. That branch also serves the case where the response comes back as a single JSON body (for example when the request opts out of streaming), so slow model-style calls printed nothing at all until the first byte arrived.

What changed

  • The streaming branch in opcmd.go.stmpl (operation and intent run functions) starts output.StartRequestProgress before the SDK call, unless the operation has a deferred body. All the existing suppression still applies: machine output modes, --jq, agent mode, --debug, --dry-run, non-interactive and non-TTY output.
  • output.StreamResult takes an optional stop function and calls it before writing anything: when the first event arrives, when the stream ends empty, or before it falls back to rendering a single non-streamed result. Errors stop the spinner before they are rendered.
  • Two PTY tests in request_progress_test.go: the spinner is shown while waiting for the first event and cleared before it is printed, and it is cleared before a non-streamed fallback result.
  • Review SDK regenerated; changeset added.

Testing

  • TARGET=review make test-cli: passes, including the new tests.
  • make check-template-cli and make lint: pass.

Public-safety check

  • This change contains no credentials, customer documents, private repository URLs, private filesystem paths, or unredacted private logs.
  • Title, body, comments, and commit messages name no customers or customer-derived identifiers, private paths or trackers, or workflow provenance, and are understandable without private context (.claude/skills/public-repo-communication/SKILL.md).
  • Generated fixtures and review SDK changes are public-safe.
  • I reviewed git diff --check.

Summary by cubic

Shows the request progress spinner in generated CLIs while streaming-capable operations wait for their first response.

Previously, streaming operations took a separate code path that never started the spinner, so slow calls printed nothing until the first byte arrived. The spinner now starts before the SDK call and clears before the first event is written, when the stream ends empty, or when the response falls back to a single non-streamed result. All existing suppression still applies, and operations with deferred bodies are excluded.

  • StreamResult now accepts an optional stop function that runs before any output is written.
  • Adds PTY tests, generated only for APIs with streaming operations, covering the spinner clearing before the first event and before a non-streamed fallback result.
  • Regenerates the review SDK and adds a changeset.

Written for commit 4da64cb. Summary will update on new commits.

View guided diff Turn on auto-fix

@TristanSpeakEasy
TristanSpeakEasy requested a review from a team as a code owner October 23, 2026 00:42

@cubic-dev-ai cubic-dev-ai 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.

All reported issues were addressed across 11 files

Shadow auto-approve: would not auto-approve because issues were found.

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread templates/templates/cli/opcmd.go.stmpl Outdated
Comment thread templates/templates/cli/request_progress_test.go.stmpl Outdated

@cubic-dev-ai cubic-dev-ai 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.

0 issues found across 1 file (changes from recent commits).

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Shadow auto-approve: would not auto-approve. Auto-approval blocked by 2 unresolved issues from previous reviews.

View guided diff | Turn on auto-fix | Re-trigger cubic

@cubic-dev-ai cubic-dev-ai 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.

0 issues found across 8 files (changes from recent commits).

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Shadow auto-approve: would auto-approve. Starts the existing stderr progress spinner on streaming CLI branches and clears it before first event/fallback/empty stream/error, with PTY tests. Bounded, clearly beneficial UX bug fix; suppression and dry-run paths unchanged.

View guided diff | Turn on auto-fix | Re-trigger cubic

@AshGodfrey AshGodfrey 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.

Verified on a regenerated CLI: the spinner shows on streaming, non-streamed and background-style operations, clears before the first byte or error, and stays off under every machine-output and non-TTY condition.

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.

2 participants