Repository navigation
fix(filetools): follow-up correctness and performance fixes - #160
Conversation
…and channels - Fix manual JSON construction in tool_search.rs to use serde_json::json! macro for proper escaping of backslashes, quotes, and newlines in descriptions - Fix UTF-8 over-truncation in plugins/read/init.lua by using utf8_boundary from tool_view.lua instead of naive byte-based truncation - Do not ignore grep search errors in n00n-agent/src/tools/grep.rs; log warnings - Warn on/propagate walk errors in grep.rs instead of silently continuing - Add path existence check before building walker in grep.rs - Replace Arc<Mutex<Vec<GrepFileEntry>>> with flume channel in grep.rs to remove Mutex hot-path contention - Fix n00n-lua/src/api/fs.rs glob: add root validation, log walk errors, warn on non-UTF-8 paths, and use bounded sort for mtime with BinaryHeap - Fix collect_dir_entries silent error swallowing with explicit warn logging - Fix dir silent default for depth parameter to return error on wrong type
- Add streaming n00n.fs.read_lines with offset/limit and a 256-line prefix window for syntax highlighting context, avoiding full-file loads. - Fix plugins/lib/n00n/truncate.lua to truncate over-long lines instead of dropping them, respect byte budgets, and strip CR line endings. - Fix plugins/read/init.lua to use read_lines, use utf8.offset for byte-safe truncation, and correct line-number width formatting. - Fix plugins/grep/init.lua to strip leading/trailing quotes from patterns and correct line-number width formatting. - Replace ToolRegistry linear scans with an Arc-swapped ToolsSnapshot backed by a HashMap, and update n00n-ui's ToolsCache accordingly.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe changes add streaming filesystem line reads, UTF-8-safe truncation, stricter grep handling, JSON-based tool output, and HashMap-indexed tool registry snapshots. Related tests, exports, documentation, and changelog entries were updated. ChangesFile tool correctness
Tool registry and output
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ReadTool
participant n00n.fs.read_lines
participant File
ReadTool->>n00n.fs.read_lines: request offset and limit
n00n.fs.read_lines->>File: stream buffered UTF-8 lines
File-->>n00n.fs.read_lines: lines and total_lines
n00n.fs.read_lines-->>ReadTool: lines, prefix, total_lines
Possibly related PRs
Poem
✨ 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 |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Criterion
Details
| Benchmark suite | Current: 8842041 | Previous: effc288 | Ratio |
|---|---|---|---|
fib/jit_mlua_hook |
5308303 ns/iter (± 168566) |
6481806 ns/iter (± 170017) |
0.82 |
fib/jit_watchdog |
1898583 ns/iter (± 10237) |
2442795 ns/iter (± 6390) |
0.78 |
fib/jit_none |
1896915 ns/iter (± 30292) |
2443849 ns/iter (± 31481) |
0.78 |
fib/interp_mlua_hook |
5955194 ns/iter (± 236981) |
7687393 ns/iter (± 21353) |
0.77 |
fib/interp_watchdog |
3027951 ns/iter (± 84255) |
3890031 ns/iter (± 13264) |
0.78 |
fib/interp_none |
3031976 ns/iter (± 8454) |
3872294 ns/iter (± 17252) |
0.78 |
buffer_rw/jit_mlua_hook |
430990 ns/iter (± 10695) |
554028 ns/iter (± 11300) |
0.78 |
buffer_rw/jit_watchdog |
130482 ns/iter (± 443) |
167820 ns/iter (± 287) |
0.78 |
buffer_rw/jit_none |
130474 ns/iter (± 3716) |
167957 ns/iter (± 1691) |
0.78 |
buffer_rw/interp_mlua_hook |
821292 ns/iter (± 9107) |
1047403 ns/iter (± 3919) |
0.78 |
buffer_rw/interp_watchdog |
499312 ns/iter (± 5537) |
646436 ns/iter (± 10073) |
0.77 |
buffer_rw/interp_none |
499452 ns/iter (± 5224) |
647732 ns/iter (± 12756) |
0.77 |
splash_render_120x40 |
47903 ns/iter (± 7808) |
73489 ns/iter (± 4090) |
0.65 |
splash_render_200x60 |
143000 ns/iter (± 4938) |
178968 ns/iter (± 12812) |
0.80 |
This comment was automatically generated by workflow using github-action-benchmark.
…orm lint Merge origin/main so CI runs against current workspace, add changelog fragments for file tool fixes, regenerate lua-api docs for read_lines, and gate n00n-daemon Unix-only code so Windows/macOS lint passes.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_5eefe5ec-7fa7-42da-9b6d-af2190e1daee) |
|
cursor review |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_da39bc1d-ef7e-479e-8b20-af737a59c1f6) |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_efdd555a-cfbe-46b2-94db-1ff151449f5b) |
1 participant
Summary
Follow-up to #152. Adds streaming line-limited reads, fixes long-line truncation and quote handling, and replaces
ToolRegistrylinear scans with aHashMap-backed snapshot.Changes
n00n-lua/src/api/fs.rs: addn00n.fs.read_linesfor offset/limit streaming reads with a 256-line prefix window.plugins/read/init.lua: useread_lines, fixtruncate_byteswithutf8.offset, and correct line-number width.plugins/lib/n00n/truncate.lua: truncate long lines instead of dropping them, respect byte budgets, and strip\r.plugins/grep/init.lua: strip leading/trailing quotes from patterns and fix line-number width.n00n-agent/src/tools/registry.rs: O(1)get/haswithToolsSnapshot, batch duplicate detection.n00n-ui/src/agent/agent_loop.rs: updateToolsCacheto holdArc<ToolsSnapshot>.Verification
cargo fmt --allcargo clippy --all --tests -- -D warningscargo nextest run --workspaceAll green.
Risk
Low; bug fixes and performance improvements stacked on #152.
Note
Low Risk
Targeted file-tool and registry improvements with tests; no auth, security, or breaking API surface beyond new
read_lines.Overview
Follow-up correctness and performance work on file tools and the tool registry.
Streaming reads: Adds
n00n.fs.read_linesfor offset/limit line reads without loading whole files, with a 256-lineprefixfor UI context. The read plugin switches fromread+ manual splitting toread_lines, and line truncation usesutf8.offsetso multibyte characters are not split mid-codepoint.Truncation and grep UX:
n00n.truncatekeeps long lines (with ellipsis) within byte/line budgets instead of dropping them, and handles CRLF. The grep plugin strips wrapping quotes from patterns and fixes line-number column width; Rust grep collects parallel results via a channel, errors on missing paths, and logs walk/search failures.Registry and JSON:
ToolRegistrystores aToolsSnapshotwith a name → indexHashMapfor O(1)get/has(agent loop cache updated accordingly).tool_search/load_namespaceemit JSON viaserde_jsoninstead of hand-built strings (with escaping tests).FS helpers: glob validates the search root is a directory, improves walk error handling, and stabilizes mtime sorting; dir validates
depthand logs directory walk issues.Reviewed by Cursor Bugbot for commit 8842041. Bugbot is set up for automated code reviews on this repo. Configure here.