Merge pull request #392 from vercel-labs/fazxes/stabilize-pgso-startup-sampling

Increase PGSO startup sample depth

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.
 

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}"

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

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",

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"

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<void>();
@@ -493,6 +501,9 @@ describe("filesystem path handling", () => {
       writeFileSync(target, fileSentinel + "\n");
 
       const childPrompt = `Read exactly ${target}.`;
+      const childSnapshot = Promise.withResolvers<
+        Awaited<ReturnType<typeof waitForCompletedSubagentChild>>
+      >();
       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();