forge: simplify _split_into_chunks control flow, remove redundant break in _hard_split_long

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 exits naturally when end>=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
+"""

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 exits naturally when end>=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)

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"

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())