Skip to content

fix(tools): allow whole-file reads of empty files - #7585

Open
MU-ty wants to merge 2 commits into
google:mainfrom
MU-ty:fix/read-empty-files
Open

MU-ty wants to merge 2 commits into
google:mainfrom
MU-ty:fix/read-empty-files

Conversation

@MU-ty

@MU-ty MU-ty commented Oct 17, 2026

Copy link
Copy Markdown

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

No matching issue or PR was found when searching for ReadFileTool and empty-file reads. Related PR #7176 changes how ranged reads are loaded; this patch preserves its explicit out-of-range behavior.

2. Or, if no issue exists, describe the change:

Problem:

Reading an existing empty file using ReadFileTool.run_async(args={'path': 'empty.txt'}, tool_context=None) returns status: error with "start_line 1 exceeds file length (0 lines)." The caller requested the file contents, not a particular line, and the file was read successfully.

Solution:

Return {'status': 'ok', 'content': ''} when the file is empty and neither line-range argument was supplied. Explicit line ranges retain their existing out-of-range errors. Add a regression test using a real LocalEnvironment and three parameterized guards for explicit ranges.

Testing Plan

Validated on Windows with the source snapshot at 783848821aabc6656a79949996c31608d5043606. The branch is based on newer main ff6c851dde00cbf3469400810c3148164ff82a18; both original changed-file blob hashes are identical between these bases.

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

Results:

  • Original source with new read-file tests: 1 failed, 9 passed.
  • Patched source: pytest tests/unittests/tools/environment -q — 15 passed on each of Python 3.11, 3.12, and 3.13.
  • Pyink (target Python 3.13), isort, Ruff, repository compliance checks, and git diff --check passed for the changed files.
  • Official pre-commit could not initialize: Git could not connect to GitHub through its configured local proxy. Equivalent Python format/import/lint/compliance checks were run individually; this is not a claim that every pre-commit hook passed.
  • The full multi-version tox suite was attempted but did not leave a final result; it is not claimed as passing. The focused Python 3.14 run could not launch the environment's system interpreter and did not collect tests. This PR is a draft pending complete project validation.

Manual End-to-End (E2E) Tests:

Run adk-empty-file-repro.py with this checkout installed. It initializes a real LocalEnvironment, creates a zero-byte file, calls ReadFileTool through run_async, and verifies the returned status and content. No cloud credentials, shell commands, or model calls are needed.

Patched output:

{'status': 'ok', 'content': ''}

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas. The two-line branch needs no additional explanatory comment.
  • I have added tests that prove my fix is effective.
  • New and existing unit tests pass locally with my changes. The focused 15-test suite passes; the complete suite is not represented by that result.
  • I have manually tested my changes end-to-end through the file tool.
  • Any dependent changes have been merged and published in downstream modules. No dependent change is required.

Additional context

Only the read-file implementation and its existing test module are changed. No new public API, dependency, or documentation contract is added.

AI disclosure: this patch, tests, reproduction, and description were prepared and verified with OpenAI Codex.

@google-cla

google-cla Bot commented Oct 17, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@Somay-kousis Somay-kousis left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question on the explicit-range case. The tool declares start_line as "Defaults to 1", but on this branch {} reads an empty file fine while {'start_line': 1} errors. The check keys on whether the arguments were passed, not on the range they produce, so every argument set that reads a whole 2-line file also errors on an empty one.

I ran the same args against a 2-line file and an empty file on this branch (89cc6a2), using LocalEnvironment + ReadFileTool.run_async:

two.txt   {}                                 {'status': 'ok', 'content': '     1\ta\n     2\tb\n'}
empty.txt {}                                 {'status': 'ok', 'content': ''}
two.txt   {'start_line': 1}                  {'status': 'ok', 'content': '     1\ta\n     2\tb\n'}
empty.txt {'start_line': 1}                  {'status': 'error', 'error': '`start_line` 1 exceeds file length (0 lines).', 'total_lines': 0}
two.txt   {'start_line': 0}                  {'status': 'ok', 'content': '     1\ta\n     2\tb\n'}
empty.txt {'start_line': 0}                  {'status': 'error', 'error': '`start_line` 1 exceeds file length (0 lines).', 'total_lines': 0}
two.txt   {'end_line': 5}                    {'status': 'ok', 'content': '     1\ta\n     2\tb\n'}
empty.txt {'end_line': 5}                    {'status': 'error', 'error': '`start_line` 1 exceeds file length (0 lines).', 'total_lines': 0}
two.txt   {'start_line': 1, 'end_line': 100} {'status': 'ok', 'content': '     1\ta\n     2\tb\n'}
empty.txt {'start_line': 1, 'end_line': 100} {'status': 'error', 'error': '`start_line` 1 exceeds file length (0 lines).', 'total_lines': 0}

