Skip to content

feat: support directory paths in get_symbols_overview with max_files safeguard (#1412)#1560

Open
weiconghe wants to merge 1 commit into
oraios:mainfrom
weiconghe:fix/directory-support-and-error-handling
Open

feat: support directory paths in get_symbols_overview with max_files safeguard (#1412)#1560
weiconghe wants to merge 1 commit into
oraios:mainfrom
weiconghe:fix/directory-support-and-error-handling

Conversation

@weiconghe

@weiconghe weiconghe commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Issue

Closes #1412

Type of change

  • New feature

What does this PR do?

GetSymbolsOverviewTool now supports directory paths, guarded by max_files (Issue #1412)

Problem: get_symbols_overview raised ValueError: Expected a file path, but got a directory path when given a directory. Agents had to list directory contents, then call the tool per file — N round-trips for a common navigation pattern.

Root cause: The underlying LSP layer (request_overview in solidlsp/ls.py) already handled directories via request_dir_overview, but the tool layer in symbol_tools.py explicitly rejected directory paths.

Maintainer concern (from @MischaPanch on the issue): A previous variant allowed directories, but it was deliberately restricted because models tended to start with the top-level directory, producing huge outputs. The maintainer hinted that a max_files option could mitigate this.

Fix:

  1. Added _apply_directory method that delegates to the existing LSP directory support. When a directory path is given, the tool returns a per-file mapping of grouped symbols:
{
  "test_repo/services.py": {"Function": ["create_service_container", ...]},
  "test_repo/models.py": {"Class": ["User"]}
}

Single-file behavior is completely unchanged.

  1. Added a max_files parameter (default 20). When a directory contains more analyzable files than the limit, the tool raises ValueError before serializing — the error message shows the actual count, a sample of file paths, and tells the agent to narrow the path or learn the layout from memories first. This directly addresses @MischaPanch's concern: the agent can't accidentally dump the top-level directory, and gets actionable guidance when it tries.
ValueError: Directory src contains 240 analyzable files, which exceeds max_files=20.
Narrow the path to a more specific subdirectory, or learn the repository layout from
memories before asking for a broad overview. Sample files found: ['src/App.tsx', ...]

How did you verify your code works?

Environment: Windows 11, Python 3.13, uv 0.11.13

New tests (all passing):

  1. test_get_symbols_overview_directory_returns_per_file_symbols — passes a directory path, asserts the result is a per-file mapping with .py file keys
  2. test_get_symbols_overview_file_returns_same_format — regression test, single-file output format is unchanged (not wrapped in per-file mapping)
  3. test_get_symbols_overview_directory_raises_when_exceeds_max_files — passes max_files=1 against a directory with multiple files, asserts ValueError with max_files=1 in the message

Regression: All other Python-language tests in TestSerenaAgent still pass. (Some Go / C# / Rust / Kotlin / PowerShell tests fail in my local environment due to missing language-server binaries — unrelated to this PR and reproducible on main.)

Screenshots / recordings

N/A

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

@weiconghe weiconghe force-pushed the fix/directory-support-and-error-handling branch 4 times, most recently from eafacb1 to 78617d6 Compare June 11, 2026 01:17
@weiconghe

Copy link
Copy Markdown
Contributor Author

Hi, I noticed the CI checks failed on this PR, but the failures don't appear to be related to my changes:

  • Ubuntu: Failed during conftest.py import — clojure -Spath returned a non-zero exit code. This is a Clojure CLI environment setup issue in the CI runner, occurring before any test runs.
  • Windows: test_find_symbol_references[typescript_helper_refs] failed — the TypeScript LSP returned a reference in index.ts instead of the expected use_helper.ts. Out of 1386 tests, only this one failed, and it's unrelated to the directory path / FileNotFoundError fix in this PR.
  • macOS: Still pending.

Could you kindly rerun the failed jobs? Thanks!

@weiconghe weiconghe force-pushed the fix/directory-support-and-error-handling branch from 00453ca to 78617d6 Compare June 12, 2026 10:40
…safeguard (issue oraios#1412)

- Add directory support to GetSymbolsOverviewTool via _apply_directory,
  returning a per-file mapping of grouped symbols
- Add max_files parameter (default 20) that raises ValueError when a
  directory contains more analyzable files than the limit, addressing
  maintainer feedback on issue oraios#1412 about unbounded top-level overviews
- Single-file behavior is unchanged
- Tests: directory happy path, single-file regression, max_files guard
@weiconghe weiconghe force-pushed the fix/directory-support-and-error-handling branch from 78617d6 to 868ad1b Compare June 15, 2026 03:36
@weiconghe weiconghe changed the title feat: support directory paths in get_symbols_overview + improve replace_content error feat: support directory paths in get_symbols_overview with max_files safeguard (#1412) Jun 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature request: support directory paths in get_symbols_overview for batch retrieval

1 participant