Skip to content

fix(bugbot-gate-selftest): pageInfo stripper loop visits every paged connection - #466

Merged
LukasWodka merged 1 commit into
developfrom
fix/3653-selftest-pageinfo-loop
Sep 11, 2026
Merged

fix(bugbot-gate-selftest): pageInfo stripper loop visits every paged connection#466
LukasWodka merged 1 commit into
developfrom
fix/3653-selftest-pageinfo-loop

Conversation

@LukasWodka

@LukasWodka LukasWodka commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes a vacuous case in scripts/tests/bugbot-gate-selftest.py: the pageInfo stripper loop only ever tested one of the two paged top-level connections. Tracked internally (Bugbot finding on the staging promotion #464, scripts/tests/bugbot-gate-selftest.py L913-L922 at that head).

Root cause, measured. The for name in gate.PAGED_TOPLEVEL loop ended in a break, so only checkSuites (the dict's first member) was exercised and reviewThreads never was. Its reviewThreads branch was also wrong on its own: QUERY.replace(" pageInfo { hasNextPage endCursor }\n", "", 1) uses a 6-space needle that is a substring of the 14-space-indented checkSuites line, which comes first in the query -- so even when reached it stripped checkSuites' pageInfo again and connections_missing_pageinfo answered ['checkSuites']. (Bugbot called it a no-op; measured, it is a wrong-target strip. Either way reviewThreads was never checked.) A pageInfo self-check that went blind for reviewThreads alone passed the suite green.

Change

  • scripts/tests/bugbot-gate-selftest.py: the loop now mirrors the totalCount stripper directly above it --
    1. the two members are also written as literals next to the module's dict, so a member removed from PAGED_TOPLEVEL fails rather than shrinking the loop;
    2. the stripper is anchored on the connection's own name and is indentation-agnostic (re.subn), and its substitution count is asserted == 1;
    3. the detector must name exactly the stripped connection (== [name]) -- the neighbour, or both, is a wrong answer;
    4. the loop's visit list is compared to the literals afterwards, so a break reddens the suite.
      146 -> 151 assertions.
  • scripts/tests/bugbot-gate-mutations.py: one new registry row, the exact regression the finding named -- connections_missing_pageinfo walking only the first connection (for name in list(PAGED_TOPLEVEL)[:1]:). The anchor is a two-line exact match; --dry resolves it once.

Test plan / evidence

Run with the CI Python (3.12 venv, pyyaml==6.0.2), from the worktree root.

  • make check (ruff + shellcheck + house-rules + action-pins + mint-scope + reusable-no-cancel + lint-targets + actionlint + mutations-dry + all 31 selftests): green -- selftests-cover: all 31 selftests and 18 mutation runner(s) are wired to a target, and CI runs both tiers, bugbot-gate-selftest: 151 assertions, all passed, ==> check: green.
  • make mutation-bugbot-gate (the CI shard for this suite): 64 mutation(s): 0 stale, 0 uncaught.
  • make mutations-dry: every runner resolves, 0 stale.

Mutation proof, both directions:

  • Registry row against the old suite (fix not yet applied): 64 mutation(s): 0 stale, 1 uncaught -- UNCAUGHT the pageInfo self-check walks only the first connection, so reviewThreads' cursor can be dropped from the query unnoticed. Against the fixed suite: caught ... by: dropping pageInfo from 'reviewThreads' is detected, and 'reviewThreads' alone is named -- detector said []. 0 uncaught.
  • Reintroducing break after pageinfo_visited.append(name) in the fixed loop: suite exits 1 with FAIL: the pageInfo stripper loop visited every paged top-level connection -- visited ['checkSuites'], expected ['checkSuites', 'reviewThreads']. Restored.

No workflow or shell files touched; scripts/bugbot-gate.py itself is unchanged.


Note

Low Risk
Test-harness and mutation-registry changes only; production gate logic is untouched.

Overview
Closes a vacuous selftest hole where the pageInfo stripper only ever exercised checkSuites (early break) and never validated reviewThreads—so a gate self-check blind to reviewThreads could still pass.

bugbot-gate-selftest.py reworks that loop to mirror the existing totalCount stripper: literal checkSuites / reviewThreads membership checks, name-anchored re.subn with applied == 1, and connections_missing_pageinfo(stripped) == [name] so the detector must name exactly the connection stripped. A final assertion requires the loop to visit both connections (a reintroduced break fails).

bugbot-gate-mutations.py adds one mutation for the measured regression—connections_missing_pageinfo iterating only list(PAGED_TOPLEVEL)[:1]—so the old suite would have reported it uncaught.

scripts/bugbot-gate.py is unchanged; assertion count rises (146 → 151 per PR description).

Reviewed by Cursor Bugbot for commit 4d721b5. Bugbot is set up for automated code reviews on this repo. Configure here.

…connection

The pageInfo stripper loop in the Bugbot gate selftest ended in a `break`,
so only the first member of PAGED_TOPLEVEL (checkSuites) was ever exercised.
Its reviewThreads branch pasted a fixed 6-space indentation into str.replace,
a needle that is a substring of the 14-space checkSuites line, so even when
reached it stripped checkSuites' pageInfo again and the detector answered
['checkSuites']. A pageInfo self-check blind to reviewThreads alone passed
the suite.

The loop now mirrors the totalCount stripper: literal member list beside the
module's dict (a removed member fails), a name-anchored indentation-agnostic
regex with its substitution count asserted, the detector required to name
exactly the stripped connection, and the visit list compared to the literals
so a reintroduced break reddens the suite. A registry mutation pins the
reviewThreads-only regression; it was UNCAUGHT against the old suite and is
caught now.

Tracked internally (Bugbot finding on the staging promotion #464).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@LukasWodka LukasWodka self-assigned this Sep 11, 2026
@LukasWodka

Copy link
Copy Markdown
Contributor Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 4d721b5. Configure here.

@saqlainsyed007 saqlainsyed007 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving. Verified against the source at 4d721b5: PAGED_TOPLEVEL has exactly checkSuites/reviewThreads, and the old break+6-space-substring needle really did leave reviewThreads unchecked (and mis-targeted checkSuites). The reworked loop is name-anchored and indentation-agnostic with applied == 1 and connections_missing_pageinfo(stripped) == [name], and the trailing sorted(pageinfo_visited) == sorted(PAGED_TOPLEVEL_LITERALS) reddens on a reintroduced break or an unlisted new member — mutation-proof both directions, and the new registry row's two-line anchor matches the live detector exactly. CI is fully green and there are no open review threads.

@LukasWodka
LukasWodka merged commit b560aa4 into develop Sep 11, 2026
45 of 47 checks passed
@LukasWodka
LukasWodka deleted the fix/3653-selftest-pageinfo-loop branch September 11, 2026 06:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants