refactor(gateway): consolidate interruption-note builders + close WhatsApp reaction gap - #59318
Closed
aldoeliacim wants to merge 1 commit into
Closed
aldoeliacim wants to merge 1 commit into
aldoeliacim wants to merge 1 commit into
Conversation
…tsApp reaction gap Groundwork for the reply-gate redesign (Phase 0 of 2). Zero behavior change by design — pure refactors, additive adapter methods, and vocabulary consolidation only. - Extract the three near-duplicate '[System note: ...]' resume/interruption bracket-text sites in gateway/run.py into two pure builders (_build_interruption_system_note, _build_tool_tail_system_note) plus one dispatch predicate (_resolve_resume_note_kind). Byte-identical output verified against golden strings captured from the pre-refactor text. - Consolidate the silence-marker vocabulary: cron/scheduler.py now imports gateway.response_filters.LIVE_GATEWAY_SILENT_MARKERS instead of maintaining an independent duplicate frozenset (the two had already drifted once). Cron's positional first/last-line matching logic is unchanged. - Close the WhatsApp reaction gap: send_message(action='react'/'unreact') already probed for add_reaction/remove_reaction on the adapter (existing code on main), but neither WhatsApp adapter implemented them and scripts/whatsapp-bridge/bridge.js had no /react endpoint at all. Added both adapter methods (Baileys bridge + Cloud API), a minimal /react bridge endpoint, and a latestForChat() lookup on the bridge's bounded message store. - New cross-mechanism regression tests: a realistic recovery-report string is never misclassified as an intentional-silence marker, and a genuine silence token on a resumed turn still suppresses correctly (both directions of composition between the resume-note and silence-detection mechanisms, previously untested). Note for reviewers: two related open PRs exist and are NOT duplicated here. #38862 adds WhatsApp reactions via a new send_message(action='react') schema field; this PR is narrower/complementary — it only implements the add_reaction/remove_reaction adapter methods the EXISTING generic react path (already on main) was missing. Our own #21977 (feat/whatsapp-status-reply-primitives) has a similar adapter method on a separate unmerged branch; this was implemented fresh against current main, not derived from that branch. Part of a larger reply-gate/smart-reply architecture redesign; the design doc and Phase 1 (the actual tool-gated delivery mechanism, config-flagged off by default) will follow as a separate PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 0 (of 2) for a reply-gate/smart-reply architecture redesign. This PR is
pure groundwork — zero behavior change by design: refactors, additive
methods, vocabulary consolidation, and new regression tests only. Phase 1
(the actual tool-gated delivery mechanism, config-flagged off by default)
will follow as a separate PR once this lands.
What changed and why
1. Consolidate the interruption/resume system-note construction.
gateway/run.pyhad three near-duplicate sites constructing the[System note: The previous turn was interrupted by ...]bracket text — oneof them (the "safety net" fallback) was a byte-for-byte copy of another's
empty-message branch. Extracted into two pure builders
(
_build_interruption_system_note,_build_tool_tail_system_note) plus onedispatch predicate (
_resolve_resume_note_kind) that formalizes which of thethree sites should fire. Verified byte-identical output via golden-string
tests captured from the pre-refactor text.
2. Consolidate the silence-marker vocabulary.
cron/scheduler.py's_CRON_SILENCE_TOKENSwas an independently-maintainedduplicate of
gateway/response_filters.py'sLIVE_GATEWAY_SILENT_MARKERS—same 4 tokens, but two separate declarations that could silently drift (and
per the inline comments, already had once). Cron now imports the canonical
set lazily (avoiding an eager gateway-package import from cron-only paths);
its own positional first/last-line matching logic is unchanged.
3. Close a real gap in the WhatsApp reaction path.
tools/send_message_tool.pyalready has a genericaction='react'/'unreact'path on
mainthat probes foradd_reaction(chat_id, emoji, message_id)/remove_reaction(chat_id, message_id)on the platform adapter viagetattr— but neither WhatsApp adapter implemented these methods, and
scripts/whatsapp-bridge/bridge.jshad no/reactendpoint at all. Added:add_reaction/remove_reactiononWhatsAppAdapter(Baileys bridge) andWhatsAppCloudAdapter(Graph API reaction message type)POST /reactendpoint on the bridge (empty emoji retracts)latestForChat()on the bridge's bounded message store, so a reaction cantarget "the most recent message in this chat" without an explicit message
id
4. New cross-mechanism regression tests.
No existing test proved the interruption/resume-note mechanism and the
silence-detection mechanism compose correctly. Added:
online now...") is never misclassified as an intentional silence marker
suppresses correctly
How to test
All 37 new/modified Python tests pass; all 24 existing + new JS bridge tests
pass. Full suite run separately confirms no regressions (see verification
note below).
What platforms tested
Linux (WSL2-adjacent dev environment). No platform-specific code paths
touched — the WhatsApp Cloud API reaction method and Baileys bridge endpoint
are both cross-platform Python/Node, no OS-specific syscalls.
Verification note (full suite)
Ran the full test suite via
scripts/run_tests.shboth with and without thisdiff (via
git stash) to confirm no regressions. Baseline on unmodifiedmain: 32 pre-existing test failures + 9tests/acp*collection errors(missing optional
acpextra,pip install hermes-agent[acp]not run inthis dev environment) — identical failure set with or without this diff.
ruff checkandscripts/check-windows-footguns.pyare clean on everytouched file.
Related / overlapping PRs (please read before reviewing)
send_message(action='react')schema field + bridge endpoint. This PR isnarrower and complementary:
send_message's react/unreact action alreadyexists on
maintoday — this PR only implements the two adapter methodsthat path was already trying to call via
getattrand was silentlyfailing to find. No schema changes here.
feat/whatsapp-status-reply-primitives, our own open PR) has asimilar
add_reaction-style adapter method on a separate, unmerged branch.This PR was implemented fresh against current
main, not derived from ordependent on that branch — no relationship beyond solving an adjacent
problem.
Happy to help reconcile with whichever of these a maintainer prefers as
canonical if there's overlap concern at review time.