improvement(nav): cut prefetch and session-recorder waste - #6656
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Workspace list: Replaces PostHog: Adds explicit Reviewed by Cursor Bugbot for commit 0936ab6. Configure here. |
- Seed the workspace list instead of prefetching it. The empty-list case was signalled by throwing inside queryFn, which the retry: 1 default re-ran the entire read to re-derive, a retry delay later. Log the failure path, which was silent — contract drift would have degraded into every viewer waterfalling with nothing in the logs. - Drop non-painted nodes from rrweb snapshots via slimDOMOptions. Enumerated rather than true/'all' so headTitleMutations stays off and replays keep document.title. Prefetch concurrency and await semantics are unchanged, so sidebar paint timing matches staging.
2eb5b38 to
0936ab6
Compare
waleedlatif1
changed the title
|
@cursor review |
Greptile SummaryThe PR replaces the retrying workspace-list prefetch with an awaited cache seed that leaves empty and failed reads for the client API fallback, and reduces PostHog replay snapshots by omitting non-painted DOM content.
Confidence Score: 5/5The PR appears safe to merge, with no concrete blocking or independently actionable non-blocking issue identified. The workspace seed uses the existing normalized response shape and query key, is awaited before dehydration, and intentionally leaves empty or failed reads uncached so the client route can perform required side effects; the recorder change only removes selected non-painted nodes.
|
| Filename | Overview |
|---|---|
| apps/sim/app/workspace/[workspaceId]/prefetch.ts | Replaces workspace-list prefetch retries with an awaited direct cache seed while preserving empty/error fallback to the client route. |
| apps/sim/app/_shell/providers/posthog-provider.tsx | Adds explicit session-recording DOM slimming options while retaining document-title mutations. |
Reviews (2): Last reviewed commit: "improvement(nav): cut prefetch and sessi..." | Re-trigger Greptile
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 0936ab6. Configure here.
… could not fail (#6659) The audit found the seed contract — the whole point of #6656 — had no test, and that one existing assertion was vacuous. - prefetchWorkspaceSidebar and seedWorkspaceList now have coverage: the empty list seeds nothing (so the client reaches the route's default-workspace creation path), a populated list seeds, a rejected read neither throws nor seeds, and a host context for another workspace seeds nothing at all. Verified falsifiable — removing the empty-list guard turns the first red. - The graceful-failure row for prefetchFilesBrowser asserted on the file-list key, which that function deliberately never writes, so it held no matter what the code did. It now asserts the folder key it owns, and the setup rejects the folder read. - The sidebar was the third hand-rolled copy of the folder prefetch the shared helper was extracted to remove; it now calls prefetchResourceFolders too. - prefetchKnowledgeBases returns early without a userId like every sibling, rather than reaching the authenticator and throwing per render. - Dropped two docblock claims about a `retry` default that no longer applies server-side.
1 participant
Summary
queryFn; theretry: 1default then re-ran the whole read, a retry delay later, purely to re-derive an outcome already known. Also logs the failure path, which was silent — contract drift between the read and the response schema would have degraded into every viewer waterfalling with nothing in the logs.slimDOMOptionsto session recording so non-painted nodes (comments, script bodies, head meta) stay out of every rrweb snapshot. Enumerated rather thantrue/'all'soheadTitleMutationsstays off and replays keepdocument.title.Prefetch concurrency and await semantics are unchanged, so sidebar paint timing matches
staging.Context
Came out of investigating a report that opening a workflow from the sidebar took a long time. These are the two contained wins that fell out of it; neither is the fix for that report.
What this does not fix
The dominant cost on the workflow canvas is
workflow-block-border.tsxrewriting a spring-animated SVG path per block per frame, which trips PostHog's mutation throttler. Noposthog-jsoption can throttle that —samplinghas nomutationkey and isn't on posthog-js's rrweb allowlist anyway, and the throttler that does exist runs after serialization, so it only trades payload for fidelity. Fixing it at the source means driving the silhouette imperatively from the rAF loop instead of React state — a deliberate rewrite, out of scope here.Sidebar navigation also still gives no feedback on click, which is what leads users to click again and escalate a soft navigation into a full document load. A pending affordance was prototyped and pulled — it didn't fit the sidebar's visual language. Worth solving with an affordance the design system already uses.
We collect no navigation timing and no INP, so none of this is measurable today. That instrumentation is the real follow-up.
Type of Change
Testing
type-check,biome, and repolint:check(23/23) pass, along with the workspace layout test. No UI changes in this PR.Checklist