mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-02 06:54:50 +01:00
fix(scorecard): page-capped reads report unavailable, releases page to the window, one instant comparison for runs
Pre-landing review fixes for GST-135: readMergedPrs, readGateRuns and readReleases throw past MAX_PAGES instead of returning a partial count; readReleases pages until it reaches the window start; listWorkflowRuns compares both bounds with Date.parse and skips unparsable timestamps; tests for a sub-second window, releases behind a full page and the PR page cap; doc wording on the red-train row and the unverified 142-PR baseline removed. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
40fe5c008e
commit
c7f30e55b8
5 files changed
+66
-22
No files matched your search
+5
-5
@@ -2,15 +2,15 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
**`gstack-factory-scorecard --now` now ends the window, so a past week reports that week's numbers.** Every line counts only what happened in `[now - since-days, now)`. Before, `--now` moved only the start, so `--now 2026-10-15T00:00:00Z --since-days 14` counted 290 merged PRs (every merge from 2026-10-01 through the day of the run) where the window holds 65, and the gate-run and release lines also counted through the day of the run. Rerun with the fix, the two weeks before 2026-10-15 show 65 merged PRs and the two weeks before 2026-10-29 show 197, matching GitHub search for both windows.
|
||||
**`gstack-factory-scorecard --now` now ends the window, so a past week no longer counts what happened after it.** Every line counts only items whose timestamps fall in `[now - since-days, now)`. Before, `--now` moved only the start, so `--now 2026-10-15T00:00:00Z --since-days 14` counted 290 merged PRs (every merge from 2026-10-01 through the day of the run) where the window holds 65, and the gate-run and release lines also counted through the day of the run. Rerun with the fix, the two weeks before 2026-10-15 show 65 merged PRs and the two weeks before 2026-10-29 show 197, matching GitHub search for both windows.
|
||||
|
||||
### Fixed
|
||||
- **`lib/factory-scorecard.ts`**: one window (`scoreWindow`, `inWindow`) bounds merged PRs and their lead times, gate runs and their cost, the red-train rate, flaky events, free-suite retries, issues and releases. The workflow-runs query asks GitHub for the window's days (`created=<start>..<end>`), and the issue searches use a closed time range instead of `>=start`.
|
||||
- **`scripts/lib/ci-history.ts`**: `listWorkflowRuns` takes an optional `until` (exclusive) and `createdFilter` builds the runs API's `created` query for both callers.
|
||||
- **`docs/recommended-harness.md`**: "Where gstack's own numbers stand" is remeasured for the exact week of 2026-10-19 through 2026-10-25 UTC: 83 merged PRs (not 103), median lead time 6.5h, 54 gate runs (52 green, 2 red), $1,361.94 of known gate cost, 0 releases. The earlier figures counted through midday on 2026-10-26.
|
||||
- **`lib/factory-scorecard.ts`**: one window (`scoreWindow`, `inWindow`) bounds merged PRs and their lead times, gate runs and their cost, the red-train rate, flaky events, free-suite retries, issues and releases. The workflow-runs query asks GitHub for the window's days (`created=<start>..<end>`), and the issue searches use a closed time range instead of `>=start`. Releases page past newer ones until they reach the window start, and a PR, gate-run or release read that runs past 20 pages now marks its line unavailable instead of printing a partial count.
|
||||
- **`scripts/lib/ci-history.ts`**: `listWorkflowRuns` takes an optional `until` (exclusive), compares both ends of the window as instants and skips a run whose `created_at` does not parse; `createdFilter` builds the runs API's `created` query for both callers.
|
||||
- **`docs/recommended-harness.md`**: "Where gstack's own numbers stand" is remeasured for the exact week of 2026-10-19 through 2026-10-25 UTC: 83 merged PRs (not 103), median lead time 6.5h, 54 gate runs (52 green, 2 red), $1,361.94 of known gate cost, 0 releases. The earlier figures came from a run before this fix, so they counted past the end of the week.
|
||||
|
||||
### Tests
|
||||
- `test/factory-scorecard.test.ts`: a PR merged exactly at `--now`, and a gate run, free run and release after it, are left out; the runs and issue queries carry the window's end.
|
||||
- `test/factory-scorecard.test.ts`: a PR merged exactly at `--now`, and a gate run, free run and release after it, are left out; the runs and issue queries carry the window's end. A sub-second `--now` keeps the issue search range inside the window, releases behind a full page of newer ones are found, and a PR read past the page cap is unavailable.
|
||||
|
||||
## [1.91.327.0] - 2026-10-30
|
||||
|
||||
|
||||
@@ -297,7 +297,7 @@ artifacts read for all 54):
|
||||
| Review passes per PR | no number: 0 of 83 PRs carried a pass count | the receipt's `passes` field lands with /ship's convergence rule; until then crews record the count in the READY block, and the marker reads it from a PR comment |
|
||||
| Gate runs | 52 green / 2 red / 0 cancelled; known cost $1,361.94 on 52 of 54 runs | green rate 96%; cost unknown only on the 2 red runs (the red train and the red screen) |
|
||||
| Known gate cost per merged PR | $16.41 (a floor) | trains averaged $32.48 each for up to 5 PRs (21 trains with a known cost), screens $21.18 each (29 with a known cost) |
|
||||
| Red-train rate | 5% (1 red of 22 trains; 0 red backstop, 1 red screen) | 1 of 22 is 4.5%, just under the bar |
|
||||
| Red-train rate | 5% (1 red of 22 trains; 0 red backstop, 1 red screen) | the tool rounds 1 of 22 (4.5%) to 5%, so this is just under the 5% bar; the 1 red screen is counted separately from trains |
|
||||
| Flaky events | 17 paid cases passed on their one rerun (rerun spend $7.20); 17 free-suite retries in 17 of 83 runs | above the bar on both: 0.31 flaky cases per gate against 0.1, and 20% of free runs retried against 1%. One case (`ship-docsync-current`) did it four times in three days |
|
||||
| Issues opened / closed | 28 / 51 | inside the bar |
|
||||
| Releases cut | 0 | none inside the week: the first weekly release, v1.91.242.0, was published at 12:01 UTC on 2026-10-26, just after the window closed (see the starvation caveat above) |
|
||||
@@ -306,8 +306,7 @@ Read the gap honestly: lead time and throughput are where they should be, the
|
||||
gate is green and its cost is mostly known, and flakiness is the line that is
|
||||
not. The daily pass exists for exactly that line; the scorecard's job is to
|
||||
show next week whether it moved. The inventory lane's per-PR baseline CSV in
|
||||
the Captain's Drive (142 merged PRs over the eleven days before this window
|
||||
closed) holds the longer history behind these numbers; from here on
|
||||
the Captain's Drive holds the longer history behind these numbers; from here on
|
||||
`trend.csv` in `--out-dir` carries the weeks.
|
||||
|
||||
## What is Capy-specific and what is not
|
||||
|
||||
@@ -6,8 +6,11 @@
|
||||
* so ("unavailable: <reason>") instead of printing 0.
|
||||
*
|
||||
* The window is [now - since-days, now): every input below counts an item only
|
||||
* when its timestamp falls inside it (inWindow), so a past window reproduces
|
||||
* after the fact.
|
||||
* when its timestamp falls inside it (inWindow), so a past window does not
|
||||
* count anything that happened after it. Attributes are read as they are now
|
||||
* (a run's latest conclusion, an issue's latest closed_at, artifacts that have
|
||||
* not expired). A read that runs past MAX_PAGES marks its line unavailable
|
||||
* instead of printing a partial count.
|
||||
*
|
||||
* Inputs, all REST reads (no write, no paid work):
|
||||
* merged PRs pulls?state=closed&base=<base>, merged_at inside the window;
|
||||
@@ -72,6 +75,7 @@ export const DEFAULTS = {
|
||||
export const CREW_ASSOCIATIONS = new Set(['OWNER', 'MEMBER', 'COLLABORATOR']);
|
||||
const PAGE = 100;
|
||||
const MAX_PAGES = 20;
|
||||
const pageCap = (what: string) => new Error(`${what} ran past ${MAX_PAGES} pages of ${PAGE}; a partial count is not reported`);
|
||||
const COST_LINE = /\*\*\$(\d+(?:\.\d+)?)\*\* rule\/judge cost|\bcost=\$(\d+(?:\.\d+)?)/g;
|
||||
|
||||
export interface MergedPr {
|
||||
@@ -165,7 +169,8 @@ function pullsPage(source: ScorecardSource, o: ScorecardOptions, page: number):
|
||||
export function readMergedPrs(source: ScorecardSource, o: ScorecardOptions, w: ScoreWindow): MergedPr[] {
|
||||
const out: MergedPr[] = [];
|
||||
const crewLogins = o.crew ? new Set(o.crew.map(l => l.toLowerCase())) : null;
|
||||
for (let page = 1; page <= MAX_PAGES; page++) {
|
||||
for (let page = 1; ; page++) {
|
||||
if (page > MAX_PAGES) throw pageCap(`PRs updated since ${w.start}`);
|
||||
const pulls = pullsPage(source, o, page);
|
||||
for (const pr of pulls) {
|
||||
const mergedAt = typeof pr.merged_at === 'string' ? pr.merged_at : null;
|
||||
@@ -266,7 +271,8 @@ export function readGateRuns(source: ScorecardSource, o: ScorecardOptions, w: Sc
|
||||
const query = [`status=completed`, `per_page=${PAGE}`, createdFilter(w.start, w.end),
|
||||
...(o.gateEvent ? [`event=${encodeURIComponent(o.gateEvent)}`] : [])].join('&');
|
||||
const gates: GateRun[] = [];
|
||||
for (let page = 1; page <= MAX_PAGES; page++) {
|
||||
for (let page = 1; ; page++) {
|
||||
if (page > MAX_PAGES) throw pageCap('gate runs in the window');
|
||||
const body = source.getJson(`repos/${o.repo}/actions/workflows/${o.gateWorkflow}/runs?${query}&page=${page}`) as { workflow_runs?: any[] };
|
||||
const runs = Array.isArray(body?.workflow_runs) ? body.workflow_runs : [];
|
||||
for (const run of runs) {
|
||||
@@ -317,10 +323,18 @@ function searchCount(source: ScorecardSource, q: string): number | null {
|
||||
return typeof body?.total_count === 'number' ? body.total_count : null;
|
||||
}
|
||||
|
||||
/** Releases published inside the window, drafts excluded; pages (newest first) stop once a page's oldest publication predates the window. */
|
||||
export function readReleases(source: ScorecardSource, repo: string, w: ScoreWindow): Array<{ tag: string; publishedAt: string }> {
|
||||
const body = source.getJson(`repos/${repo}/releases?per_page=${PAGE}&page=1`);
|
||||
return (Array.isArray(body) ? body : []).filter(r => !r.draft && inWindow(r.published_at, w))
|
||||
.map(r => ({ tag: String(r.tag_name ?? ''), publishedAt: String(r.published_at) }));
|
||||
const out: Array<{ tag: string; publishedAt: string }> = [];
|
||||
for (let page = 1; ; page++) {
|
||||
if (page > MAX_PAGES) throw pageCap(`releases published since ${w.start}`);
|
||||
const body = source.getJson(`repos/${repo}/releases?per_page=${PAGE}&page=${page}`);
|
||||
const releases = Array.isArray(body) ? body : [];
|
||||
out.push(...releases.filter(r => !r.draft && inWindow(r.published_at, w)).map(r => ({ tag: String(r.tag_name ?? ''), publishedAt: String(r.published_at) })));
|
||||
const published = releases.map(r => r.published_at).filter((at): at is string => typeof at === 'string');
|
||||
if (releases.length < PAGE || (published.length && published[published.length - 1]! < w.start)) break;
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
/** Every read, each guarded: one unreadable input marks its lines unavailable and the rest still print. */
|
||||
|
||||
@@ -102,9 +102,10 @@ export function createdFilter(since: string, until?: string): string {
|
||||
|
||||
/**
|
||||
* Every completed run of `workflow` created at or after `since` (and before
|
||||
* `until` when given), newest first, paging until a page is short or older
|
||||
* than `since`. A failure on any page throws: a partial history is never
|
||||
* reported as a complete one.
|
||||
* `until` when given), newest first, paging until a page is short (the
|
||||
* `created` filter bounds the range server-side). A run whose created_at does
|
||||
* not parse is skipped. A failure on any page throws: a partial history is
|
||||
* never reported as a complete one.
|
||||
*/
|
||||
export function listWorkflowRuns(client: GhClient, repo: string, workflow: string,
|
||||
opts: { since: string; until?: string; branch?: string; event?: string; maxPages?: number }): CiRun[] {
|
||||
@@ -115,7 +116,8 @@ export function listWorkflowRuns(client: GhClient, repo: string, workflow: strin
|
||||
const body = client.getJson(`repos/${repo}/actions/workflows/${workflow}/runs?${query.join('&')}&page=${page}`) as { workflow_runs?: any[] };
|
||||
const runs = Array.isArray(body?.workflow_runs) ? body.workflow_runs : [];
|
||||
for (const run of runs) {
|
||||
if (String(run.created_at) < opts.since || (opts.until && Date.parse(String(run.created_at)) >= Date.parse(opts.until))) continue;
|
||||
const created = Date.parse(String(run.created_at));
|
||||
if (!(created >= Date.parse(opts.since)) || (opts.until && created >= Date.parse(opts.until))) continue;
|
||||
if ((opts.branch && run.head_branch !== opts.branch) || (opts.event && run.event !== opts.event)) continue;
|
||||
out.push({ id: Number(run.id), attempt: Number(run.run_attempt ?? 1), sha: String(run.head_sha ?? ''), branch: String(run.head_branch ?? ''),
|
||||
event: String(run.event ?? ''), status: String(run.status ?? ''), conclusion: run.conclusion ?? null,
|
||||
|
||||
@@ -11,8 +11,8 @@ import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
import {
|
||||
BENCHMARKS, DEFAULTS, classifyGate, collectScorecard, fixtureSource, flakeRecords, fmtHours, gateOutcome, passCount, percentile, readComments, renderCsv,
|
||||
renderMarkdown, scorecardRows, verdictCost, type FixtureFile, type ScorecardOptions,
|
||||
BENCHMARKS, DEFAULTS, classifyGate, collectScorecard, fixtureSource, flakeRecords, fmtHours, gateOutcome, inWindow, passCount, percentile, readComments, renderCsv,
|
||||
renderMarkdown, scorecardRows, searchRange, verdictCost, type FixtureFile, type ScorecardOptions,
|
||||
} from '../lib/factory-scorecard';
|
||||
|
||||
const ROOT = path.resolve(import.meta.dir, '..');
|
||||
@@ -125,7 +125,7 @@ describe('factory-scorecard: recorded window', () => {
|
||||
});
|
||||
|
||||
test('--now closes the window: a PR, gate run, free run or release at or after now is left out, and the runs and issue queries carry the end', () => {
|
||||
// Value: protects=every windowed line counts [now - since-days, now) only, so a past window reproduces after the fact; fails_when=a read filters on the window start alone or a query drops the end bound; why_new=GST-135: --now 2026-10-15 counted 284 merged PRs (every merge through the run date) against 65 in the window, and the gate and release lines ignored --now; seam=none; no_claim=GitHub's server-side date filters, which the reads re-check per item
|
||||
// Value: protects=every windowed line counts [now - since-days, now) only, so a past window counts nothing that happened after it; fails_when=a read filters on the window start alone or a query drops the end bound; why_new=GST-135: --now 2026-10-15 counted 290 merged PRs (every merge through the 2026-10-30 run) against 65 in the window, and the gate and release lines ignored --now; seam=none; no_claim=GitHub's server-side date filters, which the reads re-check per item
|
||||
const fx = fixture();
|
||||
const key = (part: string) => Object.keys(fx.responses).find(k => k.includes(part))!;
|
||||
(fx.responses[key('/pulls?')] as any[]).unshift({ number: 106, title: 'merged at the window end', user: { login: 'garry' }, author_association: 'OWNER',
|
||||
@@ -158,6 +158,26 @@ describe('factory-scorecard: recorded window', () => {
|
||||
]);
|
||||
});
|
||||
|
||||
test('a past window pages past newer releases, and a read that runs past the page cap is unavailable instead of partial', () => {
|
||||
// Value: protects=a past window still finds its releases behind a full page of newer ones, and a PR read that never reaches the window start reports unavailable rather than a short count; fails_when=releases read page 1 only again or the PR loop exits at MAX_PAGES without throwing; why_new=adversarial review of GST-135 found both truncations silent for past windows; seam=none; no_claim=GitHub's ordering of releases beyond newest-first
|
||||
const fx = fixture();
|
||||
const releasesKey = (page: number) => `repos/acme/widgets/releases?per_page=100&page=${page}`;
|
||||
const recorded = fx.responses[releasesKey(1)] as any[];
|
||||
fx.responses[releasesKey(1)] = Array.from({ length: 100 }, (_, i) => ({ tag_name: `v2.0.${i}`, draft: false, prerelease: false, published_at: '2026-10-27T00:00:00Z' }));
|
||||
fx.responses[releasesKey(2)] = recorded;
|
||||
expect(collectScorecard(fixtureSource(fx, scratch()), options()).releases.map(r => r.tag)).toEqual(['v1.0.0']);
|
||||
const pullsKey = (page: number) => `repos/acme/widgets/pulls?state=closed&base=main&sort=updated&direction=desc&per_page=100&page=${page}`;
|
||||
const capped = fixture();
|
||||
for (let page = 1; page <= 20; page++) {
|
||||
capped.responses[pullsKey(page)] = Array.from({ length: 100 }, (_, i) => ({ number: page * 1000 + i, title: 'closed without merge', user: { login: 'garry' },
|
||||
author_association: 'OWNER', created_at: '2026-10-25T00:00:00Z', merged_at: null, updated_at: '2026-10-25T00:00:00Z' }));
|
||||
}
|
||||
const card = collectScorecard(fixtureSource(capped, scratch()), options());
|
||||
expect(card.unavailable).toHaveLength(1);
|
||||
expect(card.unavailable[0]).toStartWith('merged PRs: PRs updated since 2026-10-19T12:00:00.000Z ran past 20 pages of 100');
|
||||
expect(byId(scorecardRows(card)).merged).toStartWith('unavailable (');
|
||||
});
|
||||
|
||||
test('--max-artifact-runs caps artifact reads: later runs count without cost or flake data', () => {
|
||||
const card = collectScorecard(fixtureSource(fixture(), scratch()), options({ maxArtifactRuns: 2 }));
|
||||
expect(card.artifactRunsRead).toBe(2);
|
||||
@@ -300,6 +320,15 @@ describe('factory-scorecard: pieces', () => {
|
||||
expect(flakeRecords([path.join(dir, 'missing')])).toEqual({ flakes: 0, rerunUsd: 0 });
|
||||
});
|
||||
|
||||
test('the window is end-exclusive at millisecond precision; the issue search range stays inside it at whole seconds', () => {
|
||||
// Value: protects=a sub-second --now (the default, Date.now()) neither drops an issue at the end second nor counts one before the start; fails_when=either rounding in searchRange flips or inWindow stops rejecting unparsable timestamps; why_new=the --now test uses a whole-second now only; seam=none; no_claim=GitHub search's inclusive-range semantics
|
||||
const w = { start: '2026-10-19T12:00:00.500Z', end: '2026-10-26T12:00:00.500Z' };
|
||||
expect(searchRange(w)).toBe('2026-10-19T12:00:01Z..2026-10-26T12:00:00Z');
|
||||
expect(searchRange({ start: '2026-10-19T12:00:00.000Z', end: '2026-10-26T12:00:00.000Z' })).toBe('2026-10-19T12:00:00Z..2026-10-26T11:59:59Z');
|
||||
expect(['2026-10-19T12:00:00Z', '2026-10-19T12:00:01Z', '2026-10-26T12:00:00Z', '2026-10-26T12:00:01Z'].map(at => inWindow(at, w))).toEqual([false, true, true, false]);
|
||||
expect([undefined, null, '', 'soon', 1793000000000].map(at => inWindow(at, w))).toEqual([false, false, false, false, false]);
|
||||
});
|
||||
|
||||
test('percentile is nearest-rank: p50 and p90 of small samples, null when empty', () => {
|
||||
expect(percentile([], 0.5)).toBeNull();
|
||||
expect(percentile([4, 8, 120], 0.5)).toBe(8);
|
||||
|
||||
Reference in new issue
Block a user