feat(tags): tag system with explorer integration, and media context menu fixes

[slvnlrt]

Summary

Complete implementation of the tag system: backend handlers, search integration, explorer view, and context menu actions. Also fixes the broken media processing context menu (useJobDispatch → direct mutations).

Tags backend

• TagFilter in search.files: find_entry_ids_for_tag() + resolve_tag_filter() in query.rs
• New handlers: tags.by_id, tags.ancestors, tags.children, files.by_tag (returns full Vec<File>)
• Unapply/delete actions: tags.unapply (batch remove tags from entries), tags.delete (delete tag)
• Unique constraint migration: prevents duplicate tag applications on the same file
• Device slug fix: files.by_tag and search.files use get_current_device_slug() instead of "unknown-device" fallback

Tags frontend

• Tag view in Explorer: standard explorer with full File objects (grid/list views, context menu, selection)
• Inspector UX: tag apply/remove from file inspector
• Sidebar sync: TagsGroup refetches on tag changes
• Shared hook: useRefetchTagQueries for consistent cache invalidation
• Overview search button: navigates to /explorer from Overview

Media context menu fix

The context menu's media processing actions (Regenerate Thumbnail, Extract Text, etc.) were broken — they called jobs.dispatch, a generic RPC endpoint that doesn't exist.

Fix: replaced with calls to the existing single-file action endpoints (media.thumbnail.regenerate, media.ocr.extract, etc.). These actions call the ThumbnailProcessor/OcrProcessor directly, which is the intended pattern for UI-triggered single-file operations. All media processors follow this same dual architecture:

• Watcher path (automatic): file change → processor runs reactively
• Single-file action (user-triggered, what we use here): immediate execution with result
• Bulk job (batch): ThumbnailJob etc. for library-wide operations with progress/persistence

useJobDispatch.ts is now dead code (zero imports).

Merge note

This PR and #3041 both modify useFileContextMenu.ts. If both are merged, the second will need a minor rebase (2-3 lines in the imports/hooks section). No functional conflict.

Files changed (34 files, +1603/-468)

Backend (17 files): query.rs, files_by_tag.rs, by_id.rs, ancestors.rs, children.rs, apply/, unapply/, delete/, create/, metadata/manager.rs, migration, tags/mod.rs

Frontend (17 files): useFileContextMenu.ts, useExplorerFiles.ts, context.tsx, ExplorerView.tsx, tag/index.tsx, FileInspector.tsx, TagsGroup.tsx, TagSelector.tsx, Tag.tsx, useRefetchTagQueries.ts, OverviewTopBar.tsx, generated/types.ts

Test plan

• Apply a tag to files, verify no duplicates
• Navigate to a tag in sidebar → verify files displayed in standard explorer view
• Delete/rename/open files from tag view
• Unapply tag from context menu in tag view
• Delete a tag → verify removed from sidebar and files
• Right-click image → Image Processing → Regenerate Thumbnail (verify mutation works)
• Right-click video → Video Processing submenu (verify all actions)
• macOS/Linux: verify no regression

[coderabbitai]

Walkthrough

Adds tag-management queries and actions (ancestors, by_id, children, files_by_tag, delete, unapply), UUID-based entry targeting for tag application, a migration to dedupe and enforce unique (user_metadata_id, tag_id), metadata upsert refactor, search tag-filtering, thumbnail MIME fallback, transactional tag deletion, explorer tag mode, and frontend tag UI/hooks/integration.

Changes

Cohort / File(s)	Summary
Database Migration core/src/infra/db/migration/m20260125_000001_unique_user_metadata_tag.rs, core/src/infra/db/migration/mod.rs	Adds migration that deletes duplicate user_metadata_tag pairs and creates a unique composite index idx_umt_unique_pair; registers the migration.
Tag Queries (New) core/src/ops/tags/ancestors.rs, core/src/ops/tags/by_id.rs, core/src/ops/tags/children.rs, core/src/ops/tags/files_by_tag.rs	Adds library queries tags.ancestors, tags.by_id, tags.children, and files.by_tag with DB traversal, tag resolution, and registration; files.by_tag performs complex joins and per-entry tag merging.
Tag Delete (New) core/src/ops/tags/delete/... core/src/ops/tags/delete/action.rs, .../input.rs, .../output.rs, .../mod.rs	Introduces tags.delete action with input validation, computation of affected entries, transactional deletion via TagManager::delete_tag, and resource event emission.
Tag Unapply (New) core/src/ops/tags/unapply/... core/src/ops/tags/unapply/action.rs, .../input.rs, .../output.rs, .../mod.rs	Adds tags.unapply action to remove tag associations from entries with validation, scoped deletes, event emission, and result counts/warnings.
Tag Module Exports core/src/ops/tags/mod.rs	Re-exports new tag submodules and types (ancestors, by_id, children, delete, files_by_tag, unapply).
UUID-based Entry Targeting core/src/ops/tags/apply/input.rs, core/src/ops/tags/apply/action.rs, core/src/ops/tags/create/input.rs, core/src/ops/tags/create/action.rs	Adds EntryUuid(Vec<Uuid>) variants and flows to apply tags directly to entries by UUID; updates validation and application branches.
Metadata Upsert Refactor core/src/ops/metadata/manager.rs	Replaces insert-then-fetch control flow with a single INSERT ... ON CONFLICT(user_metadata_id, tag_id) DO UPDATE upsert and returns the final row.
Search Tag Filtering core/src/ops/search/query.rs	Adds include/exclude tag filtering to FTS and non-FTS search flows; introduces find_entry_ids_for_tag and resolve_tag_filter, applying filters before pagination.
Thumbnail MIME Fallback core/src/ops/media/thumbnail/action.rs	Adds mime_from_extension helper and falls back to extension-based MIME when content-identity lookup fails.
Tag Manager Changes core/src/ops/tags/manager.rs	Runs delete_tag inside a DB transaction with adjusted error mapping and adds get_direct_children helper.
Frontend: Tag UI & Hooks packages/interface/src/components/Inspector/primitives/Tag.tsx, packages/interface/src/hooks/useRefetchTagQueries.ts	Adds optional onRemove prop to Tag component and a useRefetchTagQueries hook to refetch related queries after tag mutations.
Frontend: File Inspector & Tag Selector packages/interface/src/components/Inspector/variants/FileInspector.tsx, packages/interface/src/components/Tags/TagSelector.tsx	Switches tag apply payloads to EntryUuid, wires tags.unapply for removals, and triggers tag-query refetch after mutations.
Frontend: Tags Sidebar & Deletion packages/interface/src/components/SpacesSidebar/TagsGroup.tsx	Adds context-menu tag deletion (tags.delete) with confirmation, active-route styling, and navigation handling when deleting the active tag.
Frontend: Explorer Tag Mode packages/interface/src/routes/explorer/context.tsx, .../hooks/useExplorerFiles.ts, .../ExplorerView.tsx, packages/interface/src/routes/tag/index.tsx	Adds explorer tag mode (enter/exit APIs), makes it a high-priority file source using files.by_tag, adjusts render guard, and refactors TagView to delegate to ExplorerView.
Frontend: Context Menu & Media Ops packages/interface/src/routes/explorer/hooks/useFileContextMenu.ts	Replaces job-based bulk dispatch with per-entry mutations for media ops and adds a tag-removal menu item calling tags.unapply.
Overview Top Bar packages/interface/src/routes/overview/OverviewTopBar.tsx	Adds navigation handler to memoized search button and updates memo dependencies.

sequenceDiagram participant FE as Frontend (UI) participant API as Backend API participant Action as LibraryAction participant TagMgr as TagManager/MetadataManager participant DB as Database FE->>API: POST library action (tags.apply / tags.unapply) with EntryUuid API->>Action: dispatch action Action->>TagMgr: apply or unapply tags (resolve metadata, upsert/delete) TagMgr->>DB: SELECT / INSERT ON CONFLICT / DELETE (transactional) DB-->>TagMgr: result TagMgr-->>Action: success/failure Action-->>API: action result API-->>FE: response FE->>FE: refetch tag/file queries (useRefetchTagQueries) 

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

"I nibbled at duplicates and stitched UUID trails,
I hopped through queries, actions, and UI details,
I fenced pairs in the DB and tuned explorer's song,
Tags applied, removed, and fetched — the warren hums along 🐇"

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 45.28% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description check	✅ Passed	The PR description is comprehensive, covering backend implementation, frontend changes, media context menu fixes, files changed, test plan, and merge notes. It addresses all the key aspects of the changeset.
Title check	✅ Passed	The title 'feat(tags): tag system with explorer integration, and media context menu fixes' accurately and concisely summarizes the main changes: implementing a complete tag system with frontend explorer integration and fixing media context menu issues.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 15

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

packages/interface/src/routes/explorer/hooks/useFileContextMenu.ts (1)

355-388: ⚠️ Potential issue | 🟠 Major

Filter the selected set by media type before fan-out.

These handlers now dispatch one mutation per selected file, but the target list is only stripped of virtual entries. Right-clicking an image/video in a mixed selection can still send blurhash/OCR/proxy jobs for directories or incompatible files. Build each action’s target list from getTargetFiles() plus the matching getContentKind() / file.kind predicate before calling forEachTarget.

