Repository navigation
fix(app-shell,plugin-detail): one sys_activity row to FeedItem constructor; stop dropping author-extended types on the console record page - #6731
Merged
Conversation
…uctor; stop dropping author-extended types on the console record page The console record page's `sys_activity` merge read the shared type table (objectui#5878) and then built the FeedItem itself, ending in `if (!feedType) continue;`. That collapsed a DELIBERATE exclusion (`commented` / `mentioned` / `login` / `logout` -> undefined) with a type the table has never heard of -- an author-extended value under the objectstack#11507 direction-4 ruling. The second kind was stored, queryable and INVISIBLE, with nothing logged: objectui#5840's failure mode reached by another route, and one the block side had already fixed (objectui#5969 / PR #6112), so the two surfaces disagreed about the same row of the same table. - `@object-ui/plugin-detail` exports the whole reading -- `activityRowToFeedItem`, `UNMAPPED_ACTIVITY_FEED_TYPE`, `resetUnknownActivityTypeWarnings` -- not just the lookup table. - `RecordDetailView` calls that constructor. The inline loop, its timestamp fallback and its second system-actor lookup are gone. - The four exclusions still produce no row and no warning, deliberately. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49
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
1 participant
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 #5896
The defect: what the console record page did with an unmapped type
RecordDetailView'ssys_activitymerge already read the shared type table (objectui#5878) — and then built theFeedItemitself. The whole of the drop was this:One
continue, two completely different situations:undefinedon purpose —commented/mentioned/login/logout;The second is an author-extended value.
sys_activity.typeis author-extensible under the objectstack#11507 direction-4 ruling (maintainer, 2026-08-24): every column onsys_activityisreadonly: true, so objectql'svalidateRecordnever validates a write to it, and ADR-0052 §5b.2 forwards an author'sactivityMilestones[].typeinto the column verbatim — that is howcompletedis produced, and it is a general door.What the user saw was nothing. An activity that happened, was written, and is queryable simply had no row on the record page. No placeholder, no empty state, no message on any channel — the row was stored, queryable and invisible. That is objectui#5840's failure mode reached by another route, and the block side had already stopped doing it: objectui#5969 (PR #6112) gave
activityRowToFeedItema defined fallback. So the two surfaces disagreed about the same row of the same table — the block rendered it, the console discarded it.The convention followed, and whose it is
Not a new one — inherited.
activityRowToFeedItemrenders an unmapped type throughUNMAPPED_ACTIVITY_FEED_TYPE('system') and warns once per distinct value.'system'is the generic bucket rather than a new kind becauseFeedItemTypeis a closed spec enum owned by@objectstack/spec; minting a kind for "we don't know" would be a platform change, not this surface's. The diagnostic reports a missing decision, not lost data: the row is visible, what it lacks is its own icon and colour.This PR makes the console surface call that constructor rather than paraphrase it, so the behaviour arrives as a consequence of there being one reading — no second decision was taken here.
⛔ The four deliberate exclusions are untouched.
commented/mentioned/login/logoutstill produce no row and no warning. They are decisions (comment content lives insys_commentwith its own reactions and threading; login/logout are account events, not record activity), and a warning about a decision teaches authors to ignore the channel. That distinction is asserted in its own leg.What changed
packages/plugin-detail/src/index.tsx— the barrel exports the whole reading, not just the lookup table:activityRowToFeedItem,UNMAPPED_ACTIVITY_FEED_TYPE, theresetUnknownActivityTypeWarningstest seam and theSysActivityRowtype, alongside the existingACTIVITY_TYPE_TO_FEED_TYPE. No new module edge: these all come from./renderers/recordActivityFeed, which the barrel already re-exported from and whichrenderers/record-activityalready pulls in eagerly.packages/app-shell/src/views/RecordDetailView.tsx— the inlineforloop calls the constructor.nullis now the one outcome that drops a row, and it means exactly one thing. The loop's private timestamp fallback and its seconddetail.systemActorlookup are gone; the label is passed in, so the console page keeps its own localisation while sharing the construction.RecordDetailView.activityUnmappedType-5896.test.tsx— new.RecordDetailView.activityMapIdentity-5878.test.tsx— one leg updated, see below..changeset/5896-feeditem-single-constructor.md—minorfor both packages (objectui'smajortracks@objectstack, so a break ships asminorwith the break spelled out).Three divergences closed, all three named on the card: the silent drop; a timestamp fallback that could hand
mergeFeedRowsanundefinedcreatedAtwhere the helper yields''; and two independently authored i18n lookups for one fallback label.An existing test leg was updated, and why that is the fix and not a workaround
RecordDetailView.activityMapIdentity-5878.test.tsxhad a regression control asserting thatzzz_not_an_activity_typeproduced no row. That leg pinned the silent drop — the defect, not the contract — and it is exactly what the PM annotation on this card warned against carrying forward. It now asserts what survives the ruling: a mapped row shows, a deliberate exclusion does not, and no diagnostic fires for a decision. Nothing was skipped, quarantined or deleted; the unmapped case moved to the new file, where it is asserted positively together with its diagnostic. That file's stale "out of scope" docblock now points at this card.Pin verification (ablation)
Every pin was run against unfixed source. Method: with the fix committed,
packages/app-shell/src/views/RecordDetailView.tsxwas reverted to the merge-base blob and the new file re-run, under atrap ... EXIT INT TERMrestore.The consumer is the right ablation subject: the barrel change is a pure export-surface addition with no behaviour, and reverting it too would only make the test file fail to import — a module-load error, not a discriminating red. No rebuild stands between the edit and the run:
vitest.config.mts:283aliases@object-ui/plugin-detailtopackages/plugin-detail/src, so nodistis in the resolution path.Mutation confirmed on disk before the run (not from an editor's exit code):
activityRowToFeedItemoccurrences3 -> 0,if (!feedType) continue;occurrences0 -> 1, blob02c0d4f0 -> 003daffa. Restore confirmed after: blob back to02c0d4f0,git diff HEADempty, working tree clean.Result — 7 failed, 1 passed:
createdAtwhen neither timestamp column is usableThe exclusions leg is green in both directions on purpose: it is the counter-probe. Without it, "everything renders" could be reached by deleting the drop entirely.
The identity leg is separate from the behaviour legs deliberately. Every behavioural assertion above is also satisfied by inlining the fallback and the warning into this view — i.e. by re-forking the constructor with today's semantics, which is the drift this card closes and which would pass ON that defect a release later. So a delegating spy is installed over the
@object-ui/plugin-detailbarrel; it records nothing at all if the view builds its own item.What was run — all at
c5dedf240pnpm --filter @object-ui/plugin-detail --filter @object-ui/app-shell run type-check— bothDone.pnpm exec vitest runover the 10 affected files (bothRecordDetailViewactivity pins,feedRecordScope,feedLoading,richtextSurfaceParity,defaults-maps-mirror-en-pack,sharedInboxFeed.rowShape, plugin-detail'srecordActivityFeed+record-activity, plugin-calendar'spropsContract) — 10 files, 143 tests passed.check:control-bytes✅,check:i18n-keys✅,check:i18n-drift✅,check:i18n-dead-keys✅,check:vi-mock-specifiers✅,check:entry-guard✅,check:self-import✅,check:phantom-deps✅,check:changeset-presence✅ (3 source file(s) of 2 released package(s) changed, and this change declares 1 changeset(s)),check:changeset-no-major✅.check:readme-exports— not measured, and not by this diff: it needs every package'sdiston disk and only this branch's build closure was built, so it reports 69 unjudged self-imports acrossplugin-gantt/plugin-map/plugin-markdown/plugin-timeline/plugin-ai/cli/app-shell.plugin-detail— the package whose export surface this PR changes — was judged, and is clean. CI builds everything and runs it for real.check:eager-closure— not measured: it readsapps/console/dist/eager-closure.json, written by a consolevite buildthat was not run here, and it says so itself ("a broken gauge, not a passing budget"). Statically it cannot move: the diff introduces zero new module specifiers — app-shell swaps one named import for another on the same specifier, and the barrel re-exports more names from a module it already re-exported from.eslint .is CI's run). Three pieces of evidence that the narrowing excluded nothing: (1) the population is eslint's own — each changed file was passed to eslint and it resolved all four as in-scope rather than ignored; (2) the count is read from--format json: 4 results, 0 errors (--max-warningsis deliberately unset inlint.yml, so warnings are not the gate, and the pre-existing warning counts onRecordDetailView.tsxare untouched); (3)eslint.config.jsconfigures noproject/projectService, so no rule is type-aware and this diff cannot change the verdict on a file it does not touch — the only cross-file rules are import resolution, and an added export cannot invalidate an existing consumer's import.Scope fence
⛔ #5877 — the
FeedItemTypekinds with no producer on any objectui surface — reads the same construction site and is deliberately not touched here. No producer census was taken.One out-of-scope finding turned up and is filed unassigned as #6730:
packages/app-shell/src/hooks/sharedUserFeeds.ts'smapActivityRowsis a third hand-written reading ofsys_activity.type(targetingActivityItem, a different four-value vocabulary), which buckets every unrecognised type — author-extended values included — asupdate, and carries its own copy of the"NOW()"timestamp quirk. It drops nothing, so it is a drift risk rather than a live defect, and converging it needs theActivityItemvsFeedItemquestion answered first.Generated by Claude Code