Repository navigation
[codex] Make Electric-backed UI cache-first - #4785
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR systematically refactors loading guards across hooks and settings pages to use LiveQuery ChangesLive Query Readiness and Loading State Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
|
Ready to review this PR? Stage has broken it down into 5 individual chapters for you: 
Chapters generated by Stage for commit 6154036 on May 20, 2026 9:05pm UTC. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/automations/$automationId/page.tsx (1)
84-86:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMake the “Automation not found.” text selectable.
The error state at Line 84 renders plain text but misses the required
select-text cursor-textclasses.Suggested fix
- <div className="flex h-full w-full items-center justify-center text-sm text-muted-foreground"> + <div className="flex h-full w-full items-center justify-center text-sm text-muted-foreground select-text cursor-text"> Automation not found. </div>As per coding guidelines
apps/desktop/**/*.{tsx,jsx}: Error text must be selectable by users with explicit select-text cursor-text classes; renderer sets user-select: none on body.🤖 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 `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/automations/`$automationId/page.tsx around lines 84 - 86, The error message div rendering "Automation not found." is not selectable because it lacks the required select-text and cursor-text classes; update the JSX element whose className currently is "flex h-full w-full items-center justify-center text-sm text-muted-foreground" (the div that displays the error) to include "select-text cursor-text" so users can copy the message.apps/desktop/src/renderer/routes/_authenticated/settings/hosts/$hostId/components/HostSettings/HostSettings.tsx (1)
123-129:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMake the not-found message selectable.
Line 126 renders an error/not-found message without
select-text cursor-text, so users can’t reliably select/copy it in the renderer.Suggested fix
- return ( - <div className="p-6 text-sm text-muted-foreground"> + return ( + <div className="p-6 text-sm text-muted-foreground select-text cursor-text"> Host not found in this organization. </div> );As per coding guidelines, "Error text must be selectable by users with explicit
select-text cursor-textclasses; renderer setsuser-select: noneonbody".🤖 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 `@apps/desktop/src/renderer/routes/_authenticated/settings/hosts/`$hostId/components/HostSettings/HostSettings.tsx around lines 123 - 129, The "Host not found in this organization." message is rendered when host is falsy (check in HostSettings component where it tests if (!host) { if (!hostReady) return null; return (<div...>Host not found...</div>); }) but lacks selectable styling; update that div to include the explicit classes "select-text cursor-text" so the message becomes selectable and copyable in the renderer while preserving existing padding and text classes.
🤖 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.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/automations/page.tsx`:
- Around line 218-219: The Mine empty-state is shown prematurely because
showMineEmptyState uses visible.length === 0 && scope === "mine" without
checking automationsReady; update the logic (wherever showMineEmptyState is
defined and the analogous checks around lines 336-340) to additionally require
automationsReady (or equivalently !showAutomationLoading) so the Mine
empty-state only renders when scope === "mine", visible.length === 0, and
automationsReady is true; adjust the variable showMineEmptyState and any
duplicated empty-state guards accordingly.
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/organization/components/OrganizationSettings/OrganizationSettings.tsx`:
- Around line 242-248: The "Organization not found." message in
OrganizationSettings.tsx is not selectable due to global renderer styles; update
the JSX returned in the !organization branch by adding the CSS classes
"select-text cursor-text" to the <p> element that renders "Organization not
found." (locate the conditional branch in the OrganizationSettings component
where it returns the div/p when !organization and add those classes to the <p>).
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/v2-project/`$projectId/components/V2ProjectSettings/V2ProjectSettings.tsx:
- Around line 116-119: The error-state div inside the V2ProjectSettings
component rendering "Project not found." is missing the selectable classes;
update that JSX element (the div that returns "Project not found." in
V2ProjectSettings) to include the classes select-text and cursor-text so the
error text is selectable despite the renderer's global user-select: none.
---
Outside diff comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/automations/`$automationId/page.tsx:
- Around line 84-86: The error message div rendering "Automation not found." is
not selectable because it lacks the required select-text and cursor-text
classes; update the JSX element whose className currently is "flex h-full w-full
items-center justify-center text-sm text-muted-foreground" (the div that
displays the error) to include "select-text cursor-text" so users can copy the
message.
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/hosts/`$hostId/components/HostSettings/HostSettings.tsx:
- Around line 123-129: The "Host not found in this organization." message is
rendered when host is falsy (check in HostSettings component where it tests if
(!host) { if (!hostReady) return null; return (<div...>Host not found...</div>);
}) but lacks selectable styling; update that div to include the explicit classes
"select-text cursor-text" so the message becomes selectable and copyable in the
renderer while preserving existing padding and text classes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f48a2030-0172-4c5e-8579-40f6227ccecd
📒 Files selected for processing (21)
apps/desktop/src/renderer/hooks/host-service/useWorkspaceHostUrl/useWorkspaceHostUrl.tsapps/desktop/src/renderer/hooks/useCurrentPlan.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/automations/$automationId/page.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/automations/page.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ResourceConsumption/ResourceConsumption.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ResourceConsumption/resourceConsumptionPolicy.test.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ResourceConsumption/resourceConsumptionPolicy.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/hooks/useRemoteHostStatus/useRemoteHostStatus.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/layout.tsxapps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useDefaultV2TerminalPresets/useDefaultV2TerminalPresets.tsapps/desktop/src/renderer/routes/_authenticated/settings/account/components/AccountSettings/AccountSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/api-keys/components/ApiKeysSettings/ApiKeysSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/hosts/$hostId/components/HostSettings/HostSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/hosts/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/members/components/MembersSettings/MembersSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/members/components/PendingInvitations/PendingInvitations.tsxapps/desktop/src/renderer/routes/_authenticated/settings/organization/components/OrganizationSettings/OrganizationSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/projects/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/teams/$teamId/components/TeamDetailSettings/TeamDetailSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/teams/components/TeamsSettings/TeamsSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/v2-project/$projectId/components/V2ProjectSettings/V2ProjectSettings.tsx
Greptile SummaryThis PR migrates 21 Electric/TanStack DB-backed UI surfaces from a "block on
Confidence Score: 4/5Safe to merge — the pattern change is consistent across all 21 files and the test suite validates the policy changes. One edge case in terminal preset initialization warrants a follow-up. The vast majority of changes correctly apply the cache-first pattern: skeleton/null gates now check both readiness and cache emptiness, 'not found' states are properly deferred until Electric confirms readiness, and the resolveCurrentPlan path is correctly updated. The one concern is useDefaultV2TerminalPresets, where switching from presetsReady to presetsUsable = presetsReady || v2Presets.length > 0 lets the initialization effect fire while Electric is mid-sync. If the cached preset list is a partial snapshot, createDefaultV2TerminalPresetRows fills in 'missing' defaults with fresh UUIDs, and once terminalPresetsInitialized is written as true, the full server-side rows that arrive later stack on top as duplicates with no cleanup path. apps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useDefaultV2TerminalPresets/useDefaultV2TerminalPresets.ts deserves a second look around the presetsUsable guard change.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useDefaultV2TerminalPresets/useDefaultV2TerminalPresets.ts | Switches from strict readiness gate to cache-first; risks premature initialization with partial preset snapshots creating duplicate entries. |
| apps/desktop/src/renderer/hooks/useCurrentPlan.ts | Returns isReady: true when cached subscription rows exist; resolves plan tier from cache, eliminating skeleton flash for paid users. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/layout.tsx | Guard updated to show blank only when workspace is absent from cache AND Electric isn't ready; correctly delegates to WorkspaceNotFoundState when ready. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ResourceConsumption/ResourceConsumption.tsx | Removes v2MetadataReady gate entirely; metrics now render immediately with whatever project/workspace names are in cache, falling back to empty strings. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/automations/page.tsx | showAutomationLoading now requires both !automationsReady AND empty cache; cached rows render eagerly with correct scope-based empty states. |
| apps/desktop/src/renderer/routes/_authenticated/settings/organization/components/OrganizationSettings/OrganizationSettings.tsx | Splits old combined loading/not-found early return into two distinct states; adds an explicit 'Organization not found' message when isReady is true but no org row exists. |
| apps/desktop/src/renderer/routes/_authenticated/settings/api-keys/components/ApiKeysSettings/ApiKeysSettings.tsx | Replaces isLoading with !isReady && apiKeys.length === 0 skeleton gate; also simplifies empty-state check by switching to apiKeys = apiKeysData ?? []. |
Sequence Diagram
sequenceDiagram
participant UI as UI Component
participant LQ as useLiveQuery
participant Cache as Local Cache
participant Electric as Electric Sync
Note over UI,Electric: Old behavior (isReady-first)
UI->>LQ: subscribe
LQ->>Cache: query rows
Cache-->>LQ: [] (empty, not ready)
LQ-->>UI: "isReady=false, data=[]"
UI-->>UI: render skeleton / null
Electric-->>LQ: sync complete
LQ-->>UI: "isReady=true, data=[rows]"
UI-->>UI: render content
Note over UI,Electric: New behavior (cache-first)
UI->>LQ: subscribe
LQ->>Cache: query rows
Cache-->>LQ: [cachedRows] (partial, not ready)
LQ-->>UI: "isReady=false, data=[cachedRows]"
UI-->>UI: render cached content immediately
Electric-->>LQ: sync complete
LQ-->>UI: "isReady=true, data=[fullRows]"
UI-->>UI: update with complete data
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
apps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useDefaultV2TerminalPresets/useDefaultV2TerminalPresets.ts:30-41
**Early preset initialization with partial Electric snapshot may create duplicates**
`presetsUsable` becomes `true` as soon as any preset rows appear in the local cache, even while Electric is mid-sync. If `createDefaultV2TerminalPresetRows` sees an incomplete `existingPresets` list and creates "missing" defaults (each with a fresh `crypto.randomUUID()`), it then writes `terminalPresetsInitialized = true`. When Electric finishes syncing, the server's copy of those same defaults also lands in the collection — both sets now coexist under different IDs. Because `terminalPresetsInitialized` is already `true`, the guard on line 39 prevents any cleanup run, leaving the user with duplicate default terminal presets that persist across sessions.
Reviews (1): Last reviewed commit: "Make Electric-backed UI cache-first" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const presetsUsable = presetsReady || v2Presets.length > 0; | ||
| const preferencesUsable = preferencesReady || preferenceRows.length > 0; | ||
|
|
||
| useEffect(() => { | ||
| if ( | ||
| !hostUrl || | ||
| !agentsFetched || | ||
| !presetsReady || | ||
| !preferencesReady || | ||
| !presetsUsable || | ||
| !preferencesUsable || | ||
| preferences.terminalPresetsInitialized | ||
| ) { | ||
| return; |
There was a problem hiding this comment.
presetsUsable becomes true as soon as any preset rows appear in the local cache, even while Electric is mid-sync. If createDefaultV2TerminalPresetRows sees an incomplete existingPresets list and creates "missing" defaults (each with a fresh crypto.randomUUID()), it then writes terminalPresetsInitialized = true. When Electric finishes syncing, the server's copy of those same defaults also lands in the collection — both sets now coexist under different IDs. Because terminalPresetsInitialized is already true, the guard on line 39 prevents any cleanup run, leaving the user with duplicate default terminal presets that persist across sessions.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useDefaultV2TerminalPresets/useDefaultV2TerminalPresets.ts
Line: 30-41
Comment:
**Early preset initialization with partial Electric snapshot may create duplicates**
`presetsUsable` becomes `true` as soon as any preset rows appear in the local cache, even while Electric is mid-sync. If `createDefaultV2TerminalPresetRows` sees an incomplete `existingPresets` list and creates "missing" defaults (each with a fresh `crypto.randomUUID()`), it then writes `terminalPresetsInitialized = true`. When Electric finishes syncing, the server's copy of those same defaults also lands in the collection — both sets now coexist under different IDs. Because `terminalPresetsInitialized` is already `true`, the guard on line 39 prevents any cleanup run, leaving the user with duplicate default terminal presets that persist across sessions.
How can I resolve this? If you propose a fix, please make it concise.![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Summary
isReadyonly to decide loading vs empty/not-found when no row exists yet.Why
During an Electric outage or slow shape hydration,
useLiveQuerycan still expose persisted rows whileisReadyis false. UI code that blanked or skeletoned on readiness alone could hide valid cached data, which matched the blank workspace/settings reports during the 1.9.9 upgrade window.Validation
bun test apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ResourceConsumption/resourceConsumptionPolicy.test.tsbun run --cwd apps/desktop typecheckbun run lintgit diff --checkSummary by cubic
Make the Electric-backed UI cache-first so cached rows render immediately instead of blank states when
isReadyis false, and remove metadata gating from the Resource Monitor. Document the cache-firstuseLiveQuerypolicy inAGENTS.md.useCurrentPlan: treat subscriptions as loaded when cached and returnisReadyaccordingly.useWorkspaceHostUrl,useRemoteHostStatus, and v2 workspace layout now gate on “not ready AND no row,” preventing blank UIs.Written for commit 6154036. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
Release Notes
Bug Fixes
Documentation