Skip to content

fix: accept GGUF byte ranges ending at EOF - #671

Open
ktz03 wants to merge 2 commits into
withcatai:masterfrom
ktz03:fix/gguf-network-eof-boundary
Open

ktz03 wants to merge 2 commits into
withcatai:masterfrom
ktz03:fix/gguf-network-eof-boundary

Conversation

@ktz03

@ktz03 ktz03 commented Oct 9, 2026

Copy link
Copy Markdown

Description of change

Reading remote GGUF metadata can fail with Expected buffer to be long enough for the requested byte range when the final metadata value ends exactly at EOF and requires another buffer expansion. The requested end offset is exclusive, so equality with the received buffer length means the range is complete.

Change the post-fetch check to reject only ranges extending beyond the buffer. Add public readGgufFileInfo regression coverage using a local HTTP range server and a synthetic GGUF with metadata larger than the reader's allocation increment. The tests cover exact EOF, trailing bytes, and truncated metadata.

Validation on Windows with Node.js 24.21.0:

  • The regression fails before the fix; the trailing-byte and truncated-input controls pass.
  • All 3 regression tests pass after the fix.
  • npm run test:typescript and npm run lint:eslint pass.
  • 151 tests pass across 28 standalone files without model inference or external network access.

The full model-dependent suite and the standalone tests requiring native bindings or live Hugging Face requests were not run.

Pull-Request Checklist

  • Code is up-to-date with the master branch
  • npm run format to apply eslint formatting
  • npm run test passes with this change — focused/offline tests, TypeScript and ESLint passed; full model-dependent suite not run
  • This pull request links relevant issues as Fixes #0000 — N/A, no existing issue found for this boundary
  • There are new or updated unit tests validating the change
  • Documentation has been updated to reflect this change — N/A, no API or usage change
  • The new commits and pull request title follow conventions explained in pull request guidelines (PRs that do not follow this convention will not be merged)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Instead of building a dummy gguf and serving it, can you please just use a URL to an example gguf from HuggingFace for testing this?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Replaced the synthetic server fixture with a pinned Hugging Face GGUF in 5a280387. The tests cover the final byte, a normal read and a read beyond EOF: the original reader fails only the EOF regression, and all three pass with the fix.

This branch has not been deployed

No deployments
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