fix(remote): publish production statistics from the sync that dumped them

src/remote/publish-stats-on-sync.test.ts

@@ -0,0 +1,50 @@
+import { expect, test } from "vitest";
+import type { ExportedStats } from "@query-doctor/core";
+import { Connectable } from "../sync/connectable.ts";
+import { ConnectionManager } from "../sync/connection-manager.ts";
+import { Remote } from "./remote.ts";
+import { assertDefined, testSpawnTarget } from "./test-utils.ts";
+
+/**
+ * A project publishes its production statistics because its analyzer synced,
+ * not because someone went and asked it to.
+ *
+ * The drift check is what pushes a re-dump, and it cannot run without a
+ * baseline. The baseline was set only by a push, so an analyzer that had never
+ * pushed never would: no snapshot on the server, nothing to seed from, no
+ * baseline, no dump, no snapshot.
+ */
+test("a sync publishes the statistics it dumped from the source", async () => {
+  const [sourceDb, targetDb] = await Promise.all([
+    // 200 rows: far under STATS_ROWS_THRESHOLD, which is the case that used to
+    // publish nothing at all.
+    testSpawnTarget({
+      content: `
+        create table items(id int primary key, sockets int);
+        insert into items select g, g % 6 from generate_series(1, 200) g;
+        analyze items;
+      `,
+    }),
+    testSpawnTarget(),
+  ]);
+
+  try {
+    await using remote = new Remote(
+      Connectable.fromString(targetDb.getConnectionUri()),
+      ConnectionManager.forLocalDatabase(),
+    );
+    const published: ExportedStats[][] = [];
+    remote.on("statsApplied", (stats) => published.push(stats));
+
+    await remote.syncFrom(Connectable.fromString(sourceDb.getConnectionUri()));
+
+    expect(published).toHaveLength(1);
+    const items = published[0].find((t) => t.tableName === "items");
+    assertDefined(items, "expected the published dump to cover items");
+    // Production's own number, not the 10M-row assumption the optimizer costs
+    // a database this small against. What we publish is what we measured.
+    expect(items.reltuples).toBe(200);
+  } finally {
+    await Promise.all([sourceDb.stop(), targetDb.stop()]);
+  }
+});

src/remote/remote.test.ts

@@ -6,33 +6,13 @@ import { RemoteController } from "./remote-controller.ts";
 import { Pool } from "pg";
 
 import { ConnectionManager } from "../sync/connection-manager.ts";
-import { normalizeQuery } from "./test-utils.ts";
+import { normalizeQuery, testSpawnTarget } from "./test-utils.ts";
 
 import { type Op } from "jsondiffpatch/formatters/jsonpatch";
 
-const TEST_TARGET_CONTAINER_NAME = "postgres:17";
 const TEST_TARGET_CONTAINER_TIMESCALEDB_NAME =
   "timescale/timescaledb:latest-pg17";
 
