Broker: prefer threaded reply hints for threaded inbound messages - #919
Conversation
bradbrok
left a comment
There was a problem hiding this comment.
Exact-head PASS — frozen #466 coordinates only
Pinned identity:
- PR #919 / branch
agent/issue466-thread-aware-reply-hint - head
c744165a65411212db58b581b3820e2dab10bdfb - tree
7d2ef0e431245658c20a5748879a60047191932d - sole parent, base, and merge-base
9f5e037fce17fea763ef24b4d81d5065506bc057 - divergence
0/1; branch ref andrefs/pull/919/headboth exact
I verified this in a wholly fresh network clone, detached at the exact SHA. The checkout was clean before submission. The delta is exactly two files, src/pinky_daemon/broker.py and tests/test_broker.py, totaling +92/-40; the production delta is confined to the reply-hint block (+19/-8).
Review result:
reply_to+message_idnow makesthread(message_id=..., text=...)the first instruction and explicitly warns that fallbacksend(...)posts OUTSIDE the thread.- Top-level inbound remains
send(...)first, withthread(...)retained as the quote/thread-reply option. - Without
message_id, no thread clause is emitted. - Existing
web/api/ empty-platform suppression is unchanged. - The original formatted prompt and delivery kwargs remain unchanged; the hint stays agent-only. No
_format_prompt, response delivery,thread_ts,pinky_messaging, config, manifest, or workflow change is present. - Tests cover thread-first, top-level send-first, no-message-ID, and all three no-hint platform cases.
Independent exact-head validation:
tests/test_broker.py: 74 passed on CPython 3.11.15, 3.12.13, and 3.13.14- scoped reply-hint slice: 6 passed / 68 deselected on each interpreter
- outside-repo adversarial matrix: 24/24 passed on each interpreter (three external platforms × four identity cases, plus web/api/empty gating); it also checked one streaming send, no fallback delivery, prompt identity, unchanged kwargs, and absence of
thread_ts - repository-wide Ruff: PASS
compileall src tests: PASS on all three interpretersgit diff --check, fullgit fsck, exact-path/scope exclusions, and detached cleanliness: PASS
At final pre-submit bind the PR was OPEN, DRAFT, MERGEABLE, merge state UNSTABLE only with checks still in progress; all completed checks were success/skipped and none had failed. Completed successes included Ruff, frontend, CodeQL analyses, and linux/amd64 Docker smoke; Python 3.11–3.13 CI tests and linux/arm64 Docker build remained in progress.
No blocking findings. This PASS is valid only for the exact head/tree/base coordinates above. Any drift requires a wholly fresh detached exact-SHA gauntlet and a new pinned/read-back artifact; do not transfer this result to another head, tree, or base.
Review only: I did not merge, deploy, release, restart/bounce the daemon, or change live configuration. I noted the PR's stated rollout: merge tonight, deploy with the NEXT release only, and no daemon bounce tonight.
Summary
thread(message_id=..., text=...)when an external inbound message is already in a thread and has a message ID.send(chat_id=..., platform=..., text=...)first for top-level messages, withthread(...)described as a quote/thread-reply option.Root cause
The broker always presented flat
send(...)as the primary reply path, even for an inbound thread reply. That wording steered agents to post outside the active thread. Credit to Chekov for the diagnosis, relayed by Brad.Impact
Agents now receive thread-aware reply guidance without changing prompt formatting, delivery behavior, platform plumbing, or the messaging tools themselves. The flat-send fallback is explicitly labeled as posting to the channel outside the thread.
Validation
tests/test_broker.py: 74 passed on Python 3.11, 3.12, and 3.13compileallandgit diff --check: passedRollout
Merge target is tonight. Deploy with the NEXT release only; do not bounce the daemon tonight.