-
Notifications
You must be signed in to change notification settings - Fork 421
Support multi-enhanced-tab #1685
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Add a new enhanced_notes feature to support multiple enhanced/summary notes per session, migrating editor view to a discriminated EditorView type. This change creates a new TinyBase schema/table for enhanced_notes, new hooks (useEnhancedNotes, useCreateEnhancedNote, etc.), updates AI task configs to accept enhancedNoteId, and wires enhanced note IDs through UI components and state (tabs, editors, streaming, auto-enhance, header, and note input). The update enables creating, regenerating, and auto-generating multiple enhanced notes, converts markdown output to JSON content, and updates related indexes/relationships and tests to use the new EditorView shape.
Introduce support for enhanced_notes in the devtool seed data to allow seeding curated and random enhanced note entries associated with sessions and optional templates. This adds curated enhanced_notes JSON entries, schema and TypeScript validation, loader wiring, builders and a generator (createEnhancedNote + buildEnhancedNotesForSessions), and includes enhanced_notes in debug and random seed outputs so seeded environments can exercise the new note type.
Replace manual JavaScript truncation logic for note titles with CSS-based truncation using a truncating span and conditional max widths. This simplifies the component by removing getTruncatedTitle and displayTitle, relying on the existing title value and responsive max-w classes for active vs inactive tabs to ensure consistent UI behavior and less complex code. Truncate note titles in header for compact display Limit the rendered note title in session header tabs to a shorter, context-aware form so long titles don't overflow or clutter the UI. Implemented getTruncatedTitle to return a shortened title based on whether the tab is active (up to 30 chars) or inactive (show a shortened first word or first 10 chars), and use displayTitle in the header rendering. Extract TruncatedTitle into a reusable component Improve readability and reduce duplication by extracting the repetitive truncated title span into a new TruncatedTitle component. This centralizes the truncate/max-width logic (different max widths for active vs inactive) and replaces inline spans in HeaderTabEnhanced and the other header tab with the new component.
|
Warning Rate limit exceeded@yujonglee has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 57 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR converts session-scoped enhanced content to per-enhanced-note storage and updates editor tab state from string literals to discriminated-union objects ({ type: "...", ... }). It adds enhanced_notes schema, store indexes/relationships, CRUD hooks, seed data generators, and threads enhancedNoteId through editor, streaming, task, and header flows. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Header as HeaderComponent
participant Store
participant AI as AITask
participant Editor as EnhancedEditor
User->>Header: Click "Create enhance" / choose template
Header->>Store: useCreateEnhancedNote(sessionId, templateId)
Store-->>Header: enhancedNoteId
Header->>Store: set tab state -> {type:"enhanced", id: enhancedNoteId}
Header-->>User: new enhanced tab shown
User->>Header: Start generation
Header->>AI: start enhance task (sessionId, enhancedNoteId, templateId)
AI->>AI: transform args (includes enhancedNoteId)
AI-->>Store: write md2json result to enhanced_notes[enhancedNoteId].content
Store-->>Editor: notify update
Editor-->>User: render enhanced content
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
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 |
Allow workspace package @hypr/tiptap to pass when there are no test files by adding the --passWithNoTests flag to its test script. This prevents pnpm -r test from failing the whole run when some packages have no tests.
Also update zustand tabs tests to expect empty state objects instead of { editor: "raw" } after state changes. These assertions align tests with the updated behavior where tab state is represented as an empty object in these cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (3)
apps/desktop/src/hooks/useEnhancedNotes.ts (3)
5-5: Add explicit return type annotation.For better type safety and code clarity, add an explicit return type annotation to the hook.
-export function useCreateEnhancedNote() { +export function useCreateEnhancedNote(): (sessionId: string, templateId?: string) => string | null {
51-62: Consider adding explicit return type annotations.While TypeScript can infer the return type, explicitly annotating the return type improves code clarity and catches potential issues early.
-export function useDeleteEnhancedNote() { +export function useDeleteEnhancedNote(): (enhancedNoteId: string) => void { const store = main.UI.useStore(main.STORE_ID); return useCallback(Similarly for
useRenameEnhancedNoteat line 64:-export function useRenameEnhancedNote() { +export function useRenameEnhancedNote(): (enhancedNoteId: string, newTitle: string) => void {
87-114: Consider usinguseRowfor better performance.Making four separate
useCellcalls creates four separate subscriptions to the store. If any of these cells change, each subscription triggers independently. UsinguseRowwould be more efficient and create a single subscription.export function useEnhancedNote(enhancedNoteId: string) { const row = main.UI.useRow( "enhanced_notes", enhancedNoteId, main.STORE_ID, ); return { title: row?.title, content: row?.content, position: row?.position, templateId: row?.template_id, }; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
apps/desktop/src/components/main/body/sessions/floating/index.tsx(1 hunks)apps/desktop/src/components/main/body/sessions/index.tsx(1 hunks)apps/desktop/src/components/main/body/sessions/note-input/enhanced/editor.tsx(3 hunks)apps/desktop/src/components/main/body/sessions/note-input/enhanced/index.tsx(2 hunks)apps/desktop/src/components/main/body/sessions/note-input/enhanced/streaming.tsx(1 hunks)apps/desktop/src/components/main/body/sessions/note-input/header.tsx(6 hunks)apps/desktop/src/components/main/body/sessions/note-input/index.tsx(4 hunks)apps/desktop/src/components/main/body/sessions/shared.tsx(1 hunks)apps/desktop/src/devtool/seed/data/curated.json(1 hunks)apps/desktop/src/devtool/seed/data/loader.ts(5 hunks)apps/desktop/src/devtool/seed/data/schema.gen.json(2 hunks)apps/desktop/src/devtool/seed/data/schema.ts(2 hunks)apps/desktop/src/devtool/seed/shared/builders.ts(3 hunks)apps/desktop/src/devtool/seed/shared/enhanced-note.ts(1 hunks)apps/desktop/src/devtool/seed/shared/index.ts(1 hunks)apps/desktop/src/devtool/seed/versions/debug.ts(1 hunks)apps/desktop/src/devtool/seed/versions/random.ts(4 hunks)apps/desktop/src/hooks/useAutoEnhance.ts(2 hunks)apps/desktop/src/hooks/useEnhancedNotes.ts(1 hunks)apps/desktop/src/hooks/useRunBatch.ts(1 hunks)apps/desktop/src/store/tinybase/main.ts(4 hunks)apps/desktop/src/store/tinybase/schema-external.ts(4 hunks)apps/desktop/src/store/zustand/ai-task/task-configs/enhance-transform.ts(1 hunks)apps/desktop/src/store/zustand/ai-task/task-configs/index.ts(1 hunks)apps/desktop/src/store/zustand/tabs/basic.test.ts(1 hunks)apps/desktop/src/store/zustand/tabs/schema.ts(2 hunks)apps/desktop/src/store/zustand/tabs/state.test.ts(2 hunks)apps/desktop/src/store/zustand/tabs/test-utils.ts(0 hunks)
💤 Files with no reviewable changes (1)
- apps/desktop/src/store/zustand/tabs/test-utils.ts
🧰 Additional context used
🧬 Code graph analysis (12)
apps/desktop/src/components/main/body/sessions/note-input/enhanced/streaming.tsx (1)
apps/desktop/src/store/zustand/ai-task/task-configs/index.ts (1)
createTaskId(56-61)
apps/desktop/src/devtool/seed/shared/builders.ts (2)
apps/desktop/src/store/tinybase/schema-external.ts (1)
EnhancedNoteStorage(179-179)apps/desktop/src/devtool/seed/shared/enhanced-note.ts (1)
createEnhancedNote(8-31)
apps/desktop/src/store/zustand/tabs/state.test.ts (1)
apps/desktop/src/store/zustand/tabs/index.ts (1)
useTabs(29-38)
apps/desktop/src/components/main/body/sessions/note-input/enhanced/index.tsx (3)
apps/desktop/src/store/zustand/ai-task/task-configs/index.ts (1)
createTaskId(56-61)apps/desktop/src/components/main/body/sessions/note-input/enhanced/streaming.tsx (1)
StreamingView(15-34)apps/desktop/src/components/main/body/sessions/note-input/enhanced/editor.tsx (1)
EnhancedEditor(9-61)
apps/desktop/src/devtool/seed/shared/enhanced-note.ts (3)
apps/desktop/src/store/tinybase/schema-external.ts (1)
EnhancedNoteStorage(179-179)apps/desktop/src/utils/index.ts (1)
DEFAULT_USER_ID(7-7)packages/tiptap/src/shared/utils.ts (1)
md2json(76-78)
apps/desktop/src/components/main/body/sessions/note-input/index.tsx (2)
apps/desktop/src/components/main/body/sessions/note-input/enhanced/index.tsx (1)
Enhanced(10-33)apps/desktop/src/components/main/body/sessions/note-input/raw.tsx (1)
RawEditor(14-66)
apps/desktop/src/components/main/body/sessions/note-input/enhanced/editor.tsx (2)
packages/tiptap/src/editor/index.tsx (1)
JSONContent(16-16)packages/tiptap/src/shared/utils.ts (1)
EMPTY_TIPTAP_DOC(8-8)
apps/desktop/src/devtool/seed/versions/random.ts (2)
packages/db/src/schema.ts (1)
templates(253-264)apps/desktop/src/devtool/seed/shared/builders.ts (1)
buildEnhancedNotesForSessions(408-435)
apps/desktop/src/hooks/useAutoEnhance.ts (5)
apps/desktop/src/hooks/useLLMConnection.ts (1)
useLanguageModel(46-119)apps/desktop/src/hooks/useEnhancedNotes.ts (1)
useCreateEnhancedNote(5-49)apps/desktop/src/store/zustand/ai-task/task-configs/index.ts (1)
createTaskId(56-61)apps/desktop/src/hooks/useAITaskTask.ts (1)
useAITaskTask(39-105)packages/tiptap/src/shared/utils.ts (1)
md2json(76-78)
apps/desktop/src/devtool/seed/data/loader.ts (3)
apps/desktop/src/store/tinybase/main.ts (1)
Schemas(74-74)apps/desktop/src/utils/index.ts (1)
DEFAULT_USER_ID(7-7)packages/tiptap/src/shared/utils.ts (1)
md2json(76-78)
apps/desktop/src/hooks/useEnhancedNotes.ts (1)
apps/desktop/src/store/tinybase/main.ts (1)
Store(73-73)
apps/desktop/src/components/main/body/sessions/note-input/header.tsx (7)
apps/desktop/src/store/zustand/tabs/schema.ts (1)
EditorView(18-18)apps/desktop/src/hooks/useEnhancedNotes.ts (1)
useCreateEnhancedNote(5-49)apps/desktop/src/hooks/useLLMConnection.ts (1)
useLanguageModel(46-119)apps/desktop/src/store/zustand/ai-task/task-configs/index.ts (1)
createTaskId(56-61)apps/desktop/src/hooks/useAITaskTask.ts (1)
useAITaskTask(39-105)packages/tiptap/src/shared/utils.ts (1)
md2json(76-78)apps/desktop/src/contexts/listener.tsx (1)
useListener(37-49)
🪛 Biome (2.1.2)
apps/desktop/src/components/main/body/sessions/note-input/header.tsx
[error] 315-315: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
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). (1)
- GitHub Check: fmt
🔇 Additional comments (29)
apps/desktop/src/components/main/body/sessions/floating/index.tsx (1)
15-15: Change verified as correct.
useCurrentNoteTabreturnsEditorView(non-optional), so removing the optional chaining operator is safe and appropriate for the discriminated union refactoring. Other parts of the codebase also access.typedirectly on the same value without optional chaining.apps/desktop/src/hooks/useRunBatch.ts (1)
66-66: All uses of the new editor state structure are consistent and correct across the codebase.Verification confirms that the change at line 66 is part of a complete and successful refactoring:
- updateSessionTabState calls: All 3 call sites use the new discriminated union structure (
{ type: "..." }format)- Editor property accesses: All consumers correctly treat
editoras an optionalEditorViewobject and access.typeappropriately- Type definitions: Schema correctly defines
EditorViewas the discriminated union, and all call sites align- Test coverage: Both positive tests and the intentional error test (using
as any) are presentapps/desktop/src/components/main/body/sessions/shared.tsx (2)
31-36: LGTM!The switch from
hasTranscripttoenhancedNoteIdscorrectly aligns with the per-note storage model. The use of optional chaining and array indexing safely handles the case when no enhanced notes exist.
38-52: Well-structured fallback logic!The priority order is correct: explicit editor state → active listener → first enhanced note → raw default. The discriminated union objects improve type safety and the memo dependencies are properly updated.
apps/desktop/src/store/zustand/tabs/schema.ts (2)
10-17: Excellent type safety improvement!The migration from string enum to discriminated union is a solid design choice. The
enhancedvariant correctly includes anidfield to reference specific enhanced notes, which aligns perfectly with the per-note storage model.
20-27: LGTM!The type guard functions are well-implemented and follow TypeScript best practices. They will enable proper type narrowing when working with
EditorViewunions throughout the codebase.apps/desktop/src/devtool/seed/versions/debug.ts (1)
278-278: LGTM!The empty
enhanced_notestable initialization is consistent with other debug seed data patterns and correctly satisfies the type constraint.apps/desktop/src/devtool/seed/shared/index.ts (1)
6-6: LGTM!The export follows the established pattern and properly exposes the
createEnhancedNotehelper for seed data generation.apps/desktop/src/store/zustand/tabs/basic.test.ts (1)
23-23: LGTM!The test correctly updates to use the new discriminated union shape with
{ type: "enhanced", id: "note-1" }. The hardcoded ID is appropriate for test data.apps/desktop/src/components/main/body/sessions/index.tsx (1)
75-75: LGTM!The update correctly adapts to the discriminated union by accessing the
typefield and adds proper null safety with optional chaining.apps/desktop/src/store/tinybase/schema-external.ts (2)
119-127: Well-structured schema!The
enhancedNoteSchemacorrectly defines all necessary fields for per-note storage. Thepositionfield enables proper ordering, and optional fields (template_id,title) are properly handled withz.preprocess. The schema follows established patterns in the file.
299-307: LGTM!The TinyBase table schema correctly maps all fields from
enhancedNoteSchemawith appropriate type constraints. Thesatisfiesclause ensures type safety and consistency.apps/desktop/src/devtool/seed/data/schema.ts (2)
95-101: LGTM!The
CuratedEnhancedNoteSchemacorrectly adapts the storage schema for human-readable seed data, using string references (session,template) instead of IDs. This follows the established pattern for curated schemas.
115-115: LGTM!The addition of
enhanced_notesto theCuratedDataSchemacorrectly integrates the new enhanced notes collection into the seed data structure.apps/desktop/src/store/zustand/ai-task/task-configs/index.ts (1)
14-14: LGTM! Clean type extension for per-note storage.The addition of
enhancedNoteIdto bothTaskArgsMap.enhanceandTaskArgsMapTransformed.enhancecorrectly propagates the new per-note identifier through the task configuration system, aligning with the PR's objective to move from session-based to per-note enhanced note storage.Also applies to: 21-21
apps/desktop/src/store/zustand/tabs/state.test.ts (2)
21-36: LGTM! Tests correctly updated for discriminated union editor state.The test updates properly reflect the migration from string literal editor states (
"enhanced") to discriminated union objects ({ type: "enhanced", id: "note-1" }). This ensures type safety and allows tracking of specific enhanced note instances.Also applies to: 45-62
54-57: <!-- [scratchpad_start]
[task verify_empty_state_expectation]
Analyzing default session tab state[observations]
- createSessionTab implementation at test-utils.ts lines 17-25 shows:
- Default state is:
state: { ...overrides.state }- When no overrides provided, this results in
state: {}- However, search results showed:
- Line 77:
state: { editor: "raw" }- Line 113:
state: { editor: "raw" }
Both appear to be expectations for default created tabs[analysis]
- Discrepancy found: createSessionTab produces
state: {}by default- But test expectations at lines 77 and 113 expect
state: { editor: "raw" }- This suggests either:
- createSessionTab should default to { editor: "raw" }
- The test expectations at 77 and 113 are wrong
- Line 54-57 is testing a specific scenario where {} is correct
[pending]
- Get full test context for lines 39-62 (including the test name and full scenario)
- Understand what the test at lines 39-62 is testing
- Verify whether lines 77 and 113 use default tabs or explicit state overrides
[actions]
- Extract lines 39-80 from state.test.ts to see complete test case and line 77 context
apps/desktop/src/devtool/seed/data/curated.json (1)
2859-2881: LGTM! Well-structured seed data for enhanced notes.The enhanced notes seed data provides realistic examples with:
- Proper position sequencing (0, 1 for multiple notes per session)
- Mix of templated and non-templated notes
- Content that aligns with the session context
- Correct linkage to existing sessions
This seed data will be valuable for testing the new per-note enhanced notes feature.
apps/desktop/src/devtool/seed/versions/random.ts (2)
11-11: LGTM! Import properly added.The
buildEnhancedNotesForSessionsimport is correctly added and used in the seed generation flow.
52-52: LGTM! Enhanced notes properly integrated into random seed generation.The enhanced notes generation is well-integrated with reasonable parameters:
- 0-3 notes per session provides good variability for testing
- 30% template probability ensures mix of templated and non-templated notes
- Properly uses templateIds and sessionIds dependencies
- Correctly included in the returned data structure
Also applies to: 89-96, 114-114
apps/desktop/src/store/zustand/ai-task/task-configs/enhance-transform.ts (1)
45-45: LGTM! Clean propagation of enhancedNoteId.The
enhancedNoteIdis correctly destructured from the input arguments and passed through to the transformed result, maintaining the data flow for per-note enhanced note processing.Also applies to: 52-52
apps/desktop/src/hooks/useAutoEnhance.ts (4)
2-2: LGTM! Hook setup properly refactored for two-phase enhanced note flow.The addition of state management (
autoEnhancedNoteId), task tracking (startedTasksRef), and theuseCreateEnhancedNotehook correctly supports the new flow where an enhanced note is created first, then the enhancement task is started with that note's ID.Also applies to: 4-4, 12-12, 19-19, 31-40
42-54: LGTM! Proper content persistence with error handling.The
onSuccesshandler correctly:
- Validates required dependencies (text, autoEnhancedNoteId, store)
- Converts markdown to JSON format via
md2json- Persists to the enhanced_notes table using
setPartialRow- Includes try-catch for conversion errors with logging
56-76: LGTM! Clean two-phase initialization function.The
createAndStartEnhancefunction properly:
- Creates the enhanced note and obtains its ID
- Stores the ID in state
- Updates the tab editor to the new discriminated union format
{ type: "enhanced", id: enhancedNoteId }- Includes appropriate guard clauses
This separates note creation from task execution, enabling better state management.
78-90: LGTM! Effect correctly starts task with deduplication.The effect properly:
- Waits for
autoEnhancedNoteIdto become available- Uses
startedTasksRefto prevent duplicate task starts for the same note- Passes both
sessionIdandenhancedNoteIdto the task- Includes appropriate dependencies
The deduplication via Set ensures each enhanced note only triggers one task execution.
apps/desktop/src/devtool/seed/data/schema.gen.json (2)
418-419: LGTM! Schema correctly requires enhanced_notes.The addition of
enhanced_notesto the top-level required array ensures seed data must include this new property.
370-406: Schema is correct; title is intentionally optional.The verification confirms that the
titlefield should be optional. The TypeScriptenhancedNoteSchemaexplicitly defines it asz.string().optional(), and the schema.gen.json correctly reflects this by excludingtitlefrom the required array. The UI's fallback to "Summary" when title is missing further confirms this is intentional design. All seed data includes titles for completeness, but this doesn't contradict the schema requirement—the application is designed to handle missing titles gracefully.apps/desktop/src/devtool/seed/shared/enhanced-note.ts (1)
8-31: LGTM! Well-implemented seed data generator.The
createEnhancedNotefunction provides clean seed data generation with:
- Realistic faker-generated titles and content
- Proper markdown-to-JSON conversion via
md2json- All required fields populated correctly
- Flexible template support (optional parameter correctly matches schema)
Good use of faker for generating varied, realistic test data.
apps/desktop/src/hooks/useEnhancedNotes.ts (1)
79-85: LGTM!Correctly uses the
enhancedNotesBySessionindex for efficient querying of enhanced notes by session.
Fix hooks order violation in note-input header by moving the useListener hook call before the early return that checks editorTabs. Hooks must be invoked unconditionally in the same order on every render; declaring isBatchProcessing before inspecting editorTabs prevents React from throwing when editorTabs changes. This preserves the previous conditional return behavior while making the component hook usage safe.
Ensure title is only set when the template title is a string. The store.getCell call may return non-string values (number, boolean, undefined), so replace the loose truthy check with a strict typeof check to avoid assigning invalid types to title and keep the fallback "Summary" when the value isn't a string.
Replace an inefficient full-table iteration with a targeted index lookup when calculating the next position for a session's enhanced note. Instead of iterating all rows with store.forEachRow and incrementing an existingCount, use store.getSliceRowIds(main.INDEXES.enhancedNotesBySession, sessionId) and derive nextPosition from that array's length. This improves performance and leverages the existing enhancedNotesBySession index.
No description provided.