Repository navigation
[finding] The sys_activity row -> FeedItem construction is still written twice, and the console surface drops unmapped types silently #5896
Description
Activity
PM annotation — the ruling that now governs the "drops unmapped types silently" half
Recording this from the
domain:uiseat (session_01CSoz9uGhaaSgiq3hshtN7L) so whoever picks this card does not have to rediscover it. ⛔ No code change here, and this card is not claimed.As of PR #6112 (
Fixes #5969), the block-side surface no longer drops.packages/plugin-detail/src/renderers/recordActivityFeed.tsnow renders an author-extendedsys_activity.typethrough a defined fallback (UNMAPPED_ACTIVITY_FEED_TYPE), warned once per distinct type, instead of returningnullfor it.That was a behaviour change, not just a pin, and the reason is this card's own finding turned out to understate it: what existed was a drop, not a fallback — a stored, queryable, author-extended activity row was simply invisible. That is #5840's failure reached by another route.
The ruling that applies
Maintainer, 2026-08-24, batch acceptance on objectstack#11507, verbatim 「四维分析一致的，接手你的建议。」 — adopting direction 4:
sys_activity.typeis author-extensible.⇒ Under that ruling, silently dropping an unrecognised type is wrong on every surface, not only in the block. So this card's console half is now a ruled inconsistency rather than an unruled one.
What that means for whoever takes this
⚠️ The console surface (RecordDetailView's own copy of the map) still drops. After fix(plugin-detail): pin recordActivityFeed's OPEN-vocabulary contract to sys_activity.type #6112 the two surfaces disagree about what an author-extended type does — the block renders it, the console discards it. ⛔ Do not close this card by pinning the current console behaviour; the fallback is what it should get.⚠️ [finding] RecordDetailView hand-copies record:activity's sys_activity map, and nothing fails when the two disagree #5878 is the same family — that copy is hand-written rather than importing the module. ✅ Fixing the drop and converging the copy are naturally one card; whoever takes either should read both.- ⛔ fix(plugin-detail): pin recordActivityFeed's OPEN-vocabulary contract to sys_activity.type #6112 deliberately did NOT touch
app-shell— the boundary forbade it andACTIVITY_TYPE_TO_FEED_TYPEitself is untouched, so nothing regressed. The divergence is new only in the sense that one side got better. ⚠️ The four deliberate exclusions still returnnullsilently (commented/mentioned/login/logout). ⛔ Those are not the drop this ruling is about — a converging fix must keep them excluded, and should say so.
One measured trap for the same work
⚠️ There is no live source for the built-in vocabulary that a UI package can read.sys_activity.typeis declared only in@objectstack/plugin-audit, a server plugin; objectui depends on@objectstack/spec,client,formulaandlintonly, and the spec publishesFeedItemType(the map's values) and never its keys. So any census of the built-in set here is a hand copy — and #6112 measured that the previous copy had already drifted:scheduledwas declared upstream (objectstack#11522) while the census still filed it as "undeclared but written".✅ Filed as objectstack#11807 — publish the built-in set where a UI consumer can read it, without re-closing the vocabulary. Until that lands, the fallback is what keeps this drift costing an icon rather than a row.
Generated by Claude Code
Concentrated triage round:
finding→pm:queue, Task,domain:uistands. Direction inherits #5878's convergence pattern one level out: converge the row→FeedItem construction ontoplugin-detail's exportedactivityRowToFeedItem(+activityTimestamp), deleting the inline copy inRecordDetailView— the console surface then inherits the helper's unmapped-type behaviour instead of silently dropping. If converging surfaces a genuine product question the helper's behaviour does not answer (what an unmapped type should render as), stop and report the fork — do not invent a rendering. Read the full thread at claim (one measurement comment).
Generated by Claude Code
Claim:
domain:uiexecution seat, sessionsession_01CRJge11jso9TpXRWFt1Z49.Branch:
claude/issue-5896-feeditem-single-constructorDispatching one developer to this card. Of the two findings in the FeedItem family, this one is taken first because it carries an actual user-visible defect (the console surface dropping unmapped types in silence), not only a duplication.
⚠️ Held back deliberately, and NOT part of this dispatch: #5877 (the eightFeedItemTypekinds with no producer). It reads the same construction site, so dispatching both at once would put two developers in one file. It becomes a candidate again once this card lands, re-verified against the merged ref — the merge that closes this one may well answer it too.Declared file face for the cross-lane in-flight check: the
sys_activityrow toFeedItemconstruction sites (console surface and its package-side twin). No other card in this batch touches them.
Generated by Claude Code
os-dev-report
{ "issue": 5896, "status": "done", "branch": "claude/issue-5896-feeditem-single-constructor", "pr": "https://github.com/objectstack-ai/objectui/pull/6731", "premise_still_valid": true, "summary": "THE SILENT DROP, QUOTED. RecordDetailView's sys_activity merge read the shared type table (objectui#5878) and then built the FeedItem itself, ending in exactly: `const feedType = ACTIVITY_TYPE_TO_FEED_TYPE[row.type]; if (!feedType) continue;`. That one `continue` collapsed two different situations -- a type the table maps to undefined ON PURPOSE (commented / mentioned / login / logout) and a type the table has never heard of. The second is an author-extended value (objectstack#11507 direction 4, ruled 2026-08-24: sys_activity columns are all readonly so objectql never validates a write, and ADR-0052 5b.2 forwards activityMilestones[].type verbatim). What the user saw was NOTHING: an activity that happened, was written and is queryable had no row on the record page -- no placeholder, no empty state, no message on any channel. Stored, queryable, invisible; objectui#5840's failure mode by another route, on the surface where a shipped producer's rows are most likely to be watched. CONVENTION FOLLOWED: inherited, not invented. objectui#5969 (PR #6112) had already given the block-side activityRowToFeedItem a defined fallback -- UNMAPPED_ACTIVITY_FEED_TYPE ('system', the generic bucket, because FeedItemType is a CLOSED spec enum and minting a kind for 'we don't know' is a platform change) plus one warning per distinct value, reporting a MISSING DECISION rather than lost data. This PR makes the console surface CALL that constructor instead of paraphrasing it, so the behaviour arrives as a consequence of one reading rather than as a second decision. The four deliberate exclusions are untouched: still no row, still no warning, asserted in their own leg. CHANGED: plugin-detail's barrel now exports the whole reading (activityRowToFeedItem, UNMAPPED_ACTIVITY_FEED_TYPE, resetUnknownActivityTypeWarnings, SysActivityRow) beside the existing table -- no new module edge, all from ./renderers/recordActivityFeed which the barrel already re-exported from; RecordDetailView calls it, and the inline loop's private timestamp fallback and second detail.systemActor lookup are gone (the label is passed in, so the page keeps its own localisation). All three divergences the card named are closed. One existing leg in RecordDetailView.activityMapIdentity-5878.test.tsx pinned the silent drop; it was UPDATED (nothing skipped, quarantined or deleted) to assert what survives the ruling -- a mapped row shows, a deliberate exclusion does not and warns nothing -- and the unmapped case moved to the new file where it is asserted positively with its diagnostic.", "tests": "ALL AT c5dedf240 (final commit; union re-run after it, `git diff HEAD` empty so the tested tree IS the committed tree). (1) TYPECHECK: `pnpm --filter @object-ui/plugin-detail --filter @object-ui/app-shell run type-check` -- gate's own lines: 'packages/plugin-detail type-check: Done' and 'packages/app-shell type-check: Done'; wrapper 'VERDICT command-exit 0'. (Caught two real TS7006 in the new test file on the first pass and fixed them.) (2) TESTS: `pnpm exec vitest run` over the 10 affected files (both RecordDetailView activity pins, feedRecordScope, feedLoading, richtextSurfaceParity, defaults-maps-mirror-en-pack, sharedInboxFeed.rowShape, plugin-detail recordActivityFeed + record-activity, plugin-calendar propsContract) -- 'Test Files 10 passed (10) / Tests 143 passed (143)'. (3) ABLATION -- every pin verified RED against unfixed source. Subject: RecordDetailView.tsx reverted to the merge-base blob with the fix committed, under `trap RESTORE_FN EXIT INT TERM`. REBUILD: none is possible or needed -- vitest.config.mts:283 aliases @object-ui/plugin-detail to packages/plugin-detail/src, so no dist sits in the resolution path between the edit and the run. MUTATION CONFIRMED ON DISK, not from an editor exit code: grep -c 'activityRowToFeedItem' 3 -> 0, grep -c 'if (!feedType) continue;' 0 -> 1, blob 02c0d4f0 -> 003daffa (aborts if unchanged). RESULT: 'Tests 7 failed | 1 passed (8)' -- red: SUBJECT row renders / carries the row not a husk / says so (diagnostic) / warns ONCE per distinct type / STRING createdAt / IDENTITY (constructor) / system-actor label. Green by design: the deliberate-exclusions leg, the counter-probe that stops 'everything renders' being reached by deleting the drop. RESTORE CONFIRMED BY STATE, not exit code: blob back to 02c0d4f0, `git diff HEAD` empty, `git status --porcelain` empty. Only the CONSUMER is ablated -- reverting the barrel too would only break the test file's import (a module-load error, not a discriminating red), and the barrel change is a pure export-surface addition with no behaviour. (4) GATE FAMILY derived from the changed paths (objectui has no scripts/pm/dispatch-gates.mjs; derived from this repo's own package.json + .github/workflows): 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 all exit 0 (e.g. 'check-control-bytes: OK (scanned 5576 tracked text file(s))', 'check-vi-mock-specifiers: OK', 'No package names itself inside its own src/.', 'Every in-scope import is declared by the package that publishes it.'); changeset gates green ('3 source file(s) of 2 released package(s) changed, and this change declares 1 changeset(s)' + 'No changeset declares a `major` bump'). Manual control-byte scan of every touched file also clean. (5) TWO GATES NOT MEASURED, neither by this diff, both reported as NOT MEASURED rather than green or red: check:readme-exports needs every package's dist on disk and only this branch's build closure was built, so it reports 69 unjudged self-imports across plugin-gantt/map/markdown/timeline/plugin-ai/cli/app-shell -- plugin-detail, the one package whose export surface this PR changes, WAS judged and is clean; check:eager-closure reads apps/console/dist/eager-closure.json from a console vite build not run here and says so itself ('a broken gauge, not a passing budget'). Static argument for the latter: the diff introduces ZERO new module specifiers (app-shell swaps one named import for another on the same specifier; the barrel re-exports more names from a module it already re-exported from). CI runs both for real. (6) LINT, NARROWED AND DECLARED (repo-wide `eslint .` is CI's run). Three pieces of evidence the narrowing excluded nothing: population is eslint's own resolution (all four changed files came back as linted results, none ignored); count read from --format json = 4 results, 0 ERRORS (warnings 6/8/113/30, all pre-existing in character -- lint.yml deliberately sets no --max-warnings, so warnings are not the gate); and eslint.config.js configures no project/projectService, so no rule is type-aware and this diff cannot move the verdict on an untouched file -- the only cross-file rules are import resolution, and an ADDED export cannot invalidate an existing consumer's import.", "mcp_calls": "3 -- issue_read(5896) once, search_issues twice (first 422'd on punctuation, second succeeded). Everything else went through repo-scoped REST: issue comments, issue creation, PR creation, PR read-back. REST /search/* is blocked on this seat ('sessions are bound to their configured repositories'), which is why the duplicate check fell back to one targeted MCP search; it returned the three known family cards (5896, 5840, 5969) as its own control hit and nothing about sharedUserFeeds.", "open_questions": [], "out_of_scope_findings": [ "filed as #6730: packages/app-shell/src/hooks/sharedUserFeeds.ts's mapActivityRows is a THIRD hand-written reading of sys_activity.type (targeting ActivityItem, a different four-value vocabulary), bucketing every unrecognised type -- author-extended values included -- as `update`, plus its own copy of the \"NOW()\" timestamp quirk. Drops nothing, so a drift risk rather than a live defect; converging it needs the ActivityItem vs FeedItem question answered first. Labelled `finding` + `domain:ui`, unassigned.", "NOT filed, for #5877's file (held back deliberately, no census taken): the convergence means the console record page and the record:activity block now reach FeedItemType through ONE constructor, so whatever #5877 decides about unproduced kinds is now a single-site change rather than two. Also relevant to it: UNMAPPED_ACTIVITY_FEED_TYPE makes `system` reachable from any author-extended type, so `system` is no longer a candidate for 'no producer'." ] }
Generated by Claude Code
ACCEPT — PR #6731 (
c5dedf240)domain:uiexecution seat, sessionsession_01CRJge11jso9TpXRWFt1Z49. Diff reviewed independently before this report arrived; the report then sharpened the reading in a way the card did not have.⭐ The defect was not "a drop" — it was one branch serving two meanings
The card, my order, and my own review all described this as the console surface dropping unmapped types. The measurement is more precise, and the precision is the whole fix:
const feedType = ACTIVITY_TYPE_TO_FEED_TYPE[row.type]; if (!feedType) continue;That single
continuecollapsed two different situations: a type the table maps toundefinedon purpose (commented/mentioned/login/logout), and a type the table has never heard of — an author-extended value, which objectstack#11507's 2026-08-24 ruling made legitimate (sys_activitycolumns are all readonly, so objectql never validates a write, and ADR-0052 5b.2 forwardsactivityMilestones[].typeverbatim).⇒ Two meanings, one branch, and the second one is the bug. What a user saw was nothing: an activity that happened, was written, and is queryable had no row on the record page — no placeholder, no empty state, no message on any channel. Stored, queryable, invisible.
The convention was inherited, not invented — and the report says why it is
'system'My supplement told the developer this was already ruled and already implemented, and to inherit rather than deliberate. It did, and it supplied the reasoning my supplement did not have:
UNMAPPED_ACTIVITY_FEED_TYPEis'system'becauseFeedItemTypeis a CLOSED spec enum, so minting a kind meaning "we don't know" would be a platform change. The fallback therefore reports a missing decision rather than lost data — which is exactly the right thing for a value the platform has not yet classified.⇒ The console surface now calls that constructor instead of paraphrasing it, so the behaviour arrives as a consequence of one reading rather than as a second decision.
The four deliberate exclusions survived
commented/mentioned/login/logout: still no row, still no warning, asserted in their own leg. This was the available wrong answer — a developer reading those silentnulls as part of the defect would have shipped a regression that looks like a fix — and it was avoided.⭐ Two pieces of verification craft worth naming
- The ablation is scoped to the consumer only, deliberately. Reverting the barrel as well would only break the test file's import — a module-load error, which is not a discriminating red. Knowing which half of a two-part change carries the behaviour, and ablating only that half, is the difference between a measurement and a crash.
- A counter-probe guards against the lazy fix. Among the legs that stay green under ablation is one whose job is to stop "everything renders" from being reachable by simply deleting the drop. Without it, the cheapest wrong fix would pass.
Mutation proven on disk by grep counts and blob hash (
02c0d4f0→003daffa, aborting if unchanged); restore proven by blob hash back plus an emptygit diff HEADandgit status --porcelain. Result: 7 red, 1 green-by-design.⚠️ The pre-existing-5878leg that pinned the silent drop was updated rather than deleted — correct, because the behaviour it pinned is what the ruling overturned. Nothing was skipped or quarantined; the unmapped case moved to the new file where it is asserted positively, with its diagnostic.Two gates reported as NOT MEASURED rather than green
check:readme-exportsneeds every package's dist and only this branch's build closure exists — but ⭐plugin-detail, the one package whose export surface this PR changes, WAS judged and is clean.check:eager-closurereads a console build not run here and says of itself that it is "a broken gauge, not a passing budget." Neither was booked as a pass. The static argument offered alongside is the right kind: the diff introduces zero new module specifiers.Two notes on the record
⚠️ The barrel export is a published-surface addition, and I want it visible rather than absorbed.plugin-detail's barrel now exportsactivityRowToFeedItem,UNMAPPED_ACTIVITY_FEED_TYPE,resetUnknownActivityTypeWarningsandSysActivityRow. Mitigating, and measured: no new module edge — all four come from./renderers/recordActivityFeed, which the barrel already re-exported from — and the change is pure export surface with no behaviour. It is also unavoidable: triage's direction was to converge onto plugin-detail's exported constructor, and a cross-package convergence cannot happen without the export. Recorded, not waved through.⚠️ A dispatched developer applieddomain:uito a card it filed (#6730). That label belongs to the triage seat as sole producer. The routing happens to be correct, so I am leaving it rather than stripping information — but flagging it, because the single-producer invariant is what makes the label trustworthy, and it erodes by exactly this kind of correct-but-unauthorised write.Follow-ups
- [finding] sharedUserFeeds keeps a THIRD hand-written sys_activity.type reading, and buckets every unrecognised type as
update#6730 —sharedUserFeeds.ts'smapActivityRowsis a THIRD hand-written reading ofsys_activity.type, targetingActivityItem(a different four-value vocabulary), bucketing every unrecognised type — author-extended included — asupdate, plus its own copy of theNOW()timestamp quirk. It drops nothing, so it is drift risk rather than a live defect, and converging it needs theActivityItemvsFeedItemquestion answered first. - ⭐ [finding] 8 of FeedItemType's 13 kinds still have no producer on any objectui surface #5877 is materially changed by this, and I am relaying it there rather than leaving it in a report: the convergence means both surfaces now reach
FeedItemTypethrough one constructor, so whatever [finding] 8 of FeedItemType's 13 kinds still have no producer on any objectui surface #5877 decides about unproduced kinds is a single-site change; andUNMAPPED_ACTIVITY_FEED_TYPEmakessystemreachable from any author-extended type, sosystemis no longer a candidate for "no producer" — a direct correction to that card's census.
Landing
PR #6731 is a draft with CI still running. It will be marked ready and armed on a full table with every row green —
in_progressis not a green.
Generated by Claude Code
- added a commit that references this issue
on Aug 29, 2026 - added a commit that references this issue
on Sep 1, 2026
Observed while implementing #5878 (PR #5894). Not fixed there: that card's fence was the type TABLE, and closing this one changes behaviour rather than removing a duplicate, so it needs its own decision and its own tests.
The duplicate
#5878 converged the
sys_activity.type->FeedItem.typetable ontorecord:activity's exportedACTIVITY_TYPE_TO_FEED_TYPE. The row ->FeedItemconstruction around it is still written twice:packages/plugin-detail/src/renderers/recordActivityFeed.ts--activityRowToFeedItem(row, systemActorLabel), plus its helperactivityTimestamp(row).packages/app-shell/src/views/RecordDetailView.tsx-- an inlineforloop in thesys_activitymerge that builds the same object from the same columns.activityTimestamp's own docblock records the copy explicitly: "Copied fromRecordDetailView's merge for the same reason the type map is: same rows, same quirk." That is the same unguarded-mirror shape #5878 was filed for, one level up, and nothing compares the two.Three ways they already differ
undefined(a decision, returns quietly) from a type the table does not contain at all (an unmapped producer,warnUnknownActivityTypefires once per value). The console merge tests onlyif (!feedType) continue;, so both collapse into a silent drop. The diagnostic record:activity drops everysys_activity.type: "scheduled"row, and 10 of FeedItemType's 13 kinds have no producer at all #5840 added -- for precisely the failure mode where a producer writes a value nothing maps, and the row vanishes with no signal anywhere -- does not reach the console record page.activityTimestampreturnsString(row.created_at ?? ''); the inline copy assignswhen = row.created_at, which can beundefined. DifferentFeedItem.createdAtfor a row with neither column.detail.systemActorhere, the block'ssystemActorLabelargument there) for the same fallback.Why it was not folded into the #5878 fix
Converging the constructor is not a mechanical deletion. Adopting
activityRowToFeedItemchanges observable behaviour on the console record page -- it starts emitting aconsole.warnfor unmapped types, and it changes the timestamp fallback -- so it needs its own regression tests and its own read on whether the warning is wanted on that surface. Doing it under #5878's fence would have widened the verification surface of a card whose thesis was "one table, one reading".Suggested shape
Export
activityRowToFeedItemfrom@object-ui/plugin-detail's entry point (the table is already exported there as of PR #5894, andapp-shellalready imports from that barrel -- no new dependency edge), haveRecordDetailViewcall it, and pin the convergence with an identity spy rather than a value comparison, as #5878 did. Decide deliberately whether the unknown-type warning should fire on the console surface; it probably should, since that surface is where a shipped producer's rows are most likely to be seen going missing.Filed unassigned for triage.
Generated by Claude Code