diff --git a/.github/workflows/quality-advisory.yml b/.github/workflows/quality-advisory.yml index 18be46f6..c982483b 100644 --- a/.github/workflows/quality-advisory.yml +++ b/.github/workflows/quality-advisory.yml @@ -7,20 +7,53 @@ name: quality-advisory # raw complexity, coverage %, and clone counts are weak / gameable signals (rubric section 4.1) and must # never be a hard gate. This workflow is NOT in branch protection; adding a job here cannot freeze the repo. # -# Signals from the rubric (renumbered by tier -- docs/Code_Quality_Standards.md v0.7): -# * signal 11 - complexity triage (ruff C901) -> BUILT (complexity job) -# * signal 9 - clone / duplication (jscpd) -> BUILT (clone job) -# * signal 8 - diff-coverage visibility -> BUILT (coverage job; PR-only) -# * signal 7 - mutation testing (highest leverage) -> BUILT (mutation job; mirror-nightly + workflow_dispatch) +# Signals from the rubric (renumbered by tier -- docs/Code_Quality_Standards.md v0.7), each with how +# it reaches a reviewer: +# * signal 11 - complexity triage (ruff C901) -> BUILT (complexity job); PR-caused DELTA as +# annotations + step summary, full list in the log +# * signal 9 - clone / duplication (jscpd) -> BUILT (clone job); step summary only, see below +# * signal 8 - diff-coverage visibility -> BUILT (coverage job; PR-only); INLINE `::notice` +# annotations on the Files changed tab + summary +# * signal 7 - mutation testing (highest leverage) -> BUILT and WORKING on mutmut 3 (2.5.1 crashed on +# Python 3.14 before producing a mutant, and `|| true` +# made that look green for months). Measured: 461 +# mutants in 3s -- 87 killed, 19 survived. Runs on +# PRs too; survivor table in the step summary. # signal 10 (lint breadth) is a coordinated one-shot sweep, not an advisory job; DORA is a context caveat, not a gate. +# +# WHY THIS USES WORKFLOW-COMMAND ANNOTATIONS AND *NOT* CODE SCANNING / SARIF. This was measured, not +# assumed, and the measurements are recorded here so a future session does not "helpfully" add a SARIF +# upload back: +# 1. All 122 C901 findings on this tree are SINGLE-LINE regions anchored on the function-name token of +# the `def`. GitHub renders an alert inline only when its lines are in the diff, so a PR adding +# branching INSIDE a function surfaces nothing, while a signature reflow fires on pre-existing debt. +# 2. jscpd emits ONE location per clone pair, chosen by scan order, so ~8 files can never be the anchor +# and roughly half of newly-introduced clones would anchor on the untouched twin. +# 3. Uploading on pull_request only leaves NO default-branch baseline, so every PR forever would report +# all ~161 findings as new -- not a one-off first-PR blip. +# Workflow commands need no token and no permission grant, and behave identically on fork PRs. Every job +# below therefore holds `contents: read` and nothing else -- least privilege, worth pinning on its own +# merits given that two of these jobs execute third-party code fetched at run time. +# +# What actually keeps this advisory is TWO things, and permissions are neither of them: (a) these job +# contexts are not in the required-checks set on `main`, and (b) every analysis step is +# `continue-on-error: true` plus `--exit-zero` / `--fail-under=0` / `|| true`, so the job reports success +# regardless of findings. Branch-protection membership is the only thing that determines gating. +# tests/test_quality_advisory_invariants.py pins (b), the absent write scopes, and the absence of SARIF. +# +# HONEST LIMIT of the annotation mechanism: a workflow-command annotation renders INLINE on the Files +# changed tab only when its line is in the diff. That always holds for diff-coverage (every line it +# flags is a line the PR changed) but usually does NOT hold for the complexity delta, whose findings +# anchor on a `def` line the PR often did not touch -- those land in the Checks tab and the step summary +# instead. The summary table is the primary surface for complexity, not a nice-to-have. on: pull_request: workflow_dispatch: schedule: - # Nightly mutation run. The mutation job's `if:` gates it to the MIRROR (free minutes); on the - # private repo mutation runs only via workflow_dispatch. The cheap jobs (complexity/clone) also - # fire on this cron but cost seconds, and coverage is PR-only so it no-ops here. + # Nightly sweep. Every job here is cheap (mutation is ~3s of actual mutating; complexity and clone + # are seconds), so the cron is a safety net that catches drift on `main` rather than a cost dodge. + # Coverage is PR-only -- it needs a base ref to diff against -- so it no-ops here. - cron: "23 4 * * *" # Deny-by-default at the workflow level; each job grants only what it needs. @@ -40,15 +73,64 @@ jobs: uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false - - name: Install ruff (pinned) - run: pipx install ruff==0.15.19 + # The delta step needs the base branch's history for `git merge-base`. + fetch-depth: 0 + - name: Install ruff (version derived from the lock) + # DERIVED, not duplicated. This job used to hardcode 0.15.19 while all four locks pinned 0.15.22, + # and that drift is load-bearing: c901_delta.py parses ruff's human-readable C901 message because + # ruff exposes the complexity number nowhere else, so a skew can change the wording under the + # parser. The obvious fix -- hardcode the right version and assert it matches the lock in a test -- + # would leave a REQUIRED check (the pytest suite) hostage to a routine Dependabot ruff bump: + # the lock moves, the workflow string does not, and a blocking context goes red for a purely + # advisory concern. Reading the lock at run time removes the drift AND the coupling. + run: | + RUFF_PIN="$(sed -n 's/^ruff==\([^ ;]*\).*/\1/p' constraints.lock | head -1)" + if [ -z "$RUFF_PIN" ]; then + echo "::notice title=Ruff pin not found::falling back to the unpinned ruff from PyPI" + pipx install ruff + else + echo "ruff pin from constraints.lock: $RUFF_PIN" + pipx install "ruff==$RUFF_PIN" + fi - name: Ruff C901 complexity triage (advisory - never gates) + continue-on-error: true run: | echo "== functions over mccabe complexity 10 (advisory triage; NEVER a merge gate) ==" ruff check --select C901 --output-format=concise --exit-zero messagefoundry echo "" echo "== summary ==" ruff check --select C901 --statistics --exit-zero messagefoundry + - name: Complexity delta vs the merge base (advisory - never gates) + # PR-only: there is no base ref to diff against on the cron or on workflow_dispatch, where the + # whole-repo console triage above remains the full picture. + if: github.event_name == 'pull_request' + continue-on-error: true + env: + BASE_REF: ${{ github.base_ref }} + run: | + echo "== complexity this PR CHANGED (advisory; pre-existing findings deliberately omitted) ==" + git fetch --no-tags origin "$BASE_REF" || true + # Steps run under `bash -e`, so an unresolvable merge base would abort this step with no + # explanation at all. Say why instead: an advisory signal that goes quiet should at least + # leave a reason in the log, since nobody watches a green advisory job for absence. + if ! MERGE_BASE="$(git merge-base HEAD "origin/$BASE_REF" 2>/dev/null)"; then + echo "::notice title=Complexity delta skipped::could not resolve a merge base against origin/$BASE_REF" + exit 0 + fi + echo "merge base: $MERGE_BASE" + ruff check --select C901 --output-format=json --exit-zero messagefoundry > c901-head.json + # A second checkout of the merge base, so the "before" numbers come from real ruff output + # rather than a reconstruction. Its absolute paths differ from HEAD's by the extra directory; + # c901_delta.py collapses both to repo-relative keys (regression-tested -- if it did not, + # every function would report as new). + git worktree add --detach base-tree "$MERGE_BASE" + ( cd base-tree && ruff check --select C901 --output-format=json --exit-zero messagefoundry ) \ + > c901-base.json + python3 scripts/quality/c901_delta.py \ + --base c901-base.json \ + --head c901-head.json \ + --repo-root . \ + --summary-file "$GITHUB_STEP_SUMMARY" clone: # Signal 9 - duplication / clone detection. Flags copy-pasted blocks (the "copy-instead-of-abstract" @@ -65,13 +147,32 @@ jobs: with: persist-credentials: false - name: jscpd clone report (advisory - never gates) + continue-on-error: true run: | echo "== copy-pasted blocks in messagefoundry/ (advisory; justified store-backend parity ignored) ==" + # Summary only, DELIBERATELY. jscpd ships a SARIF reporter and it works, but each result + # carries exactly ONE location -- whichever file jscpd scanned first -- so the case this + # signal exists for (a PR copy-pasting an existing block into a new place) anchors on the + # untouched twin about half the time. That is a coin flip, not a diff-scoped signal. + # Keep the 4.x pin: npm `latest` is a 5.x Rust rewrite with a different CLI. npx --yes jscpd@4.0.5 messagefoundry \ --min-tokens 60 \ --ignore "**/store/sqlserver.py,**/store/postgres.py" \ - --reporters console \ + --reporters console,markdown \ + --output ./jscpd-report \ || true + if [ -f jscpd-report/jscpd-report.md ]; then + { + echo "## Duplicate blocks (jscpd)" + echo "" + # $GITHUB_STEP_SUMMARY is capped at 1 MiB per step and an oversized write is DROPPED + # ENTIRELY, so truncate rather than silently lose the whole summary. + head -c 900000 jscpd-report/jscpd-report.md + echo "" + echo "Advisory only. Clone locations are in the job log — see the note in the" + echo "workflow for why these are not annotated on the diff." + } >> "$GITHUB_STEP_SUMMARY" + fi coverage: # Signal 8 - diff-coverage visibility. Runs the suite under coverage, then reports coverage of the @@ -104,28 +205,70 @@ jobs: - name: Install project + coverage tools run: | uv pip install --system --constraint constraints.lock -e ".[dev,harness,fhir,dicom,x12,xml,webauthn]" -e packaging/messagefoundry-webconsole - uv pip install --system --constraint constraints.lock pytest-cov diff-cover + # NEITHER package is in constraints.lock, so the --constraint above is a no-op for them and both + # previously floated to whatever PyPI served that day. diff-cover is now pinned exactly: the + # inline-annotation surface depends on `--format github-annotations:` and on adjacent-line + # coalescing in GitHubAnnotationsReportGenerator. Deliberately NOT in pyproject.toml -- these are + # CI-only tools, and adding them would trip the DEP-1 lock-sync gate and force four re-exports. + uv pip install --system --constraint constraints.lock pytest-cov "diff-cover==10.4.1" - name: Tests under coverage (advisory - never fails) + continue-on-error: true env: QT_QPA_PLATFORM: offscreen run: pytest -q --cov=messagefoundry --cov-report=xml --cov-report=term-missing:skip-covered || true - name: Diff-coverage vs the PR base (advisory - never fails) + continue-on-error: true env: BASE_REF: ${{ github.base_ref }} run: | echo "== coverage of the lines this PR changed (advisory; not a whole-repo % gate) ==" - git fetch --no-tags --depth=1 origin "$BASE_REF" || true - diff-cover coverage.xml --compare-branch="origin/$BASE_REF" --fail-under=0 || true + # NO --depth HERE. Running a shallow fetch against the COMPLETE clone that `fetch-depth: 0` + # just produced writes .git/shallow and grafts away everything behind origin/'s tip. + # diff-cover diffs with the three-dot range `origin/...HEAD`, which needs a merge base -- + # so once the base branch advances past the PR's merge ref (another PR merging, or a re-run of + # a stale check) the diff dies with "no merge base". Reproduced. It then fails in the worst + # possible way: the markdown reporter runs BEFORE the annotations reporter and truncates + # diff-cover.md on open, so `|| true` swallows the error, zero annotations are emitted, and the + # summary shows a bare heading that reads as "nothing uncovered". + git fetch --no-tags origin "$BASE_REF" || true + if [ ! -s coverage.xml ]; then + echo "::notice title=Diff coverage skipped::coverage.xml was not produced (the test step did not complete)" + exit 0 + fi + # `github-annotations:notice` writes `::notice file=...,line=N,endLine=M` workflow commands to + # STDOUT, which the runner turns into INLINE annotations on the Files changed tab -- no token, + # no permission grant, identical behaviour on fork PRs. Adjacent uncovered lines are coalesced + # into ranges, so a long uncovered block costs one annotation rather than one per line. Every + # line flagged is a line this PR changed, so there is no pre-existing-debt noise by construction. + # The console report still prints, so the job log is unchanged. + diff-cover coverage.xml --compare-branch="origin/$BASE_REF" --fail-under=0 \ + --format "github-annotations:notice,markdown:diff-cover.md" || true + # `-s` not `-f`: diff-cover creates and truncates this file BEFORE it can fail, so a crashed + # run leaves a 0-byte report. Testing for existence alone would append an empty section that + # looks like a clean result -- the exact silent-success shape this workflow keeps producing. + if [ -s diff-cover.md ]; then + { + echo "## Diff coverage" + echo "" + # The markdown report embeds a source snippet per uncovered line, so it can get large. + # $GITHUB_STEP_SUMMARY is capped at 1 MiB per step and an oversized write is DROPPED + # ENTIRELY -- truncate rather than silently lose the whole summary. + head -c 900000 diff-cover.md + } >> "$GITHUB_STEP_SUMMARY" + else + echo "::notice title=Diff coverage unavailable::diff-cover produced no report (see the job log)" + fi mutation: # Signal 7 - mutation testing (advisory; the HIGHEST-leverage gate -- it adversarially checks the # tests actually assert something, catching the "tests that never fail" AI-slop mode, rubric section 3). - # EXPENSIVE: mutmut runs the scoped tests once per mutant, so this is NOT per-PR. It runs on the - # mirror's nightly cron (free minutes) and on manual workflow_dispatch (so it can run on the private - # repo on demand -- e.g. to verify this job). Scope is bounded to one well-tested pure module to keep - # the run tractable; widen it later. Non-required; `|| true` so a surviving mutant never fails a build. + # + # NOT expensive, contrary to the note that used to sit here. MEASURED on Python 3.14 (linux, the + # bounded scope below): 461 mutants in 3 SECONDS -- 87 killed, 19 survived, 355 not covered by the + # scoped test file. mutmut 3 only runs the tests that actually cover each mutant, so the old + # "runs the suite once per mutant" cost model does not apply. That is why this now runs on PRs too: + # a real signal for seconds of runtime is worth having in review, which is where survivors get fixed. name: mutation (advisory) - if: github.event_name == 'workflow_dispatch' || (github.event_name == 'schedule' && github.repository == 'MEFORORG/MessageFoundry') runs-on: ubuntu-latest timeout-minutes: 30 permissions: @@ -148,15 +291,85 @@ jobs: - name: Install project + mutmut run: | uv pip install --system --constraint constraints.lock -e ".[dev,harness,fhir,dicom,x12,xml,webauthn]" -e packaging/messagefoundry-webconsole - uv pip install --system --constraint constraints.lock "mutmut<3" + # mutmut 3, pinned. `mutmut<3` resolved to 2.5.1, which CRASHES on Python 3.14 in its pony-ORM + # cache (`cannot pickle 'itertools.count'`) before generating a single mutant -- verified from + # run 30248096425. pytest-timeout is REQUIRED: mutmut 3 always passes `--timeout`/ + # `--timeout-method` to pytest, and without the plugin every test invocation dies with an + # unrecognised-argument error that surfaces only as BadTestExecutionCommandsException. + # Neither package is in constraints.lock, so the --constraint above does not pin them. + uv pip install --system "mutmut==3.6.0" pytest-timeout - name: Mutation-test a bounded scope (advisory - never fails) + continue-on-error: true env: QT_QPA_PLATFORM: offscreen run: | echo "== mutation testing messagefoundry/parsing/binary.py (advisory; bounded scope) ==" - # Ephemeral mutmut config (the repo ships none): mutate one pure module, scored by its own - # focused test file so each mutant re-runs seconds of tests, not the whole suite. - printf '[mutmut]\npaths_to_mutate=messagefoundry/parsing/binary.py\nrunner=python -m pytest -x -q tests/test_binary_carriage.py\n' > setup.cfg - mutmut run || true - echo "== results ==" - mutmut results || true + # Ephemeral mutmut config (the repo ships none). THREE things here are load-bearing, each + # found by a run that silently produced nothing: + # * source_paths is the PACKAGE, not the one file. mutmut 3 copies source_paths into + # `mutants/` and runs pytest THERE; with a single file copied, tests/conftest.py died on + # `ModuleNotFoundError: messagefoundry.config` and every mutant came back "not checked". + # * only_mutate keeps the bounded scope -- the whole package is copied, one module mutated. + # * pytest_add_cli_args_test_selection replaces mutmut 2's `runner=`; the old `runner` and + # `paths_to_mutate` keys are gone/deprecated in 3.x. + printf '[mutmut]\nsource_paths=messagefoundry\nonly_mutate=messagefoundry/parsing/binary.py\npytest_add_cli_args_test_selection=tests/test_binary_carriage.py\n' > setup.cfg + if mutmut run > mutmut-run.txt 2>&1; then + MUTMUT_OK=1 + else + MUTMUT_OK=0 + fi + tail -c 20000 mutmut-run.txt + if [ "$MUTMUT_OK" = "1" ]; then + mutmut results > mutmut-results.txt 2>&1 || true + KILLED="$(grep -c ': killed' mutmut-results.txt || true)" + SURVIVED="$(grep -c ': survived' mutmut-results.txt || true)" + NOTESTS="$(grep -c ': no tests' mutmut-results.txt || true)" + echo "== killed=$KILLED survived=$SURVIVED no-tests=$NOTESTS ==" + grep ': survived' mutmut-results.txt || true + { + echo "## Mutation testing — \`messagefoundry/parsing/binary.py\`" + echo "" + echo "| Killed | Survived | Not covered |" + echo "| --- | --- | --- |" + echo "| $KILLED | **$SURVIVED** | $NOTESTS |" + echo "" + if [ "$SURVIVED" != "0" ]; then + echo "Survivors — injected bugs the tests did **not** catch:" + echo "" + echo '```' + grep ': survived' mutmut-results.txt | head -c 100000 + echo '```' + fi + echo "" + echo "Advisory only. A survivor is a hint to strengthen an assertion, not a defect —" + echo "and mutation score is a poor single number (rubric §2), so this never gates." + } >> "$GITHUB_STEP_SUMMARY" + else + echo "::warning title=Mutation run failed::mutmut exited non-zero — see the job log" + { + echo "## Mutation testing — FAILED TO RUN" + echo "" + echo "\`mutmut run\` exited non-zero, so this signal produced nothing. Tail of the run:" + echo "" + echo '```' + tail -c 4000 mutmut-run.txt + echo '```' + } >> "$GITHUB_STEP_SUMMARY" + fi + - name: Upload the mutmut results (advisory) + if: always() + continue-on-error: true + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: mutmut-results + path: | + mutmut-results.txt + mutmut-run.txt + .mutmut-cache + # .mutmut-cache is a DOTFILE and upload-artifact skips hidden files by DEFAULT. Without this + # the step logs "No files were found", uploads nothing, and still reports success -- a silent + # no-op that looks green. + include-hidden-files: true + # Deliberately `warn`, not `ignore`: if mutmut ever renames its cache, that must be visible in + # the log rather than silently swallowed. + if-no-files-found: warn diff --git a/.gitignore b/.gitignore index a7ff2432..25907273 100644 --- a/.gitignore +++ b/.gitignore @@ -59,6 +59,22 @@ secrets/ .vscode/ .idea/ +# CI-only quality artifacts (generated by .github/workflows/quality-advisory.yml, and by anyone +# reproducing those jobs locally). None of these were ignored before, which mattered: the C901 delta +# inputs are ruff JSON carrying ABSOLUTE local paths, so a local `git add -A` would stage a home +# directory path straight into a commit -- exactly what scripts/security/scan_forbidden.py fires on. +coverage.xml +.coverage +diff-cover.md +jscpd-report/ +.mutmut-cache +mutants/ +mutmut-run.txt +mutmut-results.txt +c901-head.json +c901-base.json +base-tree/ + # Claude Code: settings.json is shared/tracked; settings.local.json is machine-local (never commit) .claude/settings.local.json diff --git a/docs/CI.md b/docs/CI.md index 29cffe2e..fe5b1375 100644 --- a/docs/CI.md +++ b/docs/CI.md @@ -15,6 +15,7 @@ which checks are *required* — this page describes the intended layout. | `scorecard.yml` | OpenSSF Scorecard analysis. | | `cla.yml` | CLA Assistant — records the Contributor License Agreement signature on each PR. | | `zizmor.yml` | Lints the workflow files themselves for insecure patterns (template injection, over-broad tokens). **Blocking.** | +| `quality-advisory.yml` | Advisory quality measurement — complexity (ruff `C901`), duplication (`jscpd`), diff-coverage (`diff-cover`) and mutation testing (`mutmut`). **Every job is advisory and none is in branch protection.** See below for how each signal reaches a reviewer. | Several heavier legs (server-DB store tests, load/throughput, service-smoke, DICOM/FHIR breadth) run **nightly on a schedule** and/or only when a PR touches their paths, so an ordinary PR does not pay for @@ -38,6 +39,32 @@ CodeQL and Scorecard run on PRs but are **advisory** (not in the required set) `security-events: write`, which fork-PR tokens do not have, so requiring them would block PRs from forks. Nightly / path-gated legs (service-smoke, load, SQL/Postgres store) are deliberately **not** required. +The `quality-advisory.yml` jobs create **no code-scanning category** and **no _required_ check context** — +they do report as ordinary advisory checks, and they **must never be added to the required list**. Two +things keep them advisory: they are absent from branch protection, and every analysis step is +`continue-on-error: true` plus `--exit-zero` / `--fail-under=0` / `|| true`, so the job reports success +whatever it finds. (The workflow also holds **no write permission on any job** — that is least privilege, +worth having because two of these jobs run third-party code fetched at run time, but it is *not* what +determines merge gating; required-checks membership is.) +`tests/test_quality_advisory_invariants.py` fails if a write scope, a SARIF upload, or a removed +`--exit-zero` ever lands there. + +### How the advisory quality signals reach a reviewer + +These use GitHub **workflow-command annotations** (`::notice` / `::warning` on stdout) rather than code +scanning. That needs no token and no permission grant, and behaves identically on fork PRs. The +reasoning — including why SARIF was measured and rejected — is recorded in the workflow's header comment. + +An annotation renders **inline on Files changed only when its line is in the diff**. That is always true +for diff-coverage and usually *not* true for complexity, so the two land in different places: + +| Signal | Where it shows up | +|---|---| +| Diff-coverage | **Inline on the Files changed tab**, one `::notice` per contiguous uncovered range of lines the PR changed, plus a step summary. Every line it flags is a line the PR touched, so this is the one signal that is reliably inline. | +| Complexity (`C901`) | A **merge-base-vs-HEAD delta** — only functions this PR introduced over the threshold or made more complex. Findings anchor on the `def` line, which a body-only edit does not touch, so **most complexity annotations appear in the Checks tab and the step summary rather than inline**. The summary table is this signal's primary surface. Pre-existing findings are never reported; the full list stays in the job log. | +| Duplication (`jscpd`) | Step summary only. jscpd emits one location per clone pair chosen by scan order, so annotating it would anchor on the untouched twin about half the time. | +| Mutation (`mutmut`) | A **killed / survived / not-covered** table in the step summary, with the surviving mutants listed — those are injected bugs the tests did not catch. Runs on PRs too: measured at **461 mutants in 3 seconds** (87 killed, 19 survived) over the bounded scope, because mutmut 3 only runs the tests that cover each mutant. Repaired 2026-07-27 — `mutmut<3` resolved to 2.5.1, which crashes on Python 3.14 before generating a single mutant and, thanks to `\|\| true`, had been reporting success in 37s while measuring nothing. | + ### The `CI gate` roll-up `CI gate` `needs:` the individual legs, runs with `if: always()`, and fails **only** on a `failure` or diff --git a/docs/Code_Quality_Standards.md b/docs/Code_Quality_Standards.md new file mode 100644 index 00000000..d7df11b5 --- /dev/null +++ b/docs/Code_Quality_Standards.md @@ -0,0 +1,378 @@ +# Code Quality & Anti-Slop Standards — Executive Summary + +**Is this code good, or is it slop? · MEFOR verdict: A− · July 14, 2026** + +How to judge whether code — human- or AI-authored — is actually good, using signals the evidence supports. + +| At a glance | | +|----|----| +| **MEFOR verdict** | **A− / low slop-risk** — all 11 signals Built | +| **Core thesis** | No single number certifies quality — judge enforced structure and verified behavior | +| **Hard rule** | Never gate on coverage %, LOC, complexity, or SonarQube counts alone — all weak or gameable | +| **Proven value** | Caught private security docs shipping in the public PyPI package (fixed, verified at v0.3.0) | +| **Evidence base** | Peer-reviewed and adversarially verified — 20 claims confirmed, 5 popular claims refuted | + +### AI slop is real — and measurable + +- AI-assisted developers wrote *less* secure code while *more* confident it was secure (Stanford). +- ~14% of distinct Python modules in real ChatGPT output didn't exist ("slopsquatting"). +- 2024: copied lines exceeded refactored lines for the first time in industry telemetry. + +Each failure mode maps to a specific machine-enforced control — not a vibe check. + +### What actually predicts quality + +Machine-enforced structure (layer boundaries, strict typing), tests whose assertions are validated by mutation testing, and dependency + published-artifact integrity. The popular scoreboards — coverage %, complexity scores, SonarQube severities — correlate weakly, sometimes *invertedly*, with real defects. + +### The MEFOR result + +All 11 signals Built and running in CI: enforced boundaries, strict typing, 5,400+ behavior-verifying tests, locked dependencies, 11 security scanners. The one durable gap ever found — the PyPI leak — was caught by this rubric and closed. + +# The 11 Signals at a Glance + +Simplified from the full rubric (§4) and MEFOR scorecard (Appendix A), which follow. A codebase is judged by the **composite** — never by any single row. + +**Tier 1 — durable, high-signal controls (these carry the verdict)** + +| \# | Signal | What it asks | MEFOR | +|----|----|----|----| +| 1 | Architecture boundaries | Are module/layer rules machine-checked in CI, not just documented? | ✅ Strong | +| 2 | Strict typing | Full strict type-checking, no blanket suppressions? | ✅ Strong | +| 3 | Tests verify behavior | Do tests assert real values and failure paths — not mock choreography? | ✅ Strong | +| 4 | Dependency integrity | Is every dependency verified to exist and hash-locked? | ✅ Strong | +| 5 | Security scanning + threat model | Blocking scanners plus a written threat model and review? | ✅ Strong | +| 6 | Published-artifact integrity | Do released packages ship *only* intended content? | ✅ Built — caught and fixed a real leak | + +**Tier 2 — measurement layer (guidance and triage — never a gate on its own)** + +| \# | Signal | What it asks | MEFOR | +|----|----|----|----| +| 7 | Mutation testing | Do the tests actually catch injected bugs? | ✅ Built (advisory) | +| 8 | Coverage visibility | Is coverage reported on changed lines, as guidance? | ✅ Built (advisory) | +| 9 | Duplication / reuse | Is new copy-paste flagged for review? | ✅ Built (advisory) | +| 10 | Lint breadth | Is a broad static-analysis ruleset enforced? | ✅ Built — enforced in the required CI leg | +| 11 | Complexity triage | Are the genuinely large units surfaced for review? | ✅ Built (advisory) | + +> **The anti-metric rule:** none of the Tier 2 numbers may ever be *the* quality gate. They surface problems for a human to judge; the Tier 1 structure carries the verdict. + +The full standard follows: the evidence review, the AI failure-mode map, the complete rubric, gate placement, the MEFOR scorecard, and the cited methodology. + +# Code Quality & Anti-Slop Standards — Evidence-Based Rubric for Judging Code (Human- or AI-Authored) + +*A companion standard to [Secure Development Standards](Secure_Development_Standards.md) (SDS) and [Secure AI-Assisted Development Standards](Secure_AI_Development_Standards.md) (the AI-build companion). The SDS says **what a secure build must satisfy**; the AI-build companion says **how to build it with an AI assistant** (process, tiers, provenance). This document is the third leg: **how to judge whether the resulting code is actually good — not "AI slop" — using signals the evidence supports, not scoreboards it refutes.** It governs the **outcome**, where the SDS governs the process and the AI companion governs the tooling.* + +> **Scope boundary — read this first.** This is a **code-quality measurement rubric**, not a security standard (that is the [SDS](Secure_Development_Standards.md)) and not the AI-build process standard (that is the [Secure AI-Assisted Development Standards](Secure_AI_Development_Standards.md)). Where a signal here is *already owned* by a companion — dependency-existence verification, human review, SAST — this doc **points to it and does not restate it.** Its own additions are the *quality-measurement* gates neither companion carries: test-signal proof (mutation), duplication/reuse detection, coverage *visibility*, complexity *triage*, lint breadth, and **published-artifact / supply-chain-*out* integrity** (signal 6). + +| | | +|----|----| +| **Document** | Code Quality & Anti-Slop Standards | +| **Applies to** | Any project developed under the [SDS](Secure_Development_Standards.md). **MessageFoundry (MEFOR)** is the reference implementation (Appendix A); future projects add Appendix B, C, … | +| **Maintained by** | Project maintainers (open-source). Each deploying/adopting organization assigns its own local owner. | +| **Status** | Draft for review | +| **Version** | 0.10 | +| **Date** | July 27, 2026 | +| **License** | Publishable under the project's open-source license; intended to be shared with adopters and reused across projects. | +| **Review cadence** | At least annually, and on any material change to the metric evidence base or the AI toolchain. | +| **Aligns to** | **ISO/IEC 25010:2023** (product-quality model — Maintainability = modularity / reusability / analyzability / modifiability / testability) · companion to [SDS](Secure_Development_Standards.md) **PW.7 / PW.8** and the [Secure AI-Assisted Development Standards](Secure_AI_Development_Standards.md) §3 failure-modes / §9 deferred-gates. Evidence base is **peer-reviewed metric-validity studies + DORA 2024 + GitClear + the METR RCT + Stanford CCS'23**, each carried with its honesty caveat (§7). **Confers no certification.** | + +------------------------------------------------------------------------ + +## Executive summary + +**The core thesis, and it is counterintuitive:** *there is no single number that certifies code quality, and every metric people reach for first is a weak or gameable predictor.* Peer-reviewed evidence shows line-coverage %, raw cyclomatic complexity, SonarSource "Cognitive Complexity", and SonarQube issue severities all correlate **weakly — sometimes invertedly** — with real defects and change-proneness (§2). What survives is **structure and behavior, verified by enforced controls**: ISO/IEC 25010 maintainability (low coupling / information hiding), strict typing, tests whose *assertions* are validated (mutation testing as *guidance*, not a gate), dependency integrity, and static analysis in the loop. + +**"AI slop" is real but specific.** The evidence names measurable failure modes — a controlled Stanford study found AI-assisted developers wrote *less* secure code yet were *more* confident it was secure; GitClear telemetry shows 2024 was the first year copy/pasted lines exceeded refactored ("moved") lines (the signature of *copy-instead-of-abstract*); ~14.4% of *distinct* Python modules in real ChatGPT output were hallucinated (slopsquatting); DORA 2024 associated AI adoption with a delivery-**stability** drop. The answer to each is a **specific control**, not a vibe (§3). + +**This document is a measurement rubric, not another process gate.** It gives (1) the signals that genuinely separate good code from slop, with the honest note that **no validated single-metric cutoff exists** — thresholds are set empirically per project; (2) an anti-metric list of what **not** to gate on; and (3) a scored MEFOR scorecard (Appendix A). **MEFOR's verdict: strong exactly where the evidence says it counts (machine-enforced structure, strict typing, behavior-verifying tests, dependency integrity, security scanning), and — as of this cycle — the *measurement* layer has closed too: mutation, coverage visibility, clone detection, complexity triage, and the broadened lint ruleset all ship as CI gates (#1028/#1040/#1047), so every one of the 11 signals is now Built. The one *durable*-control gap this cycle surfaced — a private-doc leak in the published PyPI sdist — was caught via signal 6 and fixed (#1020), verified clean at v0.3.0.** + +------------------------------------------------------------------------ + +## 1. Purpose, scope, and the lens + +This rubric answers one question: **"Is this code good, or is it slop?"** — for code that may be human- or AI-authored, in a repository built largely with an AI assistant across many parallel sessions. + +It serves three audiences: **maintainers** (a standing scorecard to re-run each release), **adopters and auditors** (evidence the code is judged against the evidence, not a badge), and **future projects** (their own Appendix). + +**The lens — structure over scoreboards.** The evidence is unambiguous that *single-number gates fail*. Therefore this rubric is **composite and structural**: it weights machine-enforced architectural boundaries and validated test signal far above any coverage or complexity number, and it explicitly **forbids** certifying quality on a single metric (§4). This is the quality-measurement analogue of the SDS's "deterministic checks, never ask the model to be secure" principle — *measure structure and behavior, never trust a scoreboard.* + +------------------------------------------------------------------------ + +## 2. The evidence — validated vs. gameable (read before setting any gate) + +Peer-reviewed, [adversarially-verified](#b.2-how-the-matrix-was-derived) findings on what actually predicts quality (full citations → [Appendix B.4](#b.4-references)): + +| Popular metric | Verdict | Evidence | +|----|----|----| +| **Cyclomatic complexity** (raw) | **Weak predictor; largely a proxy for LOC.** Use as a local *triage smell*, never a gate. | Correlation with real bugs ≈ 0.06 (Kendall); "adds little if any" beyond executable-line counts. [\[R2\]](#r2) | +| **SonarSource "Cognitive Complexity"** | **No incremental predictive value** over traditional measures. | Peer-reviewed JSS evaluation: "does not appear to fulfill the promise." [\[R3\]](#r3) | +| **SonarQube quality-gate severities** | **Weak, inconsistent, sometimes *inverted*.** Flagged "dirty" classes are no more fault-prone than clean ones. Useful as a cheap filter, **not** a quality score. | 33 Apache projects, ~27K faults. [\[R4\]](#r4) | +| **Mutation score** (as a single number) | **Poor *linear* proxy** (mostly a test-suite-size artifact) — **but** mutation testing is high-value as *guidance*: top-decile suites catch 8–46% more real faults. | ICSE 2018 (Papadakis et al.). [\[R5\]](#r5) | +| **Line-coverage % / LOC** | **Gameable.** High coverage with weak assertions is the canonical AI-slop hiding place; LOC measures size, not quality. | Corollary of the mutation and complexity findings above. | + +**What survives as durable signal:** **ISO/IEC 25010:2023** [\[R1\]](#r1) maintainability — decomposed into *modularity, reusability, analyzability, modifiability, testability*, i.e. **low coupling + information hiding** — enforced as **architectural fitness functions** (import/layer boundaries), plus **strict typing**, **behavior-verifying tests validated by mutation testing**, **dependency integrity**, and **static analysis in the loop**. + +> **Honesty note on the hype.** Several widely-quoted "AI slop" statistics **failed adversarial verification** during this document's research: the "~40% of Copilot code is vulnerable" figure, a "10× duplicate-block surge", and two GitClear copy/move percentages were all refuted — full list in [Appendix B.5](#b.5-what-was-refuted-the-verification-worked). **Judge on the structural signals below, not on alarm-bell numbers.** (Full caveats: §7.) + +------------------------------------------------------------------------ + +## 3. AI-specific slop failure modes → the control that neutralizes each + +The [Secure AI-Assisted Development Standards §3](Secure_AI_Development_Standards.md#3-the-problem-this-standard-attacks) names *five process failure modes* (intent drift, context rot, error accumulation, the speed–quality paradox, misplaced trust). This rubric maps the **code-outcome** failure modes those produce to a measurable control: + +| AI slop failure mode | Evidence | The control (owner) | +|----|----|----| +| **Insecure code + overconfidence** | Stanford CCS'23: AI-assisted users wrote less-secure code, *more* confident it was secure. [\[R6\]](#r6) | Mandatory human review + blocking SAST that cannot be waived — **owned by [SDS PW.7](Secure_Development_Standards.md) + [AI companion §6.5/§6.6](Secure_AI_Development_Standards.md)**. This rubric only *checks it is present*. | +| **Hallucinated / typosquatted dependencies** ("slopsquatting") | ~14.4% of *distinct* Python modules in real ChatGPT output did not exist. [\[R7\]](#r7) | Verify-before-add + hash-locked lockfile + new-import audit — **owned by [AI companion §6.4/§9](Secure_AI_Development_Standards.md)**. This rubric *checks it is present*. | +| **Silent duplication over reuse** (copy-instead-of-abstract) | GitClear: 2024 was the first year copy/pasted lines (12.3%) exceeded "moved"/refactored lines (9.5%). [\[R9\]](#r9) *(Correlational — §7.)* | **Clone-detection on the diff** + a "moved vs copied" review lens. **New gate — this document (§5).** | +| **Shallow tests that assert little** | Coverage % hides assertion-free tests; mutation testing exposes them. [\[R5\]](#r5) | **Mutation testing on changed code, as guidance.** **New gate — this document (§5).** | +| **Unbounded complexity / over-abstraction** | Large, tangled units are a maintainability smell (though a weak *defect* predictor — §2). | **Advisory complexity triage** (surface, don't gate). **New gate — this document (§5).** | +| **Inconsistent conventions across a codebase** | Multi-session AI authorship drifts style/structure. | Broad lint ruleset + `CLAUDE.md` as the standing convention anchor. **Partly new (lint breadth) — §5.** | +| **Velocity ≠ delivered quality** | DORA 2024: AI adoption associated with ~7.2% lower delivery *stability* per 25% adoption. [\[R8\]](#r8) · [\[R10\]](#r10) *(Correlational, partly revised in 2025 — §7.)* | Small batch sizes + DORA change-fail/MTTR awareness. **Owned by [AI companion §3](Secure_AI_Development_Standards.md).** | +| **Control-parity gap** (a guard on one path, missed on its sibling) | AI companion §6.6: an AI implements a control *where prompted* and misses its siblings — every confirmed medium-or-higher finding in the 2026-06 MEFOR audit had this shape. Real instance: the fail-closed leak gate guarded the *git-mirror* publish path but not its sibling, the *PyPI* publish path, so the sdist shipped private docs on every release. | **Enumerate sibling paths for every control; encode as one deterministic check where feasible.** Here → the published-artifact-integrity gate (**signal 6**). | + +------------------------------------------------------------------------ + +## 4. The rubric — 11 signals that separate good code from slop + +Each signal is a **risk → control → measure**, tagged by **gate type** (deterministic = machine-checked; advisory = human arbitrates) and by which document **owns** it. The signals fall into **two tiers** — weight them accordingly. **A codebase is certified "not slop" by the *composite*, never by any single row** (§2). Signals are numbered **contiguously by tier: Tier 1 = 1–6, Tier 2 = 7–11.** + +**Tier 1 — Durable, high-signal controls (weight these most).** Structural, machine-enforceable properties where quality is *hard to fake* — the ones the evidence says actually predict maintainability and catch slop. These carry the verdict. + +| \# | Signal | What "good" looks like | Gate type | Owner | +|----|----|----|----|----| +| 1 | **Enforced architecture boundaries** (ISO 25010 modularity / low coupling) | Import/layer rules are *machine-checked in CI*, not just documented | Deterministic | SDS PW.1–2; **checked here** | +| 2 | **Strict typing** | `mypy --strict`; suppressions carry error codes (no blanket ignores) | Deterministic | AI companion §6.5; **checked here** | +| 3 | **Tests verify behavior, not mocks** | Value/negative-path assertions; real integrations over mock choreography | Deterministic | SDS PW.8; **checked here** | +| 4 | **Dependency integrity** (anti-slopsquatting) | Existence-verify + hash-locked lockfile + new-import audit | Deterministic | **AI companion §6.4/§9** (pointer only) | +| 5 | **Security scanning + threat model** | Blocking SAST/SCA/secret-scan + written threat model + human review | Deterministic + advisory | **SDS PW.7 / AI companion §6.5–6.6** (pointer only) | +| 6 | **Published-artifact integrity** (supply-chain-*out*) | Published sdist/wheel ship **only intended content** — the package manifest is an *allowlist* (not a whole-repo sweep) + a fail-closed publish gate blocks any private/unintended file before the irreversible upload. Distinct from row 4's *incoming* dependency integrity | Deterministic | **This document — new** (+ SDS supply-chain) | + +**Tier 2 — Measurement / lower-signal layer (guidance & triage — never a gate on their own).** Useful for *surfacing* problems, but each is a weak or gameable predictor in isolation (§2, §4.1), so they inform review — they do not certify quality. + +| \# | Signal | What "good" looks like | Gate type | Owner | +|----|----|----|----|----| +| 7 | **Test-signal proof** (mutation) | Mutation testing on changed code exposes shallow tests | Advisory (guidance) | **This document — new** | +| 8 | **Coverage visibility** | Coverage *measured on the diff* as guidance — never a whole-repo % gate | Advisory | **This document — new** | +| 9 | **Duplication / reuse discipline** | Clone-detection flags *new* duplication in diffs; justified parity whitelisted | Advisory | **This document — new** | +| 10 | **Lint breadth** (static analysis in the loop) | Broad ruleset (bugbear, comprehensions, simplify, pyupgrade, isort) | Deterministic | **This document — expand existing** | +| 11 | **Complexity as triage** (never a gate) | Advisory complexity signal surfaces the genuinely-large units | Advisory | **This document — new** | + +> **Context caveat, not a signal — delivery stability (DORA).** "Velocity ≠ delivered quality" is a real AI-slop concern (DORA 2024 associated AI adoption with lower delivery *stability*), but it measures **delivery outcomes**, not whether a given diff is slop — a different altitude from the code-artifact signals above, on weaker (correlational, partly-revised — §7) evidence, and **owned by the [AI companion §3](Secure_AI_Development_Standards.md)**. MEFOR's small-batch discipline (one coherent layer per commit) already covers the actionable part. It stays a §3 failure-mode entry, **not** a peer signal here. +> +> **Evidence & citations for the matrix.** Every signal and claim above maps to its supporting study in [**Appendix B.3**](#b.3-evidence-behind-each-rubric-element) (per-element evidence table), with full bibliographic citations in [**Appendix B.4**](#b.4-references), the derivation method in [**Appendix B.2**](#b.2-how-the-matrix-was-derived), and the claims that *failed* verification in [**Appendix B.5**](#b.5-what-was-refuted-the-verification-worked). + +### 4.1 The anti-metric rule (hard) + +**Do NOT certify quality — or fail a build — on any single one of:** line-coverage %, LOC, raw or cognitive cyclomatic complexity, or SonarQube severity counts. Each is a weak or gameable predictor (§2). They may be *surfaced as advisory triage signals*; they must never be *the* quality gate. This mirrors the AI companion's "gates are deterministic checks, never ask the model to be secure" — here: *a scoreboard is never the verdict.* + +### 4.2 No validated single threshold (honest) + +The literature validates that single metrics fail, but supplies **no validated numeric cutoff** for a combined scorecard. Thresholds (e.g. a mutation-score floor on changed code, a max new-clone count) are therefore **set empirically per project and recorded in the Appendix**, reviewed as data accumulates — not imported as universal constants. Where this document names a directional target, it is flagged as project-set, not evidence-certified. + +------------------------------------------------------------------------ + +## 5. The new measurement gates — placement (local vs CI) and status + +The five gates this document adds (rubric rows 7–11) are *quality-measurement* gates. Placement follows the project's **three enforcement points** (identical to the AI companion §6.5): **pre-commit hook** (local), `messagefoundry check` (local + CI + IDE), and **GitHub Actions CI** (the authoritative gate, incl. self-hosted Windows runners). + +| Gate | Cost | Recommended placement | Blocking? | Taxonomy | +|----|----|----|----|----| +| **Advisory complexity triage** (`C901` via ruff) | Cheap | CI advisory job | **Advisory only** (never a hard gate — §4.1) | ✅ **Built** — `quality-advisory.yml` (#1028) | +| **Clone-detection** (jscpd) | Cheap | CI advisory job; store-parity whitelisted | Advisory (finding, not fail) | ✅ **Built** — `quality-advisory.yml` (#1028) | +| **Diff-coverage visibility** (`diff-cover`) | Moderate | CI leg (reports on changed lines) | Advisory | ✅ **Built** — `quality-advisory.yml` (#1040; PR-only) | +| **Mutation testing** (`mutmut`, bounded scope) | **Cheap — measured** (461 mutants in 3s; mutmut 3 runs only the tests covering each mutant) | CI leg (advisory), **including PRs** + opt-in local command | Advisory (guidance) | ✅ **Built** — `quality-advisory.yml` (#1040; repaired 2026-07-27, see v0.10) | +| **Expand ruff ruleset** (`B, C4, SIM, UP, I`) | Cheap | The required CI `ruff check` leg | Blocking (from a clean baseline) | ✅ **Built** — `pyproject.toml` extend-select (#1047) | + +**Answering "are these local?"** — Cheap gates (ruff breadth, `C901`) run **both** locally (pre-commit) and in CI. The expensive gates (mutation, clone, diff-coverage) are **CI-first** — they are too slow for the inner loop — but each is **exposable as an opt-in local command** so a maintainer can run it on demand. The *authoritative* enforcement is CI in every case; local runs are for fast feedback. This split is deliberate: it keeps the local edit-loop fast while the diff-scoped heavy analysis runs where wall-clock doesn't block the human. + +> **All five measurement gates are now Built.** Complexity (11) and clone (9) shipped first (#1028), then diff-coverage (8) and mutation (7) (#1040) — non-required advisory jobs in [`quality-advisory.yml`](../.github/workflows/quality-advisory.yml) that can never block a merge — and finally the **ruff-breadth expansion (signal 10, \#1047)**, which unlike the others *is* enforced by the required `ruff check` leg (from a grandfathered clean baseline; new code must comply). Nothing remains designed-but-deferred. + +------------------------------------------------------------------------ + +## 6. How this maps to the companion standards + +| This rubric | Companion anchor | +|----|----| +| Rows 1–2 (structure, typing) | SDS **PW.1–PW.2** (secure design, modularity); AI companion **§6.5** (verification gates) | +| Row 3 (behavior-verifying tests) | SDS **PW.8** (test executable code) — *this rubric adds the quality bar on top of presence* | +| Row 4 (dependency integrity) | AI companion **§6.4 / §9** — *owned there; checked here* | +| Row 5 (security review + SAST) | SDS **PW.7**; AI companion **§6.5–6.6** — *owned there; checked here* | +| Row 6 (published-artifact integrity) | **New** — this document (+ SDS supply-chain) | +| Rows 7–11 (measurement gates) | **New** — 7/8/9/11 Built (advisory) in `quality-advisory.yml`; 10 Built + enforced by the required `ruff check` leg (#1047) | +| Delivery stability (DORA) — *context caveat, not a signal* (§4) | AI companion **§3** (the METR / DORA calibration) | +| The anti-metric rule (§4.1) | The quality-side analogue of AI companion **§5** "gates are deterministic checks, never ask the model to be secure" | + +------------------------------------------------------------------------ + +## 7. Evidence caveats (carry these into every use) + +1. **Metric-invalidation findings are robust** (peer-reviewed primary studies) but are *"this metric is weak alone"* results — they argue against **single-number gates**, not against measurement. +2. **The GitClear duplication/churn trend is descriptively solid but the AI *causation* is correlational**, from a commercial vendor using a proprietary "moved" reuse heuristic, and confounded (2022–24 layoffs, a startup-growth effect). Treat the duplication *trend* as real; treat the AI attribution as interpretation. +3. **Several AI-code findings are model-era-specific.** The Stanford security study used codex-davinci-002 (2022); the package-hallucination figures skew to GPT-3.5-era output. **Frontier models plausibly perform better — re-baseline periodically.** +4. **The 14.4% Python hallucination figure is share of *distinct* modules**, not per-import rate (which is far lower — common modules are never hallucinated). +5. **DORA 2024's negative delivery effect is associational** and was partly revised in DORA 2025. +6. **No source provides a validated single-metric certification threshold** — the evidence supports composite, structural, guidance-based assessment (§4.2). + +------------------------------------------------------------------------ + +## 8. References + +*Quick links below. **Full bibliographic citations, the per-element evidence map, and the derivation method are in [Appendix B](#appendix-b-the-rubric-matrix-methodology-cited-references)** (B.4 / B.3 / B.2).* + +- **ISO/IEC 25010:2023** — Systems and software Quality Requirements and Evaluation (product-quality model). +- **Cyclomatic complexity vs. defects** — [arXiv 1912.01142](https://arxiv.org/pdf/1912.01142). +- **Cognitive Complexity has no incremental predictive value** — [JSS 2022](https://www.sciencedirect.com/science/article/abs/pii/S0164121222002370). +- **SonarQube technical-debt items weakly/invertedly related to faults** — [arXiv 1908.11590](https://arxiv.org/pdf/1908.11590). +- **Mutation score is a size artifact but valuable as guidance** — [ICSE 2018, Papadakis et al.](https://dl.acm.org/doi/pdf/10.1145/3180155.3180183). +- **AI assistants → less-secure, more-confident code** — [Stanford, CCS 2023, arXiv 2211.03622](https://arxiv.org/abs/2211.03622). +- **Package hallucination / slopsquatting (~14.4% distinct Python modules)** — [WildCode, arXiv 2512.04259](https://arxiv.org/pdf/2512.04259). +- **DORA 2024** — AI adoption vs. delivery stability/throughput — [dora.dev/research/2024](https://dora.dev/research/2024/dora-report/). +- **GitClear** — copy/paste vs. moved lines, churn — [GitClear AI Copilot Code Quality 2025](https://gitclear-public.s3.us-west-2.amazonaws.com/GitClear-AI-Copilot-Code-Quality-2025.pdf). +- **METR RCT (experienced devs 19% slower)** — cited via the [AI companion §3/§11](Secure_AI_Development_Standards.md). +- **Cross-links:** [Secure Development Standards](Secure_Development_Standards.md) · [Secure AI-Assisted Development Standards](Secure_AI_Development_Standards.md) · [`../CLAUDE.md`](../CLAUDE.md). + +------------------------------------------------------------------------ + +## Appendix A — Scorecard: MessageFoundry (MEFOR) + +*Scored from a read-only repo audit (2026-07-13). Future projects add Appendix B, C, … with identical headings. Each signal is tagged **Built / designed-but-deferred / aspirational**, per the house honesty taxonomy.* + +### A.1 Verdict + +**A− / low slop-risk.** MEFOR implements **all six durable, high-signal controls** (rubric rows 1–6) as **Built**, and its *measurement* layer has now closed as well: **complexity (11) and clone (9)** shipped as advisory gates (#1028), then **mutation (7) and diff-coverage (8)** (#1040), and finally the **ruff-breadth expansion (#10)** (#1047, enforced by the required `ruff check` leg). So **all 11 signals are now Built**. It is strong where faking is hardest (machine-enforced structure) and thin only where the metrics are gameable anyway. + +**The rubric earned its keep this cycle.** Applying **signal 6** (published-artifact integrity) surfaced a real **control-parity** gap (§3): the PyPI **sdist** was shipping the private security-posture docs on *every* release, because the fail-closed leak gate covered the git-mirror publish path but not its sibling, the PyPI path. It was fixed (#1020: a `[tool.hatch.build.targets.sdist]` allowlist + a fail-closed "sdist is package-only" gate in `release.yml`) and **verified clean at v0.3.0**. That found-and-fixed leak is the one *durable*-control gap that has now closed; the rest of the gaps are all in the measurement layer. + +### A.2 Scored signals (from the audit) + +**Tier 1 — durable, high-signal controls (signals 1–6): all Built ✅.** + +| \# | Signal | Status | Evidence in repo | +|----|----|----|----| +| 1 | Enforced architecture boundaries | ✅ **Built — Strong** | `tests/test_dependency_boundaries.py` AST-scans engine packages, blocks `fastapi`/`pyside6`/`api`/`console` imports, in the required CI `test` leg | +| 2 | Strict typing | ✅ **Built — Strong** | `[tool.mypy] strict = true`, dual-platform CI; all 33 `# type: ignore` + 100 `# noqa` carry rule codes; no blanket ignores | +| 3 | Tests verify behavior, not mocks | ✅ **Built — Strong** | 5,402 test functions; ~7,600 value-`==` asserts; ~1,000 `pytest.raises`; **0** `assert_called*`; live SQL Server + Postgres integration legs | +| 4 | Dependency integrity | ✅ **Built — Strong** | Hash-locked `requirements.lock` (DEP-1 lock-sync + `--require-hashes` CI); pip-audit; `CLAUDE.md`/AI-companion verify-before-add rule | +| 5 | Security scanning + threat model | ✅ **Built — Strong** *(caveat A.4)* | 11 scanners (CodeQL, semgrep, bandit, gitleaks, pip-audit, crypto-inventory, forbidden-content, Trivy, Scorecard, zizmor, npm-audit); SECURITY.md (735 ln) + PHI.md (688 ln) | +| 6 | Published-artifact integrity (supply-chain-*out*) | ✅ **Built — found & fixed this cycle** | Was 🔴: the PyPI **sdist** swept the whole repo, shipping `docs/security/*`, `CLAUDE.md`, `scripts/publish/*` on releases 0.1.0..0.2.15 (the mirror leak-gate never covered the PyPI path — a control-parity miss, §3). **Fixed \#1020:** `[tool.hatch.build.targets.sdist] only-include` + a fail-closed "sdist is package-only" gate in `release.yml`; **v0.3.0 verified package-only against the live PyPI artifact** (sha256 download). Historical 0.1.0..0.2.15 sdists remain public (owner-only PyPI deletion). | + +**Tier 2 — measurement / lower-signal layer (signals 7–11): all 5 Built (#7, \#8, \#9, \#11 advisory; \#10 enforced).** + +| \# | Signal | Status | Evidence in repo | +|----|----|----|----| +| 7 | Test-signal proof (mutation) | ✅ **Built (advisory)** — \#1040, **repaired 2026-07-27 (v0.10)** | `quality-advisory.yml` runs **`mutmut==3.6.0`** over one bounded, well-tested pure module (`parsing/binary.py` ↔ `test_binary_carriage.py`), on PRs + nightly cron + `workflow_dispatch`. **Measured: 461 mutants in 3s — 87 killed, 19 survived, 355 not covered by the scoped test.** Survivor table in the step summary. *Was scored Built on `mutmut<3` from 0.8 to 0.9 while producing nothing — see v0.10.* | +| 8 | Coverage visibility | ✅ **Built (advisory)** — \#1040, **surfaced 2026-07-27 (v0.10)** | `quality-advisory.yml` runs `pytest-cov` + `diff-cover` on the PR's changed lines (`--fail-under=0`), PR-only — coverage *of the diff*, never a whole-repo % gate (§4.1). Now emits **inline `::notice` annotations on the Files changed tab** (`--format github-annotations:notice`), adjacent uncovered lines coalesced into ranges. Advisory. | +| 9 | Duplication / reuse detection | ✅ **Built (advisory)** — \#1028 | `quality-advisory.yml` runs `jscpd` on `messagefoundry/`, whitelisting the ~21k-LOC justified store-backend parity (`sqlserver.py` / `postgres.py`); surfaces *un*justified copy-paste for triage, non-blocking | +| 10 | Lint breadth | ✅ **Built** — \#1047 | `[tool.ruff.lint] extend-select = ["B","C4","SIM","UP","I"]`, enforced by the required `ruff check` leg. B008 (FastAPI DI, ~460 hits) handled via `extend-immutable-calls` + a route-layer per-file ignore (real `x=list()` bugs still caught); **515 violations auto-fixed** (import sort, pyupgrade, safe simplify); **235 non-auto-fixable grandfathered** with per-line `# noqa` → clean baseline, new code must comply | +| 11 | Complexity triage | ✅ **Built (advisory)** — \#1028, **sharpened 2026-07-27 (v0.10)** | `quality-advisory.yml` runs `ruff --select C901 --exit-zero` (advisory, never gates), **plus a merge-base-vs-HEAD delta** (`scripts/quality/c901_delta.py`) that reports only functions a PR *introduced* or *made worse*. **Re-measured 2026-07-27: 122 functions exceed** `C901`**\>10** across 43 files (was 85 on 2026-07-13), complexity 11 / 14 median / 320 max. The raw list is unusable as a diff signal — all 122 findings anchor on a single `def` line — which is what the delta exists to fix | + +### A.3 The gaps, ranked → buildable gates + +Ordered by anti-slop leverage, not effort (build placement per §5). **✅ = shipped** (advisory; \#1028 or \#1040): + +1. **Mutation testing** — highest leverage; directly counters shallow-test slop, extra weight under the solo-maintainer review deviation (A.4). ✅ **shipped** (#1040 — `mutmut` over a bounded, well-tested module, mirror-nightly + `workflow_dispatch`; widen the scope later). +2. **Clone-detection on diffs** — ✅ **shipped** (`jscpd`, store-parity whitelisted) — catches the copy-instead-of-abstract signature the parallel-worktree workflow is most exposed to. +3. **Diff-coverage visibility** — measured on changed lines, guidance only (never a whole-repo % gate — §4.1). ✅ **shipped** (#1040 — `pytest-cov` + `diff-cover`, PR-only). +4. **Advisory** `C901` **complexity** — ✅ **shipped** (advisory triage). +5. **Expand ruff** `select` (`B, C4, SIM, UP, I`) — ✅ **shipped** (#1047 — extend-select enforced by the required `ruff check` leg; 515 auto-fixed, 235 grandfathered from a clean baseline). + +*All gates are now shipped.* The ruff sweep (#10, \#1047) was run in a quiescent-worktree window (a 100+-file import sort would collide with in-flight parallel sessions) after pruning the stale worktrees to a minimal set. Mutation and diff-coverage were built *blind via CI* — verified by their own gate runs, since this repo's sessions can't stand up a local venv (see \#1040). + +**Rollout record (measured 2026-07-13 — how the \#1047 sweep was executed):** `B,C4,SIM,UP,I` = **853 violations** (238 `B008` FastAPI false positives to exclude; 111 `I001` repo-wide import reorder); `C901` = **85 hits**. Safe rollout: (a) exclude framework-idiom rules (`B008` on `api/`); (b) **grandfather** the existing backlog so the *required* gate stays green (per-file-ignores / ratchet — new code only); (c) run the repo-wide import sort as a **dedicated pass when parallel worktrees are quiescent** — a 100+-file sweep conflicts with in-flight sessions; (d) keep `C901` **advisory**. (The built coverage/mutation gates install their tools CI-side via an ephemeral `uv pip install`, so they needed **no** `requirements.lock` change — DEP-1 unaffected.) + +### A.4 Documented caveat — solo-maintainer review + +Row 5's "human review" is **self-review** (the [SDS §A.6](Secure_Development_Standards.md#a6-documented-deviations) / [AI companion Appendix A.6](Secure_AI_Development_Standards.md#a6-documented-deviations) single-maintainer deviation). The Stanford overconfidence finding (§3) bites hardest exactly when the author reviews their own AI-authored code — which is the strongest argument for the mutation gate (Built this cycle — \#1040), since it is the one control that *adversarially* checks whether the tests assert anything, independent of the author's confidence. + +------------------------------------------------------------------------ + +## Appendix B — The rubric matrix: methodology & cited references + +*This appendix explains how to read the §4 signal matrix, how it was derived, and the evidence behind each element — with full citations. The §2, §3, and §4 tables link here.* + +### B.1 What "the matrix" is and how to read it + +The rubric's core is the **§4 matrix**: 11 signals, each a **risk → control → measure**, tagged by **gate type** (deterministic = machine-checked; advisory = human arbitrates) and by which document **owns** it. Read it as a **composite**, not a checklist of independent boxes: + +- **Rows 1–6 are durable, high-signal controls** — enforced structure (ISO/IEC 25010 modularity), strict typing, behavior-verifying tests, dependency integrity, security scanning, and published-artifact integrity. These are where quality is *hard to fake*. +- **Rows 7–11 are the measurement / lower-signal layer** — mutation, coverage, clone-detection, lint breadth, complexity — useful as *guidance / triage*, never as a single gate. +- **The anti-metric rule (§4.1)** forbids certifying quality on any one number (coverage %, LOC, raw or cognitive complexity, SonarQube severity), because the evidence shows each is a weak or gameable predictor (B.3). +- **Delivery stability (DORA) is deliberately *not* a signal** — it measures delivery outcomes, not the code artifact, on weaker evidence; it is kept as a context caveat under the §4 table. + +A codebase is judged "not slop" by the **composite** of the durable controls plus the guidance signals — with thresholds set **empirically per project** (§4.2), because no source validates a universal single-metric cutoff. + +### B.2 How the matrix was derived + +The matrix is **evidence-informed and adversarially verified**, not authored from opinion: + +1. **Adversarially-verified deep-research pass.** The question was decomposed into **five search angles** — (a) academic metric-validity, (b) quality frameworks & delivery metrics, (c) AI-slop empirical trends, (d) security & correctness studies, (e) practitioner controls. Parallel searches fanned out → ~24 sources fetched → ~109 candidate claims → the load-bearing ones put through **3-vote adversarial verification** (each verifier tried to *refute* the claim; ≥2 refutations killed it). Result: **20 confirmed, 5 refuted** (B.5). Only survivors entered the rubric. +2. **Structural scaffold — ISO/IEC 25010:2023** [\[R1\]](#r1): the international product-quality model, whose *maintainability* characteristic (modularity, reusability, analyzability, modifiability, testability) supplies the "structure over scoreboards" backbone (signals 1–6). +3. **The MEFOR scorecard (Appendix A)** is a separate, read-only audit of the actual repository — evidence, not estimates. +4. **Honesty discipline (§7)** — every claim carries its limitation; correlational, vendor-sourced, and model-era-specific evidence is labelled as such. + +### B.3 Evidence behind each rubric element + +| Rubric element | What the evidence establishes | Reference | +|----|----|----| +| **Structure over scoreboards** (signals 1–6) | Product quality is dominated by *maintainability* = modularity / low coupling / information hiding — a structural property, not a metric score | [\[R1\]](#r1) ISO/IEC 25010:2023 | +| **Anti-metric rule (§4.1): raw complexity is weak** (bounds signal 11) | Cyclomatic / path / NPATH complexity correlate only weakly-to-moderately with real bugs; complexity is a *triage smell*, not a gate | [\[R2\]](#r2) Chen 2019 | +| **Anti-metric rule: Cognitive Complexity adds nothing** | SonarSource's Cognitive Complexity gives **no incremental** predictive value over traditional measures | [\[R3\]](#r3) Lavazza et al. 2023 | +| **Anti-metric rule: SonarQube severities are weak / inverted** | Over 33 Apache projects, "dirty" classes are **no more fault-prone** than clean ones; effects small, sometimes inverted | [\[R4\]](#r4) Lenarduzzi et al. 2020 | +| **Signal 7 — mutation as guidance** (why coverage % hides shallow tests) | Mutation *score* is a poor linear proxy (suite-size artifact), but top-decile suites catch **8–46% more real faults** — it exposes assertion-free tests that coverage % hides | [\[R5\]](#r5) Papadakis et al. 2018 | +| **Signal 5 / §3 — insecure code + overconfidence** (mandates human review) | AI-assistant users wrote **less-secure** code yet were **more confident** it was secure | [\[R6\]](#r6) Perry et al. 2023 | +| **Signal 4 — hallucinated dependencies** (slopsquatting) | LLM output frequently references **nonexistent packages** (~14.4% of distinct Python modules) → verify-before-add + hash-locked lockfile | [\[R7\]](#r7) Khanmohammadi et al. 2025 | +| **Signal 9 / §3 — silent duplication over reuse** | 2024 was the first year copy/pasted lines exceeded refactored ("moved") lines — the copy-instead-of-abstract signature *(vendor-sourced, correlational — §7)* | [\[R9\]](#r9) GitClear 2025 | +| **§3 — velocity ≠ delivered quality** (the DORA context caveat) | AI adoption associated with **lower delivery stability** (2024, correlational, partly revised); experienced devs measured **~19% slower** with early-2025 AI tooling | [\[R8\]](#r8) DORA 2024 · [\[R10\]](#r10) METR 2025 | + +### B.4 References + +**\[R1\]** ISO/IEC 25010:2023. *Systems and software engineering — Systems and software Quality Requirements and Evaluation (SQuaRE) — Product quality model.* International Organization for Standardization, 2023. + +**\[R2\]** Chen, C. (2019). *An Empirical Investigation of Correlation between Code Complexity and Bugs.* arXiv:1912.01142 \[cs.SE\]. + +**\[R3\]** Lavazza, L., Abualkishik, A. Z., Liu, G., & Morasca, S. (2023). *An Empirical Evaluation of the "Cognitive Complexity" Measure as a Predictor of Code Understandability.* Journal of Systems and Software, 197, 111561. + +**\[R4\]** Lenarduzzi, V., Saarimäki, N., & Taibi, D. (2020). *Some SonarQube issues have a significant but small effect on faults and changes: A large-scale empirical study.* Journal of Systems and Software, 170, 110750. + +**\[R5\]** Papadakis, M., Shin, D., Yoo, S., & Bae, D.-H. (2018). *Are Mutation Scores Correlated with Real Fault Detection? A Large Scale Empirical Study on the Relationship Between Mutants and Real Faults.* ICSE 2018. + +**\[R6\]** Perry, N., Srivastava, M., Kumar, D., & Boneh, D. (2023). *Do Users Write More Insecure Code with AI Assistants?* ACM CCS 2023. arXiv:2211.03622. + +**\[R7\]** Khanmohammadi, K., Roy, P., Khoury, R., Hamou-Lhadj, A., Konan, W. P., Da Re, A., & Rebelo Melo, N. (2025). *WildCode Revisited: A Comprehensive Empirical Study on the Security of LLM-Generated Code.* arXiv:2512.04259 \[cs.CR\]. — the ~14.4% figure is the share of *distinct* Python modules; per-reference rate is far lower (§7). + +**\[R8\]** Google Cloud / DORA (2024). *Accelerate State of DevOps Report 2024.* — AI adoption vs. delivery stability/throughput; associational, partly revised in 2025. + +**\[R9\]** GitClear (2025). *AI Copilot Code Quality 2025* ("AI-Generated Code Exerts Downward Pressures on Code Quality"). — commercial vendor; proprietary "moved" reuse heuristic; AI attribution correlational (§7). + +**\[R10\]** METR (2025). *Measuring the Impact of Early-2025 AI on Experienced Open-Source Developer Productivity.* arXiv:2507.09089. — 16 experienced devs ~19% slower while believing they were faster. + +### B.5 What was refuted (the verification worked) + +Five widely-circulated claims **failed** 3-vote adversarial verification and are **deliberately not** in the rubric — the filtering is part of the basis: + +- **"~40% of GitHub Copilot programs contain security vulnerabilities."** An over-simplified reading of the NYU "Asleep at the Keyboard" study (Pearce et al., IEEE S&P 2022, arXiv:2108.09293); the headline percentage did not survive scrutiny. *(The study is real; the "40%" framing is not a supported rubric claim.)* +- **A "10× surge in duplicate blocks" (2022→2024)** — GitClear figure, refuted 0–3. +- **"Copy/pasted lines rose 8.3%→12.3% (~48%)"** and **"moved operations fell 17% (2021→2023)"** — GitClear / secondary-blog percentages that did not reconcile, refuted 0–3. + +Because the refuted claims came mostly from the *AI-slop-trend* angle (vendor telemetry, secondary blogs), the rubric leans hardest on the **peer-reviewed metric-validity studies** (\[R2\]–\[R6\]) and treats the trend data (\[R8\], \[R9\], \[R10\]) as caveated context (§7). + +### B.6 Caveats + +The evidence caveats in **§7** are part of this appendix's basis: the metric-invalidation findings are robust peer-reviewed results, but they are "this-metric-is-weak-*alone*" results (they argue against single-number gates, not against measurement); the AI-slop trend data is correlational / vendor-sourced / model-era-specific; and no source supplies a validated single-metric certification threshold. + +------------------------------------------------------------------------ + +## Version history + +| Version | Date | Change | +|----|----|----| +| 0.10 | July 27, 2026 | **Restored to the repo, and corrected three claims that did not survive measurement.** This file had been absent from the repository's entire git history despite being cited by `quality-advisory.yml` and `pyproject.toml`; it is restored here from the maintained copy. Corrections, each measured rather than reasoned: **(a) Signal 7 was scored ✅ Built in 0.8 and 0.9 while producing nothing.** `mutmut<3` resolved to 2.5.1, which crashes on Python 3.14 in its pony-ORM cache (`cannot pickle 'itertools.count'`) *before generating a single mutant*; `\|\| true` made the job report success in 37s, so the gate looked green for two versions. Repaired on `mutmut==3.6.0` (+ `pytest-timeout`, and `source_paths` must be the package, not the one file, or the mutant copy cannot import `conftest`). Now genuinely measured: **461 mutants, 87 killed, 19 survived, 3 seconds** — so the "Expensive / never per-PR" cost model in §5 was also wrong, and mutation now runs on PRs. **(b) Signal 11's "85 functions over C901>10" is now 122 across 43 files**, and the raw list was found unusable as a diff signal (every finding anchors on one `def` line), so a merge-base delta was added that reports only PR-caused changes. **(c) Signal 8 now emits inline PR annotations** rather than console-only output. The A− verdict stands, but note that (a) is exactly the failure mode this rubric exists to catch — an advisory gate that reports success while measuring nothing — and it was caught by re-verification, not by the gate itself. | +| 0.9 | July 14, 2026 | **Restatused signal 10 (lint breadth) to ✅ Built — all 11 signals now Built.** The `extend-select = [B,C4,SIM,UP,I]` sweep shipped (#1047): B008 handled via `extend-immutable-calls` + a route-layer per-file ignore, 515 auto-fixed, 235 grandfathered with `# noqa`, enforced by the required `ruff check` leg. Flipped the exec verdict, §5 gate table + callout, §6 map, and Appendix A.1 / A.2 (row 10 + Tier-2 roll-up) / A.3 (gaps list + "remaining gate" prose → rollout *record*). No scoring change (A− stands). | +| 0.8 | July 14, 2026 | **Restatused signals 7 (mutation) + 8 (diff-coverage) to ✅ Built.** Both shipped as advisory jobs in `quality-advisory.yml` (#1040) — mutation over a bounded module (mirror-nightly + `workflow_dispatch`), diff-coverage on the diff's changed lines (PR-only). Flipped every place that called them deferred: the exec verdict, §5 gate table + callout, §6 map, and Appendix A.1 / A.2 (rows 7–8 + the Tier-2 roll-up) / A.3 (gaps list + DEP-1 note) / A.4. **Only signal 10 (lint breadth) remains designed-but-deferred → 10 of 11 signals now Built.** No scoring change (A− stands). | +| 0.7 | July 13, 2026 | **Tiered the Appendix A scorecard to match §4** — split A.2 into a Tier 1 table (durable, signals 1–6, all Built) and a Tier 2 table (measurement, signals 7–11), each with a per-tier status roll-up. Presentational only; no scoring change. | +| 0.6 | July 13, 2026 | **Renumbered signals contiguously by tier** (per owner): Tier 1 durable = **1–6** (published-artifact 11→6), Tier 2 measurement = **7–11** (mutation 6→7, coverage 7→8, clone 8→9, lint 9→10, complexity 10→11). Updated every cross-reference — §3, §4, §5, §6, exec summary, scope, and Appendix A.1 / A.2 (scorecard reordered) / A.3 / B.1 / B.2 / B.3. The `[R1]–[R10]` reference IDs are unchanged (they're citations, not signals). | +| 0.5 | July 13, 2026 | **Split the §4 matrix into two tiers** for clarity: **Tier 1 — durable, high-signal controls** (signals 1–5 + 11) and **Tier 2 — measurement / lower-signal layer** (signals 6–10). Signal numbers kept **stable** (they are IDs referenced across Appendix A + B), so grouped by tier rather than renumbered. No change to content, evidence, or the scorecard. | +| 0.4 | July 13, 2026 | **Added Appendix B — the rubric matrix's methodology & cited references** (B.1 how to read the matrix, B.2 derivation, B.3 per-element evidence map, B.4 full citations \[R1–R10\], B.5 the 5 refuted claims). **Annotated the rubric with links to it:** §2 + §3 evidence cells now carry `[Rn]` citation links, §4 has an evidence/citations pointer, and §8 points to the full bibliography. Fetched-verified author/title details for the academic references. | +| 0.3 | July 13, 2026 | **Built two gates + demoted DORA.** Complexity (signal 10) and clone (signal 8) shipped as advisory jobs in `quality-advisory.yml` (#1028) — scorecard restatused to ✅ Built (§5, A.2, A.3). **Demoted delivery-stability (DORA) from a peer signal to a context caveat** (it measures delivery outcomes, not the code artifact; weak/correlational evidence; owned by the AI companion) → back to **11 signals** (published-artifact renumbered 12→11). Mutation (#6) + diff-coverage (#7) deferred to a local-venv session; ruff-breadth (#9) to a quiescent-worktree sweep. | +| 0.2 | July 13, 2026 | Added **signal 12 — published-artifact integrity** (supply-chain-*out*) and the **control-parity** failure mode (§3), after the rubric's own application surfaced a private-doc leak in the published PyPI sdist (fixed \#1020, verified clean at v0.3.0 — Appendix A.1 + A.2 row 12). Scorecard refreshed (12 signals). | +| 0.1 | July 13, 2026 | Initial rubric. Evidence base from an adversarially-verified deep-research pass (20 confirmed / 5 refuted claims) + a read-only MEFOR code-quality audit. Establishes the anti-metric rule (§4.1), the 11-signal composite rubric (§4), the five new measurement gates with local-vs-CI placement (§5), and the MEFOR scorecard (Appendix A, verdict A−). Recorded as the third companion to the SDS + Secure AI-Assisted Development Standards. | diff --git a/docs/quality-gates/HANDOFF-mutation-coverage.md b/docs/quality-gates/HANDOFF-mutation-coverage.md index 7e31f46d..990d9850 100644 --- a/docs/quality-gates/HANDOFF-mutation-coverage.md +++ b/docs/quality-gates/HANDOFF-mutation-coverage.md @@ -234,10 +234,51 @@ Add the cron to the workflow's `on:` block, then the job: ## 5. After they're green — flip the rubric +> **NOW ACTIONABLE — and done (2026-07-27).** `docs/Code_Quality_Standards.md` had never existed in this +> repo's git history despite being cited by `quality-advisory.yml`'s header and `pyproject.toml:246`. It was +> restored from the maintained copy at **v0.10**, which also carries the corrections this cycle produced: +> signal 7 had been scored ✅ Built since v0.8 while its tool crashed before generating a mutant, signal 11's +> "85 functions over C901>10" is now 122 across 43 files, and signal 8 now emits inline PR annotations. The +> citations resolve again. + Update [Code_Quality_Standards.md](../Code_Quality_Standards.md) exactly as #10/#8 were flipped in v0.3: signals 6 + 7 go from *deferred* → ✅ **Built (advisory)** in **§5** (status column), **Appendix A.2** (rows 6/7), **A.3**, and the **A.1** verdict. Bump the rubric **Version** + add a history row. +## 6. Update — how these signals now surface (2026-07-27) + +All four advisory signals were wired to reach a reviewer without buying GitHub's paid Code Quality SKU +(GA 2026-07-20, $10 per active committer/month, a standalone product **not** bundled with GHAS). They use +**workflow-command annotations**, not code scanning — no token, no permission grant, and identical +behaviour on fork PRs. The workflow still holds **no write scope on any job**. + +- **Diff-coverage** — `diff-cover --format "github-annotations:notice,markdown:diff-cover.md"` puts + `::notice` annotations **inline on the Files changed tab**. Adjacent uncovered lines are coalesced into + ranges, so a long uncovered block costs one annotation, not one per line. +- **Complexity** — a merge-base-vs-HEAD **delta** (`scripts/quality/c901_delta.py`). Raw `C901` is + unshippable as a diff signal: all 122 findings on this tree anchor on a single `def` line, so body edits + produce nothing and signature edits fire on pre-existing debt. The delta reports only what the PR caused. +- **Clones** — step summary only. jscpd emits one location per clone pair chosen by scan order. +- **Mutation** — **was dead, now repaired and running.** The signal had been reporting success while + measuring nothing: from scheduled run `30248096425` (2026-07-27) the job went green in 37 seconds because + `mutmut run || true` swallowed a crash — mutmut 2.5.1 dies in its pony-ORM cache with `TypeError: cannot + pickle 'itertools.count' object` (`cache.py:369`) **before generating a single mutant**. Repaired on + **`mutmut==3.6.0`**, verified on Linux/Python 3.14: **461 mutants, 87 killed, 19 survived, 355 not covered + by the scoped test — in 3 seconds.** The step summary now carries that table plus the survivor list, and + a non-zero `mutmut run` emits a `::warning` instead of passing silently. + + Three things are load-bearing in the mutmut 3 config, each found by a run that produced nothing: + `source_paths` must be the **package** (mutmut 3 copies it into `mutants/` and runs pytest there; with a + single file copied, `conftest.py` cannot import `messagefoundry.config` and every mutant returns "not + checked"); `only_mutate` supplies the bounded scope instead; and **`pytest-timeout` must be installed** + because mutmut 3 always passes `--timeout` to pytest. + +**Mutation-on-PR: DECIDED — yes, it now runs on pull requests.** The old "measure the wall-clock first" step +is answered: the mutating itself costs ~3 seconds, so the previous "expensive, never per-PR" cost model was +a property of mutmut 2's run-the-suite-per-mutant design, not of this scope. The job's real cost on a PR is +its install step, in line with the other jobs here. Survivors are most useful in review, which is where +someone is already looking at the test that failed to kill them. + --- *Drafted 2026-07-13 by the session that shipped the complexity + clone gates (#1028) but could not run a local diff --git a/scripts/quality/c901_delta.py b/scripts/quality/c901_delta.py new file mode 100644 index 00000000..fb6654b3 --- /dev/null +++ b/scripts/quality/c901_delta.py @@ -0,0 +1,350 @@ +"""Report the cyclomatic-complexity (ruff C901) findings a pull request actually CAUSED. + +Raw C901 cannot be surfaced on a diff, and measuring it is what proves that: every C901 finding +anchors on a SINGLE line -- the function-name token of its `def` -- so on this tree all 122 +findings are single-line regions. GitHub renders a finding inline only when its lines are inside +the diff, which makes the raw signal wrong in both directions: a PR that adds 150 lines of +branching INSIDE an existing function touches no `def` line and produces nothing, while a PR that +reflows a signature or adds a type hint fires an annotation for debt it did not cause. + +So compare instead. This reads two ruff JSON runs -- the merge base and HEAD -- keys findings on +(repo-relative file, function name), and reports only functions this PR INTRODUCED over the +threshold or whose complexity it INCREASED. Typical output is 0-3 entries, every one attributable +to the PR under review, which also keeps the count far below any annotation ceiling. + +Output is GitHub workflow-command annotations on stdout (no token, no permission grant, identical +behaviour on fork PRs) plus a markdown table for the run summary. Decreases are reported in the +summary only, so an improvement is visible without spending an annotation on it. + +Usage: + python3 scripts/quality/c901_delta.py --base c901-base.json --head c901-head.json \ + --repo-root . --summary-file "$GITHUB_STEP_SUMMARY" +""" + +import argparse +import json +import os +import re +import sys +from typing import NamedTuple + +# ruff has NO machine-readable field for the measured complexity -- the number exists only inside the +# human-readable message, e.g. "`_serve` is too complex (95 > 10)". Parsing it is therefore load-bearing, +# and a ruff wording change MUST fail loudly rather than silently classifying every finding as absent +# (which would render this script a no-op that still reports success). See _parse_message. +_MESSAGE_RE = re.compile( + r"^`(?P[^`]+)` is too complex \((?P\d+) > (?P\d+)\)$" +) + +# Default annotation ceiling. GitHub's per-step annotation cap is undocumented (the commonly cited +# ~10 traces to a 2020 reviewdog issue citing a now-dead thread) and surplus annotations are dropped +# silently, with no error. The delta keeps real output at 0-3, so this only ever bounds a pathological +# run -- and the markdown summary is always complete regardless of what the cap drops. +_DEFAULT_MAX_ANNOTATIONS = 20 + + +class Key(NamedTuple): + """Identity of a complex function: repo-relative path plus the bare function name. + + Ruff's message carries only the bare name, not a qualified path, so two same-named methods in + different classes in one file collapse to one key. Measured on this tree today: 122 findings -> + 122 unique keys, zero collisions. A collision takes the MAX complexity (see _parse_findings), + which makes the delta conservative -- it can under-report an increase, never invent one. + """ + + path: str + function: str + + +class Finding(NamedTuple): + complexity: int + threshold: int + line: int + + +def _normalise_path(filename: str, repo_root: str, scan_root: str) -> str: + """Return `filename` as a repo-relative POSIX path. + + Ruff emits ABSOLUTE paths, and they may be Windows-style even when this script runs elsewhere + (a fixture captured on a developer machine, or a test), so os.path.relpath cannot be trusted to + cross that boundary -- normalise separators and strip textually instead. + + The base run comes from a SECOND checkout of the merge base (`base-tree/`), so its absolute + paths differ from HEAD's by more than the root. Stripping only the repo root would yield + `base-tree/messagefoundry/a.py` against HEAD's `messagefoundry/a.py`, giving every function two + non-matching keys -- every one would report as new AND removed, i.e. the exact 122-annotation + flood this script exists to prevent. So: strip the repo root, and if what remains is not already + anchored at the scanned package, cut at the last occurrence of it. + """ + normalised = filename.replace("\\", "/") + root = repo_root.replace("\\", "/").rstrip("/") + anchor = f"{scan_root}/" + + # Windows paths are case-insensitive; comparing casefolded avoids a C:/ vs c:/ miss. + stripped: str | None = None + if root and normalised.casefold().startswith(f"{root.casefold()}/"): + stripped = normalised[len(root) + 1 :] + if stripped.startswith(anchor): + return stripped + + # Either a different checkout root, or an intervening directory (the base tree). Cut at the LAST + # occurrence of the scanned package so a nested copy resolves to the inner one. + index = normalised.rfind(f"/{anchor}") + if index != -1: + return normalised[index + 1 :] + + # No scan-root anchor -- e.g. a widened scope that reports a file outside the package. Prefer the + # successful root strip over the raw absolute path: returning the latter would give the base tree + # and HEAD different keys for the same file, and every such finding would flood as new. + if stripped is not None: + return stripped.lstrip("./") + + # Already relative, or a shape we cannot anchor. Return it normalised rather than guessing. + return normalised.lstrip("./") + + +def _parse_message(message: str, filename: str) -> tuple[str, int, int]: + """Extract (function, complexity, threshold) from ruff's C901 message text. + + Raises ValueError on an unrecognised message. This is deliberate and tested: reporting zero + findings is indistinguishable from a clean PR, so a ruff format change must surface as a loud + traceback in the job log rather than as a silently empty -- and permanently green -- report. + """ + match = _MESSAGE_RE.match(message) + if match is None: + raise ValueError( + f"unrecognised ruff C901 message {message!r} (from {filename!r}). " + "Ruff's message format has probably changed; update _MESSAGE_RE in " + "scripts/quality/c901_delta.py and its test before trusting this report." + ) + return match["name"], int(match["actual"]), int(match["threshold"]) + + +def _parse_findings(path: str, repo_root: str, scan_root: str) -> dict[Key, Finding]: + """Load one ruff JSON run into a {Key: Finding} map.""" + with open(path, encoding="utf-8") as handle: + raw = json.load(handle) + + findings: dict[Key, Finding] = {} + for entry in raw: + if entry.get("code") != "C901": + continue + filename = entry["filename"] + function, complexity, threshold = _parse_message(entry["message"], filename) + key = Key(_normalise_path(filename, repo_root, scan_root), function) + line = int(entry["location"]["row"]) + + previous = findings.get(key) + if previous is None or complexity > previous.complexity: + findings[key] = Finding(complexity=complexity, threshold=threshold, line=line) + return findings + + +class Change(NamedTuple): + key: Key + before: int | None # None => the function is new over the threshold + after: int + threshold: int + line: int + + +def _moved_unchanged(base: dict[Key, Finding], head: dict[Key, Finding]) -> set[Key]: + """Head keys that look like a rename/move of an identical base finding. + + A file move or a function rename makes every complex function in it read as NEW while its old key + silently disappears -- pre-existing debt annotated as PR-caused, a whole file's worth at a time. + Proper detection needs git rename data; this is the cheap floor that needs none: if a vanished base + finding has exactly the same (function, complexity, threshold) as an appearing head one, treat it as + moved rather than introduced. Conservative in the right direction -- an ACTUAL new function that + coincidentally matches a deleted one's name and exact complexity is merely under-reported, whereas + the flood it prevents is the failure that makes the whole signal untrustworthy. + """ + vanished: dict[tuple[str, int, int], int] = {} + for key, finding in base.items(): + if key not in head: + signature = (key.function, finding.complexity, finding.threshold) + vanished[signature] = vanished.get(signature, 0) + 1 + + moved: set[Key] = set() + for key, finding in sorted(head.items()): + if key in base: + continue + signature = (key.function, finding.complexity, finding.threshold) + if vanished.get(signature, 0) > 0: + vanished[signature] -= 1 + moved.add(key) + return moved + + +def classify( + base: dict[Key, Finding], head: dict[Key, Finding] +) -> tuple[list[Change], list[Change], list[Change]]: + """Split into (new, increased, decreased). Unchanged findings are dropped entirely.""" + new: list[Change] = [] + increased: list[Change] = [] + decreased: list[Change] = [] + moved = _moved_unchanged(base, head) + + for key, finding in sorted(head.items()): + previous = base.get(key) + if previous is None: + if key in moved: + continue + new.append(Change(key, None, finding.complexity, finding.threshold, finding.line)) + elif finding.complexity > previous.complexity: + increased.append( + Change( + key, previous.complexity, finding.complexity, finding.threshold, finding.line + ) + ) + elif finding.complexity < previous.complexity: + decreased.append( + Change( + key, previous.complexity, finding.complexity, finding.threshold, finding.line + ) + ) + return new, increased, decreased + + +def _escape_data(value: str) -> str: + return value.replace("%", "%25").replace("\r", "%0D").replace("\n", "%0A") + + +def _escape_property(value: str) -> str: + return _escape_data(value).replace(":", "%3A").replace(",", "%2C") + + +def _annotation(change: Change) -> str: + if change.before is None: + title = "New function over the complexity threshold" + detail = ( + f"`{change.key.function}` is new at complexity {change.after} " + f"(mccabe threshold {change.threshold})" + ) + else: + title = "Complexity increased" + detail = ( + f"`{change.key.function}` complexity {change.before} -> {change.after} " + f"(mccabe threshold {change.threshold})" + ) + return ( + f"::warning file={_escape_property(change.key.path)}," + f"line={change.line},endLine={change.line}," + f"title={_escape_property(title)}::{_escape_data(detail)}" + ) + + +def _markdown(new: list[Change], increased: list[Change], decreased: list[Change]) -> str: + lines = ["## Cyclomatic complexity (C901) — changed by this PR", ""] + + if not new and not increased: + lines.append("No function was introduced over the threshold or made more complex. ✅") + else: + lines += ["| Function | File | Complexity | Threshold |", "| --- | --- | --- | --- |"] + for change in new + increased: + before = "new" if change.before is None else str(change.before) + lines.append( + f"| `{change.key.function}` | `{change.key.path}`:{change.line} " + f"| {before} → **{change.after}** | {change.threshold} |" + ) + + if decreased: + lines += ["", "### Improved (summary only, not annotated)", ""] + lines += ["| Function | File | Complexity |", "| --- | --- | --- |"] + for change in decreased: + lines.append( + f"| `{change.key.function}` | `{change.key.path}`:{change.line} " + f"| {change.before} → **{change.after}** |" + ) + + lines += [ + "", + "Advisory only — never gates a merge. Pre-existing complexity is deliberately not " + "reported: only what this PR changed appears here.", + "", + ] + return "\n".join(lines) + + +def _non_negative_int(value: str) -> int: + parsed = int(value) + if parsed < 0: + raise argparse.ArgumentTypeError(f"must be >= 0, got {parsed}") + return parsed + + +def _threshold_of(findings: dict[Key, Finding]) -> int | None: + """The mccabe threshold these findings were measured against, if they agree on one.""" + thresholds = {f.threshold for f in findings.values()} + return thresholds.pop() if len(thresholds) == 1 else None + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--base", required=True, help="ruff C901 JSON from the merge base") + parser.add_argument("--head", required=True, help="ruff C901 JSON from HEAD") + parser.add_argument("--repo-root", default=".", help="checkout root, for relative paths") + parser.add_argument("--scan-root", default="messagefoundry", help="scanned package name") + # Non-negative: a negative cap would slice from the END of the list (reporting the wrong findings) + # and print a nonsense "further change(s)" count. + parser.add_argument( + "--max-annotations", type=_non_negative_int, default=_DEFAULT_MAX_ANNOTATIONS + ) + parser.add_argument( + "--summary-file", default=None, help="append markdown here (GITHUB_STEP_SUMMARY)" + ) + args = parser.parse_args(argv) + + repo_root = os.path.abspath(args.repo_root) + base = _parse_findings(args.base, repo_root, args.scan_root) + head = _parse_findings(args.head, repo_root, args.scan_root) + + # A PR that TIGHTENS ruff's max-complexity makes every function between the old and new thresholds + # appear as new -- pre-existing debt, annotated en masse, on the one PR whose author already knows. + # The two runs are not comparable, so say that instead of reporting a fake delta. + base_threshold, head_threshold = _threshold_of(base), _threshold_of(head) + if ( + base_threshold is not None + and head_threshold is not None + and base_threshold != head_threshold + ): + print( + f"::notice title=Complexity delta not comparable::the mccabe threshold moved " + f"{base_threshold} -> {head_threshold}; a delta against the old threshold would be noise" + ) + summary = ( + "## Cyclomatic complexity (C901) — not comparable\n\n" + f"This PR changes the mccabe threshold ({base_threshold} → {head_threshold}), so a " + "before/after delta would report pre-existing functions as new. Skipped deliberately.\n" + ) + if args.summary_file: + with open(args.summary_file, "a", encoding="utf-8") as handle: + handle.write(summary) + return 0 + + new, increased, decreased = classify(base, head) + + annotated = new + increased + for change in annotated[: args.max_annotations]: + print(_annotation(change)) + if len(annotated) > args.max_annotations: + dropped = len(annotated) - args.max_annotations + print( + f"::notice title=Complexity delta truncated::{dropped} further change(s) in the summary" + ) + + markdown = _markdown(new, increased, decreased) + if args.summary_file: + with open(args.summary_file, "a", encoding="utf-8") as handle: + handle.write(markdown) + else: + sys.stderr.write(markdown) + + # Always 0. This is an advisory reporter; a non-zero exit here would be a merge-blocking signal + # in disguise. Genuine breakage (an unparseable ruff message) raises instead, which is visible + # in the log without reddening the job. + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/test_c901_delta.py b/tests/test_c901_delta.py new file mode 100644 index 00000000..3635ecd6 --- /dev/null +++ b/tests/test_c901_delta.py @@ -0,0 +1,419 @@ +"""Tests for scripts/quality/c901_delta.py -- the PR-caused complexity reporter. + +The script exists because raw C901 is unshippable as a diff signal (all 122 findings on this tree +anchor on a single `def` line). Its whole value is that it reports ONLY what a PR changed, so the +tests that matter are: pre-existing findings never appear, and a ruff wording change fails loudly +instead of quietly reporting "nothing changed" forever. +""" + +import importlib.util +import json +import sys +from pathlib import Path +from types import ModuleType + +import pytest + +_ROOT = Path(__file__).resolve().parents[1] + + +def _load() -> ModuleType: + path = _ROOT / "scripts" / "quality" / "c901_delta.py" + assert path.is_file(), f"delta script not found at {path}" + spec = importlib.util.spec_from_file_location("c901_delta", path) + assert spec is not None and spec.loader is not None + mod = importlib.util.module_from_spec(spec) + sys.modules[spec.name] = mod + spec.loader.exec_module(mod) + return mod + + +delta = _load() + + +def _finding(filename: str, function: str, complexity: int, row: int, threshold: int = 10) -> dict: + """One ruff C901 result, in ruff's real JSON shape (captured from ruff 0.15.22).""" + return { + "code": "C901", + "message": f"`{function}` is too complex ({complexity} > {threshold})", + "filename": filename, + "location": {"column": 5, "row": row}, + "end_location": {"column": 5 + len(function), "row": row}, + "fix": None, + "url": "https://docs.astral.sh/ruff/rules/complex-structure", + } + + +def _write(tmp_path: Path, name: str, findings: list[dict]) -> str: + path = tmp_path / name + path.write_text(json.dumps(findings), encoding="utf-8") + return str(path) + + +# -------------------------------------------------------------------------------------------- +# Classification: the core promise -- unchanged pre-existing debt must never be reported. +# -------------------------------------------------------------------------------------------- + + +def _parse(tmp_path: Path, name: str, findings: list[dict], root: str = "/repo") -> dict: + return delta._parse_findings(_write(tmp_path, name, findings), root, "messagefoundry") + + +def test_unchanged_findings_are_never_reported(tmp_path: Path) -> None: + """The single most important assertion: 122 untouched findings produce zero output.""" + same = [_finding("/repo/messagefoundry/a.py", "f", 30, 10)] + base = _parse(tmp_path, "base.json", same) + head = _parse(tmp_path, "head.json", same) + + new, increased, decreased = delta.classify(base, head) + + assert new == [] + assert increased == [] + assert decreased == [] + + +def test_new_function_over_threshold_is_reported(tmp_path: Path) -> None: + base = _parse(tmp_path, "base.json", []) + head = _parse(tmp_path, "head.json", [_finding("/repo/messagefoundry/a.py", "fresh", 14, 42)]) + + new, increased, decreased = delta.classify(base, head) + + assert [c.key.function for c in new] == ["fresh"] + assert new[0].before is None + assert new[0].after == 14 + assert new[0].line == 42 + assert increased == [] and decreased == [] + + +def test_increased_complexity_is_reported_with_both_numbers(tmp_path: Path) -> None: + """The body-edit case: the `def` line never moved, but complexity rose. This is the case + SARIF/code-scanning cannot see at all, and the reason the delta exists.""" + base = _parse(tmp_path, "base.json", [_finding("/repo/messagefoundry/a.py", "grow", 12, 7)]) + head = _parse(tmp_path, "head.json", [_finding("/repo/messagefoundry/a.py", "grow", 27, 7)]) + + new, increased, decreased = delta.classify(base, head) + + assert new == [] and decreased == [] + assert len(increased) == 1 + assert (increased[0].before, increased[0].after) == (12, 27) + + +def test_decreased_complexity_is_summary_only(tmp_path: Path) -> None: + base = _parse(tmp_path, "base.json", [_finding("/repo/messagefoundry/a.py", "shrink", 40, 3)]) + head = _parse(tmp_path, "head.json", [_finding("/repo/messagefoundry/a.py", "shrink", 11, 3)]) + + new, increased, decreased = delta.classify(base, head) + + assert new == [] and increased == [] + assert len(decreased) == 1 + markdown = delta._markdown(new, increased, decreased) + assert "Improved" in markdown + assert "shrink" in markdown + + +def test_a_function_dropping_below_threshold_is_not_a_regression(tmp_path: Path) -> None: + """Fixed entirely: it leaves ruff's output, so it must not read as new/increased.""" + base = _parse(tmp_path, "base.json", [_finding("/repo/messagefoundry/a.py", "fixed", 12, 5)]) + head = _parse(tmp_path, "head.json", []) + + new, increased, decreased = delta.classify(base, head) + + assert new == [] and increased == [] and decreased == [] + + +# -------------------------------------------------------------------------------------------- +# Path normalisation -- ruff emits ABSOLUTE paths; annotations need repo-relative POSIX ones. +# -------------------------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + ("filename", "repo_root", "expected"), + [ + ("/repo/messagefoundry/api/app.py", "/repo", "messagefoundry/api/app.py"), + (r"C:\co\messagefoundry\api\app.py", r"C:\co", "messagefoundry/api/app.py"), + # Casefolded compare: a drive letter differing only in case must still strip. + (r"c:\co\messagefoundry\a.py", r"C:\co", "messagefoundry/a.py"), + # JSON captured from a DIFFERENT checkout: fall back to the scan-root segment. + (r"D:\other\tree\messagefoundry\a.py", "/repo", "messagefoundry/a.py"), + ("messagefoundry/a.py", "/repo", "messagefoundry/a.py"), + # THE BASE-TREE CASE. The merge-base run happens in a second checkout under the repo root, + # so its paths carry an extra segment. If this did not collapse to the same key as HEAD, + # every function would report as new -- a 122-annotation flood on the very first PR. + ( + "/home/runner/work/r/r/base-tree/messagefoundry/a.py", + "/home/runner/work/r/r", + "messagefoundry/a.py", + ), + # A repo root that itself ends in the scanned package name must not confuse the anchor. + ("/src/messagefoundry/messagefoundry/a.py", "/src/messagefoundry", "messagefoundry/a.py"), + ], +) +def test_paths_normalise_to_repo_relative_posix( + filename: str, repo_root: str, expected: str +) -> None: + assert delta._normalise_path(filename, repo_root, "messagefoundry") == expected + + +def test_base_tree_and_head_paths_produce_the_SAME_key(tmp_path: Path) -> None: + """Regression guard for the flood bug: the merge-base checkout lives one directory deeper, and + if that leaks into the key then nothing ever matches and every finding reads as new.""" + root = "/home/runner/work/r/r" + head = _parse( + tmp_path, "head.json", [_finding(f"{root}/messagefoundry/a.py", "f", 30, 10)], root + ) + base = _parse( + tmp_path, + "base.json", + [_finding(f"{root}/base-tree/messagefoundry/a.py", "f", 30, 10)], + root, + ) + + assert set(head) == set(base) + assert delta.classify(base, head) == ([], [], []) + + +def test_annotation_carries_a_relative_path_and_the_def_line(tmp_path: Path) -> None: + base = _parse(tmp_path, "base.json", []) + head = _parse( + tmp_path, "head.json", [_finding(r"C:\repo\messagefoundry\x.py", "g", 13, 99)], "C:/repo" + ) + new, _, _ = delta.classify(base, head) + + line = delta._annotation(new[0]) + + assert line.startswith("::warning file=messagefoundry/x.py,") + assert "line=99,endLine=99" in line + assert "C:" not in line # an absolute local path must never reach the annotation + assert "13" in line + + +# -------------------------------------------------------------------------------------------- +# Fail-loud parsing -- a silent zero-finding report is indistinguishable from a clean PR. +# -------------------------------------------------------------------------------------------- + + +def test_unparseable_message_raises_rather_than_reporting_nothing(tmp_path: Path) -> None: + broken = [ + { + "code": "C901", + "message": "`f` has a cyclomatic complexity of 30, over the limit of 10", + "filename": "/repo/messagefoundry/a.py", + "location": {"column": 5, "row": 1}, + "end_location": {"column": 6, "row": 1}, + } + ] + with pytest.raises(ValueError, match="unrecognised ruff C901 message"): + _parse(tmp_path, "head.json", broken) + + +def test_non_c901_results_are_ignored(tmp_path: Path) -> None: + mixed = [ + { + "code": "E501", + "message": "Line too long", + "filename": "/repo/messagefoundry/a.py", + "location": {"column": 1, "row": 1}, + "end_location": {"column": 2, "row": 1}, + }, + _finding("/repo/messagefoundry/a.py", "only", 12, 4), + ] + parsed = _parse(tmp_path, "head.json", mixed) + assert [k.function for k in parsed] == ["only"] + + +def test_key_collision_takes_the_max_complexity(tmp_path: Path) -> None: + """Two same-named methods in one file share a key. Taking the max keeps the delta + conservative -- it can under-report an increase, never invent one.""" + collide = [ + _finding("/repo/messagefoundry/a.py", "run", 12, 10), + _finding("/repo/messagefoundry/a.py", "run", 40, 80), + ] + parsed = _parse(tmp_path, "head.json", collide) + assert len(parsed) == 1 + assert next(iter(parsed.values())).complexity == 40 + + +# -------------------------------------------------------------------------------------------- +# Annotation cap + end-to-end main() +# -------------------------------------------------------------------------------------------- + + +def test_annotations_are_capped_and_the_remainder_is_announced( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + head = [_finding(f"/repo/messagefoundry/f{i}.py", f"fn{i}", 12, i + 1) for i in range(25)] + summary = tmp_path / "summary.md" + + rc = delta.main( + [ + "--base", + _write(tmp_path, "base.json", []), + "--head", + _write(tmp_path, "head.json", head), + "--repo-root", + "/repo", + "--max-annotations", + "20", + "--summary-file", + str(summary), + ] + ) + out = capsys.readouterr().out + + assert rc == 0 + assert out.count("::warning ") == 20 + assert "::notice title=Complexity delta truncated::5 further change(s)" in out + # The summary is ALWAYS complete, even when annotations are dropped. + assert summary.read_text(encoding="utf-8").count("| `fn") == 25 + + +def test_main_reports_clean_when_nothing_changed( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + same = [_finding("/repo/messagefoundry/a.py", "f", 30, 10)] + summary = tmp_path / "summary.md" + + rc = delta.main( + [ + "--base", + _write(tmp_path, "base.json", same), + "--head", + _write(tmp_path, "head.json", same), + "--repo-root", + "/repo", + "--summary-file", + str(summary), + ] + ) + + assert rc == 0 + assert "::warning" not in capsys.readouterr().out + assert "No function was introduced over the threshold" in summary.read_text(encoding="utf-8") + + +def test_main_appends_rather_than_truncating_the_summary(tmp_path: Path) -> None: + """GITHUB_STEP_SUMMARY accumulates across steps; overwriting would eat another step's output.""" + summary = tmp_path / "summary.md" + summary.write_text("## existing content\n", encoding="utf-8") + + delta.main( + [ + "--base", + _write(tmp_path, "base.json", []), + "--head", + _write(tmp_path, "head.json", []), + "--repo-root", + "/repo", + "--summary-file", + str(summary), + ] + ) + + assert "## existing content" in summary.read_text(encoding="utf-8") + + +def test_a_file_move_does_not_report_pre_existing_complexity_as_new(tmp_path: Path) -> None: + """Renaming a file changes every key in it. Without rename tolerance the whole file's worth of + pre-existing debt annotates as PR-caused -- the flood this script exists to prevent, arriving by + a different door.""" + old = [ + _finding(f"/repo/messagefoundry/old/m{i}.py", f"fn{i}", 12 + i, i + 1) for i in range(13) + ] + moved = [ + _finding(f"/repo/messagefoundry/new/m{i}.py", f"fn{i}", 12 + i, i + 1) for i in range(13) + ] + base = _parse(tmp_path, "base.json", old) + head = _parse(tmp_path, "head.json", moved) + + new, increased, decreased = delta.classify(base, head) + + assert new == [], f"a pure move must annotate nothing, got {[c.key for c in new]}" + assert increased == [] and decreased == [] + + +def test_a_move_that_also_increases_complexity_is_still_reported(tmp_path: Path) -> None: + """Rename tolerance must not swallow a real regression that happens to accompany a move.""" + base = _parse(tmp_path, "base.json", [_finding("/repo/messagefoundry/a.py", "fn", 12, 1)]) + head = _parse(tmp_path, "head.json", [_finding("/repo/messagefoundry/b.py", "fn", 40, 1)]) + + new, increased, decreased = delta.classify(base, head) + + assert len(new) == 1, "a moved function whose complexity changed is not an unchanged move" + assert new[0].after == 40 + + +def test_a_threshold_change_reports_not_comparable_instead_of_flooding( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """A PR tightening max-complexity would otherwise report every function between the old and new + thresholds as new.""" + base = [ + _finding(f"/repo/messagefoundry/f{i}.py", f"fn{i}", 12, 1, threshold=10) for i in range(30) + ] + head = [ + _finding(f"/repo/messagefoundry/f{i}.py", f"fn{i}", 12, 1, threshold=5) for i in range(30) + ] + summary = tmp_path / "summary.md" + + rc = delta.main( + [ + "--base", + _write(tmp_path, "base.json", base), + "--head", + _write(tmp_path, "head.json", head), + "--repo-root", + "/repo", + "--summary-file", + str(summary), + ] + ) + out = capsys.readouterr().out + + assert rc == 0 + assert "::warning" not in out + assert "not comparable" in out + assert "not comparable" in summary.read_text(encoding="utf-8") + + +def test_a_negative_annotation_cap_is_rejected(tmp_path: Path) -> None: + """A negative cap would slice from the END of the list, reporting the wrong findings.""" + with pytest.raises(SystemExit): + delta.main( + [ + "--base", + _write(tmp_path, "base.json", []), + "--head", + _write(tmp_path, "head.json", []), + "--max-annotations", + "-5", + ] + ) + + +def test_a_path_outside_the_scan_root_still_keys_consistently(tmp_path: Path) -> None: + """A widened scope reporting a file outside the package must not key the base tree copy + differently from HEAD's -- that would flood every such finding as new.""" + root = "/home/runner/work/r/r" + head = _parse(tmp_path, "head.json", [_finding(f"{root}/tee/x.py", "f", 30, 10)], root) + base = _parse( + tmp_path, "base.json", [_finding(f"{root}/base-tree/tee/x.py", "f", 30, 10)], root + ) + + assert set(head) == {delta.Key("tee/x.py", "f")} + # The base tree copy cannot be root-stripped to the same string, so rename tolerance is what + # keeps it from flooding; either way it must not be reported as new. + assert delta.classify(base, head)[0] == [] + + +def test_workflow_command_metacharacters_are_escaped() -> None: + change = delta.Change( + key=delta.Key(path="messagefoundry/a,b.py", function="weird%name"), + before=1, + after=2, + threshold=10, + line=3, + ) + line = delta._annotation(change) + + assert "file=messagefoundry/a%2Cb.py" in line + assert "weird%25name" in line diff --git a/tests/test_quality_advisory_invariants.py b/tests/test_quality_advisory_invariants.py new file mode 100644 index 00000000..2630e66d --- /dev/null +++ b/tests/test_quality_advisory_invariants.py @@ -0,0 +1,307 @@ +"""Pin the advisory guarantees of .github/workflows/quality-advisory.yml. + +Every safety property of that workflow is a string in a YAML file: one edit can grant a write +scope, drop an `--exit-zero`, or add a SARIF upload, and nothing else in the repo would notice. +This repo already parses workflow YAML in tests for exactly that reason (see +test_dependabot_automerge_guardrails.py, test_release_pipeline.py, test_lint_scope_parity.py). + +What actually keeps these jobs advisory is that their contexts are not in the required-checks set on +`main`, and that every analysis step is non-failing (`continue-on-error` + `--exit-zero` / +`--fail-under=0` / `|| true`). Only the second half is assertable from inside the repo, so that is +what test_every_analysis_step_cannot_fail_its_job pins. + +The no-write-scope assertion is a separate, narrower claim: least privilege. It does NOT by itself +prevent merge gating -- permissions and branch protection are unrelated mechanisms -- but it is +worth pinning because two of these jobs execute third-party code fetched at run time, and it is +only holdable because the jobs surface findings via workflow commands rather than SARIF upload. +""" + +import re +from pathlib import Path + +import pytest +import yaml + +_WORKFLOW = Path(__file__).resolve().parents[1] / ".github" / "workflows" / "quality-advisory.yml" + +# Steps that actually run a quality tool. Setup steps (checkout, setup-python, apt-get, the tool +# installs) are deliberately NOT required to be continue-on-error: masking an infrastructure failure +# would produce confusing downstream errors, and a red ADVISORY job blocks nothing anyway -- these +# contexts are not in the required-checks set. +_ANALYSIS_MARKERS = ( + "ruff check", + "npx --yes jscpd", + "diff-cover coverage.xml", + "mutmut run", + "mutmut results", + "c901_delta.py", + "pytest -q --cov", +) + +# An analysis command must be incapable of failing its step. +_NON_FAILING_IDIOMS = ("--exit-zero", "--fail-under=0", "|| true") + +_SHA_PINNED = re.compile(r"^[^@]+@[0-9a-f]{40}$") + + +@pytest.fixture(scope="module") +def workflow() -> dict: + assert _WORKFLOW.is_file(), f"workflow not found at {_WORKFLOW}" + return yaml.safe_load(_WORKFLOW.read_text(encoding="utf-8")) + + +@pytest.fixture(scope="module") +def raw() -> str: + return _WORKFLOW.read_text(encoding="utf-8") + + +def _strip_comments(text: str) -> str: + """Drop YAML comments. + + The header deliberately EXPLAINS why there is no SARIF upload, so a naive substring search for + "sarif" matches the very comment that documents its absence. Assert against code instead. + """ + out = [] + for line in text.splitlines(): + stripped = line.lstrip() + if stripped.startswith("#"): + continue + out.append(line.split(" #", 1)[0] if " #" in line else line) + return "\n".join(out) + + +@pytest.fixture(scope="module") +def code(raw: str) -> str: + return _strip_comments(raw) + + +def _steps(workflow: dict) -> list[tuple[str, dict]]: + return [(name, step) for name, job in workflow["jobs"].items() for step in job["steps"]] + + +def _analysis_steps(workflow: dict) -> list[tuple[str, dict]]: + return [ + (job, step) + for job, step in _steps(workflow) + if any(marker in (step.get("run") or "") for marker in _ANALYSIS_MARKERS) + ] + + +# -------------------------------------------------------------------------------------------- +# The advisory guarantee. +# -------------------------------------------------------------------------------------------- + + +def test_workflow_grants_no_permissions_by_default(workflow: dict) -> None: + assert workflow["permissions"] == {}, "workflow-level permissions must stay deny-by-default" + + +def test_no_job_holds_any_write_scope(workflow: dict) -> None: + """Least privilege. Both the clone and complexity jobs execute third-party code fetched at run + time (`npx --yes jscpd`, `pipx install ruff`) with no integrity pin, so handing them a + write-scoped repository token would be a real regression. This does NOT by itself stop the jobs + gating a merge -- see the module docstring -- it just keeps their blast radius at zero.""" + for name, job in workflow["jobs"].items(): + permissions = job.get("permissions") + assert permissions is not None, f"job {name!r} must declare explicit permissions" + for scope, level in permissions.items(): + assert level == "read", f"job {name!r} grants {scope}: {level} -- must be read" + + +def test_no_sarif_upload_and_no_security_events(code: str) -> None: + """Measured, not stylistic: all 122 C901 findings anchor on a single `def` line, jscpd emits one + scan-order location per clone pair, and a PR-only upload never builds a baseline -- so every PR + would report every finding as new, forever. See the workflow header for the full reasoning + (which is why this asserts against comment-stripped code, not the raw text).""" + assert "sarif" not in code.lower(), ( + "no SARIF surface in this workflow -- see the header comment" + ) + assert "security-events" not in code + + +def test_no_pull_request_target(raw: str) -> None: + """pull_request_target grants a read/write token even from a public fork.""" + assert "pull_request_target" not in raw + + +def test_every_analysis_step_cannot_fail_its_job(workflow: dict) -> None: + steps = _analysis_steps(workflow) + assert len(steps) >= 6, f"expected the quality tool steps to be found, got {len(steps)}" + for job, step in steps: + name = step.get("name", "") + assert step.get("continue-on-error") is True, ( + f"{job}/{name!r} runs a quality tool without continue-on-error: true" + ) + body = step["run"] + assert any(idiom in body for idiom in _NON_FAILING_IDIOMS), ( + f"{job}/{name!r} has no non-failing idiom ({_NON_FAILING_IDIOMS})" + ) + + +# -------------------------------------------------------------------------------------------- +# Supply chain. +# -------------------------------------------------------------------------------------------- + + +def test_every_action_is_sha_pinned_with_a_version_comment(workflow: dict, raw: str) -> None: + uses = [step["uses"] for _, step in _steps(workflow) if "uses" in step] + assert uses, "expected at least one action" + for ref in uses: + assert _SHA_PINNED.match(ref), f"{ref} is not pinned to a 40-hex SHA" + for line in raw.splitlines(): + if "uses:" in line: + assert re.search(r"#\s*v", line), f"missing version comment: {line.strip()}" + + +def test_jscpd_stays_on_4x(raw: str) -> None: + """npm `latest` is a 5.x Rust rewrite shipped as platform binaries with a different CLI.""" + assert re.search(r"jscpd@4\.\d+\.\d+", raw), "jscpd must stay pinned to a 4.x release" + + +def test_diff_cover_is_pinned_exactly(raw: str) -> None: + """The annotation surface depends on this version's `--format github-annotations:`.""" + assert re.search(r'"diff-cover==\d+\.\d+\.\d+"', raw), "diff-cover must be pinned with ==" + + +def test_the_ruff_version_is_derived_from_the_lock_not_hardcoded(workflow: dict, code: str) -> None: + """The delta script parses ruff's human-readable C901 message, so a version skew can change the + wording under the parser -- but the fix must NOT be a hardcoded pin asserted against the lock. + + This test file runs in the ordinary pytest suite, which IS a required check. Asserting equality + between a workflow string and constraints.lock would mean a routine Dependabot ruff bump reds a + BLOCKING context over a purely advisory concern. Deriving the version at run time removes the + drift and the coupling at once, so assert the derivation instead of the value. + """ + step = next( + s + for job, s in _steps(workflow) + if job == "complexity" and "pipx install" in (s.get("run") or "") + ) + body = step["run"] + assert "constraints.lock" in body, ( + "the complexity job must read ruff's version from constraints.lock at run time" + ) + assert not re.search(r"pipx install ruff==\d", code), ( + "do not hardcode the ruff version here -- it couples a required check to this workflow" + ) + + +# -------------------------------------------------------------------------------------------- +# The surfacing mechanisms themselves. +# -------------------------------------------------------------------------------------------- + + +def test_diff_coverage_emits_inline_github_annotations(code: str) -> None: + assert "github-annotations:" in code, "diff-cover must emit inline annotations" + + +def test_the_complexity_delta_is_wired_and_pr_gated(workflow: dict) -> None: + steps = [s for job, s in _steps(workflow) if "c901_delta.py" in (s.get("run") or "")] + assert len(steps) == 1, "expected exactly one complexity delta step" + assert steps[0].get("if") == "github.event_name == 'pull_request'", ( + "the delta needs a base ref; it must not run on the cron or workflow_dispatch" + ) + assert "--summary-file" in steps[0]["run"] + + +def test_the_delta_script_exists(workflow: dict) -> None: + script = _WORKFLOW.parents[2] / "scripts" / "quality" / "c901_delta.py" + assert script.is_file(), "the complexity job references a script that is not in the repo" + + +def test_the_complexity_job_fetches_full_history(workflow: dict) -> None: + """`git merge-base` cannot work against a shallow clone.""" + checkout = next( + step + for job, step in _steps(workflow) + if job == "complexity" and "checkout" in (step.get("uses") or "") + ) + assert checkout.get("with", {}).get("fetch-depth") == 0, ( + "the complexity job needs full history -- git merge-base cannot work on a shallow clone" + ) + + +def test_checkouts_do_not_persist_credentials(workflow: dict) -> None: + """quality-advisory.yml is not in .github/zizmor.yml's artipacked ignore list.""" + for job, step in _steps(workflow): + if "checkout" in (step.get("uses") or ""): + assert step.get("with", {}).get("persist-credentials") is False, ( + f"{job} checkout must set persist-credentials: false" + ) + + +def test_the_coverage_job_does_not_reshallow_its_own_full_clone(code: str) -> None: + """`fetch-depth: 0` then `git fetch --depth=1` writes .git/shallow and grafts away the history + behind the base tip, so diff-cover's three-dot range loses its merge base the moment the base + branch advances. It then fails in the worst way: the markdown reporter truncates diff-cover.md + on open before raising, `|| true` swallows the error, and the summary shows a clean-looking + empty section while zero annotations are emitted.""" + assert "--depth" not in code, "a shallow fetch here defeats diff-cover's merge base" + + +def test_report_guards_test_for_content_not_mere_existence(code: str) -> None: + """diff-cover creates and truncates its report before it can fail, so `-f` passes on a 0-byte + file and would append an empty section that reads as 'nothing uncovered'.""" + assert "[ -s diff-cover.md ]" in code + assert "[ -f diff-cover.md ]" not in code + + +def test_step_summary_writes_are_size_guarded(code: str) -> None: + """An oversized $GITHUB_STEP_SUMMARY write is dropped ENTIRELY, losing the whole surface. + + Asserts on the COUNT of truncation idioms rather than one magic constant: the blocks legitimately + use different limits (a survivor list needs less room than a coverage report), and pinning the + exact byte count made this fail on a change that was still correctly guarded. + """ + appends = code.count('>> "$GITHUB_STEP_SUMMARY"') + assert appends >= 3, f"expected the summary blocks to be present, found {appends}" + truncations = code.count("head -c ") + code.count("tail -c ") + assert truncations >= appends, ( + f"{appends} summary blocks but only {truncations} truncation guards -- an oversized write " + "is dropped entirely, silently losing the whole surface" + ) + + +def test_mutmut_is_pinned_to_3x_with_pytest_timeout(code: str) -> None: + """mutmut 2.5.1 crashes on Python 3.14 before generating a mutant, and `|| true` made that look + green for months. pytest-timeout is not optional: mutmut 3 always passes `--timeout` to pytest, + and without the plugin every invocation dies inside BadTestExecutionCommandsException.""" + assert re.search(r'"mutmut==3\.\d+\.\d+"', code), "mutmut must be pinned to an exact 3.x" + assert "mutmut<3" not in code, "mutmut 2.x does not run on Python 3.14" + assert "pytest-timeout" in code, "mutmut 3 requires pytest-timeout" + + +def test_mutmut_copies_the_package_not_just_the_mutated_file(code: str) -> None: + """mutmut 3 copies `source_paths` into mutants/ and runs pytest there. With a single FILE as the + source path, conftest.py cannot import the rest of the package and every mutant comes back + 'not checked' -- a green job measuring nothing. Copy the package, mutate one module.""" + # The config is emitted by `printf`, so the separators are literal backslash-n in the YAML. + assert r"source_paths=messagefoundry\n" in code, ( + "source_paths must be the package, not one file" + ) + assert "only_mutate=" in code, "the bounded scope must come from only_mutate" + assert "paths_to_mutate" not in code, "deprecated in mutmut 3" + assert "runner=" not in code, "mutmut 3 uses pytest_add_cli_args_test_selection" + + +def test_mutmut_artifact_includes_hidden_files(workflow: dict) -> None: + """.mutmut-cache is a dotfile and upload-artifact skips hidden files by default -- without this + the step logs 'No files were found', uploads nothing, and still reports success.""" + upload = next( + step for _, step in _steps(workflow) if "upload-artifact" in (step.get("uses") or "") + ) + with_ = upload.get("with", {}) + assert with_.get("include-hidden-files") is True, ( + "the mutmut cache is a dotfile; without include-hidden-files this step uploads nothing " + "and still reports success" + ) + assert with_.get("if-no-files-found") == "warn" + + +def test_no_expression_interpolation_inside_run_bodies(workflow: dict) -> None: + """Template injection: untrusted `${{ }}` expanded into a shell body. Values must be routed + through `env:` instead (zizmor enforces this in CI; assert it here too).""" + for job, step in _steps(workflow): + body = step.get("run") + if body: + assert "${{" not in body, f"{job}/{step.get('name')!r} interpolates into a run body"