chore(objectql): 退休 applyFormulaPlan 的零调用者 nowSnapshot 形参,并把 docstring 收窄到实际成立的范围 (#5699)

.changeset/objectql-retire-formula-now-snapshot-param.md

@@ -0,0 +1,36 @@
+---
+"@objectstack/objectql": patch
+---
+
+chore(objectql): retire `applyFormulaPlan`'s zero-caller `nowSnapshot` parameter and narrow its docstring to what actually holds (#5699)
+
+`applyFormulaPlan` declared a fourth optional parameter `nowSnapshot?: Date`
+whose only effect was `nowSnapshot ?? new Date()`. Not one of its three call
+sites ever passed it — `find`, `findOne`, and the write-response hydration
+`hydrateWriteFormulas` added by #5504 — so the parameter went down the
+`new Date()` branch from birth. Dormant code, removed rather than archived: a
+parameter that looks live is worse than no parameter, because everyone
+reasoning from it concludes the caller can pin the instant, and one caller
+plainly should have.
+
+No behaviour change (the removed branch was unreachable), no public API change
+(`applyFormulaPlan` is module-private and never exported).
+
+The docstring claimed the eval context "mirrors `applyFieldDefaults`". Half of
+that was true — the same keys, so `formula` and `defaultValue` expressions share
+one vocabulary — and half was not: the two pin their own `now`.
+`applyFieldDefaults` is handed the insert's pre-write snapshot, while
+`applyFormulaPlan` reads the clock once per call, because a formula is evaluated
+when a record is materialized. So inside one `insert` a `NOW()` default and a
+`now()` formula observe two instants a driver round-trip apart (sub-millisecond
+in practice; across a second/day boundary they can land on different calendar
+days). The docstring now says so, and names #5699 as where making them share one
+instant would have to be argued — it would hand the write path a determinism
+guarantee the read path cannot have, which is a semantic decision, not a cleanup.
+
+Adds the pins that the retired parameter's *appearance* was standing in for
+(`engine-write-formula-hydration.test.ts`): one snapshot per call shared by every
+row × every formula field, asserted by object identity on the eval context so a
+per-evaluation `new Date()` fails even when the milliseconds agree, on the write
+path and the read path alike; plus a tripwire that the default's instant and the
+formula's instant stay independently sourced.

packages/objectql/src/engine-write-formula-hydration.test.ts

@@ -111,6 +111,58 @@ const PLAIN = {
   },
 };
 
