feat: add Obsidian vault search support (--obsidian flag)#20
Merged
Conversation
Add ability to search markdown files in Obsidian vaults with a new --obsidian <path> flag. Uses ripgrep for fast search with pure Rust fallback. Skips hidden directories (.obsidian, .git, .trash). - Add ObsidianMatch struct and search_obsidian/search_obsidian_rust functions - Add print_obsidian_results for formatted output - Add test fixtures and 6 integration tests - Support AND semantics (all query terms must match) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Owner
Author
Automated Analysis ResultsVerdict: FAIL SummaryThe PR contains two blocker-level issues: a fragile ripgrep output parser that fails on paths containing colons (common on Windows) and a discrepancy in search semantics where multi-term AND logic is restricted to single lines rather than whole files. Several nits regarding path validation and test coverage were also identified. Required Changes
Follow-ups
Security Notes
Tool Notes
MiscThe reviewer suggests the README documentation implies file-level search which contradicts the line-level implementation. Generated by passfail.py passfail:structured-data{
"review": {
"verdict": "FAIL",
"summary": "The PR contains two blocker-level issues: a fragile ripgrep output parser that fails on paths containing colons (common on Windows) and a discrepancy in search semantics where multi-term AND logic is restricted to single lines rather than whole files. Several nits regarding path validation and test coverage were also identified.",
"relevant_files": [
"src/main.rs",
"tests/integration_tests.rs",
"tests/fixtures/obsidian-vault/Projects.md",
"Cargo.toml",
"README.md",
"tests/fixtures/obsidian-vault/Daily Notes.md"
],
"findings": [
{
"id": "F1",
"file": "src/main.rs",
"line_range": "search_obsidian",
"issue": "Ripgrep output parsing breaks on paths containing colons. The code uses line.find(':'), which fails for Windows absolute paths or vault paths with colons.",
"level": "Blocker",
"fix_hint": "Use ripgrep's --json output and parse structured records, or use --null separators for safe parsing."
},
{
"id": "F2",
"file": "src/main.rs",
"line_range": "search_obsidian",
"issue": "Ripgrep search and Rust fallback only match AND terms if they appear on the same line, whereas users expect file-level matching.",
"level": "Blocker",
"fix_hint": "Decide the intended contract. If file-level, scan entire file contents for all terms then emit relevant snippets."
},
{
"id": "F3",
"file": "src/main.rs",
"line_range": "find_markdown_files",
"issue": "Case-sensitive .md extension check skips valid files like README.MD.",
"level": "Nit",
"fix_hint": "Normalize extension case before comparison in the Rust fallback logic."
},
{
"id": "F4",
"file": "src/main.rs",
"line_range": "find_markdown_files",
"issue": "Hidden-file behavior is inconsistent; hidden directories are skipped but hidden files in visible directories are searched.",
"level": "Optional",
"fix_hint": "Either document this explicitly or skip dotfiles for consistency."
},
{
"id": "F5",
"file": "src/main.rs",
"line_range": "main",
"issue": "Vault path validation only checks for existence, not if the path is actually a directory.",
"level": "Nit",
"fix_hint": "Validate vault_path.is_dir() and emit a specific error if it is a file."
},
{
"id": "F6",
"file": "src/main.rs",
"line_range": "search_obsidian",
"issue": "Ripgrep command may search hidden items differently than the manual fallback traversal.",
"level": "Nit",
"fix_hint": "Sync the traversal policies between the two paths exactly."
},
{
"id": "F7",
"file": "tests/integration_tests.rs",
"line_range": null,
"issue": "No explicit test ensures parity between the ripgrep execution path and the Rust fallback path.",
"level": "Nit",
"fix_hint": "Add tests that force fallback mode and compare outputs to the ripgrep mode."
},
{
"id": "F8",
"file": "tests/integration_tests.rs",
"line_range": null,
"issue": "Tests do not cover vault paths containing colons, masking the parsing bug.",
"level": "Nit",
"fix_hint": "Add unit tests for the parser or use structured JSON parsing."
},
{
"id": "F9",
"file": "tests/integration_tests.rs",
"line_range": "obsidian_search",
"issue": "Tests do not disambiguate if AND terms must be on the same line or just in the same file.",
"level": "Nit",
"fix_hint": "Add a fixture where terms appear on separate lines to define expected behavior."
},
{
"id": "F10",
"file": "src/main.rs",
"line_range": "find_markdown_files",
"issue": "Potential performance issue on large vaults in fallback mode due to collecting all paths in memory before searching.",
"level": "Optional",
"fix_hint": "Search while walking the directory tree to allow for streaming results."
},
{
"id": "F11",
"file": "src/main.rs",
"line_range": null,
"issue": "The constant MAX_MATCHES_PER_SESSION is reused for Obsidian file matches, which is naming-wise misleading.",
"level": "Optional",
"fix_hint": "Rename to MAX_MATCHES_PER_SOURCE or introduce a separate constant."
},
{
"id": "F12",
"file": "src/main.rs",
"line_range": "print_obsidian_results",
"issue": "format_project_path usage for arbitrary vault files might be semantically confusing.",
"level": "FYI",
"fix_hint": "No action required unless a more neutral helper name is desired."
},
{
"id": "F13",
"file": "tests/integration_tests.rs",
"line_range": null,
"issue": "Missing negative test for passing a file instead of a directory to the vault path argument.",
"level": "Nit",
"fix_hint": "Add a test using a markdown file as the --obsidian argument."
}
],
"security_notes": [],
"aider_warnings": [
"gpt-5.4 does not support 'reasoning_effort', ignoring.",
"Repo-map can't include Cargo.lock (deleted from file system but not git?)"
],
"aider_errors": [],
"misc": "The reviewer suggests the README documentation implies file-level search which contradicts the line-level implementation.",
"is_infrastructure_error": false,
"infrastructure_error_message": ""
},
"usage": {
"total_tokens": "tokens unavailable",
"total_cost": "$0.19",
"notes": "Token counts are provided per-message but not summed as a total for the full run in the provided text; session cost is explicitly tracked and ended at $0.19."
}
} |
Owner
Author
Automated Analysis ResultsVerdict: FAIL SummaryThe PR introduces a search feature using ripgrep that contains several critical flaws, including broken search logic for multi-word queries, potential command injection vulnerabilities, and performance inefficiencies in the Rust fallback. Additionally, the integration tests use weak assertions that allow them to pass even when zero matches are found. Required Changes
Follow-ups
Security Notes
MiscThe reviewer noted that some issues, like the Windows path parsing bug, exist elsewhere in the codebase but are being highlighted here because they affect the new functionality. Generated by passfail.py passfail:structured-data{
"review": {
"verdict": "FAIL",
"summary": "The PR introduces a search feature using ripgrep that contains several critical flaws, including broken search logic for multi-word queries, potential command injection vulnerabilities, and performance inefficiencies in the Rust fallback. Additionally, the integration tests use weak assertions that allow them to pass even when zero matches are found.",
"relevant_files": [
"src/main.rs",
"tests/integration_tests.rs",
"tests/fixtures/obsidian-vault/Projects.md"
],
"findings": [
{
"id": "F1",
"file": "src/main.rs",
"line_range": "1333-1348",
"issue": "The rg command construction breaks intended AND semantics and is vulnerable to regex/argument injection. It treats multi-word queries as a single regex phrase and fails if the query starts with a dash or contains regex special characters.",
"level": "Blocker",
"fix_hint": "Use --fixed-strings and pass each query term as a separate -e argument to the ripgrep command."
},
{
"id": "F2",
"file": "tests/integration_tests.rs",
"line_range": "406-409",
"issue": "The test assertion for 'matches found' is too weak because the CLI output contains '0 matches found' on failure, which still satisfies the check. This hides the fact that the search is currently finding zero results.",
"level": "Blocker",
"fix_hint": "Strengthen assertions to check for specific non-zero match counts or expected filenames like Projects.md."
},
{
"id": "F3",
"file": "src/main.rs",
"line_range": "1428-1431",
"issue": "High memory allocation overhead due to calling file_path.clone() for every single line of every markdown file processed in the Rust fallback loop.",
"level": "Optional",
"fix_hint": "Refactor search_obsidian_rust to use a local counter outside the line loop instead of a HashMap keyed by PathBuf."
},
{
"id": "F4",
"file": "src/main.rs",
"line_range": "1376-1378",
"issue": "The parsing logic for ripgrep output assumes colons only separate path and line numbers, which will fail on Windows systems where absolute paths contain drive letter colons.",
"level": "Nit",
"fix_hint": "Consider splitting the output string from the right or utilizing ripgrep's --json output format for robust parsing."
},
{
"id": "F5",
"file": "src/main.rs",
"line_range": "38",
"issue": "The --obsidian flag is silently ignored if --openclaw is also provided due to the control flow logic in main().",
"level": "Nit",
"fix_hint": "Add conflicts_with = \"openclaw\" to the obsidian argument definition in Clap."
}
],
"security_notes": [
"Query strings are passed directly to a shell command (rg), which could lead to command argument injection if users provide inputs starting with dashes."
],
"aider_warnings": [],
"aider_errors": [],
"misc": "The reviewer noted that some issues, like the Windows path parsing bug, exist elsewhere in the codebase but are being highlighted here because they affect the new functionality.",
"is_infrastructure_error": false,
"infrastructure_error_message": ""
},
"usage": {
"total_tokens": "28k sent, 3.0k received",
"total_cost": "$0.31 session",
"notes": "Token and cost values represent the full session summary provided in the final aider output."
}
} |
Addresses passfail F5: validate that the vault path is actually a directory, not just that it exists. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Owner
Author
Passfail Findings Classification
Summary
|
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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
--obsidian <path>flag to search markdown files in Obsidian vaultsTest plan
🤖 Generated with Claude Code