Skip to content

fix(feed): stop the update stream from replaying items the list already holds - #133

Merged
lopatnov merged 5 commits into
mainfrom
refactor/maintain-feed-stream-replay
Sep 5, 2026
Merged

fix(feed): stop the update stream from replaying items the list already holds#133
lopatnov merged 5 commits into
mainfrom
refactor/maintain-feed-stream-replay

Conversation

@lopatnov

@lopatnov lopatnov commented Sep 5, 2026

Copy link
Copy Markdown
Owner

Summary

  • The personal feed's live-update stream is ordered by arrival, not by publish date: a broadcast batch of new articles arrives newest-first, so as each is prepended the batch's oldest article ends up on top of the list (items[0])
  • Reconnecting asked the server to replay from items[0].publishedAt, which replayed the rest of that same batch on every reconnect — duplicate rows (and duplicate React keys), an inflated unread badge, and the replayed copy showing as unread even when the original had been read
  • Now replays from the newest publishedAt currently in the list instead of items[0], and prependItem no-ops for an id already present as a second line of defense against any other double delivery
  • Also: the stream now retries when it ends cleanly, not only on error — a server restart could close it without surfacing an error, which previously left the page with no live updates until a reload

Related issue

Found by architect during this cycle's /maintain refactor analysis while re-verifying repo-scout's signals; not tied to a pre-existing issue.

Type of change

  • Bug fix

Checklist

  • dotnet build --configuration Release passes with 0 errors (unaffected by this branch)
  • cd src/pressmark-web && npm run build passes with 0 TypeScript errors
  • New behavior is covered by tests — feedStore.test.ts (new: prependItem dedup) and FeedPage.test.tsx (extended: reconnect replay point end-to-end, including the clean-end retry) — 42/42 frontend tests pass
  • All UI strings use t('ns:key') — no strings touched

Notes for reviewers

Independently verified (build, full test suite, typecheck, lint, format) before opening this PR, separately from the architect/tester agents that authored the fix and tests.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Hnoh6YfJwp63szeP8ZFqVg


Generated by Claude Code

…dy holds

The list is ordered by arrival, not by publish date: a live update is prepended
as it comes in and a batch of new articles is broadcast newest-first, so the item
left on top is the batch's oldest. Reconnecting asked the server to replay from
that item's timestamp, so every other article of the batch came back and was
prepended a second time — duplicate rows and duplicate React keys, an inflated
unread badge, and the replayed copy showing as unread even when the original had
been read (the catch-up projection reports no read/like/bookmark state by design).

Ask the server to replay from the newest timestamp in the list instead of from the
top item, and drop an already-known id in prependItem so any other double delivery
cannot duplicate a row either.

Also retry the stream when it ends cleanly, not only when it errors: a server
restart can close it without surfacing an error, which left the page with no live
updates until a reload, contradicting the effect's own comment.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hnoh6YfJwp63szeP8ZFqVg
Adds a regression test for the duplicate-row fix in
90d45c4:

- feedStore.test.ts: prependItem is a no-op for an id already in the list,
  and still prepends a genuinely new id afterwards.
- FeedPage.test.tsx: drives the stream's reconnect path end-to-end (a
  newest-first batch, a clean stream end, the 5s backoff retry) and asserts
  the reconnect replays from the newest publishedAt in the list rather than
  items[0] — the batch's oldest article, left on top by arrival order.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hnoh6YfJwp63szeP8ZFqVg
@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 37 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Team

Run ID: ded2c063-e4c2-4a1e-82c8-6046653e10ff

📥 Commits

Reviewing files that changed from the base of the PR and between 73ed79b and cb4b102.

📒 Files selected for processing (5)
  • CHANGELOG.md
  • src/pressmark-web/src/hooks/useFeedPage.ts
  • src/pressmark-web/src/pages/FeedPage.test.tsx
  • src/pressmark-web/src/store/feedStore.test.ts
  • src/pressmark-web/src/store/feedStore.ts

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.

… type

npm run build (tsc -b, project references) type-checks test files strictly
against the real generated protobuf message type, unlike npm run typecheck's
plain tsc --noEmit — a gap this reconnect test fell into by yielding plain
object literals instead of casting them, unlike every other mocked
streamFeedUpdates implementation already in this file.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hnoh6YfJwp63szeP8ZFqVg
Gitar review finding on this PR: newestPublishedAt compared publishedAt
values with the string `>` operator. Items loaded via getFeed and items
delivered over the live stream aren't guaranteed identical formatting (a
value can be missing its trailing Z depending on DateTimeKind, independent
of this fix) — a longer string sharing the same prefix sorts as "greater"
lexicographically even when it represents the same or an earlier instant.
Parsing both sides as dates removes that dependency on wire-format
stability entirely.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hnoh6YfJwp63szeP8ZFqVg
@sonarqubecloud

sonarqubecloud Bot commented Sep 5, 2026

Copy link
Copy Markdown

@gitar-bot

gitar-bot Bot commented Sep 5, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 1 resolved / 1 findings

Fixes the feed's live-update stream replaying duplicate items on reconnect by using the newest publishedAt instead of items[0].publishedAt as the replay point, and adds prependItem deduplication as a safeguard. Also enables stream retry on clean close, not just errors. New test coverage confirms the reconnect replay behavior and dedup logic. No issues found.

✅ 1 resolved
Edge Case: Replay point relies on lexicographic == chronological order

📄 src/pressmark-web/src/hooks/useFeedPage.ts:42-44
newestPublishedAt compares publishedAt strings with >, which only equals chronological order if every timestamp shares an identical ISO-8601 format (same UTC 'Z' suffix and same fractional-second precision). If the backend ever emits a mixed form (e.g. '...:00Z' vs '...:00.000Z', or a numeric offset), the computed maximum can exceed the true newest, making the reconnect replay start too late and silently drop articles — a gap the prependItem dedup cannot recover. The comment asserts a fixed format; consider comparing via Date.parse (or a numeric epoch) so correctness doesn't hinge on wire-format stability.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source

@lopatnov
lopatnov marked this pull request as ready for review September 5, 2026 07:51
@lopatnov
lopatnov merged commit e240f45 into main Sep 5, 2026
15 checks passed
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