-export function testSpawnTarget(
-  options: { content?: string; containerName?: string } = {
-    containerName: TEST_TARGET_CONTAINER_NAME,
-  },
-) {
-  let pg = new PostgreSqlContainer(
-    options.containerName ?? TEST_TARGET_CONTAINER_NAME,
-  );
-  if (options.content) {
-    pg = pg.withCopyContentToContainer([
-      {
-        content: options.content,
-        target: "/docker-entrypoint-initdb.d/init.sql",
-      },
-    ]);
-  }
-  return pg.start();
-}
-
 function assertOk<T>(
   result: { type: string; value?: T },
 ): asserts result is { type: "ok"; value: T } {

src/remote/remote.ts

@@ -239,6 +239,12 @@ export class Remote extends EventEmitter<RemoteEvents> {
       fullSchema.status === "fulfilled" ? fullSchema.value : undefined,
     );
 
+    // After `optimizer.start`, so a failed start doesn't leave a baseline armed
+    // for a drift check that will never run.
+    if (statsResult.dump) {
+      this.recordSourceStatistics(statsResult.dump);
+    }
+
     return {
       meta: {
         version: databaseInfo.status === "fulfilled"
@@ -394,30 +400,47 @@ export class Remote extends EventEmitter<RemoteEvents> {
     tables: FullSchemaTable[],
   ): Promise<StatsResult> {
     const connector = this.sourceManager.getConnectorFor(source);
-    const totalRows = await connector.getTotalRowCount(tables);
+
+    // Dump either way. The threshold decides what the optimizer plans against,
+    // which is a costing decision: on two hundred rows a sequential scan is the
+    // right plan, and real statistics would hide every index the table needs
+    // once it grows. It does not decide whether production's numbers reach the
+    // server. A small database has a snapshot worth publishing too.
+    //
+    // Concurrent because the row count no longer gates the dump, and the two
+    // read different catalogs.
+    const [totalRows, dumped] = await Promise.all([
+      connector.getTotalRowCount(tables),
+      this.dumpSourceStats(source),
+    ]);
 
     if (totalRows < Remote.STATS_ROWS_THRESHOLD) {
       log.info(
         `Total rows (${totalRows}) below threshold, using default stats`,
         "remote",
       );
-      return { mode: Statistics.defaultStatsMode, strategy: "default" };
+      return {
+        mode: Statistics.defaultStatsMode,
+        strategy: "default",
+        dump: dumped.stats,
+      };
     }
 
     log.info(
       `Total rows (${totalRows}) above threshold, pulling source stats`,
       "remote",
     );
-    return { mode: await this.dumpSourceStats(source), strategy: "fromSource" };
+    return { mode: dumped, strategy: "fromSource", dump: dumped.stats };
   }
 
   /**
    * Re-dump and push the source's statistics when they've drifted far enough
    * from what we last pushed (ADR 0007 §2). Runs on the schema poll tick.
    *
-   * No-ops until a `fromSource` dump has established a baseline: a synthetic or
-   * imported snapshot has nothing meaningful to drift against, and inventing a
-   * baseline for it would push someone else's numbers as this project's
+   * No-ops until a dump from the source has established a baseline. The sync at
+   * boot establishes one, so in practice this is armed from the first sync
+   * onwards; the guard is what stops an imported or synthetic snapshot standing
+   * in for a baseline and pushing someone else's numbers as this project's
    * production statistics.
    */
   private async refreshStatsIfStale(
@@ -515,7 +538,9 @@ export class Remote extends EventEmitter<RemoteEvents> {
     log.info(`Statistics refresh skipped: ${reason}`, "remote");
   }
 
-  private async dumpSourceStats(source: Connectable): Promise<StatisticsMode> {
+  private async dumpSourceStats(
+    source: Connectable,
+  ): Promise<Extract<StatisticsMode, { kind: "fromStatisticsExport" }>> {
     const pg = this.sourceManager.getOrCreateConnection(
       source,
     );
@@ -546,11 +571,11 @@ export class Remote extends EventEmitter<RemoteEvents> {
   /**
    * Adopt the server's stored snapshot as the drift baseline, without pushing.
    *
-   * Drift is measured against the last dump this process pushed, so an analyzer
-   * that has never pushed has no baseline and never drifts — which is every
-   * project whose snapshot was seeded by hand. Seeding on connect closes that
-   * gap: the first schema poll can then compare the live schema against what
-   * the server holds and re-dump if it has fallen behind.
+   * Drift is measured against the last dump this process recorded. The sync at
+   * boot records one, so this is not the only way a baseline appears — but it
+   * arrives first, and it covers a connection that reconnects without syncing
+   * again. Either way the first schema poll can compare the live schema against
+   * a baseline and re-dump if it has fallen behind.
    *
    * Deliberately does not push. These numbers came from the server; echoing
    * them back would republish a stale snapshot as if it were fresh.
@@ -563,34 +588,50 @@ export class Remote extends EventEmitter<RemoteEvents> {
       return;
     }
     this.statsBaseline = baselineFromDump(stats);
-    // Arm the daily floor too. The sync path reaches the optimizer directly
-    // rather than through `applyStatistics`, so without this `lastStatsPushAt`
-    // stays undefined and the floor never fires for a seeded analyzer — which
-    // is every analyzer that hasn't happened to drift. Dated from now rather
-    // than the snapshot's capture time, which the RPC doesn't carry: the floor
-    // then fires 24h after connect instead of immediately.
+    // Arm the daily floor too, so a connection that seeds and then never syncs
+    // still refreshes eventually. Dated from now rather than the snapshot's
+    // capture time, which the RPC doesn't carry: the floor then fires 24h after
+    // connect instead of immediately.
     this.lastStatsPushAt = Date.now();
   }
 
   async applyStatistics(statsMode: StatisticsMode): Promise<void> {
     await this.optimizer.setStatistics(statsMode);
-    // Push the statistics we were handed, not `optimizer.ownMetadata`. That is
-    // a dump of the *optimizing* database, which is restored with
-    // `--exclude-table-data-and-children` and never durably analyzed, so every
-    // table in it reports `reltuples = -1` and every column `stats: null`.
-    // Sending it would overwrite the project's real snapshot with an empty one.
-    //
-    // `fromAssumption` carries no real numbers at all, so it pushes nothing —
+    // Record the statistics we were handed, not `optimizer.ownMetadata` — see
+    // recordSourceStatistics for why that would publish an empty snapshot.
+    // `fromAssumption` carries no real numbers at all, so it records nothing:
     // synthetic defaults are not this project's production statistics.
-    if (statsMode.kind === "fromStatisticsExport" && statsMode.stats.length > 0) {
-      this.statsBaseline = baselineFromDump(statsMode.stats);
-      this.lastStatsPushAt = Date.now();
-      this.emit("statsApplied", statsMode.stats);
+    if (statsMode.kind === "fromStatisticsExport") {
+      this.recordSourceStatistics(statsMode.stats);
     }
     // don't block the reply by awaiting all optimizations
     this.optimizer.restart();
   }
 
+  /**
+   * Adopt a dump the analyzer took from the source: it becomes the drift
+   * baseline, and it reaches the server.
+   *
+   * Both halves matter. Publishing is how a project gets a snapshot at all, and
+   * the baseline is what lets the drift check re-dump later: without it,
+   * `refreshStatsIfStale` returns on its first line forever.
+   *
+   * Callers pass what they measured from production, never the optimizer's own
+   * metadata: the optimizing database is restored with
+   * `--exclude-table-data-and-children` and never durably analyzed, so every
+   * table in it reports `reltuples = -1` and every column `stats: null`.
+   * Publishing that would overwrite the project's real snapshot with an empty
+   * one.
+   */
+  private recordSourceStatistics(stats: ExportedStats[]): void {
+    if (stats.length === 0) {
+      return;
+    }
+    this.statsBaseline = baselineFromDump(stats);
+    this.lastStatsPushAt = Date.now();
+    this.emit("statsApplied", stats);
+  }
+
   async resetPgStatStatements(source: Connectable): Promise<void> {
     const connector = this.sourceManager.getConnectorFor(source);
     await connector.resetPgStatStatements();
@@ -707,6 +748,11 @@ export type InferredStatsStrategy = "default" | "fromSource" | "imported";
 type StatsResult = {
   mode: StatisticsMode;
   strategy: InferredStatsStrategy;
+  /**
+   * What production measured, when this sync measured it. Absent for a static
+   * mode: those numbers came from a file, not from this database.
+   */
+  dump?: ExportedStats[];
 };
 
 const PgStatStatementsStatus = {

src/remote/seed-stats-baseline.test.ts

@@ -91,10 +91,8 @@ describe("Remote.seedStatsBaseline — daily floor interaction", () => {
   }
 
   it("arms the daily floor, so a seeded analyzer still refreshes eventually", () => {
-    // The sync path reaches the optimizer directly rather than through
-    // applyStatistics, so seeding is the only chance to set this. Left unset,
-    // isPastRefreshFloor short-circuits on undefined and the floor never fires
-    // for any analyzer that hasn't happened to drift.
+    // Left unset, isPastRefreshFloor short-circuits on undefined and the floor
+    // never fires for a connection that seeds and then never syncs.
     const remote = makeRemote();
 
     remote.seedStatsBaseline([table("users", 10_000)]);

src/remote/test-utils.ts

@@ -1,4 +1,5 @@
 import { expect } from "vitest";
+import { PostgreSqlContainer } from "@testcontainers/postgresql";
 
 export function assert(
   condition: unknown,
@@ -17,3 +18,32 @@ export function assertDefined<T>(
 export function normalizeQuery(query: string): string {
   return query.replace(/\s+/g, " ");
 }
+
+const TEST_TARGET_CONTAINER_NAME = "postgres:17";
+
+/**
+ * Start a Postgres container, seeded from `content` as an initdb script when
+ * one is given.
+ *
+ * Lives here rather than beside the tests that use it: importing it from a
+ * `*.test.ts` registers that file's tests into the importer's suite too, so
+ * every borrower pays for the lender's containers.
+ */
+export function testSpawnTarget(
+  options: { content?: string; containerName?: string } = {
+    containerName: TEST_TARGET_CONTAINER_NAME,
+  },
+) {
+  let pg = new PostgreSqlContainer(
+    options.containerName ?? TEST_TARGET_CONTAINER_NAME,
+  );
+  if (options.content) {
+    pg = pg.withCopyContentToContainer([
+      {
+        content: options.content,
+        target: "/docker-entrypoint-initdb.d/init.sql",
+      },
+    ]);
+  }
+  return pg.start();
+}

src/runner.ci.test.ts

@@ -1,6 +1,6 @@
 import { test, expect } from "vitest";
 import { PostgreSqlContainer } from "@testcontainers/postgresql";
-import { testSpawnTarget } from "./remote/remote.test.ts";
+import { testSpawnTarget } from "./remote/test-utils.ts";
 import { Connectable } from "./sync/connectable.ts";
 import { ConnectionManager } from "./sync/connection-manager.ts";
 import { Remote } from "./remote/remote.ts";