Repository navigation
fix: make read_page advance when max_bytes is smaller than the rune at offset - #27
Merged
Merged
Conversation
read_page paginates stored content by max_bytes and offset. When the rune at offset is longer than max_bytes, sliceAtRuneBoundary snapped the cut down to zero, returning an empty chunk with next_offset equal to the requested offset. A client that follows next_offset until truncated is false looped forever. Return the whole rune at offset when it does not fit in max_bytes, so a truncated response always makes progress. Add a regression test that pages a string of 3-byte runes with max_bytes 1 and 2 and asserts each chunk is non-empty and valid UTF-8, next_offset strictly advances, and following it reassembles the page.
Owner
|
Thanks for this! I checked it end to end: I crawled a page with multi-byte characters and paged it through read_page over MCP with max_bytes 1 and 2. On main it gets stuck; with your change it makes progress every step and the chunks reassemble the page exactly. Returning one whole character even when it exceeds max_bytes matches what #19 asked for. The earlier macOS failure was a flaky test on our side, now fixed on main. Merging, thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #19.
Problem
read_pagepaginates stored content bymax_bytes/offset. Whenmax_bytesis smaller than the multi-byte rune atoffset,sliceAtRuneBoundarysnapped the cut down to0, returningcontent: ""withnext_offset == offsetandtruncated: true. A client that followsnext_offsetuntiltruncatedis false loops forever.Change
pkg/mcp/handlers.go: when the rune-boundary snap reaches0because the rune atoffsetdoes not fit inmax_bytes, return that whole rune (viautf8.DecodeRuneInString). A truncated response now always returns at least one character andnext_offsetstrictly advances, while every chunk stays valid UTF-8.Test
pkg/mcp/handlers_test.go:TestHandleReadPage_SmallMaxBytesMakesProgresspages a string of 20 U+2019 (3-byte) runes withmax_bytes1 and 2, asserting each chunk is non-empty and valid UTF-8,next_offsetstrictly advances, paging terminates, and the chunks reassemble the page exactly. It fails against the old helper and passes with the fix.Verification
go test ./...— passes (all packages)go vet ./...— cleangofmt -l pkg/mcp/handlers.go pkg/mcp/handlers_test.go— no output