Repository navigation
fix(arbor): ensure graph freshness and handle empty native results - #197
Conversation
- Expose n00n.arbor.ensure_fresh_index to Lua for freshness checks - Call ensure_fresh_index before native callers/callees/trace_path queries - Call ensure_fresh_index before map/diff/query CLI commands - Return nil from native_relations when results table is empty to trigger CLI fallback - Skipped router path vs command issue (test confirms explicit command takes priority) - Skipped trace_path error masking (error already properly propagated)
📝 WalkthroughWalkthroughThe Arbor Lua API now exposes ChangesArbor index refresh and fallback
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant ArborTool as Arbor CLI tool
participant Plugin as plugins/arbor/init.lua
participant API as n00n_arbor
participant Native as Native Arbor query
ArborTool->>Plugin: invoke an Arbor graph command
Plugin->>API: ensure_fresh_index(project)
API-->>Plugin: success or error
Plugin->>Native: run native graph operation
Native-->>Plugin: results or empty result
Plugin-->>ArborTool: native data or CLI fallback
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
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_d261783a-7ee9-4e4e-99ea-128b792afe2c) |
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_f0bca37f-5943-4662-8cfc-530b45724c61) |
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_87630fb5-dc43-4341-867a-13be81560696) |
There was a problem hiding this comment.
Criterion
Details
| Benchmark suite | Current: 0371c7b | Previous: 6fab71e | Ratio |
|---|---|---|---|
fib/jit_mlua_hook |
6670434 ns/iter (± 166098) |
6691759 ns/iter (± 109001) |
1.00 |
fib/jit_watchdog |
2222665 ns/iter (± 3488) |
2220619 ns/iter (± 4777) |
1.00 |
fib/jit_none |
2217059 ns/iter (± 8315) |
2218291 ns/iter (± 10013) |
1.00 |
fib/interp_mlua_hook |
7938706 ns/iter (± 117681) |
8200913 ns/iter (± 119203) |
0.97 |
fib/interp_watchdog |
4326655 ns/iter (± 8733) |
4475695 ns/iter (± 130509) |
0.97 |
fib/interp_none |
4299511 ns/iter (± 9420) |
4422552 ns/iter (± 26228) |
0.97 |
buffer_rw/jit_mlua_hook |
582726 ns/iter (± 1253) |
584564 ns/iter (± 829) |
1.00 |
buffer_rw/jit_watchdog |
191583 ns/iter (± 304) |
191446 ns/iter (± 434) |
1.00 |
buffer_rw/jit_none |
191563 ns/iter (± 730) |
191630 ns/iter (± 2903) |
1.00 |
buffer_rw/interp_mlua_hook |
1046042 ns/iter (± 11415) |
1040685 ns/iter (± 9589) |
1.01 |
buffer_rw/interp_watchdog |
588557 ns/iter (± 2252) |
589416 ns/iter (± 1474) |
1.00 |
buffer_rw/interp_none |
588091 ns/iter (± 4038) |
588142 ns/iter (± 2699) |
1.00 |
splash_render_120x40 |
79561 ns/iter (± 519) |
70218 ns/iter (± 3753) |
1.13 |
splash_render_200x60 |
110700 ns/iter (± 4517) |
165600 ns/iter (± 21357) |
0.67 |
This comment was automatically generated by workflow using github-action-benchmark.
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_ed7d24cc-435e-41bb-9845-da0578efcafd) |
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_8bc1b210-9491-43a0-84e7-0bbbdb0748d4) |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@plugins/arbor/init.lua`:
- Line 63: Handle the boolean returned by pcall around each ensure_fresh_index
call in the callers, callees, trace_path, and related operation flows. When
refresh fails, do not perform native index lookups; use the existing CLI
fallback where available, otherwise return or propagate the refresh error.
Preserve normal native lookup behavior when ensure_fresh_index succeeds.
🪄 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: c9c106ca-6a6e-47ee-8533-c2249e2e19d5
📒 Files selected for processing (4)
changelog.d/197.fixed.mdn00n-lua/src/api/arbor.rsplugins/arbor/init.luasite/docs/content/lua-api/_index.md
📜 Review details
⏰ Context from checks skipped due to timeout. (16)
- GitHub Check: Build (Windows)
- GitHub Check: Test (Windows)
- GitHub Check: Build
- GitHub Check: Test (macOS)
- GitHub Check: MSRV (1.97)
- GitHub Check: Rustdoc
- GitHub Check: Unused deps
- GitHub Check: Lint (Windows)
- GitHub Check: Coverage
- GitHub Check: Lint (macOS)
- GitHub Check: Docs
- GitHub Check: Lint
- GitHub Check: Test
- GitHub Check: Analyze (rust)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Criterion
🧰 Additional context used
📓 Path-based instructions (3)
site/docs/**
📄 CodeRabbit inference engine (AGENTS.md)
User documentation should be warm, simple, concise, easy for non-native English readers, story-driven, and contain no em dashes, emojis, or AI-toned language.
Files:
site/docs/content/lua-api/_index.md
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Workspace Rust code must deny unsafe code; new unsafe requires written review, an explicit crate-level lint exception, and a SAFETY comment for each allowed call.
Production Rust code must not use unwrap, expect, panic, todo!, unimplemented!, dbg!, or silent-default patterns such as unwrap_or, unwrap_or_default, or Result::ok(); use typed errors or explicit sanitized rejection instead.
Do not use wildcard imports; import types at the top of the file and use short names rather than inline fully qualified paths.
Keep constants immediately after imports, use descriptive names, avoid unnecessary state and magic values, and follow Rust idioms.
Use explicit error handling with Result<T, E>; propagate errors with ?, ok_or_else, and map_err; use thiserror for library/domain errors and color-eyre at binary edges.
Use structured logging with useful fields and provide helpful, sanitized error messages.
Only derive Copy for structs containing one primitive field.
Place unit tests in the same Rust file inside #[cfg(test)] modules; use #[test_case] and snake_case test names.
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, prompts, raw provider payloads, or user session 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; tests must be meaningful, non-flaky, and must not rely on arbitrary sleeps.
Define shared constant error/status messages in tests and assert against those constants rather than duplica...
Files:
n00n-lua/src/api/arbor.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,toml}: The workspace lint configuration in the root Cargo.toml and clippy.toml is authoritative, and every crate must opt in with [lints] workspace = true.
Pedantic Clippy warnings must pass as errors under cargo clippy --all --tests -- -D warnings.
Do not introduce global mutable state, static mut, FFI, unchecked transmute-like behavior, or new unsafe blocks without written review and the required lint exception.
Library crates use thiserror for errors, while binaries use color-eyre at binary boundaries.
Missing data, parse failures, dependency errors, closed channels, and invalid state must not be silently defaulted away; return an error, reject the operation, or use an explicitly named fallback with sanitized structured logging.
Run cargo fmt --all, cargo check --all, cargo clippy --all --tests -- -D warnings, and workspace tests; use cargo test --workspace when cargo-nextest is unavailable.
Files:
n00n-lua/src/api/arbor.rs
🪛 Luacheck (1.2.0)
plugins/arbor/init.lua
[warning] 63-63: unused variable 'ok'
(W211)
[warning] 86-86: unused variable 'ok'
(W211)
[warning] 106-106: unused variable 'ok'
(W211)
[warning] 120-120: unused variable 'ok'
(W211)
[warning] 137-137: unused variable 'ok'
(W211)
🪛 markdownlint-cli2 (0.23.1)
changelog.d/197.fixed.md
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🔇 Additional comments (4)
n00n-lua/src/api/arbor.rs (1)
3-4: LGTM!Also applies to: 83-88, 250-261
site/docs/content/lua-api/_index.md (1)
5089-5104: LGTM!plugins/arbor/init.lua (1)
34-36: LGTM!changelog.d/197.fixed.md (1)
1-1: LGTM!
Skip native callers/callees when refresh fails (CLI fallback), and surface refresh errors for trace_path/map/diff/query instead of querying a potentially stale index.
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_18df9f01-3372-4662-b793-cdbd195efc57) |
Follow-up to #170. Supersedes #196.
ensure_fresh_indexto Lua and calls it before arbor callers/callees/trace_path/map/diff/query.nilfromnative_relationsfor empty result tables so the CLI fallback is not skipped.Note
Low Risk
Behavioral fixes in the arbor plugin and Lua bindings only; may trigger extra reindexing or CLI subprocesses but does not touch auth, security, or core data paths.
Overview
Fixes Arbor graph queries returning stale or misleading results when the native
.arbor/graph.jsonindex is out of date, or when the in-memory lookup returns an empty table.n00n.arbor.ensure_fresh_indexis wired through the Rust Lua API and documented. The arbor plugin calls it before graph-backed work: callers/callees refresh best-effort (failed refresh skips native lookup and falls back to the CLI); trace_path, map, diff, and query treat refresh failure as a hard error.native_relationsnow returnsnilwhen the native result table is empty, so the plugin does not treat “no rows” as a successful native hit and incorrectly skip the Arbor CLI fallback.Reviewed by Cursor Bugbot for commit 0371c7b. Bugbot is set up for automated code reviews on this repo. Configure here.