refactor(messaging): give the [OPTIONS:] trailer parse one owner

[bolichen97]

Follow-up to #5218, which is where I noticed this: six channels each carried the same [OPTIONS:] parse, and three of them were identical down to the comment.

What was duplicated

Every widget-capable renderer had the same eight lines — search the shared trailer regex, rstrip the body, split the group on |, drop the blanks, then decide what to do with an unfinished marker:

site	partial handling
discord/renderer.py::_extract_options	hide
telegram/renderer.py::_extract_options	hide — byte-identical to Discord's, comment included
teams/renderer.py::_extract_options	hide — same again
wecom/renderer.py::_render_options_as_text	hide, then folds into format_overflow
webex/renderer.py::_split_trailer	behind a flag
messaging/renderer.py::render_options_as_text	keep

render_options_as_text could not serve any of the widget channels, which is why they each grew a copy: it returns only the body, and a widget renderer needs the choices too.

A parse duplicated per channel drifts per channel, and does it silently — every copy reads correctly in isolation, so nothing goes red when one of them stops agreeing with the others.

The one thing they genuinely disagree about

hide_partial is a parameter, not a policy baked into the helper, because the channels are each right for their own surface:

• True — a streaming surface. Text is still arriving, so an unfinished [OPTIONS fragment really may be a marker mid-flight. Hiding it keeps reserved protocol off screen, and the next frame re-renders from the full buffer, so nothing is lost.
• False — a buffered surface that sends once. It cannot tell a live fragment from the assistant's prose, and cutting prose is permanent: a reply ending see the [OPTIONS section has to keep its last four words.

The default is False because the two failure directions are not symmetric — a needless keep flashes markup for one frame, a needless cut deletes text nobody can recover. So a caller that forgets degrades toward the cosmetic failure, and every streaming caller says True out loud, which also makes the destructive choice greppable.

Webex is the one channel that needs both answers, and it keeps them: hide_partial=True on the status frame, the default on the final answer.

What I deliberately did NOT converge

slack/format.py::extract_options stays as it is. It looks like a seventh copy and isn't: it parses OPTIONS_RE_LINE (re.MULTILINE, end-of-line), not the end-of-buffer OPTIONS_RE_TRAILER every other channel uses. Routing it through this helper would silently stop matching a marker that ends a line mid-message. Different grammar, not a duplicate.

I caught this by migrating it first and then checking which regex it aliased — worth stating plainly, because on a skim it is the most obvious candidate in the set.

One behaviour trap in the shared path

render_options_as_text reaches apply_options_cap unconditionally rather than behind an if not choices guard, and that is load-bearing: a matched-but-empty trailer ([OPTIONS: ]) must still have the marker stripped, while a no-match must not touch the text. Those two are indistinguishable from the choice list alone. With no match split_options_trailer returns the text unchanged and the cap is the identity on an empty list, so one call covers all three cases. My first attempt did add that guard and would have shipped [OPTIONS: ] as visible text; there is now a test for it.

Tests

• TestSplitOptionsTrailer — both policies, the empty-vs-absent trailer distinction, the quoted-marker-mid-answer case the end anchor exists for, and a test pinning the direction of the default (not just its value) so a future edit cannot flip it to the destructive reading unnoticed.
• TestOnlyOneTrailerParseExists — a ratchet that greps the tree for a re-derived parse, records both allowed sites (the helper, and constants.split_trailing_protocol_suffix, which answers a different question) plus the Slack exemption and why, and asserts it is not vacuous so a grep that stops matching cannot make it pass forever.

Behaviour-preserving by construction: each call site keeps its own historical answer. 1,312 tests across the six affected channels pass unchanged, and the full suite is 63,232 passed / 0 failed. mypy (1101 files), flake8, isort, the black baseline, docs-lint, brand, harness-parity and scrub all clean.

Net -143/+276 (the growth is tests and the rationale; production code shrinks by ~90 lines).

Note for whoever merges second

#5708 touches the same from kiro_crew.messaging.renderer import … line in webex/renderer.py (it adds new_approval_nonce; this adds split_options_trailer). Trivial import-line conflict, no semantic overlap.

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 4a5d071efce54bb20a4b460a8d0ed505c7081a7c — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

The merged diff is confirmed behavior-preserving: each of the six call sites keeps its exact historical policy (streaming channels pass hide_partial=True, buffered paths take the default), the Slack line-grammar exemption is real (OPTIONS_RE_LINE vs OPTIONS_RE_TRAILER), and the ratchet test plus doc update land in the same commit. No divergence between description and diff; no design-level issues found.

Design-Verdict: PASS

Six drifting copies collapse into one parameterized parse, with the single real policy difference kept explicit, a non-vacuous ratchet, and the Slack non-duplicate correctly excluded.

[DESIGN-REVIEWED] 4a5d071

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 4a5d071efce54bb20a4b460a8d0ed505c7081a7c and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 4a5d071

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 4a5d071efce54bb20a4b460a8d0ed505c7081a7c: <one-sentence reason>

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 4a5d071efce54bb20a4b460a8d0ed505c7081a7c — this comment is updated in place on each push.

Review details

I've verified the refactor thoroughly. Let me confirm the equivalence of the trickiest case — render_options_as_text routing an empty/no-match through apply_options_cap — which I've confirmed via lines 333-339: cap_choices([]) yields empty kept/overflow, not overflow returns (body, kept) with body untouched, so [0] == original text. All five converted call sites preserve their hide_partial direction, and no dangling references to removed symbols remain.

No findings.

[OPUS-REVIEWED] 4a5d071

Verdict parsed from the review's SHA-scoped output markers for commit 4a5d071efce54bb20a4b460a8d0ed505c7081a7c.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 4a5d071efce54bb20a4b460a8d0ed505c7081a7c: <one-sentence reason>

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 4a5d071efce54bb20a4b460a8d0ed505c7081a7c — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

First-Principles-Verdict: CONCERNS

The consolidation is real and cause-level, but the ratchet's "one owner" claim already has a survivor: whatsapp/turn_renderer.py:90 re-derives trailer removal via .sub.

What this change ships

Intent: stop six per-channel copies of the [OPTIONS:] trailer parse from drifting silently — a FIX (of duplication), no new capability.

1. Six channel parses replaced by one shared split_options_trailer — justified, cause-level (counted: 6 consumers — Discord, Telegram, Teams, WeCom, Webex ×2, render_options_as_text)
2. hide_partial keyword, default False — justified: 5 callers pass True, 2 use the default; both variants live
3. Each channel keeps its historical partial policy — justified, behavior-preserving (verified per site against the deleted bodies)
4. Ratchet test grepping the tree for re-derived parses — justified, but see Watch
5. Unit tests pinning the default's direction and empty-vs-absent trailer — justified
6. messaging.md spec section in the same commit — derived (AGENTS.md mandate)

Watch

• The ratchet (TestOnlyOneTrailerParseExists) greps for rfind("[OPTIONS") and group(1).split("|"), so a .sub-spelled re-derivation is invisible to it — and one exists today, undeclared: whatsapp/turn_renderer.py:90 (OPTIONS_RE_TRAILER.sub("", text).strip()). Count: 1 unfixed sibling of the seven OPTIONS_RE_TRAILER consumers (grep: OPTIONS_RE_TRAILER in src/, 3 files post-change). The description's "six channels" table and the Slack exemption are both recorded; WhatsApp is neither converged nor exempted-with-reason.

Subtractions

• Delete whatsapp/turn_renderer.py::_strip_options's hand-rolled .sub strip and take split_options_trailer(text)[0] (WhatsApp buffers and sends once, so the default policy is already its policy) — or add it to the ratchet's recorded exemptions with a named reason, as Slack got.

[FIRST-PRINCIPLES-REVIEWED] 4a5d071

[iamwhatever]

Tier 1 auto-approve: refactor (8 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: code-move only — six per-channel copies of the trailing [OPTIONS:] parse collapse into one split_options_trailer owner in messaging/renderer.py, with the buffered-vs-streaming partial-marker choice lifted to a hide_partial parameter so each channel keeps the behaviour it already had (verified on the webex and wecom call sites: the extracted helper is byte-for-byte the logic it replaces). Spec files changed as a ride-along (a minority of the diff on both file count and changed lines), not reviewed as a design decision: docs/system-specs/modules/messaging.md.