feat: add hierarchical prompt tree relationships#173
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Code Review by Qodo
1. Inline style in PromptListView
|
📝 WalkthroughWalkthrough为桌面端 Prompt 引入父子层级关系:在 SQLite ChangesPrompt 层级关系树
Sequence Diagram(s)sequenceDiagram
participant PromptListView
participant usePromptStore
participant database as database.ts
participant ipcRenderer
participant MainProcess as Main Process
participant PromptDB
rect rgba(99, 132, 255, 0.5)
note over PromptListView,PromptDB: 拖拽 / Tab 键盘触发移动
PromptListView->>usePromptStore: movePrompt(id, newParentId, newOrder)
usePromptStore->>database: db.movePrompt(id, newParentId, newOrder)
end
alt window.api.prompt.move 可用(Electron 主进程)
database->>ipcRenderer: invoke("prompt:move", id, newParentId, newOrder)
ipcRenderer->>MainProcess: assertPromptMoveInput → db.movePrompt
MainProcess->>PromptDB: 事务:循环检测 + 兄弟顺序重写
PromptDB-->>MainProcess: commit
MainProcess->>MainProcess: syncWorkspace()
MainProcess-->>database: true
else IndexedDB 回退
database->>database: assertPromptMoveAllowed(循环检测)
database->>database: reorderPromptTree + IDBTransaction 批量 put
end
database-->>usePromptStore: void
usePromptStore->>usePromptStore: fetchPrompts() + scheduleAllSaveSync("prompt:move")
usePromptStore-->>PromptListView: prompts 状态刷新
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
PR Summary by Qodofeat: add hierarchical prompt tree relationships WalkthroughsDescription• Add durable prompt hierarchy fields (parentId/order) with SQLite migration and IPC support. • Replace list mode with a single hierarchical tree view supporting drag/drop and Tab indentation. • Harden move/delete semantics (no cascade delete, no cycles, contiguous sibling ordering) with tests. Diagramgraph TD
A["PromptListView"] --> B["Prompt store"] --> C["renderer movePrompt"]
C --> D["IPC prompt:move"] --> E[("SQLite PromptDB")]
C --> F[("IndexedDB prompts")]
High-Level AssessmentThe following are alternative approaches to this PR: 1. Introduce a dedicated prompt_relations table (typed edges)
2. Store order as a fractional/rank key (no full sibling rewrite)
Recommendation: The PR’s approach is appropriate for a V1: durable parent_id/sort_order in SQLite (with ON DELETE SET NULL) keeps hierarchy consistent across sessions and enables simple tree rendering and direct manipulation UX. If/when non-tree relationships ship, consider adding prompt_relations and treating parent_id/sort_order as the projection for grouped_under (with a planned migration path). File ChangesEnhancement (10)
Bug fix (1)
Tests (2)
Documentation (5)
|
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
| : '' | ||
| } | ||
| `} | ||
| style={{ paddingLeft: `${depth * 16 + 12}px` }} |
There was a problem hiding this comment.
1. Inline style in promptlistview 📘 Rule violation ⚙ Maintainability
8.6
PromptListView uses an inline style={{ paddingLeft: ... }} to render hierarchy indentation. This
violates the Tailwind-only styling rule and can lead to inconsistent theming and styling drift.
Agent Prompt
## Issue description
`PromptListView` uses inline `style` for indentation (`paddingLeft`), but UI styling must be Tailwind-only.
## Issue Context
Indentation is currently calculated from `depth` and applied via inline style. Replace it with a Tailwind-only approach (e.g., render `depth` spacer elements with fixed Tailwind widths like `w-4` / `shrink-0`).
## Fix Focus Areas
- apps/desktop/src/renderer/components/prompt/PromptListView.tsx[370-409]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| return date.toLocaleTimeString(undefined, { | ||
| hour: '2-digit', | ||
| minute: '2-digit', | ||
| }); | ||
| } | ||
|
|
||
| if (diffDays === 1) { | ||
| return t('common.yesterday') || '昨天'; | ||
| } else if (diffDays < 7) { | ||
| } | ||
|
|
||
| if (diffDays < 7) { | ||
| return `${diffDays}${t('common.daysAgo') || '天前'}`; | ||
| } else { | ||
| return date.toLocaleDateString(undefined, { month: 'short', day: 'numeric' }); | ||
| } | ||
|
|
||
| return date.toLocaleDateString(undefined, { | ||
| month: 'short', | ||
| day: 'numeric', | ||
| }); | ||
| }; | ||
|
|
||
| return ( | ||
| <div className="flex flex-col"> | ||
| {prompts.map((prompt) => ( | ||
| <div | ||
| key={prompt.id} | ||
| onClick={() => onSelect(prompt.id)} | ||
| onContextMenu={(e) => onContextMenu(e, prompt)} | ||
| className={` | ||
| flex items-center gap-3 px-3 py-2.5 border-b border-border/50 cursor-pointer | ||
| transition-colors duration-quick | ||
| ${selectedId === prompt.id | ||
| ? 'bg-primary/10 border-l-2 border-l-primary' | ||
| : 'hover:bg-accent/50' | ||
| } | ||
| `} | ||
| > | ||
| {/* Title and description */} | ||
| {/* 标题和描述 */} | ||
| <div className="flex-1 min-w-0"> | ||
| <div className="flex items-center gap-2"> | ||
| <h3 | ||
| className={`font-medium text-sm leading-snug break-words line-clamp-2 ${ | ||
| selectedId === prompt.id ? 'text-primary' : 'text-foreground' | ||
| }`} | ||
| title={prompt.title} | ||
| > | ||
| {prompt.title} | ||
| </h3> | ||
| {prompt.isFavorite && ( | ||
| <StarIcon className="w-3 h-3 flex-shrink-0 fill-yellow-400 text-yellow-400" /> | ||
| const toggleExpand = useCallback((promptId: string) => { | ||
| setExpandedIds((current) => { | ||
| const next = new Set(current); | ||
| if (next.has(promptId)) { | ||
| next.delete(promptId); | ||
| } else { | ||
| next.add(promptId); | ||
| } | ||
| return next; | ||
| }); | ||
| }, []); | ||
|
|
||
| const handleDragStart = useCallback( | ||
| (event: ReactDragEvent, promptId: string) => { | ||
| setDraggingId(promptId); | ||
| event.dataTransfer.effectAllowed = 'move'; | ||
| event.dataTransfer.setData('text/plain', promptId); | ||
| }, | ||
| [], | ||
| ); | ||
|
|
||
| const handleDragEnd = useCallback(() => { | ||
| setDraggingId(null); | ||
| setDropTargetId(null); | ||
| setDropPosition(null); | ||
| }, []); | ||
|
|
||
| const updateDropTarget = useCallback( | ||
| (event: ReactDragEvent<HTMLElement>, targetPrompt: Prompt) => { | ||
| event.preventDefault(); | ||
| event.dataTransfer.dropEffect = 'move'; | ||
|
|
||
| if (!draggingId || draggingId === targetPrompt.id) { | ||
| setDropTargetId(null); | ||
| setDropPosition(null); | ||
| return; | ||
| } | ||
|
|
||
| const nextDropPosition = getDropPosition(event); | ||
| const nextParentId = | ||
| nextDropPosition === 'inside' | ||
| ? targetPrompt.id | ||
| : getVisibleParentId(targetPrompt); | ||
|
|
||
| if (!canMoveToParent(draggingId, nextParentId)) { | ||
| setDropTargetId(null); | ||
| setDropPosition(null); | ||
| return; | ||
| } | ||
|
|
||
| setDropTargetId(targetPrompt.id); | ||
| setDropPosition(nextDropPosition); | ||
| }, | ||
| [canMoveToParent, draggingId, getVisibleParentId], | ||
| ); | ||
|
|
||
| const handleDragLeave = useCallback((event: ReactDragEvent<HTMLElement>) => { | ||
| const nextTarget = event.relatedTarget; | ||
| if (nextTarget instanceof Node && event.currentTarget.contains(nextTarget)) { | ||
| return; | ||
| } | ||
|
|
||
| setDropTargetId(null); | ||
| setDropPosition(null); | ||
| }, []); | ||
|
|
||
| const handleDrop = useCallback( | ||
| (event: ReactDragEvent, targetPrompt: Prompt) => { | ||
| event.preventDefault(); | ||
|
|
||
| if (!draggingId || draggingId === targetPrompt.id || !dropPosition) { | ||
| handleDragEnd(); | ||
| return; | ||
| } | ||
|
|
||
| if (dropPosition === 'inside') { | ||
| if (canMoveToParent(draggingId, targetPrompt.id)) { | ||
| setExpandedIds((current) => new Set(current).add(targetPrompt.id)); | ||
| onMovePrompt(draggingId, targetPrompt.id, getChildren(targetPrompt.id).length); | ||
| } | ||
| handleDragEnd(); | ||
| return; | ||
| } | ||
|
|
||
| const nextParentId = getVisibleParentId(targetPrompt); | ||
| if (!canMoveToParent(draggingId, nextParentId)) { | ||
| handleDragEnd(); | ||
| return; | ||
| } | ||
|
|
||
| const targetSiblings = getChildren(nextParentId).filter( | ||
| (prompt) => prompt.id !== draggingId, | ||
| ); | ||
| const targetIndex = targetSiblings.findIndex( | ||
| (prompt) => prompt.id === targetPrompt.id, | ||
| ); | ||
| const nextOrder = | ||
| targetIndex < 0 | ||
| ? targetSiblings.length | ||
| : targetIndex + (dropPosition === 'after' ? 1 : 0); | ||
|
|
||
| onMovePrompt(draggingId, nextParentId, nextOrder); | ||
| handleDragEnd(); | ||
| }, | ||
| [ | ||
| canMoveToParent, | ||
| draggingId, | ||
| dropPosition, | ||
| getChildren, | ||
| getVisibleParentId, | ||
| handleDragEnd, | ||
| onMovePrompt, | ||
| ], | ||
| ); | ||
|
|
||
| const isSelected = useCallback( | ||
| (promptId: string) => selectedId === promptId || selectedIds.includes(promptId), | ||
| [selectedId, selectedIds], | ||
| ); | ||
|
|
||
| const isDragging = useCallback( | ||
| (promptId: string) => draggingId === promptId, | ||
| [draggingId], | ||
| ); | ||
|
|
||
| const isDropTarget = useCallback( | ||
| (promptId: string) => dropTargetId === promptId, | ||
| [dropTargetId], | ||
| ); | ||
|
|
||
| const renderTreeNode = useCallback( | ||
| (prompt: Prompt, depth: number, ancestors: Set<string>) => { | ||
| const nextAncestors = new Set(ancestors).add(prompt.id); | ||
| const children = getChildren(prompt.id).filter( | ||
| (child) => !nextAncestors.has(child.id), | ||
| ); | ||
| const hasKids = children.length > 0; | ||
| const isExpanded = expandedIds.has(prompt.id); | ||
|
|
||
| return ( | ||
| <div key={prompt.id}> | ||
| <div | ||
| draggable | ||
| onDragStart={(event) => handleDragStart(event, prompt.id)} | ||
| onDragEnd={handleDragEnd} | ||
| onDragOver={(event) => updateDropTarget(event, prompt)} | ||
| onDragEnter={(event) => updateDropTarget(event, prompt)} | ||
| onDragLeave={handleDragLeave} | ||
| onDrop={(event) => handleDrop(event, prompt)} | ||
| onClick={() => onSelect(prompt.id)} | ||
| onContextMenu={(event) => onContextMenu(event, prompt)} | ||
| className={` | ||
| flex items-center gap-3 px-3 py-2.5 border-b border-border/50 cursor-pointer | ||
| transition-colors duration-quick relative | ||
| ${ | ||
| isSelected(prompt.id) | ||
| ? 'bg-primary/10 border-l-2 border-l-primary' | ||
| : isDropTarget(prompt.id) && dropPosition === 'inside' | ||
| ? 'bg-primary/20 border-l-2 border-l-primary' | ||
| : 'hover:bg-accent/50' | ||
| } | ||
| ${isDragging(prompt.id) ? 'opacity-50' : ''} | ||
| ${ | ||
| isDropTarget(prompt.id) && dropPosition === 'inside' | ||
| ? 'ring-2 ring-primary/50 ring-inset' | ||
| : '' | ||
| } | ||
| ${ | ||
| isDropTarget(prompt.id) && dropPosition === 'before' | ||
| ? 'border-t-2 border-t-primary' | ||
| : '' | ||
| } | ||
| ${ | ||
| isDropTarget(prompt.id) && dropPosition === 'after' | ||
| ? 'border-b-2 border-b-primary' | ||
| : '' | ||
| } | ||
| `} | ||
| style={{ paddingLeft: `${depth * 16 + 12}px` }} | ||
| > | ||
| <div className="flex items-center gap-1"> | ||
| {hasKids ? ( | ||
| <button | ||
| type="button" | ||
| onClick={(event) => { | ||
| event.stopPropagation(); | ||
| toggleExpand(prompt.id); | ||
| }} | ||
| className="p-0.5 rounded hover:bg-accent transition-colors" | ||
| aria-label={isExpanded ? 'Collapse prompt' : 'Expand prompt'} | ||
| > | ||
| {isExpanded ? ( | ||
| <ChevronDownIcon className="w-4 h-4 text-muted-foreground" /> | ||
| ) : ( | ||
| <ChevronRightIcon className="w-4 h-4 text-muted-foreground" /> | ||
| )} | ||
| </button> | ||
| ) : ( | ||
| <span className="w-5" /> | ||
| )} | ||
| <GripVerticalIcon className="w-4 h-4 text-muted-foreground cursor-grab opacity-0 hover:opacity-100 transition-opacity" /> | ||
| </div> | ||
| {prompt.description && ( | ||
| <p className="text-xs text-muted-foreground line-clamp-2 break-words mt-0.5"> | ||
| {prompt.description} | ||
| </p> | ||
| )} | ||
| {prompt.images && prompt.images.length > 0 && ( | ||
| <div className="flex items-center gap-1 mt-0.5"> | ||
| <ImageIcon className="w-3 h-3 text-muted-foreground" /> | ||
| <span className="text-xs text-muted-foreground">{prompt.images.length}</span> | ||
|
|
||
| <div className="flex-1 min-w-0"> | ||
| <div className="flex items-center gap-2"> | ||
| <h3 | ||
| className={`font-medium text-sm leading-snug break-words line-clamp-2 ${ | ||
| isSelected(prompt.id) ? 'text-primary' : 'text-foreground' | ||
| }`} | ||
| title={prompt.title} | ||
| > | ||
| {prompt.title} | ||
| </h3> | ||
| {prompt.isFavorite && ( | ||
| <StarIcon className="w-3 h-3 flex-shrink-0 fill-yellow-400 text-yellow-400" /> | ||
| )} | ||
| </div> | ||
| )} | ||
| </div> | ||
| {prompt.description && ( | ||
| <p className="text-xs text-muted-foreground line-clamp-2 break-words mt-0.5"> | ||
| {prompt.description} | ||
| </p> | ||
| )} | ||
| {prompt.images && prompt.images.length > 0 && ( | ||
| <div className="flex items-center gap-1 mt-0.5"> | ||
| <ImageIcon className="w-3 h-3 text-muted-foreground" /> | ||
| <span className="text-xs text-muted-foreground"> | ||
| {prompt.images.length} | ||
| </span> | ||
| </div> | ||
| )} | ||
| </div> | ||
|
|
||
| {/* Usage count */} | ||
| {/* 使用次数 */} | ||
| <div className="flex-shrink-0 w-12 text-center"> | ||
| <span className="text-xs text-muted-foreground"> | ||
| {prompt.usageCount || 0} | ||
| </span> | ||
| </div> | ||
| <div className="flex-shrink-0 w-12 text-center"> | ||
| <span className="text-xs text-muted-foreground"> | ||
| {prompt.usageCount || 0} | ||
| </span> | ||
| </div> | ||
|
|
||
| {/* Update time */} | ||
| {/* 更新时间 */} | ||
| <div className="flex-shrink-0 w-16 text-right"> | ||
| <span className="text-xs text-muted-foreground"> | ||
| {formatDate(prompt.updatedAt)} | ||
| </span> | ||
| </div> | ||
| <div className="flex-shrink-0 w-16 text-right"> | ||
| <span className="text-xs text-muted-foreground"> | ||
| {formatDate(prompt.updatedAt)} | ||
| </span> | ||
| </div> | ||
|
|
||
| {/* Action buttons */} | ||
| {/* 操作按钮 */} | ||
| <div className="flex items-center gap-1 flex-shrink-0"> | ||
| <button | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| onCopy(prompt); | ||
| }} | ||
| className="p-1.5 rounded-md text-muted-foreground hover:text-foreground hover:bg-accent transition-colors" | ||
| title={t('prompt.copy')} | ||
| > | ||
| <CopyIcon className="w-3.5 h-3.5" /> | ||
| </button> | ||
| <button | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| onToggleFavorite(prompt.id); | ||
| }} | ||
| className={`p-1.5 rounded-md transition-colors ${prompt.isFavorite | ||
| ? 'text-yellow-500 hover:bg-yellow-500/10' | ||
| : 'text-muted-foreground hover:text-foreground hover:bg-accent' | ||
| <div className="flex items-center gap-1 flex-shrink-0"> | ||
| <button | ||
| type="button" | ||
| onClick={(event) => { | ||
| event.stopPropagation(); | ||
| onCopy(prompt); | ||
| }} | ||
| className="p-1.5 rounded-md text-muted-foreground hover:text-foreground hover:bg-accent transition-colors" | ||
| title={t('prompt.copy')} | ||
| > | ||
| <CopyIcon className="w-3.5 h-3.5" /> | ||
| </button> | ||
| <button | ||
| type="button" | ||
| onClick={(event) => { | ||
| event.stopPropagation(); | ||
| onToggleFavorite(prompt.id); | ||
| }} | ||
| className={`p-1.5 rounded-md transition-colors ${ | ||
| prompt.isFavorite | ||
| ? 'text-yellow-500 hover:bg-yellow-500/10' | ||
| : 'text-muted-foreground hover:text-foreground hover:bg-accent' | ||
| }`} | ||
| title={prompt.isFavorite ? t('nav.favorites') : t('prompt.addToFavorites') || '添加收藏'} | ||
| > | ||
| <StarIcon className={`w-3.5 h-3.5 ${prompt.isFavorite ? 'fill-current' : ''}`} /> | ||
| </button> | ||
| title={ | ||
| prompt.isFavorite | ||
| ? t('nav.favorites') | ||
| : t('prompt.addToFavorites') || '添加收藏' | ||
| } |
There was a problem hiding this comment.
2. Hardcoded strings in promptlistview 📘 Rule violation ⚙ Maintainability
8.3
PromptListView includes hardcoded Chinese fallbacks (昨天, 天前, 添加收藏) and hardcoded English ARIA labels (Collapse prompt/Expand prompt). This violates i18n requirements and prevents full localization across supported locales.
Agent Prompt
## Issue description
User-facing strings and Chinese literals are hardcoded in `PromptListView` instead of using `t()` keys.
## Issue Context
- `formatDate()` returns Chinese fallback strings (`昨天`, `天前`).
- Favorite button title falls back to `添加收藏`.
- Expand/collapse button uses hardcoded English ARIA labels.
All user-facing text must be localized via i18next and new keys must be added to all locale JSON files.
## Fix Focus Areas
- apps/desktop/src/renderer/components/prompt/PromptListView.tsx[202-227]
- apps/desktop/src/renderer/components/prompt/PromptListView.tsx[411-425]
- apps/desktop/src/renderer/components/prompt/PromptListView.tsx[496-507]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| getAllRequest.onsuccess = () => { | ||
| const prompts = getAllRequest.result as Prompt[]; | ||
| const prompt = prompts.find((item) => item.id === promptId); |
There was a problem hiding this comment.
3. Uncommented as type assertions 📘 Rule violation ⚙ Maintainability
8.1
New code introduces as type assertions without justification comments, reducing type-safety and potentially masking runtime shape mismatches. This violates the TypeScript strictness rule for as usage.
Agent Prompt
## Issue description
Uncommented `as` assertions were added in production and test code.
## Issue Context
The compliance rule disallows `as` assertions unless truly necessary for interop and accompanied by an explanatory comment.
## Fix Focus Areas
- apps/desktop/src/renderer/services/database.ts[379-382]
- apps/desktop/tests/unit/main/database-migration-locks.test.ts[169-175]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| move: (promptId: string, newParentId: string | null, newOrder: number) => | ||
| ipcRenderer.invoke(IPC_CHANNELS.PROMPT_MOVE, promptId, newParentId, newOrder), |
There was a problem hiding this comment.
4. Exported functions missing return types 📘 Rule violation ⚙ Maintainability
8.1
New exported API surfaces omit explicit return type annotations, making contract drift harder to detect and weakening strict typing guarantees. This violates the requirement that exported functions have explicit return types.
Agent Prompt
## Issue description
Exported functions must include explicit return type annotations.
## Issue Context
- `promptApi.move` is part of an exported API object but has no explicit return type.
- `PromptListView` is an exported function component without an explicit return type annotation.
## Fix Focus Areas
- apps/desktop/src/preload/api/prompt.ts[43-44]
- apps/desktop/src/renderer/components/prompt/PromptListView.tsx[59-69]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| useEffect(() => { | ||
| const handleKeyDown = (event: KeyboardEvent) => { | ||
| if (!selectedId || draggingId) return; | ||
|
|
||
| const selectedPrompt = promptById.get(selectedId); | ||
| if (!selectedPrompt) return; | ||
| if (event.key !== 'Tab' || event.ctrlKey || event.metaKey) return; | ||
|
|
||
| event.preventDefault(); | ||
|
|
||
| const currentParentId = getVisibleParentId(selectedPrompt); | ||
|
|
||
| if (event.shiftKey) { | ||
| if (!currentParentId) return; | ||
|
|
||
| const parentPrompt = promptById.get(currentParentId); | ||
| const grandParentId = parentPrompt | ||
| ? getVisibleParentId(parentPrompt) | ||
| : null; | ||
| const parentSiblings = getChildren(grandParentId); | ||
| const parentIndex = parentSiblings.findIndex( | ||
| (prompt) => prompt.id === currentParentId, | ||
| ); | ||
|
|
||
| onMovePrompt( | ||
| selectedId, | ||
| grandParentId, | ||
| parentIndex >= 0 ? parentIndex + 1 : parentSiblings.length, | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| const siblings = getChildren(currentParentId); | ||
| const currentIndex = siblings.findIndex( | ||
| (prompt) => prompt.id === selectedId, | ||
| ); | ||
| if (currentIndex <= 0) return; | ||
|
|
||
| const previousSibling = siblings[currentIndex - 1]; | ||
| setExpandedIds((current) => new Set(current).add(previousSibling.id)); | ||
| onMovePrompt(selectedId, previousSibling.id, getChildren(previousSibling.id).length); | ||
| }; | ||
|
|
||
| window.addEventListener('keydown', handleKeyDown); | ||
| return () => window.removeEventListener('keydown', handleKeyDown); | ||
| }, [ |
There was a problem hiding this comment.
6. Global tab key hijack 🐞 Bug ≡ Correctness
PromptListView installs a window-level keydown handler that preventDefault()s Tab whenever a prompt is selected, regardless of where focus is, so Tab stops working inside other UI (e.g., EditPromptModal inputs) while list view is mounted. This breaks keyboard navigation/accessibility and can make prompt editing flows difficult or impossible without the mouse.
Agent Prompt
### Issue description
`PromptListView` captures `Tab` at the `window` level and calls `event.preventDefault()` whenever there is a `selectedId`. This intercepts Tab presses even when the user is typing in an `<input>`/`<textarea>` inside modals rendered on top of the list (e.g. `EditPromptModal`), breaking expected focus traversal.
### Issue Context
The list view is mounted at the same time as shared modals in `MainContent`, so a global handler affects the whole page.
### Fix Focus Areas
- apps/desktop/src/renderer/components/prompt/PromptListView.tsx[148-193]
### Recommended fix
Implement one of these (prefer 1):
1) **Attach the key handler to the list container instead of `window`**:
- Add `tabIndex={0}` to the list container and use `onKeyDown`/`onKeyDownCapture` there.
- Only run indentation logic when the list container (or a row inside it) is the event target.
2) **If keeping `window` listener, ignore form/editing contexts**:
- Before `preventDefault()`, check `event.target` / `document.activeElement` and return when:
- target is `INPUT`, `TEXTAREA`, `SELECT`, or `contentEditable`, or
- target is inside a modal/dialog region.
This ensures Tab continues to work for text entry and focus navigation while still supporting hierarchy editing when the list has focus.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (4)
spec/changes/active/desktop-prompt-relationship-tree/design.md (1)
41-43: ⚡ Quick win把这里明确成复合索引。
当前表述容易被实现成两个单列索引,但层级读取/重排真正依赖的是
parent_id + sort_order组合;单列索引对兄弟列表扫描和顺序重写帮助有限。建议直接写成复合索引,避免后续 schema 按错误形状落地。📎 建议改法
- Add indexes for parent and sort order. + Add a composite index on `(parent_id, sort_order)` for sibling lookup and ordering.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@spec/changes/active/desktop-prompt-relationship-tree/design.md` around lines 41 - 43, The current specification "Add indexes for parent and sort order" is ambiguous and could be misinterpreted as creating two separate single-column indexes, but the actual requirement for efficient hierarchical reading and reordering operations is a composite index on the (parent_id, sort_order) combination. Modify the index specification to explicitly state that a composite index should be created on (parent_id, sort_order) together, not as separate indexes, to ensure proper support for sibling list scanning and order rewriting operations.apps/desktop/tests/unit/main/prompt-db.test.ts (1)
276-289: ⚡ Quick win建议补充“
promptId不存在时应抛错”的回归用例。当前层级测试集已覆盖多数非法输入;再补一条
db.movePrompt("missing-id", ...)的断言,可以防止未来回归为静默 no-op。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/tests/unit/main/prompt-db.test.ts` around lines 276 - 289, The test for movePrompt in the "rejects invalid parent and order inputs" test case does not cover the scenario where the promptId being moved does not exist. Add another expect statement that calls db.movePrompt with a non-existent prompt id (like "missing-id") and verify that it throws an appropriate error message (such as "Prompt does not exist"). This will prevent future regressions where the movePrompt method silently ignores moves for non-existent prompts instead of properly throwing an error.packages/db/src/schema.ts (1)
232-233: ⚡ Quick win建议将层级排序索引改为复合索引以匹配实际查询路径。
movePrompt/getChildren的热点查询是按parent_id过滤并按sort_order排序;当前两条单列索引在该模式下命中不如复合索引稳定,建议补prompts(parent_id, sort_order)(可保留或评估替换现有单列索引)。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/db/src/schema.ts` around lines 232 - 233, The current single-column indexes idx_prompts_parent and idx_prompts_sort_order on the prompts table do not align with the actual query pattern used in movePrompt and getChildren functions, which filter by parent_id and then sort by sort_order. Add a composite index on prompts(parent_id, sort_order) to improve query performance for this common access pattern. You can keep or remove the existing single-column indexes based on other usage patterns in the codebase, but the composite index should be prioritized for the hot query path.apps/desktop/src/renderer/components/prompt/PromptListView.tsx (1)
59-68: ⚡ Quick win给导出的组件补上显式返回类型。
这里是导出函数,仓库规则要求显式返回类型;补成
: JSX.Element或等价类型后,后续重构时更容易尽早发现意外的null/非 JSX 返回。请顺手跑一次现有 typecheck,确认注解后没有暴露隐藏分支。As per coding guidelines,
All exported functions must have explicit return type annotations.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/components/prompt/PromptListView.tsx` around lines 59 - 68, The exported PromptListView function is missing an explicit return type annotation, which violates the repository's coding guidelines that require all exported functions to have explicit return types. Add an explicit return type annotation (`: JSX.Element` or equivalent React component type) to the PromptListView function signature, then run the project's typecheck command to verify that the annotation does not expose any hidden branches or unexpected type mismatches that were previously hidden.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/desktop/src/main/ipc/prompt.ipc.ts`:
- Around line 59-70: The error messages in the prompt validation block lack
context about which parameters and values failed validation. Enhance each of the
three error throws (for promptId validation around line 60, newParentId
validation around line 66, and newOrder validation around line 69) to include
the actual parameter values or descriptions in the error message. For example,
include the promptId value, the newParentId value, and the newOrder value in
their respective error messages to help callers quickly identify which parameter
caused the failure.
In `@apps/desktop/src/preload/api/prompt.ts`:
- Around line 43-44: The `move` method in the prompt API lacks an explicit
return type annotation, which weakens type safety for the IPC call chain. Add an
explicit return type annotation to the arrow function (such as `Promise<void>`
or the appropriate return type), and use generic typing on the
`ipcRenderer.invoke()` call by specifying the expected return type as a generic
parameter. Follow the same pattern used in the `request` method from `ai.ts` to
maintain consistency and comply with the coding guideline requiring all exported
functions to have explicit return type annotations.
In `@apps/desktop/src/renderer/components/layout/MainContent.tsx`:
- Around line 1987-1999: The PromptListHeader component renders sort controls
that have no effect because the PromptListView component does not accept or use
sortBy and sortOrder props, and instead uses hardcoded hierarchy-based sorting.
Either pass the current sortBy and sortOrder state as props to PromptListView
and implement sorting logic based on these props instead of the hardcoded
order/title/id sorting, or conditionally hide/disable the sort controls in
PromptListHeader when viewMode is 'list' to prevent users from interacting with
non-functional controls.
In `@apps/desktop/src/renderer/components/prompt/PromptListView.tsx`:
- Around line 19-23: The onSelect callback in the PromptListViewProps interface
only receives the id string, which loses the mouse event information
(Ctrl/Cmd/Shift modifiers) needed for multi-select functionality. Change the
onSelect callback signature to receive both the prompt object and the MouseEvent
instead of just the id string. Then update all invocations of onSelect
(including at lines 378-379) to pass the complete prompt object and the mouse
event from the row click handler, allowing the parent component to reuse its
existing multi-select logic based on the modifier keys.
- Around line 148-200: The global Tab key listener in the useEffect hook within
PromptListView is intercepting Tab presses on all focusable elements (search
boxes, buttons, menus) and preventing normal keyboard navigation. Add a check at
the beginning of the handleKeyDown function to verify that event.target is
appropriate for tree navigation by either checking that the event originated
from the tree container itself, or by explicitly allowing Tab to pass through
when event.target is an input, button, or other interactive element. This
ensures the custom Tab behavior only applies when navigating the tree list and
does not interfere with other UI components.
- Around line 27-31: The `onMovePrompt` callback in PromptListView.tsx is
declared as synchronous returning void, but since movePrompt performs async
operations (IPC and IndexedDB) that can fail, errors are silently swallowed with
no user feedback. Change the `onMovePrompt` signature to return `Promise<void>`
at line 27-31, then locate all call sites of this callback and await them with
proper error handling (passing errors to a toast or error boundary). The same
change must be applied at the other affected locations in the same file at lines
172-188 and 304-330 where the callback is invoked.
In `@apps/desktop/src/renderer/services/database.ts`:
- Around line 382-384: The condition checking if prompt does not exist currently
performs a silent failure by calling resolve() without any action, which
misleads the caller into thinking the operation succeeded. Instead of silently
returning when the prompt is not found in the if (!prompt) check, throw an
explicit error that includes the promptId context to properly signal the failure
to the caller. This ensures the function adheres to the coding guideline of "No
silent failures" and provides meaningful error information for debugging.
- Line 380: The type assertion `as Prompt[]` in the assignment to the prompts
variable is unnecessary and violates coding guidelines. Remove the `as Prompt[]`
type assertion from the getAllRequest.result assignment to match the pattern
used in similar functions like legacyGetAllPrompts() and legacyGetAllFolders()
that use store.getAll() without type assertions.
In `@packages/db/src/prompt.ts`:
- Around line 705-706: Replace the silent return statement `if (!current)
return;` with an explicit error throw that includes relevant context information
such as the promptId. Instead of returning without notifying the caller, throw
an error (e.g., using Error or a custom exception class) with a descriptive
message that includes the promptId and indicates that the prompt was not found,
ensuring the caller is aware that the operation failed.
- Around line 190-197: The update() method in the prompt.ts file directly
modifies parent_id and sort_order fields without enforcing the hierarchical
constraints and validations that exist in the movePrompt() method, such as cycle
prevention, parent node existence checks, and sibling reordering. This allows
data integrity violations to persist. Remove the parent_id and sort_order update
logic from lines 190-197 (the conditions checking data.parentId and data.order)
to prevent direct hierarchical mutations through update(), and require callers
to use movePrompt() instead for any parent or order changes, which will ensure
all necessary validations are applied.
- Around line 813-819: The stmt.all() call on line 818 wraps the parameter in an
array (stmt.all(parentId === null ? [] : [parentId])), which is inconsistent
with the pattern used on Line 779 that passes parameters directly to stmt.all().
This inconsistency causes the normalizeParams() adaptation layer to receive an
array as a single parameter instead of individual parameters, leading to
potential parameter binding errors. Fix this by following the Line 779 pattern:
when parentId is null, call stmt.all() without arguments; otherwise, call
stmt.all(parentId) to pass the parameter directly without wrapping it in an
array.
---
Nitpick comments:
In `@apps/desktop/src/renderer/components/prompt/PromptListView.tsx`:
- Around line 59-68: The exported PromptListView function is missing an explicit
return type annotation, which violates the repository's coding guidelines that
require all exported functions to have explicit return types. Add an explicit
return type annotation (`: JSX.Element` or equivalent React component type) to
the PromptListView function signature, then run the project's typecheck command
to verify that the annotation does not expose any hidden branches or unexpected
type mismatches that were previously hidden.
In `@apps/desktop/tests/unit/main/prompt-db.test.ts`:
- Around line 276-289: The test for movePrompt in the "rejects invalid parent
and order inputs" test case does not cover the scenario where the promptId being
moved does not exist. Add another expect statement that calls db.movePrompt with
a non-existent prompt id (like "missing-id") and verify that it throws an
appropriate error message (such as "Prompt does not exist"). This will prevent
future regressions where the movePrompt method silently ignores moves for
non-existent prompts instead of properly throwing an error.
In `@packages/db/src/schema.ts`:
- Around line 232-233: The current single-column indexes idx_prompts_parent and
idx_prompts_sort_order on the prompts table do not align with the actual query
pattern used in movePrompt and getChildren functions, which filter by parent_id
and then sort by sort_order. Add a composite index on prompts(parent_id,
sort_order) to improve query performance for this common access pattern. You can
keep or remove the existing single-column indexes based on other usage patterns
in the codebase, but the composite index should be prioritized for the hot query
path.
In `@spec/changes/active/desktop-prompt-relationship-tree/design.md`:
- Around line 41-43: The current specification "Add indexes for parent and sort
order" is ambiguous and could be misinterpreted as creating two separate
single-column indexes, but the actual requirement for efficient hierarchical
reading and reordering operations is a composite index on the (parent_id,
sort_order) combination. Modify the index specification to explicitly state that
a composite index should be created on (parent_id, sort_order) together, not as
separate indexes, to ensure proper support for sibling list scanning and order
rewriting operations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 66008b09-e6b6-4ecc-9dc0-404ff3ba8233
📒 Files selected for processing (18)
apps/desktop/src/main/ipc/prompt.ipc.tsapps/desktop/src/preload/api/prompt.tsapps/desktop/src/renderer/components/layout/MainContent.tsxapps/desktop/src/renderer/components/prompt/PromptListView.tsxapps/desktop/src/renderer/services/database.tsapps/desktop/src/renderer/stores/prompt.store.tsapps/desktop/tests/unit/main/database-migration-locks.test.tsapps/desktop/tests/unit/main/prompt-db.test.tspackages/db/src/init.tspackages/db/src/prompt.tspackages/db/src/schema.tspackages/shared/constants/ipc-channels.tspackages/shared/types/prompt.tsspec/changes/active/desktop-prompt-relationship-tree/design.mdspec/changes/active/desktop-prompt-relationship-tree/implementation.mdspec/changes/active/desktop-prompt-relationship-tree/proposal.mdspec/changes/active/desktop-prompt-relationship-tree/specs/prompt-relationships/spec.mdspec/changes/active/desktop-prompt-relationship-tree/tasks.md
| if (typeof promptId !== 'string' || promptId.trim().length === 0) { | ||
| throw new Error('Prompt id is required'); | ||
| } | ||
| if ( | ||
| newParentId !== null && | ||
| (typeof newParentId !== 'string' || newParentId.trim().length === 0) | ||
| ) { | ||
| throw new Error('Parent prompt id must be null or a non-empty string'); | ||
| } | ||
| if (!Number.isFinite(newOrder) || newOrder < 0) { | ||
| throw new Error('Prompt order must be a non-negative number'); | ||
| } |
There was a problem hiding this comment.
错误信息缺少上下文,定位非法入参会很慢。
Line 60、Line 66、Line 69 的报错都没有包含失败字段和值,调用端很难快速定位是哪一个参数导致失败。建议在不暴露敏感信息前提下补充 promptId/newParentId/newOrder 的上下文。
As per coding guidelines, "Error messages must include context (what failed, with what input)."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src/main/ipc/prompt.ipc.ts` around lines 59 - 70, The error
messages in the prompt validation block lack context about which parameters and
values failed validation. Enhance each of the three error throws (for promptId
validation around line 60, newParentId validation around line 66, and newOrder
validation around line 69) to include the actual parameter values or
descriptions in the error message. For example, include the promptId value, the
newParentId value, and the newOrder value in their respective error messages to
help callers quickly identify which parameter caused the failure.
Source: Coding guidelines
| move: (promptId: string, newParentId: string | null, newOrder: number) => | ||
| ipcRenderer.invoke(IPC_CHANNELS.PROMPT_MOVE, promptId, newParentId, newOrder), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/desktop/src/preload/api/prompt.ts | head -60Repository: legeling/PromptHub
Length of output: 2409
🏁 Script executed:
rg -t ts -t tsx "ipcRenderer\.invoke" apps/desktop/src/preload/api/ -A 1 -B 1Repository: legeling/PromptHub
Length of output: 90
🏁 Script executed:
# 查找是否有类型定义文件
fd -e "d.ts" "ipcRenderer\|electron" apps/Repository: legeling/PromptHub
Length of output: 44
🏁 Script executed:
# 检查编码指南中关于导出函数的适用范围
head -100 apps/desktop/src/preload/api/prompt.tsRepository: legeling/PromptHub
Length of output: 2094
🏁 Script executed:
# 查看 TypeScript 配置
find apps/desktop -name "tsconfig.json" | head -5Repository: legeling/PromptHub
Length of output: 89
🏁 Script executed:
# 查看其他 API 文件的实现方式
find apps/desktop/src/preload/api -name "*.ts" | head -10Repository: legeling/PromptHub
Length of output: 450
🏁 Script executed:
# 查看 ipcRenderer 的类型定义
rg "ipcRenderer" apps/desktop -t ts --no-heading | grep -i "type\|interface" | head -20Repository: legeling/PromptHub
Length of output: 44
🏁 Script executed:
cat -n apps/desktop/tsconfig.jsonRepository: legeling/PromptHub
Length of output: 1347
🏁 Script executed:
cat -n apps/desktop/src/preload/api/folder.tsRepository: legeling/PromptHub
Length of output: 982
🏁 Script executed:
cat -n apps/desktop/src/preload/api/ai.tsRepository: legeling/PromptHub
Length of output: 2362
move 需要显式返回类型注解,保持 IPC 调用链的类型安全。
当前 move 方法缺少返回类型,ipcRenderer.invoke() 的返回值默认推导为宽泛类型,弱化类型契约。参考 ai.ts 中 request 方法的做法,建议补充显式返回类型注解(如 Promise<void>),并使用泛型化 ipcRenderer.invoke<ReturnType>()。
符合编码指南要求:"All exported functions must have explicit return type annotations."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src/preload/api/prompt.ts` around lines 43 - 44, The `move`
method in the prompt API lacks an explicit return type annotation, which weakens
type safety for the IPC call chain. Add an explicit return type annotation to
the arrow function (such as `Promise<void>` or the appropriate return type), and
use generic typing on the `ipcRenderer.invoke()` call by specifying the expected
return type as a generic parameter. Follow the same pattern used in the
`request` method from `ai.ts` to maintain consistency and comply with the coding
guideline requiring all exported functions to have explicit return type
annotations.
Source: Coding guidelines
| <PromptListHeader count={sortedPrompts.length} /> | ||
| {viewMode === 'kanban' && ( | ||
| {viewMode === 'list' && ( | ||
| <Suspense fallback={loadingFallback}> | ||
| <PromptKanbanView | ||
| <PromptListView | ||
| prompts={visiblePrompts} | ||
| highlightTerms={highlightTerms} | ||
| selectedId={selectedId} | ||
| selectedIds={selectedIds} | ||
| onSelect={(id) => selectPrompt(id)} | ||
| onToggleFavorite={toggleFavorite} | ||
| onCopy={handleCopyPrompt} | ||
| onEdit={(prompt) => setEditingPrompt(prompt)} | ||
| onDelete={handleDeletePrompt} | ||
| onAiTest={handleAiTestFromTable} | ||
| onVersionHistory={handleVersionHistory} | ||
| onViewDetail={handleViewDetail} | ||
| onContextMenu={handleContextMenu} | ||
| onMovePrompt={movePrompt} | ||
| /> |
There was a problem hiding this comment.
列表视图里的排序控件现在是无效操作。
这一分支继续渲染 PromptListHeader,但 PromptListView 内部固定按 hierarchy 的 order/title/id 排序,而且这里也没有把 sortBy / sortOrder 传下去,所以用户改排序在 list 模式下不会生效。要么把排序状态真正接入树渲染,要么在 list 模式隐藏/禁用这些控件,避免出现“能点但没效果”的 UI。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src/renderer/components/layout/MainContent.tsx` around lines
1987 - 1999, The PromptListHeader component renders sort controls that have no
effect because the PromptListView component does not accept or use sortBy and
sortOrder props, and instead uses hardcoded hierarchy-based sorting. Either pass
the current sortBy and sortOrder state as props to PromptListView and implement
sorting logic based on these props instead of the hardcoded order/title/id
sorting, or conditionally hide/disable the sort controls in PromptListHeader
when viewMode is 'list' to prevent users from interacting with non-functional
controls.
| interface PromptListViewProps { | ||
| prompts: Prompt[]; | ||
| selectedId: string | null; | ||
| selectedIds: string[]; | ||
| onSelect: (id: string) => void; |
There was a problem hiding this comment.
恢复列表视图的多选输入通道。
selectedIds 现在只参与高亮,但行点击只把 id 传回去,组件拿不到 Ctrl/Cmd/Shift 信息,所以树视图里无法切换或扩展多选,列表批量操作会退化成单选。把 onSelect 改成接收 prompt 和 MouseEvent,然后直接复用父层现有的多选逻辑会更稳。
♻️ 建议修改
interface PromptListViewProps {
prompts: Prompt[];
selectedId: string | null;
selectedIds: string[];
- onSelect: (id: string) => void;
+ onSelect: (prompt: Prompt, event: ReactMouseEvent) => void;
onToggleFavorite: (id: string) => void;
onCopy: (prompt: Prompt) => void;
onContextMenu: (e: ReactMouseEvent, prompt: Prompt) => void;- onClick={() => onSelect(prompt.id)}
+ onClick={(event) => onSelect(prompt, event)}Also applies to: 378-379
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src/renderer/components/prompt/PromptListView.tsx` around lines
19 - 23, The onSelect callback in the PromptListViewProps interface only
receives the id string, which loses the mouse event information (Ctrl/Cmd/Shift
modifiers) needed for multi-select functionality. Change the onSelect callback
signature to receive both the prompt object and the MouseEvent instead of just
the id string. Then update all invocations of onSelect (including at lines
378-379) to pass the complete prompt object and the mouse event from the row
click handler, allowing the parent component to reuse its existing multi-select
logic based on the modifier keys.
| onMovePrompt: ( | ||
| promptId: string, | ||
| newParentId: string | null, | ||
| newOrder: number, | ||
| ) => void; |
There was a problem hiding this comment.
把移动回调改成异步并显式处理失败。
按这组改动的接线,movePrompt 会走持久化和同步路径;但这里把它声明成同步 void,并在调用后立刻结束拖拽/键盘流程。只要 IPC 或 IndexedDB 任一路径报错,用户这边看到的就是“操作没生效”,却没有任何失败反馈。建议把签名改成 Promise<void>,在这里 await,并把错误交给上层 toast 或错误边界处理。
As per coding guidelines, No silent failures. Functions must not swallow errors and return default values. If an operation can fail, the caller must know.
Also applies to: 172-188, 304-330
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src/renderer/components/prompt/PromptListView.tsx` around lines
27 - 31, The `onMovePrompt` callback in PromptListView.tsx is declared as
synchronous returning void, but since movePrompt performs async operations (IPC
and IndexedDB) that can fail, errors are silently swallowed with no user
feedback. Change the `onMovePrompt` signature to return `Promise<void>` at line
27-31, then locate all call sites of this callback and await them with proper
error handling (passing errors to a toast or error boundary). The same change
must be applied at the other affected locations in the same file at lines
172-188 and 304-330 where the callback is invoked.
Source: Coding guidelines
| const getAllRequest = store.getAll(); | ||
|
|
||
| getAllRequest.onsuccess = () => { | ||
| const prompts = getAllRequest.result as Prompt[]; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the file exists and read around line 380
head -n 385 apps/desktop/src/renderer/services/database.ts | tail -n 20Repository: legeling/PromptHub
Length of output: 677
🏁 Script executed:
# Get more context - read more lines around line 380
wc -l apps/desktop/src/renderer/services/database.tsRepository: legeling/PromptHub
Length of output: 113
🏁 Script executed:
# Read a broader context around line 380
sed -n '370,390p' apps/desktop/src/renderer/services/database.tsRepository: legeling/PromptHub
Length of output: 756
🏁 Script executed:
# Check the type of store.getAll() and understand the context better
# Let's find where getAllRequest is used and its type
sed -n '360,420p' apps/desktop/src/renderer/services/database.tsRepository: legeling/PromptHub
Length of output: 1837
🏁 Script executed:
# Find the Prompt type definition
rg "type Prompt|interface Prompt|class Prompt" apps/desktop/src/renderer --type ts --type tsxRepository: legeling/PromptHub
Length of output: 90
🏁 Script executed:
# Check imports at the top of the database.ts file
head -n 50 apps/desktop/src/renderer/services/database.tsRepository: legeling/PromptHub
Length of output: 1325
🏁 Script executed:
# Check if there's any type definition for the result of store.getAll()
# and understand the IndexedDB types being used
rg "IDBRequest|IDBObjectStore|getAll" apps/desktop/src/renderer/services/database.ts -A 2 -B 2Repository: legeling/PromptHub
Length of output: 2234
🏁 Script executed:
# Check if there are other similar type assertions in the same file with comments
rg "as (Prompt|Folder|PromptVersion)" apps/desktop/src/renderer/services/database.ts -B 2 -A 2Repository: legeling/PromptHub
Length of output: 245
🏁 Script executed:
# Check if there are any runtime type guards or validation functions used for similar cases
rg "Array.isArray|instanceof|typeof.*===|isPrompt|validatePrompt" apps/desktop/src/renderer/services/database.tsRepository: legeling/PromptHub
Length of output: 44
🏁 Script executed:
# Check the pattern in getAllPrompts to see how it handles similar situation
sed -n '90,130p' apps/desktop/src/renderer/services/database.tsRepository: legeling/PromptHub
Length of output: 1311
🏁 Script executed:
# Check if there are any comments near type assertions in the file
rg "as " apps/desktop/src/renderer/services/database.ts -B 3 | head -n 30Repository: legeling/PromptHub
Length of output: 874
🏁 Script executed:
# Look at how other functions handle similar situations with getAll()
sed -n '140,200p' apps/desktop/src/renderer/services/database.tsRepository: legeling/PromptHub
Length of output: 1885
🏁 Script executed:
# Check if getAllFolders has similar patterns
rg "getAllFolders" apps/desktop/src/renderer/services/database.ts -A 20Repository: legeling/PromptHub
Length of output: 748
🏁 Script executed:
# Check the exact type signature of IDBObjectStore.getAll() in TypeScript
# Look for how other similar patterns work
rg "request.result" apps/desktop/src/renderer/services/database.ts -B 5 -A 2Repository: legeling/PromptHub
Length of output: 1740
🏁 Script executed:
# Check if there are any validation/runtime type checks before using the data
sed -n '375,390p' apps/desktop/src/renderer/services/database.tsRepository: legeling/PromptHub
Length of output: 649
移除不必要的 as Prompt[] 类型断言。
此处的类型断言违反编码准则。查看同一文件中的其他类似函数(如 legacyGetAllPrompts() 第 171 行和 legacyGetAllFolders() 第 618 行),它们使用相同的 store.getAll() 但无需类型断言。应移除此断言,与代码库模式保持一致。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src/renderer/services/database.ts` at line 380, The type
assertion `as Prompt[]` in the assignment to the prompts variable is unnecessary
and violates coding guidelines. Remove the `as Prompt[]` type assertion from the
getAllRequest.result assignment to match the pattern used in similar functions
like legacyGetAllPrompts() and legacyGetAllFolders() that use store.getAll()
without type assertions.
Source: Coding guidelines
| if (!prompt) { | ||
| resolve(); | ||
| return; |
There was a problem hiding this comment.
目标 Prompt 不存在时直接成功返回,属于静默失败。
这里直接 resolve() 会让上层误判“移动成功”,但实际未执行任何变更。应显式抛错,并包含 promptId 上下文。
As per coding guidelines, "No silent failures. Functions must not swallow errors and return default values."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src/renderer/services/database.ts` around lines 382 - 384, The
condition checking if prompt does not exist currently performs a silent failure
by calling resolve() without any action, which misleads the caller into thinking
the operation succeeded. Instead of silently returning when the prompt is not
found in the if (!prompt) check, throw an explicit error that includes the
promptId context to properly signal the failure to the caller. This ensures the
function adheres to the coding guideline of "No silent failures" and provides
meaningful error information for debugging.
Source: Coding guidelines
| if (data.parentId !== undefined) { | ||
| updates.push("parent_id = ?"); | ||
| values.push(data.parentId); | ||
| } | ||
| if (data.order !== undefined) { | ||
| updates.push("sort_order = ?"); | ||
| values.push(data.order); | ||
| } |
There was a problem hiding this comment.
避免通过 update() 直接写层级字段,当前会绕过 movePrompt() 的约束与顺序重写。
Line 190-197 直接更新 parent_id/sort_order,但不会执行防环、父节点存在性校验、兄弟连续重排。再结合 apps/desktop/src/main/ipc/prompt.ipc.ts 的 PROMPT_UPDATE -> db.update(id, data),会留下可持久化的层级脏数据风险。
建议修复(示例)
- if (data.parentId !== undefined) {
- updates.push("parent_id = ?");
- values.push(data.parentId);
- }
- if (data.order !== undefined) {
- updates.push("sort_order = ?");
- values.push(data.order);
- }
+ if (data.parentId !== undefined || data.order !== undefined) {
+ throw new Error("Hierarchy fields must be updated via movePrompt()");
+ }📝 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.
| if (data.parentId !== undefined) { | |
| updates.push("parent_id = ?"); | |
| values.push(data.parentId); | |
| } | |
| if (data.order !== undefined) { | |
| updates.push("sort_order = ?"); | |
| values.push(data.order); | |
| } | |
| if (data.parentId !== undefined || data.order !== undefined) { | |
| throw new Error("Hierarchy fields must be updated via movePrompt()"); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/db/src/prompt.ts` around lines 190 - 197, The update() method in the
prompt.ts file directly modifies parent_id and sort_order fields without
enforcing the hierarchical constraints and validations that exist in the
movePrompt() method, such as cycle prevention, parent node existence checks, and
sibling reordering. This allows data integrity violations to persist. Remove the
parent_id and sort_order update logic from lines 190-197 (the conditions
checking data.parentId and data.order) to prevent direct hierarchical mutations
through update(), and require callers to use movePrompt() instead for any parent
or order changes, which will ensure all necessary validations are applied.
| if (!current) return; | ||
|
|
There was a problem hiding this comment.
请在 prompt 不存在时显式抛错,避免静默失败。
Line 705 当前 if (!current) return; 会让调用方误以为移动成功,建议抛出带上下文的错误(例如包含 promptId)。
As per coding guidelines, "No silent failures. Functions must not swallow errors and return default values. If an operation can fail, the caller must know."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/db/src/prompt.ts` around lines 705 - 706, Replace the silent return
statement `if (!current) return;` with an explicit error throw that includes
relevant context information such as the promptId. Instead of returning without
notifying the caller, throw an error (e.g., using Error or a custom exception
class) with a descriptive message that includes the promptId and indicates that
the prompt was not found, ensuring the caller is aware that the operation
failed.
Source: Coding guidelines
| const stmt = this.db.prepare( | ||
| parentId === null | ||
| ? "SELECT * FROM prompts WHERE parent_id IS NULL ORDER BY sort_order" | ||
| : "SELECT * FROM prompts WHERE parent_id = ? ORDER BY sort_order" | ||
| ); | ||
| const rows = stmt.all(parentId === null ? [] : [parentId]) as PromptRow[]; | ||
| return rows.map((row) => this.rowToPrompt(row)); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== adapter.ts: 查询 all/get/run 的签名 =="
fd -i "adapter.ts" | xargs -I{} sh -c 'echo "--- {} ---"; rg -n "all\\(|get\\(|run\\(" "{}" -C2'
echo "== prompt.ts: 现有 stmt.all 调用模式 =="
rg -n "stmt\\.all\\(" packages/db/src/prompt.ts -C2Repository: legeling/PromptHub
Length of output: 2740
🏁 Script executed:
# Check normalizeParams implementation in adapter.ts
rg -n "normalizeParams" packages/db/src/adapter.ts -A 10
# Check all stmt.all() calls in prompt.ts to see patterns
rg -n "stmt\.all\(" packages/db/src/prompt.ts -B 2 -A 2
# Also check if there are other similar db calls to understand expected pattern
rg -n "this\.db\.prepare" packages/db/src/prompt.ts -A 3 | head -60Repository: legeling/PromptHub
Length of output: 5773
第 818 行的参数传递方式需要修正,与同文件其他调用的约定不一致。
Line 818 使用 stmt.all(parentId === null ? [] : [parentId]) 将参数包装在数组中,而同文件 Line 779 使用 stmt.all(parentId) 直接传递参数。这种不一致会导致适配层 normalizeParams() 接收到数组作为单参数(params[0]),最终将嵌套数组传给底层 SQLite 绑定,可能导致参数数量不匹配或查询异常。
应修正为:const rows = stmt.all(parentId === null ? [] : parentId === undefined ? [] : [parentId]) as PromptRow[]; 或更简洁地采用 Line 779 的模式:(parentId === null ? stmt.all() : stmt.all(parentId))。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/db/src/prompt.ts` around lines 813 - 819, The stmt.all() call on
line 818 wraps the parameter in an array (stmt.all(parentId === null ? [] :
[parentId])), which is inconsistent with the pattern used on Line 779 that
passes parameters directly to stmt.all(). This inconsistency causes the
normalizeParams() adaptation layer to receive an array as a single parameter
instead of individual parameters, leading to potential parameter binding errors.
Fix this by following the Line 779 pattern: when parentId is null, call
stmt.all() without arguments; otherwise, call stmt.all(parentId) to pass the
parameter directly without wrapping it in an array.
Summary
feature/hierarchical-latestcontribution for hierarchical prompt drag/drop and keyboard indentation.grouped_underrelationships: no child prompt cascade delete, no self-parenting, no descendant cycles, and contiguous sibling order rewrites.prompts.parent_id/prompts.sort_order, plus IPC and IndexedDB fallback validation.related_to,variant_of,depends_on, andnext_steprelation kinds.Verification
pnpm --dir apps/desktop exec vitest run tests/unit/main/prompt-db.test.ts tests/unit/main/database-migration-locks.test.tspnpm --filter @prompthub/desktop typecheckpnpm --filter @prompthub/desktop lintpnpm --filter @prompthub/desktop buildgit diff --checkNotes
A mistaken broad
pnpm --filter @prompthub/desktop test:run -- ...invocation ran the wider desktop suite and surfaced two existing failures intests/integration/components/skill-ui.integration.test.tsx; the targeted prompt hierarchy and migration tests pass.Summary by CodeRabbit
发布说明
新功能
测试