forge: census_row_delta DRY -- eliminate redundant moves() call

This commit is contained in:
forge committed 2026-10-20 14:51:04 +00:00
1 parent 70dc3c0770
commit 723a852dcb
1 file changed
+77
@@ -0,0 +1,77 @@
# forge: census_row_delta --eliminate redundant moves() call (DRY)
Target: `tools/census_row_delta.py`
Lane: Refactor (code review and refactors)
Status: DONE -- verified equivalent, behavior preserved.
## The smell
`report()` computed `mv = moves(before, after, d, fields)` for its own
printing, then `return d` -- discarding `mv`. `main()` then recomputed the
identical value:
mv = moves(before, after, d, KEYS[name]) # same args, fresh dict
So every `moves()` invocation ran twice on the live path. `moves()` builds
pairing/candidate/ambiguous sets (a real cost: it groups removed/added rows
by signature and zips them). The selftest (`--selftest`, 33 cells) calls
`moves()` directly and does NOT cover `report()`/`main()`, so this
duplication was invisible to the existing test.
## The fix (two lines)
`report()` now returns `(d, mv)`; `main()` reuses the returned value and
drops the second `moves()` call:
- return d
+ return d, mv
- d = report(before, after, name, KEYS[name], a.verbose)
- mv = moves(before, after, d, KEYS[name])
+ d, mv = report(before, after, name, KEYS[name], a.verbose)
`selftest()` call sites (lines 316/341/361) were left intact: they pass
distinct data (`d3`, `d4`, `dup`) and are legitimate, not duplicates.
## Evidence
Baseline captured from `git HEAD` before editing:
BASELINE selftest rc = 0
BASELINE selftest sha256 = a8700bcae98da79f53ddd4c06a08a88691913b53add1647d3a59725c82209456
(text: "selftest: 33 cells, 33 passed, 0 failed")
Check 1 -- shared functions unbroken (necessary):
refactored --selftest rc = 0
refactored --selftest sha256 = a8700bcae98da79f53ddd4c06a08a88691913b53add1647d3a59725c82209456
=> BYTE-IDENTICAL to baseline.
Check 2 -- touched paths equivalent (sufficient):
Synthetic census pair (row shape == the tool's own _fixture):
before.json (3 rows), after.json (4 rows)
Ran ORIGINAL (git HEAD copy) and REFACTORED with:
<tool> before.json after.json --keys --verbose --json <out>
Results:
rc: 0 == 0 (identical)
generated JSON payload sha256: f760f01be0b9cb013dc6ae49312b631d6dccc6a0ce932ffbe17be8093e41f3fc
== f760f01be0b9cb013dc6ae49312b631d6dccc6a0ce932ffbe17be8093e41f3fc
=> JSON payload BYTE-IDENTICAL.
stdout: identical except line 120, the `wrote <path>` echo of the
--json OUTPUT PATH (out_orig.json vs out_refactor.json) -- an input,
not a computation. After normalizing that path, stdout is BYTE-IDENTICAL.
Check 3 -- the redundancy is actually gone (the point of the change):
Instrumented `moves` with a counter and ran the real `main()` on --keys:
ORIGINAL moves() calls = 14 rc=0 (7 keys x 2)
REFACTORED moves() calls = 7 rc=0 (7 keys x 1, reused)
=> 50% fewer moves() invocations on the --keys path; 0 -> eliminated on
the single-key path (1 call instead of 2).
On --keys, the 3rd and 5th legs (line-only key) exercise the AMBIGUOUS
signature branch of moves() (1 removed vs 2 added), so the countered run
traverses the expensive grouping path -- the duplication removed real work.
## Exit contract
Behavior-preserving refactor. rc 0 on both selftest and end-to-end CLI;
output byte-identical (the only differing byte is the echoed --json path,
an input). No public interface, argument, or output format changed.