Fix release-notes classifier mis-tagging skia-sync PRs; make path tags data-driven

.agents/skills/release-notes/SKILL.md

@@ -23,8 +23,8 @@ to read.
 If yes, write about it. If no (CI tweaks, internal refactors, doc/workflow
 plumbing, test infra), leave it out — the renderer already collapses that noise.
 `data.json` tags every PR `product` / `mixed` / `internal`; treat `internal` as
-invisible unless it changed shipped behaviour, and for `mixed` judge from the
-title.
+invisible unless it changed shipped behaviour, and for `mixed` (build config in
+`native/`, or a `docs` API-docs bump) judge from the title.
 
 ## Running the full pipeline (prepare → write prose → render)
 

documentation/dev/release-notes-and-api-diffs.md

@@ -342,6 +342,8 @@ scripts/infra/docs/                (all doc engines, together)
                                prunes stale -unreleased pages and retires HarfBuzz hub pages
   release-notes-schema/prose.schema.json   the prose contract the agent fills
   versions.json                supersession + baseline config (§1); shared repo-wide
+  release-notes-paths.json     PR path→tag classification (§4.4): the deterministic,
+                               editable product/mixed/internal path map read by release-notes-data.py
   docker/                      reproducible image + run.sh wrapper; `api-diffs`
                                invokes the `docs-api-diff` target directly
 
@@ -987,7 +989,7 @@ built from. It is timestamp-free and includes, at minimum:
 - `harfbuzz` on released pages — `{ "version", "api_diff_link", "prs" }` for the
   co-shipped HarfBuzzSharp section (§4.5). It is absent on `-unreleased` pages.
 - `prs` — the flat PR map, including title, URL, author, `community`, and the
-  deterministic `tag` (`product`, `mixed`, `internal`).
+  deterministic `tag` (`product`, `mixed`, or `internal`).
 - `contributors` — the authoritative non-maintainer, non-bot roster the renderer uses
   for the community table.
 - `previews` — per-preview/RC buckets, when present. Each carries a `key`, the human
@@ -1034,18 +1036,33 @@ principles are fixed here.
    every PR is whether the **shipped SkiaSharp/HarfBuzzSharp library** — its public
    API, runtime behavior, native binary, or NuGet package — changed for a consumer.
    Prepare makes this deterministic: `release-notes-data.py` tags every PR in `data.json` as
-   **`product`**, **`mixed`**, or **`internal`** by the files it changed.
-   - **`product`** — touches shipped code (`binding/`, `externals/`, `source/`). Written up.
-   - **`internal`** — touches none of those (CI, workflows, agent skills, docs site,
-     tests, samples, build/meta). Dropped into the one collapse line.
-   - **`mixed`** — touches only build config (`native/`): it may change the shipped
-     binary via a compile flag or be pure infra. Polish judges from the title/context
-     already in `data.json`; it does not open the PR.
-
-   `native/` is deliberately **not** treated as shipped code: it is build
-   configuration, and the thing that actually ships is `externals/skia/`. Polish drops
-   `internal`, writes up `product`, and inspects `mixed`; moving the classification out
-   of the LLM makes product-focus reliable run-to-run.
+   **`product`**, **`mixed`**, or **`internal`** by the files it changed. The path→tag
+   mapping is **not** hardcoded in the script — it lives in
+   [`scripts/infra/docs/release-notes-paths.json`](../../scripts/infra/docs/release-notes-paths.json),
+   the single deterministic place to edit it. It is a short list of ordered tiers (each a
+   `tag` + `patterns`) plus a `default`; the first tier with a matching file wins. A pattern
+   matches by prefix (`str.startswith`), or as a glob if it contains `* ? [`.
+   - **`product`** — touches shipped code with a real API / behaviour / native change:
+     `binding/` + `source/` (managed API & Views) and `externals/skia` (the native Skia
+     submodule, with its vendored HarfBuzz). Written up.
+   - **`mixed`** — affects the shipped package but is not itself an API/behaviour change, so
+     Polish judges from the title: `native/` (per-platform build config — compile flags/gn
+     args that shape the native binaries, usually infra) and `docs` (the mdoc API-docs
+     submodule that ships as IntelliSense XML — doc content, not behaviour).
+   - **`internal`** — the `default`: touches none of those (CI, workflows, agent skills, docs
+     *site*, tests, samples, build/meta, and the `externals/depot_tools` build-toolchain
+     submodule). Dropped into the one collapse line.
+
+   `native/` shapes the shipped binaries and `docs` ships as doc XML, so neither is
+   `internal`; but neither is a direct API/behaviour change, so both are `mixed` (inspected
+   from the title) rather than firm `product`. `docs` and `externals/skia` are submodules, so
+   in the parent repo they appear as bare gitlink paths (`docs`, `externals/skia`) and the
+   prefixes match those exactly — the `externals/skia` prefix is deliberately not just
+   `externals/`, which would sweep in the sibling `externals/depot_tools` build-toolchain
+   submodule (internal) and `externals/.gitignore`; the `docs` prefix is slash-less so it hits
+   the gitlink without colliding with `documentation/`. Polish drops `internal`, writes up
+   `product`, and inspects `mixed`; moving the classification out of the LLM (and into the
+   JSON) makes product-focus reliable run-to-run.
 2. **Highlights are a hook, not a summary.** The `## Highlights` section always exists
    and is assembled by `release-notes-render.py`. The prose targets ~80 words and is hard-capped
    at 100 words total across `highlights_headline` + `highlights_body`, naming only the

