From fef763ea3b45dc82c37d526d7cbf47627663fadd Mon Sep 17 00:00:00 2001 From: Helix <267227783+helix-nine@users.noreply.github.com> Date: Thu, 27 Aug 2026 01:30:49 +0000 Subject: [PATCH 01/17] feat(repo): measure cognitive complexity, and propose a budget protocol MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `make complexity`, `complexity-top` and `complexity-diff` — a tree-sitter census over Rust and TypeScript that runs in 0.6s with no build, and an RFC proposing the protocol that puts its numbers in front of a PR author. Cognitive complexity rather than cyclomatic: probed against a synthetic file, a 20-arm match scores cyclomatic 21 / cognitive 1, five `?` operators score 6 / 0, and four-deep nesting scores 5 / 10. Cyclomatic ranks an enum-to-string match as the worst function in the repo; cognitive does not rank it at all. Inline `#[cfg(test)]` items are stripped before measuring — this repo keeps 1,150 test functions inline against 6,249 lines in dedicated test files, so a path rule would make a PR that adds tests read as one that adds complexity. Generated trees are skipped, and the census refuses to report below 98% parse coverage, because grammar rot is otherwise silent. The RFC carries the baseline, the growth measurements that motivate it, and the enforcement questions that need a decision before any of it is wired to CI. --- Makefile | 4 +- build/complexity.mk | 21 ++++ build/complexity/.gitignore | 2 + build/complexity/census.py | 125 ++++++++++++++++++++ build/complexity/census.sh | 30 +++++ build/complexity/check.py | 39 +++++++ build/complexity/delta.py | 42 +++++++ build/complexity/fetch-tool.sh | 17 +++ rfcs/complexity-budget.md | 207 +++++++++++++++++++++++++++++++++ 9 files changed, 486 insertions(+), 1 deletion(-) create mode 100644 build/complexity.mk create mode 100644 build/complexity/.gitignore create mode 100644 build/complexity/census.py create mode 100755 build/complexity/census.sh create mode 100644 build/complexity/check.py create mode 100644 build/complexity/delta.py create mode 100755 build/complexity/fetch-tool.sh create mode 100644 rfcs/complexity-budget.md diff --git a/Makefile b/Makefile index b3888597c9..e8f684bd3d 100644 --- a/Makefile +++ b/Makefile @@ -14,8 +14,9 @@ include projects/start-tunnel/build.mk include projects/start-os/build.mk include projects/start-wrt/build.mk include projects/start-docs/build.mk +include build/complexity.mk -.PHONY: help start-os metadata start-os-install clean format format-check start-cli-install start-cli start-cli-deb start-os-uis start-os-ui start-os-emulate-reflash start-os-deb start-os-$(IMAGE_TYPE) start-os-squashfs start-os-wormhole start-os-wormhole-deb start-os-update start-os-update-from-gha test start-core-test start-sdk-test container-runtime-test start-wrt-test start-registry start-registry-install start-tunnel start-tunnel-install start-core-ts-bindings +.PHONY: help start-os metadata start-os-install clean format format-check complexity complexity-top complexity-diff complexity-check start-cli-install start-cli start-cli-deb start-os-uis start-os-ui start-os-emulate-reflash start-os-deb start-os-$(IMAGE_TYPE) start-os-squashfs start-os-wormhole start-os-wormhole-deb start-os-update start-os-update-from-gha test start-core-test start-sdk-test container-runtime-test start-wrt-test start-registry start-registry-install start-tunnel start-tunnel-install start-core-ts-bindings help: @echo "No default target — specify one. Common targets:" @@ -23,6 +24,7 @@ help: @echo " start-cli start-cli-deb start-registry start-tunnel start-wrt start-wrt-image (other products)" @echo " test start-core-test start-sdk-test container-runtime-test start-wrt-test (tests)" @echo " format format-check start-core-ts-bindings clean (tooling)" + @echo " complexity complexity-top complexity-diff (complexity)" @echo "See CONTRIBUTING.md for the full list." touch: diff --git a/build/complexity.mk b/build/complexity.mk new file mode 100644 index 0000000000..0395b36e3a --- /dev/null +++ b/build/complexity.mk @@ -0,0 +1,21 @@ +# --- cognitive-complexity census (no build; a tree-sitter pass over the sources) --- +COMPLEXITY := ./build/complexity/census.sh +BASE ?= origin/master + +.PHONY: complexity complexity-top complexity-diff complexity-check + +# Totals plus the worst 25 functions in the tree. +complexity: + @$(COMPLEXITY) census + +# The standing pay-down list. +complexity-top: + @$(COMPLEXITY) top + +# What this branch did to the numbers, against its merge-base. Paste into the PR body. +complexity-diff: + @$(COMPLEXITY) diff $(BASE) + +# Fail when a PR body's pasted block is missing, unanswered, or stale. +complexity-check: + @$(COMPLEXITY) check "$(PR_BODY_FILE)" $(BASE) diff --git a/build/complexity/.gitignore b/build/complexity/.gitignore new file mode 100644 index 0000000000..90af501d71 --- /dev/null +++ b/build/complexity/.gitignore @@ -0,0 +1,2 @@ +# Fetched by fetch-tool.sh, pinned by sha256 — never committed. +bin/ diff --git a/build/complexity/census.py b/build/complexity/census.py new file mode 100644 index 0000000000..647bc3e0b9 --- /dev/null +++ b/build/complexity/census.py @@ -0,0 +1,125 @@ +#!/usr/bin/env python3 +"""Cognitive-complexity census over the tree. Emits JSON, or a table.""" +import argparse, json, os, re, subprocess, sys, tempfile + +CFG_TEST = re.compile(r'#\[cfg\(test\)\]') +EXCLUDE = ('/node_modules/', '/dist/', '/target/', '/.angular/', '/out-tsc/', + '/osBindings/', '/locales/', '/__snapshots__/', '/__fixtures__/', + '/patch-db/client/', '/exver/exver.ts') + +def strip_rust_tests(src): + """Removes `#[cfg(test)]`-gated items by brace matching. Inline tests are never measured.""" + out, i, n = [], 0, len(src) + while i < n: + m = CFG_TEST.search(src, i) + if not m: + out.append(src[i:]); break + out.append(src[i:m.start()]) + j = src.find('{', m.end()) + if j < 0: break + depth, k = 0, j + s = ch = cl = cb = False + while k < n: + c = src[k] + if cl: + if c == '\n': cl = False + elif cb: + if src.startswith('*/', k): cb = False; k += 1 + elif s: + if c == '\\': k += 1 + elif c == '"': s = False + elif ch: + if c == '\\': k += 1 + elif c == "'": ch = False + elif src.startswith('//', k): cl = True; k += 1 + elif src.startswith('/*', k): cb = True; k += 1 + elif c == '"': s = True + elif c == '{': depth += 1 + elif c == '}': + depth -= 1 + if depth == 0: k += 1; break + k += 1 + i = k + return ''.join(out) + +def sources(root, scopes): + for scope in scopes: + for dp, dns, fns in os.walk(os.path.join(root, scope)): + dns[:] = [d for d in dns if d not in + ('node_modules', 'target', 'dist', '.angular', 'out-tsc', 'osBindings', 'locales')] + for fn in fns: + if not fn.endswith(('.rs', '.ts')) or fn.endswith(('.spec.ts', '.d.ts')): + continue + p = os.path.join(dp, fn) + rel = os.path.relpath(p, root) + if any(x in '/' + rel for x in EXCLUDE): + continue + yield p, rel + +def collect(node, rel, out): + if node.get('kind') == 'function': + m = node.get('metrics', {}) + cog = m.get('cognitive', {}).get('sum') + if cog is not None: + name = node.get('name') or '' + if not name or os.sep in name: + name = '' + out.append({'file': rel, 'name': name, + 'line': node.get('start_line'), 'cognitive': int(cog), + 'cyclomatic': int(m.get('cyclomatic', {}).get('sum') or 0), + 'sloc': int(m.get('loc', {}).get('sloc') or 0)}) + for c in node.get('spaces', []): + collect(c, rel, out) + +def census(root, scopes, rca): + staged = tempfile.mkdtemp(prefix='cx-src-') + outdir = tempfile.mkdtemp(prefix='cx-json-') + files = 0 + for src, rel in sources(root, scopes): + try: text = open(src, encoding='utf-8', errors='replace').read() + except OSError: continue + if rel.endswith('.rs'): + text = strip_rust_tests(text) + dst = os.path.join(staged, rel) + os.makedirs(os.path.dirname(dst), exist_ok=True) + open(dst, 'w', encoding='utf-8').write(text) + files += 1 + subprocess.run([rca, '-m', '-p', staged, '-O', 'json', '-o', outdir], + check=True, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) + rows, parsed = [], 0 + for dp, _, fns in os.walk(outdir): + for fn in fns: + if not fn.endswith('.json'): continue + try: d = json.load(open(os.path.join(dp, fn))) + except Exception: continue + parsed += 1 + collect(d, os.path.relpath(d.get('name', ''), staged), rows) + # A file the parser cannot read yields no functions rather than an error. + if files and parsed / files < 0.98: + sys.exit(f"complexity: parser read {parsed} of {files} files — refusing to report a partial census") + return rows + +def totals(rows): + return {'functions': len(rows), + 'cognitive': sum(r['cognitive'] for r in rows), + 'sloc': sum(r['sloc'] for r in rows), + 'over25': sum(1 for r in rows if r['cognitive'] > 25)} + +def main(): + ap = argparse.ArgumentParser() + ap.add_argument('--root', default='.') + ap.add_argument('--rca', default=os.environ.get('RCA', 'rust-code-analysis-cli')) + ap.add_argument('--scope', action='append') + ap.add_argument('--json', action='store_true') + ap.add_argument('--top', type=int, default=25) + a = ap.parse_args() + rows = census(a.root, a.scope or ['shared-libs', 'projects'], a.rca) + if a.json: + json.dump({'totals': totals(rows), 'functions': rows}, sys.stdout, sort_keys=True) + return + t = totals(rows) + print(f"functions {t['functions']} cognitive {t['cognitive']} sloc {t['sloc']} over25 {t['over25']}") + for r in sorted(rows, key=lambda r: -r['cognitive'])[:a.top]: + print(f" {r['cognitive']:>5} {r['sloc']:>5} {r['name'][:34]:<34} {r['file']}:{r['line']}") + +main() diff --git a/build/complexity/census.sh b/build/complexity/census.sh new file mode 100755 index 0000000000..c27e7ba41f --- /dev/null +++ b/build/complexity/census.sh @@ -0,0 +1,30 @@ +#!/bin/bash +# Cognitive-complexity census. `census` prints the totals and the worst 25; `diff ` +# prints this branch's delta against its merge-base; `check ` fails when +# a PR body's block is absent, unanswered, or disagrees with a fresh run. +set -euo pipefail +HERE="$(cd "$(dirname "$0")" && pwd)" +export RCA="${RCA:-$HERE/bin/rust-code-analysis-cli}" + +[ -x "$RCA" ] || "$HERE/fetch-tool.sh" + +case "${1:-census}" in + census) python3 "$HERE/census.py" --root . ;; + top) python3 "$HERE/census.py" --root . --top "${2:-25}" ;; + diff) + base="${2:-origin/master}" + mb="$(git merge-base "$base" HEAD)" + tmp="$(mktemp -d)"; trap 'rm -rf "$tmp"' EXIT + git archive "$mb" | tar -x -C "$tmp" + python3 "$HERE/census.py" --root "$tmp" --json > "$tmp/.base.json" + python3 "$HERE/census.py" --root . --json > "$tmp/.head.json" + python3 "$HERE/delta.py" "$tmp/.base.json" "$tmp/.head.json" "$mb" + ;; + check) + body="$2"; base="${3:-origin/master}" + fresh="$(mktemp)"; trap 'rm -f "$fresh"' EXIT + "$0" diff "$base" > "$fresh" + python3 "$HERE/check.py" "$body" "$fresh" + ;; + *) echo "usage: census.sh {census|top [n]|diff |check }" >&2; exit 2 ;; +esac diff --git a/build/complexity/check.py b/build/complexity/check.py new file mode 100644 index 0000000000..ceae6c1836 --- /dev/null +++ b/build/complexity/check.py @@ -0,0 +1,39 @@ +#!/usr/bin/env python3 +"""Fails when a PR body's Complexity block is absent, unanswered, or disagrees with a fresh census.""" +import re, sys + +TOTALS = re.compile( + r'functions\s+(\d+)\s*->\s*(\d+).*?' + r'cognitive\s+(\d+)\s*->\s*(\d+).*?' + r'sloc\s+(\d+)\s*->\s*(\d+)', re.S) + +QUESTIONS = ( + ('simplest alternative', r'[Ss]implest alternative[^\n]*:[^\S\n]*(\S[^\n]*)'), + ('existing helper', r'[Ee]xisting helper[^\n]*:[^\S\n]*(\S[^\n]*)'), + ('over-25 justification', r'over 25[^\n]*:[^\S\n]*(\S[^\n]*)'), +) + +def main(body_path, fresh_path): + body = open(body_path, encoding='utf-8').read() + fresh = open(fresh_path, encoding='utf-8').read() + if '## Complexity' not in body: + sys.exit("PR body has no '## Complexity' section. Run `make complexity-diff` and paste it.") + actual = TOTALS.search(fresh) + claimed = TOTALS.search(body) + if not actual: + sys.exit("internal: could not parse the fresh census") + if not claimed: + sys.exit("PR body's Complexity section carries no `make complexity-diff` output.") + if claimed.groups() != actual.groups(): + c, a = claimed.groups(), actual.groups() + sys.exit("PR body's complexity numbers do not match a fresh run.\n" + f" body: functions {c[0]}->{c[1]} cognitive {c[2]}->{c[3]} sloc {c[4]}->{c[5]}\n" + f" fresh: functions {a[0]}->{a[1]} cognitive {a[2]}->{a[3]} sloc {a[4]}->{a[5]}\n" + "Re-run `make complexity-diff` and paste the current output.") + for label, pat in QUESTIONS: + m = re.search(pat, body) + if not m or len(m.group(1).strip()) < 12: + sys.exit(f"PR body's Complexity section leaves '{label}' unanswered.") + print("Complexity block present, current, and answered.") + +main(sys.argv[1], sys.argv[2]) diff --git a/build/complexity/delta.py b/build/complexity/delta.py new file mode 100644 index 0000000000..6b6d8911b9 --- /dev/null +++ b/build/complexity/delta.py @@ -0,0 +1,42 @@ +#!/usr/bin/env python3 +"""Prints the complexity delta between two census JSON dumps.""" +import json, sys + +base, head, ref = json.load(open(sys.argv[1])), json.load(open(sys.argv[2])), sys.argv[3] +key = lambda r: (r['file'], r['name'], r['sloc'] if r['name'] == '' else 0) +B = {key(r): r for r in base['functions']} +H = {key(r): r for r in head['functions']} +tb, th = base['totals'], head['totals'] + +print(f"Complexity vs {ref[:10]}") +for label, k in (('functions', 'functions'), ('cognitive', 'cognitive'), + ('sloc', 'sloc'), ('fns over 25', 'over25')): + print(f" {label:<12}{tb[k]:>7} -> {th[k]:>7} {th[k]-tb[k]:+d}") + +new = sorted((r for k, r in H.items() if k not in B), key=lambda r: -r['cognitive']) +big = [r for r in new if r['cognitive'] > 10] +if big: + print(f"\n new functions over cognitive 10 ({len(big)} of {len(new)} new):") + for r in big[:10]: + print(f" cog {r['cognitive']:>4} {r['name']} {r['file']}:{r['line']}") + +worse = sorted(((H[k], B[k]['cognitive']) for k in H + if k in B and H[k]['cognitive'] > B[k]['cognitive']), + key=lambda x: -(x[0]['cognitive'] - x[1])) +if worse: + print(f"\n existing functions made more complex ({len(worse)}):") + for r, old in worse[:10]: + flag = ' <-- already over 25' if old > 25 else (' <-- now over 25' if r['cognitive'] > 25 else '') + print(f" cog {old} -> {r['cognitive']} {r['name']} {r['file']}:{r['line']}{flag}") + +better = sorted(((H[k], B[k]['cognitive']) for k in H + if k in B and H[k]['cognitive'] < B[k]['cognitive']), + key=lambda x: x[0]['cognitive'] - x[1]) +if better: + print(f"\n simplified ({len(better)}):") + for r, old in better[:5]: + print(f" cog {old} -> {r['cognitive']} {r['name']} {r['file']}:{r['line']}") + +gone = [r for k, r in B.items() if k not in H] +if gone: + print(f"\n removed: {len(gone)} functions, {sum(r['cognitive'] for r in gone)} cognitive") diff --git a/build/complexity/fetch-tool.sh b/build/complexity/fetch-tool.sh new file mode 100755 index 0000000000..f3210705e6 --- /dev/null +++ b/build/complexity/fetch-tool.sh @@ -0,0 +1,17 @@ +#!/bin/bash +# Fetches the pinned rust-code-analysis binary. Upstream releases linux and windows only; +# every other platform builds it with `cargo install`. +set -euo pipefail +HERE="$(cd "$(dirname "$0")" && pwd)" +VERSION=v0.0.25 +SHA256=9ec2a217b8ff191e02dab5d5f2eee6158b63fd975c532b2c5d67c2e6c7249894 +mkdir -p "$HERE/bin" +if [ "$(uname -s)" = "Linux" ] && [ "$(uname -m)" = "x86_64" ]; then + url="https://github.com/mozilla/rust-code-analysis/releases/download/$VERSION/rust-code-analysis-linux-cli-x86_64.tar.gz" + tmp="$(mktemp -d)"; trap 'rm -rf "$tmp"' EXIT + curl -fsSL "$url" -o "$tmp/rca.tar.gz" + echo "$SHA256 $tmp/rca.tar.gz" | sha256sum -c - + tar -xzf "$tmp/rca.tar.gz" -C "$HERE/bin" +else + cargo install rust-code-analysis-cli --version "${VERSION#v}" --root "$HERE" +fi diff --git a/rfcs/complexity-budget.md b/rfcs/complexity-budget.md new file mode 100644 index 0000000000..9cf854a0d1 --- /dev/null +++ b/rfcs/complexity-budget.md @@ -0,0 +1,207 @@ +# Complexity budget + +A way to track complexity in this repo, and a protocol that makes an agent state and defend +what its change cost. Every number below was measured on `7e58f9d45`. + +## Why now + +Same code, path-normalized across three directory reorgs, so the monorepo consolidation is +excluded — this is the StartOS Rust backend plus the web UI and nothing else: + +| date | backend KB | web UI KB | sum | delta | +| ---------- | ---------: | --------: | ---: | ----: | +| 2025-03-01 | 1686 | 869 | 2555 | — | +| 2025-09-01 | 1779 | 956 | 2735 | +0% | +| 2025-12-01 | 2107 | 989 | 3096 | +13% | +| 2026-04-01 | 2334 | 1033 | 3367 | +3% | +| 2026-06-01 | 2513 | 1076 | 3589 | +6% | +| 2026-08-27 | 3382 | 1098 | 4480 | +24% | + +Fifteen months to 2026-06 added 40%. The eleven weeks after it added 25%. The growth is +organic — no new top-level module, file count 324 → 356 — and concentrated: `net` went +468 → 965 KB, `tunnel` 90 → 310 KB. Over the same window commit volume went from roughly +20/month to 130–300/month, and Helix became the third-largest author, 208 of about 1,100 +commits in twelve months. + +None of that argues against the work. It argues that nobody is asked what it costs. +Zero of 566 merged PR bodies contain the string `complexit`. + +## What gets measured + +**Cognitive complexity per function**, via `rust-code-analysis` (tree-sitter, real Rust and +TypeScript grammars). Not cyclomatic complexity, and the difference is the whole argument. + +Probing both against a synthetic file settles it: + +| construct | cyclomatic | cognitive | which is right | +| ---------------------- | ---------: | --------: | ------------------------------------- | +| 20-arm `match` | 21 | **1** | cognitive — a flat table is read once | +| generic `where` bounds | 1 | **0** | cognitive | +| five `?` operators | 6 | **0** | cognitive — `?` is not cognitive load | +| four-deep nested `if` | 5 | **10** | cognitive — nesting is superlinear | + +Cyclomatic complexity ranks `error.rs::as_str` — an 84-line enum-to-string `match` — as the +worst function in the repo. It is the least risky code we have. Cognitive complexity does not +rank it at all. Lizard, the obvious cheap alternative, is worse still on Rust: its reader +never counts match arms and does count every `?` and every `where`, so adopting it would push +authors away from idiomatic error propagation and toward giant match statements. It also drops +functions silently when a TypeScript object key is named `interface`. + +For TypeScript alone, eslint is a real alternative and cheaper than it looks — eslint 9 and +typescript-eslint are already bundled dependencies of `@start9labs/start-sdk`, with a flat +config at `projects/start-sdk/eslint.config.base.mjs` and a lint gate in `s9pk.mk`, so the +dependency is shipped, just never aimed at this repo's own sources. It lints 1,146 TS files in +2.3 s with no tsconfig and no parse errors. One binary covering both languages still wins here +(the two agree closely, Spearman 0.94), but if the TS half is ever split out, eslint is the +tool and `sonarjs/cognitive-complexity` is the rule. + +The census covers `shared-libs/` and `projects/` — the code that ships. Build tooling, +`scripts/`, CI and the repo docs are outside it, which is why this RFC's own branch reports a +delta of zero. + +Three deliberate exclusions: inline `#[cfg(test)]` items are stripped before measuring (this +repo has 1,150 `#[test]` functions and only 6,249 lines in dedicated test files, so a +path-based rule would fail and a PR adding good tests would read as adding complexity); +generated trees are skipped (`osBindings`, `locales`, `exver.ts`, `dist`, `target`); and the +census refuses to report at all if the parser reads under 98% of files, because grammar rot +is otherwise silent. Parse coverage today is 530/530 Rust files. + +## Baseline + +| scope | functions | total cognitive | p90 | p99 | max | over 25 | +| ----- | --------: | --------------: | --: | --: | --: | ---------: | +| Rust | 11,687 | 20,685 | 5 | 26 | 165 | 125 (1.1%) | +| TS | 5,169 | 8,820 | 4 | 22 | 152 | 33 (0.6%) | + +`> 25` is the actionable line: about 1% of functions, 158 repo-wide. + +Worst ten, which is the standing pay-down list: + +``` +165 update shared-libs/crates/start-core/src/net/net_controller.rs:358 +157 add_public_domain shared-libs/crates/start-core/src/net/host/address.rs:388 +157 shared-libs/crates/start-core/src/net/host/address.rs:404 +152 shared-libs/ts-modules/start-core/lib/exver/index.ts:207 +145 shared-libs/crates/start-core/src/net/host/address.rs:472 +144 update_addresses shared-libs/crates/start-core/src/net/host/mod.rs:141 +135 update_profile_ips_… projects/start-wrt/backend/ctrl/src/lan.rs:668 +128 set projects/start-wrt/backend/ctrl/src/published_ports.rs:849 +119 ipv6_set projects/start-wrt/backend/ctrl/src/lan.rs:395 + 97 cmp shared-libs/crates/jsonpath/src/select/expr_term.rs:21 +``` + +`net/host/address.rs` holds three of the top five — one function and two closures inside it. + +## The tooling + +`make complexity` (totals + worst 25), `make complexity-top` (pay-down list), and +`make complexity-diff` (this branch against its merge-base). No build; the census is a +tree-sitter pass, 0.5 s for the whole repo, and a full delta including the base checkout is +about 3 s. It lives in `build/complexity/`, matching `build/fmt/`. + +Real output, for the portmap gateway PR: + +``` +Complexity vs 93d0c3cc4e + functions 16017 -> 16856 +839 + cognitive 28504 -> 29505 +1001 + sloc 190229 -> 199347 +9118 + fns over 25 156 -> 158 +2 + + new functions over cognitive 10 (29 of 599 new): + cog 53 try_apply .../net/port_map/client.rs:677 + cog 33 desired_port_maps .../net/vhost.rs:592 + + existing functions made more complex (108): + cog 36 -> 62 poll_ip_info .../net/gateway.rs:2340 <-- already over 25 + cog 24 -> 39 gc_policy_routing .../net/gateway.rs:1487 <-- now over 25 + + simplified (52): + cog 212 -> 165 update .../net/net_controller.rs:358 + cog 42 -> 2 apply .../net/port_map/client.rs:658 +``` + +The aggregate is unremarkable, and that is the point. Measured across 30 real PRs, the +threshold _counts_ — how many functions sit over a line — moved on 0 of 30, and total +cognitive tracks the LOC delta closely enough (Spearman 0.82) that it is mostly line count +wearing a hat. Neither is a budget worth defending. + +What a reviewer wants is the third block: a function already at 36 went to 62, and another +crossed 25. That list moves on nearly every PR, is not derivable from the diff size, and is +the thing a human would have flagged by hand. The report is per-function for that reason, and +it credits the 52 functions this PR simplified for the same one. + +## Three numbers that are not line count + +Per-function complexity is the headline, but it correlates with diff size. Three cheap +measures carry signal that LOC does not, and all three are things an agent inflates without +noticing: + +- **Duplication.** 10.67% of lines repo-wide by union-of-ranges (`jscpd`, one 4.4 MB binary, + under a second). `rpc-toolkit` is worst at 39.2%. Copy-paste is the most common way an agent + adds volume without adding capability. +- **Unused dependencies.** `cargo-machete` finds **28 unused direct Cargo dependencies** today, + in 0.26 s and without compiling. Cargo.lock sits at 994 crates. +- **Public surface.** The count of exported items — 923 `pub fn` in start-core alone. Widening + an API is a permanent cost that no per-function metric registers. + +These are reported alongside the delta rather than gated, at least to start. + +## The protocol + +The confrontation belongs in the PR body, because that is the artifact an agent always +produces: helix-nine wrote a body on 188 of 188 merged PRs with a median of 2,850 characters, +while humans left 28 of 198 and 26 of 75 empty. + +Documentation alone is not enough. The closest measurable precedent is the "Label every PR" +rule — inlined, bolded, with the literal command — which landed eight days before this +snapshot and has 60% agent compliance. So the rule is paired with a check. + +`make complexity-diff`'s output goes under a `## Complexity` heading, followed by three +questions. CI recomputes the census from `base.sha..head.sha` and fails when the section is +missing, unanswered, or carries numbers that disagree. **Prose can be bluffed; a number CI +recomputes cannot.** That is the load-bearing part of the design — the agent cannot write the +block without having run the tool. + +The three questions, chosen because a weak answer is visible to a human: + +- the simplest alternative considered, and what breaks if we take it +- which existing helper was checked before adding a new one, by file +- for any function pushed over 25, why the branching is intrinsic to the requirement + +27% of merged PR bodies already volunteer a rejected alternative, so the hardest of the three +is culturally native here rather than an imposition. + +## What this deliberately does not do + +**It does not ratchet.** Features cost complexity. A ratchet on a shipping repo gets suspended +during the first release crunch and never comes back. The gate asks for a number and a reason, +not for the number to stay flat. + +**It does not reward shredding.** Splitting one clear function into six poorly-named ones +lowers per-function scores while making the code worse. Function count and total cognitive are +printed next to the per-function lines precisely so that reads as what it is: total flat, +count up. + +**It does not count tests**, so there is never a reason to thin one. + +Known limits, stated rather than hidden: Angular templates are invisible — 278 components use +inline `template:` backticks holding 831 control-flow constructs, and those sit inside string +literals that no per-function metric sees. `rust-code-analysis`'s last release is v0.0.25 from +January 2023, though its grammar parses 100% of this repo today and the coverage assertion +above is what catches it if that changes. And the length check on the three answers catches +laziness, not sophistry — a determined bluff still needs a human to catch it. + +## Open questions + +1. **Advisory or required?** There is no `required_status_checks` rule on `master` today, so a + red check does not literally block; the approval does. Nothing has merged red in the last + 30 PRs, so an advisory gate is honored in practice. Making it required is a one-line + ruleset change — worth doing, or not yet? +2. **Should the gate apply to humans, or only to agent-authored PRs?** As written it applies to + everyone, which is the honest version, but it lands hardest on the author who already writes + the longest PR bodies. +3. **Is `> 25` the right line?** It flags 1% of functions today. `> 15` would flag 2.4%. +4. **Pay-down.** 75% of the twenty densest files were touched in the last 200 commits, so + "pay some down when your change already puts you in one of these files" would fire often. + Standing rule, or left to judgement? From 01aadf28a486405c7d102caf1b927f9cef55e90f Mon Sep 17 00:00:00 2001 From: Helix <267227783+helix-nine@users.noreply.github.com> Date: Thu, 27 Aug 2026 01:43:28 +0000 Subject: [PATCH 02/17] fix(repo): guard the census on parse-error rate, not just file coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A file the parser fails on yields no functions rather than an error, so file coverage catches a grammar that stops reading a file entirely — but not one that degrades inside it. Counting ERROR nodes catches both. Six Rust files carry parse errors today, all on generic associated types, which stabilized in Rust 1.65 two months after the pinned grammar was cut: 240 of 2,553,851 nodes, 0.009%. The guard trips above 0.5%, fifty times that. --- build/complexity/census.py | 15 +++++++++++++++ rfcs/complexity-budget.md | 33 +++++++++++++++++++++++++++++---- 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/build/complexity/census.py b/build/complexity/census.py index 647bc3e0b9..cf75fc3432 100644 --- a/build/complexity/census.py +++ b/build/complexity/census.py @@ -71,6 +71,18 @@ def collect(node, rel, out): for c in node.get('spaces', []): collect(c, rel, out) +def error_ratio(rca, path): + """Share of AST nodes tree-sitter could not parse.""" + out = subprocess.run([rca, '-C', 'ERROR', '-p', path], + capture_output=True, text=True).stdout + total = found = 0 + for line in out.splitlines(): + digits = line.split(':')[-1].strip().replace(',', '') + if line.startswith('Total nodes'): total = int(digits or 0) + elif line.startswith('Found nodes'): found = int(digits or 0) + return (found / total) if total else 0.0 + + def census(root, scopes, rca): staged = tempfile.mkdtemp(prefix='cx-src-') outdir = tempfile.mkdtemp(prefix='cx-json-') @@ -97,6 +109,9 @@ def census(root, scopes, rca): # A file the parser cannot read yields no functions rather than an error. if files and parsed / files < 0.98: sys.exit(f"complexity: parser read {parsed} of {files} files — refusing to report a partial census") + bad = error_ratio(rca, staged) + if bad > 0.005: + sys.exit(f"complexity: {bad:.3%} of AST nodes are parse errors — the grammar has fallen behind the language") return rows def totals(rows): diff --git a/rfcs/complexity-budget.md b/rfcs/complexity-budget.md index 9cf854a0d1..cec8ba0a1f 100644 --- a/rfcs/complexity-budget.md +++ b/rfcs/complexity-budget.md @@ -63,8 +63,14 @@ Three deliberate exclusions: inline `#[cfg(test)]` items are stripped before mea repo has 1,150 `#[test]` functions and only 6,249 lines in dedicated test files, so a path-based rule would fail and a PR adding good tests would read as adding complexity); generated trees are skipped (`osBindings`, `locales`, `exver.ts`, `dist`, `target`); and the -census refuses to report at all if the parser reads under 98% of files, because grammar rot -is otherwise silent. Parse coverage today is 530/530 Rust files. +census refuses to report if the parser reads under 98% of files or if over 0.5% of AST nodes +are parse errors, because grammar rot is otherwise silent. + +Today every one of 530 Rust files yields metrics, and 240 of 2,553,851 AST nodes — 0.009% — +are parse errors. They cluster in six Rust files, all on generic associated types +(`type Extended<'ext> where Self: 'ext`), which stabilized in Rust 1.65 two months before the +pinned grammar was cut. That is the shape grammar rot takes, and it is what the second guard +watches: fifty times the current rate still passes. ## Baseline @@ -147,6 +153,24 @@ noticing: These are reported alongside the delta rather than gated, at least to start. +## Tracking over time, without the rebase tax + +`complexity-diff` computes its base at run time, so nothing is committed and nothing conflicts. +That gives review-time confrontation but no history. For history, the instinct is to commit a +small aggregate — and simulating real merges with `git merge-file` over 60 real code commits +says that is the worst option available: + +| committed artifact | conflict rate on a median-lifetime PR | +| --------------------------------------------- | ------------------------------------: | +| one-line totals | 75.4% | +| 15-line per-scope totals | 66.7% | +| sorted per-function watchlist, over threshold | **10.5%** | + +Every PR rewrites the same totals line; PRs rarely touch the same region of a sorted 234-line +list. Splitting per-scope buys exactly nothing — 48 conflicts either way, because every +conflict is intra-scope. So if we want a committed record, it should be the watchlist of +functions over the threshold, regenerated by the existing drift idiom, not a scoreboard. + ## The protocol The confrontation belongs in the PR body, because that is the artifact an agent always @@ -174,8 +198,9 @@ is culturally native here rather than an imposition. ## What this deliberately does not do -**It does not ratchet.** Features cost complexity. A ratchet on a shipping repo gets suspended -during the first release crunch and never comes back. The gate asks for a number and a reason, +**It does not ratchet.** Features cost complexity: summed cognitive rises on 42 of 60 real code +commits and falls on 5, so a hard ratchet would block two commits in three and be suspended +during the first release crunch, never to return. The gate asks for a number and a reason, not for the number to stay flat. **It does not reward shredding.** Splitting one clear function into six poorly-named ones From 78ff05ba13925388dbc76536576711e18e34d7d2 Mon Sep 17 00:00:00 2001 From: Helix <267227783+helix-nine@users.noreply.github.com> Date: Thu, 27 Aug 2026 02:00:15 +0000 Subject: [PATCH 03/17] fix(repo): close the shredding and one-call-site holes in the census MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An adversarial pass defeated the first version with the refactor an agent reaches for first. Cognitive complexity penalises nesting superlinearly, so extracting nested blocks lowers the total however bad the split: a deliberately worse six-way split threading loop state through `&mut` parameters takes total cognitive 26 -> 9. The claim that function count and total would expose it was simply wrong. Cyclomatic is near-additive and survives relocation — the same split takes it 10 -> 16. The census now reports both and names the pattern when cognitive falls while cyclomatic rises. Adds the one question in the rubric an author cannot bluff, because the census counts it: new functions with exactly one call site. That is the premature helper, the most common complexity defect in generated code; the portmap PR added 83. Counting is a single tokenizing pass, 0.8s for the whole repo. The two conditional questions are asked only when the census reports them. 61% of source PRs carry a near-zero delta against 19% that are substantial, and a section mandatory on all of them is a rubber stamp rather than a gate. Also reports lines inside `macro_rules!` bodies. The parser does not expand macros, so wrapping a body in one takes it from cognitive 15 to 0 — no fix inside this tool, but the volume is at least visible. --- build/complexity/census.py | 55 ++++++++++++++++++++++++++++++++------ build/complexity/check.py | 31 ++++++++++++++------- build/complexity/delta.py | 14 +++++++++- rfcs/complexity-budget.md | 47 ++++++++++++++++++++++++-------- 4 files changed, 118 insertions(+), 29 deletions(-) diff --git a/build/complexity/census.py b/build/complexity/census.py index cf75fc3432..64e9c61c56 100644 --- a/build/complexity/census.py +++ b/build/complexity/census.py @@ -1,8 +1,10 @@ #!/usr/bin/env python3 """Cognitive-complexity census over the tree. Emits JSON, or a table.""" -import argparse, json, os, re, subprocess, sys, tempfile +import argparse, collections, json, os, re, subprocess, sys, tempfile CFG_TEST = re.compile(r'#\[cfg\(test\)\]') +MACRO_RULES = re.compile(r'macro_rules!\s*\w+\s*\{') +CALL_SITE = re.compile(r'\b([A-Za-z_]\w*)\s*[(:<]') EXCLUDE = ('/node_modules/', '/dist/', '/target/', '/.angular/', '/out-tsc/', '/osBindings/', '/locales/', '/__snapshots__/', '/__fixtures__/', '/patch-db/client/', '/exver/exver.ts') @@ -42,6 +44,22 @@ def strip_rust_tests(src): i = k return ''.join(out) +def macro_body_lines(src): + """Lines inside `macro_rules!` bodies. Control flow there is invisible to the metrics.""" + total = 0 + for m in MACRO_RULES.finditer(src): + depth, k, n = 0, src.index('{', m.start()), len(src) + start = k + while k < n: + if src[k] == '{': depth += 1 + elif src[k] == '}': + depth -= 1 + if depth == 0: break + k += 1 + total += src.count('\n', start, k) + return total + + def sources(root, scopes): for scope in scopes: for dp, dns, fns in os.walk(os.path.join(root, scope)): @@ -86,12 +104,13 @@ def error_ratio(rca, path): def census(root, scopes, rca): staged = tempfile.mkdtemp(prefix='cx-src-') outdir = tempfile.mkdtemp(prefix='cx-json-') - files = 0 + files = macro_lines = 0 for src, rel in sources(root, scopes): try: text = open(src, encoding='utf-8', errors='replace').read() except OSError: continue if rel.endswith('.rs'): text = strip_rust_tests(text) + macro_lines += macro_body_lines(text) dst = os.path.join(staged, rel) os.makedirs(os.path.dirname(dst), exist_ok=True) open(dst, 'w', encoding='utf-8').write(text) @@ -112,12 +131,29 @@ def census(root, scopes, rca): bad = error_ratio(rca, staged) if bad > 0.005: sys.exit(f"complexity: {bad:.3%} of AST nodes are parse errors — the grammar has fallen behind the language") - return rows + return rows, macro_lines + +def count_callers(rows, root, scopes): + """Call sites for each function, counted across the tree. The definition is not one.""" + used = collections.Counter() + for src, _ in sources(root, scopes): + try: text = open(src, encoding='utf-8', errors='replace').read() + except OSError: continue + used.update(CALL_SITE.findall(text)) + defs = collections.Counter(r['name'] for r in rows) + for r in rows: + name = r['name'] + if name == '': + continue + r['callers'] = max(0, used[name] - defs[name]) + -def totals(rows): +def totals(rows, macro_lines): return {'functions': len(rows), 'cognitive': sum(r['cognitive'] for r in rows), + 'cyclomatic': sum(r['cyclomatic'] for r in rows), 'sloc': sum(r['sloc'] for r in rows), + 'macro_lines': macro_lines, 'over25': sum(1 for r in rows if r['cognitive'] > 25)} def main(): @@ -128,12 +164,15 @@ def main(): ap.add_argument('--json', action='store_true') ap.add_argument('--top', type=int, default=25) a = ap.parse_args() - rows = census(a.root, a.scope or ['shared-libs', 'projects'], a.rca) + scopes = a.scope or ['shared-libs', 'projects'] + rows, macro_lines = census(a.root, scopes, a.rca) if a.json: - json.dump({'totals': totals(rows), 'functions': rows}, sys.stdout, sort_keys=True) + count_callers(rows, a.root, scopes) + json.dump({'totals': totals(rows, macro_lines), 'functions': rows}, sys.stdout, sort_keys=True) return - t = totals(rows) - print(f"functions {t['functions']} cognitive {t['cognitive']} sloc {t['sloc']} over25 {t['over25']}") + t = totals(rows, macro_lines) + print(f"functions {t['functions']} cognitive {t['cognitive']} cyclomatic {t['cyclomatic']} " + f"sloc {t['sloc']} macro-lines {t['macro_lines']} over25 {t['over25']}") for r in sorted(rows, key=lambda r: -r['cognitive'])[:a.top]: print(f" {r['cognitive']:>5} {r['sloc']:>5} {r['name'][:34]:<34} {r['file']}:{r['line']}") diff --git a/build/complexity/check.py b/build/complexity/check.py index ceae6c1836..58fef56c99 100644 --- a/build/complexity/check.py +++ b/build/complexity/check.py @@ -5,21 +5,27 @@ TOTALS = re.compile( r'functions\s+(\d+)\s*->\s*(\d+).*?' r'cognitive\s+(\d+)\s*->\s*(\d+).*?' + r'cyclomatic\s+(\d+)\s*->\s*(\d+).*?' r'sloc\s+(\d+)\s*->\s*(\d+)', re.S) +# Each question is asked only when the census actually reported the thing it is about. QUESTIONS = ( - ('simplest alternative', r'[Ss]implest alternative[^\n]*:[^\S\n]*(\S[^\n]*)'), - ('existing helper', r'[Ee]xisting helper[^\n]*:[^\S\n]*(\S[^\n]*)'), - ('over-25 justification', r'over 25[^\n]*:[^\S\n]*(\S[^\n]*)'), + ('simplest alternative', r'[Ss]implest alternative[^\n]*:[^\S\n]*(\S[^\n]*)', None), + ('existing helper', r'[Ee]xisting helper[^\n]*:[^\S\n]*(\S[^\n]*)', None), + ('over-25 justification', r'over 25[^\n]*:[^\S\n]*(\S[^\n]*)', '<-- '), + ('single-call-site helpers', r'one call site[^\n]*:[^\S\n]*(\S[^\n]*)', + 'new functions with one call site ('), ) +NOT_APPLICABLE = {'none', 'none.', 'n/a', 'na', 'nothing', 'not applicable'} + + def main(body_path, fresh_path): body = open(body_path, encoding='utf-8').read() fresh = open(fresh_path, encoding='utf-8').read() if '## Complexity' not in body: sys.exit("PR body has no '## Complexity' section. Run `make complexity-diff` and paste it.") - actual = TOTALS.search(fresh) - claimed = TOTALS.search(body) + actual, claimed = TOTALS.search(fresh), TOTALS.search(body) if not actual: sys.exit("internal: could not parse the fresh census") if not claimed: @@ -27,13 +33,20 @@ def main(body_path, fresh_path): if claimed.groups() != actual.groups(): c, a = claimed.groups(), actual.groups() sys.exit("PR body's complexity numbers do not match a fresh run.\n" - f" body: functions {c[0]}->{c[1]} cognitive {c[2]}->{c[3]} sloc {c[4]}->{c[5]}\n" - f" fresh: functions {a[0]}->{a[1]} cognitive {a[2]}->{a[3]} sloc {a[4]}->{a[5]}\n" + f" body: functions {c[0]}->{c[1]} cognitive {c[2]}->{c[3]} cyclomatic {c[4]}->{c[5]}\n" + f" fresh: functions {a[0]}->{a[1]} cognitive {a[2]}->{a[3]} cyclomatic {a[4]}->{a[5]}\n" "Re-run `make complexity-diff` and paste the current output.") - for label, pat in QUESTIONS: + for label, pat, trigger in QUESTIONS: + raised = trigger is None or trigger in fresh m = re.search(pat, body) - if not m or len(m.group(1).strip()) < 12: + answer = m.group(1).strip() if m else '' + if not raised: + continue + if answer.lower() in NOT_APPLICABLE and trigger is not None: + sys.exit(f"PR body answers '{label}' with '{answer}', but the census reported it.") + if len(answer) < 12: sys.exit(f"PR body's Complexity section leaves '{label}' unanswered.") print("Complexity block present, current, and answered.") + main(sys.argv[1], sys.argv[2]) diff --git a/build/complexity/delta.py b/build/complexity/delta.py index 6b6d8911b9..da65172510 100644 --- a/build/complexity/delta.py +++ b/build/complexity/delta.py @@ -10,9 +10,14 @@ print(f"Complexity vs {ref[:10]}") for label, k in (('functions', 'functions'), ('cognitive', 'cognitive'), - ('sloc', 'sloc'), ('fns over 25', 'over25')): + ('cyclomatic', 'cyclomatic'), ('sloc', 'sloc'), + ('macro lines', 'macro_lines'), ('fns over 25', 'over25')): print(f" {label:<12}{tb[k]:>7} -> {th[k]:>7} {th[k]-tb[k]:+d}") +# Cognitive falls when a function is split, however badly; cyclomatic does not. +if th['cognitive'] < tb['cognitive'] and th['cyclomatic'] > tb['cyclomatic']: + print("\n cognitive fell while cyclomatic rose — branches were relocated, not removed") + new = sorted((r for k, r in H.items() if k not in B), key=lambda r: -r['cognitive']) big = [r for r in new if r['cognitive'] > 10] if big: @@ -40,3 +45,10 @@ gone = [r for k, r in B.items() if k not in H] if gone: print(f"\n removed: {len(gone)} functions, {sum(r['cognitive'] for r in gone)} cognitive") + +single = [r for r in new if r.get('callers') == 1 and r['name'] != ''] +if single: + print(f"\n new functions with one call site ({len(single)}) — each is an abstraction the diff does not yet reuse:") + for r in single[:10]: + print(f" {r['name']} {r['file']}:{r['line']}") + diff --git a/rfcs/complexity-budget.md b/rfcs/complexity-budget.md index cec8ba0a1f..a9507d1523 100644 --- a/rfcs/complexity-budget.md +++ b/rfcs/complexity-budget.md @@ -67,10 +67,19 @@ census refuses to report if the parser reads under 98% of files or if over 0.5% are parse errors, because grammar rot is otherwise silent. Today every one of 530 Rust files yields metrics, and 240 of 2,553,851 AST nodes — 0.009% — -are parse errors. They cluster in six Rust files, all on generic associated types -(`type Extended<'ext> where Self: 'ext`), which stabilized in Rust 1.65 two months before the -pinned grammar was cut. That is the shape grammar rot takes, and it is what the second guard -watches: fifty times the current rate still passes. +are parse errors. They cluster in six Rust files, on modern trait syntax the pinned grammar +predates: generic associated types (`type Extended<'ext> where Self: 'ext`) and `impl Trait` +in argument and return position. That is the shape grammar rot takes, and it is what the +second guard watches: fifty times the current rate still passes. + +**The parser does not expand macros, and that is a hole.** Wrapping a body in `macro_rules!` +takes it from cognitive 15 to **0** and cyclomatic 11 to 1 — measured, identical logic. The +repo already carries `macro_rules!`, so the evasion would read as native. There is no fix +inside this tool; the report therefore prints the total number of lines inside macro bodies +(368 today) so that moving code there is at least visible. Note clippy has the mirror defect +in the other direction — it measures macro-_expanded_ HIR, so with 2,871 `t!()` i18n call +sites its ranking tracks i18n density rather than code, correlating with a real cognitive +metric at Spearman 0.435 and sharing 9 of its top 50. ## Baseline @@ -179,7 +188,8 @@ while humans left 28 of 198 and 26 of 75 empty. Documentation alone is not enough. The closest measurable precedent is the "Label every PR" rule — inlined, bolded, with the literal command — which landed eight days before this -snapshot and has 60% agent compliance. So the rule is paired with a check. +snapshot and runs at 60% agent compliance in that window and 28% across the last 90 merged +PRs. So the rule is paired with a check. `make complexity-diff`'s output goes under a `## Complexity` heading, followed by three questions. CI recomputes the census from `base.sha..head.sha` and fails when the section is @@ -192,6 +202,15 @@ The three questions, chosen because a weak answer is visible to a human: - the simplest alternative considered, and what breaks if we take it - which existing helper was checked before adding a new one, by file - for any function pushed over 25, why the branching is intrinsic to the requirement +- for each new function with exactly one call site, why it earns its own name + +The last one is the only question in the set an author cannot bluff, because the census counts +the call sites itself. It also targets the most common real defect in agent-written code — the +helper extracted for a reuse that never arrives. The portmap PR added 83 of them. + +**The last two questions are asked only when the census reports them.** Requiring all four on +every PR would put a four-line section on the 61% of source PRs whose delta is near zero +against the 19% that are substantial, which is a rubber-stamping machine rather than a gate. 27% of merged PR bodies already volunteer a rejected alternative, so the hardest of the three is culturally native here rather than an imposition. @@ -203,10 +222,14 @@ commits and falls on 5, so a hard ratchet would block two commits in three and b during the first release crunch, never to return. The gate asks for a number and a reason, not for the number to stay flat. -**It does not reward shredding.** Splitting one clear function into six poorly-named ones -lowers per-function scores while making the code worse. Function count and total cognitive are -printed next to the per-function lines precisely so that reads as what it is: total flat, -count up. +**It reports cyclomatic next to cognitive, because cognitive alone rewards shredding.** This +was the first version's mistake. Cognitive complexity penalises nesting superlinearly, so +pulling nested blocks out into separate functions lowers the total however bad the split is. +Measured on a deliberately worse six-way split that threads loop state through `&mut` +parameters, total cognitive falls **26 → 9** while total cyclomatic rises **10 → 16**. +Cognitive ranks a single function; cyclomatic is close to additive and so survives relocation. +The report prints both and says so outright when cognitive falls while cyclomatic rises — +branches were moved, not removed. **It does not count tests**, so there is never a reason to thin one. @@ -214,8 +237,10 @@ Known limits, stated rather than hidden: Angular templates are invisible — 278 inline `template:` backticks holding 831 control-flow constructs, and those sit inside string literals that no per-function metric sees. `rust-code-analysis`'s last release is v0.0.25 from January 2023, though its grammar parses 100% of this repo today and the coverage assertion -above is what catches it if that changes. And the length check on the three answers catches -laziness, not sophistry — a determined bluff still needs a human to catch it. +above is what catches it if that changes. And the length check on the prose answers catches +laziness, not sophistry: an LLM writes a fluent post-hoc justification easily, so do not sell +the gate as catching bad reasoning. It catches an unexamined change. Only the numbers and the +call-site count are self-verifying. ## Open questions From 5e24c690f6f24b5f52e2da9732bf8b4d45b1442a Mon Sep 17 00:00:00 2001 From: Helix <267227783+helix-nine@users.noreply.github.com> Date: Thu, 27 Aug 2026 02:22:36 +0000 Subject: [PATCH 04/17] docs(repo): evaluate SonarQube, which this RFC should have opened with MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sonar defined cognitive complexity and rust-code-analysis implements their spec, so the metric was never the homegrown part. SonarQube Cloud is free for public repositories, covers Rust and TypeScript, computes the metrics itself rather than only importing Clippy, and adds duplication, a new-code quality gate and PR decoration — strictly better than a local census for measuring and tracking. Records the three things that decide whether it replaces this: Community Build analyzes the main branch only, so self-hosting gives no pre-merge gate; Sonar identifies test code by path while this repo keeps 1,150 test functions inline; and a quality gate reports rather than asks for a justification. If Sonar handles cfg(test) and ranks this repo sensibly, the census should be deleted and only the protocol kept. --- rfcs/complexity-budget.md | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/rfcs/complexity-budget.md b/rfcs/complexity-budget.md index a9507d1523..bdca3d9eca 100644 --- a/rfcs/complexity-budget.md +++ b/rfcs/complexity-budget.md @@ -81,6 +81,38 @@ in the other direction — it measures macro-_expanded_ HIR, so with 2,871 `t!() sites its ranking tracks i18n density rather than code, correlating with a real cognitive metric at Spearman 0.435 and sharing 9 of its top 50. +## Why not SonarQube + +SonarQube is the obvious off-the-shelf answer and this RFC should have opened with it. Sonar +defined cognitive complexity; `rust-code-analysis` implements their published spec, so the +metric here is theirs either way — a sequence of like operators costs 1 rather than 1 per +operator, verified against this build (a mixed sequence diverges: 3 where the spec says 2). + +What Sonar would give us, free: **SonarQube Cloud is free for public repositories**, and this +repo is one. Rust and TypeScript are both supported, it computes cognitive and cyclomatic +itself rather than only importing Clippy, and it adds duplication, a quality gate scoped to new +code, and inline pull-request decoration. That is the whole measure-and-track half, maintained +by the people who invented the metric, and it is strictly better than a local census for that +job. + +Three things decide whether it replaces this: + +- **Self-hosting does not get you the gate.** SonarQube Community Build analyzes the main + branch only — no branch or pull-request analysis, no decoration. Free and self-hosted gives + post-merge tracking; pre-merge confrontation exists only in the cloud tier. Whether a SaaS + belongs in the development loop of a self-sovereignty company is a question for the team, + not a technical one. +- **Inline tests are unmeasured.** Sonar identifies test code by path, and this repo keeps + 1,150 `#[test]` functions inline against 6,249 lines in dedicated test files. If the Rust + analyzer counts `#[cfg(test)]` items, a PR that adds tests reads as one that adds complexity. + The documentation does not say either way. This is answerable in an afternoon against a + throwaway project and has not been answered here. +- **A quality gate is not a justification.** Sonar reports; it does not make an author state + the number and defend it, and the questions in the protocol below are not a Sonar feature. + +The honest reading: if Sonar handles `#[cfg(test)]` and ranks this repo sensibly, the census in +this PR should be deleted and only the protocol kept. That test comes first. + ## Baseline | scope | functions | total cognitive | p90 | p99 | max | over 25 | From 52cbbd6207c9b99d7c3bf51e5b3fd34d21dc12e8 Mon Sep 17 00:00:00 2001 From: Helix <267227783+helix-nine@users.noreply.github.com> Date: Thu, 27 Aug 2026 02:30:50 +0000 Subject: [PATCH 05/17] fix(repo): score each space on its own, not on its children's total MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Diffing this census against SonarSource's reference implementation of the metric found it counting the file-level container as a function and folding every nested closure into its parent. `cognitive.sum` is an aggregate over nested spaces, not a function's own score. Taking `sum` minus the direct children's sums moves rank agreement with the reference from Spearman 0.822 to 0.954 and drops the repo total 29,505 -> 22,136, with functions over 25 going 158 -> 103. `add_public_domain` leaves the worst list entirely — it was absorbing two closures. `list_conffiles` now scores 70, matching an independent measurement of the same function. Records why anything is vendored at all: for TypeScript, SonarSource's own `eslint-plugin-sonarjs` runs locally in 1.4s with no server and should be preferred outright. For Rust no local implementation of the metric exists — Clippy measures macro-expanded HIR, lizard's Rust reader never counts match arms. SonarQube itself is the wrong shape: complexity needs no server, and its free self-hosted tier analyzes the main branch only, so it cannot gate a PR. --- build/complexity/census.py | 26 ++++++--- rfcs/complexity-budget.md | 105 ++++++++++++++++++++----------------- 2 files changed, 74 insertions(+), 57 deletions(-) diff --git a/build/complexity/census.py b/build/complexity/census.py index 64e9c61c56..5e41c60cef 100644 --- a/build/complexity/census.py +++ b/build/complexity/census.py @@ -74,20 +74,30 @@ def sources(root, scopes): continue yield p, rel -def collect(node, rel, out): - if node.get('kind') == 'function': - m = node.get('metrics', {}) - cog = m.get('cognitive', {}).get('sum') +def own(node, metric): + """A space's own score. The parser's `sum` folds in every nested space.""" + total = node.get('metrics', {}).get(metric, {}).get('sum') + if total is None: + return None + nested = sum((c.get('metrics', {}).get(metric, {}).get('sum') or 0) + for c in node.get('spaces', [])) + return max(0, int(total - nested)) + + +def collect(node, rel, out, is_root=True): + # The outermost space is the file, not a function. + if node.get('kind') == 'function' and not is_root: + cog = own(node, 'cognitive') if cog is not None: name = node.get('name') or '' if not name or os.sep in name: name = '' out.append({'file': rel, 'name': name, - 'line': node.get('start_line'), 'cognitive': int(cog), - 'cyclomatic': int(m.get('cyclomatic', {}).get('sum') or 0), - 'sloc': int(m.get('loc', {}).get('sloc') or 0)}) + 'line': node.get('start_line'), 'cognitive': cog, + 'cyclomatic': own(node, 'cyclomatic') or 0, + 'sloc': int(node.get('metrics', {}).get('loc', {}).get('sloc') or 0)}) for c in node.get('spaces', []): - collect(c, rel, out) + collect(c, rel, out, False) def error_ratio(rca, path): """Share of AST nodes tree-sitter could not parse.""" diff --git a/rfcs/complexity-budget.md b/rfcs/complexity-budget.md index bdca3d9eca..048b5413a4 100644 --- a/rfcs/complexity-budget.md +++ b/rfcs/complexity-budget.md @@ -81,63 +81,70 @@ in the other direction — it measures macro-_expanded_ HIR, so with 2,871 `t!() sites its ranking tracks i18n density rather than code, correlating with a real cognitive metric at Spearman 0.435 and sharing 9 of its top 50. -## Why not SonarQube - -SonarQube is the obvious off-the-shelf answer and this RFC should have opened with it. Sonar -defined cognitive complexity; `rust-code-analysis` implements their published spec, so the -metric here is theirs either way — a sequence of like operators costs 1 rather than 1 per -operator, verified against this build (a mixed sequence diverges: 3 where the spec says 2). - -What Sonar would give us, free: **SonarQube Cloud is free for public repositories**, and this -repo is one. Rust and TypeScript are both supported, it computes cognitive and cyclomatic -itself rather than only importing Clippy, and it adds duplication, a quality gate scoped to new -code, and inline pull-request decoration. That is the whole measure-and-track half, maintained -by the people who invented the metric, and it is strictly better than a local census for that -job. - -Three things decide whether it replaces this: - -- **Self-hosting does not get you the gate.** SonarQube Community Build analyzes the main - branch only — no branch or pull-request analysis, no decoration. Free and self-hosted gives - post-merge tracking; pre-merge confrontation exists only in the cloud tier. Whether a SaaS - belongs in the development loop of a self-sovereignty company is a question for the team, - not a technical one. -- **Inline tests are unmeasured.** Sonar identifies test code by path, and this repo keeps - 1,150 `#[test]` functions inline against 6,249 lines in dedicated test files. If the Rust - analyzer counts `#[cfg(test)]` items, a PR that adds tests reads as one that adds complexity. - The documentation does not say either way. This is answerable in an afternoon against a - throwaway project and has not been answered here. -- **A quality gate is not a justification.** Sonar reports; it does not make an author state - the number and defend it, and the questions in the protocol below are not a Sonar feature. - -The honest reading: if Sonar handles `#[cfg(test)]` and ranks this repo sensibly, the census in -this PR should be deleted and only the protocol kept. That test comes first. - -## Baseline - -| scope | functions | total cognitive | p90 | p99 | max | over 25 | -| ----- | --------: | --------------: | --: | --: | --: | ---------: | -| Rust | 11,687 | 20,685 | 5 | 26 | 165 | 125 (1.1%) | -| TS | 5,169 | 8,820 | 4 | 22 | 152 | 33 (0.6%) | - -`> 25` is the actionable line: about 1% of functions, 158 repo-wide. +## Why not an off-the-shelf tool + +Mostly we should, and this RFC should have opened with that. Sonar defined cognitive +complexity, and the metric here is theirs: verified against this build, a sequence of like +operators costs 1 rather than 1 per operator, which is their distinctive rule. + +**SonarQube is the wrong shape, though, and not because of the metric.** Complexity is a pure +function of the tree — it needs no server. Sonar runs one because it is a platform: history, +dashboard, quality profiles, PR decoration by webhook. The analysis itself already happens on +the runner. And the free self-hosted tier, Community Build, analyzes the **main branch only** — +no branch or pull-request analysis, no decoration — so self-hosting buys post-merge tracking and +no pre-merge gate. The tier that gates is the hosted one. Paying a SaaS to compute a number we +can compute in a second locally is the wrong trade for this repo. + +**For TypeScript, Sonar's own implementation runs locally.** `eslint-plugin-sonarjs` is +SonarSource's ESLint plugin from their SonarJS repository, and `sonarjs/cognitive-complexity` is +the reference implementation of the metric. Measured here: 457 files in **1.4 s**, no server, no +`tsconfig`, no type information. It ranks this repo's TypeScript at Spearman **0.954** against +the census below and reports totals **27% lower**. Where the reference implementation runs +locally, use it — the TypeScript half of any gate should be this plugin, not a third-party +reimplementation. + +**For Rust there is no local Sonar implementation**, and that is the whole reason anything is +vendored here. Of what exists: Clippy is official and local but its `cognitive_complexity` +measures macro-_expanded_ HIR, so against this repo's 2,871 `t!()` sites it ranks by i18n +density (Spearman 0.435, 9 of 50 top-50 shared); lizard's Rust reader never counts match arms +while counting every `?` and `where`; `scc` and `tokei` are per-file or have no complexity +metric at all. `rust-code-analysis` is Mozilla's, tree-sitter based, peer-reviewed in SoftwareX, +and the only local tool that computes cognitive complexity for Rust — with the caveat that its +last release is January 2023. + +**Comparing the two implementations found a bug in this one.** Reading `cognitive.sum` per +space counts the file-level container as a function and folds every nested closure into its +parent. Agreement with the reference implementation was Spearman 0.822; taking each space's own +score instead — `sum` minus its children — moves it to **0.954** and drops the repo total by +33%. `list_conffiles` now scores 70, matching an independent measurement of the same function. +That is the argument for the vetted tool, made concrete: a reimplementation is wrong in ways you +only find by diffing it against the reference. + +## Baseline## Baseline + +| scope | functions | total cognitive | p95 | p99 | max | over 25 | +| ----- | --------: | --------------: | --: | --: | --: | --------: | +| Rust | 11,686 | 16,210 | 7 | 23 | 150 | 88 (0.8%) | +| TS | 5,166 | 5,926 | 6 | 14 | 72 | 15 (0.3%) | + +`> 25` is the actionable line: under 1% of functions, 103 repo-wide. Worst ten, which is the standing pay-down list: ``` -165 update shared-libs/crates/start-core/src/net/net_controller.rs:358 -157 add_public_domain shared-libs/crates/start-core/src/net/host/address.rs:388 -157 shared-libs/crates/start-core/src/net/host/address.rs:404 -152 shared-libs/ts-modules/start-core/lib/exver/index.ts:207 -145 shared-libs/crates/start-core/src/net/host/address.rs:472 +150 update shared-libs/crates/start-core/src/net/net_controller.rs:358 144 update_addresses shared-libs/crates/start-core/src/net/host/mod.rs:141 135 update_profile_ips_… projects/start-wrt/backend/ctrl/src/lan.rs:668 -128 set projects/start-wrt/backend/ctrl/src/published_ports.rs:849 -119 ipv6_set projects/start-wrt/backend/ctrl/src/lan.rs:395 - 97 cmp shared-libs/crates/jsonpath/src/select/expr_term.rs:21 +125 shared-libs/crates/start-core/src/net/host/address.rs:472 +118 ipv6_set projects/start-wrt/backend/ctrl/src/lan.rs:395 + 98 set projects/start-wrt/backend/ctrl/src/published_ports.rs:849 + 97 rebase shared-libs/crates/patch-db/core/src/patch.rs:105 + 80 up shared-libs/crates/start-core/src/version/v0_4_0_alpha_20.rs:37 + 72 projects/start-sdk/lib/version/VersionGraph.ts:98 + 70 list_conffiles projects/start-wrt/backend/ctrl/src/setup.rs:249 ``` -`net/host/address.rs` holds three of the top five — one function and two closures inside it. +The closure at `net/host/address.rs:472` outranks every named function but four. ## The tooling From 404434f63201473e8ea6f9070d45f1609759497b Mon Sep 17 00:00:00 2001 From: Helix <267227783+helix-nine@users.noreply.github.com> Date: Thu, 27 Aug 2026 03:04:39 +0000 Subject: [PATCH 06/17] docs(repo): retract the eslint-plugin-sonarjs recommendation on licence grounds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sonar's language analyzers are under the Sonar Source-Available License v1, not an open-source licence — SonarJS, sonar-rust, sonar-python, sonar-java and sonar-dotnet all carry it; only the SonarQube platform is still LGPL-3.0. SSAL grants rights solely for a Non-competitive Purpose, which excludes "employing, using, or engaging artificial intelligence technology that is not part of the Program to ingest, interpret, analyze, train on, or interact with the data provided by the Program". An agent reading a complexity report is the case this work exists to serve, so the grant does not reach it. eslint-plugin-sonarjs is the trap: package.json still declares LGPL-3.0-only while the shipped LICENSE and every source header are SSAL v1, so a scanner reading package metadata passes it. --- rfcs/complexity-budget.md | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/rfcs/complexity-budget.md b/rfcs/complexity-budget.md index 048b5413a4..681142fd60 100644 --- a/rfcs/complexity-budget.md +++ b/rfcs/complexity-budget.md @@ -95,13 +95,18 @@ no branch or pull-request analysis, no decoration — so self-hosting buys post- no pre-merge gate. The tier that gates is the hosted one. Paying a SaaS to compute a number we can compute in a second locally is the wrong trade for this repo. -**For TypeScript, Sonar's own implementation runs locally.** `eslint-plugin-sonarjs` is -SonarSource's ESLint plugin from their SonarJS repository, and `sonarjs/cognitive-complexity` is -the reference implementation of the metric. Measured here: 457 files in **1.4 s**, no server, no -`tsconfig`, no type information. It ranks this repo's TypeScript at Spearman **0.954** against -the census below and reports totals **27% lower**. Where the reference implementation runs -locally, use it — the TypeScript half of any gate should be this plugin, not a third-party -reimplementation. +**Sonar's analyzers are not open source, and their licence excludes this use case.** Every +language analyzer — `SonarJS`, `sonar-rust`, `sonar-python`, `sonar-java`, `sonar-dotnet` — is +under the Sonar Source-Available License v1, whatever GitHub's "Other" badge implies; only the +SonarQube platform itself is still LGPL-3.0. SSAL grants rights solely "for any Non-competitive +Purpose", and that term excludes, verbatim, "(c) employing, using, or engaging artificial +intelligence technology that is not part of the Program to ingest, interpret, analyze, train on, +or interact with the data provided by the Program, or to engage with the Program in any manner." +An agent reading a complexity report is the case this project exists to serve, so the grant does +not cover it. `eslint-plugin-sonarjs` is the sharp edge: its `package.json` still declares +`LGPL-3.0-only` while the shipped `LICENSE` and every source header are SSAL v1, so a scanner +reading package metadata clears it. Sonar's analyzers are usable as a calibration oracle for a +one-off comparison; they are not usable in this pipeline. **For Rust there is no local Sonar implementation**, and that is the whole reason anything is vendored here. Of what exists: Clippy is official and local but its `cognitive_complexity` From 29311fa626a6e614a3798770a7b6d9a8d1d1d9b3 Mon Sep 17 00:00:00 2001 From: Helix <267227783+helix-nine@users.noreply.github.com> Date: Thu, 27 Aug 2026 16:29:22 +0000 Subject: [PATCH 07/17] =?UTF-8?q?docs(repo):=20correct=20the=20SSAL=20anal?= =?UTF-8?q?ysis=20=E2=80=94=20clause=20(c)=20is=20not=20a=20blocker=20here?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The earlier retraction read clause (c) bare and got it wrong. SonarSource's own MCP server hands analyzer output to third-party LLM agents, ships under byte-identical SSAL v1.0, and carries a sentence added by their VP Legal in a pull request titled "Clarify SSAL language with regards to MCP usage": using it is a Non-Competitive Purpose and so allowed. That is a construction of a defined term, not a waiver, and the term is defined by purpose rather than by product. Records the honest weak point: no analyzer repository carries that sentence, and the 2024 announcement glosses (c) broadly and was never retracted — so the narrow reading rests on the licensor's conduct and construction rather than the text. Keeps the tool recommendation where it was, now on technical grounds alone. --- rfcs/complexity-budget.md | 38 ++++++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/rfcs/complexity-budget.md b/rfcs/complexity-budget.md index 681142fd60..4221ad672b 100644 --- a/rfcs/complexity-budget.md +++ b/rfcs/complexity-budget.md @@ -95,18 +95,32 @@ no branch or pull-request analysis, no decoration — so self-hosting buys post- no pre-merge gate. The tier that gates is the hosted one. Paying a SaaS to compute a number we can compute in a second locally is the wrong trade for this repo. -**Sonar's analyzers are not open source, and their licence excludes this use case.** Every -language analyzer — `SonarJS`, `sonar-rust`, `sonar-python`, `sonar-java`, `sonar-dotnet` — is -under the Sonar Source-Available License v1, whatever GitHub's "Other" badge implies; only the -SonarQube platform itself is still LGPL-3.0. SSAL grants rights solely "for any Non-competitive -Purpose", and that term excludes, verbatim, "(c) employing, using, or engaging artificial -intelligence technology that is not part of the Program to ingest, interpret, analyze, train on, -or interact with the data provided by the Program, or to engage with the Program in any manner." -An agent reading a complexity report is the case this project exists to serve, so the grant does -not cover it. `eslint-plugin-sonarjs` is the sharp edge: its `package.json` still declares -`LGPL-3.0-only` while the shipped `LICENSE` and every source header are SSAL v1, so a scanner -reading package metadata clears it. Sonar's analyzers are usable as a calibration oracle for a -one-off comparison; they are not usable in this pipeline. +**Sonar's analyzers are not open source, but their licence does not block this.** Every language +analyzer — `SonarJS`, `sonar-rust`, `sonar-python`, `sonar-java`, `sonar-dotnet` — is under the +Sonar Source-Available License v1; only the SonarQube platform is still LGPL-3.0. SSAL grants +rights solely "for any Non-competitive Purpose", and that term excludes "(c) employing, using, or +engaging artificial intelligence technology that is not part of the Program to ingest, interpret, +analyze, train on, or interact with the data provided by the Program". Read bare, that captures an +agent reading a complexity report. + +It is not read bare. SonarSource's own MCP server exists to hand analyzer output to third-party +LLM agents, ships under byte-identical SSAL v1.0, and carries a sentence its VP Legal added in a +pull request titled "Clarify SSAL language with regards to MCP usage": "Using the SonarQube MCP +Server in compliance with this documentation is a Non-Competitive Purpose and so is allowed under +the SSAL." That is a declaratory construction of a defined term rather than an additional +permission, and "Non-competitive Purpose" is defined by purpose, not by product, so it reads +across. The honest caveat is that no analyzer repository carries the same sentence, and the +licensor's 2024 announcement glosses (c) broadly and has never been retracted — so the narrow +reading rests on the licensor's later conduct and construction, not on the text. + +Two things would change that answer and neither applies here: redistributing anything containing +an analyzer triggers the source-availability duty in §3.1, and shipping a code-quality product of +our own would engage (a) and (b) directly. + +`eslint-plugin-sonarjs` still carries a metadata defect worth knowing about: `package.json` +declares `LGPL-3.0-only` while the shipped `LICENSE` and every source header are SSAL v1, so an +SBOM built from package metadata asserts the wrong licence. We gate Cargo licences with +`cargo-deny` and gate npm licences not at all. **For Rust there is no local Sonar implementation**, and that is the whole reason anything is vendored here. Of what exists: Clippy is official and local but its `cognitive_complexity` From 9083b11a447345b50b4e984171f98f93d1a37619 Mon Sep 17 00:00:00 2001 From: Helix <267227783+helix-nine@users.noreply.github.com> Date: Thu, 27 Aug 2026 16:54:53 +0000 Subject: [PATCH 08/17] feat(repo): drive the census with big-code-analysis MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the vendored rust-code-analysis wrapper with `bca`, a maintained fork of the same engine under MPL-2.0, already on the deny.toml allowlist. One binary and one pass covers Rust and TypeScript in 0.35s. Three of its flags delete code this repo was carrying itself. `--exclude-tests` skips #[test]/#[cfg(test)]/#[tokio::test]/#[rstest] subtrees and reproduces the hand-rolled brace-matching stripper exactly — 24 functions and cognitive 66 on volume.rs either way — so the stripper goes. `cognitive.value` is a space's own score, so the sum-minus-children arithmetic goes too. A generated-code detector and sha256-pinned multi-platform releases come with it. Drops the macro-body line counter. A body inside macro_rules! scores zero in every tool including SonarSource's own analyzer, and macro bodies are 0.19% of this repo's Rust — a bespoke counter for that is not worth its own code path. Not Sonar, and not on licence grounds: their analyzers are source-available rather than open source, the platform needs a server, and its free self-hosted tier analyzes the main branch only, so it cannot gate a pull request. --- build/complexity/census.py | 215 ++++++++++----------------------- build/complexity/census.sh | 4 +- build/complexity/delta.py | 2 +- build/complexity/fetch-tool.sh | 27 +++-- rfcs/complexity-budget.md | 118 ++++++++---------- 5 files changed, 132 insertions(+), 234 deletions(-) diff --git a/build/complexity/census.py b/build/complexity/census.py index 5e41c60cef..8ed83b5698 100644 --- a/build/complexity/census.py +++ b/build/complexity/census.py @@ -1,189 +1,106 @@ #!/usr/bin/env python3 -"""Cognitive-complexity census over the tree. Emits JSON, or a table.""" +"""Per-function cognitive complexity for the tree, as JSON or a table.""" import argparse, collections, json, os, re, subprocess, sys, tempfile -CFG_TEST = re.compile(r'#\[cfg\(test\)\]') -MACRO_RULES = re.compile(r'macro_rules!\s*\w+\s*\{') -CALL_SITE = re.compile(r'\b([A-Za-z_]\w*)\s*[(:<]') +SCOPES = ['shared-libs', 'projects'] EXCLUDE = ('/node_modules/', '/dist/', '/target/', '/.angular/', '/out-tsc/', '/osBindings/', '/locales/', '/__snapshots__/', '/__fixtures__/', '/patch-db/client/', '/exver/exver.ts') +CALL_SITE = re.compile(r'\b([A-Za-z_]\w*)\s*[(:<]') -def strip_rust_tests(src): - """Removes `#[cfg(test)]`-gated items by brace matching. Inline tests are never measured.""" - out, i, n = [], 0, len(src) - while i < n: - m = CFG_TEST.search(src, i) - if not m: - out.append(src[i:]); break - out.append(src[i:m.start()]) - j = src.find('{', m.end()) - if j < 0: break - depth, k = 0, j - s = ch = cl = cb = False - while k < n: - c = src[k] - if cl: - if c == '\n': cl = False - elif cb: - if src.startswith('*/', k): cb = False; k += 1 - elif s: - if c == '\\': k += 1 - elif c == '"': s = False - elif ch: - if c == '\\': k += 1 - elif c == "'": ch = False - elif src.startswith('//', k): cl = True; k += 1 - elif src.startswith('/*', k): cb = True; k += 1 - elif c == '"': s = True - elif c == '{': depth += 1 - elif c == '}': - depth -= 1 - if depth == 0: k += 1; break - k += 1 - i = k - return ''.join(out) - -def macro_body_lines(src): - """Lines inside `macro_rules!` bodies. Control flow there is invisible to the metrics.""" - total = 0 - for m in MACRO_RULES.finditer(src): - depth, k, n = 0, src.index('{', m.start()), len(src) - start = k - while k < n: - if src[k] == '{': depth += 1 - elif src[k] == '}': - depth -= 1 - if depth == 0: break - k += 1 - total += src.count('\n', start, k) - return total - - -def sources(root, scopes): - for scope in scopes: - for dp, dns, fns in os.walk(os.path.join(root, scope)): - dns[:] = [d for d in dns if d not in - ('node_modules', 'target', 'dist', '.angular', 'out-tsc', 'osBindings', 'locales')] - for fn in fns: - if not fn.endswith(('.rs', '.ts')) or fn.endswith(('.spec.ts', '.d.ts')): - continue - p = os.path.join(dp, fn) - rel = os.path.relpath(p, root) - if any(x in '/' + rel for x in EXCLUDE): - continue - yield p, rel - -def own(node, metric): - """A space's own score. The parser's `sum` folds in every nested space.""" - total = node.get('metrics', {}).get(metric, {}).get('sum') - if total is None: - return None - nested = sum((c.get('metrics', {}).get(metric, {}).get('sum') or 0) - for c in node.get('spaces', [])) - return max(0, int(total - nested)) +def kept(path): + return (path.endswith(('.rs', '.ts', '.js')) + and not path.endswith(('.spec.ts', '.d.ts')) + and not any(x in '/' + path for x in EXCLUDE)) -def collect(node, rel, out, is_root=True): - # The outermost space is the file, not a function. - if node.get('kind') == 'function' and not is_root: - cog = own(node, 'cognitive') - if cog is not None: - name = node.get('name') or '' - if not name or os.sep in name: - name = '' - out.append({'file': rel, 'name': name, - 'line': node.get('start_line'), 'cognitive': cog, - 'cyclomatic': own(node, 'cyclomatic') or 0, - 'sloc': int(node.get('metrics', {}).get('loc', {}).get('sloc') or 0)}) - for c in node.get('spaces', []): - collect(c, rel, out, False) -def error_ratio(rca, path): - """Share of AST nodes tree-sitter could not parse.""" - out = subprocess.run([rca, '-C', 'ERROR', '-p', path], - capture_output=True, text=True).stdout - total = found = 0 - for line in out.splitlines(): - digits = line.split(':')[-1].strip().replace(',', '') - if line.startswith('Total nodes'): total = int(digits or 0) - elif line.startswith('Found nodes'): found = int(digits or 0) - return (found / total) if total else 0.0 +def spaces(node, root=True): + if node.get('kind') == 'function' and not root: + yield node + for child in node.get('spaces', []): + yield from spaces(child, False) -def census(root, scopes, rca): - staged = tempfile.mkdtemp(prefix='cx-src-') - outdir = tempfile.mkdtemp(prefix='cx-json-') - files = macro_lines = 0 - for src, rel in sources(root, scopes): - try: text = open(src, encoding='utf-8', errors='replace').read() - except OSError: continue - if rel.endswith('.rs'): - text = strip_rust_tests(text) - macro_lines += macro_body_lines(text) - dst = os.path.join(staged, rel) - os.makedirs(os.path.dirname(dst), exist_ok=True) - open(dst, 'w', encoding='utf-8').write(text) - files += 1 - subprocess.run([rca, '-m', '-p', staged, '-O', 'json', '-o', outdir], +def census(root, scopes, bca): + out = tempfile.mkdtemp(prefix='cx-') + subprocess.run([bca, 'metrics', '-O', 'json', '--exclude-tests', '--output-dir', out, + *(os.path.join(root, s) for s in scopes)], check=True, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) - rows, parsed = [], 0 - for dp, _, fns in os.walk(outdir): - for fn in fns: - if not fn.endswith('.json'): continue - try: d = json.load(open(os.path.join(dp, fn))) - except Exception: continue - parsed += 1 - collect(d, os.path.relpath(d.get('name', ''), staged), rows) - # A file the parser cannot read yields no functions rather than an error. - if files and parsed / files < 0.98: - sys.exit(f"complexity: parser read {parsed} of {files} files — refusing to report a partial census") - bad = error_ratio(rca, staged) - if bad > 0.005: - sys.exit(f"complexity: {bad:.3%} of AST nodes are parse errors — the grammar has fallen behind the language") - return rows, macro_lines + rows = [] + for dirpath, _, names in os.walk(out): + for name in names: + if not name.endswith('.json'): + continue + try: + doc = json.load(open(os.path.join(dirpath, name))) + except (OSError, ValueError): + continue + rel = os.path.relpath(doc.get('name', ''), root) + if not kept(rel): + continue + for fn in spaces(doc): + m = fn.get('metrics', {}) + label = fn.get('name') or '' + rows.append({ + 'file': rel, + 'name': '' if not label or os.sep in label else label, + 'line': fn.get('start_line'), + 'cognitive': int(m.get('cognitive', {}).get('value') or 0), + 'cyclomatic': int(m.get('cyclomatic', {}).get('value') or 0), + 'sloc': int(m.get('loc', {}).get('sloc') or 0), + }) + return rows + def count_callers(rows, root, scopes): - """Call sites for each function, counted across the tree. The definition is not one.""" + """Call sites per function name across the tree. A definition is not a call site.""" used = collections.Counter() - for src, _ in sources(root, scopes): - try: text = open(src, encoding='utf-8', errors='replace').read() - except OSError: continue - used.update(CALL_SITE.findall(text)) - defs = collections.Counter(r['name'] for r in rows) + for scope in scopes: + for dirpath, dirs, names in os.walk(os.path.join(root, scope)): + dirs[:] = [d for d in dirs if d not in + ('node_modules', 'target', 'dist', '.angular', 'out-tsc', 'osBindings', 'locales')] + for name in names: + path = os.path.join(dirpath, name) + if not kept(os.path.relpath(path, root)): + continue + try: + used.update(CALL_SITE.findall(open(path, encoding='utf-8', errors='replace').read())) + except OSError: + pass + defined = collections.Counter(r['name'] for r in rows) for r in rows: - name = r['name'] - if name == '': - continue - r['callers'] = max(0, used[name] - defs[name]) + if r['name'] != '': + r['callers'] = max(0, used[r['name']] - defined[r['name']]) -def totals(rows, macro_lines): +def totals(rows): return {'functions': len(rows), 'cognitive': sum(r['cognitive'] for r in rows), 'cyclomatic': sum(r['cyclomatic'] for r in rows), 'sloc': sum(r['sloc'] for r in rows), - 'macro_lines': macro_lines, 'over25': sum(1 for r in rows if r['cognitive'] > 25)} + def main(): ap = argparse.ArgumentParser() ap.add_argument('--root', default='.') - ap.add_argument('--rca', default=os.environ.get('RCA', 'rust-code-analysis-cli')) + ap.add_argument('--bca', default=os.environ.get('BCA', 'bca')) ap.add_argument('--scope', action='append') ap.add_argument('--json', action='store_true') ap.add_argument('--top', type=int, default=25) a = ap.parse_args() - scopes = a.scope or ['shared-libs', 'projects'] - rows, macro_lines = census(a.root, scopes, a.rca) + scopes = a.scope or SCOPES + rows = census(a.root, scopes, a.bca) if a.json: count_callers(rows, a.root, scopes) - json.dump({'totals': totals(rows, macro_lines), 'functions': rows}, sys.stdout, sort_keys=True) + json.dump({'totals': totals(rows), 'functions': rows}, sys.stdout, sort_keys=True) return - t = totals(rows, macro_lines) + t = totals(rows) print(f"functions {t['functions']} cognitive {t['cognitive']} cyclomatic {t['cyclomatic']} " - f"sloc {t['sloc']} macro-lines {t['macro_lines']} over25 {t['over25']}") + f"sloc {t['sloc']} over25 {t['over25']}") for r in sorted(rows, key=lambda r: -r['cognitive'])[:a.top]: print(f" {r['cognitive']:>5} {r['sloc']:>5} {r['name'][:34]:<34} {r['file']}:{r['line']}") + main() diff --git a/build/complexity/census.sh b/build/complexity/census.sh index c27e7ba41f..5ed383fde4 100755 --- a/build/complexity/census.sh +++ b/build/complexity/census.sh @@ -4,9 +4,9 @@ # a PR body's block is absent, unanswered, or disagrees with a fresh run. set -euo pipefail HERE="$(cd "$(dirname "$0")" && pwd)" -export RCA="${RCA:-$HERE/bin/rust-code-analysis-cli}" +export BCA="${BCA:-$HERE/bin/bca}" -[ -x "$RCA" ] || "$HERE/fetch-tool.sh" +[ -x "$BCA" ] || "$HERE/fetch-tool.sh" case "${1:-census}" in census) python3 "$HERE/census.py" --root . ;; diff --git a/build/complexity/delta.py b/build/complexity/delta.py index da65172510..975707edc2 100644 --- a/build/complexity/delta.py +++ b/build/complexity/delta.py @@ -11,7 +11,7 @@ print(f"Complexity vs {ref[:10]}") for label, k in (('functions', 'functions'), ('cognitive', 'cognitive'), ('cyclomatic', 'cyclomatic'), ('sloc', 'sloc'), - ('macro lines', 'macro_lines'), ('fns over 25', 'over25')): + ('fns over 25', 'over25')): print(f" {label:<12}{tb[k]:>7} -> {th[k]:>7} {th[k]-tb[k]:+d}") # Cognitive falls when a function is split, however badly; cyclomatic does not. diff --git a/build/complexity/fetch-tool.sh b/build/complexity/fetch-tool.sh index f3210705e6..f1465a767b 100755 --- a/build/complexity/fetch-tool.sh +++ b/build/complexity/fetch-tool.sh @@ -1,17 +1,18 @@ #!/bin/bash -# Fetches the pinned rust-code-analysis binary. Upstream releases linux and windows only; -# every other platform builds it with `cargo install`. +# Fetches the pinned big-code-analysis binary and verifies it against the release checksum. set -euo pipefail HERE="$(cd "$(dirname "$0")" && pwd)" -VERSION=v0.0.25 -SHA256=9ec2a217b8ff191e02dab5d5f2eee6158b63fd975c532b2c5d67c2e6c7249894 +VERSION=2.1.0 +case "$(uname -s)-$(uname -m)" in + Linux-x86_64) TRIPLE=x86_64-unknown-linux-gnu; SHA256=6904518ff57968408dd3fa46a3fb533b8ac42cd035d5dd503090e24e19d5232a ;; + Linux-aarch64) TRIPLE=aarch64-unknown-linux-gnu; SHA256=6400d71fb8b436ee71a984a605172680eacf3ad9d4fb2046e24d2d1972f669d0 ;; + Darwin-arm64) TRIPLE=aarch64-apple-darwin; SHA256=94faaa8f6f20952147e263222df4f65a11c8994af1da2e9d7882b3caae598212 ;; + *) echo "complexity: no pinned bca build for $(uname -s)-$(uname -m); build it with 'cargo install big-code-analysis --version $VERSION --root $HERE'" >&2; exit 1 ;; +esac +url="https://github.com/dekobon/big-code-analysis/releases/download/v$VERSION/big-code-analysis-$VERSION-$TRIPLE.tar.gz" +tmp="$(mktemp -d)"; trap 'rm -rf "$tmp"' EXIT +curl -fsSL "$url" -o "$tmp/bca.tar.gz" +echo "$SHA256 $tmp/bca.tar.gz" | sha256sum -c - >/dev/null +tar -xzf "$tmp/bca.tar.gz" -C "$tmp" mkdir -p "$HERE/bin" -if [ "$(uname -s)" = "Linux" ] && [ "$(uname -m)" = "x86_64" ]; then - url="https://github.com/mozilla/rust-code-analysis/releases/download/$VERSION/rust-code-analysis-linux-cli-x86_64.tar.gz" - tmp="$(mktemp -d)"; trap 'rm -rf "$tmp"' EXIT - curl -fsSL "$url" -o "$tmp/rca.tar.gz" - echo "$SHA256 $tmp/rca.tar.gz" | sha256sum -c - - tar -xzf "$tmp/rca.tar.gz" -C "$HERE/bin" -else - cargo install rust-code-analysis-cli --version "${VERSION#v}" --root "$HERE" -fi +install -m 0755 "$tmp/big-code-analysis-$VERSION-$TRIPLE/bca" "$HERE/bin/bca" diff --git a/rfcs/complexity-budget.md b/rfcs/complexity-budget.md index 4221ad672b..891c51d0d8 100644 --- a/rfcs/complexity-budget.md +++ b/rfcs/complexity-budget.md @@ -81,86 +81,66 @@ in the other direction — it measures macro-_expanded_ HIR, so with 2,871 `t!() sites its ranking tracks i18n density rather than code, correlating with a real cognitive metric at Spearman 0.435 and sharing 9 of its top 50. -## Why not an off-the-shelf tool - -Mostly we should, and this RFC should have opened with that. Sonar defined cognitive -complexity, and the metric here is theirs: verified against this build, a sequence of like -operators costs 1 rather than 1 per operator, which is their distinctive rule. - -**SonarQube is the wrong shape, though, and not because of the metric.** Complexity is a pure -function of the tree — it needs no server. Sonar runs one because it is a platform: history, -dashboard, quality profiles, PR decoration by webhook. The analysis itself already happens on -the runner. And the free self-hosted tier, Community Build, analyzes the **main branch only** — -no branch or pull-request analysis, no decoration — so self-hosting buys post-merge tracking and -no pre-merge gate. The tier that gates is the hosted one. Paying a SaaS to compute a number we -can compute in a second locally is the wrong trade for this repo. - -**Sonar's analyzers are not open source, but their licence does not block this.** Every language -analyzer — `SonarJS`, `sonar-rust`, `sonar-python`, `sonar-java`, `sonar-dotnet` — is under the -Sonar Source-Available License v1; only the SonarQube platform is still LGPL-3.0. SSAL grants -rights solely "for any Non-competitive Purpose", and that term excludes "(c) employing, using, or -engaging artificial intelligence technology that is not part of the Program to ingest, interpret, -analyze, train on, or interact with the data provided by the Program". Read bare, that captures an -agent reading a complexity report. - -It is not read bare. SonarSource's own MCP server exists to hand analyzer output to third-party -LLM agents, ships under byte-identical SSAL v1.0, and carries a sentence its VP Legal added in a -pull request titled "Clarify SSAL language with regards to MCP usage": "Using the SonarQube MCP -Server in compliance with this documentation is a Non-Competitive Purpose and so is allowed under -the SSAL." That is a declaratory construction of a defined term rather than an additional -permission, and "Non-competitive Purpose" is defined by purpose, not by product, so it reads -across. The honest caveat is that no analyzer repository carries the same sentence, and the -licensor's 2024 announcement glosses (c) broadly and has never been retracted — so the narrow -reading rests on the licensor's later conduct and construction, not on the text. - -Two things would change that answer and neither applies here: redistributing anything containing -an analyzer triggers the source-availability duty in §3.1, and shipping a code-quality product of -our own would engage (a) and (b) directly. - -`eslint-plugin-sonarjs` still carries a metadata defect worth knowing about: `package.json` -declares `LGPL-3.0-only` while the shipped `LICENSE` and every source header are SSAL v1, so an -SBOM built from package metadata asserts the wrong licence. We gate Cargo licences with -`cargo-deny` and gate npm licences not at all. - -**For Rust there is no local Sonar implementation**, and that is the whole reason anything is -vendored here. Of what exists: Clippy is official and local but its `cognitive_complexity` -measures macro-_expanded_ HIR, so against this repo's 2,871 `t!()` sites it ranks by i18n -density (Spearman 0.435, 9 of 50 top-50 shared); lizard's Rust reader never counts match arms -while counting every `?` and `where`; `scc` and `tokei` are per-file or have no complexity -metric at all. `rust-code-analysis` is Mozilla's, tree-sitter based, peer-reviewed in SoftwareX, -and the only local tool that computes cognitive complexity for Rust — with the caveat that its -last release is January 2023. - -**Comparing the two implementations found a bug in this one.** Reading `cognitive.sum` per -space counts the file-level container as a function and folds every nested closure into its -parent. Agreement with the reference implementation was Spearman 0.822; taking each space's own -score instead — `sum` minus its children — moves it to **0.954** and drops the repo total by -33%. `list_conffiles` now scores 70, matching an independent measurement of the same function. -That is the argument for the vetted tool, made concrete: a reimplementation is wrong in ways you -only find by diffing it against the reference. - -## Baseline## Baseline - -| scope | functions | total cognitive | p95 | p99 | max | over 25 | -| ----- | --------: | --------------: | --: | --: | --: | --------: | -| Rust | 11,686 | 16,210 | 7 | 23 | 150 | 88 (0.8%) | -| TS | 5,166 | 5,926 | 6 | 14 | 72 | 15 (0.3%) | - -`> 25` is the actionable line: under 1% of functions, 103 repo-wide. +## The tool + +**`big-code-analysis` (`bca`)**, MPL-2.0 — a maintained fork of Mozilla's `rust-code-analysis`, +which is the only lineage that computes cognitive complexity locally for both Rust and +TypeScript. One binary, one pass, both languages: 527 Rust files and 759 TypeScript in 0.35 s +with no build and no `npm install`. It is pinned by sha256 against the upstream release +checksums and fetched into `build/complexity/bin/`, never committed; `cargo install` covers +platforms with no published binary. + +Three of its flags do work this repo would otherwise need bespoke code for. `--exclude-tests` +skips `#[test]`, `#[cfg(test)]`, `#[tokio::test]` and `#[rstest]` subtrees, which matters here +because tests are inline — 1,150 test functions against 6,249 lines in dedicated test files, so +a path rule would make a PR that adds tests read as one that adds complexity. `cognitive.value` +is a space's own score rather than `sum`, which folds in every nested closure. And +`--cyclomatic-count-try` decides whether Rust's `?` counts as a branch. + +**Not Sonar, and not on licence grounds.** Sonar defined cognitive complexity, their analyzers +implement it best, and their licence turns out not to block internal agentic use — SonarSource's +own MCP server hands analyzer output to third-party agents under byte-identical SSAL and carries +a clarification from their VP Legal that doing so is a Non-competitive Purpose. The reasons to +pass are simpler: the platform needs a server we do not want to run, the free self-hosted tier +analyzes the main branch only and so cannot gate a pull request at all, and the analyzers are +source-available rather than open source, where `bca` is MPL-2.0 and already on the `deny.toml` +allowlist. On our TypeScript `bca` tracks Sonar's own implementation at Spearman 0.951, and +Sonar's worst eight functions all land inside `bca`'s top twenty-five — immaterial for a +threshold gate. + +Rejected after measurement: Clippy's `cognitive_complexity` runs on macro-expanded HIR, so +against this repo's 2,871 `t!()` sites it ranks by i18n density (Spearman 0.435 against a real +cognitive metric, sharing 9 of its top 50). lizard's Rust reader never counts match arms while +counting every `?` and `where`, and its TypeScript reader silently drops functions from any file +with an object key named `interface`. `scc` and `tokei` are per-file or carry no complexity +metric at all. + +**One blind spot, and it is universal.** A body inside `macro_rules!` scores zero — in `bca`, in +`rust-code-analysis`, and in SonarSource's own analyzer. Macro bodies are 0.19% of this repo's +Rust lines, so it is disclosed rather than instrumented. + +## Baseline## Baseline## Baseline + +| scope | functions | total cognitive | p95 | p99 | max | over 25 | +| ----- | --------: | --------------: | --: | --: | --: | ---------: | +| Rust | 11,282 | 16,823 | 7 | 24 | 155 | 101 (0.9%) | +| TS/JS | 5,436 | 6,816 | 6 | 15 | 72 | 15 (0.3%) | + +`> 25` is the actionable line: under 1% of functions, 116 repo-wide. Worst ten, which is the standing pay-down list: ``` +155 ipv6_set projects/start-wrt/backend/ctrl/src/lan.rs:395 150 update shared-libs/crates/start-core/src/net/net_controller.rs:358 144 update_addresses shared-libs/crates/start-core/src/net/host/mod.rs:141 135 update_profile_ips_… projects/start-wrt/backend/ctrl/src/lan.rs:668 +133 set projects/start-wrt/backend/ctrl/src/published_ports.rs:867 125 shared-libs/crates/start-core/src/net/host/address.rs:472 -118 ipv6_set projects/start-wrt/backend/ctrl/src/lan.rs:395 - 98 set projects/start-wrt/backend/ctrl/src/published_ports.rs:849 97 rebase shared-libs/crates/patch-db/core/src/patch.rs:105 80 up shared-libs/crates/start-core/src/version/v0_4_0_alpha_20.rs:37 + 76 set projects/start-wrt/backend/ctrl/src/profiles.rs:1027 72 projects/start-sdk/lib/version/VersionGraph.ts:98 - 70 list_conffiles projects/start-wrt/backend/ctrl/src/setup.rs:249 ``` The closure at `net/host/address.rs:472` outranks every named function but four. From ad2e6cc42b391ae4680535bdea26bfa6445b18f5 Mon Sep 17 00:00:00 2001 From: Helix <267227783+helix-nine@users.noreply.github.com> Date: Thu, 27 Aug 2026 16:56:31 +0000 Subject: [PATCH 09/17] fix(repo): fail the census loudly when the analyzer does not run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rewrite onto bca dropped the parse-coverage guard, and the RFC went on promising it — the kind of claim a diff shows as unchanged context. Restores it against bca's own ERROR-node count, and adds the case the original guard missed: a census that finds no functions at all now exits non-zero instead of reporting zero complexity and passing every threshold. A failed analyzer reports its own last stderr line rather than a traceback. The margin is wide. bca finds 1 parse error in 2,681,337 nodes here; the engine it forked finds 240, clustered in six files on generic associated types and `impl Trait` in argument position — syntax postdating its January 2023 grammars. --- build/complexity/census.py | 27 ++++++++++++++++++++++++--- rfcs/complexity-budget.md | 23 +++++++++++------------ 2 files changed, 35 insertions(+), 15 deletions(-) diff --git a/build/complexity/census.py b/build/complexity/census.py index 8ed83b5698..7c742d34ea 100644 --- a/build/complexity/census.py +++ b/build/complexity/census.py @@ -24,9 +24,11 @@ def spaces(node, root=True): def census(root, scopes, bca): out = tempfile.mkdtemp(prefix='cx-') - subprocess.run([bca, 'metrics', '-O', 'json', '--exclude-tests', '--output-dir', out, - *(os.path.join(root, s) for s in scopes)], - check=True, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) + run = subprocess.run([bca, 'metrics', '-O', 'json', '--exclude-tests', '--output-dir', out, + *(os.path.join(root, s) for s in scopes)], + capture_output=True, text=True) + if run.returncode != 0: + sys.exit(f"complexity: {bca} failed ({run.returncode}): {run.stderr.strip().splitlines()[-1] if run.stderr.strip() else 'no output'}") rows = [] for dirpath, _, names in os.walk(out): for name in names: @@ -74,6 +76,24 @@ def count_callers(rows, root, scopes): r['callers'] = max(0, used[r['name']] - defined[r['name']]) +def assert_parsed(bca, root, scopes, rows): + """Grammar rot is silent: a file the parser cannot read yields no functions, not an error.""" + if not rows: + sys.exit('complexity: the census found no functions at all — the analyzer did not run') + out = subprocess.run([bca, 'count', '--type', 'ERROR', *(os.path.join(root, s) for s in scopes)], + capture_output=True, text=True).stdout + total = found = 0 + for line in out.splitlines(): + digits = line.split(':')[-1].strip().replace(',', '') + if line.startswith('Total nodes'): + total = int(digits or 0) + elif line.startswith('Found nodes'): + found = int(digits or 0) + if total and found / total > 0.005: + sys.exit(f"complexity: {found / total:.3%} of AST nodes are parse errors — " + "the grammar has fallen behind the language") + + def totals(rows): return {'functions': len(rows), 'cognitive': sum(r['cognitive'] for r in rows), @@ -92,6 +112,7 @@ def main(): a = ap.parse_args() scopes = a.scope or SCOPES rows = census(a.root, scopes, a.bca) + assert_parsed(a.bca, a.root, scopes, rows) if a.json: count_callers(rows, a.root, scopes) json.dump({'totals': totals(rows), 'functions': rows}, sys.stdout, sort_keys=True) diff --git a/rfcs/complexity-budget.md b/rfcs/complexity-budget.md index 891c51d0d8..286871e35f 100644 --- a/rfcs/complexity-budget.md +++ b/rfcs/complexity-budget.md @@ -28,8 +28,8 @@ Zero of 566 merged PR bodies contain the string `complexit`. ## What gets measured -**Cognitive complexity per function**, via `rust-code-analysis` (tree-sitter, real Rust and -TypeScript grammars). Not cyclomatic complexity, and the difference is the whole argument. +**Cognitive complexity per function**, via `bca` (tree-sitter, real Rust and TypeScript +grammars). Not cyclomatic complexity, and the difference is the whole argument. Probing both against a synthetic file settles it: @@ -63,14 +63,13 @@ Three deliberate exclusions: inline `#[cfg(test)]` items are stripped before mea repo has 1,150 `#[test]` functions and only 6,249 lines in dedicated test files, so a path-based rule would fail and a PR adding good tests would read as adding complexity); generated trees are skipped (`osBindings`, `locales`, `exver.ts`, `dist`, `target`); and the -census refuses to report if the parser reads under 98% of files or if over 0.5% of AST nodes -are parse errors, because grammar rot is otherwise silent. +census refuses to report when over 0.5% of AST nodes are parse errors, because grammar rot is +otherwise silent — a file the parser cannot read yields no functions rather than an error. -Today every one of 530 Rust files yields metrics, and 240 of 2,553,851 AST nodes — 0.009% — -are parse errors. They cluster in six Rust files, on modern trait syntax the pinned grammar -predates: generic associated types (`type Extended<'ext> where Self: 'ext`) and `impl Trait` -in argument and return position. That is the shape grammar rot takes, and it is what the -second guard watches: fifty times the current rate still passes. +The margin there is large. `bca` finds **1 parse error in 2,681,337 nodes** across this repo. +The engine it forked finds 240, clustered in six Rust files on generic associated types and +`impl Trait` in argument position — syntax that postdates its January 2023 grammars. Keeping +current with the language is most of what the fork buys. **The parser does not expand macros, and that is a hole.** Wrapping a body in `macro_rules!` takes it from cognitive 15 to **0** and cyclomatic 11 to 1 — measured, identical logic. The @@ -273,9 +272,9 @@ branches were moved, not removed. Known limits, stated rather than hidden: Angular templates are invisible — 278 components use inline `template:` backticks holding 831 control-flow constructs, and those sit inside string -literals that no per-function metric sees. `rust-code-analysis`'s last release is v0.0.25 from -January 2023, though its grammar parses 100% of this repo today and the coverage assertion -above is what catches it if that changes. And the length check on the prose answers catches +literals that no per-function metric sees. `bca` is four months old and carried by a single +maintainer, which is the trade for it being maintained at all — its parent's last release is +January 2023 and Mozilla no longer uses it. And the length check on the prose answers catches laziness, not sophistry: an LLM writes a fluent post-hoc justification easily, so do not sell the gate as catching bad reasoning. It catches an unexamined change. Only the numbers and the call-site count are self-verifying. From b3fe2cd440b38d0de4c25066ddccbbad94448ac3 Mon Sep 17 00:00:00 2001 From: Helix <267227783+helix-nine@users.noreply.github.com> Date: Thu, 27 Aug 2026 17:04:11 +0000 Subject: [PATCH 10/17] fix(repo): stop the gate taxing shared utilities MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A general-purpose utility has one call site the day it is written, so asking an author to defend every single-use function charges a toll on the library we want and collects it as inlined helpers and helpers bent to fit one caller. Measuring it showed the signal was not merely unhelpful but inverted. Against a function that retried an HTTP call inline: extracting a generic retry_with_backoff into shared-libs takes cognitive 8 -> 5 and FIRED the "branches were relocated" warning, while shredding the same logic in place into three helpers threading &mut state takes cognitive 8 -> 11 and stayed silent. A real extraction lowers cognitive and adds a function exactly as a bad split does, so that warning cannot separate them. Deleted. What separates them is where the callers are. The census now marks a function shared when anything outside its own file calls it — 76% of named functions here, against 9% single-use beside their only caller. The report credits additions to shared-libs, counts private single-use helpers without demanding a defence of each, and asks no question about either. The surviving reuse question — which existing helper you checked before adding a new one — pushes toward reuse rather than away from it. --- build/complexity/census.py | 18 ++++++++++++------ build/complexity/check.py | 2 -- build/complexity/delta.py | 21 ++++++++++++--------- rfcs/complexity-budget.md | 38 +++++++++++++++++++++++++++++++------- 4 files changed, 55 insertions(+), 24 deletions(-) diff --git a/build/complexity/census.py b/build/complexity/census.py index 7c742d34ea..e9c218200f 100644 --- a/build/complexity/census.py +++ b/build/complexity/census.py @@ -56,24 +56,30 @@ def census(root, scopes, bca): def count_callers(rows, root, scopes): - """Call sites per function name across the tree. A definition is not a call site.""" - used = collections.Counter() + """Marks each function with its call-site count and whether any caller sits outside its own file.""" + per_file = collections.defaultdict(collections.Counter) for scope in scopes: for dirpath, dirs, names in os.walk(os.path.join(root, scope)): dirs[:] = [d for d in dirs if d not in ('node_modules', 'target', 'dist', '.angular', 'out-tsc', 'osBindings', 'locales')] for name in names: path = os.path.join(dirpath, name) - if not kept(os.path.relpath(path, root)): + rel = os.path.relpath(path, root) + if not kept(rel): continue try: - used.update(CALL_SITE.findall(open(path, encoding='utf-8', errors='replace').read())) + per_file[rel].update(CALL_SITE.findall(open(path, encoding='utf-8', errors='replace').read())) except OSError: pass defined = collections.Counter(r['name'] for r in rows) for r in rows: - if r['name'] != '': - r['callers'] = max(0, used[r['name']] - defined[r['name']]) + name = r['name'] + if name == '': + continue + total = sum(c[name] for c in per_file.values()) + r['callers'] = max(0, total - defined[name]) + elsewhere = sum(c[name] for f, c in per_file.items() if f != r['file']) + r['shared'] = elsewhere > 0 def assert_parsed(bca, root, scopes, rows): diff --git a/build/complexity/check.py b/build/complexity/check.py index 58fef56c99..ed533ba11d 100644 --- a/build/complexity/check.py +++ b/build/complexity/check.py @@ -13,8 +13,6 @@ ('simplest alternative', r'[Ss]implest alternative[^\n]*:[^\S\n]*(\S[^\n]*)', None), ('existing helper', r'[Ee]xisting helper[^\n]*:[^\S\n]*(\S[^\n]*)', None), ('over-25 justification', r'over 25[^\n]*:[^\S\n]*(\S[^\n]*)', '<-- '), - ('single-call-site helpers', r'one call site[^\n]*:[^\S\n]*(\S[^\n]*)', - 'new functions with one call site ('), ) NOT_APPLICABLE = {'none', 'none.', 'n/a', 'na', 'nothing', 'not applicable'} diff --git a/build/complexity/delta.py b/build/complexity/delta.py index 975707edc2..f6134e4bf0 100644 --- a/build/complexity/delta.py +++ b/build/complexity/delta.py @@ -14,10 +14,6 @@ ('fns over 25', 'over25')): print(f" {label:<12}{tb[k]:>7} -> {th[k]:>7} {th[k]-tb[k]:+d}") -# Cognitive falls when a function is split, however badly; cyclomatic does not. -if th['cognitive'] < tb['cognitive'] and th['cyclomatic'] > tb['cyclomatic']: - print("\n cognitive fell while cyclomatic rose — branches were relocated, not removed") - new = sorted((r for k, r in H.items() if k not in B), key=lambda r: -r['cognitive']) big = [r for r in new if r['cognitive'] > 10] if big: @@ -46,9 +42,16 @@ if gone: print(f"\n removed: {len(gone)} functions, {sum(r['cognitive'] for r in gone)} cognitive") -single = [r for r in new if r.get('callers') == 1 and r['name'] != ''] -if single: - print(f"\n new functions with one call site ({len(single)}) — each is an abstraction the diff does not yet reuse:") - for r in single[:10]: - print(f" {r['name']} {r['file']}:{r['line']}") +# A helper whose only caller shares its file is the shape a shredded function takes. +# One that anything else calls is a shared utility, and is not the target here. +shared = [r for r in new if r.get('shared') and r['file'].startswith('shared-libs')] +if shared: + print(f"\n added to shared-libs and already called elsewhere: {len(shared)}") + +# A helper whose only caller shares its file is the shape a shredded function takes. +# Informational: a first caller is where every utility starts. +private = [r for r in new + if r.get('callers') == 1 and not r.get('shared') and r['name'] != ''] +if private: + print(f" single-use helpers alongside their only caller: {len(private)}") diff --git a/rfcs/complexity-budget.md b/rfcs/complexity-budget.md index 286871e35f..d4b3416700 100644 --- a/rfcs/complexity-budget.md +++ b/rfcs/complexity-budget.md @@ -239,16 +239,40 @@ The three questions, chosen because a weak answer is visible to a human: - the simplest alternative considered, and what breaks if we take it - which existing helper was checked before adding a new one, by file - for any function pushed over 25, why the branching is intrinsic to the requirement -- for each new function with exactly one call site, why it earns its own name -The last one is the only question in the set an author cannot bluff, because the census counts -the call sites itself. It also targets the most common real defect in agent-written code — the -helper extracted for a reuse that never arrives. The portmap PR added 83 of them. - -**The last two questions are asked only when the census reports them.** Requiring all four on -every PR would put a four-line section on the 61% of source PRs whose delta is near zero +**The last question is asked only when the census reports a function over 25.** Requiring a +justification on every PR would put a section on the 61% of source PRs whose delta is near zero against the 19% that are substantial, which is a rubber-stamping machine rather than a gate. +## Building shared utilities is the point, so the gate must not tax it + +A general-purpose utility has exactly one call site on the day it is written. Any rule that +makes an author defend a single-use function therefore charges a toll on the library we want, +and pays it in the two currencies we least want: helpers left inlined, and helpers bent to fit +their one caller so the justification writes itself. + +An earlier draft of this gate did exactly that, and measuring it showed the signal was not +merely unhelpful but inverted. Against a base function that retried an HTTP call inline, two +changes were compared: extracting a generic `retry_with_backoff` into `shared-libs`, and +shredding the same logic in place into three helpers threading `&mut` state. + +| | extracted utility | in-place shred | +| --------------------------------- | -------------------: | -----------------: | +| cognitive | 8 → **5** | 8 → **11** | +| "branches were relocated" warning | **fired** | silent | +| flagged as unreused abstraction | `retry_with_backoff` | the three `step_*` | + +The warning fired on the good change and stayed quiet on the bad one, because a real extraction +lowers cognitive and adds a function exactly as a bad split does. It is deleted. + +What separates them is not the call count but **where the callers are**. The census marks a +function shared when anything outside its own file calls it; repo-wide that is 76% of named +functions, against 9% single-use beside their only caller. So the report credits what lands in +`shared-libs` and is called from elsewhere, counts private single-use helpers without demanding +a defence of each, and asks no question about either. The question that does survive — +which existing helper you checked before adding a new one — pushes toward reuse rather than away +from it. + 27% of merged PR bodies already volunteer a rejected alternative, so the hardest of the three is culturally native here rather than an imposition. From 6add88b2131b54f129d42c5040c903c8d9fec0dd Mon Sep 17 00:00:00 2001 From: Helix <267227783+helix-nine@users.noreply.github.com> Date: Thu, 27 Aug 2026 17:11:23 +0000 Subject: [PATCH 11/17] fix(repo): credit a utility for being reused, not for existing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rewarding a new utility rewards writing a second one instead of finding the first. A function now earns its line in the report only once two or more distinct subsystems call it — a subsystem being a product or crate plus its first module segment. 59% of the 1,208 functions in util modules clear that bar today. Util-module complexity is reported apart rather than zeroed. Not taxing utilities and not rewarding duplicates pull against each other, and exempting util complexity resolves that the wrong way: it makes the util module free parking for the near-duplicate the rule exists to discourage. Measured against a tree already holding a generic retry_with_backoff, a second product writing its own copy costs +4 cognitive whether it lands beside its caller or inside the util module, while calling the existing one costs 0. An earlier line claimed util complexity was not counted against the delta. It was counted; the line was wrong and now states what the code does. Nothing mechanical catches a re-implemented utility: jscpd flags a renamed copy at 45% duplicated lines but finds zero clones for the same helper written afresh. The rubric question about checking for an existing helper is the lever there. --- build/complexity/census.py | 26 +++++++++++++++++++++++--- build/complexity/delta.py | 19 +++++++++++++------ rfcs/complexity-budget.md | 31 ++++++++++++++++++++++++++----- 3 files changed, 62 insertions(+), 14 deletions(-) diff --git a/build/complexity/census.py b/build/complexity/census.py index e9c218200f..aade99f3eb 100644 --- a/build/complexity/census.py +++ b/build/complexity/census.py @@ -7,6 +7,21 @@ '/osBindings/', '/locales/', '/__snapshots__/', '/__fixtures__/', '/patch-db/client/', '/exver/exver.ts') CALL_SITE = re.compile(r'\b([A-Za-z_]\w*)\s*[(:<]') +UTIL_MODULE = re.compile(r'(^|/)(util|utils|helpers)(/|\.(rs|ts|js)$)') + + +def subsystem(path): + """The product or crate a file belongs to, plus its first module segment.""" + parts = path.split('/') + if parts[0] == 'shared-libs' and len(parts) > 3: + base = parts[:3] + elif parts[0] == 'projects' and len(parts) > 2: + base = parts[:2] + else: + base = parts[:1] + rest = [p for p in parts[len(base):] if p not in ('src', 'lib')] + head = rest[0] if rest and '.' not in rest[0] else '' + return '/'.join(base) + ('/' + head if head else '') def kept(path): @@ -56,7 +71,7 @@ def census(root, scopes, bca): def count_callers(rows, root, scopes): - """Marks each function with its call-site count and whether any caller sits outside its own file.""" + """Marks each function with its call-site count and the subsystems that call it.""" per_file = collections.defaultdict(collections.Counter) for scope in scopes: for dirpath, dirs, names in os.walk(os.path.join(root, scope)): @@ -72,14 +87,19 @@ def count_callers(rows, root, scopes): except OSError: pass defined = collections.Counter(r['name'] for r in rows) + callers_of = collections.defaultdict(set) + for path, counts in per_file.items(): + for name in counts: + callers_of[name].add(subsystem(path)) for r in rows: name = r['name'] if name == '': continue total = sum(c[name] for c in per_file.values()) r['callers'] = max(0, total - defined[name]) - elsewhere = sum(c[name] for f, c in per_file.items() if f != r['file']) - r['shared'] = elsewhere > 0 + r['shared'] = sum(c[name] for f, c in per_file.items() if f != r['file']) > 0 + r['util'] = bool(UTIL_MODULE.search(r['file'])) + r['scopes'] = sorted(callers_of[name] - {subsystem(r['file'])}) def assert_parsed(bca, root, scopes, rows): diff --git a/build/complexity/delta.py b/build/complexity/delta.py index f6134e4bf0..8baa024e4f 100644 --- a/build/complexity/delta.py +++ b/build/complexity/delta.py @@ -44,14 +44,21 @@ # A helper whose only caller shares its file is the shape a shredded function takes. # One that anything else calls is a shared utility, and is not the target here. -shared = [r for r in new if r.get('shared') and r['file'].startswith('shared-libs')] -if shared: - print(f"\n added to shared-libs and already called elsewhere: {len(shared)}") +# A utility earns credit for being reused across subsystems, never for existing. +reused = [r for r in new if len(r.get('scopes') or []) >= 2] +if reused: + print(f"\n new functions already reused across subsystems ({len(reused)}):") + for r in sorted(reused, key=lambda r: -len(r['scopes']))[:5]: + print(f" {r['name']} {len(r['scopes'])} subsystems {r['file']}:{r['line']}") + +util_new = [r for r in new if r.get('util')] +if util_new: + print(f" of the new functions, {len(util_new)} sit in util modules" + f" ({sum(r['cognitive'] for r in util_new)} cognitive)") -# A helper whose only caller shares its file is the shape a shredded function takes. -# Informational: a first caller is where every utility starts. private = [r for r in new - if r.get('callers') == 1 and not r.get('shared') and r['name'] != ''] + if r.get('callers') == 1 and not r.get('shared') + and not r.get('util') and r['name'] != ''] if private: print(f" single-use helpers alongside their only caller: {len(private)}") diff --git a/rfcs/complexity-budget.md b/rfcs/complexity-budget.md index d4b3416700..f6edc16788 100644 --- a/rfcs/complexity-budget.md +++ b/rfcs/complexity-budget.md @@ -267,11 +267,32 @@ lowers cognitive and adds a function exactly as a bad split does. It is deleted. What separates them is not the call count but **where the callers are**. The census marks a function shared when anything outside its own file calls it; repo-wide that is 76% of named -functions, against 9% single-use beside their only caller. So the report credits what lands in -`shared-libs` and is called from elsewhere, counts private single-use helpers without demanding -a defence of each, and asks no question about either. The question that does survive — -which existing helper you checked before adding a new one — pushes toward reuse rather than away -from it. +functions, against 9% single-use beside their only caller. So the report counts private +single-use helpers without demanding a defence of each, and asks no question about either. + +**Credit attaches to reuse, never to creation.** Rewarding a new utility rewards writing a +second one instead of finding the first, so a function earns its line in the report only once +**two or more distinct subsystems** call it — a subsystem being a product or crate plus its +first module segment. Of the 1,208 functions in util modules today, 59% clear that bar and the +rest sit neutral. A utility written this week is not praised for existing; it is praised when +the second caller arrives, which is the moment its generality stops being a claim. + +**Util-module complexity is reported apart, not zeroed.** Those two goals — don't tax utilities, +don't reward duplicates — pull against each other, and making util complexity free is what +resolves them the wrong way: it turns the util module into free parking for exactly the +near-duplicate the rule was meant to discourage. Measured against a tree already holding a +generic `retry_with_backoff`, a second product writing its own copy costs **+4 cognitive** +whether that copy lands beside its caller or inside the util module, while calling the existing +one costs **0**. Reuse is strictly cheaper than duplication wherever the duplicate is parked, +and that property comes from counting util complexity rather than exempting it. The report +separates the figure so an author can see it; nothing gates on it. + +**Nothing mechanical catches a re-implemented utility.** A copy-pasted one is findable — `jscpd` +flags a renamed copy at 45% duplicated lines — but the same helper written afresh with a +different signature registers zero clones, because the duplication is semantic. The lever that +actually addresses it is the surviving rubric question, which existing helper you checked before +adding a new one, named by file. That question is in the gate precisely because no measurement +replaces it. 27% of merged PR bodies already volunteer a rejected alternative, so the hardest of the three is culturally native here rather than an imposition. From 8b2a41e90262b344bbd3a94a6017c1f56069a671 Mon Sep 17 00:00:00 2001 From: Helix <267227783+helix-nine@users.noreply.github.com> Date: Thu, 27 Aug 2026 17:12:40 +0000 Subject: [PATCH 12/17] fix(repo): credit a utility when a subsystem adopts it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Credit only fired for functions a diff introduced, so adopting an existing helper — the behaviour most worth encouraging — earned nothing at all. Credit now follows adoption: a function earns its line when a subsystem that did not call it before starts to, and only once the total reaches two. The first caller is the author; the second is where generality stops being a claim. That ordering makes reuse dominate duplication with no penalty on utilities. Against a tree already holding a generic retry_with_backoff, a second product writing its own near-copy earns nothing because that copy reaches one subsystem, while calling the existing helper adds no code and credits it at two subsystems. Drops the argument that exempting util complexity would make the util module free parking for duplicates. Nothing gates on the totals — the only question the gate can require concerns a function pushed over 25 — so complexity arriving in a util module is already unpenalised in the sense that operates. --- build/complexity/delta.py | 19 +++++++++++++------ rfcs/complexity-budget.md | 33 +++++++++++++++++---------------- 2 files changed, 30 insertions(+), 22 deletions(-) diff --git a/build/complexity/delta.py b/build/complexity/delta.py index 8baa024e4f..8847f7a1af 100644 --- a/build/complexity/delta.py +++ b/build/complexity/delta.py @@ -44,12 +44,19 @@ # A helper whose only caller shares its file is the shape a shredded function takes. # One that anything else calls is a shared utility, and is not the target here. -# A utility earns credit for being reused across subsystems, never for existing. -reused = [r for r in new if len(r.get('scopes') or []) >= 2] -if reused: - print(f"\n new functions already reused across subsystems ({len(reused)}):") - for r in sorted(reused, key=lambda r: -len(r['scopes']))[:5]: - print(f" {r['name']} {len(r['scopes'])} subsystems {r['file']}:{r['line']}") +# A utility earns credit when a subsystem adopts it, not when it is written. +adopted = [] +for k, r in H.items(): + before = set((B[k].get('scopes') or []) if k in B else ()) + after = set(r.get('scopes') or []) + # Two distinct subsystems is where generality stops being a claim; the first + # caller is just the author, so creating a utility earns nothing. + if after - before and len(after) >= 2: + adopted.append((r, sorted(after - before), len(after))) +if adopted: + print(f"\n utilities a second subsystem now depends on ({len(adopted)}):") + for r, gained, total in sorted(adopted, key=lambda x: -len(x[1]))[:8]: + print(f" {r['name']} +{', '.join(gained)} (now {total}) {r['file']}") util_new = [r for r in new if r.get('util')] if util_new: diff --git a/rfcs/complexity-budget.md b/rfcs/complexity-budget.md index f6edc16788..6838598f18 100644 --- a/rfcs/complexity-budget.md +++ b/rfcs/complexity-budget.md @@ -270,22 +270,23 @@ function shared when anything outside its own file calls it; repo-wide that is 7 functions, against 9% single-use beside their only caller. So the report counts private single-use helpers without demanding a defence of each, and asks no question about either. -**Credit attaches to reuse, never to creation.** Rewarding a new utility rewards writing a -second one instead of finding the first, so a function earns its line in the report only once -**two or more distinct subsystems** call it — a subsystem being a product or crate plus its -first module segment. Of the 1,208 functions in util modules today, 59% clear that bar and the -rest sit neutral. A utility written this week is not praised for existing; it is praised when -the second caller arrives, which is the moment its generality stops being a claim. - -**Util-module complexity is reported apart, not zeroed.** Those two goals — don't tax utilities, -don't reward duplicates — pull against each other, and making util complexity free is what -resolves them the wrong way: it turns the util module into free parking for exactly the -near-duplicate the rule was meant to discourage. Measured against a tree already holding a -generic `retry_with_backoff`, a second product writing its own copy costs **+4 cognitive** -whether that copy lands beside its caller or inside the util module, while calling the existing -one costs **0**. Reuse is strictly cheaper than duplication wherever the duplicate is parked, -and that property comes from counting util complexity rather than exempting it. The report -separates the figure so an author can see it; nothing gates on it. +**Credit attaches to adoption, never to creation.** Rewarding a new utility rewards writing a +second one instead of finding the first, so nothing is credited for existing. A function earns +its line only when a **subsystem that did not call it before starts to** — a subsystem being a +product or crate plus its first module segment — and only once the total reaches two. The first +caller is the author; the second is where generality stops being a claim. Of the 1,208 functions +in util modules today, 59% clear that bar. + +That ordering makes reuse dominate duplication without any penalty on utilities. Against a tree +already holding a generic `retry_with_backoff`, a second product writing its own near-copy +reports its cost and earns nothing, because that copy reaches only one subsystem; calling the +existing helper adds no code at all and credits `retry_with_backoff +projects/web (now 2)`. +Duplicating is never cheaper than reusing, and the difference is credit rather than punishment. + +**Nothing gates on the totals.** The complexity figures are reported, and the only question the +gate can require is the one about a function pushed over 25. So complexity arriving in a util +module is already unpenalised in the only sense that operates — the report separates the figure +so an author can see it, and no threshold, ratchet or check keys on it. **Nothing mechanical catches a re-implemented utility.** A copy-pasted one is findable — `jscpd` flags a renamed copy at 45% duplicated lines — but the same helper written afresh with a From 57e95db2e6b870d1afc9be7747bd332e8899329b Mon Sep 17 00:00:00 2001 From: Helix <267227783+helix-nine@users.noreply.github.com> Date: Thu, 27 Aug 2026 17:23:30 +0000 Subject: [PATCH 13/17] refactor(repo): make complexity context rather than a gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A metric with a reward attached gets optimised, and every cheap way to optimise this one makes the code worse: splitting a clear function into six poorly-named pieces lowers its score, hiding a body in macro_rules! takes it to zero, and leaving a helper inlined avoids a new function. So nothing here fails a build, blocks a merge, or caps a number. Removes the PR-body gate, its make target and its rubric enforcement. What remains prints what a branch did — the totals, every function it pushed higher, every one it simplified, and which utilities a second subsystem now depends on — so an unexpected rise reads as a symptom to look at, and a rise the author stands behind can be pointed at and explained. Complexity intrinsic to a requirement is still complexity, and the report names functions rather than totals so that case can be argued. Adds `build/complexity/history.tsv`, one row per master commit, seeded with 28 sampled points. Only master CI appends to it: a totals file that pull requests edit conflicts on 75.4% of median-lifetime branches, and one written after merge conflicts on none. Step changes in it are usually imports — 16,188 to 22,032 on 2026-07-02 is start-wrt and start-cli arriving, not a bad week. Retitles the RFC, which no longer describes a budget. --- Makefile | 2 +- build/complexity.mk | 8 +- build/complexity/census.sh | 13 +- build/complexity/check.py | 50 ---- build/complexity/delta.py | 3 +- build/complexity/history.tsv | 29 ++ build/complexity/record.sh | 15 ++ ...exity-budget.md => complexity-tracking.md} | 248 +++++++----------- 8 files changed, 153 insertions(+), 215 deletions(-) delete mode 100644 build/complexity/check.py create mode 100644 build/complexity/history.tsv create mode 100755 build/complexity/record.sh rename rfcs/{complexity-budget.md => complexity-tracking.md} (53%) diff --git a/Makefile b/Makefile index e8f684bd3d..2f5d7c0178 100644 --- a/Makefile +++ b/Makefile @@ -16,7 +16,7 @@ include projects/start-wrt/build.mk include projects/start-docs/build.mk include build/complexity.mk -.PHONY: help start-os metadata start-os-install clean format format-check complexity complexity-top complexity-diff complexity-check start-cli-install start-cli start-cli-deb start-os-uis start-os-ui start-os-emulate-reflash start-os-deb start-os-$(IMAGE_TYPE) start-os-squashfs start-os-wormhole start-os-wormhole-deb start-os-update start-os-update-from-gha test start-core-test start-sdk-test container-runtime-test start-wrt-test start-registry start-registry-install start-tunnel start-tunnel-install start-core-ts-bindings +.PHONY: help start-os metadata start-os-install clean format format-check complexity complexity-top complexity-diff complexity-record start-cli-install start-cli start-cli-deb start-os-uis start-os-ui start-os-emulate-reflash start-os-deb start-os-$(IMAGE_TYPE) start-os-squashfs start-os-wormhole start-os-wormhole-deb start-os-update start-os-update-from-gha test start-core-test start-sdk-test container-runtime-test start-wrt-test start-registry start-registry-install start-tunnel start-tunnel-install start-core-ts-bindings help: @echo "No default target — specify one. Common targets:" diff --git a/build/complexity.mk b/build/complexity.mk index 0395b36e3a..f65cfdbc5b 100644 --- a/build/complexity.mk +++ b/build/complexity.mk @@ -2,7 +2,7 @@ COMPLEXITY := ./build/complexity/census.sh BASE ?= origin/master -.PHONY: complexity complexity-top complexity-diff complexity-check +.PHONY: complexity complexity-top complexity-diff complexity-record # Totals plus the worst 25 functions in the tree. complexity: @@ -16,6 +16,6 @@ complexity-top: complexity-diff: @$(COMPLEXITY) diff $(BASE) -# Fail when a PR body's pasted block is missing, unanswered, or stale. -complexity-check: - @$(COMPLEXITY) check "$(PR_BODY_FILE)" $(BASE) +# Appends one row for HEAD to the log. Master CI runs this; a PR never writes it. +complexity-record: + @./build/complexity/record.sh $(LOG) diff --git a/build/complexity/census.sh b/build/complexity/census.sh index 5ed383fde4..d195052704 100755 --- a/build/complexity/census.sh +++ b/build/complexity/census.sh @@ -1,7 +1,6 @@ #!/bin/bash -# Cognitive-complexity census. `census` prints the totals and the worst 25; `diff ` -# prints this branch's delta against its merge-base; `check ` fails when -# a PR body's block is absent, unanswered, or disagrees with a fresh run. +# Cognitive-complexity census. `census` prints the totals and the worst 25; `top` the worst N; +# `diff ` what this branch did to them. Nothing here exits non-zero on a number. set -euo pipefail HERE="$(cd "$(dirname "$0")" && pwd)" export BCA="${BCA:-$HERE/bin/bca}" @@ -20,11 +19,5 @@ case "${1:-census}" in python3 "$HERE/census.py" --root . --json > "$tmp/.head.json" python3 "$HERE/delta.py" "$tmp/.base.json" "$tmp/.head.json" "$mb" ;; - check) - body="$2"; base="${3:-origin/master}" - fresh="$(mktemp)"; trap 'rm -f "$fresh"' EXIT - "$0" diff "$base" > "$fresh" - python3 "$HERE/check.py" "$body" "$fresh" - ;; - *) echo "usage: census.sh {census|top [n]|diff |check }" >&2; exit 2 ;; + *) echo "usage: census.sh {census|top [n]|diff }" >&2; exit 2 ;; esac diff --git a/build/complexity/check.py b/build/complexity/check.py deleted file mode 100644 index ed533ba11d..0000000000 --- a/build/complexity/check.py +++ /dev/null @@ -1,50 +0,0 @@ -#!/usr/bin/env python3 -"""Fails when a PR body's Complexity block is absent, unanswered, or disagrees with a fresh census.""" -import re, sys - -TOTALS = re.compile( - r'functions\s+(\d+)\s*->\s*(\d+).*?' - r'cognitive\s+(\d+)\s*->\s*(\d+).*?' - r'cyclomatic\s+(\d+)\s*->\s*(\d+).*?' - r'sloc\s+(\d+)\s*->\s*(\d+)', re.S) - -# Each question is asked only when the census actually reported the thing it is about. -QUESTIONS = ( - ('simplest alternative', r'[Ss]implest alternative[^\n]*:[^\S\n]*(\S[^\n]*)', None), - ('existing helper', r'[Ee]xisting helper[^\n]*:[^\S\n]*(\S[^\n]*)', None), - ('over-25 justification', r'over 25[^\n]*:[^\S\n]*(\S[^\n]*)', '<-- '), -) - -NOT_APPLICABLE = {'none', 'none.', 'n/a', 'na', 'nothing', 'not applicable'} - - -def main(body_path, fresh_path): - body = open(body_path, encoding='utf-8').read() - fresh = open(fresh_path, encoding='utf-8').read() - if '## Complexity' not in body: - sys.exit("PR body has no '## Complexity' section. Run `make complexity-diff` and paste it.") - actual, claimed = TOTALS.search(fresh), TOTALS.search(body) - if not actual: - sys.exit("internal: could not parse the fresh census") - if not claimed: - sys.exit("PR body's Complexity section carries no `make complexity-diff` output.") - if claimed.groups() != actual.groups(): - c, a = claimed.groups(), actual.groups() - sys.exit("PR body's complexity numbers do not match a fresh run.\n" - f" body: functions {c[0]}->{c[1]} cognitive {c[2]}->{c[3]} cyclomatic {c[4]}->{c[5]}\n" - f" fresh: functions {a[0]}->{a[1]} cognitive {a[2]}->{a[3]} cyclomatic {a[4]}->{a[5]}\n" - "Re-run `make complexity-diff` and paste the current output.") - for label, pat, trigger in QUESTIONS: - raised = trigger is None or trigger in fresh - m = re.search(pat, body) - answer = m.group(1).strip() if m else '' - if not raised: - continue - if answer.lower() in NOT_APPLICABLE and trigger is not None: - sys.exit(f"PR body answers '{label}' with '{answer}', but the census reported it.") - if len(answer) < 12: - sys.exit(f"PR body's Complexity section leaves '{label}' unanswered.") - print("Complexity block present, current, and answered.") - - -main(sys.argv[1], sys.argv[2]) diff --git a/build/complexity/delta.py b/build/complexity/delta.py index 8847f7a1af..25fe4a9ead 100644 --- a/build/complexity/delta.py +++ b/build/complexity/delta.py @@ -44,7 +44,8 @@ # A helper whose only caller shares its file is the shape a shredded function takes. # One that anything else calls is a shared utility, and is not the target here. -# A utility earns credit when a subsystem adopts it, not when it is written. +# Adoption is the counterweight to a rising number: complexity that moved into a +# shared helper a second subsystem now calls reads differently from complexity added. adopted = [] for k, r in H.items(): before = set((B[k].get('scopes') or []) if k in B else ()) diff --git a/build/complexity/history.tsv b/build/complexity/history.tsv new file mode 100644 index 0000000000..d9f025a50f --- /dev/null +++ b/build/complexity/history.tsv @@ -0,0 +1,29 @@ +commit date functions cognitive cyclomatic sloc over25 +2d7a06acc 2026-06-29 12428 16170 28893 145031 67 +d1f127112 2026-07-01 12492 16188 29019 145765 68 +d6b5b885e 2026-07-02 15295 22032 36649 178257 105 +82ac00d1d 2026-07-03 15287 22044 36652 178236 105 +59ea25754 2026-07-06 15466 22300 36984 180103 109 +7258d5bc7 2026-07-06 15507 22404 37109 180599 110 +78339082e 2026-07-07 15568 22543 37270 181325 110 +48bef1ca4 2026-07-08 15682 22608 37435 185549 110 +aed04df22 2026-07-13 15694 22629 37456 185668 110 +65f187247 2026-07-15 15729 22646 37544 186145 110 +cac284929 2026-07-16 15750 22641 37579 186397 109 +5c47463a1 2026-07-19 15879 22771 37840 187980 113 +2c4253e4a 2026-07-21 15926 22832 37924 188445 113 +4461979cd 2026-07-23 15987 22869 38020 189091 113 +95de9a835 2026-07-23 15997 22889 38055 189249 113 +c343b52fa 2026-07-24 16027 22908 38116 189540 113 +6850e990c 2026-07-25 16034 22906 38123 189497 113 +d289f78d7 2026-07-27 16115 22979 38345 190344 113 +07a14b15c 2026-08-06 16150 23001 38396 190667 113 +909a6f020 2026-08-10 16187 23050 38504 191132 115 +f6c454b30 2026-08-12 16234 23111 38599 191664 115 +678fbb03b 2026-08-14 16276 23112 38660 192222 115 +b73c634d3 2026-08-17 16310 23124 38714 192652 115 +142e109bc 2026-08-19 16332 23146 38750 192822 115 +2b65299f8 2026-08-22 16660 23569 39521 196414 115 +c0da6d00c 2026-08-24 16669 23572 39532 196482 115 +974587948 2026-08-25 16671 23578 39543 196527 114 +18490269c 2026-08-26 16723 23639 39654 197055 116 diff --git a/build/complexity/record.sh b/build/complexity/record.sh new file mode 100755 index 0000000000..6ca4dc0b2f --- /dev/null +++ b/build/complexity/record.sh @@ -0,0 +1,15 @@ +#!/bin/bash +# Appends one row for HEAD to the complexity log. Only master CI runs this, so a +# pull request never edits the file and never conflicts on it. +set -euo pipefail +HERE="$(cd "$(dirname "$0")" && pwd)" +LOG="${1:-build/complexity/history.tsv}" +[ -x "$HERE/bin/bca" ] || "$HERE/fetch-tool.sh" +read -r functions cognitive cyclomatic sloc over25 < <( + BCA="$HERE/bin/bca" python3 "$HERE/census.py" --root . --json \ + | python3 -c 'import json,sys; t=json.load(sys.stdin)["totals"]; print(t["functions"],t["cognitive"],t["cyclomatic"],t["sloc"],t["over25"])' +) +[ -s "$LOG" ] || printf 'commit\tdate\tfunctions\tcognitive\tcyclomatic\tsloc\tover25\n' > "$LOG" +printf '%s\t%s\t%s\t%s\t%s\t%s\t%s\n' \ + "$(git rev-parse --short HEAD)" "$(git log -1 --format=%cs)" \ + "$functions" "$cognitive" "$cyclomatic" "$sloc" "$over25" >> "$LOG" diff --git a/rfcs/complexity-budget.md b/rfcs/complexity-tracking.md similarity index 53% rename from rfcs/complexity-budget.md rename to rfcs/complexity-tracking.md index 6838598f18..b3910a64bf 100644 --- a/rfcs/complexity-budget.md +++ b/rfcs/complexity-tracking.md @@ -1,7 +1,9 @@ -# Complexity budget +# Complexity tracking -A way to track complexity in this repo, and a protocol that makes an agent state and defend -what its change cost. Every number below was measured on `7e58f9d45`. +A way to see what a change does to this repo's complexity, and to watch the figure move over +time. It gates nothing. The numbers are context for whoever is making the change — an +unexpected rise is a symptom worth looking at, and one the author should be able to explain or +refute. Every number below was measured on `7e58f9d45`. ## Why now @@ -118,7 +120,7 @@ metric at all. `rust-code-analysis`, and in SonarSource's own analyzer. Macro bodies are 0.19% of this repo's Rust lines, so it is disclosed rather than instrumented. -## Baseline## Baseline## Baseline +## Baseline | scope | functions | total cognitive | p95 | p99 | max | over 25 | | ----- | --------: | --------------: | --: | --: | --: | ---------: | @@ -146,10 +148,11 @@ The closure at `net/host/address.rs:472` outranks every named function but four. ## The tooling -`make complexity` (totals + worst 25), `make complexity-top` (pay-down list), and -`make complexity-diff` (this branch against its merge-base). No build; the census is a -tree-sitter pass, 0.5 s for the whole repo, and a full delta including the base checkout is -about 3 s. It lives in `build/complexity/`, matching `build/fmt/`. +`make complexity` (totals and the worst 25), `make complexity-top` (the same list, any length), +`make complexity-diff` (this branch against its merge-base) and `make complexity-record` (append +a row to the history log). No build: the census is a tree-sitter pass, 0.35 s for the whole repo, +and a full delta including the base extraction is about 3 s. It lives in `build/complexity/`, +matching `build/fmt/`. Real output, for the portmap gateway PR: @@ -197,144 +200,91 @@ noticing: - **Public surface.** The count of exported items — 923 `pub fn` in start-core alone. Widening an API is a permanent cost that no per-function metric registers. -These are reported alongside the delta rather than gated, at least to start. - -## Tracking over time, without the rebase tax - -`complexity-diff` computes its base at run time, so nothing is committed and nothing conflicts. -That gives review-time confrontation but no history. For history, the instinct is to commit a -small aggregate — and simulating real merges with `git merge-file` over 60 real code commits -says that is the worst option available: - -| committed artifact | conflict rate on a median-lifetime PR | -| --------------------------------------------- | ------------------------------------: | -| one-line totals | 75.4% | -| 15-line per-scope totals | 66.7% | -| sorted per-function watchlist, over threshold | **10.5%** | - -Every PR rewrites the same totals line; PRs rarely touch the same region of a sorted 234-line -list. Splitting per-scope buys exactly nothing — 48 conflicts either way, because every -conflict is intra-scope. So if we want a committed record, it should be the watchlist of -functions over the threshold, regenerated by the existing drift idiom, not a scoreboard. - -## The protocol - -The confrontation belongs in the PR body, because that is the artifact an agent always -produces: helix-nine wrote a body on 188 of 188 merged PRs with a median of 2,850 characters, -while humans left 28 of 198 and 26 of 75 empty. - -Documentation alone is not enough. The closest measurable precedent is the "Label every PR" -rule — inlined, bolded, with the literal command — which landed eight days before this -snapshot and runs at 60% agent compliance in that window and 28% across the last 90 merged -PRs. So the rule is paired with a check. - -`make complexity-diff`'s output goes under a `## Complexity` heading, followed by three -questions. CI recomputes the census from `base.sha..head.sha` and fails when the section is -missing, unanswered, or carries numbers that disagree. **Prose can be bluffed; a number CI -recomputes cannot.** That is the load-bearing part of the design — the agent cannot write the -block without having run the tool. - -The three questions, chosen because a weak answer is visible to a human: - -- the simplest alternative considered, and what breaks if we take it -- which existing helper was checked before adding a new one, by file -- for any function pushed over 25, why the branching is intrinsic to the requirement - -**The last question is asked only when the census reports a function over 25.** Requiring a -justification on every PR would put a section on the 61% of source PRs whose delta is near zero -against the 19% that are substantial, which is a rubber-stamping machine rather than a gate. - -## Building shared utilities is the point, so the gate must not tax it - -A general-purpose utility has exactly one call site on the day it is written. Any rule that -makes an author defend a single-use function therefore charges a toll on the library we want, -and pays it in the two currencies we least want: helpers left inlined, and helpers bent to fit -their one caller so the justification writes itself. - -An earlier draft of this gate did exactly that, and measuring it showed the signal was not -merely unhelpful but inverted. Against a base function that retried an HTTP call inline, two -changes were compared: extracting a generic `retry_with_backoff` into `shared-libs`, and -shredding the same logic in place into three helpers threading `&mut` state. - -| | extracted utility | in-place shred | -| --------------------------------- | -------------------: | -----------------: | -| cognitive | 8 → **5** | 8 → **11** | -| "branches were relocated" warning | **fired** | silent | -| flagged as unreused abstraction | `retry_with_backoff` | the three `step_*` | - -The warning fired on the good change and stayed quiet on the bad one, because a real extraction -lowers cognitive and adds a function exactly as a bad split does. It is deleted. - -What separates them is not the call count but **where the callers are**. The census marks a -function shared when anything outside its own file calls it; repo-wide that is 76% of named -functions, against 9% single-use beside their only caller. So the report counts private -single-use helpers without demanding a defence of each, and asks no question about either. - -**Credit attaches to adoption, never to creation.** Rewarding a new utility rewards writing a -second one instead of finding the first, so nothing is credited for existing. A function earns -its line only when a **subsystem that did not call it before starts to** — a subsystem being a -product or crate plus its first module segment — and only once the total reaches two. The first -caller is the author; the second is where generality stops being a claim. Of the 1,208 functions -in util modules today, 59% clear that bar. - -That ordering makes reuse dominate duplication without any penalty on utilities. Against a tree -already holding a generic `retry_with_backoff`, a second product writing its own near-copy -reports its cost and earns nothing, because that copy reaches only one subsystem; calling the -existing helper adds no code at all and credits `retry_with_backoff +projects/web (now 2)`. -Duplicating is never cheaper than reusing, and the difference is credit rather than punishment. - -**Nothing gates on the totals.** The complexity figures are reported, and the only question the -gate can require is the one about a function pushed over 25. So complexity arriving in a util -module is already unpenalised in the only sense that operates — the report separates the figure -so an author can see it, and no threshold, ratchet or check keys on it. - -**Nothing mechanical catches a re-implemented utility.** A copy-pasted one is findable — `jscpd` -flags a renamed copy at 45% duplicated lines — but the same helper written afresh with a -different signature registers zero clones, because the duplication is semantic. The lever that -actually addresses it is the surviving rubric question, which existing helper you checked before -adding a new one, named by file. That question is in the gate precisely because no measurement -replaces it. - -27% of merged PR bodies already volunteer a rejected alternative, so the hardest of the three -is culturally native here rather than an imposition. - -## What this deliberately does not do - -**It does not ratchet.** Features cost complexity: summed cognitive rises on 42 of 60 real code -commits and falls on 5, so a hard ratchet would block two commits in three and be suspended -during the first release crunch, never to return. The gate asks for a number and a reason, -not for the number to stay flat. - -**It reports cyclomatic next to cognitive, because cognitive alone rewards shredding.** This -was the first version's mistake. Cognitive complexity penalises nesting superlinearly, so -pulling nested blocks out into separate functions lowers the total however bad the split is. -Measured on a deliberately worse six-way split that threads loop state through `&mut` -parameters, total cognitive falls **26 → 9** while total cyclomatic rises **10 → 16**. -Cognitive ranks a single function; cyclomatic is close to additive and so survives relocation. -The report prints both and says so outright when cognitive falls while cyclomatic rises — -branches were moved, not removed. - -**It does not count tests**, so there is never a reason to thin one. - -Known limits, stated rather than hidden: Angular templates are invisible — 278 components use -inline `template:` backticks holding 831 control-flow constructs, and those sit inside string -literals that no per-function metric sees. `bca` is four months old and carried by a single -maintainer, which is the trade for it being maintained at all — its parent's last release is -January 2023 and Mozilla no longer uses it. And the length check on the prose answers catches -laziness, not sophistry: an LLM writes a fluent post-hoc justification easily, so do not sell -the gate as catching bad reasoning. It catches an unexamined change. Only the numbers and the -call-site count are self-verifying. - -## Open questions - -1. **Advisory or required?** There is no `required_status_checks` rule on `master` today, so a - red check does not literally block; the approval does. Nothing has merged red in the last - 30 PRs, so an advisory gate is honored in practice. Making it required is a one-line - ruleset change — worth doing, or not yet? -2. **Should the gate apply to humans, or only to agent-authored PRs?** As written it applies to - everyone, which is the honest version, but it lands hardest on the author who already writes - the longest PR bodies. -3. **Is `> 25` the right line?** It flags 1% of functions today. `> 15` would flag 2.4%. -4. **Pay-down.** 75% of the twenty densest files were touched in the last 200 commits, so - "pay some down when your change already puts you in one of these files" would fire often. - Standing rule, or left to judgement? +None of these are gated either; they are reported because they move independently of diff size. + +## No gate + +Nothing here fails a build, blocks a merge, or caps a number. That is deliberate, and it is what +makes the rest safe: a metric with a reward attached gets optimised, and every cheap way to +optimise this one makes the code worse. Splitting a clear function into six poorly-named pieces +lowers its cognitive score. Hiding a body in `macro_rules!` takes it to zero. Leaving a helper +inlined avoids a new function. None of those are improvements, and all of them are what a gate +would buy. + +So the numbers are **context handed to whoever is making the change**, and the only thing asked +of them is that they look. + +## What the report is for + +`make complexity-diff` prints what a branch did against its merge-base: the totals, every +function it pushed higher, every one it simplified, what it added to a util module, and which +utilities a second subsystem now depends on. It is a mirror, not a score. + +An unexpected rise is a **symptom**. It usually means one of a small number of things — a +function grew a branch it did not need, a helper was shredded rather than extracted, a special +case was threaded through a call chain instead of handled at the edge — and the per-function +lines are there so the author can tell which. The right response to a symptom is to look, and +then either fix the mess or explain why the number is wrong about it. + +**A rise that the author stands behind should be defensible, and the report is built to let it +be defended.** Complexity that is intrinsic to a requirement is still complexity: a protocol +with nine message types has nine branches wherever it is handled, and no restructuring removes +them. Because the report names functions rather than totals, an author can point at the specific +function, say what the branches are, and be right. A number is evidence, not a verdict — and the +one thing that would make it a verdict is a gate. + +## Tracking across changes + +`build/complexity/history.tsv` holds one row per master commit: commit, date, functions, +cognitive, cyclomatic, sloc, and the count over 25. It is seeded with 28 sampled points from +history and appended by `make complexity-record`. + +**Only master CI ever writes it**, which is what keeps it free. Simulating real merges over 60 +code commits, a totals file that pull requests edit conflicts on **75.4%** of median-lifetime +branches; the same file written only after merge conflicts on none, because no branch ever +touches it. + +Read the log for shape, not for precision. Step changes in it are usually imports rather than +growth — the jump from 16,188 to 22,032 on 2026-07-02 is start-wrt and start-cli arriving in the +monorepo, not a bad week. + +## Utilities + +A general-purpose utility has one call site the day it is written, so anything that charges for +a single-use function charges for the library we want, and collects in inlined helpers and +helpers bent to fit one caller. Nothing here charges for it. + +Credit, such as it is, attaches to **adoption rather than creation**: a function is named in the +report when a subsystem that did not call it before starts to, and only once the total reaches +two. The first caller is the author; the second is where generality stops being a claim. That +ordering also means writing a second utility for a job an existing one already does earns +nothing, while calling the existing one does — without anyone being penalised for either. + +Nothing mechanical catches a re-implemented utility. `jscpd` flags a renamed copy-paste at 45% +duplicated lines, but the same helper written afresh with a different signature registers zero +clones, because the duplication is semantic. Searching for an existing helper before adding one +remains a habit, not a check. + +## Known limits + +- **Angular templates are invisible.** 278 components hold their markup in inline `template:` + backticks containing 831 control-flow constructs, and those sit inside string literals that no + per-function metric sees. Sonar's analyzer has the same blind spot. +- **Macro bodies score zero**, in every tool including SonarSource's own. They are 0.19% of this + repo's Rust. +- **The metric is a proxy.** Cognitive complexity tracks nesting and branching, which is most of + what makes code hard to hold in the head, and none of what makes a name wrong, an abstraction + leaky, or an interface badly cut. +- **`bca` is four months old and carried by one maintainer.** That is the trade for it being + maintained at all; its parent's last release is January 2023 and Mozilla no longer uses it. + +## What is left to decide + +1. **Where the report reaches an agent.** A CI job can post the delta as a PR comment, or the + convention can be that whoever opens the PR runs `make complexity-diff` and pastes it. The + first is reliable and costs a job; the second costs nothing and is followed about 28% of the + time, measured against the closest existing precedent in `AGENTS.md`. +2. **Whether master CI appends to the history log**, which is the only piece that needs write + access to the repo. +3. **Whether any of this belongs in `AGENTS.md`.** It is 43 KB already, and a rule that gates + nothing competes for attention with rules that do. From 6f5f8ddaa6535ecb78878ad70d8ee96133518c08 Mon Sep 17 00:00:00 2001 From: Helix <267227783+helix-nine@users.noreply.github.com> Date: Thu, 27 Aug 2026 21:50:35 +0000 Subject: [PATCH 14/17] docs(repo): tell agents to read the complexity delta when opening a PR Three bullets under Opening PRs: run `make complexity-diff` and paste it, treat an unexpected rise as a symptom to read rather than a verdict to obey, and expect no charge for adding a utility. Nothing gates on the numbers, so the rule is only worth its space if it says plainly what to do with them and when to argue back. +2.9% on AGENTS.md. --- AGENTS.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index d1accfbea9..7f53751843 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -111,6 +111,9 @@ What `### Environment` carries, by project: - **Label every PR with the project(s) it modifies.** Nothing labels it for you: [`issue-triage.yml`](.github/workflows/issue-triage.yml) is bound to `issues:` alone, and no workflow reads a PR's diff. Pass them when you open it — `gh pr create --label StartOS --label StartSDK` — or add them after with `gh pr edit --add-label repo`. Take them from the same set as an issue, listed under [Filing issues](#filing-issues); `gh` fails on a label the repo doesn't have, and the casing is matched literally. - **The diff decides, so a PR takes as many labels as it needs.** An issue carries the one product a defect surfaces in; a PR carries every project whose files it changes, because that is what tells a reviewer and a release what a merge can break. Build, CI, and release tooling — `.github/`, `build/`, `scripts/`, `Makefile`, `debian/`, `apt/`, and the repo-root docs — is `repo`. A `shared-libs/` change has no label of its own: label the products whose behavior it changes. +- **Run `make complexity-diff` before opening a PR and paste its output into the body.** It prints what the branch did to the repo's cognitive complexity against its merge-base — the totals, every function it pushed higher, every one it simplified, and which utilities a second subsystem now depends on. Three seconds, no build. Nothing gates on the numbers; they are there to be read, and `build/complexity/history.tsv` is the same figures per master commit. +- **An unexpected rise is a symptom, not a verdict.** Read the named functions rather than the total — a function grown a branch it did not need, a helper shredded instead of extracted, a special case threaded down a call chain instead of handled at its edge. Fix what the report caught, or say in the body why the number is wrong about it: branching intrinsic to a requirement is still branching, and a protocol with nine message types has nine arms wherever it is handled. +- **Utilities are not charged for.** A general-purpose helper has one call site the day it is written, and the report names a function only once a second subsystem calls it. Nothing is owed for adding one, and nothing is earned for writing a second helper where one already exists — search before you add. ## Code style From 5ada7e9becfc4c9b3247f98c25f08cbad41632fb Mon Sep 17 00:00:00 2001 From: Helix <267227783+helix-nine@users.noreply.github.com> Date: Thu, 27 Aug 2026 22:03:11 +0000 Subject: [PATCH 15/17] docs(repo): say plainly what a complexity rise usually means 'A function grown a branch it did not need, a helper shredded instead of extracted, a special case threaded down a call chain' was three strained metaphors in one sentence. It is a condition that did not need adding, one clear function chopped into worse ones, or a special case passed down through layers that should have handled it at the top. Also drops the owed/earned framing from the utilities bullet and the tangled opening clause from the first. --- AGENTS.md | 6 +++--- rfcs/complexity-tracking.md | 9 ++++----- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 7f53751843..ed96e58075 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -111,9 +111,9 @@ What `### Environment` carries, by project: - **Label every PR with the project(s) it modifies.** Nothing labels it for you: [`issue-triage.yml`](.github/workflows/issue-triage.yml) is bound to `issues:` alone, and no workflow reads a PR's diff. Pass them when you open it — `gh pr create --label StartOS --label StartSDK` — or add them after with `gh pr edit --add-label repo`. Take them from the same set as an issue, listed under [Filing issues](#filing-issues); `gh` fails on a label the repo doesn't have, and the casing is matched literally. - **The diff decides, so a PR takes as many labels as it needs.** An issue carries the one product a defect surfaces in; a PR carries every project whose files it changes, because that is what tells a reviewer and a release what a merge can break. Build, CI, and release tooling — `.github/`, `build/`, `scripts/`, `Makefile`, `debian/`, `apt/`, and the repo-root docs — is `repo`. A `shared-libs/` change has no label of its own: label the products whose behavior it changes. -- **Run `make complexity-diff` before opening a PR and paste its output into the body.** It prints what the branch did to the repo's cognitive complexity against its merge-base — the totals, every function it pushed higher, every one it simplified, and which utilities a second subsystem now depends on. Three seconds, no build. Nothing gates on the numbers; they are there to be read, and `build/complexity/history.tsv` is the same figures per master commit. -- **An unexpected rise is a symptom, not a verdict.** Read the named functions rather than the total — a function grown a branch it did not need, a helper shredded instead of extracted, a special case threaded down a call chain instead of handled at its edge. Fix what the report caught, or say in the body why the number is wrong about it: branching intrinsic to a requirement is still branching, and a protocol with nine message types has nine arms wherever it is handled. -- **Utilities are not charged for.** A general-purpose helper has one call site the day it is written, and the report names a function only once a second subsystem calls it. Nothing is owed for adding one, and nothing is earned for writing a second helper where one already exists — search before you add. +- **Run `make complexity-diff` before opening a PR and paste its output into the body.** It compares the branch against its merge-base and prints the totals, every function it pushed higher, every one it simplified, and which utilities a second subsystem now depends on. Three seconds, no build. Nothing gates on the numbers; they are there to be read, and `build/complexity/history.tsv` is the same figures per master commit. +- **An unexpected rise is a symptom, not a verdict.** Read the named functions, not the total. It is usually a condition that did not need adding, one clear function chopped into worse ones, or a special case passed down through layers that should have handled it at the top. Fix what the report caught, or say in the body why the number is wrong: a requirement that has nine cases takes nine branches wherever it is handled, and no restructuring removes them. +- **Adding a utility costs nothing here.** A general-purpose helper has one call site the day it is written, and the report only names a function once a second subsystem calls it. Writing a second helper where one already exists therefore gains nothing either, so search before you add. ## Code style diff --git a/rfcs/complexity-tracking.md b/rfcs/complexity-tracking.md index b3910a64bf..e62b82c5ab 100644 --- a/rfcs/complexity-tracking.md +++ b/rfcs/complexity-tracking.md @@ -220,11 +220,10 @@ of them is that they look. function it pushed higher, every one it simplified, what it added to a util module, and which utilities a second subsystem now depends on. It is a mirror, not a score. -An unexpected rise is a **symptom**. It usually means one of a small number of things — a -function grew a branch it did not need, a helper was shredded rather than extracted, a special -case was threaded through a call chain instead of handled at the edge — and the per-function -lines are there so the author can tell which. The right response to a symptom is to look, and -then either fix the mess or explain why the number is wrong about it. +An unexpected rise is a **symptom**. It is usually a condition that did not need adding, one +clear function chopped into worse ones, or a special case passed down through layers that should +have handled it at the top — and the per-function lines are there so the author can tell which. +The right response is to look, then either fix the mess or explain why the number is wrong. **A rise that the author stands behind should be defensible, and the report is built to let it be defended.** Complexity that is intrinsic to a requirement is still complexity: a protocol From 346b220529c94df5937a55b3fc804aabfaa1e2b5 Mon Sep 17 00:00:00 2001 From: Helix <267227783+helix-nine@users.noreply.github.com> Date: Fri, 28 Aug 2026 00:04:39 +0000 Subject: [PATCH 16/17] feat(repo): record the census from the PR, and have CI check it ran MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An unenforced instruction to run the census gets followed about 28% of the time, measured against the closest precedent in AGENTS.md. So the PR appends its totals to the log and CI checks that a row describes the tree being shipped. That checks the census was RUN, never what it said. No value fails a build, which is what keeps the metric worth reading rather than worth gaming. Push another commit and the row goes stale, and the check says so. Requiring every PR to append to one file is normally how you manufacture the conflict this repo already gets from its append-only i18n dictionaries — two branches adding different rows at the same tail collide on every merge, which I confirmed on a scratch repo. One line of .gitattributes removes it: the log is merge=union, so git keeps both sides. Three concurrent branches each appending a distinct row merged cleanly with every row preserved. `make complexity-record` now prints the delta and appends the row in one step; `make complexity-verify` is what CI runs. --- .gitattributes | 4 ++++ .github/workflows/test.yaml | 19 +++++++++++++++++++ AGENTS.md | 2 +- Makefile | 2 +- build/complexity.mk | 10 +++++++--- build/complexity/history.tsv | 1 + build/complexity/record.sh | 15 +++++++++++---- build/complexity/verify.sh | 23 +++++++++++++++++++++++ rfcs/complexity-tracking.md | 21 ++++++++++++++------- 9 files changed, 81 insertions(+), 16 deletions(-) create mode 100644 .gitattributes create mode 100755 build/complexity/verify.sh diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000000..3b0f7b1f17 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,4 @@ +# Append-only log. Two branches appending different rows conflict on every merge +# without this; union keeps both sides, which is always the right answer for a +# file whose lines are independent records. +build/complexity/history.tsv merge=union diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 87d7ead160..fdf933dd37 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -173,3 +173,22 @@ jobs: run: make manpages-check - name: Check TS bindings up to date run: make start-core-ts-bindings-check + + # The log's last row must describe the tree being shipped, which is only true if + # the author ran `make complexity-record`. This checks that the census was run, + # never what it said — no number here fails a build. + complexity: + name: Complexity + needs: [prettier, changes] + if: >- + !cancelled() && needs.prettier.result == 'success' + && github.event.pull_request.draft != true + && (github.event_name != 'pull_request' || needs.changes.outputs.code == 'true') + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - uses: actions/setup-python@v6 + with: + python-version: '3.13' + - name: Check the complexity log describes this tree + run: make complexity-verify diff --git a/AGENTS.md b/AGENTS.md index ed96e58075..a9370f9242 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -111,7 +111,7 @@ What `### Environment` carries, by project: - **Label every PR with the project(s) it modifies.** Nothing labels it for you: [`issue-triage.yml`](.github/workflows/issue-triage.yml) is bound to `issues:` alone, and no workflow reads a PR's diff. Pass them when you open it — `gh pr create --label StartOS --label StartSDK` — or add them after with `gh pr edit --add-label repo`. Take them from the same set as an issue, listed under [Filing issues](#filing-issues); `gh` fails on a label the repo doesn't have, and the casing is matched literally. - **The diff decides, so a PR takes as many labels as it needs.** An issue carries the one product a defect surfaces in; a PR carries every project whose files it changes, because that is what tells a reviewer and a release what a merge can break. Build, CI, and release tooling — `.github/`, `build/`, `scripts/`, `Makefile`, `debian/`, `apt/`, and the repo-root docs — is `repo`. A `shared-libs/` change has no label of its own: label the products whose behavior it changes. -- **Run `make complexity-diff` before opening a PR and paste its output into the body.** It compares the branch against its merge-base and prints the totals, every function it pushed higher, every one it simplified, and which utilities a second subsystem now depends on. Three seconds, no build. Nothing gates on the numbers; they are there to be read, and `build/complexity/history.tsv` is the same figures per master commit. +- **Run `make complexity-record` before opening a PR and commit what it writes.** It compares the branch against its merge-base and prints the totals, every function it pushed higher, every one it simplified, and which utilities a second subsystem now depends on — then appends this tree's totals to [`build/complexity/history.tsv`](build/complexity/history.tsv). Three seconds, no build. CI checks that the log's last row matches the tree you are shipping, which is how it knows the census was run; it never checks what the numbers said, and no value fails a build. Re-run it if you push more commits, or the row goes stale and the check says so. The log is `merge=union`, so two branches appending different rows never conflict. - **An unexpected rise is a symptom, not a verdict.** Read the named functions, not the total. It is usually a condition that did not need adding, one clear function chopped into worse ones, or a special case passed down through layers that should have handled it at the top. Fix what the report caught, or say in the body why the number is wrong: a requirement that has nine cases takes nine branches wherever it is handled, and no restructuring removes them. - **Adding a utility costs nothing here.** A general-purpose helper has one call site the day it is written, and the report only names a function once a second subsystem calls it. Writing a second helper where one already exists therefore gains nothing either, so search before you add. diff --git a/Makefile b/Makefile index 2f5d7c0178..3a860c7240 100644 --- a/Makefile +++ b/Makefile @@ -16,7 +16,7 @@ include projects/start-wrt/build.mk include projects/start-docs/build.mk include build/complexity.mk -.PHONY: help start-os metadata start-os-install clean format format-check complexity complexity-top complexity-diff complexity-record start-cli-install start-cli start-cli-deb start-os-uis start-os-ui start-os-emulate-reflash start-os-deb start-os-$(IMAGE_TYPE) start-os-squashfs start-os-wormhole start-os-wormhole-deb start-os-update start-os-update-from-gha test start-core-test start-sdk-test container-runtime-test start-wrt-test start-registry start-registry-install start-tunnel start-tunnel-install start-core-ts-bindings +.PHONY: help start-os metadata start-os-install clean format format-check complexity complexity-top complexity-diff complexity-record complexity-verify start-cli-install start-cli start-cli-deb start-os-uis start-os-ui start-os-emulate-reflash start-os-deb start-os-$(IMAGE_TYPE) start-os-squashfs start-os-wormhole start-os-wormhole-deb start-os-update start-os-update-from-gha test start-core-test start-sdk-test container-runtime-test start-wrt-test start-registry start-registry-install start-tunnel start-tunnel-install start-core-ts-bindings help: @echo "No default target — specify one. Common targets:" diff --git a/build/complexity.mk b/build/complexity.mk index f65cfdbc5b..5de8016b1a 100644 --- a/build/complexity.mk +++ b/build/complexity.mk @@ -2,7 +2,7 @@ COMPLEXITY := ./build/complexity/census.sh BASE ?= origin/master -.PHONY: complexity complexity-top complexity-diff complexity-record +.PHONY: complexity complexity-top complexity-diff complexity-record complexity-verify # Totals plus the worst 25 functions in the tree. complexity: @@ -16,6 +16,10 @@ complexity-top: complexity-diff: @$(COMPLEXITY) diff $(BASE) -# Appends one row for HEAD to the log. Master CI runs this; a PR never writes it. +# Prints the delta and appends this tree's totals to the log. Run before opening a PR. complexity-record: - @./build/complexity/record.sh $(LOG) + @./build/complexity/record.sh + +# Fails when no row in the log describes this tree. What CI checks. +complexity-verify: + @./build/complexity/verify.sh diff --git a/build/complexity/history.tsv b/build/complexity/history.tsv index d9f025a50f..6338278d56 100644 --- a/build/complexity/history.tsv +++ b/build/complexity/history.tsv @@ -27,3 +27,4 @@ b73c634d3 2026-08-17 16310 23124 38714 192652 115 c0da6d00c 2026-08-24 16669 23572 39532 196482 115 974587948 2026-08-25 16671 23578 39543 196527 114 18490269c 2026-08-26 16723 23639 39654 197055 116 +5ada7e9be 2026-08-27 16718 23639 39649 196981 116 diff --git a/build/complexity/record.sh b/build/complexity/record.sh index 6ca4dc0b2f..76594c8e0d 100755 --- a/build/complexity/record.sh +++ b/build/complexity/record.sh @@ -1,15 +1,22 @@ #!/bin/bash -# Appends one row for HEAD to the complexity log. Only master CI runs this, so a -# pull request never edits the file and never conflicts on it. +# Prints this branch's complexity delta and appends its totals to the history log. +# The log is merge=union, so two branches appending different rows never conflict. set -euo pipefail HERE="$(cd "$(dirname "$0")" && pwd)" -LOG="${1:-build/complexity/history.tsv}" +LOG="${LOG:-build/complexity/history.tsv}" +BASE="${BASE:-origin/master}" [ -x "$HERE/bin/bca" ] || "$HERE/fetch-tool.sh" +export BCA="$HERE/bin/bca" + +"$HERE/census.sh" diff "$BASE" || true + read -r functions cognitive cyclomatic sloc over25 < <( - BCA="$HERE/bin/bca" python3 "$HERE/census.py" --root . --json \ + python3 "$HERE/census.py" --root . --json \ | python3 -c 'import json,sys; t=json.load(sys.stdin)["totals"]; print(t["functions"],t["cognitive"],t["cyclomatic"],t["sloc"],t["over25"])' ) [ -s "$LOG" ] || printf 'commit\tdate\tfunctions\tcognitive\tcyclomatic\tsloc\tover25\n' > "$LOG" printf '%s\t%s\t%s\t%s\t%s\t%s\t%s\n' \ "$(git rev-parse --short HEAD)" "$(git log -1 --format=%cs)" \ "$functions" "$cognitive" "$cyclomatic" "$sloc" "$over25" >> "$LOG" +echo +echo "recorded $functions/$cognitive/$cyclomatic/$sloc/$over25 to $LOG" diff --git a/build/complexity/verify.sh b/build/complexity/verify.sh new file mode 100755 index 0000000000..5eec7e1681 --- /dev/null +++ b/build/complexity/verify.sh @@ -0,0 +1,23 @@ +#!/bin/bash +# Fails when the log's last row does not describe the working tree. Proves the +# author ran the census on what they are actually shipping — it says nothing +# about whether the numbers went up. +set -euo pipefail +HERE="$(cd "$(dirname "$0")" && pwd)" +LOG="${LOG:-build/complexity/history.tsv}" +[ -x "$HERE/bin/bca" ] || "$HERE/fetch-tool.sh" +export BCA="$HERE/bin/bca" + +read -r functions cognitive cyclomatic sloc over25 < <( + python3 "$HERE/census.py" --root . --json \ + | python3 -c 'import json,sys; t=json.load(sys.stdin)["totals"]; print(t["functions"],t["cognitive"],t["cyclomatic"],t["sloc"],t["over25"])' +) +fresh="$functions $cognitive $cyclomatic $sloc $over25" +if ! cut -f3-7 "$LOG" | grep -qxF "$fresh"; then + echo "complexity: no row in $LOG describes this tree." >&2 + echo " tree: $fresh" >&2 + echo " last row: $(tail -1 "$LOG" | cut -f3-7)" >&2 + echo "Run 'make complexity-record' and commit the result." >&2 + exit 1 +fi +echo "complexity: the log describes this tree ($fresh)" diff --git a/rfcs/complexity-tracking.md b/rfcs/complexity-tracking.md index e62b82c5ab..84d24142a4 100644 --- a/rfcs/complexity-tracking.md +++ b/rfcs/complexity-tracking.md @@ -234,16 +234,23 @@ one thing that would make it a verdict is a gate. ## Tracking across changes -`build/complexity/history.tsv` holds one row per master commit: commit, date, functions, +`build/complexity/history.tsv` holds one row per recorded tree: commit, date, functions, cognitive, cyclomatic, sloc, and the count over 25. It is seeded with 28 sampled points from -history and appended by `make complexity-record`. +history. `make complexity-record` prints the delta and appends a row; whoever opens a PR runs it +and commits the result. -**Only master CI ever writes it**, which is what keeps it free. Simulating real merges over 60 -code commits, a totals file that pull requests edit conflicts on **75.4%** of median-lifetime -branches; the same file written only after merge conflicts on none, because no branch ever -touches it. +**CI checks that a row describes the tree being shipped, and nothing else.** That is the whole +enforcement: it proves the census was run on what is actually being merged. It does not look at +whether the numbers went up, because a threshold is the thing that would make the metric worth +gaming. Push another commit and the row goes stale, and the check says so. -Read the log for shape, not for precision. Step changes in it are usually imports rather than +Requiring every PR to append to one file is normally how you manufacture the conflict this repo +already suffers from its append-only i18n dictionaries — two branches adding different rows at +the same tail collide on every merge, which I confirmed on a scratch repo. One line of +`.gitattributes` removes it: the log is `merge=union`, so git keeps both sides. Three concurrent +branches each appending a distinct row merged cleanly, with all rows preserved. + +Read the log for shape, not precision. Step changes in it are usually imports rather than growth — the jump from 16,188 to 22,032 on 2026-07-02 is start-wrt and start-cli arriving in the monorepo, not a bad week. From a3d9d99ce5557f4fd623422fec7e18645080825d Mon Sep 17 00:00:00 2001 From: Helix <267227783+helix-nine@users.noreply.github.com> Date: Fri, 28 Aug 2026 19:53:12 +0000 Subject: [PATCH 17/17] fix(ci): measure the branch head, and compare only the last row MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first green run of the Complexity job passed for the wrong reason. A pull_request event checks out head merged into the current base, so with master seven commits ahead CI measured a tree the author never saw — and its figures happened to match a row backfilled from master two days earlier. Green, and meaningless. Two fixes. The job pins ref to the pull request's head sha, so it measures what the author measured; numbers from a merge commit are numbers nobody could have recorded. And verify compares the last row instead of searching the whole log, so a coincidental match against any historical row can no longer pass it. Confirmed both ways: a stale last row now exits 1 naming both figures, and the correct row exits 0. --- .github/workflows/test.yaml | 5 +++++ build/complexity/verify.sh | 9 +++++---- rfcs/complexity-tracking.md | 8 ++++++++ 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index fdf933dd37..e2ed867317 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -186,7 +186,12 @@ jobs: && (github.event_name != 'pull_request' || needs.changes.outputs.code == 'true') runs-on: ubuntu-latest steps: + # The author recorded their branch, so measure that. A pull_request event + # checks out head merged into the current base by default, and its numbers + # move whenever master does — numbers no author can predict. - uses: actions/checkout@v6 + with: + ref: ${{ github.event.pull_request.head.sha || github.sha }} - uses: actions/setup-python@v6 with: python-version: '3.13' diff --git a/build/complexity/verify.sh b/build/complexity/verify.sh index 5eec7e1681..549ba1aac0 100755 --- a/build/complexity/verify.sh +++ b/build/complexity/verify.sh @@ -13,11 +13,12 @@ read -r functions cognitive cyclomatic sloc over25 < <( | python3 -c 'import json,sys; t=json.load(sys.stdin)["totals"]; print(t["functions"],t["cognitive"],t["cyclomatic"],t["sloc"],t["over25"])' ) fresh="$functions $cognitive $cyclomatic $sloc $over25" -if ! cut -f3-7 "$LOG" | grep -qxF "$fresh"; then - echo "complexity: no row in $LOG describes this tree." >&2 +last="$(tail -1 "$LOG" | cut -f3-7)" +if [ "$last" != "$fresh" ]; then + echo "complexity: the last row of $LOG does not describe this tree." >&2 echo " tree: $fresh" >&2 - echo " last row: $(tail -1 "$LOG" | cut -f3-7)" >&2 + echo " last row: $last" >&2 echo "Run 'make complexity-record' and commit the result." >&2 exit 1 fi -echo "complexity: the log describes this tree ($fresh)" +echo "complexity: the last row describes this tree ($fresh)" diff --git a/rfcs/complexity-tracking.md b/rfcs/complexity-tracking.md index 84d24142a4..8527f5d1cf 100644 --- a/rfcs/complexity-tracking.md +++ b/rfcs/complexity-tracking.md @@ -239,6 +239,14 @@ cognitive, cyclomatic, sloc, and the count over 25. It is seeded with 28 sampled history. `make complexity-record` prints the delta and appends a row; whoever opens a PR runs it and commits the result. +**The check reads the branch head, not the merge commit.** A `pull_request` event checks out +head merged into the current base by default, so its numbers move every time master does — +numbers no author could have recorded. The first green run of this job proved the point by +passing for the wrong reason: master had moved seven commits, CI measured a tree the author +never saw, and its figures happened to match a backfilled row from two days earlier. The job +now pins `ref: github.event.pull_request.head.sha`, and the check compares the **last** row +rather than searching the whole log, so a coincidental historical match cannot pass it. + **CI checks that a row describes the tree being shipped, and nothing else.** That is the whole enforcement: it proves the census was run on what is actually being merged. It does not look at whether the numbers went up, because a threshold is the thing that would make the metric worth