feat(tracing): split viewer UI state out of tracingSceneLogic - #69890
Conversation
tracingSceneLogic mixed two concerns: URL sync for /tracing (a scene concern) and the drawer/compare-flame UI state plus filter-change query wiring (viewer concerns). An embedded viewer must work with no URL, and the re-query on filter change was a side effect of the scene's actionToUrl, so embeds would never refresh. Adopt the logsSceneLogic / logsViewerLogic boundary: - New keyed tracingViewerLogic owns the trace drawer (selected trace/span/ts, open/close, prefetch-or-fetch decision) and the compare-flame modal. URL-free by design. - tracingDataLogic (keyed) now owns "filter changed -> capture -> runQuery" via listeners on the connected filter actions, mirroring the logs viewer's handleQueryChange, plus an autoLoad prop so embeds can run their first query on mount. - tracingSceneLogic keeps ALL urlToAction/actionToUrl, breadcrumbs, and scene tabs; it now just listens to the keyed logics' actions to write the URL (buildUrl no longer re-runs the query) and re-exports their values/actions for the scene component. /tracing behavior is unchanged: same URL params, same deep links, same query cadence. Generated-By: PostHog Code Task-Id: 598eae22-71d0-445a-ab9d-8dcdce076aa1
🤖 CI report
|
| File | Size | Δ vs base |
|---|---|---|
posthog-app/src/scenes/feature-flags/FeatureFlagForm.js |
22.3 KiB | 🔺 +22.3 KiB (new) |
posthog-app/src/scenes/feature-flags/FeatureFlag.js |
94.4 KiB | 🟢 -21.2 KiB (-18.3%) |
posthog-app/_parent/products/feature_flags/frontend/staff/FeatureFlagsStaffToolsScene.js |
15.3 KiB | 🔺 +4.7 KiB (+44.5%) |
render-query/src/render-query/render-query.js |
21.21 MiB | 🔺 +1.5 KiB (+0.0%) |
posthog-app/_parent/products/tracing/frontend/TracingScene.js |
91.1 KiB | 🔺 +1.2 KiB (+1.3%) |
posthog-app/src/scenes/cohorts/Cohort.js |
35.4 KiB | 🔺 +1005 B (+2.9%) |
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.21 MiB · 22 files | 🔺 +7 B (+0.0%) | ███░░░░░░░ 28.1% of 4.29 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
8.11 MiB · 2,973 files | 🔺 +8.1 KiB (+0.1%) | █████████░ 87.7% of 9.25 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/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers 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/RootErrorBoundary.tsx |
| 912 B | ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js |
| 789 B | src/scenes/ChunkLoadErrorBoundary.tsx |
| 668 B | src/index.tsx |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 278.6 KiB | ../node_modules/.pnpm/posthog-js@1.399.5/node_modules/posthog-js/dist/rrweb.js |
| 267.7 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 |
| 234.9 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 222.1 KiB | ../node_modules/.pnpm/posthog-js@1.399.5/node_modules/posthog-js/dist/module.js |
| 164.0 KiB | src/queries/validators.js |
| 154.3 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 |
| 105.9 KiB | src/lib/api.ts |
| 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 |
Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479
⚠️ Dist folder size — 🔺 +2.61 MiB (+0.2%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1281.30 MiB · 🔺 +2.61 MiB (+0.2%)
Generated-By: PostHog Code Task-Id: 80c94909-92ae-4daf-97e5-7071f48210c7
Generated-By: PostHog Code Task-Id: 80c94909-92ae-4daf-97e5-7071f48210c7
🦔 Hogbox preview · ✅ ready▶ Open the preview
commit |
|
Reviews (1): Last reviewed commit: "Merge branch 'master' into posthog-code/..." | Re-trigger Greptile |
setFilters (URL restore, back/forward, saved views) routes through the data logic's runQuery, which fires a windowed fetchAggregation while a comparison is active. Mirror handleFilterChange so the Operations tab keeps its full-range aggregate instead of dividing by the narrow compare sub-window. Generated-By: PostHog Code Task-Id: fb2b1bfa-ddad-4363-a406-c3641754d68c
Generated-By: PostHog Code Task-Id: fb2b1bfa-ddad-4363-a406-c3641754d68c
There was a problem hiding this comment.
Frontend kea-logic refactor with no risky-territory footprint (no auth, billing, migrations, API contracts, deps, or CI); the sole substantive reviewer concern (Greptile's full-range aggregate regression) was fixed and covered by a new regression test, with Greptile thumbs-upping the fix.
- Author wrote 52% of the modified lines and has 44 merged PRs in these paths (familiarity MODERATE).
- 👍 on the PR from greptile-apps[bot], hex-security-app[bot].
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 382L, 4F substantive, 535L/7F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (535L, 7F, single-area, feat) |
| stamphog 2.0.0b3 | .stamphog/policy.yml @ 100f1cf · reviewed head a533988 |
Problem
tracingSceneLogicmixes two concerns: URL sync for/tracing(a scene concern) and viewer UI state — the trace drawer, the compare-flame modal, and the "filter changed → re-run query" wiring (viewer concerns). Worse, the re-query on filter change was a side effect of the scene'sactionToUrl(buildUrl()calledrunQuery()), so an embedded viewer with no URL sync would never refresh its data.This is step two of the embeddable
TracingViewerwork, following #68678 (keyed logics, merged). It adopts the boundary the logs product already established: URL sync stays in the scene logic (logsSceneLogicowns allurlToAction/actionToUrl), viewer state lives in a keyed, URL-free logic (logsViewerLogic), and the keyed data logic owns query re-runs (mirroringhandleQueryChange).Changes
tracingViewerLogic: the trace drawer (selected trace/span/ts, open/close, the prefetch-or-fetch-by-id decision) and compare-flame modal state, moved verbatim from the scene logic. Zero router imports.tracingDataLogicnow listens to its connected filter actions and dispatcheshandleFilterChange(analytics capture +runQuery). Comparison-window drags (updateComparisonWindows) refetch the aggregation here; the compare-flame refetch (viewer state) lives in the viewer logic. NewautoLoadprop (default off) lets embeds run their first query on mount — the scene's first query still comes fromurlToAction.tracingSceneLogickeeps ALL URL sync (including the new comparison URL params from feat(tracing): rework compare mode around explicit baseline presets #69677), breadcrumbs, and the scene tabs. ItsbuildUrlis now a pure URL write (syncUrlAndRunQueryrenamed tosyncUrl), and it listens to the keyed logics' actions (handleFilterChange,setFilters) to write the URL. The deterministic ordering the Operations full-range fetch relies on is preserved: the data logic mounts before the scene logic, so its windowedfetchAggregationstill fires before the scene's full-range one.connect, soTracingScene.tsxchanges are limited to attaching/binding the new logic./tracingbehavior is unchanged: same URL params, same deep links, same query cadence.How did you test this code?
Automated tests, all green locally:
tracingDataLogic.test.ts: a keyed instance re-runs its query on a filter change with no URL involvement (the embed-critical wiring this PR exists to create — before it, re-query only happened via the scene's URL write), andautoLoadruns the first query on mount (an embed's only entry point; dropping it leaves embeds permanently empty).tracingViewerLogic.test.ts: the moved (previously untested) drawer prefetch decision — cold link fetches by id, a partial prefetch batch doesn't, a full batch refetches (truncation).expectLogic.Not manually tested in a browser.
Automatic notifications
Docs update
No user-facing behavior change; internal refactor.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Authored by Claude (PostHog Code session) with Jon directing. Skills invoked:
/writing-kea-logics,/writing-tests. Jon set the key constraint: follow the logs boundary, where URL sync stays in the scene logic — studyinglogsSceneLogic/logsViewerLogicsettled the design. Originally built against the pre-merge #68678 branch; after that squash-merged and the compare-mode rework (#69677) landed, the commit was cherry-picked onto master and ported to the new comparison API (setCompareMode/setOverlayWindows→setComparison/updateComparisonWindows,fetchAggregation({ fullRange })), preserving #69677's windowed-vs-full-range fetch ordering. Notable find while porting: the query re-run hidden insidebuildUrl()— the reason embeds could never refresh. Three oxlintexpect-expectwarnings on the new tests are false positives (assertions insidetryblocks), consistent with the pattern merged in #68678.Created with PostHog Code