Uh oh!
There was an error while loading. Please reload this page.
fix(app-shell): the console record page reads record:activity's sys_activity table instead of copying it - #5894
Merged
Conversation
…ctivity table instead of copying it `RecordDetailView`'s `sys_activity` merge carried a hand-written copy of the table `record:activity` exports as `ACTIVITY_TYPE_TO_FEED_TYPE`. Neither file imported the other and nothing compared them, so the two could drift with nothing going red -- and objectui#5840 drifted them: it added `scheduled` -> `event` to the exported table (HotCRM's `schedule_meeting` writes that value) and left the copy here untouched. The same row rendered on a hand-authored record page and was dropped on the console record page. - `plugin-detail` re-exports `ACTIVITY_TYPE_TO_FEED_TYPE` from its entry point; the module holding it was already in the eager closure via the renderer beside it, so nothing new is pulled in. - `RecordDetailView` imports it and the local literal is deleted. - The pins are IDENTITY, not membership: a getter spy on the shared object and a member injected into it at runtime. A member-identical private copy -- the failure being removed -- fails both, where a value comparison would pass on the defect. Fixes#5878 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EuPCi56cnGyykygi3z9w4m
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
|
os-zhuang
marked this pull request as ready for review
August 23, 2026 18:43
Uh oh!
There was an error while loading. Please reload this page.
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 freeto 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#5878
What this fixes
RecordDetailView'ssys_activitymerge hand-copied the table thatrecord:activityexports asACTIVITY_TYPE_TO_FEED_TYPE. Neither fileimported the other and nothing compared them, so the two could drift with
nothing going red -- and #5840 drifted them: it added
scheduled->eventtothe exported table (a shipped producer, HotCRM's
schedule_meeting, writesthat value) and left the copy here untouched. The same
sys_activityrowrendered on a hand-authored record page and was dropped on the console record
page.
The copy is deleted.
RecordDetailViewreads the exported table.The measured dependency direction, and the shape it decided
Measured before choosing a shape, on
origin/main:@object-ui/app-shelldeclares@object-ui/plugin-detailinpeerDependenciesanddevDependencies(not independencies).RecordDetailView.tsxitself already imports from@object-ui/plugin-detail-- line 13 onorigin/mainpulls inRecordChatterPanel,InlineEditSaveBar,buildDefaultPageSchema,deriveFieldGroupDetailSections,extractMentions,resolveTitleFieldanduseRecordEditable. Twelve such specifiers appear acrossapp-shell/src.node scripts/check-phantom-dependencies.mjsagrees the edge is declared:Every in-scope import is declared by the package that publishes it.So the edge already exists, in this very file. Shape (a) applies:
RecordDetailViewimports the shared table and deletes its literal. No newdependency edge is created, no re-homing, no equality-pin stopgap.
One enabling change was needed: the table was exported from
renderers/recordActivityFeed.tsbut not from the package entry point, soit was not importable from outside the plugin.
packages/plugin-detail/src/index.tsxnow re-exports it. That adds no module to the eager closure -- on
origin/mainindex.tsx:24already statically imports./renderers/record-activity, whichstatically imports
./recordActivityFeedat its line 72, with zero dynamicimports anywhere on that chain.
recordActivityFeed.ts's membership is untouched.Why the pins are IDENTITY, not membership
Every value assertion here is also satisfied by a private literal holding
today's members -- i.e. by exactly the re-fork being removed, which would pass
on the defect. Two pins decide the convergence, and neither can be
satisfied by a copy:
scheduledisrecorded only if it read that object's property;
contain it, so the seeded row is dropped and the leg goes red.
(2) is deliberately mechanism-independent (it survives any change in how Vitest
implements accessor spies) and states the card's thesis executably: one edit to
the shared table must reach both surfaces.
The identity spy is real because the specifier is identical on both sides.
vitest.config.mtsaliases@object-ui/plugin-detailtopackages/plugin-detail/src, and both the test andRecordDetailViewimportthrough that specifier, so ESM hands them one module object -- no
diststandsbetween an edit and the run.
Reverse verification
Every leg: mutation applied, confirmed on disk by grepping the injected and
the removed text (never an editor's exit code), run, restored by the script's
trap ... EXIT INT TERM,git diff HEAD --statempty. Direction predictedbefore each run; both matched.
Green baseline on this branch --
Tests 7 passed (7).Leg A -- a member-identical private copy,
scheduledincluded. The pins'whole reason for existing. Disk check:
injected_read_site=1 injected_literal=1 removed_read_site=0 private_scheduled_entry=1.The
scheduledbehaviour leg passed on this defect, as predicted. That isthe demonstration: a value-only assertion would have gone green on the very
fork this change removes.
Leg B -- the historical pre-#5840 copy,
scheduledabsent. Reproduces theshipped bug. Disk check:
private_scheduled_entry=0.Leg C -- cross-package, that
tscreads the REBUILT declaration.packages/app-shell/tsconfig.test.jsonsets"paths": {}, so@object-ui/plugin-detailresolves through the builtdist/index.d.ts, notthrough source. A probe key the real type rejects was appended to the test
file:
TS1360 naming the real exported union -- not TS2305 "has no exported member",
which is what a stale declaration would have produced. That is what makes the
probe discriminating.
Legs that do NOT discriminate, and why they are kept. The three regression
controls stayed green through both ablation legs by construction -- a
member-identical copy answers them exactly as the shared table does. They are
controls against a convergence that moved membership, not pins. Likewise
ACTIVITY_TYPE_TO_FEED_TYPE.scheduled === 'event'is a statement of where thereading lives, green in every leg. And the "the row rendered at all" assertion
inside each identity leg is a LIVE control on the same read as the subject, so
a harness that stopped delivering rows fails as a broken probe rather than
reporting the consumer forked.
Verification
All commands from the repo root; exit codes captured before any pipe.
Union re-run after the final commit, on
6ee1addf6:The full
packages/app-shellsuite was deliberately not run (~784 s, and itholds the shared verify lock).
Type-check, after
pnpm --filter '@object-ui/app-shell^...' build(29 packages)-- the script name is echoed in each log, so a zero-match
--filtercannot readas green:
Gate verdict lines, each quoted from the gate's own output:
check-changeset-presence.mjs2 source file(s) of 2 released package(s) changed, and this change declares 1 changeset(s)check-changeset-no-major.mjsNo changeset declares a 'major' bump.check-control-bytes.mjscheck-control-bytes: OK (scanned 4902 tracked text file(s); skipped 85 binary).check-phantom-dependencies.mjsEvery in-scope import is declared by the package that publishes it.check-package-self-import.mjsNo package names itself inside its own src/.check-spec-symbol-derivation.mjsspec symbol derivation: 1300 files scanned against 4959 spec export namescheck-i18n-call-site-keys.mjsEvery in-scope call-site key resolves against the en pack (2929 keys)check-node-esm-load.mjs --specifiers-onlyno un-ledgered package emits an extensionless relative specifierDeclared narrowings
Lint. Ran ESLint over the 3 touched files instead of the repo, and the
narrowing is a measurement, not a skip:
isPathIgnored, not a guessabout which files count): 3524 tracked
.ts/.tsxfiles are in scope, 0ignored.
--format json: 3.eslint EXIT=0,errorCount 0, 147 warnings.
the root flat config uses
tseslint.configs.recommendedand itslanguageOptionsdeclares noparserOptions.project/projectService. Soevery rule's verdict is a function of that file's own text plus the config;
this diff changes neither the config nor any other file, and cannot move an
untouched file's verdict.
The 147 warnings were measured against the base rather than assumed. Linting
origin/main's copies of both changed files givesindex.tsx28 warningsbefore and 28 after, and
RecordDetailView.tsx113 before and 113 after --the re-export added none. The only new warnings are 6
@typescript-eslint/no-explicit-anyin the new test file, matching the fake-
dataSourceconvention of the siblingharness it is modelled on.
Eager-closure budget -- NOT RUN, and not a pass.
node scripts/check-eager-closure-budget.mjsexits 2, its documented"broken gauge, not a passing budget" code, because it reads
apps/console/dist/eager-closure.json, which only a consolevite buildwrites. Producing it locally needs
app-shell+plugin-gantt/plugin-markdown/plugin-timelinedists that are not inapp-shell'sdependency closure; the attempt failed on
TS2307 Cannot find moduleforexactly those, unrelated to this diff.
performance-budget.ymlbuilds theconsole and runs the gauge on this PR. The specific claim it would check is
supported statically above: the re-exported module was already eagerly reachable
from the barrel on
origin/main.Out of scope, deliberately
Only the table converges. The row-to-
FeedItemconstruction around it isstill written twice -- this merge builds the item inline where the block calls
activityRowToFeedItem-- so the timestamp fallback and the actor fallback arestill duplicated, and an unmapped activity type is still dropped silently
here where the block warns once. That is the same class of mirror one level up
and is filed separately; it is not addressed here.
#5877 (the unreachable
FeedItemTypekinds) and objectstack#11424 (whether theplatform vocabulary should gain
scheduled) remain open and are out of scopefor this card.
Generated by Claude Code