chore(pi): add session memory benchmark

[NathanFlurry]

• Add a reproducible Pi session memory benchmark on top of feat(pi): add pi actor integration #5767, with one prompt and a verified mock LLM response per session.
• Sample runner RSS after every session, including RivetKit NAPI/Rust memory and excluding the Engine subprocess, client, and mock server; isolate storage and clean up the benchmark process group.
• Include the recorded 100-session results, Rivet-styled chart, chart generator, and instructions for rerunning both.
• Validated with a fresh 100-session end-to-end run: 100 distinct sessions, 100 verified responses, 100 mock HTTP requests, and 101 RSS samples. Verified process cleanup, JavaScript syntax/formatting, and chart rendering with website and fallback fonts.

[claude]

Review

Reviewed the diff (6 new files, all additions, scoped entirely to integrations/pi/benchmarks/session-memory/). This is a well-built, self-contained benchmark tool with no changes to load-bearing code.

Strengths

• Uses a real mock LLM server (@copilotkit/aimock) instead of vi.mock, consistent with the repo's "no module-level mocking" testing rule.
• bench.mjs verifies end-to-end correctness (distinct session IDs, exact response text, one HTTP request per session, live-session count after every response) rather than just measuring memory blindly.
• Careful process lifecycle: detached process group for the runner + its Engine subprocess, SIGTERM then SIGKILL fallback on cleanup, ESRCH swallowed correctly, --output directory required to not pre-exist so runs can't silently clobber prior results.
• --sessions bounds-checked (2–10000, integer), platform-checked (process.platform !== "linux") since it depends on /proc/self/smaps_rollup and POSIX process groups.
• The env vars used to configure the local Engine (RIVET_RUN_ENGINE, RIVET_RUN_ENGINE_PORT, RIVET_RUN_SERVICES, RIVETKIT_STORAGE_PATH) match real, currently-used RivetKit env resolution (verified against rivetkit-typescript/packages/rivetkit/src/utils/env-vars.ts and existing tests) — no drift from actual config surface.
• sample() correctly avoids a race by calling nextMessage() before child.send("measure"), so the response listener is armed first.
• Style matches repo conventions: tabs, no em dashes, no shell-string interpolation for child process args (uses fork()/execArgv arrays, so no injection risk even though dir/paths are interpolated into file paths).

Minor / non-blocking notes

• Port selection has a TOCTOU race: a throwaway net.createServer() grabs a free port, closes it, then passes the port number to the forked runner to bind. Between server.close() and the child's bind, another process could theoretically grab that port. Very unlikely to matter for a local dev benchmark, but worth a one-line comment if this ever gets flaky in CI.
• plot.py uses bare assert for data-integrity checks (verified counts, row shape). These are stripped under python -O. Since this is dev/rendering tooling rather than shipped code, low severity, but if this is ever invoked in an automated context where -O could be set, consider explicit raise ValueError(...) instead.
• Checked-in generated artifacts: results/prompted-100.json (2434 lines) and results/prompted-100.svg (814 lines) are committed as a "recorded run" snapshot. The README already caveats that results are build/platform/allocator-dependent and not a performance guarantee, so this seems intentional as documentation rather than a regression baseline — just flagging since CLAUDE.md's asset guidance steers large generated media toward R2 for the website/dashboard case specifically; these are small text-based SVG/JSON tied to a benchmark write-up, which reads as a different, reasonable use case, but worth a maintainer nod that this is the desired convention going forward for future benchmark runs (e.g., do you want to update prompted-100.* in place on every future run, or version them like prompted-100-v2.*?).
• No automated test exists for bench.mjs/plot.py themselves (e.g., a small --sessions 2 smoke test in CI). Understandable for a manual benchmark script, but if this is meant to be run repeatedly to catch memory regressions over time, consider whether a lightweight CI smoke invocation (--sessions 2) would be valuable to keep the harness itself from silently rotting.

Bugs

