docs: expand peer-to-peer chat spec - #26
Conversation
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughUpdated P2P chat system documentation by expanding the specification scope to detail routes, providers, UI components, services, and lifecycle management. Updated navigation documentation cross-references and README index entry to reflect the expanded specification. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.specify/v1-reference/P2P_CHAT_SYSTEM.md (1)
333-335: Clarify badge decrement/reset ownership to avoid migration ambiguity.Line 334 says external handlers are “expected” to increment/decrement
chatCountProvider; adding one explicit rule for when decrement/reset happens (e.g., on room open vs. on mark-as-read success) would prevent inconsistent implementations in v2.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.specify/v1-reference/P2P_CHAT_SYSTEM.md around lines 333 - 335, Clarify ownership by making badge increments the responsibility of incoming-message handlers (push/background) and making badge decrements/resets exclusively handled by the read-confirmation path: when markChatAsRead() succeeds (e.g., on room open or explicit mark-as-read completion) it must update ChatReadStatusService and then call a single badge-update entrypoint (e.g., chatCountProvider.notifier.resetForChat/refreshBadge) rather than letting external handlers decrement directly; update the documentation to state that BottomNavBar reads chatCountProvider only and that ChatRoomNotifier._onChatEvent(), sendMessage(), and any push handlers should call chatRoomsNotifierProvider.notifier.refreshChatList() and only increment the provider for new outside-room messages, while all decrements/resets flow through markChatAsRead()/the dedicated reset API to avoid migration ambiguity..specify/v1-reference/NAVIGATION_ROUTES.md (1)
50-51: Consider normalizing link text style for consistency.Optional: use the same link-text convention as neighboring rows (short relative doc names) to keep the table visually uniform.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.specify/v1-reference/NAVIGATION_ROUTES.md around lines 50 - 51, Normalize the link text in the two table rows for `/chat_list` (ChatRoomsScreen / chat_rooms_list.dart) and `/chat_room/:orderId` (ChatRoomScreen / chat_room_screen.dart) by replacing the long `.specify/v1-reference/P2P_CHAT_SYSTEM.md` link target with the short relative doc name used by neighboring rows (e.g., `P2P_CHAT_SYSTEM.md`) so both rows use the same concise link-text convention.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.specify/v1-reference/NAVIGATION_ROUTES.md:
- Around line 50-51: Normalize the link text in the two table rows for
`/chat_list` (ChatRoomsScreen / chat_rooms_list.dart) and `/chat_room/:orderId`
(ChatRoomScreen / chat_room_screen.dart) by replacing the long
`.specify/v1-reference/P2P_CHAT_SYSTEM.md` link target with the short relative
doc name used by neighboring rows (e.g., `P2P_CHAT_SYSTEM.md`) so both rows use
the same concise link-text convention.
In @.specify/v1-reference/P2P_CHAT_SYSTEM.md:
- Around line 333-335: Clarify ownership by making badge increments the
responsibility of incoming-message handlers (push/background) and making badge
decrements/resets exclusively handled by the read-confirmation path: when
markChatAsRead() succeeds (e.g., on room open or explicit mark-as-read
completion) it must update ChatReadStatusService and then call a single
badge-update entrypoint (e.g.,
chatCountProvider.notifier.resetForChat/refreshBadge) rather than letting
external handlers decrement directly; update the documentation to state that
BottomNavBar reads chatCountProvider only and that
ChatRoomNotifier._onChatEvent(), sendMessage(), and any push handlers should
call chatRoomsNotifierProvider.notifier.refreshChatList() and only increment the
provider for new outside-room messages, while all decrements/resets flow through
markChatAsRead()/the dedicated reset API to avoid migration ambiguity.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b5220dd9-5b1d-47ba-850a-0fdc54d3baa1
📒 Files selected for processing (3)
.specify/v1-reference/NAVIGATION_ROUTES.md.specify/v1-reference/P2P_CHAT_SYSTEM.md.specify/v1-reference/README.md
Summary
Adds a full mapping for section 7 (PEER-TO-PEER CHAT), covering
/chat_list,/chat_room/:orderId, providers, navigation, lifecycle, and media handling.Updated docs
P2P_CHAT_SYSTEM.mdNAVIGATION_ROUTES.md/chat_list+/chat_roomrows now reference the chat specREADME.mdAll prose in English per the v1 spec refresh instructions.
Summary by CodeRabbit