Skip to content

Separate the chunk diagnostics and harden the separator input - #3

Merged
IvoLeist merged 1 commit into
IvoLeist:add-langchain_text_splittersfrom
arash77:langchain-text-splitters-round-2
Aug 6, 2026
Merged

Separate the chunk diagnostics and harden the separator input#3
IvoLeist merged 1 commit into
IvoLeist:add-langchain_text_splittersfrom
arash77:langchain-text-splitters-round-2

Conversation

@arash77

@arash77 arash77 commented Aug 5, 2026

Copy link
Copy Markdown

Follow-up to #2, from a second review round. Two of these produce silently wrong output.

A start index can be wrong without being negative

Once the splitter returns one negative index, the positions it derives from that one stay positive but point at an earlier part of the input, so they looked correct and were written to the JSON as such. Reproducible with a repetitive input, recursive splitter, token length, chunk size 10, overlap 3:

before:  [0, 67, 131, 186, null, 67, 131, 186, null, 67, 131]
after:   [0, 67, 131, 186, null, null, null, null, null, null, null]

Chunks 6 to 8 claimed the positions of chunks 2 to 4. The position is now checked for order and for content, so all of them are reported as null and named in the warning.

Chunks that the splitter altered were reported as a wrong position

Splitting between tokens cuts the text at token boundaries, which can fall inside a character that is encoded in several bytes and replaces it with the Unicode replacement character. With the default gpt2 encoding, chunk size 200 and overlap 50, a Chinese input gives 6 corrupted chunks out of 9. Those chunks were reported as an invalid start index, which points the user at a known upstream issue instead of at the corrupted text. They now get their own warning naming the cause and suggesting a character based splitter.

A custom separator was silently replaced by an X

Galaxy's default sanitizer only keeps letters, digits and -=_.()/+*^,:?!. Anything else, including | and every non-ASCII character, was replaced by a literal X. So a user splitting on | got the text split at every letter X in the document. The sanitizer now keeps | ; ~ % & $ @ < > " [ ] { }, and a validator rejects anything that would still be changed, pointing at the escape syntax. The apostrophe stays rejected on purpose, since the separator is passed inside single quotes.

Smaller

  • Write the JSON output as UTF-8 as well. It was the one output still using the system locale encoding.
  • special_token_error() returns the error to raise, so the special token handling has one contract instead of an unreachable re-raise at both call sites.
  • New test for a separator that the sanitizer used to mangle.

planemo lint clean, planemo test 21/21.

…he separator input

Chunk diagnostics:
- Detect a start index that is invalid without being negative. Once the
  splitter returns one negative index, the positions it derives from it stay
  positive but point at an earlier part of the input, which was reported as if
  it were correct. The position is now checked for order and for content.
- Report text that the splitter altered separately from a wrong position.
  Splitting between tokens can cut a character that is encoded in several
  bytes, which replaces it with the Unicode replacement character. Those chunks
  used to be reported as an invalid start index, pointing the user at the wrong
  cause.

Custom separator:
- Widen the sanitizer so that characters such as | ; ~ % & $ @ < > " [ ] { }
  reach the script instead of being silently replaced by an X, which split the
  text at the wrong places.
- Reject a separator that still contains a character Galaxy cannot pass on
  unchanged, with a message pointing at the escape syntax.

Other:
- Write the JSON output as UTF-8 as well, instead of the system locale
  encoding.
- Let special_token_error() return the error to raise, so the special token
  handling has a single contract instead of an unreachable re-raise at both
  call sites.

Tests:
- Add a test for a separator that the sanitizer used to mangle.
@arash77
arash77 force-pushed the langchain-text-splitters-round-2 branch from dca398d to 6b3338c Compare August 6, 2026 13:13
@IvoLeist
IvoLeist merged commit f97dd61 into IvoLeist:add-langchain_text_splitters Aug 6, 2026
6 checks passed
@arash77
arash77 deleted the langchain-text-splitters-round-2 branch August 6, 2026 13:25
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