Switch from PGLite to TanstackDb for Desktop App Query Layer#585
Conversation
…c SQL sync - Switch from neon-http to neon-serverless (dbWs) for transaction support - Fix txid extraction from execute() result using result.rows[0] - Convert txid from string to number for Electric SQL matching - Add nullable support for repositoryId in task update schema - Create HTTP tRPC client for collections (not IPC-based) - Update query hooks to handle new response format with task wrapper This fixes the flickering issue where optimistic updates weren't matching server responses. Electric SQL needs numeric txid to correlate updates. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…tchClient Fixed 401 Unauthorized errors on Electric SQL sync by using the built-in `headers` parameter in ShapeStreamOptions instead of a custom `fetchClient` wrapper. This ensures auth headers are properly passed to Electric requests. Changes: - Removed custom fetchClient wrappers from createOrgCollections, createUserCollections, and createOrgSettingsCollection - Use headers parameter directly in shapeOptions for all collections - Added nullToUndefined helper to convert null values to undefined for tRPC schema compatibility (Electric returns null, tRPC expects undefined) - Fixed duplicate id issue in onUpdate handlers 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
REAL ROOT CAUSE: Auth service wasn't emitting 'tokens-refreshed' event after
token refresh, so the onAccessToken subscription never knew to recreate collections
with fresh tokens.
The solution is simple:
1. Emit 'tokens-refreshed' event in auth service after successful token refresh
2. The existing subscription in TanStackDbProviderWithAuth already listens for this
3. Collections already recreate when accessToken changes (it's in dependency array)
4. No custom fetchClient needed - just use static headers parameter
We were overcomplicating it with custom fetch clients and dynamic token getters.
The architecture already supported token rotation - we just forgot to emit the event!
Changes:
- Added this.emit("tokens-refreshed") after successful token refresh
- Removed all custom fetchClient code from collections
- Removed getAccessToken prop passing
- Back to simple headers parameter in ShapeStreamOptions
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Flatten CollectionsProvider context value (remove nested abstraction) - Unify org/device collections creation into single function - Fix schema type mismatches with .nullish() instead of nullToUndefined - Move org/collections providers to TasksView scope only - Remove unnecessary try/catch, console.logs, and isInitializing - Inline single-use HTTP tRPC client per repo guidelines - Auto-fix import order and formatting 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughRemoves the PGlite/IndexedDB subsystem and Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Renderer as Desktop Renderer
participant AuthProv as AuthProvider
participant TRPC as TRPC Client
participant ActiveOrg as ActiveOrganizationProvider
participant Collections as CollectionsProvider
participant TasksView as TasksView Component
Renderer->>AuthProv: mount AuthProvider
AuthProv->>TRPC: subscribe onAccessToken
TRPC-->>AuthProv: emit accessToken / error
alt accessToken undefined
AuthProv->>Renderer: render null (loading)
else accessToken null
AuthProv->>Renderer: dynamically import & render SignInScreen
else accessToken present
AuthProv->>ActiveOrg: render ActiveOrganizationProvider
ActiveOrg->>TRPC: fetch organizations
TRPC-->>ActiveOrg: return org list
ActiveOrg->>Collections: createCollections(activeOrgId, headers)
Collections->>TasksView: provide collections (tasks, users, ...)
TasksView->>Collections: useLiveQuery(tasks collection)
Collections-->>TasksView: stream live tasks
TasksView->>TasksView: filter deleted tasks, render UI
end
sequenceDiagram
autonumber
participant Client as API Client
participant TRPCRouter as TRPC task router
participant DBws as dbWs (database)
participant Utils as DB utils (getCurrentTxid)
participant Sync as syncTask
Client->>TRPCRouter: call task.create(payload)
TRPCRouter->>DBws: dbWs.transaction(tx => { insert task })
DBws->>Utils: getCurrentTxid(tx)
Utils-->>TRPCRouter: txid
TRPCRouter->>Sync: optionally call syncTask(createdTask)
TRPCRouter-->>Client: return { task, txid }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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 |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (6)
apps/desktop/src/lib/trpc/routers/auth/index.ts (1)
70-70: Consider removing verbose lifecycle logs.Subscription start/end logs are useful for debugging but add noise to production logs. Consider feature-flagging or removing them.
Also applies to: 77-77
apps/desktop/src/main/lib/auth/auth-service.ts (1)
239-239: Extract event name to a constant and document the new event.The event name
"tokens-refreshed"is a magic string. Consider extracting event names to constants at the module level for consistency and type safety. Additionally, add documentation (JSDoc comment) for this new event to describe when it's emitted and what listeners should expect.🔎 Proposed refactor
+/** + * Event names emitted by AuthService + */ +const AuthEvents = { + STATE_CHANGED: 'state-changed', + TOKENS_REFRESHED: 'tokens-refreshed', +} as const; + /** * Main authentication service * Handles direct Google OAuth flow, with token exchange via API + * + * Events: + * - 'state-changed': Emitted when auth state changes (sign in/out) + * - 'tokens-refreshed': Emitted after successful token refresh */ class AuthService extends EventEmitter {Then update the emit calls:
- this.emit("tokens-refreshed"); + this.emit(AuthEvents.TOKENS_REFRESHED);- this.emit("state-changed", this.getState()); + this.emit(AuthEvents.STATE_CHANGED, this.getState());apps/desktop/src/renderer/collections/index.ts (1)
39-42: Consider stricter typing forDeviceSetting.value.The
valuefield is typed asunknown, which requires type assertions at every usage site. If device settings have a known schema, consider using a discriminated union or generic type parameter for better type safety.Example with discriminated union:
export type DeviceSetting = | { key: "theme"; value: "light" | "dark" } | { key: "language"; value: string } | { key: "notifications"; value: boolean };Or with a generic:
export interface DeviceSetting<T = unknown> { key: string; value: T; }apps/desktop/src/renderer/contexts/OrganizationProvider.tsx (1)
42-47: Prefer structured logging for easier filtering.The console.log uses separate arguments which can be harder to grep/filter in logs. Using a structured object makes it easier to parse and filter logs programmatically.
console.log( - "[OrganizationProvider] Switching from", - activeOrganizationId, - "to:", - newOrgId, + '[OrganizationProvider/switchOrganization] Organization switch', + { from: activeOrganizationId, to: newOrgId } );apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx (2)
59-84: Task update flow looks solid with proper error handling.The try/catch with prefixed console logging (
[TaskEditDialog]) and user-facing toast notifications follows best practices.Minor: The type assertion on lines 67-72 is redundant since
priorityalready has the correct type from the state initialization andsetPrioritycast.🔎 Optional simplification
await tasksCollection.update(task.id, (draft) => { draft.title = title; draft.description = description || null; - draft.priority = priority as - | "urgent" - | "high" - | "medium" - | "low" - | "none"; + draft.priority = priority; });
43-366: Consider splitting into separate files per coding guidelines.This file contains 6 components (TaskEditDialog, TaskCard, TasksList, Sidebar, TasksViewContent, TasksView). Per coding guidelines,
.tsxfiles should have one component per file.While these are tightly coupled internal components, splitting them would improve maintainability and align with project conventions. This could be structured as:
TasksView/ ├── index.tsx (re-exports TasksView) ├── TasksView.tsx ├── TasksList.tsx ├── TaskCard.tsx ├── TaskEditDialog.tsx ├── Sidebar.tsx └── TasksViewContent.tsx
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (44)
TANSTACK_DB_MIGRATION.mdapps/admin/package.jsonapps/api/src/app/api/electric/[...path]/route.tsapps/api/src/proxy.tsapps/desktop/package.jsonapps/desktop/src/lib/trpc/routers/auth/index.tsapps/desktop/src/main/lib/auth/auth-service.tsapps/desktop/src/renderer/collections/index.tsapps/desktop/src/renderer/contexts/AppProviders.tsxapps/desktop/src/renderer/contexts/AuthProvider.tsxapps/desktop/src/renderer/contexts/CollectionsProvider.tsxapps/desktop/src/renderer/contexts/OrganizationProvider.tsxapps/desktop/src/renderer/contexts/OrganizationsProvider.tsxapps/desktop/src/renderer/contexts/index.tsapps/desktop/src/renderer/index.htmlapps/desktop/src/renderer/lib/pglite/PGliteProvider/PGliteProvider.tsxapps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/index.tsapps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/useOrganizationDatabase.tsapps/desktop/src/renderer/lib/pglite/PGliteProvider/index.tsapps/desktop/src/renderer/lib/pglite/database.tsapps/desktop/src/renderer/lib/pglite/hooks.tsapps/desktop/src/renderer/lib/pglite/index.tsapps/desktop/src/renderer/lib/pglite/migrate.tsapps/desktop/src/renderer/lib/pglite/migrations/0000_init.sqlapps/desktop/src/renderer/lib/pglite/migrations/index.tsapps/desktop/src/renderer/lib/pglite/migrations/meta/0000_snapshot.jsonapps/desktop/src/renderer/lib/pglite/migrations/meta/_journal.jsonapps/desktop/src/renderer/lib/pglite/schema/index.tsapps/desktop/src/renderer/lib/pglite/schema/schema.tsapps/desktop/src/renderer/lib/pglite/sync.tsapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsxapps/web/package.jsonbiome.jsoncpackages/queries/package.jsonpackages/queries/src/index.tspackages/queries/src/task/index.tspackages/queries/src/task/useTaskCreate.tspackages/queries/src/task/useTaskDelete.tspackages/queries/src/task/useTaskUpdate.tspackages/queries/tsconfig.jsonpackages/trpc/src/router/repository/repository.tspackages/trpc/src/router/task/schema.tspackages/trpc/src/router/task/task.ts
💤 Files with no reviewable changes (24)
- packages/queries/src/task/index.ts
- packages/queries/src/task/useTaskCreate.ts
- apps/desktop/src/renderer/lib/pglite/PGliteProvider/PGliteProvider.tsx
- apps/desktop/src/renderer/lib/pglite/PGliteProvider/index.ts
- apps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/useOrganizationDatabase.ts
- apps/desktop/src/renderer/lib/pglite/hooks.ts
- apps/desktop/src/renderer/lib/pglite/database.ts
- packages/queries/src/index.ts
- apps/desktop/src/renderer/lib/pglite/migrations/index.ts
- apps/desktop/src/renderer/lib/pglite/migrations/meta/0000_snapshot.json
- packages/queries/src/task/useTaskDelete.ts
- apps/desktop/src/renderer/lib/pglite/PGliteProvider/hooks/index.ts
- packages/queries/tsconfig.json
- apps/desktop/src/renderer/lib/pglite/migrations/meta/_journal.json
- apps/web/package.json
- apps/desktop/src/renderer/lib/pglite/migrate.ts
- packages/queries/package.json
- apps/desktop/src/renderer/lib/pglite/schema/index.ts
- apps/desktop/src/renderer/lib/pglite/schema/schema.ts
- apps/desktop/src/renderer/lib/pglite/migrations/0000_init.sql
- apps/admin/package.json
- apps/desktop/src/renderer/lib/pglite/index.ts
- apps/desktop/src/renderer/lib/pglite/sync.ts
- packages/queries/src/task/useTaskUpdate.ts
🧰 Additional context used
📓 Path-based instructions (8)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/renderer/contexts/OrganizationProvider.tsxapps/desktop/src/renderer/contexts/AuthProvider.tsxapps/desktop/src/renderer/contexts/CollectionsProvider.tsxapps/desktop/src/main/lib/auth/auth-service.tsapps/desktop/src/renderer/contexts/AppProviders.tsxapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/contexts/OrganizationsProvider.tsxapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsxapps/desktop/src/renderer/contexts/index.tsapps/desktop/src/lib/trpc/routers/auth/index.tsapps/desktop/src/renderer/collections/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use object parameters for functions with 2+ parameters instead of positional arguments
Functions with 2+ parameters should accept a single params object with named properties for self-documentation and extensibility
Use prefixed console logging with context pattern: [domain/operation] message
Extract magic numbers and hardcoded values to named constants at module top
Use lookup objects/maps instead of repeated if (type === ...) conditionals
Avoid usinganytype - maintain type safety in TypeScript code
Never swallow errors silently - at minimum log them with context
Import from concrete files directly when possible - avoid barrel file abuse that creates circular dependencies
Avoid deep nesting (4+ levels) - use early returns, extract functions, and invert conditions
Use named properties in options objects instead of boolean parameters to avoid boolean blindness
Files:
apps/desktop/src/renderer/contexts/OrganizationProvider.tsxapps/desktop/src/renderer/contexts/AuthProvider.tsxapps/api/src/proxy.tsapps/desktop/src/renderer/contexts/CollectionsProvider.tsxapps/desktop/src/main/lib/auth/auth-service.tspackages/trpc/src/router/task/task.tsapps/desktop/src/renderer/contexts/AppProviders.tsxapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/contexts/OrganizationsProvider.tsxapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsxapps/desktop/src/renderer/contexts/index.tsapps/desktop/src/lib/trpc/routers/auth/index.tsapps/desktop/src/renderer/collections/index.tspackages/trpc/src/router/repository/repository.tsapps/api/src/app/api/electric/[...path]/route.tspackages/trpc/src/router/task/schema.ts
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Files:
apps/desktop/src/renderer/contexts/OrganizationProvider.tsxapps/desktop/src/renderer/contexts/AuthProvider.tsxapps/desktop/src/renderer/contexts/CollectionsProvider.tsxapps/desktop/src/renderer/contexts/AppProviders.tsxapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/contexts/OrganizationsProvider.tsxapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsxapps/desktop/src/renderer/contexts/index.tsapps/desktop/src/renderer/collections/index.ts
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
One component per file - do not create multi-component files
Files:
apps/desktop/src/renderer/contexts/OrganizationProvider.tsxapps/desktop/src/renderer/contexts/AuthProvider.tsxapps/desktop/src/renderer/contexts/CollectionsProvider.tsxapps/desktop/src/renderer/contexts/AppProviders.tsxapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/contexts/OrganizationsProvider.tsxapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsx
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations - never use raw SQL
Files:
apps/desktop/src/renderer/contexts/OrganizationProvider.tsxapps/desktop/src/renderer/contexts/AuthProvider.tsxapps/api/src/proxy.tsapps/desktop/src/renderer/contexts/CollectionsProvider.tsxapps/desktop/src/main/lib/auth/auth-service.tsapps/desktop/src/renderer/contexts/AppProviders.tsxapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/contexts/OrganizationsProvider.tsxapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsxapps/desktop/src/renderer/contexts/index.tsapps/desktop/src/lib/trpc/routers/auth/index.tsapps/desktop/src/renderer/collections/index.tsapps/api/src/app/api/electric/[...path]/route.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for formatting and linting - run at root level with
bun run lint:fixorbiome check --write
Files:
apps/desktop/src/renderer/contexts/OrganizationProvider.tsxapps/desktop/src/renderer/contexts/AuthProvider.tsxapps/api/src/proxy.tsapps/desktop/src/renderer/contexts/CollectionsProvider.tsxapps/desktop/src/main/lib/auth/auth-service.tspackages/trpc/src/router/task/task.tsapps/desktop/src/renderer/contexts/AppProviders.tsxapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/contexts/OrganizationsProvider.tsxapps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsxapps/desktop/src/renderer/contexts/index.tsapps/desktop/src/lib/trpc/routers/auth/index.tsapps/desktop/src/renderer/collections/index.tspackages/trpc/src/router/repository/repository.tsapps/api/src/app/api/electric/[...path]/route.tspackages/trpc/src/router/task/schema.ts
apps/**/proxy.ts
📄 CodeRabbit inference engine (AGENTS.md)
Never use
middleware.tsin Next.js 16. Useproxy.tsinstead for request interception.
Files:
apps/api/src/proxy.ts
apps/api/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/api/**/*.{ts,tsx}: Use TRPCError with appropriate error codes: NOT_FOUND, UNAUTHORIZED, FORBIDDEN, BAD_REQUEST, INTERNAL_SERVER_ERROR, NOT_IMPLEMENTED
tRPC procedures and API route handlers should validate and delegate; keep orchestrators thin
Extract business logic from tRPC procedures into utility functions when logic exceeds ~50 lines, is used by multiple procedures, or needs independent testing
Validate at boundaries using Zod schemas for tRPC inputs and API route bodies
Files:
apps/api/src/proxy.tsapps/api/src/app/api/electric/[...path]/route.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.662Z
Learning: Applies to apps/**/*.{ts,tsx} : Use Drizzle ORM for all database operations - never use raw SQL
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.662Z
Learning: Applies to packages/db/src/**/*.ts : Schema definitions must be in packages/db/src/ using Drizzle ORM
📚 Learning: 2026-01-02T06:50:28.662Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.662Z
Learning: Applies to packages/db/src/**/*.ts : Schema definitions must be in packages/db/src/ using Drizzle ORM
Applied to files:
packages/trpc/src/router/task/task.tspackages/trpc/src/router/repository/repository.ts
📚 Learning: 2026-01-02T06:50:28.662Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.662Z
Learning: Applies to apps/**/*.{ts,tsx} : Use Drizzle ORM for all database operations - never use raw SQL
Applied to files:
packages/trpc/src/router/task/task.tspackages/trpc/src/router/repository/repository.ts
📚 Learning: 2026-01-02T06:50:28.662Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.662Z
Learning: Applies to apps/api/**/*.{ts,tsx} : tRPC procedures and API route handlers should validate and delegate; keep orchestrators thin
Applied to files:
packages/trpc/src/router/task/task.ts
📚 Learning: 2026-01-02T06:50:28.662Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.662Z
Learning: Applies to apps/desktop/src/renderer/**/*.{ts,tsx} : Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Applied to files:
apps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsxapps/desktop/src/renderer/contexts/index.ts
📚 Learning: 2026-01-02T06:50:28.662Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.662Z
Learning: Applies to apps/desktop/src/lib/*.ts : Never import Node.js modules in shared code like electron-router-dom.ts - it runs in both main and renderer processes
Applied to files:
apps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsx
📚 Learning: 2026-01-02T06:50:28.662Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.662Z
Learning: Applies to apps/*/src/components/**/*.{ts,tsx} : Co-locate component dependencies: utils, hooks, constants, config, tests, stories in the same folder as the component using them
Applied to files:
apps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsx
📚 Learning: 2026-01-02T06:50:28.662Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.662Z
Learning: Applies to apps/api/**/*.{ts,tsx} : Validate at boundaries using Zod schemas for tRPC inputs and API route bodies
Applied to files:
packages/trpc/src/router/task/schema.ts
🧬 Code graph analysis (7)
apps/desktop/src/renderer/contexts/OrganizationProvider.tsx (3)
apps/desktop/src/renderer/contexts/index.ts (4)
useOrganizations(16-16)OrganizationProvider(9-9)useOrganization(11-11)useActiveOrganization(10-10)apps/api/src/trpc/context.ts (1)
createContext(4-12)packages/db/src/schema/schema.ts (1)
organizations(53-72)
apps/desktop/src/renderer/contexts/AuthProvider.tsx (2)
apps/api/src/trpc/context.ts (1)
createContext(4-12)apps/desktop/src/renderer/screens/sign-in/index.tsx (1)
SignInScreen(11-88)
apps/desktop/src/renderer/contexts/AppProviders.tsx (2)
apps/desktop/src/renderer/contexts/AuthProvider.tsx (1)
AuthProvider(11-49)apps/desktop/src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx (1)
PostHogUserIdentifier(8-35)
apps/desktop/src/renderer/contexts/OrganizationsProvider.tsx (2)
apps/desktop/src/renderer/lib/trpc.ts (1)
trpc(5-5)apps/desktop/src/renderer/screens/sign-in/index.tsx (1)
SignInScreen(11-88)
apps/desktop/src/renderer/collections/index.ts (1)
packages/db/src/schema/schema.ts (6)
tasks(132-203)SelectTask(206-206)repositories(103-127)SelectRepository(130-130)users(28-48)SelectUser(51-51)
packages/trpc/src/router/repository/repository.ts (1)
packages/db/src/schema/schema.ts (1)
repositories(103-127)
packages/trpc/src/router/task/schema.ts (1)
packages/db/src/schema/enums.ts (1)
taskPriorityValues(16-22)
🪛 markdownlint-cli2 (0.18.1)
TANSTACK_DB_MIGRATION.md
13-13: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
26-26: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Deploy Admin
- GitHub Check: Deploy Web
- GitHub Check: Deploy API
- GitHub Check: Deploy Docs
- GitHub Check: Deploy Marketing
- GitHub Check: Build
🔇 Additional comments (15)
apps/api/src/app/api/electric/[...path]/route.ts (1)
80-84: LGTM!Response logging follows the prefixed logging pattern and captures useful debugging information without exposing sensitive data.
apps/api/src/proxy.ts (1)
17-17: LGTM!Adding PATCH to the allowed CORS methods is appropriate for supporting partial resource updates via the API.
packages/trpc/src/router/repository/repository.ts (1)
56-85: No action required. The only caller ofrepository.create(inapps/desktop/src/renderer/collections/index.ts) already correctly handles the{ repository, txid }return type by extractingresult.txid. This pattern is consistent with other mutations in the same codebase liketask.createandrepository.update.Likely an incorrect or invalid review comment.
apps/desktop/package.json (1)
48-50: All TanStack DB and idb-keyval packages are at their latest stable versions with no known security vulnerabilities.Verification confirms that @tanstack/db (0.5.16), @tanstack/electric-db-collection (0.2.20), @tanstack/react-db (0.1.60), and idb-keyval (6.2.2) are all at the latest available versions and have no reported security advisories as of January 2026.
apps/desktop/src/renderer/screens/main/components/TasksView/components/OrganizationSwitcher/OrganizationSwitcher.tsx (1)
9-9: LGTM!Clean import consolidation that aligns with the centralized context exports pattern introduced in this PR.
apps/desktop/src/renderer/contexts/AppProviders.tsx (1)
16-19: LGTM!The AuthProvider wrapping is correctly positioned to gate authentication before PostHogUserIdentifier (which requires authenticated user data) and other authenticated components. The provider hierarchy is logical and prevents unauthorized access.
apps/desktop/src/renderer/index.html (1)
14-14: LGTM!Comment update accurately reflects the architectural shift away from Electric SQL to a unified API surface, consistent with the broader PR changes.
apps/desktop/src/renderer/contexts/index.ts (1)
2-2: LGTM!The new exports centralize the contexts public API and provide a consistent import surface for consumers. This is an appropriate use of a barrel file for a well-defined public API boundary.
Also applies to: 8-12
apps/desktop/src/renderer/collections/index.ts (1)
21-37: LGTM! Inline tRPC client follows colocation guidelines.The PR summary notes that this "inlines a single-use HTTP tRPC client per colocation guidelines," which is appropriate for this scoped usage pattern. The client is correctly configured with
httpBatchLinkandsuperjsontransformer.packages/trpc/src/router/task/task.ts (1)
84-100: Transaction pattern looks correct for txid extraction.The transaction pattern correctly:
- Wraps the mutation in
dbWs.transaction- Executes
txid_current()within the same transaction- Returns both the result and txid for sync correlation
This aligns with the Electric SQL synchronization requirements mentioned in the PR summary.
Also applies to: 115-128, 141-153
apps/desktop/src/renderer/contexts/AuthProvider.tsx (1)
30-33: This pattern is correct – no changes needed.Returning
nullduring the loading state (while waiting for the first token emission) is the intended behavior. AppProviders doesn't require special handling because this prevents rendering any children until the auth state is determined. Once a token arrives, AuthProvider either renders the SignInScreen (no token) or the authenticated UI with the context provider. This is the standard pattern for protecting initial app render until authentication is resolved.apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx (4)
1-41: Imports are well-organized and aligned with the new architecture.The transition to TanStack DB (
useLiveQuery) and the new context providers (CollectionsProvider,OrganizationProvider, etc.) is properly reflected in the imports.
149-266: TaskCard implements the camelCase property migration consistently.Good use of lookup objects (
priorityColors,statusColors) for status/priority coloring instead of conditional chains. As per coding guidelines, this is the preferred pattern.
273-290: Query pattern follows TanStack DB conventions with proper dependencies.Including both
tasksCollectionandactiveOrganizationIdin the dependency array ensures the query refreshes when the organization changes. The client-sidedeletedAtfilter is a reasonable approach for soft-delete handling with this architecture.
356-366: Provider composition correctly scopes collections to the tasks view.The hierarchy
OrganizationsProvider → OrganizationProvider → CollectionsProviderestablishes proper data flow where collections are created based on the active organization context.
| onInsert: async ({ transaction }) => { | ||
| const item = transaction.mutations[0].modified; | ||
| const result = await httpTrpcClient.task.create.mutate(item); | ||
| return { txid: result.txid }; | ||
| }, | ||
|
|
||
| onUpdate: async ({ transaction }) => { | ||
| const { modified } = transaction.mutations[0]; | ||
| const result = await httpTrpcClient.task.update.mutate(modified); | ||
| return { txid: result.txid }; | ||
| }, | ||
|
|
||
| onDelete: async ({ transaction }) => { | ||
| const item = transaction.mutations[0].original; | ||
| const result = await httpTrpcClient.task.delete.mutate(item.id); | ||
| return { txid: result.txid }; | ||
| }, |
There was a problem hiding this comment.
No error handling for write operations – mutations will fail silently.
The onInsert, onUpdate, and onDelete handlers don't catch or handle errors from the tRPC mutations. If the API call fails (network error, validation error, server error), the error will propagate to TanStack DB's transaction layer, but there's no user feedback or logging to aid debugging.
Consider adding error handling with contextual logging:
🔎 Suggested error handling pattern
onInsert: async ({ transaction }) => {
- const item = transaction.mutations[0].modified;
- const result = await httpTrpcClient.task.create.mutate(item);
- return { txid: result.txid };
+ try {
+ const item = transaction.mutations[0].modified;
+ const result = await httpTrpcClient.task.create.mutate(item);
+ return { txid: result.txid };
+ } catch (error) {
+ console.error('[collections/tasks] Insert failed:', error);
+ throw error; // Re-throw to let TanStack DB handle rollback
+ }
},
onUpdate: async ({ transaction }) => {
- const { modified } = transaction.mutations[0];
- const result = await httpTrpcClient.task.update.mutate(modified);
- return { txid: result.txid };
+ try {
+ const { modified } = transaction.mutations[0];
+ const result = await httpTrpcClient.task.update.mutate(modified);
+ return { txid: result.txid };
+ } catch (error) {
+ console.error('[collections/tasks] Update failed:', error);
+ throw error;
+ }
},
onDelete: async ({ transaction }) => {
- const item = transaction.mutations[0].original;
- const result = await httpTrpcClient.task.delete.mutate(item.id);
- return { txid: result.txid };
+ try {
+ const item = transaction.mutations[0].original;
+ const result = await httpTrpcClient.task.delete.mutate(item.id);
+ return { txid: result.txid };
+ } catch (error) {
+ console.error('[collections/tasks] Delete failed:', error);
+ throw error;
+ }
},Apply the same pattern to repositories collection (lines 104-114).
🤖 Prompt for AI Agents
In apps/desktop/src/renderer/collections/index.ts around lines 71-87, the
onInsert/onUpdate/onDelete handlers call tRPC mutations without error handling;
wrap each async mutation in a try/catch that logs contextual information
(operation name, item or id, and the caught error) using the app logger (or
console.error if no logger available) and then rethrow the error so TanStack DB
can roll back and upstream code can surface user feedback; apply the same
try/catch + log + rethrow pattern to the repositories collection handlers around
lines 104-114.
| // No token - show sign in screen | ||
| if (accessToken === null) { | ||
| // Import dynamically to avoid circular deps | ||
| const SignInScreen = require("renderer/screens/sign-in").default; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace dynamic require with lazy import for better type safety and module resolution.
Using require in TypeScript/React code bypasses static analysis and can lead to runtime errors if the module structure changes. The .default access is fragile and won't be caught by the type checker.
🔎 Recommended refactoring using React.lazy
+import { lazy, Suspense } from "react";
+
+const SignInScreen = lazy(() => import("renderer/screens/sign-in"));
export function AuthProvider({ children }: { children: ReactNode }) {
// ... existing code ...
// No token - show sign in screen
if (accessToken === null) {
- // Import dynamically to avoid circular deps
- const SignInScreen = require("renderer/screens/sign-in").default;
- return <SignInScreen />;
+ return (
+ <Suspense fallback={null}>
+ <SignInScreen />
+ </Suspense>
+ );
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/desktop/src/renderer/contexts/AuthProvider.tsx around line 38, the code
uses a dynamic require with .default which bypasses TypeScript checks and is
fragile; replace it with a typed React.lazy import: import React if not present,
create SignInScreen via React.lazy(() => import("renderer/screens/sign-in")),
remove the .default access, and render the lazy component inside a Suspense with
an appropriate fallback where SignInScreen is used so the module loads
asynchronously and preserves type safety and correct module resolution.
| id: z.string().uuid(), | ||
| title: z.string().min(1).optional(), | ||
| description: z.string().nullable().optional(), | ||
| description: z.string().nullish(), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for task update/create usage that may need null handling
echo "=== Searching for task create/update mutations ==="
rg -n -C 5 'createTask|updateTask' --type ts --type tsx -g '!**/schema.ts'
echo -e "\n=== Searching for task schema usage ==="
rg -n -C 3 'createTaskSchema|updateTaskSchema' --type ts --type tsxRepository: superset-sh/superset
Length of output: 228
🏁 Script executed:
#!/bin/bash
# First, let's look at the schema file to understand the changes
echo "=== Schema file content ==="
cat -n packages/trpc/src/router/task/schema.ts | head -40
echo -e "\n=== Searching for task create/update imports and usage ==="
# Use proper rg syntax without specifying tsx/ts
rg 'createTask|updateTask' -A 5 -B 5 packages/
echo -e "\n=== Searching for schema imports ==="
rg 'createTaskSchema|updateTaskSchema|taskSchema' -A 3 -B 1 packages/Repository: superset-sh/superset
Length of output: 6929
🏁 Script executed:
#!/bin/bash
# Get the full task.ts file to see mutation implementations
echo "=== Full task.ts file ==="
cat -n packages/trpc/src/router/task/task.ts
echo -e "\n=== Check database schema for nullable fields ==="
# Find the task table definition
find packages -name "*.ts" | xargs grep -l "tasks.*schema\|drizzle.*task" | head -10Repository: superset-sh/superset
Length of output: 5367
🏁 Script executed:
#!/bin/bash
# Find the tasks table schema definition
echo "=== Searching for tasks table schema ==="
find packages -type f -name "*.ts" | xargs grep -l "export const tasks" | head -5
# Also search more broadly
echo -e "\n=== Database schema files ==="
find packages -path "*db*" -name "*.ts" | grep -E "schema|table" | head -20Repository: superset-sh/superset
Length of output: 596
🏁 Script executed:
#!/bin/bash
# Check the tasks table definition in the main database schema
echo "=== Tasks table schema definition ==="
rg -A 30 "export const tasks" packages/db/src/schema/schema.ts
echo -e "\n=== Check if nullable columns are defined ==="
rg -B 2 -A 2 "nullable()" packages/db/src/schema/schema.ts | grep -A 2 -B 2 "description\|repositoryId\|assigneeId\|branch\|estimate\|dueDate\|labels"Repository: superset-sh/superset
Length of output: 1095
🏁 Script executed:
#!/bin/bash
# Get the full tasks table definition
echo "=== Full tasks table schema ==="
rg -A 60 "export const tasks = pgTable" packages/db/src/schema/schema.ts | head -80Repository: superset-sh/superset
Length of output: 1990
🏁 Script executed:
#!/bin/bash
# Search for schema imports and usage across the codebase
echo "=== Schema usage in client code ==="
rg "createTaskSchema|updateTaskSchema|type.*task" --type ts -g '!**/schema.ts' -g '!**/task.ts' packages/ | grep -E "import|\.input|mutation|query" | head -30
echo -e "\n=== Check if there are TypeScript types derived from schemas ==="
rg "typeof.*TaskSchema|z\.infer" packages/trpc/src/router/task/ -A 2Repository: superset-sh/superset
Length of output: 165
🏁 Script executed:
#!/bin/bash
# Broader search for task usage patterns
echo "=== Any inferred types from schemas ==="
rg "infer.*Schema|typeof.*Schema" packages/ --type ts
echo -e "\n=== Task type usage ==="
rg "type.*Task|interface.*Task" packages/ --type ts -g '!**/task.ts' -g '!**/schema.ts' | head -30
echo -e "\n=== Check if mutations are consumed elsewhere ==="
rg "createTask\|updateTask" packages/ --type ts -g '!**/task.ts' -g '!**/schema.ts'
echo -e "\n=== Search for client mutations/api calls ==="
find packages -type f -name "*.ts" -o -name "*.tsx" | xargs grep -l "trpc\|mutation\|api" | grep -i client | head -10Repository: superset-sh/superset
Length of output: 1622
🏁 Script executed:
#!/bin/bash
# Search for TRPC client usage and how mutations are called
echo "=== TRPC router structure ==="
rg "export.*taskRouter|router\.task" packages/ --type ts | head -20
echo -e "\n=== Check app usage of TRPC ==="
find packages -type f \( -name "*.ts" -o -name "*.tsx" \) | xargs grep -l "useMutation\|trpc\." | head -10
echo -e "\n=== Search for any form or validation that uses task schemas ==="
rg "createTask|updateTask|TaskSchema" packages/ --type tsx -A 3 | head -50Repository: superset-sh/superset
Length of output: 379
🏁 Script executed:
#!/bin/bash
# Find the main router definition
echo "=== Main TRPC router ==="
find packages -type f -name "*.ts" | xargs grep -l "createTRPCRouter\|t\.router" | head -5
echo -e "\n=== Router index to see all exposed routes ==="
find packages/trpc/src/router -name "index.ts" | xargs cat
echo -e "\n=== Check for any integration tests ==="
find . -type f -name "*.test.ts" -o -name "*.spec.ts" | grep -i task | head -10Repository: superset-sh/superset
Length of output: 610
Schema changes correctly handle SQL NULLs and database columns are properly nullable.
The migration from .optional() to .nullish() properly aligns with SQL NULL handling. The database schema confirms all relevant fields (description, repositoryId, assigneeId, branch, estimate, dueDate, labels, prUrl) are nullable and the mutations correctly pass null values through Drizzle ORM to the database.
However, there is an inconsistency in null handling for the labels field: the create mutation coerces null to an empty array (input.labels ?? []), while the update mutation allows null through directly. Since the labels field defaults to [] in the database schema, consider applying the same coercion in the update mutation for consistency, or document why the behavior should differ.
🤖 Prompt for AI Agents
In packages/trpc/src/router/task/schema.ts around line 22, the labels field is
nullish in the schema but the update mutation currently lets null pass through
while the create mutation coerces null to an empty array; update the update
mutation to mirror create by converting null/undefined labels to [] (e.g., use
the same labels ?? [] coercion before passing to the ORM) so behavior matches
the DB default, or alternatively add a comment explaining why update
intentionally permits null if you want different semantics.
| const [user] = await dbWs | ||
| .select() | ||
| .from(users) | ||
| .where(eq(users.clerkId, ctx.userId)) | ||
| .limit(1); | ||
| if (!user) throw new Error("User not found"); |
There was a problem hiding this comment.
User lookup outside transaction – potential inconsistency.
The user is fetched via dbWs (line 76) before starting the transaction (line 84). If the user is deleted between the lookup and the insert, the foreign key constraint will fail. The user lookup should be inside the transaction for consistency.
🔎 Proposed fix to move user lookup into transaction
create: protectedProcedure
.input(createTaskSchema)
.mutation(async ({ ctx, input }) => {
- const [user] = await dbWs
- .select()
- .from(users)
- .where(eq(users.clerkId, ctx.userId))
- .limit(1);
- if (!user) throw new Error("User not found");
-
// Execute in transaction to get txid
const result = await dbWs.transaction(async (tx) => {
+ const [user] = await tx
+ .select()
+ .from(users)
+ .where(eq(users.clerkId, ctx.userId))
+ .limit(1);
+ if (!user) throw new Error("User not found");
+
const [task] = await tx
.insert(tasks)
.values({
...input,
creatorId: user.id,
labels: input.labels ?? [],
})
.returning();
const result = await tx.execute<{ txid: string }>(
sql`SELECT txid_current()::text as txid`,
);
const txid = Number.parseInt(result.rows[0]?.txid ?? "", 10);
return { task, txid };
});🤖 Prompt for AI Agents
In packages/trpc/src/router/task/task.ts around lines 76 to 81, the code
performs a user lookup using dbWs before the transaction (started later), which
can lead to a race where the user is deleted before the insert and cause a
foreign-key failure; move the SELECT that finds the user into the same
transaction block and run it via the transaction client (tx) instead of dbWs,
check for user existence inside the tx and throw/rollback there if not found,
then perform the subsequent insert using the same tx so both read and write are
consistent.
| const result = await tx.execute<{ txid: string }>( | ||
| sql`SELECT txid_current()::text as txid`, | ||
| ); | ||
| const txid = Number.parseInt(result.rows[0]?.txid ?? "", 10); |
There was a problem hiding this comment.
Unsafe txid parsing – returns NaN on invalid input.
Number.parseInt(result.rows[0]?.txid ?? "", 10) will return NaN if:
rows[0]is undefinedtxidis undefined- The string is not a valid number
Returning { txid: NaN } will cause silent failures in the sync layer. The code should throw an error if txid extraction fails.
🔎 Proposed fix with validation
const result = await tx.execute<{ txid: string }>(
sql`SELECT txid_current()::text as txid`,
);
-const txid = Number.parseInt(result.rows[0]?.txid ?? "", 10);
+const txidStr = result.rows[0]?.txid;
+if (!txidStr) {
+ throw new Error('[task.create] Failed to retrieve transaction ID');
+}
+const txid = Number.parseInt(txidStr, 10);
+if (Number.isNaN(txid)) {
+ throw new Error(`[task.create] Invalid transaction ID: ${txidStr}`);
+}
return { task, txid };Apply the same pattern to lines 122-125 (update) and lines 147-150 (delete).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const result = await tx.execute<{ txid: string }>( | |
| sql`SELECT txid_current()::text as txid`, | |
| ); | |
| const txid = Number.parseInt(result.rows[0]?.txid ?? "", 10); | |
| const result = await tx.execute<{ txid: string }>( | |
| sql`SELECT txid_current()::text as txid`, | |
| ); | |
| const txidStr = result.rows[0]?.txid; | |
| if (!txidStr) { | |
| throw new Error('[task.create] Failed to retrieve transaction ID'); | |
| } | |
| const txid = Number.parseInt(txidStr, 10); | |
| if (Number.isNaN(txid)) { | |
| throw new Error(`[task.create] Invalid transaction ID: ${txidStr}`); | |
| } |
🤖 Prompt for AI Agents
In packages/trpc/src/router/task/task.ts around lines 94-97, the txid extraction
uses Number.parseInt on potentially missing or malformed values which yields
NaN; instead, validate that result.rows[0] exists and result.rows[0].txid is a
non-empty string of digits (e.g. /^\d+$/), throw a descriptive Error if
validation fails, and only then safely parseInt the txid; apply the same
validation-and-throw pattern to the corresponding txid parsing at lines 122-125
(update) and lines 147-150 (delete).
| if (result.task) { | ||
| syncTask(result.task.id); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the task.ts file at the specified lines
head -n 135 packages/trpc/src/router/task/task.ts | tail -n 35Repository: superset-sh/superset
Length of output: 777
🏁 Script executed:
# Find the syncTask function definition
fd syncTask packages/trpc --type f | head -20Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Search for syncTask definition
rg -l "syncTask" packages/trpc/src --type ts --type tsxRepository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Search for syncTask definition more broadly
rg "syncTask" packages/trpc/src --max-count=20Repository: superset-sh/superset
Length of output: 476
🏁 Script executed:
# Also look in the entire packages directory
rg "export.*syncTask|function syncTask" packages --max-count=30Repository: superset-sh/superset
Length of output: 246
🏁 Script executed:
# Check the imports in task.ts to see where syncTask comes from
head -n 50 packages/trpc/src/router/task/task.ts | grep -E "import|from|syncTask"Repository: superset-sh/superset
Length of output: 579
🏁 Script executed:
# Read the syncTask implementation
cat -n packages/trpc/src/lib/integrations/sync/tasks.tsRepository: superset-sh/superset
Length of output: 1681
🏁 Script executed:
# Get more context around the syncTask calls in the mutations
sed -n '95,110p' packages/trpc/src/router/task/task.ts
sed -n '120,140p' packages/trpc/src/router/task/task.tsRepository: superset-sh/superset
Length of output: 803
🏁 Script executed:
# Check if there's any global error handling or middleware
head -n 30 packages/trpc/src/trpc.ts | cat -nRepository: superset-sh/superset
Length of output: 1049
Wrap syncTask with error handling to prevent unhandled promise rejections.
The syncTask function is called without await (lines 102–104, 130–132) and can throw errors during database queries. If an error occurs, it will result in an unhandled promise rejection. Add .catch() to handle errors:
if (result.task) {
syncTask(result.task.id).catch((err) => {
console.error("[task/sync] Background sync failed:", err);
});
}🤖 Prompt for AI Agents
packages/trpc/src/router/task/task.ts around lines 102-104 and 130-132: syncTask
is invoked without awaiting and can throw, causing unhandled promise rejections;
wrap the call in a fire-and-forget error handler by adding .catch(...) to log
the error (include context like "[task/sync] Background sync failed:") so
background sync failures are captured and do not crash the process.
| ``` | ||
| Cloud PostgreSQL → Electric Sync → PGlite (idb://superset_{orgId}) | ||
| ↓ | ||
| Drizzle ORM | ||
| ↓ | ||
| useLiveDrizzle() | ||
| ↓ | ||
| React Components | ||
| ``` |
There was a problem hiding this comment.
Add language specifiers to fenced code blocks.
The markdown linter identified code blocks without language specifiers. Adding them improves syntax highlighting and readability.
🔎 Proposed fix
Add text or appropriate language identifier to the code blocks at lines 13-21 and 26-32:
**Current Desktop Data Flow:**
-```
+```text
Cloud PostgreSQL → Electric Sync → PGlite (idb://superset_{orgId})
...New Data Flow:
- +text
Cloud PostgreSQL → Electric Shapes → TanStack DB Collections
...
Also applies to: 26-32
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
13-13: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In TANSTACK_DB_MIGRATION.md around lines 13 to 21, the fenced code blocks lack
language specifiers; update those triple-backtick blocks to include an
appropriate language tag (e.g., "text" or a suitable language) so markdown
linters will apply syntax highlighting and remove the lint warning; apply the
same change to the other affected block at lines 26 to 32.
- Remove unnecessary console.logs from electric-proxy and auth routers - Remove redundant useActiveOrganization convenience hook - Rename OrganizationProvider to ActiveOrganizationProvider for clarity - Remove type redefinitions (use TaskPriority from @superset/db/enums) - Fix tRPC path from /trpc to /api/trpc in collections HTTP client - Consolidate txid_current() boilerplate into getCurrentTxid() utility - Remove unnecessary comments from tRPC routers - Delete TANSTACK_DB_MIGRATION.md (completed migration) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx (1)
1-364: Split components into separate files per coding guidelines.This file contains multiple components (TaskEditDialog, TaskCard, TasksList, Sidebar, TasksViewContent, TasksView), violating the guideline: "One component per file - do not create multi-component files."
As per coding guidelines, split into:
TaskEditDialog.tsxTaskCard.tsxTasksList.tsxSidebar.tsxTasksViewContent.tsxTasksView.tsx(main export)Or group related components into a
components/subdirectory within TasksView.
♻️ Duplicate comments (2)
packages/trpc/src/router/task/task.ts (2)
77-82: User lookup should be inside transaction to prevent race condition.The user is fetched before starting the transaction (line 77). If the user is deleted between the lookup and the insert, the foreign key constraint will fail. Move the user lookup inside the transaction for consistency.
🔎 Proposed fix
create: protectedProcedure .input(createTaskSchema) .mutation(async ({ ctx, input }) => { - const [user] = await dbWs - .select() - .from(users) - .where(eq(users.clerkId, ctx.userId)) - .limit(1); - if (!user) throw new Error("User not found"); - const result = await dbWs.transaction(async (tx) => { + const [user] = await tx + .select() + .from(users) + .where(eq(users.clerkId, ctx.userId)) + .limit(1); + if (!user) throw new Error("User not found"); + const [task] = await tx .insert(tasks) .values({
99-101: Add error handling for backgroundsyncTaskto prevent unhandled rejections.
syncTaskis called fire-and-forget but can throw errors. Unhandled promise rejections will cause silent failures or crash the process.🔎 Proposed fix
if (result.task) { - syncTask(result.task.id); + syncTask(result.task.id).catch((err) => { + console.error("[task/create] Background sync failed:", err); + }); }Apply the same pattern to the update mutation at lines 123-125.
🧹 Nitpick comments (5)
apps/desktop/src/renderer/contexts/ActiveOrganizationProvider.tsx (1)
24-24: Consider error handling for localStorage failures.While localStorage is appropriate for the renderer process, operations can fail (quota exceeded, privacy mode, etc.). Consider wrapping in try-catch blocks to handle potential errors gracefully.
🔎 Example approach
const [activeOrganizationId, setActiveOrganizationId] = useState<string>( () => { + let stored: string | null = null; + try { + stored = localStorage.getItem(ACTIVE_ORG_KEY); + } catch (error) { + console.error("[ActiveOrganizationProvider] localStorage read failed:", error); + } - const stored = localStorage.getItem(ACTIVE_ORG_KEY); const valid = organizations.find((o) => o.id === stored); return valid?.id ?? organizations[0].id; }, ); const switchOrganization = (newOrgId: string) => { - localStorage.setItem(ACTIVE_ORG_KEY, newOrgId); + try { + localStorage.setItem(ACTIVE_ORG_KEY, newOrgId); + } catch (error) { + console.error("[ActiveOrganizationProvider] localStorage write failed:", error); + } setActiveOrganizationId(newOrgId); };Also applies to: 38-38
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx (4)
57-82: Simplify priority type casting.The priority field is cast to a verbose union of string literals (lines 65-70), but
TaskPriorityis already imported and should match this union. Consider using the imported type directly or removing the cast if the type inference is correct.🔎 Proposed simplification
const handleSave = async () => { setIsSaving(true); try { await tasksCollection.update(task.id, (draft) => { draft.title = title; draft.description = description || null; - draft.priority = priority as - | "urgent" - | "high" - | "medium" - | "low" - | "none"; + draft.priority = priority as TaskPriority; }); toast.success("Task updated"); onOpenChange(false); } catch (error) { console.error("[TaskEditDialog] Update failed:", error); toast.error( `Failed to update task: ${error instanceof Error ? error.message : String(error)}`, ); } finally { setIsSaving(false); } };
202-212: Consider simplifying status badge logic.The status color logic on line 204 checks
task.statusColor ? "" : "bg-gray-400"but the lookupstatusColors[task.status]already provides a fallback via|| .... The double-fallback pattern could be clearer.🔎 Possible simplification
<Badge variant="secondary" - className={`text-xs text-white ${statusColors[task.status] || (task.statusColor ? "" : "bg-gray-400")}`} + className={`text-xs text-white ${task.statusColor ? "" : (statusColors[task.status] || "bg-gray-400")}`} style={ task.statusColor ? { backgroundColor: task.statusColor } : undefined } > {task.status} </Badge>
279-280: Type cast might be unsafe.The result is cast to
SelectTask[]after filtering. Ensure the collection's type signature matchesSelectTaskfrom the schema. If there's any mismatch between collection types and schema types, this cast could hide runtime errors.Consider adding a runtime validation or ensuring the collection type is explicitly typed as
SelectTask[]:const tasks = (allTasks?.filter((task): task is SelectTask => task.deletedAt === null) || []);
138-140: Button text implies immediate Linear sync.The button says "Save & Sync to Linear" but there's no visible Linear-specific logic or confirmation that sync completed. If the sync happens asynchronously in the API, consider updating the text to "Save" or "Save Changes" to avoid implying immediate synchronization.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/desktop/src/lib/trpc/routers/auth/index.tsapps/desktop/src/renderer/collections/index.tsapps/desktop/src/renderer/contexts/ActiveOrganizationProvider.tsxapps/desktop/src/renderer/contexts/AuthProvider.tsxapps/desktop/src/renderer/contexts/CollectionsProvider.tsxapps/desktop/src/renderer/contexts/OrganizationsProvider.tsxapps/desktop/src/renderer/contexts/index.tsapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxpackages/db/src/utils/sql.tspackages/trpc/src/router/repository/repository.tspackages/trpc/src/router/task/task.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/desktop/src/renderer/contexts/AuthProvider.tsx
- apps/desktop/src/renderer/contexts/index.ts
- apps/desktop/src/lib/trpc/routers/auth/index.ts
- apps/desktop/src/renderer/collections/index.ts
- apps/desktop/src/renderer/contexts/CollectionsProvider.tsx
- apps/desktop/src/renderer/contexts/OrganizationsProvider.tsx
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use object parameters for functions with 2+ parameters instead of positional arguments
Functions with 2+ parameters should accept a single params object with named properties for self-documentation and extensibility
Use prefixed console logging with context pattern: [domain/operation] message
Extract magic numbers and hardcoded values to named constants at module top
Use lookup objects/maps instead of repeated if (type === ...) conditionals
Avoid usinganytype - maintain type safety in TypeScript code
Never swallow errors silently - at minimum log them with context
Import from concrete files directly when possible - avoid barrel file abuse that creates circular dependencies
Avoid deep nesting (4+ levels) - use early returns, extract functions, and invert conditions
Use named properties in options objects instead of boolean parameters to avoid boolean blindness
Files:
packages/db/src/utils/sql.tspackages/trpc/src/router/task/task.tsapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/contexts/ActiveOrganizationProvider.tsxpackages/trpc/src/router/repository/repository.ts
packages/db/src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Schema definitions must be in packages/db/src/ using Drizzle ORM
Files:
packages/db/src/utils/sql.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for formatting and linting - run at root level with
bun run lint:fixorbiome check --write
Files:
packages/db/src/utils/sql.tspackages/trpc/src/router/task/task.tsapps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/contexts/ActiveOrganizationProvider.tsxpackages/trpc/src/router/repository/repository.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/contexts/ActiveOrganizationProvider.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Files:
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/contexts/ActiveOrganizationProvider.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
One component per file - do not create multi-component files
Files:
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/contexts/ActiveOrganizationProvider.tsx
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations - never use raw SQL
Files:
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsxapps/desktop/src/renderer/contexts/ActiveOrganizationProvider.tsx
🧠 Learnings (7)
📚 Learning: 2026-01-02T06:50:28.662Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.662Z
Learning: Applies to apps/api/**/*.{ts,tsx} : tRPC procedures and API route handlers should validate and delegate; keep orchestrators thin
Applied to files:
packages/trpc/src/router/task/task.ts
📚 Learning: 2026-01-02T06:50:28.662Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.662Z
Learning: Applies to apps/api/**/*.{ts,tsx} : Extract business logic from tRPC procedures into utility functions when logic exceeds ~50 lines, is used by multiple procedures, or needs independent testing
Applied to files:
packages/trpc/src/router/task/task.ts
📚 Learning: 2026-01-02T06:50:28.662Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.662Z
Learning: Applies to **/*.{ts,tsx} : Never swallow errors silently - at minimum log them with context
Applied to files:
packages/trpc/src/router/task/task.ts
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from `trpc/server/observable` instead of async generators, as the library explicitly checks `isObservable(result)` and throws an error otherwise
Applied to files:
packages/trpc/src/router/task/task.ts
📚 Learning: 2026-01-02T06:50:28.662Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.662Z
Learning: Applies to apps/api/**/*.{ts,tsx} : Use TRPCError with appropriate error codes: NOT_FOUND, UNAUTHORIZED, FORBIDDEN, BAD_REQUEST, INTERNAL_SERVER_ERROR, NOT_IMPLEMENTED
Applied to files:
packages/trpc/src/router/task/task.ts
📚 Learning: 2026-01-02T06:50:28.662Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.662Z
Learning: Applies to packages/db/src/**/*.ts : Schema definitions must be in packages/db/src/ using Drizzle ORM
Applied to files:
packages/trpc/src/router/task/task.tspackages/trpc/src/router/repository/repository.ts
📚 Learning: 2026-01-02T06:50:28.662Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.662Z
Learning: Applies to apps/**/*.{ts,tsx} : Use Drizzle ORM for all database operations - never use raw SQL
Applied to files:
packages/trpc/src/router/task/task.tspackages/trpc/src/router/repository/repository.ts
🧬 Code graph analysis (4)
packages/trpc/src/router/task/task.ts (2)
packages/db/src/utils/sql.ts (1)
getCurrentTxid(19-27)packages/trpc/src/lib/integrations/sync/tasks.ts (1)
syncTask(13-48)
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx (4)
packages/db/src/schema/schema.ts (2)
SelectTask(206-206)tasks(132-203)packages/local-db/src/schema/schema.ts (2)
SelectTask(268-268)tasks(219-265)apps/desktop/src/renderer/contexts/CollectionsProvider.tsx (2)
useCollections(53-59)CollectionsProvider(15-51)apps/desktop/src/renderer/contexts/ActiveOrganizationProvider.tsx (2)
useActiveOrganization(55-63)ActiveOrganizationProvider(15-53)
apps/desktop/src/renderer/contexts/ActiveOrganizationProvider.tsx (1)
apps/desktop/src/renderer/contexts/OrganizationsProvider.tsx (1)
useOrganizations(15-22)
packages/trpc/src/router/repository/repository.ts (2)
packages/db/src/schema/schema.ts (1)
repositories(103-127)packages/db/src/utils/sql.ts (1)
getCurrentTxid(19-27)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Deploy Web
- GitHub Check: Deploy API
- GitHub Check: Deploy Docs
- GitHub Check: Deploy Marketing
- GitHub Check: Deploy Admin
- GitHub Check: Build
🔇 Additional comments (7)
packages/trpc/src/router/repository/repository.ts (2)
69-82: Clean transactional pattern - past shadowing issues resolved.The refactor to use
getCurrentTxidutility eliminates the variable shadowing flagged in previous reviews. The transaction pattern is consistent across all mutations.
92-122: LGTM - consistent transaction handling across update and delete.Both mutations follow the same clean pattern established in
create. The txid extraction is now properly delegated to the shared utility.packages/trpc/src/router/task/task.ts (2)
111-127: Transaction pattern is correct; syncTask needs same error handling as create.The update mutation correctly uses the transactional pattern with
getCurrentTxid. ThesyncTaskcall on line 124 needs the same.catch()error handling suggested for the create mutation.
133-144: LGTM - delete mutation is clean.The soft-delete pattern with transaction and txid extraction is correctly implemented.
apps/desktop/src/renderer/contexts/ActiveOrganizationProvider.tsx (1)
1-63: Clean context pattern implementation.The provider follows React context best practices with proper typing, error boundaries, and hook enforcement. The localStorage persistence pattern is appropriate for this use case.
apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx (2)
89-89: Consistent camelCase migration.The field name changes from snake_case to camelCase (e.g.,
externalKey,dueDate,assigneeId,statusColor,externalUrl,externalProvider) are consistent throughout TaskCard and align with the schema refactor described in the PR.Also applies to: 178-180, 204-207, 223-227, 229-233, 240-249
354-364: Provider scoping is appropriate for experimental feature.The provider nesting (OrganizationsProvider → ActiveOrganizationProvider → CollectionsProvider) is scoped to TasksView as stated in the PR objectives, which is good for isolating this experimental feature. The separation of concerns is maintained.
| const [activeOrganizationId, setActiveOrganizationId] = useState<string>( | ||
| () => { | ||
| const stored = localStorage.getItem(ACTIVE_ORG_KEY); | ||
| const valid = organizations.find((o) => o.id === stored); | ||
| return valid?.id ?? organizations[0].id; | ||
| }, | ||
| ); |
There was a problem hiding this comment.
Guard against empty organizations array.
If organizations is empty, organizations[0].id will throw an error. Add a check to handle this edge case gracefully.
🔎 Proposed fix
const [activeOrganizationId, setActiveOrganizationId] = useState<string>(
() => {
+ if (organizations.length === 0) {
+ throw new Error("No organizations available");
+ }
const stored = localStorage.getItem(ACTIVE_ORG_KEY);
const valid = organizations.find((o) => o.id === stored);
return valid?.id ?? organizations[0].id;
},
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [activeOrganizationId, setActiveOrganizationId] = useState<string>( | |
| () => { | |
| const stored = localStorage.getItem(ACTIVE_ORG_KEY); | |
| const valid = organizations.find((o) => o.id === stored); | |
| return valid?.id ?? organizations[0].id; | |
| }, | |
| ); | |
| const [activeOrganizationId, setActiveOrganizationId] = useState<string>( | |
| () => { | |
| if (organizations.length === 0) { | |
| throw new Error("No organizations available"); | |
| } | |
| const stored = localStorage.getItem(ACTIVE_ORG_KEY); | |
| const valid = organizations.find((o) => o.id === stored); | |
| return valid?.id ?? organizations[0].id; | |
| }, | |
| ); |
🤖 Prompt for AI Agents
In apps/desktop/src/renderer/contexts/ActiveOrganizationProvider.tsx around
lines 22 to 28, the initializer assumes organizations[0] exists and will throw
if the organizations array is empty; update the initializer to guard for an
empty array (e.g., if organizations.length === 0 return null or an empty
string), change the state type to allow null (useState<string | null>(...)), and
ensure the logic first checks the stored value against organizations before
falling back to organizations[0]?.id ?? null so the provider handles the
empty-orgs edge case gracefully.
| const switchOrganization = (newOrgId: string) => { | ||
| localStorage.setItem(ACTIVE_ORG_KEY, newOrgId); | ||
| setActiveOrganizationId(newOrgId); | ||
| }; |
There was a problem hiding this comment.
Validate organization ID before switching.
switchOrganization doesn't verify that newOrgId exists in the organizations list. If an invalid ID is passed, it will trigger the error on line 34 on the next render.
🔎 Proposed fix
const switchOrganization = (newOrgId: string) => {
+ const exists = organizations.some((o) => o.id === newOrgId);
+ if (!exists) {
+ throw new Error(`Organization not found: ${newOrgId}`);
+ }
localStorage.setItem(ACTIVE_ORG_KEY, newOrgId);
setActiveOrganizationId(newOrgId);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const switchOrganization = (newOrgId: string) => { | |
| localStorage.setItem(ACTIVE_ORG_KEY, newOrgId); | |
| setActiveOrganizationId(newOrgId); | |
| }; | |
| const switchOrganization = (newOrgId: string) => { | |
| const exists = organizations.some((o) => o.id === newOrgId); | |
| if (!exists) { | |
| throw new Error(`Organization not found: ${newOrgId}`); | |
| } | |
| localStorage.setItem(ACTIVE_ORG_KEY, newOrgId); | |
| setActiveOrganizationId(newOrgId); | |
| }; |
🤖 Prompt for AI Agents
In apps/desktop/src/renderer/contexts/ActiveOrganizationProvider.tsx around
lines 37 to 40, switchOrganization currently writes any newOrgId to localStorage
and state without verifying it exists; update switchOrganization to first check
that newOrgId is present in the current organizations list (e.g., find by id on
the organizations array/context), and only then set localStorage and
setActiveOrganizationId; if the id is invalid, avoid changing state and log or
return an error/boolean so callers can handle the failure.
| const { data: allTasks, isLoading } = useLiveQuery( | ||
| (q) => q.from({ tasks: tasksCollection }), | ||
| [tasksCollection, activeOrganizationId], | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, examine the TasksView.tsx file to see the context
cat -n apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx | sed -n '265,285p'Repository: superset-sh/superset
Length of output: 1045
🏁 Script executed:
# Search for CollectionsProvider and tasksCollection definition
rg -B 5 -A 15 'tasksCollection\s*=' apps/desktop/src/renderer/ | head -100Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Find where tasksCollection comes from (imports)
rg -B 2 'useLiveQuery|tasksCollection' apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx | head -50Repository: superset-sh/superset
Length of output: 981
🏁 Script executed:
# Find useCollections hook implementation
rg -B 5 -A 20 'function useCollections|const useCollections' apps/desktop/src/renderer/Repository: superset-sh/superset
Length of output: 1041
🏁 Script executed:
# Find useActiveOrganization hook implementation
rg -B 5 -A 20 'function useActiveOrganization|const useActiveOrganization' apps/desktop/src/Repository: superset-sh/superset
Length of output: 1288
🏁 Script executed:
# Find the full CollectionsProvider implementation
cat -n apps/desktop/src/renderer/contexts/CollectionsProvider.tsxRepository: superset-sh/superset
Length of output: 2211
🏁 Script executed:
# Also check if useActiveOrganization is passed to or used by CollectionsProvider
rg -B 10 -A 30 'CollectionsProvider|CollectionsContext' apps/desktop/src/renderer/contexts/CollectionsProvider.tsx | head -150Repository: superset-sh/superset
Length of output: 1773
Remove redundant activeOrganizationId dependency from useLiveQuery.
The tasksCollection already changes when activeOrganizationId changes, since it's derived from the Collections object created in the useMemo at CollectionsProvider.tsx:22-40 with activeOrganizationId as a dependency. When the organization changes, the entire Collections object is recreated with a new reference, so tasksCollection automatically reflects the change. Including activeOrganizationId in the dependency array causes unnecessary re-queries and should be removed.
🤖 Prompt for AI Agents
In apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx
around lines 273 to 276, remove the redundant activeOrganizationId from the
useLiveQuery dependency array because tasksCollection already changes when the
organization changes; update the hook call to depend only on tasksCollection
(i.e., pass [tasksCollection]) so re-queries happen only when the collection
reference actually changes.
| export async function getCurrentTxid( | ||
| // biome-ignore lint/suspicious/noExplicitAny: Transaction type varies by client (Neon, PostgresJs, etc) | ||
| tx: PgTransaction<any, any, any>, | ||
| ): Promise<number> { | ||
| const result = await tx.execute<{ txid: string }>( | ||
| sql`SELECT txid_current()::text as txid`, | ||
| ); | ||
| return Number.parseInt(result.rows[0]?.txid ?? "", 10); | ||
| } |
There was a problem hiding this comment.
Add validation to prevent returning NaN on missing/invalid txid.
Number.parseInt("", 10) returns NaN when rows[0] is undefined. This silently propagates invalid txids to all callers (repository/task mutations), potentially breaking the sync layer.
🔎 Proposed fix with validation
export async function getCurrentTxid(
// biome-ignore lint/suspicious/noExplicitAny: Transaction type varies by client (Neon, PostgresJs, etc)
tx: PgTransaction<any, any, any>,
): Promise<number> {
const result = await tx.execute<{ txid: string }>(
sql`SELECT txid_current()::text as txid`,
);
- return Number.parseInt(result.rows[0]?.txid ?? "", 10);
+ const txidStr = result.rows[0]?.txid;
+ if (!txidStr) {
+ throw new Error("[db/getCurrentTxid] Failed to retrieve transaction ID");
+ }
+ const txid = Number.parseInt(txidStr, 10);
+ if (Number.isNaN(txid)) {
+ throw new Error(`[db/getCurrentTxid] Invalid transaction ID: ${txidStr}`);
+ }
+ return txid;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function getCurrentTxid( | |
| // biome-ignore lint/suspicious/noExplicitAny: Transaction type varies by client (Neon, PostgresJs, etc) | |
| tx: PgTransaction<any, any, any>, | |
| ): Promise<number> { | |
| const result = await tx.execute<{ txid: string }>( | |
| sql`SELECT txid_current()::text as txid`, | |
| ); | |
| return Number.parseInt(result.rows[0]?.txid ?? "", 10); | |
| } | |
| export async function getCurrentTxid( | |
| // biome-ignore lint/suspicious/noExplicitAny: Transaction type varies by client (Neon, PostgresJs, etc) | |
| tx: PgTransaction<any, any, any>, | |
| ): Promise<number> { | |
| const result = await tx.execute<{ txid: string }>( | |
| sql`SELECT txid_current()::text as txid`, | |
| ); | |
| const txidStr = result.rows[0]?.txid; | |
| if (!txidStr) { | |
| throw new Error("[db/getCurrentTxid] Failed to retrieve transaction ID"); | |
| } | |
| const txid = Number.parseInt(txidStr, 10); | |
| if (Number.isNaN(txid)) { | |
| throw new Error(`[db/getCurrentTxid] Invalid transaction ID: ${txidStr}`); | |
| } | |
| return txid; | |
| } |
🤖 Prompt for AI Agents
In packages/db/src/utils/sql.ts around lines 19 to 27, the function may return
NaN when rows[0] or txid is missing; validate the query result and parsed value
and throw a clear error instead of returning NaN. After executing the query,
confirm result.rows[0] exists and that result.rows[0].txid is a non-empty
string, parse it with Number.parseInt(..., 10), check Number.isFinite(parsed)
(or !Number.isNaN(parsed)), and if invalid throw a descriptive Error (including
context like the query and transaction id if available) so callers never receive
NaN; otherwise return the parsed number.
Add missing document.head, getElementsByTagName, and createElement
mocks to test-setup.ts to support Sonner toast library's CSS
injection during test execution.
Fixes CI test failure where Sonner tried to call
document.getElementsByTagName('head') but the method was undefined.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add document.createTextNode() mock to complete Sonner toast library support. Sonner's __insertCSS function uses createTextNode to append CSS code to style elements. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Token refresh fix: - Change collections to use dynamic header function for tRPC client - Use ref in CollectionsProvider to always read current accessToken - tRPC httpBatchLink calls getHeaders() on each request for fresh token - Electric Shape uses snapshot of token at creation (auth only on init) Test fixes: - Fix lint error: unused tag parameter in createElement mock - Add document.createTextNode mock for Sonner CSS injection This ensures API mutations work after token refresh without clearing the collections cache (which would destroy Electric sync state). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
getHeaders is always provided, no need for optional chaining. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Replace fragile dynamic require with proper static import of SignInScreen. No need for code splitting in Electron app. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
saddlepaddle
changed the title
The desktop drizzle.config.ts still referenced the deleted pglite schema at ./src/renderer/lib/pglite/schema/schema.ts after #585 migrated to TanstackDb. This caused drizzle-kit commands to fail. The desktop app now uses @superset/local-db package which has its own drizzle.config.ts at packages/local-db/.
* fix(desktop): remove orphaned pglite drizzle config The desktop drizzle.config.ts still referenced the deleted pglite schema at ./src/renderer/lib/pglite/schema/schema.ts after #585 migrated to TanstackDb. This caused drizzle-kit commands to fail. The desktop app now uses @superset/local-db package which has its own drizzle.config.ts at packages/local-db/. * chore(desktop): remove unused drizzle-kit devDependency drizzle-kit is no longer used in the desktop app since schema management moved to packages/local-db. * chore: update lockfile * chore(desktop): remove remaining pglite references - Remove orphaned `@electric-sql/pglite` from Vite optimizeDeps.exclude - Add DEPRECATED banner to LOCAL_FIRST_SYNC.md (describes old PGLite architecture) * remove sync doc
1 participant
Summary
Refactored the TanStack DB collections provider architecture to be simpler and follow repo guidelines.
Changes
Architecture Simplification
CollectionsProvidernow providesCollectionsobject directly instead of spreading individual collection fieldscreateOrgCollectionsandcreateDeviceCollectionsinto singlecreateCollectionsfunctionOrganizationsProvider,OrganizationProvider, andCollectionsProviderto only wrapTasksView(experimental feature isolation)Type Safety Improvements
.optional()to.nullish()in task schemas to properly handle SQLnullvaluesnullToUndefinedhelper function (no longer needed)Code Quality
isInitializing- provider now returnsnullinstead of exposing initialization statecreateHttpTrpcClientinline per repo colocation guidelinesFiles Changed
apps/desktop/src/renderer/contexts/CollectionsProvider.tsx- Main refactoringapps/desktop/src/renderer/contexts/AppProviders.tsx- Removed providers (moved to TasksView)apps/desktop/src/renderer/screens/main/components/TasksView/TasksView.tsx- Added provider wrapping, removed debug logspackages/db/src/schema/schema.ts- Formatting fixesTest Plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores
Bug Fixes
Tests
UX
✏️ Tip: You can customize this high-level summary in your review settings.