fix(app-shell): Home's action centre badges the full unread count, not its capped list - #4348
Merged
Merged
Conversation
…#4329) The bell badges distinct unread topics + pending approvals over the shared feed's 20-row window; Home's action centre badged `pendingApprovalsCount + notifications.length` — and `notifications` is the list it renders, capped at 5. Nine unread read as 9 on the bell and 5 on the card 200px below, one page, one question, two numbers. `useHomeInbox` grows `unreadTopicCount` (additive) and `HomeActionCenter` takes it as a required prop. Badge = total waiting, list = the newest few of it, still capped at 5. The count is the bell's own fold (`groupNotifications`, by `(topic, title)`) over the bell's own rows rather than the pre-slice length of Home's list: that length is title-folded and drops blank titles, so it would agree with the bell on ordinary data and disagree when two topics share a title — the "agrees usually" shape of #4316. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
yinlianghui
marked this pull request as ready for review
August 11, 2026 19:27
This was referenced Aug 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4329. Filed from #4225 / PR #4327, which measured the disagreement and pinned it.
The defect
/homeshowed two numbers for one question. The bell badges distinct unread topics plus pending approvals over the shared feed's full 20-row window; the action centre two hundred pixels below badgedpendingApprovalsCount + notifications.length— andnotificationsis the list it renders, whichuseHomeInboxcaps atlimit(5). With nine unread the bell said 9 and the card said 5, on one page, about one set of rows. The badge was reporting the size of a preview as if it were a total.Before #4225 the card could not have said anything else: its own read was
$top: 5, so nine was not a number it had. Both surfaces now cut from one already-joined feed, so the true count is in hand at Home's call site and the cap is a presentation slice over data the card already holds.The change
Per the ruling on the issue: the badge shows the full unread count (the same number as the bell), the list stays capped at five. Badge = "how much needs you", list = "the newest few of it".
useHomeInbox's public shape grows one additive field,unreadTopicCount— existing fields and their meanings untouched.HomeActionCentertakes it as a required prop, andtotalbecomespendingApprovalsCount + unreadTopicCount. Required rather than optional-with-a-default for the same reasonnotificationsStatusis: a call site that cannot say how much is waiting must not be able to badge its own preview length by saying nothing."You're all caught up"is now gated on that total rather than on the rows on show, so it can no longer contradict the badge above it.Why the count is the bell's fold and not the list's pre-slice length
The obvious two-line version is "count the list before
.slice()". That length is folded by title and drops blank titles, while the bell folds by(topic, title)— so it agrees with the bell on ordinary data and disagrees whenever two topics share a title. Two derivations of one number that agree usually is precisely the shape of #4316. The count is thereforegroupNotifications— the bell's own fold — applied to the bell's own rows. One fold, applied twice, cannot drift.packages/app-shell/src/layout/inboxGrouping.tsis only imported; nothing in it (or insharedUserFeeds.ts, or the bell) is restructured.Tests
Red-first. #4327's pin that MEASURED the inconsistency (
sharedInboxFeed.twoSurfaces.test.tsx, "an unread message with no receipt at all still counts":homeBadge'5'vs bell'9') is flipped into the consistency assertion — both'9', list still exactly 5 rows. A second pin of the same old number lived inHomeActionCenter.unansweredInbox.test.tsx("lists the QA payload and badges it": badge'5'on nine unread); flipped too, its own claim unchanged.New
#4329block, all joint (realAppHeader+ realHomeActionCenter, one tree, one fake adapter):'2', Home lists 1 row, bell lists 2'6'on both; approvals row still reads "2 pending approvals"'12', bell'9+', list still 5 rowsControls left untouched and green: the caught-up case (no badge on either surface when nothing is unread and no approvals), the existing "reads the SAME badge number on both surfaces — 4 unread topics", the whole read-state block, and the one-feed-one-read block.
Reverse verification (predictions first, both measured)
total = pendingApprovalsCount + notifications.length. Predicted: the two flipped pins plus the topic-fold and clamp cases go red, the approvals case and the entire read-state block stay green. Measured:Tests 4 failed | 44 passed— exactly those four (Expected "9" / Received "5","2"/"1","12"/"5","9"/"5"), nothing else.groupNotifications. Measured:Tests 1 failed | 47 passed, the single failure beingExpected "2" / Received "1".Both taken out with
git checkout -- < path >against the commit, nevergit stash.Local verification
pnpm exec vitest run packages/app-shell/(repo root) →Test Files 349 passed (349),Tests 3331 passed | 1 skipped (3332)turbo run build --filter=@object-ui/app-shell^...→28 successful, 28 total(build closure before type-check)tsc --noEmitandtsc -p tsconfig.typetests.jsoninpackages/app-shell→ both exit 0eslinton the changed files → 0 errors (15 pre-existingany/unused-arg warnings in the test file's mock boilerplate, none on changed lines)check-changeset-presence/check-changeset-no-major/check-control-bytes→ all greenScope
useHomeInbox.ts,HomeRail.tsx(HomeActionCenter) andHomePage.tsx's wiring, plus the tests. The bell (AppHeader.tsx/InboxPopover.tsx),sharedUserFeeds.tsandinboxGrouping.tsare read from and not modified.HomeActionCenteranduseHomeInboxare not exported frompackages/app-shell/src/index.ts, so the required prop is not a public-API change: app-shell's dependents (apps/console,examples/console-starter,examples/byo-backend-console) can only reachHomePage/HomeLayout, whose props are unchanged.One thing to note, deliberately not changed
Past 9 the bell renders
"9+"(its badge is a 20px circle in the top bar, #2765) while Home's card prints the number —"9+"and"12"are two renderings of one count, not two counts. Left as-is and pinned as measured; clamping Home would mean teaching the shared card badge a display rule it has no other use for. Flagged for the maintainer rather than decided here.Generated by Claude Code