Fixed: surface grep/glob partial errors to callers#38
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (9)
WalkthroughThis PR extends glob and grep tools to track and report partial results and processing errors. In the core libraries, Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/llm-coding-tools-core/src/tools/glob.rs`:
- Around line 60-63: The push of format!("walk error: {err}") into the errors
Vec leaks filesystem paths; change both places that push this string (the
Err(err) arms that currently use format!("walk error: {err}")) to redact path
details by logging only a non-path error kind or a generic message (e.g., use
err.io_error().kind() or err.kind() if using walkdir::Error, or replace with
"walk error: <redacted>" / "walk error: <IO error: PermissionDenied>" ), so the
errors Vec never contains concrete filesystem paths; update both occurrences
referenced in glob.rs (the Err(err) arms).
In `@src/llm-coding-tools-core/src/tools/grep.rs`:
- Around line 145-148: The error string currently embeds the raw filesystem
error (format!("walk error: {err}")) which can leak host paths; change these to
sanitize/remove path details by replacing the embedded error text with a
non-path-specific token such as the IO error kind or a generic message (e.g.,
format!("walk error: {}", err.kind()) or simply "walk error") wherever
errors.push(...) is used in this file (the occurrences around the shown snippet
and the later block at lines ~237-251). Ensure you do the same substitution for
all similar pushes in grep.rs so callers no longer receive raw filesystem path
information.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/llm-coding-tools-core/src/tools/glob.rssrc/llm-coding-tools-core/src/tools/grep.rssrc/llm-coding-tools-serdesai/src/absolute/glob.rssrc/llm-coding-tools-serdesai/src/absolute/grep.rssrc/llm-coding-tools-serdesai/src/allowed/glob.rssrc/llm-coding-tools-serdesai/src/allowed/grep.rs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build and Test (Async/Tokio) (ubuntu-latest, x86_64-unknown-linux-gnu, false)
- GitHub Check: Build and Test (Blocking) (macos-latest, aarch64-apple-darwin, false)
- GitHub Check: Build and Test (Blocking) (windows-latest, x86_64-pc-windows-msvc, false)
- GitHub Check: Build and Test (Async/Tokio) (macos-latest, aarch64-apple-darwin, false)
- GitHub Check: Build and Test (Async/Tokio) (windows-latest, x86_64-pc-windows-msvc, false)
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.rs
📄 CodeRabbit inference engine (src/AGENTS.md)
src/**/*.rs: Preallocate collections when size is known or estimable usingString::with_capacity(estimated_len),Vec::with_capacity(count), orBufReader::with_capacity(size, reader)
Prefer&str/&[T]returns over owned types when lifetime allows
UseCow<'_, str>for conditional ownership (e.g.,String::from_utf8_lossy)
Use&'static strfor compile-time constant strings
Reuse buffers by calling.clear()and reusingVec/Stringinstead of reallocating
Use const generics for compile-time branching (e.g.,<const LINE_NUMBERS: bool>)
Use#[inline]on small, hot-path functions
Prefercoreoverstdwhere possible (e.g.,core::memoverstd::mem)
Usememchrcrate for fast byte searching over manual iteration
Keep modules under 500 lines (excluding tests); split if larger
Placeusestatements inside functions only for#[cfg]conditional compilation
Document public items with///and add examples in docs where helpful
Use//!for module-level documentation
Use [TypeName] rustdoc links instead of backticks in documentation
Files:
src/llm-coding-tools-serdesai/src/absolute/grep.rssrc/llm-coding-tools-serdesai/src/absolute/glob.rssrc/llm-coding-tools-serdesai/src/allowed/glob.rssrc/llm-coding-tools-core/src/tools/grep.rssrc/llm-coding-tools-core/src/tools/glob.rssrc/llm-coding-tools-serdesai/src/allowed/grep.rs
🧠 Learnings (3)
📚 Learning: 2026-02-07T22:53:26.067Z
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.067Z
Learning: Organize code with `llm-coding-tools-serdesai/` containing `src/absolute/`, `src/allowed/`, `src/schema.rs`, and `src/convert.rs` for serdesAI framework implementations
Applied to files:
src/llm-coding-tools-serdesai/src/absolute/grep.rssrc/llm-coding-tools-serdesai/src/absolute/glob.rssrc/llm-coding-tools-serdesai/src/allowed/glob.rssrc/llm-coding-tools-serdesai/src/allowed/grep.rs
📚 Learning: 2026-02-07T22:53:26.067Z
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.067Z
Learning: Applies to src/**/*.rs : Document public items with `///` and add examples in docs where helpful
Applied to files:
src/llm-coding-tools-serdesai/src/absolute/grep.rssrc/llm-coding-tools-serdesai/src/absolute/glob.rssrc/llm-coding-tools-serdesai/src/allowed/glob.rssrc/llm-coding-tools-core/src/tools/glob.rs
📚 Learning: 2026-02-07T22:53:26.067Z
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.067Z
Learning: Organize code with `llm-coding-tools-core/` as framework-agnostic core library containing `src/tools/`, `src/path/`, `src/error.rs`, `src/output.rs`, and `src/util.rs`
Applied to files:
src/llm-coding-tools-serdesai/src/absolute/grep.rssrc/llm-coding-tools-serdesai/src/absolute/glob.rssrc/llm-coding-tools-serdesai/src/allowed/glob.rssrc/llm-coding-tools-serdesai/src/allowed/grep.rs
🧬 Code graph analysis (3)
src/llm-coding-tools-serdesai/src/absolute/glob.rs (4)
src/llm-coding-tools-core/src/tools/glob.rs (1)
glob_files(31-123)src/llm-coding-tools-serdesai/src/convert.rs (1)
to_serdes_result(51-58)src/llm-coding-tools-serdesai/src/allowed/glob.rs (3)
output_content(15-21)glob_output_to_return(24-44)partial_glob_output_returns_json_payload(171-182)src/llm-coding-tools-core/src/output.rs (1)
truncated(29-34)
src/llm-coding-tools-serdesai/src/allowed/glob.rs (4)
src/llm-coding-tools-core/src/tools/glob.rs (1)
glob_files(31-123)src/llm-coding-tools-serdesai/src/convert.rs (1)
to_serdes_result(51-58)src/llm-coding-tools-serdesai/src/absolute/glob.rs (3)
output_content(15-21)glob_output_to_return(24-44)partial_glob_output_returns_json_payload(148-159)src/llm-coding-tools-core/src/output.rs (1)
truncated(29-34)
src/llm-coding-tools-core/src/tools/glob.rs (1)
src/llm-coding-tools-core/src/tools/grep.rs (2)
format(69-104)output(304-304)
🔇 Additional comments (23)
src/llm-coding-tools-serdesai/src/allowed/grep.rs (3)
9-9:serde_json::jsonimport is a clean fit for structured partial responses.
126-136: Partial grep outcomes are correctly promoted to structured payloads.This preserves readable formatted content while exposing
partial/errors/match_count/truncatedfor robust caller handling.
227-252: Good test coverage for the new partial JSON response path.src/llm-coding-tools-serdesai/src/absolute/glob.rs (4)
4-10: Import updates look correct for the new glob output mapping path.
14-44: Helper extraction for content/render branching is clear and maintainable.
96-99: Result handling now cleanly separates core error conversion from output shaping.
147-159: The new unit test correctly validates partial JSON payload semantics.src/llm-coding-tools-serdesai/src/absolute/grep.rs (3)
9-9:serde_json::jsonimport is appropriate for the new structured return branch.
120-130: Partial grep handling is correctly surfaced as JSON with full metadata.
263-289: Nice test addition for partial/error JSON behavior on search failures.src/llm-coding-tools-serdesai/src/allowed/glob.rs (4)
4-10: Import changes are coherent with the new structured glob output path.
14-44: The helper-based output conversion is straightforward and easy to reason about.
102-105: The updated match branch cleanly preserves error mapping and formats success outputs consistently.
170-182: Good focused unit test for partial JSON payload behavior.src/llm-coding-tools-core/src/tools/glob.rs (4)
20-25:GlobOutputmetadata extension is clean and well-documented.
48-48: Nice preallocation for the error buffer in the traversal loop.
117-122: Partial/error propagation in the finalGlobOutputis correctly wired.
223-250: Serialization tests for partial metadata are solid and targeted.src/llm-coding-tools-core/src/tools/grep.rs (5)
50-55:GrepOutputnow carrying partial/error metadata is a good API evolution.
95-101: Partial-results marker in formatter is a useful human-readable signal.
133-134: Good call preallocating the error accumulator.
170-220: The newFileSearchResultflow correctly keeps matches while accumulating partial errors.
294-337: Great additions for partial-formatting and error-path tests.
0952beb to
e77d318
Compare
Capture per-file traversal and search failures in core grep/glob as structured partial metadata (partial, errors) instead of silently dropping them. Propagate partial state through serdesAI absolute/allowed wrappers using explicit JSON payloads while preserving existing non-partial text behavior. Deduplicate wrapper conversion logic by extracting shared helpers in common::glob and common::grep so absolute and allowed variants stay consistent.
e77d318 to
3db04e8
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #38 +/- ##
==========================================
- Coverage 75.25% 75.25% -0.01%
==========================================
Files 65 67 +2
Lines 1924 1964 +40
==========================================
+ Hits 1448 1478 +30
- Misses 476 486 +10
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Summary
What Changed
Core grep (
llm-coding-tools-core/src/tools/grep.rs)searcher.search_path(...)failures per file while preserving readable-file matchesGrepOutputwithpartial: boolanderrors: Vec<String>Core glob (
llm-coding-tools-core/src/tools/glob.rs)strip_prefixprocessing failures with path contextGlobOutputwithpartial: boolanderrors: Vec<String>serdesAI wrappers
content,partial,errors,match_count,truncatedcontent,partial,errors,truncatedDedup follow-ups
llm-coding-tools-serdesai/src/common/glob.rsllm-coding-tools-serdesai/src/common/grep.rsCommit
e77d318— Fixed: Surface grep/glob partial errors end-to-endBehavior Notes
Testing
cargo fmt.cargo/verify.shcargo publish --dry-run -p llm-coding-tools-agentsbecause crates.io currently cannot resolvellm-coding-tools-core = ^0.2.0(pre-existing publish environment issue)Review
@coderabbitreview run on committed changes; no findings