Uh oh!
There was an error while loading. Please reload this page.
fix(app-shell): one reading for sys_activity.type, and the unrecognised case stops claiming update - #6814
Conversation
…ed case stops claiming update `mapActivityRows` (hooks/sharedUserFeeds.ts) carried the third hand-written reading of `sys_activity.type` in this repo and bucketed every unrecognised value as `update` — a specific, often wrong claim about the row. #5878 shared the table between `record:activity` and `RecordDetailView`, #5896 shared the constructor; this copy survived both, including its own `"NOW()"` timestamp fallback. The two readings are deliberately NOT converged: `activityRowToFeedItem` builds a `FeedItem`, whose vocabulary CROSSES this one — it collapses create/update/delete into a single `field_change` and drops `commented` / `mentioned` outright, so routing this surface through it would cost the bell every comment row and every create/delete distinction. What is shared instead is a pin, not an import. - new `layout/activityItemType.ts`: the whole reading (table, generic bucket, `"NOW()"` fallback, row constructor), DOM-free so it is directly assertable - `ActivityItem['type']` gains `system`, the generic bucket; `tsc` forces the icon, label and notification toggle for it in `ActivityFeed` - `system` / `completed` / `scheduled` / `login` / `logout` land in the bucket instead of claiming `update`; unrecognised values render through it and are named once, following `UNMAPPED_ACTIVITY_FEED_TYPE`'s precedent - `activityItemType-6730.test.ts` reads plugin-detail's real table (devDependency, no runtime edge) and fails when the declared vocabulary grows an entry this side has not read, or when the two readings stop disagreeing Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49
`tsc -p tsconfig.test.json` covers test files in this package; the timestamp equivalence table now declares `SysActivityRow` (the shape the folded copy accepts) and the e2e suite collects warnings into a typed array rather than reaching into an untyped spy handle. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49
✅ 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
|
zhuangjianguo
commented
Aug 30, 2026
Contract review PASS at head Full-diff review (16 files, +775/−45): ① union widening Flipping ready + arming auto-merge (queue). Verdict recorded here on the PR per the #13417 visibility rule. Generated by Claude Code |
Uh oh!
There was an error while loading. Please reload this page.
Fixes#6730
A drift card, answered in the order the card asks: first what the right shared
shape is, then the one thing that was a defect regardless.
1. The architecture question: the two vocabularies CROSS
The card warns that this is not simply "adopt
activityRowToFeedItem". Measuredagainst
ACTIVITY_TYPE_TO_FEED_TYPEas it stands today, it is worse than that —the two readings are lossy in both directions, so neither type is a
projection of the other:
FeedItem(plugin-detail)ActivityItem(app-shell)created/updated/deleted/assigned/sharedfield_changecreate/update/deletecommented/mentionedundefined— deliberately dropped, content lives insys_commentcommentkind, one of foursystem/completed/scheduledsystem/task/event)Routing this surface through the shared constructor would therefore cost the
bell every comment row (
activityRowToFeedItemreturnsnullfor them) andevery create/delete distinction (
field_changecannot be decomposed). Ashared reading would have to be a third table keyed on
sys_activity.typewithtwo value columns — which is not a convergence, it is the same two decisions
written next to each other, plus a runtime edge from the shell's header chrome
onto a record-detail widget plugin (
@object-ui/plugin-detailis a peerdependency of
@object-ui/app-shell, so that edge is a real install-timerequirement).
So the two readings stay separate — and what is shared is a pin, not an
import.
activityItemType-6730.test.tsreads plugin-detail's real table viathe devDependency (no runtime edge) and fails when:
(
map ⊇ built-ins, the one-directional shape objectui#5969 landed on — thecolumn is author-extensible, so an equality pin would be false by
construction); or
convergence viable. That is then a decision, taken against a red test that
says what changed.
Before this PR nothing red-flagged the disagreement, which is exactly how
scheduledreached the console record page after objectui#5878 and neverreached the bell.
2. The silent default — measured, and what it actually costs today
Today every value outside the four named ones renders as
update. That is not amissing decision, it is a wrong one stated out loud: a
scheduledmeeting, alogin, a nightlysystemrollup and an author'scontract_countersignedallclaim "somebody updated this record".
What the reader deserves to know is how visible that is. Measured on
origin/main:InboxPopover's Activity tab (layout/InboxPopover.tsx:545) rendersa.user,a.description,a.timestampanda.objectName— nevera.type.HomeActivity(console/home/HomeRail.tsx:262) renders the same four —never
a.type.ActivityItem['type']is theActivityFeedSheet, whichis exported from the package barrel and has zero in-repo call sites. There
the type drives the icon, the colour, the badge label and the
notification-preference filter — and that filter is a
Record(ActivityItemType, boolean)lookup, so a kind missing from the recordis silently filtered OUT of the panel.
So the wrong bucket is invisible on both surfaces the card names, and visible
only on a published component this repo never mounts. That is why nothing was
ever observed — and it is also why this change cannot regress the bell or Home:
they do not read the field. It does not make the defect unreal; the union and
the component are both published API.
3. What changed
layout/activityItemType.ts— the whole reading in one place: thetable, the generic bucket, the
"NOW()"timestamp fallback and the rowconstructor that applies all three. DOM-free, so what a row becomes is
assertable without mounting a Sheet (
recordActivityFeed.ts's own precedenton the other side). Exported as the whole reading rather than its pieces, for
the reason objectui#5896 gave: publishing the table alone left the mirror one
level up, and the constructions drifted where the tables did not.
ActivityItem['type']gains a fifth kind,system— the generic bucket.ActivityFeedkeeps threeRecord(ActivityItemType, …)tables, sotscrefused the member until it had an icon, a label and a default notification
toggle. That compile-enforcement is what makes the bucket safe to add.
land in the bucket instead of claiming
update:system,completed,scheduled,login,logout.assignedandsharedstayupdate— bothwrite to the record, which is also why they sit in
field_changeon the otherside. The stated rule:
create/update/deletemean the record's ownstored state changed, split by which way.
console.warn, followingUNMAPPED_ACTIVITY_FEED_TYPE's precedent — a bucket,not a drop, because
sys_activity.typeis author-extensible(objectstack#11507 direction 4, ruled 2026-08-24) and dropping is
objectui#5840's failure mode. A value the table maps to
systemon purposestays silent, which is why the lookup asks
hasOwnPropertyrather thancomparing its result to the bucket.
"NOW()"quirk stays a copy, deliberately, and is now pinned. It isthe only target-type-independent part of the reading, so it is the one thing
that genuinely could be shared — but
activityTimestampis not onplugin-detail's barrel (objectui#5896 published the whole
FeedItemreading onpurpose, not its pieces), and importing it would buy one five-line predicate
for a peer-dependency edge on the header chrome. There is no package that owns
"how to read a
sys_activitycolumn" for both a widget plugin and the shell.Until there is, the honest instrument is a pin: the suite asserts the two
copies agree value for value over the quirk's whole input table.
layout.activityFeed.typeSystem.Scope held: nothing under
record:activityorRecordDetailViewis touched —objectui#5878 and objectui#5896 own those. The plugin-detail import added is
test-only.
4. Reverse verification (direction predicted BEFORE running)
Predicted: restoring the pre-#6730 inline reading in
mapActivityRowsonlyturns the e2e suite RED while the pure suite stays GREEN — because the pure
suite's subject is the table, and the defect was never in a table, it was in the
reading done inline around it.
Measured, mutation and restore both confirmed on disk by blob hash (script kept
a
traprestore;git diff HEADempty afterwards, restored blob identical tothe HEAD blob
687fd0125519ba7c436a2db8b1f3d7c015eb9183):r5isscheduled,r6is an author-extendedcontract_countersigned,r7is
system. Two directions worth naming:files exist: a pin on a table the producer does not call is objectui#5896's
failure mode.
"NOW()"case stayed green in both legs, which is the measurement thecard asks for: the fold is behaviour-preserving on this surface — the pin
cannot tell the two implementations apart, which is the whole point of it.
No rebuild was needed for either leg, and this is a property of the harness
rather than an assumption:
vitest.config.mts:289aliases@object-ui/plugin-detailtopackages/plugin-detail/src, andsharedUserFeeds.tsis reached by relative path — nodistparticipates inthese runs.
And the honest drift-card answer to "does anything fail when you revert?":
before this PR, nothing did. No test in the repo referenced
mapActivityRowsor pinned any part of this mapping; reverting the whole change with only the
pre-existing suites green is a no-op. That absence is the finding the card was
filed for, and closing it is what the two new suites are.
5. Verification
All heavy runs serialized through the shared verify lock; verdicts quoted from
the lock's own VERDICT line, not from a piped
$?.pnpm --filter '@object-ui/app-shell^...' build—VERDICT command-exit 0pnpm --filter @object-ui/app-shell type-check && pnpm --filter @object-ui/i18n type-check—
VERDICT command-exit 0. app-shell'stype-checkistsc --noEmit && tsc -p tsconfig.test.json, so the new suites are covered byit (they were not, at first — three errors, fixed in the second commit).
pnpm exec vitest run --maxWorkers=2 packages/app-shell/ packages/i18n/ate3cb0dbcc(the final commit) —VERDICT command-exit 0,Test Files 634 passed (634),Tests 6573 passed | 1 skipped (6574).EXIT=0:check:control-bytes,check:i18n-keys,check:i18n-drift,check:phantom-deps,check:self-import,check:vi-mock-specifiers,changeset:check.check:readme-exportsandcheck:eager-closureare NOT MEASURED locally,not red: both say so themselves — the first needs
pnpm buildfor sixpackages this diff never touches (app-shell's own entries pass now that it is
built), the second needs an
apps/consolebuild and prints "This is a brokengauge, not a passing budget." CI runs both properly.
Lint was narrowed to the changed files, and the narrowing is declared, with
its three pieces of evidence. (1) The population is eslint's own — the run used
--no-inline-config, the same flag the rootlintscript uses, and it isstricter than the per-package
eslint .CI actually runs. (2) File count readfrom
--format json: 6 files, 0 errors, 0 warnings. (3) Invariance foruntouched files:
eslint.config.jssets noproject/projectServiceandextends
tseslint.configs.recommended(notrecommendedTypeChecked), so norule reads another file's types and this diff cannot move a verdict on a file
it does not touch; the diff also removes no export, so no importer's resolution
changes. CI runs the full farm regardless.
Generated by Claude Code