From c57872f27ab1eddaf83f43cebecab054c0836ec6 Mon Sep 17 00:00:00 2001 From: Cervantes Hernandez <11169707+cervantesh@users.noreply.github.com> Date: Wed, 15 Jul 2026 19:57:22 -0400 Subject: [PATCH] perf: enforce pagination regression budget --- .github/workflows/ci.yml | 14 ++-- CHANGELOG.md | 3 + README.md | 4 +- docs/ARCHITECTURE.md | 8 +-- docs/PAGINATION_BENCHMARK.md | 6 +- docs/PAGINATION_BUDGET_DECISION.md | 40 +++++++++++ package.json | 3 +- scripts/enforce-pagination-budget.d.mts | 25 +++++++ scripts/enforce-pagination-budget.mjs | 95 +++++++++++++++++++++++++ scripts/pagination-budget.json | 9 +++ tests/pagination-budget.test.ts | 60 ++++++++++++++++ 11 files changed, 251 insertions(+), 16 deletions(-) create mode 100644 docs/PAGINATION_BUDGET_DECISION.md create mode 100644 scripts/enforce-pagination-budget.d.mts create mode 100644 scripts/enforce-pagination-budget.mjs create mode 100644 scripts/pagination-budget.json create mode 100644 tests/pagination-budget.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3aaafcd..878cb6c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -41,9 +41,8 @@ jobs: - name: Inspect publish contents run: npm pack --dry-run - pagination-baseline: - name: Pagination baseline (nonblocking) - continue-on-error: true + pagination-budget: + name: Pagination regression budget runs-on: ubuntu-latest timeout-minutes: 10 steps: @@ -59,14 +58,17 @@ jobs: - name: Install run: npm ci - - name: Measure guarded pagination baseline + - name: Measure guarded pagination run: npm run benchmark:pagination - - name: Upload machine-readable baseline + - name: Enforce pagination regression budget + run: npm run benchmark:pagination:enforce + + - name: Upload machine-readable evidence if: always() uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 with: name: pagination-benchmark-results - path: artifacts/pagination-benchmark.json + path: artifacts/pagination-*.json if-no-files-found: error retention-days: 30 diff --git a/CHANGELOG.md b/CHANGELOG.md index 6289d01..9cc4d46 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,9 @@ - Exported pagination hard-guard maximums, deterministic 100-page/10,000-item measurement with guard fixtures and machine-readable latency/memory evidence, and a nonblocking hosted baseline artifact job. +- Required pagination regression enforcement for p99 latency, peak RSS, and peak heap, + derived from three merged-workflow hosted baselines with documented headroom and + deterministic policy tests. ### Changed diff --git a/README.md b/README.md index 1909282..06a2fd5 100644 --- a/README.md +++ b/README.md @@ -129,8 +129,8 @@ This package now covers the smallest high-value GitHub admin workflow end to end - internal GitHub requests use a 10-second default timeout, retry bounded read-only transient failures, and surface sanitized structured recovery metadata - pagination hard guard maximums are exported as `MAX_COLLECTION_PAGES` and - `MAX_COLLECTION_ITEMS`; the deterministic offline benchmark records hosted baseline - evidence without enforcing a budget until stable shared-runner samples exist + `MAX_COLLECTION_ITEMS`; the deterministic offline benchmark enforces reviewed p99, + peak RSS, and peak heap regression limits from hosted baseline evidence - security verification keeps legacy `state` booleans and `ok` while adding per-control capability, enabled-state, sanitized reason/status, and bounded recovery results; masked 404 responses remain `unknown` rather than being treated as proof of unsupported state diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index d4baee9..1f3935b 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -91,9 +91,9 @@ npm pack --dry-run `npm run verify` runs type checking, the Vitest suite with committed coverage thresholds, and release/changelog metadata verification. CI runs the same checks on -pull requests and pushes to `main`. `npm run benchmark` runs the helper microbenchmarks -and the deterministic guarded-pagination measurement but does not enforce a shared-runner -budget. CI uploads the nonblocking hosted pagination baseline described in -[`PAGINATION_BENCHMARK.md`](PAGINATION_BENCHMARK.md). +pull requests and pushes to `main`. `npm run benchmark` runs the helper microbenchmarks, +the deterministic guarded-pagination measurement, and the reviewed regression budget. +CI enforces the same pagination budget as a required job and uploads the measurement +artifact described in [`PAGINATION_BENCHMARK.md`](PAGINATION_BENCHMARK.md). There is no separate compile or bundle artifact: Pi loads the TypeScript extension entry point declared in `package.json`. diff --git a/docs/PAGINATION_BENCHMARK.md b/docs/PAGINATION_BENCHMARK.md index 9645024..da8f89b 100644 --- a/docs/PAGINATION_BENCHMARK.md +++ b/docs/PAGINATION_BENCHMARK.md @@ -12,8 +12,8 @@ npm run benchmark:pagination The command performs three warmup runs and 25 measured runs. It writes machine-readable JSON to `artifacts/pagination-benchmark.json` (override with `PAGINATION_BENCHMARK_OUTPUT`) and reports p50, p95, and p99 latency plus the peak observed RSS and heap usage. Guard fixtures cover maximum-item truncation, maximum-page truncation, repeated links, and an exact-boundary non-truncated result. -## Measurement versus decision +## Measurement and decision -This change establishes evidence collection only. The hosted `pagination-baseline` job is deliberately nonblocking and uploads `pagination-benchmark-results` for comparison across GitHub-hosted runs. It does not encode a latency or memory budget. +The measurement remains independent from policy: it records raw values and fixture results without embedding thresholds. `npm run benchmark:pagination:enforce` deterministically validates that JSON against `scripts/pagination-budget.json` and writes `artifacts/pagination-budget-result.json`. `npm run benchmark` runs both measurement and enforcement. -After enough comparable hosted samples exist, maintainers should review run-to-run variance and open a separate budget PR if stable bounds can be justified. That decision PR should document the selected statistic, tolerated variance, runner assumptions, and failure/rebaseline process before turning a threshold into a required CI gate. A single local or hosted sample is not sufficient evidence for a hard budget. +The required hosted `pagination-budget` job uploads both JSON files as `pagination-benchmark-results`, even when enforcement fails. Threshold selection, hosted evidence, headroom, failure handling, and rebaseline rules are recorded in [`PAGINATION_BUDGET_DECISION.md`](PAGINATION_BUDGET_DECISION.md). diff --git a/docs/PAGINATION_BUDGET_DECISION.md b/docs/PAGINATION_BUDGET_DECISION.md new file mode 100644 index 0000000..dad8e73 --- /dev/null +++ b/docs/PAGINATION_BUDGET_DECISION.md @@ -0,0 +1,40 @@ +# Pagination regression budget decision + +## Decision + +The hosted pagination job is a required CI gate. After measuring the deterministic 100-page/10,000-item fake, CI rejects any run whose machine-readable result exceeds one of these bounds: + +| Metric | Required limit | +| --- | ---: | +| p99 latency | 12 ms | +| Peak RSS | 134,217,728 bytes (128 MiB) | +| Peak heap used | 33,554,432 bytes (32 MiB) | + +The policy lives in `scripts/pagination-budget.json`; `scripts/enforce-pagination-budget.mjs` validates the workload/fixture contract before applying all three limits. + +## Hosted evidence + +Three fresh GitHub-hosted executions used the merged workflow at commit `6aef64df0f8f5223334b50f3614f77d58247aed1`, `ubuntu-latest`, and Node `v22.23.1`. They are independent attempts of [CI run 29459726382](https://github.com/T50-Systems/pi-github-admin/actions/runs/29459726382), each on a fresh hosted job. + +| Run attempt | p50 ms | p95 ms | p99 ms | Peak RSS bytes | Peak heap bytes | +| ---: | ---: | ---: | ---: | ---: | ---: | +| 1 | 3.863 | 4.818 | 5.819 | 90,746,880 | 22,546,576 | +| 2 | 2.937 | 4.052 | 5.165 | 90,787,840 | 22,545,512 | +| 3 | 2.975 | 3.894 | 4.999 | 88,993,792 | 22,544,248 | +| **Observed maximum** | — | — | **5.819** | **90,787,840** | **22,546,576** | + +All attempts completed three warmups and 25 samples, passed all four guard fixtures, and declared `liveGitHubApi: false`. + +## Headroom rationale + +- The 12 ms p99 bound is 106.2% above the observed 5.819 ms maximum. Tail latency from 25 samples is the noisiest signal on a shared runner, so a roughly 2x rounded bound avoids flaky failures while still detecting a material regression. +- The 128 MiB RSS bound is 47.8% above the observed 90,787,840-byte maximum. +- The 32 MiB heap bound is 48.8% above the observed 22,546,576-byte maximum. + +The memory limits use recognizable binary boundaries and leave similar headroom. They remain bounded enough to catch sustained allocation growth rather than normal hosted-runner noise. + +## Failure and rebaseline policy + +A failing gate is a regression until investigated. Confirm the workload and fixtures are unchanged, reproduce locally, and inspect the uploaded JSON artifact. Do not raise a limit solely to make one run pass. + +Rebaseline only after an intentional pagination/runtime change or a demonstrated hosted-runner shift. Collect at least three fresh successful hosted runs under the same workload, document every observed value and new headroom calculation in a reviewed PR, then update the policy. The benchmark must remain offline and the budget job must remain required. diff --git a/package.json b/package.json index 2abd7a5..e8378b8 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,8 @@ "test:coverage": "vitest run --coverage", "benchmark:core": "vitest bench --run", "benchmark:pagination": "vitest run --config benchmarks/vitest.measurement.config.ts", - "benchmark": "npm run benchmark:core && npm run benchmark:pagination", + "benchmark:pagination:enforce": "node scripts/enforce-pagination-budget.mjs", + "benchmark": "npm run benchmark:core && npm run benchmark:pagination && npm run benchmark:pagination:enforce", "verify:release": "node scripts/verify-release.mjs", "verify:workflows": "node scripts/verify-workflows.mjs", "verify": "npm run verify:workflows && npm run typecheck && npm run test:coverage && npm run verify:release" diff --git a/scripts/enforce-pagination-budget.d.mts b/scripts/enforce-pagination-budget.d.mts new file mode 100644 index 0000000..d13cfe1 --- /dev/null +++ b/scripts/enforce-pagination-budget.d.mts @@ -0,0 +1,25 @@ +export interface PaginationBudgetResult { + ok: boolean; + benchmark?: string; + observed: { + p99LatencyMs?: number; + peakRssBytes?: number; + peakHeapUsedBytes?: number; + }; + limits: { + p99LatencyMs?: number; + peakRssBytes?: number; + peakHeapUsedBytes?: number; + }; + violations: string[]; +} + +export function evaluatePaginationBudget( + measurement: unknown, + budget: unknown, +): PaginationBudgetResult; + +export function enforcePaginationBudget( + measurementPath: string, + budgetPath: string, +): Promise; diff --git a/scripts/enforce-pagination-budget.mjs b/scripts/enforce-pagination-budget.mjs new file mode 100644 index 0000000..feb8f5f --- /dev/null +++ b/scripts/enforce-pagination-budget.mjs @@ -0,0 +1,95 @@ +import { mkdir, readFile, writeFile } from "node:fs/promises"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const EXPECTED_BENCHMARK = "guarded-pagination-maximum-collection"; +const EXPECTED_FIXTURES = [ + "exact-boundary-not-truncated", + "max-items", + "max-pages", + "repeated-link", +]; + +function finiteNumber(value, label, violations) { + if (typeof value !== "number" || !Number.isFinite(value) || value < 0) { + violations.push(`${label} must be a finite non-negative number`); + return undefined; + } + return value; +} + +export function evaluatePaginationBudget(measurement, budget) { + const violations = []; + if (measurement?.schemaVersion !== 1) violations.push("measurement schemaVersion must be 1"); + if (budget?.schemaVersion !== 1) violations.push("budget schemaVersion must be 1"); + if (measurement?.benchmark !== EXPECTED_BENCHMARK) violations.push(`measurement benchmark must be ${EXPECTED_BENCHMARK}`); + if (budget?.benchmark !== EXPECTED_BENCHMARK) violations.push(`budget benchmark must be ${EXPECTED_BENCHMARK}`); + if (measurement?.configuration?.liveGitHubApi !== false) violations.push("measurement must declare liveGitHubApi=false"); + if (measurement?.configuration?.transport !== "deterministic-in-memory-fake") violations.push("measurement must use the deterministic in-memory fake"); + if (measurement?.configuration?.pages !== 100 || measurement?.configuration?.items !== 10_000) { + violations.push("measurement must cover exactly 100 pages and 10,000 items"); + } + const warmupRuns = measurement?.configuration?.warmupRuns; + const sampleRuns = measurement?.configuration?.sampleRuns; + if (!Number.isInteger(warmupRuns) || warmupRuns < 3 || !Number.isInteger(sampleRuns) || sampleRuns < 25) { + violations.push("measurement must include at least 3 warmups and 25 samples"); + } + if (!Array.isArray(measurement?.latencyMs?.samples) || measurement.latencyMs.samples.length < sampleRuns) { + violations.push("measurement latency samples must contain every declared sample"); + } + + const fixtureNames = Array.isArray(measurement?.fixtures) + ? measurement.fixtures.filter((fixture) => fixture?.passed === true).map((fixture) => fixture.name).sort() + : []; + if (JSON.stringify(fixtureNames) !== JSON.stringify(EXPECTED_FIXTURES)) { + violations.push(`measurement must pass fixtures: ${EXPECTED_FIXTURES.join(", ")}`); + } + + const observed = { + p99LatencyMs: finiteNumber(measurement?.latencyMs?.p99, "latencyMs.p99", violations), + peakRssBytes: finiteNumber(measurement?.memoryBytes?.peakRss, "memoryBytes.peakRss", violations), + peakHeapUsedBytes: finiteNumber(measurement?.memoryBytes?.peakHeapUsed, "memoryBytes.peakHeapUsed", violations), + }; + const limits = { + p99LatencyMs: finiteNumber(budget?.limits?.p99LatencyMs, "limits.p99LatencyMs", violations), + peakRssBytes: finiteNumber(budget?.limits?.peakRssBytes, "limits.peakRssBytes", violations), + peakHeapUsedBytes: finiteNumber(budget?.limits?.peakHeapUsedBytes, "limits.peakHeapUsedBytes", violations), + }; + + for (const metric of Object.keys(observed)) { + if (observed[metric] !== undefined && limits[metric] !== undefined && observed[metric] > limits[metric]) { + violations.push(`${metric} ${observed[metric]} exceeds limit ${limits[metric]}`); + } + } + + return { ok: violations.length === 0, benchmark: budget?.benchmark, observed, limits, violations }; +} + +export async function enforcePaginationBudget(measurementPath, budgetPath) { + const [measurement, budget] = await Promise.all([ + readFile(measurementPath, "utf8").then(JSON.parse), + readFile(budgetPath, "utf8").then(JSON.parse), + ]); + return evaluatePaginationBudget(measurement, budget); +} + +const invokedPath = process.argv[1] ? path.resolve(process.argv[1]) : ""; +if (invokedPath === fileURLToPath(import.meta.url)) { + const measurementPath = path.resolve(process.argv[2] ?? "artifacts/pagination-benchmark.json"); + const budgetPath = path.resolve(process.argv[3] ?? "scripts/pagination-budget.json"); + const outputPath = path.resolve(process.env.PAGINATION_BUDGET_OUTPUT ?? "artifacts/pagination-budget-result.json"); + try { + const result = await enforcePaginationBudget(measurementPath, budgetPath); + const serialized = JSON.stringify(result); + await mkdir(path.dirname(outputPath), { recursive: true }); + await writeFile(outputPath, `${JSON.stringify(result, null, 2)}\n`, "utf8"); + console.log(`PAGINATION_BUDGET_RESULT=${serialized}`); + if (!result.ok) { + for (const violation of result.violations) console.error(`Pagination budget violation: ${violation}`); + process.exitCode = 1; + } + } catch (error) { + console.error(`Pagination budget enforcement failed: ${error instanceof Error ? error.message : String(error)}`); + process.exitCode = 1; + } +} diff --git a/scripts/pagination-budget.json b/scripts/pagination-budget.json new file mode 100644 index 0000000..046d64e --- /dev/null +++ b/scripts/pagination-budget.json @@ -0,0 +1,9 @@ +{ + "schemaVersion": 1, + "benchmark": "guarded-pagination-maximum-collection", + "limits": { + "p99LatencyMs": 12, + "peakRssBytes": 134217728, + "peakHeapUsedBytes": 33554432 + } +} diff --git a/tests/pagination-budget.test.ts b/tests/pagination-budget.test.ts new file mode 100644 index 0000000..2b2a357 --- /dev/null +++ b/tests/pagination-budget.test.ts @@ -0,0 +1,60 @@ +import { describe, expect, it } from "vitest"; +import { evaluatePaginationBudget } from "../scripts/enforce-pagination-budget.mjs"; + +const budget = { + schemaVersion: 1, + benchmark: "guarded-pagination-maximum-collection", + limits: { p99LatencyMs: 12, peakRssBytes: 134_217_728, peakHeapUsedBytes: 33_554_432 }, +}; + +function measurement(overrides: Record = {}) { + return { + schemaVersion: 1, + benchmark: "guarded-pagination-maximum-collection", + configuration: { + warmupRuns: 3, + sampleRuns: 25, + pages: 100, + items: 10_000, + transport: "deterministic-in-memory-fake", + liveGitHubApi: false, + }, + latencyMs: { p99: 5.819, samples: Array.from({ length: 25 }, () => 5) }, + memoryBytes: { peakRss: 90_787_840, peakHeapUsed: 22_546_576 }, + fixtures: [ + { name: "max-items", passed: true }, + { name: "max-pages", passed: true }, + { name: "repeated-link", passed: true }, + { name: "exact-boundary-not-truncated", passed: true }, + ], + ...overrides, + }; +} + +describe("pagination budget enforcement", () => { + it("accepts a complete hosted-style measurement within every limit", () => { + expect(evaluatePaginationBudget(measurement(), budget)).toMatchObject({ ok: true, violations: [] }); + }); + + it.each([ + ["p99LatencyMs", { latencyMs: { p99: 12.001 } }], + ["peakRssBytes", { memoryBytes: { peakRss: 134_217_729, peakHeapUsed: 22_546_576 } }], + ["peakHeapUsedBytes", { memoryBytes: { peakRss: 90_787_840, peakHeapUsed: 33_554_433 } }], + ])("rejects a %s regression", (metric, override) => { + const result = evaluatePaginationBudget(measurement(override), budget); + expect(result.ok).toBe(false); + expect(result.violations).toContainEqual(expect.stringContaining(`${metric} `)); + }); + + it("rejects measurements that bypass the deterministic fixture contract", () => { + const result = evaluatePaginationBudget(measurement({ + configuration: { ...measurement().configuration, liveGitHubApi: true }, + fixtures: [{ name: "max-items", passed: true }], + }), budget); + expect(result.ok).toBe(false); + expect(result.violations).toEqual(expect.arrayContaining([ + "measurement must declare liveGitHubApi=false", + expect.stringContaining("measurement must pass fixtures"), + ])); + }); +});