Address PR review comments on fair benchmark files

- Fix round-trip count: "4 SQL/ORM round-trips" to "5" in doc comment
  and FAIR-BENCHMARK.md table (BEGIN+SELECT+UPDATE+UPDATE+COMMIT = 5)
- Fix getAccount() balance type: annotate as string (from JSON) and
  parse with BigInt() to match the RpcConnector interface
- Fix amount precision loss in rpcTransfer: parse directly to BigInt
  instead of going through Number() which truncates values > 2^53
- Remove misleading --pipeline-depth flag and related claims; the fair
  benchmark runs sequentially (non-pipelined) so the setting had no
  effect
- Add CLI numeric arg validation: reject NaN, Infinity, and <= 0
- Fix bare catch on dynamic import: only fall back to rpc_single_call
  for MODULE_NOT_FOUND errors, rethrow genuine errors

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: zalo

templates/keynote-2/FAIR-BENCHMARK.md

@@ -9,17 +9,17 @@ This is an alternative benchmark configuration that levels the playing field bet
 | **SpacetimeDB client** | Custom Rust client with 16,384 in-flight ops | Same TypeScript client as everyone else |
 | **TPS counting** | Server-side Prometheus metrics (fire-and-forget) | Client-side round-trip counting for ALL systems |
 | **Durability** | `confirmedReads=false` (no durability guarantee) | `confirmedReads=true` (durable commits, like Postgres fsync) |
-| **Pipeline depth** | 16,384 for SpacetimeDB vs 8 for competitors | Same for all (configurable, default 8) |
+| **Concurrency model** | 16,384 in-flight for SpacetimeDB vs 8 for competitors | Sequential (non-pipelined) for all systems |
 | **Postgres isolation** | `serializable` (non-default, worst-case for contention) | `read_committed` (Postgres actual default) |
 | **Postgres sync commit** | `synchronous_commit=off` | `synchronous_commit=on` (matches SpacetimeDB confirmed reads) |
-| **Postgres transfer** | 4 ORM round-trips via Drizzle | Also tested with stored procedure (single DB call) |
+| **Postgres transfer** | 5 ORM round-trips via Drizzle | Also tested with stored procedure (single DB call) |
 | **Warmup** | 5s warmup for Rust client only | No warmup for any system (equal cold start) |
 
 ## Why These Changes Matter
 
 ### 1. Same Client Language (TypeScript for All)
 
-The original benchmark uses a hand-tuned **Rust client** for SpacetimeDB that sends 16,384 concurrent operations per connection via binary WebSocket, while all competitors use a TypeScript client with HTTP/JSON and 8 in-flight operations. This alone is a ~2000x difference in pipeline depth.
+The original benchmark uses a hand-tuned **Rust client** for SpacetimeDB that sends 16,384 concurrent operations per connection via binary WebSocket, while all competitors use a TypeScript client with HTTP/JSON and 8 in-flight operations. This alone is a ~2000x difference in concurrency per connection.
 
 The README justifies this by saying "we were bottlenecked on our test TypeScript client" — but then no competitor gets the same optimization. A fair comparison uses the same client for all.
 
@@ -79,9 +79,6 @@ npm run fair-bench -- --alpha 1.5
 # Include more systems
 npm run fair-bench -- --systems spacetimedb,postgres_rpc,postgres_storedproc_rpc,sqlite_rpc
 
