Skip to content

Mark active wave only as read#1583

Merged
prxt6529 merged 4 commits intomainfrom
mark-as-read-only-active-wave
Oct 30, 2025
Merged

Mark active wave only as read#1583
prxt6529 merged 4 commits intomainfrom
mark-as-read-only-active-wave

Conversation

@prxt6529
Copy link
Copy Markdown
Collaborator

@prxt6529 prxt6529 commented Oct 30, 2025

Summary by CodeRabbit

  • Enhancements

    • Real-time updates now respect which wave you’re actively viewing.
    • Notifications for a wave are only removed and marked read when that wave is active.
    • Mark-as-read behavior is invoked immediately for active waves.
  • Tests

    • Added tests verifying notification removal and mark-as-read occur only for the active wave and not for others.

Signed-off-by: prxt6529 <prxt@6529.io>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Oct 30, 2025

Walkthrough

Added activeWaveId to the wave realtime updater hook and threaded it through initialization; the hook now only removes delivered notifications and marks a wave as read when an incoming drop’s wave matches activeWaveId. Tests and API mocks were updated to cover the new behavior.

Changes

Cohort / File(s) Change Summary
Context initialization
contexts/wave/MyStreamContext.tsx
Added imports (types/utilities) and passed activeWaveId into useWaveRealtimeUpdater; adjusted import ordering.
Real-time updater hook
contexts/wave/hooks/useWaveRealtimeUpdater.ts
Added activeWaveId: string | null to UseWaveRealtimeUpdaterProps; threaded activeWaveId into hook params; conditionally remove delivered notifications and call markWaveAsRead only when drop.waveId === activeWaveId; removed debounce around mark-as-read; updated imports and dependency arrays.
Tests & API mocks
__tests__/useWaveRealtimeUpdater.test.ts, .../services/api/common-api
Exported new commonApiPostWithoutBodyAndResponse mock in common-api; added activeWaveId to test setup; added tests verifying active-wave read/removal behavior and that non-active waves are skipped; adjusted test wiring for new export.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Context as MyStreamContext
    participant Hook as useWaveRealtimeUpdater
    participant WS as WebSocket
    participant API as Notifications API

    rect rgb(240,248,255)
    Context->>Hook: init(activeWaveId)
    end

    WS->>Hook: incoming drop (waveId)
    Hook->>Hook: compare drop.waveId == activeWaveId?
    alt Match (active)
        Hook->>API: removeWaveDeliveredNotifications(waveId)
        Hook->>API: POST /notifications/wave/{waveId}/read
        API-->>Hook: 200 OK
    else No match (non-active)
        Note over Hook: skip delivered-notifications removal and read POST
    end
    Hook-->>Context: apply drop to state
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Inspect activeWaveId falsy/null handling and equality comparison logic.
  • Verify updated dependency arrays to avoid stale closures.
  • Review removal of debounce for markWaveAsRead for potential duplicate or rate issues.
  • Confirm tests and new commonApiPostWithoutBodyAndResponse mock match runtime contract.

Suggested reviewers

  • simo6529
  • ragnep

Poem

🐰 I hopped through webs and waves today,
Active IDs now guide my way.
When a drop lands where I can see,
I clear the crumbs and mark it free.
Quick hops, clean trails — hooray! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "Mark active wave only as read" directly captures the core functionality introduced in this changeset. The primary change across all three modified files is adding conditional logic to mark waves as read only when they match the active wave ID (activeWaveId), rather than marking all incoming waves as read. The title is concise, specific, and avoids vague language while clearly communicating the main behavioral change that a developer scanning the git history would need to understand.
✨ 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 mark-as-read-only-active-wave

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7459f86 and 9488dfc.

