Add Note search#44
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThis PR implements the keyword search feature for FrilVault notes with end-to-end changes: error handling for missing notes, case-insensitive search logic in the service, repository enumeration across the vault directory, CLI command definition, unified output formatting, command routing, API re-exports, comprehensive tests, and documentation updates clarifying the line-anchored note model. ChangesSearch Feature Implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
crates/frilvault-core/src/lib.rs (1)
8-12: 💤 Low valueWildcard re-exports broaden the public API surface.
pub use module::*exposes every public item and risks name collisions or unintentionally stabilizing internals as part of the crate's public contract. Consider explicit re-exports of the types you intend to expose (e.g.,Note,NoteAnchor,FrilVaultError,YamlNoteRepository,PathResolver,Workspace).🤖 Prompt for 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. In `@crates/frilvault-core/src/lib.rs` around lines 8 - 12, The wildcard re-exports in lib.rs expose the entire modules; replace each `pub use ...::*` with explicit re-exports of the public types you intend to stabilize (for example export `Note`, `NoteAnchor` from note, `FrilVaultError` from error, `YamlNoteRepository` from storage, `PathResolver` from parser, and `Workspace` from workspace) so only those symbols become part of the crate API; update the lines that currently read `pub use error::*`, `pub use note::*`, `pub use parser::*`, `pub use storage::*`, `pub use workspace::*` to individual `pub use` statements naming the specific types/functions/traits you want to expose.
🤖 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 `@apps/frilvault-cli/src/cli/mod.rs`:
- Line 16: The CLI name is inconsistent: the command attribute #[command(name =
"flvt", ...)] in src/cli/mod.rs and the [[bin]] name = "flvt" in Cargo.toml
differ from README examples that use "flv"; pick one canonical name and make
them consistent—either update README.md Quick Start examples to use "flvt"
(preferred if keeping current code) or change the binary and command attribute
to "flv" (edit Cargo.toml [[bin]] name and the #[command(name = "...")] value in
src/cli/mod.rs) so the documented command matches the actual binary.
In `@apps/frilvault-cli/src/output.rs`:
- Around line 32-36: The message in print_note_count always uses the plural
"notes"; change print_note_count to choose "note" when count == 1 and "notes"
otherwise (e.g., compute a suffix or full word based on count) and use that in
the println! call so it prints "Found 1 note" for a single result and "Found N
notes" for all other counts; keep the existing blank println! after the message.
In `@crates/frilvault-core/src/storage/yaml_repository.rs`:
- Around line 108-110: In collect_note_files, replace the hardcoded extension
check path.extension().and_then(|ext| ext.to_str()) != Some("yml") with a
comparison against the NOTE_FILE_EXTENSION constant (e.g. !=
Some(NOTE_FILE_EXTENSION) or compare ext.to_str() to NOTE_FILE_EXTENSION) so the
filter uses the same NOTE_FILE_EXTENSION as PathResolver::note_file_name; ensure
NOTE_FILE_EXTENSION is in scope or imported where collect_note_files is defined.
---
Nitpick comments:
In `@crates/frilvault-core/src/lib.rs`:
- Around line 8-12: The wildcard re-exports in lib.rs expose the entire modules;
replace each `pub use ...::*` with explicit re-exports of the public types you
intend to stabilize (for example export `Note`, `NoteAnchor` from note,
`FrilVaultError` from error, `YamlNoteRepository` from storage, `PathResolver`
from parser, and `Workspace` from workspace) so only those symbols become part
of the crate API; update the lines that currently read `pub use error::*`, `pub
use note::*`, `pub use parser::*`, `pub use storage::*`, `pub use workspace::*`
to individual `pub use` statements naming the specific types/functions/traits
you want to expose.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 591acc05-643e-4d02-979b-d955644ec6af
📒 Files selected for processing (20)
.github/PULL_REQUEST_TEMPLATE.mdREADME.mdapps/frilvault-cli/Cargo.tomlapps/frilvault-cli/src/cli/mod.rsapps/frilvault-cli/src/cli/search.rsapps/frilvault-cli/src/command/list.rsapps/frilvault-cli/src/command/mod.rsapps/frilvault-cli/src/command/search.rsapps/frilvault-cli/src/main.rsapps/frilvault-cli/src/output.rscrates/frilvault-core/src/error/errors.rscrates/frilvault-core/src/lib.rscrates/frilvault-core/src/note/service.rscrates/frilvault-core/src/parser/mod.rscrates/frilvault-core/src/storage/yaml_repository.rscrates/frilvault-core/src/tests/mod.rscrates/frilvault-core/src/tests/note_service_test.rscrates/frilvault-core/src/tests/yaml_parser_test.rscrates/frilvault-core/src/tests/yaml_repository_test.rscrates/frilvault-core/src/workspace/path_resolver.rs
| version, | ||
| about = "Personal note vault for source code" | ||
| )] | ||
| #[command(name = "flvt", version, about = "Personal note vault for source code")] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the binary target name and any other references to flv/flvt
fd -t f Cargo.toml apps/frilvault-cli -x cat {}
echo "---- references ----"
rg -n -w 'flv|flvt' --glob '!**/target/**'Repository: FrilLab/frilvault
Length of output: 734
🏁 Script executed:
#!/bin/bash
# Inspect CLI wiring to see whether there's any flv alias/wrapper.
set -euo pipefail
echo "---- apps/frilvault-cli/src/main.rs ----"
sed -n '1,200p' apps/frilvault-cli/src/main.rs
echo "---- apps/frilvault-cli/src/cli/mod.rs (around command derive) ----"
sed -n '1,120p' apps/frilvault-cli/src/cli/mod.rs
echo "---- any flv/flvt mentions in cli folder ----"
rg -n -S 'flv|flvt' apps/frilvault-cli/srcRepository: FrilLab/frilvault
Length of output: 1551
CLI name mismatch: binary/Clap uses flvt, README documents flv.
apps/frilvault-cli/Cargo.toml sets [[bin]] name = "flvt", and apps/frilvault-cli/src/cli/mod.rs uses #[command(name = "flvt", ...)], but README.md Quick Start examples invoke flv ...—so the documented commands won’t run as written. Align the canonical name by updating the README to flvt or changing the binary/command to flv.
🤖 Prompt for 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.
In `@apps/frilvault-cli/src/cli/mod.rs` at line 16, The CLI name is inconsistent:
the command attribute #[command(name = "flvt", ...)] in src/cli/mod.rs and the
[[bin]] name = "flvt" in Cargo.toml differ from README examples that use "flv";
pick one canonical name and make them consistent—either update README.md Quick
Start examples to use "flvt" (preferred if keeping current code) or change the
binary and command attribute to "flv" (edit Cargo.toml [[bin]] name and the
#[command(name = "...")] value in src/cli/mod.rs) so the documented command
matches the actual binary.
Comment on lines
+32
to
+36
| pub fn print_note_count(count: usize) { | ||
| println!("Found {} notes", count); | ||
|
|
||
| println!(); | ||
| } |
There was a problem hiding this comment.
Pluralization: "Found 1 notes" reads incorrectly for a single result.
The count message is always plural. Consider conditional pluralization.
✏️ Proposed fix
pub fn print_note_count(count: usize) {
- println!("Found {} notes", count);
+ println!("Found {} note{}", count, if count == 1 { "" } else { "s" });
println!();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| pub fn print_note_count(count: usize) { | |
| println!("Found {} notes", count); | |
| println!(); | |
| } | |
| pub fn print_note_count(count: usize) { | |
| println!("Found {} note{}", count, if count == 1 { "" } else { "s" }); | |
| println!(); | |
| } |
🤖 Prompt for 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.
In `@apps/frilvault-cli/src/output.rs` around lines 32 - 36, The message in
print_note_count always uses the plural "notes"; change print_note_count to
choose "note" when count == 1 and "notes" otherwise (e.g., compute a suffix or
full word based on count) and use that in the println! call so it prints "Found
1 note" for a single result and "Found N notes" for all other counts; keep the
existing blank println! after the message.
Comment on lines
+108
to
+110
| if path.extension().and_then(|ext| ext.to_str()) != Some("yml") { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP --type=rust 'NOTE_FILE_EXTENSION\s*[:=]'Repository: FrilLab/frilvault
Length of output: 147
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="crates/frilvault-core/src/storage/yaml_repository.rs"
nl -ba "$FILE" | sed -n '80,140p'Repository: FrilLab/frilvault
Length of output: 104
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="crates/frilvault-core/src/storage/yaml_repository.rs"
# Show relevant section with line numbers
awk 'NR>=80 && NR<=140 {printf "%d\t%s\n", NR, $0}' "$FILE"
echo "---- imports/const usage ----"
rg -n --fixed-string 'NOTE_FILE_EXTENSION' "$FILE" || true
rg -n --fixed-string 'Some("yml")' "$FILE" || true
rg -n --fixed-string '"yml"' "$FILE" || trueRepository: FrilLab/frilvault
Length of output: 2544
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="crates/frilvault-core/src/storage/yaml_repository.rs"
# Search within file
rg -n --fixed-strings 'NOTE_FILE_EXTENSION' "$FILE" || true
rg -n --fixed-strings '"yml"' "$FILE" || true
echo "---- top of file ----"
awk 'NR>=1 && NR<=80 {printf "%d\t%s\n", NR, $0}' "$FILE"Repository: FrilLab/frilvault
Length of output: 2667
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate PathResolver::note_file_name implementation
rg -n "fn\s+note_file_name" crates/frilvault-core/src/workspace -S
rg -n "NOTE_FILE_EXTENSION" crates/frilvault-core/src/workspace -S || trueRepository: FrilLab/frilvault
Length of output: 429
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# show surrounding code for note_file_name if found
FILE=$(rg -l "fn\s+note_file_name" crates/frilvault-core/src/workspace -S | head -n 1 || true)
echo "NOTE_FILE_NAME_FILE=$FILE"
if [ -n "$FILE" ]; then
awk 'NR>=1 && NR<=220 {print}' "$FILE" | sed -n '1,220p'
fiRepository: FrilLab/frilvault
Length of output: 1495
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# tighter display: print around NOTE_FILE_EXTENSION usage in workspace
rg -n "NOTE_FILE_EXTENSION" crates/frilvault-core/src/workspace -S -n --context 3 || trueRepository: FrilLab/frilvault
Length of output: 1131
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Find PathResolver::note_file_name and any NOTE_FILE_EXTENSION usage
rg -n "fn\s+note_file_name" crates/frilvault-core/src/workspace -S
rg -n "NOTE_FILE_EXTENSION" crates/frilvault-core/src/workspace -S || trueRepository: FrilLab/frilvault
Length of output: 429
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 2) Print the file content region around note_file_name (limit output)
FILE=$(rg -l "fn\s+note_file_name" crates/frilvault-core/src/workspace -S | head -n 1 || true)
echo "NOTE_FILE_NAME_FILE=$FILE"
if [ -n "$FILE" ]; then
# print ~80 lines around the first match
MATCH_LINE=$(rg -n "fn\s+note_file_name" "$FILE" -S | head -n 1 | cut -d: -f1)
START=$((MATCH_LINE-40))
END=$((MATCH_LINE+60))
awk -v s="$START" -v e="$END" 'NR>=s && NR<=e {printf "%d\t%s\n", NR, $0}' "$FILE"
fiRepository: FrilLab/frilvault
Length of output: 1624
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 3) Show exact NOTE_FILE_EXTENSION usage with nearby lines (if any) in workspace
rg -n "NOTE_FILE_EXTENSION" crates/frilvault-core/src/workspace -S --context 3 || trueRepository: FrilLab/frilvault
Length of output: 1131
Use NOTE_FILE_EXTENSION instead of hardcoding "yml" in collect_note_files.
crates/frilvault-core/src/storage/yaml_repository.rs (lines 108-110) filters by Some("yml"), while note paths are generated via PathResolver::note_file_name(..., NOTE_FILE_EXTENSION) (where NOTE_FILE_EXTENSION is exactly "yml"). This keeps enumeration consistent if the constant ever changes.
♻️ Proposed fix
-use crate::constants::VAULT_DIR_NAME;
+use crate::constants::{NOTE_FILE_EXTENSION, VAULT_DIR_NAME};- if path.extension().and_then(|ext| ext.to_str()) != Some("yml") {
+ if path.extension().and_then(|ext| ext.to_str()) != Some(NOTE_FILE_EXTENSION) {
continue;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if path.extension().and_then(|ext| ext.to_str()) != Some("yml") { | |
| continue; | |
| } | |
| if path.extension().and_then(|ext| ext.to_str()) != Some(NOTE_FILE_EXTENSION) { | |
| continue; | |
| } |
🤖 Prompt for 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.
In `@crates/frilvault-core/src/storage/yaml_repository.rs` around lines 108 - 110,
In collect_note_files, replace the hardcoded extension check
path.extension().and_then(|ext| ext.to_str()) != Some("yml") with a comparison
against the NOTE_FILE_EXTENSION constant (e.g. != Some(NOTE_FILE_EXTENSION) or
compare ext.to_str() to NOTE_FILE_EXTENSION) so the filter uses the same
NOTE_FILE_EXTENSION as PathResolver::note_file_name; ensure NOTE_FILE_EXTENSION
is in scope or imported where collect_note_files is defined.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #20
Type of Change
Checklist
Summary by CodeRabbit
New Features
Documentation
Chores
flvt