-# Custom pipeline depth
-npm run fair-bench -- --pipeline-depth 16
-
 # Skip seeding (if already seeded)
 npm run fair-bench -- --skip-prep
 ```
@@ -103,7 +100,7 @@ With a leveled playing field, SpacetimeDB's genuine architectural advantage (col
 
 The factors that are **not** architectural and were removed:
 - Custom Rust client vs shared TypeScript client
-- 16,384 vs 8 pipeline depth
+- 16,384 vs 8 in-flight operations per connection
 - Server-side vs client-side TPS counting
 - Unequal durability guarantees
 - Non-default Postgres isolation level penalizing competitors

templates/keynote-2/src/connectors/rpc/postgres_storedproc_rpc.ts

@@ -64,13 +64,13 @@ export default function postgres_storedproc_rpc(
     async getAccount(id: number) {
       const result = (await httpCall('getAccount', { id })) as {
         id: number;
-        balance: bigint;
+        balance: string;
       } | null;
 
       if (!result) return null;
       return {
         id: result.id,
-        balance: result.balance,
+        balance: BigInt(result.balance),
       };
     },
 

templates/keynote-2/src/fair-bench.ts

@@ -6,7 +6,7 @@
  * 1. Using the TypeScript client for ALL systems (no custom Rust client)
  * 2. Forcing STDB_CONFIRMED_READS=1 (durable commits, like Postgres with fsync)
  * 3. Forcing USE_SPACETIME_METRICS_ENDPOINT=0 (client-side TPS counting for all)
- * 4. Using the same pipeline depth for all systems
+ * 4. Sequential (non-pipelined) operations for all systems
  * 5. Including postgres_storedproc_rpc (stored procedure, eliminates ORM overhead)
  * 6. Using read_committed isolation for Postgres (its actual default)
  *
@@ -59,8 +59,12 @@ function getArg(name: string, defaultValue: number): number {
   );
   if (idx === -1) return defaultValue;
   const arg = args[idx];
-  if (arg.includes('=')) return Number(arg.split('=')[1]);
-  return Number(args[idx + 1] ?? defaultValue);
+  const raw = arg.includes('=') ? arg.split('=')[1] : args[idx + 1];
+  const value = Number(raw ?? defaultValue);
+  if (!Number.isFinite(value) || value <= 0) {
+    throw new Error(`Invalid value for --${name}: "${raw}" (must be a finite number > 0)`);
+  }
+  return value;
 }
 
 function getStringArg(name: string, defaultValue: string): string {
@@ -86,15 +90,11 @@ const systems = getStringArg(
 )
   .split(',')
   .map((s) => s.trim());
-const pipelineDepth = getArg('pipeline-depth', 8);
 const skipPrep = hasFlag('skip-prep');
 
 const accounts = Number(process.env.SEED_ACCOUNTS ?? 100_000);
 const initialBalance = Number(process.env.SEED_INITIAL_BALANCE ?? 10_000_000);
 
-// Set the same pipeline depth for all systems
-process.env.MAX_INFLIGHT_PER_WORKER = String(pipelineDepth);
-
 // ============================================================================
 // ANSI Colors
 // ============================================================================
@@ -228,10 +228,14 @@ async function runBenchmark(system: string): Promise<BenchResult | null> {
   try {
     const testMod = await import(`./tests/test-1/${system}.ts`);
     scenario = testMod.default.run;
-  } catch {
-    // Fallback to rpc_single_call for RPC-based systems
-    const { rpc_single_call } = await import('./scenario_recipes/rpc_single_call.ts');
-    scenario = rpc_single_call;
+  } catch (err: any) {
+    // Only fall back for missing modules; rethrow genuine errors
+    if (err?.code === 'ERR_MODULE_NOT_FOUND' || err?.code === 'MODULE_NOT_FOUND') {
+      const { rpc_single_call } = await import('./scenario_recipes/rpc_single_call.ts');
+      scenario = rpc_single_call;
+    } else {
+      throw err;
+    }
   }
 
   const result = await runOne({
@@ -276,15 +280,14 @@ async function main() {
   console.log(`    Duration:          ${seconds}s`);
   console.log(`    Concurrency:       ${concurrency} connections`);
   console.log(`    Alpha (contention): ${alpha}`);
-  console.log(`    Pipeline depth:    ${pipelineDepth} per worker (same for all)`);
   console.log(`    Systems:           ${systems.join(', ')}`);
   console.log('');
 
   console.log(c('bold', '  Fairness guarantees:'));
   console.log(`    ${c('green', '\u2713')} TypeScript client for ALL systems (no custom Rust client)`);
   console.log(`    ${c('green', '\u2713')} STDB_CONFIRMED_READS=1 (durable commits)`);
   console.log(`    ${c('green', '\u2713')} Round-trip-awaited operations for ALL systems`);
-  console.log(`    ${c('green', '\u2713')} Same pipeline depth (${pipelineDepth}) for all`);
+  console.log(`    ${c('green', '\u2713')} Sequential (non-pipelined) operations for all systems`);
   console.log(`    ${c('green', '\u2713')} Postgres: read_committed isolation (actual default)`);
   console.log(`    ${c('green', '\u2713')} Postgres: synchronous_commit=on`);
   console.log('');
@@ -381,11 +384,11 @@ async function main() {
             confirmed_reads: true,
             metrics_endpoint: false,
             client: 'typescript (same for all)',
-            pipeline_depth: pipelineDepth,
+            pipelined: false,
             postgres_isolation: 'read_committed',
             postgres_synchronous_commit: 'on',
           },
-          config: { seconds, concurrency, alpha, accounts, pipelineDepth },
+          config: { seconds, concurrency, alpha, accounts },
           results: results.map((r) => ({
             system: r.system,
             tps: r.tps,

templates/keynote-2/src/rpc-servers/postgres-storedproc-rpc-server.ts

@@ -8,7 +8,7 @@ import { poolMaxFromEnv } from '../helpers.ts';
  * Fair benchmark variant: Postgres RPC server using a stored procedure
  * for the transfer operation instead of Drizzle ORM with multiple round-trips.
  *
- * This eliminates the ORM overhead and reduces the transfer from 4 SQL
+ * This eliminates the ORM overhead and reduces the transfer from 5 SQL
  * round-trips (BEGIN + SELECT FOR UPDATE + UPDATE + UPDATE + COMMIT via Drizzle)
  * to a single SQL call (SELECT do_transfer(...)).
  *
@@ -78,19 +78,25 @@ async function ensureStoredProcedure() {
 async function rpcTransfer(args: Record<string, unknown>) {
   const fromId = Number(args.from_id ?? args.from);
   const toId = Number(args.to_id ?? args.to);
-  const amount = Number(args.amount);
 
-  if (
-    !Number.isInteger(fromId) ||
-    !Number.isInteger(toId) ||
-    !Number.isFinite(amount)
-  ) {
+  if (!Number.isInteger(fromId) || !Number.isInteger(toId)) {
     throw new Error('invalid transfer args');
   }
-  if (fromId === toId || amount <= 0) return;
+
+  // Parse amount directly to BigInt to avoid precision loss for large values.
+  // Accepts string, number, or bigint input from JSON.
+  let amount: bigint;
+  try {
+    const raw = args.amount;
+    amount = typeof raw === 'bigint' ? raw : BigInt(raw as string | number);
+  } catch {
+    throw new Error('invalid transfer args: amount is not a valid integer');
+  }
+
+  if (fromId === toId || amount <= 0n) return;
 
   // Single database call - no ORM, no multiple round-trips
-  await pool.query('SELECT do_transfer($1, $2, $3)', [fromId, toId, BigInt(amount)]);
+  await pool.query('SELECT do_transfer($1, $2, $3)', [fromId, toId, amount]);
 }
 
 async function rpcGetAccount(args: Record<string, unknown>) {