From 7907c8d286bfcf481e1a8a6c4bda72e658cbb312 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Mon, 21 Sep 2026 18:17:11 +0200 Subject: [PATCH] fix(port): check-citations was red, and its selftest BROKEN, on a clean tree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two independent defects, both making the script report a correct checkout as wrong. Neither is new; both were invisible because nobody ran it here. 1. `export/` is the exporter's OUTPUT and is gitignored (`/export*/`). A checkout where nobody has run the exporter has no `export/` at all, so the four `DECISIONS.md`/`BLOCKED.md` citations of `export/manifest.json` and `export/screens/...` landed in "resolve NOWHERE" and the check exited 1 -- red for a state no edit can fix, which is the exact shape its own docstring says it exists to avoid. `check-capture-citations` learned this for `docs/re/captures/`; same rule now: absent BECAUSE UNBUILT is reported, absent while the tree IS built still fails. Verified both ways -- `mkdir export` and the same four go back to failing. 2. The selftest's peer-branch fixture cited `docs/re/f5-a-press-snaps-the-plate.md`, which the consolidation made an ordinary local file. The fixture stopped testing the scanner and started reporting it broken; `PEER_REFS` also still named `origin/auto/frame-blend-draw-path`, a branch that no longer exists. The selftest now FINDS a peer-only path at runtime, and where none exists -- the normal case on a clean checkout, measured: zero -- it says the class is empty here rather than claiming a failure. The class itself stays: the next topic branch that lands a finding recreates the condition exactly. Measured: check 123 citations, 119 resolve, 4 unbuilt, 0 nowhere, exit 0 (was exit 1). Selftest ok (was 🔴 BROKEN, on `main` too). The new generated-tree case was confirmed to FAIL against the unfixed function first. Co-Authored-By: Claude Opus 5 --- tools/port/check-citations | 113 ++++++++++++++++++++++++++++++++----- 1 file changed, 99 insertions(+), 14 deletions(-) diff --git a/tools/port/check-citations b/tools/port/check-citations index 465aefc0..5f381e39 100755 --- a/tools/port/check-citations +++ b/tools/port/check-citations @@ -36,7 +36,60 @@ CITE = re.compile( r"`?((?:docs|crates|port|tools|authored|export)/[\w./-]+" r"\.(?:md|rs|gd|json|txt|py|tsv|csv))`?" ) -PEER_REFS = ("origin/auto/frame-blend-draw-path", "origin/main") +# ⚠️ `origin/auto/frame-blend-draw-path` used to head this list and NO LONGER +# EXISTS -- the consolidation merged it and the branch was removed. The class is +# kept because it is about the workflow, not about that one branch: the next +# topic branch that lands a finding recreates the condition exactly. What must +# not happen again is the selftest asserting the class against a fixture path +# that has since become an ordinary local file, which is how it came to print +# 🔴 BROKEN on a correct checkout. It now finds its own fixture, or says the +# condition does not exist here. +PEER_REFS = ("origin/main",) + + +def a_peer_only_path() -> str | None: + """A path carried by a PEER_REF but absent from this working tree. + + The selftest needs a REAL one: a hardcoded fixture silently stops testing + the moment that file lands locally, and then reports the scanner broken + instead of itself. On a clean, up-to-date checkout there is usually no such + path at all -- which is not a failure, it is the class being empty here. + """ + for ref in PEER_REFS: + out = subprocess.run(["git", "ls-tree", "-r", "--name-only", ref], + capture_output=True, text=True) + for f in out.stdout.splitlines(): + if f.endswith((".md", ".rs", ".gd", ".json", ".txt", ".py", + ".tsv", ".csv")) and not os.path.exists(f): + return f + return None + + +def gitignored(path: str) -> bool: + """Is this path deliberately untracked? Pattern match -- existence not needed.""" + return subprocess.run(["git", "check-ignore", "-q", path], + capture_output=True).returncode == 0 + + +def unbuilt(path: str) -> bool: + """A citation of GENERATED output whose tree has not been built here. + + 🔴 THE THIRD ABSENCE, AND IT IS NOT AN ERROR. `export/` is the exporter's + output and is gitignored (`.gitignore` `/export*/`). A checkout where nobody + has run the exporter has no `export/` at all, so the four `DECISIONS.md` and + `BLOCKED.md` citations of `export/manifest.json` and `export/screens/...` + were counted as "resolve NOWHERE" and this check was red on a clean tree -- + for a state no edit can fix, which is precisely the shape its own docstring + says it exists to avoid. + + `check-capture-citations` already learned this for `docs/re/captures/`. + Same rule here: absent BECAUSE UNBUILT is reported; absent while the tree + IS built is a real broken citation and still fails. + """ + if not gitignored(path): + return False + root = path.split("/")[0] + return not os.path.exists(root) def on_a_ref(path: str) -> str | None: @@ -49,7 +102,7 @@ def on_a_ref(path: str) -> str | None: def scan(files): - resolves, peer, nowhere = 0, {}, {} + resolves, peer, nowhere, ungenerated = 0, {}, {}, {} for p in files: try: text = open(p, encoding="utf-8").read() @@ -58,11 +111,13 @@ def scan(files): for m in sorted(set(CITE.findall(text))): if os.path.exists(m): resolves += 1 + elif unbuilt(m): + ungenerated.setdefault(m, p) elif (ref := on_a_ref(m)): peer.setdefault(m, (p, ref)) else: nowhere.setdefault(m, p) - return resolves, peer, nowhere + return resolves, peer, nowhere, ungenerated def main() -> int: @@ -82,28 +137,51 @@ def main() -> int: # exists) and does not resolve here (the reader still gets nothing), and # a scanner that collapsed it into either would make the flag meaningless # while still passing the two checks above. + peer_probe = a_peer_only_path() peerfile = os.path.join(tmp, "peer.md") - open(peerfile, "w").write("see `docs/re/f5-a-press-snaps-the-plate.md`\n") - rp, pp, np_ = scan([peerfile]) + open(peerfile, "w").write("see `%s`\n" % (peer_probe or "docs/port/PORT-MISSION.md")) + # The FOURTH class: generated output whose tree is not built here. + # It has to be told apart from "resolves nowhere", which is the whole + # point -- a scanner that lumped them together is what made this check + # red on a clean checkout. + genfile = os.path.join(tmp, "gen.md") + open(genfile, "w").write("see `export/screens/title/main_menu.json`\n") - _, _, nb = scan([bad]) - r, _, ng = scan([good]) + rp, pp, np_, gp = scan([peerfile]) + + _, _, nb, _ = scan([bad]) + r, _, ng, _ = scan([good]) + rg, pg, ng2, gg = scan([genfile]) caught = len(nb) == 1 passed = len(ng) == 0 and r == 1 - peer_ok = len(pp) == 1 and rp == 0 and len(np_) == 0 - ok = caught and passed and peer_ok + # None == the class is empty in this checkout, not that it is broken. + peer_ok = (len(pp) == 1 and rp == 0 and len(np_) == 0 and len(gp) == 0) \ + if peer_probe else None + # Only meaningful while `export/` is absent; if someone ran the exporter + # in this checkout the citation legitimately resolves instead. + gen_ok = (len(gg) == 1 and len(ng2) == 0 and len(pg) == 0) \ + if not os.path.exists("export") else (rg == 1) + ok = caught and passed and gen_ok and peer_ok is not False print("selftest: planted dangling caught=%s, real citation passed=%s, " - "peer-branch classed separately=%s -> %s" - % (caught, passed, peer_ok, "ok" if ok else "🔴 BROKEN")) - if not peer_ok: + "peer-branch classed separately=%s, unbuilt-generated classed " + "separately=%s -> %s" + % (caught, passed, + "n/a (no peer-only path exists here)" if peer_ok is None + else peer_ok, + gen_ok, "ok" if ok else "🔴 BROKEN")) + if not gen_ok: + print(" 🔴 a citation of unbuilt generated output must NOT be " + "dangling; got resolves=%d peer=%d nowhere=%d ungenerated=%d" + % (rg, len(pg), len(ng2), len(gg))) + if peer_ok is False: print(" 🔴 --for-merge cannot mean anything if the peer class is " "not distinguished; got resolves=%d peer=%d nowhere=%d" % (rp, len(pp), len(np_))) return 0 if ok else 2 files = sorted(glob.glob("docs/port/*.md")) - resolves, peer, nowhere = scan(files) - total = resolves + len(peer) + len(nowhere) + resolves, peer, nowhere, ungenerated = scan(files) + total = resolves + len(peer) + len(nowhere) + len(ungenerated) print("citations of repo paths in docs/port/*.md: %d" % total) print(" resolve here : %d" % resolves) # 🔴 --for-merge TURNS THE PEER CLASS INTO A FAILURE. @@ -130,6 +208,13 @@ def main() -> int: print(" After this merges they resolve NOWHERE -- the reader gets a dead") print(" path. Land the finding first and make it a dependency of this PR.") return 1 + if ungenerated: + print(" not built in this checkout : %d (reported, not failed)" + % len(ungenerated)) + for m, src in sorted(ungenerated.items()): + print(" %-52s <- %s" % (m, os.path.basename(src))) + print(" run the exporter and these resolve; a path still missing") + print(" afterwards IS dangling and fails below.") if nowhere: print(" 🔴 resolve NOWHERE : %d" % len(nowhere)) for m, src in sorted(nowhere.items()):