From 9f24d143d477cd0f7e486a8609c1b5d43799629c Mon Sep 17 00:00:00 2001 From: kevin9327 <5299031+kevin9327@users.noreply.github.com> Date: Sun, 6 Sep 2026 20:50:49 +0900 Subject: [PATCH] fix(sim): keep a region switch routing when its numbers are not numbers --- src/sim/behaviour-control.region.test.ts | 81 ++++++++++++++++++++++++ src/sim/behaviour-control.ts | 30 +++++++-- 2 files changed, 105 insertions(+), 6 deletions(-) create mode 100644 src/sim/behaviour-control.region.test.ts diff --git a/src/sim/behaviour-control.region.test.ts b/src/sim/behaviour-control.region.test.ts new file mode 100644 index 0000000..6305685 --- /dev/null +++ b/src/sim/behaviour-control.region.test.ts @@ -0,0 +1,81 @@ +import { describe, expect, it } from 'vitest'; +import { Engine } from './engine'; +import { makeNode } from './presets'; +import type { NodeStats, Topology } from './types'; + +/* + * `regions` and `activeRegion` are not among the nine config numbers + * `isTopology` checks, so a shared link, a `.breakscale` file and a restored + * session carry whatever they say into the region switch. + */ + +function topology(patch: Record): Topology { + const client = { ...makeNode('client', 0, 0), id: 'client' }; + client.config = { ...client.config, rps: 60 }; + const region = { ...makeNode('region', 200, 0), id: 'r' }; + region.config = { ...region.config, ...patch } as typeof region.config; + const east = { ...makeNode('service', 400, 0), id: 'east' }; + const west = { ...makeNode('service', 400, 120), id: 'west' }; + return { + nodes: [client, region, east, west], + edges: [ + { id: 'e1', from: 'client', to: 'r', weight: 1 }, + { id: 'e2', from: 'r', to: 'east', weight: 1 }, + { id: 'e3', from: 'r', to: 'west', weight: 1 }, + ], + }; +} + +function run(patch: Record) { + const engine = new Engine(topology(patch), 7); + for (let i = 0; i < 200; i += 1) engine.advance(1000 / 60); + const snapshot = engine.snapshot(); + return { + region: snapshot.nodes['r'] as NodeStats, + east: snapshot.nodes['east'] as NodeStats, + west: snapshot.nodes['west'] as NodeStats, + }; +} + +describe('a region switch given numbers the editor cannot produce', () => { + it('still routes when the active region is not a number', () => { + // `Math.floor(NaN)` is NaN and every comparison against it is false, so + // NaN used to be adopted as the live region -- and `out[NaN]` is nothing, + // so the switch served no region at all. + const { region, east, west } = run({ activeRegion: Number.NaN }); + expect(Number.isNaN(region.activeRegion as number)).toBe(false); + expect(east.totalCompleted + west.totalCompleted).toBeGreaterThan(0); + expect(region.totalFailed).toBe(0); + }); + + it('counts its regions when the declared count is not a number', () => { + // The census loop runs `i < count`, which is false immediately for NaN, + // so the node reported no healthy region while it was serving one. + const { region } = run({ regions: Number.NaN }); + expect(Number.isNaN(region.regionsTotal as number)).toBe(false); + expect(region.regionsHealthy).toBe(2); + }); + + it('keeps the readout and the routing agreeing', () => { + // decorateStats builds the census from the same predicate pickEdge routes + // with, so a node that is serving traffic cannot report zero healthy + // regions. That is the invariant the NaN broke. + const { region, east, west } = run({ regions: Number.NaN }); + expect(east.totalCompleted + west.totalCompleted).toBeGreaterThan(0); + expect(region.regionsHealthy).toBeGreaterThan(0); + }); + + it('leaves a design the editor can produce exactly where it was', () => { + const plain = run({}); + const explicit = run({ regions: 2, activeRegion: 0 }); + expect(plain.region.regionsTotal).toBe(2); + expect(plain.region.activeRegion).toBe(0); + expect(explicit.east.totalCompleted).toBe(plain.east.totalCompleted); + }); + + it('still honours a second region the student selected', () => { + const { east, west } = run({ activeRegion: 1 }); + expect(west.totalCompleted).toBeGreaterThan(0); + expect(east.totalCompleted).toBe(0); + }); +}); diff --git a/src/sim/behaviour-control.ts b/src/sim/behaviour-control.ts index 973713c..7246014 100644 --- a/src/sim/behaviour-control.ts +++ b/src/sim/behaviour-control.ts @@ -394,8 +394,28 @@ function asRegion(state: NodeStateLike): RegionState | null { /** How many of this node's out edges count as regions. */ function regionCount(state: NodeStateLike): number { - const declared = Math.floor(state.config.regions ?? state.out.length); - return Math.max(1, Math.min(declared, state.out.length)); + const declared = state.config.regions; + // NaN fails every comparison, so `Math.min` and `Math.max` hand it back + // unchanged. It is then published as the region total, while the census + // loop that counts healthy regions runs zero times -- and the readout is + // meant to be derived from the very predicate the routing uses. + if (!Number.isFinite(declared)) return state.out.length; + return Math.max(1, Math.min(Math.floor(declared as number), state.out.length)); +} + +/** + * The region the student asked for, clamped into the regions that exist. + * + * A value that is not a finite number is the first region rather than + * itself: `Math.floor(NaN)` is NaN, every comparison below is false for it, + * so it used to be adopted as the live region -- and `out[NaN]` is nothing, + * which makes the switch route nowhere at all. + */ +function configuredRegion(state: NodeStateLike, count: number): number { + const raw = state.config.activeRegion; + if (!Number.isFinite(raw)) return 0; + const configured = Math.floor(raw as number); + return configured < 0 ? 0 : configured >= count ? count - 1 : configured; } /** @@ -476,8 +496,7 @@ const region: ComponentBehaviour = { // between it and the live region -- after a failover the two differ by // design, and treating that as a manual switch would drag traffic back // onto the region that just died. - const configured = Math.floor(cfg.activeRegion ?? 0); - const wanted = configured < 0 ? 0 : configured >= count ? count - 1 : configured; + const wanted = configuredRegion(state, count); if (st.active === -1 || wanted !== st.lastConfigured) { st.lastConfigured = wanted; st.active = wanted; @@ -606,8 +625,7 @@ function liveRegionIndex( const active = st.failoverDueMs >= 0 ? st.failoverTarget : st.active; if (active < 0) { // Nothing has been adopted yet; pickEdge would take the configured region. - const configured = Math.floor(state.config.activeRegion ?? 0); - const wanted = configured < 0 ? 0 : configured >= count ? count - 1 : configured; + const wanted = configuredRegion(state, count); return regionHealthy(ctx, state, wanted) ? wanted : -1; }