Also applies to: 403-467, 483-528, 544-574

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@packages/interface/src/routes/explorer/hooks/useFileContextMenu.ts` around lines 355 - 388, The handlers currently call getTargetFiles() and fan out mutations with forEachTarget without filtering by media type; update each action (those invoking regenerateThumbnail.mutateAsync, extractText.mutateAsync, proxy/generate jobs, etc.) to build the target list by taking getTargetFiles() and filtering by the appropriate content kind using getContentKind(file) or file.kind (e.g., only image/video for thumbnail/blurhash, only image/pdf for OCR, only files—not directories—for proxy) before passing to forEachTarget so incompatible entries are excluded. 

core/src/ops/search/query.rs (1)

203-229: ⚠️ Potential issue | 🟠 Major

Tag filters need to run before final pagination.

In fast/FTS mode, execute_fts5_search() has already applied LIMIT/OFFSET before the new retain() runs here, so a tag-filtered search can return short pages and skip valid tagged hits just beyond the first slice. get_total_count() also still ignores filters.tags, so pagination metadata won’t match the returned items. Push the tag constraint into the query/count path, or over-fetch and paginate after filtering.

Also applies to: 948-957

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@core/src/ops/search/query.rs` around lines 203 - 229, The FTS tag-filtering is applied after execute_fts5_search’s LIMIT/OFFSET causing truncated pages and mismatched get_total_count; modify the search path so tag constraints are applied before pagination by either (A) extending execute_fts5_search and get_total_count to accept the tags filter and include JOIN/WHERE clauses (or additional FTS query terms) so results and counts respect filters, or (B) over-fetch by calling execute_fts5_search without LIMIT/OFFSET (or with an increased fetch size), apply resolve_tag_filter and retain on fts_results, then apply pagination (slice OFFSET/LIMIT) and compute total from the filtered set; update usages of execute_fts5_search, resolve_tag_filter, fts_results, and get_total_count accordingly so both returned items and total count honor filters.tags. 

🧹 Nitpick comments (7)

core/src/ops/media/thumbnail/action.rs (2)

127-149: Database errors are silently swallowed in MIME resolution.

The use of .ok() on lines 128-131 and .ok().flatten() on lines 137-138 converts database errors into None, silently falling back to extension-based MIME detection. While the graceful degradation is reasonable for user-facing thumbnail regeneration, this could mask underlying database connectivity issues.

Consider at minimum logging a warning when the DB lookup fails unexpectedly:

