feat(mcp): detail limit, param auto-resolve, and known behavior docs#327
feat(mcp): detail limit, param auto-resolve, and known behavior docs#327
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds a 25-item default limit for detail-mode pagination, applies it across MCP walk handlers, enables parameter auto-resolution for in/name filters, labels component-level responses as "(component)", updates docs and lint globs, and adds tests covering these behaviors. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #327 +/- ##
==========================================
+ Coverage 84.57% 84.60% +0.02%
==========================================
Files 184 184
Lines 26820 26845 +25
==========================================
+ Hits 22684 22711 +27
+ Misses 2814 2813 -1
+ Partials 1322 1321 -1
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@internal/mcpserver/tools_walk_operations.go`:
- Around line 55-57: Move the constant defaultDetailLimit from
tools_walk_operations.go into server.go and place it next to the detailLimit()
helper so the consumer and constant are co-located; update any references to
defaultDetailLimit to use the relocated symbol (no renaming needed), and ensure
the constant remains in the same package and visible to detailLimit() so
compilation and behavior are unchanged.
In `@internal/mcpserver/tools_walk_parameters_test.go`:
- Around line 357-436: Add a negative test (e.g.,
TestWalkParameters_BrokenRefAutoResolves) that uses a spec where a parameter is
a dangling $ref such as $ref: "#/components/parameters/DoesNotExist", call
callWalkParameters with a filter that forces auto-resolution (use
walkParametersInput + specInput and Path/In/Name as appropriate), and assert
that the call returns an error (use require.Error) and that the error message
mentions the unresolved ref (or at least that no parameters were matched).
Reference callWalkParameters, walkParametersInput, specInput, and the
output.Matched/output.Summaries to locate where to add the assertions.
- Around line 360-386: Extract the duplicated inline OpenAPI YAML used by
TestWalkParameters_InFilterAutoResolves and
TestWalkParameters_GroupByLocation_RefLabel into a package-level constant (e.g.,
walkParametersRefTestSpec) and replace the inline spec strings in both tests
with that const; ensure the const contains the superset spec (includes
TokenHeader) so both tests can reuse it and update any test helper calls that
reference the former inline variable names to use walkParametersRefTestSpec
instead.
|
@claude please respond and resolve the review comments made by coderabbitai[bot] above. I agree with the first nitpick that calls out:
but I leave the other 2 for you to consider. I'm fine if you reason those away with words. |
|
Claude finished @erraggy's task in 1m 58s —— View job Resolving CodeRabbit Review Comments
1. ✅
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/mcpserver/tools_walk_operations.go (1)
25-25:⚠️ Potential issue | 🟡 MinorUpdate jsonschema descriptions for all walk input structs to clarify detail-mode limit override.
All seven walk input structs have stale
Limitdescriptions saying"default 100"without mentioning the detail-mode override. Whendetail=true, the effective default becomes 25 (viadetailLimit()in server.go). This discrepancy is misleading to LLM clients consuming the schema.Update the descriptions in:
- internal/mcpserver/tools_walk_operations.go:25
- internal/mcpserver/tools_walk_paths.go:19
- internal/mcpserver/tools_walk_security.go:19
- internal/mcpserver/tools_walk_refs.go:19
- internal/mcpserver/tools_walk_parameters.go:23
- internal/mcpserver/tools_walk_responses.go:21
- internal/mcpserver/tools_walk_headers.go:23
Example fix for tools_walk_operations.go:
Proposed change
- Limit int `json:"limit,omitempty" jsonschema:"Maximum number of results to return (default 100)"` + Limit int `json:"limit,omitempty" jsonschema:"Maximum number of results to return (default 100; 25 in detail mode)"`
…own behaviors - Lower default limit for detail mode to 25 (vs 100 for summaries) across all 7 walk tools via shared detailLimit() helper - Auto-resolve $ref parameters when `in` or `name` filter is specified in walk_parameters, so filters work without manual resolve_refs=true - Add (component) label for walk_responses group_by=method empty keys - Document filter behaviors, cross-tool limitations, and detail defaults in mcp-server.md - Add best practice notes to plugin/CLAUDE.md (#9 auto-resolve, #10 detail limit) - Update mcp-ux-findings.md status table with addressed/partial items - Add corpus-analysis.md with real-world spec analysis results Note: parameter auto-resolution is limited by a parser issue where resolve_refs discards all resolution on specs with circular $refs (#326). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
erraggy
force-pushed
the
feat/detail-limit
branch
from
b6d0da6 to
714eb31
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/mcp-server.md (1)
347-347:⚠️ Potential issue | 🟡 MinorMinor: Walk tools table
limitdescription doesn't mention detail-mode default.The shared walk tools table on this line describes
limitas "Max results to return (default: 100)" without noting the 25-item detail-mode default. While the new "Detail Mode Defaults" section below covers this, the table is the first thing a reader encounters for walk tool parameters.Consider updating for consistency with the per-tool jsonschema descriptions:
📝 Suggested update
-| `limit` | number | Max results to return (default: 100) | +| `limit` | number | Max results to return (default: 100; 25 in detail mode) |
…erver.md Good catch, @coderabbitai[bot]
1 participant
Summary
Follow-up to #324 — addresses remaining UX findings from corpus stress-testing the MCP plugin against 10 real-world OpenAPI specs.
detail=truemode now default to 25 results instead of 100 via shareddetailLimit()helper, keeping output manageable for LLM context windowswalk_parametersautomatically resolves$refparameters wheninornamefilter is specified, so users don't need to know aboutresolve_refs=truewalk_responses group_by=methodnow labels component responses as(component)instead of""mcp-server.mdandplugin/CLAUDE.mddocs/corpus-analysis.mdwith real-world spec analysis resultsLimitation
Parameter auto-resolution doesn't take effect on specs with circular
$refs due to a parser limitation whereresolve_refsdiscards all resolution when circularity is detected. Tracked in #326.Test plan
detailLimit()unit tests (zero, negative, explicit, boundary, max int)walk_operationsdetail default limit integration testwalk_parametersinandnamefilter auto-resolution testswalk_responsesgroup_by=method(component)label testmake checkpasses (8176 tests, 0 lint issues)🤖 Generated with Claude Code