+/**
+ * Two `now()` formulas — the determinism surface (#5699).
+ *
+ * `applyFormulaPlan` builds ONE eval context per call and reuses it for every
+ * row × every formula field, so an object declaring two clock formulas is the
+ * smallest shape that can observe the guarantee in both directions at once.
+ * After the zero-caller `nowSnapshot` parameter was retired, this per-call
+ * snapshot is the ONLY thing pinning the function's determinism.
+ */
+const CLOCK = {
+  name: 'wf_clock',
+  label: 'Clock',
+  fields: {
+    id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true },
+    name: { name: 'name', label: 'Name', type: 'text' as const },
+    seen_at: {
+      name: 'seen_at', label: 'Seen At', type: 'formula' as const,
+      expression: { dialect: 'cel', source: 'now()' },
+    },
+    seen_again: {
+      name: 'seen_again', label: 'Seen Again', type: 'formula' as const,
+      expression: { dialect: 'cel', source: 'now()' },
+    },
+  },
+};
+
+/**
+ * A `defaultValue` Expression AND a `formula`, both reading `now()` — the two
+ * instants #5699 is about.
+ *
+ * `created_at` is `readonly`, the shape the ~100 platform `created_at` /
+ * `updated_at` declarations use; `validateRecord` skips readonly fields, so the
+ * Date the default resolves to reaches the driver unexamined and the test
+ * observes the engine's own snapshot rather than a validator's coercion of it.
+ */
+const STAMPED = {
+  name: 'wf_stamped',
+  label: 'Stamped',
+  fields: {
+    id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true },
+    name: { name: 'name', label: 'Name', type: 'text' as const },
+    created_at: {
+      name: 'created_at', label: 'Created At', type: 'datetime' as const, readonly: true,
+      defaultValue: { dialect: 'cel', source: 'now()' },
+    },
+    stamped_at: {
+      name: 'stamped_at', label: 'Stamped At', type: 'formula' as const,
+      expression: { dialect: 'cel', source: 'now()' },
+    },
+  },
+};
+
 /** Formula referencing the caller — pins the context passthrough (#1979 status quo). */
 const MEMO = {
   name: 'wf_memo',
@@ -226,7 +278,7 @@ async function makeEngine() {
   const rig = makeStubDriver();
   engine.registerDriver(rig.driver as never, true);
   await engine.init();
-  for (const obj of [ACCOUNT, FORECAST, PLAIN, MEMO]) {
+  for (const obj of [ACCOUNT, FORECAST, PLAIN, MEMO, CLOCK, STAMPED]) {
     engine.registry.registerObject(obj as never);
   }
   const protocol = new ObjectStackProtocolImplementation(engine);
@@ -516,3 +568,96 @@ describe('#5504 — cost threshold: same gate the read path uses', () => {
     expect(evaluate).toHaveBeenCalledTimes(3);
   });
 });
+
+/**
+ * #5699 — what pins `applyFormulaPlan`'s determinism once its zero-caller
+ * `nowSnapshot?: Date` parameter is gone.
+ *
+ * The parameter was dormant from birth: none of the three call sites (`find`,
+ * `findOne`, and the write hydration this file was written for) ever passed it,
+ * so `?? new Date()` was the only branch that ever ran. Retiring it changes no
+ * behaviour — which is exactly why the guarantee it appeared to provide has to
+ * be pinned somewhere real. It is the per-call snapshot, and nothing else:
+ *
+ *  - ONE `new Date()` per call, shared by every row × every formula field, on
+ *    the write path and the read path alike (they are the same helper);
+ *  - and NOT shared with `applyFieldDefaults` — the insert's `defaultValue`
+ *    instant and the response formula's instant stay independent.
+ *
+ * The second bullet is the observation #5699 recorded rather than changed, and
+ * the last test here is its tripwire: making the two share one snapshot would
+ * hand the write path a determinism guarantee the read path cannot have, so it
+ * is a semantic decision that belongs in that issue, not in a cleanup.
+ */
+describe('#5699 — one `now` per `applyFormulaPlan` call', () => {
+  let rig: Rig;
+  let evaluate: ReturnType<typeof vi.spyOn>;
+  beforeEach(async () => {
+    rig = await makeEngine();
+    evaluate = vi.spyOn(ExpressionEngine, 'evaluate');
+  });
+  afterEach(() => { evaluate.mockRestore(); });
+
+  /** The `now` each observed evaluation was handed, in call order. */
+  const nowsSeen = (): Date[] =>
+    (evaluate.mock.calls as unknown as Array<[unknown, { now?: Date }]>)
+      .map(([, ctx]) => ctx.now as Date);
+
+  it('a batch insert hydrates every row × every formula field from ONE snapshot', async () => {
+    const rows = await rig.engine.insert('wf_clock', [{ name: 'a' }, { name: 'b' }]) as Rec[];
+
+    // 2 formula fields × 2 rows, and `wf_clock` declares no `defaultValue`
+    // expression, so every evaluation observed here is the hydration's.
+    expect(evaluate).toHaveBeenCalledTimes(4);
+
+    // The mechanism: one `new Date()`, handed to all four evaluations by
+    // IDENTITY. A per-evaluation `new Date()` would produce four distinct
+    // objects even when their milliseconds happen to agree — which is why this
+    // is asserted on the object and not on the value.
+    const nows = nowsSeen();
+    expect(nows).toHaveLength(4);
+    expect(nows.every((n) => n === nows[0])).toBe(true);
+
+    // …and the consequence a caller can see.
+    const values = [rows[0].seen_at, rows[0].seen_again, rows[1].seen_at, rows[1].seen_again];
+    expect(values[0]).toBeDefined();
+    for (const v of values) expect(v).toEqual(values[0]);
+  });
+
+  it('a find hydrates every row × every formula field from the SAME one-snapshot rule', async () => {
+    // Same helper, so the read path carries the guarantee for the same reason.
+    // Pinned here next to the write path because the retirement removed the one
+    // parameter that could ever have made the two differ.
+    await rig.engine.insert('wf_clock', [{ name: 'a' }, { name: 'b' }]);
+    evaluate.mockClear();
+
+    const found = await rig.engine.find('wf_clock', {} as never) as Rec[];
+    expect(found).toHaveLength(2);
+    expect(evaluate).toHaveBeenCalledTimes(4);
+
+    const nows = nowsSeen();
+    expect(nows).toHaveLength(4);
+    expect(nows.every((n) => n === nows[0])).toBe(true);
+  });
+
+  it("the insert's `defaultValue` instant and the response formula's instant are INDEPENDENT", async () => {
+    const row = await rig.engine.insert('wf_stamped', { name: 'two clocks' }) as Rec;
+    expect(row.stamped_at).toBeDefined();
+
+    // Exactly two evaluations, and their order is structural rather than
+    // incidental: `applyFieldDefaults` runs at the top of the insert middleware
+    // (pre-write, from the insert's own `nowSnap`), `applyFormulaPlan` runs on
+    // the driver's readback (post-write, from its own clock read).
+    expect(evaluate).toHaveBeenCalledTimes(2);
+    const [defaultNow, formulaNow] = nowsSeen();
+    expect(defaultNow).toBeInstanceOf(Date);
+    expect(formulaNow).toBeInstanceOf(Date);
+
+    // Two `new Date()`s one driver round-trip apart — NOT one shared snapshot.
+    // Status quo, deliberately: see this block's header. If a later change makes
+    // them share one, this line goes red and the decision has to be made out
+    // loud.
+    expect(formulaNow).not.toBe(defaultNow);
+    expect(formulaNow.getTime()).toBeGreaterThanOrEqual(defaultNow.getTime());
+  });
+});

packages/objectql/src/engine.ts

@@ -529,15 +529,39 @@ function planFormulaProjection(
 }
 
 /**
- * Evaluate read-time formula virtual fields against the raw rows.
+ * Evaluate formula virtual fields against the raw rows a driver handed back —
+ * the read path (`find` / `findOne`) and, since #5504, the write path's
+ * response hydration.
  *
- * The eval context mirrors `applyFieldDefaults` so formula and default
- * expressions see the same shape: a `now` pinned ONCE per operation (every row
- * and every formula field in one `find()` observes the same instant —
- * determinism, and no per-eval `new Date()` drift), plus `os.user` / `os.org`
- * resolved from the execution context (so a computed field can reference the
- * caller, e.g. `os.user.id`). Previously this passed only `{ record }`, so
- * `now()`/`today()` ran against live wall-clock and user/org were unreachable.
+ * The eval context is built ONCE per call and reused for every row × every
+ * formula field, and that is where this function's determinism comes from: one
+ * `now`, so a `now()`/`today()` formula cannot drift mid-operation, plus
+ * `os.user` / `os.org` resolved from the execution context (so a computed field
+ * can reference the caller, e.g. `os.user.id`). Previously this passed only
+ * `{ record }`, so `now()`/`today()` ran against live wall-clock and user/org
+ * were unreachable.
+ *
+ * That context has the same SHAPE as `applyFieldDefaults`' — the same keys, so
+ * one expression vocabulary serves `formula` and `defaultValue` alike — but NOT
+ * the same `now` value, and the two are sourced independently on purpose
+ * (#5699):
+ *  - `applyFieldDefaults` is handed the insert's `nowSnapshot`, so every
+ *    defaulted field of every row in one write carries the same PRE-write
+ *    instant;
+ *  - this function reads the clock itself, once per call, because a formula is
+ *    evaluated when a record is MATERIALIZED — at read time, and on the write
+ *    response — not at the moment that row's defaults were resolved.
+ *
+ * So inside a single `insert` a `NOW()` default and a `now()` formula observe
+ * two instants one driver round-trip apart (sub-millisecond in practice; across
+ * a second/day boundary they can land on different calendar days). Making them
+ * share one instant would hand the write path a determinism guarantee the read
+ * path cannot have — a semantic decision, not a tidy-up, argued in #5699. Until
+ * it is decided this function takes NO snapshot parameter: the zero-caller
+ * `nowSnapshot?: Date` it carried from birth was retired there, in the same
+ * enforce-or-remove reflex ADR-0049 applies to spec properties, because a
+ * dormant parameter reads as a live one and anyone reasoning from it concludes
+ * the two sides already share an instant.
  *
  * (ADR-0053 Phase 2 will additionally thread `timezone` here once
  * `ExecutionContext.timezone` exists — see #1980; this change is independent
@@ -547,10 +571,9 @@ function applyFormulaPlan(
   plan: FormulaPlanEntry[],
   records: any[],
   execCtx?: ExecutionContextInput,
-  nowSnapshot?: Date,
 ): void {
   if (!plan.length) return;
-  const now = nowSnapshot ?? new Date();
+  const now = new Date();
   const timezone = execCtx?.timezone;
   const user = execCtx?.userId ? { id: String(execCtx.userId), positions: execCtx?.positions ?? [] } : undefined;
   const org = execCtx?.tenantId ? { id: String(execCtx.tenantId) } : undefined;