feat(toolbar): load the full clickmap automatically after first paint - #68063
Conversation
Ninety days of data on a busy page needs far more than one 5,000-row page, and clicking load more repeatedly re-runs the full server-side aggregation per click since LIMIT/OFFSET over a live GROUP BY cannot reuse work — while also missing rows that shift across page boundaries between scans. Keep the 5,000-row first page for a fast first paint, then fetch up to 50,000 rows in one background request from offset zero, replacing the first page rather than merging with it: one extra scan, no boundary gaps, and load more only appears past that budget. A per-row match cache keyed on the server's chain hash means the refetch reuses the first page's matching work instead of redoing it, and clears whenever the page-elements cache invalidates or href matching is toggled. The run's trigger and cache reuse are visible in the clickmap telemetry. Generated-By: PostHog Code Task-Id: 2d3886bf-73f2-40a4-8644-f3a1eb44ff27
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
Reviews (1): Last reviewed commit: "feat(toolbar): load the full clickmap au..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7fc1c587ba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
🤖 CI report
|
| File | Size | Δ vs base |
|---|---|---|
toolbar/src/toolbar/toolbar.js |
12.98 MiB | 🔺 +1.5 KiB (+0.0%) |
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 |
3.76 MiB · 695 files | 🔺 +68 B (+0.0%) | █████████░ 87.7% of 4.29 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
11.52 MiB · 2,937 files | 🔺 +458 B (+0.0%) | █████████░ 89.5% of 12.87 MiB |
🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers 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
Largest files eagerly shipped from src/index.tsx
| Size | File |
|---|---|
| 275.2 KiB | ../node_modules/.pnpm/posthog-js@1.396.5/node_modules/posthog-js/dist/rrweb.js |
| 266.9 KiB | ../node_modules/.pnpm/@posthog+icons@0.37.4_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 224.5 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 211.3 KiB | ../node_modules/.pnpm/posthog-js@1.396.5/node_modules/posthog-js/dist/module.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 |
| 69.7 KiB | src/products.tsx |
| 61.7 KiB | src/lib/lemon-ui/icons/icons.tsx |
| 57.2 KiB | src/lib/utils/eventUsageLogic.ts |
| 39.6 KiB | src/lib/KeaDevTools.tsx |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 458.3 KiB | ../node_modules/.pnpm/@posthog+brand@0.3.0_react@18.3.1/node_modules/@posthog/brand/dist/generated/hoggies/svg/driving-hogzilla.mjs |
| 302.9 KiB | ../node_modules/.pnpm/@posthog+brand@0.3.0_react@18.3.1/node_modules/@posthog/brand/dist/generated/hoggies/svg/coffee-run.mjs |
| 275.2 KiB | ../node_modules/.pnpm/posthog-js@1.396.5/node_modules/posthog-js/dist/rrweb.js |
| 266.9 KiB | ../node_modules/.pnpm/@posthog+icons@0.37.4_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 253.2 KiB | ../node_modules/.pnpm/@posthog+brand@0.3.0_react@18.3.1/node_modules/@posthog/brand/dist/generated/hoggies/svg/chart-hog.mjs |
| 239.2 KiB | ../node_modules/.pnpm/@posthog+brand@0.3.0_react@18.3.1/node_modules/@posthog/brand/dist/generated/hoggies/svg/x-ray.mjs |
| 224.5 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 211.6 KiB | ../node_modules/.pnpm/@posthog+brand@0.3.0_react@18.3.1/node_modules/@posthog/brand/dist/generated/hoggies/svg/money.mjs |
| 211.3 KiB | ../node_modules/.pnpm/posthog-js@1.396.5/node_modules/posthog-js/dist/module.js |
| 185.9 KiB | ../node_modules/.pnpm/@posthog+brand@0.3.0_react@18.3.1/node_modules/@posthog/brand/dist/generated/hoggies/svg/hogpatch.mjs |
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 — 🟢 -365.98 MiB (-20.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1464.56 MiB · 🟢 -365.98 MiB (-20.0%)
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Two independent bot reviewers (Greptile and Codex) both flagged plausible race conditions on the current head — one where the auto-load fetch's success can supersede/abort the still-processing initial processElements run via the shared breakpoint, and one where trigger classification could read a stale lastElementStatsRequest — and neither concern has been addressed or resolved in the diff. Combined with the author not being on the owning team for this behavioral logic change, this needs human (or team) review before merge.
There was a problem hiding this comment.
This is a behavioral change to toolbar clickmap loading logic owned by team-web-analytics, authored by someone outside that team, and two independent automated reviewers (Greptile, Codex) both flagged unresolved race-condition concerns in the new async auto-load/processElements interaction that the author hasn't addressed or replied to.
pauldambra
left a comment
pauldambra
left a comment
There was a problem hiding this comment.
Note
🤖 Automated comment by QA Swarm — not written by a human
QA Swarm review complete. See inline comments.
One general finding with no line anchor — [qa-team/generalist-a] 🟡 MEDIUM: the auto-load dispatch, trigger derivation, loop guard, and match-cache invalidation ship with zero test changes in this PR; the mounted-logic tests that cover this behaviour exist only in the stacked child PR #68079. Tests belong with the behaviour they cover.
|
Note 🤖 Automated comment by QA Swarm — not written by a human Multi-perspective review: qa-team (specialists + generalists), paul-reviewer, xp-reviewer, security-audit Verdict: 💬 APPROVE WITH NITS (round 1 @ 7fc1c58)The design is sound and deliberately reasoned (fetch-bigger-from-zero with the rationale documented, structural loop guard, order-independent match-cache identity, thorough invalidation). The substantive findings — a failed background auto-load silently disabling the heatmap, missing Key findings
Reviewer summaries
Automated by QA Swarm — not a human review |
Co-authored-by: paul <paul@pauls-MacBook-Air.local>
|
🎭 Playwright report · View test results →
These issues are not necessarily caused by your changes. |
2 participants
Problem
The clickmap now loads a single 5,000-row page (#67549/#67755) — fast, but for a 90-day range on a busy page that's nowhere near the full picture, and recovering it means clicking "Load more" repeatedly. Each click is also expensive server-side: the element stats query is LIMIT/OFFSET over a live GROUP BY, so ClickHouse re-runs the full aggregation for every page (measured 3-7s per scan on heavy teams) — and rows that shift position between scans can fall through page boundaries entirely (offset pagination misses them; the hash dedupe only catches duplicates).
Changes
initial-triggered result starts the refetch, so an auto-load result can never re-trigger itself.type:chain_hashidentity: the auto-load refetch reuses the first page's 5,000 match results instead of redoing them, and definitive misses are cached too. Cleared whenever the page-elements cache invalidates (DOM mutation, href change, refresh, toggle-off, unmount) and when "Match links by their target URL" is toggled, since that changes matching semantics.triggergainsauto-load, and a newmatch_cache_hit_countdistinguishes cached resolutions from fresh index/fallback matches (documented in TELEMETRY.md).No backend changes — the
limitparameter and offset-0 replace semantics already existed.How did you test this code?
toolbar clickmap processedtelemetry (trigger=auto-load,match_cache_hit_count), which is how the rollout will be verified, mirroring how the serialization win in feat(toolbar): serialize element stats without models and trim attributes #67755 was verified from production spans.Automatic notifications
Docs update
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
chain_hashintroduced in feat(toolbar): serialize element stats without models and trim attributes #67755 and skipped for legacy null-hash rows.Created with PostHog Code