Skip to content

fix: Chat backfill would send notifications#165

Merged
highesttt merged 2 commits into
mainfrom
chat-backfill-notifications
Jun 3, 2026
Merged

fix: Chat backfill would send notifications#165
highesttt merged 2 commits into
mainfrom
chat-backfill-notifications

Conversation

@highesttt

Copy link
Copy Markdown
Collaborator

No description provided.

@indent-zero

indent-zero Bot commented May 30, 2026

Copy link
Copy Markdown
Contributor
PR Summary

Fixes a UX bug where unblocking a LINE contact fired a Matrix push notification for every old message that got re-bridged into the recreated DM portal. The repopulation now goes through the bridgev2 forward-backfill API (FetchMessages + MarkRead: true), so prior history is batch-sent silently and the chat is marked read instead of being delivered as fresh live messages.

  • LineClient now implements bridgev2.BackfillingNetworkAPI; FetchMessages calls GetRecentMessagesV2, reverses to oldest-first, decrypts/converts with the shared helpers, and returns MarkRead: true. Backward fetches are a no-op (HasMore: false) since LINE has no pagination here.
  • OpUnblockContact replaces the goroutine that ran backfillRecentMessages with queueDMChatResync(create=true) + a new queueDMBackfill(mid). The follow-up ChatResync carries a CheckNeedsBackfillFunc that forces a forward backfill, which is required because the framework skips the backfill check on the resync that creates the portal.
  • pkg/connector/handle_message.go is refactored to extract isBridgeableContentType, portalMIDForMessage, parseMessageTimestamp, decryptMessageBody, and convertLineMessage, so the live and backfill paths share the same decode/decrypt/convert pipeline (behavior-preserving).

Issues

All clear! No issues remaining. 🎉

1 issue already resolved
  • Stale doc comment on backfillRecentMessages still lists OpUnblockContact as a caller, but this PR removed that callsite — only prefetchMessages remains. (fixed by commit 3101b9e)

CI Checks

Waiting for CI checks...

@coderabbitai

coderabbitai Bot commented May 30, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a22a8fed-73c8-4cda-8bab-e29fef71485e

📥 Commits

Reviewing files that changed from the base of the PR and between c933fd2 and 3101b9e.

📒 Files selected for processing (2)
  • pkg/connector/handle_message.go
  • pkg/connector/sync.go
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: build-docker
  • GitHub Check: build-docker
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

**/*.go: Use go fmt for code formatting across all Go files
Use goimports with -local "github.com/highesttt/matrix-line-messenger" flag to group project-local imports correctly
Use zerolog for logging throughout the codebase
Do not use Msgf in logging; use Msg with structured fields instead
Use Stringer interface where applicable in Go code

Files:

  • pkg/connector/sync.go
  • pkg/connector/handle_message.go
**/!(ltsm)/**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

**/!(ltsm)/**/*.go: Run staticcheck on all Go files excluding pkg/ltsm package (transpiled WASM code)
Run go vet on all Go files excluding pkg/ltsm package (transpiled WASM code)

Files:

  • pkg/connector/sync.go
  • pkg/connector/handle_message.go
🔇 Additional comments (3)
pkg/connector/handle_message.go (2)

115-120: LGTM!


305-308: LGTM!

pkg/connector/sync.go (1)

238-240: LGTM!


📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Automatic silent message history backfill after unblocking contacts; improved DM resync behavior.
  • Bug Fixes

    • Better filtering of unsupported message types while preserving call/contact notifications.
    • Improved mention detection and resolution (including room mentions) and more accurate emoji/stamp rendering.
    • Fewer duplicate/empty messages by skipping whitespace-only content and preserving correct message ordering.

Walkthrough

This PR implements the BackfillingNetworkAPI for the LINE connector, centralizes message filtering, timestamp parsing, and decryption, extracts a unified convertLineMessage pipeline, and adds LineClient.FetchMessages plus a queued DM backfill integrated into the unblock flow.

Changes

BackfillingNetworkAPI Implementation and Message Handling Refactor

