From 8d13cf943806ab6b4cbe469a14426177c651e73d Mon Sep 17 00:00:00 2001 From: Matti Airas Date: Thu, 13 Aug 2026 18:41:57 +0300 Subject: [PATCH 1/2] feat(translation-status): gate stale on the change, not the repo Whole-repository --check has a cost that only shows up in use: editing one English page marks all nine translations of it stale, so a typo fix cannot go green until nine translations land in the same pull request. And because the gate reads the repository rather than the diff, a page somebody left behind last month fails a pull request that touched no documentation at all. --since REF gates on stale only for English pages whose content differs from REF. missing, unstamped and orphaned still fail wherever they came from: they are structural, and none of them asks an author for translation work their own change did not create. What the gate passed over is printed. A green run that quietly forgave two stale translations reads exactly like a repository with none. An unresolvable REF exits 2, not 0. A gate that forgives every stale page because it could not work out which ones this change touched forgives the whole repository. Verified against hatlabs/halpi2 rather than only the fixture: an English edit made by the change fails, the same tree with that edit already committed and an unrelated working change passes and names what it skipped, and hatlabs/sh-rpi's nine missing pages still fail under --since. AGENTS.md claimed no consumer pinned the package yet. halos-org/docs has since merged its pin and four more have open pull requests. Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 19 ++++- README.md | 23 ++++- pyproject.toml | 2 +- src/halos_docs_tools/translation_status.py | 76 +++++++++++++++++ tests/test_translation_gate_since.py | 99 ++++++++++++++++++++++ uv.lock | 2 +- 6 files changed, 214 insertions(+), 7 deletions(-) create mode 100644 tests/test_translation_gate_since.py diff --git a/AGENTS.md b/AGENTS.md index a8aff0c..06ad309 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -8,11 +8,16 @@ Six documentation checkers, packaged so that a MkDocs documentation repository can pin them and get the identical code in CI and on a developer's machine. The checkers were extracted from `hatlabs/halpi2`, where they lived as `scripts/`. -Intended consumers. Neither pins the package yet; both migrations are still -open, so `hatlabs/halpi2` still runs its own copies under `scripts/`: +Consumers, all pinning a tag: -- `hatlabs/halpi2` — nine locales, full translation gate -- `halos-org/docs.halos.fi` — no translations, anchor validation only +- `hatlabs/halpi2`, `hatlabs/halmet`, `hatlabs/sh-rpi` — nine locales each, full + translation gate through `halos-org/shared-workflows` +- `hatlabs/sh-esp32` — one locale, same gate +- `halos-org/docs` (docs.halos.fi) — no translations, `check-anchors` only, + called directly from its own build job + +`halos-org/shared-workflows` calls the CLI by name and by flag, so it is a +consumer too even though it installs nothing: see `translation-status.yml`. ## The six commands @@ -32,6 +37,12 @@ Each module lives at `src/halos_docs_tools/.py` and exposes `main()`. Where a checker needs them, the path is a CLI option with a default. - The translation stamp format — `translated_from` in frontmatter, holding a git blob hash — is fixed. Consumers have thousands of pages carrying it. +- `translation-status --check` fails on `missing`, `unstamped` and `orphaned` + wherever they came from, and on `stale` only for pages changed since + `--since REF` when that is given. The split is deliberate: the first three are + structural, while gating every stale page in the repository makes an + English-only edit unmergeable until all its translations land in the same + change. Do not quietly widen either half. ## Distribution diff --git a/README.md b/README.md index 750f2c8..ced8579 100644 --- a/README.md +++ b/README.md @@ -97,9 +97,30 @@ exits non-zero when any page in any configured locale is anything but `current`, and names every entry responsible. Without `--check` the command only reports, whatever it finds. -The gate is a property of the repository, not of a pull request's diff, so `--only-pages` narrows the report and never the rule. +### Gating on the staleness this change caused + +``` +translation-status --check --since origin/main +``` + +Whole-repository `--check` has a cost that shows up the first time someone +fixes a typo: editing one English page marks every translation of it stale, so +the change cannot go green until all of them land in the same pull request. And +a page somebody left behind last month fails a pull request that touched no +documentation at all. + +`--since REF` gates on `stale` only for English pages whose content differs +from `REF`. `missing`, `unstamped` and `orphaned` still fail wherever they came +from — those are structural, and none of them asks an author for translation +work their change did not create. + +What it passed over is printed, so a green run is not mistaken for a clean +repository. If `REF` does not resolve — an unknown ref, or a clone too shallow +to hold it — the command exits 2 rather than forgiving everything it could not +measure. + One consequence is worth knowing before you meet it: adding a locale to `mkdocs.yml` makes every page `missing` in that locale immediately. A new locale therefore arrives in a single pull request, together with its pages. diff --git a/pyproject.toml b/pyproject.toml index 1984e57..bb958ba 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "halos-docs-tools" -version = "0.1.0" +version = "0.2.0" description = "Documentation checkers for HaLOS and Hat Labs MkDocs sites" readme = "README.md" requires-python = ">=3.11" diff --git a/src/halos_docs_tools/translation_status.py b/src/halos_docs_tools/translation_status.py index b43cb72..032ce58 100644 --- a/src/halos_docs_tools/translation_status.py +++ b/src/halos_docs_tools/translation_status.py @@ -282,6 +282,53 @@ def render_comment(entries: list[Entry]) -> str: ) +def changed_sources(ref: str, default: str) -> set[str] | None: + """English pages differing from `ref`, relative to the language directory. + + None when the diff could not be computed -- an unknown ref, or a clone too + shallow to contain it. The caller must not read that as "nothing changed": + a gate that forgives every stale page because it could not tell which ones + this change touched forgives the whole repository. + """ + root = DOCS / default + result = subprocess.run( + ["git", "diff", "--name-only", ref, "--", str(root)], + capture_output=True, + text=True, + ) + if result.returncode != 0: + return None + changed = set() + for line in result.stdout.splitlines(): + try: + changed.add(str(Path(line).relative_to(root))) + except ValueError: + continue + return changed + + +def render_excused(excused: list[Entry], ref: str) -> str: + """Say what the gate passed over, so a green run is not read as a clean one.""" + pages = sorted({e.page for e in excused}) + translations = f"{len(excused)} stale translation" + ( + "s" if len(excused) > 1 else "" + ) + of_pages = f"{len(pages)} page" + ("s" if len(pages) > 1 else "") + out = [ + "", + f"{translations} of {of_pages} were already stale at {ref} and were " + "not gated on:", + "", + ] + out += [f" {page}" for page in pages] + out += [ + "", + "They are still in the report above, and still block whichever change " + "edits their English source next.", + ] + return "\n".join(out) + + def render_failure(behind: list[Entry]) -> str: """Name every entry the gate is failing on. @@ -331,8 +378,19 @@ def main(argv: list[str] | None = None) -> int: help="exit non-zero when any translation is stale, missing, unstamped " "or orphaned, across the whole repository", ) + parser.add_argument( + "--since", + metavar="REF", + help="gate on stale translations only for English pages that changed " + "since REF. Missing, unstamped and orphaned still fail whatever " + "change introduced them", + ) args = parser.parse_args(argv) + if args.since and not args.check: + print("--since narrows what --check fails on, so it needs --check.") + return 2 + default, languages = configured_languages() if not languages: print("No translation languages configured.") @@ -375,6 +433,24 @@ def main(argv: list[str] | None = None) -> int: return 2 behind = [e for e in entries if e.state != "current"] + + if args.since: + changed = changed_sources(args.since, default) + if changed is None: + print( + f"\nCannot diff against '{args.since}'. The gate was asked " + "to fail only on what this change made stale, and it " + "cannot tell what that is.\n\nCheck the ref exists in this " + "clone -- a shallow checkout is the usual cause." + ) + return 2 + excused = [ + e for e in behind if e.state == "stale" and e.page not in changed + ] + if excused: + print(render_excused(excused, args.since)) + behind = [e for e in behind if e not in excused] + if behind: print(render_failure(behind)) return 1 diff --git a/tests/test_translation_gate_since.py b/tests/test_translation_gate_since.py new file mode 100644 index 0000000..2336921 --- /dev/null +++ b/tests/test_translation_gate_since.py @@ -0,0 +1,99 @@ +"""--since: gate on the staleness this change caused, not the repository's. + +Without it the gate reads the whole repository, so one edit to an English page +cannot go green without every translation of it in the same change, and a page +someone left behind last month reddens a pull request that touched no +documentation at all. + +--since narrows *stale* to the English pages that changed against a ref. +missing, unstamped and orphaned stay absolute: they are structural, and none of +them asks the author for translation work their change did not create. +""" + +from __future__ import annotations + +from conftest import DocsRepo + +from halos_docs_tools import translation_status + + +def run(*argv: str) -> int: + return translation_status.main(list(argv)) + + +def test_a_page_this_change_did_not_touch_does_not_fail(docs_repo: DocsRepo): + docs_repo.source("other.md") + docs_repo.translation("fi", "other.md") + docs_repo.translation("sv", "other.md") + base = docs_repo.commit() + + # Someone else's page went stale earlier; this change edits nothing. + docs_repo.source("other.md", "# Title\n\nEdited without translating.\n") + docs_repo.commit() + stale_base = docs_repo.git("rev-parse", "HEAD") + + docs_repo.write("unrelated.txt", "not documentation\n") + + assert run("--check") == 1 + assert run("--check", "--since", stale_base) == 0 + assert base # the earlier state is what stale_base was measured against + + +def test_a_page_this_change_edited_still_fails(docs_repo: DocsRepo): + base = docs_repo.commit() + docs_repo.source("index.md", "# Title\n\nEdited in this change.\n") + + assert run("--check", "--since", base) == 1 + + +def test_a_missing_translation_fails_however_old(docs_repo: DocsRepo): + docs_repo.source("lonely.md") + base = docs_repo.commit() + docs_repo.write("unrelated.txt", "not documentation\n") + + assert run("--check", "--since", base) == 1 + + +def test_an_unstamped_translation_fails_however_old(docs_repo: DocsRepo): + docs_repo.translation("fi", "index.md", stamp=None) + base = docs_repo.commit() + docs_repo.write("unrelated.txt", "not documentation\n") + + assert run("--check", "--since", base) == 1 + + +def test_an_orphaned_translation_fails_however_old(docs_repo: DocsRepo): + # An orphan has no English source left, so it cannot be stamped from one. + docs_repo.translation("fi", "gone.md", stamp="0" * 40) + base = docs_repo.commit() + docs_repo.write("unrelated.txt", "not documentation\n") + + assert run("--check", "--since", base) == 1 + + +def test_what_was_not_gated_on_is_stated(docs_repo: DocsRepo, capsys): + docs_repo.source("other.md") + docs_repo.translation("fi", "other.md") + docs_repo.translation("sv", "other.md") + docs_repo.source("other.md", "# Title\n\nEdited without translating.\n") + base = docs_repo.commit() + + assert run("--check", "--since", base) == 0 + out = capsys.readouterr().out + assert "other.md" in out + # A green run that quietly forgave two stale translations reads exactly + # like a repository with none. + assert "2" in out and "not gated on" in out + + +def test_a_ref_that_does_not_resolve_is_not_a_pass(docs_repo: DocsRepo, capsys): + docs_repo.source("index.md", "# Title\n\nEdited in this change.\n") + + assert run("--check", "--since", "no-such-ref") == 2 + assert "no-such-ref" in capsys.readouterr().out + + +def test_since_without_check_is_refused(docs_repo: DocsRepo, capsys): + # Silently inert would be worse: the caller believes it scoped the gate. + assert run("--since", "HEAD") == 2 + assert "--check" in capsys.readouterr().out diff --git a/uv.lock b/uv.lock index 584f1f3..e73ba25 100644 --- a/uv.lock +++ b/uv.lock @@ -13,7 +13,7 @@ wheels = [ [[package]] name = "halos-docs-tools" -version = "0.1.0" +version = "0.2.0" source = { editable = "." } dependencies = [ { name = "pyyaml" }, From 3e53cd52fa3cf35cd92a92df1c4e72c830a08e2f Mon Sep 17 00:00:00 2001 From: Matti Airas Date: Thu, 13 Aug 2026 18:50:25 +0300 Subject: [PATCH 2/2] fix(translation-status): scope the comment before rendering it The comment body is the verdict, so it has to be built from what the gate will actually fail on. render_comment ran before the --since filter, so --comment --check --since listed excused pages as failures and appended the excused report into the body -- in comment mode stdout is the body. --since now resolves before anything renders, and the excused block prints only when the output is a report rather than a comment. --since also no longer demands --check. The workflow's comment step wants the scope without the exit status, and refusing that combination was the reason the two could disagree in the first place. It is still refused on a plain report, which shows everything by design. The exit-status table promised status 2 only for check-anchors. It was already wrong -- translation-status returns 2 for no configured locales, no source pages and unclassified markdown -- and --since adds a fourth. Verified on hatlabs/halpi2 with one English page stale at the base: the body names neither that page nor the excused report, and exits 0. Found by CodeRabbit on PR #16. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 2 +- src/halos_docs_tools/translation_status.py | 65 ++++++++++++++-------- tests/test_translation_gate_since.py | 47 +++++++++++++++- 3 files changed, 88 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index ced8579..ba66f23 100644 --- a/README.md +++ b/README.md @@ -46,7 +46,7 @@ A workflow branches on these, so they are part of the interface. |:---|:---| | 0 | the check passed | | 1 | the check found problems — broken anchors, unused glossary terms, typography faults, or (with `--check`) translations that are not current | -| 2 | the check could not run: `check-anchors` was given a site directory holding no built pages | +| 2 | the check could not run, so a pass would prove nothing: `check-anchors` was given a site directory holding no built pages, or `translation-status` found no configured locales, no source pages, markdown outside every configured locale, or a `--since` ref it could not resolve | Glossaries and per-language rules stay in the documentation repository. This package brings the checkers, not the terminology. diff --git a/src/halos_docs_tools/translation_status.py b/src/halos_docs_tools/translation_status.py index 032ce58..3f85181 100644 --- a/src/halos_docs_tools/translation_status.py +++ b/src/halos_docs_tools/translation_status.py @@ -12,8 +12,13 @@ it report as stale on its own. Reports by default. With --check it also fails: any page that is not current, -in any configured locale, exits non-zero. That is a property of the repository, -so the gate ignores --only-pages, which narrows the report and not the rule. +in any configured locale, exits non-zero. --only-pages narrows the report and +never the rule. + +--since REF narrows the rule, and only for stale: it fails on the pages this +change made stale rather than on every stale page in the repository. Missing, +unstamped and orphaned stay absolute. Without it, editing one English page +cannot go green until every translation of it lands in the same change. """ from __future__ import annotations @@ -387,8 +392,11 @@ def main(argv: list[str] | None = None) -> int: ) args = parser.parse_args(argv) - if args.since and not args.check: - print("--since narrows what --check fails on, so it needs --check.") + if args.since and not (args.check or args.comment): + print( + "--since narrows what counts as a gate failure, so it needs " + "--check or --comment. The plain report shows everything by design." + ) return 2 default, languages = configured_languages() @@ -399,8 +407,29 @@ def main(argv: list[str] | None = None) -> int: return 2 if args.check else 0 entries = collect(default, languages, want_diff=args.diff or args.comment) + + # Resolved before anything is rendered. The comment body is the verdict, so + # it has to be built from what the gate will actually fail on -- naming a + # page the run passed on sends its author to translate something nobody + # asked them for. + excused: list[Entry] = [] + if args.since: + changed = changed_sources(args.since, default) + if changed is None: + print( + f"\nCannot diff against '{args.since}'. The gate was asked to " + "fail only on what this change made stale, and it cannot tell " + "what that is.\n\nCheck the ref exists in this clone -- a " + "shallow checkout is the usual cause." + ) + return 2 + excused = [e for e in entries if e.state == "stale" and e.page not in changed] + spared = {(e.language, e.page) for e in excused} + if args.comment: - print(render_comment(entries)) + print( + render_comment([e for e in entries if (e.language, e.page) not in spared]) + ) elif args.format == "markdown": only = set(args.only_pages) if args.only_pages else None print(render_markdown(entries, only)) @@ -432,24 +461,16 @@ def main(argv: list[str] | None = None) -> int: ) return 2 - behind = [e for e in entries if e.state != "current"] + behind = [ + e + for e in entries + if e.state != "current" and (e.language, e.page) not in spared + ] - if args.since: - changed = changed_sources(args.since, default) - if changed is None: - print( - f"\nCannot diff against '{args.since}'. The gate was asked " - "to fail only on what this change made stale, and it " - "cannot tell what that is.\n\nCheck the ref exists in this " - "clone -- a shallow checkout is the usual cause." - ) - return 2 - excused = [ - e for e in behind if e.state == "stale" and e.page not in changed - ] - if excused: - print(render_excused(excused, args.since)) - behind = [e for e in behind if e not in excused] + # Not in comment mode: there stdout is the comment body, and what the + # gate spared belongs in the report rather than in a verdict. + if excused and not args.comment: + print(render_excused(excused, args.since)) if behind: print(render_failure(behind)) diff --git a/tests/test_translation_gate_since.py b/tests/test_translation_gate_since.py index 2336921..4051bd1 100644 --- a/tests/test_translation_gate_since.py +++ b/tests/test_translation_gate_since.py @@ -93,7 +93,48 @@ def test_a_ref_that_does_not_resolve_is_not_a_pass(docs_repo: DocsRepo, capsys): assert "no-such-ref" in capsys.readouterr().out -def test_since_without_check_is_refused(docs_repo: DocsRepo, capsys): - # Silently inert would be worse: the caller believes it scoped the gate. +def test_since_on_a_plain_report_is_refused(docs_repo: DocsRepo, capsys): + # The report shows everything by design, so --since would change nothing. + # Silently inert is worse: the caller believes it scoped something. assert run("--since", "HEAD") == 2 - assert "--check" in capsys.readouterr().out + out = capsys.readouterr().out + assert "--check" in out and "--comment" in out + + +def test_the_comment_omits_what_the_gate_excused(docs_repo: DocsRepo, capsys): + """The comment body is the verdict, so it must agree with the verdict. + + render_comment covers every entry the gate fails on. Under --since the gate + fails on fewer, and a comment naming a page the run passed on sends its + author to translate something nobody asked them for. + """ + docs_repo.source("stale-already.md") + docs_repo.translation("fi", "stale-already.md") + docs_repo.translation("sv", "stale-already.md") + docs_repo.source("stale-already.md", "# Title\n\nEdited without translating.\n") + base = docs_repo.commit() + + assert run("--comment", "--check", "--since", base) == 0 + body = capsys.readouterr().out + assert "stale-already.md" not in body + assert "not gated on" not in body + + +def test_the_comment_can_be_scoped_without_gating(docs_repo: DocsRepo, capsys): + """The workflow's comment step wants the scope, not the exit status.""" + docs_repo.source("stale-already.md") + docs_repo.translation("fi", "stale-already.md") + docs_repo.translation("sv", "stale-already.md") + docs_repo.source("stale-already.md", "# Title\n\nEdited without translating.\n") + base = docs_repo.commit() + + assert run("--comment", "--since", base) == 0 + assert "stale-already.md" not in capsys.readouterr().out + + +def test_the_comment_still_names_what_this_change_broke(docs_repo: DocsRepo, capsys): + base = docs_repo.commit() + docs_repo.source("index.md", "# Title\n\nEdited in this change.\n") + + assert run("--comment", "--check", "--since", base) == 1 + assert "index.md" in capsys.readouterr().out