Skip to content

feat(graph): add limit to codebase_symbol, codebase_impact and codebase_flow - #204

Merged
giancarloerra merged 1 commit into
giancarloerra:mainfrom
shaitourchin:feat/symbol-limit
Oct 10, 2026
Merged

giancarloerra merged 1 commit into
giancarloerra:mainfrom
shaitourchin:feat/symbol-limit

Conversation

@shaitourchin

@shaitourchin shaitourchin commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Adds an optional limit to codebase_symbol, and to the ambiguity-candidate lists of codebase_impact and codebase_flow, as agreed in #203. Without limit, results and output are unchanged.

For a common name such as main, codebase_symbol resolves callers and callees for every definition, one file-payload read each, in a serial loop. The two sibling tools list every candidate, and codebase_impact reads a payload for each. The issue measures 1,082,364 characters after 91–139 s for 1,624 definitions on a real monorepo, and 231 s for 5,000 on a synthetic one. With limit, the caller gets only the first N matches, and the payload reads stop with them.

Changes

  • index.ts: limit (z.number().int().min(1), optional) in the schemas of codebase_symbol, codebase_impact and codebase_flow.
  • graph-impact.ts:
    • getSymbolContextResult(cache, name, fileHint?, limit?) is new and returns { contexts, total, fileCount }, where total and fileCount count the matches after the file filter. With limit, the loop stops once limit definitions are resolved, so no payload is read past them. A name-index entry whose payload or symbol does not resolve is skipped as before, and the next match fills its place.
    • getSymbolContext(cache, name, fileHint?) keeps its signature and its SymbolContext[] return. It calls getSymbolContextResult with no limit and returns only contexts.
    • getImpactRadius takes limit in ImpactOptions. In the ambiguous branch it reads payloads only until it has limit candidates, and it returns totalCandidates, the match count after the file filter.
  • graph-tools.ts: when an explicit limit leaves matches out, the answer says so above the list, with the shown and total counts:
    Showing 20 of 1624 definitions of "main". Pass file to narrow to one file, or pass a larger limit to see more.
    Showing 20 of 1624 candidates for "main". Pass a larger limit to see more.
    
    The first line is codebase_symbol's. It names file only when the matches span more than one file, and asks for a longer path when a suffix file hint still matches several. The second is codebase_flow's and codebase_impact's, whose headers already say how to disambiguate. When limit covers every match, no line is printed.
  • README.md: the three rows of the Impact Analysis table name limit.

Automatic entry-point discovery (codebase_flow with no entrypoint) is unchanged.

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Test coverage improvement

Testing

  • Unit tests pass (npm run test:unit)
  • Integration tests pass (npm run test:integration) — not run; no Docker Qdrant available locally. No integration or e2e test reads the changed outputs.
  • TypeScript compiles cleanly (npx tsc --noEmit)
  • New tests added for new/changed functionality

New tests, covering the four cases from #203:

  • Unchanged without limit: three definitions of main. The test compares the full output of all three tools with the exact text. It also passes against the code on main.
  • Explicit limits: all three tools list only limit matches and print the shown/total line. At limit equal to the match count, none prints it.
  • Filtered totals: a file hint that keeps two of three matches gives "Showing 1 of 2" in all three tools.
  • getSymbolContext contract: the two- and three-argument calls return the array of matching definitions, narrowed by the file hint.
  • Avoided payload reads: spies on getFilePayload show that getSymbolContextResult and getImpactRadius stop reading at the limit. A further test shows that a stale name-index entry does not use up the limit.

Each new test fails against a mutation of the code it guards, for example a default cap, a limit applied after the reads, or a total counted before the file filter.

Checklist

  • My code follows the existing code style and conventions
  • I have added/updated JSDoc comments where appropriate
  • I have updated documentation (README.md / DEVELOPER.md) if needed
  • I have addressed all CodeRabbit review comments (or marked as resolved with explanation)
  • I have read the Contributing Guide
  • I agree to the Contributor License Agreement

Related issues

Closes #203

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added an optional result limit to codebase impact, flow, and symbol searches. Without a limit, results remain uncapped.
    • When a limit omits matches, results show how many were displayed and the total number found, with guidance for refining or expanding the search.

@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: giancarloerra/SocratiCode/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 573c2eac-ec2a-40fc-9ca9-d5e39b5c1b52


📥 Commits

Reviewing files that changed from the base of the PR and between e5bf916 and 0f48a19.



📒 Files selected for processing (6)
  • README.md
  • src/index.ts
  • src/services/graph-impact.ts
  • src/tools/graph-tools.ts
  • tests/unit/graph-impact.test.ts
  • tests/unit/symbol-graph-contract.test.ts


Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.




📝 Walkthrough
📝 Walkthrough

Walkthrough

The three graph tools now accept optional limits for ambiguous candidates or definitions. Capped results report displayed and total counts. The change also updates graph-impact result data, tests, and README descriptions.

Changes

Graph Tool Result Limits

