fix: drop non-string scalar aliases in metadata conversion - #382
Conversation
…lizer Follow-up to #377: a truthy non-string scalar (e.g. aliases: 42) still leaked into NoteMetadata. Centralize frontmatter alias normalization in one helper used by both searchNotes and convertToMetadata.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds a shared helper to normalize frontmatter aliases. Note search and metadata conversion now use the same logic. Non-string and empty-string aliases are excluded. ChangesAlias normalization flow
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/services/vault-service.ts`:
- Around line 127-128: Update the alias normalization logic near the list
construction and string filter so scalar and array inputs apply the same
empty-string rule; if empty aliases are invalid, exclude empty strings from both
forms. Add tests covering aliases supplied as "" and [""] and verify neither
produces an alias or empty search field.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 09fd9c4b-83b8-4550-ab38-0af8b49116b4
📒 Files selected for processing (1)
src/services/vault-service.ts
Review follow-up on #382: scalar "" was dropped by truthiness while [""] survived the type-only filter. Unify on "non-empty strings only".
Description
Follow-up to #377, covering the remaining CodeRabbit comment: the scalar branch of
convertToMetadata()was still unfiltered, so a truthy non-string scalar (e.g.aliases: 42) leaked intoNoteMetadataas[42].Instead of patching that one branch, this centralizes frontmatter alias normalization into a single module-level helper (
normalizeFrontmatterAliases) used by bothsearchNotes()andconvertToMetadata()— the duplicated, slightly different normalization is what let the two sites drift apart in the first place. The helper takesunknownand proves stringness with a type predicate, which also removes the misleadingas string[] | string | undefinedcasts (the root cause behind the crash in #354).Also included:
undefinedinNoteMetadata(previouslyundefinedor[]depending on the input shape; the field has no consumers).Verified in Obsidian with the repro notes from #354 (
aliases: [null, "TestAlias"],aliases: 42, and the trailing-colon YAML template): no crash, filtering narrows correctly, and normal alias search is unaffected.npm testpasses (190/190).Related issue
Follow-up to #377. Related: #354.
Type of change
Checklist
npm run lintpasses ("Use sentence case for UI text" errors are acceptable for brand names)npm run buildpassesTesting environment
Screenshots
N/A
Summary by CodeRabbit