diff --git a/wizard-work/forge/CENSUS_ROW_DELTA_DRY_REFACTOR_VERIFICATION.md b/wizard-work/forge/CENSUS_ROW_DELTA_DRY_REFACTOR_VERIFICATION.md new file mode 100644 index 0000000..1678e77 --- /dev/null +++ b/wizard-work/forge/CENSUS_ROW_DELTA_DRY_REFACTOR_VERIFICATION.md @@ -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: + before.json after.json --keys --verbose --json + Results: + rc: 0 == 0 (identical) + generated JSON payload sha256: f760f01be0b9cb013dc6ae49312b631d6dccc6a0ce932ffbe17be8093e41f3fc + == f760f01be0b9cb013dc6ae49312b631d6dccc6a0ce932ffbe17be8093e41f3fc + => JSON payload BYTE-IDENTICAL. + stdout: identical except line 120, the `wrote ` 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.