feat(photon): render multiple-choice clarify as a native iMessage poll - #48194
feat(photon): render multiple-choice clarify as a native iMessage poll#48194vaibhavjnf wants to merge 1 commit into
Conversation
The `clarify` tool's multiple-choice prompts flattened to a numbered text
list on Photon/iMessage, even though iMessage has a native poll bubble and
spectrum-ts already exposes it via the `poll()` content builder. Two gaps
caused the flattening:
* Outbound: the sidecar only had `/send` (text); there was no way to send
a poll, so the base adapter's numbered-text fallback was used.
* Inbound: `normalizeContent()` handled only text/attachment/voice, so a
poll vote (`poll_option`) was dropped on the floor ("[Photon content
type not handled: poll_option]") and never resolved the clarify.
Fix, end to end:
* Sidecar: import `poll` from spectrum-ts; add a `/send-poll` route
(`space.send(poll(title, ...options))`); serialize inbound `poll_option`
(the vote: chosen title + selected bool) and `poll` content in
`normalizeContent()`.
* Adapter: override `send_clarify` — for choices, send a native poll via
`_sidecar_send_poll` and call `mark_awaiting_text` so the gateway's
existing pending-clarify text-intercept resolves the answer; open-ended
clarifies keep the plain-text path. Inbound `poll_option` selections are
dispatched as a plain-text MessageEvent carrying the chosen option
(deselections / empty votes are dropped). If the poll send fails (an
older sidecar without `/send-poll`, or a send error) it falls back to the
numbered-text clarify, so nothing regresses on a half-upgraded restart.
No new model tool, no new env var, no core change — the capability lives at
the platform edge. The poll vote reuses the existing clarify text-intercept
resolution path, so no new gateway resolution mechanism is introduced.
Tests: tests/plugins/platforms/photon/test_poll_clarify.py — inbound vote ->
choice text, deselection/empty-vote dropped, send_clarify sends a poll +
enables text-capture, open-ended stays text, and poll-failure falls back to
the text list. Full photon suite green.
Contributed by Vaibhav Sharma (X: @vabbyshabby).
|
Related: #43665 (feat(photon): add native poll sending). That PR adds the generic outbound poll primitive ( |
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Renders multiple-choice clarify questions as native iMessage polls via spectrum-ts. Selection streams back as a poll_option event resolved as plain text. Graceful fallback to numbered text if native poll fails.
Looks Good
- Clean feature implementation with proper error handling
- Well-documented code with clear architectural comments
- Fallback path is well-thought-out
- No debug or diagnostic artifacts
- Appropriate logging for production
Reviewed by Hermes Agent
|
Cant wait to become a hermes contributor 😭 |
|
Thanks for the focused edge-platform implementation. The current main premise is valid: Photon still inherits the numbered clarify fallback in Problems
Suggested changes
Automated hermes-sweeper review. |
… clarify Follow-up to the #48194 pick: it was written before #43665 landed and re-added its own /send-poll sidecar route and poll import. Collapse the duplicates: - keep #43665's /send-poll route (>=2 trimmed string options) as the single sidecar implementation; drop #48194's variant - drop the duplicated poll import in the sidecar destructure - make adapter.send_poll() a thin wrapper over _sidecar_send_poll(), the one /send-poll client (shared with the poll-backed clarify path), and align its validation to the sidecar's >=2-options contract
… clarify Follow-up to the #48194 pick: it was written before #43665 landed and re-added its own /send-poll sidecar route and poll import. Collapse the duplicates: - keep #43665's /send-poll route (>=2 trimmed string options) as the single sidecar implementation; drop #48194's variant - drop the duplicated poll import in the sidecar destructure - make adapter.send_poll() a thin wrapper over _sidecar_send_poll(), the one /send-poll client (shared with the poll-backed clarify path), and align its validation to the sidecar's >=2-options contract
… clarify Follow-up to the #48194 pick: it was written before #43665 landed and re-added its own /send-poll sidecar route and poll import. Collapse the duplicates: - keep #43665's /send-poll route (>=2 trimmed string options) as the single sidecar implementation; drop #48194's variant - drop the duplicated poll import in the sidecar destructure - make adapter.send_poll() a thin wrapper over _sidecar_send_poll(), the one /send-poll client (shared with the poll-backed clarify path), and align its validation to the sidecar's >=2-options contract
… clarify Follow-up to the #48194 pick: it was written before #43665 landed and re-added its own /send-poll sidecar route and poll import. Collapse the duplicates: - keep #43665's /send-poll route (>=2 trimmed string options) as the single sidecar implementation; drop #48194's variant - drop the duplicated poll import in the sidecar destructure - make adapter.send_poll() a thin wrapper over _sidecar_send_poll(), the one /send-poll client (shared with the poll-backed clarify path), and align its validation to the sidecar's >=2-options contract
for poll clarify Follow-up to the NousResearch#48194 pick: it was written before NousResearch#43665 landed and re-added its own /send-poll sidecar route and poll import. Collapse the duplicates: - keep NousResearch#43665's /send-poll route (>=2 trimmed string options) as the single sidecar implementation; drop NousResearch#48194's variant - drop the duplicated poll import in the sidecar destructure - make adapter.send_poll() a thin wrapper over _sidecar_send_poll(), the one /send-poll client (shared with the poll-backed clarify path), and align its validation to the sidecar's >=2-options contract
for poll clarify Follow-up to the NousResearch#48194 pick: it was written before NousResearch#43665 landed and re-added its own /send-poll sidecar route and poll import. Collapse the duplicates: - keep NousResearch#43665's /send-poll route (>=2 trimmed string options) as the single sidecar implementation; drop NousResearch#48194's variant - drop the duplicated poll import in the sidecar destructure - make adapter.send_poll() a thin wrapper over _sidecar_send_poll(), the one /send-poll client (shared with the poll-backed clarify path), and align its validation to the sidecar's >=2-options contract
Summary
On Photon/iMessage, the
clarifytool's multiple-choice prompts flattened to a numbered text list — even though iMessage has a native poll bubble andspectrum-tsalready exposes it via thepoll()content builder. This wiresclarifychoices to a native iMessage poll: the user taps a choice and the vote resolves the clarify.Root cause
Two gaps, both directions:
/send(text). No way to send a poll → base adapter's numbered-text fallback was used.normalizeContent()handled only text/attachment/voice. A poll vote arrived aspoll_optionand was dropped — surfaced as[Photon content type not handled: poll_option]— so the clarify never resolved.Reproduced live: sending a
poll()to a DM rendered the native bubble; tapping a choice produced exactly thepoll_option"content type not handled" marker, and the pending clarify hung to timeout.Fix (end to end, edge-only)
Sidecar (
sidecar/index.mjs)pollfromspectrum-tsPOST /send-poll→space.send(poll(title, ...options))poll_option(vote: chosentitle+selectedbool) andpollcontent innormalizeContent()Adapter (
adapter.py)send_clarify: for choices, send a native poll via_sidecar_send_polland callmark_awaiting_textso the existing pending-clarify text-intercept resolves it; open-ended clarifies keep the plain-text pathpoll_optionselections dispatch as a plain-textMessageEventcarrying the chosen option (deselections / empty votes dropped)/send-poll, or a send error) → fall back to the numbered-text clarify, so nothing regresses on a half-upgraded restartNo new model tool, no new env var, no core change — capability lives at the platform edge, and the vote reuses the existing clarify text-intercept resolution path (no new gateway resolution mechanism).
Test plan
tests/plugins/platforms/photon/test_poll_clarify.py(6 tests): inbound vote → choice text; deselection dropped; empty-title vote dropped;send_clarifywith choices sends a poll + enables text-capture; open-ended stays text; poll-failure falls back to the text listpytest tests/plugins/platforms/photon/)node --checksidecar,py_compileadapter — clean tree, no dependency on local modspoll_optionevent the new code consumesCompatibility
Additive (+379/-1).
/send-pollis a new route; the adapter degrades to the text-list clarify if the sidecar predates it, and open-ended clarify is unchanged.Contributed by Vaibhav Sharma (X @vabbyshabby).