fix(data-warehouse): show a catalog placeholder while new source loads - #105634
Conversation
The new-source page rendered a single full-width LemonSkeleton bar while the connector list loaded. With over a thousand source configs in that request, the wait runs for seconds and the page reads as blank, so people leave before the catalog arrives. The page now keeps its title and renders a catalog-shaped placeholder: the category sidebar, the search field, and a grid of tiles on the same layout the real catalog uses. Generated-By: PostHog Desktop Task-Id: b66dd596-8579-45e2-9bb3-c2db018e2bcb
|
😎 Merged directly without going through the merge queue, as the queue was empty and the PR was up to date with the target branch - details. |
Gilbert09
added
the
stamphog
Talyn App
🤖 CI report✅ Trunk lane — non-backend laneThis PR is assigned to the non-backend lane. It does not run backend Python tests and may merge in parallel with PRs in other lanes.
|
| Function | Location | Complexity | Limit |
|---|---|---|---|
InternalSourcesWizard |
products/data_warehouse/frontend/scenes/NewSourceScene/NewSourceScene.tsx:231 |
19 | 10 |
CDCSelfManagedSetupDialog |
products/data_warehouse/frontend/scenes/NewSourceScene/NewSourceScene.tsx:415 |
15 | 10 |
SecondStep |
products/data_warehouse/frontend/scenes/NewSourceScene/NewSourceScene.tsx:571 |
12 | 10 |
✅ Duplication (Python) — clean
New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.
✅ Duplication (TypeScript) — clean
New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.
🚨 Comment density — 14% of added code lines are comments (11 of 76)
This section warns when comments are more than 3% of the code lines a PR adds, and alerts above 6%. Before agent-assisted PRs, the typical share was about 2%. Only full-line comments count. Docstrings, generated files, snapshots, migrations, and workflow files are left out.
Comments that restate the code, record how the change came about, or narrate the next line add noise for the next reader. Keep the comments that explain a reason the code cannot show, and remove the rest. See .agents/skills/writing-code-comments/SKILL.md for the house rules.
Files with the most added comment lines:
| File | Comment lines | Added lines |
|---|---|---|
products/data_warehouse/frontend/scenes/NewSourceScene/SourceCatalogSkeleton.tsx |
6 | 37 |
products/data_warehouse/frontend/scenes/NewSourceScene/SourceCatalogSkeleton.stories.tsx |
3 | 22 |
products/data_warehouse/frontend/scenes/NewSourceScene/SourceCatalog.tsx |
2 | 9 |
This check does not block merging. It updates on every push and clears when the share drops.
✅ Bundle size — no change
Uncompressed size of every built .js bundle, compared against the base branch.
Total: 68.88 MiB · no change
No file changed by more than 1000 B.
Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report
✅ Eager graph — within budget
How much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy import() / React.lazy chunks are not counted.
| Root | Eager (shipped) | Δ vs base | Budget |
|---|---|---|---|
entry (logged-out pages, app bootstrap)src/index.tsx |
1.57 MiB · 22 files | 🔺 +91 B (+0.0%) | █████████░ 85.2% of 1.84 MiB |
logged-out boot: index + App + bootApp (preloaded by every page, including /login)src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts |
3.58 MiB · 628 files | 🔺 +91 B (+0.0%) | █████████░ 88.8% of 4.03 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
7.37 MiB · 2,326 files | 🔺 +91 B (+0.0%) | █████████░ 88.4% of 8.34 MiB |
🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/layout/navigation-3000/navigationLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/scenes/dashboard/dashboardLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/lemon-ui/LemonMarkdown/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/RichContentEditor/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/CodeSnippet/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/taxonomy/core-filter-definitions-by-group.json stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
Largest files eagerly shipped from src/index.tsx
| Size | File |
|---|---|
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 24.6 KiB | ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js |
| 6.3 KiB | ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js |
| 4.5 KiB | ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js |
| 3.9 KiB | ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js |
| 1.4 KiB | ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js |
| 1.3 KiB | src/index.tsx |
| 1.3 KiB | src/RootErrorBoundary.tsx |
| 912 B | ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js |
| 854 B | src/scenes/ChunkLoadErrorBoundary.tsx |
Largest files eagerly shipped from src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
| Size | File |
|---|---|
| 301.8 KiB | ../node_modules/.pnpm/posthog-js@1.434.14_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 267.6 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 100.4 KiB | src/lib/api.ts |
| 85.5 KiB | src/products.tsx |
| 69.1 KiB | src/lib/lemon-ui/icons/icons.tsx |
| 63.9 KiB | src/lib/utils/eventUsageLogic.ts |
| 38.7 KiB | ../node_modules/.pnpm/@dnd-kit+core@6.0.8_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@dnd-kit/core/dist/core.esm.js |
| 33.9 KiB | ../node_modules/.pnpm/kea@4.0.0-pre.6_patch_hash=139b8d1f1304f9d9da452a9a1244c94ea679dbcb85687d8999563146879fb6f5_react@18.3.1/node_modules/kea/lib/index.cjs.js |
| 28.3 KiB | src/scenes/scenes.ts |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 301.8 KiB | ../node_modules/.pnpm/posthog-js@1.434.14_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 271.7 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 267.6 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 153.7 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 100.4 KiB | src/lib/api.ts |
| 98.5 KiB | ../packages/quill/packages/quill/dist/index.js |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 90.6 KiB | ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js |
| 85.5 KiB | src/products.tsx |
Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479
✅ Toolbar bundle — eager 2.37 MiB within budget
What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.
| Metric | Size | Δ vs base | Budget |
|---|---|---|---|
| Eager (shipped) entry + static imports |
2.37 MiB · 19 files | 🔺 +91 B (+0.0%) | ████░░░░░░ 41.4% of 5.72 MiB |
| Deferred (lazy) | 2.10 MiB · 44 files | no change | n/a — loads on demand |
Loader dist/toolbar.js |
1.2 KiB | no change | █░░░░░░░░░ 6.0% of 19.5 KiB |
Largest eagerly-shipped chunks
| Size | File |
|---|---|
| 791.8 KiB | dist/toolbar/toolbar-app-TDLXUZLE.css |
| 650.8 KiB | dist/toolbar/chunk-chunk-OLZXKW3U.js |
| 483.6 KiB | dist/toolbar/chunk-chunk-LP5DDLVQ.js |
| 138.3 KiB | dist/toolbar/chunk-chunk-FVYKO6VU.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-FDH2IBXT.js |
| 75.2 KiB | dist/toolbar/toolbar-app-M6TDBA3J.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-TSAL54PB.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-GL4SRUHV.js |
| 21.0 KiB | dist/toolbar/chunk-chunk-Z4YQYAC3.js |
| 6.8 KiB | dist/toolbar/chunk-chunk-DV7IWQNF.js |
Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile
✅ Dist folder size — 🔺 +10.6 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 944.91 MiB · 🔺 +10.6 KiB (+0.0%)
🦔 Hogbox preview · ✅ ready▶ Open the preview
commit |
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approved.
Purely additive UI change (loading skeleton placeholder) with no data/API/auth surface touched; diff matches the description and there are no unresolved review concerns.
- Author wrote 0% of the modified lines and has 47 merged PRs in these paths (familiarity MODERATE).
- copilot-pull-request-reviewer[bot] reviewed the current head.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 78L, 4F substantive — within ceiling |
| tier | ✓ | T1-agent / T1b-small (78L, 4F, single-area, fix) |
| stamphog 2.0.0 | .stamphog/policy.yml @ 694a70d · reviewed head 694a70d |
fuziontech
left a comment
fuziontech
left a comment
There was a problem hiding this comment.
Automated review agent on behalf of @fuziontech.
APPROVE. I reviewed the complete diff against master and the surrounding loading/catalog paths. The loading branches are consistent, the skeleton reuses the catalog grid/tile sizing, and the added story covers the narrow scene width. No blocking issues found.
Non-blocking: Node/pnpm are unavailable in this VM, so the focused frontend lint/type checks could not run. git diff --check passed.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⚙️ Run configurationConfiguration used: Repository: PostHog/posthog/.coderabbit.yaml Review profile: QUIET Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: PostHog/posthog/.coderabbit.yaml Review profile: QUIET Plan: Enterprise Run ID: 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughThe source catalog now has a skeleton loading component that uses shared tile and grid classes. Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to The PR adds loading placeholders for the source catalog and their dedicated snapshots. No concrete merge risk is identified in the supplied changes. Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 2 systems. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (1)
products/data_warehouse/frontend/scenes/NewSourceScene/SourceCatalogSkeleton.tsx-14-15 (1)
14-15: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep mobile category skeletons visible and scrollable.
Below
sm,LemonSkeleton repeat={...}renders each item inside an unstyled wrapper. Theh-8class provides no width, so the placeholders can collapse to their 1px accessibility content. Add a mobile width to each wrapper and horizontal scrolling.Suggested fix
- <div className="flex flex-row sm:flex-col gap-1 sm:w-56 sm:shrink-0"> - <LemonSkeleton className="h-8" repeat={PLACEHOLDER_CATEGORY_COUNT} /> + <div className="flex flex-row sm:flex-col gap-1 overflow-x-auto sm:w-56 sm:shrink-0"> + {Array.from({ length: PLACEHOLDER_CATEGORY_COUNT }, (_, index) => ( + <div key={index} className="w-32 shrink-0 sm:w-full"> + <LemonSkeleton className="h-8" /> + </div> + ))} </div>
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 8a6f88c2-f6f0-4629-8743-4d2b74a76c26
📒 Files selected for processing (4)
products/data_warehouse/frontend/scenes/NewSourceScene/NewSourceScene.tsxproducts/data_warehouse/frontend/scenes/NewSourceScene/SourceCatalog.tsxproducts/data_warehouse/frontend/scenes/NewSourceScene/SourceCatalogSkeleton.stories.tsxproducts/data_warehouse/frontend/scenes/NewSourceScene/SourceCatalogSkeleton.tsx
Included review availability: Your plan provides up to 12 included reviews per hour; 10 remain after this review.
…pshots The snapshot runner waits for every `.LemonSkeleton` to disappear before it captures, so a story that is entirely skeletons never settles. Opt the story out of that wait. Also give each category placeholder its own width and let the row scroll. The `repeat` prop wraps each skeleton in a bare div with no width, which a flex row collapses to nothing. Generated-By: PostHog Desktop Task-Id: b66dd596-8579-45e2-9bb3-c2db018e2bcb
A new stamphog review started for this PR — the fresh verdict replaces this approval.
|
Applied, plus the cause of the failing Storybook flake check: the snapshot runner waits for every |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
|
|
Status check: no unresolved review threads (0 open), and GitHub reports The only red checks are Committing a new visual baseline is a ship action that PostHog Visual Review policy requires an explicit human "yes" for on this specific run — I can't finalize it myself from here (and can't render the artifact PNGs from this sandbox to substitute my own sign-off). Ask: could a human reviewer open the Visual Review run, confirm the 4 new skeleton screenshots look right, and approve/finalize the run? That should turn both checks green with no further code changes needed. 🦉 via talyn.dev |
|
✅ Visual changes approved by @Gilbert09 — baseline updated in 4 new. |
|
Update after syncing this branch with Ask (same as before, updated link): could a human reviewer open the current Visual Review run, confirm the 4 new skeleton screenshots look right, and approve/finalize it? That's the only thing keeping 🦉 via talyn.dev |
|
/trunk merge |
|
Re-checked from a clean clone: no unresolved review threads (0 open), One correction to my earlier summary of that run, from reading its job log directly: it reports 1370 snapshots — 1365 unchanged, 1 changed, 4 new, not "4 new, no regressions" as I said last time. The 4 I can't view the rendered snapshot images or approve/finalize a Visual Review run from this sandbox — that's a sign-off action outside what I can do here, and outside this PR's scope for the drifted Ask: could a human reviewer open the current Visual Review run and:
No further code changes are needed on this PR to turn the gate green once that's resolved. 🦉 via talyn.dev |
4 updated Run: 3b8d85a9-d5b5-4b31-b147-c62f34036fab Co-authored-by: Gilbert09 <1459269+Gilbert09@users.noreply.github.com>
A new stamphog review started for this PR — the fresh verdict replaces this approval.
|
/trunk merge |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
A new stamphog review started for this PR — the fresh verdict replaces this approval.
There was a problem hiding this comment.
Approved.
Purely additive, reversible frontend loading-state UI change with no touch on data models, APIs, auth, or dependencies; an agent reviewer approved and the one minor CodeRabbit suggestion was addressed in a follow-up commit, and the author has strong recent familiarity with these exact files.
- Author wrote 100% of the modified lines and has 86 merged PRs in these paths (familiarity STRONG).
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 99L, 5F substantive — within ceiling |
| tier | ✓ | T1-agent / T1b-small (99L, 5F, two-areas, fix) |
| stamphog 2.2.0 | .stamphog/policy.yml @ 8bc9f58 · reviewed head 8bc9f58 |
|
Synced this branch with CI on the new head commit ( /trunk merge |
Problem
Someone opening "New source" sees a blank page for several seconds before the connector catalog appears, and some of them leave rather than wait. Replay Vision scanner sessions on this flow caught it twice: one user waited on an empty page and closed the wizard, another got nothing past the layout shell.
The page renders one full-width
LemonSkeletonwhileavailableSourcesLogicloads. That is a 4px-tall bar on an otherwise empty page, and the request behind it carries over a thousand source configs.Changes
SourceCatalogexports those two class strings instead of holding them privately.NewSourcesWizard) gets the same placeholder without the scene title.No behavior changes. The loading branch is the only thing this touches.
Before
After
How did you test this code?
Release status
Automatic notifications
Docs update
None.
🤖 Agent context
Autonomy: Fully autonomous
Agent: Claude Code, Opus 5
Opened from a sweep over Replay Vision observations of the data warehouse new-source onboarding flow. No duplicate: searched open PRs for the source catalog, the new source page and skeletons, and checked the maintainer's own open PRs. Nothing covers this loading state.
Skills invoked:
/writing-ui-components,/writing-user-facing-copy,/writing-tests,/writing-code-comments,/writing-pr-descriptions.CodeRabbit CLI was unavailable in this sandbox, so this PR opened without a local review pass.
Public artifact: the session material was customer session summaries. Nothing from them reaches the diff or this description, and the screenshots show placeholders only.