diff --git a/docs/MATCHING_COOKBOOK.md b/docs/MATCHING_COOKBOOK.md index 2dcc12f..4aada27 100644 --- a/docs/MATCHING_COOKBOOK.md +++ b/docs/MATCHING_COOKBOOK.md @@ -3146,3 +3146,56 @@ to **stderr** so "which ledger did that read?" is always answerable while stdout > Derive it, print it, and make the previous session's data unreachable rather than merely older. > **And when you read a coordination artefact, ask what a wrong answer looks like in each direction** — > a false FREE and a false TAKEN are not symmetric, and only one of them corrupts work. + +### 190. A CHARTER is a file every worker copies — so a FORMAT written into one recreates finding 179 exactly (coordinator, Phase 12 cycle 1) + +Finding 179 says: *a rule every worker must follow belongs in a TRACKED tool, not in a file each +worker copies, because a copied script cannot be fixed for the people who already copied it.* Phase 11 +earned it from a worker's `free.sh`. + +**Phase 12 earned its mirror image from the coordinator's own charter.** I specified the claims staging +format as + +``` +rangesourcesource_md5differing_bytesresultoptions <- WRONG +``` + +with the range written `0xSTART..0xEND`. `tools/sf3_merge` reads + +``` +startendsource[overrides] <- RIGHT +``` + +two separate hex columns and no md5 column. **The phase's first merge was rejected** — +`expected three or four fields` — with all four workers already staging files in my format. + +**A charter IS a file each worker copies.** Writing a format into prose rather than deriving it from +the enforcing tool reproduces 179 one level up: last time the unfixable copied artefact was a worker's +script, this time it was the coordinator's charter. + +**And the tool had already learned this lesson.** `validate_overrides` exists because a Phase 11 worker +put `md5=...` in the 4th column, and its error message says in terms: *"Per-claim metadata such as a +source md5 belongs in report.tsv, not in the registry row."* **The convention was documented inside the +tool, I did not read it, and I then wrote prose that contradicted it** — including a `report.tsv`-shaped +evidence file that I renamed to `evidence.tsv`, and the established name was in the tool's own error +string and in every Phase 8/9/11 worker's staging directory. + +> **Standing rule: before writing a staging format, an interface, or an exit-code contract into a +> charter, READ THE TOOL THAT ENFORCES IT. "The tool is the contract" is not advice for workers only.** +> Workers compute addresses instead of eyeballing them; the coordinator must derive formats instead of +> inventing them. The charter even said *"if a staging format is ever ambiguous, read the tool"* — while +> the charter itself was the ambiguity. + +**What worked, and it is worth keeping:** the error was caught by **fail-fast validation**, in under a +second, at the merge — not by a confusing failure at the gate hours later. Phase 11's override-key guard +was added for exactly this reason and it paid off again here. A staging format that a tool can check is +worth more than a staging format that is documented well. + +**The durable fix is a command, not corrected prose.** `sf3_merge check-claims --claims F [--regions R]` +validates the format, flags a duplicate start, flags a missing source file and flags an already-registered +row, so a worker answers *"is my staging mergeable?"* itself before reporting. Eleven tests pin it, +including one that asserts **the charter's own wrong format is rejected** — so the error message stays +honest for the next coordinator, who will also write prose. + +**Cost, honestly:** one rejected merge, four correction messages, and roughly ten minutes. The phase's +first successful merge followed immediately. The defect was cheap because the tool refuses to guess. diff --git a/phase-ends/logs/Phase12.md b/phase-ends/logs/Phase12.md index 1952f47..0c2bb51 100644 --- a/phase-ends/logs/Phase12.md +++ b/phase-ends/logs/Phase12.md @@ -282,3 +282,92 @@ status. `result=DIFF` is the authoritative field and the byte arithmetic above i check. This is workflow §6's rule ("use the gate's **exit code** as the gate") learned by breaking it, in the same phase that quotes it. Standing note for every future gate call: capture the exit code directly, never through a pipeline. + +--- + +## Cycle 1 + +### Merge 1 — worker C's two ADJACENT-TWIN negatives -> **604 bodies / 613 regions** + +Both bodies came from the **negatives index**, not the worklist, and both came from the *named +untried lever* handed to worker C: `sf3_family` driven over the 101 negatives against the 611 +registered regions. I had measured `candidates=0` for the fresh band and told C that said nothing +about its pool. It did not. + +| row | size | twin | outcome | +|---|---|---|---| +| `0x800518BC` | 88 B | `0x80051864` | **MATCHED** | +| `0x800B34A4` | 88 B | `0x800B34FC` | **MATCHED** | +| `0x8010804C` | 16 B | `0x80085B80` | blocked class (gp-thunk) — correctly not attempted | + +C's full score distribution over the 101: **3 rows >= 1.000** (not 0), 4 >= 0.99, 15 >= 0.95. +Transfer rate **2/2** on the unblocked score-1.000 rows, against finding 166's 7/7. + +I re-verified both from fresh `--work` dirs (`differing_bytes=0`, `result=MATCH`, exit 0), merged to a +candidate, gated whole-binary, promoted only on `result=MATCH`: + +``` +sf3_merge apply exit 0 added_regions=2 +sf3_match gate exit 0 c_regions=613 differing_bytes=0 result=MATCH + sha1 e173426c157384ebf1b6caf8c6fea18a85a14af9 (unchanged) +make check exit 0 +extents-verify regions=613 disagreements=0 result=AGREE +registry audit 613 rows, ordered, non-overlapping, 604 distinct sources, 0 missing +``` + +**WORKER C'S FINDING IS WORTH MORE THAN THE TWO BODIES, and it is Goal-B-grade: both rows' +recorded class strings were WRONG ABOUT THEIR OWN MECHANISM.** + +- `0x800518BC` was classed `return-merge/sltiu`, with a note that three spellings all came out 80 B. + **21 of 22 raw words are IDENTICAL to matched `0x80051864`.** The only difference is the forward `j` + to the shared return, and its target differs only because `j` encodes an ABSOLUTE address and the two + bases are 0x58 apart. A literal twin — no re-spelling was ever needed. +- `0x800B34A4` was classed `alloc-tiebreak` (register pair a0/a1 vs a0/v1). **20 of 22 raw words are + identical to matched `0x800B34FC`**; both differences are the OFFSET CONSTANT (`li a0,3` vs `li a0,20`). + The register pair the note asked for was already there. The lever was the constant, not the allocator. + +Both rows abut their matched twin, so **the negatives index contains whole RUNS of repeated bodies**, +and C's conclusion is that the cheapest finder is a **raw-word Hamming scan** against the registered +regions — which the histogram tool only half-finds. C is running that scan across all 101 rows. + +**This extends finding 182, and the extension is the point:** an unclassified negative is not +overturnable — but these two WERE classified, **wrongly, in the same dimension, by sessions that then +wrote the row off**. A recorded mechanism is a HYPOTHESIS, and a *wrong* named mechanism is as +unattemptable as no mechanism at all until someone reads the raw words. + +### The claims-format defect — MY charter was wrong, and it is finding 179 in reverse + +My charter told all four workers to stage `claims.tsv` as +`rangesourcemd5differing_bytesresultoptions`. **`sf3_merge` reads +`startendsource[overrides]`.** The phase's first merge was rejected: +`expected three or four fields`. + +A charter **is** a file each worker copies, so writing a format into prose recreates exactly the defect +finding 179 warns about, one level up — the unfixable copied artefact was the coordinator's charter +instead of a worker's script. And the tool had already learned it: `validate_overrides` exists because a +Phase 11 worker put `md5=` in the 4th column, and its message says in terms *"per-claim metadata such as +a source md5 belongs in report.tsv"*. **The convention was documented inside the tool and I did not read +it** — I also renamed the evidence file to `evidence.tsv` when `report.tsv` is the established name used +by every Phase 8/9/11 worker's staging directory. + +Fixed as a **command rather than corrected prose**: `sf3_merge check-claims --claims F [--regions R]` +validates the format, duplicate starts, missing sources and already-registered rows, so a worker answers +"is my staging mergeable?" itself. 11 tests, including one that asserts **the charter's own wrong format +is rejected** so the message stays honest for the next coordinator. Charter §10 rewritten; §5 gained +rule 17 ("a coordinator's charter can be wrong, and this one was"). + +Cost: 1 rejected merge, 4 correction messages, ~10 minutes. Cheap because the tool **refuses to guess**. +Cookbook 190. + +### A defect in my own verification method, caught twice + +`grep -i 8006B5F0` across the lever files reported the address in BOTH `lever-b` and `lever-d`, which +looks like a double-assignment. It is not: in `lever-d` it is the exclusive **END** of +`0x8006B4C4..0x8006B5F0`; in `lever-b` it is the **START** of `0x8006B5F0..0x8006B66C`. The address-column +comparison gives **0 overlaps, union == worklist**. My grep was the defective instrument — the same +`0x8006B7C0` false-TAKEN that `sf3_free`'s containment rule exists to prevent. + +And I then **repeated my own pipeline mistake on a fresh tool**: `./tools/sf3_merge check-claims ... | +tail -4` reported `exit=0` for cases that actually exit 1. Twice now. The ledger's T0 note says *capture +the exit code directly, never through a pipeline* — and it is a habit, not a slip, so it is now recorded +as one. diff --git a/tools/sf3_merge b/tools/sf3_merge index d7ed0c4..85c3628 100755 --- a/tools/sf3_merge +++ b/tools/sf3_merge @@ -416,6 +416,67 @@ def command_apply(args: argparse.Namespace) -> int: return 0 +def command_check_claims(args: argparse.Namespace) -> int: + """Validate claim files WITHOUT merging, so a worker can self-check before reporting. + + WHY THIS EXISTS -- Phase 12. The coordinator's charter told every worker to stage + `claims.tsv` as `rangesourcemd5differing_bytesresultoptions` + with the range written `0xSTART..0xEND`. That format is not what this tool reads, so + the FIRST merge of the phase was rejected with `expected three or four fields` -- and + all four workers were already writing files in the wrong format. + + The tool was right and the prose was wrong. That is cookbook 179 exactly: a rule every + worker must follow belongs in a TRACKED tool, not in a file each worker copies -- and a + charter is a file each worker copies. The durable fix is not to correct the prose but to + give the worker a command that answers "is my staging acceptable?" before the merge is + attempted, so the next coordinator's prose error costs one command instead of a batch. + + Checks the format, the absence of duplicate starts, and that every claimed source file + exists. It does NOT check extents, overlaps with the registry, or the whole-binary gate: + those belong to `apply` and to `sf3_match gate`, and `--regions` here is only used to + warn about a row that is already registered. + """ + claim_paths = [require_file(path, "claim file") for path in args.claims] + if not claim_paths: + raise ToolError("at least one --claims file is required") + root = Path(args.root).resolve() + if not root.is_dir(): + raise ToolError(f"--root is not a directory: {root}") + + registered: list[tuple[int, int]] = [] + if args.regions is not None: + registered = [(start, end) for start, end, _s, _o in load_regions(args.regions)[1]] + + total = 0 + problems = 0 + for path in claim_paths: + try: + claims = load_claims(path) + except Rejection as exc: + print(f"rejected: {exc}", file=sys.stderr) + return 2 + print(f"{path}: {len(claims)} claim(s)") + seen: dict[int, str] = {} + for claim in claims: + issues = [] + source_path = root / claim.source + if not source_path.is_file(): + issues.append("SOURCE MISSING") + if claim.start in seen: + issues.append(f"DUPLICATE START with {seen[claim.start]}") + if any(start <= claim.start < end for start, end in registered): + issues.append("ALREADY REGISTERED") + seen[claim.start] = claim.origin + problems += len(issues) + flag = "ok" if not issues else " <-- " + "; ".join(issues) + print(f" {claim.describe()} overrides={claim.overrides or '-'} {flag}") + total += len(claims) + + print(f"claims={total} problems={problems}") + print("result=" + ("OK" if problems == 0 else "PROBLEMS")) + return 0 if problems == 0 else 1 + + def build_parser() -> argparse.ArgumentParser: parser = argparse.ArgumentParser( description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter @@ -440,6 +501,16 @@ def build_parser() -> argparse.ArgumentParser: help="skip a claim that is already registered exactly as claimed") apply_parser.set_defaults(handler=command_apply) + check_parser = subparsers.add_parser( + "check-claims", help="validate claim files without merging (worker self-check)" + ) + check_parser.add_argument("--claims", required=True, type=Path, action="append", default=[]) + check_parser.add_argument("--root", type=Path, default=Path("."), + help="root the claimed source paths are relative to") + check_parser.add_argument("--regions", type=Path, default=None, + help="optional: warn about rows already in this registry") + check_parser.set_defaults(handler=command_check_claims) + return parser diff --git a/tools/tests/test_sf3_merge.py b/tools/tests/test_sf3_merge.py index 4449a29..7c4079d 100644 --- a/tools/tests/test_sf3_merge.py +++ b/tools/tests/test_sf3_merge.py @@ -400,9 +400,6 @@ class MainTests(unittest.TestCase): self.assertIn("requires --symbols", stderr) -if __name__ == "__main__": - unittest.main() - class OverrideKeyValidationTests(unittest.TestCase): """Phase 11: an unknown override key must be rejected at MERGE time. @@ -432,3 +429,137 @@ class OverrideKeyValidationTests(unittest.TestCase): self.assertIn("report.tsv", str(err)) else: self.fail("expected Rejection") + + +class CheckClaimsTests(unittest.TestCase): + """Phase 12: the worker self-check, added because a CHARTER got the format wrong. + + The coordinator's Phase 12 charter specified + `rangesourcemd5differing_bytesresultoptions` with the range + written `0xSTART..0xEND`. That is not what `load_claims` reads, so the phase's FIRST + merge was rejected with "expected three or four fields" -- while all four workers were + already staging files in the wrong format. + + The tool was right and the prose was wrong. Cookbook 179 says a rule every worker must + follow belongs in a TRACKED tool rather than a file each worker copies, and a charter is + a file each worker copies. So the durable fix is a command that answers "is my staging + acceptable?" BEFORE the merge is attempted -- plus this test, which keeps the format's + error message and the charter's own wrong format pinned. + """ + + def setUp(self) -> None: + self._tmp = tempfile.TemporaryDirectory() + self.addCleanup(self._tmp.cleanup) + root = Path(self._tmp.name) + self.fx = Fixture( + root, + extents=[(A1, A2, "exact"), (A2, A3, "exact")], + regions=[(A1, A2, "src/func_80010000.c")], + ) + # Fixture creates only parent directories; a check flags a claim whose SOURCE is absent. + self.fx.source("src/func_80010000.c") + self.fx.source("src/func_80010010.c") + + def _check(self, *claims: Path, **extra: object) -> tuple[int, str, str]: + argv = ["check-claims", "--root", str(self.fx.root)] + for path in claims: + argv += ["--claims", str(path)] + for key, value in extra.items(): + argv += [f"--{key.replace('_', '-')}", str(value)] + return _run(argv) + + def _write(self, name: str, text: str) -> Path: + path = self.fx.root / name + path.write_text(text, encoding="ascii") + return path + + def test_a_mergeable_claims_file_reports_ok(self) -> None: + claims = self._write("c.tsv", f"0x{A2:08X}\t0x{A3:08X}\tsrc/func_80010010.c\n") + code, out, _err = self._check(claims) + self.assertEqual(code, 0, out) + self.assertIn("result=OK", out) + self.assertIn("claims=1 problems=0", out) + + def test_the_charter_s_original_wrong_format_is_rejected(self) -> None: + """The exact row the Phase 12 charter told every worker to write.""" + claims = self._write( + "wrong.tsv", + f"0x{A2:08X}..0x{A3:08X}\tsrc/func_80010010.c\t" + "565691d5836fe2bd85b92829e2f9a7df\t0\tMATCH\t\n", + ) + code, _out, err = self._check(claims) + self.assertEqual(code, 2) + self.assertIn("expected three or four fields", err) + + def test_a_missing_source_is_reported_and_fails(self) -> None: + claims = self._write("missing.tsv", f"0x{A2:08X}\t0x{A3:08X}\tsrc/func_DEADBEEF.c\n") + code, out, _err = self._check(claims) + self.assertEqual(code, 1) + self.assertIn("SOURCE MISSING", out) + self.assertIn("result=PROBLEMS", out) + + def test_a_duplicate_start_is_reported(self) -> None: + claims = self._write( + "dup.tsv", + f"0x{A2:08X}\t0x{A3:08X}\tsrc/func_80010010.c\n" + f"0x{A2:08X}\t0x{A3:08X}\tsrc/func_80010010.c\n", + ) + code, out, _err = self._check(claims) + self.assertEqual(code, 1) + self.assertIn("DUPLICATE START", out) + + def test_an_already_registered_row_is_flagged_only_when_regions_are_given(self) -> None: + claims = self._write("c.tsv", f"0x{A1:08X}\t0x{A2:08X}\tsrc/func_80010000.c\n") + code, out, _err = self._check(claims, regions=self.fx.regions) + self.assertEqual(code, 1) + self.assertIn("ALREADY REGISTERED", out) + code_alone, out_alone, _err = self._check(claims) + self.assertEqual(code_alone, 0, out_alone) + + def test_valid_overrides_are_accepted(self) -> None: + claims = self._write( + "c.tsv", + f"0x{A2:08X}\t0x{A3:08X}\tsrc/func_80010010.c\tmaspsx=epilogue\n", + ) + code, out, _err = self._check(claims) + self.assertEqual(code, 0, out) + self.assertIn("overrides=maspsx=epilogue", out) + + def test_an_unknown_override_key_is_rejected(self) -> None: + """The Phase 11 incident: md5= in the 4th column must never reach a candidate.""" + claims = self._write( + "c.tsv", f"0x{A2:08X}\t0x{A3:08X}\tsrc/func_80010010.c\tmd5=abc123\n" + ) + code, _out, err = self._check(claims) + self.assertEqual(code, 2) + self.assertIn("md5", err) + self.assertIn("report.tsv", err) + + def test_an_uncommented_header_row_is_rejected(self) -> None: + claims = self._write( + "hdr.tsv", + "start\tend\tsource\n" f"0x{A2:08X}\t0x{A3:08X}\tsrc/func_80010010.c\n", + ) + code, _out, err = self._check(claims) + self.assertEqual(code, 2) + self.assertIn("HEADER", err) + + def test_no_claims_file_is_a_usage_error(self) -> None: + """argparse enforces --claims, so this exits before the handler runs.""" + with self.assertRaises(SystemExit): + _run(["check-claims", "--root", str(self.fx.root)]) + + def test_a_missing_claims_file_exits_two(self) -> None: + code, _out, err = _run(["check-claims", "--claims", str(self.fx.root / "nope.tsv")]) + self.assertEqual(code, 2) + self.assertIn("not a regular file", err) + + def test_it_writes_nothing(self) -> None: + claims = self._write("c.tsv", f"0x{A2:08X}\t0x{A3:08X}\tsrc/func_80010010.c\n") + before = sorted(p.name for p in self.fx.root.iterdir()) + self._check(claims) + self.assertEqual(before, sorted(p.name for p in self.fx.root.iterdir())) + + +if __name__ == "__main__": + unittest.main()