fix: save selected view in configuration - #502
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
skill-check — worker0 verified, 42 skipped (no docs/).
Four for four. Nicely done. |
📝 WalkthroughWalkthroughTraces V2 active-view and follow-turn preferences now use server-side console configuration instead of localStorage. Tab-local choices remain responsive, view deletion updates its pointer atomically, and the Vite WebSocket proxy accepts a configurable Engine URL. ChangesTraces preferences persistence
Development Engine WebSocket proxy
Estimated code review effort: 3 (Moderate) | ~30 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
console/web/src/pages/TracesV2/hooks/useFollowTurns.ts (2)
70-88: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDestructure
mutateAsyncto preserve reference stability.The
mutationobject returned byuseMutationchanges reference on every status update (e.g., idle → pending → success). Passing it in the dependency array causestoggleFollowTurnsto be recreated repeatedly, which can trigger unnecessary re-renders of downstream child components likeTimelineStrip.Destructure
mutateAsync(which has a guaranteed stable reference) to prevent this.♻️ Proposed refactor
- const mutation = useMutation({ + const { mutateAsync } = useMutation({ mutationFn: async (on: boolean) => { const current = (await fetchConsoleConfigValue()) ?? {} const next = withFollowTurns(current, on) await setConsoleConfigValue(next) return next }, onSuccess: (next) => { qc.setQueryData(CONSOLE_CONFIG_QUERY_KEY, next) }, }) const toggleFollowTurns = useCallback(() => { const next = !followTurns setChosen(next) // Best-effort server persist; the in-memory choice stays live even when // the configuration worker is unreachable. - mutation.mutateAsync(next).catch(() => {}) - }, [followTurns, mutation]) + mutateAsync(next).catch(() => {}) + }, [followTurns, mutateAsync])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@console/web/src/pages/TracesV2/hooks/useFollowTurns.ts` around lines 70 - 88, Destructure mutateAsync from the useMutation result and update toggleFollowTurns to call it directly. Replace the mutation object in the callback dependency array with the stable mutateAsync reference, preserving the existing best-effort error handling and toggle behavior.
21-23: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCentralize the shared query key.
Consider exporting this query key from
@/lib/console-configinstead of redefining it locally in each hook. Centralizing the key prevents subtle cache-miss bugs caused by typos across different consumers (likeuseTraceViewsanduseSpanFilterSelection).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@console/web/src/pages/TracesV2/hooks/useFollowTurns.ts` around lines 21 - 23, Centralize CONSOLE_CONFIG_QUERY_KEY in the `@/lib/console-config` module by exporting it there, then update useFollowTurns and the related consumers such as useTraceViews and useSpanFilterSelection to import and reuse that shared symbol instead of defining local keys.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@console/web/src/pages/TracesV2/hooks/useFollowTurns.ts`:
- Around line 70-88: Destructure mutateAsync from the useMutation result and
update toggleFollowTurns to call it directly. Replace the mutation object in the
callback dependency array with the stable mutateAsync reference, preserving the
existing best-effort error handling and toggle behavior.
- Around line 21-23: Centralize CONSOLE_CONFIG_QUERY_KEY in the
`@/lib/console-config` module by exporting it there, then update useFollowTurns
and the related consumers such as useTraceViews and useSpanFilterSelection to
import and reuse that shared symbol instead of defining local keys.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c41f93c5-01cb-4788-96e8-86d51410b3b7
📒 Files selected for processing (4)
console/src/configuration.rsconsole/web/src/lib/storage.tsconsole/web/src/pages/TracesV2/hooks/useFollowTurns.tsconsole/web/src/pages/TracesV2/index.tsx
💤 Files with no reviewable changes (1)
- console/web/src/lib/storage.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- console/src/configuration.rs
Problem
The Traces tab's saved views live server-side in the
consoleconfiguration entry, but the active view selection was stored in per-browser localStorage. The selection didn't follow the engine — every new browser (or cleared storage) lost the choice, and the seeded "sessions" view was only selected via a frontend fallback.Solution
Persist the pointer next to the views as
traces.activeViewIdin theconsoleconfiguration entry:activeViewId: "view-sessions"and documents the field in the entry schema; when the pointer is absent (configs seeded before this change) the UI still defaults to the sessions view.useTraceViewsreads/writes the pointer through the existing read-modify-write mutation funnel; the in-tab selection stays live even when the configuration worker is unreachable.Also updates
vite.config.tsto match the binary's server posture: binds0.0.0.0so the dev server accepts external connections, and the/wsproxy engine target is configurable viaIII_ENGINE_URL(same default as the binary's--url).🤖 Generated with Claude Code
Summary by CodeRabbit