refactor(app-shell): retire the dead InboxNotification.actor_name field - #5487
Merged
Merged
Conversation
…ld (objectui#5203)
`InboxNotification` (`layout/inboxGrouping.ts`) declared `actor_name`, and the
field was dead at both ends. `mergeInboxRows` (`hooks/sharedUserFeeds.ts`) is the
single producer of every row the bell and Home's action centre render and never
mapped it; neither consumer (`InboxPopover`, `useHomeInbox`) read it; and
`sys_inbox_message` declares no actor column for it to have been mapped FROM. It
was the last declared-but-unfilled member of the interface after objectui#5190
removed the sibling `source_object` / `source_id` pair.
Not a published type surface: `InboxNotification` is unreachable from this
package's public entry (absent from `src/index.ts`, `src/layout/index.ts` and the
built `dist/index.d.ts`; the `exports` map has no deep subpath), so this is an
internal narrowing and the changeset declares an empty frontmatter.
Two pins, in opposite directions:
- a TYPE PIN in `layout/__tests__/inboxGrouping.test.ts` fails if the field is
re-declared;
- a runtime key-set pin in `hooks/__tests__/sharedInboxFeed.rowShape.test.tsx`
fails if the producer is ever changed to spread raw `sys_inbox_message`
columns through instead of mapping them field by field — the rot direction a
type pin cannot see.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RV6yuVCxymHYE16PL9vQkE
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
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 #5203
Retires
InboxNotification.actor_name(packages/app-shell/src/layout/inboxGrouping.ts) — the last declared-but-unfilled member of that interface after objectui#5190 removed the siblingsource_object/source_idpair.The producer needed no change:
mergeInboxRowsnever mapped the field, so the whole retirement is one deleted line plus the pins that keep it deleted. That is whyhooks/sharedUserFeeds.tsappears in this diff only as a new test.Both ends dead — measured on
origin/main@490f4824, not inherited from the cardProducer and consumer were searched separately, with two independent instruments, and every zero was counter-probed against a sibling field known to be live.
Instrument 1 — text search, repo-wide
actor_nameoccurs 26 times in tracked.ts/.tsxonorigin/main. Exactly 6 of those are reads, and every one of them belongs to a different declaration — the conflation trap the card flags:sys_activityrowmapActivityRowshooks/sharedUserFeeds.ts:587ApprovalActivityhooks/useRecordApprovals.ts:141views/RecordApprovalsPanel.tsx:353views/RecordDetailView.tsxviews/RecordDetailView.tsx:1573RecordActivityRowplugin-detail/.../recordActivityFeed.ts:94recordActivityFeed.ts:159,record-history.tsx:100apps/console/src/services/approvalsApi.ts:156ApprovalsInboxPage.tsx:1920InboxNotificationlayout/inboxGrouping.ts:41mergeInboxRows(hooks/sharedUserFeeds.ts:675) is the single producer of every row the bell and Home render; it mapsid,notification_id,receipt_id,type,title,body,action_url,is_read,created_atand nothing else.InboxPopoverreadsid/title/body/created_at/is_read/action_url;useHomeInboxreadsid/title/action_url/created_at/is_read. Neither names the field.Instrument 2 — type-aware probe, immune to the name conflation
Rename the member on the interface and let
tscreport everything bound to it, across both projects —tsc --noEmit(src) andtsc -p tsconfig.test.json(tests). Baseline: both green before either probe.Probe — rename
actor_name. Predicted 0 errors in both projects. Observed:Counter-probe — the same rename applied to
action_url, a sibling that IS produced and read. Predicted: errors at the producer, at both consumers, and in the fixtures. Observed 3 + 12 errors:The instrument fires on all three binding kinds — producer literal, consumer read, test fixture. So the probe's zero is a reading of a working instrument, not a broken search.
Third leg — there was nothing to fill it FROM
sys_inbox_message(objectstack@7679f8b54,packages/services/service-messaging/src/objects/inbox-message.object.ts) declaresid,user_id,notification_id,delivery_id,topic,title,body_md,severity,action_url,created_at. No actor column. So "make it live" was never a one-line producer change — it is a backend capability expansion, exactly as the card suspected. That settles the disposition the card left open, on evidence rather than preference.Published type surface: unchanged
InboxNotificationis not reachable from@object-ui/app-shell's public entry:src/index.tsand fromsrc/layout/index.ts(the barrel it would have to pass through);dist/index.d.ts(checked afterturbo run build --filter=@object-ui/app-shell) — 0 occurrences;exportsmap offers only.and./styles.css, so there is no deep subpath an external consumer could import it through.The type is internal to the package, so removing an optional member is not an externally observable narrowing. Nothing user-visible ships, which is why the changeset carries an empty frontmatter — the repo's explicit "this publishes nothing" declaration, not a missing changeset. (
node scripts/check-changeset-presence.mjsgreen.) This is recorded here rather than left implicit precisely because a published narrowing would have needed the opposite treatment.Two pins, in opposite directions
A type pin alone cannot see the direction this contract is most likely to rot in, because that direction does not go through the type at all: the producer builds its row from an explicit field-by-field literal, and the cheap "improvement" is to spread the raw record into it. So there are two:
layout/__tests__/inboxGrouping.test.ts: a@ts-expect-errorthat fails if the member is re-declared. Checked bytsc -p tsconfig.test.json, which is this package'stype-checkscript and the CIType Checkjob.hooks/__tests__/sharedInboxFeed.rowShape.test.tsx: feeds the producer a rawsys_inbox_messagerow loaded with unmapped columns (user_id,delivery_id,severity, and anactor_name) and asserts the produced row's whole key set, not merely the absence of one key. It also asserts the mapping still works, so the key-set assertion cannot be satisfied by a producer that maps nothing.Reverse verification — direction predicted before running
Both legs ablate a committed state, so restoring is
git checkoutand the tree is provably clean afterwards. Neither leg needs a rebuild: both suites import their subject through relative specifiers (../inboxGrouping,../sharedUserFeeds), so no packagedistis on the resolution path and no stale artifact can fake a green.Leg 1 — re-declare
actor_nameon the interface.Predicted: test type-check RED on an unused directive; the runtime suite stays GREEN, because a declaration nobody fills changes no runtime shape. Observed exactly that:
Leg 2 — give the producer a
...mraw-row pass-through.Predicted: the runtime key-set pin RED, the type pin GREEN, and
satisfies InboxNotificationsilent — a spread is not excess-property-checked. Observed exactly that:Neither pin substitutes for the other: each is the only one that can see its own leg. After both legs,
git status --porcelainis empty and the tree matchesHEAD.Local verification — all at
7c0a3fecb, on a clean treepnpm exec vitest run packages/app-shell/tsc --noEmit(app-shell src)tsc -p tsconfig.test.json(app-shell tests)eslint .in app-shelleslint --max-warnings=0on the 3 changed filescheck:control-bytescheck:phantom-deps/check:self-import/check:esm-specifierscheck:changeset-presence/changeset:checkNo test was skipped, disabled or quarantined by this change; the single skip in that run is pre-existing on
origin/main.check:eager-closurewas not run — its gauge readsapps/console/dist/eager-closure.json, which only exists after a consolevite buildthis worktree never ran; it is unrelated to this diff and CI builds the console.Scope
Four files: the interface, its two pins, the changeset. PR #5480's
packages/app-shell/src/preview/andpackages/app-shell/src/views/are untouched — verified against the three-dot diff, no breach. No governed surface touched.Generated by Claude Code