Repository navigation
bug(ci): the print guard scans whole changed files, so a multi-line call's noqa is invisible and the file becomes uneditable #14051
Description
Activity
Delivered by PR #14112, merged as
2437c375f.$ git show origin/Dev_new_gui:autobot-infrastructure/shared/scripts/hooks/lib/strip-significant.awk -> presentBoth halves of the issue
The multi-line
noqais recognised on the closing-paren line as well as the opening one, and the CI step now runs throughcheck-pre-commit-hook-pr.sh --changed-lines-only, reusing #13950's mechanism rather than a second implementation — so a PR is no longer blamed for a pre-existing violation in a file it merely touched.Three review rounds, and what each one caught
Round 1 found the first fix weakened the guard in four ways. The forward scan seeded its paren balance from a stripped line but then read raw lines, so anywhere raw parens diverged from real syntax the scan ran past the end of the call and accepted a
noqabelonging to a different statement. An unrelated// noqa: consoleup to 19 lines below silently exempted a live call. It also created a new bypass: deleting a closing-line noqa turned a suppressed call live while the reported line was untouched, so CI passed.Round 2 replaced the sed strip with a character tokenizer — and turned the guard off for
.vuetemplates, because"delimits an attribute in markup, not a string. Six live sites inNpuWorkerPairConfirmDialog.stories.tsthat base flagged went unreported. The finding-4 bypass also returned by a different route: when the scan exits with the balance still open, the close line was never advanced, so a single-line key was emitted and the wrapper's range check had nothing to widen.Round 3 fixed both and added a second, independent layer: a suppression is accepted only on the line where the balance actually reaches zero. That closes the whole class rather than the instances — the Python triple-quote and TS
!/++/---before-division cases stop mattering without the tokenizer having to be perfect for them.Why the layering is the important part
This is a hand-written lexer for three languages, so it will mis-lex something eventually. Verified the failure direction myself, since
a="b"is valid compact TypeScript as well as Vue markup:c1.ts const a="text with ( an open paren"; -> VIOLATION (reported, not suppressed console.log(a); by the noqa two lines below) c2.vue <button @click="console.log(x)">Go</button> -> VIOLATIONWhen the attribute heuristic misfires on compact TS, the string body stays in the balance, the balance never returns to zero on the noqa line, and the tightening refuses the suppression. A mis-lex produces a spurious report, never a missed violation.
Evidence
Miss set 30 → 24 (the legitimate closing-line-noqa sites), measured over the ~475 files that can contain a print/console call at all. The single new hit is the
mapfileimprovement — a finalprint("x")with no trailing newline that the oldreadloop silently skipped. 49 tests pass; mutation-tested against each prior round's hook.Also fixed: the hook failed open if
lib/strip-significant.awkwas missing or unparseable — every stripped line came back empty, nothing matched, exit 0. It now self-tests at startup and exits 2._common.shissourced so its absence kills the script underset -u; this new dependency was the only silent one.Behaviour change, deliberate
Editing any line inside a pre-existing unsuppressed multi-line call now fails the build, because the violation reports as a range and touching any line in that range takes responsibility for the whole call. Defensible — the PR did touch it — but stricter than #13950's stated intent, so it is recorded rather than left to be discovered.
Related
#14115 — the hook spawns a subprocess per line, so a whole-repo scan does not finish. Kept out of scope with the 50-violation baseline as its acceptance criterion.
- added a commit that references this issue
on Aug 12, 2026
Found while landing #13149 Slice A (PR #14046). Pre-existing.
Problem
The
print()/console.*guard scans whole changed files and matches line by line. A suppression comment on the closing-paren line of a multi-line call is therefore invisible to it:The guard sees
print(on the first line, finds no suppression there, and reports a violation.Consequence
autobot-backend/cli/doctor.pyhad three such calls. PR #14046 added zero prints — it changed a function signature elsewhere in the file — and still failedcode-qualitywith:So the file was effectively uneditable: any PR touching it failed CI on pre-existing, already-suppressed code it did not write. That is the same class as #13950 (the hardcoded-value guard blaming a PR for a file's backlog), which was fixed by scoping that guard to added lines — this guard did not get the same treatment.
Two separable fixes
noqaanywhere in the statement, not only on the matched line. A minimal version: when the matched line ends with an unclosed(, scan forward to the closing paren for the comment.(2) is the more valuable of the two — it is the same fix already proven for the sibling guard, and it removes the whole class rather than one shape of it.
Workaround applied meanwhile
PR #14046 moved the three suppressions onto the
print(line. Note that collapsing such calls to a single line does not surviveblack(over 120 chars once the comment is appended), so the opening-line form is the black-stable placement. Worth documenting wherever the guard is described, since the obvious fix is the one that silently reverts.Acceptance criteria
Related