Repository navigation
feat: 009 explore tooling enhancement - #221
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change defines the Explore Tooling Enhancement and implements expanded routing, native and CLI-backed CodeGraph and Arbor operations, Semblem hybrid search, RTK command handling, Lua APIs, prompts, contracts, tests, and validation guides. ChangesExplore Tooling Enhancement
Estimated code review effort: 5 (Critical) | ~90+ minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Explore
participant Backend
participant Index
Client->>Explore: submit exploration query
Explore->>Backend: route intent to Arbor, CodeGraph, or Semblem
Backend->>Index: query native index or invoke CLI fallback
Index-->>Backend: return results
Backend-->>Explore: return formatted output
Explore-->>Client: return exploration response
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
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_bf3bb5d4-d47a-45ff-bc20-62a427914896) |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
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_00b846e0-47cc-4dcc-a1d1-34f257151f07) |
…(phase 2) - Extend router.lua with new intents: search, skeleton, symbol, impact, trace - Update explore/init.lua schema to include new intents in enum - Add routing logic for search→semblem, skeleton→index, symbol→codegraph, impact→arbor, trace→arbor - Create explore-queries.json fixture with 20 labeled queries - Add router classification accuracy test (≥90% threshold) - Update NATIVE_EFFICIENT_TOOLS to remove 'optional' qualifiers from arbor/codegraph - Update system.md, general.md, research.md prompts to position explore/index/arbor/codegraph/semblem before grep/bash - Update arbor/init.lua and codegraph/init.lua tool descriptions to remove external CLI installation notes
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_a476802d-21cf-4c6c-b53b-c35c13fe11cd) |
- Update router test to load queries from fixture file - Add auto-detection patterns for search, skeleton, and trace intents - Expand fixture with 30 labeled queries for better coverage
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_67d94d17-3706-49d5-a59c-68fedc9ba8e1) |
![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: 6fb41eb | Previous: e419592 | Ratio |
|---|---|---|---|
fib/jit_mlua_hook |
6672592 ns/iter (± 131450) |
5067363 ns/iter (± 184829) |
1.32 |
fib/jit_watchdog |
2326075 ns/iter (± 6858) |
1897339 ns/iter (± 4143) |
1.23 |
fib/jit_none |
2320841 ns/iter (± 68723) |
1896098 ns/iter (± 11489) |
1.22 |
fib/interp_mlua_hook |
8154297 ns/iter (± 36884) |
5965016 ns/iter (± 18494) |
1.37 |
fib/interp_watchdog |
4256012 ns/iter (± 23198) |
3137033 ns/iter (± 10630) |
1.36 |
fib/interp_none |
4258480 ns/iter (± 103715) |
3144704 ns/iter (± 10846) |
1.35 |
buffer_rw/jit_mlua_hook |
584702 ns/iter (± 2323) |
431688 ns/iter (± 814) |
1.35 |
buffer_rw/jit_watchdog |
192066 ns/iter (± 270) |
130527 ns/iter (± 178) |
1.47 |
buffer_rw/jit_none |
191844 ns/iter (± 208) |
130232 ns/iter (± 151) |
1.47 |
buffer_rw/interp_mlua_hook |
1045980 ns/iter (± 21607) |
810555 ns/iter (± 1655) |
1.29 |
buffer_rw/interp_watchdog |
582996 ns/iter (± 1753) |
498424 ns/iter (± 6130) |
1.17 |
buffer_rw/interp_none |
582252 ns/iter (± 3989) |
493653 ns/iter (± 5498) |
1.18 |
splash_render_120x40 |
66414 ns/iter (± 6478) |
39289 ns/iter (± 7422) |
1.69 |
splash_render_200x60 |
151893 ns/iter (± 26352) |
144043 ns/iter (± 1132) |
1.05 |
This comment was automatically generated by workflow using github-action-benchmark.
… sync, files commands - Add new CLI wrapper functions in n00n-codegraph/src/lib.rs for all CodeGraph 1.5.0 commands - Add native SQLite query functions in n00n-codegraph/src/db.rs for callers, callees, impact, node, query, files - Add Lua API bindings in n00n-lua/src/api/codegraph.rs for new commands - Update plugins/codegraph/init.lua to accept command enum and dispatch to new API functions - Add tests for new database query functions - Refactor explore_cli to use shared wait_for_output helper - Update tasks.md to mark Phase 1 verification as deferred due to system load
…rgo.toml comment - Add affected_database function in n00n-codegraph/src/db.rs for native SQLite queries - Update affected function in n00n-codegraph/src/lib.rs to use database when available - Add CodeGraph 1.5.0 target comment to n00n-codegraph/Cargo.toml - Mark all Phase 3 tasks as complete in tasks.md
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_6066d87f-2979-4f6f-b349-da70c449fac8) |
- Add explicit jq/yq passthrough in rtk_rewrite (FR-018) - Update tool description to include podman, docker, npm, pip, python (FR-017) - Update prompt hints to explicitly recommend rtk-wrapped bash (FR-019) - Add Lua tests for rtk command table and jq/yq passthrough (T086-T092) - RTK availability caching already implemented via module-level rtk_available variable
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_696e7ad0-fa2d-42b1-b599-9ffd40ee4c63) |
- Update AGENTS.md token-efficient section to mention explore router with semblem routing and index priority - Update AGENTS.md RTK patterns to include podman, docker, npm, pip, python - Prompts (system.md, general.md, research.md) already position explore/index/arbor/codegraph/semblem before grep/bash from Phase 2 - NATIVE_EFFICIENT_TOOLS already includes tools without optional qualifiers from Phase 2 - n00n-config tool output budgets unchanged (no new budget requirements) - Defer just gen-docs due to system load
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_37b37beb-8e62-4dc7-9913-0c89b238fc5e) |
- Replace unwrap_or with unwrap_or_else for DEFAULT_TIMEOUT_SECS fallbacks - Use inlined format args for command_name in error messages - All 30 tests pass, clippy clean for n00n-codegraph package
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_dd2e7e14-a9ec-4f18-87c2-b6b90065205b) |
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_fe028080-98bf-4f89-a6fd-a79be9ec3bb8) |
w0wl0lxd
changed the title
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_68191d32-b937-4977-b99a-2634ecf1a234) |
…tion The explore router was routing impact questions to arbor with an unsupported impact command. Codegraph has the impact command, so route there instead and use projectPath. Also tighten auto-detection so queries like "impact of changing parse_config.py" that look like a file path are not misclassified as impact; they fall through to cross_file. Update tests to expect codegraph for explicit impact and impact for plain symbol queries.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 76dbfe1a6f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Pushed additional fixes for the review feedback:
Verification:
|
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_0a8d8269-eefc-4b46-83ff-493af1894884) |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4fef11f15c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
specs/009-explore-tooling-enhancement/contracts/arbor.md (1)
10-33: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftAdd conditional requirements for Arbor commands.
The schema permits an empty object and does not require
command. It also permitscallers,callees, andinspectwithoutsymbol,trace_pathandpathwithout both symbols, andrefactorwithoutoperation. Add conditional schema requirements or explicit validation errors for each command.🤖 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 `@specs/009-explore-tooling-enhancement/contracts/arbor.md` around lines 10 - 33, Update the Arbor request schema around the command property to require command and enforce command-specific fields: require symbol for callers, callees, and inspect; require both from_symbol and to_symbol for trace_path and path; and require operation for refactor. Reject empty objects and invalid command payloads through conditional schema requirements or explicit validation errors.n00n-lua/src/api/codegraph.rs (1)
302-325: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
nodedescription overstates the native SQLite result.Line 306 documents
nodeas returning "source + caller/callee trail" for both the native and CLI paths.node_database(n00n-codegraph/src/db.rs, lines 99-108) only runssearch_nodeswith a limit of 1 and formats the single matched node; it never joinsedgesto compute callers or callees. When the native SQLite path is used (the preferred path whenever a database exists), the agent receives node details only, not a caller/callee trail. The description misrepresents the native-path output and can mislead the agent into skipping a needed follow-upcallers/calleescall.📝 Proposed fix
- desc: "Get one symbol's source + caller/callee trail using native SQLite when available, otherwise `codegraph node`.", + desc: "Get one symbol's source location and signature using native SQLite when available, otherwise `codegraph node` (which may include a caller/callee trail).",🤖 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 `@n00n-lua/src/api/codegraph.rs` around lines 302 - 325, Update the `FnDoc` entry for `node` so its `desc` accurately describes the native SQLite behavior as returning the matched symbol’s source/details without claiming a caller/callee trail; retain the CLI fallback distinction only if it remains accurate, and leave the parameters and return documentation unchanged.n00n-semble/src/lib.rs (1)
301-342: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated native-fallback logic.
search_hybridbuilds the sameSearchRequestand callsSelf::search(&request)twice: once in the CLIErrbranch, once as the final fallback.find_related_hybridduplicates its full index-open/find_related/format_resultssequence the same way.Extract each duplicated block into a private helper (e.g.,
fn native_search(...)andfn native_find_related(...)) and call the helper from both the CLI-error branch and the CLI-unavailable path. This keeps the two fallback paths from diverging when one is updated but not the other.♻️ Proposed refactor sketch for `find_related_hybrid`
+ fn native_find_related( + repo_path: &Path, + file_path: &str, + line: usize, + top_k: Option<usize>, + ) -> Result<String, SembleError> { + let index_dir = Self::index_dir(repo_path); + let mut index = SearchIndex::open_or_create(&index_dir, &SearchConfig::default()) + .map_err(map_search_err)?; + if !Self::has_index(repo_path) { + index.update(repo_path, |_| {}).map_err(map_search_err)?; + } + let results = index.find_related(file_path, line).map_err(map_search_err)?; + Ok(format_results( + &results.into_iter().take(resolve_top_k(top_k)).collect::<Vec<_>>(), + )) + } + pub fn find_related_hybrid( repo: &str, file_path: &str, line: usize, top_k: Option<usize>, ) -> Result<String, SembleError> { let (repo_path, _temp_dir) = Self::resolve_repo_path(repo)?; if Self::cli_available() { match Self::cli_find_related(&repo_path, file_path, line, top_k) { Ok(result) => return Ok(result), Err(e) => { let mut fallback_output = format!("[semblem CLI unavailable: {e}]\n"); - let index_dir = Self::index_dir(&repo_path); - let mut index = - SearchIndex::open_or_create(&index_dir, &SearchConfig::default()) - .map_err(map_search_err)?; - if !Self::has_index(&repo_path) { - index.update(&repo_path, |_| {}).map_err(map_search_err)?; - } - let results = index - .find_related(file_path, line) - .map_err(map_search_err)?; - let native_result = format_results( - &results - .into_iter() - .take(resolve_top_k(top_k)) - .collect::<Vec<_>>(), - ); + let native_result = Self::native_find_related(&repo_path, file_path, line, top_k)?; fallback_output.push_str(&native_result); return Ok(fallback_output); } } } - let index_dir = Self::index_dir(&repo_path); - let mut index = SearchIndex::open_or_create(&index_dir, &SearchConfig::default()) - .map_err(map_search_err)?; - if !Self::has_index(&repo_path) { - index.update(&repo_path, |_| {}).map_err(map_search_err)?; - } - let results = index - .find_related(file_path, line) - .map_err(map_search_err)?; - Ok(format_results( - &results - .into_iter() - .take(resolve_top_k(top_k)) - .collect::<Vec<_>>(), - )) + Self::native_find_related(&repo_path, file_path, line, top_k) }Also applies to: 344-401
🤖 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 `@n00n-semble/src/lib.rs` around lines 301 - 342, Extract the duplicated native fallback logic into private helpers, such as native_search for constructing SearchRequest and calling Self::search, and native_find_related for the existing index-open/find_related/format_results sequence. Update search_hybrid and find_related_hybrid to call these helpers from both CLI error branches and CLI-unavailable paths, preserving their current outputs and error behavior.
♻️ Duplicate comments (13)
.specify/memory/constitution.md (1)
13-15: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse the required library/binary error split.
Line 15 still permits
anyhowwithout distinguishing library crates from binary edges. Requirethiserrorfor library domain errors andcolor-eyreat binary edges.As per coding guidelines, Rust libraries must use
thiserrorfor domain-specific errors and binary edges must usecolor-eyre.🤖 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 @.specify/memory/constitution.md around lines 13 - 15, Update the “Safety & No Silent Failures” constitution rule to require thiserror for domain-specific errors in Rust library crates and color-eyre for error handling at binary edges, replacing the broad anyhow allowance while retaining the existing no-silent-failure requirements.Source: Coding guidelines
specs/009-explore-tooling-enhancement/contracts/explore.md (2)
10-16: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMake
queryconditional on the command.The schema requires
queryfor every request, butmap,diff, andstatuscan be constructed without a query inplugins/explore/router.lua. This prevents valid queryless commands. Define conditional requirements for these commands or remove them from the enum.This follows the supplied
plugins/explore/router.luabehavior.🤖 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 `@specs/009-explore-tooling-enhancement/contracts/explore.md` around lines 10 - 16, Update the explore request schema around the query property to make query required only for commands that need it, while allowing map, diff, and status requests without query as supported by the router command handling. Preserve query validation for commands that require a target, and align the command conditions with the existing router behavior.
54-55: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAlign the
symbolandimpactbackend mappings.The contract lists both
arborandcodegraph, but the supplied router sends explicitsymbolandimpactintents tocodegraph. Document the deterministic CodeGraph route or define an actual fallback condition.This follows
plugins/explore/router.lua:208-279.🤖 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 `@specs/009-explore-tooling-enhancement/contracts/explore.md` around lines 54 - 55, Update the contract entries for symbol and impact to document the deterministic CodeGraph routing used by the router’s explicit intents, replacing the ambiguous “arbor or codegraph” mapping; only describe an Arbor fallback if an actual fallback condition exists in the implementation.specs/009-explore-tooling-enhancement/contracts/arbor.md (1)
42-46: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDefine zero-budget behavior or reject it.
Line 44 allows
token_budget: 0, but the contract does not define whether zero means empty output or another mode. If the map implementation requires a positive budget, setminimum: 1; otherwise document and test zero-budget behavior.🤖 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 `@specs/009-explore-tooling-enhancement/contracts/arbor.md` around lines 42 - 46, Resolve the undefined zero-budget behavior for the token_budget contract used by the map command: either change its minimum to 1 if positive budgets are required, or document the intended zero-budget result and add coverage for it. Keep the existing default and token_budget schema context unchanged.specs/009-explore-tooling-enhancement/research.md (1)
22-31: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftEnforce the external CodeGraph version requirement.
Both documents require CodeGraph 1.5.0 but do not define an executable compatibility gate. A 1.4.1 binary can pass a presence check and still fail when new commands are used.
specs/009-explore-tooling-enhancement/research.md#L22-L31: define version/capability validation and fallback or typed-error behavior.specs/009-explore-tooling-enhancement/plan.md#L121-L128: replace the manifest-only task with an implementation and validation task.🤖 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 `@specs/009-explore-tooling-enhancement/research.md` around lines 22 - 31, Define an executable CodeGraph compatibility gate requiring version 1.5.0 or equivalent command capabilities, with an explicit fallback or typed-error behavior when validation fails; update specs/009-explore-tooling-enhancement/research.md lines 22-31 to document this validation, and replace the manifest-only task in specs/009-explore-tooling-enhancement/plan.md lines 121-128 with implementation and validation work.specs/009-explore-tooling-enhancement/contracts/semblem.md (1)
19-21: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMake Semblem fallback capability-aware.
contentfiltering and remote repository support are documented as upstream-only features. The fallback still sends allsearchandfind_relatedrequests to native BM25 when the upstream CLI is unavailable. Native BM25 cannot honor these inputs unless it implements equivalent support.If
contentor a remoterepois provided, implement equivalent native behavior or return an explicit unsupported error. Also detect older or incomplete upstream CLI installations instead of checking availability only.This follows the Semblem contract and the local-index behavior documented in
specs/009-explore-tooling-enhancement/research.md.Also applies to: 41-44, 83-90, 93-98
🤖 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 `@specs/009-explore-tooling-enhancement/contracts/semblem.md` around lines 19 - 21, The Semblem fallback must not silently send unsupported requests to native BM25. Update the native fallback paths for search and find_related to detect content filters and remote repo values, either implementing equivalent behavior or returning an explicit unsupported error; also validate required upstream CLI capabilities rather than availability alone. Apply the same capability-aware handling to the documented request and fallback sections.n00n-codegraph/src/db.rs (1)
145-239: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winHoist the
file_match_stmtprepare out of the loop.Lines 208-215 prepare a new statement on every iteration of the
for file in filesloop. This reproduces the pattern flagged in the earlier review: SQLite re-parses and re-plans the SQL text on each iteration.find_nodes_stmt,callers_stmt, andcallees_stmtwere already hoisted, but this new statement was not.The query text at line 209 duplicates
find_nodes_stmt'sWHEREclause. Selectfile_pathalongsideidinfind_nodes_stmtand removefile_match_stmtentirely.⚡ Proposed fix to eliminate the redundant statement
// Prepare statement once for finding nodes in changed files let mut find_nodes_stmt = conn - .prepare("SELECT id FROM nodes WHERE file_path LIKE ?1 ESCAPE '\\'") + .prepare("SELECT id, file_path FROM nodes WHERE file_path LIKE ?1 ESCAPE '\\'") .map_err(|source| CodegraphError::Sqlite { source })?; @@ // Find all nodes in the changed file let node_rows = find_nodes_stmt - .query_map((pattern.as_str(),), |row| row.get::<_, String>(0)) + .query_map((pattern.as_str(),), |row| { + Ok((row.get::<_, String>(0)?, row.get::<_, String>(1)?)) + }) .map_err(|source| CodegraphError::Sqlite { source })?; - for node_id_result in node_rows { - let node_id = node_id_result.map_err(|source| CodegraphError::Sqlite { source })?; + for node_result in node_rows { + let (node_id, own_file_path) = + node_result.map_err(|source| CodegraphError::Sqlite { source })?; + seen.insert(own_file_path); @@ } - - // Also include the changed file itself - // Since we're using LIKE pattern, we need to get the actual file paths - let mut file_match_stmt = conn - .prepare("SELECT DISTINCT file_path FROM nodes WHERE file_path LIKE ?1 ESCAPE '\\'") - .map_err(|source| CodegraphError::Sqlite { source })?; - - let file_matches = file_match_stmt - .query_map((pattern.as_str(),), |row| row.get::<_, String>(0)) - .map_err(|source| CodegraphError::Sqlite { source })?; - - for file_path_result in file_matches { - seen.insert(file_path_result.map_err(|source| CodegraphError::Sqlite { source })?); - } }🤖 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 `@n00n-codegraph/src/db.rs` around lines 145 - 239, Update affected_database by changing find_nodes_stmt to select each node’s id and file_path, then use the returned file_path while iterating node_rows to populate seen. Remove the per-file file_match_stmt preparation and file_matches query entirely, preserving the existing pattern matching and affected-file results.n00n-lua/src/api/codegraph.rs (1)
278-301: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
affecteddescription still omits the native SQLite path.Line 281 no longer claims the result is limited to "test files," which fixes half of the earlier concern. The description still does not mention the native SQLite path. Every sibling entry (
callers,callees,impact,node,query,files) states "using native SQLite when available, otherwisecodegraph <cmd>."Client::affectedfollows the same has-database-then-CLI-fallback pattern as those operations, so theaffecteddescription should match.📝 Proposed fix
- desc: "Accept an array of file paths and compute the affected file set using `codegraph affected`.", + desc: "Find indexed files affected by the given changed files using native SQLite when available, otherwise `codegraph affected`.",🤖 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 `@n00n-lua/src/api/codegraph.rs` around lines 278 - 301, Update the FnDoc entry for affected so its desc documents the native SQLite execution path when available and the codegraph affected CLI fallback otherwise, matching the wording and behavior documented by sibling entries such as callers, callees, impact, node, query, and files.n00n-semble/src/lib.rs (1)
213-234: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftAdd a timeout to
git cloneand sanitize failure output.
Command::new("git").arg("clone")...output()has no timeout. A slow or unresponsive remote hangs the calling thread indefinitely. The earlier review named this as an open concern ("no size or timeout limit"), and this diff does not add one.Separately,
format!("git clone failed: {stderr}")embeds git's raw stderr, which can include the repository URL. Ifrepocontains embedded credentials, this text can leak them into the returned error and thus into the tool output shown to the caller.Bound the clone with a timeout, and strip userinfo/credentials from
repobefore including it in any returned message.As per coding guidelines: "Provide helpful, sanitized error messages" and "Do not commit or log credentials, API keys, tokens... in code or logs."
🤖 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 `@n00n-semble/src/lib.rs` around lines 213 - 234, Update the git clone flow around the Command::new("git") invocation to enforce a finite timeout, returning a sanitized SembleError::Cli when the operation exceeds it. Before constructing any returned clone-failure message, sanitize repo by removing embedded userinfo or credentials, and avoid exposing raw stderr; retain only safe diagnostic context alongside the sanitized repository reference.Source: Coding guidelines
n00n-arbor/src/graph_query.rs (1)
192-208: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winBound
graph_entry_pointsresults; the definition fix is only half the past comment.The entry-point definition now matches
graph_map(caller_count(idx) == 0 && callee_count(idx) > 0). This resolves the mismatch flagged before.The result set is still unbounded.
graph_entry_pointsreturns every matching node without a limit.graph_mapacceptstoken_budgetfor the same reason.plugins/arbor/init.luaformats the full list intollm_output, so a large repository still floods the agent context.Add a size or token limit to
graph_entry_points, similar to thetoken_budgetparameter ongraph_map.🤖 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 `@n00n-arbor/src/graph_query.rs` around lines 192 - 208, Update graph_entry_points to accept and enforce a size or token budget, mirroring the existing token_budget behavior in graph_map. Stop collecting matching Relation entries once the configured limit is reached, while preserving the current entry-point predicate and return type.plugins/arbor/init.lua (1)
144-184: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winReport the native-query failure instead of discarding it.
In
map(146-155) andentry_points(246-252), whenreadyis true but the nativepcallfails, the code setsentries/resultstoniland drops the error value. The laterready_errcheck only reports index-readiness failures, not this failure. If the native call fails for another reason (for example, a corrupted.arbor/graph.json), the plugin silently falls through to the CLI and the real cause is never surfaced.Capture the
pcallerror message in both branches and report it, the same wayready_erris already reported.🔧 Proposed fix for `map`; apply the same pattern to `entry_points`
local ready, ready_err = native_index_ready(project) local entries + local native_err if ready then local ok, result = pcall(n00n_arbor.graph_map, project, token_budget) if ok then entries = result else - entries = nil + native_err = tostring(result) end end if not entries then if not n00n_arbor.available() then + if native_err then + return { + llm_output = "error: native graph map failed and arbor CLI is unavailable: " .. native_err, + is_error = true, + } + end return missing_cli(command) endAlso applies to: 241-280
🤖 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 `@plugins/arbor/init.lua` around lines 144 - 184, Preserve the native-query error in both the map branch around n00n_arbor.graph_map and the entry_points branch around its native pcall instead of discarding it when pcall returns false. Capture each failure message separately from ready_err, and report it through the existing error response before falling back to CLI execution, while retaining the current handling for index-readiness and refresh failures.n00n-arbor/src/lib.rs (2)
534-582: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCollapse the seven near-identical tests and tighten the assertion.
The seven tests differ only in the method called, and
assert_cli_requires_cliaccepts eitherArborError::ExecorArborError::Clias a pass. Any error satisfies the assertion, and the outcome still depends on whetherarboris installed on the test machine — when it is, these tests pass on aClierror from a missing index rather than testing "CLI absent" behavior.Use
#[test_case]to remove the duplication. Where the intent is "the binary is absent," skip the test whenClient::check_binary()succeeds and assertArborError::Execspecifically.As per coding guidelines: "Place unit tests in the same Rust file inside
#[cfg(test)]modules; use#[test_case], snake_case test names, shared constants for error/status assertions, and avoid tautological or flaky tests."🤖 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 `@n00n-arbor/src/lib.rs` around lines 534 - 582, Replace the seven duplicated CLI-required tests and permissive assert_cli_requires_cli helper with a parameterized #[test_case] test covering each Client method. Skip the test when Client::check_binary() succeeds, then assert the invocation returns specifically ArborError::Exec rather than accepting ArborError::Cli or any other error; retain the existing method arguments and snake_case test naming conventions.Source: Coding guidelines
345-348: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftUngated
refactormutation is exposed end-to-end without a safeguard. The mutatingarbor refactor <operation> <project>call is reachable from native Rust through the Lua binding to the LLM-callable tool, with no allowlist, scoped authorization, confirmation, or audit event at any layer. The shared root cause is the missing gate onoperation.
n00n-arbor/src/lib.rs#L345-L348: restrictClient::refactor'soperationparameter to an allowlist of known-safe operations, or require an explicit confirmation flag before executing.n00n-lua/src/api/arbor.rs#L169-L175: gate therefactorbinding behind the existing tool-permission path, with an audit log entry, before registering it in the Lua table.plugins/arbor/init.lua#L343-L362: require explicit user context or confirmation before invokingn00n_arbor.refactor, and emit an audit event for the mutating call.As per coding guidelines: "Tool execution requires allowlisted tools, scoped credentials, explicit user context, audit events, and refusal or denial tests."
🤖 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 `@n00n-arbor/src/lib.rs` around lines 345 - 348, The mutating refactor operation lacks authorization, confirmation, and auditing across its execution path. In n00n-arbor/src/lib.rs lines 345-348, gate Client::refactor’s operation with an allowlist of safe operations or explicit confirmation; in n00n-lua/src/api/arbor.rs lines 169-175, use the existing tool-permission path and emit an audit event before registering the refactor binding; in plugins/arbor/init.lua lines 343-362, require explicit user context or confirmation before calling n00n_arbor.refactor and emit an audit event for the mutation.Source: Coding guidelines
🤖 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 `@n00n-arbor/src/lib.rs`:
- Around line 87-108: Update run_arbor_cmd to execute the arbor subprocess using
the existing wait_for_output timeout pattern and DEFAULT_TIMEOUT_SECS convention
from n00n-codegraph, replacing the blocking Command::output call. Preserve the
current ArborError mapping for spawn failures, non-success status, and
successful stdout output, while ensuring timed-out commands return the
established timeout error.
In `@n00n-codegraph/src/lib.rs`:
- Around line 517-632: Check workspace usage and dev-dependencies for the
test-case crate before adopting it. If available, replace the repeated Client
operation tests covering missing inputs and missing index directories with
parameterized #[test_case] tests, preserving assert_cli_error and the shared
constants (QUERY_REQUIRED, SYMBOL_REQUIRED, NAME_REQUIRED, SEARCH_REQUIRED,
FILES_REQUIRED, and NO_INDEX). Keep snake_case test names and ensure each case
still invokes the correct Client method and validates its specific message.
In `@n00n-semble/src/lib.rs`:
- Around line 199-211: Update the allowlist matching in the remote repository
authorization block to accept a prefix only when repo exactly equals prefix or
has a valid boundary immediately after it, such as '/'. Replace the unrestricted
repo.starts_with(prefix) check while preserving the existing wildcard behavior
and rejection error in the surrounding SembleError::Cli flow.
In `@plugins/explore/router.lua`:
- Line 85: Rename the local `path` binding in `normalize_intent` near the
`input.path or M.extract_file_path(input.query)` assignment to `skeleton_path`,
and update all references in that scope accordingly to eliminate shadowing of
the existing `path` variable.
In `@plugins/explore/tests/spec.lua`:
- Around line 5-10: Update try_read_one to capture both return values from
n00n.fs.read, and use the error result to distinguish successful reads from
failures. Preserve the existing fallback behavior by returning content only when
the read succeeds, allowing failures to continue trying alternate fixture paths.
In `@plugins/semblem/init.lua`:
- Line 27: Update the `savings` entry in the semblem command documentation to
say “token savings” instead of “time savings,” matching the terminology
documented by the corresponding API command. Preserve the existing
semantic-search and CLI requirement details.
In `@specs/009-explore-tooling-enhancement/research.md`:
- Line 45: Resolve the undocumented audit command discrepancy in the research
for Arbor tooling: either remove audit from the missing-upstream-commands list,
or add it consistently to the command schema, implementation plan, and tests.
Keep the chosen scope aligned across all documented contract and implementation
references.
- Around line 135-141: Align the skeleton-routing decision across the research,
the explore contract, and the router: either document `skeleton` as routing to
`index` as the file-skeleton alias, or update both `contracts/explore.md` and
the router implementation to consistently route it to `arbor map`; ensure no
conflicting behavior remains.
---
Outside diff comments:
In `@n00n-lua/src/api/codegraph.rs`:
- Around line 302-325: Update the `FnDoc` entry for `node` so its `desc`
accurately describes the native SQLite behavior as returning the matched
symbol’s source/details without claiming a caller/callee trail; retain the CLI
fallback distinction only if it remains accurate, and leave the parameters and
return documentation unchanged.
In `@n00n-semble/src/lib.rs`:
- Around line 301-342: Extract the duplicated native fallback logic into private
helpers, such as native_search for constructing SearchRequest and calling
Self::search, and native_find_related for the existing
index-open/find_related/format_results sequence. Update search_hybrid and
find_related_hybrid to call these helpers from both CLI error branches and
CLI-unavailable paths, preserving their current outputs and error behavior.
In `@specs/009-explore-tooling-enhancement/contracts/arbor.md`:
- Around line 10-33: Update the Arbor request schema around the command property
to require command and enforce command-specific fields: require symbol for
callers, callees, and inspect; require both from_symbol and to_symbol for
trace_path and path; and require operation for refactor. Reject empty objects
and invalid command payloads through conditional schema requirements or explicit
validation errors.
---
Duplicate comments:
In @.specify/memory/constitution.md:
- Around line 13-15: Update the “Safety & No Silent Failures” constitution rule
to require thiserror for domain-specific errors in Rust library crates and
color-eyre for error handling at binary edges, replacing the broad anyhow
allowance while retaining the existing no-silent-failure requirements.
In `@n00n-arbor/src/graph_query.rs`:
- Around line 192-208: Update graph_entry_points to accept and enforce a size or
token budget, mirroring the existing token_budget behavior in graph_map. Stop
collecting matching Relation entries once the configured limit is reached, while
preserving the current entry-point predicate and return type.
In `@n00n-arbor/src/lib.rs`:
- Around line 534-582: Replace the seven duplicated CLI-required tests and
permissive assert_cli_requires_cli helper with a parameterized #[test_case] test
covering each Client method. Skip the test when Client::check_binary() succeeds,
then assert the invocation returns specifically ArborError::Exec rather than
accepting ArborError::Cli or any other error; retain the existing method
arguments and snake_case test naming conventions.
- Around line 345-348: The mutating refactor operation lacks authorization,
confirmation, and auditing across its execution path. In n00n-arbor/src/lib.rs
lines 345-348, gate Client::refactor’s operation with an allowlist of safe
operations or explicit confirmation; in n00n-lua/src/api/arbor.rs lines 169-175,
use the existing tool-permission path and emit an audit event before registering
the refactor binding; in plugins/arbor/init.lua lines 343-362, require explicit
user context or confirmation before calling n00n_arbor.refactor and emit an
audit event for the mutation.
In `@n00n-codegraph/src/db.rs`:
- Around line 145-239: Update affected_database by changing find_nodes_stmt to
select each node’s id and file_path, then use the returned file_path while
iterating node_rows to populate seen. Remove the per-file file_match_stmt
preparation and file_matches query entirely, preserving the existing pattern
matching and affected-file results.
In `@n00n-lua/src/api/codegraph.rs`:
- Around line 278-301: Update the FnDoc entry for affected so its desc documents
the native SQLite execution path when available and the codegraph affected CLI
fallback otherwise, matching the wording and behavior documented by sibling
entries such as callers, callees, impact, node, query, and files.
In `@n00n-semble/src/lib.rs`:
- Around line 213-234: Update the git clone flow around the Command::new("git")
invocation to enforce a finite timeout, returning a sanitized SembleError::Cli
when the operation exceeds it. Before constructing any returned clone-failure
message, sanitize repo by removing embedded userinfo or credentials, and avoid
exposing raw stderr; retain only safe diagnostic context alongside the sanitized
repository reference.
In `@plugins/arbor/init.lua`:
- Around line 144-184: Preserve the native-query error in both the map branch
around n00n_arbor.graph_map and the entry_points branch around its native pcall
instead of discarding it when pcall returns false. Capture each failure message
separately from ready_err, and report it through the existing error response
before falling back to CLI execution, while retaining the current handling for
index-readiness and refresh failures.
In `@specs/009-explore-tooling-enhancement/contracts/arbor.md`:
- Around line 42-46: Resolve the undefined zero-budget behavior for the
token_budget contract used by the map command: either change its minimum to 1 if
positive budgets are required, or document the intended zero-budget result and
add coverage for it. Keep the existing default and token_budget schema context
unchanged.
In `@specs/009-explore-tooling-enhancement/contracts/explore.md`:
- Around line 10-16: Update the explore request schema around the query property
to make query required only for commands that need it, while allowing map, diff,
and status requests without query as supported by the router command handling.
Preserve query validation for commands that require a target, and align the
command conditions with the existing router behavior.
- Around line 54-55: Update the contract entries for symbol and impact to
document the deterministic CodeGraph routing used by the router’s explicit
intents, replacing the ambiguous “arbor or codegraph” mapping; only describe an
Arbor fallback if an actual fallback condition exists in the implementation.
In `@specs/009-explore-tooling-enhancement/contracts/semblem.md`:
- Around line 19-21: The Semblem fallback must not silently send unsupported
requests to native BM25. Update the native fallback paths for search and
find_related to detect content filters and remote repo values, either
implementing equivalent behavior or returning an explicit unsupported error;
also validate required upstream CLI capabilities rather than availability alone.
Apply the same capability-aware handling to the documented request and fallback
sections.
In `@specs/009-explore-tooling-enhancement/research.md`:
- Around line 22-31: Define an executable CodeGraph compatibility gate requiring
version 1.5.0 or equivalent command capabilities, with an explicit fallback or
typed-error behavior when validation fails; update
specs/009-explore-tooling-enhancement/research.md lines 22-31 to document this
validation, and replace the manifest-only task in
specs/009-explore-tooling-enhancement/plan.md lines 121-128 with implementation
and validation work.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: efc1aabc-b639-4f1f-9847-c646f9082a58
📒 Files selected for processing (26)
.specify/memory/constitution.mdn00n-agent/src/prompts/research.mdn00n-arbor/src/graph_json.rsn00n-arbor/src/graph_query.rsn00n-arbor/src/lib.rsn00n-codegraph/src/db.rsn00n-codegraph/src/lib.rsn00n-lua/src/api/arbor.rsn00n-lua/src/api/codegraph.rsn00n-lua/src/api/semblem.rsn00n-semble/src/lib.rsplugins/arbor/init.luaplugins/bash/init.luaplugins/bash/tests/spec.luaplugins/codegraph/init.luaplugins/explore/init.luaplugins/explore/router.luaplugins/explore/tests/spec.luaplugins/semblem/init.luaspecs/009-explore-tooling-enhancement/contracts/arbor.mdspecs/009-explore-tooling-enhancement/contracts/explore.mdspecs/009-explore-tooling-enhancement/contracts/index.mdspecs/009-explore-tooling-enhancement/contracts/rtk.mdspecs/009-explore-tooling-enhancement/contracts/semblem.mdspecs/009-explore-tooling-enhancement/plan.mdspecs/009-explore-tooling-enhancement/research.md
💤 Files with no reviewable changes (1)
- plugins/codegraph/init.lua
📜 Review details
⏰ Context from checks skipped due to timeout. (17)
- GitHub Check: Unused deps
- GitHub Check: Test (Windows)
- GitHub Check: Coverage
- GitHub Check: MSRV (1.97)
- GitHub Check: Test (macOS)
- GitHub Check: Lint (macOS)
- GitHub Check: Rustdoc
- GitHub Check: Lint (Windows)
- GitHub Check: Build (Windows)
- GitHub Check: Build
- GitHub Check: Test
- GitHub Check: Docs
- GitHub Check: Lint
- GitHub Check: Analyze (rust)
- GitHub Check: Criterion
- GitHub Check: cargo-audit
- GitHub Check: cargo-deny
⚠️ CI failures not shown inline (4)
GitHub Actions: Changelog / 0_Fragment required.txt: feat: 009 explore tooling enhancement
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mgit fetch -q origin "$BASE" || true�[0m
�[36;1mchanged="$(git diff --name-only "origin/$BASE...HEAD" 2>/dev/null || git diff --name-only HEAD~1)"�[0m
�[36;1mecho "Changed files:"�[0m
�[36;1mecho "$changed"�[0m
�[36;1m�[0m
�[36;1mif ! printf '%s\n' "$changed" | grep -qE '^(src/|n00n-[^/]+/src/|plugins/)'; then�[0m
�[36;1m echo "no source changes detected; changelog fragment not required."�[0m
�[36;1m exit 0�[0m
�[36;1mfi�[0m
�[36;1m�[0m
�[36;1mlabels="$LABELS"�[0m
�[36;1mfor l in $labels; do�[0m
�[36;1m case "$l" in�[0m
�[36;1m no-changelog|dependencies|ci) echo "label '$l' set; skipping."; exit 0 ;;�[0m
�[36;1m esac�[0m
�[36;1mdone�[0m
�[36;1m�[0m
�[36;1mif printf '%s\n' "$changed" | grep -qE '^changelog\.d/[^/]+\.md$'; then�[0m
�[36;1m echo "changelog fragment present; good."�[0m
�[36;1m exit 0�[0m
�[36;1mfi�[0m
�[36;1m�[0m
�[36;1mecho "::error::Source changes require a changelog.d fragment. See changelog.d/README.md." >&2�[0m
GitHub Actions: Changelog / Fragment required: feat: 009 explore tooling enhancement
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mgit fetch -q origin "$BASE" || true�[0m
�[36;1mchanged="$(git diff --name-only "origin/$BASE...HEAD" 2>/dev/null || git diff --name-only HEAD~1)"�[0m
�[36;1mecho "Changed files:"�[0m
�[36;1mecho "$changed"�[0m
�[36;1m�[0m
�[36;1mif ! printf '%s\n' "$changed" | grep -qE '^(src/|n00n-[^/]+/src/|plugins/)'; then�[0m
�[36;1m echo "no source changes detected; changelog fragment not required."�[0m
�[36;1m exit 0�[0m
�[36;1mfi�[0m
�[36;1m�[0m
�[36;1mlabels="$LABELS"�[0m
�[36;1mfor l in $labels; do�[0m
�[36;1m case "$l" in�[0m
�[36;1m no-changelog|dependencies|ci) echo "label '$l' set; skipping."; exit 0 ;;�[0m
�[36;1m esac�[0m
�[36;1mdone�[0m
�[36;1m�[0m
�[36;1mif printf '%s\n' "$changed" | grep -qE '^changelog\.d/[^/]+\.md$'; then�[0m
�[36;1m echo "changelog fragment present; good."�[0m
�[36;1m exit 0�[0m
�[36;1mfi�[0m
�[36;1m�[0m
�[36;1mecho "::error::Source changes require a changelog.d fragment. See changelog.d/README.md." >&2�[0m
GitHub Actions: Secrets / Gitleaks: feat: 009 explore tooling enhancement
Conclusion: failure
##[group]Run gitleaks detect --source . --redact --no-banner --config .gitleaks.toml
�[36;1mgitleaks detect --source . --redact --no-banner --config .gitleaks.toml�[0m
shell: /usr/bin/bash -e {0}
env:
MISE_LOG_LEVEL: info
MISE_GITHUB_***REDACTED_SECRET_ASSIGNMENT***
MISE_TRUSTED_CONFIG_PATHS: /home/runner/work/n00n/n00n
MISE_YES: 1
##[endgroup]
�[90m5:54AM�[0m �[32mINF�[0m �[1m2565 commits scanned.�[0m
�[90m5:54AM�[0m �[32mINF�[0m �[1mscanned ~19727750 bytes (19.73 MB) in 2.63s�[0m
�[90m5:54AM�[0m �[33mWRN�[0m �[1mleaks found: 4�[0m
##[error]Process completed with exit code 1.
GitHub Actions: Secrets / 0_Gitleaks.txt: feat: 009 explore tooling enhancement
Conclusion: failure
##[group]Run gitleaks detect --source . --redact --no-banner --config .gitleaks.toml
�[36;1mgitleaks detect --source . --redact --no-banner --config .gitleaks.toml�[0m
shell: /usr/bin/bash -e {0}
env:
MISE_LOG_LEVEL: info
MISE_GITHUB_***REDACTED_SECRET_ASSIGNMENT***
MISE_TRUSTED_CONFIG_PATHS: /home/runner/work/n00n/n00n
MISE_YES: 1
##[endgroup]
�[90m5:54AM�[0m �[32mINF�[0m �[1m2565 commits scanned.�[0m
�[90m5:54AM�[0m �[32mINF�[0m �[1mscanned ~19727750 bytes (19.73 MB) in 2.63s�[0m
�[90m5:54AM�[0m �[33mWRN�[0m �[1mleaks found: 4�[0m
##[error]Process completed with exit code 1.
🧰 Additional context used
📓 Path-based instructions (4)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Rust workspace code must deny unsafe code; new unsafe requires written review, an explicit crate-level lint exception, and a SAFETY comment.
Production Rust code must not use unwrap, expect, panic, todo!, unimplemented!, dbg!, silent defaults, or.ok()to discardResulterrors.
Use explicit typed error handling withResult<T, E>,?,ok_or_else, andmap_err; do not silently discard missing data, parse failures, dependency errors, closed channels, or invalid state.
Do not use wildcard imports; import types at the top of the file and use short imported names.
Keep constants immediately after imports, use descriptive names, avoid unnecessary state, and avoid inline magic numbers or strings.
Usethiserrorfor domain-specific errors in library crates andcolor-eyreat binary edges.
Use#[derive(Copy)]only for structs containing one primitive field.
Prefer structured logging with useful fields and provide helpful, sanitized error messages.
Treat LLM and provider output as untrusted input; validate it against schemas, domain constraints, and source evidence before persistence or action.
Do not commit or log credentials, API keys, tokens, cookies, auth headers, raw provider payloads, prompts, user session data, or private user data.
Validate and authorize HTTP, file, queue, configuration/environment, LLM-output, and provider-callback inputs before mutation or persistence.
Apply prompt-injection defenses when documents, user text, or provider messages are included in prompts or tool calls.
Tool execution requires allowlisted tools, scoped credentials, explicit user context, audit events, and refusal or denial tests.
Use TDD with a failing test, implementation, and refactor; follow Rust idioms and keep code minimal, focused, and free of trivial comments or bloat.
Files:
n00n-lua/src/api/codegraph.rsn00n-arbor/src/graph_json.rsn00n-lua/src/api/semblem.rsn00n-arbor/src/graph_query.rsn00n-lua/src/api/arbor.rsn00n-codegraph/src/lib.rsn00n-codegraph/src/db.rsn00n-arbor/src/lib.rsn00n-semble/src/lib.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (AGENTS.md)
All workspace crates must opt into the root
[workspace.lints]configuration with[lints] workspace = true, and code must pass the configured clippy lints.
Files:
n00n-lua/src/api/codegraph.rsn00n-arbor/src/graph_json.rsn00n-lua/src/api/semblem.rsn00n-arbor/src/graph_query.rsn00n-lua/src/api/arbor.rsn00n-codegraph/src/lib.rsn00n-codegraph/src/db.rsn00n-arbor/src/lib.rsn00n-semble/src/lib.rs
**/*.{rs,rs.in}
📄 CodeRabbit inference engine (AGENTS.md)
Place unit tests in the same Rust file inside
#[cfg(test)]modules; use#[test_case], snake_case test names, shared constants for error/status assertions, and avoid tautological or flaky tests.
Files:
n00n-lua/src/api/codegraph.rsn00n-arbor/src/graph_json.rsn00n-lua/src/api/semblem.rsn00n-arbor/src/graph_query.rsn00n-lua/src/api/arbor.rsn00n-codegraph/src/lib.rsn00n-codegraph/src/db.rsn00n-arbor/src/lib.rsn00n-semble/src/lib.rs
**/src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
In the n00n workspace, preserve the documented crate architecture: UI in
n00n-ui, provider integrations inn00n-providers, agent loop inn00n-agent, interpreter inn00n-interpreter, storage inn00n-storage, configuration inn00n-config, Lua plugins inn00n-lua, and ACP server inn00n-acp.
Files:
n00n-lua/src/api/codegraph.rsn00n-arbor/src/graph_json.rsn00n-lua/src/api/semblem.rsn00n-arbor/src/graph_query.rsn00n-lua/src/api/arbor.rsn00n-codegraph/src/lib.rsn00n-codegraph/src/db.rsn00n-arbor/src/lib.rsn00n-semble/src/lib.rs
🪛 Luacheck (1.2.0)
plugins/explore/router.lua
[warning] 85-85: shadowing definition of variable 'path' on line 36
(W421)
🔇 Additional comments (28)
specs/009-explore-tooling-enhancement/contracts/index.md (1)
1-43: LGTM!specs/009-explore-tooling-enhancement/contracts/rtk.md (1)
1-78: LGTM!n00n-agent/src/prompts/research.md (1)
4-11: LGTM!Also applies to: 20-35
plugins/explore/init.lua (1)
24-26: LGTM!Also applies to: 57-62, 85-85, 97-101
plugins/explore/router.lua (2)
50-59: LGTM!Also applies to: 70-74, 76-82, 86-94, 110-110, 134-134, 150-162, 194-232, 274-275
256-271: 🗄️ Data Integrity & IntegrationNo change needed for missing CodeGraph symbols.
codegraphimpactreturns an explicit symbol-required error wheninput.symbolis missing, andnodefalls back toinput.symbolbefore rejecting missingnode_id/name/symbol.plugins/explore/tests/spec.lua (1)
97-97: LGTM!Also applies to: 127-128, 150-178, 180-197, 199-222, 224-242
plugins/bash/tests/spec.lua (2)
42-43: Make the test fixture path independent of the working directory.If the spec runner starts outside the repository root,
loadfile("plugins/bash/init.lua")fails before the assertions. Resolve the path from the test file or use a harness-provided repository root.
45-57: Add a behavior test forrtk_rewrite.These assertions call only
strip_leading_assignments. They do not execute the newfirst_wordguard, so they cannot catch the assignment-only case or verifyjqandyqpassthrough. Add cases through the public rewrite path.plugins/bash/init.lua (1)
438-440: LGTM!n00n-codegraph/src/db.rs (3)
21-142: LGTM!
319-440: LGTM!
671-792: LGTM!n00n-codegraph/src/lib.rs (1)
74-95: LGTM!n00n-lua/src/api/codegraph.rs (1)
41-133: 🩺 Stability & AvailabilityNo Lua binding issue for
affected.The file-array conversion is present and owned
Stringvalues are kept alive for the borrowed&Vec<&str>argument. The Lua binding also propagatesClient::affectederrors instead of discarding them.n00n-lua/src/api/semblem.rs (2)
46-51: LGTM!
135-146: LGTM!n00n-semble/src/lib.rs (3)
12-12: LGTM!Also applies to: 78-84, 87-128, 131-158, 160-179
556-611: LGTM!
182-184: 🗄️ Data Integrity & IntegrationNo dependency change needed.
plugins/semblem/init.lua (1)
84-107: LGTM!n00n-arbor/src/graph_json.rs (1)
226-237: LGTM!Also applies to: 367-370
n00n-arbor/src/graph_query.rs (2)
7-10: LGTM!Also applies to: 134-136
210-218: LGTM!n00n-arbor/src/lib.rs (1)
5-5: LGTM!Also applies to: 16-19, 131-319
n00n-lua/src/api/arbor.rs (1)
43-114: LGTM!Also applies to: 116-194, 304-316, 417-564
plugins/arbor/init.lua (2)
73-76: LGTM!
186-239: LGTM!Also applies to: 282-341, 364-398
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_59c104b7-d80e-44b3-92c9-f3bfc62326dd) |
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_1d3419b2-3817-416a-b03a-070aaca8d6e4) |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
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_6b660a84-a7a9-4c9e-818f-4c53da9d1f77) |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
n00n-lua/tests/real_plugins_restore.rs (1)
904-916: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert that forbidden arguments fail parsing.
This test only checks that
model,model_tier, andauto_tierare absent from the schema. It does not prove thattool.parserejects inputs containing those fields. Add one parse-failure assertion for each key.Proposed test
assert!(!properties.contains_key("auto_tier")); + + let base = json!({ + "description": "brief", + "goal": "g", + "definition_of_done": "d", + }); + for key in ["model", "model_tier", "auto_tier"] { + let mut input = base.clone(); + input[key] = json!("forbidden"); + assert!(tool.parse(&input).is_err(), "{key} must be rejected"); + }🤖 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 `@n00n-lua/tests/real_plugins_restore.rs` around lines 904 - 916, Extend fusion_rejects_model_selection_arguments to call tool.parse with inputs containing model, model_tier, and auto_tier, asserting each call fails. Keep the existing schema-property assertions and use the tool obtained from fusion_delegate.
🤖 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 `@n00n-agent/src/prompts/research.md`:
- Line 1: Add a top-level Markdown heading at the beginning of the research
prompt template, or add a narrowly scoped MD041 markdownlint exemption for this
prompt file. Preserve the existing prompt text and behavior unchanged.
In `@site/docs/content/lua-api/_index.md`:
- Around line 5579-5583: Update the search API description to state the
observable behavior of hybrid and semantic modes when no embedder is configured,
using the actual error, warning, or BM25 fallback behavior. Revise the repo
parameter description to say “an HTTPS Git URL” and keep the wording warm,
simple, and concise.
- Line 5583: Update resolve_repo_path() to validate repository URLs before
invoking git clone: permit only safe HTTPS authorities, reject embedded
credentials, private or local targets, unsafe redirects, and all non-HTTPS
schemes, while preserving local project-root paths. Ensure every remote URL
passes these checks before cloning.
---
Outside diff comments:
In `@n00n-lua/tests/real_plugins_restore.rs`:
- Around line 904-916: Extend fusion_rejects_model_selection_arguments to call
tool.parse with inputs containing model, model_tier, and auto_tier, asserting
each call fails. Keep the existing schema-property assertions and use the tool
obtained from fusion_delegate.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a4f5a898-2b64-4697-8f7d-8b7d03b16385
📒 Files selected for processing (9)
.gitleaks.tomlchangelog.d/221.added.mdn00n-agent/src/prompts/research.mdn00n-docgen/src/gen_tools.rsn00n-lua/tests/plugin_host.rsn00n-lua/tests/real_plugins_restore.rsn00n-token-profile/baselines/cold_start.jsonsite/docs/content/lua-api/_index.mdsite/docs/content/tools/_index.md
📜 Review details
⏰ Context from checks skipped due to timeout. (15)
- GitHub Check: Coverage
- GitHub Check: Test (macOS)
- GitHub Check: Lint (Windows)
- GitHub Check: Build
- GitHub Check: Build (Windows)
- GitHub Check: Test (Windows)
- GitHub Check: Lint (macOS)
- GitHub Check: MSRV (1.97)
- GitHub Check: Test
- GitHub Check: Lint
- GitHub Check: Docs
- GitHub Check: Criterion
- GitHub Check: Analyze (rust)
- GitHub Check: cargo-deny
- GitHub Check: cargo-audit
🧰 Additional context used
📓 Path-based instructions (3)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Deny unsafe code workspace-wide. New unsafe requires a written review, an explicit crate-level lint exception, and a SAFETY comment.
Do not use unwrap, expect, panic, todo!, unimplemented!, dbg!, silent-default patterns such as unwrap_or or Result::ok(), or unchecked transmute-like behavior in production code. Use typed errors or explicit sanitized rejection instead.
Propagate typed errors with ?, ok_or_else, and map_err. Use thiserror for library/domain errors and color-eyre at binary edges.
Do not use wildcard imports. Import types at the top of the file and use short names rather than fully qualified inline paths.
Keep constants immediately after imports, use descriptive names, avoid unnecessary state and magic values, and follow Rust idioms.
Use structured logging with useful fields and provide helpful, sanitized error messages.
Treat LLM and provider output as untrusted input. Validate it against schemas, domain constraints, and source evidence before persistence or action.
Do not commit or log credentials, API keys, tokens, cookies, authorization headers, prompts, raw provider payloads, or user session data.
Validate and authorize HTTP, file, queue, configuration/environment, LLM output, and provider callbacks before mutation or persistence.
Tool execution requires allowlisted tools, scoped credentials, explicit user context, audit events, and refusal or denial tests.
Files:
n00n-lua/tests/plugin_host.rsn00n-docgen/src/gen_tools.rsn00n-lua/tests/real_plugins_restore.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,toml}: All workspace crates must opt into the root workspace lint configuration with [lints] workspace = true, and changes must pass cargo fmt, cargo check, and clippy with warnings denied.
Use the repository's Rust workspace architecture and crate responsibilities: UI in n00n-ui, providers in n00n-providers, agent loop in n00n-agent, storage in n00n-storage, configuration in n00n-config, Lua plugins in n00n-lua, and ACP server in n00n-acp.
Files:
n00n-lua/tests/plugin_host.rsn00n-docgen/src/gen_tools.rsn00n-lua/tests/real_plugins_restore.rs
site/**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
User documentation should be warm, simple, concise, easy for non-native English speakers, story-oriented, and contain no em-dashes, emojis, or AI-like tone.
Files:
site/docs/content/tools/_index.mdsite/docs/content/lua-api/_index.md
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: w0wl0lxd/n00n
Timestamp: 2026-08-02T06:59:19.684Z
Learning: Use TDD: write a failing test, implement the change, then refactor. Avoid tautological or flaky tests and do not use arbitrary sleeps.
Learnt from: CR
Repo: w0wl0lxd/n00n
Timestamp: 2026-08-02T06:59:19.684Z
Learning: Non-trivial or multi-file changes should use a dedicated git worktree and new branch; do not alter unrelated user changes; finish with a clear Conventional Commit, pushed branch, and draft PR without force-pushing or pushing to main.
Learnt from: CR
Repo: w0wl0lxd/n00n
Timestamp: 2026-08-02T06:59:19.684Z
Learning: Use structural, indexed exploration tools before broad searches; parallelize independent calls, filter large outputs, and research unfamiliar external behavior before fixing it.
📚 Learning: 2026-07-31T05:40:20.137Z
Learnt from: w0wl0lxd
Repo: w0wl0lxd/n00n PR: 203
File: changelog.d/203.fixed.md:1-2
Timestamp: 2026-07-31T05:40:20.137Z
Learning: Files in changelog.d/ whose names begin with a numeric fragment identifier are headingless changelog fragments. Treat their contents as entry bodies because generated release sections provide the headings; do not report Markdown MD041 or add an H1 heading to these fragments. This does not apply to changelog.d/README.md.
Applied to files:
changelog.d/221.added.md
📚 Learning: 2026-07-31T19:15:04.814Z
Learnt from: w0wl0lxd
Repo: w0wl0lxd/n00n PR: 206
File: changelog.d/orchestration-hardening.fixed.md:1-1
Timestamp: 2026-07-31T19:15:04.814Z
Learning: Files in changelog.d are changelog fragments intended for user-facing release notes and may begin directly with summary prose. Do not flag a missing Markdown H1 or require an H1 solely because Markdownlint MD041 reports it in these fragment files.
Applied to files:
changelog.d/221.added.md
🪛 LanguageTool
site/docs/content/tools/_index.md
[grammar] ~108-~108: Ensure spelling is correct
Context: ... mode | string | no | Search mode for semblem (bm25, hybrid, or semantic). | | project | ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.23.1)
changelog.d/221.added.md
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
n00n-agent/src/prompts/research.md
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🔇 Additional comments (17)
n00n-agent/src/prompts/research.md (1)
29-35: LGTM!n00n-lua/tests/real_plugins_restore.rs (7)
21-34: LGTM!
81-91: LGTM!
172-272: LGTM!
506-506: LGTM!Also applies to: 529-529
596-611: LGTM!
847-902: LGTM!
918-952: LGTM!changelog.d/221.added.md (1)
1-1: LGTM!n00n-token-profile/baselines/cold_start.json (1)
7-25: LGTM!n00n-docgen/src/gen_tools.rs (1)
425-425: LGTM!n00n-lua/tests/plugin_host.rs (1)
27-27: LGTM!site/docs/content/lua-api/_index.md (2)
5282-5295: 🔒 Security & PrivacyVerify authorization before publishing this mutating API.
n00n.arbor.refactor()acceptsoperationandprojectand can modify source files. The supplied binding passes both values toClient::refactor. Verify that the runtime path enforces explicit user approval, project-path confinement, scoped credentials, audit events, and denial tests. The warning in this documentation is not an enforcement control.As per coding guidelines, tool execution requires allowlisted tools, scoped credentials, explicit user context, audit events, and refusal or denial tests.
Source: Coding guidelines
817-818: LGTM!Also applies to: 5061-5061, 5077-5077, 5179-5280, 5297-5328, 5408-5549, 5610-5625, 6013-6015
site/docs/content/tools/_index.md (2)
90-95: LGTM!Also applies to: 108-108, 179-183
159-169: 🗄️ Data Integrity & IntegrationNo documented tool field mismatch to address.
The
codegraphadapter documentsnode_id,projectPath, andname; thearbordocs documentprojectandpath..gitleaks.toml (1)
15-15: 🔒 Security & PrivacyNo change needed.
- Document codegraph node native path accurately. - Harden semble remote URL handling and extract native find_related helper. - Validate arbor refactor operation characters. - Allow explore queryless command mode and fix router path shadowing. - Align semblem savings terminology with the Lua API. - Clarify command-specific input requirements in Arbor, CodeGraph, and Explore contracts; use projectPath consistently in quickstart. - Update constitution error-handling guidance.
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_b568f981-4edc-422d-9586-07e7e33db15a) |
- Add markdownlint MD041 exemption to research prompt and changelog fragment. - Clarify arbor refactor requires allowlisted operation and user confirmation. - Document semblem repo parameter as HTTPS-only and embedder fallback. - Regenerate lua-api docs.
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_611a610b-4b33-4642-8209-ed7f9b2c91e2) |
`webpki-roots` 1.0.9 (transitive via reqwest/hyper-rustls) uses the Community Data License Agreement Permissive 2.0 license, which is a permissive data license comparable to CC-BY. Add it to the cargo-deny allowlist so the dependency graph continues to pass license checks.
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_cfbc0bf0-94f4-49d3-9968-6a29930a61b6) |
Addressed: research.md MD041 exemption added, lua-api docs regenerated with clarified semblem embedder/repo behavior and arbor refactor warnings, and the fusion test assertion was validated against the schema validation pipeline. CodeRabbit is rate-limited and cannot re-review.
1 participant
This PR adds a complete Spec Kit for the explore tooling enhancement feature.
Summary
Expands and refines CodeGraph, Semble, and Arbor integrations to make them first-tier exploration tools alongside index. Key changes include:
Spec Kit Contents
User Decisions
Next Steps
Review the spec kit for completeness and approve to begin implementation.
Note
Medium Risk
Large surface area across agent routing, external CLIs, and SQLite/graph query paths;
refactorand remote git clone paths need careful review, but changes are mostly read-only exploration with tests and validation guards.Overview
Explore becomes the primary entry point: the router gains intents (
search,skeleton,symbol,impact,trace), routes to semblem and codegraph subcommands, and propagates backend validation errors. Agent prompts,AGENTS.md, and the speckit constitution now steer agents toward structural tools before grep/bash.Codegraph is command-driven (
explore,callers,callees,impact,affected,node,query,sync,files) with new native SQLite paths (call graph, impact, affected files, escaped LIKE) and shared CLI timeout handling when.codegraph/codegraph.dbis missing.Arbor adds native
graph_map/graph_entry_pointsfromgraph.json, more CLI wrappers (entry-points,file-graph,inspect,path,refactor,check,summary) with--argv fixes, and plugin fallbacks so map / entry_points work without the CLI when the graph index exists.Semblem uses a hybrid path: optional semble CLI for hybrid/semantic/content filters and savings, with BM25 native fallback, content-type filtering, and allowlisted HTTPS remote repos via
N00N_SEMBLE_ALLOWED_REMOTE_REPOS.Bash documents broader rtk rewrite coverage (podman, docker, npm, pip, python) and keeps jq/yq passthrough. Tool-definition byte budgets and cold-start token baselines are bumped; spec 009 contracts/checklists and changelog 221 document the feature.
Reviewed by Cursor Bugbot for commit 6a65b33. Bugbot is set up for automated code reviews on this repo. Configure here.