feat: hierarchical prompt structure with drag-drop and keyboard indentation#172
Open
legeling wants to merge 1 commit into
Open
feat: hierarchical prompt structure with drag-drop and keyboard indentation#172legeling wants to merge 1 commit into
legeling wants to merge 1 commit into
Conversation
|
@jazzson51569 is attempting to deploy a commit to the legeling's projects Team on Vercel. A member of the Team first needs to authorize it. |
Code Review by Qodo
1. Chinese comments in source
|
📝 WalkthroughWalkthrough该 PR 为提示词增加层级与排序能力，新增移动通道与数据库重排逻辑，并在渲染器中接入 store 与数据库服务，最终在列表视图实现树形展示、拖拽移动和键盘缩进/反缩进交互。 Changes提示词层级与拖拽重排
Sequence Diagram(s)sequenceDiagram
participant User
participant ListView
participant PromptStore
participant RendererDB
participant MainIPC
participant PromptDB
User->>ListView: Drop prompt node
ListView->>ListView: Compute newParentId and newOrder
ListView->>PromptStore: movePrompt(promptId, newParentId, newOrder)
PromptStore->>RendererDB: movePrompt(...)
RendererDB->>MainIPC: prompt.move(...)
MainIPC->>PromptDB: movePrompt(...)
PromptDB->>PromptDB: Update parent_id and sort_order transactionally
PromptDB-->>MainIPC: Success
MainIPC->>MainIPC: syncWorkspace()
MainIPC-->>RendererDB: true
RendererDB-->>PromptStore: resolved
PromptStore->>PromptStore: fetchPrompts() and scheduleAllSaveSync()
PromptStore-->>ListView: Updated prompts
ListView-->>User: Render updated tree
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 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 QodoAdd hierarchical prompts with list view drag-drop and Tab indentation WalkthroughsDescription• Add parent/child + ordering fields to Prompt types and SQLite schema. • Implement hierarchical prompt list UI with expand/collapse, drag-drop reorder, and Tab/Shift+Tab indent. • Wire move flow end-to-end (store → renderer DB → IPC/preload → main SQLite). Diagramgraph TD
UI["PromptListView (tree UI)"] --> Store["prompt.store movePrompt"] --> RDB["renderer database.ts"] --> Preload["preload promptApi.move"] --> IPC["main prompt.ipc PROMPT_MOVE"] --> DB[("SQLite prompts: parent_id/sort_order")]
High-Level AssessmentThe following are alternative approaches to this PR: 1. Dedicated hierarchy table (separate from prompts)
2. Fractional ordering (LexoRank/float ranks) instead of integer reindexing
3. Adopt a drag-drop tree pattern/library (e.g., dnd-kit tree recipes)
Recommendation: The current adjacency-list design (parent_id + sort_order) is a pragmatic fit for SQLite and integrates cleanly with the existing prompt model and IPC flow. During review, focus on (1) ordering correctness when moving within the same parent vs across parents (including root-level moves), and (2) the chosen ON DELETE CASCADE behavior for parent prompts, since it can delete entire subtrees and impacts user expectations. File ChangesEnhancement (8)
Other (2)
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+2019
to
+2021
| {/* List view mode: hierarchical list with drag-and-drop */} | ||
| {/* 列表视图模式：分层列表支持拖拽 */} | ||
| <div |
There was a problem hiding this comment.
1. Chinese comments in source 📘 Rule violation ⚙ Maintainability
Chinese text was added directly in non-locale source files, which violates the rule forbidding Chinese characters outside locale JSON. This can lead to untranslated/inconsistent language output and fails the localization completeness requirement.
Agent Prompt
## Issue description
Chinese characters were added in non-locale source files (comments), which is forbidden by i18n compliance rules.
## Issue Context
Rule requires that Chinese characters appear only in locale JSON files (and any user-facing text must be translated via i18next keys).
## Fix Focus Areas
- apps/desktop/src/renderer/components/layout/MainContent.tsx[2019-2021]
- packages/db/src/prompt.ts[688-691]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| ${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` }} |
There was a problem hiding this comment.
2. Inline style paddingleft used 📘 Rule violation ⚙ Maintainability
PromptListView uses an inline style prop to compute indentation, violating the Tailwind-only styling requirement. Inline styles can bypass design tokens and theming conventions (light/dark).
Agent Prompt
## Issue description
An inline `style={{ paddingLeft: ... }}` is used for indentation.
## Issue Context
UI styling must be Tailwind-only; inline styles are prohibited.
## Fix Focus Areas
- apps/desktop/src/renderer/components/prompt/PromptListView.tsx[210-225]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+253
to
+257
| ipcMain.handle(IPC_CHANNELS.PROMPT_MOVE, async (_, promptId: string, newParentId: string | null, newOrder: number) => { | ||
| db.movePrompt(promptId, newParentId, newOrder); | ||
| syncWorkspace(); | ||
| return true; | ||
| }); |
There was a problem hiding this comment.
3. Prompt_move ipc unvalidated 📘 Rule violation ⛨ Security
The new IPC_CHANNELS.PROMPT_MOVE handler accepts promptId, newParentId, and newOrder without validating payload types/ranges before calling database logic. Malformed IPC inputs could cause runtime errors or unintended DB updates across the process boundary.
Agent Prompt
## Issue description
IPC handler processes unvalidated inputs (`promptId`, `newParentId`, `newOrder`).
## Issue Context
IPC is a process boundary; handlers must validate and reject malformed payloads before performing DB operations.
## Fix Focus Areas
- apps/desktop/src/main/ipc/prompt.ipc.ts[253-257]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+228
to
+240
| <button | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| toggleExpand(prompt.id); | ||
| }} | ||
| className="p-0.5 rounded hover:bg-accent transition-colors" | ||
| > | ||
| {isExpanded ? ( | ||
| <ChevronDown className="w-4 h-4 text-muted-foreground" /> | ||
| ) : ( | ||
| <ChevronRight className="w-4 h-4 text-muted-foreground" /> | ||
| )} | ||
| </button> |
There was a problem hiding this comment.
4. Expand button lacks aria-label 📘 Rule violation ≡ Correctness
The expand/collapse icon-only <button> in PromptListView has no accessible name (no aria-label), which harms screen-reader usability. This violates the accessibility requirement for interactive elements.
Agent Prompt
## Issue description
Icon-only expand/collapse button has no ARIA label.
## Issue Context
Interactive elements need accessible labels for assistive technologies.
## Fix Focus Areas
- apps/desktop/src/renderer/components/prompt/PromptListView.tsx[228-240]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+695
to
+697
| const current = this.db | ||
| .prepare("SELECT parent_id, sort_order FROM prompts WHERE id = ?") | ||
| .get(promptId) as { parent_id: string | null; sort_order: number } | undefined; |
There was a problem hiding this comment.
5. Uncommented as type assertion 📘 Rule violation ⚙ Maintainability
New database code uses TypeScript as assertions without an interop justification comment. This can hide type mismatches at the DB boundary and violates the strictness rule on as usage.
Agent Prompt
## Issue description
Uncommented `as` assertions were introduced in DB code.
## Issue Context
Rule allows `as` only when necessary for interop and requires an explanatory comment.
## Fix Focus Areas
- packages/db/src/prompt.ts[695-697]
- packages/db/src/prompt.ts[742-752]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+70
to
+72
| user_prompt_en, variables, tags, folder_id, parent_id, sort_order, images, videos, source, notes, | ||
| last_ai_response, is_favorite, current_version, usage_count, created_at, updated_at | ||
| ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) | ||
| ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) |
There was a problem hiding this comment.
6. Missing parent/order migration 🐞 Bug ☼ Reliability
Existing SQLite databases won’t have prompts.parent_id and prompts.sort_order, but the updated PromptDB/schema now reference them, which will cause runtime SQL errors (e.g., failing INSERTs and CREATE INDEX) on upgrade.
Agent Prompt
### Issue description
The PR adds `prompts.parent_id` and `prompts.sort_order` usage in SQL, but the SQLite init/migration logic does not add these columns for existing databases. On upgrade, operations that reference these columns (including index creation) can fail.
### Issue Context
- Schema creation uses `CREATE TABLE IF NOT EXISTS`, which does not modify existing tables.
- `SCHEMA_INDEXES` now creates indexes on `parent_id` and `sort_order`, which will fail if the columns don’t exist.
### Fix Focus Areas
- Add migrations to backfill `parent_id` and `sort_order` columns in SQLite init.
- Update the “database appears current” column requirements to include the new columns.
- packages/db/src/init.ts[59-72]
- packages/db/src/init.ts[273-333]
- packages/db/src/schema.ts[198-234]
- packages/db/src/prompt.ts[63-100]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+253
to
+256
| ipcMain.handle(IPC_CHANNELS.PROMPT_MOVE, async (_, promptId: string, newParentId: string | null, newOrder: number) => { | ||
| db.movePrompt(promptId, newParentId, newOrder); | ||
| syncWorkspace(); | ||
| return true; |
There was a problem hiding this comment.
7. Workspace hierarchy not persisted 🐞 Bug ≡ Correctness
PROMPT_MOVE triggers syncWorkspace(), but workspace serialization/parsing omits parentId/order, so the on-disk workspace (and any restore/sync based on it) will lose the hierarchy and flatten relationships.
Agent Prompt
### Issue description
The new hierarchical relationships are not written to (or read from) the prompt workspace files. After a move, the DB is correct, but the synced workspace representation drops `parentId`/`order`, so hierarchy is lost when restoring/importing from workspace.
### Issue Context
- `PROMPT_MOVE` explicitly calls `syncWorkspace()`.
- `promptFrontmatter()` does not include `parentId`/`order`.
- `parsePromptFile()` also does not parse `parentId`/`order`.
### Fix Focus Areas
- Include `parentId` and `order` in `promptFrontmatter()`.
- Parse `parentId` and `order` in `parsePromptFile()`.
- Consider backward compatibility (missing fields default to null/0).
- apps/desktop/src/main/ipc/prompt.ipc.ts[253-256]
- apps/desktop/src/main/services/prompt-workspace.ts[243-264]
- apps/desktop/src/main/services/prompt-workspace.ts[633-675]
- apps/desktop/src/main/services/prompt-workspace.ts[864-884]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+151
to
+174
| const handleDrop = useCallback((e: React.DragEvent, targetPromptId: string) => { | ||
| e.preventDefault(); | ||
| if (draggingId && draggingId !== targetPromptId) { | ||
| const targetPrompt = prompts.find(p => p.id === targetPromptId); | ||
| const draggingPrompt = prompts.find(p => p.id === draggingId); | ||
|
|
||
| if (targetPrompt && draggingPrompt) { | ||
| if (dropPosition === 'inside') { | ||
| const childCount = prompts.filter(p => p.parentId === targetPromptId).length; | ||
| onMovePrompt(draggingId, targetPromptId, childCount); | ||
| } else { | ||
| const newParentId = targetPrompt.parentId; | ||
| let targetOrder = targetPrompt.order || 0; | ||
|
|
||
| if (dropPosition === 'after') { | ||
| targetOrder += 1; | ||
| } | ||
|
|
||
| if (draggingPrompt.parentId === newParentId && draggingPrompt.order && draggingPrompt.order < targetOrder) { | ||
| targetOrder -= 1; | ||
| } | ||
|
|
||
| onMovePrompt(draggingId, newParentId, targetOrder); | ||
| } |
There was a problem hiding this comment.
8. Cycle can crash tree 🐞 Bug ☼ Reliability
PromptListView allows moving a prompt inside any other prompt without preventing drops onto descendants, enabling parentId cycles; the recursive renderer has no cycle detection and can recurse indefinitely when expanded.
Agent Prompt
### Issue description
The UI can create cyclic parent relationships (A parent of B, then moving A under B). The tree renderer recursively renders children by `parentId` without a visited-set guard, which can cause infinite recursion/stack overflow.
### Issue Context
- `handleDrop` allows `onMovePrompt(draggingId, targetPromptId, ...)` for `inside` drops.
- Only `draggingId !== targetPromptId` is checked; descendants are not.
- Rendering recurses via `renderTreeNode(child, depth + 1)`.
### Fix Focus Areas
- Before calling `onMovePrompt`, detect whether `targetPromptId` is a descendant of `draggingId` (build a parent->children map from `prompts` and walk).
- Add a defensive cycle guard in rendering (e.g., pass a `visited` set per render path) to prevent hangs even if data is corrupted.
- Consider also validating in main/DB layer (reject moves that introduce cycles).
- apps/desktop/src/renderer/components/prompt/PromptListView.tsx[151-176]
- apps/desktop/src/renderer/components/prompt/PromptListView.tsx[312-316]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+35
to
+69
| useEffect(() => { | ||
| const handleKeyDown = (e: KeyboardEvent) => { | ||
| if (!selectedId || draggingId) return; | ||
|
|
||
| const selectedPrompt = prompts.find(p => p.id === selectedId); | ||
| if (!selectedPrompt) return; | ||
|
|
||
| if (e.key === 'Tab' && !e.ctrlKey && !e.metaKey) { | ||
| e.preventDefault(); | ||
|
|
||
| if (e.shiftKey) { | ||
| // Shift+Tab: outdent (move to parent level) | ||
| if (selectedPrompt.parentId) { | ||
| const siblings = prompts.filter(p => p.parentId === selectedPrompt.parentId); | ||
| const parentPrompt = prompts.find(p => p.id === selectedPrompt.parentId); | ||
| const newOrder = parentPrompt ? prompts.filter(p => p.parentId === parentPrompt?.parentId).length : prompts.filter(p => p.parentId === null).length; | ||
| onMovePrompt(selectedId, parentPrompt?.parentId || null, newOrder); | ||
| } | ||
| } else { | ||
| // Tab: indent (move to previous sibling's child) | ||
| const siblings = prompts.filter(p => p.parentId === selectedPrompt.parentId).sort((a, b) => (a.order || 0) - (b.order || 0)); | ||
| const currentIndex = siblings.findIndex(s => s.id === selectedId); | ||
|
|
||
| if (currentIndex > 0) { | ||
| const prevSibling = siblings[currentIndex - 1]; | ||
| const childCount = prompts.filter(p => p.parentId === prevSibling.id).length; | ||
| onMovePrompt(selectedId, prevSibling.id, childCount); | ||
| } | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| window.addEventListener('keydown', handleKeyDown); | ||
| return () => window.removeEventListener('keydown', handleKeyDown); | ||
| }, [selectedId, prompts, onMovePrompt, draggingId]); |
There was a problem hiding this comment.
9. Tab key hijacked globally 🐞 Bug ≡ Correctness
PromptListView attaches a window-level keydown listener that prevents default on Tab whenever a prompt is selected, breaking standard Tab focus navigation across the entire app (including inputs) even when the list isn’t focused.
Agent Prompt
### Issue description
A global `window` keydown listener intercepts Tab for indentation/outdent, which prevents normal keyboard focus traversal throughout the application whenever `selectedId` is set.
### Issue Context
- The handler is registered on `window` and calls `e.preventDefault()` on Tab.
- There is no check for `event.target` (e.g., input/textarea/contentEditable) and no focus scoping to the list view.
### Fix Focus Areas
- Restrict the handler to the list container (e.g., use `onKeyDown` on a focusable wrapper with `tabIndex={0}`), or at least ignore events when `e.target` is an input/textarea/select/button or `contentEditable`.
- Optionally require a modifier (e.g., Alt+Tab) to avoid overriding standard Tab navigation.
- apps/desktop/src/renderer/components/prompt/PromptListView.tsx[35-69]
- apps/desktop/src/renderer/stores/prompt.store.ts[150-155]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (2)
packages/db/src/schema.ts (1)
232-233: ⚡ Quick win建议将父子查询索引改为联合索引。
当前分离的
parent_id与sort_order单列索引，对WHERE parent_id = ? ORDER BY sort_order这类核心路径不如联合索引稳定。建议改为(parent_id, sort_order)，减少排序回表成本。♻️ 建议修改
-CREATE INDEX IF NOT EXISTS idx_prompts_parent ON prompts(parent_id); -CREATE INDEX IF NOT EXISTS idx_prompts_sort_order ON prompts(sort_order); +CREATE INDEX IF NOT EXISTS idx_prompts_parent_sort_order ON 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, Replace the two separate single-column indexes idx_prompts_parent and idx_prompts_sort_order on the prompts table with a composite index on (parent_id, sort_order): drop or remove idx_prompts_parent and idx_prompts_sort_order and create a new index (e.g. idx_prompts_parent_sort) defined as CREATE INDEX IF NOT EXISTS idx_prompts_parent_sort ON prompts(parent_id, sort_order) so queries using WHERE parent_id = ? ORDER BY sort_order can use the combined index efficiently.apps/desktop/src/preload/api/prompt.ts (1)
43-44: ⚡ Quick win为
move显式声明返回类型，固定 IPC 合同。建议显式标注为
Promise<boolean>，避免调用侧推断漂移。♻️ 建议修改
- move: (promptId: string, newParentId: string | null, newOrder: number) => + move: (promptId: string, newParentId: string | null, newOrder: number): Promise<boolean> => ipcRenderer.invoke(IPC_CHANNELS.PROMPT_MOVE, promptId, newParentId, newOrder),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/preload/api/prompt.ts` around lines 43 - 44, The exported function move lacks an explicit return type; update its signature to declare a return type of Promise<boolean> (e.g., change move: (promptId: string, newParentId: string | null, newOrder: number) => Promise<boolean>) and ensure the IPC call is typed accordingly (use ipcRenderer.invoke<boolean>(IPC_CHANNELS.PROMPT_MOVE, ...) or equivalent) so the IPC contract is fixed and callers get a stable boolean Promise type.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/renderer/components/layout/MainContent.tsx`:
- Around line 2019-2039: There are two rendered containers for the 'list' view
causing overlap: remove the duplicate branch so only one container uses
getViewClass('list') and renders PromptListHeader plus the PromptListView when
viewMode === 'list'; specifically, keep the single canonical container (the one
that includes PromptListHeader and the Suspense/PromptListView) and delete the
other getViewClass('list') block (or change its mode name if it represents a
different UI), leaving only one conditional render path that uses viewMode ===
'list', PromptListHeader, and PromptListView (and preserves handlers like
selectPrompt, toggleFavorite, handleCopyPrompt, handleContextMenu, movePrompt).
In `@apps/desktop/src/renderer/components/prompt/PromptListView.tsx`:
- Around line 35-69: The global Tab key handler in useEffect (handleKeyDown)
currently preventDefault() for Tab without excluding editable elements, causing
input/textarea/select/contentEditable to be hijacked; update handleKeyDown to
early-return when e.target is an editable/focusable element by checking
(e.target instanceof HTMLElement) and then if tagName is INPUT, TEXTAREA, SELECT
or element.isContentEditable (and optionally when closest('[role="textbox"]') or
contenteditable ancestor exists), so the handler only runs for non-editable
areas before any preventDefault()/onMovePrompt logic; keep references to
selectedId, prompts, draggingId and call onMovePrompt unchanged when allowed.
- Around line 169-171: The condition that adjusts targetOrder incorrectly treats
draggingPrompt.order === 0 as falsy; update the check in the block that compares
draggingPrompt.parentId to newParentId so it uses an explicit presence test
(e.g., draggingPrompt.order !== undefined/null) instead of a truthiness test,
ensuring the branch runs for order === 0 and properly decrements targetOrder
when needed.
- Around line 19-28: The exported function PromptListView is missing an explicit
return type; update its signature to include a JSX return type (e.g., change
"export function PromptListView({...}: PromptListViewProps)" to "export function
PromptListView({...}: PromptListViewProps): JSX.Element" or ":
React.JSX.Element" so TypeScript has an explicit component return type; keep the
function body unchanged and ensure any necessary React types are available from
your TS config/imports.
- Around line 14-16: The onMovePrompt prop in PromptListView.tsx is declared as
returning void but is implemented/used with async movePrompt (usePromptStore)
and is invoked without awaiting in keyboard handlers (Tab-related code) and
handleDrop; change the prop signature to return Promise<void> (onMovePrompt:
(promptId: string, newParentId: string | null, newOrder: number) =>
Promise<void>), update the callers in MainContent.tsx/PromptListView keyboard
Tab handlers and the handleDrop function to await onMovePrompt(...) and wrap
calls in try/catch, and in the catch include contextual details
(promptId/newParentId/newOrder) and surface or rethrow the error so Promise
rejections are handled and user feedback/logging is possible; ensure
usePromptStore.movePrompt still returns Promise<void> and no callers remain
unawaited.
In `@apps/desktop/src/renderer/services/database.ts`:
- Around line 371-383: The current IndexedDB update only changes the moved
prompt's parentId/order (in the transaction created via database.transaction and
STORES.PROMPTS using getRequest/putRequest) but must also reorder siblings in
both the source and destination parents to avoid duplicate or missing `order`
values and match main-process db.movePrompt semantics; fix by, inside the same
readwrite transaction, query the PROMPTS store for siblings of the original
parent and the new parent (e.g., via an index on parentId or by cursor),
decrement/increment or reindex their `order` fields to close the gap in the
source parent and make room in the destination at `newOrder`, update each
affected prompt.record.updatedAt and call store.put for each before resolving,
and ensure all store.put.onerror handlers reject the transaction so the
operation is atomic and consistent with db.movePrompt.
- Around line 384-386: The code currently calls resolve() when no prompt record
is found, causing silent success; instead, modify the branch that handles the
"no record" case to return an error with context (e.g., reject(new Error(`Prompt
not found: ${promptId}`)) or reject({ message: 'Prompt not found', promptId }))
so callers can detect the failure; locate the branch that checks for the record
(the code using promptId) and replace the resolve() with a reject/Error that
includes the promptId and a clear message.
In `@packages/db/src/prompt.ts`:
- Around line 709-726: The reorder logic in movePrompt produces gaps when moving
within the same parent or moving out of root; update movePrompt to handle three
branches: when oldParentId === newParentId and newOrder < oldOrder (same-parent
up): increment sort_order by 1 for rows with parent_id = oldParentId and
sort_order BETWEEN newOrder AND oldOrder-1; when oldParentId === newParentId and
newOrder > oldOrder (same-parent down): decrement sort_order by 1 for rows with
parent_id = oldParentId and sort_order BETWEEN oldOrder+1 AND newOrder; when
changing parent (oldParentId !== newParentId): decrement sort_order by 1 for
rows in the old parent where sort_order > oldOrder (use parent_id IS NULL when
oldParentId is null) and increment sort_order by 1 for rows in the new parent
where sort_order >= newOrder (use parent_id IS NULL when newParentId is null),
then update the prompt row (id = promptId) to its new parent_id and sort_order;
use the existing DB prepare/run calls and the variables oldParentId,
newParentId, oldOrder, newOrder, and promptId to implement these exact SQL
ranges so sibling sequences remain contiguous.
- Around line 699-700: The check `if (!current) return;` silently swallows a
missing-target error; instead locate the function that performs the move (the
block referencing `current` in packages/db/src/prompt.ts) and replace the early
return with throwing a descriptive Error (or a custom error type) that includes
identifying context (e.g., target prompt id/name and the attempted operation
like "movePrompt" or similar) so IPC callers can detect and handle the failure;
ensure the thrown error propagates rather than returning a success value.
---
Nitpick comments:
In `@apps/desktop/src/preload/api/prompt.ts`:
- Around line 43-44: The exported function move lacks an explicit return type;
update its signature to declare a return type of Promise<boolean> (e.g., change
move: (promptId: string, newParentId: string | null, newOrder: number) =>
Promise<boolean>) and ensure the IPC call is typed accordingly (use
ipcRenderer.invoke<boolean>(IPC_CHANNELS.PROMPT_MOVE, ...) or equivalent) so the
IPC contract is fixed and callers get a stable boolean Promise type.
In `@packages/db/src/schema.ts`:
- Around line 232-233: Replace the two separate single-column indexes
idx_prompts_parent and idx_prompts_sort_order on the prompts table with a
composite index on (parent_id, sort_order): drop or remove idx_prompts_parent
and idx_prompts_sort_order and create a new index (e.g. idx_prompts_parent_sort)
defined as CREATE INDEX IF NOT EXISTS idx_prompts_parent_sort ON
prompts(parent_id, sort_order) so queries using WHERE parent_id = ? ORDER BY
sort_order can use the combined index efficiently.
🪄 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: 9ad379b8-d115-4760-8b07-e9fae0c20c6f
📒 Files selected for processing (10)
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.tspackages/db/src/prompt.tspackages/db/src/schema.tspackages/shared/constants/ipc-channels.tspackages/shared/types/prompt.ts
Comment on lines
+2019
to
+2039
| {/* List view mode: hierarchical list with drag-and-drop */} | ||
| {/* 列表视图模式：分层列表支持拖拽 */} | ||
| <div | ||
| className={getViewClass('list')} | ||
| > | ||
| <PromptListHeader count={sortedPrompts.length} /> | ||
| {viewMode === 'list' && ( | ||
| <Suspense fallback={loadingFallback}> | ||
| <PromptListView | ||
| prompts={visiblePrompts} | ||
| selectedId={selectedId} | ||
| selectedIds={selectedIds} | ||
| onSelect={(id) => selectPrompt(id)} | ||
| onToggleFavorite={toggleFavorite} | ||
| onCopy={handleCopyPrompt} | ||
| onContextMenu={handleContextMenu} | ||
| onMovePrompt={movePrompt} | ||
| /> | ||
| </Suspense> | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
list 模式被渲染了两套容器，会出现重叠和重复渲染。
这里新增了第二个 getViewClass('list') 分支，而上方已存在一个同 mode 的 list 容器。viewMode === 'list' 时两者都会处于激活态。建议只保留一套 list 视图实现（或给旧表格分支独立 mode）。
🤖 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
2019 - 2039, There are two rendered containers for the 'list' view causing
overlap: remove the duplicate branch so only one container uses
getViewClass('list') and renders PromptListHeader plus the PromptListView when
viewMode === 'list'; specifically, keep the single canonical container (the one
that includes PromptListHeader and the Suspense/PromptListView) and delete the
other getViewClass('list') block (or change its mode name if it represents a
different UI), leaving only one conditional render path that uses viewMode ===
'list', PromptListHeader, and PromptListView (and preserves handlers like
selectPrompt, toggleFavorite, handleCopyPrompt, handleContextMenu, movePrompt).
Comment on lines
+14
to
+16
| onMovePrompt: (promptId: string, newParentId: string | null, newOrder: number) => void; | ||
| sortBy?: string; | ||
| sortOrder?: 'asc' | 'desc'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
repo_root="$(pwd)"
file="apps/desktop/src/renderer/components/prompt/PromptListView.tsx"
echo "== File exists? =="
ls -l "$file"
echo "== Snippet around lines 1-220 =="
sed -n '1,220p' "$file" | nl -ba | sed -n '1,220p'
echo "== Grep onMovePrompt occurrences =="
rg -n "onMovePrompt" "$file"
echo "== Find where onMovePrompt is passed to MainContent (search MainContent props) =="
rg -n "MainContent" "$file" -SRepository: legeling/PromptHub
Length of output: 289
🏁 Script executed:
#!/bin/bash
set -euo pipefail
repo_root="$(pwd)"
file="apps/desktop/src/renderer/components/prompt/PromptListView.tsx"
echo "== File exists? =="
ls -l "$file"
echo "== Snippet around start to 220 =="
sed -n '1,220p' "$file" | nl -ba
echo "== Grep onMovePrompt occurrences =="
rg -n "onMovePrompt" "$file"
echo "== Grep MainContent =="
rg -n "MainContent" "$file" -SRepository: legeling/PromptHub
Length of output: 290
🏁 Script executed:
#!/bin/bash
set -euo pipefail
repo_root="$(pwd)"
file="apps/desktop/src/renderer/components/prompt/PromptListView.tsx"
ls -l "$file"
sed -n '1,240p' "$file" | nl -ba
rg -n "onMovePrompt" "$file"
rg -n "MainContent" "$file" -SRepository: legeling/PromptHub
Length of output: 236
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="apps/desktop/src/renderer/components/prompt/PromptListView.tsx"
echo "== File =="
ls -l "$file"
echo "== Lines 1-220 =="
sed -n '1,220p' "$file" | nl -ba
echo "== onMovePrompt =="
rg -n "onMovePrompt" "$file"
echo "== MainContent =="
rg -n "MainContent" "$file" -SRepository: legeling/PromptHub
Length of output: 265
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="apps/desktop/src/renderer/components/prompt/PromptListView.tsx"
# show relevant ranges mentioned: 1-90, 35-110, 130-210
for start in 1 35 120 140; do
end=$((start+70))
echo "== $file lines $start-$end =="
sed -n "${start},${end}p" "$file" | nl -ba
done
echo "== all onMovePrompt occurrences =="
rg -n "onMovePrompt" "$file"
echo "== all invocations calling onMovePrompt (looking for identifier usage) =="
rg -n "onMovePrompt\s*\(" "$file"Repository: legeling/PromptHub
Length of output: 203
🏁 Script executed:
ls -lRepository: legeling/PromptHub
Length of output: 1083
🏁 Script executed:
file="apps/desktop/src/renderer/components/prompt/PromptListView.tsx"
sed -n '1,220p' "$file" | nl -ba
rg -n "onMovePrompt" "$file"
rg -n "onMovePrompt\s*\(" "$file"
rg -n "keyboard|drag|drop|onDrop|onKey|Enter|Space" "$file" -SRepository: legeling/PromptHub
Length of output: 3005
🏁 Script executed:
file="apps/desktop/src/renderer/components/prompt/PromptListView.tsx"
echo "== Content 1-90 =="
sed -n '1,90p' "$file" | awk '{printf "%4d: %s\n", NR, $0}'
echo "== Content 90-140 =="
sed -n '90,140p' "$file" | awk '{printf "%4d: %s\n", NR+90, $0}'
echo "== Content 140-210 =="
sed -n '140,210p' "$file" | awk '{printf "%4d: %s\n", NR+140, $0}'
echo "== Content 210-260 =="
sed -n '210,260p' "$file" | awk '{printf "%4d: %s\n", NR+210, $0}'
echo "== Search keyboard handlers in file (key/KeyDown/Enter/Space) =="
rg -n "onKeyDown|onKeyUp|keydown|KeyDown|Enter|Space" "$file" -S
echo "== Search for onMovePrompt assigned/returned in repo (apps/desktop/src) =="
rg -n "onMovePrompt" apps/desktop/src -SRepository: legeling/PromptHub
Length of output: 13465
🏁 Script executed:
file="apps/desktop/src/renderer/components/prompt/PromptListView.tsx"
echo "== Content 1-90 =="
sed -n '1,90p' "$file" | awk '{printf "%4d: %s\n", NR, $0}'
echo "== Content 90-140 =="
sed -n '90,140p' "$file" | awk '{printf "%4d: %s\n", NR+90, $0}'
echo "== Content 140-210 =="
sed -n '140,210p' "$file" | awk '{printf "%4d: %s\n", NR+140, $0}'
echo "== Content 210-260 =="
sed -n '210,260p' "$file" | awk '{printf "%4d: %s\n", NR+210, $0}'
echo "== Search keyboard handlers in file (key/KeyDown/Enter/Space) =="
rg -n "onKeyDown|onKeyUp|keydown|KeyDown|Enter|Space" "$file" -S
echo "== Search for onMovePrompt assigned/returned in repo (apps/desktop/src) =="
rg -n "onMovePrompt" apps/desktop/src -SRepository: legeling/PromptHub
Length of output: 13465
🏁 Script executed:
file="apps/desktop/src/renderer/components/prompt/PromptListView.tsx"
echo "== Content 1-90 =="
sed -n '1,90p' "$file" | awk '{printf "%4d: %s\n", NR, $0}'
echo "== Content 90-140 =="
sed -n '90,140p' "$file" | awk '{printf "%4d: %s\n", NR+90, $0}'
echo "== Content 140-210 =="
sed -n '140,210p' "$file" | awk '{printf "%4d: %s\n", NR+140, $0}'
echo "== Content 210-260 =="
sed -n '210,260p' "$file" | awk '{printf "%4d: %s\n", NR+210, $0}'
echo "== Search keyboard handlers in file =="
rg -n "onKeyDown|onKeyUp|keydown|Enter|Space" "$file" -S
echo "== Search for onMovePrompt occurrences in apps/desktop/src =="
rg -n "onMovePrompt" apps/desktop/src -SRepository: legeling/PromptHub
Length of output: 13373
🏁 Script executed:
file="apps/desktop/src/renderer/components/layout/MainContent.tsx"
rg -n "movePrompt" "$file" -SRepository: legeling/PromptHub
Length of output: 177
🏁 Script executed:
rg -n "movePrompt" apps/desktop/src/renderer -SRepository: legeling/PromptHub
Length of output: 1357
🏁 Script executed:
sed -n '1,120p' apps/desktop/src/renderer/stores/prompt.store.ts | awk '{print NR ":" $0}'
sed -n '200,280p' apps/desktop/src/renderer/stores/prompt.store.ts | awk '{print NR ":" $0}'
sed -n '300,420p' apps/desktop/src/renderer/services/database.ts | awk '{print NR ":" $0}'Repository: legeling/PromptHub
Length of output: 10013
🏁 Script executed:
sed -n '1,120p' apps/desktop/src/renderer/stores/prompt.store.ts | awk '{print NR ":" $0}'
sed -n '200,280p' apps/desktop/src/renderer/stores/prompt.store.ts | awk '{print NR ":" $0}'
sed -n '300,420p' apps/desktop/src/renderer/services/database.ts | awk '{print NR ":" $0}'Repository: legeling/PromptHub
Length of output: 10013
onMovePrompt 声明为 void 与实际异步实现不匹配，可能导致未处理的 Promise 错误
PromptListView.tsx 中 onMovePrompt: (...) => void，但 MainContent.tsx 传入的是 usePromptStore 的 movePrompt: async (...) => Promise<void>（内部 await db.movePrompt(...) 等；无 try/catch）。在键盘 Tab（约 51/61）和拖拽 handleDrop（约 160/173）里直接调用 onMovePrompt(...)，调用方未 await/catch，Promise reject 可能变成未处理异常且缺少用户反馈。
建议把 onMovePrompt 签名改为 Promise<void>，并在键盘/拖拽触发处统一 await，catch 中带上 promptId/newParentId/newOrder 上下文并处理/上抛错误。
Also applies to: 42-62, 151-174
onMovePrompt: (promptId: string, newParentId: string | null, newOrder: number) => void;🤖 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
14 - 16, The onMovePrompt prop in PromptListView.tsx is declared as returning
void but is implemented/used with async movePrompt (usePromptStore) and is
invoked without awaiting in keyboard handlers (Tab-related code) and handleDrop;
change the prop signature to return Promise<void> (onMovePrompt: (promptId:
string, newParentId: string | null, newOrder: number) => Promise<void>), update
the callers in MainContent.tsx/PromptListView keyboard Tab handlers and the
handleDrop function to await onMovePrompt(...) and wrap calls in try/catch, and
in the catch include contextual details (promptId/newParentId/newOrder) and
surface or rethrow the error so Promise rejections are handled and user
feedback/logging is possible; ensure usePromptStore.movePrompt still returns
Promise<void> and no callers remain unawaited.
Source: Coding guidelines
Comment on lines
19
to
28
| export function PromptListView({ | ||
| prompts, | ||
| selectedId, | ||
| selectedIds, | ||
| onSelect, | ||
| onToggleFavorite, | ||
| onCopy, | ||
| onContextMenu, | ||
| onMovePrompt, | ||
| }: PromptListViewProps) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ls -la
# show file existence and relevant lines
FILE="apps/desktop/src/renderer/components/prompt/PromptListView.tsx"
if [ -f "$FILE" ]; then
echo "FOUND: $FILE"
nl -ba "$FILE" | sed -n '1,120p'
else
echo "MISSING: $FILE"
fd -a "PromptListView.tsx" .
fiRepository: legeling/PromptHub
Length of output: 1563
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ls -la
# show file existence and relevant lines
FILE="apps/desktop/src/renderer/components/prompt/PromptListView.tsx"
if [ -f "$FILE" ]; then
echo "FOUND: $FILE"
nl -ba "$FILE" | sed -n '1,160p'
else
echo "MISSING: $FILE"
fd -a "PromptListView.tsx" .
fiRepository: legeling/PromptHub
Length of output: 1563
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/desktop/src/renderer/components/prompt/PromptListView.tsx"
echo "FOUND? $(test -f "$FILE" && echo yes || echo no): $FILE"
echo "---- signature area ----"
python3 - <<'PY'
import itertools,sys
path="apps/desktop/src/renderer/components/prompt/PromptListView.tsx"
start=1
end=120
with open(path,'r',encoding='utf-8') as f:
for i,line in enumerate(f,1):
if start<=i<=end:
print(f"{i:5d}\t{line.rstrip()}")
if i>end:
break
PY
echo "---- exact match for export function PromptListView ----"
rg -n --fixed-string "export function PromptListView" "$FILE" || trueRepository: legeling/PromptHub
Length of output: 5818
为导出的 PromptListView 补充显式返回类型（TS 规范）
apps/desktop/src/renderer/components/prompt/PromptListView.tsx 第 19 行的 export function PromptListView(...) 缺少返回类型注解，建议显式标注为 : JSX.Element（或 : React.JSX.Element）。
🤖 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 - 28, The exported function PromptListView is missing an explicit return
type; update its signature to include a JSX return type (e.g., change "export
function PromptListView({...}: PromptListViewProps)" to "export function
PromptListView({...}: PromptListViewProps): JSX.Element" or ":
React.JSX.Element" so TypeScript has an explicit component return type; keep the
function body unchanged and ensure any necessary React types are available from
your TS config/imports.
Source: Coding guidelines
Comment on lines
+35
to
+69
| useEffect(() => { | ||
| const handleKeyDown = (e: KeyboardEvent) => { | ||
| if (!selectedId || draggingId) return; | ||
|
|
||
| const selectedPrompt = prompts.find(p => p.id === selectedId); | ||
| if (!selectedPrompt) return; | ||
|
|
||
| if (e.key === 'Tab' && !e.ctrlKey && !e.metaKey) { | ||
| e.preventDefault(); | ||
|
|
||
| if (e.shiftKey) { | ||
| // Shift+Tab: outdent (move to parent level) | ||
| if (selectedPrompt.parentId) { | ||
| const siblings = prompts.filter(p => p.parentId === selectedPrompt.parentId); | ||
| const parentPrompt = prompts.find(p => p.id === selectedPrompt.parentId); | ||
| const newOrder = parentPrompt ? prompts.filter(p => p.parentId === parentPrompt?.parentId).length : prompts.filter(p => p.parentId === null).length; | ||
| onMovePrompt(selectedId, parentPrompt?.parentId || null, newOrder); | ||
| } | ||
| } else { | ||
| // Tab: indent (move to previous sibling's child) | ||
| const siblings = prompts.filter(p => p.parentId === selectedPrompt.parentId).sort((a, b) => (a.order || 0) - (b.order || 0)); | ||
| const currentIndex = siblings.findIndex(s => s.id === selectedId); | ||
|
|
||
| if (currentIndex > 0) { | ||
| const prevSibling = siblings[currentIndex - 1]; | ||
| const childCount = prompts.filter(p => p.parentId === prevSibling.id).length; | ||
| onMovePrompt(selectedId, prevSibling.id, childCount); | ||
| } | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| window.addEventListener('keydown', handleKeyDown); | ||
| return () => window.removeEventListener('keydown', handleKeyDown); | ||
| }, [selectedId, prompts, onMovePrompt, draggingId]); |
There was a problem hiding this comment.
全局 Tab 监听会劫持输入控件的键盘导航。
Line 42-44 对 Tab 直接 preventDefault()，未排除 input/textarea/select/contenteditable 场景。用户在可编辑控件中按 Tab 时也会触发层级移动，影响可用性与无障碍操作。
🤖 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
35 - 69, The global Tab key handler in useEffect (handleKeyDown) currently
preventDefault() for Tab without excluding editable elements, causing
input/textarea/select/contentEditable to be hijacked; update handleKeyDown to
early-return when e.target is an editable/focusable element by checking
(e.target instanceof HTMLElement) and then if tagName is INPUT, TEXTAREA, SELECT
or element.isContentEditable (and optionally when closest('[role="textbox"]') or
contenteditable ancestor exists), so the handler only runs for non-editable
areas before any preventDefault()/onMovePrompt logic; keep references to
selectedId, prompts, draggingId and call onMovePrompt unchanged when allowed.
Comment on lines
+169
to
+171
| if (draggingPrompt.parentId === newParentId && draggingPrompt.order && draggingPrompt.order < targetOrder) { | ||
| targetOrder -= 1; | ||
| } |
There was a problem hiding this comment.
同级重排时 order=0 会触发错误分支。
Line 169 使用 draggingPrompt.order && ...，当 order 为 0 时条件为假，targetOrder 不会被正确回调，导致插入位置偏移。应改为显式判空判断（如 draggingPrompt.order !== undefined）。
建议修复
- if (draggingPrompt.parentId === newParentId && draggingPrompt.order && draggingPrompt.order < targetOrder) {
+ if (
+ draggingPrompt.parentId === newParentId &&
+ draggingPrompt.order !== undefined &&
+ draggingPrompt.order < targetOrder
+ ) {
targetOrder -= 1;
}📝 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.
Suggested change
| if (draggingPrompt.parentId === newParentId && draggingPrompt.order && draggingPrompt.order < targetOrder) { | |
| targetOrder -= 1; | |
| } | |
| if ( | |
| draggingPrompt.parentId === newParentId && | |
| draggingPrompt.order !== undefined && | |
| draggingPrompt.order < targetOrder | |
| ) { | |
| targetOrder -= 1; | |
| } |
🤖 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
169 - 171, The condition that adjusts targetOrder incorrectly treats
draggingPrompt.order === 0 as falsy; update the check in the block that compares
draggingPrompt.parentId to newParentId so it uses an explicit presence test
(e.g., draggingPrompt.order !== undefined/null) instead of a truthiness test,
ensuring the branch runs for order === 0 and properly decrements targetOrder
when needed.
Comment on lines
+371
to
+383
| const transaction = database.transaction(STORES.PROMPTS, "readwrite"); | ||
| const store = transaction.objectStore(STORES.PROMPTS); | ||
| const getRequest = store.get(promptId); | ||
|
|
||
| getRequest.onsuccess = () => { | ||
| const prompt = getRequest.result; | ||
| if (prompt) { | ||
| prompt.parentId = newParentId; | ||
| prompt.order = newOrder; | ||
| prompt.updatedAt = new Date().toISOString(); | ||
| const putRequest = store.put(prompt); | ||
| putRequest.onsuccess = () => resolve(); | ||
| putRequest.onerror = () => reject(putRequest.error); |
There was a problem hiding this comment.
IndexedDB 回退路径缺少同级重排，可能破坏层级顺序一致性。
Line 371-383 只更新了被移动节点的 parentId/order，没有同步调整目标同级与来源同级的 order。这会产生重复顺序或空洞顺序，和主进程 db.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 `@apps/desktop/src/renderer/services/database.ts` around lines 371 - 383, The
current IndexedDB update only changes the moved prompt's parentId/order (in the
transaction created via database.transaction and STORES.PROMPTS using
getRequest/putRequest) but must also reorder siblings in both the source and
destination parents to avoid duplicate or missing `order` values and match
main-process db.movePrompt semantics; fix by, inside the same readwrite
transaction, query the PROMPTS store for siblings of the original parent and the
new parent (e.g., via an index on parentId or by cursor), decrement/increment or
reindex their `order` fields to close the gap in the source parent and make room
in the destination at `newOrder`, update each affected prompt.record.updatedAt
and call store.put for each before resolving, and ensure all store.put.onerror
handlers reject the transaction so the operation is atomic and consistent with
db.movePrompt.
Comment on lines
+384
to
+386
| } else { | ||
| resolve(); | ||
| } |
There was a problem hiding this comment.
不要在未命中 Prompt 时静默返回成功。
Line 384-386 在找不到记录时直接 resolve()，调用方会误判移动成功。这里应返回包含 promptId 的错误上下文。
建议修复
- } else {
- resolve();
- }
+ } else {
+ reject(new Error(`movePrompt failed: prompt not found (promptId=${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."
📝 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.
Suggested change
| } else { | |
| resolve(); | |
| } | |
| } else { | |
| reject(new Error(`movePrompt failed: prompt not found (promptId=${promptId})`)); | |
| } |
🤖 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 384 - 386, The
code currently calls resolve() when no prompt record is found, causing silent
success; instead, modify the branch that handles the "no record" case to return
an error with context (e.g., reject(new Error(`Prompt not found: ${promptId}`))
or reject({ message: 'Prompt not found', promptId })) so callers can detect the
failure; locate the branch that checks for the record (the code using promptId)
and replace the resolve() with a reject/Error that includes the promptId and a
clear message.
Source: Coding guidelines
Comment on lines
+699
to
+700
| if (!current) return; | ||
|
|
There was a problem hiding this comment.
目标 Prompt 不存在时发生静默成功，调用方无法感知失败。
if (!current) return; 会让 IPC 侧继续返回成功，UI/Store 无法区分“移动成功”与“目标不存在”。这里应抛出带上下文的错误。
🛠️ 建议修改
- if (!current) return;
+ if (!current) {
+ throw new Error(`movePrompt failed: prompt not found (promptId=${promptId})`);
+ }As per coding guidelines, "No silent failures. Functions must not swallow errors and return default values."
📝 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.
Suggested change
| if (!current) return; | |
| if (!current) { | |
| throw new Error(`movePrompt failed: prompt not found (promptId=${promptId})`); | |
| } | |
🤖 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 699 - 700, The check `if (!current)
return;` silently swallows a missing-target error; instead locate the function
that performs the move (the block referencing `current` in
packages/db/src/prompt.ts) and replace the early return with throwing a
descriptive Error (or a custom error type) that includes identifying context
(e.g., target prompt id/name and the attempted operation like "movePrompt" or
similar) so IPC callers can detect and handle the failure; ensure the thrown
error propagates rather than returning a success value.
Source: Coding guidelines
Comment on lines
+709
to
+726
| // If parent changed, adjust orders in old parent | ||
| if (oldParentId !== newParentId && oldParentId !== null) { | ||
| this.db | ||
| .prepare("UPDATE prompts SET sort_order = sort_order - 1 WHERE parent_id = ? AND sort_order > ?") | ||
| .run(oldParentId, oldOrder); | ||
| } | ||
|
|
||
| // Adjust orders in new parent to make room | ||
| if (newParentId !== null) { | ||
| this.db | ||
| .prepare("UPDATE prompts SET sort_order = sort_order + 1 WHERE parent_id = ? AND sort_order >= ? AND id != ?") | ||
| .run(newParentId, newOrder, promptId); | ||
| } else { | ||
| // No parent (root level) | ||
| this.db | ||
| .prepare("UPDATE prompts SET sort_order = sort_order + 1 WHERE parent_id IS NULL AND sort_order >= ? AND id != ?") | ||
| .run(newOrder, promptId); | ||
| } |
There was a problem hiding this comment.
movePrompt 的重排逻辑会产生排序空洞/错位。
当前算法在“同父级重排”以及“从根节点迁出”场景下不会正确压缩旧位置，sort_order 会出现跳号或错位，后续拖拽与键盘缩进将基于脏序列继续计算，导致顺序持续漂移。
建议按三种分支处理并保持同父级连续序列：
- 同父级上移：
[newOrder, oldOrder)区间 +1 - 同父级下移：
(oldOrder, newOrder]区间 -1 - 跨父级移动：旧父级
> oldOrder全部 -1（含oldParentId IS NULL），新父级>= newOrder全部 +1，然后再更新目标节点。
🤖 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 709 - 726, The reorder logic in
movePrompt produces gaps when moving within the same parent or moving out of
root; update movePrompt to handle three branches: when oldParentId ===
newParentId and newOrder < oldOrder (same-parent up): increment sort_order by 1
for rows with parent_id = oldParentId and sort_order BETWEEN newOrder AND
oldOrder-1; when oldParentId === newParentId and newOrder > oldOrder
(same-parent down): decrement sort_order by 1 for rows with parent_id =
oldParentId and sort_order BETWEEN oldOrder+1 AND newOrder; when changing parent
(oldParentId !== newParentId): decrement sort_order by 1 for rows in the old
parent where sort_order > oldOrder (use parent_id IS NULL when oldParentId is
null) and increment sort_order by 1 for rows in the new parent where sort_order
>= newOrder (use parent_id IS NULL when newParentId is null), then update the
prompt row (id = promptId) to its new parent_id and sort_order; use the existing
DB prepare/run calls and the variables oldParentId, newParentId, oldOrder,
newOrder, and promptId to implement these exact SQL ranges so sibling sequences
remain contiguous.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Scope
Notes
jazzson51569/PromptHub:feature/hierarchical-latestlegeling/PromptHub:mainSummary by CodeRabbit