Fix {{FIELD:tags}} suggestions from vault tag index#1016
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughModifications to the tag collection workflow introduce specialized handling for the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Use Obsidian's tag index for tags/tag field suggestions when no file filters are applied, with a filtered-files fallback. Adds a regression test for issue #671.
c7587a9 to
5665876
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/utils/FieldValueCollector.ts (2)
111-132: LGTM - Consider adding debug logging for troubleshooting.The tag normalization correctly strips
#prefixes and handles the untyped Obsidian API. The empty catch block at line 127-129 silently swallows errors, which is acceptable since the caller falls back to file-based collection. However, routing errors through the logger utility could aid debugging in production.- } catch { - // ignore and fall back to file-based collection - } + } catch (e) { + // Fall back to file-based collection; log for debugging + console.debug?.("collectAllVaultTags: vault tag index unavailable", e); + }
134-200: The batch processing and tag extraction from frontmatter (tags/tag) and inline hashtags (metadataCache.tags) are correctly implemented.Minor design note:
collectTagValuesreturns early when tags are found (line 72), which bypasses the manual collection path that would callInlineFieldParser.getFieldValuesiffilters.inlineis true. This means Dataview-style inline fields liketags:: workwould not be collected if vault or frontmatter tags exist. However, no search results show users combining|inline:truewith tag fields, and this appears to be the intentional design where tags receive special-case handling rather than generic field handling.src/utils/FieldValueCollector.issue671.test.ts (1)
10-31: Good regression test coverage for Issue #671.The test correctly validates that vault tags are included in suggestions when no filters are present. The mock setup is appropriate, and clearing the cache before each test prevents cross-test contamination.
Consider adding edge case coverage:
it("uses file-based collection when filters are present", async () => { const app = new App(); // @ts-expect-error - getTags exists in Obsidian but is not typed app.metadataCache.getTags = () => ({ "#vaultTag": 1 }); app.vault.getMarkdownFiles = () => []; // With folder filter, should use file-based scan (empty result since no files) const values = await collectFieldValuesProcessed(app, "tags", { folder: "notes" }); expect(values).not.toContain("vaultTag"); }); it("handles 'tag' field name (singular)", async () => { const app = new App(); // @ts-expect-error app.metadataCache.getTags = () => ({ "#test": 1 }); app.vault.getMarkdownFiles = () => []; const values = await collectFieldValuesProcessed(app, "tag", {}); expect(values).toContain("test"); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/utils/FieldValueCollector.issue671.test.ts(1 hunks)src/utils/FieldValueCollector.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Source code lives in
src/: core logic underengine/,services/, andutils/; Svelte UI insrc/gui; shared types insrc/types; settings entry insrc/quickAddSettingsTab.ts
Files:
src/utils/FieldValueCollector.issue671.test.tssrc/utils/FieldValueCollector.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Biome enforces tab indentation (width 2), LF endings, and an 80-character line guide; align editor settings
Use camelCase for variables and functions
Prefer type-only imports in TypeScript files
Route logging through theloggerutilities for consistent output
Structure production code so Obsidian dependencies are injected behind interfaces; unit tests target pure logic and swap in adapters ortests/obsidian-stub.ts
Files:
src/utils/FieldValueCollector.issue671.test.tssrc/utils/FieldValueCollector.ts
src/**/*.{ts,tsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for classes and Svelte components
Files:
src/utils/FieldValueCollector.issue671.test.tssrc/utils/FieldValueCollector.ts
🧠 Learnings (2)
📚 Learning: 2025-12-09T21:20:52.398Z
Learnt from: CR
Repo: chhoumann/quickadd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T21:20:52.398Z
Learning: Applies to tests/**/*.{ts,tsx} : Add regression coverage for bug fixes
Applied to files:
src/utils/FieldValueCollector.issue671.test.ts
📚 Learning: 2025-12-09T21:20:52.398Z
Learnt from: CR
Repo: chhoumann/quickadd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T21:20:52.398Z
Learning: Applies to src/**/*.{ts,tsx} : Structure production code so Obsidian dependencies are injected behind interfaces; unit tests target pure logic and swap in adapters or `tests/obsidian-stub.ts`
Applied to files:
src/utils/FieldValueCollector.issue671.test.ts
🧬 Code graph analysis (2)
src/utils/FieldValueCollector.issue671.test.ts (3)
src/utils/FieldSuggestionCache.ts (1)
FieldSuggestionCache(8-171)tests/obsidian-stub.ts (1)
App(23-51)src/utils/FieldValueCollector.ts (1)
collectFieldValuesProcessed(27-42)
src/utils/FieldValueCollector.ts (2)
src/utils/FieldSuggestionParser.ts (1)
FieldFilter(1-12)src/utils/EnhancedFieldSuggestionFileFilter.ts (1)
EnhancedFieldSuggestionFileFilter(4-170)
🔇 Additional comments (2)
src/utils/FieldValueCollector.ts (2)
69-73: LGTM!The special-case handling for tag fields integrates cleanly with the existing fallback flow. Normalizing to lowercase ensures case-insensitive matching, and returning early when tags are found avoids unnecessary Dataview calls.
95-109: LGTM!The filter detection logic correctly distinguishes when vault-wide index retrieval is appropriate versus when file-level scanning is needed to respect user-specified filters.
|
🎉 This PR is included in version 2.9.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🚀 Release has been published: v2.9.1 |
Fixes #671.
What changed
{{FIELD:tags}}/{{FIELD:tag}}now suggests existing tags from Obsidian's vault-wide tag index (metadataCache.getTags()), so tags show up even when they're not present as a frontmatter field.|folder:,|tag:, exclusions), we fall back to scanning the filtered file set to preserve filtering semantics.Tests
src/utils/FieldValueCollector.issue671.test.ts.bun run testpasses.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.