♻️ Suggested improvement with tracing

	let mime_type = if let Some(content_id) = entry.content_id { -if let Ok(Some(ci)) = -entities::content_identity::Entity::find_by_id(content_id) -.one(db) -.await -{ +match entities::content_identity::Entity::find_by_id(content_id) +.one(db) +.await +{ +Ok(Some(ci)) => {	if let Some(mime_id) = ci.mime_type_id { -entities::mime_type::Entity::find_by_id(mime_id) +match entities::mime_type::Entity::find_by_id(mime_id)	.one(db)	.await -.ok() -.flatten() -.map(|m| m.mime_type) -.or_else(|| mime_from_extension(&path)) +{ +Ok(Some(m)) => Some(m.mime_type), +Ok(None) => mime_from_extension(&path), +Err(e) => { +tracing::warn!(?e, "Failed to fetch mime_type, falling back to extension"); +mime_from_extension(&path) +} +}	} else {	mime_from_extension(&path)	} -} else { -mime_from_extension(&path) +} +Ok(None) => mime_from_extension(&path), +Err(e) => { +tracing::warn!(?e, "Failed to fetch content_identity, falling back to extension"); +mime_from_extension(&path) +}	}	} else {	mime_from_extension(&path)	};

As per coding guidelines: "Use tracing macros (info!, debug!, warn!, error!) instead of println! for logging".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@core/src/ops/media/thumbnail/action.rs` around lines 127 - 149, The DB lookups in the mime_type resolution (the block that sets mime_type using entities::content_identity::Entity::find_by_id and entities::mime_type::Entity::find_by_id) are swallowing errors via .ok()/.flatten(), so change the logic to detect and log unexpected DB errors with tracing (e.g., warn! or error!) before falling back to mime_from_extension(&path); specifically, handle the Result from .one(db).await and .one(db).await for mime type explicitly, log the Err case with context (content_id or mime_id and path) using tracing macros, and only then return mime_from_extension(&path) on error or missing values while preserving the existing success path that extracts m.mime_type into mime_type. 

---

3-15: Optional: Reorganize imports per coding guidelines.

The imports are not grouped according to the guidelines (standard library → external crates → local modules with blank lines between groups). The std::path::PathBuf and std::sync::Arc imports should be grouped together at the top, followed by external crates (specta, uuid), then local modules.

♻️ Suggested import reorganization

-use super::{ -job::{ThumbnailJob, ThumbnailJobConfig}, -processor::ThumbnailProcessor, -}; -use crate::{ -context::CoreContext, -infra::action::{error::ActionError, LibraryAction}, -ops::indexing::{path_resolver::PathResolver, processor::ProcessorEntry}, -}; -use specta::Type; -use std::path::PathBuf; -use std::sync::Arc; -use uuid::Uuid; +use std::path::PathBuf; +use std::sync::Arc; + +use specta::Type; +use uuid::Uuid; + +use super::{ +job::{ThumbnailJob, ThumbnailJobConfig}, +processor::ThumbnailProcessor, +}; +use crate::{ +context::CoreContext, +infra::action::{error::ActionError, LibraryAction}, +ops::indexing::{path_resolver::PathResolver, processor::ProcessorEntry}, +};

As per coding guidelines: "Group imports with blank lines: standard library, external crates, then local modules".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@core/src/ops/media/thumbnail/action.rs` around lines 3 - 15, Reorder the import blocks in this file to follow the project guideline: group standard library imports together (move std::path::PathBuf and std::sync::Arc to a single std block at the top), then list external crates (specta::Type and uuid::Uuid) in a separate block, and finally the local/module imports (the super::{job::..., processor::ThumbnailProcessor} and crate::{context::CoreContext, infra::action::{error::ActionError, LibraryAction}, ops::indexing::{path_resolver::PathResolver, processor::ProcessorEntry}}) with a blank line between each group; adjust only the use statements near ThumbnailJob/ThumbnailProcessor and CoreContext so the symbols ThumbnailJob, ThumbnailJobConfig, ThumbnailProcessor, CoreContext, ActionError, LibraryAction, PathResolver, ProcessorEntry, Type, and Uuid are preserved but reordered into the three groups. 

core/src/ops/tags/ancestors.rs (1)

1-64: Split new tag queries into the standard ops module layout.

This file already mixes request types, response types, and execution logic together, and the sibling query files added in this PR repeat the same pattern. Keeping those pieces in separate per-feature files will be much easier to extend and test as the tag RPC surface grows.

As per coding guidelines "Feature modules in src/ops/ should contain action.rs, input.rs, output.rs, and job.rs files where applicable".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@core/src/ops/tags/ancestors.rs` around lines 1 - 64, The file mixes request/response types and execution logic; split into the ops module layout by moving GetTagAncestorsInput into input.rs, GetTagAncestorsOutput into output.rs, and the query implementation (GetTagAncestorsQuery, its impl including execute, and the crate::register_library_query! call) into action.rs (create job.rs only if background work is needed), then update the module (mod.rs) to pub use those symbols so callers still import GetTagAncestorsQuery, GetTagAncestorsInput, and GetTagAncestorsOutput; ensure the execute method and references to CoreContext, TagManager, and session remain identical when moved. 

core/src/ops/tags/unapply/output.rs (1)

7-10: Add rustdoc on the public output type and fields.

Please document the public contract here with /// comments (including a brief example) to match core module documentation standards.

Proposed doc update

 #[derive(Debug, Clone, Serialize, Deserialize, Type)] +/// Result payload for `tags.unapply`. +/// +/// Example: removing two tags from three selected entries can return +/// `entries_affected = 3` and `tags_removed = 6`. pub struct UnapplyTagsOutput { +/// Number of unique entries impacted by the unapply operation.	pub entries_affected: usize, +/// Total number of tag-entry associations removed.	pub tags_removed: usize, +/// Non-fatal issues encountered while processing specific targets.	pub warnings: Vec<String>, }

As per coding guidelines, core/**/*.rs: "Use //! for module documentation, /// for public items, and include examples".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@core/src/ops/tags/unapply/output.rs` around lines 7 - 10, Add Rustdoc comments to the public struct UnapplyTagsOutput and each of its public fields (entries_affected, tags_removed, warnings) describing their meaning and expected units/types; include a short example usage showing how to inspect the output (e.g., call an unapply function and check entries_affected/tags_removed) and any guarantees (e.g., warnings contain human-readable messages). Use /// comments above the struct and above each field to match core module standards and include a brief example snippet in the struct-level doc. 

packages/interface/src/components/Tags/TagSelector.tsx (1)

44-50: Remove the any escape hatch from the tag-search projection.

This is on the cache-normalization path, so a tags.search response-shape drift will compile and fail at runtime. Please bind useNormalizedQuery to the generated output type and narrow result instead of casting both values to any.

As per coding guidelines, "Never use any type in TypeScript - use unknown with type guards if needed".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@packages/interface/src/components/Tags/TagSelector.tsx` around lines 44 - 50, The select projection for useNormalizedQuery bound to 'tags.search' uses any for both the response and result which bypasses TS checks; update the useNormalizedQuery call to be generically typed with the generated response type for 'tags.search' and replace the any casts in the select callback by narrowing the union (e.g., type-guard or discriminant check) on the result so you safely access result.tag when present and otherwise return result, ensuring the select returns the filtered array (refer to useNormalizedQuery, the select callback, the 'tags.search' query and the result.tag shape). 

packages/interface/src/routes/explorer/hooks/useExplorerFiles.ts (1)

146-150: Type the files.by_tag query instead of casting its data to any.

Tag mode now feeds the main explorer file list, so this unchecked cast removes compile-time protection for one of the core data sources. useNormalizedQuery already uses explicit generics earlier in this hook; doing the same here would let you read tagQuery.data.files directly without any.

As per coding guidelines, "Never use any type in TypeScript - use unknown with type guards if needed".

Also applies to: 181-195

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@packages/interface/src/routes/explorer/hooks/useExplorerFiles.ts` around lines 146 - 150, The tagQuery call uses an unchecked any cast for the "files.by_tag" response; replace that by supplying the correct generic type to useNormalizedQuery so tagQuery is strongly typed (e.g., the response shape that includes files), remove the any casts where tagQuery.data.files is accessed, and update the other occurrences around the hook (the similar usage at the block referenced as 181-195) to use the same typed useNormalizedQuery so you can read tagQuery.data.files directly with compile-time safety. 

packages/interface/src/components/SpacesSidebar/TagsGroup.tsx (1)

120-126: Remove the any escape hatch from the tag query select.

tags.search already has a generated result shape, so this TODO throws away the type-safety the hook is supposed to provide and will hide schema drift in the sidebar. Type the callback against the generated search types (or use unknown + a guard) and drop the anys.

As per coding guidelines Never use any type in TypeScript - use unknown with type guards if needed.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@packages/interface/src/components/SpacesSidebar/TagsGroup.tsx` around lines 120 - 126, The select callback for useNormalizedQuery (query: 'tags.search') uses `any`, removing type-safety; replace the `any` types in the select parameter with the generated response type for the tags.search endpoint (or use `unknown` and add a runtime type guard) and update the select body to use that typed shape to extract `data.tags[].tag` (or the fallback) without `any`; update or remove the // TODO comment and ensure the signature of the select callback matches the generated types used by useNormalizedQuery. 

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed. Inline comments: In `@core/src/infra/db/migration/m20260125_000001_unique_user_metadata_tag.rs`: - Around line 11-18: The current dedupe SQL deletes all but MIN(id) in user_metadata_tag, which preserves the oldest row and loses newer mutable state; change the logic to keep the newest tag application instead—use MAX(id) (or, if available, pick the row with greatest version or latest updated_at) in the subquery that groups by user_metadata_id, tag_id so the DELETE removes rows NOT IN (SELECT MAX(id) ...) or otherwise selects the row with highest version/updated_at; update the DELETE statement that references user_metadata_tag and the MIN(id) subselect accordingly to retain the latest payload. In `@core/src/ops/metadata/manager.rs`: - Around line 266-299: The code that upserts user metadata tags (the block building and saving user_metadata_tag::ActiveModel when `existing` is Some vs None) currently returns only the saved model and hides whether it was an Insert or Update; change the method return to include the change kind (e.g., an enum or ChangeType) so callers can distinguish paths. Modify the upsert logic around `existing`, `update_model.update(&*db).await...` and the `.insert(&*db).await...` branches to return a tuple or struct containing the persisted user_metadata_tag record and a change marker (e.g., ChangeType::Update for the update branch and ChangeType::Insert for the insert branch), update the function signature and any call sites (notably core/src/ops/tags/apply/action.rs) to handle the new return shape so sync events can emit Update vs Insert correctly. - Around line 258-299: The current find-then-insert TOCTOU around user_metadata_tag::Entity::find() and user_metadata_tag::ActiveModel::insert() must be made atomic: wrap the logic in a DB transaction (begin_transaction / transaction closure) and perform an upsert using SQL ON CONFLICT (user_metadata_id, tag_id) DO UPDATE that sets applied_context, applied_variant, confidence, source, instance_attributes, updated_at, device_uuid and increments version, or alternatively catch the unique-violation error from ActiveModel::insert(), then in the same transaction re-query the row and perform the update on the resulting user_metadata_tag::ActiveModel; ensure you reference and update the same fields (applied_context/applied_variant/confidence/source/instance_attributes/updated_at/device_uuid/version) and return the final model. In `@core/src/ops/tags/apply/action.rs`: - Around line 169-199: In the TagTargets::EntryUuid branch, validate each entry_uuid against the entry table before calling metadata_manager.apply_semantic_tags_to_entry: for each entry_uuid, attempt to resolve it via the entry entity lookup (e.g., Entry::find / entry::Entity::find_by_id or the existing repository method you have) and if not found push a warning like "Skipped tagging missing entry {entry_uuid}" and continue; only call metadata_manager.apply_semantic_tags_to_entry, library.sync_model, increment successfully_tagged_count, and push to affected_entry_uuids when the entry lookup succeeds to avoid creating orphaned user_metadata and emitting sync events for non-existent files. In `@core/src/ops/tags/children.rs`: - Around line 55-58: The current implementation uses manager.get_descendants(self.input.tag_id) which returns all deeper descendants (grandchildren, etc.) instead of only direct children; change the call to the manager method that returns only immediate children (e.g., manager.get_children or manager.get_direct_children) or perform a query/filter that selects nodes whose parent_id == self.input.tag_id, keeping the same error mapping to QueryError::Internal(...) so the returned `children` variable contains only direct children for the tags.children RPC. In `@core/src/ops/tags/delete/action.rs`: - Around line 33-39: The cascade deletes in TagManager::delete_tag are executed as separate delete_many() calls and must be made atomic: modify TagManager::delete_tag (the method on struct TagManager that currently orchestrates the delete_many() calls) to begin a database transaction, perform all dependent deletes and the final tag deletion inside that transaction, and commit at the end (rolling back on any error) so partial deletions cannot persist; use the DB connection's transaction API (e.g., conn.transaction(...) or begin/commit/rollback) around the sequence referenced in the delete_many() calls (lines ~360-419) so errors propagate and trigger rollback. - Around line 41-42: The delete action currently skips cross-device sync (see TODO) causing divergence; in the tags::delete handler (action.rs) call the existing sync_model function with ChangeType::Delete when a tag is removed, ensuring you import or reference ChangeType::Delete, await the sync_model call (or handle its Result) and propagate/log any errors; remove the placeholder TODO comment and, if necessary, create a GitHub issue reference instead of leaving the inline TODO so deletions are propagated to peers consistently. In `@core/src/ops/tags/files_by_tag.rs`: - Around line 285-304: The code currently uses tags_by_entry.insert(...) and .or_insert_with(...) which makes first-wins behavior and prevents merging content-scoped tags into an existing entry-scoped tag list; update the logic in the loop over metadata_records (symbols: metadata_records, tags_by_metadata, metadata.entry_uuid, metadata.content_identity_uuid, rows, tags_by_entry) so that when adding tags for an entry you merge/extend into an existing Vec/set instead of replacing it: for entry-scoped (metadata.entry_uuid) and content-scoped (inside the rows loop for content_identity_uuid) use tags_by_entry.entry(eu).and_modify(|existing| merge existing with tags).or_insert(tags.clone()) (or equivalent merge that deduplicates if needed) so both metadata scopes contribute to the final tags for a file. - Around line 131-150: The current decoding in files_by_tag.rs fabricates critical File fields on failure (e.g. entry_id, entry_kind, entry_created_at, entry_modified_at) by using unwrap_or/unwrap_or_else; instead, make these mandatory and propagate the error from row.try_get so callers fail fast: replace the unwrap_or/unwrap_or_else calls for required fields (entry_id, entry_uuid if required, entry_kind, entry_name if required, entry_size, entry_aggregate_size, entry_child_count, entry_file_count, entry_created_at, entry_modified_at) with fallible returns (use row.try_get(...)? or map_err to return the error) and only keep .ok() for truly optional fields (entry_extension, entry_accessed_at, content_kind_name, parent_path, device_slug). Ensure the decoder/signature (the function surrounding these rows) returns a Result so the error can be propagated rather than constructing fake values. - Around line 21-26: GetFilesByTagInput's include_children and min_confidence are never used: update the files-by-tag logic so the tag resolution that currently matches only direct tag_id (the code around the direct match resolution) expands to include descendant/child tag IDs when GetFilesByTagInput.include_children is true (use the tag hierarchy lookup/recursive query or Tag children relation to produce a list of tag IDs), and apply GetFilesByTagInput.min_confidence when joining/filtering taggings (or the equivalent confidence column) so only taggings with confidence >= min_confidence are returned. Modify the query/filters in the function that produces the result set (the files-by-tag handler/query logic that currently ignores include_children and min_confidence) to accept the expanded tag ID list and add a confidence predicate, and update any tests to cover both include_children true/false and min_confidence thresholds. In `@core/src/ops/tags/unapply/action.rs`: - Around line 66-89: The current logic discovers additional user_metadata IDs via the content_identity path (variables entries → content_ids → cis → ci_uuids → um_by_content → um_ids) but still computes entries_affected and emits events only from self.input.entry_ids, under-reporting mutations; modify the flow to first resolve the full set of affected entry UUIDs (start from self.input.entry_ids, then expand to sibling entries that share content_identity by querying entry::Entity for entries whose content_id matches the discovered content_ids or whose content_identity.uuid matches ci_uuids), deduplicate into a single affected_entries set, and then use that set for both entries_affected and all emitted resource events (apply the same change where the symmetric content-identity fan-out logic repeats around the later block referencing um_ids/entries_affected). In `@packages/interface/src/components/Inspector/primitives/Tag.tsx`: - Around line 32-42: The remove control button in the Tag component is missing an explicit type and therefore defaults to "submit" inside forms; update the <button> element used when onRemove is present in packages/interface/src/components/Inspector/primitives/Tag.tsx to include type="button" so clicking the remove control won't submit parent forms (locate the JSX block that calls onRemove and renders the X icon). In `@packages/interface/src/components/Inspector/variants/FileInspector.tsx`: - Around line 634-645: The optimistic tag updater is only replacing the closed-over selectedFiles snapshot via updateSelectedFileTags and so leaves the inspector (which renders from file.tags via fileQuery.data || file) stale and risks clobbering newer selections; change updateSelectedFileTags to (1) update the source-of-truth query/state you render from (e.g. update the file/query data used by the inspector such as fileQuery.setData or the library query that contains file.tags) and (2) if you still need to sync selection, use the functional form setSelectedFiles(prev => ...) to merge tags into the selectedFiles list without overwriting concurrent changes; apply the same fixes to the other occurrence noted (lines ~920-955) and keep onSuccess refetchTagQueries for backend reconciliation. In `@packages/interface/src/components/SpacesSidebar/TagsGroup.tsx`: - Around line 42-57: Replace the browser confirm/alert flow inside handleContextMenu with the app-wide useContextMenu pattern: remove confirm() and alert() and instead invoke the existing useContextMenu (or create a tag-specific wrapper modeled on useVolumeContextMenu / useSpaceItemContextMenu) to open a menu/action with variant: "danger" that, when confirmed, calls deleteTag.mutateAsync({ tag_id: tag.id }); preserve the post-delete behavior (if isActive then navigate('/')), and ensure errors are routed to the app logger/UI rather than alert(). Update any other direct confirm/alert uses in this component (the other deletion branch lines) to the same useContextMenu pattern. In `@packages/interface/src/routes/explorer/context.tsx`: - Around line 266-276: The reducer cases for ENTER_TAG_MODE and EXIT_TAG_MODE only update state.mode but not the boolean state.tagModeActive, leaving consumers reading tagModeActive out of sync; update the reducer returns for the ENTER_TAG_MODE and EXIT_TAG_MODE branches (and any other similar branches like the other ENTER_/EXIT_ tag-mode cases) to set tagModeActive: true when entering tag mode and tagModeActive: false when exiting, ensuring both mode (mode.type === "tag") and tagModeActive reflect the same state; locate the cases by the action names ENTER_TAG_MODE and EXIT_TAG_MODE in the reducer and add the corresponding tagModeActive property to the returned state objects. --- Outside diff comments: In `@core/src/ops/search/query.rs`: - Around line 203-229: The FTS tag-filtering is applied after execute_fts5_search’s LIMIT/OFFSET causing truncated pages and mismatched get_total_count; modify the search path so tag constraints are applied before pagination by either (A) extending execute_fts5_search and get_total_count to accept the tags filter and include JOIN/WHERE clauses (or additional FTS query terms) so results and counts respect filters, or (B) over-fetch by calling execute_fts5_search without LIMIT/OFFSET (or with an increased fetch size), apply resolve_tag_filter and retain on fts_results, then apply pagination (slice OFFSET/LIMIT) and compute total from the filtered set; update usages of execute_fts5_search, resolve_tag_filter, fts_results, and get_total_count accordingly so both returned items and total count honor filters.tags. In `@packages/interface/src/routes/explorer/hooks/useFileContextMenu.ts`: - Around line 355-388: The handlers currently call getTargetFiles() and fan out mutations with forEachTarget without filtering by media type; update each action (those invoking regenerateThumbnail.mutateAsync, extractText.mutateAsync, proxy/generate jobs, etc.) to build the target list by taking getTargetFiles() and filtering by the appropriate content kind using getContentKind(file) or file.kind (e.g., only image/video for thumbnail/blurhash, only image/pdf for OCR, only files—not directories—for proxy) before passing to forEachTarget so incompatible entries are excluded. --- Nitpick comments: In `@core/src/ops/media/thumbnail/action.rs`: - Around line 127-149: The DB lookups in the mime_type resolution (the block that sets mime_type using entities::content_identity::Entity::find_by_id and entities::mime_type::Entity::find_by_id) are swallowing errors via .ok()/.flatten(), so change the logic to detect and log unexpected DB errors with tracing (e.g., warn! or error!) before falling back to mime_from_extension(&path); specifically, handle the Result from .one(db).await and .one(db).await for mime type explicitly, log the Err case with context (content_id or mime_id and path) using tracing macros, and only then return mime_from_extension(&path) on error or missing values while preserving the existing success path that extracts m.mime_type into mime_type. - Around line 3-15: Reorder the import blocks in this file to follow the project guideline: group standard library imports together (move std::path::PathBuf and std::sync::Arc to a single std block at the top), then list external crates (specta::Type and uuid::Uuid) in a separate block, and finally the local/module imports (the super::{job::..., processor::ThumbnailProcessor} and crate::{context::CoreContext, infra::action::{error::ActionError, LibraryAction}, ops::indexing::{path_resolver::PathResolver, processor::ProcessorEntry}}) with a blank line between each group; adjust only the use statements near ThumbnailJob/ThumbnailProcessor and CoreContext so the symbols ThumbnailJob, ThumbnailJobConfig, ThumbnailProcessor, CoreContext, ActionError, LibraryAction, PathResolver, ProcessorEntry, Type, and Uuid are preserved but reordered into the three groups. In `@core/src/ops/tags/ancestors.rs`: - Around line 1-64: The file mixes request/response types and execution logic; split into the ops module layout by moving GetTagAncestorsInput into input.rs, GetTagAncestorsOutput into output.rs, and the query implementation (GetTagAncestorsQuery, its impl including execute, and the crate::register_library_query! call) into action.rs (create job.rs only if background work is needed), then update the module (mod.rs) to pub use those symbols so callers still import GetTagAncestorsQuery, GetTagAncestorsInput, and GetTagAncestorsOutput; ensure the execute method and references to CoreContext, TagManager, and session remain identical when moved. In `@core/src/ops/tags/unapply/output.rs`: - Around line 7-10: Add Rustdoc comments to the public struct UnapplyTagsOutput and each of its public fields (entries_affected, tags_removed, warnings) describing their meaning and expected units/types; include a short example usage showing how to inspect the output (e.g., call an unapply function and check entries_affected/tags_removed) and any guarantees (e.g., warnings contain human-readable messages). Use /// comments above the struct and above each field to match core module standards and include a brief example snippet in the struct-level doc. In `@packages/interface/src/components/SpacesSidebar/TagsGroup.tsx`: - Around line 120-126: The select callback for useNormalizedQuery (query: 'tags.search') uses `any`, removing type-safety; replace the `any` types in the select parameter with the generated response type for the tags.search endpoint (or use `unknown` and add a runtime type guard) and update the select body to use that typed shape to extract `data.tags[].tag` (or the fallback) without `any`; update or remove the // TODO comment and ensure the signature of the select callback matches the generated types used by useNormalizedQuery. In `@packages/interface/src/components/Tags/TagSelector.tsx`: - Around line 44-50: The select projection for useNormalizedQuery bound to 'tags.search' uses any for both the response and result which bypasses TS checks; update the useNormalizedQuery call to be generically typed with the generated response type for 'tags.search' and replace the any casts in the select callback by narrowing the union (e.g., type-guard or discriminant check) on the result so you safely access result.tag when present and otherwise return result, ensuring the select returns the filtered array (refer to useNormalizedQuery, the select callback, the 'tags.search' query and the result.tag shape). In `@packages/interface/src/routes/explorer/hooks/useExplorerFiles.ts`: - Around line 146-150: The tagQuery call uses an unchecked any cast for the "files.by_tag" response; replace that by supplying the correct generic type to useNormalizedQuery so tagQuery is strongly typed (e.g., the response shape that includes files), remove the any casts where tagQuery.data.files is accessed, and update the other occurrences around the hook (the similar usage at the block referenced as 181-195) to use the same typed useNormalizedQuery so you can read tagQuery.data.files directly with compile-time safety. 

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 8f3f27fe-8bd4-4267-ae40-0006c6447f88

📥 Commits

Reviewing files that changed from the base of the PR and between 4d87617 and 4087784.

⛔ Files ignored due to path filters (1)

• packages/ts-client/src/generated/types.ts is excluded by !**/generated/**, !**/generated/**

📒 Files selected for processing (33)

• core/src/infra/db/migration/m20260125_000001_unique_user_metadata_tag.rs
• core/src/infra/db/migration/mod.rs
• core/src/ops/media/thumbnail/action.rs
• core/src/ops/metadata/manager.rs
• core/src/ops/search/query.rs
• core/src/ops/tags/ancestors.rs
• core/src/ops/tags/apply/action.rs
• core/src/ops/tags/apply/input.rs
• core/src/ops/tags/by_id.rs
• core/src/ops/tags/children.rs
• core/src/ops/tags/create/action.rs
• core/src/ops/tags/create/input.rs
• core/src/ops/tags/delete/action.rs
• core/src/ops/tags/delete/input.rs
• core/src/ops/tags/delete/mod.rs
• core/src/ops/tags/delete/output.rs
• core/src/ops/tags/files_by_tag.rs
• core/src/ops/tags/mod.rs
• core/src/ops/tags/unapply/action.rs
• core/src/ops/tags/unapply/input.rs
• core/src/ops/tags/unapply/mod.rs
• core/src/ops/tags/unapply/output.rs
• packages/interface/src/components/Inspector/primitives/Tag.tsx
• packages/interface/src/components/Inspector/variants/FileInspector.tsx
• packages/interface/src/components/SpacesSidebar/TagsGroup.tsx
• packages/interface/src/components/Tags/TagSelector.tsx
• packages/interface/src/hooks/useRefetchTagQueries.ts
• packages/interface/src/routes/explorer/ExplorerView.tsx
• packages/interface/src/routes/explorer/context.tsx
• packages/interface/src/routes/explorer/hooks/useExplorerFiles.ts
• packages/interface/src/routes/explorer/hooks/useFileContextMenu.ts
• packages/interface/src/routes/overview/OverviewTopBar.tsx
• packages/interface/src/routes/tag/index.tsx

[coderabbitai]

Actionable comments posted: 4

♻️ Duplicate comments (1)

core/src/ops/metadata/manager.rs (1)

299-308: ⚠️ Potential issue | 🟠 Major

Return whether each upserted row was inserted or updated.

This method now returns models from both upsert branches, but core/src/ops/tags/apply/action.rs:77-116 still syncs every returned model as ChangeType::Insert. Reapplying an existing tag will therefore publish the wrong sync mutation unless the return value carries the change kind.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@core/src/ops/metadata/manager.rs` around lines 299 - 308, The upsert re-query in manager.rs currently always pushes models into created_models without indicating whether the row was inserted or updated, causing apply/action.rs to treat every result as Insert; modify the upsert path in the function that builds created_models (the block around user_metadata_tag::Entity::find() and created_models.push(model)) to record the change kind alongside the model (e.g., push a tuple or a small struct of (model, ChangeType)) by detecting whether the operation did an insert vs update in the upsert branches, update the function return type to return Vec<(UserMetadataTagModel, ChangeType)> (or equivalent), and adjust callers such as core/src/ops/tags/apply/action.rs to consume the new pair and use ChangeType::Insert or ChangeType::Update when syncing. 

🧹 Nitpick comments (1)

core/src/ops/tags/delete/action.rs (1)

41-45: Replace TODO comment with a GitHub issue reference.

As per coding guidelines: "Never use placeholder comments like 'TODO: extract this later' or 'for now'; track future work in GitHub issues instead."

While this comment is detailed and explains the limitation well, consider creating a GitHub issue to track the sync-deletion work and reference it here with a brief note like:

-// TODO(sync): Tag deletion is not synced to other devices. -// The sync infrastructure supports ChangeType::Delete but the tag deletion -// path does not yet call library.sync_model() with it. This means deleted -// tags will reappear on other devices after sync. Tracked for a dedicated -// sync-deletion PR. +// Note: Tag deletion sync not yet implemented. See issue `#XXXX`.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@core/src/ops/tags/delete/action.rs` around lines 41 - 45, Replace the inline TODO comment about unsynced tag deletion with a short reference to a created GitHub issue (e.g., "See issue #<N>") instead of leaving the placeholder; create an issue tracking the work to call library.sync_model() with ChangeType::Delete for the tag deletion path, then update the comment in action.rs to a single-line note referencing that issue and remove the "TODO" block. 

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed. Inline comments: In `@core/src/ops/metadata/manager.rs`: - Around line 275-290: The conflict-update path currently updates several columns but omits the version field, so include user_metadata_tag::Column::Version in the OnConflict update and make it increment (not set to a constant) so re-applies bump the version; modify the OnConflict::update_columns/exprs in the upsert code in manager.rs (the block constructing OnConflict for user_metadata_tag) to update Version (e.g., use an expression that sets Version = Version + 1 or the equivalent sea-query/sea-orm increment expression) alongside the other updated columns. In `@core/src/ops/tags/delete/action.rs`: - Around line 47-57: The resource emission happens after the tag is deleted so from_ids() returns nothing; move the call to ResourceManager::emit_resource_events (or call a specialized deletion-only event) to run before the deletion in the delete action (action.rs) so the event is emitted with the tag ID; specifically, invoke resource_manager.emit_resource_events("tag", vec![self.input.tag_id]) (or use a "tag:deleted" deletion-only event variant) prior to calling the deletion code so frontend cache invalidation is guaranteed even though the DB row will be removed. In `@core/src/ops/tags/unapply/action.rs`: - Around line 110-114: The unapply action currently deletes rows from user_metadata_tag locally but does not publish a sync deletion/tombstone, so other devices can resurrect the tag; update the tag unapply logic (the function in action.rs that removes user_metadata_tag rows) to emit a sync deletion for the tag (same pattern used in delete/action.rs), e.g. create a ChangeType::Delete/tombstone for the tag and invoke the library sync publish path (library.sync_model() or the project's sync-publish helper) so the deletion is propagated to other devices. - Around line 116-149: The current logic builds all_affected_uuids from self.input.entry_ids and content-siblings even when no tags were actually deleted, causing overreporting; change the flow in the UnapplyTags action (action.rs) so you either short-circuit and return early when total_removed == 0, or (preferred) build all_affected_uuids from the actual rows selected for deletion (use the UUIDs from the query that produced the user_metadata_tag rows to delete and their content-id siblings) and then emit_resource_events("file", affected_vec) and set entries_affected based on that set; update any uses of all_affected_uuids, resource_manager, emit_resource_events and UnapplyTagsOutput accordingly so only entries that actually lost a tag are reported and notified. --- Duplicate comments: In `@core/src/ops/metadata/manager.rs`: - Around line 299-308: The upsert re-query in manager.rs currently always pushes models into created_models without indicating whether the row was inserted or updated, causing apply/action.rs to treat every result as Insert; modify the upsert path in the function that builds created_models (the block around user_metadata_tag::Entity::find() and created_models.push(model)) to record the change kind alongside the model (e.g., push a tuple or a small struct of (model, ChangeType)) by detecting whether the operation did an insert vs update in the upsert branches, update the function return type to return Vec<(UserMetadataTagModel, ChangeType)> (or equivalent), and adjust callers such as core/src/ops/tags/apply/action.rs to consume the new pair and use ChangeType::Insert or ChangeType::Update when syncing. --- Nitpick comments: In `@core/src/ops/tags/delete/action.rs`: - Around line 41-45: Replace the inline TODO comment about unsynced tag deletion with a short reference to a created GitHub issue (e.g., "See issue #<N>") instead of leaving the placeholder; create an issue tracking the work to call library.sync_model() with ChangeType::Delete for the tag deletion path, then update the comment in action.rs to a single-line note referencing that issue and remove the "TODO" block. 

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: f66ca8f0-3de2-4226-9d53-2a465da1ce1b

📥 Commits

Reviewing files that changed from the base of the PR and between 4087784 and a07277c.

📒 Files selected for processing (8)

• core/src/infra/db/migration/m20260125_000001_unique_user_metadata_tag.rs
• core/src/ops/metadata/manager.rs
• core/src/ops/tags/children.rs
• core/src/ops/tags/delete/action.rs
• core/src/ops/tags/files_by_tag.rs
• core/src/ops/tags/manager.rs
• core/src/ops/tags/unapply/action.rs
• packages/interface/src/routes/explorer/context.tsx

🚧 Files skipped from review as they are similar to previous changes (2)

• packages/interface/src/routes/explorer/context.tsx
• core/src/ops/tags/files_by_tag.rs

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

packages/interface/src/routes/explorer/context.tsx (1)

609-618: ⚠️ Potential issue | 🟠 Major

Unify mode reset across all navigation paths.

navigateToPath clears tag mode (Line 604), but navigateToView/goBack/goForward only clear search mode (Lines 615, 627, 638). This leaves mode.type === "tag" reachable after non-path navigation and can drive the wrong data mode in consumers keyed off mode.type.

Suggested fix

+const exitSpecialModes = useCallback(() => { +uiDispatch({ type: "EXIT_SEARCH_MODE" }); +uiDispatch({ type: "EXIT_TAG_MODE" }); +}, []); + const navigateToPath = useCallback(	(path: SdPath) => {	const target: NavigationTarget = { type: "path", path };	navDispatch({ type: "NAVIGATE", target });	routerNavigate(targetToUrl(target)); -// Exit special modes when navigating to a path -uiDispatch({ type: "EXIT_SEARCH_MODE" }); -uiDispatch({ type: "EXIT_TAG_MODE" }); +exitSpecialModes();	}, -[routerNavigate], +[routerNavigate, exitSpecialModes], ); const navigateToView = useCallback(	(view: string, id?: string, params?: Record<string, string>) => {	const target: NavigationTarget = { type: "view", view, id, params };	navDispatch({ type: "NAVIGATE", target });	routerNavigate(targetToUrl(target)); -// Exit search mode when navigating -uiDispatch({ type: "EXIT_SEARCH_MODE" }); +exitSpecialModes();	}, -[routerNavigate], +[routerNavigate, exitSpecialModes], ); const goBack = useCallback(() => {	navDispatch({ type: "GO_BACK" });	const targetIndex = navState.index - 1;	if (targetIndex >= 0) {	const target = navState.history[targetIndex];	routerNavigate(targetToUrl(target), { replace: true }); -// Exit search mode when navigating -uiDispatch({ type: "EXIT_SEARCH_MODE" }); +exitSpecialModes();	} -}, [navState.index, navState.history, routerNavigate]); +}, [navState.index, navState.history, routerNavigate, exitSpecialModes]); const goForward = useCallback(() => {	navDispatch({ type: "GO_FORWARD" });	const targetIndex = navState.index + 1;	if (targetIndex < navState.history.length) {	const target = navState.history[targetIndex];	routerNavigate(targetToUrl(target), { replace: true }); -// Exit search mode when navigating -uiDispatch({ type: "EXIT_SEARCH_MODE" }); +exitSpecialModes();	} -}, [navState.index, navState.history, routerNavigate]); +}, [navState.index, navState.history, routerNavigate, exitSpecialModes]);

Also applies to: 620-640

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@packages/interface/src/routes/explorer/context.tsx` around lines 609 - 618, navigateToView, goBack, and goForward only exit search mode while navigateToPath also clears tag mode, leaving mode.type === "tag" active after non-path navigation; update navigateToView, goBack, and goForward to perform the same mode reset as navigateToPath (either dispatch the same tag-mode-clearing action used in navigateToPath or replace both search/tag clears with the unified RESET_MODE action) by adding the matching uiDispatch call(s) alongside the existing EXIT_SEARCH_MODE dispatch so all navigation paths consistently reset mode.type. 

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed. Outside diff comments: In `@packages/interface/src/routes/explorer/context.tsx`: - Around line 609-618: navigateToView, goBack, and goForward only exit search mode while navigateToPath also clears tag mode, leaving mode.type === "tag" active after non-path navigation; update navigateToView, goBack, and goForward to perform the same mode reset as navigateToPath (either dispatch the same tag-mode-clearing action used in navigateToPath or replace both search/tag clears with the unified RESET_MODE action) by adding the matching uiDispatch call(s) alongside the existing EXIT_SEARCH_MODE dispatch so all navigation paths consistently reset mode.type. 

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 976bc743-e12f-4f48-ae4e-2f478c040b6c

📥 Commits

Reviewing files that changed from the base of the PR and between a07277c and ca25988.

📒 Files selected for processing (1)

• packages/interface/src/routes/explorer/context.tsx

[coderabbitai]

🧹 Nitpick comments (3)

packages/interface/src/routes/explorer/context.tsx (1)

266-276: Consider consolidating tagModeActive with mode.type === "tag".

There are now two mechanisms for tracking tag mode:

• tagModeActive boolean (via SET_TAG_MODE action)
• mode.type === "tag" (via ENTER_TAG_MODE/EXIT_TAG_MODE actions)

This duplication could confuse consumers—calling enterTagMode() leaves tagModeActive === false, and calling setTagModeActive(true) leaves mode.type === "browse".

If tagModeActive is legacy state, consider deprecating it in favor of deriving the boolean from mode.type === "tag". If both are needed for different purposes, document the distinction.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@packages/interface/src/routes/explorer/context.tsx` around lines 266 - 276, The reducer currently tracks tag mode in two places (the boolean tagModeActive via SET_TAG_MODE / setTagModeActive() and the mode object via ENTER_TAG_MODE / EXIT_TAG_MODE / enterTagMode()), causing inconsistent state; pick one canonical source and unify: either remove tagModeActive and any SET_TAG_MODE/setTagModeActive() usages and derive the boolean as mode.type === "tag" everywhere, or update ENTER_TAG_MODE/EXIT_TAG_MODE to also set tagModeActive so both stay in sync and deprecate the other approach with comments—search for tagModeActive, SET_TAG_MODE, setTagModeActive(), ENTER_TAG_MODE, EXIT_TAG_MODE, enterTagMode(), and mode.type to update all callers and tests accordingly. 

core/src/ops/metadata/manager.rs (1)

255-259: Consider replacing unwrap() with unwrap_or_default() for defensive coding.

While serializing a HashMap<String, serde_json::Value> to JSON is infallible in practice, using unwrap() could panic if an unexpected type is introduced in the future. A safer alternative:

🛡️ Defensive alternative

 let instance_attributes_value = if app.instance_attributes.is_empty() { None } else { - Some(serde_json::to_value(&app.instance_attributes).unwrap().into()) + serde_json::to_value(&app.instance_attributes) + .ok() + .map(|v| v.into()) };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@core/src/ops/metadata/manager.rs` around lines 255 - 259, The code uses serde_json::to_value(&app.instance_attributes).unwrap() when building instance_attributes_value, which can panic; change this to a non-panicking fallback by replacing unwrap() with unwrap_or_default() (i.e., serde_json::to_value(&app.instance_attributes).unwrap_or_default().into()) or otherwise map the Result to a default JSON value so instance_attributes_value is Some(default) instead of triggering a panic; update the expression around instance_attributes_value and its use with app.instance_attributes and serde_json::to_value accordingly. 

core/src/ops/tags/unapply/action.rs (1)

117-131: Affected entries set may include entries that didn't actually lose a tag.

When total_removed > 0, the code notifies all input entries plus all content-sharing siblings, even if only a subset had the tag applied. This is a conservative approach ensuring UI consistency, but could cause unnecessary refreshes.

If precise reporting is desired, you could query the user_metadata_ids from the deleted rows before deletion (using a SELECT with the same filters), then resolve only those to entry UUIDs.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@core/src/ops/tags/unapply/action.rs` around lines 117 - 131, The current logic builds all_affected_uuids from self.input.entry_ids and all entries with matching content_ids (using entry::Entity::find() and entry::Column::ContentId) whenever total_removed > 0, which can include entries that never had the specific tag removed; instead, before performing the DELETE, run a SELECT for the same filters to collect the exact user_metadata_id values (the rows that will be deleted), then resolve those user_metadata_id -> entry UUIDs and use that resulting set to populate all_affected_uuids so only entries that actually lost metadata are notified; reference the same filter criteria used for deletion and replace the current content-scoped extension logic that blindly includes all siblings. 

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed. Nitpick comments: In `@core/src/ops/metadata/manager.rs`: - Around line 255-259: The code uses serde_json::to_value(&app.instance_attributes).unwrap() when building instance_attributes_value, which can panic; change this to a non-panicking fallback by replacing unwrap() with unwrap_or_default() (i.e., serde_json::to_value(&app.instance_attributes).unwrap_or_default().into()) or otherwise map the Result to a default JSON value so instance_attributes_value is Some(default) instead of triggering a panic; update the expression around instance_attributes_value and its use with app.instance_attributes and serde_json::to_value accordingly. In `@core/src/ops/tags/unapply/action.rs`: - Around line 117-131: The current logic builds all_affected_uuids from self.input.entry_ids and all entries with matching content_ids (using entry::Entity::find() and entry::Column::ContentId) whenever total_removed > 0, which can include entries that never had the specific tag removed; instead, before performing the DELETE, run a SELECT for the same filters to collect the exact user_metadata_id values (the rows that will be deleted), then resolve those user_metadata_id -> entry UUIDs and use that resulting set to populate all_affected_uuids so only entries that actually lost metadata are notified; reference the same filter criteria used for deletion and replace the current content-scoped extension logic that blindly includes all siblings. In `@packages/interface/src/routes/explorer/context.tsx`: - Around line 266-276: The reducer currently tracks tag mode in two places (the boolean tagModeActive via SET_TAG_MODE / setTagModeActive() and the mode object via ENTER_TAG_MODE / EXIT_TAG_MODE / enterTagMode()), causing inconsistent state; pick one canonical source and unify: either remove tagModeActive and any SET_TAG_MODE/setTagModeActive() usages and derive the boolean as mode.type === "tag" everywhere, or update ENTER_TAG_MODE/EXIT_TAG_MODE to also set tagModeActive so both stay in sync and deprecate the other approach with comments—search for tagModeActive, SET_TAG_MODE, setTagModeActive(), ENTER_TAG_MODE, EXIT_TAG_MODE, enterTagMode(), and mode.type to update all callers and tests accordingly. 

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: e4300ec1-3cb1-4b89-a00c-1d6c3b5b88b3

📥 Commits

Reviewing files that changed from the base of the PR and between ca25988 and 96fd6e8.

📒 Files selected for processing (3)

• core/src/ops/metadata/manager.rs
• core/src/ops/tags/unapply/action.rs
• packages/interface/src/routes/explorer/context.tsx

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

core/src/ops/tags/files_by_tag.rs (1)

321-342: Index rows by content_identity_uuid before this merge.

This branch rescans every SQL row for each content-scoped metadata record. On popular tags, hydration becomes O(metadata_records × rows) on top of the existing queries, which is likely to show up in Explorer tag views. Precomputing a content_identity_uuid -> Vec<entry_uuid> map once would keep this step linear.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@core/src/ops/tags/files_by_tag.rs` around lines 321 - 342, The current loop under the branch checking metadata.content_identity_uuid rescans all SQL rows for each metadata record, causing O(metadata_records × rows) work; instead, precompute an index mapping content_identity_uuid to Vec<entry_uuid> once and use it to populate tags_by_entry. Create a temporary HashMap (e.g., content_index) by iterating rows once and extracting each row's content_identity_uuid and entry_uuid (using the same try_get calls used now), then in the metadata.content_identity_uuid branch look up content_index.get(&content_uuid) and extend tags_by_entry.entry(eu) for each entry_uuid found; update places referencing rows and the existing try_get logic to read from the index to avoid repeated scans. 

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed. Inline comments: In `@core/src/ops/tags/files_by_tag.rs`: - Around line 178-190: The bug is that root-level files lose their extension because the else branch builds format!("/{}", entry_name) instead of using the assembled file_name; move the file_name computation out so it's always available (compute file_name from entry_name and entry_extension before the parent_path if-let), then change the else branch to format!("/{}", file_name) so file_path preserves extensions for root files (affecting file_path, parent_path, entry_extension, entry_name, file_name and resulting SdPath). --- Nitpick comments: In `@core/src/ops/tags/files_by_tag.rs`: - Around line 321-342: The current loop under the branch checking metadata.content_identity_uuid rescans all SQL rows for each metadata record, causing O(metadata_records × rows) work; instead, precompute an index mapping content_identity_uuid to Vec<entry_uuid> once and use it to populate tags_by_entry. Create a temporary HashMap (e.g., content_index) by iterating rows once and extracting each row's content_identity_uuid and entry_uuid (using the same try_get calls used now), then in the metadata.content_identity_uuid branch look up content_index.get(&content_uuid) and extend tags_by_entry.entry(eu) for each entry_uuid found; update places referencing rows and the existing try_get logic to read from the index to avoid repeated scans. 

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 089929a2-0bac-4e9f-93d1-f1bfb58bf7d1

📥 Commits

Reviewing files that changed from the base of the PR and between 96fd6e8 and fc2d5a2.

📒 Files selected for processing (1)

• core/src/ops/tags/files_by_tag.rs

[slvnlrt]

@CodeRabbit resume

[coderabbitai]

✅ Actions performed

Reviews resumed.

[coderabbitai]

Actionable comments posted: 2

♻️ Duplicate comments (3)

core/src/ops/tags/files_by_tag.rs (1)

165-166: ⚠️ Potential issue | 🟠 Major

Don't coerce an unreadable entry_kind to 0.

File::from_entity_model needs a real kind to preserve file-vs-directory behavior. Falling back to 0 turns a corrupt row into a valid-looking entry and can reintroduce bad Explorer/context-menu behavior; handle entry_kind like the other required fields instead.

Proposed fix

-let entry_kind: i32 = row.try_get("", "entry_kind").unwrap_or(0); +let Ok(entry_kind) = row.try_get::<i32>("", "entry_kind") else { +tracing::warn!(entry_id = entry_id, "Skipping row: failed to decode entry_kind"); +continue; +};

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@core/src/ops/tags/files_by_tag.rs` around lines 165 - 166, The code currently coerces a missing or unreadable entry_kind to 0 (let entry_kind: i32 = row.try_get("", "entry_kind").unwrap_or(0)), which masks corruption and breaks File::from_entity_model’s file-vs-directory logic; change the handling of entry_kind to treat it as a required field like the others by using row.try_get("", "entry_kind").unwrap() (or otherwise propagate/return an error) so an unreadable/missing entry_kind fails fast instead of silently defaulting to 0, and update any surrounding error/context messages as needed. 

packages/interface/src/components/SpacesSidebar/TagsGroup.tsx (1)

42-56: ⚠️ Potential issue | 🟠 Major

Don’t use browser confirm() in @sd/interface.

This right-click delete path is tied to a web-only API and bypasses the app’s shared context-menu flow. Route it through useContextMenu (or the existing destructive dialog pattern) instead.

As per coding guidelines "The @sd/interface package is platform agnostic and works on Tauri, Web, and React Native - never use platform-specific APIs directly" and "Use the useContextMenu hook for platform-agnostic context menus with conditional items, icons, keybinds, and variants".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@packages/interface/src/components/SpacesSidebar/TagsGroup.tsx` around lines 42 - 56, The right-click handler handleContextMenu uses the browser confirm() which is web-only and must be replaced with the shared context-menu/destructive flow; refactor handleContextMenu to open the app-wide context menu (useContextMenu hook) or the existing destructive dialog pattern instead of confirm(), present a "Delete tag" destructive action for tag.canonical_name, and when that action is confirmed call deleteTag.mutateAsync({ tag_id: tag.id }) and then navigate('/') if isActive; ensure you remove direct use of window.confirm and wire the menu item to the same error handling currently around deleteTag.mutateAsync. 

core/src/ops/tags/delete/action.rs (1)

101-105: ⚠️ Potential issue | 🟡 Minor

Replace the inline sync TODO with the tracking reference.

This still ships a placeholder comment in production code. Point at the follow-up issue/PR here instead of leaving a raw TODO(sync) block.

As per coding guidelines "Never use placeholder comments like 'TODO: extract this later' or 'for now'; track future work in GitHub issues instead".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@core/src/ops/tags/delete/action.rs` around lines 101 - 105, Replace the inline TODO(sync) block in the tag deletion path with a tracked reference: create (or locate) the follow-up GitHub issue/PR that will implement calling library.sync_model() with ChangeType::Delete for tag deletions, then replace the TODO in core/src/ops/tags/delete/action.rs with a short comment referencing that issue/PR (include the issue/PR number or URL) and mention the relevant symbols — e.g., the tag deletion code path, ChangeType::Delete, and library.sync_model() — so future readers can find the tracked work. 

🧹 Nitpick comments (1)

core/src/ops/tags/files_by_tag.rs (1)

1-1: Document the new query module and public API.

This new core/src/ops query still only has a one-line module doc, and GetFilesByTagInput/GetFilesByTagOutput/GetFilesByTagQuery are undocumented. The repo guidelines require a titled //! module doc that explains why the module exists and includes an example, plus /// docs on public items.

As per coding guidelines, "Module documentation should explain WHY the module exists, use prose with one code example, and include a title with #" and "Use //! for module documentation, /// for public items, and include examples".

Also applies to: 22-37

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@core/src/ops/tags/files_by_tag.rs` at line 1, Add proper module-level documentation to core/src/ops/tags/files_by_tag.rs using a titled //! doc (e.g., "# Get Files By Tag") that explains why this query module exists, outlines its behavior, and includes a short usage example; then add /// docs to the public types GetFilesByTagInput, GetFilesByTagOutput, and GetFilesByTagQuery describing their purpose, fields/semantics, and an example snippet where appropriate, following the repo guideline to use prose, a code example, and the specified doc comment styles. 

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed. Inline comments: In `@core/src/ops/tags/files_by_tag.rs`: - Around line 317-347: The content-scoped branch currently rescans and decodes every SQL row for each metadata record (metadata_records / metadata.content_identity_uuid), making the merge O(metadata × rows); instead, pre-index rows once into a HashMap from content_identity_uuid -> Vec<entry_uuid> by iterating rows a single time and extracting the two UUIDs (same try_get used now), then in the existing loop over metadata_records (where you consult tags_by_metadata and update tags_by_entry) simply lookup the content_uuid in that index and extend tags for each entry_uuid found; update references to rows/rescans to use the new index to avoid repeated decoding and quadratic behavior. In `@packages/interface/src/components/SpacesSidebar/TagsGroup.tsx`: - Around line 123-124: Update the tag search selector to use proper generated types: import type { Tag, SearchTagsInput, SearchTagsOutput } from '@sd/ts-client', bind those generics on the useNormalizedQuery call (useNormalizedQuery<SearchTagsInput, SearchTagsOutput>), and change the select callback signature to (data: SearchTagsOutput) => Tag[]; inside select map the results using (result) => result.tag || result and filter(Boolean) to return a Tag[] instead of using any. Ensure the local variables (e.g., const { data: tags = [], isLoading }) reflect the typed result. --- Duplicate comments: In `@core/src/ops/tags/delete/action.rs`: - Around line 101-105: Replace the inline TODO(sync) block in the tag deletion path with a tracked reference: create (or locate) the follow-up GitHub issue/PR that will implement calling library.sync_model() with ChangeType::Delete for tag deletions, then replace the TODO in core/src/ops/tags/delete/action.rs with a short comment referencing that issue/PR (include the issue/PR number or URL) and mention the relevant symbols — e.g., the tag deletion code path, ChangeType::Delete, and library.sync_model() — so future readers can find the tracked work. In `@core/src/ops/tags/files_by_tag.rs`: - Around line 165-166: The code currently coerces a missing or unreadable entry_kind to 0 (let entry_kind: i32 = row.try_get("", "entry_kind").unwrap_or(0)), which masks corruption and breaks File::from_entity_model’s file-vs-directory logic; change the handling of entry_kind to treat it as a required field like the others by using row.try_get("", "entry_kind").unwrap() (or otherwise propagate/return an error) so an unreadable/missing entry_kind fails fast instead of silently defaulting to 0, and update any surrounding error/context messages as needed. In `@packages/interface/src/components/SpacesSidebar/TagsGroup.tsx`: - Around line 42-56: The right-click handler handleContextMenu uses the browser confirm() which is web-only and must be replaced with the shared context-menu/destructive flow; refactor handleContextMenu to open the app-wide context menu (useContextMenu hook) or the existing destructive dialog pattern instead of confirm(), present a "Delete tag" destructive action for tag.canonical_name, and when that action is confirmed call deleteTag.mutateAsync({ tag_id: tag.id }) and then navigate('/') if isActive; ensure you remove direct use of window.confirm and wire the menu item to the same error handling currently around deleteTag.mutateAsync. --- Nitpick comments: In `@core/src/ops/tags/files_by_tag.rs`: - Line 1: Add proper module-level documentation to core/src/ops/tags/files_by_tag.rs using a titled //! doc (e.g., "# Get Files By Tag") that explains why this query module exists, outlines its behavior, and includes a short usage example; then add /// docs to the public types GetFilesByTagInput, GetFilesByTagOutput, and GetFilesByTagQuery describing their purpose, fields/semantics, and an example snippet where appropriate, following the repo guideline to use prose, a code example, and the specified doc comment styles. 

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 8919e5f2-0ba9-4578-bdd6-c791780ede73

📥 Commits

Reviewing files that changed from the base of the PR and between 96fd6e8 and 0978dbf.

📒 Files selected for processing (6)

• core/src/ops/tags/apply/action.rs
• core/src/ops/tags/delete/action.rs
• core/src/ops/tags/files_by_tag.rs
• packages/interface/src/components/Inspector/variants/FileInspector.tsx
• packages/interface/src/components/SpacesSidebar/TagsGroup.tsx
• packages/interface/src/hooks/useRefetchTagQueries.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• packages/interface/src/hooks/useRefetchTagQueries.ts

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash # Locate the generated tags.search output type and existing typed useNormalizedQuery patterns. rg -n "tags\\.search|type .*Tag.*Search|interface .*Tag.*Search|Search.*Tag" packages/ts-client/src/generated/types.ts packages/interface/src --type ts --type tsx rg -n "useNormalizedQuery<" packages/interface/src --type ts --type tsx -A2 -B2

Repository: spacedriveapp/spacedrive

Length of output: 123

---

🏁 Script executed:

#!/bin/bash # Find the file and read the context around lines 123-124 cat -n packages/interface/src/components/SpacesSidebar/TagsGroup.tsx | sed -n '115,135p'

Repository: spacedriveapp/spacedrive

Length of output: 869

---

🏁 Script executed:

#!/bin/bash # Search for tags.search and related type definitions (corrected rg syntax) rg "tags\.search|type.*TagSearch|interface.*TagSearch" packages/ts-client/src/generated/ -A2 -B1

Repository: spacedriveapp/spacedrive

Length of output: 1337

---

🏁 Script executed:

#!/bin/bash # Find useNormalizedQuery usage patterns with proper typing rg "useNormalizedQuery<" packages/interface/src -A3 -B1 -g "*.ts" -g "*.tsx"

Repository: spacedriveapp/spacedrive

Length of output: 10095

---

🏁 Script executed:

#!/bin/bash # Check what ts-client exports related to tags rg "tags.*search|SearchTags" packages/ts-client/src/generated/ -A2 -B1 | head -50

Repository: spacedriveapp/spacedrive

Length of output: 1387

---

🏁 Script executed:

#!/bin/bash # Get complete SearchTagsOutput type definition rg -A20 "export type SearchTagsOutput" packages/ts-client/src/generated/types.ts

Repository: spacedriveapp/spacedrive

Length of output: 464

---

🏁 Script executed:

#!/bin/bash # Find Tag type definition and TagSearchResult structure rg -B2 -A10 "export type Tag " packages/ts-client/src/generated/types.ts | head -40

Repository: spacedriveapp/spacedrive

Length of output: 322

---

🏁 Script executed:

#!/bin/bash # Check what the select output should be - look at the data structure rg -B5 -A15 "export type TagSearchResult" packages/ts-client/src/generated/types.ts

Repository: spacedriveapp/spacedrive

Length of output: 492

---

🏁 Script executed:

#!/bin/bash # Check imports in TagsGroup.tsx head -30 packages/interface/src/components/SpacesSidebar/TagsGroup.tsx

Repository: spacedriveapp/spacedrive

Length of output: 1126

---

🏁 Script executed:

#!/bin/bash # Verify if SearchTagsInput/SearchTagsOutput are exported from ts-client rg "export.*SearchTags(Input|Output)" packages/ts-client/src/index.ts packages/ts-client/src/generated/index.ts

Repository: spacedriveapp/spacedrive

Length of output: 149

---

🏁 Script executed:

#!/bin/bash # Find the correct ts-client export file find packages/ts-client/src -name "index.ts" -o -name "*.ts" | grep -E "(index|export)" | head -10

Repository: spacedriveapp/spacedrive

Length of output: 137

---

🏁 Script executed:

#!/bin/bash # Check what's exported from ts-client rg "export.*SearchTags" packages/ts-client/src/

Repository: spacedriveapp/spacedrive

Length of output: 218

---

🏁 Script executed:

#!/bin/bash # Check if SearchTagsInput/SearchTagsOutput are exported from main index rg "SearchTagsInput|SearchTagsOutput" packages/ts-client/src/index.ts

Repository: spacedriveapp/spacedrive

Length of output: 50

---

🏁 Script executed:

#!/bin/bash # See the full structure of what's exported from ts-client cat packages/ts-client/src/index.ts | head -50

Repository: spacedriveapp/spacedrive

Length of output: 1457

---

🏁 Script executed:

#!/bin/bash # Check more of the index.ts file to see all exports cat packages/ts-client/src/index.ts | tail -100

Repository: spacedriveapp/spacedrive

Length of output: 2087

---

🏁 Script executed:

#!/bin/bash # Look for re-exports of generated types rg "export.*from.*generated" packages/ts-client/src/index.ts

Repository: spacedriveapp/spacedrive

Length of output: 102

---

Replace any types in the tag search selector with proper generated types.

The select function should be typed as (data: SearchTagsOutput) => Tag[]. Import SearchTagsInput and SearchTagsOutput from @sd/ts-client and bind the generic type parameters to useNormalizedQuery<SearchTagsInput, SearchTagsOutput>.

import type { Tag, SearchTagsInput, SearchTagsOutput } from '@sd/ts-client'; const { data: tags = [], isLoading } = useNormalizedQuery<SearchTagsInput, SearchTagsOutput>({ query: 'tags.search', input: { query: '' }, resourceType: 'tag', select: (data: SearchTagsOutput) => data?.tags?.map((result) => result.tag || result).filter(Boolean) ?? [] });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@packages/interface/src/components/SpacesSidebar/TagsGroup.tsx` around lines 123 - 124, Update the tag search selector to use proper generated types: import type { Tag, SearchTagsInput, SearchTagsOutput } from '@sd/ts-client', bind those generics on the useNormalizedQuery call (useNormalizedQuery<SearchTagsInput, SearchTagsOutput>), and change the select callback signature to (data: SearchTagsOutput) => Tag[]; inside select map the results using (result) => result.tag || result and filter(Boolean) to return a Tag[] instead of using any. Ensure the local variables (e.g., const { data: tags = [], isLoading }) reflect the typed result. 

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed. Inline comments: In `@core/src/ops/tags/files_by_tag.rs`: - Around line 201-205: The code constructs an SdPath::Physical using device_slug.unwrap_or_else(|| crate::device::get_current_device_slug()) which can be an empty String (get_current_device_slug returns "" if uninitialized), producing invalid SdPath; update the block in files_by_tag.rs so after resolving device_slug (the value passed to SdPath::Physical) you validate it is non-empty and either return an error (propagate a suitable error type) or log a warning and skip/handle the row, and ensure any callers of this code handle that error; reference the symbols device_slug, SdPath::Physical, and get_current_device_slug when adding the guard. 

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 6973d875-49ff-4cb4-9853-dd506102d473

📥 Commits

Reviewing files that changed from the base of the PR and between 0978dbf and 3282597.

📒 Files selected for processing (1)

• core/src/ops/tags/files_by_tag.rs

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Empty device_slug fallback may produce invalid SdPath.

get_current_device_slug() returns an empty String when CURRENT_DEVICE_SLUG is uninitialized (see core/src/device/id.rs:64-67). If the SQL row lacks a device slug and the global isn't set—e.g., in tests or library-only contexts—this silently constructs an SdPath::Physical with an empty device_slug, which could cause downstream file operations to fail or target the wrong device.

Consider logging a warning or returning an error when both the row and fallback yield empty/missing slugs.

💡 Suggested defensive check

 let sd_path = SdPath::Physical { - device_slug: device_slug - .unwrap_or_else(|| crate::device::get_current_device_slug()), + device_slug: device_slug.or_else(|| { + let current = crate::device::get_current_device_slug(); + if current.is_empty() { + tracing::warn!("No device_slug for entry_id={}, using empty slug", entry_id); + } + Some(current) + }).unwrap_or_default(), path: PathBuf::from(file_path), };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@core/src/ops/tags/files_by_tag.rs` around lines 201 - 205, The code constructs an SdPath::Physical using device_slug.unwrap_or_else(|| crate::device::get_current_device_slug()) which can be an empty String (get_current_device_slug returns "" if uninitialized), producing invalid SdPath; update the block in files_by_tag.rs so after resolving device_slug (the value passed to SdPath::Physical) you validate it is non-empty and either return an error (propagate a suitable error type) or log a warning and skip/handle the row, and ensure any callers of this code handle that error; reference the symbols device_slug, SdPath::Physical, and get_current_device_slug when adding the guard.