mirror of
https://github.com/garrytan/gstack.git
synced 2026-10-31 22:14:25 +01:00
fix: pre-landing review fixes
Review pass 1 (core checklist, testing, maintainability, security, performance, simplification, state-machines, red team) over the whole diff: - the flaky line and cost-per-merge depend on every input they read, so a failed free-run or PR read prints unavailable (...) on its half instead of 0 - a PR verdict-comment cost backs one gate run per PR; a READY comment after the merge is ignored - one unreadable artifact leaves its run without cost or flake data instead of dropping the gate section - --repo defaults to the origin owner/name and is required without one (no built-in repository) - --max-artifact-runs, --pass-cap and --pass-marker are validated; --csv creates its directory - one artifact listing per gate run; PR comments are paged; issues use the exact window instant - every run appends its rows to trend.csv in --out-dir - new row: review passes per PR (median, p90, PRs at the cap) from the ship receipt or a comment marker - /retro: report format gains the Factory scorecard slot; the Step 7.5 fence quotes its placeholder - tests for each; <window-days> joins the placeholder allowlist Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
garrytanandcapy-ai[bot] committed
1 parent
3f5e4bd0ed
commit
2219c78ebe
11 files changed
+283
-90
No files matched your search
+2
-2
@@ -5,8 +5,8 @@
|
||||
**`/retro` can now say whether the factory is getting better, and a new team has one page that takes it from zero to its first merged train on gstack + Capy.** `bin/gstack-factory-scorecard` reads one window (default 7 days) from GitHub through `gh` and prints a Markdown table plus a CSV: merged PRs (crew vs outside contributors), lead time (PR created → merged, and first READY comment → merged), gate runs green / red / cancelled with their known cost, cost per merge, the red-train rate, flaky events from the same two sources the daily flaky pass reads, issues opened vs closed, and releases cut. Each line carries its counting rule and what one of the best software factories would show on it; a line whose input cannot be read says `unavailable (<reason>)`, never 0. `docs/recommended-harness.md` is the adoption guide: install, the Captain and its Drive, the first train, the daily flaky pass, the weekly release, the scorecard with gstack's own numbers against the bar, what is Capy-specific, the options where two crews differ, and the known Capy limits as of 2026-10-26.
|
||||
|
||||
### Added
|
||||
- **`bin/gstack-factory-scorecard`** (`lib/factory-scorecard.ts`): `--repo`, `--base`, `--since-days`, `--crew login,login` (else author association OWNER/MEMBER/COLLABORATOR), `--gate-workflow`, `--gate-event`, `--free-workflow`, `--train-prefix`, `--ready-marker`, `--max-artifact-runs`, `--out-dir`, `--csv`, `--json`, `--fixtures` (recorded gh responses, no network) and `--now`. Gate cost comes from the run's `report-verdict-a*` artifact (`collector-outcomes.json` `.verdict.costUsd`), else the `**$N** rule/judge cost` or `cost=$N` line of the PR's verdict comment; flaky events from `flake-records-a*` (paid) and `flake-ledger-*` (free) artifacts. Read-only; exits 0 with the table even when an input is unavailable, 2 on bad usage.
|
||||
- **`/retro` Step 7.5 (Factory scorecard, optional)**: runs the bin for the retro window when `gh` is authenticated and pastes the table, third column included, into the report.
|
||||
- **`bin/gstack-factory-scorecard`** (`lib/factory-scorecard.ts`): `--repo`, `--base`, `--since-days`, `--crew login,login` (else author association OWNER/MEMBER/COLLABORATOR), `--gate-workflow`, `--gate-event`, `--free-workflow`, `--train-prefix`, `--ready-marker`, `--pass-marker` and `--pass-cap` (review passes per PR: median, p90 and how many PRs sat at the cap, from the ship receipt's `"passes"` in the PR body or a `passes=N` / `REVIEW_PASSES: N` comment), `--max-artifact-runs`, `--out-dir`, `--csv`, `--json`, `--fixtures` (recorded gh responses, no network) and `--now`. `--repo` defaults to the origin remote's owner/name and is required without one; every run appends its rows to `trend.csv` in `--out-dir`. Gate cost comes from the run's `report-verdict-a*` artifact (`collector-outcomes.json` `.verdict.costUsd`), else the `**$N** rule/judge cost` or `cost=$N` line of the PR's verdict comment; flaky events from `flake-records-a*` (paid) and `flake-ledger-*` (free) artifacts. A READY comment posted after the merge is ignored, a PR's verdict-comment cost backs one gate run, and a run whose artifacts cannot be read is counted and named rather than scored 0. Read-only; exits 0 with the table even when an input is unavailable, 2 on bad usage.
|
||||
- **`/retro` Step 7.5 (Factory scorecard, optional)**: the report format gains a **Factory scorecard** slot; the step runs the bin for the retro window when `gh` is authenticated and pastes the table, third column included, into the report.
|
||||
- **`docs/recommended-harness.md`**, linked from the README docs table; `docs/standalone-binaries.md` lists the new CLI.
|
||||
|
||||
### Tests
|
||||
|
||||
@@ -11,23 +11,28 @@
|
||||
* gstack-factory-scorecard [--repo owner/name] [--base main] [--since-days 7]
|
||||
* [--crew login,login] [--gate-workflow evals.yml] [--gate-event workflow_dispatch|any]
|
||||
* [--free-workflow free-tests.yml] [--train-prefix train/] [--ready-marker <regex>]
|
||||
* [--max-artifact-runs 60] [--out-dir <dir>] [--csv <file>] [--json] [--cache-dir <dir>]
|
||||
* [--pass-marker <regex>] [--pass-cap 3] [--max-artifact-runs 60]
|
||||
* [--out-dir <dir>] [--csv <file>] [--json] [--cache-dir <dir>]
|
||||
* [--fixtures <recorded gh responses.json>] [--now <iso>]
|
||||
*
|
||||
* --repo defaults to the GitHub owner/name of the origin remote; without one,
|
||||
* pass it. Every run writes this window's scorecard.{csv,md} and appends its
|
||||
* rows to trend.csv in --out-dir, so the weeks line up.
|
||||
* Exit 0 with the table even when an input is unavailable (the line says so);
|
||||
* exit 2 on bad usage. Needs gh with read access to the repository unless
|
||||
* --fixtures replays recorded responses.
|
||||
*/
|
||||
import { spawnSync } from 'node:child_process';
|
||||
import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
import { repoSlug } from '../scripts/lib/ci-history';
|
||||
import {
|
||||
DEFAULTS, collectScorecard, fixtureSource, ghSource, renderCsv, renderMarkdown, scorecardRows, type FixtureFile, type ScorecardOptions,
|
||||
} from '../lib/factory-scorecard';
|
||||
|
||||
function usage(code: number): never {
|
||||
const text = fs.readFileSync(new URL(import.meta.url), 'utf8').split('\n').slice(1, 21).map(l => l.replace(/^ \*\s?/, '')).join('\n');
|
||||
const lines = fs.readFileSync(new URL(import.meta.url), 'utf8').split('\n');
|
||||
const text = lines.slice(1, lines.indexOf(' */')).map(l => l.replace(/^\/?\*+\s?/, '')).join('\n');
|
||||
(code ? console.error : console.log)(text);
|
||||
process.exit(code);
|
||||
}
|
||||
@@ -49,10 +54,23 @@ const now = flags.has('now') ? Date.parse(flags.get('now')!) : Date.now();
|
||||
if (!Number.isFinite(now)) { console.error('--now must be an ISO-8601 instant'); usage(2); }
|
||||
let readyMarker: RegExp;
|
||||
try { readyMarker = new RegExp(flags.get('ready-marker') ?? DEFAULTS.readyMarker, 'm'); } catch (e: any) { console.error(`--ready-marker: ${e.message}`); usage(2); }
|
||||
let passMarker: RegExp;
|
||||
try { passMarker = new RegExp(flags.get('pass-marker') ?? DEFAULTS.passMarker, 'm'); } catch (e: any) { console.error(`--pass-marker: ${e.message}`); usage(2); }
|
||||
const passCap = Number(flags.get('pass-cap') ?? DEFAULTS.passCap);
|
||||
if (!Number.isInteger(passCap) || passCap < 1) { console.error('--pass-cap must be a positive integer'); usage(2); }
|
||||
const gateEvent = flags.get('gate-event') ?? DEFAULTS.gateEvent;
|
||||
const maxArtifactRuns = Number(flags.get('max-artifact-runs') ?? DEFAULTS.maxArtifactRuns);
|
||||
if (!Number.isInteger(maxArtifactRuns) || maxArtifactRuns < 0) { console.error('--max-artifact-runs must be a non-negative integer'); usage(2); }
|
||||
|
||||
function originRepo(): string {
|
||||
const origin = spawnSync('git', ['remote', 'get-url', 'origin'], { encoding: 'utf8', timeout: 10_000 });
|
||||
const slug = /[:/]([^/:]+\/[^/]+?)(?:\.git)?\/?$/.exec((origin.stdout ?? '').trim())?.[1];
|
||||
if (origin.status !== 0 || !slug) { console.error('--repo owner/name is required: the origin remote is missing or not owner/name'); usage(2); }
|
||||
return slug;
|
||||
}
|
||||
|
||||
const options: ScorecardOptions = {
|
||||
repo: flags.get('repo') ?? repoSlug(),
|
||||
repo: flags.get('repo') ?? originRepo(),
|
||||
base: flags.get('base') ?? DEFAULTS.base,
|
||||
now, sinceDays,
|
||||
crew: flags.has('crew') ? flags.get('crew')!.split(',').map(s => s.trim()).filter(Boolean) : null,
|
||||
@@ -60,8 +78,8 @@ const options: ScorecardOptions = {
|
||||
gateEvent: gateEvent === 'any' ? null : gateEvent,
|
||||
freeWorkflow: flags.get('free-workflow') ?? DEFAULTS.freeWorkflow,
|
||||
trainPrefix: flags.get('train-prefix') ?? DEFAULTS.trainPrefix,
|
||||
readyMarker,
|
||||
maxArtifactRuns: Number(flags.get('max-artifact-runs') ?? DEFAULTS.maxArtifactRuns),
|
||||
readyMarker, passMarker, passCap,
|
||||
maxArtifactRuns,
|
||||
};
|
||||
|
||||
const cacheDir = flags.get('cache-dir') ?? path.join(os.tmpdir(), 'gstack-factory-scorecard', options.repo.replace('/', '__'));
|
||||
@@ -74,10 +92,15 @@ const rows = scorecardRows(card);
|
||||
const outDir = flags.get('out-dir') ?? path.join('.context', 'factory-scorecard');
|
||||
fs.mkdirSync(outDir, { recursive: true });
|
||||
const csvFile = flags.get('csv') ?? path.join(outDir, 'scorecard.csv');
|
||||
fs.mkdirSync(path.dirname(csvFile), { recursive: true });
|
||||
fs.writeFileSync(csvFile, renderCsv(card, rows));
|
||||
fs.writeFileSync(path.join(outDir, 'scorecard.md'), renderMarkdown(card, rows));
|
||||
if (json) fs.writeFileSync(path.join(outDir, 'scorecard.json'), JSON.stringify({ card, rows }, null, 2) + '\n');
|
||||
const trendFile = path.join(outDir, 'trend.csv');
|
||||
const [csvHeader, ...csvRows] = renderCsv(card, rows).split('\n');
|
||||
fs.appendFileSync(trendFile, (fs.existsSync(trendFile) ? '' : csvHeader + '\n') + csvRows.join('\n'));
|
||||
|
||||
process.stdout.write(renderMarkdown(card, rows));
|
||||
process.stdout.write(`SCORECARD_CSV: ${csvFile}\n`);
|
||||
process.stdout.write(`SCORECARD_MD: ${path.join(outDir, 'scorecard.md')}\n`);
|
||||
process.stdout.write(`SCORECARD_TREND: ${trendFile}\n`);
|
||||
+19
-10
@@ -253,18 +253,25 @@ line whose input could not be read says `unavailable (<reason>)` rather than 0.
|
||||
```bash
|
||||
gstack-factory-scorecard --repo owner/name --since-days 7 \
|
||||
[--crew login,login] [--gate-workflow evals.yml] [--free-workflow free-tests.yml] \
|
||||
[--train-prefix train/] [--ready-marker 'READY #\d+|SHIP_RECEIPT:'] [--out-dir .context/factory-scorecard]
|
||||
[--train-prefix train/] [--ready-marker 'READY #\d+|SHIP_RECEIPT:'] [--pass-marker '"passes": (\d+)|passes=(\d+)'] \
|
||||
[--pass-cap 3] [--out-dir .context/factory-scorecard]
|
||||
```
|
||||
|
||||
`--repo` defaults to the origin remote's GitHub owner/name and is required
|
||||
without one. Each run writes this window's `scorecard.md` and `scorecard.csv`
|
||||
to `--out-dir` and appends the same rows to `trend.csv` there, so a weekly
|
||||
retro accumulates its own trend without renaming files.
|
||||
|
||||
| Line | How it is counted | What one of the best software factories would show |
|
||||
|---|---|---|
|
||||
| Merged PRs | `merged_at` in the window, base = default branch; crew by `--crew` logins, else author association OWNER/MEMBER/COLLABORATOR | several merges a day, every day; outside contributions land in the same week |
|
||||
| Lead time, created → merged | PR `created_at` to `merged_at`, median and p90 | median under 24h, p90 under 3 days (DORA "elite" change lead time is under a day) |
|
||||
| Lead time, READY → merged | first PR comment matching `--ready-marker` to `merged_at`; PRs with no such comment are left out and counted | under 2h; every merged PR carries a READY comment |
|
||||
| Lead time, READY → merged | first PR comment matching `--ready-marker` at or before `merged_at`; PRs with no such comment are left out and counted | under 2h; every merged PR carries a READY comment |
|
||||
| Review passes per PR | the largest count `--pass-marker` captures in the PR body (the ship receipt's `"passes": N` once /ship writes it) or a PR comment (`passes=N`, `REVIEW_PASSES: N`), median and p90, plus how many PRs sat at `--pass-cap` (3: pass 1 reviews the whole diff, later passes only the delta, a pass that finds nothing new is the exit) | median 2 passes; none at the cap |
|
||||
| Gate runs green / red / cancelled | completed runs of the gate workflow (`workflow_dispatch` by default; `--gate-event any`); cost from the run's `report-verdict-a*` artifact (`collector-outcomes.json` `.verdict.costUsd`), else the `**$N** rule/judge cost` or `cost=$N` line of the PR's verdict comment | green on 95%+ of runs, cancelled near 0, cost known for every run |
|
||||
| Known gate cost per merged PR | known cost / merged PRs; unknown-cost runs make it a floor | known, stable or falling week over week |
|
||||
| Red-train rate | runs whose head branch starts with `--train-prefix`, cancelled excluded; red = conclusion other than success; red backstops and screens shown beside it | under 5% (each red train is a bisect plus a rebuild, about two gates) |
|
||||
| Flaky events | `flake-records-a*` records of the newest gate runs (count, known rerun spend) and `flake-ledger-*` entries of free runs on the default branch | 0 repeat cases; under 1 flaky case per 10 gates; free retries under 1% of runs |
|
||||
| Flaky events | `flake-records-a*` records of the newest gate runs (count, known rerun spend) and `flake-ledger-*` entries of free runs on the default branch; a run whose artifacts cannot be read is counted and named, never scored 0 | 0 repeat cases; under 1 flaky case per 10 gates; free retries under 1% of runs |
|
||||
| Issues opened / closed | GitHub search, created and closed in the window | closed ≥ opened; nothing severe older than a week |
|
||||
| Releases cut | releases published in the window, drafts excluded | at least one tagged release a week, on schedule |
|
||||
|
||||
@@ -275,26 +282,28 @@ the gap next to the number.
|
||||
### Where gstack's own numbers stand
|
||||
|
||||
Measured on gstack's repository with this tool for 2026-10-19 to 2026-10-26
|
||||
(the first full week of trains and `/ship`-before-READY; 102 merged PRs, 77
|
||||
(the first full week of trains and `/ship`-before-READY; 103 merged PRs, 77
|
||||
gate runs, artifacts read for the newest 60):
|
||||
|
||||
| Line | gstack, that week | Against the bar |
|
||||
|---|---|---|
|
||||
| Merged PRs | 102 (crew 99, outside 3) | volume is there; outside contributions are few and are ported by the crew rather than merged from forks |
|
||||
| Merged PRs | 103 (crew 100, outside 3) | volume is there; outside contributions are few and are ported by the crew rather than merged from forks |
|
||||
| Lead time, created → merged | median 7.9h, p90 23.7h | inside the bar on both |
|
||||
| Lead time, READY → merged | no number: 0 of 102 PRs had a READY comment | the crew reports READY in-thread; step 6 above (post it on the PR) is what makes this measurable |
|
||||
| Lead time, READY → merged | no number: 0 of 103 PRs had a READY comment | the crew reports READY in-thread; step 6 above (post it on the PR) is what makes this measurable |
|
||||
| Review passes per PR | no number: 0 of 103 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 | 73 green / 4 red / 0 cancelled; known cost $1,628 on 59 of 77 runs | green rate 95%; cost unknown on 18 runs (screens that planned no cases, and runs older than the artifact cap) |
|
||||
| Known gate cost per merged PR | $15.96 (a floor) | about $24 per train of up to 5 PRs, plus screens; one gate per PR would have been about $33 each |
|
||||
| Known gate cost per merged PR | $15.81 (a floor) | about $24 per train of up to 5 PRs, plus screens; one gate per PR would have been about $33 each |
|
||||
| Red-train rate | 4% (1 red of 28 trains; 0 red backstop, 3 red screens) | inside the bar; the three red screens were one judge reading a moved file |
|
||||
| Flaky events | 21 paid cases passed on their one rerun (rerun spend $10.72); 17 free-suite retries in 17 of 102 runs | above the bar on both: 0.27 flaky cases per gate against 0.1, and 17% of free runs retried against 1%. One family (`ship-docsync-*`) repeated three times in two days |
|
||||
| Issues opened / closed | 52 / 56 | inside the bar |
|
||||
| Flaky events | 21 paid cases passed on their one rerun (rerun spend $10.72); 17 free-suite retries in 17 of 103 runs | above the bar on both: 0.27 flaky cases per gate against 0.1, and 17% of free runs retried against 1%. One family (`ship-docsync-*`) repeated three times in two days |
|
||||
| Issues opened / closed | 50 / 56 | inside the bar |
|
||||
| Releases cut | 1 (the first weekly release) | on schedule, with the starvation caveat above |
|
||||
|
||||
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 baseline CSV, when it
|
||||
lands in the Captain's Drive, supersedes these numbers for trend tracking.
|
||||
lands in the Captain's Drive, supersedes these numbers for trend tracking;
|
||||
from then on `trend.csv` in `--out-dir` carries the weeks.
|
||||
|
||||
## What is Capy-specific and what is not
|
||||
|
||||
|
||||
+100
-50
@@ -14,7 +14,7 @@
|
||||
* kind by head branch (train/<nonce>) and run title (screen #, backstop);
|
||||
* cost from the run's report-verdict artifact (collector-outcomes.json
|
||||
* .verdict.costUsd), else the "**$N** rule/judge cost" / "cost=$N" line of
|
||||
* the PR's verdict comment, else unknown
|
||||
* the PR's verdict comment (used by one run per PR), else unknown
|
||||
* flaky events flake-records-a<n> artifacts of gate runs (paid, one record per case
|
||||
* that failed once and passed its one rerun) and flake-ledger-* artifacts
|
||||
* of <free workflow> runs on <base> (free-suite serial retries); the same
|
||||
@@ -35,8 +35,8 @@ import {
|
||||
|
||||
export interface ScorecardSource {
|
||||
getJson(apiPath: string): unknown;
|
||||
/** Downloaded, unzipped artifact directories of one run whose names match. */
|
||||
artifactDirs(runId: number, match: (name: string) => boolean): string[];
|
||||
/** Downloaded, unzipped artifact directories of one run whose names match, keyed by artifact name; one listing per call. */
|
||||
artifactDirs(runId: number, match: (name: string) => boolean): Map<string, string>;
|
||||
}
|
||||
|
||||
export interface ScorecardOptions {
|
||||
@@ -51,22 +51,28 @@ export interface ScorecardOptions {
|
||||
freeWorkflow: string;
|
||||
trainPrefix: string;
|
||||
readyMarker: RegExp;
|
||||
/** Matches a review pass count in the PR body (the ship receipt's "passes": N) or a comment; the first captured number is the count. */
|
||||
passMarker: RegExp;
|
||||
/** The review-pass cap (passes at or above it are counted as capped). */
|
||||
passCap: number;
|
||||
/** Newest runs whose artifacts are read; the rest count without cost or flake data. */
|
||||
maxArtifactRuns: number;
|
||||
}
|
||||
|
||||
export const DEFAULTS = {
|
||||
base: 'main', sinceDays: 7, gateWorkflow: 'evals.yml', gateEvent: 'workflow_dispatch', freeWorkflow: 'free-tests.yml',
|
||||
trainPrefix: 'train/', readyMarker: 'READY #\\d+|SHIP_RECEIPT:|gstack-ship-receipt', maxArtifactRuns: 60,
|
||||
trainPrefix: 'train/', readyMarker: 'READY #\\d+|SHIP_RECEIPT:|gstack-ship-receipt',
|
||||
passMarker: '"passes"\\s*:\\s*(\\d+)|\\bpasses=(\\d+)|\\bREVIEW_PASS(?:ES)?[:= ]\\s*(\\d+)', passCap: 3, maxArtifactRuns: 60,
|
||||
} as const;
|
||||
|
||||
export const CREW_ASSOCIATIONS = new Set(['OWNER', 'MEMBER', 'COLLABORATOR']);
|
||||
const PAGE = 100;
|
||||
const MAX_PAGES = 20;
|
||||
const COST_LINE = /\*\*\$(\d+(?:\.\d+)?)\*\* rule\/judge cost|\bcost=\$(\d+(?:\.\d+)?)/g;
|
||||
|
||||
export interface MergedPr {
|
||||
number: number; author: string; association: string; crew: boolean; title: string;
|
||||
createdAt: string; mergedAt: string; readyAt: string | null; commentCostUsd: number | null;
|
||||
createdAt: string; mergedAt: string; readyAt: string | null; commentCostUsd: number | null; passes: number | null;
|
||||
}
|
||||
export type GateKind = 'train' | 'screen' | 'backstop' | 'other';
|
||||
export type GateOutcome = 'green' | 'red' | 'cancelled';
|
||||
@@ -74,10 +80,13 @@ export interface GateRun {
|
||||
id: number; kind: GateKind; outcome: GateOutcome; createdAt: string; branch: string; title: string; pr: number | null;
|
||||
costUsd: number | null; costSource: 'artifact' | 'comment' | null; flakes: number; flakeRerunUsd: number;
|
||||
}
|
||||
export interface FreeRetryRun { id: number; conclusion: string | null; retries: number }
|
||||
/** retries is null when the run's artifacts could not be read. */
|
||||
export interface FreeRetryRun { id: number; conclusion: string | null; retries: number | null }
|
||||
export interface Scorecard {
|
||||
repo: string; base: string; windowStart: string; windowEnd: string;
|
||||
prs: MergedPr[]; gates: GateRun[]; freeRuns: FreeRetryRun[]; artifactRunsRead: number;
|
||||
prs: MergedPr[]; gates: GateRun[]; freeRuns: FreeRetryRun[]; artifactRunsRead: number; passCap: number;
|
||||
/** Gate runs whose artifacts could not be read; they count without cost or flake data. */
|
||||
artifactErrors: number;
|
||||
issuesOpened: number | null; issuesClosed: number | null; releases: Array<{ tag: string; publishedAt: string }>;
|
||||
unavailable: string[];
|
||||
}
|
||||
@@ -89,7 +98,7 @@ export function ghSource(repo: string, cacheDir: string, client: GhClient = GH_C
|
||||
const fetcher = historyFetcher(client);
|
||||
return {
|
||||
getJson: apiPath => client.getJson(apiPath),
|
||||
artifactDirs: (runId, match) => downloadRunArtifacts({ repo, run: { id: runId }, match, cacheDir, fetcher }),
|
||||
artifactDirs: (runId, match) => new Map(downloadRunArtifacts({ repo, run: { id: runId }, match, cacheDir, fetcher }).map(dir => [path.basename(dir), dir])),
|
||||
};
|
||||
}
|
||||
|
||||
@@ -106,15 +115,17 @@ export function fixtureSource(fixture: FixtureFile, scratch = fs.mkdtempSync(pat
|
||||
return fixture.responses[apiPath];
|
||||
},
|
||||
artifactDirs: (runId, match) => {
|
||||
const dirs: string[] = [];
|
||||
const dirs = new Map<string, string>();
|
||||
for (const [name, files] of Object.entries(fixture.artifacts?.[String(runId)] ?? {})) {
|
||||
if (!match(name)) continue;
|
||||
const dir = path.join(scratch, String(runId), name);
|
||||
const dir = path.resolve(scratch, String(runId), name);
|
||||
for (const [file, text] of Object.entries(files)) {
|
||||
fs.mkdirSync(path.dirname(path.join(dir, file)), { recursive: true });
|
||||
fs.writeFileSync(path.join(dir, file), text);
|
||||
const target = path.resolve(dir, file);
|
||||
if (!target.startsWith(dir + path.sep)) throw new Error(`fixture artifact file escapes its directory: ${file}`);
|
||||
fs.mkdirSync(path.dirname(target), { recursive: true });
|
||||
fs.writeFileSync(target, text);
|
||||
}
|
||||
dirs.push(dir);
|
||||
dirs.set(name, dir);
|
||||
}
|
||||
return dirs;
|
||||
},
|
||||
@@ -132,17 +143,19 @@ function pullsPage(source: ScorecardSource, o: ScorecardOptions, page: number):
|
||||
export function readMergedPrs(source: ScorecardSource, o: ScorecardOptions, since: string): MergedPr[] {
|
||||
const out: MergedPr[] = [];
|
||||
const crewLogins = o.crew ? new Set(o.crew.map(l => l.toLowerCase())) : null;
|
||||
for (let page = 1; page <= 20; page++) {
|
||||
for (let page = 1; page <= MAX_PAGES; page++) {
|
||||
const pulls = pullsPage(source, o, page);
|
||||
for (const pr of pulls) {
|
||||
const mergedAt = typeof pr.merged_at === 'string' ? pr.merged_at : null;
|
||||
if (!mergedAt || mergedAt < since) continue;
|
||||
const author = String(pr.user?.login ?? '');
|
||||
const association = String(pr.author_association ?? 'NONE');
|
||||
const comments = readComments(source, o.repo, Number(pr.number), o.readyMarker);
|
||||
const comments = readComments(source, o.repo, Number(pr.number), { readyMarker: o.readyMarker, passMarker: o.passMarker, mergedAt });
|
||||
const bodyPasses = passCount(String(pr.body ?? ''), o.passMarker);
|
||||
out.push({ number: Number(pr.number), author, association, title: String(pr.title ?? ''),
|
||||
crew: crewLogins ? crewLogins.has(author.toLowerCase()) : CREW_ASSOCIATIONS.has(association),
|
||||
createdAt: String(pr.created_at), mergedAt, readyAt: comments.readyAt, commentCostUsd: comments.costUsd });
|
||||
createdAt: String(pr.created_at), mergedAt, readyAt: comments.readyAt, commentCostUsd: comments.costUsd,
|
||||
passes: bodyPasses === null ? comments.passes : Math.max(bodyPasses, comments.passes ?? 0) });
|
||||
}
|
||||
const oldest = pulls[pulls.length - 1]?.updated_at;
|
||||
if (pulls.length < PAGE || (typeof oldest === 'string' && oldest < since)) break;
|
||||
@@ -150,19 +163,36 @@ export function readMergedPrs(source: ScorecardSource, o: ScorecardOptions, sinc
|
||||
return out.sort((a, b) => a.mergedAt.localeCompare(b.mergedAt));
|
||||
}
|
||||
|
||||
/** The first READY-style comment's time and the newest verdict comment's cost, from one page of a PR's comments. */
|
||||
export function readComments(source: ScorecardSource, repo: string, pr: number, readyMarker: RegExp): { readyAt: string | null; costUsd: number | null } {
|
||||
const body = source.getJson(`repos/${repo}/issues/${pr}/comments?per_page=${PAGE}&page=1`);
|
||||
const comments = Array.isArray(body) ? body : [];
|
||||
/** The largest pass count the marker captures in a text, null when it never matches. */
|
||||
export function passCount(text: string, marker: RegExp): number | null {
|
||||
let max: number | null = null;
|
||||
for (const m of text.matchAll(new RegExp(marker.source, marker.flags.includes('g') ? marker.flags : marker.flags + 'g'))) {
|
||||
const n = Number(m.slice(1).find(g => g !== undefined));
|
||||
if (Number.isFinite(n) && (max === null || n > max)) max = n;
|
||||
}
|
||||
return max;
|
||||
}
|
||||
|
||||
/** The first READY-style comment's time (comments after the merge are ignored), the newest verdict comment's cost and the largest pass count, across every page of a PR's comments. */
|
||||
export function readComments(source: ScorecardSource, repo: string, pr: number, m: { readyMarker: RegExp; passMarker: RegExp; mergedAt?: string }):
|
||||
{ readyAt: string | null; costUsd: number | null; passes: number | null } {
|
||||
let readyAt: string | null = null;
|
||||
let costUsd: number | null = null;
|
||||
for (const c of comments) {
|
||||
const text = String(c.body ?? '');
|
||||
const at = String(c.created_at ?? '');
|
||||
if (readyMarker.test(text) && (!readyAt || at < readyAt)) readyAt = at;
|
||||
for (const m of text.matchAll(COST_LINE)) costUsd = Number(m[1] ?? m[2]);
|
||||
let passes: number | null = null;
|
||||
for (let page = 1; page <= MAX_PAGES; page++) {
|
||||
const body = source.getJson(`repos/${repo}/issues/${pr}/comments?per_page=${PAGE}&page=${page}`);
|
||||
const comments = Array.isArray(body) ? body : [];
|
||||
for (const c of comments) {
|
||||
const text = String(c.body ?? '');
|
||||
const at = String(c.created_at ?? '');
|
||||
if (m.readyMarker.test(text) && (!m.mergedAt || at <= m.mergedAt) && (!readyAt || at < readyAt)) readyAt = at;
|
||||
for (const match of text.matchAll(COST_LINE)) costUsd = Number(match[1] ?? match[2]);
|
||||
const n = passCount(text, m.passMarker);
|
||||
if (n !== null && (passes === null || n > passes)) passes = n;
|
||||
}
|
||||
if (comments.length < PAGE) break;
|
||||
}
|
||||
return { readyAt, costUsd };
|
||||
return { readyAt, costUsd, passes };
|
||||
}
|
||||
|
||||
export function classifyGate(run: { branch: string; title: string }, trainPrefix: string): GateKind {
|
||||
@@ -208,11 +238,11 @@ export function flakeRecords(dirs: string[]): { flakes: number; rerunUsd: number
|
||||
}
|
||||
|
||||
/** Gate runs in the window with kind, outcome, cost and flake records; artifacts are read for the newest maxArtifactRuns only. */
|
||||
export function readGateRuns(source: ScorecardSource, o: ScorecardOptions, since: string, prCost: Map<number, number>): { gates: GateRun[]; artifactRunsRead: number } {
|
||||
export function readGateRuns(source: ScorecardSource, o: ScorecardOptions, since: string, prCost: Map<number, number>): { gates: GateRun[]; artifactRunsRead: number; artifactErrors: number } {
|
||||
const query = [`status=completed`, `per_page=${PAGE}`, `created=${encodeURIComponent(`>=${since.slice(0, 10)}`)}`,
|
||||
...(o.gateEvent ? [`event=${encodeURIComponent(o.gateEvent)}`] : [])].join('&');
|
||||
const gates: GateRun[] = [];
|
||||
for (let page = 1; page <= 20; page++) {
|
||||
for (let page = 1; page <= MAX_PAGES; page++) {
|
||||
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) {
|
||||
@@ -227,25 +257,35 @@ export function readGateRuns(source: ScorecardSource, o: ScorecardOptions, since
|
||||
}
|
||||
gates.sort((a, b) => b.createdAt.localeCompare(a.createdAt));
|
||||
const artifactRunsRead = Math.min(gates.length, o.maxArtifactRuns);
|
||||
const commentCostUsed = new Set<number>();
|
||||
let artifactErrors = 0;
|
||||
gates.forEach((gate, index) => {
|
||||
if (index < artifactRunsRead) {
|
||||
gate.costUsd = verdictCost(source.artifactDirs(gate.id, name => /^report-verdict-a\d+$/.test(name)));
|
||||
if (gate.costUsd !== null) gate.costSource = 'artifact';
|
||||
const flakes = flakeRecords(source.artifactDirs(gate.id, name => /^flake-records-a\d+$/.test(name)));
|
||||
gate.flakes = flakes.flakes;
|
||||
gate.flakeRerunUsd = flakes.rerunUsd;
|
||||
try {
|
||||
const dirs = source.artifactDirs(gate.id, name => /^(report-verdict|flake-records)-a\d+$/.test(name));
|
||||
const named = (prefix: string) => [...dirs].filter(([name]) => name.startsWith(prefix)).map(([, dir]) => dir);
|
||||
gate.costUsd = verdictCost(named('report-verdict-'));
|
||||
if (gate.costUsd !== null) gate.costSource = 'artifact';
|
||||
const flakes = flakeRecords(named('flake-records-'));
|
||||
gate.flakes = flakes.flakes;
|
||||
gate.flakeRerunUsd = flakes.rerunUsd;
|
||||
} catch { artifactErrors++; }
|
||||
}
|
||||
if (gate.costUsd === null && gate.pr !== null && prCost.has(gate.pr) && !commentCostUsed.has(gate.pr)) {
|
||||
gate.costUsd = prCost.get(gate.pr)!; gate.costSource = 'comment'; commentCostUsed.add(gate.pr);
|
||||
}
|
||||
if (gate.costUsd === null && gate.pr !== null && prCost.has(gate.pr)) { gate.costUsd = prCost.get(gate.pr)!; gate.costSource = 'comment'; }
|
||||
});
|
||||
return { gates, artifactRunsRead };
|
||||
return { gates, artifactRunsRead, artifactErrors };
|
||||
}
|
||||
|
||||
/** Free-suite runs on base in the window and the flaky-pass entries their flake-ledger-* artifacts hold. */
|
||||
export function readFreeRetries(source: ScorecardSource, o: ScorecardOptions, since: string): FreeRetryRun[] {
|
||||
const client: GhClient = { getJson: p => source.getJson(p), downloadZip: () => { throw new Error('not used'); } };
|
||||
const runs = listWorkflowRuns(client, o.repo, o.freeWorkflow, { since, branch: o.base });
|
||||
return runs.map((run, index) => ({ id: run.id, conclusion: run.conclusion,
|
||||
retries: index < o.maxArtifactRuns ? source.artifactDirs(run.id, name => name.startsWith('flake-ledger-')).flatMap(readFlakeLedgerDir).length : 0 }));
|
||||
const retries = (runId: number): number | null => {
|
||||
try { return [...source.artifactDirs(runId, name => name.startsWith('flake-ledger-')).values()].flatMap(readFlakeLedgerDir).length; } catch { return null; }
|
||||
};
|
||||
return runs.map((run, index) => ({ id: run.id, conclusion: run.conclusion, retries: index < o.maxArtifactRuns ? retries(run.id) : 0 }));
|
||||
}
|
||||
|
||||
function searchCount(source: ScorecardSource, q: string): number | null {
|
||||
@@ -263,7 +303,7 @@ export function readReleases(source: ScorecardSource, repo: string, since: strin
|
||||
export function collectScorecard(source: ScorecardSource, o: ScorecardOptions): Scorecard {
|
||||
const since = new Date(o.now - o.sinceDays * 86_400_000).toISOString();
|
||||
const card: Scorecard = { repo: o.repo, base: o.base, windowStart: since, windowEnd: new Date(o.now).toISOString(),
|
||||
prs: [], gates: [], freeRuns: [], artifactRunsRead: 0, issuesOpened: null, issuesClosed: null, releases: [], unavailable: [] };
|
||||
prs: [], gates: [], freeRuns: [], artifactRunsRead: 0, passCap: o.passCap, artifactErrors: 0, issuesOpened: null, issuesClosed: null, releases: [], unavailable: [] };
|
||||
const guard = (what: string, fn: () => void) => { try { fn(); } catch (e) { card.unavailable.push(`${what}: ${unavailableReason(e)}`); } };
|
||||
guard('merged PRs', () => { card.prs = readMergedPrs(source, o, since); });
|
||||
guard('gate runs', () => {
|
||||
@@ -271,10 +311,10 @@ export function collectScorecard(source: ScorecardSource, o: ScorecardOptions):
|
||||
Object.assign(card, readGateRuns(source, o, since, prCost));
|
||||
});
|
||||
guard('free retries', () => { card.freeRuns = readFreeRetries(source, o, since); });
|
||||
const day = since.slice(0, 10);
|
||||
const instant = since.replace(/\.\d{3}Z$/, 'Z');
|
||||
guard('issues', () => {
|
||||
card.issuesOpened = searchCount(source, `repo:${o.repo} is:issue created:>=${day}`);
|
||||
card.issuesClosed = searchCount(source, `repo:${o.repo} is:issue closed:>=${day}`);
|
||||
card.issuesOpened = searchCount(source, `repo:${o.repo} is:issue created:>=${instant}`);
|
||||
card.issuesClosed = searchCount(source, `repo:${o.repo} is:issue closed:>=${instant}`);
|
||||
});
|
||||
guard('releases', () => { card.releases = readReleases(source, o.repo, since); });
|
||||
return card;
|
||||
@@ -305,6 +345,7 @@ export const BENCHMARKS: Record<string, string> = {
|
||||
merged: 'several merges a day, every day; outside contributions land in the same week',
|
||||
lead_time: 'median under 24h, p90 under 3 days (DORA elite: change lead time under a day)',
|
||||
ready_to_merge: 'under 2h from READY to merge; every merged PR carries a READY comment',
|
||||
review_passes: 'median 2 passes, none at the cap (a pass that finds nothing new is the exit; what is left at the cap is named in the PR body)',
|
||||
gates: 'green on 95%+ of runs, cancelled near 0, cost known for every run',
|
||||
cost_per_merge: 'known, stable or falling week over week',
|
||||
red_train: 'under 5% of trains red (each red train costs a bisect and a rebuild)',
|
||||
@@ -324,25 +365,34 @@ export function scorecardRows(card: Scorecard): ScorecardRow[] {
|
||||
const trains = gates.filter(g => g.kind === 'train' && g.outcome !== 'cancelled');
|
||||
const flakes = gates.reduce((s, g) => s + g.flakes, 0);
|
||||
const flakeUsd = gates.reduce((s, g) => s + g.flakeRerunUsd, 0);
|
||||
const retries = card.freeRuns.reduce((s, r) => s + r.retries, 0);
|
||||
const retryRuns = card.freeRuns.filter(r => r.retries > 0).length;
|
||||
const retries = card.freeRuns.reduce((s, r) => s + (r.retries ?? 0), 0);
|
||||
const retryRuns = card.freeRuns.filter(r => (r.retries ?? 0) > 0).length;
|
||||
const unreadableFree = card.freeRuns.filter(r => r.retries === null).length;
|
||||
const passes = card.prs.filter(p => p.passes !== null).map(p => p.passes!);
|
||||
const unavailable = (what: string) => card.unavailable.find(u => u.startsWith(what + ':'));
|
||||
const row = (id: string, metric: string, what: string, value: () => string, rule: string): ScorecardRow =>
|
||||
({ id, metric, value: unavailable(what) ? `unavailable (${unavailable(what)!.slice(what.length + 2)})` : value(), benchmark: BENCHMARKS[id]!, rule });
|
||||
const unavailableText = (what: string) => `unavailable (${unavailable(what)!.slice(what.length + 2)})`;
|
||||
const row = (id: string, metric: string, what: string | string[], value: () => string, rule: string): ScorecardRow => {
|
||||
const failed = (Array.isArray(what) ? what : [what]).find(w => unavailable(w));
|
||||
return { id, metric, value: failed ? unavailableText(failed) : value(), benchmark: BENCHMARKS[id]!, rule };
|
||||
};
|
||||
const paidFlakes = () => `${flakes} paid case(s) passed on their one rerun (rerun spend ${usd(flakeUsd)})${card.artifactErrors ? `; artifacts unreadable on ${card.artifactErrors} run(s)` : ''}`;
|
||||
const freeRetries = () => `${retries} free-suite retr${retries === 1 ? 'y' : 'ies'} in ${retryRuns} of ${card.freeRuns.length} runs${unreadableFree ? ` (${unreadableFree} unreadable)` : ''}`;
|
||||
return [
|
||||
row('merged', 'Merged PRs', 'merged PRs', () => `${card.prs.length} (crew ${crewPrs.length}, outside ${card.prs.length - crewPrs.length})`,
|
||||
`merged_at inside the window, base ${card.base}; crew by login list or author_association OWNER/MEMBER/COLLABORATOR`),
|
||||
row('lead_time', 'Lead time, created to merged', 'merged PRs', () => `median ${fmtHours(percentile(lead, 0.5))}, p90 ${fmtHours(percentile(lead, 0.9))}`,
|
||||
'PR created_at to merged_at over the merged PRs above'),
|
||||
row('ready_to_merge', 'Lead time, READY to merged', 'merged PRs', () => `median ${fmtHours(percentile(ready, 0.5))}, p90 ${fmtHours(percentile(ready, 0.9))} (${ready.length} of ${card.prs.length} PRs with a READY comment)`,
|
||||
'first PR comment matching the READY marker to merged_at; PRs without one are left out'),
|
||||
row('gates', 'Gate runs green / red / cancelled', 'gate runs', () => `${by('green')} / ${by('red')} / ${by('cancelled')}; known cost ${usd(knownUsd)} on ${known.length} of ${gates.length} runs`,
|
||||
'completed runs of the gate workflow in the window; cost from the verdict artifact, else the PR verdict comment'),
|
||||
row('cost_per_merge', 'Known gate cost per merged PR', 'gate runs', () => (card.prs.length ? `${usd(knownUsd / card.prs.length)} (${gates.length - known.length} runs without a known cost)` : 'n/a (no merges)'),
|
||||
'first PR comment matching the READY marker, at or before merged_at; PRs without one are left out'),
|
||||
row('review_passes', 'Review passes per PR', 'merged PRs', () => `median ${percentile(passes, 0.5) ?? 'n/a'}, p90 ${percentile(passes, 0.9) ?? 'n/a'} (${passes.length} of ${card.prs.length} PRs report a pass count; ${passes.filter(n => n >= card.passCap).length} at the cap of ${card.passCap})`,
|
||||
'largest count matching the pass marker in the PR body (the ship receipt\'s "passes") or a PR comment; PRs without one are left out'),
|
||||
row('gates', 'Gate runs green / red / cancelled', 'gate runs', () => `${by('green')} / ${by('red')} / ${by('cancelled')}; known cost ${usd(knownUsd)} on ${known.length} of ${gates.length} runs${unavailable('merged PRs') ? ' (comment costs unavailable: merged PRs unreadable)' : ''}`,
|
||||
'completed runs of the gate workflow in the window; cost from the verdict artifact, else the PR verdict comment (one run per PR)'),
|
||||
row('cost_per_merge', 'Known gate cost per merged PR', ['gate runs', 'merged PRs'], () => (card.prs.length ? `${usd(knownUsd / card.prs.length)} (${gates.length - known.length} runs without a known cost)` : 'n/a (no merges)'),
|
||||
'known gate cost divided by merged PRs; unknown-cost runs make this a floor'),
|
||||
row('red_train', 'Red-train rate', 'gate runs', () => `${pct(trains.filter(t => t.outcome === 'red').length, trains.length)} (${trains.filter(t => t.outcome === 'red').length} red of ${trains.length} trains; ${by('red', 'backstop')} red backstop, ${by('red', 'screen')} red screens)`,
|
||||
`runs whose head branch starts with the train prefix, cancelled runs excluded; red means the run's conclusion was not success`),
|
||||
row('flaky', 'Flaky events', 'gate runs', () => `${flakes} paid case(s) passed on their one rerun (rerun spend ${usd(flakeUsd)}); ${retries} free-suite retr${retries === 1 ? 'y' : 'ies'} in ${retryRuns} of ${card.freeRuns.length} runs`,
|
||||
row('flaky', 'Flaky events', [], () => `${unavailable('gate runs') ? unavailableText('gate runs') : paidFlakes()}; ${unavailable('free retries') ? unavailableText('free retries') : freeRetries()}`,
|
||||
`flake-records artifacts of the newest ${card.artifactRunsRead} gate runs and flake-ledger artifacts of free runs on the base branch`),
|
||||
row('issues', 'Issues opened / closed', 'issues', () => `${card.issuesOpened ?? 'n/a'} / ${card.issuesClosed ?? 'n/a'}`, 'GitHub search: created, closed inside the window'),
|
||||
row('releases', 'Releases cut', 'releases', () => `${card.releases.length}${card.releases.length ? ` (${card.releases.map(r => r.tag).join(', ')})` : ''}`, 'releases published inside the window, drafts excluded'),
|
||||
|
||||
+2
-2
@@ -675,11 +675,11 @@ Report `COMMIT_SIZE_BUCKETS`:
|
||||
|
||||
### Step 7.5: Factory scorecard (optional)
|
||||
|
||||
With `gh` authenticated, add what git cannot show: merged PRs (crew vs outside), lead time (PR open → merge, first READY comment → merge), gate runs and known cost, red-train rate, flaky events, issues opened vs closed, releases. Read-only, no spend; a busy 7-day window takes a few minutes.
|
||||
With `gh` authenticated, add what git cannot show: merged PRs (crew vs outside), lead time (PR open → merge, READY → merge), review passes per PR, gate runs and known cost, red-train rate, flaky events, issues opened vs closed, releases. Read-only, no spend; a busy week takes minutes.
|
||||
|
||||
```bash
|
||||
_FS="$HOME/.claude/skills/gstack/bin/gstack-factory-scorecard"
|
||||
gh auth status >/dev/null 2>&1 && "$_FS" --base "<default>" --since-days <window-days> \
|
||||
gh auth status >/dev/null 2>&1 && "$_FS" --base "<default>" --since-days "<window-days>" \
|
||||
|| echo "FACTORY_SCORECARD: unavailable"
|
||||
```
|
||||
|
||||
|
||||
+2
-2
@@ -271,11 +271,11 @@ Report `COMMIT_SIZE_BUCKETS`:
|
||||
|
||||
### Step 7.5: Factory scorecard (optional)
|
||||
|
||||
With `gh` authenticated, add what git cannot show: merged PRs (crew vs outside), lead time (PR open → merge, first READY comment → merge), gate runs and known cost, red-train rate, flaky events, issues opened vs closed, releases. Read-only, no spend; a busy 7-day window takes a few minutes.
|
||||
With `gh` authenticated, add what git cannot show: merged PRs (crew vs outside), lead time (PR open → merge, READY → merge), review passes per PR, gate runs and known cost, red-train rate, flaky events, issues opened vs closed, releases. Read-only, no spend; a busy week takes minutes.
|
||||
|
||||
```bash
|
||||
_FS="{{RUNTIME_ROOT}}/bin/gstack-factory-scorecard"
|
||||
gh auth status >/dev/null 2>&1 && "$_FS" --base "<default>" --since-days <window-days> \
|
||||
gh auth status >/dev/null 2>&1 && "$_FS" --base "<default>" --since-days "<window-days>" \
|
||||
|| echo "FACTORY_SCORECARD: unavailable"
|
||||
```
|
||||
|
||||
|
||||
@@ -35,6 +35,9 @@ Narrative covering:
|
||||
- Fix-chain detection (sequences of fix commits on the same subsystem)
|
||||
- Version bump discipline
|
||||
|
||||
### Factory scorecard
|
||||
(from Step 7.5 when it ran; the table as printed, `unavailable (...)` lines kept; omit the section when the step was skipped)
|
||||
|
||||
### Code Quality Signals
|
||||
- Test LOC ratio trend
|
||||
- Hotspot analysis (are the same files churning?)
|
||||
|
||||
@@ -33,6 +33,9 @@ Narrative covering:
|
||||
- Fix-chain detection (sequences of fix commits on the same subsystem)
|
||||
- Version bump discipline
|
||||
|
||||
### Factory scorecard
|
||||
(from Step 7.5 when it ran; the table as printed, `unavailable (...)` lines kept; omit the section when the step was skipped)
|
||||
|
||||
### Code Quality Signals
|
||||
- Test LOC ratio trend
|
||||
- Hotspot analysis (are the same files churning?)
|
||||
|
||||
@@ -11,7 +11,7 @@ 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, percentile, readComments, renderCsv,
|
||||
BENCHMARKS, DEFAULTS, classifyGate, collectScorecard, fixtureSource, flakeRecords, fmtHours, gateOutcome, passCount, percentile, readComments, renderCsv,
|
||||
renderMarkdown, scorecardRows, verdictCost, type FixtureFile, type ScorecardOptions,
|
||||
} from '../lib/factory-scorecard';
|
||||
|
||||
@@ -28,7 +28,8 @@ const fixture = (): FixtureFile => JSON.parse(fs.readFileSync(FIXTURE, 'utf8'));
|
||||
const options = (over: Partial<ScorecardOptions> = {}): ScorecardOptions => ({
|
||||
repo: 'acme/widgets', base: DEFAULTS.base, now: NOW, sinceDays: DEFAULTS.sinceDays, crew: null, gateWorkflow: DEFAULTS.gateWorkflow,
|
||||
gateEvent: DEFAULTS.gateEvent, freeWorkflow: DEFAULTS.freeWorkflow, trainPrefix: DEFAULTS.trainPrefix,
|
||||
readyMarker: new RegExp(DEFAULTS.readyMarker, 'm'), maxArtifactRuns: DEFAULTS.maxArtifactRuns, ...over,
|
||||
readyMarker: new RegExp(DEFAULTS.readyMarker, 'm'), passMarker: new RegExp(DEFAULTS.passMarker, 'm'), passCap: DEFAULTS.passCap,
|
||||
maxArtifactRuns: DEFAULTS.maxArtifactRuns, ...over,
|
||||
});
|
||||
const byId = (rows: ReturnType<typeof scorecardRows>) => Object.fromEntries(rows.map(r => [r.id, r.value]));
|
||||
|
||||
@@ -42,6 +43,7 @@ describe('factory-scorecard: recorded window', () => {
|
||||
expect(card.prs.find(p => p.number === 101)!.readyAt).toBe('2026-10-24T16:00:00Z');
|
||||
expect(card.prs.find(p => p.number === 101)!.commentCostUsd).toBe(12.5);
|
||||
expect(card.prs.find(p => p.number === 102)!.readyAt).toBeNull();
|
||||
expect(card.prs.map(p => p.passes)).toEqual([2, 3, null]);
|
||||
expect(card.gates.map(g => [g.id, g.kind, g.outcome, g.costUsd, g.costSource, g.flakes])).toEqual([
|
||||
[9004, 'backstop', 'cancelled', null, null, 0],
|
||||
[9003, 'screen', 'green', 12.5, 'comment', 0],
|
||||
@@ -57,6 +59,7 @@ describe('factory-scorecard: recorded window', () => {
|
||||
expect(values.merged).toBe('3 (crew 2, outside 1)');
|
||||
expect(values.lead_time).toBe('median 8.0h, p90 5.0d');
|
||||
expect(values.ready_to_merge).toBe('median 1.0h, p90 2.0h (2 of 3 PRs with a READY comment)');
|
||||
expect(values.review_passes).toBe('median 2, p90 3 (2 of 3 PRs report a pass count; 1 at the cap of 3)');
|
||||
expect(values.gates).toBe('2 / 1 / 1; known cost $43.00 on 2 of 4 runs');
|
||||
expect(values.cost_per_merge).toBe('$14.33 (2 runs without a known cost)');
|
||||
expect(values.red_train).toBe('50% (1 red of 2 trains; 0 red backstop, 0 red screens)');
|
||||
@@ -79,6 +82,48 @@ describe('factory-scorecard: recorded window', () => {
|
||||
expect(source.calls.filter(c => c.includes('/pulls?'))).toHaveLength(1);
|
||||
});
|
||||
|
||||
test('a full page inside the window reads the next page; a full page whose oldest update predates the window does not', () => {
|
||||
// Value: protects=the PR paging loop continues past page 1 only while the page is full and still inside the window; fails_when=the break condition stops after one page or ignores the oldest update; why_new=the recorded fixture never fills a page, so the continue branch was untested; seam=none; no_claim=none beyond protects
|
||||
const pullsKey = (page: number) => `repos/acme/widgets/pulls?state=closed&base=main&sort=updated&direction=desc&per_page=100&page=${page}`;
|
||||
const page = (first: number, updatedAt: string) => Array.from({ length: 100 }, (_, i) => ({ number: first + i, title: `pr ${first + i}`, user: { login: 'alice' }, author_association: 'MEMBER',
|
||||
created_at: '2026-10-24T00:00:00Z', merged_at: '2026-10-25T00:00:00Z', updated_at: updatedAt }));
|
||||
const withPages = (oldest: string) => {
|
||||
const fx = fixture();
|
||||
fx.responses[pullsKey(1)] = page(1000, oldest);
|
||||
fx.responses[pullsKey(2)] = page(2000, oldest).slice(0, 5);
|
||||
for (let n = 1000; n < 1100; n++) fx.responses[`repos/acme/widgets/issues/${n}/comments?per_page=100&page=1`] = [];
|
||||
for (let n = 2000; n < 2005; n++) fx.responses[`repos/acme/widgets/issues/${n}/comments?per_page=100&page=1`] = [];
|
||||
return fx;
|
||||
};
|
||||
const inWindow = fixtureSource(withPages('2026-10-25T00:00:00Z'), scratch());
|
||||
expect(collectScorecard(inWindow, options()).prs).toHaveLength(105);
|
||||
expect(inWindow.calls.filter(c => c.includes('/pulls?'))).toHaveLength(2);
|
||||
const stale = fixtureSource(withPages('2026-10-01T00:00:00Z'), scratch());
|
||||
expect(collectScorecard(stale, options()).prs).toHaveLength(100);
|
||||
expect(stale.calls.filter(c => c.includes('/pulls?'))).toHaveLength(1);
|
||||
});
|
||||
|
||||
test('a PR comment cost backs one gate run per PR; the gate-run read failing marks only its lines unavailable', () => {
|
||||
// Value: protects=two artifact-less gate runs on one PR do not both copy the PR's comment cost into known cost; fails_when=the once-per-PR guard is removed; why_new=review found the fallback keyed by PR number only; seam=none; no_claim=none beyond protects
|
||||
const fx = fixture();
|
||||
const runsKey = Object.keys(fx.responses).find(k => k.includes('/actions/workflows/evals.yml/runs?') && k.includes('event=workflow_dispatch'))!;
|
||||
const body = fx.responses[runsKey] as { workflow_runs: any[] };
|
||||
const screen = body.workflow_runs.find(r => r.id === 9003)!;
|
||||
body.workflow_runs.push({ ...screen, id: 9006, created_at: '2026-10-25T13:00:00Z' });
|
||||
const card = collectScorecard(fixtureSource(fx, scratch()), options());
|
||||
expect(card.gates.filter(g => g.id === 9003 || g.id === 9006).map(g => [g.pr, g.costSource])).toEqual([[101, 'comment'], [101, null]]);
|
||||
expect(byId(scorecardRows(card)).gates).toBe('3 / 1 / 1; known cost $43.00 on 2 of 5 runs');
|
||||
const broken = fixture();
|
||||
delete broken.responses[runsKey];
|
||||
const noGates = collectScorecard(fixtureSource(broken, scratch()), options());
|
||||
expect(noGates.unavailable).toHaveLength(1);
|
||||
expect(noGates.unavailable[0]).toStartWith('gate runs: ');
|
||||
const values = byId(scorecardRows(noGates));
|
||||
expect(values.gates).toStartWith('unavailable (');
|
||||
expect(values.red_train).toStartWith('unavailable (');
|
||||
expect(values.merged).toBe('3 (crew 2, outside 1)');
|
||||
});
|
||||
|
||||
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);
|
||||
@@ -156,10 +201,31 @@ describe('factory-scorecard: recorded window', () => {
|
||||
const values = byId(scorecardRows(card));
|
||||
expect(values.merged).toStartWith('unavailable (');
|
||||
expect(values.lead_time).toStartWith('unavailable (');
|
||||
expect(values.gates).toBe('2 / 1 / 1; known cost $30.50 on 1 of 4 runs');
|
||||
expect(values.gates).toBe('2 / 1 / 1; known cost $30.50 on 1 of 4 runs (comment costs unavailable: merged PRs unreadable)');
|
||||
expect(values.cost_per_merge).toStartWith('unavailable (');
|
||||
expect(values.review_passes).toStartWith('unavailable (');
|
||||
expect(card.gates.find(g => g.id === 9003)!.costSource).toBeNull();
|
||||
expect(values.issues).toBe('5 / 7');
|
||||
});
|
||||
|
||||
test('a failed free-run read marks only the free half of the flaky line; an unreadable artifact leaves its run without cost or flake data', () => {
|
||||
// Value: protects=the flaky line never prints a clean 0 for a read that failed, and one bad artifact does not drop the whole gate section; fails_when=the flaky row is keyed to one guard again or the per-run try/catch is removed; why_new=red-team review found both; seam=none; no_claim=none beyond protects
|
||||
const fx = fixture();
|
||||
for (const key of Object.keys(fx.responses)) if (key.includes('/actions/workflows/free-tests.yml/')) delete fx.responses[key];
|
||||
const values = byId(scorecardRows(collectScorecard(fixtureSource(fx, scratch()), options())));
|
||||
expect(values.flaky).toMatch(/^2 paid case\(s\) passed on their one rerun \(rerun spend \$1\.10\); unavailable \(/);
|
||||
expect(values.gates).toBe('2 / 1 / 1; known cost $43.00 on 2 of 4 runs');
|
||||
const inner = fixtureSource(fixture(), scratch());
|
||||
const flaky = { getJson: (p: string) => inner.getJson(p), artifactDirs: (id: number, m: (n: string) => boolean) => { if (id === 9001 || id === 8001) throw new Error('unzip failed'); return inner.artifactDirs(id, m); } };
|
||||
const card = collectScorecard(flaky, options());
|
||||
expect(card.unavailable).toEqual([]);
|
||||
expect(card.artifactErrors).toBe(1);
|
||||
expect(card.gates.find(g => g.id === 9001)!.costUsd).toBeNull();
|
||||
expect(card.freeRuns.find(r => r.id === 8001)!.retries).toBeNull();
|
||||
const v = byId(scorecardRows(card));
|
||||
expect(v.gates).toBe('2 / 1 / 1; known cost $12.50 on 1 of 4 runs');
|
||||
expect(v.flaky).toBe('0 paid case(s) passed on their one rerun (rerun spend $0.00); artifacts unreadable on 1 run(s); 0 free-suite retries in 0 of 3 runs (1 unreadable)');
|
||||
});
|
||||
});
|
||||
|
||||
describe('factory-scorecard: pieces', () => {
|
||||
@@ -174,9 +240,15 @@ describe('factory-scorecard: pieces', () => {
|
||||
|
||||
test('READY marker: the first matching comment wins and the newest cost line is kept', () => {
|
||||
const source = fixtureSource(fixture(), scratch());
|
||||
expect(readComments(source, 'acme/widgets', 101, /READY #\d+/m)).toEqual({ readyAt: '2026-10-24T16:00:00Z', costUsd: 12.5 });
|
||||
expect(readComments(source, 'acme/widgets', 101, /never matches/m)).toEqual({ readyAt: null, costUsd: 12.5 });
|
||||
expect(readComments(source, 'acme/widgets', 103, new RegExp(DEFAULTS.readyMarker, 'm'))).toEqual({ readyAt: '2026-10-25T03:00:00Z', costUsd: null });
|
||||
const markers = (readyMarker: RegExp, mergedAt?: string) => ({ readyMarker, passMarker: new RegExp(DEFAULTS.passMarker, 'm'), mergedAt });
|
||||
expect(readComments(source, 'acme/widgets', 101, markers(/READY #\d+/m))).toEqual({ readyAt: '2026-10-24T16:00:00Z', costUsd: 12.5, passes: null });
|
||||
expect(readComments(source, 'acme/widgets', 101, markers(/never matches/m))).toEqual({ readyAt: null, costUsd: 12.5, passes: null });
|
||||
expect(readComments(source, 'acme/widgets', 103, markers(new RegExp(DEFAULTS.readyMarker, 'm')))).toEqual({ readyAt: '2026-10-25T03:00:00Z', costUsd: null, passes: 3 });
|
||||
// Value: protects=a READY-style comment posted after the merge (a controller recap) does not become the READY time; fails_when=the mergedAt bound is dropped; why_new=review found negative READY-to-merge durations possible; seam=none; no_claim=none beyond protects
|
||||
expect(readComments(source, 'acme/widgets', 102, markers(/READY #\d+/m, '2026-10-25T12:00:00Z')).readyAt).toBeNull();
|
||||
expect(readComments(source, 'acme/widgets', 102, markers(/READY #\d+/m)).readyAt).toBe('2026-10-26T01:00:00Z');
|
||||
expect(passCount('"passes": 4 then passes=1 then REVIEW_PASSES: 2', new RegExp(DEFAULTS.passMarker, 'm'))).toBe(4);
|
||||
expect(passCount('no count here', new RegExp(DEFAULTS.passMarker, 'm'))).toBeNull();
|
||||
});
|
||||
|
||||
test('verdict cost reads collector-outcomes.json and ignores an unreadable one; flake records skip torn lines and unknown rerun cost', () => {
|
||||
@@ -230,6 +302,18 @@ describe('factory-scorecard: the bin', () => {
|
||||
expect(r.stdout).toContain('(1 of 3 PRs with a READY comment)');
|
||||
expect(fs.existsSync(path.join(cwd, 'card.csv'))).toBe(true);
|
||||
expect(fs.existsSync(path.join(out, 'scorecard.md'))).toBe(true);
|
||||
// Value: protects=--csv into a directory that does not exist yet is created instead of crashing after every read; fails_when=the mkdirSync before the CSV write is removed; why_new=review found only --out-dir was created; seam=none; no_claim=none beyond protects
|
||||
const nested = run(['--repo', 'acme/widgets', '--fixtures', FIXTURE, '--now', '2026-10-26T12:00:00Z', '--csv', path.join(cwd, 'trends', 'w43', 'card.csv')], cwd);
|
||||
expect(nested.status, nested.stderr).toBe(0);
|
||||
expect(fs.existsSync(path.join(cwd, 'trends', 'w43', 'card.csv'))).toBe(true);
|
||||
// Value: protects=every run appends its rows to trend.csv with one header, so weeks line up without renaming files; fails_when=the append is removed or the header repeats; why_new=review found scorecard.csv overwritten every run; seam=none; no_claim=none beyond protects
|
||||
const trend = fs.readFileSync(path.join(cwd, '.context', 'factory-scorecard', 'trend.csv'), 'utf8').split('\n').filter(Boolean);
|
||||
expect(trend.filter(l => l.startsWith('repo,')).length).toBe(1);
|
||||
expect(trend.length).toBe(1 + Object.keys(BENCHMARKS).length);
|
||||
const again = run(['--repo', 'acme/widgets', '--fixtures', FIXTURE, '--now', '2026-10-27T12:00:00Z'], cwd);
|
||||
expect(again.status, again.stderr).toBe(0);
|
||||
expect(fs.readFileSync(path.join(cwd, '.context', 'factory-scorecard', 'trend.csv'), 'utf8').split('\n').filter(Boolean).length).toBe(1 + 2 * Object.keys(BENCHMARKS).length);
|
||||
expect(nested.stdout).toContain(`SCORECARD_TREND: ${path.join('.context', 'factory-scorecard', 'trend.csv')}`);
|
||||
});
|
||||
|
||||
test('bad usage exits 2 with the usage text; --help exits 0', () => {
|
||||
@@ -238,6 +322,17 @@ describe('factory-scorecard: the bin', () => {
|
||||
// Value: protects=an unparsable --now exits 2 before any read; fails_when=the --now validation branch is removed; why_new=the other flag validations were covered and this one was not; seam=none; no_claim=none beyond protects
|
||||
expect(run(['--now', 'yesterday-ish', '--fixtures', FIXTURE], cwd).status).toBe(2);
|
||||
expect(run(['--ready-marker', '(', '--fixtures', FIXTURE], cwd).status).toBe(2);
|
||||
// Value: protects=a non-numeric or negative --max-artifact-runs exits 2 instead of silently reading no artifact; fails_when=the validation is removed and Number('lots') flows into Math.min; why_new=review found this flag unvalidated; seam=none; no_claim=none beyond protects
|
||||
const lots = run(['--max-artifact-runs', 'lots', '--fixtures', FIXTURE], cwd);
|
||||
expect(lots.status).toBe(2);
|
||||
expect(lots.stderr).toContain('--max-artifact-runs must be a non-negative integer');
|
||||
expect(run(['--max-artifact-runs', '-1', '--fixtures', FIXTURE], cwd).status).toBe(2);
|
||||
expect(run(['--pass-cap', '0', '--fixtures', FIXTURE], cwd).status).toBe(2);
|
||||
expect(run(['--pass-marker', '[', '--fixtures', FIXTURE], cwd).status).toBe(2);
|
||||
// Value: protects=without --repo and without a GitHub origin the bin refuses instead of guessing a repository; fails_when=a hard-coded fallback repository returns; why_new=red-team review found the shared helper's fallback would score the wrong repository; seam=none; no_claim=none beyond protects
|
||||
const noRepo = run(['--fixtures', FIXTURE], cwd);
|
||||
expect(noRepo.status).toBe(2);
|
||||
expect(noRepo.stderr).toContain('--repo owner/name is required');
|
||||
const stray = run(['stray'], cwd);
|
||||
expect(stray.status).toBe(2);
|
||||
expect(stray.stderr).toContain('unexpected argument: stray');
|
||||
|
||||
+21
-12
@@ -60,7 +60,8 @@
|
||||
"updated_at": "2026-10-24T18:00:00Z",
|
||||
"base": {
|
||||
"ref": "main"
|
||||
}
|
||||
},
|
||||
"body": "```gstack-ship-receipt\n{\n \"schema_version\": 1,\n \"pr\": 101,\n \"passes\": 2\n}\n```\n\n## What changed\n"
|
||||
},
|
||||
{
|
||||
"number": 104,
|
||||
@@ -85,7 +86,7 @@
|
||||
"login": "github-actions[bot]"
|
||||
},
|
||||
"created_at": "2026-10-24T15:00:00Z",
|
||||
"body": "## E2E Evals: ✅ PASS\n\n**24 executed, 0 reused** rule/judge records | **0 manual accepted** | **$12.50** rule/judge cost | reconcile exit: 0"
|
||||
"body": "## E2E Evals: \u2705 PASS\n\n**24 executed, 0 reused** rule/judge records | **0 manual accepted** | **$12.50** rule/judge cost | reconcile exit: 0"
|
||||
},
|
||||
{
|
||||
"id": 2,
|
||||
@@ -112,6 +113,14 @@
|
||||
},
|
||||
"created_at": "2026-10-21T09:00:00Z",
|
||||
"body": "Thanks for the review, fixed."
|
||||
},
|
||||
{
|
||||
"id": 6,
|
||||
"user": {
|
||||
"login": "minder"
|
||||
},
|
||||
"created_at": "2026-10-26T01:00:00Z",
|
||||
"body": "merged in train 7: READY #102 recap"
|
||||
}
|
||||
],
|
||||
"repos/acme/widgets/issues/103/comments?per_page=100&page=1": [
|
||||
@@ -121,7 +130,7 @@
|
||||
"login": "bo"
|
||||
},
|
||||
"created_at": "2026-10-25T03:00:00Z",
|
||||
"body": "ship receipt follows\nSHIP_RECEIPT: pr=103 head=bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"
|
||||
"body": "ship receipt follows\nSHIP_RECEIPT: pr=103 head=bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb passes=3"
|
||||
}
|
||||
],
|
||||
"repos/acme/widgets/actions/workflows/evals.yml/runs?status=completed&per_page=100&created=%3E%3D2026-10-19&event=workflow_dispatch&page=1": {
|
||||
@@ -260,14 +269,6 @@
|
||||
}
|
||||
]
|
||||
},
|
||||
"search/issues?q=repo%3Aacme%2Fwidgets%20is%3Aissue%20created%3A%3E%3D2026-10-19&per_page=1": {
|
||||
"total_count": 5,
|
||||
"items": []
|
||||
},
|
||||
"search/issues?q=repo%3Aacme%2Fwidgets%20is%3Aissue%20closed%3A%3E%3D2026-10-19&per_page=1": {
|
||||
"total_count": 7,
|
||||
"items": []
|
||||
},
|
||||
"repos/acme/widgets/releases?per_page=100&page=1": [
|
||||
{
|
||||
"tag_name": "v1.1.0",
|
||||
@@ -287,7 +288,15 @@
|
||||
"prerelease": false,
|
||||
"published_at": "2026-10-01T07:00:00Z"
|
||||
}
|
||||
]
|
||||
],
|
||||
"search/issues?q=repo%3Aacme%2Fwidgets%20is%3Aissue%20created%3A%3E%3D2026-10-19T12%3A00%3A00Z&per_page=1": {
|
||||
"total_count": 5,
|
||||
"items": []
|
||||
},
|
||||
"search/issues?q=repo%3Aacme%2Fwidgets%20is%3Aissue%20closed%3A%3E%3D2026-10-19T12%3A00%3A00Z&per_page=1": {
|
||||
"total_count": 7,
|
||||
"items": []
|
||||
}
|
||||
},
|
||||
"artifacts": {
|
||||
"8001": {
|
||||
|
||||
@@ -90,6 +90,7 @@ export const IDENTIFIER_PLACEHOLDERS: Record<string, IdentifierGrammar> = {
|
||||
'<start_date>': { grammar: DATE, what: 'YYYY-MM-DD' },
|
||||
'<since>': { grammar: /^[0-9]+[dhwm]$|^[0-9]{4}-[0-9]{2}-[0-9]{2}$/, what: 'retro window (7d, 24h, 2w) or date' },
|
||||
'<window>': { grammar: /^[0-9]+[dhwm]$/, what: 'retro window (7d, 24h, 2w)' },
|
||||
'<window-days>': { grammar: NUM, what: 'retro window in whole days' },
|
||||
|
||||
// Git refs. Branch names from remote data travel as data (ENG-8), never through this list.
|
||||
'<base>': { grammar: REF, what: 'base branch detected by the skill' },
|
||||
|
||||
Reference in new issue
Block a user