Skip to content

feat(chunking): block-aware chunks with canonical offsets - #106

Merged
mrsibe merged 1 commit into
mainfrom
feat/block-aware-chunking
Sep 25, 2026
Merged

mrsibe merged 1 commit into
mainfrom
feat/block-aware-chunking

Conversation

@mrsibe

@mrsibe mrsibe commented Sep 25, 2026

Copy link
Copy Markdown
Owner

What does this PR do?

Makes chunks.start_offset/end_offset address documents.content again, and makes the chunker structure-aware: it consumes the document_blocks sequence from #66, never splits a heading, never crosses a PDF page by default, and records which blocks each chunk covers.

This is the chunking half of #67. Persisting chunk_blocks and chunks.page_start/page_end is the stacked #68 PR, which builds on this branch.

Why?

ChunkingService.chunk() ran preprocessText() first and then computed offsets against the cleaned string, while documents.content holds the loader's text. So chunks did not address the stored document at all — the root cause behind "we show a source title but cannot highlight the source" (#67, epic #82).

Related issue

Refs #67 (this PR is the chunking half; #68 persists the mapping)

What changed?

  • ChunkingService rewritten around chunkBlocks(content, blocks):
    • Splits at semantic unit boundaries (line → sentence → whitespace) inside each block. A heading block is one atomic unit and is never cut; a chunk boundary never silently falls inside it.
    • Performs no text normalization. Every chunk's content is exactly content.slice(startOffset, endOffset), so the offset contract holds by construction.
    • Returns blockSpans (ordered blocks covered + startInBlock/endInBlock) and pageStart/pageEnd for each chunk. Paged documents break at page boundaries unless allowSpanPages: true.
    • Overlap is expressed in canonical offsets and aligned to unit starts, so two overlapping chunks share the exact same bytes.
    • chunk() remains as the char-window fallback for block-less sources and now also produces exact canonical offsets.
    • Removed the length-changing preprocessText() and the unused chunkBySentence().
  • assignBlockIds(documentId, drafts) in documentBlocks.ts generates block ids once. KnowledgeService builds one identified batch, persists it, and passes the same batch to the chunker — so [Feat] Persist chunk↔block mapping and denormalized page range on chunks #68 can persist spans without rebuilding anything.
  • KnowledgeService calls chunkBlocks() in both ingestion paths.

How was this tested?

  • npm test — 108 tests pass (8 new in test/chunkBlocks.test.ts): the exact-slice invariant, dense/ordered indexing, block spans resolving back into their blocks in document order, heading atomicity under a tiny chunkSize, page ranges, cross-page opt-in, byte-identical overlap, and the char-window fallback.
  • npm run typecheck — clean.
  • npm run build — clean.
  • npx eslint on every changed file — clean.

Not run locally: build:unpack + smoke:packaged (no xvfb). CI runs them on all three platforms.

Screenshots / recordings

Not applicable — no UI change.

Checklist

  • I have reviewed my own changes.
  • npm run typecheck passes.
  • npm run build passes.
  • I have tested the affected user workflow. (delegated to CI smoke)
  • I have not included unrelated changes.
  • I have updated documentation when necessary.

Desktop / build changes

  • Not applicable
  • npm run build:unpack passes. (delegated to CI)
  • npm run smoke:packaged passes. (delegated to CI)

`ChunkingService.chunk()` ran `preprocessText()` first and computed
`startOffset`/`endOffset` against the cleaned string, so `chunks` did not
address `documents.content` at all — the root cause behind "we show a source
title but cannot highlight the source".

Rewrite the chunker around the block sequence from #66:

- `chunkBlocks(content, blocks)` splits at semantic unit boundaries
  (line → sentence → whitespace) inside each block, so a boundary can never
  fall inside a heading: a heading block is one atomic unit. It never
  re-cleans text, and every chunk's `content` is exactly
  `content.slice(startOffset, endOffset)`.
- Chunks carry `blockSpans` (the ordered blocks covered, with the character
  range consumed in each) and `pageStart`/`pageEnd`. Paged documents break at
  page boundaries unless `allowSpanPages` is set.
- Overlap is expressed in canonical offsets and aligned to unit starts, so
  two overlapping chunks share the exact same bytes.
- The char-window fallback (`chunk()`) remains for block-less sources and now
  also produces exact canonical offsets; the old length-changing
  `preprocessText()` and the unused `chunkBySentence()` are gone.
- `KnowledgeService` builds one batch of identified blocks, persists it, and
  hands the same batch to the chunker, so chunk spans can be persisted next
  (#68) without rebuilding anything.

Persisting `chunk_blocks` and the `chunks.page_start/page_end` columns is the
separate #68 step; this change already makes new chunks address
`documents.content`.

Verification: `test/chunkBlocks.test.ts` (8 tests) asserts the slice
invariant, dense indexing, span resolution, heading atomicity, page ranges,
cross-page opt-in, overlap byte-identity, and the fallback — 108 tests pass.
`npm run typecheck` and `npm run build` are clean.

Refs #67
@github-actions github-actions Bot added the enhancement New feature or request label Sep 25, 2026
@mrsibe
mrsibe merged commit 6d54415 into main Sep 25, 2026
4 checks passed
@mrsibe
mrsibe deleted the feat/block-aware-chunking branch September 25, 2026 06:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant