ci: track annotation coverage of commons by Rathoz · Pull Request #7863 · Liquipedia/Lua-Modules

Rathoz · 2026-07-27T18:26:34Z

What

New metric: share of exported functions in lua/wikis/commons whose doc block carries @param or @return.

At ~96% it's a regression guard — the useful signal is whether a PR adds unannotated functions
Functions taking no params and returning nothing are exempt, not counted as gaps (19 of them)
Reports deltas vs the base branch; new job needs no Lua toolchain, runs in seconds

Annotations (lua/wikis/commons)
Exported functions annotated	95.74% (+0.00 pp)
Needs annotation	2397 (+0)
Unannotated	102 (+0)
Exempt (no params, no return)	19 (+0)
Exported functions	2416 (+0)

The LuaLS problem-count fix that was here has moved to #7877.

How it was tested

Green in CI; the table above is actual rendered output
Deterministic over 3 runs; passes ruff check + format
No-baseline path checked via workflow_dispatch

@return

Two of the three code-health metrics I set out to add turned out not to be worth having, so this adds one and fixes a number. Annotation coverage (new): share of exported functions in lua/wikis/commons whose preceding doc block carries @param or @return. Annotations are what make commons statically understandable, and at 94.99% the metric works as a regression guard rather than a growth target -- the actionable figures are the 121 unannotated functions and whether a PR adds more. Reported with deltas against the base commit, same shape as the coverage job. The script takes a root argument so the base tree is measured with this PR's script rather than the base's, keeping the delta about code rather than measurement. LuaLS problem count (fixed): the summary reported `grep -c 'Warning'`, which gives 263 where LuaLS itself reports 238 -- a single diagnostic can span several lines. Use the totals LuaLS prints, and add files-with-problems. Deliberately not added: - luacheck findings. Already 0 warnings / 0 errors across 1707 files and gated there, so a trend line would be permanently flat. - A delta on LuaLS problems. The count is not deterministic: 237-240 over 8 runs on identical code, in both pretty and json output, with no --threads flag to pin it. A per-PR delta would show phantom movement of a few either way. Files-with-problems was stable at 142 across all runs, so that is the number to watch. The summary now says as much. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Matches the coverage job. Step summaries are not retrievable from the API, so the numbers could not be checked without opening the run page. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

hjpalpha

does the anno coverage check account for the existence of exported functions that have neither input nor return?

@hjpalpha

Exempt functions that need no annotation (@hjpalpha). A function taking no parameters and returning no value needs neither @param nor @return, but was being counted as unannotated -- so the metric penalised correctly written code. 19 of the 121 "unannotated" functions were in that position. Excluding them puts coverage at 95.74% over 2397 functions that do need tags, with 102 genuinely missing them. Whether a body returns a value is decided by scanning to the `end` at the function's own indentation. That is a heuristic, so it is biased toward asking for an annotation rather than excusing one, and the exempt count is now a reported row so the exemption stays visible instead of silently shrinking the denominator. Have the script emit the Markdown table itself, taking --base for the comparison tree (@ElectricalBoy). Removes 33 lines of shell that existed only to re-parse what the script already knew. --raw still prints key=value for debugging. Add actions/setup-python rather than relying on the runner image's python (@ElectricalBoy), and use `"test" in path.parts` over string matching (@ElectricalBoy) -- verified equivalent here, 4 files either way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Rathoz · 2026-07-28T09:43:12Z

does the anno coverage check account for the existence of exported functions that have neither input nor return?

It did not — good catch, that was a real flaw. Breaking down the 121 the metric called unannotated:

	Count	Verdict
Has params	80	genuinely needs `@param`
No params, returns a value	22	genuinely needs `@return`
No params, no return value	19	needs neither

So 19 were being counted against a metric they can't satisfy — it was penalising correctly written code.

Fixed in 7ed443e. Those functions are now reported as exempt and excluded from the ratio, which puts coverage at 95.74% over the 2397 functions that do need tags, with 102 genuinely missing them (was 94.99% / 121).

Two deliberate choices in how it's done:

Whether a body returns a value is decided by scanning to the end at the function's own indentation. That's a heuristic, not parsing, so it's biased toward asking for an annotation rather than excusing one — if the scan is unsure it assumes a value is returned. Better to over-report a missing tag than to quietly exempt something.
Exempt is its own row in the table rather than just vanishing, so the exemption can't silently grow and flatter the number.

