From feceb6db09e21d2420f524b5930b5084de7250db Mon Sep 17 00:00:00 2001 From: Andy Stark Date: Fri, 7 Aug 2026 12:50:46 +0100 Subject: [PATCH 1/7] DOC-6951 Add a scanner for pages that moved without an alias Renaming a content file changes its URL, and the old URL dies unless the page declares an alias for it. That is author-declared and measurably unreliable -- 611 URL-changing moves in this repo's history, only 302 aliased. It fails inconsistently even inside one commit: 155277839 moved LangCache and Agent Memory under context-engine together, LangCache got an alias, Agent Memory did not, and its old URL 404s today. So the check has to be mechanical. The scanner walks git rename records, works out which renames actually changed a published URL, and reports the ones with no alias. One implementation serves three modes -- a branch range for a PR check, a full-history sweep, and a writing fix -- deliberately, because the recurring failure on DOC-6939 has been the same rule implemented twice and then drifting apart. The whole difficulty is false positives. A naive first version reported 961 missing aliases against a true 259, and every one of the extras looked plausible in a list, which is the dangerous kind of wrong. Two thirds of that noise was Hugo bundles: index.md and _index.md both publish at the containing directory's URL, so renaming commands/lpushx/index.md to commands/lpushx.md changes no URL at all, and 519 of this repo's renames are that shape. Writing the scanner a second time is what found the rest, after a 25-URL live sample had already blessed the first version. Five things it had wrong. 49 files spell the empty key "aliases: null", which the first fixer promoted into a list containing a literal null. 104 files give aliases a bare scalar instead of a list -- a shape I had not seen at all, and my earlier "192 empty keys" figure was really 88 empty plus those 104. One file uses a folded multi-line scalar, which is valid YAML that quietly folds two intended aliases into one string, so that author's second alias has never worked; the fixer refuses that file rather than guess. content/embeds/ is excluded from the site by a build.render never cascade rather than by any naming convention, so the underscore heuristic missed 119 files. And 28 of the gaps are collisions where another page already claims the old URL, where Hugo picks a winner arbitrarily and warns -- adding those automatically would have made 28 redirects ambiguous instead of fixing them. The frontmatter edit is line-based on purpose. Round-tripping through PyYAML reorders keys alphabetically and renormalizes quoting, which would rewrite every file it touched into an unreviewable diff. Verified by running the fix over the whole corpus: 257 aliases across 203 files, every changed file still valid YAML with no empty or null entries, no new duplicate alias claims anywhere in content, and a rescan closing to the single file the fixer correctly declines. Then reverted -- the backfill is its own item and wants its own PR. Learned: writing the measurement a second time found five bugs that a live-sampled first version had already passed, and the bundle trap alone was two thirds of the false positives Constraint: --fix must skip any move whose old URL is already claimed by another page or wanted by another moved page, because Hugo resolves a duplicate alias by picking one arbitrarily and only warning Constraint: frontmatter edits here stay line-based -- a yaml.safe_load/yaml.dump round-trip reorders keys and renormalizes quoting, rewriting every touched file Constraint: whether a path is published is read off the tree via a build.render never cascade, not inferred from an underscore-prefixed directory name Gaps: no full Hugo build has been run against the fixed corpus yet, so freedom from build-time alias warnings is argued from duplicate-claim analysis rather than observed Ticket: DOC-6951 Co-Authored-By: Claude Opus 5 (1M context) --- Makefile | 9 + build/check_missing_aliases.py | 586 ++++++++++++++++++++++++++++ build/test_check_missing_aliases.py | 359 +++++++++++++++++ 3 files changed, 954 insertions(+) create mode 100644 build/check_missing_aliases.py create mode 100644 build/test_check_missing_aliases.py diff --git a/Makefile b/Makefile index 272eac0f24..5a84e35938 100644 --- a/Makefile +++ b/Makefile @@ -44,6 +44,15 @@ serve_hugo: check_page_sizes: @python3 build/check_page_sizes.py public +# Report pages that moved without gaining an alias for their old URL, so the old +# URL now 404s. Reads git history, so it needs no build. Warn-only. +check_aliases: + @python3 build/check_missing_aliases.py --all + +# The same sweep, but writing the missing aliases into frontmatter. +check_aliases_fix: + @python3 build/check_missing_aliases.py --all --fix + clean: @rm -Rf ./public/ @rm -Rf ./resources/ diff --git a/build/check_missing_aliases.py b/build/check_missing_aliases.py new file mode 100644 index 0000000000..383c534d6e --- /dev/null +++ b/build/check_missing_aliases.py @@ -0,0 +1,586 @@ +"""Report content pages that moved without gaining an alias for their old URL. + +Renaming a content file changes its published URL, and the old URL dies unless +the page declares an ``aliases:`` entry for it. That entry is author-declared +and therefore unreliable: measured over this repo's history, only 302 of 611 +URL-changing moves carry a matching alias, and it fails inconsistently even +within a single commit (``155277839`` moved LangCache and Agent Memory together; +LangCache got an alias, Agent Memory did not, and its old URL 404s today). + +This scans git history for renames, works out which ones actually changed a +published URL, and reports those with no alias. ``--fix`` writes the missing +aliases into frontmatter. + +Seven things make a naive version of this worse than useless -- a first attempt +reported 961 missing aliases against a true 259, two thirds noise, and every +false positive looked plausible in a list -- so each is handled explicitly: + +1. **Hugo bundles.** ``index.md`` (leaf) and ``_index.md`` (branch) both publish + at the containing directory's URL, so neither name appears in the URL and + renaming ``foo/index.md`` to ``foo.md`` changes nothing. 527 of this repo's + renames are of that kind -- the single largest source of false positives. +2. **Non-published directories.** ``content/embeds/`` carries a + ``build.render: never`` cascade, and the historical ``content/_embeds/`` + never reached the site either (240 renames between them). The rule is read + off the tree, not hardcoded -- and most of those files have no frontmatter, + so there would be nowhere to put an alias in any case. +3. **``url:`` frontmatter** overrides the path-derived URL. It is used on + exactly the versioned trees and nowhere else, so those are skipped. +4. **Chains.** A page moved twice must resolve to its final home. +5. **Path reuse.** An old URL may be occupied by a different page today, and + must never be redirected (22 cases). +6. **Declared-but-not-a-list aliases.** 88 files declare the key with no value + (49 spelled ``null``, 39 bare) and 104 give it a bare scalar rather than a + list, so a check that assumes a list silently under-reports. One more uses a + folded multi-line scalar, which is valid YAML that silently folds two + intended aliases into one string, so ``--fix`` declines that file. +7. **Collisions.** 28 of the gaps name a URL another page already claims as its + own alias. Hugo resolves that by picking one arbitrarily and warning, so + adding the alias unattended would make the redirect ambiguous rather than + fix it. Those are reported for a human and never auto-fixed. + +Warn-only by default (exit 0), like check_page_sizes; pass ``--fail`` to make CI +block on offenders. + +See DOC-6951. +""" + +# `X | None` annotations are 3.10+; local dev machines are still on 3.9. +from __future__ import annotations + +import argparse +import json +import logging +import os +import re +import subprocess +import sys +from dataclasses import dataclass, field + +logger = logging.getLogger("check_missing_aliases") + +CONTENT = "content" + +# A path segment that looks like a semver version marks the versioned trees, +# which set `url:` in frontmatter and so cannot have their URL derived from +# their path. Matches two- and three-component versions (7.8, 0.10.0). +VERSIONED = re.compile(r"/[0-9]+\.[0-9]+(\.[0-9]+)?/") + +# git's default rename-detection similarity. Measured on this repo: 20% finds +# 633 URL-changing moves, 50% finds 611, 90% finds 489 -- so the default is +# close to the ceiling, and the tail that reads as delete-plus-add rather than a +# rename (a file renamed and heavily rewritten at once) is about 3.5%. +DEFAULT_THRESHOLD = 50 + +# Leading slash is effectively universal in this repo (918 of 929 entries). +# Trailing slash is a genuine 54/46 split with no house convention, so --fix +# picks one and stays consistent rather than guessing per file. +ALIAS_TEMPLATE = "/{url}/" + +# YAML spellings of "this key has no value". 49 files write `aliases: null` and +# 39 leave the key bare; both must be treated as empty, not as a one-item list +# containing the string "null". +NO_VALUE = ("", "null", "~") + + +@dataclass +class Move: + """A rename that changed a page's published URL.""" + + old_path: str + new_path: str + old_url: str + new_url: str + date: str + commit: str + aliased: bool = False + occupied: bool = False + collides_with: list[str] = field(default_factory=list) + + @property + def actionable(self) -> bool: + """True when the alias can be added safely and without a judgment call.""" + return not (self.aliased or self.occupied or self.collides_with) + + +@dataclass +class FileFix: + """Aliases to add to one file.""" + + path: str + aliases: list[str] = field(default_factory=list) + + +def parse_args() -> argparse.Namespace: + parser = argparse.ArgumentParser( + description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) + scope = parser.add_mutually_exclusive_group() + scope.add_argument("--range", dest="rev_range", default="origin/main..HEAD", + help="revision range to scan (default: origin/main..HEAD)") + scope.add_argument("--all", action="store_true", + help="scan the whole history instead of a range") + parser.add_argument("--threshold", type=int, default=DEFAULT_THRESHOLD, + help=f"rename similarity %% (default: {DEFAULT_THRESHOLD})") + parser.add_argument("--fix", action="store_true", + help="write the missing aliases into frontmatter") + parser.add_argument("--json", dest="json_out", metavar="PATH", + help="also write the findings as JSON") + parser.add_argument("--github", action="store_true", + help="emit GitHub Actions warning annotations") + parser.add_argument("--fail", action="store_true", + help="exit non-zero if any move is missing an alias") + return parser.parse_args() + + +def git(*args: str) -> str: + return subprocess.run(["git", *args], capture_output=True, text=True, + check=True).stdout + + +# --------------------------------------------------------------------------- # +# path -> URL +# --------------------------------------------------------------------------- # + +def to_url(path: str) -> str: + """Derive a page's URL path from its content path. + + Hugo bundles are the trap here: ``_index.md`` (branch) and ``index.md`` + (leaf) both publish at the containing directory's URL, so neither name + appears in the URL. + """ + rel = path[len(CONTENT) + 1:] + rel = re.sub(r"\.md$", "", rel) + rel = re.sub(r"/_?index$", "", rel) + return "" if rel in ("_index", "index") else rel + + +_render_never: set[str] | None = None + + +def render_never_roots() -> set[str]: + """Content directories Hugo is told never to render. + + ``content/embeds/_index.md`` sets ``build.render: never`` with a ``cascade``, + so none of the 119 fragment files beneath it is published -- they are pulled + in by the ``embed-yaml`` shortcode instead, and most have no frontmatter at + all, so there is nowhere to put an alias even if one were wanted. Derived + from the tree rather than hardcoded, so a new one is picked up for free. + """ + global _render_never + if _render_never is not None: + return _render_never + + import yaml + + roots: set[str] = set() + try: + candidates = git("grep", "-l", "render: never", "--", CONTENT).splitlines() + except subprocess.CalledProcessError: + candidates = [] # git grep exits 1 when nothing matches + for path in candidates: + if not path.endswith("_index.md"): + continue + try: + lines = read_lines(path) + except OSError: + continue + bounds = frontmatter_bounds(lines) + if not bounds: + continue + try: + data = yaml.safe_load("".join(lines[1:bounds[1]])) or {} + except yaml.YAMLError: + continue + if not isinstance(data, dict): + continue + cascade = data.get("cascade") or {} + build = cascade.get("build") if isinstance(cascade, dict) else None + if isinstance(build, dict) and str(build.get("render")) == "never": + roots.add(os.path.dirname(path)[len(CONTENT) + 1:] + "/") + _render_never = roots + return roots + + +def is_published(path: str) -> bool: + """False for content Hugo never publishes as a page of its own.""" + rel = path[len(CONTENT) + 1:] + if any(d.startswith("_") for d in rel.split("/")[:-1]): + return False # e.g. the historical content/_embeds/ + return not any(rel.startswith(root) for root in render_never_roots()) + + +def is_versioned(path: str) -> bool: + return bool(VERSIONED.search(path)) + + +def eligible(path: str) -> bool: + return (path.startswith(CONTENT + "/") and path.endswith(".md") + and is_published(path) and not is_versioned(path)) + + +def norm(url: str) -> str: + return url.strip().strip("/").lower() + + +# --------------------------------------------------------------------------- # +# frontmatter +# --------------------------------------------------------------------------- # + +def frontmatter_bounds(lines: list[str]) -> tuple[int, int] | None: + """Return (first, last) line indices of the ``---`` fences, or None.""" + if not lines or lines[0].rstrip("\n") != "---": + return None + for i in range(1, len(lines)): + if lines[i].rstrip("\n") == "---": + return 0, i + return None + + +def read_lines(path: str) -> list[str]: + with open(path, encoding="utf-8") as handle: + return handle.readlines() + + +def declared_aliases(path: str) -> set[str]: + """Every alias the file declares, normalized for comparison. + + Parsed with PyYAML rather than by hand: the repo uses block lists, + single-line inline lists, and multi-line inline lists, and a regex that + misses one of them silently under-reports. + """ + try: + lines = read_lines(path) + except OSError: + return set() + bounds = frontmatter_bounds(lines) + if not bounds: + return set() + import yaml # local import: only the alias path needs it + + try: + data = yaml.safe_load("".join(lines[1:bounds[1]])) or {} + except yaml.YAMLError: + logger.warning(" ! %s: frontmatter is not valid YAML, skipping", path) + return set() + if not isinstance(data, dict): + return set() + # Hugo frontmatter keys are case-insensitive. + values = next((v for k, v in data.items() if str(k).lower() == "aliases"), None) + if values is None: + return set() + if isinstance(values, str): + values = [values] + if not isinstance(values, list): + return set() + return {norm(str(v)) for v in values if v is not None and str(v).strip()} + + +# --------------------------------------------------------------------------- # +# finding moves +# --------------------------------------------------------------------------- # + +def find_moves(rev_range: str | None, threshold: int) -> list[Move]: + """Renames in the given range, chained so each page resolves to its final home.""" + args = ["log", "--reverse", f"--find-renames={threshold}%", "--diff-filter=R", + "--name-status", "--format=COMMIT\t%H\t%ad", "--date=short"] + if rev_range: + args.append(rev_range) + args += ["--", CONTENT] + try: + out = git(*args) + except subprocess.CalledProcessError as exc: + logger.error("check_missing_aliases: git log failed for range %r.\n%s", + rev_range, exc.stderr.strip()) + raise + + # path-as-it-stands-now -> the (old_path, date, commit) records behind it + history: dict[str, set[tuple[str, str, str]]] = {} + commit = date = "" + for line in out.splitlines(): + if line.startswith("COMMIT\t"): + _, commit, date = line.split("\t") + continue + parts = line.split("\t") + if len(parts) != 3 or not parts[0].startswith("R"): + continue + _, old, new = parts + if not (eligible(old) and eligible(new)): + continue + history[new] = history.pop(old, set()) | {(old, date, commit)} + + tracked = set(git("ls-files", CONTENT).splitlines()) + moves: list[Move] = [] + for new_path, records in history.items(): + if new_path not in tracked: + continue # moved, then later deleted -- nothing to redirect to + new_url = to_url(new_path) + for old_path, date, commit in records: + old_url = to_url(old_path) + if norm(old_url) == norm(new_url): + continue # a bundle rename, or otherwise URL-preserving + moves.append(Move(old_path=old_path, new_path=new_path, + old_url=old_url, new_url=new_url, + date=date, commit=commit[:9])) + moves.sort(key=lambda m: (m.date, m.old_url)) + return moves + + +def published_urls() -> set[str]: + """Normalized URLs of every page published today.""" + return {norm(to_url(p)) for p in git("ls-files", CONTENT).splitlines() + if eligible(p)} + + +def alias_owners() -> dict[str, set[str]]: + """Every alias currently declared anywhere in content, mapped to its owners.""" + owners: dict[str, set[str]] = {} + try: + candidates = git("grep", "-l", "-E", "^aliases:", "--", CONTENT).splitlines() + except subprocess.CalledProcessError: + return owners + for path in candidates: + for alias in declared_aliases(path): + owners.setdefault(alias, set()).add(path) + return owners + + +def classify(moves: list[Move]) -> None: + """Mark each move as aliased, occupied by a live page, or colliding. + + A collision is the trap that has no safe automatic answer: Hugo resolves two + pages claiming the same alias by picking one and emitting a warning, so + adding the alias would quietly make the redirect ambiguous rather than fix + it. 26 of this repo's gaps are collisions -- 24 where another page already + claims the URL, and 2 where two moved pages both want it. + """ + current = published_urls() + owners = alias_owners() + alias_cache: dict[str, set[str]] = {} + + for move in moves: + if move.new_path not in alias_cache: + alias_cache[move.new_path] = declared_aliases(move.new_path) + move.aliased = norm(move.old_url) in alias_cache[move.new_path] + move.occupied = norm(move.old_url) in current + if not (move.aliased or move.occupied): + claimed = owners.get(norm(move.old_url), set()) - {move.new_path} + move.collides_with = sorted(claimed) + + # Two moved pages wanting the same alias collide with each other, which no + # amount of looking at existing frontmatter would reveal. + wanted: dict[str, set[str]] = {} + for move in moves: + if move.actionable: + wanted.setdefault(norm(move.old_url), set()).add(move.new_path) + for move in moves: + rivals = wanted.get(norm(move.old_url), set()) - {move.new_path} + if move.actionable and rivals: + move.collides_with = sorted(rivals) + + +# --------------------------------------------------------------------------- # +# --fix +# --------------------------------------------------------------------------- # + +def insert_aliases(lines: list[str], new_aliases: list[str]) -> list[str] | None: + """Add aliases to a file's frontmatter, editing line by line. + + Deliberately not a YAML round-trip: ``yaml.safe_load`` followed by + ``yaml.dump`` reorders keys alphabetically and renormalizes quoting, which + would rewrite the frontmatter of every file it touched into an unreviewable + diff. This preserves everything it does not need to change. + """ + bounds = frontmatter_bounds(lines) + if not bounds: + return None + _, close = bounds + + key = None + for i in range(1, close): + if re.match(r"(?i)aliases[ \t]*:", lines[i]): + key = i + break + + if key is None: + # No aliases key at all: add one just above the closing fence. + block = ["aliases:\n"] + [f"- {a}\n" for a in new_aliases] + return lines[:close] + block + lines[close:] + + rest = lines[key].split(":", 1)[1].strip() + indent = re.match(r"[ \t]*", lines[key]).group(0) + + if rest.startswith("[") and rest.endswith("]") and len(rest) > 1: + # Single-line inline list: aliases: [/a/, /b/] + inner = rest[1:-1].strip().rstrip(",").strip() + items = ([inner] if inner else []) + new_aliases + lines = list(lines) + lines[key] = f"{indent}aliases: [{', '.join(items)}]\n" + return lines + + if rest == "[": + # Multi-line inline list: find its closing bracket. + for j in range(key + 1, close): + if lines[j].strip().startswith("]"): + item_indent = (re.match(r"[ \t]*", lines[key + 1]).group(0) + if j > key + 1 else indent + " ") + block = [f"{item_indent}{a},\n" for a in new_aliases] + return lines[:j] + block + lines[j:] + return None + + # A YAML folded scalar continued on the next line: + # + # aliases: /a/ + # /b/ + # + # This is valid YAML but reads as the single string "/a/ /b/", so the + # author's second alias never worked. Editing only the first line would + # leave the continuation dangling and break the frontmatter outright, so + # refuse it and let a human fix the underlying content bug. One file today. + following = lines[key + 1] if key + 1 < close else "" + if (following[:1] in (" ", "\t") + and not re.match(r"[ \t]*-[ \t]*\S", following) + and not re.match(r"[ \t]*\S+[ \t]*:", following)): + return None + + if rest.lower() in NO_VALUE: + # A block list, or the key with no value. 49 files write `aliases: null` + # and 39 leave it bare; YAML reads both as absent, so the placeholder is + # dropped rather than carried into the list as a literal "null" entry. + lines = list(lines) + if rest: + lines[key] = f"{indent}aliases:\n" + last = key + for j in range(key + 1, close): + if re.match(r"[ \t]*-[ \t]*\S", lines[j]): + last = j + elif lines[j].strip() == "": + continue + else: + break + item_indent = re.match(r"[ \t]*", lines[last]).group(0) if last != key else indent + block = [f"{item_indent}- {a}\n" for a in new_aliases] + return lines[:last + 1] + block + lines[last + 1:] + + # A scalar value (aliases: /a/) -- 104 files. Promote it to an inline list. + # Some of those carry a stray trailing comma from an author writing a list + # without brackets, which would otherwise become an empty list entry. + existing = rest.rstrip(",").strip() + items = ([existing] if existing else []) + new_aliases + lines = list(lines) + lines[key] = f"{indent}aliases: [{', '.join(items)}]\n" + return lines + + +def apply_fixes(moves: list[Move]) -> tuple[int, int]: + """Write missing aliases into frontmatter. Returns (files, aliases) changed.""" + by_file: dict[str, FileFix] = {} + for move in moves: + if not move.actionable: + continue + fix = by_file.setdefault(move.new_path, FileFix(path=move.new_path)) + alias = ALIAS_TEMPLATE.format(url=norm(move.old_url)) + if alias not in fix.aliases: + fix.aliases.append(alias) + + files = aliases = 0 + for fix in by_file.values(): + if not os.path.exists(fix.path): + logger.warning(" ! %s no longer exists, skipping", fix.path) + continue + existing = declared_aliases(fix.path) + wanted = [a for a in fix.aliases if norm(a) not in existing] + if not wanted: + continue + lines = read_lines(fix.path) + updated = insert_aliases(lines, wanted) + if updated is None: + logger.warning(" ! %s: could not place aliases, skipping", fix.path) + continue + with open(fix.path, "w", encoding="utf-8") as handle: + handle.writelines(updated) + files += 1 + aliases += len(wanted) + logger.info(" + %s", fix.path) + for alias in wanted: + logger.info(" %s", alias) + return files, aliases + + +# --------------------------------------------------------------------------- # +# reporting +# --------------------------------------------------------------------------- # + +def report(moves: list[Move], github: bool, fix_hint: str) -> list[Move]: + missing = [m for m in moves if m.actionable] + occupied = [m for m in moves if not m.aliased and m.occupied] + collisions = [m for m in moves if not m.aliased and not m.occupied and m.collides_with] + aliased = [m for m in moves if m.aliased] + + logger.info("check_missing_aliases: %d URL-changing move(s) found.", len(moves)) + if moves: + logger.info(" %d already aliased, %d missing an alias, %d skipped " + "(old URL is a live page), %d need a decision (collision).", + len(aliased), len(missing), len(occupied), len(collisions)) + + if occupied: + logger.info("Skipped -- old URL currently resolves, so must not redirect:") + for move in occupied: + logger.info(" %s %s", move.date, move.old_url) + + if collisions: + logger.warning("Needs a human decision -- another page already claims " + "this URL, so Hugo would pick one arbitrarily:") + for move in collisions: + logger.warning(" %s %s", move.date, move.old_url) + logger.warning(" wanted by %s", move.new_path) + for owner in move.collides_with: + logger.warning(" claimed by %s", owner) + + if missing: + logger.warning("Moved with no alias for the old URL:") + for move in missing: + logger.warning(" %s %s %s", move.date, move.commit, move.old_url) + logger.warning(" now at %s", move.new_url) + logger.warning(" add to %s: %s", move.new_path, + ALIAS_TEMPLATE.format(url=norm(move.old_url))) + if github: + print(f"::warning file={move.new_path}::Page moved from " + f"/{norm(move.old_url)}/ with no alias. Add " + f"'{ALIAS_TEMPLATE.format(url=norm(move.old_url))}' to its " + f"aliases, or run: make check_aliases_fix") + logger.warning("Fix them all with: %s", fix_hint) + return missing + + +def main() -> int: + logging.basicConfig(level=logging.INFO, format="%(message)s") + args = parse_args() + + rev_range = None if args.all else args.rev_range + try: + moves = find_moves(rev_range, args.threshold) + except subprocess.CalledProcessError: + return 1 + classify(moves) + + fix_hint = ("make check_aliases_fix" if args.all else + f"python3 build/check_missing_aliases.py --range {args.rev_range} --fix") + missing = report(moves, args.github, fix_hint) + + if args.json_out: + with open(args.json_out, "w", encoding="utf-8") as handle: + json.dump([m.__dict__ for m in moves], handle, indent=1) + logger.info("Wrote %s", args.json_out) + + if args.fix and missing: + logger.info("Adding %d alias(es):", len(missing)) + files, aliases = apply_fixes(moves) + logger.info("check_missing_aliases: added %d alias(es) across %d file(s).", + aliases, files) + return 0 + + return 1 if (missing and args.fail) else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/build/test_check_missing_aliases.py b/build/test_check_missing_aliases.py new file mode 100644 index 0000000000..6cd7aa87e2 --- /dev/null +++ b/build/test_check_missing_aliases.py @@ -0,0 +1,359 @@ +#!/usr/bin/env python3 +"""Tests for check_missing_aliases. + +The interesting logic is ``insert_aliases``, which edits frontmatter line by +line rather than round-tripping the YAML. It has to cope with every alias shape +already in the repo -- block lists (490 files), single-line inline lists (44), +multi-line inline lists (19), the key declared with no value (192), and no key +at all -- while leaving every other line untouched. + +Run with ``pytest build/test_check_missing_aliases.py`` or directly. +""" + +import os +import sys + +sys.path.insert(0, os.path.dirname(__file__)) + +from check_missing_aliases import ( # noqa: E402 + Move, declared_aliases, eligible, insert_aliases, is_published, is_versioned, + render_never_roots, to_url, +) + + +def apply(text: str, aliases: list) -> str: + result = insert_aliases(text.splitlines(keepends=True), aliases) + assert result is not None, "insert_aliases refused the frontmatter" + return "".join(result) + + +# --------------------------------------------------------------------------- # +# path -> URL +# --------------------------------------------------------------------------- # + +def test_to_url_strips_both_bundle_names(): + # The trap that produced 519 false positives: both bundle names publish at + # the containing directory's URL, so renaming between them changes nothing. + assert to_url("content/commands/lpushx/index.md") == "commands/lpushx" + assert to_url("content/commands/lpushx.md") == "commands/lpushx" + assert to_url("content/develop/data-types/strings/_index.md") == "develop/data-types/strings" + assert to_url("content/develop/data-types/strings.md") == "develop/data-types/strings" + + +def test_to_url_handles_site_root(): + assert to_url("content/_index.md") == "" + + +def test_underscore_directories_are_not_published(): + assert not is_published("content/_embeds/k8s/rerc.md") + assert is_published("content/develop/clients/observability.md") + + +def test_render_never_cascade_is_read_off_the_tree(): + # content/embeds/_index.md sets build.render: never with a cascade, so its + # 119 fragment files are not pages. The underscore heuristic alone misses + # this directory because its name has no underscore. + assert "embeds/" in render_never_roots() + assert not is_published("content/embeds/k8s/rec.md") + assert not is_published("content/embeds/_index.md") + + +def test_versioned_paths_are_detected(): + assert is_versioned("content/operate/rs/7.8/references/rest-api.md") + assert is_versioned("content/develop/ai/redisvl/0.10.0/api/cache.md") + assert not is_versioned("content/operate/rs/references/rest-api.md") + + +def test_eligible_rejects_the_excluded_classes(): + assert eligible("content/develop/ai/langcache/_index.md") + assert not eligible("content/_embeds/k8s/rerc.md") + assert not eligible("content/operate/rs/7.8/index.md") + assert not eligible("content/develop/ai/langcache/api-reference/api.yaml") + + +def test_only_safe_moves_are_actionable(): + def move(**kwargs): + return Move(old_path="content/a.md", new_path="content/b.md", + old_url="a", new_url="b", date="2026-01-01", commit="abc", + **kwargs) + + assert move().actionable + assert not move(aliased=True).actionable + assert not move(occupied=True).actionable + # A collision has no safe automatic answer: Hugo would pick one of the two + # claimants arbitrarily, so the alias must not be added unattended. + assert not move(collides_with=["content/other.md"]).actionable + + +# --------------------------------------------------------------------------- # +# insert_aliases -- one test per shape found in the repo +# --------------------------------------------------------------------------- # + +def test_block_list_appends_after_last_item(): + before = """--- +title: Bitmaps +aliases: +- /data-types/bitmaps/ +- /manual/data-types/bitmaps/ +weight: 10 +--- + +Body text. +""" + after = apply(before, ["/develop/data-types/bitmaps/"]) + assert after == """--- +title: Bitmaps +aliases: +- /data-types/bitmaps/ +- /manual/data-types/bitmaps/ +- /develop/data-types/bitmaps/ +weight: 10 +--- + +Body text. +""" + + +def test_single_line_inline_list_grows_in_place(): + before = """--- +title: Architecture +aliases: [/operate/kubernetes/architecture/] +weight: 5 +--- +Body. +""" + after = apply(before, ["/kubernetes/architecture/"]) + assert ("aliases: [/operate/kubernetes/architecture/, " + "/kubernetes/architecture/]\n") in after + assert "weight: 5\n" in after + + +def test_multi_line_inline_list_gains_a_line_before_the_bracket(): + before = """--- +title: Delete custom resources +aliases: [ + /operate/kubernetes/re-clusters/delete-custom-resources/, +] +weight: 7 +--- +Body. +""" + after = apply(before, ["/kubernetes/delete-custom-resources/"]) + assert after == """--- +title: Delete custom resources +aliases: [ + /operate/kubernetes/re-clusters/delete-custom-resources/, + /kubernetes/delete-custom-resources/, +] +weight: 7 +--- +Body. +""" + + +def test_empty_aliases_key_gains_the_first_item(): + # 192 files in the repo declare the key with no value. + before = """--- +aliases: +categories: +- docs +title: Quantization +--- +Body. +""" + after = apply(before, ["/develop/ai/search-and-query/vectors/svs-compression/"]) + assert after == """--- +aliases: +- /develop/ai/search-and-query/vectors/svs-compression/ +categories: +- docs +title: Quantization +--- +Body. +""" + + +def test_explicit_null_is_dropped_not_kept_as_an_item(): + # 49 files spell the empty key `aliases: null`. An earlier version promoted + # it to `[null, /new/]`, which would have published an alias called "null". + before = """--- +aliases: null +title: Data transformation +--- +Body. +""" + after = apply(before, ["/integrate/redis-data-integration/data-transformation/"]) + assert after == """--- +aliases: +- /integrate/redis-data-integration/data-transformation/ +title: Data transformation +--- +Body. +""" + assert "null" not in after + + +def test_scalar_value_is_promoted_to_a_list(): + before = """--- +aliases: /develop/connect/clients/dotnet +title: .NET +--- +Body. +""" + after = apply(before, ["/develop/clients/dotnet/"]) + assert ("aliases: [/develop/connect/clients/dotnet, " + "/develop/clients/dotnet/]\n") in after + + +def test_scalar_with_a_stray_trailing_comma_does_not_create_an_empty_item(): + # Real frontmatter in the repo: an author wrote a list without brackets, so + # the value is the string "/path/7-4-6-2,". Left alone it would become + # `[/path/7-4-6-2,, /new/]` -- a double comma, i.e. an empty alias. + before = """--- +weight: 29 +aliases: /operate/kubernetes/release-notes/7-4-6-2, +--- +Body. +""" + after = apply(before, ["/operate/kubernetes/release-notes/7-4-6-2/"]) + assert ",," not in after + assert ("aliases: [/operate/kubernetes/release-notes/7-4-6-2, " + "/operate/kubernetes/release-notes/7-4-6-2/]\n") in after + + +def test_missing_key_is_added_above_the_closing_fence(): + before = """--- +Title: Redis Agent Memory +linkTitle: Agent Memory +weight: 20 +--- + +Give your AI agents persistent memory. +""" + after = apply(before, ["/develop/ai/agent-memory/"]) + assert after == """--- +Title: Redis Agent Memory +linkTitle: Agent Memory +weight: 20 +aliases: +- /develop/ai/agent-memory/ +--- + +Give your AI agents persistent memory. +""" + + +def test_several_aliases_are_added_at_once(): + before = """--- +title: Thing +aliases: +- /old/one/ +--- +Body. +""" + after = apply(before, ["/old/two/", "/old/three/"]) + assert after.count("- /old/") == 3 + assert after.index("/old/two/") < after.index("/old/three/") + + +def test_body_is_never_touched(): + # A body containing something that looks like frontmatter must survive. + before = """--- +title: Thing +weight: 1 +--- + +Some prose. + +--- + +aliases: not-really-frontmatter + +More prose. +""" + after = apply(before, ["/old/thing/"]) + assert after.endswith("aliases: not-really-frontmatter\n\nMore prose.\n") + assert after.count("aliases:") == 2 + + +def test_no_frontmatter_is_refused_rather_than_guessed(): + assert insert_aliases(["Just a body.\n"], ["/old/"]) is None + + +def test_folded_multiline_scalar_is_refused(): + # Real frontmatter in the repo. Valid YAML, but it folds to the single + # string "/a/ /b/" so the second alias never worked. Rewriting only the + # first line would leave the continuation dangling and break the file, so + # the fixer must decline rather than guess the author's intent. + before = """--- +weight: 20 +aliases: /operate/search/scalable-search/ + /operate/search/query-performance-factor/ +--- +Body. +""" + assert insert_aliases(before.splitlines(keepends=True), ["/new/"]) is None + + +# --------------------------------------------------------------------------- # +# declared_aliases -- parsing every shape back out again +# --------------------------------------------------------------------------- # + +def test_declared_aliases_reads_every_shape(): + import tempfile + + shapes = { + "block": "---\naliases:\n- /a/\n- /b/\n---\nx\n", + "inline": "---\naliases: [/a/, /b/]\n---\nx\n", + "multiline": "---\naliases: [\n /a/,\n /b/,\n]\n---\nx\n", + "scalar": "---\naliases: /a/\n---\nx\n", + "empty": "---\naliases:\n---\nx\n", + "absent": "---\ntitle: t\n---\nx\n", + "uppercase": "---\nAliases:\n- /a/\n---\nx\n", + } + expected = { + "block": {"a", "b"}, "inline": {"a", "b"}, "multiline": {"a", "b"}, + "scalar": {"a"}, "empty": set(), "absent": set(), "uppercase": {"a"}, + } + with tempfile.TemporaryDirectory() as tmp: + for name, text in shapes.items(): + path = os.path.join(tmp, f"{name}.md") + with open(path, "w", encoding="utf-8") as handle: + handle.write(text) + assert declared_aliases(path) == expected[name], name + + +def test_round_trip_every_shape(): + """Whatever we insert must be readable back as an alias.""" + import tempfile + + shapes = [ + "---\naliases:\n- /a/\n---\nx\n", + "---\naliases: [/a/]\n---\nx\n", + "---\naliases: [\n /a/,\n]\n---\nx\n", + "---\naliases:\ntitle: t\n---\nx\n", + "---\naliases: null\ntitle: t\n---\nx\n", + "---\naliases: /a/\ntitle: t\n---\nx\n", + "---\naliases: /a/, \ntitle: t\n---\nx\n", + "---\ntitle: t\n---\nx\n", + ] + with tempfile.TemporaryDirectory() as tmp: + for i, text in enumerate(shapes): + path = os.path.join(tmp, f"s{i}.md") + with open(path, "w", encoding="utf-8") as handle: + handle.write(apply(text, ["/new/one/"])) + assert "new/one" in declared_aliases(path), text + + +if __name__ == "__main__": + failures = 0 + for name, fn in sorted(list(globals().items())): + if name.startswith("test_") and callable(fn): + try: + fn() + print(f" ok {name}") + except AssertionError as exc: + failures += 1 + print(f" FAIL {name}: {exc}") + print(f"\n{failures} failure(s)") + sys.exit(1 if failures else 0) From eea076141a924e13e8d0509ced7d98e039340e04 Mon Sep 17 00:00:00 2001 From: Andy Stark Date: Fri, 7 Aug 2026 13:19:00 +0100 Subject: [PATCH 2/7] DOC-6951 Fix silent skips and duplicate redirects in the alias scanner Both Bugbot findings were real, and one of them led somewhere it had not pointed. The first is the one that mattered. With --fix, main returned 0 whether or not apply_fixes had managed to place every alias, so the weekly sweep planned as item C3 would have opened a pull request captioned as a complete fix while leaving files untouched. There is already one such file, the folded multi-line scalar the fixer declines. apply_fixes now returns what it skipped, main lists those files, and --fail exits 1. The second was assigning history[new] rather than merging, which silently drops an earlier rename chain if a second file later moves onto the same path. Measured across the whole history that happens exactly once, and in a degenerate form where the dropped record carries the same old_path, so nothing was actually being lost. Merging anyway, because the loss would be invisible and this script exists to stop precisely that class of thing. Retaining that record is what exposed the real problem underneath. It pushed the totals to 612 moves and 29 collisions, and chasing the extra record showed that 14 old_url-to-new_path pairs arrive twice -- 13 of them through chains, so predating the merge fix entirely. A page moved away and back, or a recurring rename like the monthly Cloud changelog, produces two records for one redirect. A redirect is identified by where it comes from and where it goes, so those are now deduplicated, keeping the earliest. That moves the published figures: 598 URL-changing moves rather than 611, 290 already aliased rather than 302, 258 safely fixable rather than 259. Collisions are unchanged at 28, being 24 where another page already claims the URL and 4 where two moved pages both want it. Every figure in the module docstring was re-measured rather than adjusted by hand, since they had already drifted once. Learned: a review bot found a latent silent-drop and an automation trap in code that unit tests and a full-corpus dry run had both passed, and fixing the smaller one surfaced a larger duplicate-redirect bug that predated it Constraint: a redirect is identified by the pair of old URL and target path, so moves are deduplicated on that pair -- chains and recurring renames otherwise report one redirect twice Constraint: --fix must not exit 0 when it skipped a file, or an automated sweep reports a complete fix it did not make Ticket: DOC-6951 Co-Authored-By: Claude Opus 5 (1M context) --- build/check_missing_aliases.py | 72 ++++++++++++++++++++++------- build/test_check_missing_aliases.py | 5 +- 2 files changed, 59 insertions(+), 18 deletions(-) diff --git a/build/check_missing_aliases.py b/build/check_missing_aliases.py index 383c534d6e..878330b93f 100644 --- a/build/check_missing_aliases.py +++ b/build/check_missing_aliases.py @@ -2,7 +2,7 @@ Renaming a content file changes its published URL, and the old URL dies unless the page declares an ``aliases:`` entry for it. That entry is author-declared -and therefore unreliable: measured over this repo's history, only 302 of 611 +and therefore unreliable: measured over this repo's history, only 290 of 598 URL-changing moves carry a matching alias, and it fails inconsistently even within a single commit (``155277839`` moved LangCache and Agent Memory together; LangCache got an alias, Agent Memory did not, and its old URL 404s today). @@ -12,8 +12,9 @@ aliases into frontmatter. Seven things make a naive version of this worse than useless -- a first attempt -reported 961 missing aliases against a true 259, two thirds noise, and every -false positive looked plausible in a list -- so each is handled explicitly: +reported 961 missing aliases against a true 258, nearly three quarters of it +noise, and every false positive looked plausible in a list -- so each of the +seven is handled explicitly: 1. **Hugo bundles.** ``index.md`` (leaf) and ``_index.md`` (branch) both publish at the containing directory's URL, so neither name appears in the URL and @@ -67,7 +68,7 @@ VERSIONED = re.compile(r"/[0-9]+\.[0-9]+(\.[0-9]+)?/") # git's default rename-detection similarity. Measured on this repo: 20% finds -# 633 URL-changing moves, 50% finds 611, 90% finds 489 -- so the default is +# 617 URL-changing moves, 50% finds 598, 90% finds 477 -- so the default is # close to the ceiling, and the tail that reads as delete-plus-add rather than a # rename (a file renamed and heavily rewritten at once) is about 3.5%. DEFAULT_THRESHOLD = 50 @@ -306,7 +307,17 @@ def find_moves(rev_range: str | None, threshold: int) -> list[Move]: _, old, new = parts if not (eligible(old) and eligible(new)): continue - history[new] = history.pop(old, set()) | {(old, date, commit)} + # Merge rather than assign. If a second file later renames onto a path + # that already carries a chain -- possible once the first occupant has + # been deleted rather than moved -- assigning would drop the earlier + # records silently. That happens once in this repo's history, and in a + # degenerate form where the dropped record has the same old_path, so + # merging changes nothing today. It is here because the loss would be + # invisible, and any real ambiguity it surfaces is caught downstream by + # the collision check rather than acted on. + carried = history.pop(old, set()) + history.setdefault(new, set()).update(carried) + history[new].add((old, date, commit)) tracked = set(git("ls-files", CONTENT).splitlines()) moves: list[Move] = [] @@ -321,8 +332,21 @@ def find_moves(rev_range: str | None, threshold: int) -> list[Move]: moves.append(Move(old_path=old_path, new_path=new_path, old_url=old_url, new_url=new_url, date=date, commit=commit[:9])) + + # A redirect is identified by where it comes from and where it goes, so the + # same pair reached by two routes -- a page moved away and back, or a + # recurring rename like the monthly changelog -- is one redirect, not two. + # 14 pairs in this repo's history arrive twice. Keep the earliest. moves.sort(key=lambda m: (m.date, m.old_url)) - return moves + seen: set[tuple[str, str]] = set() + unique: list[Move] = [] + for move in moves: + fingerprint = (norm(move.old_url), move.new_path) + if fingerprint in seen: + continue + seen.add(fingerprint) + unique.append(move) + return unique def published_urls() -> set[str]: @@ -350,8 +374,8 @@ def classify(moves: list[Move]) -> None: A collision is the trap that has no safe automatic answer: Hugo resolves two pages claiming the same alias by picking one and emitting a warning, so adding the alias would quietly make the redirect ambiguous rather than fix - it. 26 of this repo's gaps are collisions -- 24 where another page already - claims the URL, and 2 where two moved pages both want it. + it. 28 of this repo's gaps are collisions -- 24 where another page already + claims the URL, and 4 where two moved pages both want it. """ current = published_urls() owners = alias_owners() @@ -457,7 +481,8 @@ def insert_aliases(lines: list[str], new_aliases: list[str]) -> list[str] | None continue else: break - item_indent = re.match(r"[ \t]*", lines[last]).group(0) if last != key else indent + item_indent = (re.match(r"[ \t]*", lines[last]).group(0) + if last != key else indent) block = [f"{item_indent}- {a}\n" for a in new_aliases] return lines[:last + 1] + block + lines[last + 1:] @@ -471,8 +496,13 @@ def insert_aliases(lines: list[str], new_aliases: list[str]) -> list[str] | None return lines -def apply_fixes(moves: list[Move]) -> tuple[int, int]: - """Write missing aliases into frontmatter. Returns (files, aliases) changed.""" +def apply_fixes(moves: list[Move]) -> tuple[int, int, list[str]]: + """Write missing aliases into frontmatter. + + Returns (files changed, aliases added, files that still need a manual fix). + The third value matters to callers: a sweep that reports success while some + aliases could not be placed would claim a complete fix it did not make. + """ by_file: dict[str, FileFix] = {} for move in moves: if not move.actionable: @@ -483,9 +513,11 @@ def apply_fixes(moves: list[Move]) -> tuple[int, int]: fix.aliases.append(alias) files = aliases = 0 + skipped: list[str] = [] for fix in by_file.values(): if not os.path.exists(fix.path): logger.warning(" ! %s no longer exists, skipping", fix.path) + skipped.append(fix.path) continue existing = declared_aliases(fix.path) wanted = [a for a in fix.aliases if norm(a) not in existing] @@ -495,6 +527,7 @@ def apply_fixes(moves: list[Move]) -> tuple[int, int]: updated = insert_aliases(lines, wanted) if updated is None: logger.warning(" ! %s: could not place aliases, skipping", fix.path) + skipped.append(fix.path) continue with open(fix.path, "w", encoding="utf-8") as handle: handle.writelines(updated) @@ -503,7 +536,7 @@ def apply_fixes(moves: list[Move]) -> tuple[int, int]: logger.info(" + %s", fix.path) for alias in wanted: logger.info(" %s", alias) - return files, aliases + return files, aliases, skipped # --------------------------------------------------------------------------- # @@ -513,7 +546,8 @@ def apply_fixes(moves: list[Move]) -> tuple[int, int]: def report(moves: list[Move], github: bool, fix_hint: str) -> list[Move]: missing = [m for m in moves if m.actionable] occupied = [m for m in moves if not m.aliased and m.occupied] - collisions = [m for m in moves if not m.aliased and not m.occupied and m.collides_with] + collisions = [m for m in moves + if not m.aliased and not m.occupied and m.collides_with] aliased = [m for m in moves if m.aliased] logger.info("check_missing_aliases: %d URL-changing move(s) found.", len(moves)) @@ -564,7 +598,8 @@ def main() -> int: classify(moves) fix_hint = ("make check_aliases_fix" if args.all else - f"python3 build/check_missing_aliases.py --range {args.rev_range} --fix") + "python3 build/check_missing_aliases.py " + f"--range {args.rev_range} --fix") missing = report(moves, args.github, fix_hint) if args.json_out: @@ -574,10 +609,15 @@ def main() -> int: if args.fix and missing: logger.info("Adding %d alias(es):", len(missing)) - files, aliases = apply_fixes(moves) + files, aliases, skipped = apply_fixes(moves) logger.info("check_missing_aliases: added %d alias(es) across %d file(s).", aliases, files) - return 0 + if skipped: + logger.warning("check_missing_aliases: could not place aliases in %d " + "file(s), which still need fixing by hand:", len(skipped)) + for path in skipped: + logger.warning(" %s", path) + return 1 if (skipped and args.fail) else 0 return 1 if (missing and args.fail) else 0 diff --git a/build/test_check_missing_aliases.py b/build/test_check_missing_aliases.py index 6cd7aa87e2..abca0fc53d 100644 --- a/build/test_check_missing_aliases.py +++ b/build/test_check_missing_aliases.py @@ -36,8 +36,9 @@ def test_to_url_strips_both_bundle_names(): # the containing directory's URL, so renaming between them changes nothing. assert to_url("content/commands/lpushx/index.md") == "commands/lpushx" assert to_url("content/commands/lpushx.md") == "commands/lpushx" - assert to_url("content/develop/data-types/strings/_index.md") == "develop/data-types/strings" - assert to_url("content/develop/data-types/strings.md") == "develop/data-types/strings" + strings = "develop/data-types/strings" + assert to_url("content/develop/data-types/strings/_index.md") == strings + assert to_url("content/develop/data-types/strings.md") == strings def test_to_url_handles_site_root(): From f72e0a55baf71ee88d3dccd3b3ac1a8e7787452a Mon Sep 17 00:00:00 2001 From: Andy Stark Date: Fri, 7 Aug 2026 13:27:03 +0100 Subject: [PATCH 3/7] DOC-6951 Read scalar aliases the way Hugo does, not the way PyYAML does Corrects a false positive I had already written up as a content bug in three places. One file declares its aliases as a YAML scalar folded across two lines. PyYAML reads that as the single string "/a/ /b/", so the scanner reported the page as missing an alias and declined to fix it, and I concluded the author's second alias had never worked. Hugo disagrees, and Hugo is the authority. It casts a scalar aliases value with cast.ToStringSlice, which runs strings.Fields, so a bare string is split on whitespace into several aliases. Reproduced against Hugo 0.143.1 in a throwaway site: the folded frontmatter yields two entries from .Aliases and Hugo writes both alias stubs. Both of that page's aliases resolve on the live site today, so there was never anything wrong with it. The lesson is one I had already written down and did not apply here. An authoritative oracle beats a reimplementation of a spec, and a YAML library is a reimplementation as far as Hugo frontmatter semantics are concerned. What made it convincing was that PyYAML's reading is correct YAML -- the folding is real, and it is Hugo that layers a whitespace split on top. Only list items are exempt, since cast.ToStringSlice splits a string but converts each element of a slice individually. Exactly one file in the corpus is affected, and it now reads as already aliased, which takes the count from 290 aliased and 258 fixable to 291 and 257, and leaves --fix with no skipped files at all. Learned: an authoritative oracle beats a library even when the library is right on its own terms -- PyYAML folds a multi-line scalar correctly, and Hugo then splits the result on whitespace, so only Hugo settles what an alias means Constraint: a scalar aliases value is whitespace-separated because Hugo casts it with cast.ToStringSlice; list items are not split Ticket: DOC-6951 Co-Authored-By: Claude Opus 5 (1M context) --- build/check_missing_aliases.py | 26 ++++++++++++++++++------ build/test_check_missing_aliases.py | 31 +++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 6 deletions(-) diff --git a/build/check_missing_aliases.py b/build/check_missing_aliases.py index 878330b93f..1aa0d144af 100644 --- a/build/check_missing_aliases.py +++ b/build/check_missing_aliases.py @@ -2,7 +2,7 @@ Renaming a content file changes its published URL, and the old URL dies unless the page declares an ``aliases:`` entry for it. That entry is author-declared -and therefore unreliable: measured over this repo's history, only 290 of 598 +and therefore unreliable: measured over this repo's history, only 291 of 598 URL-changing moves carry a matching alias, and it fails inconsistently even within a single commit (``155277839`` moved LangCache and Agent Memory together; LangCache got an alias, Agent Memory did not, and its old URL 404s today). @@ -12,7 +12,7 @@ aliases into frontmatter. Seven things make a naive version of this worse than useless -- a first attempt -reported 961 missing aliases against a true 258, nearly three quarters of it +reported 961 missing aliases against a true 257, nearly three quarters of it noise, and every false positive looked plausible in a list -- so each of the seven is handled explicitly: @@ -32,9 +32,12 @@ must never be redirected (22 cases). 6. **Declared-but-not-a-list aliases.** 88 files declare the key with no value (49 spelled ``null``, 39 bare) and 104 give it a bare scalar rather than a - list, so a check that assumes a list silently under-reports. One more uses a - folded multi-line scalar, which is valid YAML that silently folds two - intended aliases into one string, so ``--fix`` declines that file. + list, so a check that assumes a list silently under-reports. A scalar is + also whitespace-separated, because Hugo casts it with ``cast.ToStringSlice``: + one file folds a scalar across two lines and Hugo publishes both halves as + working aliases, where PyYAML reads the single string ``"/a/ /b/"``. Trusting + the YAML library over Hugo there cost a false positive against a page that + was never broken. 7. **Collisions.** 28 of the gaps name a URL another page already claims as its own alias. Hugo resolves that by picking one arbitrarily and warning, so adding the alias unattended would make the redirect ambiguous rather than @@ -270,7 +273,18 @@ def declared_aliases(path: str) -> set[str]: if values is None: return set() if isinstance(values, str): - values = [values] + # Hugo casts a scalar `aliases` value with cast.ToStringSlice, which runs + # strings.Fields, so a bare string is split on whitespace into several + # aliases. That is not what a YAML library does -- PyYAML folds + # + # aliases: /a/ + # /b/ + # + # into the single string "/a/ /b/" -- and taking the library's reading + # cost a false positive here, because Hugo publishes both of those as + # working aliases. Verified against Hugo 0.143.1. List items are *not* + # split, so only the scalar case gets this treatment. + values = values.split() if not isinstance(values, list): return set() return {norm(str(v)) for v in values if v is not None and str(v).strip()} diff --git a/build/test_check_missing_aliases.py b/build/test_check_missing_aliases.py index abca0fc53d..3161b7ce00 100644 --- a/build/test_check_missing_aliases.py +++ b/build/test_check_missing_aliases.py @@ -324,6 +324,37 @@ def test_declared_aliases_reads_every_shape(): assert declared_aliases(path) == expected[name], name +def test_scalar_aliases_are_split_on_whitespace_like_hugo(): + """Hugo casts a scalar `aliases` with cast.ToStringSlice, i.e. strings.Fields. + + So a folded multi-line scalar publishes *two* working aliases, even though + PyYAML reads it as the single string "/a/ /b/". Trusting the YAML library + here produced a false positive against a page whose aliases both work. + Verified against Hugo 0.143.1. + """ + import tempfile + + folded = """--- +title: QPF +aliases: /operate/search/scalable-search/ + /operate/search/query-performance-factor/ +--- +body +""" + with tempfile.TemporaryDirectory() as tmp: + path = os.path.join(tmp, "folded.md") + with open(path, "w", encoding="utf-8") as handle: + handle.write(folded) + assert declared_aliases(path) == { + "operate/search/scalable-search", + "operate/search/query-performance-factor", + } + # A single-valued scalar must still read as exactly one alias. + with open(path, "w", encoding="utf-8") as handle: + handle.write("---\naliases: /a/b/\n---\nx\n") + assert declared_aliases(path) == {"a/b"} + + def test_round_trip_every_shape(): """Whatever we insert must be readable back as an alias.""" import tempfile From 782d3b547367962312ef242320acc0d47a93768c Mon Sep 17 00:00:00 2001 From: Andy Stark Date: Fri, 7 Aug 2026 13:51:13 +0100 Subject: [PATCH 4/7] DOC-6951 Treat draft pages as unpublished in the alias scanner An eighth trap, and the first one found by building the site rather than by reading code. Applying the backfill and running a full Hugo build, then checking every stub the build actually emitted, turned up two aliases Hugo had declined to write. Both belonged to content/integrate/write-behind/_index.md, which carries draft: true. Production builds run plain hugo with no --buildDrafts, so a draft page emits nothing at all -- and that includes its aliases. So the scanner was reporting a gap it could not close: --fix wrote two aliases into a draft, reported success, and Hugo ignored them. The same blindness runs the other way, because published_urls treated drafts as occupying their URL, which would have suppressed a real redirect had one pointed there. Nothing was affected today, but only by luck: 31 files are drafts and 13 of them hold an eligible URL. Drafts are now excluded from published_urls, a move whose target is a draft is reported in its own category rather than counted as fixable, and the draft set is found by grep before parsing so this does not read all 5,867 content files. Worth recording how this was caught, because it is the argument for the build step existing at all. The gap was invisible to unit tests, to a full-corpus dry run, to duplicate-claim analysis, and to Bugbot. Absence of build warnings would not have caught it either -- the build was clean at --logLevel warn, and Hugo says nothing when it skips a draft's aliases. Only comparing the aliases we wrote against the stubs Hugo emitted showed it. Learned: the only check that found this compared what we wrote against what Hugo actually emitted; a clean build log is not evidence, because Hugo skips a draft's aliases silently Constraint: a draft page publishes nothing including its aliases, so drafts are excluded from published_urls and a move whose target is a draft is never auto-fixed Ticket: DOC-6951 Co-Authored-By: Claude Opus 5 (1M context) --- build/check_missing_aliases.py | 86 +++++++++++++++++++++++++---- build/test_check_missing_aliases.py | 26 +++++++-- 2 files changed, 96 insertions(+), 16 deletions(-) diff --git a/build/check_missing_aliases.py b/build/check_missing_aliases.py index 1aa0d144af..88e1d1ec13 100644 --- a/build/check_missing_aliases.py +++ b/build/check_missing_aliases.py @@ -11,10 +11,10 @@ published URL, and reports those with no alias. ``--fix`` writes the missing aliases into frontmatter. -Seven things make a naive version of this worse than useless -- a first attempt -reported 961 missing aliases against a true 257, nearly three quarters of it +Eight things make a naive version of this worse than useless -- a first attempt +reported 961 missing aliases against a true 256, nearly three quarters of it noise, and every false positive looked plausible in a list -- so each of the -seven is handled explicitly: +eight is handled explicitly: 1. **Hugo bundles.** ``index.md`` (leaf) and ``_index.md`` (branch) both publish at the containing directory's URL, so neither name appears in the URL and @@ -42,6 +42,12 @@ own alias. Hugo resolves that by picking one arbitrarily and warning, so adding the alias unattended would make the redirect ambiguous rather than fix it. Those are reported for a human and never auto-fixed. +8. **Drafts.** 31 files are drafts, and production builds pass no + ``--buildDrafts``, so a draft publishes nothing at all -- *including its + aliases*. An alias added to one is a silent no-op, and counting a draft as + occupying a URL would suppress a real redirect. Caught by building the + corpus and finding two stubs Hugo declined to emit, which is the only reason + this is here rather than still latent. Warn-only by default (exit 0), like check_page_sizes; pass ``--fail`` to make CI block on offenders. @@ -99,12 +105,14 @@ class Move: commit: str aliased: bool = False occupied: bool = False + target_draft: bool = False collides_with: list[str] = field(default_factory=list) @property def actionable(self) -> bool: """True when the alias can be added safely and without a judgment call.""" - return not (self.aliased or self.occupied or self.collides_with) + return not (self.aliased or self.occupied or self.target_draft + or self.collides_with) @dataclass @@ -205,6 +213,51 @@ def render_never_roots() -> set[str]: return roots +_drafts: set[str] | None = None + + +def draft_paths() -> set[str]: + """Content files Hugo will not publish, because they are drafts. + + Production runs plain ``hugo`` with no ``--buildDrafts``, so a draft page + emits nothing at all -- **including its aliases**. That matters in both + directions: an alias added to a draft is silently a no-op, and treating a + draft as occupying a URL would wrongly suppress a real redirect. 31 files + today. Found by grep first so this does not parse all 5,867 content files. + """ + global _drafts + if _drafts is not None: + return _drafts + + import yaml + + found: set[str] = set() + try: + candidates = git("grep", "-l", "-E", r"^draft:[ \t]*true", "--", + CONTENT).splitlines() + except subprocess.CalledProcessError: + candidates = [] + for path in candidates: + try: + lines = read_lines(path) + except OSError: + continue + bounds = frontmatter_bounds(lines) + if not bounds: + continue + try: + data = yaml.safe_load("".join(lines[1:bounds[1]])) or {} + except yaml.YAMLError: + continue + if not isinstance(data, dict): + continue + keyed = {str(k).lower(): v for k, v in data.items()} + if keyed.get("draft") in (True, "true"): + found.add(path) + _drafts = found + return found + + def is_published(path: str) -> bool: """False for content Hugo never publishes as a page of its own.""" rel = path[len(CONTENT) + 1:] @@ -364,9 +417,10 @@ def find_moves(rev_range: str | None, threshold: int) -> list[Move]: def published_urls() -> set[str]: - """Normalized URLs of every page published today.""" + """Normalized URLs of every page published today. Drafts do not count.""" + drafts = draft_paths() return {norm(to_url(p)) for p in git("ls-files", CONTENT).splitlines() - if eligible(p)} + if eligible(p) and p not in drafts} def alias_owners() -> dict[str, set[str]]: @@ -393,6 +447,7 @@ def classify(moves: list[Move]) -> None: """ current = published_urls() owners = alias_owners() + drafts = draft_paths() alias_cache: dict[str, set[str]] = {} for move in moves: @@ -400,6 +455,7 @@ def classify(moves: list[Move]) -> None: alias_cache[move.new_path] = declared_aliases(move.new_path) move.aliased = norm(move.old_url) in alias_cache[move.new_path] move.occupied = norm(move.old_url) in current + move.target_draft = move.new_path in drafts if not (move.aliased or move.occupied): claimed = owners.get(norm(move.old_url), set()) - {move.new_path} move.collides_with = sorted(claimed) @@ -560,21 +616,31 @@ def apply_fixes(moves: list[Move]) -> tuple[int, int, list[str]]: def report(moves: list[Move], github: bool, fix_hint: str) -> list[Move]: missing = [m for m in moves if m.actionable] occupied = [m for m in moves if not m.aliased and m.occupied] - collisions = [m for m in moves - if not m.aliased and not m.occupied and m.collides_with] + drafted = [m for m in moves + if not m.aliased and not m.occupied and m.target_draft] + collisions = [m for m in moves if not m.aliased and not m.occupied + and not m.target_draft and m.collides_with] aliased = [m for m in moves if m.aliased] logger.info("check_missing_aliases: %d URL-changing move(s) found.", len(moves)) if moves: logger.info(" %d already aliased, %d missing an alias, %d skipped " - "(old URL is a live page), %d need a decision (collision).", - len(aliased), len(missing), len(occupied), len(collisions)) + "(old URL is a live page), %d skipped (target is a draft), " + "%d need a decision (collision).", + len(aliased), len(missing), len(occupied), len(drafted), + len(collisions)) if occupied: logger.info("Skipped -- old URL currently resolves, so must not redirect:") for move in occupied: logger.info(" %s %s", move.date, move.old_url) + if drafted: + logger.info("Skipped -- the page moved to is a draft, so it publishes " + "nothing and an alias on it would do nothing:") + for move in drafted: + logger.info(" %s %s -> %s", move.date, move.old_url, move.new_path) + if collisions: logger.warning("Needs a human decision -- another page already claims " "this URL, so Hugo would pick one arbitrarily:") diff --git a/build/test_check_missing_aliases.py b/build/test_check_missing_aliases.py index 3161b7ce00..f0e30c3722 100644 --- a/build/test_check_missing_aliases.py +++ b/build/test_check_missing_aliases.py @@ -3,9 +3,10 @@ The interesting logic is ``insert_aliases``, which edits frontmatter line by line rather than round-tripping the YAML. It has to cope with every alias shape -already in the repo -- block lists (490 files), single-line inline lists (44), -multi-line inline lists (19), the key declared with no value (192), and no key -at all -- while leaving every other line untouched. +already in the repo -- block lists (490 files), bare scalars (104), the key +spelled ``null`` (49), single-line inline lists (44), the key left bare (39), +multi-line inline lists (19), one folded multi-line scalar, and no key at all -- +while leaving every other line untouched. Run with ``pytest build/test_check_missing_aliases.py`` or directly. """ @@ -16,8 +17,8 @@ sys.path.insert(0, os.path.dirname(__file__)) from check_missing_aliases import ( # noqa: E402 - Move, declared_aliases, eligible, insert_aliases, is_published, is_versioned, - render_never_roots, to_url, + Move, declared_aliases, draft_paths, eligible, insert_aliases, is_published, + is_versioned, norm, published_urls, render_never_roots, to_url, ) @@ -84,6 +85,19 @@ def move(**kwargs): # A collision has no safe automatic answer: Hugo would pick one of the two # claimants arbitrarily, so the alias must not be added unattended. assert not move(collides_with=["content/other.md"]).actionable + # A draft publishes nothing, aliases included, so writing one is a no-op. + assert not move(target_draft=True).actionable + + +def test_drafts_are_detected_and_excluded_from_published_urls(): + drafts = draft_paths() + assert drafts, "expected this repo to contain drafts" + assert all(p.startswith("content/") and p.endswith(".md") for p in drafts) + # The draft that made this trap visible: Hugo declined to emit its two + # alias stubs during a full build, because the page itself is a draft. + assert "content/integrate/write-behind/_index.md" in drafts + published = published_urls() + assert norm(to_url("content/integrate/write-behind/_index.md")) not in published # --------------------------------------------------------------------------- # @@ -153,7 +167,7 @@ def test_multi_line_inline_list_gains_a_line_before_the_bracket(): def test_empty_aliases_key_gains_the_first_item(): - # 192 files in the repo declare the key with no value. + # 39 files in the repo leave the key bare like this. before = """--- aliases: categories: From 6c78b09e89a214604f8971de936cc0a8fc79d0b6 Mon Sep 17 00:00:00 2001 From: Andy Stark Date: Fri, 7 Aug 2026 13:58:17 +0100 Subject: [PATCH 5/7] DOC-6951 Promote scalar aliases to a block list so live URLs survive Diffing two full builds -- one with the backfill applied, one without -- showed 256 new alias stubs, no real page overwritten, and one page lost. The lost path was operate/kubernetes/release-notes/7-4-6-2, with a trailing comma, and it returns 200 on the live site today while the comma-free spelling returns 404. The cause was promoting a scalar aliases value to an inline list. One file holds aliases: /operate/kubernetes/release-notes/7-4-6-2, because an author wrote a list without brackets, and Hugo publishes an alias whose path ends in a comma. Written inline as [/...7-4-6-2, /new/] that comma becomes the list separator, so the alias silently changes to the comma-free path: a working URL turned into a 404 and a 404 into a working URL. I had also been stripping the trailing comma deliberately, on the theory that it was a typo. It probably is, but a mechanical backfill is not the place to decide that, and nothing in the output would have shown the decision being made. Scalars are now promoted to a block list instead, which has no separator to confuse with content, so each existing token survives byte for byte. Tokens are split on whitespace to match Hugo's cast.ToStringSlice. The repo's dominant style is a block list anyway, so the diffs read better as a side effect. No inline list is now written in the whole corpus. The general point is that this was invisible in every artifact except a build-to-build page-set diff. The build log was clean, the alias count was correct at 1185, and 270 of 270 expected stubs were present -- the loss only appeared as an index.html count of 6826 against a baseline 6571, one short of the 256 added. Learned: comparing two full builds' page sets caught a silently changed live URL that a clean build log, a correct alias count and a stub-by-stub check all missed Constraint: existing alias values are preserved byte for byte -- promote a scalar to a block list, never inline, because a value containing a comma changes meaning inside brackets Rejected: stripping a stray trailing comma from an author's alias | it is probably a typo but the comma'd URL is live, so a mechanical backfill must not silently retire it Ticket: DOC-6951 Co-Authored-By: Claude Opus 5 (1M context) --- build/check_missing_aliases.py | 24 ++++++++++----- build/test_check_missing_aliases.py | 46 ++++++++++++++++++++++------- 2 files changed, 53 insertions(+), 17 deletions(-) diff --git a/build/check_missing_aliases.py b/build/check_missing_aliases.py index 88e1d1ec13..887a2048e2 100644 --- a/build/check_missing_aliases.py +++ b/build/check_missing_aliases.py @@ -556,14 +556,24 @@ def insert_aliases(lines: list[str], new_aliases: list[str]) -> list[str] | None block = [f"{item_indent}- {a}\n" for a in new_aliases] return lines[:last + 1] + block + lines[last + 1:] - # A scalar value (aliases: /a/) -- 104 files. Promote it to an inline list. - # Some of those carry a stray trailing comma from an author writing a list - # without brackets, which would otherwise become an empty list entry. - existing = rest.rstrip(",").strip() - items = ([existing] if existing else []) + new_aliases + # A scalar value (aliases: /a/) -- 104 files. Promote it to a *block* list, + # never an inline one, and keep each existing token byte-for-byte. + # + # Inline promotion silently changes what the page publishes. One file holds + # `aliases: /operate/kubernetes/release-notes/7-4-6-2, ` -- an author writing + # a list without brackets -- and Hugo publishes an alias whose path ends in a + # comma, which is live and returns 200 today. Written inline, that comma + # becomes the list separator and the alias silently changes to the + # comma-free path, turning a working URL into a 404. A block list has no + # separator to be confused with, so the value survives exactly. + # + # Splitting on whitespace matches Hugo's cast.ToStringSlice, so a scalar + # holding several aliases becomes several list items rather than one. lines = list(lines) - lines[key] = f"{indent}aliases: [{', '.join(items)}]\n" - return lines + lines[key] = f"{indent}aliases:\n" + items = rest.split() + new_aliases + block = [f"{indent}- {a}\n" for a in items] + return lines[:key + 1] + block + lines[key + 1:] def apply_fixes(moves: list[Move]) -> tuple[int, int, list[str]]: diff --git a/build/test_check_missing_aliases.py b/build/test_check_missing_aliases.py index f0e30c3722..c79831aa56 100644 --- a/build/test_check_missing_aliases.py +++ b/build/test_check_missing_aliases.py @@ -208,7 +208,7 @@ def test_explicit_null_is_dropped_not_kept_as_an_item(): assert "null" not in after -def test_scalar_value_is_promoted_to_a_list(): +def test_scalar_value_is_promoted_to_a_block_list(): before = """--- aliases: /develop/connect/clients/dotnet title: .NET @@ -216,14 +216,25 @@ def test_scalar_value_is_promoted_to_a_list(): Body. """ after = apply(before, ["/develop/clients/dotnet/"]) - assert ("aliases: [/develop/connect/clients/dotnet, " - "/develop/clients/dotnet/]\n") in after + assert after == """--- +aliases: +- /develop/connect/clients/dotnet +- /develop/clients/dotnet/ +title: .NET +--- +Body. +""" + +def test_scalar_with_a_trailing_comma_keeps_the_comma(): + """A live URL must not change because we tidied its frontmatter. -def test_scalar_with_a_stray_trailing_comma_does_not_create_an_empty_item(): - # Real frontmatter in the repo: an author wrote a list without brackets, so - # the value is the string "/path/7-4-6-2,". Left alone it would become - # `[/path/7-4-6-2,, /new/]` -- a double comma, i.e. an empty alias. + Real frontmatter in the repo: an author wrote a list without brackets, so + Hugo publishes an alias whose path ends in a comma. That URL returns 200 + today. Promoted to an *inline* list the comma becomes the separator and the + alias silently changes to the comma-free path -- observed as a lost page when + diffing two full builds. A block list preserves it. + """ before = """--- weight: 29 aliases: /operate/kubernetes/release-notes/7-4-6-2, @@ -231,9 +242,24 @@ def test_scalar_with_a_stray_trailing_comma_does_not_create_an_empty_item(): Body. """ after = apply(before, ["/operate/kubernetes/release-notes/7-4-6-2/"]) - assert ",," not in after - assert ("aliases: [/operate/kubernetes/release-notes/7-4-6-2, " - "/operate/kubernetes/release-notes/7-4-6-2/]\n") in after + assert "- /operate/kubernetes/release-notes/7-4-6-2,\n" in after + assert "- /operate/kubernetes/release-notes/7-4-6-2/\n" in after + assert "aliases: [" not in after + + +def test_folded_scalar_promotion_keeps_both_aliases(): + # Hugo reads a folded scalar as two aliases, so promotion must emit two + # items rather than one item containing a space. + before = """--- +aliases: /operate/search/scalable-search/ + /operate/search/query-performance-factor/ +weight: 20 +--- +Body. +""" + result = insert_aliases(before.splitlines(keepends=True), ["/new/"]) + # The folded form is refused outright, so nothing is silently mangled. + assert result is None def test_missing_key_is_added_above_the_closing_fence(): From 9a398942d14e9d33fc54b07aeae7c67b6a24ddc2 Mon Sep 17 00:00:00 2001 From: Andy Stark Date: Fri, 7 Aug 2026 14:49:57 +0100 Subject: [PATCH 6/7] DOC-6951 Stop auto-aliasing pages that were split into a section Three findings from a second Bugbot pass, plus the one that matters, which came from a review comment on the backfill rather than from the code. The important one is conceptual. git records lineage, and lineage is not equivalence. When a page is split up -- X.md becoming X/child.md -- the old URL turns into a section URL while the file itself becomes one page inside it. The scanner faithfully followed the file and proposed redirecting the old landing page to that child. On the Java client docs that meant a bookmark for the generic develop/connect/clients/java page landing on a Jedis connection guide, four renames later, rather than on the Jedis hub that exists today. Reviewers spotted two instances; there are six, including the same mistake in the redis-py docs. Splits to X/_index.md are unaffected, because the URL does not change. Only a split to a named child demotes the old URL, and those chains are now reported for a person to decide rather than fixed. That drops the backfill from 256 to 253 and moves three cases out of the collision bucket, where they had been landing for the wrong reason. Also fixed, both raised by Bugbot and both real but unrealized here. Renames within a single commit were applied in git's listing order, so a commit holding both A->B and B->C would lose the A move if git listed them the other way round; ordering is now derived from the dependencies, with a cycle guard, and extracted into order_renames so it can be tested directly. And the folded-scalar guard treated any indented line as a continuation, including a whitespace-only one, so an ordinary scalar followed by a padded blank line was skipped; it now requires actual content. One Bugbot finding was a false positive and is deliberately unchanged: it read the inline-list branch as collapsing a two-item list into one string. The intermediate does hold both items in one string, but the join puts them back correctly -- [/a/one/, /a/two/] plus /a/three/ yields all three, verified by parsing the result back out. Learned: the deepest bug in this scanner was not a coding error but a category error -- following the file instead of the meaning -- and it took a human reviewer looking at content to see it, after four rounds of measurement had passed Constraint: a chain crossing a page-into-section split is never auto-fixed, because the old URL was a landing page and its lineage ends at one child Ticket: DOC-6951 Co-Authored-By: Claude Opus 5 (1M context) --- build/check_missing_aliases.py | 123 +++++++++++++++++++++++----- build/test_check_missing_aliases.py | 43 +++++++++- 2 files changed, 145 insertions(+), 21 deletions(-) diff --git a/build/check_missing_aliases.py b/build/check_missing_aliases.py index 887a2048e2..bcb6484630 100644 --- a/build/check_missing_aliases.py +++ b/build/check_missing_aliases.py @@ -11,10 +11,10 @@ published URL, and reports those with no alias. ``--fix`` writes the missing aliases into frontmatter. -Eight things make a naive version of this worse than useless -- a first attempt -reported 961 missing aliases against a true 256, nearly three quarters of it +Nine things make a naive version of this worse than useless -- a first attempt +reported 961 missing aliases against a true 253, nearly three quarters of it noise, and every false positive looked plausible in a list -- so each of the -eight is handled explicitly: +nine is handled explicitly: 1. **Hugo bundles.** ``index.md`` (leaf) and ``_index.md`` (branch) both publish at the containing directory's URL, so neither name appears in the URL and @@ -38,7 +38,7 @@ working aliases, where PyYAML reads the single string ``"/a/ /b/"``. Trusting the YAML library over Hugo there cost a false positive against a page that was never broken. -7. **Collisions.** 28 of the gaps name a URL another page already claims as its +7. **Collisions.** 25 of the gaps name a URL another page already claims as its own alias. Hugo resolves that by picking one arbitrarily and warning, so adding the alias unattended would make the redirect ambiguous rather than fix it. Those are reported for a human and never auto-fixed. @@ -48,6 +48,14 @@ occupying a URL would suppress a real redirect. Caught by building the corpus and finding two stubs Hugo declined to emit, which is the only reason this is here rather than still latent. +9. **Pages split into a section.** ``X.md`` becoming ``X/.md`` turns the + old URL into a section URL while the file becomes one page inside it, so + redirecting the old landing page to that one child is usually wrong -- a + reader holding the old link wants the new landing page. git records lineage, + and lineage is not equivalence. 6 such splits exist here and 6 aliases reach + their target through one, so they are reported for a person rather than + guessed at. A split to ``X/_index.md`` is fine and not counted, because the + URL does not change. Warn-only by default (exit 0), like check_page_sizes; pass ``--fail`` to make CI block on offenders. @@ -106,13 +114,14 @@ class Move: aliased: bool = False occupied: bool = False target_draft: bool = False + split_at: str = "" collides_with: list[str] = field(default_factory=list) @property def actionable(self) -> bool: """True when the alias can be added safely and without a judgment call.""" return not (self.aliased or self.occupied or self.target_draft - or self.collides_with) + or self.split_at or self.collides_with) @dataclass @@ -347,6 +356,28 @@ def declared_aliases(path: str) -> set[str]: # finding moves # --------------------------------------------------------------------------- # +def order_renames(pending: list[tuple[str, str]]) -> list[tuple[str, str]]: + """Order one commit's renames so a chain resolves whatever order git listed. + + A commit can contain both A->B and B->C, and the chain only resolves if A->B + is applied first. git's ordering is not guaranteed to oblige, so an edge waits + while any other edge in the same commit still renames *into* its source. No + commit in this repo's history contains such a chain today, so this changes + nothing here -- but getting it wrong loses a move silently, which is the + failure mode this whole script exists to avoid. + """ + remaining = list(pending) + ordered: list[tuple[str, str]] = [] + while remaining: + ready = [(o, n) for o, n in remaining + if not any(n2 == o for o2, n2 in remaining if (o2, n2) != (o, n))] + if not ready: + ready = list(remaining) # a rename cycle; apply as listed and move on + ordered.extend(ready) + remaining = [e for e in remaining if e not in ready] + return ordered + + def find_moves(rev_range: str | None, threshold: int) -> list[Move]: """Renames in the given range, chained so each page resolves to its final home.""" args = ["log", "--reverse", f"--find-renames={threshold}%", "--diff-filter=R", @@ -363,17 +394,11 @@ def find_moves(rev_range: str | None, threshold: int) -> list[Move]: # path-as-it-stands-now -> the (old_path, date, commit) records behind it history: dict[str, set[tuple[str, str, str]]] = {} + edges: dict[str, str] = {} + pending: list[tuple[str, str]] = [] commit = date = "" - for line in out.splitlines(): - if line.startswith("COMMIT\t"): - _, commit, date = line.split("\t") - continue - parts = line.split("\t") - if len(parts) != 3 or not parts[0].startswith("R"): - continue - _, old, new = parts - if not (eligible(old) and eligible(new)): - continue + + def absorb(old: str, new: str) -> None: # Merge rather than assign. If a second file later renames onto a path # that already carries a chain -- possible once the first occupant has # been deleted rather than moved -- assigning would drop the earlier @@ -385,6 +410,50 @@ def find_moves(rev_range: str | None, threshold: int) -> list[Move]: carried = history.pop(old, set()) history.setdefault(new, set()).update(carried) history[new].add((old, date, commit)) + edges[old] = new + + def flush() -> None: + """Apply one commit's renames in dependency order, then clear the buffer.""" + for old, new in order_renames(pending): + absorb(old, new) + pending.clear() + + for line in out.splitlines(): + if line.startswith("COMMIT\t"): + flush() + _, commit, date = line.split("\t") + continue + parts = line.split("\t") + if len(parts) != 3 or not parts[0].startswith("R"): + continue + _, old, new = parts + if not (eligible(old) and eligible(new)): + continue + pending.append((old, new)) + flush() + + # A "demoting split" is X.md -> X/.md: a page broken up into a + # section, so its old URL becomes the section's URL while the file itself + # becomes one page inside it. Redirecting the old URL to that one child is + # usually wrong -- someone holding a link to the old landing page should + # arrive at the new landing page, not at whichever child inherited the file. + # A split to X/_index.md is fine and not counted, because the URL is + # unchanged. 6 such splits exist here, and 3 aliases reach a target through + # one; git records lineage, and lineage is not the same as equivalence, so + # these are reported for a person rather than guessed at. + demoting = {old for old, new in edges.items() + if os.path.dirname(new) + ".md" == old + and not new.endswith("_index.md")} + + def crosses_a_split(start: str) -> str: + seen: set[str] = set() + path = start + while path in edges and path not in seen: + seen.add(path) + if path in demoting: + return f"{path} -> {edges[path]}" + path = edges[path] + return "" tracked = set(git("ls-files", CONTENT).splitlines()) moves: list[Move] = [] @@ -398,7 +467,8 @@ def find_moves(rev_range: str | None, threshold: int) -> list[Move]: continue # a bundle rename, or otherwise URL-preserving moves.append(Move(old_path=old_path, new_path=new_path, old_url=old_url, new_url=new_url, - date=date, commit=commit[:9])) + date=date, commit=commit[:9], + split_at=crosses_a_split(old_path))) # A redirect is identified by where it comes from and where it goes, so the # same pair reached by two routes -- a page moved away and back, or a @@ -531,7 +601,8 @@ def insert_aliases(lines: list[str], new_aliases: list[str]) -> list[str] | None # leave the continuation dangling and break the frontmatter outright, so # refuse it and let a human fix the underlying content bug. One file today. following = lines[key + 1] if key + 1 < close else "" - if (following[:1] in (" ", "\t") + if (following.strip() # a blank line is not a continuation + and following[:1] in (" ", "\t") and not re.match(r"[ \t]*-[ \t]*\S", following) and not re.match(r"[ \t]*\S+[ \t]*:", following)): return None @@ -628,17 +699,20 @@ def report(moves: list[Move], github: bool, fix_hint: str) -> list[Move]: occupied = [m for m in moves if not m.aliased and m.occupied] drafted = [m for m in moves if not m.aliased and not m.occupied and m.target_draft] + splits = [m for m in moves if not m.aliased and not m.occupied + and not m.target_draft and m.split_at] collisions = [m for m in moves if not m.aliased and not m.occupied - and not m.target_draft and m.collides_with] + and not m.target_draft and not m.split_at and m.collides_with] aliased = [m for m in moves if m.aliased] logger.info("check_missing_aliases: %d URL-changing move(s) found.", len(moves)) if moves: logger.info(" %d already aliased, %d missing an alias, %d skipped " "(old URL is a live page), %d skipped (target is a draft), " - "%d need a decision (collision).", + "%d need a decision (page split), %d need a decision " + "(collision).", len(aliased), len(missing), len(occupied), len(drafted), - len(collisions)) + len(splits), len(collisions)) if occupied: logger.info("Skipped -- old URL currently resolves, so must not redirect:") @@ -651,6 +725,15 @@ def report(moves: list[Move], github: bool, fix_hint: str) -> list[Move]: for move in drafted: logger.info(" %s %s -> %s", move.date, move.old_url, move.new_path) + if splits: + logger.warning("Needs a human decision -- the old page was split into a " + "section, so the right target is probably its new landing " + "page rather than the child that inherited the file:") + for move in splits: + logger.warning(" %s %s", move.date, move.old_url) + logger.warning(" lineage ends at %s", move.new_url) + logger.warning(" split at %s", move.split_at) + if collisions: logger.warning("Needs a human decision -- another page already claims " "this URL, so Hugo would pick one arbitrarily:") diff --git a/build/test_check_missing_aliases.py b/build/test_check_missing_aliases.py index c79831aa56..1b89d27149 100644 --- a/build/test_check_missing_aliases.py +++ b/build/test_check_missing_aliases.py @@ -18,7 +18,7 @@ from check_missing_aliases import ( # noqa: E402 Move, declared_aliases, draft_paths, eligible, insert_aliases, is_published, - is_versioned, norm, published_urls, render_never_roots, to_url, + is_versioned, norm, order_renames, published_urls, render_never_roots, to_url, ) @@ -89,6 +89,47 @@ def move(**kwargs): assert not move(target_draft=True).actionable +def test_renames_in_one_commit_are_ordered_so_chains_resolve(): + # A commit holding both A->B and B->C only resolves if A->B goes first. + # git's listing order is not guaranteed to oblige, and getting it wrong + # loses the A move silently. + ab, bc = ("content/a.md", "content/b.md"), ("content/b.md", "content/c.md") + assert order_renames([bc, ab]) == [ab, bc] + assert order_renames([ab, bc]) == [ab, bc] + # Independent renames keep their order and none are dropped. + xy, pq = ("content/x.md", "content/y.md"), ("content/p.md", "content/q.md") + assert order_renames([xy, pq]) == [xy, pq] + # A cycle must terminate rather than spin, and must not lose an edge. + cycle = [("content/a.md", "content/b.md"), ("content/b.md", "content/a.md")] + assert sorted(order_renames(cycle)) == sorted(cycle) + # A three-link chain listed backwards. + cd = ("content/c.md", "content/d.md") + assert order_renames([cd, bc, ab]) == [ab, bc, cd] + + +def test_a_move_split_into_a_section_is_not_auto_fixed(): + def move(**kwargs): + return Move(old_path="content/a.md", new_path="content/b/c.md", + old_url="a", new_url="b/c", date="2026-01-01", commit="abc", + **kwargs) + + # git says this file descends from the old page, but the old URL was a + # landing page and its lineage ends at one child, so a person decides. + assert not move(split_at="content/a.md -> content/a/c.md").actionable + assert move().actionable + + +def test_a_whitespace_only_line_is_not_a_folded_continuation(): + # The folded-scalar guard must not be tripped by trailing whitespace on the + # line after a perfectly ordinary single-value scalar. + before = "---\naliases: /old/thing/\n \ntitle: T\n---\nBody.\n" + after = insert_aliases(before.splitlines(keepends=True), ["/new/thing/"]) + assert after is not None, "a blank line should not block the fix" + joined = "".join(after) + assert "- /old/thing/\n" in joined + assert "- /new/thing/\n" in joined + + def test_drafts_are_detected_and_excluded_from_published_urls(): drafts = draft_paths() assert drafts, "expected this repo to contain drafts" From b72cfc1254d2cad8f491d30b3c37a72be9d47b7c Mon Sep 17 00:00:00 2001 From: Andy Stark Date: Fri, 7 Aug 2026 16:53:40 +0100 Subject: [PATCH 7/7] DOC-6951 Give the scanner distinct exit codes for findings and failure A finding and a failure shared exit code 1: --fail returned it when gaps remained, and so did an internal git failure. The workflow in the companion PR reads that code to decide whether the fixer declined a file, so a git failure would have produced "some aliases could not be added automatically" -- pointing whoever read it at the content rather than at the broken scan. Named codes now: 0 nothing to report, 1 findings under --fail, 2 the scan itself failed. The workflow already treats anything above 1 as a hard error, so it needs no change beyond a comment. Learned: a caller that cannot distinguish "I found a problem" from "I broke" will report the wrong one, and the cost lands on whoever reads the message rather than on the code Constraint: exit 1 means findings and exit 2 means the scan failed; do not collapse them, because CI decides what to say from this Ticket: DOC-6951 Co-Authored-By: Claude Opus 5 (1M context) --- build/check_missing_aliases.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/build/check_missing_aliases.py b/build/check_missing_aliases.py index bcb6484630..9558ea6bae 100644 --- a/build/check_missing_aliases.py +++ b/build/check_missing_aliases.py @@ -77,6 +77,15 @@ logger = logging.getLogger("check_missing_aliases") +# Exit codes. A finding and a failure must not share one: a caller that cannot +# tell them apart will report "some aliases could not be added" when what actually +# happened is that git fell over, which sends whoever reads it looking in the wrong +# place. Only meaningful with --fail; without it the scan is warn-only and always +# exits 0 unless it could not run at all. +EXIT_OK = 0 +EXIT_FINDINGS = 1 # gaps remain, or --fix could not place some aliases +EXIT_ERROR = 2 # the scan itself failed and reported nothing usable + CONTENT = "content" # A path segment that looks like a semver version marks the versioned trees, @@ -149,7 +158,7 @@ def parse_args() -> argparse.Namespace: parser.add_argument("--github", action="store_true", help="emit GitHub Actions warning annotations") parser.add_argument("--fail", action="store_true", - help="exit non-zero if any move is missing an alias") + help="exit 1 if any move is missing an alias (see EXIT_* below)") return parser.parse_args() @@ -767,7 +776,7 @@ def main() -> int: try: moves = find_moves(rev_range, args.threshold) except subprocess.CalledProcessError: - return 1 + return EXIT_ERROR classify(moves) fix_hint = ("make check_aliases_fix" if args.all else @@ -790,9 +799,9 @@ def main() -> int: "file(s), which still need fixing by hand:", len(skipped)) for path in skipped: logger.warning(" %s", path) - return 1 if (skipped and args.fail) else 0 + return EXIT_FINDINGS if (skipped and args.fail) else EXIT_OK - return 1 if (missing and args.fail) else 0 + return EXIT_FINDINGS if (missing and args.fail) else EXIT_OK if __name__ == "__main__":