Migration 1008 #1546
Migration 1008 #1546
Conversation
📝 WalkthroughWalkthroughReplaces inline calendar header with a new CalendarStructure and refactors calendar day rendering into dedicated subcomponents; adds a FloatingChatButton popover and removes the right-panel chat layout; adds a Tinybase index for sessions without an event. Changes
Sequence Diagram(s)sequenceDiagram
actor U as User
participant MA as MainArea (Calendar Tab)
participant CS as CalendarStructure
participant Day as TabContentCalendarDay
participant Store as Tinybase Store
participant Ev as DayEvents
participant Ses as DaySessions
U->>CS: Click Prev/Today/Next
CS-->>MA: onPreviousMonth/onToday/onNextMonth
MA->>CS: Provide monthLabel, weekDays, startDayOfWeek, children
CS->>MA: Render weekday headers and 7x6 grid placeholders + children
loop For each visible day
CS->>Day: Render day cell (isCurrentMonth)
Day->>Store: Query eventsByDate / sessionByDateWithoutEvent
Store-->>Day: Return eventIds, sessionIds
Day->>Ev: Render event entries
Day->>Ses: Render session entries
end
sequenceDiagram
actor U as User
participant FCB as FloatingChatButton
participant Pop as Popover
U->>FCB: Click button
FCB->>Pop: set open = true
Pop-->>U: Show chat scaffold
note right of FCB: Image onError -> hide image, show "💬" fallback
U->>FCB: Click Close
FCB->>Pop: set open = false
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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: 2
🧹 Nitpick comments (1)
packages/ui/src/components/block/calendar-structure.tsx (1)
63-68: Fill trailing day placeholders for a complete 6×7 grid.We only render leading blanks plus the actual days, so months that end mid-week leave the remainder of the 42 slots empty. The last row collapses without borders, breaking the calendar grid layout. Please add trailing placeholder divs to ensure all 42 cells are present each month.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/desktop2/src/components/main/main-area.tsx(4 hunks)packages/ui/src/components/block/calendar-day.tsx(1 hunks)packages/ui/src/components/block/calendar-item.tsx(1 hunks)packages/ui/src/components/block/calendar-structure.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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:
packages/ui/src/components/block/calendar-item.tsxpackages/ui/src/components/block/calendar-day.tsxpackages/ui/src/components/block/calendar-structure.tsxapps/desktop2/src/components/main/main-area.tsx
🪛 Biome (2.1.2)
apps/desktop2/src/components/main/main-area.tsx
[error] 323-323: This hook is being called from a nested function, but all hooks must be called unconditionally from the top-level component.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
⏰ 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)
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/desktop2/src/components/main/main-area.tsx (1)
329-330: React hook order is broken by mappinguseRowcalls.Both
TabContentCalendarDay2andTabContentCalendarDay3callpersisted.UI.useRow(a React hook), but they are invoked inside.map()functions. When the number of events or sessions changes between renders, React's hook order shifts and state corruption occurs.Fetch the rows without hooks by using a single top-level hook that returns all required rows, or use a selector with
useMemoto ensure hooks remain at the top level in a consistent order.See the existing review comment from the previous commit that flagged this same issue.
🧹 Nitpick comments (1)
apps/desktop2/src/components/main/main-area.tsx (1)
335-343: Minimal component implementation.
TabContentCalendarDay2andTabContentCalendarDay3only render plain<div>elements with titles, with no styling, structure, or interaction.Consider whether these components should:
- Be styled to match the calendar design (borders, padding, truncation)
- Handle click interactions to open the event/session
- Display additional metadata (time, participants, etc.)
If this is intentional for this migration, document it in a TODO comment.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop2/src/components/main/main-area.tsx(4 hunks)apps/desktop2/src/tinybase/store/persisted.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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/tinybase/store/persisted.tsapps/desktop2/src/components/main/main-area.tsx
🧬 Code graph analysis (2)
apps/desktop2/src/tinybase/store/persisted.ts (1)
packages/utils/src/datetime.ts (1)
format(4-11)
apps/desktop2/src/components/main/main-area.tsx (1)
packages/ui/src/components/block/calendar-structure.tsx (1)
CalendarStructure(14-71)
⏰ 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 (1)
apps/desktop2/src/components/main/main-area.tsx (1)
321-325: Good use of the new index.The new
sessionByDateWithEventindex correctly retrieves sessions grouped by date. Assuming the hook order issue is resolved, this integration looks solid.Note: See the review comment on
persisted.tsabout the misleading index name—once renamed, update the reference here as well.
| .setIndexDefinition( | ||
| INDEXES.sessionByDateWithEvent, | ||
| "sessions", | ||
| (getCell) => { | ||
| if (getCell("event_id")) { | ||
| return ""; | ||
| } | ||
|
|
||
| const d = new Date(getCell("created_at")!); | ||
| return format(d, "yyyy-MM-dd"); | ||
| }, | ||
| "created_at", | ||
| (a, b) => a.localeCompare(b), | ||
| (a, b) => String(a).localeCompare(String(b)), | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
Verify null safety for created_at.
The index assumes created_at is always present (line 382 uses !), but the schema defines it as a string without explicit non-null validation. Ensure that sessions always have a created_at value, or add a null check.
Run the following script to check if sessions can have null/undefined created_at:
🏁 Script executed:
#!/bin/bash
# Check schema validation and usage of created_at field
ast-grep --pattern $'sessions: {
$$$
created_at: $SCHEMA
$$$
}'Length of output: 0
🏁 Script executed:
#!/bin/bash
# Locate schema definition for sessions table to check created_at nullability
rg -nP --type=ts "setSchema" -C5 .
rg -nP --type=ts "sessions\s*:" -C3 .Length of output: 8966
🏁 Script executed:
#!/bin/bash
# Show session schema seeding in seed.ts
sed -n '300,380p' apps/desktop2/src/tinybase/seed.ts
# Show Session type and createSessionStore in utils
sed -n '1,200p' packages/utils/src/stores/session.tsLength of output: 6102
Guard against missing created_at in session index
In apps/desktop2/src/tinybase/store/persisted.ts around line 382, the sessionByDateWithEvent index uses getCell("created_at")!. Add a null/undefined check (or default) before calling new Date(...) to prevent runtime errors when created_at is absent.
🤖 Prompt for AI Agents
In apps/desktop2/src/tinybase/store/persisted.ts around lines 374 to 388, the
index function for INDEXES.sessionByDateWithEvent uses getCell("created_at")!
which will throw if created_at is null/undefined; remove the non-null assertion
and guard the value before constructing a Date. Specifically, if
getCell("created_at") is missing, return a safe empty key (e.g. "") or a default
date string (e.g. new Date(0) formatted), otherwise parse the value and format
it; ensure you do not call new Date(...) on undefined and preserve the existing
return type.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/desktop2/src/components/main/main-area.tsx (1)
264-272: TabHeader props remain hardcoded placeholders.The placeholder props (
isEnhancing={false},onVisibilityChange={() => {}}, etc.) are still present. This was flagged in the previous review and remains unresolved.
🧹 Nitpick comments (1)
apps/desktop2/src/components/main/main-area.tsx (1)
449-460: Add interaction handlers to calendar items.Both components include
cursor-pointerstyling but lackonClickhandlers, suggesting incomplete implementation. Users expect these items to be clickable to open the associated event or session.Consider adding navigation handlers:
For
TabContentCalendarDayEvents:+import { useTabs } from "../../hooks/useTabs"; + function TabContentCalendarDayEvents({ eventId }: { eventId: string }) { const event = persisted.UI.useRow("events", eventId, persisted.STORE_ID); + const { openNew } = useTabs(); return ( - <div className="flex items-center space-x-1 px-0.5 py-0.5 cursor-pointer rounded hover:bg-neutral-200 transition-colors h-5"> + <div + className="flex items-center space-x-1 px-0.5 py-0.5 cursor-pointer rounded hover:bg-neutral-200 transition-colors h-5" + onClick={() => openNew({ type: "events", id: eventId, active: true })} + > <CalendarIcon className="w-2.5 h-2.5 text-neutral-500 flex-shrink-0" /> <div className="flex-1 text-xs text-neutral-800 truncate"> {event.title} </div> </div> ); }For
TabContentCalendarDaySessions:function TabContentCalendarDaySessions({ sessionId }: { sessionId: string }) { const session = persisted.UI.useRow("sessions", sessionId, persisted.STORE_ID); + const { openNew } = useTabs(); + return ( - <div className="flex items-center space-x-1 px-0.5 py-0.5 cursor-pointer rounded hover:bg-neutral-200 transition-colors h-5"> + <div + className="flex items-center space-x-1 px-0.5 py-0.5 cursor-pointer rounded hover:bg-neutral-200 transition-colors h-5" + onClick={() => openNew({ type: "sessions", id: sessionId, active: true })} + > <NotebookIcon className="w-2.5 h-2.5 text-neutral-500 flex-shrink-0" /> <div className="flex-1 text-xs text-neutral-800 truncate"> {session.title} </div> </div> ); }Also applies to: 462-472
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/desktop2/public/assets/dynamic.gifis excluded by!**/*.gif
📒 Files selected for processing (3)
apps/desktop2/src/components/floating-chat-button.tsx(1 hunks)apps/desktop2/src/components/main/main-area.tsx(5 hunks)apps/desktop2/src/routes/app/main/_layout.index.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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/floating-chat-button.tsxapps/desktop2/src/routes/app/main/_layout.index.tsxapps/desktop2/src/components/main/main-area.tsx
🧬 Code graph analysis (3)
apps/desktop2/src/components/floating-chat-button.tsx (1)
packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/desktop2/src/routes/app/main/_layout.index.tsx (3)
apps/desktop2/src/components/main/left-sidebar.tsx (1)
LeftSidebar(13-36)apps/desktop2/src/components/main/main-area.tsx (1)
MainHeader(28-83)apps/desktop2/src/components/floating-chat-button.tsx (1)
FloatingChatButton(5-72)
apps/desktop2/src/components/main/main-area.tsx (1)
packages/ui/src/components/block/calendar-structure.tsx (1)
CalendarStructure(14-71)
⏰ 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 (8)
apps/desktop2/src/components/main/main-area.tsx (3)
3-4: LGTM!The new imports are properly used in the calendar refactoring and component additions.
Also applies to: 9-10, 12-12
359-387: LGTM!The CalendarStructure integration is clean and correctly delegates header, navigation, and grid layout to the shared component while maintaining proper month navigation and day rendering.
397-401: Index definition confirmed:sessionByDateWithEventis defined inapps/desktop2/src/tinybase/store/persisted.ts.apps/desktop2/src/components/floating-chat-button.tsx (3)
1-3: LGTM!The imports are appropriate and all are utilized in the component.
5-6: LGTM!The component definition and state management follow standard React patterns.
37-69: LGTM!The popover content structure is well-organized with appropriate placeholder UI elements. The close button correctly manages the open state, and the disabled input clearly indicates this is a work-in-progress implementation.
apps/desktop2/src/routes/app/main/_layout.index.tsx (2)
5-6: LGTM!The import changes correctly reflect the layout refactoring: removing the right panel context and adding the new FloatingChatButton component.
80-99: LGTM!The layout refactoring successfully simplifies the component structure by removing the right panel logic and introducing the FloatingChatButton. Rendering the floating button outside the ResizablePanelGroup is appropriate for its fixed positioning, and the left sidebar conditional rendering is preserved correctly.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
apps/desktop2/src/store/tinybase/persisted.ts (2)
374-388: Previous concerns remain unaddressed.Two issues flagged in earlier reviews have not been resolved:
Null safety (line 382): The non-null assertion
getCell("created_at")!assumescreated_atis always present. While the Zod schema defines it as required, there's no runtime guarantee at the TinyBase level. Ifcreated_atis missing,new Date(undefined)will create an Invalid Date, andformat()may throw or return unexpected results.Misleading name: The index returns an empty string when
event_idis present, effectively grouping sessions without events by date. The namesessionByDateWithEventcontradicts this behavior.Recommended fixes:
- Guard against missing
created_at:(getCell) => { if (getCell("event_id")) { return ""; } - const d = new Date(getCell("created_at")!); - return format(d, "yyyy-MM-dd"); + const createdAt = getCell("created_at"); + if (!createdAt) { + return ""; + } + return format(new Date(createdAt), "yyyy-MM-dd"); },
- Rename the index to reflect its actual behavior (e.g.,
sessionByDateWithoutEventorunlinkedSessionsByDate) and update references inmain-area.tsx.
430-430: Index name remains misleading.As noted in previous reviews,
sessionByDateWithEventshould be renamed to accurately reflect that it groups sessions without events (e.g.,sessionByDateWithoutEventorunlinkedSessionsByDate). Remember to update all references in consuming files.apps/desktop2/src/components/main/main-area.tsx (1)
275-283: TabHeader placeholder props still present.This was previously flagged in the past review. All props remain hardcoded placeholders (isEnhancing, onVisibilityChange, onTabChange, etc.).
🧹 Nitpick comments (1)
apps/desktop2/src/store/tinybase/persisted.ts (1)
1-1: Consider using the custom format wrapper for consistent timezone handling.The import uses
date-fnsdirectly, but the codebase provides a customformatwrapper inpackages/utils/src/datetime.tsthat adds first-class timezone support via theinoption. Using the custom wrapper ensures consistent timezone handling across the application.Apply this diff to use the custom format function:
-import { format } from "date-fns"; +import { format } from "@hypr/utils/datetime";Note: Verify that
@hypr/utilsis the correct package alias by checking the import paths used elsewhere in the codebase.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/desktop2/src/components/main/main-area.tsx(4 hunks)apps/desktop2/src/routes/app/main/_layout.index.tsx(2 hunks)apps/desktop2/src/store/tinybase/persisted.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop2/src/routes/app/main/_layout.index.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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/store/tinybase/persisted.tsapps/desktop2/src/components/main/main-area.tsx
🧬 Code graph analysis (2)
apps/desktop2/src/store/tinybase/persisted.ts (1)
packages/utils/src/datetime.ts (1)
format(4-11)
apps/desktop2/src/components/main/main-area.tsx (1)
packages/ui/src/components/block/calendar-structure.tsx (1)
CalendarStructure(14-71)
⏰ 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 (macos, macos-14)
- GitHub Check: ci (windows, windows-latest)
🔇 Additional comments (4)
apps/desktop2/src/components/main/main-area.tsx (4)
1-13: LGTM: Imports are clean and used.All new imports (date-fns utilities, CalendarStructure, TabHeader, FileTextIcon) are properly utilized in the refactored calendar rendering.
386-397: LGTM: CalendarStructure integration is correct.The calendar structure is properly wired with month navigation (previous/next/today), and the hooks-in-map issue from the previous review has been fixed by extracting rendering into separate component functions (TabContentCalendarDayEvents, TabContentCalendarDaySessions) where hooks are called at the top level—this is the correct React pattern.
452-453: LGTM: Component extraction pattern is correct.Mapping over IDs to render component instances (where each component calls hooks at its top level) is the proper React pattern and resolves the hooks-in-map issue from the previous review.
464-464: Add click handlers to calendar entries
TabContentCalendarDayEvents(line 464) andTabContentCalendarDaySessions(line 476) render divs withcursor-pointerbut noonClick. Implement click handlers to open the corresponding event/session (e.g. via your tabs API) or replace these divs with<button>for proper semantics.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/desktop2/src/components/main/main-area.tsx (1)
476-479: Consider reusing the Date object.The
daystring is converted to a Date object three times (lines 476, 478, and 457). Extract it to a variable to avoid redundant parsing.Apply this diff:
+ const dayDate = new Date(day); - const dayNumber = format(new Date(day), "d"); + const dayNumber = format(dayDate, "d"); const isToday = format(new Date(), "yyyy-MM-dd") === day; - const dayOfWeek = getDay(new Date(day)); + const dayOfWeek = getDay(dayDate); const isWeekend = dayOfWeek === 0 || dayOfWeek === 6;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop2/src/components/main/main-area.tsx(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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/main/main-area.tsx
🧬 Code graph analysis (1)
apps/desktop2/src/components/main/main-area.tsx (1)
packages/ui/src/components/block/calendar-structure.tsx (1)
CalendarStructure(14-71)
⏰ 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 (macos, macos-14)
- GitHub Check: ci (windows, windows-latest)
🔇 Additional comments (3)
apps/desktop2/src/components/main/main-area.tsx (3)
448-460: LGTM! CalendarStructure integration is clean.The CalendarStructure component is correctly wired with all required props (monthLabel, weekDays, startDayOfWeek, navigation handlers) and the day children are properly mapped with the
isCurrentMonthprop derived fromisSameMonth.
522-533: LGTM! Clean event rendering component.The
TabContentCalendarDayEventscomponent correctly fetches event data and renders it with appropriate styling and truncation.
535-545: LGTM! Clean session rendering component.The
TabContentCalendarDaySessionscomponent correctly fetches session data and maintains consistency with the event component structure.
Don't merge this yet