From 7cc69c7193fcc8d11b0e3681b9d259dceccc1b6a Mon Sep 17 00:00:00 2001 From: Brent Rager Date: Sat, 22 Aug 2026 21:27:16 -0400 Subject: [PATCH 1/4] G2: chunking contract test + four silent chunker defects it found MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gap G2 was recorded open, but the Chunker the connectors feed shipped with G1. What was missing was the contract suite — and writing it found the chunker wrong on four counts, each of which fails silently rather than loudly. tests/chunking.rs (16 tests) pins chunk count/indices/ids, overlap as a shared word run, metadata+title+source+acl propagation, oversized spill with no word lost, UTF-8 integrity at the boundary, and degenerate configs. Fixed against it: - max_chars is now a hard cap on the emitted chunk, overlap included. Overlap was prepended on top of an already-full chunk, so a default 500/64 chunker emitted up to 565 chars — past the limit the embedding API truncates at. - Text without spaces now spills. Word-boundary-only splitting returned a Chinese/Japanese/Thai document, a long URL or a minified blob as ONE unbounded chunk. The fallback cuts on character boundaries. - CRLF documents split on paragraphs. "\r\n\r\n" contains no "\n\n", so every Windows-authored document arrived as one giant paragraph. - A markdown heading is a hard boundary, so no chunk straddles two sections. Characters, never bytes: slicing to a byte offset to hit a character cap cuts an em-dash or emoji in half. The UTF-8 guard is built on unspaced mixed-width text because spaced text never reaches the character-split path — the first version, built from words, passed against a deliberately byte-slicing implementation. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/g2-chunking-contract.md | 48 +++ docs/Architecture/Ingestion Pipeline.md | 24 +- docs/Planning/Feature Gaps.md | 5 +- rust/ingestion/src/chunker.rs | 138 ++++++--- rust/ingestion/tests/chunking.rs | 379 ++++++++++++++++++++++++ 5 files changed, 553 insertions(+), 41 deletions(-) create mode 100644 .changeset/g2-chunking-contract.md create mode 100644 rust/ingestion/tests/chunking.rs diff --git a/.changeset/g2-chunking-contract.md b/.changeset/g2-chunking-contract.md new file mode 100644 index 00000000..85f75b8a --- /dev/null +++ b/.changeset/g2-chunking-contract.md @@ -0,0 +1,48 @@ +--- +"@smooai/smooth-operator": patch +--- + +fix(ingestion): the chunking contract (G2) — and the four ways the chunker was wrong + +Gap **G2** was recorded as open ("our knowledge store assumes pre-chunked text"), +but the `Chunker` the connectors feed shipped with G1. What was actually missing +was the **contract suite** the gap doc asked for — and writing it found the +chunker wrong on four counts. Every one of them fails *silently*: no error, no +panic, just worse retrieval or a chunk the embedding API quietly truncates. + +`rust/ingestion/tests/chunking.rs` (16 tests) pins chunk count / dense indices / +stable ids, overlap as a shared run of words, metadata + title + source + acl +propagation onto every chunk, oversized spill with no word lost, UTF-8 integrity +at the boundary, and degenerate configs. Fixed against it: + +- **`max_chars` is a hard cap on the emitted chunk, overlap included.** Overlap + was prepended on top of a chunk that already filled the cap, so a default + 500/64 chunker emitted chunks of up to 565 characters. The cap is the contract + with the embedding model's input limit — over it, the API truncates rather + than rejects, so the tail of a chunk is dropped from the index with nothing + logged. Overlap now comes out of the packing budget. +- **Text without spaces now spills.** Splitting on word boundaries only meant a + Chinese, Japanese or Thai document — or a long URL, or a minified blob — had + exactly one "word", so it came back as a **single unbounded chunk**. A 50k-char + document became one chunk, embedded truncated, retrievable as nothing useful. + The fallback cuts on **character** boundaries. +- **CRLF documents split on paragraphs.** `"\r\n\r\n"` contains no `"\n\n"`, so + every Windows-authored file and many HTTP-fetched pages arrived as one giant + paragraph and lost all paragraph structure. Content is CRLF-normalized first. +- **A markdown heading is a hard chunk boundary.** A chunk spanning two sections + attributes section A's text to section B's heading at retrieval time. + +Characters, never bytes, throughout: slicing to a byte offset to hit a character +cap cuts an em-dash or an emoji in half — a panic in Rust, silent mojibake in a +port to the other four languages. + +Proof of red: reverting `chunker.rs` in place, leaving the tests, fails 6 of 16. +The UTF-8 guard needed a second pass and is worth calling out — built from +space-separated words it passed against a deliberately byte-slicing +implementation, because spaced text never reaches the character-split path at +all. Rebuilt on unspaced mixed-width text it panics on that same mutant. A guard +that cannot fail is the defect, not the reassurance. + +Rust only. Chunking runs server-side in the ingestion crate; the other four +languages have no ingestion pipeline to port it into, so there is nothing to +port yet. No public API change — `Chunker::new` / `chunk` keep their signatures. diff --git a/docs/Architecture/Ingestion Pipeline.md b/docs/Architecture/Ingestion Pipeline.md index 780e655f..0c7d295f 100644 --- a/docs/Architecture/Ingestion Pipeline.md +++ b/docs/Architecture/Ingestion Pipeline.md @@ -144,12 +144,27 @@ Chunker::new(max_chars, overlap_chars) // or Chunker::default() (500 / 64) Strategy: -1. split content into paragraphs on blank lines (`\n\n`), -2. greedily pack paragraphs into a chunk up to `max_chars`, -3. a single paragraph larger than the cap is hard-split on word boundaries, +1. normalize CRLF, then split content into paragraphs on blank lines, +2. greedily pack paragraphs into a chunk, **breaking before a markdown heading** + so no chunk straddles two sections, +3. an oversized paragraph spills on word boundaries; a single token longer than + the budget — a URL, or any script that does not use spaces (Chinese, + Japanese, Thai) — spills on **character** boundaries, 4. successive chunks overlap by `overlap_chars` of trailing whole words so a fact spanning a boundary stays retrievable. +`max_chars` is a **hard cap on the emitted chunk, overlap included** — it is the +contract with the embedding model's input limit, and a chunk over it is silently +truncated by the API rather than rejected. Overlap is therefore spent out of the +packing budget rather than added on top of a chunk that already fills it. + +Everything is denominated in **characters, never bytes**. Slicing to a byte +offset to hit a character cap cuts an em-dash or an emoji in half: a panic in +Rust, silent mojibake in a port. `tests/chunking.rs` guards this on unspaced +mixed-width text, which is the only input that reaches the character-split path +at all — a guard built from space-separated words passes against a byte-slicing +implementation and protects nothing. + `overlap_chars` is clamped below `max_chars` so chunking always makes forward progress. Each `Chunk` gets a **stable id** `"{doc_id}#{index}"` and inherits the source document's title, metadata (`title`, `source`, plus any custom keys), and @@ -216,7 +231,7 @@ keyed and unkeyed paths can never silently mix dimensions. | Tier | What | When | | ----------- | ---------------------------------------------------- | -------------- | -| `unit` | chunker, embedder, file connector, web strip/guard, GitHub path/issue/ACL filters + `tests/github_connector.rs` (mock GitHub API → prose/code/issue RawDocuments + ingest→retrieve), `tests/ingestion_contract.rs` (chunk→embed→store→retrieve + idempotency) | every PR, no creds | +| `unit` | chunker, embedder, file connector, web strip/guard, GitHub path/issue/ACL filters + `tests/github_connector.rs` (mock GitHub API → prose/code/issue RawDocuments + ingest→retrieve), `tests/ingestion_contract.rs` (chunk→embed→store→retrieve + idempotency), `tests/chunking.rs` (the chunking contract: cap-with-overlap, boundary rules, CRLF, unspaced/multi-byte spill, metadata propagation) | every PR, no creds | | `external` | `WebConnector::live_fetch_example`, `GithubConnector` live pull | gated on `SMOOTH_AGENT_E2E=1`, nightly | The headline acceptance is `rust/ingestion/tests/ingestion_contract.rs`: it wires @@ -238,6 +253,7 @@ SMOOTH_AGENT_E2E=1 cargo test -p smooai-smooth-operator-ingestion \ - `rust/ingestion/src/connector.rs` — `Connector` trait, `RawDocument`, `MockConnector` - `rust/ingestion/src/chunker.rs` — `Chunker`, `Chunk` (G2) +- `rust/ingestion/tests/chunking.rs` — the chunking contract (G2) - `rust/ingestion/src/embedder.rs` — `Embedder`, `DeterministicEmbedder` - `rust/ingestion/src/pipeline.rs` — `ingest`, `IngestOptions`, `IngestLedger`, `IngestReport` - `rust/ingestion/src/connectors/file.rs` — `FileConnector` diff --git a/docs/Planning/Feature Gaps.md b/docs/Planning/Feature Gaps.md index 9326c34b..12419864 100644 --- a/docs/Planning/Feature Gaps.md +++ b/docs/Planning/Feature Gaps.md @@ -49,9 +49,10 @@ Mature platforms ship 50+ connectors (confluence, jira, github, gmail, google_dr - ⚠️ **Deviation from the sketch above: `pull` returns `Vec`, not `Stream`.** A `Vec` materializes the whole source per pull. That is fine for the three shipped connectors and wrong for a large Confluence space or Jira project — which is precisely what the next connectors are. Re-shaping `pull` into a stream (or a paged `pull_page(cursor)`) is a **breaking change to the trait**, so it should land *before* the SaaS connectors, not after them. - **Remains**: the SaaS long tail — confluence, jira, notion, slack, zendesk, google_drive, salesforce, sharepoint. Confluence and Jira are the hardest shape (deep pagination, incremental `since`, per-document permissions) and should be designed against first; their per-document permissions must map onto `RawDocument::acl` the way `GithubConnectorConfig::acl_groups` already does, or ingesting them reopens G3. -### G2. Document processing / chunking pipeline -Mature knowledge platforms have a tested chunking + metadata-extraction pipeline. Our knowledge store assumes pre-chunked text. +### G2. Document processing / chunking pipeline — ✅ done +Mature knowledge platforms have a tested chunking + metadata-extraction pipeline. Our knowledge store assumed pre-chunked text. - **TDD**: `tests/chunking.rs` first — feed a long doc + assert chunk count, overlap, boundary rules, metadata propagation, and that oversized items spill correctly. Then implement the chunker the connectors feed. +- ✅ **Done.** The `Chunker` the connectors feed shipped with G1 (`rust/ingestion/src/chunker.rs`); what was missing was the **contract suite** — and writing it found the chunker wrong on four counts, each of which fails silently rather than loudly. `rust/ingestion/tests/chunking.rs` (16 tests) now pins: chunk count/dense indices/stable ids, overlap as a shared word-run, metadata + title + source + acl propagation on every chunk, oversized spill without word loss, and degenerate configs. Fixed against it: (a) **`max_chars` is now a hard cap on the emitted chunk** — overlap was prepended *on top of* an already-full chunk, so real chunks ran over the cap the embedding API truncates at; it now comes out of the packing budget; (b) **unspaced scripts now spill** — splitting on word boundaries only meant a Chinese/Japanese/Thai document, a long URL, or a minified blob returned as **one unbounded chunk**, no error; the fallback cuts on **character** boundaries; (c) **CRLF documents split on paragraphs** — `"\r\n\r\n"` contains no `"\n\n"`, so every Windows-authored and many HTTP-fetched documents arrived as one giant paragraph; (d) **a markdown heading is a hard boundary**, so a chunk never straddles two sections and attributes section A's text to section B's heading. Proof of red: reverting `chunker.rs` in place fails 6 of the 16. The UTF-8 guard needed a second pass — built from space-separated words it passed against a deliberately byte-slicing implementation, i.e. guarded nothing; rebuilt on unspaced mixed-width text it panics on the same mutant, which is the only version worth keeping. Rust only: chunking runs server-side in the ingestion crate, and the other four languages have no ingestion pipeline to port it into. See [[Ingestion Pipeline]]. ### G3. Access control / permissions (document-level) — ✅ enforced on the live chat path Mature knowledge platforms sync per-connector permissions and filters retrieval by user entitlement. We filter by `organizationId` only. diff --git a/rust/ingestion/src/chunker.rs b/rust/ingestion/src/chunker.rs index c3ce1b4b..bd6420dc 100644 --- a/rust/ingestion/src/chunker.rs +++ b/rust/ingestion/src/chunker.rs @@ -7,12 +7,22 @@ //! //! ## Strategy //! -//! 1. Split content into paragraphs on blank lines (`\n\n`). -//! 2. Greedily pack paragraphs into a chunk up to [`Chunker::max_chars`]. -//! 3. A single paragraph larger than the cap is hard-split on word boundaries. +//! 1. Normalize CRLF, then split content into paragraphs on blank lines. +//! 2. Greedily pack paragraphs into a chunk, **breaking before a markdown +//! heading** so one chunk never straddles two sections. +//! 3. An oversized paragraph spills on word boundaries; a single "word" longer +//! than the budget (a URL, or any script that does not use spaces — Chinese, +//! Japanese, Thai) spills on *character* boundaries. Characters, never bytes: +//! slicing an em-dash or an emoji in half is a panic in Rust and mojibake in +//! a port. //! 4. Successive chunks overlap by [`Chunker::overlap_chars`] of trailing text //! (carried as whole words) so a fact spanning a boundary stays retrievable. //! +//! [`Chunker::max_chars`] is a **hard cap on the emitted chunk**, overlap +//! included: it is the contract with the embedding model's input limit, and a +//! chunk that exceeds it is silently truncated by the API rather than rejected. +//! Overlap is therefore spent out of the packing budget, not added on top of it. +//! //! Each [`Chunk`] gets a **stable id** — `"{doc_id}#{index}"` — and inherits the //! source document's title/metadata/acl, so retrieval can attribute and (later) //! access-control every chunk. @@ -79,6 +89,17 @@ impl Chunker { self.overlap_chars } + /// Characters available for *packed content*, leaving room for the overlap + /// the next chunk will prepend (plus its joining space). This is what keeps + /// `max_chars` a hard cap on the emitted chunk rather than on its content. + fn pack_budget(&self) -> usize { + if self.overlap_chars == 0 { + self.max_chars + } else { + self.max_chars.saturating_sub(self.overlap_chars + 1).max(1) + } + } + /// Chunk a [`RawDocument`], returning its ordered [`Chunk`]s. /// /// An empty / whitespace-only document yields no chunks. @@ -113,17 +134,23 @@ impl Chunker { /// Split raw content into chunk-sized texts (no metadata; pure string work). fn split_text(&self, content: &str) -> Vec { - // 1. Paragraph units (blank-line separated), oversized ones hard-split. + // CRLF-authored files and many HTTP responses separate paragraphs with + // "\r\n\r\n", which contains no "\n\n" at all — without this, every + // Windows-authored document arrives as one giant paragraph. + let content = content.replace("\r\n", "\n"); + let budget = self.pack_budget(); + + // 1. Paragraph units (blank-line separated), oversized ones spilled. let mut units: Vec = Vec::new(); for para in content.split("\n\n") { let trimmed = para.trim(); if trimmed.is_empty() { continue; } - if trimmed.chars().count() <= self.max_chars { + if trimmed.chars().count() <= budget { units.push(trimmed.to_string()); } else { - units.extend(self.hard_split_words(trimmed)); + units.extend(self.spill(trimmed, budget)); } } @@ -133,7 +160,12 @@ impl Chunker { for unit in units { if current.is_empty() { current = unit; - } else if current.chars().count() + 2 + unit.chars().count() <= self.max_chars { + } else if is_heading(&unit) { + // A chunk spanning two sections attributes section A's text to + // section B's heading at retrieval time. Break before headings. + chunks.push(std::mem::take(&mut current)); + current = unit; + } else if current.chars().count() + 2 + unit.chars().count() <= budget { current.push_str("\n\n"); current.push_str(&unit); } else { @@ -148,19 +180,23 @@ impl Chunker { self.apply_overlap(chunks) } - /// Hard-split a single oversized paragraph at word boundaries. - fn hard_split_words(&self, para: &str) -> Vec { + /// Spill one oversized paragraph into budget-sized pieces, preferring word + /// boundaries and falling back to character boundaries for a single token + /// that is itself too long. + fn spill(&self, para: &str, budget: usize) -> Vec { let mut out = Vec::new(); let mut current = String::new(); for word in para.split_whitespace() { - if current.is_empty() { - current.push_str(word); - } else if current.chars().count() + 1 + word.chars().count() > self.max_chars { - out.push(std::mem::take(&mut current)); - current.push_str(word); - } else { - current.push(' '); - current.push_str(word); + for piece in split_oversized_word(word, budget) { + if current.is_empty() { + current = piece; + } else if current.chars().count() + 1 + piece.chars().count() > budget { + out.push(std::mem::take(&mut current)); + current = piece; + } else { + current.push(' '); + current.push_str(&piece); + } } } if !current.is_empty() { @@ -170,7 +206,8 @@ impl Chunker { } /// Prepend the trailing `overlap_chars` (rounded to whole words) of each - /// chunk onto the next, so a boundary-spanning fact appears in both. + /// chunk onto the next, so a boundary-spanning fact appears in both — never + /// pushing the result past `max_chars`. fn apply_overlap(&self, chunks: Vec) -> Vec { if self.overlap_chars == 0 || chunks.len() < 2 { return chunks; @@ -181,7 +218,12 @@ impl Chunker { out.push(chunk.clone()); continue; } - let tail = self.trailing_words(&chunks[i - 1]); + // Whatever room is left under the cap, never more than the overlap. + let room = self + .max_chars + .saturating_sub(chunk.chars().count() + 1) + .min(self.overlap_chars); + let tail = trailing_words(&chunks[i - 1], room); if tail.is_empty() { out.push(chunk.clone()); } else { @@ -190,25 +232,51 @@ impl Chunker { } out } +} - /// The last whole words of `s` totaling at most `overlap_chars` characters. - fn trailing_words(&self, s: &str) -> String { - let words: Vec<&str> = s.split_whitespace().collect(); - let mut take = 0usize; - let mut len = 0usize; - for word in words.iter().rev() { - let add = word.chars().count() + usize::from(take > 0); - if len + add > self.overlap_chars { - break; - } - len += add; - take += 1; - } - if take == 0 { - return String::new(); +/// A markdown ATX heading line (`# `, `## `, …) — a hard chunk boundary. +fn is_heading(unit: &str) -> bool { + unit.starts_with('#') +} + +/// Split one word into pieces of at most `budget` **characters**. +/// +/// A word that fits is returned whole. One that does not — a long URL, a +/// minified blob, or a run of Chinese/Japanese/Thai, none of which contain a +/// space to break on — is cut on character boundaries. `chars()` is what makes +/// that safe: slicing the same string by bytes would cut a multi-byte codepoint +/// in half. +fn split_oversized_word(word: &str, budget: usize) -> Vec { + if word.chars().count() <= budget { + return vec![word.to_string()]; + } + let chars: Vec = word.chars().collect(); + chars + .chunks(budget.max(1)) + .map(|piece| piece.iter().collect()) + .collect() +} + +/// The last whole words of `s` totaling at most `limit` characters. +fn trailing_words(s: &str, limit: usize) -> String { + if limit == 0 { + return String::new(); + } + let words: Vec<&str> = s.split_whitespace().collect(); + let mut take = 0usize; + let mut len = 0usize; + for word in words.iter().rev() { + let add = word.chars().count() + usize::from(take > 0); + if len + add > limit { + break; } - words[words.len() - take..].join(" ") + len += add; + take += 1; + } + if take == 0 { + return String::new(); } + words[words.len() - take..].join(" ") } impl Default for Chunker { diff --git a/rust/ingestion/tests/chunking.rs b/rust/ingestion/tests/chunking.rs new file mode 100644 index 00000000..dc2d2850 --- /dev/null +++ b/rust/ingestion/tests/chunking.rs @@ -0,0 +1,379 @@ +//! Chunking-pipeline contract (feature gap G2). +//! +//! The unit tests inside `src/chunker.rs` cover the happy path on ASCII prose. +//! This suite is the *contract*: the invariants a downstream embedder and a +//! retrieval path depend on, exercised over the inputs real connectors actually +//! deliver — CRLF-authored files, scripts without spaces, and text whose chunk +//! boundary lands on a multi-byte codepoint. +//! +//! Every assertion here is one the pipeline would silently violate rather than +//! fail loudly: an over-cap chunk gets truncated by the embedding API, a +//! never-split CJK document becomes one useless 100k-char "chunk", and a +//! codepoint sliced in half is a panic or mojibake far from the cause. + +use smooth_operator_ingestion::{Chunk, Chunker, RawDocument}; + +/// Characters (not bytes) — the unit the chunker's cap is denominated in. +fn chars(s: &str) -> usize { + s.chars().count() +} + +fn text_of(chunks: &[Chunk]) -> Vec<&str> { + chunks.iter().map(|c| c.text.as_str()).collect() +} + +// --------------------------------------------------------------------------- +// Chunk count, order, identity +// --------------------------------------------------------------------------- + +#[test] +fn chunk_count_matches_the_content_and_indices_are_dense() { + // Five paragraphs of ~30 chars with a 40-char cap and no overlap: no two + // paragraphs fit together, so the count is exactly the paragraph count. + let content = (0..5) + .map(|i| format!("paragraph number {i} of the doc")) + .collect::>() + .join("\n\n"); + let doc = RawDocument::new("doc-a", "test", content); + let chunks = Chunker::new(40, 0).chunk(&doc); + + assert_eq!(chunks.len(), 5, "got {:?}", text_of(&chunks)); + for (i, c) in chunks.iter().enumerate() { + assert_eq!(c.index, i, "indices must be dense and ordered"); + assert_eq!(c.id, format!("doc-a#{i}"), "id is doc-scoped + positional"); + assert_eq!(c.document_id, "doc-a"); + } +} + +#[test] +fn chunking_is_deterministic() { + let doc = RawDocument::new("doc-d", "test", "alpha beta\n\ngamma delta\n\nepsilon zeta"); + let chunker = Chunker::new(20, 5); + assert_eq!(chunker.chunk(&doc), chunker.chunk(&doc)); +} + +// --------------------------------------------------------------------------- +// The size cap — the invariant the embedder depends on +// --------------------------------------------------------------------------- + +#[test] +fn no_chunk_exceeds_the_cap_even_with_overlap_on() { + // The cap is the contract with the embedding model's token limit. Overlap + // is a retrieval nicety; it must be spent *inside* the budget, never added + // on top of a chunk that already fills it. + let content = (0..12) + .map(|i| format!("paragraph {i} carries enough words to nearly fill a chunk by itself")) + .collect::>() + .join("\n\n"); + let doc = RawDocument::new("doc-cap", "test", content); + + for (max, overlap) in [(80usize, 20usize), (120, 40), (500, 64), (40, 15)] { + let chunks = Chunker::new(max, overlap).chunk(&doc); + assert!(!chunks.is_empty()); + for c in &chunks { + assert!( + chars(&c.text) <= max, + "cap {max}/overlap {overlap}: chunk is {} chars: {:?}", + chars(&c.text), + c.text + ); + } + } +} + +// --------------------------------------------------------------------------- +// Overlap +// --------------------------------------------------------------------------- + +#[test] +fn successive_chunks_share_trailing_context() { + let content = (0..6) + .map(|i| format!("sentence {i} about widgets and gears")) + .collect::>() + .join("\n\n"); + let doc = RawDocument::new("doc-o", "test", content); + let chunks = Chunker::new(60, 20).chunk(&doc); + + assert!(chunks.len() >= 3, "need several chunks to test overlap"); + for pair in chunks.windows(2) { + // The invariant is a shared *run of words*, not one word: chunk N+1 + // must open with some non-empty word sequence that chunk N closes with. + let next_words: Vec<&str> = pair[1].text.split_whitespace().collect(); + let shared = (1..=next_words.len()) + .rev() + .map(|n| next_words[..n].join(" ")) + .find(|prefix| pair[0].text.ends_with(prefix.as_str())); + let shared = shared.unwrap_or_else(|| { + panic!( + "chunk {} shares no leading text with the previous chunk\n prev: {:?}\n next: {:?}", + pair[1].index, pair[0].text, pair[1].text + ) + }); + assert!( + shared.chars().count() <= 20, + "overlap {shared:?} exceeds the configured 20 chars" + ); + } +} + +#[test] +fn zero_overlap_means_no_repetition() { + let doc = RawDocument::new("doc-z", "test", "alpha alpha\n\nbeta beta\n\ngamma gamma"); + let chunks = Chunker::new(15, 0).chunk(&doc); + assert_eq!( + text_of(&chunks), + vec!["alpha alpha", "beta beta", "gamma gamma"] + ); +} + +// --------------------------------------------------------------------------- +// Boundary rules +// --------------------------------------------------------------------------- + +#[test] +fn a_markdown_heading_starts_a_new_chunk() { + // A chunk that straddles two sections attributes section A's text to + // section B's heading at retrieval time. The heading is a hard boundary. + let content = "## Refunds\n\nRefunds take five days.\n\n## Shipping\n\nShipping is free."; + let doc = RawDocument::new("doc-h", "test", content); + // Cap is wide enough that a naive packer would merge all four paragraphs. + let chunks = Chunker::new(500, 0).chunk(&doc); + + assert!( + chunks.len() >= 2, + "the two sections must not share a chunk: {:?}", + text_of(&chunks) + ); + let refunds = chunks.iter().find(|c| c.text.contains("Refunds")).unwrap(); + assert!( + !refunds.text.contains("Shipping"), + "section bleed: {:?}", + refunds.text + ); +} + +#[test] +fn crlf_documents_split_on_paragraphs_like_lf_ones() { + // Windows-authored files and many HTTP responses use CRLF. If the splitter + // only knows "\n\n", the whole document is one paragraph and paragraph + // structure is lost for every such source. + let lf = "alpha paragraph\n\nbeta paragraph\n\ngamma paragraph"; + let crlf = "alpha paragraph\r\n\r\nbeta paragraph\r\n\r\ngamma paragraph"; + let chunker = Chunker::new(20, 0); + + let lf_chunks = chunker.chunk(&RawDocument::new("d", "test", lf)); + let crlf_chunks = chunker.chunk(&RawDocument::new("d", "test", crlf)); + + assert_eq!( + text_of(&crlf_chunks), + text_of(&lf_chunks), + "CRLF must chunk identically to LF" + ); + for c in &crlf_chunks { + assert!( + !c.text.contains('\r'), + "stray CR in chunk text: {:?}", + c.text + ); + } +} + +// --------------------------------------------------------------------------- +// Oversized items spill +// --------------------------------------------------------------------------- + +#[test] +fn an_oversized_paragraph_spills_across_chunks_without_losing_words() { + let words: Vec = (0..200).map(|i| format!("w{i}")).collect(); + let doc = RawDocument::new("doc-s", "test", words.join(" ")); + let chunks = Chunker::new(50, 0).chunk(&doc); + + assert!(chunks.len() > 1, "oversized paragraph must spill"); + let rejoined: Vec = chunks + .iter() + .flat_map(|c| c.text.split_whitespace().map(str::to_string)) + .collect(); + assert_eq!( + rejoined, words, + "spilling must preserve every word, in order" + ); +} + +#[test] +fn text_with_no_whitespace_still_spills() { + // Chinese, Japanese and Thai carry no spaces, and so do minified payloads + // and long URLs. A word-boundary-only splitter returns the whole document + // as one chunk — silently, with no error — and the embedder truncates it. + let doc = RawDocument::new( + "doc-cjk", + "test", + "宽带上网服务的开通与故障处理流程说明".repeat(20), + ); + let chunks = Chunker::new(60, 0).chunk(&doc); + + assert!( + chunks.len() > 1, + "unspaced text must still spill, got {} chunk(s)", + chunks.len() + ); + for c in &chunks { + assert!(chars(&c.text) <= 60, "chunk is {} chars", chars(&c.text)); + } +} + +#[test] +fn a_single_word_longer_than_the_cap_is_split_not_emitted_whole() { + let long_token = "a".repeat(250); + let doc = RawDocument::new("doc-t", "test", format!("prefix {long_token} suffix")); + let chunks = Chunker::new(40, 0).chunk(&doc); + for c in &chunks { + assert!(chars(&c.text) <= 40, "chunk is {} chars", chars(&c.text)); + } +} + +// --------------------------------------------------------------------------- +// UTF-8 integrity at the boundary +// --------------------------------------------------------------------------- + +#[test] +fn chunk_boundaries_never_split_a_codepoint() { + // The hazard: a splitter that slices by *bytes* to hit a character cap cuts + // an em-dash or an emoji in half. In Rust that is an outright panic; in a + // port of this logic it is silent mojibake. + // + // Reaching the byte-slicing path takes unspaced text — spaced text never + // needs a character-level cut, so a test built from words would pass + // against a byte-slicing implementation and guard nothing. The unit below + // is deliberately mixed-width (1/2/3/4 bytes) so no cap can accidentally + // land on a character boundary every time. + let unit = "aé数🚀—"; + let content = unit.repeat(40); + let doc = RawDocument::new("doc-u", "test", &content); + + for max in [7usize, 11, 17, 23] { + let chunks = Chunker::new(max, 0).chunk(&doc); + assert!(chunks.len() > 1, "cap {max}: expected the run to spill"); + let rejoined: String = chunks.iter().map(|c| c.text.as_str()).collect(); + assert_eq!( + rejoined, content, + "cap {max}: spilling an unspaced run must reproduce it exactly" + ); + assert!( + !rejoined.contains('\u{FFFD}'), + "cap {max}: replacement char — a codepoint was cut" + ); + for ch in ['é', '数', '🚀', '—'] { + assert_eq!( + rejoined.matches(ch).count(), + content.matches(ch).count(), + "cap {max}: lost or duplicated {ch:?} at a chunk boundary" + ); + } + for c in &chunks { + assert!( + chars(&c.text) <= max, + "cap {max}: chunk is {} chars", + chars(&c.text) + ); + } + } +} + +#[test] +fn spilling_multibyte_text_with_overlap_keeps_every_chunk_under_the_cap() { + // Same hazard on the overlap path: the tail is measured in characters, so a + // byte-denominated implementation would over- or under-count it. + let doc = RawDocument::new("doc-uo", "test", "aé数🚀— ".repeat(40)); + let chunks = Chunker::new(24, 8).chunk(&doc); + assert!(chunks.len() > 1); + for c in &chunks { + assert!( + chars(&c.text) <= 24, + "chunk is {} chars: {:?}", + chars(&c.text), + c.text + ); + } +} + +// --------------------------------------------------------------------------- +// Metadata propagation +// --------------------------------------------------------------------------- + +#[test] +fn every_chunk_carries_the_documents_title_metadata_source_and_acl() { + let doc = RawDocument::new( + "doc-m", + "confluence", + "alpha paragraph here\n\nbeta paragraph here\n\ngamma paragraph here", + ) + .with_title("Refund Policy") + .with_metadata("space", "SUPPORT") + .with_acl(vec!["group:support".to_string()]); + + let chunks = Chunker::new(25, 0).chunk(&doc); + assert!(chunks.len() >= 3, "need several chunks"); + for c in &chunks { + assert_eq!( + c.metadata.get("title").map(String::as_str), + Some("Refund Policy") + ); + assert_eq!(c.metadata.get("space").map(String::as_str), Some("SUPPORT")); + assert_eq!( + c.metadata.get("source").map(String::as_str), + Some("confluence") + ); + assert_eq!(c.acl.as_deref(), Some(&["group:support".to_string()][..])); + } +} + +#[test] +fn explicit_metadata_wins_over_the_derived_title_and_source_keys() { + let doc = RawDocument::new("doc-w", "web", "some content") + .with_title("Derived") + .with_metadata("title", "Explicit") + .with_metadata("source", "explicit-source"); + let chunks = Chunker::default().chunk(&doc); + assert_eq!( + chunks[0].metadata.get("title").map(String::as_str), + Some("Explicit") + ); + assert_eq!( + chunks[0].metadata.get("source").map(String::as_str), + Some("explicit-source") + ); +} + +// --------------------------------------------------------------------------- +// Degenerate input +// --------------------------------------------------------------------------- + +#[test] +fn whitespace_only_and_empty_documents_yield_nothing() { + for content in ["", " ", "\n\n\n", "\r\n\r\n", " \t \n "] { + let doc = RawDocument::new("d", "test", content); + assert!( + Chunker::default().chunk(&doc).is_empty(), + "expected no chunks for {content:?}" + ); + } +} + +#[test] +fn a_degenerate_config_still_terminates_and_respects_its_cap() { + // overlap >= max would loop forever if it were not clamped. + let doc = RawDocument::new("d", "test", "alpha beta gamma delta epsilon zeta eta"); + for (max, overlap) in [(1usize, 999usize), (2, 999), (5, 5), (10, 9)] { + let chunker = Chunker::new(max, overlap); + let chunks = chunker.chunk(&doc); + assert!(!chunks.is_empty(), "cap {max}/overlap {overlap}: no chunks"); + for c in &chunks { + assert!( + chars(&c.text) <= max.max(1), + "cap {max}/overlap {overlap}: chunk is {} chars: {:?}", + chars(&c.text), + c.text + ); + } + } +} From ec86ea39ceedf8c29b995b7e8fb043e583734adb Mon Sep 17 00:00:00 2001 From: Brent Rager Date: Sat, 22 Aug 2026 21:38:29 -0400 Subject: [PATCH 2/4] G2: fold format-extraction remainder into the gap doc, note the green-tests trap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Takes sole ownership of §G2 in the gap doc so #535 and #536 don't conflict on the same section. Folds in #535's correct finding — format extraction (PDF / DOCX / rich HTML → text) still has to land ahead of the chunker before any document-store connector works. Also records why the pre-existing green unit tests were not evidence the chunker was correct: each asserted its rule on the one input shape that never triggers the bug (cap checked only with overlap off, spill only on spaced ASCII, paragraphs only with LF). Co-Authored-By: Claude Opus 5 (1M context) --- docs/Planning/Feature Gaps.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/Planning/Feature Gaps.md b/docs/Planning/Feature Gaps.md index 12419864..8de933ae 100644 --- a/docs/Planning/Feature Gaps.md +++ b/docs/Planning/Feature Gaps.md @@ -49,10 +49,12 @@ Mature platforms ship 50+ connectors (confluence, jira, github, gmail, google_dr - ⚠️ **Deviation from the sketch above: `pull` returns `Vec`, not `Stream`.** A `Vec` materializes the whole source per pull. That is fine for the three shipped connectors and wrong for a large Confluence space or Jira project — which is precisely what the next connectors are. Re-shaping `pull` into a stream (or a paged `pull_page(cursor)`) is a **breaking change to the trait**, so it should land *before* the SaaS connectors, not after them. - **Remains**: the SaaS long tail — confluence, jira, notion, slack, zendesk, google_drive, salesforce, sharepoint. Confluence and Jira are the hardest shape (deep pagination, incremental `since`, per-document permissions) and should be designed against first; their per-document permissions must map onto `RawDocument::acl` the way `GithubConnectorConfig::acl_groups` already does, or ingesting them reopens G3. -### G2. Document processing / chunking pipeline — ✅ done +### G2. Document processing / chunking pipeline — ✅ chunker done + contract-tested; format extraction remains Mature knowledge platforms have a tested chunking + metadata-extraction pipeline. Our knowledge store assumed pre-chunked text. - **TDD**: `tests/chunking.rs` first — feed a long doc + assert chunk count, overlap, boundary rules, metadata propagation, and that oversized items spill correctly. Then implement the chunker the connectors feed. - ✅ **Done.** The `Chunker` the connectors feed shipped with G1 (`rust/ingestion/src/chunker.rs`); what was missing was the **contract suite** — and writing it found the chunker wrong on four counts, each of which fails silently rather than loudly. `rust/ingestion/tests/chunking.rs` (16 tests) now pins: chunk count/dense indices/stable ids, overlap as a shared word-run, metadata + title + source + acl propagation on every chunk, oversized spill without word loss, and degenerate configs. Fixed against it: (a) **`max_chars` is now a hard cap on the emitted chunk** — overlap was prepended *on top of* an already-full chunk, so real chunks ran over the cap the embedding API truncates at; it now comes out of the packing budget; (b) **unspaced scripts now spill** — splitting on word boundaries only meant a Chinese/Japanese/Thai document, a long URL, or a minified blob returned as **one unbounded chunk**, no error; the fallback cuts on **character** boundaries; (c) **CRLF documents split on paragraphs** — `"\r\n\r\n"` contains no `"\n\n"`, so every Windows-authored and many HTTP-fetched documents arrived as one giant paragraph; (d) **a markdown heading is a hard boundary**, so a chunk never straddles two sections and attributes section A's text to section B's heading. Proof of red: reverting `chunker.rs` in place fails 6 of the 16. The UTF-8 guard needed a second pass — built from space-separated words it passed against a deliberately byte-slicing implementation, i.e. guarded nothing; rebuilt on unspaced mixed-width text it panics on the same mutant, which is the only version worth keeping. Rust only: chunking runs server-side in the ingestion crate, and the other four languages have no ingestion pipeline to port it into. See [[Ingestion Pipeline]]. +- ⚠️ **The green unit tests were not evidence the chunker was right.** `src/chunker.rs` shipped with a unit test per rule this plan names — count, packing, cap split, oversized spill, overlap carry, overlap clamp, metadata, id stability — all passing, all passing *through* the four defects above. Each test asserted its rule on the one input shape that never triggers the bug: the cap was checked only with overlap **off**, spill only on space-separated ASCII, paragraphs only with LF. A rule-by-rule checklist against the author's own choice of input is not a contract; the contract is the rule asserted across the inputs the connectors actually deliver. +- **Remains**: **format extraction** (PDF / DOCX / rich HTML → text) ahead of the chunker. The pipeline still takes text a connector already extracted — the web connector strips HTML, and the GitHub connector *skips* binary extensions rather than reading them. Any connector over a document store (Drive, SharePoint, Confluence attachments) needs this first. (Credit: `g1conn`, via #535.) ### G3. Access control / permissions (document-level) — ✅ enforced on the live chat path Mature knowledge platforms sync per-connector permissions and filters retrieval by user entitlement. We filter by `organizationId` only. From 792b947b2854e764081dc9b21997f600da06aa95 Mon Sep 17 00:00:00 2001 From: Brent Rager Date: Sat, 22 Aug 2026 21:53:33 -0400 Subject: [PATCH 3/4] G2: pin Chunk.acl across every split path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A dropped ACL in the chunker fails open, not closed: the pipeline turns Chunk.acl into the DocAcl the adapters filter on, so a chunk arriving with acl: None is stored org-public and retrievable by anyone, with nothing logged. The existing assertion covered the packed path only; this covers the word-spill, character-spill and heading-break paths added in the previous commit. Mutation-checked — forcing acl: None in the chunk builder fails it. Reported by g1conn while tracing adapter-side ACL enforcement for #535. Co-Authored-By: Claude Opus 5 (1M context) --- docs/Planning/Feature Gaps.md | 2 +- rust/ingestion/tests/chunking.rs | 49 ++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/docs/Planning/Feature Gaps.md b/docs/Planning/Feature Gaps.md index 8de933ae..7f9861b3 100644 --- a/docs/Planning/Feature Gaps.md +++ b/docs/Planning/Feature Gaps.md @@ -52,7 +52,7 @@ Mature platforms ship 50+ connectors (confluence, jira, github, gmail, google_dr ### G2. Document processing / chunking pipeline — ✅ chunker done + contract-tested; format extraction remains Mature knowledge platforms have a tested chunking + metadata-extraction pipeline. Our knowledge store assumed pre-chunked text. - **TDD**: `tests/chunking.rs` first — feed a long doc + assert chunk count, overlap, boundary rules, metadata propagation, and that oversized items spill correctly. Then implement the chunker the connectors feed. -- ✅ **Done.** The `Chunker` the connectors feed shipped with G1 (`rust/ingestion/src/chunker.rs`); what was missing was the **contract suite** — and writing it found the chunker wrong on four counts, each of which fails silently rather than loudly. `rust/ingestion/tests/chunking.rs` (16 tests) now pins: chunk count/dense indices/stable ids, overlap as a shared word-run, metadata + title + source + acl propagation on every chunk, oversized spill without word loss, and degenerate configs. Fixed against it: (a) **`max_chars` is now a hard cap on the emitted chunk** — overlap was prepended *on top of* an already-full chunk, so real chunks ran over the cap the embedding API truncates at; it now comes out of the packing budget; (b) **unspaced scripts now spill** — splitting on word boundaries only meant a Chinese/Japanese/Thai document, a long URL, or a minified blob returned as **one unbounded chunk**, no error; the fallback cuts on **character** boundaries; (c) **CRLF documents split on paragraphs** — `"\r\n\r\n"` contains no `"\n\n"`, so every Windows-authored and many HTTP-fetched documents arrived as one giant paragraph; (d) **a markdown heading is a hard boundary**, so a chunk never straddles two sections and attributes section A's text to section B's heading. Proof of red: reverting `chunker.rs` in place fails 6 of the 16. The UTF-8 guard needed a second pass — built from space-separated words it passed against a deliberately byte-slicing implementation, i.e. guarded nothing; rebuilt on unspaced mixed-width text it panics on the same mutant, which is the only version worth keeping. Rust only: chunking runs server-side in the ingestion crate, and the other four languages have no ingestion pipeline to port it into. See [[Ingestion Pipeline]]. +- ✅ **Done.** The `Chunker` the connectors feed shipped with G1 (`rust/ingestion/src/chunker.rs`); what was missing was the **contract suite** — and writing it found the chunker wrong on four counts, each of which fails silently rather than loudly. `rust/ingestion/tests/chunking.rs` (17 tests) now pins: chunk count/dense indices/stable ids, overlap as a shared word-run, metadata + title + source + acl propagation on every chunk, oversized spill without word loss, degenerate configs, and **`Chunk.acl` surviving every split path** — packed, word-spill, character-spill and heading-break — because a dropped ACL here fails *open*: the pipeline turns it into the `DocAcl` the adapters filter on, so a chunk arriving with `acl: None` is stored org-public and retrievable by anyone, silently (credit: `g1conn`). Fixed against it: (a) **`max_chars` is now a hard cap on the emitted chunk** — overlap was prepended *on top of* an already-full chunk, so real chunks ran over the cap the embedding API truncates at; it now comes out of the packing budget; (b) **unspaced scripts now spill** — splitting on word boundaries only meant a Chinese/Japanese/Thai document, a long URL, or a minified blob returned as **one unbounded chunk**, no error; the fallback cuts on **character** boundaries; (c) **CRLF documents split on paragraphs** — `"\r\n\r\n"` contains no `"\n\n"`, so every Windows-authored and many HTTP-fetched documents arrived as one giant paragraph; (d) **a markdown heading is a hard boundary**, so a chunk never straddles two sections and attributes section A's text to section B's heading. Proof of red: reverting `chunker.rs` in place fails 6 of the 17; the remaining 11 are regression guards, each mutation-checked to confirm it actually fails when the behaviour it names is broken. The UTF-8 guard needed a second pass — built from space-separated words it passed against a deliberately byte-slicing implementation, i.e. guarded nothing; rebuilt on unspaced mixed-width text it panics on the same mutant, which is the only version worth keeping. Rust only: chunking runs server-side in the ingestion crate, and the other four languages have no ingestion pipeline to port it into. See [[Ingestion Pipeline]]. - ⚠️ **The green unit tests were not evidence the chunker was right.** `src/chunker.rs` shipped with a unit test per rule this plan names — count, packing, cap split, oversized spill, overlap carry, overlap clamp, metadata, id stability — all passing, all passing *through* the four defects above. Each test asserted its rule on the one input shape that never triggers the bug: the cap was checked only with overlap **off**, spill only on space-separated ASCII, paragraphs only with LF. A rule-by-rule checklist against the author's own choice of input is not a contract; the contract is the rule asserted across the inputs the connectors actually deliver. - **Remains**: **format extraction** (PDF / DOCX / rich HTML → text) ahead of the chunker. The pipeline still takes text a connector already extracted — the web connector strips HTML, and the GitHub connector *skips* binary extensions rather than reading them. Any connector over a document store (Drive, SharePoint, Confluence attachments) needs this first. (Credit: `g1conn`, via #535.) diff --git a/rust/ingestion/tests/chunking.rs b/rust/ingestion/tests/chunking.rs index dc2d2850..83f379ab 100644 --- a/rust/ingestion/tests/chunking.rs +++ b/rust/ingestion/tests/chunking.rs @@ -327,6 +327,55 @@ fn every_chunk_carries_the_documents_title_metadata_source_and_acl() { } } +#[test] +fn acl_survives_every_split_path() { + // Losing an ACL here fails *open*: the pipeline turns `Chunk.acl` into the + // `DocAcl` the adapters filter on, so a chunk that arrives with `acl: None` + // is stored org-public and retrievable by anyone — no error, no log. The + // check above covers the packed path; these are the paths where a future + // chunker change would actually drop it. + let acl = vec!["group:support".to_string(), "user:alice".to_string()]; + let cases: [(&str, String); 4] = [ + ( + "packed paragraphs", + "alpha text\n\nbeta text\n\ngamma text".to_string(), + ), + ( + "word spill", + (0..80) + .map(|i| format!("w{i}")) + .collect::>() + .join(" "), + ), + ( + "character spill (unspaced)", + "宽带上网服务的开通与故障处理流程".repeat(10), + ), + ( + "heading break", + "## A\n\nfirst body\n\n## B\n\nsecond body".to_string(), + ), + ]; + + for (label, content) in cases { + let doc = RawDocument::new("doc-acl", "test", content).with_acl(acl.clone()); + let chunks = Chunker::new(30, 8).chunk(&doc); + assert!(chunks.len() > 1, "{label}: expected a split to exercise"); + for c in &chunks { + assert_eq!( + c.acl.as_deref(), + Some(&acl[..]), + "{label}: chunk {} lost its ACL — it would store org-public", + c.index + ); + } + } + + // And the absent case stays absent rather than becoming an empty allowlist. + let open_doc = RawDocument::new("doc-open", "test", "no acl on this one"); + assert_eq!(Chunker::default().chunk(&open_doc)[0].acl, None); +} + #[test] fn explicit_metadata_wins_over_the_derived_title_and_source_keys() { let doc = RawDocument::new("doc-w", "web", "some content") From 12f627103dadada800f1a07af552ef4763c45a66 Mon Sep 17 00:00:00 2001 From: Brent Rager Date: Sat, 22 Aug 2026 22:29:54 -0400 Subject: [PATCH 4/4] =?UTF-8?q?G2:=20correct=20the=20port-scope=20claim=20?= =?UTF-8?q?=E2=80=94=20.NET=20has=20its=20own=20diverging=20chunker?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit My §G2 said the other four languages have no ingestion pipeline to port the fixes into. That is true of Go, TypeScript and Python, which never split text, but false of .NET: dotnet/server/src/Chunker.cs is a second chunker carrying all four defects, plus a fifth unique to it — C# string.Length counts UTF-16 code units where Rust counts chars(), so the two implementations chunk the same document to different boundaries for any astral-plane codepoint. Filed as pearl th-ed78a9 rather than fixed here, to keep this PR to the reference implementation. Co-Authored-By: Claude Opus 5 (1M context) --- docs/Planning/Feature Gaps.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Planning/Feature Gaps.md b/docs/Planning/Feature Gaps.md index 7f9861b3..c747be0d 100644 --- a/docs/Planning/Feature Gaps.md +++ b/docs/Planning/Feature Gaps.md @@ -52,7 +52,7 @@ Mature platforms ship 50+ connectors (confluence, jira, github, gmail, google_dr ### G2. Document processing / chunking pipeline — ✅ chunker done + contract-tested; format extraction remains Mature knowledge platforms have a tested chunking + metadata-extraction pipeline. Our knowledge store assumed pre-chunked text. - **TDD**: `tests/chunking.rs` first — feed a long doc + assert chunk count, overlap, boundary rules, metadata propagation, and that oversized items spill correctly. Then implement the chunker the connectors feed. -- ✅ **Done.** The `Chunker` the connectors feed shipped with G1 (`rust/ingestion/src/chunker.rs`); what was missing was the **contract suite** — and writing it found the chunker wrong on four counts, each of which fails silently rather than loudly. `rust/ingestion/tests/chunking.rs` (17 tests) now pins: chunk count/dense indices/stable ids, overlap as a shared word-run, metadata + title + source + acl propagation on every chunk, oversized spill without word loss, degenerate configs, and **`Chunk.acl` surviving every split path** — packed, word-spill, character-spill and heading-break — because a dropped ACL here fails *open*: the pipeline turns it into the `DocAcl` the adapters filter on, so a chunk arriving with `acl: None` is stored org-public and retrievable by anyone, silently (credit: `g1conn`). Fixed against it: (a) **`max_chars` is now a hard cap on the emitted chunk** — overlap was prepended *on top of* an already-full chunk, so real chunks ran over the cap the embedding API truncates at; it now comes out of the packing budget; (b) **unspaced scripts now spill** — splitting on word boundaries only meant a Chinese/Japanese/Thai document, a long URL, or a minified blob returned as **one unbounded chunk**, no error; the fallback cuts on **character** boundaries; (c) **CRLF documents split on paragraphs** — `"\r\n\r\n"` contains no `"\n\n"`, so every Windows-authored and many HTTP-fetched documents arrived as one giant paragraph; (d) **a markdown heading is a hard boundary**, so a chunk never straddles two sections and attributes section A's text to section B's heading. Proof of red: reverting `chunker.rs` in place fails 6 of the 17; the remaining 11 are regression guards, each mutation-checked to confirm it actually fails when the behaviour it names is broken. The UTF-8 guard needed a second pass — built from space-separated words it passed against a deliberately byte-slicing implementation, i.e. guarded nothing; rebuilt on unspaced mixed-width text it panics on the same mutant, which is the only version worth keeping. Rust only: chunking runs server-side in the ingestion crate, and the other four languages have no ingestion pipeline to port it into. See [[Ingestion Pipeline]]. +- ✅ **Done.** The `Chunker` the connectors feed shipped with G1 (`rust/ingestion/src/chunker.rs`); what was missing was the **contract suite** — and writing it found the chunker wrong on four counts, each of which fails silently rather than loudly. `rust/ingestion/tests/chunking.rs` (17 tests) now pins: chunk count/dense indices/stable ids, overlap as a shared word-run, metadata + title + source + acl propagation on every chunk, oversized spill without word loss, degenerate configs, and **`Chunk.acl` surviving every split path** — packed, word-spill, character-spill and heading-break — because a dropped ACL here fails *open*: the pipeline turns it into the `DocAcl` the adapters filter on, so a chunk arriving with `acl: None` is stored org-public and retrievable by anyone, silently (credit: `g1conn`). Fixed against it: (a) **`max_chars` is now a hard cap on the emitted chunk** — overlap was prepended *on top of* an already-full chunk, so real chunks ran over the cap the embedding API truncates at; it now comes out of the packing budget; (b) **unspaced scripts now spill** — splitting on word boundaries only meant a Chinese/Japanese/Thai document, a long URL, or a minified blob returned as **one unbounded chunk**, no error; the fallback cuts on **character** boundaries; (c) **CRLF documents split on paragraphs** — `"\r\n\r\n"` contains no `"\n\n"`, so every Windows-authored and many HTTP-fetched documents arrived as one giant paragraph; (d) **a markdown heading is a hard boundary**, so a chunk never straddles two sections and attributes section A's text to section B's heading. Proof of red: reverting `chunker.rs` in place fails 6 of the 17; the remaining 11 are regression guards, each mutation-checked to confirm it actually fails when the behaviour it names is broken. The UTF-8 guard needed a second pass — built from space-separated words it passed against a deliberately byte-slicing implementation, i.e. guarded nothing; rebuilt on unspaced mixed-width text it panics on the same mutant, which is the only version worth keeping. Rust only in this PR — but **not a non-event for the ports, as first stated.** Go, TypeScript and Python genuinely never split text (Go's `knowledge_store.go` only *reads* a stored chunk; TS and Python store "a document as a single chunk keyed by its id"), so they are read-only consumers. **.NET is not**: `dotnet/server/src/Chunker.cs` is a second chunker, a line-by-line port of the pre-fix Rust one, and it still carries all four defects — plus a fifth unique to it, because C# `string.Length` counts **UTF-16 code units** where Rust counts `chars()`, so the same document chunks to *different boundaries* in the two implementations for any astral-plane codepoint. Its tests mirror the old Rust unit tests exactly, which is why they are green through all five. Tracked as pearl `th-ed78a9`; deliberately not fixed here to keep this PR to the reference implementation. See [[Ingestion Pipeline]]. - ⚠️ **The green unit tests were not evidence the chunker was right.** `src/chunker.rs` shipped with a unit test per rule this plan names — count, packing, cap split, oversized spill, overlap carry, overlap clamp, metadata, id stability — all passing, all passing *through* the four defects above. Each test asserted its rule on the one input shape that never triggers the bug: the cap was checked only with overlap **off**, spill only on space-separated ASCII, paragraphs only with LF. A rule-by-rule checklist against the author's own choice of input is not a contract; the contract is the rule asserted across the inputs the connectors actually deliver. - **Remains**: **format extraction** (PDF / DOCX / rich HTML → text) ahead of the chunker. The pipeline still takes text a connector already extracted — the web connector strips HTML, and the GitHub connector *skips* binary extensions rather than reading them. Any connector over a document store (Drive, SharePoint, Confluence attachments) needs this first. (Credit: `g1conn`, via #535.)