From 1ce8bdddb70b0d106ac838ad4f4d5e95512e7329 Mon Sep 17 00:00:00 2001 From: Jonathan Borduas Date: Sat, 5 Sep 2026 23:53:59 +0100 Subject: [PATCH] =?UTF-8?q?tools:=20late-push=20=E2=80=94=20#224's=20instr?= =?UTF-8?q?ument,=20and=20detection=20is=20by=20CONTENT=20not=20SHA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #224 specified this and it did not exist. A squash merge captures every commit IN THE PR AT MERGE TIME and cannot capture one the author has not pushed yet, so an author who keeps pushing loses everything after the merge — and the PR shows MERGED with no indication anything is missing. Measured there: 37 of 100 merged inside 120s of creation, 25 inside 60s, one at 42 seconds. ⛔ DETECTION IS BY CONTENT, AND #224 IS EXPLICIT. A squash does not preserve shas, so `git merge-base --is-ancestor` answers a different question: whether THIS COMMIT is on main, never whether ITS CONTENT is. Every squashed commit is a non-ancestor, so ancestry alone reports the whole board lost. This compares `git patch-id --stable`. KNOWN-POSITIVE — #224's own, re-verified live before building: PR #339 merged 2026-08-20T19:56:18Z commit 7447b1d authored 19:57:18Z — 60s later docs/DEFECT-CLASSES.md +10, not an ancestor, patch-id absent from 400 commits of main ⇒ `--pr 339` reports exactly that, exit 1. Ten lines genuinely lost. ⚠ #224 says 61s; that is author-vs-commit time. ⛔ TWO DEVIATIONS FROM #224's STATED COMMANDS, and both are honest: 1. `--since 24h` no longer reaches #339, which is 16 days and ~250 merges down. The command was written when the specimen was fresh. `--pr N` is added so the known-positive stays runnable without a git-log per PR over the whole board — and `--limit` is documented as a RECENCY WINDOW, not a filter, on every run. 2. `--since 1s` returns exit 2 VOID, not the exit 0 #224 expected. With no commits on main in the window the patch-id comparison set is EMPTY, so every candidate would read as LOST. Refusing is the correct answer; a clean 0 there would be a lie. ⇒ The condition asked for a detector that "cannot return both is not one" — it returns three, and the third is the one that matters. ⚠ A FINDING IS A CANDIDATE, NOT A PROVEN LOSS. Content that reached main via a conflict resolution or an altered rebase has a different patch-id and reads as lost here. #445 holds: ORPHANED ≠ CONTENT-LOST, and only the first is mechanically decidable. An unreadable head ref is counted and named UNCHECKED, never folded into clean. 7 suite tests including the empty-patch-id VOID and test_control_can_fail; 5 self-test legs, both directions, driven with synthetic state (#402). ⚠ One test failure was MY STUB, not the tool: patch_id() calls `git show` with allow_fail=True, so keying the stub on that returned a stat block where a diff was expected and the finding silently vanished. Keyed on --stat instead. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DTMf4EvaTEnDKXY47efRZZ --- tools/README.md | 17 +-- tools/late-push.py | 259 ++++++++++++++++++++++++++++++++++++++++ tools/test_late_push.py | 116 ++++++++++++++++++ 3 files changed, 385 insertions(+), 7 deletions(-) create mode 100644 tools/late-push.py create mode 100644 tools/test_late_push.py diff --git a/tools/README.md b/tools/README.md index 83351a5..7fd2040 100644 --- a/tools/README.md +++ b/tools/README.md @@ -10,7 +10,7 @@ below is taken over it.** ``` INSTRUMENT ≡ a non-test executable directly under tools/, EXCLUDING quarantined subdirectories - ls tools/*.py tools/*.sh | grep -v '/test_' ⇒ 60. Run it; do not trust the number below. + ls tools/*.py tools/*.sh | grep -v '/test_' ⇒ 61. Run it; do not trust the number below. ``` ⚠ **The first draft of this section declared 54 and published a command that returns 55** — off by the @@ -22,12 +22,12 @@ that excludes it because of its extension is drawing the population around a fil readings, so a reader meeting an older figure can place it:** ``` -top-level non-test executables (.py + .sh) 60 ⇐ THE DECLARED POPULATION - of which .py 59 ⚠ every count published on 2026-08-21 used a SMALLER subset -top-level tools/*.py, including test_ 118 -ALL .py under tools/ recursively 141 -ALL .py under tools/ excluding test_ 79 -rows in the index table below 62 +top-level non-test executables (.py + .sh) 61 ⇐ THE DECLARED POPULATION + of which .py 60 ⚠ every count published on 2026-08-21 used a SMALLER subset +top-level tools/*.py, including test_ 120 +ALL .py under tools/ recursively 143 +ALL .py under tools/ excluding test_ 80 +rows in the index table below 63 files under tools/teamlead/ 23 ⛔ QUARANTINED — belonging is an OPEN QUESTION ``` @@ -327,6 +327,7 @@ of them, which is why it is stated here rather than in a docstring. | `doctrine-deliver.py` | put a doctrine POINTER in front of a role that is behind — and refuse when the pane is in another estate | 0 nothing to deliver · 1 pointers pending or queued · **2 established nothing (no pane in this estate, zero panes enumerated, no roles named, or a payload that is not a pointer)** · `--self-test` | | `doctrine-uncommitted.py` | which doctrine is the fleet READING that main does not carry? | 0 reads == landed · 1 drift in any of three directions · **2 established nothing (no repo, bad ref, no file)** | | `label-precedence.py` | when `role:` and `dev:N` disagree, which does a pane obey? | 0 no HAZARD collisions · 1 at least one HAZARD -- a finding, established · 2 established nothing: forge unreadable, or the buckets did not sum | ⚙ GENERATED-FROM: --states | +| `late-push.py` | which commits were pushed AFTER their PR merged and never reached main? | 0 none lost · 1 FINDINGS · **2 established nothing (no PRs enumerated, or an EMPTY main patch-id set — every candidate would then read as lost)** · `--self-test` | | `label-exists.py` | does the label you are about to query actually exist? | 0 all exist · 1 one is absent · **2 established nothing** | | `verdict-census.py` | has each indexed instrument ever produced a verdict? (`--ledger` keeps the record · `--stale-check` asks in 0.1s whether it is current) | 0 no finding · 1 a finding · **2 established nothing** · ⚠ `--stale-check`'s 0 means *the record is current*, NOT *they all produce verdicts* | | `wake-yield.py` | did that interruption produce work, or churn? | 0 | @@ -876,6 +877,8 @@ means a population leg **exists**, not that it is a good one; `DRAWN` is a state is a **stated exception with a reason**. ★ Its own control is DRAWN, and it reports itself as such — a tool that measured this property and exempted itself would be the joke version of itself. +**`late-push.py`** — ⛔ **#224's instrument, specified there and absent until now.** *A squash merge captures every commit IN THE PR AT MERGE TIME; it cannot capture one the author has not pushed yet.* Measured across 100 merged PRs: **37 merged inside 120s of creation, 25 inside 60s** — and #201 at **42 seconds**. ★ *At 42 seconds the author is not late; the merger is early*, and `MERGED` erases the difference. ⛔⛔ **DETECTION IS BY CONTENT, NOT SHA, and #224 is explicit.** A squash does not preserve shas, so `git merge-base --is-ancestor` answers a different question — it says whether THIS COMMIT is on main, never whether ITS CONTENT is, and **every squashed commit is a non-ancestor**, so ancestry alone reports the whole board lost. This compares `git patch-id --stable`: same diff, same id, regardless of sha or parent — exactly the equivalence a squash preserves. ★ **The known-positive is #224's own and re-verified live:** PR **#339** merged `19:56:18Z`, commit **`7447b1d`** authored `19:57:18Z` — **60s later** — `docs/DEFECT-CLASSES.md +10`, not an ancestor, patch-id absent from 400 commits of main. Ten lines genuinely lost. ⚠ #224 says 61s; that is author-vs-commit time. ⛔ **The VOID path is the load-bearing refusal:** an EMPTY main patch-id set makes every candidate read as lost, so it exits **2**, never a finding. #224's stated control `--since 1s` returns exactly that — **2, not 0** — which is a better answer than the condition expected. ⚠ And `--limit` is a **recency window, not a filter**: a PR older than the most recent N is NOT examined and is not clean, which the output says on every run. `--pr N` exists because #224's own specimen is ~250 merges down and reaching it by raising `--limit` costs a git-log per PR over the whole board. ⚠ **A finding is a CANDIDATE, not a proven loss** — content that reached main through a conflict resolution or an altered rebase has a different patch-id and reads as lost here. ⇒ **#445 holds: ORPHANED ≠ CONTENT-LOST**, and only the first is mechanically decidable. An unreadable head ref is counted and named as **UNCHECKED**, never folded into clean. + **`label-precedence.py`** — **when `role:` and `dev:N` disagree about who owns an issue, which one is a pane meant to obey?** ⛔ Measured on #461: both fields are queryable, both populated, and until 2026-08-21 nothing stated which wins — **15 open issues carried a `dev:N` beside a `role:` that is not diff --git a/tools/late-push.py b/tools/late-push.py new file mode 100644 index 0000000..b53ee4b --- /dev/null +++ b/tools/late-push.py @@ -0,0 +1,259 @@ +#!/usr/bin/env python3 +"""Which commits were pushed to a branch AFTER its PR merged — and never reached main? + +⛔ #224's instrument, specified there and absent until now. The defect it exists for: + + #201 commits in the PR: 1 created 09:48:12Z merged 09:48:54Z 42 SECONDS + across 100 merged PRs: 37 merged <120s after creation · 25 merged <60s + +★ A squash merge captures every commit IN THE PR AT MERGE TIME. It cannot capture one +the author has not pushed yet. So an author who opens a PR and keeps pushing loses +everything after the merge — and the PR shows `MERGED` with no indication anything is +missing. ⇒ *At 42 seconds the author is not late. The merger is early.* And `MERGED` +erases the difference. + +⛔⛔ DETECTION IS BY CONTENT, NOT BY SHA, AND #224 IS EXPLICIT ABOUT IT. A squash merge +does not preserve SHAs, so `git merge-base --is-ancestor` answers a different question: +it says whether THIS COMMIT is on main, never whether ITS CONTENT is. Every squashed +commit is a non-ancestor, so ancestry alone reports the whole board as lost. + +⇒ This compares `git patch-id --stable`. Two commits with the same diff have the same +patch-id regardless of sha, parent or message, which is exactly the equivalence a squash +preserves and ancestry does not. + +⚠ WHAT PATCH-ID DOES NOT SURVIVE, stated because a silent miss here reads as a finding: +a commit whose content reached main **through a conflict resolution or a rebase that +altered the diff** has a different patch-id and will be reported as lost. ⇒ Findings are +CANDIDATES for a human read, not a proven loss. #445's distinction holds and is the +reason: ORPHANED ≠ CONTENT-LOST, and only the first is mechanically decidable. + +The known-positive, live and re-verified 2026-09-05: + + PR #339 merged 2026-08-20T19:56:18Z + commit 7447b1d authored 19:57:18Z — SIXTY SECONDS after the merge + docs/DEFECT-CLASSES.md, +10 lines + ancestry: not an ancestor · patch-id: absent from 400 commits of main + +Exit: 0 no late pushes lost · 1 FINDINGS · 2 established nothing. +""" +import argparse +import json +import os +import subprocess +import sys +from datetime import datetime, timezone + +for _s in (sys.stdout, sys.stderr): + try: + _s.reconfigure(encoding="utf-8") + except Exception: + pass + +DEFAULT_REPO = "nForma-AI/nForma-NEXT" + + +class Void(Exception): + """Established nothing. ⇒ exit 2, never a verdict.""" + + +def sh(args, allow_fail=False, stdin=None): + try: + p = subprocess.run(args, capture_output=True, text=True, input=stdin) + except OSError as exc: + raise Void(f"cannot run {args[0]}: {exc}") + if p.returncode != 0 and not allow_fail: + raise Void(f"{args[0]} exited {p.returncode}: {(p.stderr or '').strip()[:200]}") + return p.stdout + + +def iso(s): + return datetime.fromisoformat(s.replace("Z", "+00:00")) + + +# ── The decision, separated from the data (#402) ────────────────────────────── + +def classify(merged_at, commit_at, patch_on_main): + """⇒ one of LATE-LOST · LATE-LANDED · IN-PR. + + ★ Separated from every subprocess call so the controls drive it with synthetic + state — #402: for a stateful instrument, a caller that still runs it means one that + drives the DECISION with synthetic prior-state, because the transition itself is not + re-runnable.""" + if commit_at <= merged_at: + return "IN-PR" + return "LATE-LANDED" if patch_on_main else "LATE-LOST" + + +def patch_id(rev): + out = sh(["git", "show", rev], allow_fail=True) + if not out.strip(): + return None + pid = sh(["git", "patch-id", "--stable"], allow_fail=True, stdin=out) + return (pid.split() or [None])[0] + + +def main_patch_ids(since): + """⛔ Built ONCE. Computing it per candidate is O(n·m) and was measured at minutes.""" + revs = sh(["git", "log", "origin/main", f"--since={since}", "--format=%H"], + allow_fail=True).split() + ids = set() + for r in revs: + p = patch_id(r) + if p: + ids.add(p) + if not ids: + raise Void(f"no commits on origin/main since {since} — the comparison set is " + f"EMPTY, so every candidate would read as lost. That is a VOID " + f"reading, not a clean board.") + return ids + + +def run(repo, since, limit, only=None): + sh(["git", "fetch", "origin", "--quiet"], allow_fail=True) + if only: + # ⛔ --pr exists because --limit is a RECENCY window, not a filter. #224's own + # known-positive (#339) is 16 days and ~250 merges down; reaching it by raising + # --limit costs a git-log per PR over the whole board. A named PR makes the + # specimen re-runnable, which is what an acceptance criterion needs. + prs = [] + for n in only: + raw = sh(["gh", "pr", "view", str(n), "--repo", repo, + "--json", "number,mergedAt,headRefName,headRefOid"]) + try: + prs.append(json.loads(raw)) + except json.JSONDecodeError as exc: + raise Void(f"gh pr view {n} was not JSON: {exc}") + else: + raw = sh(["gh", "pr", "list", "--repo", repo, "--state", "merged", + "--limit", str(limit), + "--json", "number,mergedAt,headRefName,headRefOid"]) + try: + prs = json.loads(raw or "[]") + except json.JSONDecodeError as exc: + raise Void(f"gh pr list was not JSON: {exc}") + if not prs: + raise Void(f"no merged PRs returned for {repo} — nothing was enumerated, which " + f"is not the same as nothing being late.") + + cutoff = iso(since) if "T" in since else None + ids = main_patch_ids(since if not cutoff else since) + + findings, checked, unreachable = [], 0, 0 + for pr in prs: + if not pr.get("mergedAt"): + continue + merged = iso(pr["mergedAt"]) + ref = f"origin/{pr['headRefName']}" + log = sh(["git", "log", ref, "--format=%H %cI", "-50"], allow_fail=True) + if not log.strip(): + unreachable += 1 # branch deleted or never fetched — NOT a clean read + continue + for line in log.splitlines(): + parts = line.split() + if len(parts) < 2: + continue + sha, when = parts[0], parts[1] + try: + cat = iso(when) + except ValueError: + continue + if cat <= merged: + continue + checked += 1 + verdict = classify(merged, cat, (patch_id(sha) in ids)) + if verdict == "LATE-LOST": + files = sh(["git", "show", "--stat", "--format=", sha], + allow_fail=True).strip().splitlines() + findings.append((pr["number"], sha, int((cat - merged).total_seconds()), + [f.strip() for f in files[:3]])) + + scope = f"PR(s) {','.join(str(p['number']) for p in prs)}" if only else \ + f"{len(prs)} most-recently-merged PR(s)" + print(f"POPULATION {scope} from {repo}; main patch-ids since {since}") + if not only: + print(f" ⚠ --limit is a RECENCY window, not a filter: a PR older than " + f"the\n most recent {limit} is NOT examined and is not clean.") + print(f"PREDICATE a commit on the head ref dated AFTER mergedAt whose PATCH-ID is " + f"absent from origin/main") + print(f"CHANNEL git patch-id --stable — ⛔ NOT ancestry: a squash preserves the " + f"diff and not the sha\n") + print(f" late commits examined: {checked}") + print(f" ⚠ head refs unreadable (deleted or unfetched): {unreachable} — these are " + f"UNCHECKED, not clean") + + if not findings: + print("\n no late push lost content in this window") + return 0 + + print(f"\n── {len(findings)} FINDING(S) ──") + for num, sha, secs, files in findings: + print(f" ⛔ #{num} {sha[:8]} pushed {secs}s AFTER its PR merged") + for f in files: + print(f" {f}") + print("\n⚠ A finding is a CANDIDATE, not a proven loss. Content that reached main " + "through\n a conflict resolution or an altered rebase has a different " + "patch-id and reads as\n lost here. ⇒ #445: ORPHANED ≠ CONTENT-LOST, and " + "only the first is mechanical.") + return 1 + + +# ── Controls ────────────────────────────────────────────────────────────────── + +def self_test(): + """⛔ TWO-SIDED AND NAMED, driving the DECISION with synthetic state so no forge, + no network and no clock are needed (#402).""" + ok = True + + def check(name, got, want): + nonlocal ok + good = got == want + ok &= good + print(f" {'PASS' if good else 'FAIL'} {name}: got {got!r}, want {want!r}") + + M = iso("2026-08-20T19:56:18Z") + after = iso("2026-08-20T19:57:18Z") # #339 / 7447b1d — sixty seconds later + before = iso("2026-08-20T19:50:00Z") + + print("⛔ the specimen this exists for (#224: #339, 7447b1d, +60s)") + check("late AND absent from main -> LATE-LOST", classify(M, after, False), "LATE-LOST") + + print("★ the known-NEGATIVES, without which every squash reads as a loss") + check("late but its CONTENT landed -> LATE-LANDED", classify(M, after, True), "LATE-LANDED") + check("committed BEFORE the merge -> IN-PR", classify(M, before, False), "IN-PR") + check("exactly at the merge instant is IN-PR, not late", classify(M, M, False), "IN-PR") + + print("⚠ one second late is still late — the boundary is not fudged") + check("+1s -> LATE-LOST", classify(M, iso("2026-08-20T19:56:19Z"), False), "LATE-LOST") + + print(f"\n{'all controls pass' if ok else '⛔ CONTROLS FAILED'} — 5 legs, both directions") + return 0 if ok else 1 + + +def main(): + ap = argparse.ArgumentParser( + description="Commits pushed after their PR merged whose content never reached main.") + ap.add_argument("--repo", default=DEFAULT_REPO) + ap.add_argument("--since", default="24 hours ago", + help="git-log window, also bounding main's patch-id set") + ap.add_argument("--limit", type=int, default=100) + ap.add_argument("--pr", type=int, action="append", + help="examine only these PR numbers (repeatable)") + ap.add_argument("--self-test", action="store_true") + args = ap.parse_args() + + if args.self_test: + extra = [a for a in sys.argv[1:] if a != "--self-test"] + if extra: + print(f"⛔ unrecognised argument(s) alongside --self-test: {extra}", + file=sys.stderr) + return 2 + return self_test() + try: + return run(args.repo, args.since, args.limit, args.pr) + except Void as exc: + print(f"⛔ VOID — established nothing: {exc}", file=sys.stderr) + return 2 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tools/test_late_push.py b/tools/test_late_push.py new file mode 100644 index 0000000..c01a59e --- /dev/null +++ b/tools/test_late_push.py @@ -0,0 +1,116 @@ +#!/usr/bin/env python3 +"""Exit-code contract for late-push.py. + +⛔ `--self-test` covers classify(), which is pure. It cannot cover the paths a caller +reads as "the board is clean", and those are where a detector fails safe-looking: + + an empty patch-id set every candidate then reads as LOST — a VOID, not a finding + no PRs enumerated "nothing was late" vs "nothing was looked at" (#317) + an unreadable head ref a deleted branch is UNCHECKED, never clean + +★ Every case drives the real main() with `sh` stubbed, so the exit code under test is +the one the tool returns. +""" +import importlib.util, io, json, os, sys, unittest +from contextlib import redirect_stderr, redirect_stdout +from pathlib import Path + +sys.dont_write_bytecode = True +os.environ["PYTHONDONTWRITEBYTECODE"] = "1" +HERE = Path(__file__).resolve().parent + + +def load(): + s = importlib.util.spec_from_file_location("lp", HERE / "late-push.py") + m = importlib.util.module_from_spec(s); s.loader.exec_module(m); return m + + +def drive(mod, prs, mainlog, headlog, patchids, argv=None): + """Stub `sh` by command shape. patchids maps rev -> patch-id.""" + def fake(args, allow_fail=False, stdin=None): + if args[:2] == ["git", "fetch"]: + return "" + if args[:2] == ["gh", "pr"]: + return json.dumps(prs if args[2] == "list" else prs[0]) + if args[:2] == ["git", "log"] and "origin/main" in args: + return "\n".join(mainlog) + if args[:2] == ["git", "log"]: + return "\n".join(headlog) + if args[:2] == ["git", "show"]: + # ⚠ Distinguish by --stat, NOT by allow_fail: patch_id() calls `git show` + # with allow_fail=True, so keying on that made the stub return the stat + # block where the tool expected a diff, and the finding silently vanished. + # My stub's defect, found by the test failing — which is the suite working. + if "--stat" in args: + return " f.md | 1 +\n 1 file changed" + return f"DIFF-OF-{args[-1]}" + if args[:2] == ["git", "patch-id"]: + return f"{patchids.get((stdin or '').replace('DIFF-OF-', ''), 'none')} x" + return "" + mod.sh = fake + old = sys.argv; sys.argv = ["late-push.py"] + (argv or []) + out, err = io.StringIO(), io.StringIO() + try: + with redirect_stdout(out), redirect_stderr(err): + rc = mod.main() + finally: + sys.argv = old + return rc, out.getvalue(), err.getvalue() + + +PR = [{"number": 339, "mergedAt": "2026-08-20T19:56:18Z", + "headRefName": "dev2/x", "headRefOid": "abc"}] + + +class ExitContract(unittest.TestCase): + def setUp(self): self.mod = load() + + def test_late_and_absent_is_a_finding(self): + rc, out, _ = drive(self.mod, PR, ["M1"], ["L1 2026-08-20T19:57:18+00:00"], + {"M1": "pidM", "L1": "pidL"}) + self.assertEqual(rc, 1) + self.assertIn("#339", out) + self.assertIn("60s AFTER", out) + + def test_late_but_content_landed_is_NOT_a_finding(self): + """⛔ The known-negative. A squash gives every commit a new sha, so ancestry + would report this lost; patch-id must not.""" + rc, out, _ = drive(self.mod, PR, ["M1"], ["L1 2026-08-20T19:57:18+00:00"], + {"M1": "same", "L1": "same"}) + self.assertEqual(rc, 0) + self.assertIn("no late push lost content", out) + + def test_commit_before_merge_is_not_late(self): + rc, _, _ = drive(self.mod, PR, ["M1"], ["L1 2026-08-20T19:00:00+00:00"], + {"M1": "a", "L1": "b"}) + self.assertEqual(rc, 0) + + def test_empty_main_patchid_set_is_VOID_not_a_finding(self): + """⛔ The load-bearing refusal: with no comparison set every candidate reads as + lost, which is the loudest possible wrong answer.""" + rc, _, err = drive(self.mod, PR, [], ["L1 2026-08-20T19:57:18+00:00"], {}) + self.assertEqual(rc, 2) + self.assertIn("comparison set is EMPTY", err) + + def test_no_prs_is_VOID_not_clean(self): + rc, _, err = drive(self.mod, [], ["M1"], [], {"M1": "p"}) + self.assertEqual(rc, 2) + self.assertIn("nothing was enumerated", err) + + def test_unreadable_head_ref_is_counted_not_ignored(self): + rc, out, _ = drive(self.mod, PR, ["M1"], [], {"M1": "p"}) + self.assertEqual(rc, 0) + self.assertIn("unreadable (deleted or unfetched): 1", out) + self.assertIn("UNCHECKED, not clean", out) + + def test_control_can_fail(self): + mod = load() + with redirect_stdout(io.StringIO()): + self.assertEqual(mod.self_test(), 0) + mod.classify = lambda m, c, p: "IN-PR" # never reports anything late + with redirect_stdout(io.StringIO()): + self.assertEqual(mod.self_test(), 1) + + +if __name__ == "__main__": + unittest.main(verbosity=2)