fix(sim): bound the fleet counts the data components read - #60
Open
kevin9327 wants to merge 1 commit into
Open
Conversation
|
@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. |
This was referenced Sep 6, 2026
Owner
|
The fix is good, but Also, five PRs at once is a lot to review in parallel. No problem with these, but a couple at a time lands faster. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
A design carrying a large
shardCount,replicaCountorshardCapacitytakes the tab down, andone carrying a non-numeric
shardCountorreplicaCountthrows.This is the same shape as #58, one layer down.
clampIntis the data tier's own version of theguard:
Math.floor(NaN)isNaNandNaN < minis false, so a value that is not a number walks straightout of the clamp, and there is no ceiling at all. What is on the other side is structure sizing:
Measured
Running each decoded topology through the engine for 120 ticks:
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
shardCapacitycase is the quiet one. Nothing throws; the capacity is the divisor of theutilisation, 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.
isTopologychecks the nine core config numbers —capacity,serviceMs,serviceCv,queueLimit,hitRate,errorRate,timeoutMs,retries,rps— andnone of these three, so a shared link, a
.breakscalefile and a restored session carry themthrough untouched.
Fix
clampIntkeeps the promise its name makes. A value that is not a finite number ismin, and thecount is capped at
max, which each caller passes as the ceiling the inspector already offers forthat field: 64 shards, 64 replicas, 512 slots per shard. Nothing a reader can build changes.
capacitykeeps no ceiling — it is one of the nineisTopologyalready checks, and bounding itwould 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:NaN,Infinityand1e9all run, and the publishedshardUtilizationis atmost 64 long
NaN,Infinityand1e9all runshardCapacityofNaNpublishes a realutilization, notNaNreader can reach
Against
mainthe file cannot finish: the worker exits rather than reporting.How I tested
Windows 11, Bun 1.3.14.
bun run testis 937 passed across 40 files, up from 929 by the eight newtests, with nothing else moving.
bun run typecheck,bun run lintandbun run format:checkareclean.
I have not touched
isTopology. Bounding it here puts the guard on the seam every path shares — adesign 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.