Skip to content

Handle UTF-8 JSONL tail windows losslessly - #109

Draft
theamazinghenk wants to merge 1 commit into
tinyhumansai:mainfrom
theamazinghenk:codex/fix-jsonl-utf8-tail-window
Draft

Handle UTF-8 JSONL tail windows losslessly#109
theamazinghenk wants to merge 1 commit into
tinyhumansai:mainfrom
theamazinghenk:codex/fix-jsonl-utf8-tail-window

Conversation

@theamazinghenk

Copy link
Copy Markdown

Summary

  • read the fixed-size JSONL tail window as bytes before decoding it
  • decode the window lossily so an arbitrary seek offset can start inside a multi-byte UTF-8 character
  • add a deterministic regression test that forces the 4 KiB window to begin on a UTF-8 continuation byte

Problem and root cause

JsonlAppendStore::next_offset seeks to len - 4096 for larger streams and then calls read_to_string. That seek position is an arbitrary byte offset, not necessarily a UTF-8 character boundary. If it lands inside a multi-byte character, the read fails with stream did not contain valid UTF-8, so the append is rejected and the stream remains stuck at the same failing window.

The code now reads bytes and uses String::from_utf8_lossy. This is safe for the tail scan because, whenever the window starts after byte zero, the first potentially partial line is already discarded by complete_from. A window beginning at byte zero starts on a valid character boundary.

Impact

JSONL streams containing non-ASCII text no longer lose an append or become permanently stuck merely because the 4 KiB tail window starts midway through a character. Existing stuck streams recover on their next append without migration.

Validation

Validated against current upstream main at c6a5f24bd015fc2acfa423b161cedfdbc47bae12.

  • regression test on the unmodified implementation: fails with Validation("append store read error: stream did not contain valid UTF-8")
  • regression test with this change: passes
  • cargo fmt --check
  • cargo clippy --all-targets -- -D warnings
  • cargo build --all-targets
  • cargo test (including doctests)
  • cargo run --example basic_graph

No public API changes and no new dependencies.

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.

1 participant