Skip to content

test: census of standalone toHaveBeenCalledWith sites where the call count is the contract (#15607) - #15782

Merged
baozhoutao merged 2 commits into
mainfrom
claude/issue-15607-called-with-count-census
Sep 5, 2026
Merged

test: census of standalone toHaveBeenCalledWith sites where the call count is the contract (#15607)#15782
baozhoutao merged 2 commits into
mainfrom
claude/issue-15607-called-with-count-census

Conversation

@claude

@claude claude Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Fixes #15607

The card is an existence proof, not a population: one measured instance where a
standalone toHaveBeenCalledWith hid a doubled ql.insert. So the first job was to
size the class, and only then to act on the part of it where the count is actually
the contract.

1. The census

Rule. A toHaveBeenCalledWith assertion is standalone when no
toHaveBeenCalledTimes / toHaveBeenCalledOnce / SPY.mock.calls.length /
SPY.mock.calls + toHaveLength sits on the same spy in the same it(/test(
block. Scanned: every *.test.* / *.spec.* file under packages/**, examples/**,
apps/**, with comments and literal content masked through the repo's own
scripts/js-comment-mask.mjs, so a mention inside a docblock or a string is not counted
as an assertion.

Result on origin/main:

test files carrying the idiom 171
toHaveBeenCalledWith assertions total 811
standalone (no sibling count, same test) 772
of those, .not.toHaveBeenCalledWith (asserts absence — a repeat cannot satisfy it) 30
positive standalone — the blind idiom 742

Per package (standalone, on origin/main):

 165  packages/plugins          19  packages/metadata-protocol
 161  packages/runtime          12  packages/metadata
 120  packages/rest              9  packages/triggers
  96  packages/objectql          8  packages/cli
  69  packages/services          7  packages/core
  40  packages/adapters          4  packages/cloud-connection
  31  packages/mcp               1  packages/client-react
  27  packages/client            1  packages/connectors
                                 1  packages/drivers
                                 1  packages/spec

The population is large. That is the card's own predicted branch, and it is why
this PR is a census plus nine pins, not a sweep. 742 assertions that do not care
about arity are left exactly as they are: adding a count where none is meant would pin a
number nobody chose.

Controls

Positive control — the #15262 seed rig must come back COVERED where it now counts
calls. PR #15605 is still open, so this had to be run against its head blob, not
origin/main (on origin/main that file has no count at all):

git show pull/15605/head:packages/runtime/src/app-plugin.seed.test.ts > /tmp/pr15605-seed.test.ts
node census.mjs --files /tmp/pr15605-seed.test.ts
  toHaveBeenCalledWith assertions total:     11
  STANDALONE (no sibling count, same test):  8

11 minus 8 = the 3 assertions #15605 pinned alongside its toHaveBeenCalledTimes
(its lines 327, 345, 346) — scored covered, and no others. Control holds.

Negative control — the site the card actually measured,
packages/runtime/src/app-plugin.seed.test.ts:78 (insert), is found by the scan and
appears in the standalone list. Control holds.

Two blind spots the scan had, both found by reconciling its own total against a raw
count and both fixed before any number above was reported
(a census that cannot see
part of its corpus is the very failure this card is about):

  • expect.soft(spy) was not recognised as an expect call — 1 assertion invisible. That
    site turned out to be a one-shot leader-fence test, i.e. a strong candidate.
  • vitest's expect(value, message) two-argument form folded the message into the spy key,
    so a spy counted with a message and asserted without one read as two different
    spies. That reported a genuinely covered site (db-job-adapter.once-leader.test.ts,
    which asserts toHaveBeenCalledTimes(2) two lines above) as standalone.

After both fixes the parsed total (811) equals the masked-code total exactly, so no
occurrence in the corpus is unaccounted for.

Method (reproducible)

Not added under scripts/ — it earns no gate, so per the card it lives here. Save as
census.mjs at the repo root and run node census.mjs [--sites|--json].

#!/usr/bin/env node
// Census: `toHaveBeenCalledWith` assertions with no sibling call-count assertion
// on the SAME spy in the SAME `it(`/`test(` block. Run from the repo root:
//   node census.mjs            # summary + per-package split
//   node census.mjs --sites    # one line per standalone site
//   node census.mjs --json
import { readFileSync } from 'node:fs';
import { execFileSync } from 'node:child_process';
import { scanSource, blank } from './scripts/js-comment-mask.mjs';

const ROOTS = ['packages', 'examples', 'apps'];
const TEST_RE = /\.(test|spec)\.[cm]?[jt]sx?$/;

const argv = process.argv.slice(2);
const filesFlag = argv.indexOf('--files');
const files =
  filesFlag >= 0
    ? argv[filesFlag + 1].split(',')
    : execFileSync('git', ['ls-files', '--', ...ROOTS], { encoding: 'utf8' })
        .split('\n')
        .filter((f) => f && TEST_RE.test(f));

const IDENT = /[\w$]/;

/** Balanced-paren match starting at the '(' at `open` in the structural view. */
function matchParen(code, open) {
  let depth = 0;
  for (let i = open; i < code.length; i++) {
    const c = code[i];
    if (c === '(') depth++;
    else if (c === ')') {
      depth--;
      if (depth === 0) return i;
    }
  }
  return -1;
}

/** Blocks of `it(...)` / `test(...)` calls, innermost-last, as [start,end] spans. */
let noCommentsRef = '';
function testBlocks(code) {
  const spans = [];
  const re = /\b(it|test|bench)\b/g;
  let m;
  while ((m = re.exec(code))) {
    const before = code[m.index - 1];
    if (before && (IDENT.test(before) || before === '.')) continue; // `.it`, `unit`
    // skip an optional `.only` / `.skip` / `.each(...)` / `.concurrent` chain
    let i = m.index + m[0].length;
    for (;;) {
      while (i < code.length && /\s/.test(code[i])) i++;
      if (code[i] !== '.') break;
      let j = i + 1;
      while (j < code.length && IDENT.test(code[j])) j++;
      i = j;
      while (i < code.length && /\s/.test(code[i])) i++;
      if (code[i] === '(') {
        // `.each(...)` or a tagged-template-ish call in the chain
        const close = matchParen(code, i);
        if (close < 0) break;
        i = close + 1;
      }
    }
    while (i < code.length && /\s/.test(code[i])) i++;
    if (code[i] !== '(') continue;
    const close = matchParen(code, i);
    if (close < 0) continue;
    const title = /^[\s(]*(['\"`])([\s\S]{0,160}?)\1/.exec(noCommentsRef.slice(i, i + 240));
    spans.push([m.index, close, title ? title[2].replace(/\s+/g, ' ') : '']);
  }
  return spans;
}

function innermost(spans, offset) {
  let best = null;
  for (const s of spans) {
    if (offset >= s[0] && offset <= s[1]) {
      if (!best || s[0] > best[0]) best = s;
    }
  }
  return best;
}

const norm = (s) => s.replace(/\s+/g, '');

/**
 * The SUBJECT of `expect(...)`: its first argument only.
 * vitest takes an optional second argument as the assertion message
 * (`expect(spy, 'why').toHaveBeenCalledTimes(2)`), and folding that into the
 * spy key made a counted spy look like a different spy from the same spy
 * asserted without a message -- i.e. it reported a COVERED site as standalone.
 */
function firstArg(text) {
  let depth = 0;
  for (let i = 0; i < text.length; i++) {
    const c = text[i];
    if (c === '(' || c === '[' || c === '{') depth++;
    else if (c === ')' || c === ']' || c === '}') depth--;
    else if (c === ',' && depth === 0) return text.slice(0, i);
  }
  return text;
}

/** Every `expect(ARG).CHAIN` in the structural view. */
function expectCalls(code) {
  const out = [];
  // `expect(`, and the modifier forms `expect.soft(` / `expect.poll(`
  const re = /\bexpect\s*(?:\.\s*(?:soft|poll)\s*)?\(/g;
  let m;
  while ((m = re.exec(code))) {
    const before = code[m.index - 1];
    if (before && (IDENT.test(before) || before === '.')) continue;
    const open = m.index + m[0].length - 1;
    const close = matchParen(code, open);
    if (close < 0) continue;
    let i = close + 1;
    let chain = '';
    while (i < code.length) {
      if (/\s/.test(code[i])) { i++; continue; }
      if (code[i] === '.') { chain += '.'; i++; continue; }
      if (IDENT.test(code[i])) { chain += code[i]; i++; continue; }
      break;
    }
    out.push({ at: m.index, arg: norm(firstArg(code.slice(open + 1, close))), chain });
  }
  return out;
}

const COUNT_MATCHER = /toHaveBeenCalledTimes|toHaveBeenCalledOnce/;
const CALLS_LENGTH = /^(.*)\.mock\.calls\.length$/;
const CALLS_ONLY = /^(.*)\.mock\.calls$/;

const sites = [];
const perPkg = new Map();
let totalWith = 0;
let filesScanned = 0;

for (const file of files) {
  const src = readFileSync(file, 'utf8');
  if (!src.includes('toHaveBeenCalledWith')) continue;
  filesScanned++;
  const flags = scanSource(src);
  // structural view: comments AND literal CONTENT blanked, offsets preserved
  const noComments = blank(src, flags.comment);
  const code = blank(noComments, flags.literal);
  noCommentsRef = noComments;

  const lineOf = (off) => src.slice(0, off).split('\n').length;
  const spans = testBlocks(code);
  const calls = expectCalls(code);

  // bare `SPY.mock.calls.length` reads anywhere (not only inside expect)
  const bareCounts = [];
  const bare = /([\w$.\]\[]+)\.mock\.calls\.length/g;
  let bm;
  while ((bm = bare.exec(code))) bareCounts.push({ at: bm.index, spy: norm(bm[1]) });

  const withSites = [];
  const countSites = [];
  for (const c of calls) {
    if (/\btoHaveBeenCalledWith\b/.test(c.chain)) {
      // `.not.toHaveBeenCalledWith` asserts ABSENCE of a matching call, which a
      // repeat cannot satisfy -- it is not the blind idiom this census is about.
      withSites.push({ at: c.at, spy: c.arg, negated: /\.not\b/.test(c.chain) });
      totalWith++;
    }
    if (COUNT_MATCHER.test(c.chain)) countSites.push({ at: c.at, spy: c.arg });
    const mL = CALLS_LENGTH.exec(c.arg);
    if (mL) countSites.push({ at: c.at, spy: mL[1] });
    const mC = CALLS_ONLY.exec(c.arg);
    if (mC && /toHaveLength|toEqual|toStrictEqual|toMatchObject/.test(c.chain))
      countSites.push({ at: c.at, spy: mC[1] });
  }
  for (const b of bareCounts) countSites.push(b);

  const sameBlock = (a, b) => {
    const sa = innermost(spans, a);
    const sb = innermost(spans, b);
    if (!sa && !sb) return true; // both at file scope
    if (!sa || !sb) return false;
    return sa[0] === sb[0];
  };

  for (const w of withSites) {
    const covered = countSites.some((cs) => cs.spy === w.spy && sameBlock(w.at, cs.at));
    if (covered) continue;
    const anywhereInFile = countSites.some((cs) => cs.spy === w.spy);
    const block = innermost(spans, w.at);
    sites.push({
      file,
      line: lineOf(w.at),
      spy: w.spy,
      inTestBlock: Boolean(block),
      test: block ? block[2] : '',
      negated: w.negated,
      countForSpyElsewhereInFile: anywhereInFile,
    });
    const pkg = file.split('/').slice(0, 2).join('/');
    perPkg.set(pkg, (perPkg.get(pkg) || 0) + 1);
  }
}

const args = argv;
if (args.includes('--json')) {
  console.log(JSON.stringify({ filesScanned, totalWith, standalone: sites.length, perPkg: Object.fromEntries([...perPkg].sort((a, b) => b[1] - a[1])), sites }, null, 2));
} else {
  if (args.includes('--sites')) {
    for (const s of sites)
      console.log(`${s.file}:${s.line}\t${s.spy}\t${s.negated ? 'NOT:' : ''}${s.test}${s.inTestBlock ? '' : '\t[not in it()]'}${s.countForSpyElsewhereInFile ? '\t[count elsewhere in file]' : ''}`);
    console.log('');
  }
  console.log(`test files scanned (containing the idiom): ${filesScanned}`);
  console.log(`toHaveBeenCalledWith assertions total:     ${totalWith}`);
  console.log(`STANDALONE (no sibling count, same test):  ${sites.length}`);
  const neg = sites.filter((s) => s.negated).length;
  console.log(`  of which .not.toHaveBeenCalledWith (absence): ${neg}`);
  console.log(`  POSITIVE standalone (the blind idiom):        ${sites.length - neg}`);
  console.log('');
  console.log('per package:');
  for (const [p, n] of [...perPkg].sort((a, b) => b[1] - a[1])) console.log(`  ${String(n).padStart(4)}  ${p}`);
}

2. Classification — would a doubled call be a defect here?

Applied to the standalone population. The honest answer for the overwhelming majority is
no: they assert the shape of one call (a route forwarding its argument, a log line, a
one-shot lookup) and arity is not part of what they pin.

YES — nine sites, all one class: a repeat is a silent correctness bug and nothing else
in the test can see it.
These are pinned in this PR.

file:line (post-change) spy why the count is the contract
packages/services/service-storage/src/attachment-lifecycle.test.ts:424 s.delete Reap sweep, irreversible byte delete. One tombstoned row in ⇒ exactly one delete.
.../attachment-lifecycle.test.ts:476 s.delete Gated field-file reap; a second call re-reaps a key already gone.
.../attachment-lifecycle.test.ts:577 s.delete Best-effort cleanup of one abandoned upload.
packages/services/service-storage/src/lax-deviation-reclamation-gate.test.ts:188 s.delete Strongest. The guard runs twice in this test — withheld, then authorised. The count is what proves the withheld pass deleted nothing.
.../lax-deviation-reclamation-gate.test.ts:209 s.delete One attachment-scope tombstone ⇒ one byte delete.
packages/spec/src/shared/resilient-fetch.test.ts:69 sleep Retry path — one 429 ⇒ exactly one backoff. Every neighbouring test in this file already counts fetchImpl.
packages/runtime/src/seed-loader.test.ts:1123 engine.insert Seed application — the card's own class. One record, mode: 'insert' ⇒ one write.
.../seed-loader.test.ts:1158 engine.insert Same; a repeat is a double seed.
.../seed-loader.test.ts:1190 engine.insert Same, on the per-tenant replay path.

Each of the three files already establishes this idiom itself — attachment-lifecycle.test.ts:548
and :564, resilient-fetch.test.ts throughout, seed-loader.test.ts:309 — so the
correct form was fixed by existing evidence, not invented here.

Deliberately NOT pinned, though they sit in card-named categories: the ~30
.not.toHaveBeenCalledWith sites (they assert absence already); route-forwarding and
registration assertions (registerService, registerFlow, dispatch) where the test
pins the argument shape and no once-semantics exists in the code under test; and
packages/runtime/src/app-plugin.seed.test.ts itself, which is both out of scope
per the card and held by the open PR #15605.

3. Ablation — the count assertions were driven red, and the thesis measured

Mutation: double every byte delete in the reap guard
(packages/services/service-storage/src/attachment-lifecycle.ts, both call sites), i.e.
exactly the defect shape the card describes. Trap-guarded, absolute paths.

The test files import ./attachment-lifecycle.js, which resolves to the package's own
src/ under vitest — no dist is involved, so no rebuild is needed for the mutation to
take effect
, and none was performed.

Mutation proven on disk before measuring (not by an editor's exit code): injected marker
x2, un-doubled call sites remaining x0, git diff --stat showing 2 insertions(+), 2 deletions(-). A first attempt matched zero anchors; the script refused to run the
suite and exited 93 rather than report a reading from an unmutated tree.

With this PR's pins:

Tests  6 failed | 49 passed (55)

All six failures are AssertionError: expected "vi.fn()" to be called 1 times, but got 2 times.

Baseline leg — the same mutation with the two test files rolled back to origin/main
(this PR's pins removed):

Tests  1 failed | 54 passed (55)

⇒ Under a doubled irreversible byte delete, the tree today reds one test.
lax-deviation-reclamation-gate.test.ts goes entirely green. Every standalone
toHaveBeenCalledWith in both files passes under the doubling — which is the card's
thesis, measured rather than restated.

Restore proven both legs: git checkout HEAD -- ABSOLUTE_PATH, then
git hash-object equal to the HEAD blob (2e0349f5…) and git diff HEAD empty.

4. Verification

Run at the final commit 0e9dad969.

  • pnpm --filter @objectstack/service-storage exec vitest run src/attachment-lifecycle.test.ts src/lax-deviation-reclamation-gate.test.tsTests 55 passed (55) (exit 0)
  • pnpm --filter @objectstack/spec exec vitest run src/shared/resilient-fetch.test.tsTests 9 passed (9) (exit 0)
  • pnpm --filter @objectstack/runtime exec vitest run src/seed-loader.test.tsTests 41 passed (41) (exit 0)
  • typecheck + check:test-typecheck for all three packages → exit 0. The test layer is
    covered explicitly: check:test-typecheck: OK — @objectstack/service-storage's test layer compiles under packages/services/service-storage/tsconfig.test.json; 0 file(s) / 0 error(s),
    and likewise for @objectstack/spec and @objectstack/runtime.
  • node scripts/pm/dispatch-gates.mjs --changed --commands --repo objectstack-ai/objectstackEXIT=0,
    69 commands, derived from the merged tree (the first derivation warned STALE TREE
    against 3 changed family files; origin/main was merged in and it was derived again —
    same 69).
  • 66 of 69 green. pnpm check:nul-bytes
    check-nul-bytes: OK (scanned 7615 text file(s) ... no raw ASCII control bytes).
    Five @objectstack/spec gates first returned "build first"; after
    pnpm --filter @objectstack/spec build all five are exit 0.
  • node scripts/pm/check-governed-merges.mjs --test on the final four paths →
    ✅ NOT governed — ordinary queue landing applies to a PR with exactly this file list.

NOT MEASURED (exit 3 — prerequisite not met, a whole-repo pnpm build; these are
declared to CI, not read as passes):
pnpm check:dual-build-cjs-loads,
pnpm check:i18n, pnpm check:type-check-debt.

Repo-wide pnpm lint was not run locally — it is CI's, per the local-scope rule.

Changeset: none, and skip-changeset applied. AGENTS.md: "that label is for a diff
that publishes nothing from any released package."
This diff is four *.test.ts files,
and none of the three packages ships tests — their files are ["dist", "README.md", "CHANGELOG.md"] (@objectstack/spec additionally src/**/*.zod.ts, which a
.test.ts is not).

🤖 Generated with Claude Code

https://claude.ai/code/session_012zGPuVVX3deAx9LdjK8jCk


Generated by Claude Code

A census of `toHaveBeenCalledWith` across `packages/**`, `examples/**` and
`apps/**` found 811 assertions, 763 of them standalone — no sibling
`toHaveBeenCalledTimes` / `toHaveBeenCalledOnce` / `mock.calls.length` on the
same spy in the same test. Most of those genuinely do not care about arity and
are left alone on purpose.

Nine do care, and all nine sit on a path where a doubled call is a defect the
assertion cannot see: irreversible byte deletes in a reap sweep, a retry
backoff, and seed application — the class the card was filed from.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012zGPuVVX3deAx9LdjK8jCk
@github-actions github-actions Bot added the size/s label Sep 5, 2026
@claude claude Bot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 5, 2026
@github-actions github-actions Bot added the tests label Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

Nothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs.

What this run could not see
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json aedbaeff1c5e9ba03159258bef8d61960fd355c5packageMentionDocs.

@claude

claude Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

ACCEPT — PR #15782 (head 0e9dad969, 4 test files, +26/−0) reviewed against the GitHub three-dot diff, not the report.

The ruling's shape held: the population is large (811 toHaveBeenCalledWith across 171 test files; 772 standalone; 742 after the 30 .not forms), the YES class — "would a doubled call be a defect?" — is small, and it is pinned in-PR at exactly nine sites of one shape: toHaveBeenCalledTimes(1) on s.delete (five irreversible byte-delete / reap sites, incl. the strongest one where the guard runs twice and the count proves the withheld pass deleted nothing), sleep (one 429 ⇒ one backoff) and engine.insert ×3 (seed application, the card's own class). No mass conversion; the 733 arity-agnostic sites are left alone by name of the criterion. No production file touched. The instrument's two blind spots (expect.soft, expect(value, message)) were found by reconciling against a raw count and fixed before the census was reported — the right instinct on a card about instruments that report clean answers over what they cannot see.

Measured by this seat: 9 added count assertions read from the diff; only *.test.ts files; git merge-tree --write-tree origin/main <branch> → EXIT=0; no live PR of this lane touches these packages' tests; governed test per the report NOT governed. NOT MEASURED here: the three package test runs (no deps in this checkout — the dev's readings 55 / 9 / 41 passed, typecheck + test-typecheck exit 0, and the doubled-delete ablation 6 failed | 49 passed with pins vs 1 failed | 54 passed without are the record), and the three whole-repo-build gates by name.

Positive control accepted on PR #15605's head blob rather than main (that PR is still open) — stated, not hidden. Outcome per the ruling: the class is small; no follow-up cards owed.

Flip + arm when both required jobs report success on 0e9dad969.


Generated by Claude Code

@claude

claude Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Flipped ready + auto-merge enabled (08:03Z, method: MERGE). Required jobs on 0e9dad969: Lint & Repo Gates = success, TypeScript Type Check = success; git merge-tree --write-tree origin/main <branch> → EXIT=0 against the current main. Watched; on landing #15607 closes.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/s skip-changeset PR has no user-facing published change; bypasses the changeset gate tests

Projects

None yet

2 participants