From b464ed9202bfb894894825566698f15865eaea37 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 05:24:17 +0200 Subject: [PATCH] fix: count prose that holds the pin as review work The triage, --changed-only and --strict counted refs only. Prose that --realign-prose would move or refuse blocks every recale and bump, and blastem read 0 to review. Each report now carries that reading. The triage also gets a header, a closing count, and one sampling notice per run instead of one per profile. --- scripts/profile_sync.py | 82 +++++++++++++++++++++++++++++++++----- tests/test_profile_sync.py | 66 ++++++++++++++++++++++++++++++ wiki/tools.md | 10 ++++- 3 files changed, 147 insertions(+), 11 deletions(-) diff --git a/scripts/profile_sync.py b/scripts/profile_sync.py index b0de1ba8..968e42ad 100644 --- a/scripts/profile_sync.py +++ b/scripts/profile_sync.py @@ -945,6 +945,11 @@ class ProfileReport: page_version: str | None = None # An announced version searched for and found only rebuilt or re-signed. no_genuine_copy: str | None = None + # What realign_prose has to settle before a recale or a bump can run: + # prose written under an older pin. None when not looked at. Counted to + # review, or the triage read "0 to review" on a profile whose pin could + # not move (blastem, xenia-edge, lindbergh-loader). + stale_prose: list[str] | None = None # Skips that say the profile cannot be checked as written: a repository # that no longer answers, no pin to read, a death the forge contradicts. @@ -963,8 +968,10 @@ class ProfileReport: def needs_review(self) -> int: counts = self.counts or {} - return sum(counts.get(status, 0) for status in REVIEW_STATUSES) + len( - self.orphaned + return ( + sum(counts.get(status, 0) for status in REVIEW_STATUSES) + + len(self.orphaned) + + len(self.stale_prose or ()) ) @@ -1964,6 +1971,8 @@ def format_report(report: ProfileReport, changed_only: bool = False) -> str: f"{count} {status.lower()}" for status, count in sorted(report.counts.items()) ) lines.append(f" {len(report.entries)} refs: {summary}") + for line in report.stale_prose or (): + lines.append(f" prose under an older pin: {line}") lines.append(f" {report.needs_review()} to review") return "\n".join(lines) @@ -2200,6 +2209,12 @@ def drift_score( """Rank profiles by how far they have drifted from their pin.""" counts = report.counts or {} score = sum(DRIFT_WEIGHTS.get(k, 0) * v for k, v in counts.items()) + # Prose a reader has to settle weighs as an ambiguous ref, prose + # --realign-prose moves by itself as a shifted one. + score += sum( + DRIFT_WEIGHTS["SHIFTED" if line.startswith("would recale") else "AMBIGUOUS"] + for line in report.stale_prose or () + ) if version is not None and not version.tag_matches_declared: if version.latest_release and version.latest_release != version.declared: score += 10 @@ -3490,14 +3505,21 @@ def _check_quota(count: int, offline: bool) -> None: raise SystemExit(1) -def _stale_prose_blocks(name: str, path: Path, args: argparse.Namespace) -> bool: +def _stale_prose_blocks( + name: str, path: Path, args: argparse.Namespace, + known: list[str] | None = None, +) -> bool: """Whether prose written under an older pin has to move first. Anchored from the current pin, a recale or a bump follows someone else's code (boom3 FileSystem.cpp:2125, written at b810234e, an unrelated OpenFileRead at 132dfddb). It moves first, from the pin it was written at. + `known` is the answer the report already holds for the same text. """ - stale = realign_prose(path, args.cache_dir, args.offline, dry_run=True) + stale = ( + known if known is not None + else realign_prose(path, args.cache_dir, args.offline, dry_run=True) + ) if stale: print( f"{name}: {len(stale)} prose citation(s) written under an older pin; " @@ -3507,6 +3529,28 @@ def _stale_prose_blocks(name: str, path: Path, args: argparse.Namespace) -> bool return bool(stale) +def _read_stale_prose( + name: str, path: Path, args: argparse.Namespace, failed: set[str] +) -> list[str]: + """What --realign-prose would do or refuse on this profile, as a report. + + Read on every pass, not only before a write: prose that blocks the pin + is review work whether or not this run tries to move it. + """ + try: + return realign_prose(path, args.cache_dir, args.offline, dry_run=True) + except upstream.RateLimitError: + raise + except upstream.NotCachedError as exc: + # Unread offline, as a report is: not clean, and not a failure. + return [f"not read offline: {exc}"] + except upstream.UpstreamError as exc: + # Not read is not clean: the bump would refuse on the same error. + print(f"{name}: prose not read, {exc}", file=sys.stderr) + failed.add(name) + return [f"not read: {exc}"] + + def _backfill( args: argparse.Namespace, name: str, profile: dict, report: ProfileReport, path: Path, @@ -3540,7 +3584,11 @@ def _apply_writes( if not args.dry_run: path.write_bytes(target.read_bytes()) return - if not (args.rebase_refs or args.bump_commit) or _stale_prose_blocks(name, path, args): + # A backfill has just written a pin: the report's reading predates it. + known = None if args.backfill_commits else report.stale_prose + if not (args.rebase_refs or args.bump_commit) or _stale_prose_blocks( + name, path, args, known + ): return with tempfile.TemporaryDirectory() as scratch: # Always work on a copy. A dry run reports what it left there, a real @@ -3706,6 +3754,7 @@ def _print_triage( ) -> None: """Profiles ranked by drift, worst first.""" ranked = [] + sampled = 0 for report in reports: profile = selected[report.name] version = None @@ -3727,20 +3776,31 @@ def _print_triage( refused = "commit history unreachable" print(f"{report.name}: {exc}", file=sys.stderr) if len(paths) > TRIAGE_PATH_SAMPLE and not refused: - print( - f"{report.name}: commit count sampled on " - f"{TRIAGE_PATH_SAMPLE} of {len(paths)} paths", - file=sys.stderr, - ) + sampled += 1 ranked.append((drift_score(report, version, commits), report, refused)) + if sampled: + # One line for the run: a notice per profile buried the ranking + # under 254 lines of stderr. + print( + f"commit count sampled on {TRIAGE_PATH_SAMPLE} cited paths for " + f"{sampled} profiles citing more", + file=sys.stderr, + ) + print(f"{'score':>5s} {'profile':30s} state (worst first)") + pending = 0 for score, report, refused in sorted(ranked, key=lambda item: -item[0]): state = report.skipped or f"{report.needs_review()} to review" + stale = len(report.stale_prose or ()) + if stale and not report.skipped: + state += f", {stale} prose under an older pin" unchecked = (report.counts or {}).get("UNCHECKED", 0) if unchecked and not report.skipped: state += f", {unchecked} unchecked" if refused: state += f", {refused}" + pending += bool(report.needs_review()) print(f"{score:5d} {report.name:30s} {state}") + print(f"{len(ranked)} profiles, {pending} with something to review") def _refuse_flag_combinations(args: argparse.Namespace) -> None: @@ -3895,6 +3955,8 @@ def main() -> None: name=name, entries=[], counts={}, skipped=f"upstream error: {exc}" ) failed.add(name) + if not report.skipped and profile_path.is_file(): + report.stale_prose = _read_stale_prose(name, profile_path, args, failed) if args.check_version and profile.get("version_source"): report.declared_version = str(profile.get("core_version") or "") or None report.page_version = page_version(profile, args.cache_dir, args.offline) diff --git a/tests/test_profile_sync.py b/tests/test_profile_sync.py index aa852bd5..aa47269c 100644 --- a/tests/test_profile_sync.py +++ b/tests/test_profile_sync.py @@ -4002,6 +4002,25 @@ class TestOutputModesApplyTheirFlags(unittest.TestCase): code, _, _ = self._main("--strict") self.assertEqual(code, 0) + def test_strict_fails_on_prose_that_blocks_the_pin(self): + """blastem read "0 to review" while its prose refused every bump.""" + from unittest import mock + + with tempfile.TemporaryDirectory() as tmp: + for name in ("clean", "drift"): + (Path(tmp) / f"{name}.yml").write_text("emulator: x\n") + stale = {"clean": ["read again: notes: a.c:5 (pin moved)"], "drift": []} + with mock.patch.object( + profile_sync, "realign_prose", + side_effect=lambda path, *a, **k: stale[path.stem], + ): + code, out, _ = self._main( + "--changed-only", "--strict", "--emulators-dir", tmp, + ) + self.assertEqual(code, 1) + self.assertIn("clean", out) + self.assertIn("prose under an older pin", out) + def test_extras_are_refused_with_json(self): code, _, err = self._main("--json", "--check-version") self.assertEqual(code, 1) @@ -4504,5 +4523,52 @@ class ARoleIsReadPerPart(unittest.TestCase): ]) +class TheTriageCountsWhatBlocksThePin(unittest.TestCase): + """Prose written under an older pin refuses the recale and the bump, yet + the triage, --changed-only and --strict read "0 to review" on blastem, + xenia-edge and lindbergh-loader. And one stderr notice per profile put + 254 lines above an unlabelled ranking.""" + + def _report(self, name: str, paths: int, stale: list[str] | None = None): + entries = [ + EntryReport(f"f{i}", f"d/{i}.c:1", "ANCHORED", [ + PartResult(RefPart(f"d/{i}.c", 1, 1, "1"), "ANCHORED", + None, None, None, []) + ]) + for i in range(paths) + ] + return ProfileReport( + name=name, pin="p", head="h", entries=entries, + counts={"ANCHORED": paths}, stale_prose=stale, + ) + + def test_stale_prose_is_review_work(self): + report = self._report("b", 1, ["read again: notes: a.c:5 (pin moved)"]) + self.assertEqual(report.needs_review(), 1) + self.assertIn("prose under an older pin", format_report(report)) + self.assertEqual(report_to_dict(report)["needs_review"], 1) + + def test_the_ranking_is_labelled_and_the_sampling_said_once(self): + reports = [ + self._report("clean", 7), + self._report("stuck", 7, ["read again: notes: a.c:5 (pin moved)"]), + ] + selected = {r.name: {"source": "https://github.com/o/n"} for r in reports} + args = argparse.Namespace(cache_dir="unused", offline=True) + out, err = io.StringIO(), io.StringIO() + with mock.patch.object(profile_sync, "check_version", return_value=None), \ + mock.patch.object(profile_sync.upstream, "commits_touching", + return_value=0), \ + contextlib.redirect_stdout(out), contextlib.redirect_stderr(err): + profile_sync._print_triage(args, selected, reports) + lines = out.getvalue().splitlines() + self.assertIn("profile", lines[0]) + self.assertIn("stuck", lines[1]) + self.assertIn("1 to review, 1 prose under an older pin", lines[1]) + self.assertIn("0 to review", lines[2]) + self.assertEqual(lines[-1], "2 profiles, 1 with something to review") + self.assertEqual(len(err.getvalue().splitlines()), 1, err.getvalue()) + + if __name__ == "__main__": unittest.main() diff --git a/wiki/tools.md b/wiki/tools.md index 8164c5d4..fe9f1c36 100644 --- a/wiki/tools.md +++ b/wiki/tools.md @@ -415,7 +415,15 @@ not declare, so no revision can confirm it), `UNCHECKED` (a pin equal to HEAD is judged on self-consistency, and an entry with neither a hash nor a name gives that check nothing to look for). An entry carries the worst status of its parts. Only `CHANGED`, `GONE` and `AMBIGUOUS` count as -needing a re-read. A zero next to a row of `unchecked` is not a proof. +needing a re-read, with each prose citation `--realign-prose` would move or +refuse (written under an older pin, it holds the pin back). A zero next to +a row of `unchecked` is not a proof. + +A part opening with `source` or `upstream`, in a source_ref or in prose, +is read in that repository. The word belongs to the part, not to its path: +one profile can cite the same file under both. `--triage` prints a header, +ranks worst first and closes on the number of profiles with something to +review. A single cited line is often not distinctive, so the anchor widens by steps of ±3, ±6, ±12, ±25 and ±50 lines until it is unique. Three further