I'd like to pick up the read offset=0 issue — checking the fix direction

[Evan-bunny]

Hi! I saw the issue about the read tool's offset=0 bug and I'd like to take a stab at fixing it.

My plan: explicitly check offset == 0 before the offset conversion and return a clear error (explaining that offset is 1-indexed). I'd also fix the bogus "[Showing lines 1-0 ...]" note when limit=0 (skip the continuation note when no lines are returned). For regression tests, I'd add two cases covering offset=0 and limit=0.

Does this direction sound good, or would you prefer silently treating offset=0 as 1?

[Ariasu123]

Hi @Evan-bunny, thanks for picking this up!

Your direction is exactly right — rejecting offset=0 with a clear error is preferable to silently treating it as 1, since a 0 usually indicates a caller-side mistake that the caller (an LLM, in this case) should be told about so it can correct itself. The error message you proposed (explaining the 1-indexed convention and mentioning negative offsets) looks good.

Also agreed on the limit=0 note: skipping the continuation note when no lines are returned is the right fix.

One request: please include the two regression tests (offset=0 and limit=0) in tests/test_tools.py — you can follow the style of the existing test_read_offset_beyond_eof case. Otherwise, go ahead and open the PR!