feat(chat): render the (recommended) option marker as a badge - #7157
feat(chat): render the (recommended) option marker as a badge#7157rnoack1 wants to merge 1 commit into
Conversation
GPT 5.6 Review (fork) — ✅ no blocking findingsReviewed Review detailsNo findings. |
UX Review (Fable 5, fork) — 🟡 CONCERNSUX-level review of Reviewed the full patch: UX-Verdict: CONCERNS Sound badge design and honest dispatch, but every user-visible surface — chip badge, Slack Recommended line, raw-marker fallback — has been seen by no cold reader. Watch
Evidence gaps
Suggestions
[UX-REVIEWED] 8d5dc7e |
Opus 4.8 Review (fork) — ✅ no blocking findingsReviewed |
Design Review (Fable 5, fork) — 🟡 CONCERNSDesign-level review of Design-Verdict: CONCERNS In-band marker placement forces the dispatch grammar to live in three copies — TS guard, Python guard, agent prompt — held together only by parity tests. Watch
Suggestions
[DESIGN-REVIEWED] 8d5dc7e |
First Principles Review (Fable 5, fork) — 🟡 CONCERNSPremise-level review of I have everything I need. Final check of my counts: First-Principles-Verdict: CONCERNS The description claims a channel-wide marker strip the diff deliberately removed — and the guard apparatus contains a hazard this PR's own in-label placement creates. Not justified as shipped
What this change shipsIntent: let the agent visibly steer the user toward one option by marking it
Watch
Subtractions
[FIRST-PRINCIPLES-REVIEWED] 8d5dc7e |
e4fcbc6 to
a440191
Compare
a440191 to
a1c3b2d
Compare
a1c3b2d to
9b0718c
Compare
9b0718c to
e29337c
Compare
0ed8a41 to
31dbb9a
Compare
31dbb9a to
29c56c6
Compare
29c56c6 to
df34915
Compare
iamwhatever
left a comment
There was a problem hiding this comment.
I am not sure if this is a better design honestly. With limited chips, adding tags does not help much. I would rather we reorder these chips to put the most reasonable stuff at the beginning as always. Will let other designers decide.
@iamwhatever I added steering in my local environment to say explicitly mark your recommended option(s) , and though the first option is usually recommended, sometimes it is not, and something the last option is "Nothing more (recommended)". Also sometimes 2 options are (recommended first), (recommended then) so although the intention may be to have the most recommended options in the front, sometimes more options are presented than the agent "recommends". I don't think it necessarily has to be a core feature like this PR, as it can be accomplished with steering - but was trying to figure out if there is a way for everyone / team members to get a similar behavior without needing to add steering. The in button chip is potentially cool because you could label things other than recommendations potentially with custom logic or hooks, not sure. Maybe another option for just recommendations could be rendering with different color or outline? |
Open PR relationship auditThis is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion. Relationship findings
No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit. |
|
The chips are capped at On reordering: worth noting the two are not exclusive — ordering communicates rank, the marker communicates which single option the agent actually recommends, and the agent emits that marker as text today with no way to signal position. Happy to defer to designers on which carries it. One caveat if you compare the screenshots: they were generated before the ordering-variant forms were dropped, so they show a second |
|
Closing out the screenshot caveat from my previous comment: the regenerated pair is already on the branch, so the stale second badge is gone — my "I will regenerate them" was a beat behind the branch. Verified at head
The design question you raised is untouched by any of this — badge versus reordering is still yours and the other designers' call, and I have not moved it. |
|
One change since you looked, on the "does not help much" point specifically — at head The badge previously shared the picked chip's own fill, so once an option was selected the tag melted into it and stopped being readable at exactly the moment it mattered. It now uses a distinct fill: Nothing else about the badge changed, and I have not touched the ordering question — badge versus reordering is still yours and the other designers' call, and I have deliberately left it open rather than implementing either side. |
|
Thanks for looking. Measurements at the current head Your premise is right, and this branch does not touch it. Where the badge sits relative to the label. On reordering "as always" — there is no ordering behaviour in place to build on yet. That last measurement is the useful part of your suggestion: because nothing sorts, "most reasonable first" would be a change to the producer guidance rather than to this component — which is cheap, and not mutually exclusive with a badge. Ordering could carry the ranking while the badge names the pick explicitly for a screen reader or a user who does not read position as ranking. Which of the two we want, or both, is your and the other designers' call — I have not implemented the reordering, and I will follow whichever way you land. |
|
@rnoack1 the 2026-09-08 open-PR relationship audit found two newer open PRs writing into the same lines as this one. The audit read this PR at #8613 (session_escalate, @CrysisDeu) shares #8003 (subagent credit usage, @welikoiwanenko) shares Neither PR duplicates this one. A note on each about landing order is enough, and @CrysisDeu / @welikoiwanenko may want to watch this PR for the merge order. Posted from the 2026-09-08 open-PR relationship audit (read-only, one auditor per PR); reply here if any of this is wrong. |
The marker sat inside the chip's single truncated line and the ellipsis reached it first, so the recommendation was readable only by clicking every chip.
Problem / Motivation
This PR introduces a convention and the rendering it needs: an agent marks its
recommended choice by writing
(recommended)at the START of an option label. Nothing inthe base repo asked for that marker, so the producer rule below is part of this change
rather than behaviour already in the field — see "Backend" under What changed.
Once the marker exists, two things are wrong with leaving it in the label.
The first is dispatch: the label doubles as the user's next message. Clicking
(recommended) Merge it nowsends exactly that, so the user appears to be recommendingsomething to the assistant. Worse, stripping it naively is its own hazard — a cleaned
label can open with
/,@or[, or BE a sigil-less channel command, and then a clickdispatches something the agent authored rather than words the user chose. That is the
harm the guards below exist for, and it is the one the feature is bought for.
The second is presentation: the marker is plain label text, styled identically to the
instruction and competing with it for a single clamped line's width budget. A badge is
one word, outside the truncating span, and visually distinct — so the recommendation is
legible at a glance instead of being read out of a sentence.
Scope note, stated plainly because an earlier revision of this description claimed more:
the marker is only admitted at the START of a label, and a leading marker is never what
an ellipsis reaches. So this change does not rescue a marker from being clipped away;
that only happened for a trailing marker, a shape nothing emits and which this PR
deliberately does not recognise.
ChipBodyalso already puts the full option on thechip's
title, so the text was never wholly unreachable by hover.Why it matters
A recommendation nobody can read is worse than no recommendation, because the
turn was written on the assumption it would be visible — the prose above defers
to "the recommended option" that the buttons never name. The failure is also
silent from the authoring side: the marker IS in the response text, so any check
that greps the text passes while the rendered menu shows nothing.
Shortening labels does not fix it. Truncation depends on rendered width, font and
the chip's own box, so any character budget leaves a band of labels visibly cut —
the same reasoning already recorded above
chipTooltipfor why a 60-charthreshold was the wrong proxy.
What changed
(recommended)is now treated as protocol rather than label text.protocol/recommendation.ts—splitRecommendation(option)returns thelabel with the marker removed plus a boolean saying whether it carried one. The grammar is
deliberately narrow (
recommendedand nothing else — therecommended first/recommended thenordering variants are NOT admitted, since nothing in the repo emits them): admitting a markerpaints a badge, so an open-ended parenthetical rule would let
(destructive)or(see below)style themselves as a recommendation. A label that is only themarker is left alone — a badge names no action, and stripping it would send an
empty message. A label that IS or WOULD BECOME a dispatched command is also left
alone: the label is dispatched verbatim, and the dashboard treats three leading
characters as more than plain text — a leading-slash first word is forwarded to the
harness, a leading
@is resolved by_expand_prompt_mentionto a storedprompt whose CONTENT replaces the message, and a leading
[opens a reservedprovenance prefix the backend byte-matches. Stripping a front marker would
promote inert text into something that runs —
(recommended) /clearwouldleave as
/clearand erase the transcript, and(recommended) @deploywouldleave as
@deployand execute that prompt. For such a label the splitter is ano-op returning the original text and no marker, so it behaves exactly as if
the feature were absent and introduces no dispatch path that did not already
exist. The test is prefix-only; either character inside a label is ordinary prose.
parseOptionsapplies the split, sooptionsare clean at the protocolboundary and the four TypeScript hosts send the instruction alone. The messaging
channels are covered too:
split_options_trailer(
src/kiro_crew/messaging/renderer.py), which 6 renderer modules import, nowruns
strip_recommended_markerover every choice, so a marked label is neitherdisplayed nor dispatched verbatim on any channel. That strip carries the SAME
dispatch guard as the frontend for every form a channel can dispatch — the
/and@sigils, theaction::dispatch protocol, the reserved provenance openers, thesigil-less channel commands and the plan actions — because a channel
sends the label as the user's message and an unguarded strip would turn
(recommended) /clearinto/clear. The frontend carries ONE arm the backend does not,deliberately: a leading orchestration stop word (
stop/cancel/abort), matched onthe FIRST word rather than the whole label because the dashboard chat endpoint reads
it that way, so
(recommended) stop after this stagewould otherwise strip anddispatch as a cancellation of a live run. That consumer is reachable only from the
dashboard, so mirroring the arm onto the channel guard would reserve a word no
channel dispatches. A channel has no badge to render and is not
asked for the marker at all, so the strip there is purely defensive, keeping the
marker carried instead of dropping it.
parseOptionsalso returnsrecommended: the one marked label, ornull. A single label rather than a setbecause the only sanctioned producer marks at most one option, and rather than a
widened
optionselement type becauseoptionsmust stay astring[]— it isthe canonical string a click dispatches and every
pickedset is keyed on, so alabel that differed between display and dispatch would break selection tracking.
FollowUpBarrenders the marker as a badge in a newChipBody, as ashrink-0sibling of the clamped span rather than inside it. Position is theentire fix. It stays on the same line, so chips remain the same height — the
constraint
ChipLabel's own comment exists to protect.followUpRecommendedthreaded through the four hosts that render the bar(
ChatPage,ChatPane,ChatEmbed,SideChat) plusChatInput. All four,not a subset: the marker is stripped for everyone, so a surface left unwired
would lose the recommendation entirely rather than merely render it plainly.
ChipBadgerather than threaded through the protocol:the grammar admits one spelling, so there is no second word to carry. It renders
through
i18nT('components.followUpBar.recommended'), and that key is added toall 13 locale catalogs because the badge is UI copy the user reads and the repo's
catalog-parity gate requires every key in every catalog. The
(recommended)marker itself is NOT translated — it is protocol vocabulary, matched
case-insensitively against the literal English word, so translating it would stop
the parser recognising what an agent wrote.
src/kiro_crew/context.pygains
_OPTIONS_RECOMMENDED_RULE, which instructs the agent to mark one option.Before this PR nothing in the repo asked an agent to emit
(recommended), so thisrule — not pre-existing behaviour — is what makes the marker systematic. It changes
the critical-rules block of every dashboard session. It is added to the DASHBOARD
variant only:
_CRITICAL_RULES_TAILis shared by both surfaces, so appending therewould ask channel agents for a marker no channel can render as a badge — the strip
above removes it, so the instruction would buy nothing while adding a way to get it
wrong. Splitting the
[END CRITICAL RULES]terminator into_CRITICAL_RULES_ENDis what makes a surface-specific slot possible, mirroring theexisting
_DIFF_RULE_DASHBOARD/_DIFF_RULE_CHANNELpair._critical_rules_forpicks the block per turn from the LIVE runtime source, not once atsession start, so a channel turn simply never receives the marker instruction and has
nothing to retract. An earlier revision did carry an explicit retraction; it was removed
once the selector made its premise false, and
TestNoSurfaceRetractsTheMarkerInstructionnow pins that absence on every surface.ParsedOptions.recommended.splitRecommendationis deliberatelyNOT exported from
protocol/index.ts,app-sdk/index.tsorpublic/vendor/kirocrew-app-sdk.mjs: every consumer imports./recommendationdirectly, so a barrel export would be a zero-consumerone-way-door addition riding along in a fix. Apps already get the behaviour
through the exported
parseOptions, whoserecommendedfield subsumes it.docs/app-kit/api-reference.mddocuments that FIELD for out-of-tree appsand explicitly withholds the marker's wire spelling and placement from the
published contract, so apps bind to
recommendedrather than to the grammar.A SECOND, INDEPENDENT change ships in the same commit: Slack option clicks stop being
command-interpreted. This is not part of the marker feature above and is not scoped to
marked labels — it changes behaviour for EVERY Slack option click, including options that
carry no
(recommended)marker at all. It is declared separately here because the repoenforces one commit per PR, so it could not be landed as its own change without splitting
this PR — a scope call left open below.
Slack option clicks no longer interpret a leading command token.
handle_message(src/kiro_crew/slack/handler.py) takes a newinterpret_commandskeyword,Trueby default so a human-typed message isunaffected, and the three Slack option-dispatch sites
(
src/kiro_crew/slack/interactions.py) passFalse. It resolves to oneinterpret_as_commanddecision every command branch reads, so a new branchcannot forget it. This applies to every Slack option click, not only the
ones carrying a marker, because an option label is model-authored either way.
The harm it closes is separate from the marker work and predates it: a label
whose first word is one of the nine bangs at
handler.py:155-166was executedon click rather than sent as turn content, so an agent-authored
!yolo onbecame
/kirocrew yolo onand changed the approval mode. The marker-stripguard cannot cover this —
!is not one of the reserved dispatch sigils(
/and@), so the guard has nothing to match and would strip such a labelhappily. Stated here as its own change rather than folded into the badge work,
since reverting the badge would reopen it.
The two promotion fences can no longer drift apart. The fence is implemented
twice by construction —
strip_recommended_marker(Python, channel dispatch) andsplitRecommendation(TypeScript, dashboard dispatch) — because each runtimedispatches a clicked label itself. The shared vector fixture pinned the forms that
exist today, which says nothing about the next one: a slash command added to one
grammar and forgotten in the other would let a marker strip promote a
model-authored label into a real command on the surface that was missed, with every
existing test still green.
test/test_promotion_fence_drift.pyasserts theinvariant instead, in both directions. Every reserved list must be either PAIRED
with its frontend symbol and match it, or declared single-surface with a written
reason; a list or symbol that is neither fails the completeness test, so a seventh
one cannot skip the pairing. Verified by mutation in four shapes — a Python entry
the TS array lacks, an alternation branch it lacks, a brand-new backend list, and a
brand-new frontend symbol — each of which the gate catches.
The marker is instructed on the dashboard ONLY. The dashboard is the surface that paints the badge. No channel producer rule asks for the marker, so a channel label carrying one is model drift rather than protocol, and
strip_recommended_markerkeeps it out of the dispatched string as a defensive measure rather than as half of a paired feature. An earlier revision added a channel-side producer rule and aRecommended: <label>restatement; both are withdrawn, which also removes the hardcoded English prefix that restatement shipped to every locale.Tests
src/test/recommendation.test.ts(new, 33): the split at the leading andtrailing edge; that both ordering variants are REJECTED; case-insensitivity; a negative
control that
(destructive),(see below)and(recommended by nobody at all)are not recommendations; the marker-only label; a no-regex-state guard;and the three regressions this turns on — an EDGE marker never survives into a
dispatchable label, an INTERIOR one is never removed from it, and stripping
never promotes a label into a dispatched command, by slash or by
@mention. Plan chips(
Go,Go All,Cancel) are asserted byte-identical, sinceusePlanActionMutationdispatches on exact label equality.src/test/FollowUpBar.test.tsx: its form is pinned against the existingspec-decision badge — trailing mono text rather than a leading pill, matching
apps/spec-builder/components/SpecStatePanel.tsxso the same word does not render intwo shapes. Colour is the one deliberate difference: that sibling tints itself with the
accent, which on a chip is the SELECTION signal, so this badge keeps a neutral text
colour and the test pins zero accent tokens on an unpicked recommended chip. The badge
is also asserted to be a
sibling of the clamped
.truncatespan and never a descendant, with that spancarrying only the label. An assertion on the rendered text alone would pass
either way and prove nothing.
test/test_messaging_recommended_marker.py(new): the channel half. A marked labelis not dispatched verbatim through the shared parse, an interior marker survives as
prose, and a label that would become a slash command, a mention or reserved
provenance is left exactly as it arrived. Mutation-verified in both directions:
removing the strip and removing its dispatch guard each fail five cases.
test/test_slack_option_click_not_a_command.py(new): every top-level command branchin the Slack handler tests the flag, and both option-click dispatches pass it as
False. Mutation-verified: ungating the!family fails 4, dropping the flag from onedispatch fails 1, and removing all of it fails 7.
test/test_slack_linked_option_routing.py(new): a command-shaped label clicked as anoption lands in the linked slot's queue rather than falling through, and a user-typed
bang still falls through to the command handler. Mutation-verified: ungating the
fallthrough fails 5.
test/test_dispatch_sigil_parity.py(new): fails if the/,@,[deny listdiverges between the backend tuple and the frontend list, so a fourth dispatch
prefix cannot be added to one surface alone. The same suite pins the
action::dispatch protocol, which the single-character sigil tuple cannot express, against the action router's own spelling, and asserts the frontend guard actually consults the mirrored list rather than merely declaring it. It also asserts the extraction found anon-empty list, since an unreadable list would make the comparison vacuous.
test/test_context_critical_rules_assembly.py(new): pins the backend producerhalf — that the dashboard block instructs the marker and the channel block does
not, that the LIVE surface rather than the session's origin decides which block a
turn receives, that NO surface retracts the instruction, and that every reserved
dispatch prefix the backend byte-matches opens with
[, which is the property thefrontend guard relies on. Each arm is mutation-verified: re-adding a retraction
paragraph fails the absence test, and unbracketing one backend prefix fails the
prefix test. A positive control asserts the scan reaches a real channel turn, and a
guard asserts the channel's own diff rule was not removed alongside the retraction.
capture/followup-recommended-badge.{html,tsx}+scripts/capture-followup-recommended-badge.mjs(new): real-layoutmeasurement, because the DOM contract above is necessary and not sufficient — a
badge outside the span but pushed past the chip's edge satisfies every class
assertion and is still invisible.
__measure()takes a Range over the marker'sown characters and compares its rect against the chip's painted box.
fix=offrenders upstream's shape verbatim, so the before arm is asserted to reproduce
rather than assumed. 12 of 12 assertions pass across both themes.
1683/1683files,26613tests.tsc -bclean.eslinton thechanged files: 0 errors, and the 22 pre-existing hook warnings in
ChatPage/SideChatare unchanged from main (measured both ways).main;git patch-id --stableidentical before and after.The harness earned its place on its first run: it failed, because the initial
measurement tested horizontal overflow.
line-clamp-1isdisplay:-webkit-box+-webkit-box-orient:vertical, so an over-long labelwraps and later lines are clipped — the marker disappears downward, and a
horizontal test reports it visible. The paint test now checks both axes, so it is
correct under either clipping mechanism.
It caught a second thing later, which is why the framing above is what it is. When the
fixture was moved to the sanctioned leading marker, the old
fix=offassertion — "markerNOT painted" — went red:
markerVisible=true, because a leading marker is never clipped.The measured defect is therefore not invisibility, and the arm now asserts what is
actually true pre-fix:
markerInLabelis true, i.e. the marker is part of the label aclick dispatches. The
fix=onarm asserts the converse plus the badge.Screenshots
Before — the marker sits inside the label, styled as instruction text and part of what a
click would send:
After — one badge outside the clamped span, and the label holds the instruction alone:
Light theme, both arms
Note on overlap
#7125 is open on the same
ChipLabelline, swappingline-clamp-1 break-wordsforblock truncate. Thetwo are complementary and this PR does not depend on which lands first: that one
changes how the label is trimmed, this one moves the marker out of the trimmed
element. If it merges first, the badge is unaffected and only this PR's test
selector for the clamped span needs updating — happy to rebase onto it.
Security-relevant behaviour change on Slack, stated explicitly
The channel-side marker strip alone does not cover
!-commands, and this is thepart of the diff a reviewer would otherwise have to find for themselves.
Stripping
(recommended)cleans the label, but it says nothing about what the cleanedlabel is.
(recommended) !yolo onstrips to!yolo on, and Slack read a clickedoption through the same path as a typed message — so the click ran the command, turning
on process-wide auto-approval. The frontend sigil guard cannot close this: it can only
enumerate prefixes (
/,@,[) and!commands are a separate family reached by adifferent branch.
interpret_commandsis new onslack/handler.py::handle_messageand passed asFalsebyboth option-click callbacks. It fixes a PRE-EXISTING privilege escalation and does not
depend on this feature: before any marker existed, a model-authored label spelled
!yolo onwas already click-executable, because an OPTIONS label is turn content whateverit looks like. So this half stands on its own, and reverting the badge would reopen the hole
rather than restore a safe state. With the flag off,
!yolo,!agent,!compact,status,sessionsand the!temporary/!incognitoprivacy modifiers are no longerreachable by clicking a chip. A user who TYPES any of them is unaffected —
the flag defaults to
True. This mirrors the Telegram and Discord dispatchers, whichalready pass the same flag on their callback paths.
maybe_route_linked_threadtakes the flag too. It deliberately lets a bang command fallthrough to normal handling so one can still be run inside a linked thread; for a
model-authored label that fallthrough would persist the turn under the dashboard key
while the linked slot's transcript never saw it, so a command-shaped click would resolve
to a different slot than an ordinary message from the same thread. With the flag off the
fallthrough is suppressed and both resolve identically.
Channels strip a drifted marker and restate nothing
Six renderers parse the trailer through
messaging.renderer.split_options_trailer, whichstrips the marker so no channel displays or dispatches it verbatim, and appends nothing in
its place. That is a decision, not a gap, and two facts drive it. No channel producer rule
asks for the marker — only the dashboard is instructed to emit one, and
_OPTIONS_RECOMMENDED_RULEis wired into the dashboard block alone — so a marker arrivingat this seam is model drift, and restating drift would give it protocol standing. And the
two-value wrapper has no recommendation identity to restate WITH: it drops that identity by
contract, which
test/test_recommendation_seam_parity.pypolices, so the choice a linewould name is not knowable there.
test/test_channel_recommendation_not_restated.pypinsboth halves — the marker leaves the dispatched value, and the body gains no line.
Slack is the one surface that CAN name the choice, because it does not use this parse at
all:
slack.format.extract_options_with_recommendationcarries the identity explicitly andrenders its own
*Recommended:*context block, cut at the same checkbox cap the optionitself is cut at.
Cost of moving the marker out of the label
Recorded as a cost statement, not a proposal: the marker's placement is an open design
question and this section only measures what the alternative would remove.
The marker sits inside the label a click dispatches, so the strip needs a guard for every
value that must not be promoted. Moving it to an out-of-band control tag removes the need
for that guard entirely. Measured by AST at this head:
action::)test/test_dispatch_sigil_parity.pytest/test_injected_provenance_parity.pytest/test_reserved_text_commands_parity.pytest/test_reserved_plan_actions_parity.pytest/test_reserved_provenance_parity.pytest/test_dispatch_prefix_declared.pySo 4 of 5 suites and 345 lines go, along with the frontend mirrors of all four lists. The
badge itself survives the move:
keepVisibleMarker.tsis the in-repo precedent for acontrol tag that renders as nothing, and
strip_control_commentsalready removes such tagsfrom every plain-text projection. The remaining cost is that label-to-tag correspondence becomes a validation concern
rather than a structural one. The grammar is NO LONGER published SDK surface: this PR
documents only the
recommendedfield and states that the marker's spelling andplacement are not part of the published contract, so moving out-of-band later needs no
app-author migration and is no longer a one-way door.
Why the Slack interpretation flag stays in this change
Two reviews disagree about scope, so both positions are recorded rather than settled here.
One asks for the
interpret_commands=Falsework to land as its own PR. The other holds thatit is load-bearing in this change specifically:
!is not a reserved dispatch sigil, sowithout the flag the strip would turn
(recommended) !compactinto an executed Slackcommand. That second reading is why it is not split out. The scope call is open.
Why the channel seam restates nothing, given two lanes asked for opposite things
Worth stating plainly, because the two remedies for the doc/code mismatch pull apart. One
asks for the append to be implemented; the other asks for the channel strip to be deleted
entirely and a drifted marker left as inert label text. Both would cure the mismatch. The
shipped shape is the middle one, and it is chosen from what the tree already asserts rather
than from taste: the strip stays, because a marker must not reach a dispatched value, and
nothing is appended, because the seam that would append has no recommendation identity —
test_recommendation_seam_parity.pystates that the two-value wrapper drops it by contract,so an append there could only name a choice it cannot know is the right one. Implementing it
would mean either widening that seam's signature or guessing, and it would restate a steer
no channel producer rule ever asked for.
The narrower reading also matters: with the channel rule instructing no marker, the code
path exists only to contain drift, so documenting it as containment is the honest
description. This is the reasoning, not a rebuttal — the mismatch is fixed in the tree, in
docs/system-specs/modules/messaging.md, in this section, and in a test that fails if arestatement is ever appended.
The two-value
extract_optionsis deletedA review asked whether
slack/format.py::extract_optionswas dead. An earlier revision ofthis description argued it was live and should be kept; that was wrong about the outcome, and
this diff DELETES it. Its one real consumer discards the choices it returns, so that call site
now uses the three-value
extract_options_with_recommendationdirectly(
subagent.py:101imports it,subagent_manager/run.py:1634calls it). Only one spellingsurvives, and two ratchets pin that:
test_recommendation_seam_parity.py:80asserts nodef extract_options(exists in the module, andtest_slack_mirror_recommendation.py:186asserts the attribute is absent, so a re-add fails the suite rather than passing silently.