Layer / File(s) Summary
Interface compliance declaration
pkg/connector/client.go
Compile-time assertion confirms LineClient implements bridgev2.BackfillingNetworkAPI.
Message filtering and decryption refactoring
pkg/connector/handle_message.go
queueIncomingMessage gates processing via isBridgeableContentType and returns early for unsupported types. parseMessageTimestamp centralizes CreatedTime parsing with fallback to time.Now(). decryptMessageBody now returns both bodyText and unwrappedText from a single call, initializing unwrappedText from bodyText. portalMIDForMessage now returns only portalIDStr.
Message conversion pipeline
pkg/connector/handle_message.go
New convertLineMessage method extracted from queued-event closure; dispatches by ContentType, handles ORGCONTP call/contact cases, supports inline emoji/stamp metadata, skips whitespace-only messages, and processes MENTION metadata to populate Mentions and HTML formatted bodies.
DM backfill API implementation
pkg/connector/sync.go
New exported FetchMessages method: forward-only semantics with default Count 50, token-refresh retry on fetch, reverses newest-first to oldest-first, filters bridgeable content, resolves per-message intent via Portal.GetIntentFor, decrypts/converts messages, caches group members for system messages, and returns HasMore: false with MarkRead: true. queueDMBackfill enqueues a ChatResync forcing a forward backfill.
Unblock contact handler integration
pkg/connector/sync.go
OpUnblockContact now queues queueDMChatResync(ctx, mid, true) then queueDMBackfill(mid), replacing the previous goroutine-based call to backfillRecentMessages.

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive No description was provided by the author, making it impossible to assess relevance to the changeset. Add a pull request description explaining the issue being fixed and how the changes address the notification problem during chat backfill.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main fix: preventing chat backfill from sending notifications, which aligns with the primary changes across all three modified files.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chat-backfill-notifications

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/connector/handle_message.go`:
- Around line 324-379: MENTION span indices S/E are character offsets but the
code slices unwrappedText and body using byte indices; convert character offsets
to byte offsets before storing in mentionEntry (used in the entries slice) and
before doing any slicing/replacements. Specifically, when parsing ment.S/ment.E
and creating entries in the blocks that append mentionEntry (and when handling
"`@room`" entries), map the rune index s and e to byte offsets into unwrappedText
(or into the current part body) using a rune-to-byte conversion (walk the string
with utf8.DecodeRuneInString or build []rune->byte mapping) and validate bounds
against len(unwrappedText) in bytes; then store those byte offsets in
mentionEntry.start/end so the subsequent sorting,
html.EscapeString(unwrappedText[lastEnd:entry.start]) and body = body[:start]+
"`@room`" + body[end:] use correct byte ranges.
- Around line 107-118: The code currently converts msg.CreatedTime to tsInt and
then uses time.UnixMilli(tsInt).IsZero() to decide the fallback, but
time.UnixMilli(0) is the Unix epoch (not Go's zero time), so zero timestamps
still get bridged as 1970-01-01; change the fallback check to examine the
integer timestamp itself (e.g., if tsInt == 0 or tsInt <= 0) before constructing
ts and return time.Now() in that case, keeping the existing error-path that logs
the Int64 conversion failure; update references around msg.CreatedTime, tsInt,
time.UnixMilli and the subsequent return time.Now() accordingly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 265915a6-9540-4901-8a98-025fe818ee8e

📥 Commits

Reviewing files that changed from the base of the PR and between 3142f0f and c933fd2.

📒 Files selected for processing (3)
  • pkg/connector/client.go
  • pkg/connector/handle_message.go
  • pkg/connector/sync.go
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build-docker
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

**/*.go: Use go fmt for code formatting across all Go files
Use goimports with -local "github.com/highesttt/matrix-line-messenger" flag to group project-local imports correctly
Use zerolog for logging throughout the codebase
Do not use Msgf in logging; use Msg with structured fields instead
Use Stringer interface where applicable in Go code

Files:

  • pkg/connector/client.go
  • pkg/connector/sync.go
  • pkg/connector/handle_message.go
**/!(ltsm)/**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

**/!(ltsm)/**/*.go: Run staticcheck on all Go files excluding pkg/ltsm package (transpiled WASM code)
Run go vet on all Go files excluding pkg/ltsm package (transpiled WASM code)

Files:

  • pkg/connector/client.go
  • pkg/connector/sync.go
  • pkg/connector/handle_message.go
🔇 Additional comments (1)
pkg/connector/client.go (1)

137-141: LGTM!

Comment thread pkg/connector/handle_message.go
Comment thread pkg/connector/handle_message.go

@Fizzadar Fizzadar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of tweaks!

Comment thread pkg/connector/handle_message.go Outdated
Comment thread pkg/connector/handle_message.go Outdated
@highesttt highesttt requested a review from Fizzadar June 1, 2026 22:06
@highesttt highesttt merged commit e1e585e into main Jun 3, 2026
10 checks passed
@highesttt highesttt deleted the chat-backfill-notifications branch June 3, 2026 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants