From ee2815754d76366a3ef6e1b90bfbc15d895ca77f Mon Sep 17 00:00:00 2001 From: Rikard Blixt Date: Mon, 7 Sep 2026 12:06:28 +0200 Subject: [PATCH 1/7] chore: fix luacov config and add manual coverage script The luacov config has been dead since #5596 ("wikis top level", 2025-03-10) moved lua/standard/* to lua/wikis/commons/standard/*. Its `include` still pointed at `standard/*`, which no longer matches anything, while `exclude = {'wikis/*'}` covered the entire codebase. With `deletestats = true` on top, every run produced an empty stats file, so no coverage was ever measured. Point include/exclude at the current layout, scoped to lua/wikis/commons, and keep `includeuntestedfiles` so modules no spec loads count as 0% rather than being dropped from the total. Add scripts/metrics/coverage.py to run the suite under coverage and print the summary. --skip-run reuses an existing report, worth having when a full run is ~25s against ~1s for the suite alone. Also un-anchor the luacov entries in .gitignore. Output lands in lua/ because busted runs with `-C lua`, so the root-anchored patterns never matched it. Co-Authored-By: Claude Opus 5 --- .gitignore | 5 +- lua/.luacov | 18 +++++-- scripts/metrics/coverage.py | 103 ++++++++++++++++++++++++++++++++++++ 3 files changed, 119 insertions(+), 7 deletions(-) create mode 100644 scripts/metrics/coverage.py diff --git a/.gitignore b/.gitignore index 3ce76ade683..fe9b39aa0c9 100644 --- a/.gitignore +++ b/.gitignore @@ -1,12 +1,11 @@ /.idea/ -/.claude/settings.local.json /cookies /node_modules /lua/output /lua/spec/snapshots/diffs /package-lock.json -/luacov.report.out -/luacov.stats.out +luacov.report.out +luacov.stats.out **/__pycache__/ .env **/*.ck diff --git a/lua/.luacov b/lua/.luacov index 369a7ea5610..3d43c494adc 100644 --- a/lua/.luacov +++ b/lua/.luacov @@ -1,8 +1,18 @@ return { configfile = '.luacov', - includeuntestedfiles = {'standard'}, - include = {'standard/*'}, - exclude = {'wikis/*'}, + -- Report modules no spec ever loads as 0%, so coverage reflects all of commons + -- rather than only the files the suite happens to touch. + includeuntestedfiles = {'wikis/commons'}, + include = {'wikis/commons/'}, + exclude = { + 'spec/', + 'definitions/', + '3rd/', + 'plugins/', + 'test_assets/', + -- ScribuntoUnit testcases that run on-wiki, not under busted. + 'wikis/commons/test/', + }, runreport = false, -- Don't work properly for some reason, have to do a manual `luacov` call afterwards - deletestats = true, + deletestats = false, -- Keep stats so the report can be regenerated / inspected after a run } diff --git a/scripts/metrics/coverage.py b/scripts/metrics/coverage.py new file mode 100644 index 00000000000..086877698a6 --- /dev/null +++ b/scripts/metrics/coverage.py @@ -0,0 +1,103 @@ +"""Run the busted suite under luacov and report coverage of lua/wikis/commons. + +Coverage instrumentation is a per-line debug hook, so the suite takes roughly +20x longer than normal -- expect a couple of minutes rather than a second. + +Needs busted and luacov on PATH, both for Lua 5.1: + + luarocks install --lua-version=5.1 busted + luarocks install --lua-version=5.1 luacov + +Scope and exclusions come from lua/.luacov. Modules no spec ever loads count as +0% rather than being left out, so the figure covers all of commons rather than +only the files the suite happens to touch. + +Usage: + python scripts/metrics/coverage.py [--skip-run] +""" + +import argparse +import pathlib +import re +import subprocess +import sys + +REPO_ROOT = pathlib.Path(__file__).resolve().parents[2] +LUA_DIR = REPO_ROOT / "lua" +REPORT = LUA_DIR / "luacov.report.out" + +TOTAL = re.compile(r"^Total\s+(\d+)\s+(\d+)\s+([\d.]+)%") +FILE_ROW = re.compile(r"^wikis/commons/\S+\s+\d+\s+\d+\s+([\d.]+)%$") + + +INSTALL_HINT = ( + "not found on PATH. Install both for Lua 5.1:\n" + " luarocks install --lua-version=5.1 busted\n" + " luarocks install --lua-version=5.1 luacov" +) + + +def run(command, cwd): + try: + subprocess.run(command, cwd=cwd, check=True) + except FileNotFoundError: + raise SystemExit(f"{command[0]} {INSTALL_HINT}") from None + except subprocess.CalledProcessError as error: + raise SystemExit(f"{command[0]} failed (exit {error.returncode})") from None + + +def run_suite(): + """Run the test suite under coverage, then render the luacov report.""" + run(["busted", "-C", "lua", "--run=ci", "-c"], REPO_ROOT) + run(["luacov"], LUA_DIR) + + +def parse(report): + lines = report.read_text(encoding="utf-8", errors="replace").split("\n") + total = next((TOTAL.match(line) for line in lines if TOTAL.match(line)), None) + if total is None: + raise SystemExit(f"no Total line in {report}; did luacov run?") + + shares = [m.group(1) for m in map(FILE_ROW.match, lines) if m] + return { + "hits": int(total.group(1)), + "missed": int(total.group(2)), + "coverage_pct": float(total.group(3)), + "files": len(shares), + "uncovered": sum(1 for s in shares if float(s) == 0), + } + + +def table(m): + return [ + "| Coverage (lua/wikis/commons) | |", + "|-|-|", + f"| Coverage | {m['coverage_pct']:.2f}% |", + f"| Lines hit | {m['hits']} |", + f"| Lines missed | {m['missed']} |", + f"| Files | {m['files']} |", + f"| Files at 0% | {m['uncovered']} |", + ] + + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument( + "--skip-run", + action="store_true", + help="reuse the existing luacov report instead of re-running the suite", + ) + args = parser.parse_args() + + if not args.skip_run: + run_suite() + elif not REPORT.exists(): + print(f"::error::no report at {REPORT}; run without --skip-run first") + return 1 + + print("\n".join(table(parse(REPORT)))) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) From 58b7dabdaeca1d09be24ce7c30827a4e47c87c9f Mon Sep 17 00:00:00 2001 From: Rikard Blixt Date: Mon, 7 Sep 2026 12:06:29 +0200 Subject: [PATCH 2/7] chore: add manual annotation coverage script Share of exported functions in lua/wikis/commons whose doc block carries @param or @return. At ~96% it works as a regression guard rather than a growth target: the useful signal is whether new unannotated functions appear. Functions taking no parameters and returning no value need neither tag, so they are reported as exempt and left out of the ratio rather than counted against it. Whether a body returns a value is a heuristic scan to the `end` at the function's own indentation, biased toward asking for an annotation rather than excusing one. Co-Authored-By: Claude Opus 5 --- scripts/metrics/annotation_coverage.py | 165 +++++++++++++++++++++++++ 1 file changed, 165 insertions(+) create mode 100644 scripts/metrics/annotation_coverage.py diff --git a/scripts/metrics/annotation_coverage.py b/scripts/metrics/annotation_coverage.py new file mode 100644 index 00000000000..381a877555b --- /dev/null +++ b/scripts/metrics/annotation_coverage.py @@ -0,0 +1,165 @@ +"""Measure LuaLS annotation coverage of exported functions in lua/wikis/commons. + +Annotations are what make commons modules statically understandable, so this +tracks how much of the exported surface carries them. + +A function counts as annotated when the contiguous comment block directly above +it mentions `@param` or `@return`. Only `function M.name(...)` and +`function M:name(...)` forms are counted -- that is the shape the doc-block +convention applies to in this codebase. + +A function taking no parameters and returning no value needs neither tag, so it +is reported as exempt and left out of the ratio rather than counted against it. +Whether a body returns a value is decided by scanning to the `end` at the +function's own indentation, which is a heuristic: when in doubt it assumes a +value is returned, so a function is more likely to be asked for an annotation +than excused from one. + +Prints a Markdown table, optionally with deltas against a second tree. Pass +--raw for `key=value` output instead. + +Usage: + python scripts/metrics/annotation_coverage.py [root] [--base BASE_ROOT] [--raw] +""" + +import argparse +import pathlib +import re +import sys + +FUNCTION = re.compile(r"^(\s*)function\s+[A-Za-z_][\w.]*[.:]\w+\s*\(([^)]*)\)") +RETURNS_VALUE = re.compile(r"\breturn\s+\S") +DEFAULT_ROOT = "lua/wikis/commons" + + +def doc_block(lines, index): + """Return the contiguous comment block immediately above lines[index].""" + block = [] + cursor = index - 1 + while cursor >= 0 and lines[cursor].lstrip().startswith("--"): + block.append(lines[cursor]) + cursor -= 1 + return "\n".join(block) + + +def returns_value(lines, index, indent): + """Whether the function starting at lines[index] returns a value.""" + end = re.compile(rf"^{indent}end\b") + for line in lines[index + 1 :]: + if end.match(line): + return False + if RETURNS_VALUE.search(line): + return True + return True # no clear end found: assume it returns, so we still ask for a tag + + +def measure(root): + functions = annotated = exempt = params_total = params_annotated = 0 + + for path in sorted(root.rglob("*.lua")): + # ScribuntoUnit testcases run on-wiki and are not part of the surface. + if "test" in path.parts: + continue + lines = path.read_text(encoding="utf-8", errors="replace").split("\n") + for index, line in enumerate(lines): + match = FUNCTION.match(line) + if not match: + continue + indent, raw = match.group(1), match.group(2) + params = [p.strip() for p in raw.split(",") if p.strip()] + params = [p for p in params if p != "self"] + block = doc_block(lines, index) + + functions += 1 + if "@param" in block or "@return" in block: + annotated += 1 + elif not params and not returns_value(lines, index, indent): + exempt += 1 + if params: + params_total += 1 + if block.count("@param") >= len(params): + params_annotated += 1 + + needs = functions - exempt + return { + "functions": functions, + "annotated": annotated, + "exempt": exempt, + "needs_annotation": needs, + "unannotated": needs - annotated, + "coverage_pct": round(annotated / needs * 100, 2) if needs else 0.0, + "params_total": params_total, + "params_annotated": params_annotated, + "params_pct": ( + round(params_annotated / params_total * 100, 2) if params_total else 0.0 + ), + } + + +def count_cell(value, base): + return f"{value}" if base is None else f"{value} ({value - base:+d})" + + +def pct_cell(value, base): + if base is None: + return f"{value:.2f}%" + return f"{value:.2f}% ({value - base:+.2f} pp)" + + +def table(head, base): + def of(key): + return None if base is None else base[key] + + rows = [ + ( + "Exported functions annotated", + pct_cell(head["coverage_pct"], of("coverage_pct")), + ), + ( + "Needs annotation", + count_cell(head["needs_annotation"], of("needs_annotation")), + ), + ("Unannotated", count_cell(head["unannotated"], of("unannotated"))), + ("Exempt (no params, no return)", count_cell(head["exempt"], of("exempt"))), + ("Exported functions", count_cell(head["functions"], of("functions"))), + ( + "Functions with all params annotated", + pct_cell(head["params_pct"], of("params_pct")), + ), + ] + lines = ["| Annotations (lua/wikis/commons) | |", "|-|-|"] + lines += [f"| {label} | {value} |" for label, value in rows] + return lines + + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument("root", nargs="?", type=pathlib.Path, default=DEFAULT_ROOT) + parser.add_argument( + "--base", type=pathlib.Path, help="second tree to compare against" + ) + parser.add_argument("--raw", action="store_true", help="print key=value instead") + args = parser.parse_args() + + if not args.root.is_dir(): + print(f"::error::annotation root not found: {args.root}") + return 1 + + head = measure(args.root) + if args.raw: + for key, value in head.items(): + print(f"{key}={value}") + return 0 + + base = None + if args.base: + if args.base.is_dir(): + base = measure(args.base) + else: + print(f"::warning::base root not found, omitting deltas: {args.base}") + print("\n".join(table(head, base))) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) From a508d486004dd4337c823e611ccb811a107129fb Mon Sep 17 00:00:00 2001 From: Rikard Blixt Date: Mon, 7 Sep 2026 12:06:29 +0200 Subject: [PATCH 3/7] chore: add manual per-wiki customization script Everything in this repo is standardized; what varies is how much a wiki has to override to get the behaviour it wants. Override code is the part that carries maintenance cost, so it is the number worth watching. Declarative data/config and legacy shims are counted separately -- a wiki adding 500 lines of faction data is not the same event as one adding 500 lines of overrides. Classification is by file content, not filename. GetMatchGroupCopyPaste/ wiki.lua, FilterButtons/Config.lua and NotabilityChecker/config.lua all read as declarative from their names and are not. Shares are against all of lua/wikis so "override code is 37.5% of all Lua" is answerable. Also add the missing final newline to Widget/Table2/Row.lua, the only file in the repo without one. It changes no count, but it did make a cat-based cross-check of the commons total read one line short. Co-Authored-By: Claude Opus 5 --- .gitignore | 1 + lua/wikis/commons/Widget/Table2/Row.lua | 2 +- scripts/metrics/customization_metrics.py | 169 +++++++++++++++++++++++ 3 files changed, 171 insertions(+), 1 deletion(-) create mode 100644 scripts/metrics/customization_metrics.py diff --git a/.gitignore b/.gitignore index fe9b39aa0c9..9fec2ee4305 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ /.idea/ +/.claude/settings.local.json /cookies /node_modules /lua/output diff --git a/lua/wikis/commons/Widget/Table2/Row.lua b/lua/wikis/commons/Widget/Table2/Row.lua index 31e8ec2bff0..929eb7bf464 100644 --- a/lua/wikis/commons/Widget/Table2/Row.lua +++ b/lua/wikis/commons/Widget/Table2/Row.lua @@ -136,4 +136,4 @@ end return Component.component( Table2Row -) \ No newline at end of file +) diff --git a/scripts/metrics/customization_metrics.py b/scripts/metrics/customization_metrics.py new file mode 100644 index 00000000000..16b6497fad3 --- /dev/null +++ b/scripts/metrics/customization_metrics.py @@ -0,0 +1,169 @@ +"""Measure how much per-wiki customization code lives outside lua/wikis/commons. + +Everything in this repo is standardized; what varies is how much a wiki has to +override to get the behaviour it wants. Override code is the part that costs +maintenance and has to be carried forward, so it is the number worth watching. +Declarative data and config, and legacy shims, are reported separately. + +Classification is by file *content*, not filename -- `GetMatchGroupCopyPaste/ +wiki.lua`, `FilterButtons/Config.lua` and `NotabilityChecker/config.lua` all +look declarative and are not. A file is override code when it defines a +non-local function, or when a `local function` is exported via a `return` +statement (the widget pattern: `local function X` ... `return wrap(X)`). +Purely-local helpers inside an otherwise declarative file do not promote it. + +Shares are against the whole of lua/wikis (commons included), so "override code +is 37% of all Lua" is answerable. Vendored code, type stubs, specs and test +assets under lua/ are excluded from the denominator -- they are not the product. + +Prints a Markdown table, optionally with deltas against a second tree. Pass +--raw for `key=value` output instead. + +Usage: + python scripts/metrics/customization_metrics.py [wikis-root] [--base BASE_ROOT] [--raw] +""" + +import argparse +import pathlib +import re +import sys + +DEFAULT_ROOT = "lua/wikis" +COMMONS = "commons" + +EXPORTED_FUNCTION = re.compile(r"\bfunction\b") +LOCAL_FUNCTION = re.compile( + r"^[ \t]*local[ \t]+(?:function|[A-Za-z_][A-Za-z0-9_]*[ \t]*=[ \t]*function)" +) +LOCAL_FUNCTION_NAME = re.compile(r"^[ \t]*local[ \t]+function[ \t]+([A-Za-z_]\w*)") +RETURN_STATEMENT = re.compile(r"^[ \t]*return\b") + + +def is_override_code(lines): + """True when the file exposes behaviour rather than just data or config.""" + local_names = [] + returns = [] + for line in lines: + if not EXPORTED_FUNCTION.search(line): + if RETURN_STATEMENT.match(line): + returns.append(line) + continue + if not LOCAL_FUNCTION.match(line): + return True # a non-local function definition + name = LOCAL_FUNCTION_NAME.match(line) + if name: + local_names.append(name.group(1)) + + # A local function handed out through `return` is the module's interface. + return any(name in line for line in returns for name in local_names) + + +def measure(root): + declarative = legacy = override = commons = 0 + + for path in sorted(root.rglob("*.lua")): + text = path.read_text(encoding="utf-8", errors="replace") + count = len(text.splitlines()) + + # commons is the shared implementation, not per-wiki customization. It is + # still counted, so shares have the whole of lua/wikis as denominator. + if COMMONS in path.parts: + commons += count + # Substring, not a path component: legacy lives in directories + # (TeamCard/Legacy/Custom.lua) *and* in filenames (Match/Legacy.lua). + # `"Legacy" in path.parts` would miss the latter -- 52 of 78 files, + # moving 6279 lines out of legacy and into override code. + elif "Legacy" in path.as_posix(): + legacy += count + elif is_override_code(text.splitlines()): + override += count + else: + declarative += count + + per_wiki = override + declarative + legacy + total = per_wiki + commons + + def share(count): + return round(count / total * 100, 2) if total else 0.0 + + return { + "override_code": override, + "declarative": declarative, + "legacy": legacy, + "per_wiki_total": per_wiki, + "commons": commons, + "total": total, + "override_code_pct": share(override), + "declarative_pct": share(declarative), + "legacy_pct": share(legacy), + "per_wiki_total_pct": share(per_wiki), + "commons_pct": share(commons), + } + + +def count_cell(value, base): + return f"{value}" if base is None else f"{value} ({value - base:+d})" + + +def pct_cell(value, base): + if base is None: + return f"{value:.2f}%" + return f"{value:.2f}% ({value - base:+.2f} pp)" + + +def table(head, base): + def of(key): + return None if base is None else base[key] + + rows = [ + ("**Override code**", "override_code", "override_code_pct"), + ("Declarative (data + config)", "declarative", "declarative_pct"), + ("Legacy", "legacy", "legacy_pct"), + ("Per-wiki total", "per_wiki_total", "per_wiki_total_pct"), + ("lua/wikis/commons (shared)", "commons", "commons_pct"), + ] + lines = [ + "| Per-wiki customization | LOC | Share of all Lua |", + "|-|-|-|", + ] + for label, loc_key, pct_key in rows: + loc = count_cell(head[loc_key], of(loc_key)) + pct = pct_cell(head[pct_key], of(pct_key)) + if label.startswith("**"): + loc, pct = f"**{loc}**", f"**{pct}**" + lines.append(f"| {label} | {loc} | {pct} |") + lines.append(f"| All of lua/wikis | {count_cell(head['total'], of('total'))} | |") + return lines + + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument("root", nargs="?", type=pathlib.Path, default=DEFAULT_ROOT) + parser.add_argument( + "--base", type=pathlib.Path, help="second tree to compare against" + ) + parser.add_argument("--raw", action="store_true", help="print key=value instead") + args = parser.parse_args() + + if not args.root.is_dir(): + print(f"::error::wikis root not found: {args.root}") + return 1 + + head = measure(args.root) + if args.raw: + for key, value in head.items(): + print(f"{key}={value}") + return 0 + + base = None + if args.base: + if args.base.is_dir(): + base = measure(args.base) + else: + print(f"::warning::base root not found, omitting deltas: {args.base}") + print("\n".join(table(head, base))) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) From 35254230342fda0abff701d708aedaf32286bb3b Mon Sep 17 00:00:00 2001 From: Rikard Blixt Date: Mon, 7 Sep 2026 12:21:22 +0200 Subject: [PATCH 4/7] chore: match the existing metrics conventions, add a README scripts/metrics/ already had three scripts from #7939 and I had not matched their conventions. Bring the new three in line: - numbered as metrics 4-6, following 1-3 - `#!/usr/bin/env python3` - --csv / --no-header with a leading date column, so all six can be appended to a time-series file. This is what the folder is for, and it is the trend history the markdown-table output could not give. - plain aligned tables by default, not markdown. The markdown was for $GITHUB_STEP_SUMMARY, which these no longer write to. - `from pathlib import Path`, type hints Metric 4 now reports loc as metric 1 defines it -- non-blank, non-comment-only lines -- alongside total physical lines. Previously it counted physical lines and called them LOC, so two scripts in the same folder measured the same code differently. Totals now reconcile exactly with metric 1: 1651 files, 203738 lines, 146516 loc. Metric 4 decomposes metric 1 rather than disagreeing with it, and the share column is of loc. Avoided `X | None` annotations: they need python 3.10, and while the workflows pin 3.14 the scripts should still run on an older local interpreter. README covers all six, what each answers, and what they need to run. Co-Authored-By: Claude Opus 5 --- scripts/metrics/README.md | 47 +++++ scripts/metrics/annotation_coverage.py | 112 +++++------ scripts/metrics/coverage.py | 64 ++++--- scripts/metrics/customization_metrics.py | 229 ++++++++++++----------- 4 files changed, 267 insertions(+), 185 deletions(-) create mode 100644 scripts/metrics/README.md mode change 100644 => 100755 scripts/metrics/annotation_coverage.py mode change 100644 => 100755 scripts/metrics/coverage.py mode change 100644 => 100755 scripts/metrics/customization_metrics.py diff --git a/scripts/metrics/README.md b/scripts/metrics/README.md new file mode 100644 index 00000000000..0935bb7bc2e --- /dev/null +++ b/scripts/metrics/README.md @@ -0,0 +1,47 @@ +# Metrics + +Scripts that measure the codebase. Run them by hand whenever you want a number. + +Each one prints a table. Add `--csv` to get rows you can append to a file instead. + +## What's here + +| Script | What it tells you | +|-|-| +| `repo_loc.py` | Lua LOC per wiki, in this repo | +| `onwiki_loc.py` | Lua LOC that lives on-wiki and isn't in this repo | +| `deprecated_patterns.py` | Call sites still using legacy patterns | +| `coverage.py` | Test coverage of `lua/wikis/commons` | +| `annotation_coverage.py` | How much of commons has `@param` / `@return` | +| `customization_metrics.py` | How much per-wiki override code there is | + +## Running them + +Most need nothing but Python: + +```bash +python3 scripts/metrics/repo_loc.py +``` + +`coverage.py` runs the test suite, so it needs busted and luacov on Lua 5.1: + +```bash +luarocks install --lua-version=5.1 busted +luarocks install --lua-version=5.1 luacov +python3 scripts/metrics/coverage.py +``` + +`onwiki_loc.py` calls the live wiki API: + +```bash +pip install -r requirements.txt +WIKI_BASE_URL=https://liquipedia.net python3 scripts/metrics/onwiki_loc.py +``` + +## Tracking over time + +```bash +python3 scripts/metrics/repo_loc.py --csv --no-header >> metrics.csv +``` + +Run any script with `--help` for the rest of its options. diff --git a/scripts/metrics/annotation_coverage.py b/scripts/metrics/annotation_coverage.py old mode 100644 new mode 100755 index 381a877555b..d358a7f4499 --- a/scripts/metrics/annotation_coverage.py +++ b/scripts/metrics/annotation_coverage.py @@ -1,4 +1,5 @@ -"""Measure LuaLS annotation coverage of exported functions in lua/wikis/commons. +#!/usr/bin/env python3 +"""Metric 5: LuaLS annotation coverage of exported functions in commons. Annotations are what make commons modules statically understandable, so this tracks how much of the exported surface carries them. @@ -15,24 +16,42 @@ value is returned, so a function is more likely to be asked for an annotation than excused from one. -Prints a Markdown table, optionally with deltas against a second tree. Pass ---raw for `key=value` output instead. - Usage: - python scripts/metrics/annotation_coverage.py [root] [--base BASE_ROOT] [--raw] + python3 scripts/metrics/annotation_coverage.py [--csv] [--no-header] + [--base BASE_ROOT] [root] + +Intended to be run on a schedule (e.g. weekly CI job) with --csv appended to a +time-series file, so standardization / Phoenix progress can be charted. """ import argparse -import pathlib +import csv import re import sys +from datetime import date +from pathlib import Path +from typing import Optional + +REPO_ROOT = Path(__file__).resolve().parent.parent.parent +DEFAULT_ROOT = REPO_ROOT / "lua" / "wikis" / "commons" FUNCTION = re.compile(r"^(\s*)function\s+[A-Za-z_][\w.]*[.:]\w+\s*\(([^)]*)\)") RETURNS_VALUE = re.compile(r"\breturn\s+\S") -DEFAULT_ROOT = "lua/wikis/commons" +FIELDS = [ + "functions", + "annotated", + "exempt", + "needs_annotation", + "unannotated", + "coverage_pct", + "params_total", + "params_annotated", + "params_pct", +] -def doc_block(lines, index): + +def doc_block(lines: list[str], index: int) -> str: """Return the contiguous comment block immediately above lines[index].""" block = [] cursor = index - 1 @@ -42,7 +61,7 @@ def doc_block(lines, index): return "\n".join(block) -def returns_value(lines, index, indent): +def returns_value(lines: list[str], index: int, indent: str) -> bool: """Whether the function starting at lines[index] returns a value.""" end = re.compile(rf"^{indent}end\b") for line in lines[index + 1 :]: @@ -53,14 +72,14 @@ def returns_value(lines, index, indent): return True # no clear end found: assume it returns, so we still ask for a tag -def measure(root): +def collect(root: Path) -> dict: functions = annotated = exempt = params_total = params_annotated = 0 for path in sorted(root.rglob("*.lua")): # ScribuntoUnit testcases run on-wiki and are not part of the surface. if "test" in path.parts: continue - lines = path.read_text(encoding="utf-8", errors="replace").split("\n") + lines = path.read_text(encoding="utf-8", errors="replace").splitlines() for index, line in enumerate(lines): match = FUNCTION.match(line) if not match: @@ -96,68 +115,51 @@ def measure(root): } -def count_cell(value, base): - return f"{value}" if base is None else f"{value} ({value - base:+d})" - +def print_table(head: dict, base: Optional[dict]) -> None: + for field in FIELDS: + value = head[field] + cell = f"{value:.2f}%" if field.endswith("_pct") else str(value) + if base is not None: + delta = value - base[field] + cell += ( + f" ({delta:+.2f} pp)" if field.endswith("_pct") else f" ({delta:+d})" + ) + print(f"{field.replace('_', ' '):<20}{cell:>22}") -def pct_cell(value, base): - if base is None: - return f"{value:.2f}%" - return f"{value:.2f}% ({value - base:+.2f} pp)" - -def table(head, base): - def of(key): - return None if base is None else base[key] - - rows = [ - ( - "Exported functions annotated", - pct_cell(head["coverage_pct"], of("coverage_pct")), - ), - ( - "Needs annotation", - count_cell(head["needs_annotation"], of("needs_annotation")), - ), - ("Unannotated", count_cell(head["unannotated"], of("unannotated"))), - ("Exempt (no params, no return)", count_cell(head["exempt"], of("exempt"))), - ("Exported functions", count_cell(head["functions"], of("functions"))), - ( - "Functions with all params annotated", - pct_cell(head["params_pct"], of("params_pct")), - ), - ] - lines = ["| Annotations (lua/wikis/commons) | |", "|-|-|"] - lines += [f"| {label} | {value} |" for label, value in rows] - return lines +def print_csv(head: dict, header: bool) -> None: + writer = csv.writer(sys.stdout) + if header: + writer.writerow(["date"] + FIELDS) + writer.writerow([date.today().isoformat()] + [head[f] for f in FIELDS]) -def main(): +def main() -> int: parser = argparse.ArgumentParser() - parser.add_argument("root", nargs="?", type=pathlib.Path, default=DEFAULT_ROOT) + parser.add_argument("root", nargs="?", type=Path, default=DEFAULT_ROOT) + parser.add_argument("--csv", action="store_true", help="CSV for appending") + parser.add_argument("--no-header", action="store_true", help="omit the CSV header") parser.add_argument( - "--base", type=pathlib.Path, help="second tree to compare against" + "--base", type=Path, help="compare against this tree (table mode only)" ) - parser.add_argument("--raw", action="store_true", help="print key=value instead") args = parser.parse_args() if not args.root.is_dir(): - print(f"::error::annotation root not found: {args.root}") + print(f"annotation root not found: {args.root}", file=sys.stderr) return 1 - head = measure(args.root) - if args.raw: - for key, value in head.items(): - print(f"{key}={value}") + head = collect(args.root) + if args.csv: + print_csv(head, header=not args.no_header) return 0 base = None if args.base: if args.base.is_dir(): - base = measure(args.base) + base = collect(args.base) else: - print(f"::warning::base root not found, omitting deltas: {args.base}") - print("\n".join(table(head, base))) + print(f"base root not found, omitting deltas: {args.base}", file=sys.stderr) + print_table(head, base) return 0 diff --git a/scripts/metrics/coverage.py b/scripts/metrics/coverage.py old mode 100644 new mode 100755 index 086877698a6..f18084e2506 --- a/scripts/metrics/coverage.py +++ b/scripts/metrics/coverage.py @@ -1,7 +1,9 @@ -"""Run the busted suite under luacov and report coverage of lua/wikis/commons. +#!/usr/bin/env python3 +"""Metric 6: busted test coverage of lua/wikis/commons. +Runs the test suite under luacov and reports how much of commons it exercises. Coverage instrumentation is a per-line debug hook, so the suite takes roughly -20x longer than normal -- expect a couple of minutes rather than a second. +20x longer than normal -- expect about half a minute rather than a second. Needs busted and luacov on PATH, both for Lua 5.1: @@ -13,22 +15,28 @@ only the files the suite happens to touch. Usage: - python scripts/metrics/coverage.py [--skip-run] + python3 scripts/metrics/coverage.py [--csv] [--no-header] [--skip-run] + +Intended to be run on a schedule (e.g. weekly CI job) with --csv appended to a +time-series file, so standardization / Phoenix progress can be charted. """ import argparse -import pathlib +import csv import re import subprocess import sys +from datetime import date +from pathlib import Path -REPO_ROOT = pathlib.Path(__file__).resolve().parents[2] +REPO_ROOT = Path(__file__).resolve().parent.parent.parent LUA_DIR = REPO_ROOT / "lua" REPORT = LUA_DIR / "luacov.report.out" TOTAL = re.compile(r"^Total\s+(\d+)\s+(\d+)\s+([\d.]+)%") FILE_ROW = re.compile(r"^wikis/commons/\S+\s+\d+\s+\d+\s+([\d.]+)%$") +FIELDS = ["coverage_pct", "hits", "missed", "files", "files_at_zero"] INSTALL_HINT = ( "not found on PATH. Install both for Lua 5.1:\n" @@ -37,7 +45,7 @@ ) -def run(command, cwd): +def run(command: list[str], cwd: Path) -> None: try: subprocess.run(command, cwd=cwd, check=True) except FileNotFoundError: @@ -46,42 +54,46 @@ def run(command, cwd): raise SystemExit(f"{command[0]} failed (exit {error.returncode})") from None -def run_suite(): +def run_suite() -> None: """Run the test suite under coverage, then render the luacov report.""" run(["busted", "-C", "lua", "--run=ci", "-c"], REPO_ROOT) run(["luacov"], LUA_DIR) -def parse(report): - lines = report.read_text(encoding="utf-8", errors="replace").split("\n") +def collect(report: Path) -> dict: + lines = report.read_text(encoding="utf-8", errors="replace").splitlines() total = next((TOTAL.match(line) for line in lines if TOTAL.match(line)), None) if total is None: raise SystemExit(f"no Total line in {report}; did luacov run?") shares = [m.group(1) for m in map(FILE_ROW.match, lines) if m] return { + "coverage_pct": float(total.group(3)), "hits": int(total.group(1)), "missed": int(total.group(2)), - "coverage_pct": float(total.group(3)), "files": len(shares), - "uncovered": sum(1 for s in shares if float(s) == 0), + "files_at_zero": sum(1 for s in shares if float(s) == 0), } -def table(m): - return [ - "| Coverage (lua/wikis/commons) | |", - "|-|-|", - f"| Coverage | {m['coverage_pct']:.2f}% |", - f"| Lines hit | {m['hits']} |", - f"| Lines missed | {m['missed']} |", - f"| Files | {m['files']} |", - f"| Files at 0% | {m['uncovered']} |", - ] +def print_table(metrics: dict) -> None: + for field in FIELDS: + value = metrics[field] + cell = f"{value:.2f}%" if field.endswith("_pct") else str(value) + print(f"{field.replace('_', ' '):<16}{cell:>12}") + + +def print_csv(metrics: dict, header: bool) -> None: + writer = csv.writer(sys.stdout) + if header: + writer.writerow(["date"] + FIELDS) + writer.writerow([date.today().isoformat()] + [metrics[f] for f in FIELDS]) -def main(): +def main() -> int: parser = argparse.ArgumentParser() + parser.add_argument("--csv", action="store_true", help="CSV for appending") + parser.add_argument("--no-header", action="store_true", help="omit the CSV header") parser.add_argument( "--skip-run", action="store_true", @@ -92,10 +104,14 @@ def main(): if not args.skip_run: run_suite() elif not REPORT.exists(): - print(f"::error::no report at {REPORT}; run without --skip-run first") + print(f"no report at {REPORT}; run without --skip-run first", file=sys.stderr) return 1 - print("\n".join(table(parse(REPORT)))) + metrics = collect(REPORT) + if args.csv: + print_csv(metrics, header=not args.no_header) + else: + print_table(metrics) return 0 diff --git a/scripts/metrics/customization_metrics.py b/scripts/metrics/customization_metrics.py old mode 100644 new mode 100755 index 16b6497fad3..bb9da843390 --- a/scripts/metrics/customization_metrics.py +++ b/scripts/metrics/customization_metrics.py @@ -1,34 +1,42 @@ -"""Measure how much per-wiki customization code lives outside lua/wikis/commons. +#!/usr/bin/env python3 +"""Metric 4: per-wiki customization of repo-managed Lua code. Everything in this repo is standardized; what varies is how much a wiki has to -override to get the behaviour it wants. Override code is the part that costs -maintenance and has to be carried forward, so it is the number worth watching. -Declarative data and config, and legacy shims, are reported separately. +override to get the behaviour it wants. Override code is the part that carries +maintenance cost, so it is the number worth watching. Declarative data/config +and legacy shims are counted separately -- a wiki adding 500 lines of faction +data is not the same event as one adding 500 lines of overrides. -Classification is by file *content*, not filename -- `GetMatchGroupCopyPaste/ +Categories are decided by file *content*, not filename: `GetMatchGroupCopyPaste/ wiki.lua`, `FilterButtons/Config.lua` and `NotabilityChecker/config.lua` all look declarative and are not. A file is override code when it defines a -non-local function, or when a `local function` is exported via a `return` -statement (the widget pattern: `local function X` ... `return wrap(X)`). -Purely-local helpers inside an otherwise declarative file do not promote it. +non-local function, or exports a `local function` through a `return` statement +(the widget pattern: `local function X` ... `return wrap(X)`). Purely-local +helpers in an otherwise declarative file do not promote it. Any path containing +`Legacy` is counted as legacy before the content check. -Shares are against the whole of lua/wikis (commons included), so "override code -is 37% of all Lua" is answerable. Vendored code, type stubs, specs and test -assets under lua/ are excluded from the denominator -- they are not the product. - -Prints a Markdown table, optionally with deltas against a second tree. Pass ---raw for `key=value` output instead. +commons is counted too, so shares have the whole of lua/wikis as denominator +and "override code is 37% of all Lua" is answerable. LOC matches metric 1: +non-blank, non-comment-only lines, with total physical lines also reported. Usage: - python scripts/metrics/customization_metrics.py [wikis-root] [--base BASE_ROOT] [--raw] + python3 scripts/metrics/customization_metrics.py [--csv] [--no-header] + [--base BASE_ROOT] [root] + +Intended to be run on a schedule (e.g. weekly CI job) with --csv appended to a +time-series file, so standardization / Phoenix progress can be charted. """ import argparse -import pathlib +import csv import re import sys +from datetime import date +from pathlib import Path +from typing import Optional -DEFAULT_ROOT = "lua/wikis" +REPO_ROOT = Path(__file__).resolve().parent.parent.parent +DEFAULT_ROOT = REPO_ROOT / "lua" / "wikis" COMMONS = "commons" EXPORTED_FUNCTION = re.compile(r"\bfunction\b") @@ -38,8 +46,11 @@ LOCAL_FUNCTION_NAME = re.compile(r"^[ \t]*local[ \t]+function[ \t]+([A-Za-z_]\w*)") RETURN_STATEMENT = re.compile(r"^[ \t]*return\b") +# Order is the display order; per-wiki categories first, then commons. +CATEGORIES = ["override", "declarative", "legacy", "commons"] + -def is_override_code(lines): +def is_override_code(lines: list[str]) -> bool: """True when the file exposes behaviour rather than just data or config.""" local_names = [] returns = [] @@ -58,110 +69,116 @@ def is_override_code(lines): return any(name in line for line in returns for name in local_names) -def measure(root): - declarative = legacy = override = commons = 0 +def categorise(path: Path, lines: list[str]) -> str: + if COMMONS in path.parts: + return "commons" + # Substring, not a path component: legacy lives in directories + # (TeamCard/Legacy/Custom.lua) *and* in filenames (Match/Legacy.lua). + # `"Legacy" in path.parts` would miss the latter -- 52 of 78 files. + if "Legacy" in path.as_posix(): + return "legacy" + return "override" if is_override_code(lines) else "declarative" + + +def count_file(lines: list[str]) -> tuple[int, int]: + """Return (physical_lines, loc) -- loc as metric 1 defines it.""" + loc = sum(1 for line in lines if line.strip() and not line.strip().startswith("--")) + return len(lines), loc + + +def collect(root: Path) -> list[dict]: + totals = {c: {"files": 0, "lines": 0, "loc": 0} for c in CATEGORIES} for path in sorted(root.rglob("*.lua")): - text = path.read_text(encoding="utf-8", errors="replace") - count = len(text.splitlines()) - - # commons is the shared implementation, not per-wiki customization. It is - # still counted, so shares have the whole of lua/wikis as denominator. - if COMMONS in path.parts: - commons += count - # Substring, not a path component: legacy lives in directories - # (TeamCard/Legacy/Custom.lua) *and* in filenames (Match/Legacy.lua). - # `"Legacy" in path.parts` would miss the latter -- 52 of 78 files, - # moving 6279 lines out of legacy and into override code. - elif "Legacy" in path.as_posix(): - legacy += count - elif is_override_code(text.splitlines()): - override += count - else: - declarative += count - - per_wiki = override + declarative + legacy - total = per_wiki + commons - - def share(count): - return round(count / total * 100, 2) if total else 0.0 - - return { - "override_code": override, - "declarative": declarative, - "legacy": legacy, - "per_wiki_total": per_wiki, - "commons": commons, - "total": total, - "override_code_pct": share(override), - "declarative_pct": share(declarative), - "legacy_pct": share(legacy), - "per_wiki_total_pct": share(per_wiki), - "commons_pct": share(commons), - } - - -def count_cell(value, base): - return f"{value}" if base is None else f"{value} ({value - base:+d})" - - -def pct_cell(value, base): - if base is None: - return f"{value:.2f}%" - return f"{value:.2f}% ({value - base:+.2f} pp)" - - -def table(head, base): - def of(key): - return None if base is None else base[key] - - rows = [ - ("**Override code**", "override_code", "override_code_pct"), - ("Declarative (data + config)", "declarative", "declarative_pct"), - ("Legacy", "legacy", "legacy_pct"), - ("Per-wiki total", "per_wiki_total", "per_wiki_total_pct"), - ("lua/wikis/commons (shared)", "commons", "commons_pct"), - ] - lines = [ - "| Per-wiki customization | LOC | Share of all Lua |", - "|-|-|-|", - ] - for label, loc_key, pct_key in rows: - loc = count_cell(head[loc_key], of(loc_key)) - pct = pct_cell(head[pct_key], of(pct_key)) - if label.startswith("**"): - loc, pct = f"**{loc}**", f"**{pct}**" - lines.append(f"| {label} | {loc} | {pct} |") - lines.append(f"| All of lua/wikis | {count_cell(head['total'], of('total'))} | |") - return lines - - -def main(): + lines = path.read_text(encoding="utf-8", errors="replace").splitlines() + physical, loc = count_file(lines) + bucket = totals[categorise(path, lines)] + bucket["files"] += 1 + bucket["lines"] += physical + bucket["loc"] += loc + + total_loc = sum(b["loc"] for b in totals.values()) + rows = [] + for category in CATEGORIES: + bucket = totals[category] + rows.append( + { + "category": category, + "files": bucket["files"], + "lines": bucket["lines"], + "loc": bucket["loc"], + "share": round(bucket["loc"] / total_loc * 100, 2) + if total_loc + else 0.0, + } + ) + return rows + + +def print_table(rows: list[dict], base: Optional[list[dict]]) -> None: + by_category = {r["category"]: r for r in base} if base else {} + print(f"{'category':<14}{'files':>7}{'lines':>10}{'loc':>10}{'share':>9}") + for row in rows: + share = f"{row['share']:.2f}%" + if row["category"] in by_category: + delta = row["loc"] - by_category[row["category"]]["loc"] + share += f" ({delta:+d} loc)" + print( + f"{row['category']:<14}{row['files']:>7}{row['lines']:>10}" + f"{row['loc']:>10}{share:>9}" + ) + per_wiki = [r for r in rows if r["category"] != "commons"] + print( + f"{'per-wiki total':<14}{sum(r['files'] for r in per_wiki):>7}" + f"{sum(r['lines'] for r in per_wiki):>10}{sum(r['loc'] for r in per_wiki):>10}" + f"{sum(r['share'] for r in per_wiki):>8.2f}%" + ) + + +def print_csv(rows: list[dict], header: bool) -> None: + writer = csv.writer(sys.stdout) + if header: + writer.writerow(["date", "category", "files", "lines", "loc", "share"]) + today = date.today().isoformat() + for row in rows: + writer.writerow( + [ + today, + row["category"], + row["files"], + row["lines"], + row["loc"], + row["share"], + ] + ) + + +def main() -> int: parser = argparse.ArgumentParser() - parser.add_argument("root", nargs="?", type=pathlib.Path, default=DEFAULT_ROOT) + parser.add_argument("root", nargs="?", type=Path, default=DEFAULT_ROOT) + parser.add_argument("--csv", action="store_true", help="CSV for appending") + parser.add_argument("--no-header", action="store_true", help="omit the CSV header") parser.add_argument( - "--base", type=pathlib.Path, help="second tree to compare against" + "--base", type=Path, help="compare loc against this tree (table mode only)" ) - parser.add_argument("--raw", action="store_true", help="print key=value instead") args = parser.parse_args() if not args.root.is_dir(): - print(f"::error::wikis root not found: {args.root}") + print(f"wikis root not found: {args.root}", file=sys.stderr) return 1 - head = measure(args.root) - if args.raw: - for key, value in head.items(): - print(f"{key}={value}") + rows = collect(args.root) + if args.csv: + print_csv(rows, header=not args.no_header) return 0 base = None if args.base: if args.base.is_dir(): - base = measure(args.base) + base = collect(args.base) else: - print(f"::warning::base root not found, omitting deltas: {args.base}") - print("\n".join(table(head, base))) + print(f"base root not found, omitting deltas: {args.base}", file=sys.stderr) + print_table(rows, base) return 0 From bc1f246e0c28498b627970ec6d2a0e429bd2610d Mon Sep 17 00:00:00 2001 From: Rikard Blixt Date: Mon, 7 Sep 2026 12:32:17 +0200 Subject: [PATCH 5/7] fix: stale coverage stats, and tighten the override classifier Both from Copilot's review, plus one it did not spot. Coverage was reporting the previous run. lua/.luacov keeps stats so a report can be regenerated, but nothing cleared them, so luacov reused whatever was on disk. Running a single spec with full-suite stats present reported 49.38% against a true 23.92% -- a 25 point error, and silent. run_suite() now clears the stats and report first. collect() also fails with a message rather than a traceback when no report is produced. The classifier matched the bare `function` keyword, so a comment mentioning the word could class a data file as override code, and it only learned local names from `local function name`, missing `local name = function` exported via `return`. Now matches actual definition forms, strips line comments first, and records names from both local forms. No file in the repo hits either case today, so the numbers are unchanged -- this removes latent fragility rather than fixing a live miscount. Third one, found while testing the above: export detection used a substring match, so `local function h` plus `return {a = h()}` counted as exported, and a short local name could match an unrelated identifier. It is now word anchored and requires the name not be followed by `(`, which distinguishes handing a local out (`return F`, `return wrap(F)`) from calling it to build data (`return {a = h()}`) -- the latter leaves the file declarative, matching how MainPageLayout data files are already treated. Metric 4 still reconciles with metric 1: 1651 files, 203738 lines, 146516 loc. Co-Authored-By: Claude Opus 5 --- scripts/metrics/coverage.py | 7 ++++++ scripts/metrics/customization_metrics.py | 31 +++++++++++++++--------- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/scripts/metrics/coverage.py b/scripts/metrics/coverage.py index f18084e2506..3c0e2ba0b43 100755 --- a/scripts/metrics/coverage.py +++ b/scripts/metrics/coverage.py @@ -32,6 +32,7 @@ REPO_ROOT = Path(__file__).resolve().parent.parent.parent LUA_DIR = REPO_ROOT / "lua" REPORT = LUA_DIR / "luacov.report.out" +STATS = LUA_DIR / "luacov.stats.out" TOTAL = re.compile(r"^Total\s+(\d+)\s+(\d+)\s+([\d.]+)%") FILE_ROW = re.compile(r"^wikis/commons/\S+\s+\d+\s+\d+\s+([\d.]+)%$") @@ -56,11 +57,17 @@ def run(command: list[str], cwd: Path) -> None: def run_suite() -> None: """Run the test suite under coverage, then render the luacov report.""" + # lua/.luacov keeps stats so a report can be regenerated, which means a + # stale file would be reused and report the previous run's coverage. + STATS.unlink(missing_ok=True) + REPORT.unlink(missing_ok=True) run(["busted", "-C", "lua", "--run=ci", "-c"], REPO_ROOT) run(["luacov"], LUA_DIR) def collect(report: Path) -> dict: + if not report.exists(): + raise SystemExit(f"no report at {report}; luacov produced nothing") lines = report.read_text(encoding="utf-8", errors="replace").splitlines() total = next((TOTAL.match(line) for line in lines if TOTAL.match(line)), None) if total is None: diff --git a/scripts/metrics/customization_metrics.py b/scripts/metrics/customization_metrics.py index bb9da843390..98ba90cd4f7 100755 --- a/scripts/metrics/customization_metrics.py +++ b/scripts/metrics/customization_metrics.py @@ -39,12 +39,14 @@ DEFAULT_ROOT = REPO_ROOT / "lua" / "wikis" COMMONS = "commons" -EXPORTED_FUNCTION = re.compile(r"\bfunction\b") -LOCAL_FUNCTION = re.compile( - r"^[ \t]*local[ \t]+(?:function|[A-Za-z_][A-Za-z0-9_]*[ \t]*=[ \t]*function)" -) -LOCAL_FUNCTION_NAME = re.compile(r"^[ \t]*local[ \t]+function[ \t]+([A-Za-z_]\w*)") +# A definition is the `function` keyword followed by an optional name and `(`. +# Matching the bare keyword would also catch prose in comments. +FUNCTION_DEF = re.compile(r"\bfunction\b\s*[A-Za-z_][\w.:]*\s*\(|\bfunction\b\s*\(") +# Both local forms, so a local exported via `return` is found either way. +LOCAL_DEF = re.compile(r"^[ \t]*local[ \t]+function[ \t]+([A-Za-z_]\w*)") +LOCAL_ASSIGN = re.compile(r"^[ \t]*local[ \t]+([A-Za-z_]\w*)[ \t]*=[ \t]*function\b") RETURN_STATEMENT = re.compile(r"^[ \t]*return\b") +LINE_COMMENT = re.compile(r"--.*$") # Order is the display order; per-wiki categories first, then commons. CATEGORIES = ["override", "declarative", "legacy", "commons"] @@ -54,19 +56,24 @@ def is_override_code(lines: list[str]) -> bool: """True when the file exposes behaviour rather than just data or config.""" local_names = [] returns = [] - for line in lines: - if not EXPORTED_FUNCTION.search(line): + for raw in lines: + line = LINE_COMMENT.sub("", raw) + if not FUNCTION_DEF.search(line): if RETURN_STATEMENT.match(line): returns.append(line) continue - if not LOCAL_FUNCTION.match(line): + local = LOCAL_DEF.match(line) or LOCAL_ASSIGN.match(line) + if not local: return True # a non-local function definition - name = LOCAL_FUNCTION_NAME.match(line) - if name: - local_names.append(name.group(1)) + local_names.append(local.group(1)) # A local function handed out through `return` is the module's interface. - return any(name in line for line in returns for name in local_names) + # `name` not followed by `(` distinguishes handing it out (`return F`, + # `return wrap(F)`) from calling it to build data (`return {a = h()}`), + # which leaves the file declarative. Word-anchored so a short local name + # cannot match an unrelated identifier. + exported = [re.compile(rf"\b{re.escape(n)}\b\s*(?!\()") for n in local_names] + return any(pattern.search(line) for line in returns for pattern in exported) def categorise(path: Path, lines: list[str]) -> str: From 1a73f15e9d763442b31acd7f67d2f4aa7ba0092a Mon Sep 17 00:00:00 2001 From: Rikard Blixt Date: Mon, 7 Sep 2026 17:26:43 +0200 Subject: [PATCH 6/7] chore: match the metrics scripts' CLI, and fix the total share All from Copilot's second pass. The three scripts took `--no-header` as a plain flag and built a bare ArgumentParser, where the existing three use `description=__doc__` and a `--header` BooleanOptionalAction. Copilot flagged this as inconsistent and it was right -- I had checked the shebang, the metric numbering and --csv when aligning to the folder, but not the parser itself. --help now shows what each script measures, and --no-header still works, from the same declaration. The per-wiki total share added up the already-rounded per-category shares rather than dividing summed loc by total loc. Both give 59.13% today (59.1300 against a true 59.1335), so nothing visible changes, but the wrong one drifts as categories move. Also the singular-subject grammar in the .luacov comment. Co-Authored-By: Claude Opus 5 --- lua/.luacov | 2 +- scripts/metrics/annotation_coverage.py | 16 ++++++++++++---- scripts/metrics/coverage.py | 16 ++++++++++++---- scripts/metrics/customization_metrics.py | 24 ++++++++++++++++++------ 4 files changed, 43 insertions(+), 15 deletions(-) diff --git a/lua/.luacov b/lua/.luacov index 3d43c494adc..bab9bbe1a85 100644 --- a/lua/.luacov +++ b/lua/.luacov @@ -13,6 +13,6 @@ return { -- ScribuntoUnit testcases that run on-wiki, not under busted. 'wikis/commons/test/', }, - runreport = false, -- Don't work properly for some reason, have to do a manual `luacov` call afterwards + runreport = false, -- Doesn't work properly for some reason, have to do a manual `luacov` call afterwards deletestats = false, -- Keep stats so the report can be regenerated / inspected after a run } diff --git a/scripts/metrics/annotation_coverage.py b/scripts/metrics/annotation_coverage.py index d358a7f4499..68c1ad52626 100755 --- a/scripts/metrics/annotation_coverage.py +++ b/scripts/metrics/annotation_coverage.py @@ -135,10 +135,18 @@ def print_csv(head: dict, header: bool) -> None: def main() -> int: - parser = argparse.ArgumentParser() + parser = argparse.ArgumentParser(description=__doc__) parser.add_argument("root", nargs="?", type=Path, default=DEFAULT_ROOT) - parser.add_argument("--csv", action="store_true", help="CSV for appending") - parser.add_argument("--no-header", action="store_true", help="omit the CSV header") + parser.add_argument( + "--csv", action="store_true", help="CSV output (for appending to a time series)" + ) + parser.add_argument( + "--header", + action=argparse.BooleanOptionalAction, + default=True, + help="write the CSV header row (--no-header when appending " + "to an existing time-series file)", + ) parser.add_argument( "--base", type=Path, help="compare against this tree (table mode only)" ) @@ -150,7 +158,7 @@ def main() -> int: head = collect(args.root) if args.csv: - print_csv(head, header=not args.no_header) + print_csv(head, header=args.header) return 0 base = None diff --git a/scripts/metrics/coverage.py b/scripts/metrics/coverage.py index 3c0e2ba0b43..71e64c6a90b 100755 --- a/scripts/metrics/coverage.py +++ b/scripts/metrics/coverage.py @@ -98,9 +98,17 @@ def print_csv(metrics: dict, header: bool) -> None: def main() -> int: - parser = argparse.ArgumentParser() - parser.add_argument("--csv", action="store_true", help="CSV for appending") - parser.add_argument("--no-header", action="store_true", help="omit the CSV header") + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--csv", action="store_true", help="CSV output (for appending to a time series)" + ) + parser.add_argument( + "--header", + action=argparse.BooleanOptionalAction, + default=True, + help="write the CSV header row (--no-header when appending " + "to an existing time-series file)", + ) parser.add_argument( "--skip-run", action="store_true", @@ -116,7 +124,7 @@ def main() -> int: metrics = collect(REPORT) if args.csv: - print_csv(metrics, header=not args.no_header) + print_csv(metrics, header=args.header) else: print_table(metrics) return 0 diff --git a/scripts/metrics/customization_metrics.py b/scripts/metrics/customization_metrics.py index 98ba90cd4f7..a9b8e2d61dd 100755 --- a/scripts/metrics/customization_metrics.py +++ b/scripts/metrics/customization_metrics.py @@ -135,10 +135,14 @@ def print_table(rows: list[dict], base: Optional[list[dict]]) -> None: f"{row['loc']:>10}{share:>9}" ) per_wiki = [r for r in rows if r["category"] != "commons"] + per_wiki_loc = sum(r["loc"] for r in per_wiki) + total_loc = sum(r["loc"] for r in rows) + # From the loc totals, not by adding up the rounded per-category shares. + share = per_wiki_loc / total_loc * 100 if total_loc else 0.0 print( f"{'per-wiki total':<14}{sum(r['files'] for r in per_wiki):>7}" - f"{sum(r['lines'] for r in per_wiki):>10}{sum(r['loc'] for r in per_wiki):>10}" - f"{sum(r['share'] for r in per_wiki):>8.2f}%" + f"{sum(r['lines'] for r in per_wiki):>10}{per_wiki_loc:>10}" + f"{share:>8.2f}%" ) @@ -161,10 +165,18 @@ def print_csv(rows: list[dict], header: bool) -> None: def main() -> int: - parser = argparse.ArgumentParser() + parser = argparse.ArgumentParser(description=__doc__) parser.add_argument("root", nargs="?", type=Path, default=DEFAULT_ROOT) - parser.add_argument("--csv", action="store_true", help="CSV for appending") - parser.add_argument("--no-header", action="store_true", help="omit the CSV header") + parser.add_argument( + "--csv", action="store_true", help="CSV output (for appending to a time series)" + ) + parser.add_argument( + "--header", + action=argparse.BooleanOptionalAction, + default=True, + help="write the CSV header row (--no-header when appending " + "to an existing time-series file)", + ) parser.add_argument( "--base", type=Path, help="compare loc against this tree (table mode only)" ) @@ -176,7 +188,7 @@ def main() -> int: rows = collect(args.root) if args.csv: - print_csv(rows, header=not args.no_header) + print_csv(rows, header=args.header) return 0 base = None From 80410340e816dd294c164dd868b4b5a360da5b7a Mon Sep 17 00:00:00 2001 From: Rikard Blixt Date: Mon, 7 Sep 2026 17:48:11 +0200 Subject: [PATCH 7/7] fix: count parameters, not functions, in the params metric Per Copilot: params_total and params_annotated incremented once per function rather than once per parameter, so a five-parameter signature counted the same as a one-parameter one while the field names said otherwise. This was a regression from the move to the house table format. The markdown version labelled the row "Functions with all params annotated", which was accurate; deriving labels from field names dropped that and left the table printing "params total 2046" for a count of functions. Now counts individual parameters, which is what the names imply: 2991 of 3148 annotated, 95.01%, against the 95.60% the function-level count reported. Clamped per function, since a doc block can carry more @param than the signature takes. Worth doing now rather than later: no time series exists yet, so redefining these columns costs nothing. After the first append it would break comparability. Co-Authored-By: Claude Opus 5 --- scripts/metrics/annotation_coverage.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/scripts/metrics/annotation_coverage.py b/scripts/metrics/annotation_coverage.py index 68c1ad52626..d764cdd6077 100755 --- a/scripts/metrics/annotation_coverage.py +++ b/scripts/metrics/annotation_coverage.py @@ -11,6 +11,10 @@ A function taking no parameters and returning no value needs neither tag, so it is reported as exempt and left out of the ratio rather than counted against it. + +`params_total` / `params_annotated` count individual parameters, not functions, +so a five-parameter signature carries five times the weight of a one-parameter +one. Whether a body returns a value is decided by scanning to the `end` at the function's own indentation, which is a heuristic: when in doubt it assumes a value is returned, so a function is more likely to be asked for an annotation @@ -94,10 +98,12 @@ def collect(root: Path) -> dict: annotated += 1 elif not params and not returns_value(lines, index, indent): exempt += 1 - if params: - params_total += 1 - if block.count("@param") >= len(params): - params_annotated += 1 + # Per parameter, not per function: a five-parameter function with + # one tag is not as annotated as a one-parameter function with one. + # Clamped, since a doc block may carry more @param than the + # signature takes. + params_total += len(params) + params_annotated += min(block.count("@param"), len(params)) needs = functions - exempt return {