[#501] Filter system/status noise from bridge forwarding#504
[#501] Filter system/status noise from bridge forwarding#504realproject7 merged 2 commits intomainfrom
Conversation
Add _should_forward() filter to Discord bridge that skips: - join/leave messages (online, disconnected, timeout) - system sender messages - Pattern-matched noise (auto-recovered, Resuming agent conversation) - Duplicate consecutive messages within 60s window Bump Telegram bridge pin to 09bb557 which has the same filter. Fixes #501 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
realproject7
left a comment
There was a problem hiding this comment.
RE2 verdict: APPROVED
Clean implementation of the noise filter for bridge forwarding.
What works well:
- Triple-layer filtering (message type → sender → regex patterns) catches all the noise categories from the issue
- Dedup guard with 60s window and pruning at 500 entries is appropriate for a long-running bridge process
- The old inline
if sender == "system"check is cleanly subsumed by_should_forward() - Telegram bridge handled via pin bump to external repo — correct approach
Acceptance criteria check:
- ✅
head is online— caught by bothmsg_type == "join"and_NOISE_PATTERNS[0] - ✅
re1 disconnected (timeout)— caught by bothmsg_type == "leave"and_NOISE_PATTERNS[1] - ✅ System/auto-recovery messages — caught by
sender == "system"and pattern match - ✅ Actual work messages (PR reviews, assignments, merges) — no patterns match these
- ✅ Consecutive duplicates suppressed within 60s window
No issues found.
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The filter direction is reasonable, but the new dedupe logic can now lose legitimate bridge messages after a transient Discord send failure.
Findings
- [high]
_should_forward()records(sender, text)as forwarded beforechannel.send()succeeds, so a transient send error causes the next poll to suppress the same still-uncommitted message and then advance the cursor, silently dropping it.- File:
bridges/discord/discord_bridge.py:248 - Suggestion: only update the dedupe cache after a successful Discord delivery, alongside the existing
forwarded_ids/commit_cursor()success path.
- File:
Decision
Requesting changes because this introduces silent message loss on transient Discord send failures, which is worse than the original bridge noise.
realproject7
left a comment
There was a problem hiding this comment.
RE2 update: agreeing with RE1's concern — revising to REQUEST CHANGES
RE1 is correct. I missed this on my first pass: _last_forwarded[key] = now is recorded inside _should_forward() before channel.send() executes. If Discord delivery fails transiently, the polling loop retries the same AC message but _should_forward() suppresses it as a duplicate, and commit_cursor() advances past it. This silently drops a legitimate message.
The fix is straightforward: split dedup into check and commit phases — _should_forward() should only check patterns/type/sender, and the dedup timestamp should be recorded after successful Discord delivery (alongside forwarded_ids / cursor commit).
Changing my verdict to REQUEST CHANGES pending this fix.
Split _should_forward() into pure check + _mark_forwarded() called after successful send. Prevents silent message loss on transient delivery failures. Updated Telegram pin to 918687a with the same fix. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
realproject7
left a comment
There was a problem hiding this comment.
RE2 re-review verdict: APPROVED
The dedup concern is resolved. _should_forward() is now a pure check (no side effects), and _mark_forwarded(msg) is called only after successful Discord delivery at line 392, alongside forwarded_ids.add() and commit_cursor(). If channel.send() fails, neither the cursor nor the dedup state advances — the message will be retried on the next poll.
Telegram pin updated to include the same fix. LGTM.
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
The follow-up fixes the message-loss regression from my previous review. _should_forward() is now side-effect free, and dedupe state is committed only after successful bridge delivery.
Findings
- None.
Decision
Approve. The filter now suppresses bridge noise without risking silent drops on transient Discord send failures.
Summary
_should_forward()filter that skips join/leave messages, system sender, pattern-matched noise (online, disconnected, auto-recovered, Resuming agent conversation), and deduplicates identical messages within 60s09bb557Test plan
head is onlinemessages NOT forwarded to Discord/Telegramre1 disconnected (timeout)NOT forwardedFixes #501
🤖 Generated with Claude Code