diff --git a/README.md b/README.md index 4b89051e..37e4c157 100644 --- a/README.md +++ b/README.md @@ -581,15 +581,17 @@ or its private bare repository. Batches alternate between the oldest and the newest queued PR as the base. Add up to floor(log2 n) further PRs that merge cleanly in that order, resolving -quest links with `quest resolve`. A failed batch halves the next batch's size, -down to one PR, and each batch that lands grows it back by one; the order and -size persist in the state directory. Conflicting -additions stay queued. A PR whose required checks are still running stays queued +quest links with `quest resolve`. The batch size lands the most PRs per check +at the per-PR failure rate measured over the last 30 checked batches: a batch +of n PRs passes with probability (1 - p)^n, so the size maximizes n(1 - p)^n, +four PRs at a 22% rate. The order, size, and those batches persist in the +state directory. Conflicting additions stay queued. A PR whose required checks are still running stays queued and sits out batches until they pass. If any are still pending two hours after its pick (twice the default `--timeout`), the daemon comments once on that pick, naming them, since a required check that never reports would hold it silently. The sandbox checks the tip once, without -bisection: success lands one squash commit per PR. A failure, or a base that -conflicts with the batch's first PR, parks those PRs for the fixer: they keep their +bisection: success lands one squash commit per PR. A failure parks its PRs for +the fixer, as does a conflict with the base on a PR that would start the batch, +which then starts from the next PR in the same round. Parked PRs keep their auto-merge, `quest:merge`, and place, get no comment, and sit out batches while its job runs, so the rest of the queue keeps landing. The fixer reads the log and blames the PRs that caused it; the others go back to batches unchanged. When it blames diff --git a/quest/m0/land-fixer/README.md b/quest/m0/land-fixer/README.md index 3e93a473..c76433c2 100644 --- a/quest/m0/land-fixer/README.md +++ b/quest/m0/land-fixer/README.md @@ -142,7 +142,7 @@ work, done once the Required quests below have merged. - [Lockfile merges](/quest/m0/land-fixer/lockfile.md) - a conflicted lockfile is rebuilt from the base's copy, so a fix never reverts the base's dependency changes - [Batch quest check](/quest/m0/land-fixer/batch-check.md) - the daemon runs `quest check` on the batch's chain before spending a sandbox check - [Pre-check queued PRs](/quest/m0/land-fixer/precheck.md) - compile and lint the next PRs against the base while a batch runs, parking broken ones before they batch -- [Throughput tuning](/quest/m0/land-fixer/tuning.md) - batch size from the failure rate, no wasted base-conflict rounds, rerere, a fixer cap, and an explicit reasoning effort +- [Throughput tuning](/quest/m0/land-fixer/tuning.md) - fixer containers run below the batch check's priority; rerere only if it can replay anything - [CI failures park](/quest/m0/land-fixer/ci-park.md) - a PR whose own required CI fails is parked for the fixer with the failing log, not ejected - [Queue status in the required check](/quest/m0/land-fixer/check-status.md) - the gate shows each PR's queue state in its required check run, never completing it - [PR CI as a signal](/quest/m0/land-fixer/pr-signal.md) - `/quest-merge` and the fixer read a PR's own CI without gating on it; public repos may run it, private ones need not diff --git a/quest/m0/land-fixer/tuning.md b/quest/m0/land-fixer/tuning.md index c9230247..e1c5f2ef 100644 --- a/quest/m0/land-fixer/tuning.md +++ b/quest/m0/land-fixer/tuning.md @@ -1,34 +1,45 @@ -# [S] Tune batching and the fixer for throughput +# [XS] Fixer priority and rerere on the land host ## Goal -Once failing PRs are parked instead of ejected, the gate stops paying for -choices made while a failure ejected a whole batch: +The fixer stops slowing the batch check, and the gate stops paying twice for +a conflict it has already seen: -- Batch size follows the measured per-PR failure rate, not halving on each - failure. -- A conflict on the batch's base PR parks that PR and fills the batch in the - same round, without spending a round on it. +- Fixer containers run below the batch check's CPU and IO priority on the + land host, so checks under load stop timing out. - The gate's repository enables `git rerere`, so it replays conflict - resolutions it has already made. -- Concurrent fixer jobs are capped and run below the batch check's priority. -- The fixer runs with an explicit reasoning effort; today Codex reports - `reasoning effort: none`. + resolutions it has already made, if a measurement shows it can. ## Plan Decided 2026-11-01 (maintainer, from the audit of the merge-queue plan; see -the [epic](/quest/m0/land-fixer/README.md)). Audit findings on moq's 24 hours: - -- Halving after each failure kept batches at 2-3 PRs. With failing PRs - parked, size from the recent failure rate instead (about 4-5 at today's - rate), keeping `batch-policy.json` as the state. -- 156 rounds were a base-PR conflict that ran no check, about 1.5-2.5 hours - of lane time a day. -- 44 of 79 conflicting PRs conflicted 2-7 times; `rerere` lets the gate's - own base merges replay resolutions. -- Seven fixer jobs started within three minutes on a host already at load - 18-25 of 32 cores, and timeouts under load caused several flaky failures. -- Fixer job logs show `reasoning effort: none`. `land-fix.ts` passes a model - but no `model_reasoning_effort`; set one, defaulting to `medium`, with a - daemon option to override it. +the [epic](/quest/m0/land-fixer/README.md)). What landed of the original +quest: + +- Batch size from the measured failure rate, and a base-PR conflict that + parks the PR and fills the batch in the same round: + [moq-dev/quest#376](https://github.com/moq-dev/quest/pull/376). +- The fixer's explicit `medium` reasoning effort: + [moq-dev/quest#370](https://github.com/moq-dev/quest/pull/370). +- The fixer cap: the host's `quest-land-fixer.socket` accepts at most three + fixer containers across every gate + ([kixelated/nixos@aa155d1](https://github.com/kixelated/nixos/commit/aa155d1)). + +Remaining: + +- Priority is host configuration in + [kixelated/nixos#16](https://github.com/kixelated/nixos/pull/16) + (`modules/quest-land.nix`). As of 2026-11-01 neither + `quest-land-fixer@.service` nor the `quest-land-*-check@.service` units set + `CPUWeight` or `IOWeight`; both run at the default 100 in `ci.slice`. Give + the fixer unit a lower weight, such as `CPUWeight=20` and `IOWeight=20`. + Check with `systemctl show quest-land-fixer@ -p CPUWeight`. +- rerere: open question. On moq-dev/moq from 2026-10-30 to 11-01, 43 of the + 89 PRs that conflicted with the base conflicted more than once, but only 20 + of the 96 repeats named the same files as the PR's previous conflict. The + gate never resolves a conflict itself: `squash` in `src/cli/land.ts` resets + on any conflict, and the fixer resolves in a fresh container clone, so the + gate's `rr-cache` would stay empty. Replaying would mean training rerere + from the fixer's pushed merges and letting the gate's own squashes apply + them, which lands a resolution the PR's head never had. Recommended: drop + this item and delete the quest once priority lands. diff --git a/src/cli/land-policy.ts b/src/cli/land-policy.ts new file mode 100644 index 00000000..0a4c1c99 --- /dev/null +++ b/src/cli/land-policy.ts @@ -0,0 +1,103 @@ +// How big the next batch is: sized from the measured per-PR failure rate of recent checks. +import { readFileSync, renameSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; + +/** How the next batch is chosen: its base end of the queue, and at most how many PRs it checks. */ +export interface BatchPolicy { + order: "oldest" | "newest"; + size: number; +} + +/** One checked batch: how many PRs it held and whether its check passed. */ +export interface CheckedBatch { + size: number; + passed: boolean; +} + +const POLICY = "batch-policy.json"; + +/** + * How many checked batches the failure rate is measured over: about six hours of checks. A + * simulation at moq's measured rate lands within 2% of the best fixed size per check with this + * window, and a shorter one follows a changed rate sooner but lands less. + */ +export const HISTORY = 30; + +/** The next batch's policy and the batches it was sized from; a missing or unreadable record starts oldest-first at full size. */ +export function readPolicy(state: string): BatchPolicy & { history: CheckedBatch[] } { + try { + const data = JSON.parse(readFileSync(join(state, POLICY), "utf8")) as { + order?: unknown; + size?: unknown; + history?: unknown; + }; + const order = data.order === "newest" ? "newest" : "oldest"; + // An unbounded size is saved as null, since JSON has no infinity. + const size = typeof data.size === "number" && data.size >= 1 ? Math.floor(data.size) : Number.POSITIVE_INFINITY; + const history = Array.isArray(data.history) + ? data.history.filter( + (item): item is CheckedBatch => + typeof item === "object" && + item !== null && + Number.isSafeInteger(item.size) && + item.size >= 1 && + typeof item.passed === "boolean", + ) + : []; + return { order, size, history: history.slice(-HISTORY) }; + } catch { + return { order: "oldest", size: Number.POSITIVE_INFINITY, history: [] }; + } +} + +/** After a checked batch of `size` PRs: flip the order and size the next batch from the recent failure rate. */ +export function recordBatch(state: string, previous: BatchPolicy & { history: CheckedBatch[] }, batch: CheckedBatch) { + const history = [...previous.history, batch].slice(-HISTORY); + const policy = { + order: previous.order === "oldest" ? "newest" : "oldest", + size: batchSize(failureRate(history)), + history, + }; + const path = join(state, POLICY); + writeFileSync(`${path}.new`, JSON.stringify(policy), { mode: 0o600 }); + renameSync(`${path}.new`, path); +} + +/** + * The per-PR failure rate that best explains `history`, assuming each PR breaks a batch + * independently with the same probability p: a batch of n passes with probability (1 - p)^n. + * The log-likelihood is concave in p, so bisecting its derivative finds the maximum. + */ +export function failureRate(history: CheckedBatch[]): number { + if (!history.some((batch) => !batch.passed)) return 0; + if (!history.some((batch) => batch.passed)) return 1; + const slope = (p: number) => { + const q = 1 - p; + let total = 0; + for (const { size, passed } of history) + total += passed ? -size / q : (size * q ** (size - 1)) / (1 - q ** size); + return total; + }; + let low = 0; + let high = 1; + for (let i = 0; i < 60; i++) { + const mid = (low + high) / 2; + if (slope(mid) > 0) low = mid; + else high = mid; + } + return (low + high) / 2; +} + +/** + * The batch size that lands the most PRs per check at failure rate `p`: n(1 - p)^n, which peaks + * at n = -1 / ln(1 - p). A failed batch lands nothing but costs only its check, since the fixer + * blames its culprits and the rest rejoin later batches. + */ +export function batchSize(p: number): number { + if (p <= 0) return Number.POSITIVE_INFINITY; + if (p >= 1) return 1; + const peak = -1 / Math.log(1 - p); + const landed = (n: number) => n * (1 - p) ** n; + const low = Math.max(1, Math.floor(peak)); + return landed(low + 1) > landed(low) ? low + 1 : low; +} diff --git a/src/cli/land-serve.ts b/src/cli/land-serve.ts index 7053035e..5fbcb140 100644 --- a/src/cli/land-serve.ts +++ b/src/cli/land-serve.ts @@ -3,6 +3,7 @@ import { authenticatedRead } from "./land-git"; import { startFixer, startBaseFixer, collectFixers, handbackRepeated } from "./land-fixer"; import { closeFix, fixing, haltNotice, readHalt, resume, saveHalt } from "./land-halt"; import { batchable } from "./land-park"; +import { readPolicy, recordBatch } from "./land-policy"; import { spawn } from "node:child_process"; import { createPrivateKey, createSign } from "node:crypto"; import { @@ -23,7 +24,6 @@ import { setTimeout as sleep } from "node:timers/promises"; import { GitHub, type GitHubDependencies, type Repository, object, string } from "./github"; import { forgetHeads, prepareHead, resolvePreparedQueue, landFallback, finishFallback } from "./land-head"; import { - type BatchPolicy, baseRemote, build, commandEnvironment, @@ -393,27 +393,6 @@ async function mergePinned(github: GitHub, path: string, head: string, previous } } -/** One batch: check once, then squash through GitHub in order, verifying each resulting tree. */ -const POLICY = "batch-policy.json"; - -/** The next batch's order and size; a missing or unreadable record starts oldest-first at full size. */ -export function readPolicy(state: string): BatchPolicy { - try { - const data = JSON.parse(readFileSync(join(state, POLICY), "utf8")) as { order?: unknown; size?: unknown }; - const order = data.order === "newest" ? "newest" : "oldest"; - const size = typeof data.size === "number" && data.size >= 1 ? Math.floor(data.size) : Number.POSITIVE_INFINITY; - return { order, size }; - } catch { - return { order: "oldest", size: Number.POSITIVE_INFINITY }; - } -} - -/** After a checked batch of `checked` PRs: flip the order, halve on failure, and grow by one on success. */ -function nextPolicy(state: string, policy: BatchPolicy, checked: number, passed: boolean): void { - const size = passed ? checked + 1 : Math.max(1, Math.floor(checked / 2)); - save(join(state, POLICY), { order: policy.order === "oldest" ? "newest" : "oldest", size }); -} - /** What a batch may do while its check runs. */ export interface BatchOptions extends Pick { /** Collect finished fixer jobs while a check runs, so their PRs join the next batch at once. */ @@ -518,6 +497,7 @@ export async function checkBase( return false; } +/** One batch: check once, then squash through GitHub in order, verifying each resulting tree. */ export async function serveBatch( root: string, plan: Plan, @@ -581,27 +561,27 @@ export async function serveBatch( ); } output.push(...(await reportOverlaps(github, plan, options.state, chain.overlaps))); - const conflict = chain.conflict; - if (!conflict && chain.links.length === 0) return output; - const log = join(options.state, `batch-${Date.now()}-${batch.queue[0].number}.log`); - if (conflict) { - writeFileSync(log, `Conflicts with the base: ${conflict.files.join(", ")}\n`); + // A PR that conflicts with the base is parked for the fixer, and the batch went on without it. + for (const { entry, files } of chain.conflicts) { + const log = join(options.state, `batch-${Date.now()}-${entry.number}.log`); + writeFileSync(log, `Conflicts with the base: ${files.join(", ")}\n`, { mode: 0o600 }); output.push( ...(await fail(root, options, deps, { plan: batch, - tip: conflict.entry.head, + tip: entry.head, log, - entries: [conflict.entry], - blame: [conflict.entry.number], + entries: [entry], + blame: [entry.number], reason: "merge_conflict", detail: excerpt(readFileSync(log, "utf8")), })), ); - return output; } + if (chain.links.length === 0) return output; + const log = join(options.state, `batch-${Date.now()}-${chain.links[0].entry.number}.log`); const tip = chain.links.at(-1)!; const code = await runSandbox(root, options, tip.commit, chain.links.length, plan.base.sha, log); - nextPolicy(options.state, policy, chain.links.length, code === 0); + recordBatch(options.state, policy, { size: chain.links.length, passed: code === 0 }); if (code !== 0) { // The fixer reads the failure first and says which PR broke the batch; until then, all of them // sit out. Without a fixer, the whole batch is ejected. diff --git a/src/cli/land.ts b/src/cli/land.ts index 56a9c2de..08fe7b08 100644 --- a/src/cli/land.ts +++ b/src/cli/land.ts @@ -15,6 +15,7 @@ import { requirements, string, } from "./github"; +import type { BatchPolicy } from "./land-policy"; import { classifyCheck } from "./status"; import { committedSources, snapshot } from "./tree"; @@ -470,7 +471,7 @@ interface Link { tree: string; } -/** The oldest PR conflicts with the base. Conflicting additions are skipped. */ +/** A PR that conflicts with the base, or, in an overlap report, with a PR queued ahead of it. */ interface Conflict { entry: Entry; files: string[]; @@ -491,32 +492,24 @@ export interface Overlap { } /** - * A bounded batch of squash commits, or a conflict on its base PR. Rejected PRs are left out of - * it; overlaps only inform. + * A bounded batch of squash commits. PRs that conflicted with the base before the batch had a base + * PR, and rejected PRs, are left out of it; overlaps only inform. */ interface Chain { links: Link[]; - conflict?: Conflict; + conflicts: Conflict[]; rejected: Rejection[]; overlaps: Overlap[]; } -/** - * How the next batch is chosen. Batches alternate between the oldest and the newest queued PR as their base, and a - * failed batch halves the next one's size, which each landed batch grows back toward 1 + floor(log2 n). - */ -export interface BatchPolicy { - order: "oldest" | "newest"; - size: number; -} - /** * Fetch the base and every queued head, then fill a batch in `policy`'s order in a scratch * worktree inside the git directory, resolving quest files with `quest resolve`. Each candidate * is first squashed alone onto the base: one that fails `quest check` there with findings the * base lacks is rejected, and a fresh one is also squashed onto the PRs landing soonest ahead of - * it, to report which ones it conflicts with. Stop at the policy's batch limit, skipping - * conflicting additions; a conflicting base stops the batch. + * it, to report which ones it conflicts with. Stop at the policy's batch limit, which is + * at most 1 + floor(log2 n), skipping conflicting additions. A PR reached while the batch has no + * base PR yet that conflicts with the base is set aside for the caller, and the next one is tried. */ export async function build( root: string, @@ -534,7 +527,7 @@ export async function build( ...plan.queue.map((entry) => entry.head), ]); return mergeWorktree(root, plan.base.sha, (dir, driver) => { - const chain: Chain = { links: [], rejected: [], overlaps: [] }; + const chain: Chain = { links: [], conflicts: [], rejected: [], overlaps: [] }; const ordered = policy.order === "newest" ? [...plan.queue].reverse() : plan.queue; const limit = Math.max(1, Math.min(policy.size, 1 + Math.floor(Math.log2(plan.queue.length)))); const screening = screener(dir, driver, plan); @@ -546,8 +539,8 @@ export async function build( if (alone === null) { if (chain.links.length > 0) continue; git(dir, ["reset", "--quiet", "--hard", plan.base.sha]); - chain.conflict = { entry, files: squash(dir, driver, entry)! }; - break; + chain.conflicts.push({ entry, files: squash(dir, driver, entry)! }); + continue; } if (alone.findings.length > 0) { chain.rejected.push({ entry, commit: alone.commit, findings: alone.findings.map(formatFinding) }); diff --git a/src/cli/main.ts b/src/cli/main.ts index ff5454bc..9fd06e72 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -554,7 +554,7 @@ export function main(argv: string[], out: Output): number | Promise { landCommand .command("serve") .description( - "Run one daemon per repository. Every squash, and every base merged into a PR head, drops each list entry linking to a quest the base deleted since the PR branched, as quest rm would. As a batch is filled, park alone for the fixer (eject alone, without one) each PR it reaches whose squash onto the base fails quest check with findings the base lacks, and comment once on each newly queued PR that conflicts with the four PRs landing soonest ahead of it. Check a base PR plus up to floor(log2 n) cleanly merging PRs once through the sandbox, alternating the oldest and the newest queued PR as the base. A failed batch is never bisected: its PRs are parked for the fixer, which blames the ones at fault, or ejected together without a fixer. It halves the next batch's size down to one PR; each landed batch grows it back by one. When the fixer blames none and the base fails its check alone, the queue halts until a moved base passes, and the fixer proposes a fix of the base as a PR. Requires quest on PATH and flock. Sandbox and fixer wrappers MUST run code as a different user from the App key owner (for example in nspawn containers); environment filtering alone cannot isolate /proc or host files. The sandbox receives , with the base also in QUEST_LAND_BASE and present in the standalone shallow clone; the optional tracked background fixer receives the log path followed by PR numbers, or none for a base that fails alone. Neither receives credentials. State retains logs, fixer jobs, and one mechanical attempt per maintainer pick across restarts. Fixer results are validated, lease-pushed, recorded in Review, and pinned for requeue, and the comment a result carries for its PR is posted there; invalid results, crashes, and timeouts are handed back. A validated fix whose push GitHub refuses is handed back with its resolution for `quest land apply`. After revalidating the queue, merge the base into each behind-base PR head with quest resolve, verify the tested tree, and lease-push that head before the pinned GitHub squash API merge. A permissions-refused branch push falls back to a leased fast-forward of the tested squash to the base, closing the PR with a linked-commit comment and removing quest:merge. A recorded daemon merge atop the original pin is recognized after restart; other moved heads require maintainer requeueing. Pending mergeability, and GitHub still showing the pin after the daemon's own push, are retried briefly; a confirmed conflict parks only that PR for the fixer and a moved head ejects only that PR, and the rest of the batch stays queued. A refusal that would repeat, such as squash being disabled, ejects as merge_refused without the fixer. An unexpected squash parent rebuilds the queue without ejecting; a tree mismatch logs CRITICAL, ejects only a PR whose prepared tree differs as tree_mismatch (an already squashed PR stays merged), and leaves the rest of the batch queued to be rebuilt on the new base. Merged PRs drop out of the queue after restart.", + "Run one daemon per repository. Every squash, and every base merged into a PR head, drops each list entry linking to a quest the base deleted since the PR branched, as quest rm would. As a batch is filled, park alone for the fixer (eject alone, without one) each PR it reaches whose squash onto the base fails quest check with findings the base lacks, and comment once on each newly queued PR that conflicts with the four PRs landing soonest ahead of it. A PR that would be the batch's base but conflicts with the base branch is parked alone for the fixer (ejected alone, without one), and the batch starts from the next PR in the same round. Check a base PR plus up to floor(log2 n) cleanly merging PRs once through the sandbox, alternating the oldest and the newest queued PR as the base. A failed batch is never bisected: its PRs are parked for the fixer, which blames the ones at fault, or ejected together without a fixer. The next batch's size lands the most PRs per check, n(1 - p)^n, at the per-PR failure rate p measured over the last 30 checked batches. When the fixer blames none and the base fails its check alone, the queue halts until a moved base passes, and the fixer proposes a fix of the base as a PR. Requires quest on PATH and flock. Sandbox and fixer wrappers MUST run code as a different user from the App key owner (for example in nspawn containers); environment filtering alone cannot isolate /proc or host files. The sandbox receives , with the base also in QUEST_LAND_BASE and present in the standalone shallow clone; the optional tracked background fixer receives the log path followed by PR numbers, or none for a base that fails alone. Neither receives credentials. State retains logs, fixer jobs, and one mechanical attempt per maintainer pick across restarts. Fixer results are validated, lease-pushed, recorded in Review, and pinned for requeue, and the comment a result carries for its PR is posted there; invalid results, crashes, and timeouts are handed back. A validated fix whose push GitHub refuses is handed back with its resolution for `quest land apply`. After revalidating the queue, merge the base into each behind-base PR head with quest resolve, verify the tested tree, and lease-push that head before the pinned GitHub squash API merge. A permissions-refused branch push falls back to a leased fast-forward of the tested squash to the base, closing the PR with a linked-commit comment and removing quest:merge. A recorded daemon merge atop the original pin is recognized after restart; other moved heads require maintainer requeueing. Pending mergeability, and GitHub still showing the pin after the daemon's own push, are retried briefly; a confirmed conflict parks only that PR for the fixer and a moved head ejects only that PR, and the rest of the batch stays queued. A refusal that would repeat, such as squash being disabled, ejects as merge_refused without the fixer. An unexpected squash parent rebuilds the queue without ejecting; a tree mismatch logs CRITICAL, ejects only a PR whose prepared tree differs as tree_mismatch (an already squashed PR stays merged), and leaves the rest of the batch queued to be rebuilt on the new base. Merged PRs drop out of the queue after restart.", ) .option("--base ", "Base branch (default: repository default)") .option("--state ", "Private daemon state directory (default: $STATE_DIRECTORY or the XDG state home)") diff --git a/test/land.test.ts b/test/land.test.ts index b31cf584..032c05f2 100644 --- a/test/land.test.ts +++ b/test/land.test.ts @@ -60,11 +60,11 @@ import { keyPath, lockCheckout, lockRepository, - readPolicy, resumeFailure, serveBatch, STALLED_AFTER, } from "../src/cli/land-serve"; +import { HISTORY, batchSize, failureRate, readPolicy, recordBatch } from "../src/cli/land-policy"; import { createHash, generateKeyPairSync } from "node:crypto"; import { GIT_SETTINGS, configure, initRepository, template, tempDir } from "./fixture"; @@ -1162,9 +1162,9 @@ describe.skipIf(process.platform === "win32")("host daemon", () => { expect(await numbers("oldest", 2)).toEqual([1, 2]); }); - test("alternates the batch order, halving after a failure and growing after a pass", async () => { + test("alternates the batch order and sizes it from the measured failure rate", async () => { const forge = new Forge(); - for (let i = 0; i < 5; i++) forge.pull(`PR ${i}`, { [`${i}.txt`]: "ok" }); + for (let i = 0; i < 7; i++) forge.pull(`PR ${i}`, { [`${i}.txt`]: "ok" }); const failing = sandbox("exit 1"); const round = async (options: ReturnType) => { const deps = await forge.deps(); @@ -1173,15 +1173,72 @@ describe.skipIf(process.platform === "win32")("host daemon", () => { forge.settle(); return output.map((line) => line.split(" ").slice(0, 2).join(" ")); }; - // Oldest-first at the full 1 + floor(log2 5): three fail together, and the next batch halves. + const F = (size: number) => ({ size, passed: false }); + const T = (size: number) => ({ size, passed: true }); + // Oldest-first at the full 1 + floor(log2 7): three fail together, and with only failures measured, the next is one PR. expect(await round(failing)).toEqual(["ejected 1", "ejected 2", "ejected 3"]); - expect(readPolicy(failing.state)).toEqual({ order: "newest", size: 1 }); - // Newest-first at one. - expect(await round(failing)).toEqual(["ejected 5"]); - expect(readPolicy(failing.state)).toEqual({ order: "oldest", size: 1 }); - // The same state with a passing check: oldest-first at one lands, and the next grows. - expect(await round({ ...sandbox(), state: failing.state })).toEqual(["landed 4"]); - expect(readPolicy(failing.state)).toEqual({ order: "newest", size: 2 }); + expect(readPolicy(failing.state)).toEqual({ order: "newest", size: 1, history: [F(3)] }); + expect(await round(failing)).toEqual(["ejected 7"]); + expect(readPolicy(failing.state)).toEqual({ order: "oldest", size: 1, history: [F(3), F(1)] }); + // Passing checks lower the measured rate (57%, then 43%) until two PRs land the most per check. + const passing = { ...sandbox(), state: failing.state }; + expect(await round(passing)).toEqual(["landed 4"]); + expect(readPolicy(failing.state)).toEqual({ order: "newest", size: 1, history: [F(3), F(1), T(1)] }); + expect(await round(passing)).toEqual(["landed 6"]); + expect(readPolicy(failing.state)).toEqual({ order: "oldest", size: 2, history: [F(3), F(1), T(1), T(1)] }); + }); + + test("measures the failure rate over the most recent checked batches", () => { + const state = tempDir(); + for (let i = 0; i < HISTORY + 5; i++) + recordBatch(state, readPolicy(state), { size: i < 5 ? 1 : 4, passed: i >= 5 && i % 2 === 0 }); + const { history, size } = readPolicy(state); + expect(history).toHaveLength(HISTORY); + expect(history.every((batch) => batch.size === 4)).toBe(true); + // Half of the four-PR batches fail: 1 - 0.5^(1/4), about 16% per PR, which six-PR batches land the most of. + expect(failureRate(history)).toBeCloseTo(1 - 0.5 ** 0.25, 6); + expect(size).toBe(6); + // A record from before the history reads as its saved size with none measured yet. + writeFileSync(join(state, "batch-policy.json"), JSON.stringify({ order: "newest", size: 2 })); + expect(readPolicy(state)).toEqual({ order: "newest", size: 2, history: [] }); + }); + + test("sizes a batch for the most PRs landed per check", () => { + expect(failureRate([])).toBe(0); + expect(failureRate([{ size: 3, passed: true }])).toBe(0); + expect(failureRate([{ size: 3, passed: false }])).toBe(1); + // With one-PR batches the rate is simply the share that failed. + expect( + failureRate([ + { size: 1, passed: true }, + { size: 1, passed: true }, + { size: 1, passed: false }, + { size: 1, passed: true }, + ]), + ).toBeCloseTo(0.25, 6); + expect(batchSize(0)).toBe(Number.POSITIVE_INFINITY); + expect(batchSize(1)).toBe(1); + // moq's measured 22-24% lands the most at four PRs, where halving after each failure held it at one to three. + expect([0.05, 0.1, 0.15, 0.22, 0.243, 0.3, 0.6].map(batchSize)).toEqual([19, 9, 6, 4, 4, 3, 1]); + }); + + test("sets aside a base PR that conflicts with the base and fills the batch in the same round", async () => { + const forge = new Forge(); + forge.pull("Clash", { "code.txt": "one\nII\nthree\n" }); + forge.pull("Also clash", { "code.txt": "one\nTWO\nthree\n" }); + forge.advance({ "code.txt": "one\n2\nthree\n" }); + forge.pull("Clean", { "clean.txt": "ok" }); + forge.pull("Also clean", { "also.txt": "ok" }); + const { checked, landing } = await forge.round(); + // Both conflicts go to the fixer (ejected here, without one) and the clean PRs behind them are checked at once. + expect(checked).toEqual([[3, 4]]); + expect(landing.map((line) => line.split(" ").slice(0, 2).join(" "))).toEqual([ + "ejected 1", + "ejected 2", + "landed 3", + "landed 4", + ]); + expect(forge.comment(1)).toContain("code.txt"); }); test("skips a conflicting addition and leaves it queued, adding a later clean PR", async () => {