scripts/infra/docs/release-notes-data.py

@@ -94,7 +94,7 @@
 
 from __future__ import annotations
 
-import argparse
+import fnmatch
 import hashlib
 import json
 import re
@@ -141,6 +141,12 @@
 
 VERSIONS_JSON_PATH = Path("scripts/infra/docs/versions.json")
 
+# PR path-classification config (loaded lazily from
+# scripts/infra/docs/release-notes-paths.json). The single deterministic place that
+# owns which touched paths map to which release-note tag — see _pr_category().
+PATH_TAGS_JSON_PATH = Path("scripts/infra/docs/release-notes-paths.json")
+_PATH_TAGS_CONFIG = None  # type: Optional[tuple]
+
 # HarfBuzz-owned paths (spec §1.5/§4.5). A HarfBuzz family page lists ONLY the
 # commits touching these — the HarfBuzzSharp binding + its native assets, the
 # native libHarfBuzzSharp build, and HarfBuzz tests — NEVER any SkiaSharp.* source
@@ -1305,26 +1311,61 @@ def determine_diff_range(branch):
 # can FOCUS on product, INSPECT mixed, and MENTION internal (§4.4) — a lexical filter
 # instead of re-judging every PR from its title each run (the source of leak variance).
 #
-#   product  — touches SHIPPED code (managed API, native code, Views). A real change a
-#              consumer can see. Companion test/benchmark/generated files are ignored.
-#   mixed    — touches only BUILD config (native/): may change the shipped binary via a
-#              compile define (e.g. a rasteriser flag), or may be pure infra (Docker
-#              image, SDK pin). Polish guesses from the title/context in data.json
-#              (it does not open the PR) — surface a behaviour change, drop pure infra.
-#   internal — touches NEITHER: a pure repository process (CI, workflows, agent skills,
-#              docs site, tests, samples, build/meta files). Dropped into the collapse line.
-_SHIP_PATH_PREFIXES = ("binding/", "externals/", "source/")
-_BUILD_PATH_PREFIXES = ("native/",)
+# The path→tag mapping is NOT hardcoded here: it lives in the committed
+# scripts/infra/docs/release-notes-paths.json, the single deterministic place to edit it
+# (ordered tiers of prefixes/globs + a default). See that file's `description`/`notes`
+# and spec §4.4 for the rationale (why `externals/skia` is exact, why `docs` is
+# slash-less, why `native/` is mixed, etc.).
+
+
+def _load_path_tags():
+    # type: () -> tuple
+    """Load release-notes-paths.json as ``([(tag, patterns), ...], default)`` (cached).
+
+    Ordered tiers + a default tag — the deterministic PR path-classification config
+    (spec §4.4). Read once and memoised; a missing/malformed file is a hard error (the
+    file is committed and required — do not silently fall back to a guessed mapping).
+    """
+    global _PATH_TAGS_CONFIG
+    if _PATH_TAGS_CONFIG is None:
+        if not PATH_TAGS_JSON_PATH.exists():
+            raise FileNotFoundError(
+                "Missing %s — the PR path-classification config (spec §4.4)."
+                % PATH_TAGS_JSON_PATH)
+        with open(PATH_TAGS_JSON_PATH) as f:
+            cfg = json.load(f)
+        tiers = [(t["tag"], tuple(t.get("patterns", [])))
+                 for t in cfg.get("tiers", [])]
+        _PATH_TAGS_CONFIG = (tiers, cfg.get("default", "internal"))
+    return _PATH_TAGS_CONFIG
+
+
+def _path_matches(path, pattern):
+    # type: (str, str) -> bool
+    """Does a file ``path`` match a config ``pattern``?
+
+    Plain patterns are PREFIXES (``str.startswith`` — so ``binding/`` catches everything
+    under it and a bare submodule gitlink like ``externals/skia`` / ``docs`` matches
+    itself). A pattern containing a glob metacharacter (``* ? [``) is matched with
+    ``fnmatch.fnmatchcase`` (case-sensitive, platform-independent) instead.
+    """
+    if any(c in pattern for c in "*?["):
+        return fnmatch.fnmatchcase(path, pattern)
+    return path.startswith(pattern)
 
 
 def _pr_category(files):
     # type: (set) -> str
-    """Classify a PR ``product`` / ``mixed`` / ``internal`` by touched files (§4.4)."""
-    if any(f.startswith(_SHIP_PATH_PREFIXES) for f in files):
-        return "product"
-    if any(f.startswith(_BUILD_PATH_PREFIXES) for f in files):
-        return "mixed"
-    return "internal"
+    """Classify a PR ``product`` / ``mixed`` / ``internal`` by touched files (§4.4).
+
+    Tiers from release-notes-paths.json are tried in order; the first tier with any
+    matching file wins, else the configured default.
+    """
+    tiers, default = _load_path_tags()
+    for tag, patterns in tiers:
+        if any(_path_matches(f, p) for f in files for p in patterns):
+            return tag
+    return default
 
 
 # Automation accounts — never credited as human contributors (§4.5). The workflow
@@ -1371,17 +1412,31 @@ def _files_by_commit(from_ref, to_ref):
     One ``git log --name-only`` call (no pathspec filter — we want each PR's FULL
     file set to classify it product vs internal). A ``\\x1e`` record separator
     prefixes each hash line so file lines can never be mistaken for a hash.
+
+    We SPLIT on ``\\x1e`` rather than matching a leading ``\\x1e`` per line: git
+    lists commits newest-first, and ``run()`` returns ``stdout.strip()`` — and
+    ``\\x1e`` is whitespace to Python's ``str.strip()``, so the FIRST (newest)
+    record loses its leading separator. A ``startswith("\\x1e")`` scan would then
+    silently drop that newest commit's file list, tagging it ``internal`` — which
+    is exactly how the latest automated ``[skia-sync]`` submodule bump (usually the
+    newest commit in the range) kept getting mis-tagged. Splitting recovers every
+    record whether or not its leading separator survived.
     """
     out = run(["git", "log", "--no-renames", "--name-only",
                "--format=%x1e%H", "{}..{}".format(from_ref, to_ref)])
     files_by = {}  # type: dict
-    cur = None
-    for line in out.split("\n"):
-        if line.startswith("\x1e"):
-            cur = line[1:].strip()
-            files_by[cur] = set()
-        elif line.strip() and cur is not None:
-            files_by[cur].add(line.strip())
+    for record in out.split("\x1e"):
+        record = record.strip()
+        if not record:
+            continue
+        # First line is the commit hash; the rest (past the blank line git emits
+        # before --name-only output) are the touched paths. Merge commits have no
+        # --name-only output, so they map to an empty set (still `internal`).
+        lines = record.split("\n")
+        commit = lines[0].strip()
+        if not commit:
+            continue
+        files_by[commit] = {ln.strip() for ln in lines[1:] if ln.strip()}
     return files_by
 
 

scripts/infra/docs/release-notes-paths.json

@@ -0,0 +1,30 @@
+{
+  "description": "Path-based PR classification for the release notes. release-notes-data.py reads this file to tag every merged PR by the files it touched (behaviour spec: documentation/dev/release-notes-and-api-diffs.md, section 4.4). This JSON is the single deterministic place to edit the mapping - the script does not hardcode which paths are product/mixed/internal.",
+  "matching": "Tiers are evaluated top-to-bottom; the FIRST tier with any file matching any of its patterns wins, otherwise `default`. A pattern matches a file path by PREFIX (str.startswith) - so `binding/` catches everything under it, and a bare submodule gitlink such as `externals/skia` or `docs` matches itself. A pattern that contains a glob metacharacter (* ? [) is matched with fnmatch instead (e.g. `native/*/libHarfBuzzSharp/**`).",
+  "tiers": [
+    {
+      "tag": "product",
+      "why": "Shipped code - a real API / behaviour / native change a consumer can see. Written up in the notes.",
+      "patterns": [
+        "binding/",
+        "source/",
+        "externals/skia"
+      ]
+    },
+    {
+      "tag": "mixed",
+      "why": "Affects the shipped package but is not itself an API/behaviour change; Polish judges from the PR title/context and surfaces only a real behaviour change, dropping pure infra / doc churn.",
+      "patterns": [
+        "native/",
+        "docs"
+      ]
+    }
+  ],
+  "default": "internal",
+  "notes": [
+    "`externals/skia` is intentionally exact, NOT `externals/`: the sibling `externals/depot_tools` (build toolchain) and `externals/.gitignore` are build infra and must stay `default` (internal).",
+    "`docs` is the mono/SkiaSharp-API-docs submodule (mdoc XML that ships as the packages' IntelliSense docs). It is slash-less so it matches the bare `docs` gitlink and does not collide with `documentation/` - the docs site, which stays internal.",
+    "`native/` covers the per-platform build config (gn args / compile flags) and the shipped WinUI native subtree under `native/winui/`.",
+    "Anything not matched (CI, workflows, agent skills, the `documentation/` docs site, tests, samples, build/meta files, and `externals/depot_tools`) falls through to `default`."
+  ]
+}