Layer / File(s) Summary
Limit inputs and result contracts
src/index.ts, src/services/graph-impact.ts
The tool schemas add optional limits. Graph-impact result types include total candidate counts and symbol-context totals and file counts.
Bound graph result resolution
src/services/graph-impact.ts, tests/unit/graph-impact.test.ts
Impact candidate loading and symbol-context resolution stop at the limit while retaining full match counts. Tests cover limited resolution, unresolved entries, and payload reads.
Tool output and validation
src/tools/graph-tools.ts, tests/unit/symbol-graph-contract.test.ts, README.md
The tools apply limits and report omitted-result counts. Symbol results include file-narrowing guidance when multiple files match. Contract tests cover limited and uncapped results and file filters. The README documents limit behavior.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant codebase_symbol
  participant getSymbolContextResult
  participant cache_getFilePayload
  Client->>codebase_symbol: name and optional limit
  codebase_symbol->>getSymbolContextResult: resolve contexts with limit
  loop Until limit is reached
    getSymbolContextResult->>cache_getFilePayload: load definition payload
    cache_getFilePayload-->>getSymbolContextResult: payload
  end
  getSymbolContextResult-->>codebase_symbol: contexts and full counts
  codebase_symbol-->>Client: displayed results and count notice
Loading

Suggested reviewers: giancarloerra



Merge Risk: ⚪ Minimal · up to 0f48a

No actionable issue is established for this change; it is mergeable after normal checks.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to 0f48a

The new option reduces returned results without granting additional access. Ambiguous queries remain ambiguous, and callers that omit the option retain unlimited behavior. No material security risk was identified in the inspected change.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The inspected changes affect graph metadata returned for the selected project. They do not add a new project selector, cross-project lookup, write operation, or privileged sink. New totals can reveal filtered index cardinality, including stale references, within that existing project scope.

Trust Boundaries and Controls

  • observed — The public schemas validate the new limit. In the changed name-resolution paths, caller-provided file hints filter existing index references rather than introducing unrestricted file access. A shortened candidate list still requires disambiguation before traversal.

Resilience and Maintainability Implications

  • inferred — Early stopping can reduce payload-read work and response size for highly ambiguous names without changing generation ownership. It is a caller-selected efficiency control, not a new mandatory denial-of-service defense.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 62.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 5 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check Passed Issue #203 coding requirements are met. src/index.ts adds an optional positive-integer limit to codebase_symbol. src/services/graph-impact.ts applies the file filter before counting, stops pay…
Out of Scope Changes check Passed The codebase_impact and codebase_flow limits are stated in the current PR scope. Their schema, candidate handling, output messages, tests, and the README updates support that scope. The reviewed s…
Title check Passed The title clearly and concisely identifies the main change: adding an optional limit to codebase_symbol, codebase_impact, and codebase_flow.
Description check Passed The description includes all required sections, explains the scope and compatibility behavior, lists key changes, identifies the change types, documents unit and TypeScript testing, explains why integ…

Full details: Docstring Coverage

Explanation

Docstring coverage is 62.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 5 files. (1 skipped: 1 unsupported.)


  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR



  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@giancarloerra giancarloerra added the review-ready Maintainer-approved for CodeRabbit review label Oct 9, 2026
@giancarloerra

Copy link
Copy Markdown
Owner

The exported getSymbolContext(cache, name, fileHint?) must retain its existing SymbolContext[] return contract. This change returns { contexts, total, fileCount } even when limit is omitted. The service and declarations are shipped in the package, so callers using the array interface would break.

Please keep the original array-returning function as a compatibility wrapper, and use a separate result helper for the new limit and count metadata. Add a regression for the unchanged three-argument call. Keep the MCP behavior and the agreed graph/index and entry-point-discovery scope unchanged.

@giancarloerra giancarloerra removed the review-ready Maintainer-approved for CodeRabbit review label Oct 9, 2026
…se_flow

codebase_symbol resolves callers and callees for every definition of a
name, one file-payload read each, and codebase_impact and codebase_flow
list every candidate of an ambiguous name. For a common name such as
`main` on a large monorepo the answers run to hundreds of thousands of
characters, and the symbol and impact paths take minutes.

Add an optional `limit` to all three tools. It applies after the file
filter: codebase_symbol resolves only the first `limit` definitions, and
codebase_impact and codebase_flow list only the first `limit` ambiguity
candidates. For codebase_symbol and codebase_impact, payload reads stop
once `limit` matches are resolved. When matches are left out, the answer
says so with the shown and total counts, for example:

  Showing 20 of 1624 definitions of "main". Pass file to narrow to one
  file, or pass a larger limit to see more.

Without `limit`, the tool output is unchanged.

Closes giancarloerra#203

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@shaitourchin

shaitourchin commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor Author

Re: your comment on getSymbolContext

Thanks. getSymbolContext(cache, name, fileHint?) keeps its signature and its SymbolContext[] return. It calls a new exported helper, getSymbolContextResult(cache, name, fileHint?, limit?), which returns { contexts, total, fileCount }, and codebase_symbol uses that helper. A new unit test checks that the two- and three-argument getSymbolContext calls return the array of matching definitions, narrowed by the file hint. The MCP output, the graph and index, and entry-point discovery are unchanged.

@giancarloerra

Copy link
Copy Markdown
Owner

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@giancarloerra giancarloerra added the review-ready Maintainer-approved for CodeRabbit review label Oct 10, 2026
@giancarloerra

Copy link
Copy Markdown
Owner

The compatibility requirement is resolved: getSymbolContext(cache, name, fileHint?) retains its array return, with regression coverage for both existing call forms. The limit remains opt-in; graph/index formats and entry-point discovery are unchanged.

@giancarloerra giancarloerra self-assigned this Oct 10, 2026
@giancarloerra
giancarloerra merged commit f99283a into giancarloerra:main Oct 10, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review-ready Maintainer-approved for CodeRabbit review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: codebase_symbol resolves and prints every definition of a name — add limit, as codebase_symbols has

2 participants