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..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. @@ -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..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 @@ -282,6 +287,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 +383,22 @@ 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 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() if not languages: print("No translation languages configured.") @@ -341,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)) @@ -374,7 +461,17 @@ 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 + ] + + # 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)) return 1 diff --git a/tests/test_translation_gate_since.py b/tests/test_translation_gate_since.py new file mode 100644 index 0000000..4051bd1 --- /dev/null +++ b/tests/test_translation_gate_since.py @@ -0,0 +1,140 @@ +"""--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_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 + 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 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" },