Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused Buzz threading work. The current-main premise is valid: plugins/platforms/buzz/adapter.py:1012 still gates every unmentioned non-DM message, while this PR adds the requested active-thread exception.
Problems
- The public configuration documentation is incomplete.
website/docs/user-guide/messaging/buzz.md:47-57lists the Buzz environment-variable interface, andwebsite/docs/user-guide/messaging/buzz.md:82,98-102describes only the existing strict mention behavior. This PR addsthread_require_mention/BUZZ_THREAD_REQUIRE_MENTION, but does not update either the canonical Buzz guide orwebsite/docs/reference/environment-variables.md.
Suggested changes
- Document the YAML and environment forms, precedence, strict defaults, and the rule that only threads with a successful Hermes reply permit unmentioned follow-ups.
This is an automated hermes-sweeper review.
| # buzz: | ||
| # extra: | ||
| # require_mention: true # Require @mention in shared channels (default: true) | ||
| # thread_require_mention: true # Require @mention in active threads (default: true) |
There was a problem hiding this comment.
Please also update the canonical Buzz guide and environment-variable reference. website/docs/user-guide/messaging/buzz.md:47-57 currently enumerates the supported Buzz variables, and its mention-policy text at lines 82 and 98-102 does not describe active-thread follow-ups or this new override.
There was a problem hiding this comment.
Addressed in c9ac06279; the canonical Buzz guide and environment-variable reference are now updated.
|
Added a follow-up for the Dashboard integration boundary: live mention-policy refresh now reads canonical gateway.platforms.buzz.extra.* values while retaining legacy buzz.extra.* precedence until a |
Heads-up: regression on
|
| Base | Top-level | Threaded reply |
|---|---|---|
main |
dispatches | dispatches |
| this PR | dispatches | dropped |
The failing assertion is:
# adapter with require_mention: false, thread_require_mention left at default
assert [d["message_id"] for d in adapter._dispatched] == ["root1", "reply1"]
# this PR: AssertionError: assert ['root1'] == ['root1', 'reply1']Cause
The gate returns early on any un-mentioned reply when thread_require_mention is true (the default), before consulting require_mention:
if not is_dm and not self._is_mentioned(content):
if reply_target and self.thread_require_mention:
return # ← fires even when require_mention is False
if self.require_mention and not in_agent_thread:
...So the new setting at its default tightens behavior for existing free-listening installs — every threaded reply that used to dispatch is now silently dropped. The PR body claims strict backward-compatible defaults; this breaks that for the require_mention: false path.
Suggested fix
Keep require_mention as the outer condition (the Discord shape): only evaluate thread_require_mention / in_agent_thread when the top-level gate is actually on. That way require_mention: false still means free-listening everywhere, and the new setting can only ever relax the gate.
I opened a smaller alternative that does this: #TBD (will edit with the number once live). Happy to close mine in favor of this PR once the regression is fixed — the Dashboard surface here is valuable either way.
|
Follow-up: the alternative PR is now up at #83775. |
Signed-off-by: Reinhold <310554180+reinhold-ph@users.noreply.github.com>
7ced04b to
19c69ea
Compare
|
Thanks @gering — confirmed. The report exposed a real backward-compatibility regression in the I added a focused RED test on the previous head: an unmentioned top-level message dispatched, but its unmentioned threaded reply failed with The PR has been rebased onto current Verification on the rebased head:
Thanks for providing the exact matrix case and for cross-linking #83775. This PR keeps its broader Dashboard and hot-reload scope while adopting the backward-compatible outer-gate semantics. |
|
Superseded by #91020, which carries this behavior forward as part of the profile-scoped Buzz policy and Config integration on current |
What does this PR do?
Adds configurable Buzz mention policy with strict backward-compatible defaults. Shared-channel messages can continue requiring an initial mention while follow-ups are allowed only in threads where Hermes has successfully participated.
The adapter preserves explicit environment-variable precedence, hot-reloads Dashboard-saved YAML policy, resolves nested replies to the thread root, reconstructs participation after restart, and handles self-echo ordering without permanently dropping follow-ups. DMs and allowlists are unchanged.
Related Issue
N/A — this is separate from dynamic joined-channel discovery in #74823 and from the top-level reply behavior proposed in #74985.
Type of Change
Changes Made
buzz.extra.require_mentionandbuzz.extra.thread_require_mentiondefaults inhermes_cli/config_defaults.pyandcli-config.yaml.example.BuzzIconcomponent for the config page, using the supplied canonical 24×24 Buzz asset.web/src/components/AutoField.tsx.plugins/platforms/buzz/adapter.pywhile preserving explicit environment precedence and last-known-good policy.require_mentionisfalse, unmentioned top-level and threaded messages both continue to dispatch regardless of the thread-policy default.How to Test
scripts/run_tests.sh tests/gateway/test_buzz_adapter.py -q(40 passed).scripts/run_tests.sh tests/hermes_cli/test_web_server.py -q(146 passed).web/, runnpm run typecheck,npm test,npm run lint, andnpm run build(199 tests passed; typecheck/build passed; ESLint reported no errors).Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passFocused backend and complete frontend suites were run; the repository-wide Python suite was not run, so that checkbox is intentionally left unchecked.
Documentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/ACONTRIBUTING.md/AGENTS.mdand tool descriptions are N/A. Configuration documentation is included incli-config.yaml.example; runtime behavior is documented in adapter comments/docstrings.Screenshots / Logs
N/A. Dashboard schema, icon layout, and accessible switch output are covered by frontend tests.