Skip to content

gate-50's 10-line window counts comment lines — 8 of 16 findings in opencatalogi were guards pushed out of range by the comment explaining them #415

Description

@rubenvdlinde

Summary

gate-50 looks for a fail-mode guard in a 10-line window anchored at the end of the config read. In opencatalogi, 8 of its 16 findings were reads whose guard genuinely exists, 12 to 89 lines downstream — and in the sharpest case the thing separating the read from its guard was the comment explaining the guard.

I fixed opencatalogi by moving each read to sit directly above its own check (ConductionNL/opencatalogi#867), which is better code and which I would do again. But the window will keep producing this class, and it will produce it most often in the code that documents itself best — which is the code this fleet asks people to write.

The measurement

opencatalogi at 74e12c82, canonical package 5e73e640, full tree. For each of the 16 findings I measured the distance from the read to the nearest line matching gate-50's own guard vocabulary:

8 reads   guard 12-89 lines downstream    -> real guard, outside the window
8 reads   no guard within 60 lines        -> genuinely unguarded (all 8 fixed as real defects)

The eight false positives, with the distance and the guard that was already there:

site distance guard
SetupController.php:299 / :300 13 / 12 if ($publicationRegister !== ''), if ($publicationSchema !== '')
CatalogiService.php:371 / :372 22 / 21 if ($schema === '' || $register === '') + logger->error
DirectoryService.php:2258 / :2259 27 / 26 if (empty($listingSchema) === false && empty($listingRegister) === false)
DirectoryService.php:2260 / :2261 89 / 88 if (empty($catalogSchema) === false && empty($catalogRegister) === false)

🔑 CatalogiService::getCatalogFilters is the one that matters. Its guard is 22 lines away because a 20-line comment explaining why the guard is necessary sits between the read and the check. That comment is exactly what this fleet's review culture asks for, and it is what made the gate call the site unguarded.

Same repo, same file: DirectoryService.php:277-284 is the identical pattern with the guard 6 lines away, and it passes. So the gate is not measuring whether a guard exists; it is measuring how much prose the author wrote between the two.

Why this matters more than a normal false-positive rate

The gate's own source already carries the argument against unclosable findings (#252, quoted in the _ARRAY_VALUE exemption): "A finding here names a line at which no closing action exists… 47 of them in one repo is how a gate teaches people to stop reading it." A finding whose only remedy is move your comment is close to that shape — the code is already correct, and the gate is asking for a formatting change.

And the failure direction is the dangerous one for adoption: eight of sixteen is a 50% false-positive rate on first contact, which is how a gate's future silences stop being believed.

Suggestions, in order of preference

  1. Skip comment and blank lines when counting the window. Ten lines of code is the intent; ten lines of text is what is implemented. This is a small change to the window construction and it fixes every case above except the 88/89 pair, without loosening the rule for the single-line shape the gate was built on.
  2. Widen the window to the end of the enclosing statement block or ~30 lines, whichever is shorter.
  3. Failing either, report the distance in the finding ("guard found 22 lines away, outside the 10-line window" vs "no guard found"), so a reader can tell a formatting finding from a security finding without opening the file. Right now the two are byte-identical in the log, and that is what cost the triage time here.

Sibling

.github#358 records gate-26 being fooled by comments (a comment saying you have not written a baseline satisfied the check for having written one). This is the same seam from the other side: one gate reads comments it should ignore, the other is defeated by their length. Worth a single pass over both windows with js_comment_mask-style handling on the PHP side.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions