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..e2ed867317 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -173,3 +173,27 @@ 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: + # 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' + - name: Check the complexity log describes this tree + run: make complexity-verify diff --git a/AGENTS.md b/AGENTS.md index d1accfbea9..a9370f9242 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-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. ## Code style diff --git a/Makefile b/Makefile index b3888597c9..3a860c7240 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-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:" @@ -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..5de8016b1a --- /dev/null +++ b/build/complexity.mk @@ -0,0 +1,25 @@ +# --- 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-record complexity-verify + +# 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) + +# Prints the delta and appends this tree's totals to the log. Run before opening a PR. +complexity-record: + @./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/.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..aade99f3eb --- /dev/null +++ b/build/complexity/census.py @@ -0,0 +1,153 @@ +#!/usr/bin/env python3 +"""Per-function cognitive complexity for the tree, as JSON or a table.""" +import argparse, collections, json, os, re, subprocess, sys, tempfile + +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*[(:<]') +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): + 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 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, bca): + out = tempfile.mkdtemp(prefix='cx-') + 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: + 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): + """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)): + 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) + rel = os.path.relpath(path, root) + if not kept(rel): + continue + try: + 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) + 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]) + 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): + """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), + 'cyclomatic': sum(r['cyclomatic'] 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('--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 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) + return + t = totals(rows) + print(f"functions {t['functions']} cognitive {t['cognitive']} cyclomatic {t['cyclomatic']} " + 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 new file mode 100755 index 0000000000..d195052704 --- /dev/null +++ b/build/complexity/census.sh @@ -0,0 +1,23 @@ +#!/bin/bash +# 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}" + +[ -x "$BCA" ] || "$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" + ;; + *) echo "usage: census.sh {census|top [n]|diff }" >&2; exit 2 ;; +esac diff --git a/build/complexity/delta.py b/build/complexity/delta.py new file mode 100644 index 0000000000..25fe4a9ead --- /dev/null +++ b/build/complexity/delta.py @@ -0,0 +1,72 @@ +#!/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'), + ('cyclomatic', 'cyclomatic'), ('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") + +# 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. +# 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 ()) + 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: + print(f" of the new functions, {len(util_new)} sit in util modules" + f" ({sum(r['cognitive'] for r in util_new)} cognitive)") + +private = [r for r in new + 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/build/complexity/fetch-tool.sh b/build/complexity/fetch-tool.sh new file mode 100755 index 0000000000..f1465a767b --- /dev/null +++ b/build/complexity/fetch-tool.sh @@ -0,0 +1,18 @@ +#!/bin/bash +# Fetches the pinned big-code-analysis binary and verifies it against the release checksum. +set -euo pipefail +HERE="$(cd "$(dirname "$0")" && pwd)" +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" +install -m 0755 "$tmp/big-code-analysis-$VERSION-$TRIPLE/bca" "$HERE/bin/bca" diff --git a/build/complexity/history.tsv b/build/complexity/history.tsv new file mode 100644 index 0000000000..6338278d56 --- /dev/null +++ b/build/complexity/history.tsv @@ -0,0 +1,30 @@ +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 +5ada7e9be 2026-08-27 16718 23639 39649 196981 116 diff --git a/build/complexity/record.sh b/build/complexity/record.sh new file mode 100755 index 0000000000..76594c8e0d --- /dev/null +++ b/build/complexity/record.sh @@ -0,0 +1,22 @@ +#!/bin/bash +# 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="${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 < <( + 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..549ba1aac0 --- /dev/null +++ b/build/complexity/verify.sh @@ -0,0 +1,24 @@ +#!/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" +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: $last" >&2 + echo "Run 'make complexity-record' and commit the result." >&2 + exit 1 +fi +echo "complexity: the last row describes this tree ($fresh)" diff --git a/rfcs/complexity-tracking.md b/rfcs/complexity-tracking.md new file mode 100644 index 0000000000..8527f5d1cf --- /dev/null +++ b/rfcs/complexity-tracking.md @@ -0,0 +1,304 @@ +# Complexity tracking + +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 + +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 `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: + +| 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 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. + +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 +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. + +## 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 + +| 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 + 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 +``` + +The closure at `net/host/address.rs:472` outranks every named function but four. + +## The tooling + +`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: + +``` +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. + +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 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 +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 recorded tree: commit, date, functions, +cognitive, cyclomatic, sloc, and the count over 25. It is seeded with 28 sampled points from +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 +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 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. + +## 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.