Uh oh!
There was an error while loading. Please reload this page.
cli: align Rust MCP read catalog (#519) - #531
Conversation
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe Rust MCP server expands from two to seven read-only tools. It adds validated schemas, shared dispatch and result framing, SDK-backed analytical handlers, session defaults, documentation, and unit, integration, and stdio protocol tests. ChangesMCP read tool catalog
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk:⚪ Minimal · up to The PR adds the Rust MCP read catalog with documented schema and session-scoping behavior, and no actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant MCPClient
participant burn_mcp_server
participant relayburn_sdk
participant Ledger
MCPClient->>burn_mcp_server: tools/call with validated tool arguments
burn_mcp_server->>relayburn_sdk: execute summary, hotspots, overhead, trim, or compare query
relayburn_sdk->>Ledger: read ledger data
Ledger-->>relayburn_sdk: query data
relayburn_sdk-->>burn_mcp_server: structured result or error
burn_mcp_server-->>MCPClient: MCP tool result or JSON-RPC error
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:6c3bb38e51
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
All reported issues were addressed
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
crates/relayburn-cli/src/commands/mcp_server.rs (1)
1017-1085: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding a happy-path case for
tagsandgroupByTag.The fixture test covers
session,groupBy,patterns,kind,top,includeDiff, andmodels. It does not cover a successfultagsobject orgroupByTag.optional_string_recordis only exercised by its failure case at Line 1095. A single success assertion would pin theEnrichmentmapping.🤖 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 `@crates/relayburn-cli/src/commands/mcp_server.rs` around lines 1017 - 1085, Extend new_tools_invoke_sdk_verbs_against_fixture_ledger with a successful hotspots invocation using a valid tags object and groupByTag option, then assert the response succeeds and reports the expected tag grouping. Ensure this exercises optional_string_record’s success path and verifies the Enrichment mapping without changing existing coverage.crates/relayburn-cli/tests/mcp_server.rs (1)
70-81: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDo not assert JSON object key order.
property_namesrelies onserde_json::Mapiteration order, which changes ifserde_jsonloses thepreserve_orderfeature. Since the MCP tool schema allows any order for these properties, compare the key set instead.♻️ Proposed order-independent assertion
- let property_names = |tool: &Value| {- tool["inputSchema"]["properties"]- .as_object()- .expect("schema properties")- .keys()- .cloned()- .collect::<Vec<_>>()- };- assert_eq!(- property_names(&tools[2]),- ["session", "project", "since", "tags", "groupByTag"]- );+ let property_names = |tool: &Value| {+ let mut keys = tool["inputSchema"]["properties"]+ .as_object()+ .expect("schema properties")+ .keys()+ .cloned()+ .collect::<Vec<_>>();+ keys.sort();+ keys+ };+ assert_eq!(+ property_names(&tools[2]),+ ["groupByTag", "project", "session", "since", "tags"]+ );🤖 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 `@crates/relayburn-cli/tests/mcp_server.rs` around lines 70 - 81, Update the property_names assertion in the MCP server test to compare the returned property keys as an order-independent set rather than a Vec sequence. Preserve validation of the exact expected keys while removing reliance on serde_json::Map iteration order.
🤖 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.
Nitpick comments:
In `@crates/relayburn-cli/src/commands/mcp_server.rs`:
- Around line 1017-1085: Extend
new_tools_invoke_sdk_verbs_against_fixture_ledger with a successful hotspots
invocation using a valid tags object and groupByTag option, then assert the
response succeeds and reports the expected tag grouping. Ensure this exercises
optional_string_record’s success path and verifies the Enrichment mapping
without changing existing coverage.
In `@crates/relayburn-cli/tests/mcp_server.rs`:
- Around line 70-81: Update the property_names assertion in the MCP server test
to compare the returned property keys as an order-independent set rather than a
Vec sequence. Preserve validation of the exact expected keys while removing
reliance on serde_json::Map iteration order.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f7e7c326-9b70-42dc-b9a0-6c1659e7307c
📒 Files selected for processing (5)
CHANGELOG.mdREADME.mdcrates/relayburn-cli/src/cli.rscrates/relayburn-cli/src/commands/mcp_server.rscrates/relayburn-cli/tests/mcp_server.rs
…rity # Conflicts: # CHANGELOG.md
Uh oh!
There was an error while loading. Please reload this page.
Summary
burn__summary,burn__hotspots,burn__overhead,burn__overheadTrim, andburn__compareto the Rustburn mcp-servercatalog.LedgerHandlewrapper and returns SDK JSON unchanged as MCP text and structured content.Deliberate Rust schema difference
The npm overhead tools describe
projectas a path or key, while RustOverheadOptionsandOverheadTrimOptionsacceptPathBufand scan that filesystem root. The Rust MCP schemas therefore describe a project filesystem path only. A ledger project key does not resolve as a key on this surface; it is interpreted as a relative filesystem path and will normally produce an empty scan.Verification
cargo test --workspacecargo fmt --all -- --checkcargo clippy -p relayburn-cli --all-targets -- -D warningsgit diff --checkFixes#519
🤖 Generated with Claude Code.