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..c747be0d 100644 --- a/docs/Planning/Feature Gaps.md +++ b/docs/Planning/Feature Gaps.md @@ -49,9 +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 -Mature knowledge platforms have a tested chunking + metadata-extraction pipeline. Our knowledge store assumes pre-chunked text. +### 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 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.) ### 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..83f379ab --- /dev/null +++ b/rust/ingestion/tests/chunking.rs @@ -0,0 +1,428 @@ +//! 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 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") + .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 + ); + } + } +}