Skip to content

Say when an RSI fetch was capped and keep skipped runs out of the rate (#814) - #38

Merged
Birfy merged 1 commit into
mainfrom
fix/814-rsi-pagination-buckets
Oct 23, 2026
Merged

Birfy merged 1 commit into
mainfrom
fix/814-rsi-pagination-buckets

Conversation

@Birfy

@Birfy Birfy commented Oct 23, 2026

Copy link
Copy Markdown
Owner

Written-by: Dev4

Tip SHA: 018daf8576b4db7764ec1106d768440a77a8b7a8 (base f20cfa37f9f2ffd07fa1387eece36756fbd49dbb).

Fixes ThinkFlowLab#814

Reproduce

Both under-counts were reproduced by running the code at f20cfa37, not by reading it.

skipped filed under other and counted in the denominator. The issue's own case, run against the old module:

summarizeRuns({ runs: [3 × success, 2 × skipped] })
→ all {"success":3,"failure":0,"cancelled":0,"other":2,"unknown":0}
  denominator 5, successRate 0.6

The bucket list had no skipped key at all (success,failure,cancelled,other,unknown), so a run that measured nothing moved the rate in exactly the wrong direction. The same defect is visible in the checked-in baseline: the 2026-10-22 report printed 720/825 = 87.3% with other: 49, and all 49 of those runs are conclusion: skipped.

A capped fetch returned as a total. allPages was driven with a stubbed fetch that answers 20 pages of 100 runs, all inside the day:

old code:   requests 20  returned keys runs            runs 2000  truncated undefined
new code:   requests 20  returned keys runs,truncated  runs 2000  truncated true

So a busy day's listing came back as 2000 items with no flag, no marker and exit code 0 — the number looks complete precisely when it is not. The new report line for that repository reads:

- `o/r`: the issue/PR listing stopped at the page cap — **at least 2000 items, possibly truncated**; the throughput counts above are a lower bound.
- `o/r`: the run listing stopped at the page cap — **at least 2000 runs, possibly truncated**; the counts above are a lower bound.

What changed

  • .github/scripts/rsi-baseline.mjs
    • allPages now returns { items, truncated } and takes an injectable request so the cap is tested without the network. truncated is true when the last allowed page (page 20 of 20) came back full — that is "2000 or more", the case that cannot be told apart from "exactly 2000". A page that comes back short returns truncated: false.
    • MAX_PAGES = 20 is exported and used both by the loop and by the report line, so the marker cannot drift from the cap.
    • CONCLUSION_BUCKETS gains skipped, and summarizeRuns routes it to its own bucket instead of other (timed_out, action_required, stale still go to other, unchanged).
    • rates keeps its formula success + failure + cancelled + other; skipped and unknown stay out. Comment states why cancelled is in and skipped is out.
    • renderReport gains a Skipped column (in the per-event rows, the all row and the all-unknown row), names the exclusion in the formula line, and prints the truncation bullet per repository under each table, only when that fetch was capped.
    • main carries truncatedByRepo into the report and writes one line to stderr when a listing was capped. The exit code stays 0 on purpose: a truncated day is still a day measured, and the marker — not the exit code — is the signal.
  • .github/scripts/rsi-baseline.test.mjs — five new cases: the skipped bucket and denominator arithmetic (3 success + 2 skipped → skipped: 2, denominator 3, 3/3 = 100.0%), a full 20-page sequence → truncated, 1999 items → not truncated, a short page stopping the walk at 3 requests, and a rendered report with the Skipped column, the formula wording and exactly two truncation markers for the capped repository and none for the short one.
  • docs/rsi-baseline.md — the Skipped column and the denominator rule, "a capped listing is a floor, not a total", and the test list.
  • docs/reports/2026-10-22-rsi-baseline.md — regenerated with the new bucketing. Overall 720/776 = 92.8% (was 720/825 = 87.3%), the other row 39/43 = 90.7% (was 39/92 = 42.4%), and a new Skipped column carrying the 49.

What I ran

Local, at this tip:

  • node --test .github/scripts/rsi-baseline.test.mjs → 10/10 pass.

  • node --test .github/scripts/*.test.mjs → 276/276 pass (the four ds-bot-patches cases need git on PATH; without it they fail with spawnSync git ENOENT, which is the box, not this diff — 271 before this branch, 276 with the five added here).

  • node .github/scripts/testing-layers.mjs --check → 325 test file(s) in 13 workflow(s), every test file and workflow classified.

  • node .github/scripts/changelog.mjs check → 7 fragment(s) in changelog.d/ are well formed (no fragment added: this changes a report tool and its docs, no user-visible behaviour).

  • End to end against the live API, the acceptance command:

    RSI_BASELINE_NOTES='…' node .github/scripts/rsi-baseline.mjs \
      --repo ThinkFlowLab/FlyBot --repo Birfy/FlyBot --day 2026-10-22 --out /tmp/rsi-new.md
    

    exit 0, and the diff against the checked-in report is exactly three things: the Generated timestamp, the added Skipped column with its two rate changes, and the formula line. Everything else — 158 lines, the run links, the notes — is byte-identical, so the two under-counts are the only thing that moved.

The 2026-10-22 report itself is the reproduction of the rate change: 49 skipped runs leave the denominator and the rate goes 87.3% → 92.8%.

Red without the source change

Three cuts, each made alone on a copy of this tree and run against the new tests:

  1. Put skipped back under other (KNOWN_CONCLUSIONS loses it) → 2 red: a skipped run is its own bucket and never moves the rate — skipped has its own bucket, not other / 0 !== 2; and the report carries the skipped column … — the skipped run has its own cell.
  2. Count skipped in the denominator (+ buckets.skipped in rates) → 3 red: cancelled runs stay in the denominator, missing conclusions do not (an empty day becomes denominator: NaN, successRate: NaN instead of 0 / null), a skipped run … — only the runs that produced an outcome divide the rate / 5 !== 3, and the render case — skipped is out of the rate.
  3. Make the cap silent again (allPages breaks on a short page and always returns truncated: false) → 1 red: a capped listing says it was capped, a short page does not — a full last page means 2000 or more / false !== true.

Restoring each cut returns 10/10.

Not run

  • npm run verify and the core/ suites: nothing under core/ or plugins/ changed, the whole diff is .github/scripts plus docs/.
  • No live 2000-item listing exists to exercise the cap end to end; today's busiest listing for that day is 825 runs and the issue list is in the hundreds. The cap is therefore driven through the injected request, which runs the real loop and the real return-value contract, and the marker is asserted through renderReport.
  • Fork CI on this branch: not run yet at the time of writing. The package job's Tests of the repo scripts step is what will run the 276 here.
  • actionlint: no workflow file is touched.

Risk

  • The published 87.3% becomes 92.8%. That is a record being corrected, not a measurement improving: the same 825 runs, with 49 that measured nothing moved out of the denominator. The docs say this in the "First baseline" section so the two numbers are not read as a 5.5-point improvement.
  • .github/** is T3: this needs a PASS at this tip.
  • Anyone who parses the report text rather than the numbers will see a new column and a longer formula line; per-event rows also gain a cell, so a positional parser would break. Nothing in this repository parses the report.
  • A truncated fetch still exits 0 and writes to stderr only. If a scheduled caller ever ignores stderr, the marker survives in the report body, which is the artefact people read; making truncation a non-zero exit was rejected because the day is still measured, just not entirely.
  • The cap stays at 2000 items by choice: raising it trades a silent under-count for a much slower run and more API budget, and the issue asked for visibility, not a bigger window.

ThinkFlowLab#814)

allPages ended at 20 pages of 100 without saying so, so a listing of 2000+ items was reported
as a complete total; it now returns whether the last allowed page came back full, and the
report prints "at least 2000 items, possibly truncated" for that repository with a note that
the counts are a lower bound.

skipped was filed under other and counted in the success-rate denominator, so 49 runs that
measured nothing held the 2026-10-22 rate down. It now has its own bucket, its own column and
stays out of the denominator with unknown.

The 2026-10-22 report is regenerated: 720/776 = 92.8% overall instead of 720/825 = 87.3%.
@Birfy Birfy added the agent-pr An agent opened this pull request label Oct 23, 2026
@Birfy Birfy self-assigned this Oct 23, 2026
@Birfy

Birfy commented Oct 23, 2026

Copy link
Copy Markdown
Owner Author

Verdict: PASS at 018daf8
Written-by: Reviewer

Both under-counts are closed, the regenerated report is reproducible against the live API, and every acceptance item checks out. One non-blocking formatting defect in the truncation path.

Ran:

  • node --test .github/scripts/rsi-baseline.test.mjs at this tip → 10/10.
  • node --test .github/scripts/*.test.mjs → 270 pass / 4 fail / 2 skip; the 4 are the ds-bot-patches hunk-header cases and fail identically on base f20cfa37 here (no git on PATH on this box), so they are not this diff.
  • node .github/scripts/rsi-baseline.mjs --day 2026-10-22 --repo ThinkFlowLab/FlyBot --repo Birfy/FlyBot --out … against the live API (read-only) → the output is byte-identical to the committed docs/reports/2026-10-22-rsi-baseline.md apart from the Generated line and the Notes block, which only appears when RSI_BASELINE_NOTES is set. So the regenerated numbers are not hand-edited: 825 = 720 + 56 + 0 + 49 + 0 + 0, denominator 776, 720/776 = 92.8%, other row 39/43 = 90.7%.
  • The per-event rows are unchanged (pull_request 492/515 = 95.5%, merge_group 130/152 = 85.5%): all 49 skipped runs are in event: other, so the "First baseline" prose in docs/rsi-baseline.md stays true at 95.5% and 85.5%.
  • Four mutations on copies, each restored: skipped back into KNOWN_CONCLUSIONS-less other → 2 red (the bucket test and the report test); skipped added to the denominator → 2 red; truncated: true → false at the cap → 1 red (the capped-listing test).
  • node .github/scripts/changelog.mjs check → 7 fragments well formed; testing-layers --check → 325 test files / 13 workflows, all classified.
  • CI at this tip: 9/9 success, mergeable_state: clean, labels agent-pr.

Checked, and right: MAX_PAGES is one constant used by both the loop and the message, so the marker cannot drift from the cap; a full page 20 marks "2000 or more" and a short page ends the walk at that request (the test pins 3 requests, page 4 never read); truncated is threaded per repository for both the listing and the runs, and printed per repository; the denominator keeps cancelled (a real outcome) and drops skipped and unknown; the header, per-event rows, all row and the unknown row all carry the new Skipped cell, so the markdown stays 10 columns; timed_out/action_required/stale still land in other, as the out-of-scope line asks; the docs state both rules and match the code.

One non-blocking defect: the runs marker is pushed inside the per-repository loop, so it lands between two table rows:

| `o/busy` | **all** | 1 | 1 | 0 | 0 | 0 | 0 | 0 | 1/1 = 100.0% |
- `o/busy`: the run listing stopped at the page cap — **at least 2000 runs, possibly truncated**; the counts above are a lower bound.
| `o/quiet` | **all** | 0 | 0 | 0 | 0 | 0 | 0 | 0 | unknown/not available |

In rendered markdown the - line ends the table and the next repository's row stops being a table row — exactly on the day the marker fires. Fix: collect the run markers and push them after the table loop, like the issue/PR markers already are, and put one blank line before that bullet block; then assert in the test that both marker lines come after the last table row. Related nit in the same line: "the throughput counts above are a lower bound" over-claims for net open, which comes from the search counts and is not truncated — "the issue and PR counts above" would be exact.

@github-actions github-actions Bot added the pass label Oct 23, 2026
@Birfy
Birfy merged commit f5f20b0 into main Oct 23, 2026
9 checks passed
@Birfy
Birfy deleted the fix/814-rsi-pagination-buckets branch October 23, 2026 08:07
@github-actions github-actions Bot removed the agent-pr An agent opened this pull request label Oct 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rsi-baseline.mjs stops silently at 2000 items and files skipped under other, so a day can be understated and its success rate overstated

1 participant