Consolidate the reply-prefill readers into one shared helper - #384
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e84aacfe3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Consolidates reply-prefill parsing into the shared internal/mail domain package for consistent CLI and TUI behavior.
Changes:
- Adds shared reply-prefill types and reader.
- Migrates CLI and TUI reply flows to the helper.
- Adds focused tests for success, fallback, and failure paths.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
internal/mail/reply_prefill.go |
Implements shared prefill parsing. |
internal/mail/reply_prefill_test.go |
Tests parsing and fallback behavior. |
internal/cmd/thread_reply.go |
Adopts the shared helper for CLI replies. |
internal/tui/compose.go |
Adopts the shared helper for TUI replies. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
All reported issues were addressed
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Follow-up from #370 and #372, carded and approved as [CLI][TUI] Consolidate the reply-prefill readers into one helper.
The CLI (
hey reply,hey compose --thread-id) and the TUI reply form each parsed the reply prefill (GET /entries/{id}/replies/new.json) — subject, acting sender, recipients — with its own reader. Reviewers on those PRs nudged four times toward a shared helper; declined there to keep each PR single-concern, and carded as a follow-up instead. This PR is that card.What moved:
mail.ReplyPrefillFromServer(internal/mail/reply_prefill.go) is the one reader: it parses subject, acting sender, and recipients from HEY's prefill, and answersfalse— routing the caller to its local fallback — on a failed read or an empty recipient list, still carrying the subject and sender it did get.resolveThreadReplyininternal/cmd/thread_reply.gonow calls it; the package-localreplyPrefilltype andreplyPrefillFromServerreader are gone.replyRecipientsis now an alias ofmail.ReplyRecipients, so the local fallback and the existing tests read unchanged.loadReplyContextininternal/tui/compose.godrops its inline parsing block for the same call.Pure refactor: no wire changes, and the existing reply tests in
cmdandtuipass unchanged as the behavior lock, joined by a focused test for the helper itself. The auto_quoting invariant stays visibly intact and is now documented on the shared type: the prefill's quoted content is never carried or echoed back — HEY appends the quote at delivery.One normalization note: the two readers disagreed only on degenerate prefill addresses (the CLI kept HEY's lists verbatim minus blanks; the TUI trimmed and case-deduped). The shared reader keeps HEY's computed lists verbatim, dropping blanks — identical output for anything HEY actually serves, and the tests lock that.
Summary by cubic
Consolidates the CLI and TUI reply-prefill readers into one shared helper in
internal/mail, so both reply flows parse HEY's prefill identically. Pure refactor — no wire changes; existing reply tests pass unchanged.Refactors
mail.ReplyPrefillFromServerreads subject, acting sender, and recipients, and returns false to trigger the caller's local fallback when the read fails or recipients are empty.internal/cmd/thread_reply.goandinternal/tui/compose.gonow call it; their package-local readers are deleted.AGENTS.mdnow points at the shared helper.Written for commit af22ba0. Summary will update on new commits.