-
Notifications
You must be signed in to change notification settings - Fork 14
fix: convert WASM byte offsets to code-unit offsets before slicing #68
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
technoligest
wants to merge
1
commit into
feyninc:main
Choose a base branch
from
technoligest:fix/utf8-byte-offsets-in-chunkers
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| /** | ||
| * Helpers for moving between the WASM splitter's coordinate system and | ||
| * JavaScript's. | ||
| * | ||
| * `split_offsets` and `chunk_offsets` return **UTF-8 byte** offsets, while | ||
| * `String.prototype.slice` indexes **UTF-16 code units**. The two only agree | ||
| * for ASCII text, so byte offsets must be converted before they touch a string. | ||
| */ | ||
|
|
||
| /** | ||
| * Number of UTF-8 bytes needed to encode a single code point. | ||
| */ | ||
| function utf8Length(codePoint: number): number { | ||
| if (codePoint < 0x80) return 1; | ||
| if (codePoint < 0x800) return 2; | ||
| if (codePoint < 0x10000) return 3; | ||
| return 4; | ||
| } | ||
|
|
||
| /** | ||
| * Convert UTF-8 byte offsets into UTF-16 code-unit offsets for the given text. | ||
| * | ||
| * Walks the string once, so the cost is linear in the length of the text | ||
| * regardless of how many offsets are supplied. ASCII-only text is detected up | ||
| * front and returned untouched, which keeps the common case free. | ||
| * | ||
| * @param text - The text the offsets refer to | ||
| * @param byteOffsets - Array of [start, end] UTF-8 byte offset pairs | ||
| * @returns Array of [start, end] UTF-16 code-unit offset pairs | ||
| */ | ||
| export function toCharOffsets( | ||
| text: string, | ||
| byteOffsets: Array<[number, number]> | ||
| ): Array<[number, number]> { | ||
| if (byteOffsets.length === 0) return []; | ||
|
|
||
| // Byte and code-unit offsets coincide for ASCII, which is the common case. | ||
| if (!/[^\x00-\x7F]/.test(text)) return byteOffsets; | ||
|
|
||
| const targets = new Set<number>(); | ||
| for (const [start, end] of byteOffsets) { | ||
| targets.add(start); | ||
| targets.add(end); | ||
| } | ||
|
|
||
| const sorted = [...targets].sort((a, b) => a - b); | ||
| const charByByte = new Map<number, number>(); | ||
|
|
||
| let bytePos = 0; | ||
| let unitPos = 0; | ||
| let cursor = 0; | ||
|
|
||
| for (const char of text) { | ||
| while (cursor < sorted.length && sorted[cursor] <= bytePos) { | ||
| charByByte.set(sorted[cursor], unitPos); | ||
| cursor++; | ||
| } | ||
| bytePos += utf8Length(char.codePointAt(0)!); | ||
| unitPos += char.length; | ||
| } | ||
|
|
||
| // Any remaining target is at or past the end of the text. | ||
| while (cursor < sorted.length) { | ||
| charByByte.set(sorted[cursor], unitPos); | ||
| cursor++; | ||
| } | ||
|
|
||
| return byteOffsets.map(([start, end]) => [ | ||
| charByByte.get(start)!, | ||
| charByByte.get(end)!, | ||
| ]); | ||
| } |
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,147 @@ | ||
| import { RecursiveChunker, SentenceChunker } from '../src'; | ||
| import { toCharOffsets } from '../src/offsets'; | ||
|
|
||
| // The WASM splitter reports UTF-8 byte offsets while String.prototype.slice | ||
| // indexes UTF-16 code units. The two agree only for ASCII, so these tests pair | ||
| // each non-ASCII input with an ASCII input of identical code-unit length: | ||
| // chunking must not care which one it is given. | ||
| // 'é' / '–' -> 1 code unit, 2-3 UTF-8 bytes (ASCII twin: 'e' / '-') | ||
| // '🔗' -> 2 code units, 4 UTF-8 bytes (ASCII twin: 'ab') | ||
| const PARITY_CASES = { | ||
| accented: { wide: 'café '.repeat(400), ascii: 'cafe '.repeat(400) }, | ||
| dashed: { wide: 'a – b '.repeat(400), ascii: 'a - b '.repeat(400) }, | ||
| emoji: { wide: '🔗 link '.repeat(300), ascii: 'ab link '.repeat(300) }, | ||
| }; | ||
|
|
||
| describe('Unicode offset handling', () => { | ||
| describe('toCharOffsets', () => { | ||
| it('should leave ASCII offsets untouched', () => { | ||
| expect(toCharOffsets('ab cd', [[0, 2], [3, 5]])).toEqual([[0, 2], [3, 5]]); | ||
| }); | ||
|
|
||
| it('should convert byte offsets for two-byte characters', () => { | ||
| expect(toCharOffsets('é cd', [[0, 2], [3, 5]])).toEqual([[0, 1], [2, 4]]); | ||
| }); | ||
|
|
||
| it('should convert byte offsets for astral characters', () => { | ||
| expect(toCharOffsets('🔗 cd', [[0, 4], [5, 7]])).toEqual([[0, 2], [3, 5]]); | ||
| }); | ||
|
|
||
| it('should handle an empty offset list', () => { | ||
| expect(toCharOffsets('🔗', [])).toEqual([]); | ||
| }); | ||
| }); | ||
|
|
||
| describe('RecursiveChunker', () => { | ||
| // A long line without sentence delimiters forces the recursion down to the | ||
| // whitespace level, which is where offsets are applied to the string. | ||
| for (const [name, { wide, ascii }] of Object.entries(PARITY_CASES)) { | ||
| for (const chunkSize of [64, 256, 1024]) { | ||
| it(`should chunk ${name} text like its ASCII twin (chunkSize ${chunkSize})`, async () => { | ||
| const chunker = await RecursiveChunker.create({ | ||
| chunkSize, | ||
| minCharactersPerChunk: 16, | ||
| }); | ||
|
|
||
| const wideChunks = await chunker.chunk(wide); | ||
| const asciiChunks = await chunker.chunk(ascii); | ||
|
|
||
| expect(wideChunks.map(chunk => chunk.text.length)).toEqual( | ||
| asciiChunks.map(chunk => chunk.text.length) | ||
| ); | ||
| expect(wideChunks.map(chunk => chunk.startIndex)).toEqual( | ||
| asciiChunks.map(chunk => chunk.startIndex) | ||
| ); | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| it('should not drop characters from the middle of the text', async () => { | ||
| const chunker = await RecursiveChunker.create({ | ||
| chunkSize: 64, | ||
| minCharactersPerChunk: 16, | ||
| }); | ||
| const text = 'café '.repeat(400); | ||
|
|
||
| const chunks = await chunker.chunk(text); | ||
|
|
||
| // The whitespace level drops the final delimiter for ASCII text too, so | ||
| // compare against the trimmed source rather than the raw input. | ||
| expect(chunks.map(chunk => chunk.text).join('')).toBe(text.trimEnd()); | ||
| }); | ||
|
|
||
| it('should never split a surrogate pair', async () => { | ||
| const chunker = await RecursiveChunker.create({ | ||
| chunkSize: 64, | ||
| minCharactersPerChunk: 16, | ||
| }); | ||
|
|
||
| const chunks = await chunker.chunk(PARITY_CASES.emoji.wide); | ||
|
|
||
| for (const chunk of chunks) { | ||
| expect(chunk.text.isWellFormed()).toBe(true); | ||
| } | ||
| }); | ||
|
|
||
| it('should report offsets that index back into the source text', async () => { | ||
| const chunker = await RecursiveChunker.create({ | ||
| chunkSize: 64, | ||
| minCharactersPerChunk: 16, | ||
| }); | ||
| const text = 'café '.repeat(400); | ||
|
|
||
| const chunks = await chunker.chunk(text); | ||
|
|
||
| for (const chunk of chunks) { | ||
| expect(text.slice(chunk.startIndex, chunk.endIndex)).toBe(chunk.text); | ||
| } | ||
| }); | ||
| }); | ||
|
|
||
| describe('SentenceChunker', () => { | ||
| // Single-character delimiters take the WASM path; the multi-character | ||
| // defaults are split in JS and were never affected. | ||
| const delim = ['.', '!', '?']; | ||
| const text = 'Le café était très bon. Un peu cher. Mais bon quand même. Voilà.'; | ||
|
|
||
| it('should not run offsets past the end of the text', async () => { | ||
| const chunker = await SentenceChunker.create({ | ||
| chunkSize: 32, | ||
| minCharactersPerSentence: 4, | ||
| delim, | ||
| }); | ||
|
|
||
| const chunks = await chunker.chunk(text); | ||
|
|
||
| expect(chunks[chunks.length - 1].endIndex).toBe(text.length); | ||
| }); | ||
|
|
||
| it('should split sentences at delimiters rather than mid-word', async () => { | ||
| const chunker = await SentenceChunker.create({ | ||
| chunkSize: 32, | ||
| minCharactersPerSentence: 4, | ||
| delim, | ||
| }); | ||
|
|
||
| const chunks = await chunker.chunk(text); | ||
|
|
||
| for (const chunk of chunks) { | ||
| expect(chunk.text.trimEnd().endsWith('.')).toBe(true); | ||
| } | ||
| }); | ||
|
|
||
| it('should never split a surrogate pair', async () => { | ||
| const chunker = await SentenceChunker.create({ | ||
| chunkSize: 64, | ||
| minCharactersPerSentence: 8, | ||
| delim, | ||
| }); | ||
|
|
||
| const chunks = await chunker.chunk('Voir le lien 🔗 ici. '.repeat(60)); | ||
|
|
||
| for (const chunk of chunks) { | ||
| expect(chunk.text.isWellFormed()).toBe(true); | ||
| } | ||
| }); | ||
| }); | ||
| }); | ||
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: feyninc/chonkiejs
Length of output: 2641
🏁 Script executed:
Repository: feyninc/chonkiejs
Length of output: 9926
🏁 Script executed:
Repository: feyninc/chonkiejs
Length of output: 10387
Raise the test/CI Node lower bound before using
String.isWellFormed().packages/core/testsinheritspackages/core/tsconfig.json, which still useslib: ["ES2021"], so these calls will fail type-checking unless that lib is updated. Add Node20to the repo andpnpm-workspace.yamldeclarations too; the current workspace/engine config only requires Node18, butString.prototype.isWellFormed()needs Node20.🤖 Prompt for AI Agents