Repository navigation
fix(app-shell): bind the host discussion fallback to the feed pipeline - #9011
Merged
os-steve merged 1 commit intoSep 10, 2026
Merged
Conversation
A record page shows its discussion through one of two surfaces: the authored/synthesized `record:discussion` / `record:chatter` block, which renders through `RecordChatterRenderer`, or the host's bottom auto-append for authored pages that omit the block. Since objectui#8934 the renderer runs `applyFeedConfig`; the fallback reached past it to `RecordChatterPanel` with raw rows, so the two rendered different feeds from the same rows — a completed activity showed on one and not the other, and a feed past twenty rows rendered whole on one and paged on the other. Bind, do not copy: the fallback now mounts `RecordChatterRenderer` with no authored schema, so it reads the feed off the `DiscussionContext` the view already provides and runs the one pipeline. No new package dependency — `@object-ui/plugin-detail` is already app-shell's peer dependency and already the source of this file's detail imports. The new pin asserts the two surfaces render the same rows, with the pre-fix fallback shape rendered on the same feed as the control that proves they used to differ, and a neutral fixture on which all three agree. Part of #8983 Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MPaVWWMuWeT5LgB1qoXjVB
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-steve
marked this pull request as ready for review
September 10, 2026 17:59
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Sep 10, 2026
os-steve
deleted the
claude/issue-8983-host-discussion-fallback-feed-pipeline
branch
September 10, 2026 18:52
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 #8983
One pipeline, not two. The host discussion fallback now mounts the same
renderer the authored / synthesized
record:discussion/record:chatterblock does, instead of reaching past it to
RecordChatterPanelwith raw rows.Drafted by an agent seat in Claude Code session
session_01MPaVWWMuWeT5LgB1qoXjVB(the id is written here as prose because a footer link does not survive an edit
to this body).
Why the fallback exists — measured, not assumed
The triage ruling required establishing this before binding anything, since a
host context where the pipeline genuinely cannot run would call for saying so
visibly rather than binding.
It can run. Measured against
origin/mainate8b7b0785:showAutoDiscussion = !disableDiscussion && !hasDiscussion && feedsEnabledin
packages/app-shell/src/views/RecordDetailView.tsx. So the fallback firesfor exactly one population: an authored page that omits a discussion
block. A synthesized default page always emits
record:discussion(
buildDefaultPageSchema.ts, thehideDiscussionbranch), so it neverreaches the fallback. The tree says the same in as many words two lines above
the gate: "the legacy bottom-of-page append below stays only as the fallback
for authored pages that don't slot the panel themselves."
DiscussionContextProviderthis same view opens (provider opens at theitems={feedItems}mount, closes 172 lines later; the fallback is betweenthem), itself inside
RecordContextProvider.RecordChatterRendererneedsexactly those two contexts and nothing else.
mutation handlers, plus mention suggestions — are the same six the provider
above it is already given, from the same closure variables.
So the honest answer is the binding one, and the "say so visibly" branch of the
ruling does not apply.
Dependency measurement
Binding adds no package dependency edge:
@object-ui/plugin-detailis already declared by@object-ui/app-shellinboth
peerDependencies(workspace:^) anddevDependencies(workspace:*).RecordDetailView.tsxalready imports from that specifier — the edit swapsone named import (
RecordChatterPanel) for another (RecordChatterRenderer)in the same existing import statement. No new specifier, no new edge.
RecordChatterRendereris exported from the package barrel(
packages/plugin-detail/src/index.tsx), not a deep path.How it binds
packages/app-shell/src/views/RecordDetailView.tsx, theshowAutoDiscussionbranch, is now a single
RecordChatterRendererwithclassName="mt-6"and noschema. That is the component
ComponentRegistryresolves bothrecord:discussionandrecord:chatterto, so it is literally the samepipeline rather than a second copy kept in agreement by hand.
branch used to hard-code (
position: 'bottom',collapsible: false,reactions / threading / comment input) plus the protocol's
feeddefaults.itemsprop; it reads the feed off theDiscussionContextalready in scope. DOM is unchanged: the renderer emits itsown
divcarryingclassName, so the old wrapperdivis not lost.SchemaRendererwas the alternativeconsidered and not taken: it would add
hidden/disabledexpressionevaluation over a node nobody authored, to arrive at the component mounted
here anyway.
Two comments that this change makes false were rewritten in the same edit — the
record-chatter.tsxnote that recorded the divergence as live, and theEMPTY_FEEDnote inRecordDetailView.tsxthat namedRecordChatterPanelas adirect prop consumer.
The equivalence assertions, and the controls that make them readings
New pin:
packages/app-shell/src/views/RecordDetailView.discussionFallbackPipeline-8983.test.tsx.It drives the real
RecordDetailViewtwice on the same feed — once with anauthored page that omits the block (fallback), once with no authored page at all
(synthesized default, so the block) — and asserts the rendered row lists are
equal and equal to a stated value.
"The two agree" is satisfiable by a harness in which they could never have
disagreed, so three legs hold it up:
PreFixFallbackShapeis the fallback's JSX as written before this card — same panel, same hard-coded affordances, rawitems, nohasMore/onLoadMore. Rendered on the same feed, derived from the same source rows through the exportedactivityRowToFeedItem, and asserted to produce a different row set.The three fixtures are the card's own probe table, re-run through the real view:
A fourth case clicks "Load more" on the fallback and asserts the window grows to
all 25 rows and the button retires —
hasMorewithoutonLoadMorerenders abutton that does nothing, which satisfies any assertion that only counts it.
Ablation
Two arms, both run from the committed tree so the restore leg has something to
restore to, and both proving the edit reached disk before any result was read.
MUTATION arm — the fallback branch and its import reverted to the pre-fix
raw shape (
RecordChatterPanelwithitems={feedItems}). On-disk proof takenbefore the run, anchored on the exact text each way:
RecordChatterRenderermount sites went 1 to 0,
RecordChatterPanelmount sites 0 to 1, and the file'sblob hash moved from
f4fb0b7to6e18560(a hash comparison, because anediting tool exits 0 on zero matches). Result: 3 of 4 failed, and they
failed for the stated reason rather than by crashing —
i.e. exactly the divergence the card measured: the completed activity back on
screen, and the page window gone. The fourth case passed — and it is the
NEUTRAL FIXTURE, which is the predicted arm, not a miss: on a feed with nothing
to filter and nothing to page, binding the pipeline changes nothing, so that leg
must stay green under the mutation or it was never a control.
RESTORE arm —
git checkout HEAD -- packages/app-shell/src/views/RecordDetailView.tsx,verified by
git diff HEADbeing empty and the blob hash back tof4fb0b7(not by the restore command's exit code, and not by the trap having fired).
Re-run: 4 of 4 pass.
Gates
turbo run type-check --filter=@object-ui/app-shell --filter=@object-ui/plugin-detailtsc --noEmit && tsc -p tsconfig.test.json, so the new pin is type-checked tooturbo run lint --filter=@object-ui/app-shell --filter=@object-ui/plugin-detail0 errors(2967 pre-existing warnings, unchanged in kind)pnpm exec vitest run packages/plugin-detail/pnpm exec vitest run packages/app-shell/find packages/app-shell -name "*.test.ts*"counts 676), so the filter did not silently narrowpnpm exec vitest run ...discussionFallbackPipeline-8983.test.tsxcheck:control-bytescheck:changeset-presencenode scripts/check-changeset-presence.mjscheck:governed-queue-guard --testcheck:doc-snippetscheck:doc-typescheck:doc-examplescheck:new-line-citationscheck:vi-mock-specifiers/-inherit/-override-shapevi.mockcheck:phantom-deps/check:unused-deps/check:unreferenced-sourcescheck:eager-closureapps/consolebuild to have a gauge at all, and it is a CI job. Reasoned unaffected: bothRecordChatterPanelandRecordChatterRendererare re-exported from the SAME already-eagerly-imported@object-ui/plugin-detailbarrel, so no module enters or leaves the closureAll heavy runs went through the container's shared verify lock; the wall-clock
figures in those logs are shared-box readings, so only the verdicts are quoted.
Both doc gates first returned exit 2 — PREREQUISITE NOT MET (packages
unbuilt), which is not a pass; the scoped build each names was run and they were
re-run to the exit codes above.
Assumptions from the dispatch, checked
buildDefaultPageSchema.ts:880-883emitsrecord:discussionon everysynthesized default page" — holds, and the anchors happen to be exact at
this base. Re-derived by symbol:
buildDefaultDiscussionis defined at:882and called at:1031, guarded only by an authoredslots.discussionoverride or the explicit
hideDiscussionoption. Same for the triage'sRecordDetailViewanchors (:2182,:2549,:1551) — all three landed onthe named construct.
measurement above. Not a manual-floor item.
RecordChatterPanelmount sites outside tests finds exactly two, plus thecomponent's own definition.
record:activityis a different block, and thedesigner palette entry in
metadata-admin/previews/block-types.tsis acatalogue row, not a mount.
the dispatch — see the report; no overlap with the four PRs named there.
Not in scope
objectui#8934's own ruling is untouched; this is its residue. objectui#8968
(
filterMode/enableMentionsstill unread on the chatter path) is unaffectedeither way — it is a gap in the shared pipeline, so binding the fallback to that
pipeline neither widens nor narrows it.
Acceptance notes
Seen while working here, deliberately NOT filed and NOT fixed in this PR — none
is a reproducible defect, a broken declared contract, or a trap that makes an
author write metadata the runtime drops:
RecordChatterPanelnow has no mount site anywhere in this repository outsideRecordChatterRendererand this PR's own pre-fix control. It remains apublished export of
@object-ui/plugin-detailand consumers mount it, so thisis not dead code — recorded only so the single remaining internal mount does
not read as an accident to the next person here.
AGENTS.md's testing section quotesapp-shellas having "281" test fileswhile illustrating a historical trap. Measured today: 676. Nothing reads that
number mechanically, it sits inside an anecdote rather than a judged
population, and
AGENTS.mdis a governed surface where a drive-by edit costsa human merge. No taker identified; noted rather than filed for that reason.
objectui#8920,#8996,#8998,#8999). Re-derived live: eight PRs are open and none ofthem is any of those four. Adjacency was therefore re-derived rather than
cited, and no open PR touches either file this PR edits.
🤖 Generated with Claude Code
https://claude.ai/code/session_01MPaVWWMuWeT5LgB1qoXjVB
Generated by Claude Code
Generated by Claude Code