Remove the batch --stream flag#30
Merged
Merged
Conversation
The --stream flag on `batch verify` and `batch get` never earned its place. For an already-completed batch — the normal case when you `get` one — the completed API payload drops the top-level total/processed counts, so the progress gate never fired and the command emitted a single `{"event":"complete",...,"emails":[...]}` line. That's just `--json` wrapped in an envelope, not streaming.
There's nothing to stream for a completed batch: the API returns the whole `emails` array in one response. And the NDJSON-rows shape that would have been useful is already achievable with the built-in `--jq`:
emailable batch get <id> --jq '.emails[]'
Since --stream is unreleased, remove it rather than redefine it. This drops batchStreamer, the emit* helpers, newStreamerIfEnabled, and applyStreamImplications, and simplifies waitForCompletion back to a plain poller whose progress bar writes to stderr. README and the emailable skill now document the `--jq '.emails[]'` recipe for NDJSON output.
There was a problem hiding this comment.
Pull request overview
This PR removes the unreleased --stream flag from batch verify / batch get, deleting the NDJSON “event streaming” implementation and simplifying batch polling back to a plain --wait workflow, while documenting --jq '.emails[]' as the preferred NDJSON row output approach.
Changes:
- Removed the
--streamflag and associated streaming/event-emission code paths from batch commands. - Simplified
waitForCompletionto a non-streaming poller (UI progress on stderr when not in JSON mode). - Updated README and the Emailable skill docs to recommend NDJSON row output via
--jqon the completed batch payload.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| skills/emailable/SKILL.md | Removes --stream guidance; documents NDJSON row output via --jq '.emails[]'. |
| README.md | Removes --stream docs and replaces “NDJSON streaming” section with the --jq '.emails[]' recipe. |
| cmd/jq_test.go | Removes tests specific to filtering --stream event envelopes. |
| cmd/batch.go | Deletes --stream flag handling and streamer helpers; simplifies waitForCompletion signature/logic. |
| cmd/batch_e2e_test.go | Removes end-to-end and helper/unit tests that validated --stream behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- `batch get --wait` now suppresses the polling UI under --quiet, matching `batch verify` (the --quiet help promises to suppress progress). - README and skill NDJSON recipes pair `.emails[]` with --wait, since a still-verifying batch has no `emails` field and `.emails[]` would error under --jq.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
--streamonbatch verify/batch getdoesn't meaningfully stream. For an already-completed batch — the normal case when yougetone — the completed API payload drops the top-leveltotal/processedcounts, so the progress gates.Total > 0never fires and the command emits a single{"event":"complete",...,"emails":[...]}line. That's effectively--jsonwrapped in an envelope, not streaming.There's nothing to stream for a completed batch: the API returns the whole
emailsarray in one response (large batches return adownload_fileURL, not inline rows). The only genuinely useful piece — NDJSON result rows — is already achievable with the built-in--jq:Since
--streamis unreleased, removing it is cleaner than redefining it.What
--streamflag frombatch verifyandbatch get.batchStreamer, theemit*helpers,newStreamerIfEnabled, andapplyStreamImplications.waitForCompletionback to a plain poller (progress bar on stderr, as before).batch get --waitnow honors--quietfor the polling UI, matchingbatch verify.--jq '.emails[]'NDJSON recipe (paired with--wait) in the README and the emailable skill.All tests pass.