📒 Files selected for processing (1)
  • contexts/wave/hooks/useWaveRealtimeUpdater.ts (4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{ts,tsx}: Do not include any comments in the code
Use react-query for data fetching
Always add readonly before props

Use TypeScript across the codebase

Files:

  • contexts/wave/hooks/useWaveRealtimeUpdater.ts
🧬 Code graph analysis (1)
contexts/wave/hooks/useWaveRealtimeUpdater.ts (2)
contexts/wave/hooks/types.ts (1)
  • WaveDataStoreUpdater (5-9)
services/api/common-api.ts (1)
  • commonApiPostWithoutBodyAndResponse (193-209)
⏰ 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: Analyze (javascript-typescript)
🔇 Additional comments (1)
contexts/wave/hooks/useWaveRealtimeUpdater.ts (1)

238-250: Stale closure resolved.

Adding activeWaveId and removeWaveDeliveredNotifications to the dependency array keeps processIncomingDrop aligned with the active wave, fixing the stale capture called out earlier. Nicely handled.


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.

Signed-off-by: prxt6529 <prxt@6529.io>
Signed-off-by: prxt6529 <prxt@6529.io>
Copy link
Copy Markdown

@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

📥 Commits

Reviewing files that changed from the base of the PR and between 4f0eaa2 and 7459f86.

📒 Files selected for processing (2)
  • __tests__/useWaveRealtimeUpdater.test.ts (3 hunks)
  • contexts/wave/hooks/useWaveRealtimeUpdater.ts (4 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{ts,tsx}: Do not include any comments in the code
Use react-query for data fetching
Always add readonly before props

Use TypeScript across the codebase

Files:

  • contexts/wave/hooks/useWaveRealtimeUpdater.ts
  • __tests__/useWaveRealtimeUpdater.test.ts
__tests__/**

📄 CodeRabbit inference engine (tests/AGENTS.md)

Place Jest test suites under the __tests__ directory mirroring source folders (e.g., components, contexts, hooks, utils)

Files:

  • __tests__/useWaveRealtimeUpdater.test.ts
**/__tests__/**

📄 CodeRabbit inference engine (AGENTS.md)

Place tests in __tests__ directories when organizing test suites

Files:

  • __tests__/useWaveRealtimeUpdater.test.ts
**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Mock external dependencies and APIs in tests

Files:

  • __tests__/useWaveRealtimeUpdater.test.ts
🧠 Learnings (9)
📚 Learning: 2025-09-28T12:33:07.561Z
Learnt from: CR
PR: 6529-Collections/6529seize-frontend#0
File: __mocks__/AGENTS.md:0-0
Timestamp: 2025-09-28T12:33:07.561Z
Learning: Applies to __mocks__/**/__mocks__/**/*.{js,jsx,ts,tsx} : Keep mocks up to date with the real implementations they represent

Applied to files:

  • __tests__/useWaveRealtimeUpdater.test.ts
📚 Learning: 2025-10-23T06:36:34.125Z
Learnt from: CR
PR: 6529-Collections/6529seize-frontend#0
File: AGENTS.md:0-0
Timestamp: 2025-10-23T06:36:34.125Z
Learning: Applies to **/*.test.{ts,tsx} : Mock external dependencies and APIs in tests

Applied to files:

  • __tests__/useWaveRealtimeUpdater.test.ts
📚 Learning: 2025-09-28T12:33:07.561Z
Learnt from: CR
PR: 6529-Collections/6529seize-frontend#0
File: __mocks__/AGENTS.md:0-0
Timestamp: 2025-09-28T12:33:07.561Z
Learning: Applies to __mocks__/**/__mocks__/**/*.{js,jsx,ts,tsx} : Document non-obvious expected behaviour directly in the mock file

Applied to files:

  • __tests__/useWaveRealtimeUpdater.test.ts
📚 Learning: 2025-09-28T12:33:07.561Z
Learnt from: CR
PR: 6529-Collections/6529seize-frontend#0
File: __mocks__/AGENTS.md:0-0
Timestamp: 2025-09-28T12:33:07.561Z
Learning: Applies to __mocks__/**/__mocks__/**/*.{js,jsx,ts,tsx} : Keep mock implementations minimal—only what’s necessary for the test scenarios

Applied to files:

  • __tests__/useWaveRealtimeUpdater.test.ts
📚 Learning: 2025-09-28T12:33:07.561Z
Learnt from: CR
PR: 6529-Collections/6529seize-frontend#0
File: __mocks__/AGENTS.md:0-0
Timestamp: 2025-09-28T12:33:07.561Z
Learning: Applies to __mocks__/**/__mocks__/**/*.{js,jsx,ts,tsx} : Review mocks periodically and remove unused mock modules

Applied to files:

  • __tests__/useWaveRealtimeUpdater.test.ts
📚 Learning: 2025-09-28T12:33:07.561Z
Learnt from: CR
PR: 6529-Collections/6529seize-frontend#0
File: __mocks__/AGENTS.md:0-0
Timestamp: 2025-09-28T12:33:07.561Z
Learning: Applies to __mocks__/**/__mocks__/**/*.{js,jsx,ts,tsx} : Mock only external dependencies or heavy functionality; avoid over-mocking internal logic

Applied to files:

  • __tests__/useWaveRealtimeUpdater.test.ts
📚 Learning: 2025-09-28T12:33:07.561Z
Learnt from: CR
PR: 6529-Collections/6529seize-frontend#0
File: __mocks__/AGENTS.md:0-0
Timestamp: 2025-09-28T12:33:07.561Z
Learning: Applies to __mocks__/**/__mocks__/**/*.{js,jsx,ts,tsx} : Organise mocks to mirror the real module structure so import paths remain consistent

Applied to files:

  • __tests__/useWaveRealtimeUpdater.test.ts
📚 Learning: 2025-09-28T12:33:07.561Z
Learnt from: CR
PR: 6529-Collections/6529seize-frontend#0
File: __mocks__/AGENTS.md:0-0
Timestamp: 2025-09-28T12:33:07.561Z
Learning: Applies to __mocks__/**/*.{test,spec}.{js,jsx,ts,tsx} : In tests, use jest.mock('module') with a bare module specifier to load the corresponding manual mock

Applied to files:

  • __tests__/useWaveRealtimeUpdater.test.ts
📚 Learning: 2025-09-28T12:33:07.561Z
Learnt from: CR
PR: 6529-Collections/6529seize-frontend#0
File: __mocks__/AGENTS.md:0-0
Timestamp: 2025-09-28T12:33:07.561Z
Learning: Use Jest’s built-in mocking for module replacement; keep manual mocks simple and lightweight

Applied to files:

  • __tests__/useWaveRealtimeUpdater.test.ts
🧬 Code graph analysis (2)
contexts/wave/hooks/useWaveRealtimeUpdater.ts (2)
contexts/wave/hooks/types.ts (1)
  • WaveDataStoreUpdater (5-9)
services/api/common-api.ts (1)
  • commonApiPostWithoutBodyAndResponse (193-209)
__tests__/useWaveRealtimeUpdater.test.ts (2)
contexts/wave/hooks/useWaveRealtimeUpdater.ts (1)
  • useWaveRealtimeUpdater (37-307)
services/api/common-api.ts (1)
  • commonApiPostWithoutBodyAndResponse (193-209)

Comment thread contexts/wave/hooks/useWaveRealtimeUpdater.ts
Signed-off-by: prxt6529 <prxt@6529.io>
@sonarqubecloud
Copy link
Copy Markdown

@prxt6529 prxt6529 merged commit b2242d6 into main Oct 30, 2025
8 checks passed
@prxt6529 prxt6529 deleted the mark-as-read-only-active-wave branch October 30, 2025 09:04
@coderabbitai coderabbitai Bot mentioned this pull request Oct 31, 2025
@coderabbitai coderabbitai Bot mentioned this pull request Nov 17, 2025
@coderabbitai coderabbitai Bot mentioned this pull request Dec 17, 2025
@coderabbitai coderabbitai Bot mentioned this pull request Dec 29, 2025
This was referenced Apr 6, 2026
@coderabbitai coderabbitai Bot mentioned this pull request Apr 21, 2026
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.

2 participants