Fix sidebar reordering on vault data changes#53
Merged
Conversation
Two root causes: HashMap non-deterministic iteration in Rust and rayon parallel scan ordering meant the "natural order" of entities was effectively random on every refresh. Additionally, new items created on disk were invisible in the sidebar when a manual order existed. Fixes: - Sort entity lists alphabetically by title in Rust VaultIndex for stable default ordering - Append new entity IDs to stored sidebar order instead of silently dropping them Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughDeterministic entity ordering and wikilink-aware matching were introduced: vault index methods now return alphabetically sorted vectors; sidebar ordering preserves stored order and appends new items; multiple task-to-project/area matches were changed from substring checks to a new Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tdn-desktop/src-tauri/src/vault/manager.rs (1)
883-917: Consider adding a test to verify alphabetical sort order.The existing tests verify counts but don't confirm the sorting behavior. Adding a test with entities having titles in non-alphabetical order would validate the deterministic ordering fix.
📝 Example test for sort order verification
#[test] fn vault_index_all_tasks_sorted_alphabetically() { let tasks = vec![ Task { title: "Zebra Task".to_string(), ..test_task("task-z", "/tasks/z.md") }, Task { title: "alpha task".to_string(), ..test_task("task-a", "/tasks/a.md") }, Task { title: "Beta Task".to_string(), ..test_task("task-b", "/tasks/b.md") }, ]; let index = VaultIndex::from_scans(tasks, vec![], vec![]); let all = index.all_tasks(); assert_eq!(all[0].title, "alpha task"); // lowercase 'a' sorts first assert_eq!(all[1].title, "Beta Task"); assert_eq!(all[2].title, "Zebra Task"); }Based on learnings: "Write comprehensive tests for business logic".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tdn-desktop/src-tauri/src/vault/manager.rs` around lines 883 - 917, Add tests that assert alphabetical sort order for VaultIndex results: the current tests only check counts, so add new unit tests (e.g., vault_index_all_tasks_sorted_alphabetically, vault_index_all_projects_sorted_alphabetically, vault_index_all_areas_sorted_alphabetically) that create items with out-of-order titles and assert VaultIndex::from_scans(...).all_tasks()/all_projects()/all_areas() return entries in deterministic alphabetical order by title; locate the implementations that produce the lists (VaultIndex::from_scans and the all_tasks/all_projects/all_areas methods) and ensure the tests reference the Task/Project/Area title fields via the existing test helper test_task/test_project/test_area to validate ordering.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tdn-desktop/src/hooks/use-sidebar-order.ts`:
- Around line 64-66: Replace substring matching p.area?.includes(area.title)
with an exact WikiLink equality check: compare p.area to the full wiki link
string built from area.title (e.g., p.area === `[[${area.title}]]`) or normalize
both sides by stripping/adding the double-bracket wiki syntax before comparing;
update the same change wherever p.area?.includes(area.title) appears
(use-sidebar-order.ts, queries.ts occurrences, WeekView.tsx, TodayView.tsx) so
projects only match when the wiki link equals the area title exactly.
---
Nitpick comments:
In `@tdn-desktop/src-tauri/src/vault/manager.rs`:
- Around line 883-917: Add tests that assert alphabetical sort order for
VaultIndex results: the current tests only check counts, so add new unit tests
(e.g., vault_index_all_tasks_sorted_alphabetically,
vault_index_all_projects_sorted_alphabetically,
vault_index_all_areas_sorted_alphabetically) that create items with out-of-order
titles and assert
VaultIndex::from_scans(...).all_tasks()/all_projects()/all_areas() return
entries in deterministic alphabetical order by title; locate the implementations
that produce the lists (VaultIndex::from_scans and the
all_tasks/all_projects/all_areas methods) and ensure the tests reference the
Task/Project/Area title fields via the existing test helper
test_task/test_project/test_area to validate ordering.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b3fdb1b2-140d-4e53-bf50-3973f962c41e
📒 Files selected for processing (3)
tdn-desktop/src-tauri/src/vault/manager.rstdn-desktop/src/hooks/use-sidebar-order.test.tstdn-desktop/src/hooks/use-sidebar-order.ts
Comment on lines
+64
to
+66
| const area = areas.find(a => a.id === containerId) | ||
| if (!area) return [] | ||
| return projects.filter(p => p.area?.includes(area.title)).map(p => p.id) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for consistent wikilink matching patterns across the codebase
rg -n --type=ts "\.area\??\.(includes|match|indexOf)" tdn-desktop/src/Repository: dannysmith/taskdn
Length of output: 777
🏁 Script executed:
cat -n tdn-desktop/src/hooks/use-sidebar-order.ts | head -80Repository: dannysmith/taskdn
Length of output: 3732
🏁 Script executed:
# Check type definitions for area property
fd --type=ts -e ".ts" tdn-desktop/src | xargs grep -l "area.*:" | head -5Repository: dannysmith/taskdn
Length of output: 230
🏁 Script executed:
# Look for type definitions or interfaces that define the area property
rg -n "area\s*\?*:\s*" tdn-desktop/src --type=ts -B 2 -A 1 | head -40Repository: dannysmith/taskdn
Length of output: 2484
🏁 Script executed:
# Search for tests that cover area filtering
rg -n "area.*includes\|Homework.*Work\|substring" tdn-desktop/src/test --type=ts -A 3 -B 3Repository: dannysmith/taskdn
Length of output: 43
🏁 Script executed:
# Look for tests of getEffectiveProjectOrder or area filtering logic
rg -n "getEffectiveProjectOrder\|filter.*area" tdn-desktop/src/test --type=ts -A 5Repository: dannysmith/taskdn
Length of output: 43
🏁 Script executed:
# Check if there are any tests with multiple areas that have similar names
rg -n "Work.*Homework\|area.*title" tdn-desktop/src/test --type=ts -B 2 -A 2Repository: dannysmith/taskdn
Length of output: 43
🏁 Script executed:
# Verify the exact type definition for area property to be completely certain
rg -n "interface.*Project\|type.*Project" tdn-desktop/src --type=ts -A 15 | head -50Repository: dannysmith/taskdn
Length of output: 43
🏁 Script executed:
# Check the actual Project type definition
fd -name "*.ts" -path "*/types/*" tdn-desktop/src | xargs grep -l "Project" | head -3Repository: dannysmith/taskdn
Length of output: 291
🏁 Script executed:
# Search for Project type definition with simpler pattern
rg "export (type|interface) Project" tdn-desktop/src -A 20Repository: dannysmith/taskdn
Length of output: 6475
🏁 Script executed:
# Alternative: look in types directory
fd "." tdn-desktop/src/types -type f | head -10Repository: dannysmith/taskdn
Length of output: 231
Fix substring matching bug in area filtering — affects multiple files.
p.area?.includes(area.title) incorrectly matches projects when one area title is a substring of another. For example, a project with area: "[[Homework]]" would match both "Work" and "Homework" because "[[Homework]]".includes("Work") is true.
This bug affects 6 locations: use-sidebar-order.ts:66, queries.ts:188, queries.ts:202, WeekView.tsx:137, WeekView.tsx:148, and TodayView.tsx:130. Apply the fix consistently across all occurrences using exact string equality for WikiLink matching:
Proposed fix
- return projects.filter(p => p.area?.includes(area.title)).map(p => p.id)
+ return projects
+ .filter(p => p.area === `[[${area.title}]]`)
+ .map(p => p.id)📝 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
| const area = areas.find(a => a.id === containerId) | |
| if (!area) return [] | |
| return projects.filter(p => p.area?.includes(area.title)).map(p => p.id) | |
| const area = areas.find(a => a.id === containerId) | |
| if (!area) return [] | |
| return projects | |
| .filter(p => p.area === `[[${area.title}]]`) | |
| .map(p => p.id) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tdn-desktop/src/hooks/use-sidebar-order.ts` around lines 64 - 66, Replace
substring matching p.area?.includes(area.title) with an exact WikiLink equality
check: compare p.area to the full wiki link string built from area.title (e.g.,
p.area === `[[${area.title}]]`) or normalize both sides by stripping/adding the
double-bracket wiki syntax before comparing; update the same change wherever
p.area?.includes(area.title) appears (use-sidebar-order.ts, queries.ts
occurrences, WeekView.tsx, TodayView.tsx) so projects only match when the wiki
link equals the area title exactly.
Replace count-only assertions with tests that create items in reverse order and verify alphabetical output. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace p.area?.includes(area.title) with exact wikilink comparison using a new matchesWikilinkTitle() utility. The substring check caused false positives — e.g., area "Work" would match projects in "[[Homework]]". Affects: sidebar ordering, queries, TodayView, WeekView, ProjectView, AreaView, NoAreaView, AreaKanbanBoard. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tdn-desktop/src/services/vault/queries.ts (1)
214-234: Consider extracting a shared project-task filter helper.Line 217-219 and Line 232-234 duplicate the same filter (also repeated in Line 196). A small helper would reduce drift risk.
♻️ Proposed refactor
export function useVaultHelpers() { const { tasks, projects, areas } = useVaultData() @@ + const tasksForProjectTitle = (projectTitle: string) => + tasks.filter(t => matchesWikilinkTitle(t.project, projectTitle)) @@ getTasksByProjectId: (projectId: string) => { const project = projectsById.get(projectId) if (!project) return [] - return tasks.filter(t => matchesWikilinkTitle(t.project, project.title)) + return tasksForProjectTitle(project.title) }, @@ - const projectTasks = tasks.filter(t => - matchesWikilinkTitle(t.project, project.title) - ) + const projectTasks = tasksForProjectTitle(project.title) @@ - const projectTasks = tasks.filter(t => - matchesWikilinkTitle(t.project, project.title) - ) + const projectTasks = tasksForProjectTitle(project.title)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tdn-desktop/src/services/vault/queries.ts` around lines 214 - 234, Duplicate filtering logic for project tasks appears in getProjectCompletion and getTaskCounts (and elsewhere); extract a small helper such as getTasksForProject(projectId: string) or getTasksForProjectTitle(title: string) that looks up projectsById, returns [] when missing, and runs tasks.filter(t => matchesWikilinkTitle(t.project, project.title)); replace the inline filters in getProjectCompletion, getTaskCounts (and the other occurrence) to call this helper so behavior and null-handling remain identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tdn-desktop/src/services/vault/queries.ts`:
- Around line 214-234: Duplicate filtering logic for project tasks appears in
getProjectCompletion and getTaskCounts (and elsewhere); extract a small helper
such as getTasksForProject(projectId: string) or getTasksForProjectTitle(title:
string) that looks up projectsById, returns [] when missing, and runs
tasks.filter(t => matchesWikilinkTitle(t.project, project.title)); replace the
inline filters in getProjectCompletion, getTaskCounts (and the other occurrence)
to call this helper so behavior and null-handling remain identical.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e775624d-d39c-4d24-9709-e3651558f44d
📒 Files selected for processing (11)
tdn-desktop/src-tauri/src/vault/manager.rstdn-desktop/src/components/kanban/AreaKanbanBoard.tsxtdn-desktop/src/components/views/AreaView.tsxtdn-desktop/src/components/views/NoAreaView.tsxtdn-desktop/src/components/views/ProjectView.tsxtdn-desktop/src/components/views/TodayView.tsxtdn-desktop/src/components/views/WeekView.tsxtdn-desktop/src/hooks/use-sidebar-order.tstdn-desktop/src/lib/wikilink.test.tstdn-desktop/src/lib/wikilink.tstdn-desktop/src/services/vault/queries.ts
✅ Files skipped from review due to trivial changes (1)
- tdn-desktop/src/lib/wikilink.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- tdn-desktop/src/hooks/use-sidebar-order.ts
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
1 participant
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
Closes #51
all_tasks(),all_projects(),all_areas()) alphabetically by title inVaultIndex, so the "natural order" returned to the frontend is deterministic. Previously,HashMapiteration order + rayon parallel scanning made this effectively random on every vault refresh.Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Improvements
Bug Fixes
Tests