feat: drag-to-reorder assistants in the sidebar#324
Conversation
Lifts the Assistants section out of the conversation List into its own VStack so legacy `.onDrag` / `.onDrop` actually fire (the List ancestor was killing the drop). Adds `AssistantDragReorderModifier` which uses the `public.text` UTI with an NSString payload, and a `reorderAssistant(sourceID:onto:)` helper that renumbers `sortOrder` and persists via SwiftData. Drag is gated to the custom sort option so it doesn't fight alphabetical/recent ordering. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds drag-and-drop reordering for assistant tiles in the sidebar. It introduces a new ChangesDrag-Reorder for Assistants
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@Sources/UI/AssistantDragReorder.swift`:
- Around line 4-10: Remove the hardcoded absolute path in the top-of-file
comment in AssistantDragReorder.swift (the "/Users/hinrayleung/.claude/..."
path) and replace it with a neutral reference or remove it entirely; update the
comment to either reference a generic document name (e.g., "internal diagnostic
notes") or a repo-relative path if the note is checked into source control,
ensuring no personal usernames or local filesystem paths remain in the file
header.
In `@Sources/UI/ContentView`+AssistantManagement.swift:
- Line 95: The silent discard of save errors via "try? modelContext.save()" can
make the function return true even when persistence failed; replace it with a
do-catch that calls "try modelContext.save()" and either propagate the thrown
error (make the enclosing function throws) or catch and log the error (e.g.,
with os_log or a logger) and return false on failure so callers know the save
did not succeed; update the surrounding function signature/return logic
accordingly to ensure failures are not ignored.
🪄 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: ccf8567f-b842-4ad8-b07b-afd24e1b1b82
📒 Files selected for processing (4)
Sources/UI/AssistantDragReorder.swiftSources/UI/ContentView+AssistantManagement.swiftSources/UI/ContentView+SidebarSections.swiftSources/UI/ContentView.swift
| } | ||
| guard changed else { return false } | ||
|
|
||
| try? modelContext.save() |
There was a problem hiding this comment.
Handle save errors instead of silently ignoring them.
Using try? silently discards save failures, which means the function can return true even when changes weren't persisted. This creates a data-loss risk where the UI shows the new order but it's lost on app restart.
Proposed fix: propagate or log save errors
- try? modelContext.save()
- return true
+ do {
+ try modelContext.save()
+ return true
+ } catch {
+ print("⚠️ Failed to persist assistant reorder: \(error)")
+ return false
+ }📝 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.
| try? modelContext.save() | |
| do { | |
| try modelContext.save() | |
| return true | |
| } catch { | |
| print("⚠️ Failed to persist assistant reorder: \(error)") | |
| return false | |
| } |
🤖 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 `@Sources/UI/ContentView`+AssistantManagement.swift at line 95, The silent
discard of save errors via "try? modelContext.save()" can make the function
return true even when persistence failed; replace it with a do-catch that calls
"try modelContext.save()" and either propagate the thrown error (make the
enclosing function throws) or catch and log the error (e.g., with os_log or a
logger) and return false on failure so callers know the save did not succeed;
update the surrounding function signature/return logic accordingly to ensure
failures are not ignored.
Per CodeRabbit: the previous comment referenced a developer-local plan file that doesn't exist for other contributors. Keep the diagnostic summary inline instead. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 participant
Summary
Listso legacy.onDrag/.onDropfire correctly — being a List child was silently dropping the drop callback.AssistantDragReorderModifier(usespublic.text+ NSString payload, the combination that survives macOS quirks documented in the file header) and applies it to both the grid and list assistant layouts.reorderAssistant(sourceID:onto:)onContentViewwhich renumbersAssistantEntity.sortOrder, bumpsupdatedAt, and persists viamodelContext.save().Test plan
swift buildclean🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor