forge: refactor edge-sweep probe to DRY check_case helper
This commit is contained in:
1 parent
f85a621247
commit
6e22692f94
2 files changed
+178
No files matched your search
@@ -0,0 +1,73 @@
|
||||
# Edge-sweep DRY refactor verification
|
||||
|
||||
## Work
|
||||
Refactored `_edge_sweep.py` — a 100-line probe that sweeps 600 chunker
|
||||
edge cases across four scenarios (multi-space tabs, no-internal-space words,
|
||||
degenerate overlap ot=max-1, punctuation runs).
|
||||
|
||||
## Problem (code smell)
|
||||
Each of the four cases re-implemented the identical verification sequence
|
||||
inline:
|
||||
|
||||
```
|
||||
cs = chunker.chunk_text(text, max_tokens=mt, overlap_tokens=ot)
|
||||
r = chunker.reconstruct(cs, ot)
|
||||
if r != text.strip(): ...roundtrip fail...
|
||||
for c in cs: body = c.partition(S)[2] ... bounds check ...
|
||||
```
|
||||
|
||||
That is ~15 lines of logic duplicated 4x with no abstraction (0 `def`s).
|
||||
Cases C and D additionally omitted the bounds check entirely — an oversight.
|
||||
|
||||
## Refactor
|
||||
Extracted the shared logic into a single `check_case(text, mt, ot, label)`
|
||||
helper. Each case is now just a data generator calling `check_case`.
|
||||
Bounds checking is applied uniformly to every case (including C and D).
|
||||
|
||||
## Evidence (measured)
|
||||
|
||||
Both runs executed against the live chunker at
|
||||
`/root/.hermes/wizard-outputs/forge/chunker.py` with `random.Random(99)`
|
||||
(fixed seed => deterministic, comparable output).
|
||||
|
||||
```
|
||||
$ python3 _edge_sweep.py (original, 100 lines, 0 defs)
|
||||
total=600 roundtrip_fails=0 bounds_fails=0
|
||||
RESULT: ALL PASS
|
||||
EXIT=0
|
||||
|
||||
$ python3 _edge_sweep_refactored.py (refactored, 105 lines, 1 def)
|
||||
total=600 roundtrip_fails=0 bounds_fails=0
|
||||
RESULT: ALL PASS
|
||||
EXIT=0
|
||||
```
|
||||
|
||||
Byte-for-byte output equivalence:
|
||||
```
|
||||
$ diff /tmp/original_out.txt /tmp/refactored_out.txt
|
||||
OUTPUT_IDENTICAL
|
||||
```
|
||||
|
||||
py_compile:
|
||||
```
|
||||
$ python3 -m py_compile _edge_sweep_refactored.py
|
||||
PY_COMPILE_OK
|
||||
```
|
||||
|
||||
## Metrics
|
||||
|
||||
| metric | original | refactored |
|
||||
|-------------------|----------|------------|
|
||||
| lines | 100 | 105 |
|
||||
| function defs | 0 | 1 |
|
||||
| duplicated verify | 4x | 0 (1 helper) |
|
||||
| cases w/ bounds | 2/4 | 4/4 |
|
||||
| total cases | 600 | 600 |
|
||||
| roundtrip_fails | 0 | 0 |
|
||||
| bounds_fails | 0 | 0 |
|
||||
| exit code | 0 | 0 |
|
||||
|
||||
## Verdict
|
||||
Refactoring is behavior-preserving and measurably equivalent (identical
|
||||
output, exit 0). The duplicated verification logic is eliminated behind one
|
||||
helper, and bounds coverage was extended to all four cases with no regressions.
|
||||
@@ -0,0 +1,105 @@
|
||||
"""Broad edge-case sweep: token-bound + lossiness for chunker fix.
|
||||
|
||||
Refactored: Cases A-D previously each re-implemented the identical
|
||||
``chunk_text -> reconstruct -> compare -> per-chunk bounds-check`` sequence
|
||||
inline, duplicating ~15 lines four times. That logic now lives in
|
||||
:func:`check_case`, so each case is reduced to a data generator that feeds the
|
||||
shared verifier. Behaviour and output format are unchanged (and bounds
|
||||
checking is applied uniformly to every case, including C and D where it was
|
||||
previously omitted as an oversight).
|
||||
"""
|
||||
import random
|
||||
import sys
|
||||
|
||||
sys.path.insert(0, "/root/.hermes/wizard-outputs/forge")
|
||||
import chunker
|
||||
|
||||
S = chunker._OVERLAP_SENTINEL
|
||||
|
||||
rng = random.Random(99)
|
||||
WORDS = ["alpha", "bravo", "charlie", "delta", "echo", "foxtrot", "golf", "hotel"]
|
||||
|
||||
# Counters shared across all cases.
|
||||
total = 0
|
||||
roundtrip_fails = 0
|
||||
bounds_fails = 0
|
||||
|
||||
|
||||
def check_case(text: str, mt: int, ot: int, label: str) -> None:
|
||||
"""Verify a single chunker input: lossless round-trip + per-chunk token bound.
|
||||
|
||||
Mirrors the inline checks that previously existed in every case block:
|
||||
reconstruct must reproduce ``text.strip()`` exactly, and every chunk body
|
||||
(the chunk with its sentinel-delimited context head stripped) must fit within
|
||||
``max_tokens``. Updates the module-level counters and prints the first few
|
||||
failures for diagnosis.
|
||||
"""
|
||||
global roundtrip_fails, bounds_fails
|
||||
|
||||
expected = text.strip()
|
||||
cs = chunker.chunk_text(text, max_tokens=mt, overlap_tokens=ot)
|
||||
r = chunker.reconstruct(cs, ot)
|
||||
|
||||
if r != expected:
|
||||
roundtrip_fails += 1
|
||||
if roundtrip_fails <= 3:
|
||||
print(f"RT-FAIL {label} mt={mt} ot={ot}")
|
||||
print(f" exp_len={len(expected)} got_len={len(r)}")
|
||||
|
||||
for c in cs:
|
||||
body = c.partition(S)[2] if S in c else c
|
||||
est = chunker.estimate_tokens(body)
|
||||
if est > mt and body.strip():
|
||||
bounds_fails += 1
|
||||
if bounds_fails <= 3:
|
||||
print(f"BOUNDS-FAIL {label} mt={mt} ot={ot} est={est} body={body[:40]!r}")
|
||||
|
||||
|
||||
# --- Case A: sentences with multi-space and tab separators -----------------
|
||||
for trial in range(300):
|
||||
n = rng.randint(2, 30)
|
||||
sents = []
|
||||
for _ in range(n):
|
||||
w = rng.randint(2, 40)
|
||||
s = " ".join(rng.choice(WORDS) for _ in range(w))
|
||||
sep = " " * rng.randint(1, 5) + "\t" * rng.randint(0, 2)
|
||||
sents.append(s + "." + sep)
|
||||
text = "".join(sents)
|
||||
mt = rng.choice([4, 6, 8, 12, 16, 24, 32, 48])
|
||||
ot = rng.randint(0, mt - 1)
|
||||
total += 1
|
||||
check_case(text, mt, ot, f"A trial={trial}")
|
||||
|
||||
# --- Case B: single long no-internal-space word + trailing whitespace -------
|
||||
for trial in range(100):
|
||||
text = "x" * rng.randint(20, 500) + " " * rng.randint(0, 10)
|
||||
mt = rng.choice([4, 8, 12, 16, 24, 32])
|
||||
ot = rng.randint(0, mt - 1)
|
||||
total += 1
|
||||
check_case(text, mt, ot, f"B trial={trial}")
|
||||
|
||||
# --- Case C: degenerate overlap (ot = mt - 1) -------------------------------
|
||||
for trial in range(100):
|
||||
n = rng.randint(3, 20)
|
||||
sents = []
|
||||
for _ in range(n):
|
||||
w = rng.randint(2, 15)
|
||||
s = " ".join(rng.choice(WORDS) for _ in range(w)) + "."
|
||||
sents.append(s)
|
||||
text = " ".join(sents)
|
||||
mt = rng.choice([4, 8, 12, 16, 24])
|
||||
ot = mt - 1
|
||||
total += 1
|
||||
check_case(text, mt, ot, f"C trial={trial}")
|
||||
|
||||
# --- Case D: no-after-space sentences (punctuation runs) --------------------
|
||||
for trial in range(100):
|
||||
text = "Hello.World!How.Are.You?I.Am.Fine. " * rng.randint(1, 20)
|
||||
mt = rng.choice([6, 12, 16, 24, 32])
|
||||
ot = rng.randint(0, mt - 1)
|
||||
total += 1
|
||||
check_case(text, mt, ot, f"D trial={trial}")
|
||||
|
||||
print(f"\ntotal={total} roundtrip_fails={roundtrip_fails} bounds_fails={bounds_fails}")
|
||||
print("RESULT:", "ALL PASS" if roundtrip_fails == 0 and bounds_fails == 0 else "HAS FAILURES")
|
||||
sys.exit(0 if roundtrip_fails == 0 and bounds_fails == 0 else 1)
|
||||
Reference in new issue
Block a user