CI checks out the PR merge commit, so HEAD includes everything on the base branch, but the comparison used pull_request.base.sha -- the fork point, which drifts behind as the base branch advances. The delta then reports commits from everyone else that got merged in, not just this PR's. Use the merge commit's first parent, which is the base branch tip. Falls back to omitting deltas when HEAD is not a merge commit (workflow_dispatch). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Two unrelated changes were sharing a branch: a new metric and a fix to an existing one. Split so they can be reviewed and reverted independently. The LuaLS summary fix now lives in #7877. luals-code-style here is byte identical to main again; this branch only adds annotation coverage. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Rathoz · 2026-07-29T09:16:56Z

Split the LuaLS problem-count fix out into #7877 — it was an unrelated fix to an existing metric sharing a branch with a new one. luals-code-style here is byte-identical to main again; this PR is annotation coverage only.

The setup-python and script-emits-markdown points still apply and are addressed here.

ElectricalBoy · 2026-07-30T01:30:14Z

+      - name: Setup python
+        uses: actions/setup-python@v6
+        with:
+          python-version: '3.13'


same as #7864 (comment)

Changed to 3.14 in 6aba51f, matching the six other python workflows.

ElectricalBoy · 2026-07-30T01:32:56Z

+
+def main():
+    parser = argparse.ArgumentParser()
+    parser.add_argument("root", nargs="?", default=DEFAULT_ROOT)


Suggested change

parser.add_argument("root", nargs="?", default=DEFAULT_ROOT)

parser.add_argument("root", nargs="?", type=pathlib.Path, default=DEFAULT_ROOT)

same as #7864 (comment)

Applied in 6aba51f — same as #7864. Dropped the pathlib.Path() constructors from measure() and both is_dir() checks; the string default still converts, so the no-argument case gets a Path.

@ElectricalBoy

Both from review (@ElectricalBoy): - `type=pathlib.Path` on the root and --base arguments, which drops the pathlib.Path() constructors at the call sites. argparse applies the conversion to the string default too, so the no-argument case still gets a Path. - python-version 3.14 rather than 3.13, matching the six other python workflows in the repo. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings July 27, 2026 18:26

Rathoz requested review from a team as code owners July 27, 2026 18:26

Copilot started reviewing on behalf of Rathoz July 27, 2026 18:27 View session

Copilot AI reviewed Jul 27, 2026

Rathoz and others added 2 commits July 27, 2026 20:30

ci: echo the LuaLS summary to the job log as well

96a78de

Matches the coverage job. Step summaries are not retrievable from the API, so the numbers could not be checked without opening the run page. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

chore: update visual snapshots

9b24d2b

hjpalpha reviewed Jul 27, 2026

View reviewed changes

Rathoz mentioned this pull request Jul 27, 2026

ci: track per-wiki customization code #7864

Open

chore: update visual snapshots

13af841

ElectricalBoy reviewed Jul 28, 2026

View reviewed changes

Comment thread .github/workflows/luacheck.yml Outdated

Comment thread scripts/annotation_coverage.py Outdated

Comment thread .github/workflows/luacheck.yml Outdated

ElectricalBoy added the CI/CD label Jul 28, 2026

Rathoz mentioned this pull request Jul 29, 2026

ci: report the LuaLS problem count LuaLS actually reports #7877

Open

Rathoz changed the title ~~ci: track annotation coverage, fix the LuaLS problem count~~ ci: track annotation coverage of commons Jul 29, 2026

ElectricalBoy reviewed Jul 30, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

ci: track annotation coverage of commons - #7863

ci: track annotation coverage of commons#7863
Rathoz wants to merge 8 commits into
mainfrom
chore/tier2-code-health-metrics

Rathoz commented Jul 27, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

hjpalpha left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Rathoz commented Jul 28, 2026

Uh oh!

Rathoz commented Jul 29, 2026

Uh oh!

ElectricalBoy Jul 30, 2026

Uh oh!

Rathoz Jul 30, 2026

Uh oh!

ElectricalBoy Jul 30, 2026

Uh oh!

Rathoz Jul 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	parser.add_argument("root", nargs="?", default=DEFAULT_ROOT)
	parser.add_argument("root", nargs="?", type=pathlib.Path, default=DEFAULT_ROOT)

Uh oh!

Conversation

Rathoz commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

How it was tested

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

hjpalpha left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Rathoz commented Jul 28, 2026

Uh oh!

Rathoz commented Jul 29, 2026

Uh oh!

ElectricalBoy Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

Rathoz Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

ElectricalBoy Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

Rathoz Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Rathoz commented Jul 27, 2026 •

edited

Loading