diff --git a/scripts/pgso/README.md b/scripts/pgso/README.md index e4d4af877..dfeb2a974 100644 --- a/scripts/pgso/README.md +++ b/scripts/pgso/README.md @@ -75,7 +75,7 @@ Candidate behavior qualification records each scenario's debug trace under `cand ## Qualification policy -Startup compares `help`, `--version`, `status --json`, `background --json`, `doctor --json`, and `sessions --json`. It first executes each verified immutable artifact once to require successful output and empty stderr. Timing then uses pinned Hyperfine with no intermediate shell, ten warmups per artifact in each of at least ten rounds, at least 100 measured samples per artifact, and alternating control-then-candidate and candidate-then-control order. No contiguous block exceeds ten measured runs, so larger manual campaigns do not concentrate short machine-noise bursts on one artifact. Startup measurement sets `FX_DISABLE_KEYCHAIN=1` so the compiler comparison cannot be dominated by host-global macOS Keychain subprocess latency; the deterministic behavior corpus remains responsible for exercising Keychain integration. No per-sample Python process management or evidence-file write is included in the timed boundary, and measurement never replaces `zig-out/bin/fx`. Heavy qualification compares file indexing at 100,000 paths, UI activity, and approval transcript, diff, combined, and large-payload workloads. +Startup compares `help`, `--version`, `status --json`, `background --json`, `doctor --json`, and `sessions --json`. It first executes each verified immutable artifact once to require successful output and empty stderr. Timing then uses pinned Hyperfine with no intermediate shell, ten warmups per artifact in each of at least 100 alternating rounds, and at least 1,000 measured samples per artifact. No contiguous block exceeds ten measured runs, so short machine-noise bursts are distributed between control and candidate while p95 retains 50 tail observations. Startup measurement sets `FX_DISABLE_KEYCHAIN=1` so the compiler comparison cannot be dominated by host-global macOS Keychain subprocess latency; the deterministic behavior corpus remains responsible for exercising Keychain integration. No per-sample Python process management or evidence-file write is included in the timed boundary, and measurement never replaces `zig-out/bin/fx`. Heavy qualification compares file indexing at 100,000 paths, UI activity, and approval transcript, diff, combined, and large-payload workloads. Heavy comparisons use at least 50 measured samples for each artifact and alternate pair order AB then BA. Command failures and timeouts fail qualification and are never replaced. A candidate fails when either p50 or p95 is more than 10% slower than its matching control. The existing Linux startup workflow remains the authority for the repository's absolute 2 ms command budget. diff --git a/scripts/pgso/distributed.py b/scripts/pgso/distributed.py index 7926b47ab..9956dedb1 100644 --- a/scripts/pgso/distributed.py +++ b/scripts/pgso/distributed.py @@ -40,6 +40,7 @@ from scripts.pgso.pipeline import ( from scripts.pgso.qualify import ( BENCHMARK_PLANS, STARTUP_COMMANDS, + STARTUP_MINIMUM_SAMPLES, BenchmarkPair, build_profile_linked_benchmarks, measure_heavy_workloads, @@ -341,7 +342,7 @@ def aggregate_measurement_shards( raise PgsoError(f"duplicate {phase} measurement: {name}") if measurement.get("passed") is not True: raise PgsoError(f"{phase} measurement failed: {name}") - required_samples = 100 if phase == "startup" else 50 + required_samples = STARTUP_MINIMUM_SAMPLES if phase == "startup" else 50 if measurement.get("requested_samples") != required_samples: raise PgsoError( f"{phase} measurement requires exactly {required_samples} samples: {name}" diff --git a/scripts/pgso/qualify.py b/scripts/pgso/qualify.py index 350fa87c9..b82c3760b 100644 --- a/scripts/pgso/qualify.py +++ b/scripts/pgso/qualify.py @@ -35,7 +35,7 @@ from scripts.pgso.toolchain import Toolchain MINIMUM_SAMPLES = 50 -STARTUP_MINIMUM_SAMPLES = 100 +STARTUP_MINIMUM_SAMPLES = 1_000 STARTUP_WARMUP_RUNS = 10 STARTUP_MINIMUM_ROUNDS = 10 STARTUP_MAX_RUNS_PER_ROUND = 10 diff --git a/scripts/pgso/tests/test_distributed.py b/scripts/pgso/tests/test_distributed.py index de225682e..82de674d9 100644 --- a/scripts/pgso/tests/test_distributed.py +++ b/scripts/pgso/tests/test_distributed.py @@ -354,15 +354,15 @@ class ShardAggregationTests(unittest.TestCase): "measurement": { "name": name, "argv": ["help"], - "requested_samples": 100, - "control_samples": [1.0] * 100, - "candidate_samples": [0.9] * 100, + "requested_samples": 1_000, + "control_samples": [1.0] * 1_000, + "candidate_samples": [0.9] * 1_000, "control_failures": 0, "candidate_failures": 0, "errors": [], "comparison": { - "control_samples": [1.0] * 100, - "candidate_samples": [0.9] * 100, + "control_samples": [1.0] * 1_000, + "candidate_samples": [0.9] * 1_000, "control_p50": 1.0, "control_p95": 1.0, "candidate_p50": 0.9, @@ -440,10 +440,10 @@ class ShardAggregationTests(unittest.TestCase): def test_measurement_aggregate_rejects_truncated_samples(self) -> None: document = self.measurement("startup-help") measurement = dict(document["measurement"]) - measurement["candidate_samples"] = [0.9] * 99 + measurement["candidate_samples"] = [0.9] * 999 document["measurement"] = measurement - with self.assertRaisesRegex(PgsoError, "requires exactly 100 samples"): + with self.assertRaisesRegex(PgsoError, "requires exactly 1000 samples"): aggregate_measurement_shards( (document,), phase="startup", diff --git a/scripts/pgso/tests/test_qualify.py b/scripts/pgso/tests/test_qualify.py index 820d96895..c8a1fe96b 100644 --- a/scripts/pgso/tests/test_qualify.py +++ b/scripts/pgso/tests/test_qualify.py @@ -279,9 +279,9 @@ class PgsoQualificationTests(unittest.TestCase): ) self.assertEqual(("startup-doctor",), tuple(item.name for item in results)) - self.assertEqual(10, sum(command[0] == str(hyperfine) for command in calls)) + self.assertEqual(100, sum(command[0] == str(hyperfine) for command in calls)) - def test_startup_measurement_balances_warmed_hyperfine_rounds(self) -> None: + def test_startup_measurement_uses_one_thousand_samples_in_balanced_blocks(self) -> None: control = self.root / "control" / "fx" candidate = self.root / "candidate" / "fx" hyperfine = self.root / "tools" / "hyperfine" @@ -308,9 +308,9 @@ class PgsoQualificationTests(unittest.TestCase): hyperfine_calls = [ command for command in calls if command[0] == str(hyperfine) ] - self.assertEqual(60, len(hyperfine_calls)) - for command_start in range(0, len(hyperfine_calls), 10): - command_rounds = hyperfine_calls[command_start : command_start + 10] + self.assertEqual(600, len(hyperfine_calls)) + for command_start in range(0, len(hyperfine_calls), 100): + command_rounds = hyperfine_calls[command_start : command_start + 100] for round_index, command in enumerate(command_rounds): self.assertIn("--shell=none", command) self.assertEqual("10", command[command.index("--warmup") + 1]) @@ -328,9 +328,9 @@ class PgsoQualificationTests(unittest.TestCase): if value == "--command-name" ], ) - self.assertTrue(all(result.requested_samples == 100 for result in results)) - self.assertTrue(all(len(result.control_samples) == 100 for result in results)) - self.assertTrue(all(len(result.candidate_samples) == 100 for result in results)) + self.assertTrue(all(result.requested_samples == 1_000 for result in results)) + self.assertTrue(all(len(result.control_samples) == 1_000 for result in results)) + self.assertTrue(all(len(result.candidate_samples) == 1_000 for result in results)) def test_startup_measurement_caps_large_campaign_blocks_at_ten_runs(self) -> None: control = self.root / "control" / "fx" diff --git a/tests/e2e/file-tool-paths.test.ts b/tests/e2e/file-tool-paths.test.ts index 5fde1977f..34ace438e 100644 --- a/tests/e2e/file-tool-paths.test.ts +++ b/tests/e2e/file-tool-paths.test.ts @@ -242,13 +242,10 @@ type SubagentControlRecord = { type SubagentToolResult = { tool_name: string; status: string; output: string }; type SubagentTurn = { - kind: string; - completed_tool_names?: string[]; execution?: { tool_steps?: Array<{ tool_results?: SubagentToolResult[] }> }; }; -// Interrupted and completed child turns persist tool outcomes in different fields. -function readSubagentChild(home: string) { +function readSubagentChildIfPresent(home: string) { const sessionsDir = join(home, ".fx", "sessions"); const children = readdirSync(sessionsDir) .map((entry) => join(sessionsDir, entry)) @@ -260,10 +257,11 @@ function readSubagentChild(home: string) { history: readFileSync(join(dir, "events.jsonl"), "utf8"), })) .filter(({ control }) => !!control.parent_id); - if (children.length !== 1) { + if (children.length > 1) { throw new Error(`expected one persisted child record, found ${children.length}`); } - const child = children[0]!; + const child = children[0]; + if (!child) return null; const turns = child.history .split("\n") .filter((line) => line.length > 0) @@ -281,12 +279,22 @@ function readSubagentChild(home: string) { ); return { ...child, - interrupted: turns.some((turn) => turn.kind === "interrupted"), - completedToolNames: turns.flatMap((turn) => turn.completed_tool_names ?? []), readResult: toolResults.find((result) => result.tool_name === "read_file"), }; } +async function waitForCompletedSubagentChild(home: string, deadlineMs: number) { + const deadline = Date.now() + deadlineMs; + while (Date.now() < deadline) { + const child = readSubagentChildIfPresent(home); + if (child?.control.state === "completed" && child.readResult) { + return child; + } + await Bun.sleep(10); + } + throw new Error("timed out waiting for completed persisted child record"); +} + // Hold the parent open until the child read completes; the deadline prevents hangs. function createChildReadGate(deadlineMs: number) { const { promise: opened, resolve: release } = Promise.withResolvers(); @@ -493,6 +501,9 @@ describe("filesystem path handling", () => { writeFileSync(target, fileSentinel + "\n"); const childPrompt = `Read exactly ${target}.`; + const childSnapshot = Promise.withResolvers< + Awaited> + >(); const isChildTurn = (body: string) => body.includes(childPrompt) && !body.includes("parent_create_1"); const gate = createChildReadGate(8_000); @@ -508,6 +519,9 @@ describe("filesystem path handling", () => { }); } await gate.opened; + childSnapshot.resolve( + await waitForCompletedSubagentChild(root.home, TIMEOUT), + ); return finalText("Parent received the admitted child handle."); }; const gateway = startFakeGateway([ @@ -572,7 +586,7 @@ describe("filesystem path handling", () => { expect(request.body).toContain('"name":"read_file"'); } - const child = readSubagentChild(root.home); + const child = await childSnapshot.promise; expect(child.control.configuration.name).toBe("added-root-reader"); expect(child.control.mode).toBe("one_off"); expect(child.control.queue.some((item) => item.content.includes(target))).toBe( @@ -581,15 +595,12 @@ describe("filesystem path handling", () => { expect(child.control.events.some((event) => event.current === "running")).toBe( true, ); - expect(["interrupted", "completed"]).toContain(child.control.state); + expect(child.control.state).toBe("completed"); expect(child.history).not.toContain(instructionSentinel); expect(child.readResult).toBeDefined(); expect(child.readResult!.status).toBe("success"); expect(child.readResult!.output).toContain(fileSentinel); - if (child.interrupted) { - expect(child.completedToolNames).toContain("read_file"); - } } finally { gate.dispose(); gateway.stop();