Skip to content

fix(sim): bound the fleet counts the data components read - #60

Open
kevin9327 wants to merge 1 commit into
xevrion:mainfrom
kevin9327:fix/data-fleet-bounds
Open

fix(sim): bound the fleet counts the data components read#60
kevin9327 wants to merge 1 commit into
xevrion:mainfrom
kevin9327:fix/data-fleet-bounds

Conversation

@kevin9327

Copy link
Copy Markdown
Contributor

What

A design carrying a large shardCount, replicaCount or shardCapacity takes the tab down, and
one carrying a non-numeric shardCount or replicaCount throws.

This is the same shape as #58, one layer down. clampInt is the data tier's own version of the
guard:

function clampInt(v: number, min: number): number {
  const n = Math.floor(v);
  return n < min ? min : n;
}

Math.floor(NaN) is NaN and NaN < min is false, so a value that is not a number walks straight
out of the clamp, and there is no ceiling at all. What is on the other side is structure sizing:

new Array<number>(count).fill(0)   // makeShardExt
scratch.length = replicas + 1      // instanceScratch

Measured

Running each decoded topology through the engine for 120 ticks:

field                     value       what happens
shardCount     shard      NaN         RangeError: Array length must be a positive integer
shardCount     shard      1e9         does not return (killed at 15s)
shardCount     shard      Infinity    does not return (killed at 15s)
replicaCount   replica    NaN         RangeError: Invalid array length
replicaCount   replica    Infinity    RangeError: Invalid array length
replicaCount   replica    1e9         does not return (killed at 15s)
shardCapacity  shard      NaN         nodes.target.utilization is NaN, and every
                                      shardUtilization[] and perInstance[] entry with it

The cost is linear in the count, so it is allocation rather than a slow frame: 1e3 partitions of
work is 8ms, 1e5 is 133ms, 1e6 is 1149ms.

The shardCapacity case is the quiet one. Nothing throws; the capacity is the divisor of the
utilisation, so the reader is shown NaN% on the node and on every shard bar under it.

Where it comes from

Not the inspector: its numbers stop at 64 shards, 64 replicas and 512 slots. It comes from every
other way a design arrives. isTopology checks the nine core config numbers — capacity,
serviceMs, serviceCv, queueLimit, hitRate, errorRate, timeoutMs, retries, rps — and
none of these three, so a shared link, a .breakscale file and a restored session carry them
through untouched.

Fix

clampInt keeps the promise its name makes. A value that is not a finite number is min, and the
count is capped at max, which each caller passes as the ceiling the inspector already offers for
that field: 64 shards, 64 replicas, 512 slots per shard. Nothing a reader can build changes.

capacity keeps no ceiling — it is one of the nine isTopology already checks, and bounding it
would be a behaviour change rather than a guard — but it picks up the finite check for free.

Tests

src/sim/behaviour-data.bounds.test.ts, new, on the published snapshot:

  • a shard count of NaN, Infinity and 1e9 all run, and the published shardUtilization is at
    most 64 long
  • a replica count of NaN, Infinity and 1e9 all run
  • a shardCapacity of NaN publishes a real utilization, not NaN
  • eight shards are still eight shards, which is the check that the ceilings changed nothing a
    reader can reach

Against main the file cannot finish: the worker exits rather than reporting.

Caused by: Error: Worker exited unexpectedly
 Test Files   (1)
      Tests   (8)

How I tested

Windows 11, Bun 1.3.14. bun run test is 937 passed across 40 files, up from 929 by the eight new
tests, with nothing else moving. bun run typecheck, bun run lint and bun run format:check are
clean.

I have not touched isTopology. Bounding it here puts the guard on the seam every path shares — a
design reaches the engine from the editor, a link, a file and a restored session, and only one of
those goes through the validator — which is the same reasoning as #58. Happy to add the validator
check as well if you would rather have both.

@kevin9327
kevin9327 requested a review from xevrion as a code owner September 6, 2026 11:43
@vercel

vercel Bot commented Sep 6, 2026

Copy link
Copy Markdown

@kevin9327 is attempting to deploy a commit to the whoarrryou's projects Team on Vercel.

A member of the Team first needs to authorize it.

@xevrion

xevrion commented Sep 7, 2026

Copy link
Copy Markdown
Owner

The fix is good, but probe.ts in the repo root is your exploration script rather than part of the change. Could you drop it and I'll merge?

Also, five PRs at once is a lot to review in parallel. No problem with these, but a couple at a time lands faster.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants