Repository navigation
bug(ci): the inline_generics ratchet counts comments, so documenting the fix fails the build #15771
Description
Activity
A second independent case, and the count is 2 of 5 known-affected
Verified against
origin/Dev_new_gui. The file carries five source-scanning
regexes, and all five are applied to raw_read()output with no comment
stripping anywhere in the file::109 _CLIENT_CLASS_RE -> :159 findall(_read(path)) :114 _RAW_FETCH_RE -> :169 search(_read(path)) :115 _AXIOS_IMPORT_RE -> :177 search(_read(path)) :116 _RESPONSE_INTERFACE_RE -> :182 findall(_read(path)) :126 _INLINE_GENERIC_RE -> :187 findall(_read(path))grep -cfor any comment-stripping helper in the file returns 0.Two hits, two months apart, same trap
_RAW_FETCH_RE— hit first. Prose like"a failed fetch (#14064)"and a
test name"share ONE voices fetch (no self-aborting race)"were counted as
transport sites. Diagnosed and fixed by narrowing that one regex: the
(?<![.\w])lookbehind and the removal of\s*before the paren, with the
reasoning written in the comment at:110-113._INLINE_GENERIC_RE— hit again today, by a different session, in the
most pointed way available: a comment written to explain why an inline
generic had been avoided contained the token, the ratchet counted it, and
documenting the fix re-failed the check that the fix had just passed.
So the count is 2 of 5 known-affected. The remaining three —
_CLIENT_CLASS_RE,_AXIOS_IMPORT_RE,_RESPONSE_INTERFACE_RE— are
untested, not proven safe._AXIOS_IMPORT_REmatchingfrom 'axios'inside
a comment quoting an import it replaced is the same shape and is a normal thing
for a migration comment to contain.Why this sharpens the fix rather than just adding a case
The first fix worked and taught nobody, because the mitigation lived
inside the regex it repaired. Two months later the identical failure arrived
through a sibling regex nine lines below, in a file that already documented the
trap in its own source.A per-pattern fix cannot generalise by construction: it leaves the next pattern,
and every pattern added afterwards, to rediscover the problem in production CI.
A per-file fix — strip comments and string literals once, before any pattern
runs — retires the whole class and makes the:110-113comment a historical
note rather than a rule the reader has to remember to re-apply.Revised acceptance criteria
Superseding the list above:
- Comments and string literals are stripped once, for all five patterns,
before counting - A fixture file containing each of the five patterns only inside a
comment counts zero for all five — one fixture, five assertions - The same for each pattern inside a string literal
- Counts re-baselined in the same change if stripping moves them, so the
ratchet does not fail on its own fix -
_RAW_FETCH_RE's narrowing at:114is left in place but the comment at
:110-113is updated to say the general mitigation now lives at file
level, so the next contributor does not copy the per-regex approach
Provenance
Second case reported by a concurrent session; the five-regex inventory and the
zero-comment-stripping check verified independently against base before adding._AXIOS_IMPORT_REis the one to fix first — a prediction, checkedPromoting this from one of three untested patterns to the headline, because it
is the most exposed and because I can now say precisely where it stands.Checked in base, and it has not fired yet. Searching for
from 'axios'
inside a comment across both frontends returns nothing. The search had subjects
— 10 files matchfrom 'axios'at all — so this is a meaningful negative and
not an empty loop reading as clean.So today's
axios_importscount is trustworthy. That is exactly what makes it
worth acting on now rather than after it breaks.Why this pattern is the most exposed of the three untested ones. Both known
hits were a comment describing the code that was replaced:_RAW_FETCH_RE— prose about a removedfetch(call_INLINE_GENERIC_RE— a comment explaining an inline generic that had been
avoided
_AXIOS_IMPORT_REmatchesfrom 'axios'. A comment quoting the axios import it
replaced is not a hypothetical edge case — it is the single most natural comment
to write while performing the migration this ratchet exists to measure. The
migrated files inautobot-slm-frontendalready carry exactly this style of
comment for other constructs:useOrchestration.ts:24,useExternalAgents.ts:23
anduseOrchestrationManagement.ts:42each quote theaxios.create(...)they
replaced. Those quote the call, which no pattern here matches. The moment one
quotes the import, the count moves and the ratchet fails a correct change.The prediction, stated so it can be checked later: the next contributor who
migrates a file off axios and documents what they removed will fail
frontend_api_contract_ratchet_testonaxios_imports, for a change that
reduced axios usage. Same as the two previous hits, on the pattern where the
triggering comment is most idiomatic.That is the argument for a file-level strip over a third per-regex narrowing:
the fix should land before the case, not after it, and this is the rare instance
where we can see which one is next.Ordering for whoever takes this
- File-level comment/string stripping for all five patterns — retires the class
_AXIOS_IMPORT_REfixture asserting a comment-onlyfrom 'axios'counts zero
— the predicted case, pinned before it happens_CLIENT_CLASS_REand_RESPONSE_INTERFACE_REfixtures — lower risk, both
need a fairly specific comment to trigger
Cross-link: this guard's defect has a second consequence beyond the false red.
repo_tests/frontend_api_contract_ratchet_test.py:71-94freezes"inline_generics": 577and87, and those numbers were produced by_inline_generic_countat:185— the same counter that counts comments. So the baseline is not a count of inline generics; it is 577 detector-hits, an unknown number of which are comment-borne, and no inspection of the number separates them.Practical consequence for whoever fixes this: regenerate the baseline after the fix, don't carry it. A corrected counter against an uncorrected baseline will read as a large improvement that nobody made, and the new number will be the first honest one.
Filed the general pattern as #15897 — every ratchet baseline in the repo is produced by its own detector, so a detector blind spot becomes a silent permanent exemption. This is its worked case where the detector is already proven defective.
Re-derived the baseline independently — the corrected numbers your fix should adopt are 573 (autobot-frontend, from 577) and 86 (autobot-slm-frontend, from 87).
Method and full working on #15897. The control that makes it trustworthy: a separate enumeration (
git ls-treerather thanrglob) reproduces the detector's raw 577/87 exactly, so the delta is attributable to matching rather than to scope. Comments were stripped with a character-wise lexer that preserves string literals, so//inside"https://…"isn't mistaken for a comment.All five over-counted hits are JSDoc examples —
useApiResource.ts:17and:25,useErrorHandler.ts,useFakeProgress.ts,usePrometheusMetrics.ts:170. I also checked for a compensating undercount fromapiClient\n .get<T>(split across lines: zero in both apps.So the correction is small — 4 and 1. Lower the baseline in the same commit as the counter fix. Leaving 577/87 against a corrected counter grants four and one free violations that nobody decided to give, and afterwards that slack is indistinguishable from work that was never done.
The
inline_genericsratchet counts the construct's own prose as the constructrepo_tests/frontend_api_contract_ratchet_test.py:187_read()returns raw file text. Nothing strips comments, so the patternmatches inside a comment exactly as it does in code. A contributor who removes an
inline generic and explains the removal in a comment leaves the count
unchanged, and the ratchet — shrink-only, currently 577 for
autobot-frontend—fails their build for a correct change.
Observed today: a session fixed a real inline generic by taking the type from
components['schemas'][...]and annotating the assignment, and CI still failed,because the comment describing what had been removed spelled the token out.
The file already knows about this trap
Nine lines above, on a sibling pattern (
:110-114):So the failure mode was hit, diagnosed, and mitigated for
_RAW_FETCH_REonly — by tightening that one regex. The lesson was not generalised, and
_INLINE_GENERIC_REhas neither a lookbehind nor comment stripping.That is the substance of this issue: not that one regex is loose, but that a
per-pattern mitigation was applied where a per-file one was needed. The next
pattern added will inherit the same defect.
Why it bites harder than the sibling did
_RAW_FETCH_RE's false positives inflated a count. This one actively punishesthe fix: the more clearly a contributor documents removing an inline generic,
the more likely their build fails. It rewards silent changes over explained ones,
which is backwards for a rule whose whole purpose is to teach a pattern.
Fix
Strip comments and string literals before counting, once, for every pattern —
rather than hardening each regex against prose individually. A minimal
line-oriented strip (
//,/* */, and template/quoted strings) is enough for.ts/.vuesources and removes the whole class.Failing that, at minimum give
_INLINE_GENERIC_REthe same treatment_RAW_FETCH_REreceived, and note in the file that every future pattern needs it.Acceptance criteria
api.get<Shape>countszero inline generics — proven by a fixture, not by inspection
change, so the ratchet does not fail on the fix itself
inherits it
Provenance
Reported by a concurrent session that hit it on #15753; verified independently
against
origin/Dev_new_guibefore filing, including the_RAW_FETCH_REprecedent that shows the trap was already known here.