None found. Traced through the retry loop (route_resolve_query_timeout handling), the nextMessage()/sample() message-race avoidance, and the count/live bookkeeping — all paths that skip setting session also throw before it's used, and the runner/controller branch split via process.argv[2] === "runner" is consistent throughout.

Security

No concerns. All subprocess arguments are passed via array-form fork()/execArgv, not shell strings. Only dummy mock credentials (apiKey: "mock") are used, scoped to a local temp/output directory.

Overall: solid, well-tested addition. Nothing blocking.

[claude]

Review

Reviewed bench.mjs, plot.py, and README.md (skipped the checked-in results/prompted-100.json and .svg data files). This is a self-contained benchmark tool, so most of the repo's Rust/RivetKit-layering conventions don't apply; feedback below is mostly minor.

Nits / suggestions (non-blocking)

1. bench.mjs retry loop masks a known flake instead of fixing it.

   } catch (error) {
     if (error?.code !== "route_resolve_query_timeout" || attempt === 3)
       throw error;
     console.log(`Retrying creation of session ${n} after route timeout`);
   }

   The repo's testing guidance is explicit about not papering over flakes with retry loops — root-causing route_resolve_query_timeout (or documenting why a bounded retry is the correct permanent behavior here, e.g. it's an expected transient race in route resolution rather than a bug) would be preferable to a silent bounded retry. If this is intentional/expected behavior of route resolution under load, a one-line comment explaining why retrying (rather than fixing) is correct would help future readers.

2. Port reservation has a TOCTOU race.

   const server = createServer();
   await new Promise((r) => server.listen(0, "127.0.0.1", r));
   const port = server.address().port;
   await new Promise((r) => server.close(r));

   Binding to port 0 to discover a free port, then closing and handing that port number to the child Engine process, leaves a window where another process could grab the port on a busy CI host, causing an occasional flaky bind failure in the runner. Common pattern for test harnesses, but worth a one-line note if it's a known accepted tradeoff.

3. Misleading import alias in bench.mjs:

   import { setImmediate as nextTick } from "node:timers/promises";

   This is setImmediate, not process.nextTick — they have different scheduling semantics (macrotask vs. microtask-ish). Suggest naming it setImmediateAsync or similar to avoid confusion for anyone reading the GC-settling loop.

4. plot.py's tempfile.TemporaryDirectory() is never explicitly cleaned up — it relies on the finalizer running at GC/process exit rather than a with block or explicit .cleanup(). Low risk given the script's short lifetime, but the explicit form is more robust (e.g. under uv run's process model or if the script is ever imported instead of run standalone).

5. Checked-in benchmark artifacts are sizable (results/prompted-100.json at ~2.4k lines, results/prompted-100.svg at ~815 lines). Not the same category as the dashboard/website media the repo routes through the rivet-assets R2 bucket, so this is likely fine as reproducibility evidence for a one-off benchmark, but worth confirming this is the intended long-term home for recorded benchmark runs rather than growing unbounded in git history as more runs are added later.

What looks solid

• Good end-to-end verification: distinct session IDs, exact response-text match, one mock HTTP request per session, and live-session-count assertions after every sample — this catches real regressions, not just "did it not crash."
• Process-group lifecycle (detached: true, SIGTERM then SIGKILL after a grace period, ESRCH-tolerant signal helper) is handled carefully, including cleanup on SIGINT/SIGTERM to the parent.
• plot.py's assertions (count >= 2, row count, live-sequence) guard against silently plotting malformed/truncated results.
• README is clear, includes exact repro commands, and clearly caveats the recorded numbers as a single dev-machine run rather than a performance guarantee.
• Platform/argument validation up front (--sessions bounds, Linux-only guard for /proc and process groups) fails fast with actionable errors.

No correctness bugs or security concerns found. This is tooling/chore code with no production code paths affected, so the lack of automated tests is appropriate here — reproducibility instructions in the README serve that role.

🤖 Generated with Claude Code