Conversation
…o .hito.json Implement mutually exclusive category groups: - Add mutuallyExclusiveWith field to Category type - Update CategoryDialog to allow selecting mutually exclusive categories - Implement bidirectional mutual exclusivity (if A excludes B, B also excludes A) - Update toggleImageCategory and assignImageCategory to automatically remove mutually exclusive categories when assigning - Support mutually exclusive categories in batch operations Refactor data storage architecture: - Move categories and hotkeys from app-config.json to directory-specific .hito.json files - Update HitoFile struct to include categories and hotkeys fields - Update loadHitoConfig and saveHitoConfig to handle categories and hotkeys - Replace all loadAppData/saveAppData calls with loadHitoConfig/saveHitoConfig - AppData now only stores data_file_paths mapping - Remove backward compatibility migration code Update tests: - Fix all test expectations to use saveHitoConfig instead of saveAppData - Update test mocks and state setup - Fix Rust test HitoFile initializations - Remove unnecessary skipped test Update documentation: - Update README to reflect directory-specific configuration - Document mutually exclusive categories feature
|
Note Reviews pausedUse the following commands to manage reviews:
WalkthroughAdds bidirectional "mutually exclusive" category relationships and moves category/hotkey persistence from centralized app data to per-directory Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CategoryDialog
participant categories.ts
participant Rust
Note over CategoryDialog,categories.ts: Edit or add category (bidirectional setup)
User->>CategoryDialog: Open & select mutual exclusions
CategoryDialog->>CategoryDialog: Build direct + reverse exclusives set
User->>CategoryDialog: Save
CategoryDialog->>categories.ts: saveHitoConfig(directory, imageCategories, filename, categories, hotkeys)
categories.ts->>Rust: invoke save_hito_config
Rust->>Rust: persist `.hito.json` (image_categories, categories, hotkeys)
Rust-->>categories.ts: success
categories.ts-->>CategoryDialog: confirm saved
sequenceDiagram
participant User
participant ImageView
participant categories.ts
participant Rust
Note over categories.ts: Assigning category enforces mutual exclusivity
User->>ImageView: Assign category X to image
ImageView->>categories.ts: assignImageCategory(image, X)
categories.ts->>categories.ts: compute X's mutuallyExclusiveWith
categories.ts->>categories.ts: remove conflicting assigned categories (forward+reverse)
categories.ts->>categories.ts: update assignment
categories.ts->>Rust: save_hito_config(... imageCategories, categories, hotkeys)
Rust-->>categories.ts: persisted
categories.ts-->>ImageView: updated state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #40 +/- ##
==========================================
- Coverage 94.46% 94.35% -0.11%
==========================================
Files 19 19
Lines 3358 3456 +98
Branches 568 597 +29
==========================================
+ Hits 3172 3261 +89
- Misses 186 195 +9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/core/browse.ts (1)
96-103: loadHitoConfig is skipped for empty directories, which can leak config from the previous directoryBecause
browseImagesreturns early whenimages.length === 0 && directories.length === 0, the newawait loadHitoConfig()is never run for empty directories, andallImagePathsAtom/allDirectoryPathsAtomare left unchanged. This means:
- Any
.hito.jsonfor an empty directory won’t be loaded.- Categories/hotkeys from the previously browsed directory can remain active while the UI shows the new path and “No images or directories found”.
Consider loading the config (and clearing arrays) before the early return, e.g.:
- const contents = await invokeTauri<DirectoryContents>("list_images", { path }); - - // Store directories and images - ensure they are arrays - const directories = Array.isArray(contents.directories) ? contents.directories : []; - const images = Array.isArray(contents.images) ? contents.images : []; - - if (images.length === 0 && directories.length === 0) { - showNotification("No images or directories found in this directory."); - store.set(isLoadingAtom, false); - return; - } + const contents = await invokeTauri<DirectoryContents>("list_images", { path }); + + // Store directories and images - ensure they are arrays + const directories = Array.isArray(contents.directories) ? contents.directories : []; + const images = Array.isArray(contents.images) ? contents.images : []; + + // Load image category assignments, categories, and hotkeys for this directory, + // even if there are no images yet. + await loadHitoConfig(); + + if (images.length === 0 && directories.length === 0) { + showNotification("No images or directories found in this directory."); + store.set(allDirectoryPathsAtom, []); + store.set(allImagePathsAtom, []); + store.set(isLoadingAtom, false); + return; + } @@ - store.set(allDirectoryPathsAtom, directories); - store.set(allImagePathsAtom, images); - - // Load image category assignments, categories, and hotkeys for this directory - await loadHitoConfig(); + store.set(allDirectoryPathsAtom, directories); + store.set(allImagePathsAtom, images);This keeps per-directory configuration consistent even when the directory is empty.
Also applies to: 110-121
src/ui/categories.ts (1)
761-788:deleteCategorysaves before updating categories/hotkeys (persistence bug)Right now
deleteCategorycallssaveHitoConfig()before updatinghotkeysAtomandcategoriesAtom. SincesaveHitoConfignow also persists categories/hotkeys, the.hito.jsonwill continue to contain the deleted category and old hotkeys, even though in‑memory state has been updated. On the next load, stale categories/hotkeys will come back.You should move the
saveHitoConfigcall to after all three atoms (imageCategoriesAtom,hotkeysAtom,categoriesAtom) have been updated, or call it a second time at the end and remove the earlier call.export async function deleteCategory(categoryId: string): Promise<void> { // ...build updatedImageCategories... - store.set(imageCategoriesAtom, updatedImageCategories); - - // Save the updated image category assignments to .hito.json - await saveHitoConfig(); + store.set(imageCategoriesAtom, updatedImageCategories); // Clean up hotkeys that reference this category const hotkeys = store.get(hotkeysAtom); const updatedHotkeys = hotkeys.map((hotkey) => { // ... }); store.set(hotkeysAtom, updatedHotkeys); // Remove category const categories = store.get(categoriesAtom); store.set(categoriesAtom, categories.filter((c) => c.id !== categoryId)); - // Categories are now saved via saveHitoConfig (already called above) + // Persist updated image assignments, categories, and hotkeys to .hito.json + await saveHitoConfig(); }
🧹 Nitpick comments (5)
src/core/browse.test.ts (1)
40-43: Tests correctly assert per-directory config loading via loadHitoConfigMocking loadHitoConfig and asserting it’s called in the “process valid directory contents” test matches the new browseImages behavior. You might optionally add
expect(loadAppData).not.toHaveBeenCalled()here (or drop the loadAppData mock entirely) to guard against regressions back to the old API, but the current assertions are functionally sound.Also applies to: 246-247
src/ui/categories.ts (3)
140-155: Saving default hotkeys viasaveHitoConfigfromloadAppDataUsing
saveHitoConfig()here to persist newly initialized default hotkeys into the per‑directory.hito.jsonis reasonable and matches the migration towards directory configs. Behavior is guarded bygetDataFileDirectory()insidesaveHitoConfig, so it’s a no‑op when no directory is active.
496-557: Mutual exclusivity handling intoggleImageCategorylooks correctThe added logic:
- Looks up the category being assigned;
- Collects IDs to remove from both sides of the relation (the new category’s
mutuallyExclusiveWithand any already‑assigned categories whosemutuallyExclusiveWithincludes the new ID);- Filters out those assignments before adding the new one.
This enforces bidirectional exclusivity and also behaves correctly in asymmetric configs (legacy or hand‑edited). No functional issues found.
A small helper like
resolveMutuallyExclusiveAssignments(currentAssignments, categoryId, categories)used by bothtoggleImageCategoryandassignImageCategorywould remove the duplicated block and keep the logic in one place.
583-635:assignImageCategorymutual exclusivity mirrors toggle behaviorThe assign‑only path reuses the same exclusivity resolution (compute
categoriesToRemove, filter, then append new assignment) before persisting and handling navigation, which keeps semantics consistent withtoggleImageCategory. This is good for predictability.Same as above: consider extracting the shared exclusivity‑resolution into a helper to avoid future divergence between toggle/assign.
src/components/CategoryDialog.tsx (1)
137-187: Editing flow maintains bidirectional mutual exclusivity with rollbackThe editing branch:
- Computes
previousExclusiveSetvsnewExclusiveSet;- Updates the edited category’s
mutuallyExclusiveWith(usingundefinedfor empty to keep JSON lean);- Adds reverse links on newly added exclusives;
- Removes reverse links where exclusivity was removed;
- Updates
categoriesAtomoptimistically and rolls back tooriginalCategoriesifsaveHitoConfigfails.This satisfies the bidirectional invariant while keeping persistence and error handling tidy.
You could extract the “update bidirectional exclusivity for edited category” into a pure helper to reuse in tests or other admin flows, but it’s fine inlined for now.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
README.md(5 hunks)src-tauri/src/lib.rs(10 hunks)src/App.tsx(1 hunks)src/components/CategoryDialog.tsx(8 hunks)src/components/HotkeyDialog.tsx(2 hunks)src/core/browse.test.ts(1 hunks)src/core/browse.ts(2 hunks)src/styles.css(1 hunks)src/types.ts(1 hunks)src/ui/categories.test.ts(2 hunks)src/ui/categories.ts(9 hunks)src/ui/hotkeys.test.ts(9 hunks)src/ui/hotkeys.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursorrules)
Use soft tabs (spaces) for indentation, never hard tabs, following the existing codebase style of typically 2 spaces for TypeScript/React files
Files:
src/types.tssrc/core/browse.test.tssrc/App.tsxsrc/components/HotkeyDialog.tsxsrc/core/browse.tssrc/ui/hotkeys.tssrc/ui/categories.test.tssrc/ui/hotkeys.test.tssrc/components/CategoryDialog.tsxsrc/ui/categories.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
When calling Tauri commands from TypeScript, use
camelCasefor parameters (e.g.,dataFilePath: value). Do not use snake_case in TypeScript
Files:
src/types.tssrc/core/browse.test.tssrc/App.tsxsrc/components/HotkeyDialog.tsxsrc/core/browse.tssrc/ui/hotkeys.tssrc/ui/categories.test.tssrc/ui/hotkeys.test.tssrc/components/CategoryDialog.tsxsrc/ui/categories.ts
**/*.rs
📄 CodeRabbit inference engine (.cursorrules)
When defining Tauri commands in Rust, use
snake_casefor parameters (e.g.,data_file_path: String). Tauri automatically converts to camelCase in TypeScript
Files:
src-tauri/src/lib.rs
🧬 Code graph analysis (7)
src/components/HotkeyDialog.tsx (1)
src/ui/categories.ts (1)
saveHitoConfig(285-327)
src/ui/hotkeys.ts (1)
src/ui/categories.ts (1)
saveHitoConfig(285-327)
src-tauri/src/lib.rs (1)
src/types.ts (1)
CategoryAssignment(44-47)
src/ui/categories.test.ts (2)
src/utils/jotaiStore.ts (1)
store(6-6)src/state.ts (2)
categoriesAtom(14-14)hotkeysAtom(12-12)
src/ui/hotkeys.test.ts (1)
src/ui/categories.ts (1)
saveHitoConfig(285-327)
src/components/CategoryDialog.tsx (2)
src/ui/categories.ts (1)
saveHitoConfig(285-327)src/types.ts (1)
Category(37-42)
src/ui/categories.ts (4)
src/types.ts (2)
Category(37-42)HotkeyConfig(30-35)src/utils/jotaiStore.ts (1)
store(6-6)src/state.ts (3)
imageCategoriesAtom(15-15)categoriesAtom(14-14)hotkeysAtom(12-12)src/utils/tauri.ts (1)
invokeTauri(18-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: codecov
🔇 Additional comments (32)
src/styles.css (1)
2420-2476: Mutually-exclusive category styles look consistent with existing dialog UILayout, spacing, and colors align with the rest of the dark category dialog; scrollable list and empty state are handled cleanly. No functional issues from these styles.
src/ui/hotkeys.ts (1)
5-5: Hotkey persistence correctly routed through saveHitoConfigSwitching deleteHotkey and autoAssignHotkeyToCategory to await saveHitoConfig aligns these flows with the new per-directory config model, and error handling matches the expectations in src/ui/hotkeys.test.ts (rollback only for auto-assign, user-facing error for delete). No further changes needed here.
Also applies to: 191-198, 336-355
src/types.ts (1)
37-42: Category.mutuallyExclusiveWith extension is type-safe and backward compatibleAdding an optional
mutuallyExclusiveWith?: string[]on Category cleanly models mutual exclusivity without breaking existing data or callers that don’t know about it.src/components/HotkeyDialog.tsx (1)
6-6: HotkeyDialog save now persists via saveHitoConfig with reasonable error handlingUsing
saveHitoConfighere correctly routes hotkey edits into the per-directory config pipeline, and the try/catch with an inline error message provides a clear failure mode without breaking local state.Also applies to: 216-222
src/App.tsx (1)
34-36: Startup comment now accurately reflects per-directory config loadingThe revised comment in the App
useEffectmatches the new behavior where categories/hotkeys are loaded vialoadHitoConfigwhen browsing a directory, rather than via an app-wideloadAppDataon startup.src/ui/hotkeys.test.ts (1)
15-17: Tests correctly exercise saveHitoConfig integration for hotkeysMocking
saveHitoConfigand updating expectations in the deleteHotkey and autoAssignHotkeyToCategory suites line up with the new implementation: success paths assert thatsaveHitoConfigis invoked, and failure paths verify logging, user-facing errors, and rollback behavior. This gives good coverage of the new persistence wiring.Also applies to: 1079-1083, 1207-1208, 1232-1233, 1266-1267, 1320-1321
README.md (5)
31-31: Mutually exclusive categories docs align with implementationDescription here matches the CategoryDialog UI and the toggle/assign logic that automatically removes mutually exclusive categories per image. No issues.
53-57: Per‑directory .hito.json description is consistentThese bullets correctly describe storing image assignments, categories, and hotkeys in per‑directory
.hito.jsonplus separate app‑data mappings for custom paths. Reads clearly and matches the code.
131-131: Category creation section correctly explains mutual exclusivityThe behavior explained here (A excludes B ⇒ assigning A removes B and vice versa) matches the enforced logic in
toggleImageCategory/assignImageCategory.
158-158: Note about directory‑specific categories/hotkeys is accurateThis note reflects the per‑directory persistence model in
saveHitoConfig/loadHitoConfig. No action needed.
182-187: Automatic category hotkeys description matches behaviorOrdering and constraints (1‑9 then 0, only unassigned bare number keys, editable/removable) align with the hotkey auto‑assignment logic and tests.
src/ui/categories.ts (3)
21-30: Extending HitoFile/AppData with categories and hotkeys is consistentAdding optional
categories?: Category[]andhotkeys?: HotkeyConfig[](plusAppDatamirrors) lines up with the RustHitoFile/AppDatastructs and the new per‑directory persistence path. No functional issues.
222-260: Loading categories/hotkeys from.hito.jsonand defaulting hotkeysThe extended
loadHitoConfiglogic correctly:
- Logs counts for debugging;
- Hydrates
categoriesAtomandhotkeysAtomwhen present;- Normalizes hotkeys (id/key/modifiers/action);
- Initializes default hotkeys and saves via
saveHitoConfigwhen none are present.The file‑not‑found branch only clears
imageCategoriesAtom, intentionally leaving categories/hotkeys to act as a fallback, which is consistent with the tests.
300-320: Including categories and hotkeys insaveHitoConfigpayloadFetching
categories/hotkeysfrom atoms, logging counts, and only sending them when non‑empty keeps the.hito.jsonlean while still persisting configuration. This matches the Rustsave_hito_configsignature that takes optional categories/hotkeys.src/components/CategoryDialog.tsx (7)
11-14: Switch tosaveHitoConfigis consistent with per‑directory persistenceUsing
saveHitoConfigfrom the dialog keeps category edits aligned with the new.hito.json‑centric model and avoids touching app‑wideapp-config.jsonfrom here. Good move.
37-40: LocalmutuallyExclusiveWithstate is appropriateTracking
mutuallyExclusiveWithin component state as a string[] mirrors theCategoryshape and simplifies the checkbox UI. No issues.
52-68: On-open initialization merges direct and reverse exclusivity correctlyBuilding
allExclusiveas the union of:
editingCategory.mutuallyExclusiveWith, and- Any categories whose
mutuallyExclusiveWithalready includes the current category IDensures the dialog represents the full bidirectional relation, even if the underlying data wasn’t perfectly symmetric before. This is exactly what you want for cleanup/migration.
80-85: ResettingmutuallyExclusiveWithon close avoids stale selectionsClearing the exclusivity list alongside name/color/error state when the dialog closes prevents leakage of previous selections into the next open. Looks correct.
210-241: New category flow correctly wires up exclusivity and persistenceFor adds:
- A stable UUID is generated (preferring
crypto.randomUUID).- The new category’s
mutuallyExclusiveWithis set from local state (orundefinedwhen empty).- Existing categories referenced by
mutuallyExclusiveWithare patched to include the new ID.- State is updated optimistically, saved via
saveHitoConfig, and rolled back on failure.- Hotkey auto‑assignment runs after a successful save and is treated as best‑effort.
This behavior matches the feature description.
274-283: IncludingmutuallyExclusiveWithinhandleSavedependenciesAdding
mutuallyExclusiveWithto the dependency array ensures the callback sees the current selection of exclusive categories. Correct and necessary.
362-404: Mutually exclusive checkbox UI is clear and robustThe UI:
- Lists all other categories (excluding the one being edited) with colored indicators;
- Toggles membership in
mutuallyExclusiveWithvia checkbox;- Shows a “No other categories available” message when appropriate.
This aligns with the README description and the underlying data model.
src/ui/categories.test.ts (3)
417-418: ExplicitcategoriesAtomreset in UI testsResetting
categoriesAtomexplicitly before invokingresetStateAtomin the “categories UI and management” suite is harmless and makes the initial state for these tests explicit. AssumingresetStateAtommay touch other atoms, this is fine.
1240-1252:saveHitoConfigerror‑handling test updated for new payloadAsserting that
save_hito_configreceivescategories: store.get(categoriesAtom)andhotkeys: store.get(hotkeysAtom)aligns with the updatedsaveHitoConfigimplementation (which includes these when non‑empty). This keeps the Rust/TS contract tested.
2774-2790:loadAppDatatest now verifies default hotkeys are persistedThe test case expecting:
- two default hotkeys (J/K), and
- at least one
save_hito_configcallmatches the new behavior where
loadAppDatainitializes defaults and then persists them viasaveHitoConfig. This is consistent with the production code.src-tauri/src/lib.rs (8)
391-398:CategoryDatamutual exclusivity field matches frontend modelAdding
#[serde( default, skip_serializing_if = "Option::is_none", rename = "mutuallyExclusiveWith" )] mutually_exclusive_with: Option<Vec<String>>,keeps JSON shape (
mutuallyExclusiveWith) aligned with the TSCategoryinterface and safely defaults toNonefor old configs. Good use of serde attributes.
424-432: ExtendingHitoFilewith optional categories/hotkeys is soundIncluding
categories: Option<Vec<CategoryData>>andhotkeys: Option<Vec<HotkeyData>>(withdefault+skip_serializing_if) allows.hito.jsonto grow to carry config while remaining backward compatible with files that only haveimage_categories.
435-440: DerivingDefaultforAppDatafits synchronized app-config updatesDeriving
Defaultsimplifies initialization inupdate_app_data_sync/load_app_dataand preserves existing semantics (empty categories/hotkeys, optional data_file_paths). No issues.
609-618:load_hito_configdefault return updated for new fieldsWhen the
.hito.jsonfile is absent, returningHitoFile { image_categories: Vec::new(), categories: None, hotkeys: None, }is compatible with TS expectations and keeps optionals unset rather than forcing empty arrays.
631-655:save_hito_configsignature and body match TS callerThe updated command:
fn save_hito_config( directory: String, image_categories: Vec<(String, Vec<CategoryAssignment>)>, filename: Option<String>, categories: Option<Vec<CategoryData>>, hotkeys: Option<Vec<HotkeyData>>, )and the corresponding
HitoFileconstruction align with the TSsaveHitoConfigpayload (imageCategories, categories?, hotkeys?). Serialization usesto_string_prettyand writes atomically to the requested path; behavior is correct.
1608-1610: HitoFile serialization test updated for optional fieldsExplicitly setting
categories: Noneandhotkeys: Nonein tests ensures serialization/deserialization remains stable when those fields are absent in JSON, which is important for backward compatibility.
1628-1634: AppData serialization covers newmutually_exclusive_withfieldIncluding
mutually_exclusive_with: Nonein theCategoryDataused intest_app_data_serializationkeeps that path tested with the extended struct and confirms serde handling for the renamed optional field.
1786-1792:save_hito_configtests updated for new parametersTests that pass
Nonefor the newcategories/hotkeysparameters in both default and custom‑filename cases correctly exercise the extended signature while preserving prior behavior for “image categories only” scenarios.
Fix empty directory config loading: - Move loadHitoConfig() before early return in browseImages - Clear arrays when directory is empty to prevent config leakage - Ensures per-directory configuration is loaded even for empty directories Fix deleteCategory persistence bug: - Move saveHitoConfig() call to after all atoms are updated - Ensures categories, hotkeys, and image assignments are all saved correctly - Prevents stale data from persisting after category deletion Add comprehensive tests: - Add tests for mutually exclusive categories feature - Add test for deleteCategory saving all three atoms correctly - Add test for empty directory config loading - All 164 category tests and 19 browse tests pass
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/ui/categories.test.ts (1)
2920-2936: Make loadAppData tests independent of prior stateYou now assert that creating default hotkeys results in a
save_hito_configcall, which goes throughsaveHitoConfig()and depends oncurrentDirectoryAtombeing non-empty. Currently that relies on whatever previous describe left in the store; consider settingcurrentDirectoryAtom(and optionallydataFilePathAtom) explicitly in this describe’sbeforeEachto avoid hidden coupling between tests.src/ui/categories.ts (2)
222-260: Consider clearing categories/hotkeys on missing .hito.json to avoid leakage
loadHitoConfig’s success path now correctly loadsimage_categories,categories, andhotkeysfrom.hito.json, and initializes default hotkeys when none are present, which fits the per-directory config model. However, in the ENOENT branch you only clearimageCategoriesAtom, leavingcategoriesAtomandhotkeysAtomuntouched. That means opening a directory with no.hito.jsonwill still show whatever categories/hotkeys were loaded for the previous directory, which conflicts with the goal of per-directory configuration and with the comment inbrowseImagesabout preventing config leakage.If the intended semantics are “no
.hito.json⇒ no per-directory categories/hotkeys”, consider also resetting those atoms in the file-not-found branch, for example:- if (isFileNotFound) { - // File doesn't exist - clear assignments - console.log("[loadHitoConfig] Data file not found, clearing assignments"); - store.set(imageCategoriesAtom, new Map()); - } else { + if (isFileNotFound) { + // File doesn't exist - clear per-directory data + console.log("[loadHitoConfig] Data file not found, clearing assignments"); + store.set(imageCategoriesAtom, new Map()); + store.set(categoriesAtom, []); + store.set(hotkeysAtom, []); + } else { // Other errors (permission, parse, network, etc.) - log and rethrow console.error("[loadHitoConfig] Failed to load .hito.json:", error); throw error; }Please confirm whether you want globals from
loadAppDatato persist in this case, or if this stricter per-directory isolation is preferred.
500-557: Mutual-exclusion logic in toggleImageCategory is correct but duplicatedThe logic to resolve mutually exclusive categories—finding the category being assigned, scanning both its
mutuallyExclusiveWithlist and the assigned categories’ lists, buildingcategoriesToRemove, then filtering and appending the new assignment—looks sound and is well-covered by the new tests.The same block is effectively duplicated in
assignImageCategory. Consider extracting a small helper such as:function applyMutualExclusion( currentAssignments: CategoryAssignment[], categoryId: string, categories: Category[], ): CategoryAssignment[] { /* shared logic */ }and using it from both functions. That would reduce the cognitive load and the risk of future divergence between the two paths.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
package.json(1 hunks)src-tauri/Cargo.toml(1 hunks)src/core/browse.test.ts(1 hunks)src/core/browse.ts(2 hunks)src/ui/categories.test.ts(3 hunks)src/ui/categories.ts(8 hunks)
✅ Files skipped from review due to trivial changes (1)
- src-tauri/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (1)
- src/core/browse.test.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursorrules)
Use soft tabs (spaces) for indentation, never hard tabs, following the existing codebase style of typically 2 spaces for TypeScript/React files
Files:
src/ui/categories.test.tssrc/ui/categories.tssrc/core/browse.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
When calling Tauri commands from TypeScript, use
camelCasefor parameters (e.g.,dataFilePath: value). Do not use snake_case in TypeScript
Files:
src/ui/categories.test.tssrc/ui/categories.tssrc/core/browse.ts
🧬 Code graph analysis (2)
src/ui/categories.test.ts (3)
src/state.ts (5)
categoriesAtom(14-14)hotkeysAtom(12-12)currentDirectoryAtom(16-16)dataFilePathAtom(17-17)imageCategoriesAtom(15-15)src/utils/dialog.ts (1)
confirm(15-104)src/ui/categories.ts (3)
deleteCategory(735-786)toggleImageCategory(496-578)assignImageCategory(583-655)
src/ui/categories.ts (4)
src/types.ts (2)
Category(37-42)HotkeyConfig(30-35)src/utils/jotaiStore.ts (1)
store(6-6)src/state.ts (3)
imageCategoriesAtom(15-15)categoriesAtom(14-14)hotkeysAtom(12-12)src/utils/tauri.ts (1)
invokeTauri(18-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: codecov
🔇 Additional comments (8)
package.json (1)
4-4: Version bump aligns with new feature setIncrementing to
0.3.6is consistent with the scope of changes; scripts already coordinate version syncing, so this change is fine as-is.src/core/browse.ts (1)
99-110: Per-directory config load ordering looks goodCalling
await loadHitoConfig()after listing contents but before the empty-directory early-return ensures.hito.json(including categories/hotkeys) is loaded even when there are no images, and the additional clearing ofallDirectoryPathsAtom,allImagePathsAtom, andcurrentIndexAtomon the empty case keeps browsing state consistent. No issues spotted with this flow.src/ui/categories.test.ts (3)
1240-1252: saveHitoConfig payload expectation matches new contractAsserting that
save_hito_configreceivescategoriesandhotkeysalongsideimageCategorieswhen they’re non-empty matches the updatedsaveHitoConfigbehavior. OncesaveHitoConfigis adjusted to omit these keys entirely when arrays are empty (rather than sendingundefined), this and the “empty categories” test will both be satisfied.
2766-2795: Good coverage of persistence after deleteCategoryThis test usefully verifies that
deleteCategorynot only updates in-memorycategoriesAtomandhotkeysAtom(clearing actions that referenced the deleted category) but also persists the updated arrays viasave_hito_config. The expected payload shape matches the newsaveHitoConfigcontract.
2799-2911: Mutually exclusive categories tests are thoroughThese tests exercise both
toggleImageCategoryandassignImageCategoryacross key scenarios: one-to-one, bidirectional, one-to-many mutual exclusion, and ensuring non-exclusive categories (like a “tagged” category) are preserved. They line up with the mutual-exclusion logic incategories.tsand should catch regressions.src/ui/categories.ts (3)
141-154: Confirm saving default hotkeys via saveHitoConfig is always meaningfulSwitching
loadAppDatato persist newly created default hotkeys throughsaveHitoConfig()aligns with the move to.hito.json. Just be aware this call is a no-op whengetDataFileDirectory()returns an empty string (no current directory / data file path yet), which is a likely situation at app startup. If you rely on this initial save, ensurecurrentDirectoryAtom/dataFilePathAtomare set beforeloadAppDatais used; otherwise, you’re effectively deferring the first write until the next operation that callssaveHitoConfig.
587-635: assignImageCategory mutual-exclusion behavior matches toggleImageCategoryThe mutual-exclusion handling here mirrors
toggleImageCategory(checking both the assigned category’smutuallyExclusiveWithand the existing assignments’ lists), ensuring consistent behavior whether the caller is “toggle” or “assign only”. Given the new tests, this looks correct and complete.
784-785: Persisting after deleteCategory aligns with per-directory configCalling
await saveHitoConfig()after updating image assignments, categories, and hotkeys ensures.hito.jsonstays in sync with deletions, which is consistent with the new per-directory persistence strategy and validated by the added test incategories.test.ts.
|
@coderabbitai pause |
✅ Actions performedReviews paused. |
… payload Update saveHitoConfig to build payload object conditionally: - Only include categories field when categories.length > 0 - Only include hotkeys field when hotkeys.length > 0 - Only include filename field when it exists - Base payload always includes directory and imageCategories - Console.log still prints all counts for debugging - Actual payload sent omits empty optional fields Add test to verify empty arrays are omitted from payload
iomz
deleted the
feat/mutually-exclusive-categories-and-directory-specific-config
branch
1 participant
Description
This PR implements mutually exclusive category groups and refactors the data storage architecture to use directory-specific configuration files.
Features
Mutually Exclusive Categories
Directory-Specific Configuration
.hito.jsonfiles (directory-specific)app-config.jsonnow only stores data file path mappingsChanges
mutuallyExclusiveWithfield to Category typeTesting
Resolves #22
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.