Skip to content

Conversation

@debdutdeb
Copy link
Member

@debdutdeb debdutdeb commented Sep 26, 2025

FDR-165

RocketChat/Rocket.Chat#37077

Summary by CodeRabbit

  • New Features

    • Improved reply handling with support for rich replies using a nested "in-reply-to" structure and an optional fallback flag.
  • Bug Fixes

    • Aligns reply representation with the official spec to reduce inconsistencies in how replies are parsed and displayed.
  • Chores

    • Updated event content schema to support nested reply metadata for more reliable threading and rendering.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 26, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Adds a new alternative variant to homeserver.matrix.message.content.m.relates_to that supports rich replies via a nested m.in_reply_to object containing event_id and an optional is_falling_back flag; existing rel_type-based variants are retained.

Changes

Cohort / File(s) Summary
Matrix message schema update
packages/federation-sdk/src/index.ts
Added a new variant under homeserver.matrix.message.content.m.relates_to for rich replies: a nested 'm.in_reply_to': { event_id: EventID } plus optional is_falling_back?: boolean. Included SPEC comment documenting the m.in_reply_to approach. No existing variants removed.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Client as Matrix Client
  participant SDK as Federation SDK (types)
  participant Consumer as Downstream Consumer

  Client->>SDK: Send homeserver.matrix.message (content)
  alt Rich reply (new)
    Note over SDK: content includes "m.relates_to": {"m.in_reply_to":{event_id}}
    SDK-->>Consumer: Message typed with nested m.in_reply_to
    Consumer->>Consumer: Inspect m.in_reply_to.event_id for reply handling
  else Legacy rel_type (existing)
    Note over SDK: content uses rel_type / event_id variants
    SDK-->>Consumer: Message typed with rel_type-based relation
    Consumer->>Consumer: Handle according to rel_type semantics
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

In tunnels of dots I nibble and peep,
I tuck an event_id for replies to keep.
is_falling_back hums if the old voice is near,
I hop through the schema — message threads clear.
🥕🐰

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Linked Issues Check ⚠️ Warning This PR only adds TypeScript definitions for nested reply properties and does not include any logic to parse or render thread messages differently, so it does not address the symptom of Synapse thread messages appearing as quotes described in FDR-165. The linked issue requires functional changes to message handling and rendering in Rocket.Chat rather than type updates alone. As such, the objectives from FDR-165 remain unmet. Introduce the runtime parsing and rendering logic for m.in_reply_to events so that thread messages from Synapse are displayed correctly instead of as quote blocks, and update tests to verify the new behavior per FDR-165.
Title Check ❓ Inconclusive The title indicates that a type for rich messages was added but is overly generic and does not specify the actual nested m.in_reply_to structure or is_falling_back flag introduced in the message event signature. It also labels the change as a chore, which may understate its significance to message handling functionality. This vagueness makes it unclear at a glance what the core update entails. Rename the title to explicitly reference the new reply semantics properties, for example “types: add m.in_reply_to and is_falling_back fields to message content for rich replies.”
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Out of Scope Changes Check ✅ Passed All modifications are limited to the homeserver.matrix.message type definitions to include nested reply properties and no other files or unrelated features have been altered, so there are no out-of-scope changes.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 979f00e and 43f37b2.

📒 Files selected for processing (1)
  • packages/federation-sdk/src/index.ts (1 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@codecov-commenter
Copy link

codecov-commenter commented Sep 26, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.67%. Comparing base (5a69089) to head (43f37b2).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #231   +/-   ##
=======================================
  Coverage   81.67%   81.67%           
=======================================
  Files          63       63           
  Lines        4682     4682           
=======================================
  Hits         3824     3824           
  Misses        858      858           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@debdutdeb debdutdeb marked this pull request as ready for review September 26, 2025 17:23
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 5a69089 and 979f00e.

📒 Files selected for processing (1)
  • packages/federation-sdk/src/index.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/federation-sdk/src/index.ts (1)
packages/room/src/types/_common.ts (1)
  • EventID (8-8)

@ggazzo ggazzo merged commit 2541e9e into main Sep 26, 2025
2 of 3 checks passed
@ggazzo ggazzo deleted the chore/message-event-signature-impr branch September 26, 2025 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants