Data binding for most of settings ops #1562
Conversation
📝 WalkthroughWalkthroughRemoves the persisted Config model and related schema/table, introduces internal TinyBase schemas for general/AI, renames a hook, updates chat and session components to source user_id from internal store, modularizes Settings into separate components and updates settings routes/layout, seeds a fixed USER_ID, and adjusts review path filters. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant R as Router (settings)
participant L as Left Nav (Group)
participant C as Settings Component
U->>L: Click tab
L->>R: Route.navigate({ tab })
R->>R: Route.useSearch().tab
R->>C: Render selected Settings* component
Note over C: Component reads/updates via shared hooks
sequenceDiagram
autonumber
participant UI as Internal UI Store
participant Chat as Chat View/Session
participant DB as Persisted Store
Chat->>UI: internal.UI.useValues(STORE_ID)
UI-->>Chat: { user_id }
Chat->>DB: Create chat/message rows with user_id
DB-->>Chat: Persisted rows
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop2/src/components/main/body/sessions/outer-header/participant.tsx (1)
123-138: Adduser_idto the dependencies array.The
handleSelectcallback usesuser_id(Line 131) but doesn't include it in the dependencies array (Line 138). This creates a stale closure risk where the callback may use an outdateduser_idvalue.Apply this diff to fix the dependency array:
- }, [store, sessionId]); + }, [store, sessionId, user_id]);
🧹 Nitpick comments (6)
apps/desktop2/src/components/settings/integrations.tsx (1)
1-3: Placeholder component for future implementation.This component is part of the settings modularization but currently has no UI. Ensure it's tracked for implementation.
Would you like me to open an issue to track the implementation of the Integrations settings UI?
apps/desktop2/src/components/settings/feedback.tsx (1)
1-3: Placeholder component for future implementation.Similar to other settings components in this PR, this is a placeholder. Track implementation separately.
apps/desktop2/src/components/settings/calendar.tsx (1)
1-3: Placeholder component for future implementation.apps/desktop2/src/components/settings/billing.tsx (1)
1-3: Placeholder component for future implementation.apps/desktop2/src/hooks/useSafeObjectUpdate.ts (1)
6-6: Good refactor!The rename from
useValidatedRowtouseSafeObjectUpdatebetter describes the hook's purpose of safely updating objects with validation.apps/desktop2/src/store/tinybase/internal.ts (1)
88-92: Address the TODO for hardcoded user_id.The user_id is currently hardcoded to a fixed UUID. This works for development but should be replaced with actual user authentication before production.
Based on the broader PR context, this appears to be temporary scaffolding. Do you want me to:
- Open an issue to track implementing proper user authentication?
- Suggest a pattern for user session management that integrates with this store?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
.coderabbit.yaml(1 hunks)apps/desktop2/src/components/chat/session.tsx(2 hunks)apps/desktop2/src/components/chat/view.tsx(2 hunks)apps/desktop2/src/components/human.tsx(2 hunks)apps/desktop2/src/components/main/body/sessions/outer-header/index.tsx(3 hunks)apps/desktop2/src/components/main/body/sessions/outer-header/participant.tsx(5 hunks)apps/desktop2/src/components/settings/ai.tsx(1 hunks)apps/desktop2/src/components/settings/billing.tsx(1 hunks)apps/desktop2/src/components/settings/calendar.tsx(1 hunks)apps/desktop2/src/components/settings/developers.tsx(1 hunks)apps/desktop2/src/components/settings/feedback.tsx(1 hunks)apps/desktop2/src/components/settings/general.tsx(1 hunks)apps/desktop2/src/components/settings/integrations.tsx(1 hunks)apps/desktop2/src/components/settings/notification.tsx(1 hunks)apps/desktop2/src/components/settings/shared.ts(1 hunks)apps/desktop2/src/components/settings/templates.tsx(1 hunks)apps/desktop2/src/hooks/useSafeObjectUpdate.ts(1 hunks)apps/desktop2/src/routes/app/settings/_layout.index.tsx(1 hunks)apps/desktop2/src/routes/app/settings/_layout.tsx(1 hunks)apps/desktop2/src/store/seed.ts(1 hunks)apps/desktop2/src/store/tinybase/internal.ts(1 hunks)apps/desktop2/src/store/tinybase/persisted.ts(0 hunks)packages/db/src/schema.ts(0 hunks)
💤 Files with no reviewable changes (2)
- packages/db/src/schema.ts
- apps/desktop2/src/store/tinybase/persisted.ts
🧰 Additional context used
📓 Path-based instructions (2)
apps/desktop2/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop2/.cursor/rules/style.mdc)
apps/desktop2/**/*.{ts,tsx,js,jsx}: Use the cn utility for conditional Tailwind classNames, importing it as: import { cn } from "@hypr/ui/lib/utils"; avoid using clsx/classnames directly.
When calling cn, always pass an array of class segments (e.g., cn(["base", condition && "modifier"]))
Files:
apps/desktop2/src/components/settings/feedback.tsxapps/desktop2/src/components/settings/general.tsxapps/desktop2/src/components/chat/view.tsxapps/desktop2/src/components/main/body/sessions/outer-header/index.tsxapps/desktop2/src/components/settings/calendar.tsxapps/desktop2/src/components/settings/billing.tsxapps/desktop2/src/components/settings/notification.tsxapps/desktop2/src/store/tinybase/internal.tsapps/desktop2/src/components/human.tsxapps/desktop2/src/components/settings/templates.tsxapps/desktop2/src/components/settings/ai.tsxapps/desktop2/src/components/settings/shared.tsapps/desktop2/src/components/chat/session.tsxapps/desktop2/src/routes/app/settings/_layout.tsxapps/desktop2/src/hooks/useSafeObjectUpdate.tsapps/desktop2/src/components/main/body/sessions/outer-header/participant.tsxapps/desktop2/src/store/seed.tsapps/desktop2/src/components/settings/integrations.tsxapps/desktop2/src/components/settings/developers.tsxapps/desktop2/src/routes/app/settings/_layout.index.tsx
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. Do not add any error handling. Keep the existing one.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
apps/desktop2/src/components/settings/feedback.tsxapps/desktop2/src/components/settings/general.tsxapps/desktop2/src/components/chat/view.tsxapps/desktop2/src/components/main/body/sessions/outer-header/index.tsxapps/desktop2/src/components/settings/calendar.tsxapps/desktop2/src/components/settings/billing.tsxapps/desktop2/src/components/settings/notification.tsxapps/desktop2/src/store/tinybase/internal.tsapps/desktop2/src/components/human.tsxapps/desktop2/src/components/settings/templates.tsxapps/desktop2/src/components/settings/ai.tsxapps/desktop2/src/components/settings/shared.tsapps/desktop2/src/components/chat/session.tsxapps/desktop2/src/routes/app/settings/_layout.tsxapps/desktop2/src/hooks/useSafeObjectUpdate.tsapps/desktop2/src/components/main/body/sessions/outer-header/participant.tsxapps/desktop2/src/store/seed.tsapps/desktop2/src/components/settings/integrations.tsxapps/desktop2/src/components/settings/developers.tsxapps/desktop2/src/routes/app/settings/_layout.index.tsx
🧬 Code graph analysis (9)
apps/desktop2/src/components/settings/general.tsx (2)
apps/desktop2/src/components/settings/shared.ts (1)
useUpdateGeneral(6-24)apps/desktop2/src/routes/app/settings.tsx (1)
SettingsGeneral(65-100)
apps/desktop2/src/components/settings/notification.tsx (1)
apps/desktop2/src/components/settings/shared.ts (1)
useUpdateGeneral(6-24)
apps/desktop2/src/store/tinybase/internal.ts (1)
apps/desktop2/src/store/tinybase/shared.ts (3)
jsonObject(8-18)ToStorageType(45-48)InferTinyBaseSchema(30-33)
apps/desktop2/src/components/human.tsx (1)
apps/desktop2/src/hooks/useSafeObjectUpdate.ts (1)
useSafeObjectUpdate(6-42)
apps/desktop2/src/components/settings/templates.tsx (2)
packages/db/src/schema.ts (1)
templates(83-88)apps/desktop2/src/components/settings/shared.ts (1)
useUpdateTemplate(38-55)
apps/desktop2/src/components/settings/ai.tsx (2)
packages/ui/src/lib/utils.ts (1)
cn(4-6)packages/ui/src/components/ui/button.tsx (1)
Button(37-89)
apps/desktop2/src/components/settings/shared.ts (3)
apps/desktop2/src/store/tinybase/internal.ts (4)
General(30-30)GeneralStorage(31-31)AI(27-27)AIStorage(28-28)apps/desktop2/src/hooks/useSafeObjectUpdate.ts (1)
useSafeObjectUpdate(6-42)apps/desktop2/src/store/tinybase/persisted.ts (2)
Template(106-106)TemplateStorage(112-112)
apps/desktop2/src/routes/app/settings/_layout.tsx (2)
apps/desktop/src/components/settings/components/types.ts (1)
TABS(28-40)packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/desktop2/src/components/settings/developers.tsx (1)
apps/desktop2/src/auth.tsx (1)
useAuth(124-132)
⏰ 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). (2)
- GitHub Check: ci (windows, windows-latest)
- GitHub Check: ci (macos, macos-14)
🔇 Additional comments (17)
apps/desktop2/src/store/seed.ts (1)
30-30: LGTM: Deterministic seed data.Replacing the dynamic
id()with a fixed UUID makes seed data reproducible, which improves testing reliability.apps/desktop2/src/components/human.tsx (1)
1-1: LGTM: Hook rename applied correctly.The transition from
useValidatedRowtouseSafeObjectUpdateis a straightforward rename with no functional changes. The usage is correct.Also applies to: 15-15
apps/desktop2/src/components/chat/session.tsx (1)
29-29: LGTM: Consistent user_id sourcing pattern.This change matches the pattern in
apps/desktop2/src/components/main/body/sessions/outer-header/index.tsx, sourcinguser_idfrom the internal store. The implementation is correct.apps/desktop2/src/components/main/body/sessions/outer-header/index.tsx (1)
20-20: Confirm internal store seedsuser_idbefore rendering
ThegeneralSchemaandSCHEMAincludeuser_id, but no initialization was found—ensure a validuser_idis set (e.g. via your store setup or auth flow) before callinginternal.UI.useValues.apps/desktop2/src/components/settings/developers.tsx (1)
5-16: LGTM!The component correctly retrieves and displays session data, and the Auth button properly triggers the window command. The minimal styling appears intentional for a developer-facing component.
apps/desktop2/src/components/settings/notification.tsx (1)
3-21: LGTM!The component correctly uses the
useUpdateGeneralhook and properly binds controlled checkboxes to the notification settings fields. The implementation follows React best practices.apps/desktop2/src/components/chat/view.tsx (1)
4-4: LGTM!The switch from persisted config to internal store for
user_idretrieval aligns with the PR's data layer changes. The implementation is consistent and correct.Also applies to: 35-35
apps/desktop2/src/components/settings/general.tsx (1)
3-16: LGTM!The component correctly implements settings for the
save_recordingsfield using theuseUpdateGeneralhook. The controlled checkbox pattern is properly implemented.apps/desktop2/src/components/main/body/sessions/outer-header/participant.tsx (1)
17-17: LGTM!The switch to internal store for
user_idis implemented correctly, andhandleAddproperly includesuser_idin its dependencies.Also applies to: 164-187
apps/desktop2/src/routes/app/settings/_layout.index.tsx (1)
3-11: LGTM!The refactor to import modularized settings components improves code organization and maintainability. The conditional rendering based on
search.tabis clean and straightforward.Also applies to: 20-32
apps/desktop2/src/routes/app/settings/_layout.tsx (2)
40-60: LGTM!The refactored layout with group-based tab organization improves the UI structure. The new
Groupcomponent properly encapsulates tab rendering logic, and the conditional styling forexpandHeightandincludeTrafficLightis correctly implemented.Also applies to: 102-150
14-14: Good adherence to coding guidelines!All
cnutility calls correctly pass arrays of class segments as required by the coding guidelines.As per coding guidelines
Also applies to: 45-99
apps/desktop2/src/components/settings/ai.tsx (3)
93-93: LGTM!The
cnutility is correctly called with an array of class segments, following the coding guidelines.
136-136: LGTM!The
cnutility is correctly called with an array of class segments, following the coding guidelines.
8-177: Consider connecting to actual data sources.This component currently renders mock/placeholder data for models and providers. While this is fine for initial UI development, ensure that data binding is added before release to connect these cards to actual provider configurations and API keys.
For reference, the PR introduces hooks like
useUpdateAIinapps/desktop2/src/components/settings/shared.tsthat could be used to read and update AI provider configurations. Consider integrating these hooks to make the provider cards functional.apps/desktop2/src/store/tinybase/internal.ts (2)
9-31: LGTM!The schema definitions are well-structured:
generalSchemaproperly defines all general settings fields with appropriate defaultsaiSchemacaptures provider configuration (base_url, api_key)- Uses
jsonObjecthelper for array fields that need JSON serialization/deserialization- Exports both parsed types (AI, General) and storage types (AIStorage, GeneralStorage) for type safety across the stack
The implementation aligns with TinyBase best practices and the broader PR objective of introducing internal schemas for settings.
35-66: LGTM!The SCHEMA definition correctly:
- Uses
InferTinyBaseSchemato ensure type consistency between Zod schemas and TinyBase schemas- Adds all general settings fields to
valuewith appropriate types- Adds the new
aitable with base_url and api_key fields- Maintains existing tables (changes, electric) unchanged
The type-safe approach prevents schema drift between validation and storage layers.
No description provided.