diff --git a/wizard-work/forge/REFACTOR_SPLIT_INTO_CHUNKS_VERIFICATION.md b/wizard-work/forge/REFACTOR_SPLIT_INTO_CHUNKS_VERIFICATION.md new file mode 100644 index 0000000..7490bfe --- /dev/null +++ b/wizard-work/forge/REFACTOR_SPLIT_INTO_CHUNKS_VERIFICATION.md @@ -0,0 +1,96 @@ +"""SIMPLIFY_SPLIT_INTO_CHUNKS_VERIFICATION.md -- forge refactor verification. + +Target: /root/.hermes/wizard-outputs/forge/chunker.py +Lane: Refactor (code review + refactors) +Date: 2026-10-24 + +PURPOSE + Two behavior-preserving refactors to chunker.py, verified against the full + existing test corpus. + +CHANGE 1 -- _hard_split_long: remove redundant break + File: chunker.py, function _hard_split_long + Before: + ... + if end >= n: + break + start = end + After: + ... + start = end # while start=n; no break needed + + Rationale: the enclosing `while start < n:` loop checks the condition at the + top of every iteration. After `start = end`, if `end >= n` then `start >= n` + and the loop exits on the next condition check. The explicit `break` was + dead code (a pure micro-optimization saving one assignment that never + mattered for correctness). + +CHANGE 2 -- _split_into_chunks: simplify control flow (DRY) + File: chunker.py, function _split_into_chunks + Before (13 lines of nested if/else + candidate ternary): + candidate = cur + piece if cur else piece + if estimate_tokens(candidate) > max_tokens: + if cur: + chunks.append(cur) + if estimate_tokens(piece) > max_tokens: + chunks.extend(_hard_split_long(piece, max_chars)) + cur = "" + else: + cur = piece + else: + cur = candidate + + After (9 lines, linear flow): + if cur and estimate_tokens(cur + piece) > max_tokens: + chunks.append(cur) + cur = "" + if estimate_tokens(piece) > max_tokens: + chunks.extend(_hard_split_long(piece, max_chars)) + else: + cur += piece + + Rationale: the original used a `candidate` ternary to handle the empty-cur + case, then nested a flush-inside-flush (flush cur, then decide hard-split vs + reset). The refactored version linearizes this into two independent checks. + + SAFETY PROOF (monotonicity): + estimate_tokens(s) = max(1, ceil(len(s) / 4)) for non-empty s, and 0 for + empty s. This is MONOTONIC in len(s): len(a) <= len(b) => et(a) <= et(b). + (Verified empirically for all len pairs 0..399.) + + Case A -- cur is empty: + Original: candidate = piece (ternary picks `piece` when cur is falsy). + If et(piece) > max_tokens: flush guard `if cur:` is False + (skip), then et(piece) > max_tokens => hard-split, cur = "". + Else: cur = candidate = piece. + Refactored: `if cur and ...` is False (cur falsy), skip. + If et(piece) > max_tokens: hard-split, cur stays "". + Else: cur += piece = "" + piece = piece. + => IDENTICAL. + + Case B -- cur non-empty, et(cur + piece) > max_tokens (overflow): + Original: candidate = cur + piece, et(candidate) > max, flush cur, + then et(piece) check: hard-split+cur="" OR cur=piece. + Refactored: flush cur, cur = "". Then et(piece) check: same branches. + hard-split => cur stays "". else => cur += piece = piece. + => IDENTICAL. + + Case C -- cur non-empty, et(cur + piece) <= max_tokens (fits): + Original: candidate fits, so else-branch: cur = candidate = cur + piece. + Refactored: `if cur and et(cur+piece) > max` is False (fits), skip. + et(piece) <= max (by monotonicity: et(cur+piece) >= et(piece) + since len(cur+piece) >= len(piece), so et(piece) <= max). + => else-branch: cur += piece. + => IDENTICAL. (The monotonicity guarantee is what makes this case safe: + when cur+piece fits, piece alone MUST also fit, so the second `if` + correctly falls through to `cur += piece`.) + +VERIFICATION (all executed, real exit codes): + 1. pytest test_chunker_refactor.py -v -> 13 passed + 2. _test_roundtrip_refactored.py -> 34 passed, 0 failed + 3. _fuzz_probe_refactored.py -> 383 cases, ALL PASS + (0 roundtrip_fails, 0 bounds_failures) + 4. verify_chunker_refactor.py -> 66/66 lossless, exit 0 + 5. _probe_forge.py (forge profile) -> equal: True, nchunks: 12 + 6. _edge_sweep.py (forge profile) -> 600 cases, 0 fails, 0 bounds_fails +""" diff --git a/wizard-work/forge/chunker.py b/wizard-work/forge/chunker.py index 16b41a2..b9fd850 100644 --- a/wizard-work/forge/chunker.py +++ b/wizard-work/forge/chunker.py @@ -159,9 +159,7 @@ def _hard_split_long(text: str, max_chars: int) -> list[str]: pieces.append(piece) # if pieces is empty this is leading whitespace; reconstruct() contracts # to text.strip(), so it is intentionally not retained here. - if end >= n: - break - start = end + start = end # while start=n; no break needed return pieces @@ -200,17 +198,19 @@ def _split_into_chunks( cur = "" for sent, sep in segments: piece = sent + sep - candidate = cur + piece if cur else piece - if estimate_tokens(candidate) > max_tokens: - if cur: - chunks.append(cur) # cur already ends with its segment's sep - if estimate_tokens(piece) > max_tokens: - chunks.extend(_hard_split_long(piece, max_chars)) - cur = "" - else: - cur = piece + # Flush cur exactly when appending *piece* would overflow max_tokens. + # Safe because estimate_tokens is monotonic in string length: when + # cur is empty the guard (cur and ...) is skipped and the single + # estimate_tokens(piece) check below decides. When cur is non-empty + # and cur+piece fits, piece alone must also fit (monotonicity), so the + # else-branch cur += piece matches the original cur = candidate. + if cur and estimate_tokens(cur + piece) > max_tokens: + chunks.append(cur) + cur = "" + if estimate_tokens(piece) > max_tokens: + chunks.extend(_hard_split_long(piece, max_chars)) else: - cur = candidate + cur += piece if cur: chunks.append(cur) return _apply_overlap(chunks, overlap_chars) diff --git a/wizard-work/forge/test_chunker_refactor.py b/wizard-work/forge/test_chunker_refactor.py new file mode 100644 index 0000000..3c79554 --- /dev/null +++ b/wizard-work/forge/test_chunker_refactor.py @@ -0,0 +1,127 @@ +"""Regression + round-trip tests for the refactored chunker (forge lane build). + +These mirror the upstream suite in wizard-outputs/null but pin the corrected +lossless-round-trip contract that the original ``reconstruct`` violated on +repetitive / sentence-grouped input. +""" +import random + +import chunker +from chunker import _OVERLAP_SENTINEL + + +# --- argument validation (unchanged contract) --- +def test_invalid_args(): + import pytest + with pytest.raises(ValueError): + chunker.chunk_text("x", max_tokens=0) + with pytest.raises(ValueError): + chunker.chunk_text("x", max_tokens=10, overlap_tokens=10) + with pytest.raises(ValueError): + chunker.chunk_text("x", max_tokens=10, overlap_tokens=-1) + + +# --- chunk-size limits (tokens stay bounded by max + overlap allowance) --- +def test_long_splits_bounded(): + text = " ".join(f"Sentence number {i}." for i in range(200)) + out = chunker.chunk_text(text, max_tokens=64, overlap_tokens=8) + assert len(out) > 1 + for c in out: + assert chunker._estimate_tokens(c) <= 64 + 8 + 2 + + +def test_long_sentence_hard_split_bounded(): + out = chunker.chunk_text("a" * 5000, max_tokens=32, overlap_tokens=4) + assert len(out) >= 2 + for c in out: + assert len(c) <= 32 * 4 + 4 * 4 + 5 + + +def test_hard_split_no_empty_chunks(): + out = chunker.chunk_text("z" * 200, max_tokens=8, overlap_tokens=2) + assert all(c.strip() for c in out) + assert len(out) >= 2 + + +# --- round-trip: the property the original bug broke --- +def test_roundtrip_varied_sentences(): + base = ("The quick brown fox jumps over the lazy dog. Pack my box with five " + "dozen liquor jugs. How vexingly quick daft zebras jump. Variant " + "sentence here. ") + text = (base + "Another distinct sentence. ") * 20 + cs = chunker.chunk_text(text, max_tokens=64, overlap_tokens=8) + assert chunker.reconstruct(cs, 8) == text.strip() + + +def test_roundtrip_hard_split_single_char(): + text = "a" * 5000 + cs = chunker.chunk_text(text, max_tokens=32, overlap_tokens=4) + assert chunker.reconstruct(cs, 4) == text + + +def test_roundtrip_empty_and_single(): + assert chunker.reconstruct([], 4) == "" + cs = chunker.chunk_text("Hello world. This is a test.", max_tokens=100, overlap_tokens=4) + assert chunker.reconstruct(cs, 4) == "Hello world. This is a test." + + +def test_roundtrip_zero_overlap(): + text = "One. Two. Three. Four. Five. Six. Seven. Eight. " * 15 + cs = chunker.chunk_text(text, max_tokens=16, overlap_tokens=0) + assert chunker.reconstruct(cs, 0) == text.strip() + + +def test_roundtrip_repetitive_sentences_exact(): + """Repeated identical sentence groups -- the adversarial case.""" + text = ("Sentence one. Sentence two. Sentence three. ") * 40 + cs = chunker.chunk_text(text, max_tokens=24, overlap_tokens=4) + assert chunker.reconstruct(cs, 4) == text.strip() + + +# --- reconstruct standalone contract --- +def test_reconstruct_passthrough(): + r = chunker.reconstruct(["alpha beta", "gamma delta"], 0) + assert "alpha beta" in r and "gamma delta" in r + + +def test_reconstruct_empty(): + assert chunker.reconstruct([], 32) == "" + + +# --- fuzz: lossless round-trip over many random inputs --- +def test_fuzz_roundtrip_fifty_hundred(): + rng = random.Random(2026) + words = ["alpha", "bravo", "charlie", "delta", "echo", "foxtrot"] + fails = 0 + for _ in range(500): + n = rng.randint(3, 60) + text = " ".join(rng.choice(words) for _ in range(n)) + # sprinkle sentence punctuation sometimes + if rng.random() < 0.5: + text = text + ". " + " ".join(rng.choice(words) for _ in range(rng.randint(2, 12))) + "." + mt = rng.choice([12, 16, 32, 64]) + ot = rng.choice([0, 2, 4, 8]) + if ot >= mt: + ot = max(0, mt - 1) + cs = chunker.chunk_text(text, max_tokens=mt, overlap_tokens=ot) + r = chunker.reconstruct(cs, ot) + if r != text.strip(): + fails += 1 + assert fails == 0, f"{fails}/500 fuzz trials failed lossless round-trip" + + +def test_fuzz_hard_split_only(): + """Pure repeated-token hard-split path under many sizes.""" + rng = random.Random(7) + fails = 0 + for _ in range(500): + text = ("x" * rng.randint(50, 2000)) + mt = rng.choice([8, 16, 32, 40]) + ot = rng.choice([2, 4, 8]) + if ot >= mt: + ot = max(1, mt - 1) + cs = chunker.chunk_text(text, max_tokens=mt, overlap_tokens=ot) + r = chunker.reconstruct(cs, ot) + if r != text: + fails += 1 + assert fails == 0, f"{fails}/500 hard-split fuzz trials failed" diff --git a/wizard-work/forge/verify_chunker_refactor.py b/wizard-work/forge/verify_chunker_refactor.py new file mode 100644 index 0000000..c3c7b56 --- /dev/null +++ b/wizard-work/forge/verify_chunker_refactor.py @@ -0,0 +1,151 @@ +#!/usr/bin/env python3 +"""Independent verification of the chunker refactor (forge-lane Refactor build). + +The module docstring of ``chunker.py`` claims two things the refactor achieved: + + 1. The *fixed* ``reconstruct`` is **lossless** on all inputs (including the + highly-repetitive / sentence-grouped cases the original broke on). + 2. The **pre-refactor** ``reconstruct`` stripped the carried-overlap tail from + the previous chunk's content *unconditionally*, which is only correct for + genuine hard-split overlaps and is **lossy** for sentence-grouped chunks + (where the carried tail is pure context, not duplicated into the chunk's + own content). + +This harness proves BOTH claims with real execution against the genuine +``chunker`` module under test: + + * ``reconstruct`` (fixed) -- imported from the real module. + * ``reconstruct_naive`` -- a faithful reproduction of the pre-fix algorithm + (identical to the fixed code except the ``rest.startswith(head)`` + structural discriminator is dropped, exactly as the docstring describes the + original "strip unconditionally" behaviour). + +Exit code: + 0 fixed is lossless on every corpus case AND naive is lossy on >=1 case + 1 fixed is lossy on some case (the refactor is broken) + 2 naive is lossless everywhere (cannot demonstrate the fix was necessary) + +Stdlib only; no external data files. +""" +from __future__ import annotations + +import pathlib +import random +import sys + +sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent)) + +from chunker import ( # the module under test + _OVERLAP_SENTINEL, + chunk_text, + reconstruct, +) + + +def reconstruct_naive(chunks: list, overlap_tokens: int = 32) -> str: + """Pre-refactor ``reconstruct``: strip the carried tail unconditionally. + + This is the fixed algorithm verbatim EXCEPT the ``rest.startswith(head)`` + branch discriminator is removed -- i.e. whenever the carried ``head`` is a + suffix of the accumulator, it is deleted regardless of whether ``rest`` + itself begins with ``head``. That is precisely the lossy behaviour + described in chunker.py's module docstring. + """ + if not chunks: + return "" + parts: list[str] = [] + for i, c in enumerate(chunks): + if i == 0: + # First chunk never carries an overlap; peel a stray sentinel off + # defensively in case callers pass pre-tagged content. + parts.append(c.partition(_OVERLAP_SENTINEL)[2] if _OVERLAP_SENTINEL in c else c) + continue + prev = parts[-1] + if _OVERLAP_SENTINEL in c: + head, _, rest = c.partition(_OVERLAP_SENTINEL) + head_len = len(head) + # PRE-FIX: no `rest.startswith(head)` check -> always strip. + if head_len and head_len <= len(prev) and prev.endswith(head): + parts[-1] = prev[: len(prev) - head_len] + else: + parts.append(" " + rest if rest else "") + continue + parts.append(rest) + else: + parts.append(" " + c) + return "".join(parts).strip() + + +def corpus() -> list[tuple[str, str, int, int]]: + """(label, text, max_tokens, overlap_tokens) cases to round-trip.""" + items = [ + # Adversarial repetitive sentences (the exact class the docstring cites). + ("repetitive", ("Sentence one. Sentence two. Sentence three. ") * 40, 24, 4), + # Varied distinct sentences. + ("varied", ("The quick brown fox jumps over the lazy dog. Pack my box with five " + "dozen liquor jugs. How vexingly quick daft zebras jump. ") * 20, 64, 8), + # Single-character hard-split path (genuine overlap; both must be lossless). + ("monorep", "a" * 5000, 32, 4), + # Zero overlap (no sentinels inserted; both must be lossless). + ("zero-overlap", "One. Two. Three. Four. Five. Six. Seven. Eight. " * 15, 16, 0), + # Single chunk. + ("single", "Hello world. This is a test. A test of chunker. ", 100, 4), + # Tiny max_tokens to force many sentence-grouped chunks. + ("tiny", ("The quick brown fox. " * 40), 12, 4), + ] + rng = random.Random(2026) + words = ["alpha", "bravo", "charlie", "delta", "echo", "foxtrot", "golf", "hotel"] + for k in range(60): + n = rng.randint(3, 60) + t = " ".join(rng.choice(words) for _ in range(n)) + if rng.random() < 0.5: + t = t + ". " + " ".join(rng.choice(words) for _ in range(rng.randint(2, 12))) + "." + mt = rng.choice([12, 16, 32, 64]) + ot = rng.choice([0, 2, 4, 8]) + if ot >= mt: + ot = max(0, mt - 1) + items.append((f"fuzz-{k}", t, mt, ot)) + return items + + +def main() -> int: + cases = corpus() + n = len(cases) + fixed_losses = 0 + naive_losses = 0 + print("chunker refactor verification (forge lane)") + print("=" * 60) + for name, text, mt, ot in cases: + chunks = chunk_text(text, max_tokens=mt, overlap_tokens=ot) + expect = text.strip() + r_fixed = reconstruct(chunks, ot) + r_naive = reconstruct_naive(chunks, ot) + fixed_ok = r_fixed == expect + naive_ok = r_naive == expect + fixed_losses += 0 if fixed_ok else 1 + naive_losses += 0 if naive_ok else 1 + status = "OK " if (fixed_ok and not naive_ok) else ( + "LOSSY-BOTH" if (not fixed_ok and not naive_ok) else "lossless-both" + ) + print(f" [{status:10s}] {name:10s} mt={mt:<3} ot={ot:<2} " + f"len={len(text):>5} fixed={'ok' if fixed_ok else f'LOSS {len(expect)-len(r_fixed)}'} " + f"naive={'ok' if naive_ok else f'LOSS {len(expect)-len(r_naive)}'}") + + print("-" * 60) + print(f"corpus size : {n}") + print(f"fixed reconstruct : {n - fixed_losses}/{n} lossless") + print(f"naive reconstruct : {n - naive_losses}/{n} lossless (pre-refactor algo)") + + if fixed_losses != 0: + print("FAIL: the refactored reconstruct is lossy -- refactor is broken.") + return 1 + if naive_losses == 0: + print("WARN: naive algorithm is lossless everywhere; fix's necessity not shown.") + return 2 + print("PASS: fixed=lossless on all cases AND naive=lossy (>=1 case) => " + "fix is necessary and correct.") + return 0 + + +if __name__ == "__main__": + sys.exit(main())