Repository navigation
tools/progress.py: the stub-wrapper pattern only matches void wrappers, so a non-void wrapper counts its callers as implemented #2971
Description
tools/progress.py's stub detector only recognises void wrappers, so a wrapper with any other return type counts its callers as implemented. The completion number can therefore move without any behaviour change, and reviewers can't tell from the progress comment that nothing happened.
The gap
tools/progress.py:18:
STUB_WRAPPER = re.compile(r"\bstatic\s+(?:\[\[noreturn\]\]\s+)?void\s+(\w+)\s*\([^;{]*\)\s*\{")It collects local wrappers that call the not-implemented helper so their callers still count as stubs (tools/progress.py:79-82). The pattern requires void. A wrapper declared static int is invisible to it, even if every caller is otherwise unchanged.
Worked example
PR #2721 (libSceFontFt) is the clearest case; the progress comment on it reports +14 implemented. It is closed, but only by the intake check — its sole comment is the bot's and it has no reviews, so the +14 was never examined by anyone. That is the part worth noting: if intake reopens, nothing in the tooling would flag it.
Before, each of the fourteen exports was:
int APS5_VABI sceFontFtSupportBdf() {
NotImplemented_nid_no_patch(__func__);
return 0;
}After:
static int SupportFormat() {
return 0;
}
int APS5_VABI sceFontFtSupportBdf() {
return SupportFormat();
}The observable behaviour of all fourteen is unchanged — each still returns 0 — and the only functional difference is that the diagnostic is gone. Because SupportFormat returns int rather than void, STUB_WRAPPER does not match it, so the fourteen callers are no longer seen as stubs and the count rises by 14. I am not suggesting the author did this deliberately; the point is that the tool cannot distinguish this from real work, so neither can a reviewer reading only the progress comment.
The wider case
A function counts as implemented as soon as it stops calling the not-implemented helper, so a body that only throws or returns a constant error also scores. That shows up repeatedly in review:
- feat(libScePsml_debug): implement the MFSR shared resource and context exports #2498 — "none does anything: each returns
0x8A810001without reading its arguments... moves the progress number without changing behaviour" - feat(audio): implement AJM split decode, resample, control and AudioOut2 mastering #2495 — "only swap
NotImplemented_nid_no_patchforstd::runtime_error; behaviour is identical and nothing is implemented" - feat(font): accept libSceFontFt format support functions as silent stubs #1450 — refused because non-UI exports must not be silent stubs
Those were all caught by a human reading the bodies. The tool reported progress for each.
I measured how common this is while auditing unmerged PRs against a local tree: of 28 PRs that moved the implemented count at all, the four largest increases were behaviourally empty — #2721 (+14, the case above), #2719 (+7, all return SCE_KERNEL_ERROR_EOPNOTSUPP;), and #2849 (+5) with #2851 (+4), which are #2494's nine already-refused libSceAgcDriver exports split across two PRs and still (void)arg; return SCE_AGC_DRIVER_ERROR_DEBUG_UNAVAILABLE;. Checking a count without reading the bodies is not safe.
Suggestions
- Widen
STUB_WRAPPERto any return type —\bstatic\s+(?:\[\[noreturn\]\]\s+)?[\w:<>,\s*&]+?\s+(\w+)\s*\([^;{]*\)\s*\{or similar — so the existing wrapper logic coversintand the rest. This closes the specific hole above. - Optionally, have the progress comment mark functions whose body is a single
return <constant>;or a barethrow, so a reviewer sees which part of a+Nis substantive. This would have flagged all four PRs listed above without anyone reading the diffs.
The second is a judgement call about how much the tool should assert; the first looks like a plain bug in the pattern.
Checked
main 4a747c48. The regex and the wrapper-collection logic are read from tools/progress.py; #2721's before/after is read from its diff and its progress comment. I have not run progress.py with a widened pattern, so I can't say how many currently-counted functions across the tree would be reclassified by suggestion 1 — that is worth measuring before changing it, since it may move the headline number.
AI-assisted: yes (Claude Code).
Activity
ycbuzi commentedon Oct 10, 2026
I can realistically say that my pr stacks against a local tree and cross reference should put my build at least at 89% close to 90% for System libraries. Not checked thoroughly though would wish a certain someone reopens so i can commit and push. Anyways feel free to look at my repo and pr stacks on my profile: https://github.com/ycbuzi/AnyPS5/tree/agent/pr2503 https://github.com/ycbuzi/AnyPS5/tree/stack/open-prs
MarcZX10 commentedon Oct 10, 2026
Follow-up: PR #2979 covers only non-void static wrappers that still call
NotImplemented_nid_no_patch. The Windows validation confirms that narrow behavior and also confirms that the #2721 constant-return example remains +14 under both scanners. I updated the PR description to removeFixes #2971; this issue remains open for the separate constant-return reporting concern.fix: detect non-void static stub wrappers in progress.py
LeandroSSB commentedon Oct 11, 2026
Suggestion 2 (the constant-return / bare-throw marking) is implemented on my fork:
LeandroSSB/AnyPS5:tools/progress-inert-bodies(one commit,2c7b102c, onmaine28fbc27; the 24 commits onmainsince then don't touch these files).The scanner classifies an implemented definition as behaviourally empty when its body is only
(void)x;discards plus at most one terminal statement:return <integer literal, bare identifier or string>;,return <inert wrapper call with no nested calls>;, a barereturn;, orthrow ...;. Inert wrappers (staticfunctions whose own bodies qualify) are followed by fixpoint iteration, so #2721'sreturn SupportFormat();shape is caught, and a wrapper that calls the not-implemented helper never qualifies. Counting is unchanged:done,todo,percentand the name sets are byte-identical, and the progress comment gains one extra⚪details block ("behaviourally empty") next to the ✅ implemented block, so a reviewer sees which part of a+Nis substantive. Older JSON snapshots without the new key are accepted, likeshared_sources.Measured on the real tree:
2772/3236 (85.66%)before and after, and 361 of the 2772 implemented functions (13%) are currently marked empty — the biggest being libSceAgc 49/197, libSceHttp 36/57, libSceHttp2 28/35, libSceAgcDriver 23/40 and libSceNpWebApi2 17/24. That is the reporting-only answer to the "wider case": it does not reclassify anything, it makes the four PR shapes visible in the progress comment itself.tools/tests/test_progress.pygains the #2721/#2719/#2849/#2495/#2498 shapes, a real implementation, a non-inert wrapper, negative cases (F-only reports stay unmarked, identical snapshots still compare to nothing) and older-snapshot tolerance; all 63 tests pass,tools/check_conventions.py --base origin/mainpasses, and the branch does not touchSTUB_WRAPPER/stub_calls, so #2979 merges independently.Not opened as a PR because of the intake pause; a maintainer is welcome to carry the commit.
AI-assisted (Claude Code); I reviewed the change and ran everything above on my machine.
rekcilyssup commentedon Oct 11, 2026
Carried LeandroSSB's suggestion-2 fix onto current
mainin case a maintainer wants to pick it up (intake is paused, so no PR from me):Branch:
rekcilyssup:carry/progress-inert-bodies— one commit (f1be92e9, original authorship kept) onmain99c1c263.What it is: LeandroSSB's
tools/progress-inert-bodies(2c7b102c) rebased past the two newer main commits (f3216874,ef94a36b) that landed after their base. The rebase is mechanical: the only conflict was the new-regex block (kept both sides), and the fork's scan now runs on main's literal/comment-blanked text. Nothing else changed;STUB_WRAPPER/stub_callsuntouched, so #2979 still merges independently.Verified on macOS (Python 3.14, AppleClang host — pure-Python change, no C++ build needed):
tools/tests/test_progress.py: 8/8 pass (6 from main + 2 carried); all othertools/tests/test_*.pypass.tools/check_conventions.py --base origin/main: pass.git diff --check: clean.2890/3370 (85.76%)with byte-identical done/todo name sets; the only delta is the newinert_nameskey (363 inert, top libs libSceAgc 49 / libSceHttp 35 / libSceHttp2 28 / libSceAgcDriver 23 / libSceNpWebApi2 16 — vs 361 at the time of the original measurement).return SupportFormat();): reports+14 implementedwith all 14 also listed under the new ⚪ behaviourally-empty block.No PR opened per the intake pause — happy for a maintainer to carry or cherry-pick; the branch will stay up.
AI-assisted: yes.
MarcZX10 commentedon Oct 11, 2026
Follow-up: inert-function reporting
I opened #3498 for the remaining reporting gap discussed here. It adds a separate detail section for newly implemented functions whose bodies only discard arguments and return a constant, return through an inert local wrapper, or throw. The implementation counts and percentages remain unchanged.
The PR carries Leandro Soares's tested implementation with its original commit authorship preserved, and adds a regression for string literals that contain braces and semicolons. On Linux with Python 3.11.2, all 95
tools/testspassed; the progress generator completed; and the done/todo counts and name sets matched currentmainexactly (2,893/3,374, 85.74%). Conventions, intake, report, relinker, and applicable platform checks currently pass. Full Linux and Windows builds are skipped automatically for pull requests under the contribution policy.oneandonlydean commentedon Oct 11, 2026
Closing: the wrapper pattern was fixed by #2979, and #3498 marks behaviourally empty bodies in the report.