The end_line-only error also names a start_line the caller never sent.

I see test_read_empty_file_with_line_range_returns_error pins {'start_line': 1} and {'end_line': 1} to the error on purpose. If that's not the intent, checking after start is computed, e.g. if total == 0 and start == 1: return {'status': 'ok', 'content': ''}. That covers {}, start_line 0 or 1 and any end_line, and still errors for start_line >= 2. The parametrized test would then cover those inputs as ok and keep only {'start_line': 2} as the error case. I tried that check on this branch: all six empty-file calls above return {'status': 'ok', 'content': ''}, and {'start_line': 2} still errors.

Checked: PR tests on main's _read_file_tool.py give 1 failed, 9 passed (test_read_empty_file_without_line_range). On the branch, 10 passed.

Evaluate empty-file reads after normalizing the requested range so explicit
defaults behave like omitted arguments. Preserve errors for out-of-range
starts and negative end lines, and cover empty and nonempty files.
@MU-ty

MU-ty commented Oct 17, 2026

Copy link
Copy Markdown
Author

Question on the explicit-range case. The tool declares start_line as "Defaults to 1", but on this branch {} reads an empty file fine while {'start_line': 1} errors. The check keys on whether the arguments were passed, not on the range they produce, so every argument set that reads a whole 2-line file also errors on an empty one.

I ran the same args against a 2-line file and an empty file on this branch (89cc6a2), using LocalEnvironment + ReadFileTool.run_async:

two.txt   {}                                 {'status': 'ok', 'content': '     1\ta\n     2\tb\n'}
empty.txt {}                                 {'status': 'ok', 'content': ''}
two.txt   {'start_line': 1}                  {'status': 'ok', 'content': '     1\ta\n     2\tb\n'}
empty.txt {'start_line': 1}                  {'status': 'error', 'error': '`start_line` 1 exceeds file length (0 lines).', 'total_lines': 0}
two.txt   {'start_line': 0}                  {'status': 'ok', 'content': '     1\ta\n     2\tb\n'}
empty.txt {'start_line': 0}                  {'status': 'error', 'error': '`start_line` 1 exceeds file length (0 lines).', 'total_lines': 0}
two.txt   {'end_line': 5}                    {'status': 'ok', 'content': '     1\ta\n     2\tb\n'}
empty.txt {'end_line': 5}                    {'status': 'error', 'error': '`start_line` 1 exceeds file length (0 lines).', 'total_lines': 0}
two.txt   {'start_line': 1, 'end_line': 100} {'status': 'ok', 'content': '     1\ta\n     2\tb\n'}
empty.txt {'start_line': 1, 'end_line': 100} {'status': 'error', 'error': '`start_line` 1 exceeds file length (0 lines).', 'total_lines': 0}

The end_line-only error also names a start_line the caller never sent.

I see test_read_empty_file_with_line_range_returns_error pins {'start_line': 1} and {'end_line': 1} to the error on purpose. If that's not the intent, checking after start is computed, e.g. if total == 0 and start == 1: return {'status': 'ok', 'content': ''}. That covers {}, start_line 0 or 1 and any end_line, and still errors for start_line >= 2. The parametrized test would then cover those inputs as ok and keep only {'start_line': 2} as the error case. I tried that check on this branch: all six empty-file calls above return {'status': 'ok', 'content': ''}, and {'start_line': 2} still errors.

Checked: PR tests on main's _read_file_tool.py give 1 failed, 9 passed (test_read_empty_file_without_line_range). On the branch, 10 passed.

Thanks for the detailed reproduction. You are right: explicit defaults should behave like omitted arguments. I updated the PR in commit 638d932 to check the normalized range instead of whether the arguments were supplied.

The empty-file guard now requires total == 0, start == 1, and end == 0. This covers the examples you listed, including start_line=0/1 and end-only bounds, while preserving errors for start_line >= 2. I also require end == 0 so negative end lines retain their previous error behavior.

The expanded tests cover equivalent whole-file ranges on empty and two-line files, explicit first-line bounds, and invalid ranges. With the previous PR implementation they produce 8 failed and 17 passed; with the update, the environment-tool suite has 30 passed on each of Python 3.11, 3.12, and 3.13. The manual LocalEnvironment matrix and formatting/lint/compliance checks also pass. Full-suite validation remains pending.

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.

2 participants