Skip to content

Conversation

@ggazzo
Copy link
Member

@ggazzo ggazzo commented Oct 9, 2025

Summary by CodeRabbit

  • Refactor

    • Streamlined federation join event handling for improved consistency.
    • Standardized federation API schemas for join responses (state, auth chain, and event) to enhance validation.
    • Exposed the Room Member event schema for broader reuse by integrations.
  • Chores

    • Removed unnecessary type assertions to simplify typing without changing behavior.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 9, 2025

Walkthrough

Type/schema adjustments for federation join handling: removed type assertions in room.service, switched federation API schemas to use PduSchema/EventPduTypeRoomMember, and exported EventPduTypeRoomMember. SendJoinResponseSchema.event is now required and typed, and state/auth_chain arrays now use PduSchema.

Changes

Cohort / File(s) Summary
Federation Room Service typing simplification
packages/federation-sdk/src/services/room.service.ts
Removed generic/unknown casts when constructing PersistentEvent instances during join; directly uses response events. No control-flow changes.
Federation API schema updates
packages/federation-sdk/src/specs/federation-api.ts
Imported PduSchema and EventPduTypeRoomMember. Updated schemas: MakeJoinResponseSchema.event: PduSchema; SendJoinResponseSchema.state/auth_chain: z.array(PduSchema); SendJoinResponseSchema.event: EventPduTypeRoomMember (required).
Room PDU export visibility
packages/room/src/types/v3-11.ts
Promoted EventPduTypeRoomMember from internal const to exported const; structure unchanged.

Sequence Diagram(s)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • debdutdeb
  • rodrigok

Poem

A nibble of types, a hop through the room,
PDUs aligned, schemas go vroom—zoom!
Casts fall away, clearer fields in bloom,
Join flows steady, no hint of doom.
Thump-thump—approved? I’ll assume! 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title accurately and concisely summarizes the main changes by indicating a refactor focused on event type handling in RoomService along with schema updates; it is clear and directly related to the modifications made in the pull request.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/better-result-types

📜 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 1252e71 and f0443e9.

📒 Files selected for processing (3)
  • packages/federation-sdk/src/services/room.service.ts (4 hunks)
  • packages/federation-sdk/src/specs/federation-api.ts (3 hunks)
  • packages/room/src/types/v3-11.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/federation-sdk/src/specs/federation-api.ts (1)
packages/room/src/types/v3-11.ts (2)
  • PduSchema (697-727)
  • EventPduTypeRoomMember (599-603)
🔇 Additional comments (5)
packages/room/src/types/v3-11.ts (1)

599-599: LGTM!

Exporting EventPduTypeRoomMember enables external modules to reference this schema, aligning with the federation-api changes that import it.

packages/federation-sdk/src/specs/federation-api.ts (3)

7-10: LGTM!

Importing EventPduTypeRoomMember and PduSchema from the room package improves type consistency and eliminates duplication.


118-118: LGTM!

Using PduSchema instead of the local MakeJoinEventSchema improves consistency with the room package's type system.


146-148: Approve schema update
Replacing z.any() with PduSchema improves type safety; making event required aligns with the Matrix spec, since /send_join responses always include the event field.

packages/federation-sdk/src/services/room.service.ts (1)

849-849: LGTM! Type assertions cleanly removed.

The removal of type assertions is now safe because the upstream federation-api schemas properly type these fields as PduSchema and EventPduTypeRoomMember. This simplifies the code while maintaining type safety through schema validation.

Also applies to: 867-867, 876-876, 904-904


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.

@ggazzo ggazzo force-pushed the chore/better-result-types branch from b9a5c7c to f0443e9 Compare October 9, 2025 13:18
@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.31%. Comparing base (d85073e) to head (f0443e9).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #269      +/-   ##
==========================================
- Coverage   62.33%   62.31%   -0.02%     
==========================================
  Files          67       67              
  Lines        6385     6387       +2     
==========================================
  Hits         3980     3980              
- Misses       2405     2407       +2     

☔ 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.

@ggazzo ggazzo marked this pull request as ready for review October 9, 2025 13:21
@ggazzo ggazzo merged commit c72fa69 into main Oct 9, 2025
3 checks passed
@ggazzo ggazzo deleted the chore/better-result-types branch October 9, 2025 13:29
@coderabbitai coderabbitai bot mentioned this pull request Nov 5, 2025
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.

3 participants