Skip to content

fix(kanban-archive): report every completeness finding, not the first (backend#2526) - #357

Merged
LukasWodka merged 2 commits into
developfrom
fix/2526-report-all-completeness-findings
Aug 27, 2026
Merged

fix(kanban-archive): report every completeness finding, not the first (backend#2526)#357
LukasWodka merged 2 commits into
developfrom
fix/2526-report-all-completeness-findings

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Summary

The Assert the board is clean step has four checks. Each exited on the spot, and the most diagnostic one was ordered last — so the first to fire hid it.

Measured on the 2026-08-27 06:28 run (manual dispatch; the 05:00 cron was dropped, GitHub status still yellow):

Items this credential could read: 92 (un-archived: 92, unreadable Status: 1)
Un-archived terminal items: 3
=== Archived: 3 ok, 0 failed ===
Post-archive re-read: 92 item(s), 0 still terminal + un-archived.
##[error]1 un-archived item(s) returned an unreadable Status ...

It exited on the null Status. totalCount was never compared — and that is the only number in the whole job that does not come from this credential's own view of nodes.

So the run could not distinguish:

  • this credential sees a small board (totalCount ~ 92), from
  • this credential is served an incomplete view of a large one (totalCount ~ 1362, nodes 92)

That distinction is the open question in backend#2526, and it determines the fix. One short-circuit cost a full run's worth of diagnosis, and the next run would have cost another.

For scale: that run read 92 items. A human PAT paginating the same board reads 1362. The archiver is blind to ~93% of the board — which is why it reported green for weeks while 668 terminal cards accumulated.

What changed

All four checks record into fail and the step exits once, at the end:

  1. server totalCount vs paginated count — moved first, because it is the independent number
  2. terminal items still un-archived (was exiting first, now recorded)
  3. un-archived items with an unreadable Status
  4. a re-read that shrank versus the first read

Plus a step-summary table of the four numbers, and on a clean board an explicit Completeness: paginated N item(s), server totalCount=N (agree). — so a green run states what it verified rather than being silent, which is how this degraded unnoticed in the first place.

What this does NOT do

It does not fix the blindness. It makes the next scheduled run name it.

I went in expecting the cause to be the repositories: input narrowing the token to this repo, and measured that and was wrong — only 21 board items are .github-backed and only 1 is a draft, so 22, not 92. I am not guessing again in the same PR; the next run's totalCount line answers it.

Type

fix

Test plan

  • yaml.safe_load parses.
  • bash -n clean on the extracted run block.
  • Mutation-proved on the extracted decision block:
caseexpectedgot
all four badexit 1, 4 findingsexit 1, 4 (previously 1)
all cleanexit 0, 0 findings + agreement lineexit 0, 0
only totalCount mismatchexit 1, 1 findingexit 1, 1 (previously masked)

Checklist

  • Targets develop
  • One self-contained change (one step in one workflow)
  • Assignee set, one reviewer requested
  • No secrets, tokens or customer data

Note

Low Risk
Workflow-only change to failure ordering and logging in the kanban archive job; no production code or credential scope changes.

Overview
The Assert the board is clean step used to exit 1 on the first failing check, so a run could stop on e.g. an unreadable Status and never compare paginated items to the server’s totalCount—the only independent signal for “small board vs incomplete view.”

What changes: all four guards record into fail and the step exits once at the end. The totalCount vs paginated comparison runs in the evaluation block (with an explicit agreement line on success). Terminal leftovers, null Status on un-archived items, and a shrunk re-read vs the first read still fail the step but no longer short-circuit earlier checks.

Issue references in comments move from backend#2526 / #2528 to backend#2623. A completeness table is appended to the job step summary. This does not fix credential blindness; it ensures the next run surfaces every diagnostic number.

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

… (backend#2526)
The "Assert the board is clean" step had four checks, each exiting on the
spot. The most diagnostic one was ordered last, so the first to fire hid it.
Measured on the 2026-08-27 06:28 run: the step exited on a single null
Status, and `totalCount` -- the only number in the job that does not come
from this credential's own view of `nodes` -- was never compared. The run
therefore could not say whether the credential sees a SMALL board or is
being served an incomplete view of a LARGE one, which is the exact question
backend#2526 needs answered. That run read 92 items; a human PAT reads 1362.
One short-circuit cost a whole run's worth of diagnosis, and the next run
would have cost another.
All four now record into `fail` and the step exits once at the end, with the
server's own count checked FIRST and a step-summary table of the four
numbers. A clean board additionally prints the paginated-vs-totalCount
agreement, so a green run says what it verified instead of staying silent.
Mutation-proved on the extracted decision block:
all four bad -> exit 1, 4 findings (was 1)
all clean -> exit 0, 0 findings, prints agreement
only totalCount wrong -> exit 1, 1 finding (previously masked)
@LukasWodkaLukasWodka self-assigned this Aug 27, 2026
The workflow cited backend#2526 (7x) and backend#2528 (2x) as the source of
its own guards, including inside the ::error:: text an operator reads when
one fires. Both resolve to unrelated merged PRs:
backend#2526 -> fix(fr-gate): re-run the gate when a PR changes its base
backend#2528 -> release-train: develop -> staging (a promotion PR)
So a guard that fired sent whoever read it to an fr-gate change. No ticket
for the archiver's blindness existed at all; backend#2623 is now that ticket
and carries the 92-of-1361 measurement plus the two refuted causes.

@saadqbalsaadqbal left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is the second time I've approved a guard on this file without checking whether it could actually run, and the two misses are the same shape. On #339 I verified the shrink guard couldn't false-positive — true, and I never asked whether its two numbers were independent. On #347 I verified all three new guards fail closed — also true, and I never asked whether an earlier check exits before reaching them. So the totalCount comparison I signed off as the fix has, on the evidence here, never once executed against the real board. I checked each guard in isolation and not the control flow that reaches it, twice.

The 06:28 run makes it concrete: it exited on the null Status, and totalCount"the only number in the whole job that does not come from this credential's own view of nodes" — was never compared. So the run couldn't separate "small board" from "incomplete view of a large one", which is precisely the open question the guard exists to answer.

And the number underneath is the one to put in front of people: 92 items read, 1362 by a human PAT, blind to ~93% of the board. That is why it reported green for weeks while 668 terminal cards piled up — and it means #347's guard, had it been reachable, would have caught exactly this. Ordering turned a correct guard into a decorative one.

Verified the fix rather than taking it:

All four completeness checks now record into fail and there is a single [ "$fail" -eq 0 ] || exit 1 at the end. The exit 1s that remain are inside the pagination loop — empty response, non-array nodes, absent totalCount — and those are right to exit on the spot: you cannot accumulate findings about data you don't have, and each already says unverified is not verified-clean.

totalCount first among the comparisons is the correct ordering for the reason given — it's the independent number, so it's the one whose absence you least want masked.

The unconditional summary table is the part that makes this hold under pressure. Printing paginated / totalCount / first read / null-Status on every run including a failing one means the next person diagnosing this doesn't have to re-run to see the other three numbers. A step that reports one finding per run costs a full cycle per finding, and that cost is what this PR is actually about.

Green, no threads. 👍

@LukasWodka
LukasWodka merged commit fc052df into developAug 27, 2026
14 checks passed
@LukasWodka
LukasWodka deleted the fix/2526-report-all-completeness-findings branch August 27, 2026 06:56
Sign up for freeto 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

@LukasWodka@saadqbal