From 095b50f4b2ece4d9ef98698af2894269d2ba17a4 Mon Sep 17 00:00:00 2001 From: Matt Linville Date: Fri, 29 May 2026 13:38:12 -0700 Subject: [PATCH 1/5] ci: Readability-score delta Github Action Depends on a skill in https://github.com/coreweave/docs-skills/pull/42 Adding a submodule reference is a followup action after that PR merges, some time before merging this into wandb/docs The submodule will depend on the DOCENGINE_TOKEN already in wandb/docs for cross-org authentication. --- .github/workflows/README.md | 44 ++ .github/workflows/readability-delta.yml | 195 +++++++ scripts/readability/README.md | 53 ++ scripts/readability/pr_report.py | 532 ++++++++++++++++++++ scripts/readability/requirements.txt | 9 + scripts/readability/tests/__init__.py | 0 scripts/readability/tests/conftest.py | 13 + scripts/readability/tests/test_pr_report.py | 254 ++++++++++ 8 files changed, 1100 insertions(+) create mode 100644 .github/workflows/readability-delta.yml create mode 100644 scripts/readability/README.md create mode 100644 scripts/readability/pr_report.py create mode 100644 scripts/readability/requirements.txt create mode 100644 scripts/readability/tests/__init__.py create mode 100644 scripts/readability/tests/conftest.py create mode 100644 scripts/readability/tests/test_pr_report.py diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 97f0b42a38..9717649fbf 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -144,3 +144,47 @@ git add . git commit -m "Sync code examples from docs-code-eval" git push ``` + +## Readability delta + +**Workflow**: `readability-delta.yml` + +Posts an informational, **non-blocking** PR comment describing how the PR affects the readability of the English docs it changes (DOCS-2626). It reports the *delta* (before/after) for well-established formulas (Flesch-Kincaid grade, Flesch reading ease, Gunning fog, SMOG), word-weighted across the changed pages, plus an optional AI-agent-comprehension rating from a W&B Inference LLM judge. + +### Triggers + +- **Pull request**: `opened`, `synchronize`, `reopened` on PRs that touch `**/*.mdx` +- **Manual**: `workflow_dispatch` (writes the report to the job summary instead of a comment) + +### What it does + +1. Diffs the PR base and head, scoring each changed English `.mdx` file (localized content under `ja/`, `ko/`, and `fr/` is skipped). +2. Extracts narrative prose and scores it with `textstat` via the analyzer in the `coreweave/docs-skills` submodule (`.claude/scripts/_readability.py`). +3. Optionally runs the AI agent comprehension judge (W&B Inference) when `WANDB_API_KEY` is set. +4. Upserts a single PR comment identified by the `` marker. + +The check **never fails** a PR. If scoring is unavailable it posts a brief notice and exits successfully. + +### Configuration + +- **Python**: 3.11 +- **Permissions**: `contents: read`, `pull-requests: write` +- **Report glue**: `scripts/readability/pr_report.py` +- **Scoring logic**: `.claude/scripts/_readability.py` and `_docs_eval_lib.py` (submodule) + +### Authentication + +- The main checkout uses the default `GITHUB_TOKEN`. +- The private, cross-org `coreweave/docs-skills` submodule is initialized in a separate step with the `DOCENGINE_TOKEN` secret (the same `x-access-token` credential used for the `gitsubmodule` ecosystem in `.github/dependabot.yml`; it rotates ~every 30 days and needs no `wandb/docs` scope). +- The AI agent comprehension judge calls W&B Inference with the `WANDB_DOCS_INFERENCE_API_KEY` secret (a W&B API key whose entity has Inference credits), passed to the scorer as `WANDB_API_KEY`. When that secret is absent, the deterministic `textstat` delta still runs. + +### Forks + +Fork PRs have no access to repo secrets, so the first step detects a fork, posts an Actions notice, and makes the whole job a no-op (still reporting success). Forks are uncommon in `wandb/docs` and coreweave repos cannot use forks at all. + +### Related Files + +- **Report glue**: `scripts/readability/pr_report.py` +- **Dependencies**: `scripts/readability/requirements.txt` +- **Tests**: `scripts/readability/tests/` +- **Documentation**: `scripts/readability/README.md` diff --git a/.github/workflows/readability-delta.yml b/.github/workflows/readability-delta.yml new file mode 100644 index 0000000000..b6b44fdd5e --- /dev/null +++ b/.github/workflows/readability-delta.yml @@ -0,0 +1,195 @@ +# ------------------------------------------------------------------ +# Readability delta check (DOCS-2626) +# ------------------------------------------------------------------ +# +# What this workflow does +# ----------------------- +# Posts an informational, non-blocking PR comment describing how the PR affects +# the readability of the English docs it touches. The scoring logic lives in the +# coreweave/docs-skills submodule (.claude/scripts/_readability.py and +# _docs_eval_lib.py); scripts/readability/pr_report.py is the wandb/docs glue +# that diffs base..head, scores each changed .mdx file, and builds the Markdown. +# +# This check never fails a PR. Every step exits 0; the worst case is a comment +# noting that scoring was unavailable. +# +# Auth +# ---- +# - Main checkout uses the default GITHUB_TOKEN (the workflow runs in the +# wandb/docs PR context). +# - The coreweave/docs-skills submodule is private and cross-org, so a separate +# step initializes it with DOCENGINE_TOKEN (the same x-access-token credential +# used for the gitsubmodule ecosystem in .github/dependabot.yml). That token +# needs no wandb/docs scope. +# - The AI agent comprehension judge calls W&B Inference with the +# WANDB_DOCS_INFERENCE_API_KEY secret (a W&B key with Inference credits), +# passed to the scorer as WANDB_API_KEY. When that secret is absent the +# deterministic textstat delta still runs. +# +# Forks +# ----- +# Fork PRs have no access to repo secrets, so neither the submodule fetch nor the +# judge can run. The first step detects a fork and makes the whole job a no-op +# (posts an Actions notice, exits 0). Forks are uncommon in wandb/docs and +# coreweave repos cannot use forks at all, so the lost coverage is negligible. +# ------------------------------------------------------------------ + +name: Readability delta + +on: + pull_request: + types: [opened, synchronize, reopened] + paths: + - "**/*.mdx" + workflow_dispatch: + +concurrency: + group: readability-delta-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read + pull-requests: write + +jobs: + readability: + name: Report readability delta + runs-on: ubuntu-latest + steps: + # Fork no-op guard: fork PRs cannot reach the private submodule or W&B + # Inference, so skip the whole check (still reports success). + - name: Skip on fork PRs + id: fork + run: | + if [ "${{ github.event_name }}" = "pull_request" ] && \ + [ "${{ github.event.pull_request.head.repo.fork }}" = "true" ]; then + echo "is_fork=true" >> "$GITHUB_OUTPUT" + echo "::notice::Readability check skipped: fork PRs have no access to the docs-skills submodule or W&B Inference." + else + echo "is_fork=false" >> "$GITHUB_OUTPUT" + fi + + - name: Check out repository + if: steps.fork.outputs.is_fork != 'true' + uses: actions/checkout@v6 + with: + # Need full history to diff base..head. + fetch-depth: 0 + submodules: false + + # The docs-skills submodule is a private repo in another org. Initialize it + # with DOCENGINE_TOKEN so the main checkout token needs no extra scope. + - name: Check out docs-skills submodule + if: steps.fork.outputs.is_fork != 'true' + env: + DOCENGINE_TOKEN: ${{ secrets.DOCENGINE_TOKEN }} + run: | + git config --global url."https://x-access-token:${DOCENGINE_TOKEN}@github.com/".insteadOf "https://github.com/" + git submodule update --init --recursive .claude + + - name: Set up Python 3.11 + if: steps.fork.outputs.is_fork != 'true' + uses: actions/setup-python@v6 + with: + python-version: "3.11" + + - name: Cache pip packages + if: steps.fork.outputs.is_fork != 'true' + uses: actions/cache@v5 + with: + path: ~/.cache/pip + key: ${{ runner.os }}-pip-readability-${{ hashFiles('scripts/readability/requirements.txt') }} + restore-keys: | + ${{ runner.os }}-pip-readability- + ${{ runner.os }}-pip- + + - name: Install dependencies + if: steps.fork.outputs.is_fork != 'true' + run: pip install -r scripts/readability/requirements.txt + + # Resolve base and head SHAs. For pull_request, GitHub provides them. For + # workflow_dispatch, diff the merge-base of the current ref against the + # default branch. + - name: Resolve base and head + id: refs + if: steps.fork.outputs.is_fork != 'true' + env: + EVENT_NAME: ${{ github.event_name }} + PR_BASE: ${{ github.event.pull_request.base.sha }} + PR_HEAD: ${{ github.event.pull_request.head.sha }} + DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} + run: | + set -euo pipefail + if [ "$EVENT_NAME" = "pull_request" ]; then + echo "base=$PR_BASE" >> "$GITHUB_OUTPUT" + echo "head=$PR_HEAD" >> "$GITHUB_OUTPUT" + else + git fetch --no-tags --depth=200 origin "$DEFAULT_BRANCH" || true + BASE=$(git merge-base "origin/${DEFAULT_BRANCH}" HEAD || git rev-parse HEAD~1) + echo "base=$BASE" >> "$GITHUB_OUTPUT" + echo "head=$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT" + fi + + - name: Build readability report + id: report + if: steps.fork.outputs.is_fork != 'true' + env: + # Optional: enables the AI agent comprehension judge via W&B Inference. + # The scorer reads WANDB_API_KEY; the repo secret is named + # WANDB_DOCS_INFERENCE_API_KEY (a W&B key with Inference credits). + WANDB_API_KEY: ${{ secrets.WANDB_DOCS_INFERENCE_API_KEY }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + run: | + set -euo pipefail + JUDGE_FLAG="" + if [ -n "${WANDB_API_KEY:-}" ]; then + JUDGE_FLAG="--judge" + fi + # Never fail the PR: capture failures and fall back to a notice. + if python scripts/readability/pr_report.py \ + --base "${{ steps.refs.outputs.base }}" \ + --head "${{ steps.refs.outputs.head }}" \ + --include-marker \ + --run-url "${RUN_URL}" \ + --run-id "${GITHUB_RUN_ID}" \ + ${JUDGE_FLAG} \ + > comment-body.md 2> report-errors.log; then + echo "ok=true" >> "$GITHUB_OUTPUT" + else + echo "ok=false" >> "$GITHUB_OUTPUT" + echo "::warning::Readability report failed to build; see logs." + cat report-errors.log || true + { + echo "" + echo "" + echo "## Readability impact" + echo "" + echo "The readability check could not run for this push. This is informational and does not block the PR." + } > comment-body.md + fi + + - name: Upsert readability report on PR + if: steps.fork.outputs.is_fork != 'true' && github.event_name == 'pull_request' + env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + MARKER="" + COMMENT_ID=$(gh api \ + "repos/${{ github.repository }}/issues/${PR_NUMBER}/comments" \ + --paginate -q ".[] | select(.body | contains(\"${MARKER}\")) | .id" \ + | head -n1) + + if [ -n "${COMMENT_ID}" ]; then + gh api \ + "repos/${{ github.repository }}/issues/comments/${COMMENT_ID}" \ + -X PATCH -F "body=@comment-body.md" + echo "Updated existing readability report comment ${COMMENT_ID}" + else + gh pr comment "${PR_NUMBER}" --body-file comment-body.md + echo "Created new readability report comment" + fi + + - name: Write readability report to job summary + if: steps.fork.outputs.is_fork != 'true' && github.event_name == 'workflow_dispatch' + run: cat comment-body.md >> "${GITHUB_STEP_SUMMARY}" diff --git a/scripts/readability/README.md b/scripts/readability/README.md new file mode 100644 index 0000000000..d518b78cad --- /dev/null +++ b/scripts/readability/README.md @@ -0,0 +1,53 @@ +# Readability delta check + +An informational, non-blocking CI check that reports how a pull request affects +the readability of the docs it touches (DOCS-2626). It posts a single PR comment +with a word-weighted Flesch-Kincaid grade delta across the changed pages, plus an +optional AI-agent-comprehension rating. + +We report the *delta*, not an absolute grade. Technical docs have an inherently +high reading grade level, so "this page went from 12.4 to 11.1" is meaningful +where "12.4" on its own is not. A lower Flesch-Kincaid grade and a higher Flesch +reading ease both mean easier to read. + +## How it works + +The scoring logic lives in the `coreweave/docs-skills` submodule (`.claude/`), +shared with `coreweave/documentation`: + +- `.claude/scripts/_readability.py` extracts narrative prose from MDX (stripping + frontmatter, code, JSX/Mintlify components, tables, and links) and scores it + with `textstat` (Flesch reading ease, Flesch-Kincaid grade, Gunning fog, SMOG). +- `.claude/scripts/_docs_eval_lib.py` provides the AI-agent-comprehension LLM + judge (W&B Inference, authenticated with `WANDB_API_KEY`). +- `.claude/scripts/readability_baselines.json` holds per-doc-type baseline grade + levels (regenerate with `.claude/scripts/readability_baselines.py`). + +This directory holds the wandb/docs plumbing: + +- `pr_report.py` diffs the PR base and head, scores each changed English `.mdx` + file, aggregates a word-weighted delta, and builds the Markdown report. +- `tests/` covers the parsing and Markdown-building logic (no network needed). + +The workflow is `.github/workflows/readability-delta.yml`. + +## Behavior + +- **Non-blocking**: the check never fails a PR. It only posts a comment. +- **Forks**: skipped (no access to the submodule or W&B Inference); the workflow + posts an Actions notice and exits successfully. +- **Deterministic backbone**: the `textstat` delta always runs. The LLM judge is + supplementary and degrades gracefully when the W&B Inference key is absent. In + CI the key comes from the `WANDB_DOCS_INFERENCE_API_KEY` repo secret, passed to + the scorer as `WANDB_API_KEY`. +- **Localized content** under `ja/`, `ko/`, and `fr/` is skipped. + +## Run locally + +```bash +pip install -r scripts/readability/requirements.txt +git submodule update --init .claude +python3 scripts/readability/pr_report.py --base main --head HEAD +``` + +Add `--judge` to include the comprehension judge (requires `WANDB_API_KEY`). diff --git a/scripts/readability/pr_report.py b/scripts/readability/pr_report.py new file mode 100644 index 0000000000..06e8af3c6f --- /dev/null +++ b/scripts/readability/pr_report.py @@ -0,0 +1,532 @@ +#!/usr/bin/env python3 +""" +Readability delta PR report +=========================== + +Turns the readability impact of a pull request into Markdown for humans. GitHub +Actions either posts that Markdown as a single (upserted) PR comment or writes it +to the workflow job summary. + +What this script does (high level) +---------------------------------- +1. Lists the ``.mdx`` files changed between the PR base and head commit + (``git diff --name-status base head``), skipping localized content under + ``ja/``, ``ko/``, and ``fr/`` (translation is handled separately). +2. For each changed file, reads the **before** version (``git show base:path``) + and the **after** version (``git show head:path``), extracts narrative prose, + and scores both with the readability analyzer from the ``coreweave/docs-skills`` + submodule (``.claude/scripts/_readability.py``). +3. Optionally runs the AI-agent-comprehension LLM judge (W&B Inference) on the + before and after versions when ``--judge`` is set and ``WANDB_API_KEY`` is + available. The deterministic readability delta always runs; the judge is + supplementary and degrades gracefully. +4. Aggregates a word-weighted Flesch-Kincaid grade delta across the changed + pages and builds a Markdown report, optionally prefixed with an HTML marker so + the workflow can upsert a single comment. + +The deterministic analyzer and the LLM judge both live in the submodule so the +logic is shared with ``coreweave/documentation``. This script is the thin +wandb/docs plumbing around them. The Markdown-building functions are pure and +take plain numbers, so they can be unit tested without ``textstat`` or network +access. + +This check is informational. It never fails a PR. + +See also +-------- +- ``.claude/scripts/_readability.py`` for the analyzer. +- ``.claude/scripts/_docs_eval_lib.py`` for the comprehension judge rubric. +- ``.github/workflows/readability-delta.yml`` for when this runs. +""" + +from __future__ import annotations + +import argparse +import json +import os +import subprocess +import sys +from pathlib import Path +from typing import Dict, List, Optional + +# --------------------------------------------------------------------------- +# Constants +# --------------------------------------------------------------------------- + +# GitHub issue comments are upserted by searching for this exact HTML comment. +REPORT_MARKER = "" + +# Visible heading in PR comments and job summaries (sentence case per docs style). +REPORT_TITLE = "## Readability impact" + +# Shown when no scorable English .mdx prose changed in the PR. +REPORT_FALLBACK_PARAGRAPH = ( + "No English documentation prose changed in this PR, so there is no " + "readability delta to report." +) + +REPORT_FALLBACK_BODY = f"{REPORT_TITLE}\n\n{REPORT_FALLBACK_PARAGRAPH}" + +# One-line explanation included under the headline so reviewers know how to read +# the numbers and that the check is advisory. +REPORT_LEGEND = ( + "Lower Flesch-Kincaid grade and higher reading ease both mean easier to " + "read. This check is informational and never blocks a PR." +) + +# Localized content prefixes to skip. Translation is handled separately, so +# scoring localized prose with English-tuned formulas is not meaningful. +_LOCALE_PREFIXES = ("ja/", "ko/", "fr/") + +# Cap on how many pages get the (paid, slower) LLM comprehension judge per run. +# The deterministic delta still covers every changed page. +MAX_JUDGED_FILES = int(os.environ.get("READABILITY_MAX_JUDGED_FILES", "10")) + + +# --------------------------------------------------------------------------- +# Git helpers +# --------------------------------------------------------------------------- + + +def _run_git(args: List[str], repo_root: Path) -> subprocess.CompletedProcess: + return subprocess.run( + ["git", *args], + cwd=repo_root, + capture_output=True, + text=True, + check=False, + ) + + +def changed_mdx_name_status(base: str, head: str, repo_root: Path) -> str: + """Return ``git diff --name-status base head`` output (raw).""" + result = _run_git(["diff", "--name-status", base, head], repo_root) + if result.returncode != 0: + print(result.stderr, file=sys.stderr) + sys.exit(result.returncode or 1) + return result.stdout + + +def git_show(ref_path: str, repo_root: Path) -> str: + """Return the content of ``ref:path`` (``git show``), or "" if absent.""" + result = _run_git(["show", ref_path], repo_root) + if result.returncode != 0: + return "" + return result.stdout + + +# --------------------------------------------------------------------------- +# Parsing changed files +# --------------------------------------------------------------------------- + + +def parse_changed_files(name_status: str) -> List[Dict[str, str]]: + """ + Parse ``git diff --name-status`` into changed English ``.mdx`` entries. + + Returns a list of dicts with ``status`` (``A``/``M``/``D``/``R``) and + ``path`` (the new path for renames). Localized files and non-mdx files are + skipped. Deletions are kept so the report can note removed pages, but they + are not scored (no after version). + """ + entries: List[Dict[str, str]] = [] + for raw in name_status.splitlines(): + line = raw.rstrip("\n\r") + if not line.strip(): + continue + parts = line.split("\t") + if len(parts) < 2: + continue + status = parts[0] + # Renames/copies: use the new path (last field). + path = parts[-1] + if not path.endswith(".mdx"): + continue + if path.startswith(_LOCALE_PREFIXES): + continue + entries.append({"status": status[0], "path": path}) + return entries + + +# --------------------------------------------------------------------------- +# Scoring (requires the submodule analyzer; isolated for testability) +# --------------------------------------------------------------------------- + + +def _import_readability(skills_scripts: Path): + """Import the _readability module from the docs-skills submodule.""" + skills_scripts = skills_scripts.resolve() + if str(skills_scripts) not in sys.path: + sys.path.insert(0, str(skills_scripts)) + import _readability # type: ignore + + return _readability + + +def score_entry( + entry: Dict[str, str], + base: str, + head: str, + repo_root: Path, + readability, +) -> Dict[str, object]: + """Score one changed file, returning a plain-number result row.""" + path = entry["path"] + status = entry["status"] + + if status == "D": + return {"path": path, "status": "deleted"} + + before_text = "" if status == "A" else git_show(f"{base}:{path}", repo_root) + after_text = git_show(f"{head}:{path}", repo_root) + + before_prose = readability.extract_prose_mdx(before_text) + after_prose = readability.extract_prose_mdx(after_text) + report = readability.analyze_texts(before_prose, after_prose) + + before = report["before"] + after = report["after"] + delta = report["delta"] + headline = report["headline"] + + # A new file (or one with no scorable before) has no delta. + if after["insufficient_prose"]: + return {"path": path, "status": "insufficient_prose", + "after_word_count": after["word_count"]} + + return { + "path": path, + "status": "new" if status == "A" or before["insufficient_prose"] else "scored", + "fk_before": before["metrics"]["flesch_kincaid_grade"] if before["metrics"] else None, + "fk_after": after["metrics"]["flesch_kincaid_grade"] if after["metrics"] else None, + "fk_delta": delta["flesch_kincaid_grade"], + "ease_delta": delta["flesch_reading_ease"], + "direction": headline["direction"], + "after_word_count": after["word_count"], + } + + +def run_comprehension_judge( + skills_scripts: Path, + before_text: str, + after_text: str, +) -> Optional[Dict[str, object]]: + """ + Score AI agent comprehension on the before and after text via W&B Inference. + + Returns a dict with before/after ratings and the delta, or None if the judge + is unavailable (no WANDB_API_KEY, missing deps, or an API error). Imported + lazily so the deterministic path and the unit tests do not need weave/openai. + """ + if not os.environ.get("WANDB_API_KEY"): + return None + try: + skills_scripts = skills_scripts.resolve() + if str(skills_scripts) not in sys.path: + sys.path.insert(0, str(skills_scripts)) + from _docs_eval_lib import _RUBRIC_AI_COMPREHENSION, _call_judge # type: ignore + except Exception as exc: # pragma: no cover - depends on CI env + print(f"[readability] comprehension judge unavailable: {exc}", file=sys.stderr) + return None + + try: + before_res = ( + _call_judge(_RUBRIC_AI_COMPREHENSION, before_text) if before_text.strip() else None + ) + after_res = _call_judge(_RUBRIC_AI_COMPREHENSION, after_text) + except Exception as exc: # pragma: no cover - network/runtime + print(f"[readability] comprehension judge error: {exc}", file=sys.stderr) + return None + + before_rating = before_res.get("rating") if before_res else None + after_rating = after_res.get("rating") if after_res else None + rating_delta = ( + after_rating - before_rating + if isinstance(before_rating, int) and isinstance(after_rating, int) + else None + ) + return { + "before_rating": before_rating, + "after_rating": after_rating, + "rating_delta": rating_delta, + } + + +# --------------------------------------------------------------------------- +# Aggregation +# --------------------------------------------------------------------------- + + +def aggregate(results: List[Dict[str, object]]) -> Optional[Dict[str, object]]: + """ + Word-weighted Flesch-Kincaid grade delta across scored files. + + Weighting by the after-version word count keeps a one-line tweak on a tiny + page from dominating a large rewrite. Returns None when nothing was scored. + """ + weighted_sum = 0.0 + weight_total = 0 + scored = 0 + for r in results: + if r.get("status") != "scored": + continue + fk_delta = r.get("fk_delta") + words = r.get("after_word_count") or 0 + if fk_delta is None or words <= 0: + continue + weighted_sum += float(fk_delta) * int(words) + weight_total += int(words) + scored += 1 + + if weight_total == 0: + return None + + weighted_delta = round(weighted_sum / weight_total, 1) + if weighted_delta < 0: + direction = "easier" + elif weighted_delta > 0: + direction = "harder" + else: + direction = "unchanged" + return { + "weighted_fk_delta": weighted_delta, + "direction": direction, + "pages_scored": scored, + } + + +# --------------------------------------------------------------------------- +# Markdown building (pure; safe to unit test) +# --------------------------------------------------------------------------- + + +def _fmt(value: Optional[float]) -> str: + return "—" if value is None else f"{value:+.1f}" if isinstance(value, float) else str(value) + + +def _fmt_abs(value: Optional[float]) -> str: + return "—" if value is None else f"{value:.1f}" + + +def build_report_markdown( + results: List[Dict[str, object]], + summary: Optional[Dict[str, object]], + *, + baselines: Optional[Dict[str, object]] = None, + judge_by_path: Optional[Dict[str, Dict[str, object]]] = None, + run_url: Optional[str] = None, + run_id: Optional[str] = None, +) -> str: + """Build the report Markdown body (without the HTML marker).""" + scorable = [r for r in results if r.get("status") in ("scored", "new", "insufficient_prose", "deleted")] + if not scorable: + return REPORT_FALLBACK_BODY + + lines: List[str] = [REPORT_TITLE, ""] + + if summary: + n = summary["pages_scored"] + lines.append( + f"Word-weighted Flesch-Kincaid grade change across " + f"{n} changed page{'s' if n != 1 else ''}: " + f"**{summary['weighted_fk_delta']:+.1f} ({summary['direction']})**." + ) + else: + lines.append("No pages had enough prose to score a readability delta.") + lines.append("") + lines.append(REPORT_LEGEND) + lines.append("") + + # --- Human readability table --- + lines.append("### Human readability") + lines.append("") + lines.append("| Page | FK grade before | FK grade after | FK Δ | Reading ease Δ | Direction |") + lines.append("|------|-----------------|----------------|------|----------------|-----------|") + for r in results: + status = r.get("status") + path = r.get("path") + if status == "scored" or status == "new": + lines.append( + f"| `{path}` | {_fmt_abs(r.get('fk_before'))} | " + f"{_fmt_abs(r.get('fk_after'))} | {_fmt(r.get('fk_delta'))} | " + f"{_fmt(r.get('ease_delta'))} | {r.get('direction')} |" + ) + elif status == "insufficient_prose": + lines.append( + f"| `{path}` | — | — | — | — | too little prose to score |" + ) + elif status == "deleted": + lines.append(f"| `{path}` | — | — | — | — | page removed |") + + # --- AI agent comprehension table --- + if judge_by_path: + lines.append("") + lines.append("### AI agent comprehension") + lines.append("") + lines.append("Rated 0-3 (higher is easier for an agent to parse and act on).") + lines.append("") + lines.append("| Page | Before | After | Δ |") + lines.append("|------|--------|-------|---|") + for path, j in judge_by_path.items(): + br = j.get("before_rating") + ar = j.get("after_rating") + rd = j.get("rating_delta") + rd_str = f"{rd:+d}" if isinstance(rd, int) else "—" + lines.append( + f"| `{path}` | {br if br is not None else '—'} | " + f"{ar if ar is not None else '—'} | {rd_str} |" + ) + + # --- Corpus baseline context --- + baseline_line = _baseline_context_line(baselines) + if baseline_line: + lines.append("") + lines.append(baseline_line) + + # --- Footer --- + if run_url: + lines.append("") + if run_id: + lines.append(f"*From [workflow run {run_id}]({run_url})*") + else: + lines.append(f"*From [workflow run]({run_url})*") + + return "\n".join(lines) + + +def _baseline_context_line(baselines: Optional[Dict[str, object]]) -> str: + """One-line median FK grade context per doc type, if baselines are present.""" + if not baselines: + return "" + by_type = baselines.get("by_doc_type") or {} + if not isinstance(by_type, dict) or not by_type: + overall = baselines.get("overall") or {} + fk = (overall.get("flesch_kincaid_grade") or {}) if isinstance(overall, dict) else {} + median = fk.get("median") + if median is None: + return "" + return f"Corpus baseline: median FK grade {median} across curated docs." + parts = [] + for doc_type in sorted(by_type): + fk = by_type[doc_type].get("flesch_kincaid_grade") or {} + median = fk.get("median") + if median is not None: + parts.append(f"{doc_type} {median}") + if not parts: + return "" + return "Curated-docs baseline median FK grade by type: " + ", ".join(parts) + "." + + +def format_pr_body(inner_markdown: str, *, include_marker: bool = True) -> str: + """Optionally prefix the report with REPORT_MARKER for GitHub upserts.""" + if not include_marker: + return inner_markdown + return f"{REPORT_MARKER}\n\n{inner_markdown}" + + +# --------------------------------------------------------------------------- +# CLI +# --------------------------------------------------------------------------- + + +def _load_baselines(path: Optional[Path]) -> Optional[Dict[str, object]]: + if not path: + return None + try: + return json.loads(Path(path).read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError): + return None + + +def main() -> None: + parser = argparse.ArgumentParser( + description="Build the readability delta PR comment or job-summary Markdown.", + ) + parser.add_argument("--base", required=True, help="Base commit SHA or ref.") + parser.add_argument("--head", required=True, help="Head commit SHA or ref.") + parser.add_argument( + "--repo-root", type=Path, default=Path("."), + help="Repository root (default: current directory).", + ) + parser.add_argument( + "--skills-scripts", type=Path, default=Path(".claude/scripts"), + help="Path to the docs-skills scripts dir (the submodule).", + ) + parser.add_argument( + "--baselines", type=Path, default=Path(".claude/scripts/readability_baselines.json"), + help="Path to readability_baselines.json for corpus context.", + ) + parser.add_argument( + "--judge", action="store_true", + help="Also run the AI comprehension LLM judge (needs WANDB_API_KEY).", + ) + parser.add_argument("--run-url", default=None, help="Link to the Actions run.") + parser.add_argument("--run-id", default=None, help="Actions run id for link text.") + parser.add_argument( + "--include-marker", action="store_true", + help="Prefix output with the HTML marker for PR comment upserts.", + ) + parser.add_argument( + "--name-status", default=None, + help="For tests: use this git diff --name-status text instead of running git.", + ) + args = parser.parse_args() + + readability = _import_readability(args.skills_scripts) + + if args.name_status is not None: + name_status = args.name_status + else: + name_status = changed_mdx_name_status(args.base, args.head, args.repo_root) + + entries = parse_changed_files(name_status) + results = [ + score_entry(e, args.base, args.head, args.repo_root, readability) + for e in entries + ] + summary = aggregate(results) + baselines = _load_baselines(args.baselines) + + judge_by_path: Optional[Dict[str, Dict[str, object]]] = None + if args.judge: + judge_by_path = {} + judged = 0 + # Judge the most-changed pages first (by after word count), capped. + scored_paths = sorted( + [r for r in results if r.get("status") in ("scored", "new")], + key=lambda r: int(r.get("after_word_count") or 0), + reverse=True, + ) + for r in scored_paths: + if judged >= MAX_JUDGED_FILES: + break + path = str(r["path"]) + status = r.get("status") + before_text = ( + "" if status == "new" + else git_show(f"{args.base}:{path}", args.repo_root) + ) + after_text = git_show(f"{args.head}:{path}", args.repo_root) + j = run_comprehension_judge(args.skills_scripts, before_text, after_text) + if j is not None: + judge_by_path[path] = j + judged += 1 + if not judge_by_path: + judge_by_path = None + + inner = build_report_markdown( + results, + summary, + baselines=baselines, + judge_by_path=judge_by_path, + run_url=args.run_url, + run_id=args.run_id, + ) + out = format_pr_body(inner, include_marker=args.include_marker) + sys.stdout.write(out) + if not out.endswith("\n"): + sys.stdout.write("\n") + + +if __name__ == "__main__": + main() diff --git a/scripts/readability/requirements.txt b/scripts/readability/requirements.txt new file mode 100644 index 0000000000..dbf2620fd4 --- /dev/null +++ b/scripts/readability/requirements.txt @@ -0,0 +1,9 @@ +# Dependencies for the readability delta CI check (scripts/readability/pr_report.py). +# +# Deterministic readability scoring (always used). Imported by the analyzer in +# the coreweave/docs-skills submodule (.claude/scripts/_readability.py): +textstat +# AI agent comprehension judge (optional; only when WANDB_API_KEY is set). +# Imported lazily from the submodule's _docs_eval_lib.py: +openai +weave diff --git a/scripts/readability/tests/__init__.py b/scripts/readability/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/scripts/readability/tests/conftest.py b/scripts/readability/tests/conftest.py new file mode 100644 index 0000000000..dde7fbf0f5 --- /dev/null +++ b/scripts/readability/tests/conftest.py @@ -0,0 +1,13 @@ +""" +Pytest configuration for readability tests. + +Registers custom markers so ``pytest -m integration`` does not emit +``PytestUnknownMarkWarning``. +""" + + +def pytest_configure(config): + config.addinivalue_line( + "markers", + "integration: tests that need textstat and the docs-skills submodule.", + ) diff --git a/scripts/readability/tests/test_pr_report.py b/scripts/readability/tests/test_pr_report.py new file mode 100644 index 0000000000..343051e54e --- /dev/null +++ b/scripts/readability/tests/test_pr_report.py @@ -0,0 +1,254 @@ +""" +Unit tests for ``pr_report.py`` (Readability delta PR report). + +What these tests do +------------------- +They check the **pure functions**: parsing fake ``git diff --name-status`` lines, +aggregating word-weighted deltas, and building Markdown. These need neither a +real Git repository, ``textstat``, nor network access, because the +Markdown-building functions take plain numbers. + +A separate, optional ``integration``-marked test exercises the real +``_readability`` analyzer from the docs-skills submodule when it is available. + +Run with:: + + pytest scripts/readability/tests/test_pr_report.py -v +""" + +import sys +from pathlib import Path + +import pytest + +# --------------------------------------------------------------------------- +# Import the module under test. +# --------------------------------------------------------------------------- +_script_dir = Path(__file__).resolve().parent.parent +if str(_script_dir) not in sys.path: + sys.path.insert(0, str(_script_dir)) + +import pr_report # noqa: E402 + + +# --------------------------------------------------------------------------- +# parse_changed_files +# --------------------------------------------------------------------------- + + +def test_parse_changed_files_filters_and_keeps_status(): + name_status = "\n".join([ + "M\tweave/guides/tracking/threads.mdx", + "A\tmodels/new-page.mdx", + "D\tmodels/old-page.mdx", + "M\tdocs.json", # not mdx -> skip + "M\tja/weave/guides/tracking/threads.mdx", # localized -> skip + "M\tko/models/foo.mdx", # localized -> skip + ]) + entries = pr_report.parse_changed_files(name_status) + assert entries == [ + {"status": "M", "path": "weave/guides/tracking/threads.mdx"}, + {"status": "A", "path": "models/new-page.mdx"}, + {"status": "D", "path": "models/old-page.mdx"}, + ] + + +def test_parse_changed_files_rename_uses_new_path(): + # Rename rows have three tab fields: status, old path, new path. + name_status = "R096\tmodels/old.mdx\tmodels/renamed.mdx" + entries = pr_report.parse_changed_files(name_status) + assert entries == [{"status": "R", "path": "models/renamed.mdx"}] + + +def test_parse_changed_files_empty(): + assert pr_report.parse_changed_files("") == [] + + +# --------------------------------------------------------------------------- +# aggregate +# --------------------------------------------------------------------------- + + +def test_aggregate_word_weighted(): + results = [ + # Big page improved a little; small page got much worse. + {"status": "scored", "fk_delta": -1.0, "after_word_count": 900}, + {"status": "scored", "fk_delta": 9.0, "after_word_count": 100}, + {"status": "new", "fk_delta": None, "after_word_count": 500}, # ignored + {"status": "deleted"}, # ignored + ] + summary = pr_report.aggregate(results) + # (-1.0*900 + 9.0*100) / 1000 = 0.0 + assert summary["weighted_fk_delta"] == 0.0 + assert summary["pages_scored"] == 2 + assert summary["direction"] == "unchanged" + + +def test_aggregate_easier_direction(): + results = [{"status": "scored", "fk_delta": -2.0, "after_word_count": 100}] + summary = pr_report.aggregate(results) + assert summary["weighted_fk_delta"] == -2.0 + assert summary["direction"] == "easier" + + +def test_aggregate_none_when_nothing_scored(): + results = [{"status": "deleted"}, {"status": "insufficient_prose"}] + assert pr_report.aggregate(results) is None + + +# --------------------------------------------------------------------------- +# build_report_markdown +# --------------------------------------------------------------------------- + + +def test_build_report_fallback_when_no_scorable(): + assert pr_report.build_report_markdown([], None) == pr_report.REPORT_FALLBACK_BODY + + +def test_build_report_includes_headline_and_table(): + results = [ + { + "status": "scored", + "path": "weave/guides/threads.mdx", + "fk_before": 12.4, + "fk_after": 11.1, + "fk_delta": -1.3, + "ease_delta": 5.2, + "direction": "easier", + "after_word_count": 800, + }, + { + "status": "new", + "path": "models/new.mdx", + "fk_before": None, + "fk_after": 9.8, + "fk_delta": None, + "ease_delta": None, + "direction": "n/a", + "after_word_count": 300, + }, + {"status": "deleted", "path": "models/old.mdx"}, + {"status": "insufficient_prose", "path": "models/tiny.mdx", "after_word_count": 5}, + ] + summary = pr_report.aggregate(results) + md = pr_report.build_report_markdown(results, summary) + + assert pr_report.REPORT_TITLE in md + assert "Word-weighted Flesch-Kincaid grade change" in md + assert "-1.3 (easier)" in md + assert "`weave/guides/threads.mdx`" in md + assert "11.1" in md + assert "-1.3" in md + assert "+5.2" in md + assert "page removed" in md + assert "too little prose to score" in md + # Never blocks: the legend says so. + assert "informational" in md + + +def test_build_report_judge_table(): + results = [ + { + "status": "scored", + "path": "a.mdx", + "fk_before": 12.0, + "fk_after": 10.0, + "fk_delta": -2.0, + "ease_delta": 4.0, + "direction": "easier", + "after_word_count": 500, + }, + ] + judge = {"a.mdx": {"before_rating": 1, "after_rating": 3, "rating_delta": 2}} + md = pr_report.build_report_markdown( + results, pr_report.aggregate(results), judge_by_path=judge + ) + assert "AI agent comprehension" in md + assert "| `a.mdx` | 1 | 3 | +2 |" in md + + +def test_build_report_baseline_context_line(): + results = [ + { + "status": "scored", + "path": "a.mdx", + "fk_before": 12.0, + "fk_after": 10.0, + "fk_delta": -2.0, + "ease_delta": 4.0, + "direction": "easier", + "after_word_count": 500, + }, + ] + baselines = { + "by_doc_type": { + "procedural": {"flesch_kincaid_grade": {"median": 8.8}}, + "conceptual": {"flesch_kincaid_grade": {"median": 10.5}}, + } + } + md = pr_report.build_report_markdown( + results, pr_report.aggregate(results), baselines=baselines + ) + assert "baseline median FK grade by type" in md + assert "procedural 8.8" in md + assert "conceptual 10.5" in md + + +def test_build_report_footer_link(): + results = [ + { + "status": "scored", + "path": "a.mdx", + "fk_before": 12.0, + "fk_after": 10.0, + "fk_delta": -2.0, + "ease_delta": 4.0, + "direction": "easier", + "after_word_count": 500, + }, + ] + md = pr_report.build_report_markdown( + results, pr_report.aggregate(results), + run_url="https://example.com/run/9", run_id="9", + ) + assert "*From [workflow run 9](https://example.com/run/9)*" in md + + +def test_format_pr_body_marker(): + body = pr_report.format_pr_body("hello", include_marker=True) + assert body.startswith(pr_report.REPORT_MARKER) + assert "hello" in body + assert pr_report.format_pr_body("hello", include_marker=False) == "hello" + + +# --------------------------------------------------------------------------- +# Optional integration test against the real analyzer in the submodule. +# --------------------------------------------------------------------------- + + +@pytest.mark.integration +def test_analyzer_round_trip(): + """Exercise the real _readability analyzer if textstat + submodule exist.""" + skills_scripts = Path(__file__).resolve().parents[3] / ".claude" / "scripts" + if not (skills_scripts / "_readability.py").exists(): + pytest.skip("docs-skills submodule not checked out") + try: + readability = pr_report._import_readability(skills_scripts) + except ImportError: + pytest.skip("submodule analyzer not importable") + try: + import textstat # noqa: F401 + except ImportError: + pytest.skip("textstat not installed") + + before = "In order to utilize the system, you must subsequently initialize " \ + "the configuration parameters prior to the commencement of operation, " \ + "which necessitates considerable and careful deliberation throughout." + after = "To set up the system, set the config values first. Do this before you start." + report = readability.analyze_texts( + readability.extract_prose_mdx(before), + readability.extract_prose_mdx(after), + ) + # The simpler "after" text should read at a lower grade if both are scorable. + if report["headline"]["delta"] is not None: + assert report["headline"]["direction"] in ("easier", "harder", "unchanged") From 5d0084f86fb85805f13ee1f4f1a90c2f2b7861a8 Mon Sep 17 00:00:00 2001 From: Matt Linville Date: Thu, 9 Jul 2026 12:45:19 -0700 Subject: [PATCH 2/5] chore: bump .claude submodule to docs-skills main (readability analyzer) Points the .claude submodule at coreweave/docs-skills main (11d7344), which lands the readability delta analyzer and AI comprehension scorer (#42) that the readability-delta CI check imports from .claude/scripts/. Co-Authored-By: Claude Opus 4.8 (1M context) --- .claude | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.claude b/.claude index f595d1a0cb..11d7344f33 160000 --- a/.claude +++ b/.claude @@ -1 +1 @@ -Subproject commit f595d1a0cb192c7a46dc28c0189803238345aff1 +Subproject commit 11d7344f33684fe7eacae5533cd8fbdcfd7dac1e From 9c13165f26caec21fbc179a2c29e0a27ccec002c Mon Sep 17 00:00:00 2001 From: Matt Linville Date: Thu, 9 Jul 2026 13:24:36 -0700 Subject: [PATCH 3/5] =?UTF-8?q?ci(readability):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20rename=20scoring,=20SHA=20pins,=20graceful=20degrad?= =?UTF-8?q?ation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Preserve the old path for renamed/copied .mdx so the base version is read from the old path; a renamed+edited page is now scored as a delta instead of being misreported as brand-new. Covers both the deterministic scorer and the comprehension judge. Adds parse + score_entry unit tests. - Pin actions/checkout, actions/setup-python, actions/cache to the commit SHAs already used across this repo's workflows (supply-chain hardening). - Mark the submodule-fetch and dependency-install steps continue-on-error so a transient failure degrades to the informational fallback comment instead of failing the check, matching the workflow's "never blocks a PR" contract. - Pin textstat/openai/weave to the verified-working versions, matching the repo's requirements-pinning convention. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/readability-delta.yml | 13 ++++-- scripts/readability/pr_report.py | 23 +++++---- scripts/readability/requirements.txt | 6 +-- scripts/readability/tests/test_pr_report.py | 52 ++++++++++++++++++--- 4 files changed, 74 insertions(+), 20 deletions(-) diff --git a/.github/workflows/readability-delta.yml b/.github/workflows/readability-delta.yml index b6b44fdd5e..5e9d8ecff4 100644 --- a/.github/workflows/readability-delta.yml +++ b/.github/workflows/readability-delta.yml @@ -71,7 +71,7 @@ jobs: - name: Check out repository if: steps.fork.outputs.is_fork != 'true' - uses: actions/checkout@v6 + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 with: # Need full history to diff base..head. fetch-depth: 0 @@ -80,6 +80,10 @@ jobs: # The docs-skills submodule is a private repo in another org. Initialize it # with DOCENGINE_TOKEN so the main checkout token needs no extra scope. - name: Check out docs-skills submodule + # Non-fatal: a transient auth/network failure here must not fail the PR. + # The report step below detects the missing analyzer and posts the + # informational fallback comment instead. + continue-on-error: true if: steps.fork.outputs.is_fork != 'true' env: DOCENGINE_TOKEN: ${{ secrets.DOCENGINE_TOKEN }} @@ -89,13 +93,13 @@ jobs: - name: Set up Python 3.11 if: steps.fork.outputs.is_fork != 'true' - uses: actions/setup-python@v6 + uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6 with: python-version: "3.11" - name: Cache pip packages if: steps.fork.outputs.is_fork != 'true' - uses: actions/cache@v5 + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6 with: path: ~/.cache/pip key: ${{ runner.os }}-pip-readability-${{ hashFiles('scripts/readability/requirements.txt') }} @@ -104,6 +108,9 @@ jobs: ${{ runner.os }}-pip- - name: Install dependencies + # Non-fatal: if deps fail to install, the report step still runs and + # falls back to the informational notice rather than failing the PR. + continue-on-error: true if: steps.fork.outputs.is_fork != 'true' run: pip install -r scripts/readability/requirements.txt diff --git a/scripts/readability/pr_report.py b/scripts/readability/pr_report.py index 06e8af3c6f..272d7d4d6a 100644 --- a/scripts/readability/pr_report.py +++ b/scripts/readability/pr_report.py @@ -124,10 +124,11 @@ def parse_changed_files(name_status: str) -> List[Dict[str, str]]: """ Parse ``git diff --name-status`` into changed English ``.mdx`` entries. - Returns a list of dicts with ``status`` (``A``/``M``/``D``/``R``) and - ``path`` (the new path for renames). Localized files and non-mdx files are - skipped. Deletions are kept so the report can note removed pages, but they - are not scored (no after version). + Returns a list of dicts with ``status`` (``A``/``M``/``D``/``R``/``C``), + ``path`` (the new path for renames/copies), and ``old_path`` (the base-side + path, which differs from ``path`` only for renames/copies). Localized files + and non-mdx files are skipped. Deletions are kept so the report can note + removed pages, but they are not scored (no after version). """ entries: List[Dict[str, str]] = [] for raw in name_status.splitlines(): @@ -138,13 +139,15 @@ def parse_changed_files(name_status: str) -> List[Dict[str, str]]: if len(parts) < 2: continue status = parts[0] - # Renames/copies: use the new path (last field). + # Renames/copies (R###/C###) have three fields: status, old, new. + # Score against the new path but read the base version from the old one. path = parts[-1] + old_path = parts[1] if status[0] in ("R", "C") and len(parts) >= 3 else path if not path.endswith(".mdx"): continue if path.startswith(_LOCALE_PREFIXES): continue - entries.append({"status": status[0], "path": path}) + entries.append({"status": status[0], "path": path, "old_path": old_path}) return entries @@ -173,11 +176,13 @@ def score_entry( """Score one changed file, returning a plain-number result row.""" path = entry["path"] status = entry["status"] + # For renames/copies the base version lives at the old path. + old_path = entry.get("old_path", path) if status == "D": return {"path": path, "status": "deleted"} - before_text = "" if status == "A" else git_show(f"{base}:{path}", repo_root) + before_text = "" if status == "A" else git_show(f"{base}:{old_path}", repo_root) after_text = git_show(f"{head}:{path}", repo_root) before_prose = readability.extract_prose_mdx(before_text) @@ -196,6 +201,7 @@ def score_entry( return { "path": path, + "old_path": old_path, "status": "new" if status == "A" or before["insufficient_prose"] else "scored", "fk_before": before["metrics"]["flesch_kincaid_grade"] if before["metrics"] else None, "fk_after": after["metrics"]["flesch_kincaid_grade"] if after["metrics"] else None, @@ -501,10 +507,11 @@ def main() -> None: if judged >= MAX_JUDGED_FILES: break path = str(r["path"]) + old_path = str(r.get("old_path", path)) status = r.get("status") before_text = ( "" if status == "new" - else git_show(f"{args.base}:{path}", args.repo_root) + else git_show(f"{args.base}:{old_path}", args.repo_root) ) after_text = git_show(f"{args.head}:{path}", args.repo_root) j = run_comprehension_judge(args.skills_scripts, before_text, after_text) diff --git a/scripts/readability/requirements.txt b/scripts/readability/requirements.txt index dbf2620fd4..85f3e79a71 100644 --- a/scripts/readability/requirements.txt +++ b/scripts/readability/requirements.txt @@ -2,8 +2,8 @@ # # Deterministic readability scoring (always used). Imported by the analyzer in # the coreweave/docs-skills submodule (.claude/scripts/_readability.py): -textstat +textstat==0.7.13 # AI agent comprehension judge (optional; only when WANDB_API_KEY is set). # Imported lazily from the submodule's _docs_eval_lib.py: -openai -weave +openai==2.45.0 +weave==0.53.1 diff --git a/scripts/readability/tests/test_pr_report.py b/scripts/readability/tests/test_pr_report.py index 343051e54e..9dc01db374 100644 --- a/scripts/readability/tests/test_pr_report.py +++ b/scripts/readability/tests/test_pr_report.py @@ -47,17 +47,57 @@ def test_parse_changed_files_filters_and_keeps_status(): ]) entries = pr_report.parse_changed_files(name_status) assert entries == [ - {"status": "M", "path": "weave/guides/tracking/threads.mdx"}, - {"status": "A", "path": "models/new-page.mdx"}, - {"status": "D", "path": "models/old-page.mdx"}, + {"status": "M", "path": "weave/guides/tracking/threads.mdx", + "old_path": "weave/guides/tracking/threads.mdx"}, + {"status": "A", "path": "models/new-page.mdx", + "old_path": "models/new-page.mdx"}, + {"status": "D", "path": "models/old-page.mdx", + "old_path": "models/old-page.mdx"}, ] -def test_parse_changed_files_rename_uses_new_path(): - # Rename rows have three tab fields: status, old path, new path. +def test_parse_changed_files_rename_keeps_both_paths(): + # Rename rows have three tab fields: status, old path, new path. Keep the + # old path so scoring can compare base(old) vs head(new) rather than + # treating the page as brand-new. name_status = "R096\tmodels/old.mdx\tmodels/renamed.mdx" entries = pr_report.parse_changed_files(name_status) - assert entries == [{"status": "R", "path": "models/renamed.mdx"}] + assert entries == [ + {"status": "R", "path": "models/renamed.mdx", "old_path": "models/old.mdx"} + ] + + +def test_score_entry_rename_reads_base_from_old_path(monkeypatch): + # A renamed page must load its before text from the OLD path; otherwise the + # base lookup returns empty and the page is misreported as new. + entry = {"status": "R", "path": "models/renamed.mdx", "old_path": "models/old.mdx"} + seen = {} + + def fake_git_show(ref_path, repo_root): + seen[ref_path] = True + return "before" if ref_path.endswith(":models/old.mdx") else "after" + + class FakeReadability: + def extract_prose_mdx(self, text): + return text + + def analyze_texts(self, before_prose, after_prose): + return { + "before": {"insufficient_prose": before_prose == "", + "metrics": {"flesch_kincaid_grade": 10.0}}, + "after": {"insufficient_prose": False, "word_count": 100, + "metrics": {"flesch_kincaid_grade": 9.0}}, + "delta": {"flesch_kincaid_grade": -1.0, "flesch_reading_ease": 1.0}, + "headline": {"direction": "easier"}, + } + + monkeypatch.setattr(pr_report, "git_show", fake_git_show) + result = pr_report.score_entry(entry, "BASE", "HEAD", Path("."), FakeReadability()) + + assert "BASE:models/old.mdx" in seen # base read from the old path + assert "HEAD:models/renamed.mdx" in seen # head read from the new path + assert result["status"] == "scored" # has a real before -> scored, not new + assert result["fk_delta"] == -1.0 def test_parse_changed_files_empty(): From dffc50913912b2f1f4e3535966099683d834d3fe Mon Sep 17 00:00:00 2001 From: Matt Linville Date: Thu, 9 Jul 2026 13:36:30 -0700 Subject: [PATCH 4/5] ci(readability): clarify no-delta message for add-only/delete-only PRs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit aggregate() returns None whenever no page has a scorable before-and-after version — which includes PRs that only add new pages or only delete pages, not just insufficient prose. Generalize the message and point at the per-page table for specifics. Addresses Copilot review follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) --- scripts/readability/pr_report.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/readability/pr_report.py b/scripts/readability/pr_report.py index 272d7d4d6a..3a5d8c08ae 100644 --- a/scripts/readability/pr_report.py +++ b/scripts/readability/pr_report.py @@ -338,7 +338,10 @@ def build_report_markdown( f"**{summary['weighted_fk_delta']:+.1f} ({summary['direction']})**." ) else: - lines.append("No pages had enough prose to score a readability delta.") + lines.append( + "No changed page had a scorable before-and-after version, so there " + "is no readability delta to report. See the per-page details below." + ) lines.append("") lines.append(REPORT_LEGEND) lines.append("") From 35bea8265f52ab83d3dab84a58ea7789eaf081fc Mon Sep 17 00:00:00 2001 From: Matt Linville Date: Thu, 9 Jul 2026 13:36:30 -0700 Subject: [PATCH 5/5] TEMP: reword artifact privacy intro to exercise the readability check Temporary prose simplification (FK 10.0 -> 9.5, "easier") to trigger and demonstrate the readability-delta workflow on this PR. REVERT before merge. Co-Authored-By: Claude Opus 4.8 (1M context) --- models/artifacts/data-privacy-and-compliance.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/artifacts/data-privacy-and-compliance.mdx b/models/artifacts/data-privacy-and-compliance.mdx index c60086c9f4..acb02776dd 100644 --- a/models/artifacts/data-privacy-and-compliance.mdx +++ b/models/artifacts/data-privacy-and-compliance.mdx @@ -4,7 +4,7 @@ description: Learn where W&B files are stored by default. Explore how to save, s title: Artifact data privacy and compliance --- -Files are uploaded to a Google Cloud bucket managed by W&B when you log artifacts. The contents of the bucket are encrypted both at rest and in transit. Artifact files are only visible to users who have access to the corresponding project. +When you log artifacts, W&B uploads the files to a Google Cloud bucket. W&B encrypts the bucket at rest and in transit. Only users with access to the project can see the files. GCS W&B Client Server diagram