From 6a807976cb858e3fb646b5700eb3e691e98a304f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 21:57:05 +0000 Subject: [PATCH 1/6] Keep delay and queue pipelines in step with their counts, and stop losing long loops Round-4 review findings 1, 5 and 6, plus a sibling of 1 found while fixing it. - A Delay's _queue and a Queue's _fifo ARE its resources in transit, but they were only ever built in reset(). The app's Run and Step buttons bootstrap at step 0 without resetting, so a freshly authored delay held its starting stock forever and released nothing. Both reset() and the step-0 bootstrap now seed the pipelines through _seedPipelines(). - The properties panel's Amount field and +/- steppers wrote the count through the model primitives, which know nothing about the pipeline. Editing a delay or queue mid-run left it owing the old amount: three '-' clicks on a delay holding ten in transit still delivered ten, creating three units from nothing; three '+' clicks stranded three forever. They now route through SimEngine.setLiveCount, which moves the pipeline with the count. This is the panel-side sibling of the trader guard added in the previous round. - The generated economy module's eco.set() wrote resources and let reconcile() backfill, typing every added unit untyped grey, so colour-filtered flows and converter recipes stopped accepting a pool the host game had just topped up. It now adds and removes through the primitives, keeping the pool's own type. - detectLoops chased cycles only ten nodes deep, so a twelve-stage ring economy reported 'No feedback loops found' when the diagram is nothing but one loop. The edge budget is what actually bounds the runtime (~12ms worst case on a dense 120-node graph), so the depth limit is now 32. The Loops panel and the CLI also stop claiming there are no loops when the search was cut short. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SH7ouoNymubArm7VEgALHg --- cli.js | 7 ++- js/app-analysis.js | 7 ++- js/app-props.js | 26 +++++++++-- js/codegen.js | 14 +++++- js/engine.js | 82 +++++++++++++++++++++++++++------ js/loops.js | 8 +++- test/run.js | 111 +++++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 234 insertions(+), 21 deletions(-) diff --git a/cli.js b/cli.js index 865e345..5ec697e 100644 --- a/cli.js +++ b/cli.js @@ -167,7 +167,12 @@ if (opts.toJson) { } if (opts.loops) { const { loops, truncated } = detectLoops(diagram); - if (!loops.length) process.stdout.write('No feedback loops found.\n'); + // A truncated search that found nothing has not shown there is nothing. + if (!loops.length) { + process.stdout.write(truncated + ? 'No feedback loops found within the search limit (the graph is large, so a longer loop could still exist).\n' + : 'No feedback loops found.\n'); + } for (const l of loops) { const chain = [...l.labels, l.labels[0]].join(' -> '); const linkStr = l.links.map(x => `${x.sign > 0 ? '+' : (x.sign < 0 ? '-' : '?')}${[...x.kinds].join('/')}`).join(', '); diff --git a/js/app-analysis.js b/js/app-analysis.js index c891c7a..90ae3a1 100644 --- a/js/app-analysis.js +++ b/js/app-analysis.js @@ -647,7 +647,12 @@ class AppAnalysis { if (!loops.length) { const p = document.createElement('p'); p.className = 'props-empty'; - p.textContent = 'No feedback loops found. Loops appear when influence returns to where it started, for example a pool feeding a converter whose output flows back, or a register that modifies the pool it reads.'; + // A truncated search that found nothing has not shown there is nothing: + // saying so flatly told users of one long ring that their diagram had no + // loops at all. + p.textContent = truncated + ? 'No feedback loops found within the search limit. This graph is large enough that the search stopped early, so a longer or more tangled loop could still exist. Try splitting the diagram into smaller pieces to check.' + : 'No feedback loops found. Loops appear when influence returns to where it started, for example a pool feeding a converter whose output flows back, or a register that modifies the pool it reads.'; panel.appendChild(p); return; } diff --git a/js/app-props.js b/js/app-props.js index ebf18e8..7d84f6c 100644 --- a/js/app-props.js +++ b/js/app-props.js @@ -1407,6 +1407,27 @@ class AppProps { panel.appendChild(delRow); } + // Apply an amount edit from the properties panel. Mid-run a Delay's or a + // Queue's count is not free-standing state: it is the total of the in-flight + // pipeline (`_queue`, or `_fifo` plus the units in service), which the model + // primitives know nothing about. Editing the count alone leaves the pipeline + // owing the old amount, so it releases resources the node no longer has, or + // strands ones nothing will ever release. At rest the count is the starting + // baseline and the engine seeds the pipeline from it when the run begins, so + // the plain primitives are right there. + // `nudge` keeps a mixed holding's colours intact for the +/- steppers; the + // typed field deliberately retypes the whole amount to a single colour. + _setLiveAmount(node, target, color, nudge = false) { + if (this.engine.step > 0 && (node.type === NodeType.DELAY || node.type === NodeType.QUEUE)) { + this.engine.setLiveCount(node, target, color); + return; + } + if (!nudge) { node.setCount(Math.max(0, target), color); return; } + const delta = Math.max(0, target) - node.resources; + if (delta > 0) node.addResources(delta, color); + else if (delta < 0) node.takeResources(-delta); + } + _nodeProps(panel, node) { const typeColor = (typeof NODE_STROKE !== 'undefined' && NODE_STROKE[node.type]) || 'var(--accent)'; this._titleTyped(panel, `${node.type} node`, node.label || '(unnamed)', typeColor, `node-${node.type}`); @@ -1460,7 +1481,7 @@ class AppProps { // came back to a number typed mid-run rather than the run's start. const baseCount = node._initialResources; const baseMap = { ...node._initialColorMap }; - node.setCount(Math.max(0, parseInt(v) || 0), color); + this._setLiveAmount(node, Math.max(0, parseInt(v) || 0), color); node._initialResources = baseCount; node._initialColorMap = baseMap; } else { @@ -1489,8 +1510,7 @@ class AppProps { b.addEventListener('click', () => { if (delta < 0 && node.resources <= 0) return; if (delta > 0 && node.capacity !== Infinity && node.resources >= node.capacity) return; - if (delta < 0) node.takeResources(1); - else node.addResources(1, node.displayColor || DEFAULT_COLOR); + this._setLiveAmount(node, node.resources + delta, node.displayColor || DEFAULT_COLOR, true); this.renderer.render(); this._refreshResourceCount(); this._refreshTypeReadouts(); diff --git a/js/codegen.js b/js/codegen.js index 6b40b4c..09f85af 100644 --- a/js/codegen.js +++ b/js/codegen.js @@ -85,8 +85,18 @@ function createEconomy(opts = {}) { if (!n) throw new Error('set(): no node labeled ' + JSON.stringify(name)); if (n.type === NodeType.REGISTER) { n.value = Number(v) || 0; return api; } if (n.type === NodeType.POOL || (n.type === NodeType.SOURCE && n.limited)) { - n.resources = Math.max(0, Number(v) || 0); - n.reconcile(); + // Keep the node's own resource type. Writing \`resources\` and letting + // reconcile() backfill made every added unit untyped grey, so a + // colour-filtered connection or a converter recipe stopped accepting + // the contents of a pool the host game had just topped up. + const want = Math.max(0, Number(v) || 0); + const delta = want - n.resources; + if (delta > 0) { + n.addResources(delta, dominantColor(n.colorMap) + || dominantColor(n._initialColorMap || {}) || n.resourceColor || DEFAULT_COLOR); + } else if (delta < 0) { + n.takeResources(-delta); + } return api; } throw new Error('set() supports pools, limited sources and registers'); diff --git a/js/engine.js b/js/engine.js index c1706f0..9432696 100644 --- a/js/engine.js +++ b/js/engine.js @@ -31,6 +31,72 @@ class SimEngine { } } + // Rebuild the in-flight pipelines from what each node currently holds. A + // Delay's `_queue` and a Queue's `_fifo` ARE its resources in transit: a count + // with no pipeline behind it never releases, and a pipeline with no count + // behind it releases resources the node does not have. Both reset() and the + // step-0 bootstrap seed them from colorMap, so a freshly authored delay + // releases its starting stock on Run instead of stranding it forever (the app + // does not reset() before Run). At step 0 the count is the baseline, so + // rebuilding from it is always correct and safe to repeat. + _seedPipelines() { + for (const n of this.diagram.nodes.values()) { + if (n.type === NodeType.DELAY) { + n._queue = Object.entries(n.colorMap).filter(([, a]) => a > 0) + .map(([color, amount]) => ({ amount, color, stepsLeft: n.delay })); + } else if (n.type === NodeType.QUEUE) { + // Units already in service are re-lined rather than kept mid-service: + // at a run start nothing has been served yet. + n._procs = []; + n._fifo = Object.entries(n.colorMap).filter(([, a]) => a > 0) + .map(([color, amount]) => ({ amount, color, enq: 0 })); + } + } + } + + // Set a Delay's or Queue's live count while keeping its pipeline in step with + // it. Their count is not free-standing state, so writing `resources` alone + // (what setCount/addResources/takeResources do) desyncs the two: the pipeline + // still owes what it held, so the difference is released out of nothing, or + // stranded forever with nothing left to release it. Returns the applied delta. + setLiveCount(node, target, color = DEFAULT_COLOR) { + const want = Math.max(0, Math.round(target)); + const delta = want - node.resources; + if (!isFinite(delta) || delta === 0) return 0; + if (delta > 0) { + node.addResources(delta, color); + if (node.type === NodeType.DELAY) { + (node._queue = node._queue || []) + .push({ amount: delta, color, stepsLeft: Math.max(1, Math.round(node.delay || 1)) }); + } else { + (node._fifo = node._fifo || []).push({ amount: delta, color, enq: this.step }); + } + return delta; + } + // Removing takes from the newest end first, so a "-" undoes the most recent + // arrival rather than something that was about to be released. + let rem = -delta; + const drop = (list) => { + for (let i = list.length - 1; i >= 0 && rem > 0; i--) { + const take = Math.min(list[i].amount, rem); + list[i].amount -= take; rem -= take; + node.takeResources(take, list[i].color); + if (list[i].amount <= 0) list.splice(i, 1); + } + }; + if (node.type === NodeType.DELAY) drop(node._queue = node._queue || []); + else { + drop(node._fifo = node._fifo || []); + const procs = node._procs = node._procs || []; + while (rem > 0 && procs.length) { + const p = procs.pop(); + node.takeResources(1, p.color); + rem--; + } + } + return delta + rem; // negative: what was actually removed + } + reset() { this.stop(); // Apply the diagram's run seed (or clear back to Math.random when unset) so @@ -51,28 +117,16 @@ class SimEngine { const infiniteSource = n.type === NodeType.SOURCE && !n.limited; n.resources = n._initialResources ?? (infiniteSource ? Infinity : 0); n.colorMap = { ...(n._initialColorMap || {}) }; - if (n.type === NodeType.DELAY) { - // Rebuild in-flight batches from any pre-loaded resources (e.g. a - // diagram saved mid-run keeps its counts but not its _queue): one - // batch per color holding the full amount, releasing after the node's - // delay — mirrors the QUEUE _fifo rebuild below. Without this the - // resources would sit in the node forever, never released. - n._queue = Object.entries(n.colorMap).filter(([, a]) => a > 0) - .map(([color, amount]) => ({ amount, color, stepsLeft: n.delay })); - } if (n.type === NodeType.QUEUE) { - n._procs = []; n.processed = 0; n.totalWait = 0; n.maxWait = 0; n.maxLen = 0; n.balked = 0; n.reneged = 0; - // Rebuild the FIFO from any pre-loaded starting resources (treated as - // enqueued at the run start, step 0). - n._fifo = Object.entries(n.colorMap).filter(([, a]) => a > 0).map(([color, amount]) => ({ amount, color, enq: 0 })); } if (n.type === NodeType.SOURCE) n.produced = 0; if (n.type === NodeType.DRAIN) n.drained = 0; if (n.type === NodeType.REGISTER) n.value = 0; if (n.type === NodeType.TRADER) n.trades = 0; } + this._seedPipelines(); this.diagram.variables = {}; // Per-connection trigger counters (for "every Nth firing" triggers). this._trigCounts = new Map(); @@ -148,6 +202,7 @@ class SimEngine { doStep() { if (this.step === 0) { this.saveInitial(); + this._seedPipelines(); this._updateVariables(); this._evalRegisters(); } @@ -183,6 +238,7 @@ class SimEngine { this._sampleCustomVars('play'); if (this.step === 0) { this.saveInitial(); + this._seedPipelines(); this._updateVariables(); this._evalRegisters(); } diff --git a/js/loops.js b/js/loops.js index 5d653aa..0f9c6c6 100644 --- a/js/loops.js +++ b/js/loops.js @@ -77,7 +77,13 @@ function _loopOpSign(op) { function detectLoops(diagram, opts = {}) { const maxLoops = opts.maxLoops ?? 100; - const maxLen = opts.maxLen ?? 10; + // Longest cycle the DFS will chase. A ring economy (Stage1 -> ... -> StageN + // -> Stage1) is a perfectly ordinary diagram and was silently invisible at + // the old limit of 10, so the panel reported "no feedback loops" for a + // diagram that is nothing but one. The `budget` below is what actually bounds + // the runtime (~12ms worst case measured on a dense 120-node graph), so the + // depth limit can afford to be generous. + const maxLen = opts.maxLen ?? 32; const d = diagram; // Publishers: variable name → node id whose value the engine writes there. diff --git a/test/run.js b/test/run.js index f656386..5874577 100644 --- a/test/run.js +++ b/test/run.js @@ -804,6 +804,92 @@ test('a trader cannot pay out of a delay or a queue', () => { } }); +test('detectLoops finds a ring economy longer than ten nodes', () => { + // The depth limit was 10, so a 12-stage production ring - one loop and + // nothing else - enumerated no cycles at all and the Loops panel reported + // "No feedback loops found" for a diagram that is entirely one loop. + const ring = (n) => { + const d = new Diagram(); + const ns = []; + for (let i = 0; i < n; i++) { + const p = new MNode(NodeType.POOL, i * 60, 0); p.label = 'Stage' + (i + 1); + p.setCount(5); d.addNode(p); ns.push(p); + } + for (let i = 0; i < n; i++) { + d.addConnection(new MConnection(ns[i].id, ns[(i + 1) % n].id, ConnectionType.RESOURCE)); + } + return d; + }; + for (const n of [12, 20, 30]) { + const { loops, truncated } = detectLoops(ring(n)); + eq(loops.length, 1, `${n}-stage ring: exactly one cycle`); + eq(loops[0].nodes.length, n, `${n}-stage ring: the whole ring`); + eq(loops[0].type, 'F', `${n}-stage ring: a pure resource circulation`); + eq(truncated, false, `${n}-stage ring: search was not cut short`); + } + // Past the limit the caller is told the search stopped early rather than + // being handed a bare empty list. + const big = detectLoops(ring(40)); + eq(big.loops.length, 0, 'a 40-stage ring is past the depth limit'); + eq(big.truncated, true, 'and reports that the search was truncated'); +}); + +test('a delay or queue releases its authored starting stock without a Reset first', () => { + // _queue/_fifo were only ever built in reset(), but the app's Run and Step + // buttons do not reset: they bootstrap at step 0 with saveInitial() alone. So + // a freshly authored delay held its stock forever, releasing nothing. + for (const kind of [NodeType.DELAY, NodeType.QUEUE]) { + const d = new Diagram(); + const belt = new MNode(kind, 0, 0); belt.label = 'Belt'; + belt.delay = 2; belt.processTime = 2; belt.servers = 4; + belt.setCount(10, '#8d6e63'); + const out = new MNode(NodeType.POOL, 200, 0); out.label = 'Out'; + d.addNode(belt); d.addNode(out); + const c = new MConnection(belt.id, out.id, ConnectionType.RESOURCE); c.rate = 99; + d.addConnection(c); + + const e = new SimEngine(d); // deliberately NO reset(), like pressing Run + for (let i = 0; i < 30; i++) e.doStep(); + eq(out.resources, 10, `${kind}: the starting stock reaches the output`); + eq(belt.resources, 0, `${kind}: nothing is stranded in the node`); + } +}); + +test('setLiveCount moves a delay or queue pipeline, not just the count', () => { + // The properties panel's Amount field and +/- steppers reach for the model + // primitives, which write `resources` and leave `_queue`/`_fifo` owing the old + // amount: the node then releases units it no longer has (created from + // nothing), or keeps ones nothing will ever release. Same hazard as the + // trader guard, at the panel's call site. + const pipeline = (n) => (n._queue || []).reduce((s, b) => s + b.amount, 0) + + (n._fifo || []).reduce((s, b) => s + b.amount, 0) + (n._procs || []).length; + + for (const kind of [NodeType.DELAY, NodeType.QUEUE]) { + for (const [nudge, want] of [[-3, 7], [3, 13]]) { + const d = new Diagram(); + const belt = new MNode(kind, 0, 0); belt.label = 'Belt'; + belt.delay = 4; belt.processTime = 4; belt.servers = 4; + belt.setCount(10, '#8d6e63'); + const out = new MNode(NodeType.POOL, 200, 0); out.label = 'Out'; + d.addNode(belt); d.addNode(out); + const c = new MConnection(belt.id, out.id, ConnectionType.RESOURCE); c.rate = 99; + d.addConnection(c); + + const e = new SimEngine(d); e.reset(); + e.doStep(); e.doStep(); // everything is in transit inside the node + eq(pipeline(belt), belt.resources, `${kind}: pipeline agrees before the edit`); + for (let k = 0; k < Math.abs(nudge); k++) { + e.setLiveCount(belt, belt.resources + Math.sign(nudge), '#8d6e63'); + } + eq(belt.resources, want, `${kind} ${nudge}: the count follows the edit`); + eq(pipeline(belt), belt.resources, `${kind} ${nudge}: pipeline follows too`); + for (let i = 0; i < 60; i++) e.doStep(); + eq(belt.resources + out.resources, want, + `${kind} ${nudge}: nothing created or stranded over the whole run`); + } + } +}); + test('.econ round-trip survives a node labelled with a DSL head keyword', () => { // dslParse dispatches on tokens[0] before scanning for an arrow, so a node // labelled `pool` that is the SOURCE of a connection emitted `pool -> Gold` @@ -3370,6 +3456,31 @@ test('generated module simulates the embedded economy', () => { eq(eco.get('Gold'), 0, 'reset restores the baseline'); }); +test('generated module set() keeps a pool\'s resource type', () => { + // set() wrote `resources` and let reconcile() backfill the difference, which + // types every added unit DEFAULT_COLOR grey. A colour-filtered connection or + // a converter recipe then refused the units the host game had just added. + const WOOD = '#8d6e63'; + const { d } = setup(); + const p = node(d, NodeType.POOL); p.label = 'Warehouse'; p.setCount(10, WOOD); + const mill = node(d, NodeType.POOL); mill.label = 'Mill'; + const c = conn(d, p, mill); c.rate = 5; c.colorFilter = WOOD; + + const Economy = buildTestModule(d.toJSON()); + const eco = Economy.createEconomy(); + eco.set('Warehouse', 50); + eq(eco.get('Warehouse'), 50, 'set() applies the amount'); + eco.run(20); + eq(eco.get('Mill'), 50, 'the colour-filtered flow accepts every unit set() added'); + eq(eco.get('Warehouse'), 0, 'and the warehouse empties'); + + // Setting downward must not invent a type either. + const eco2 = Economy.createEconomy(); + eco2.set('Warehouse', 4); + eco2.run(20); + eq(eco2.get('Mill'), 4, 'trimming leaves only typed units behind'); +}); + test('generated module honors seed and param overrides deterministically', () => { const { d } = setup(); const s = node(d, NodeType.SOURCE); s.label = 'Mine'; From e8d9fbe222ecf17cf54d3415afda0f2e72dca752 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 22:03:46 +0000 Subject: [PATCH 2/6] Make undo, redo and File > Open reach the autosave slot Round-4 review findings 7 and 8. - undo() and redo() moved _lastState and repainted the canvas but never wrote the autosave slot, which only _commit() and _commitReplace() did. An accidental Ctrl+A Delete looked repaired by Ctrl+Z, and the next reload brought back the deletion with the undo stack gone, so it was permanent. All four paths now go through _persistAutosave(). - File > Open replaced everything on the canvas with no confirmation, and reset the history instead of committing the swap, so Ctrl+Z did nothing and the replaced work could not be recovered. It now asks first (only once the file has parsed, so a cancelled or invalid pick never nags) and commits the swap like New, Load template and Load from library already do. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SH7ouoNymubArm7VEgALHg --- js/app.js | 33 ++++++++++++++++++++++---- test/smoke.js | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 5 deletions(-) diff --git a/js/app.js b/js/app.js index d3b9c09..8585b71 100644 --- a/js/app.js +++ b/js/app.js @@ -513,7 +513,15 @@ class App { this._redoStack = []; this._lastState = snap; this._updateUndoButtons(); - try { localStorage.setItem('sim_autosave', this._lastState); } catch {} + this._persistAutosave(); + } + + // Mirror the current state into the autosave slot. Every path that changes + // what is on the canvas has to call this: undo and redo moved _lastState + // without it, so an undo looked repaired on screen and was thrown away on the + // next reload, taking the mistake it had just undone with it. + _persistAutosave() { + try { localStorage.setItem('sim_autosave', this._lastState); } catch { /* blocked storage */ } } // Record that the diagram changed (push the previous state onto the stack). @@ -534,7 +542,7 @@ class App { this._redoStack = []; this._lastState = snap; this._updateUndoButtons(); - try { localStorage.setItem('sim_autosave', this._lastState); } catch {} + this._persistAutosave(); // Mark any open MC results as potentially stale since the diagram changed. this._markMCStale(); // A structural edit may satisfy the current tour step (placed a node / drew @@ -556,6 +564,7 @@ class App { this._redoStack.push(this._lastState); this._lastState = this._undoStack.pop(); this._restoreState(this._lastState); + this._persistAutosave(); this._updateUndoButtons(); } @@ -564,6 +573,7 @@ class App { this._undoStack.push(this._lastState); this._lastState = this._redoStack.pop(); this._restoreState(this._lastState); + this._persistAutosave(); this._updateUndoButtons(); } @@ -789,7 +799,7 @@ class App { // the URL is the document, and an embed must not write over the host // page's autosave. if (!document.body.classList.contains('embed')) { - try { localStorage.setItem('sim_autosave', this._lastState); } catch { /* blocked storage */ } + this._persistAutosave(); try { history.replaceState(null, '', location.pathname + location.search); } catch { /* ignore */ } } return; @@ -1391,7 +1401,7 @@ class App { const file = e.target.files[0]; if (!file) return; const reader = new FileReader(); - reader.onload = ev => { + reader.onload = async ev => { // Parse + validate on a throwaway Diagram BEFORE touching the current // one: loadJSON clears everything first, so a corrupt file would // otherwise wreck the diagram (and the next autosave persists that). @@ -1405,6 +1415,19 @@ class App { this._toast(`Invalid file: ${err.message}. Your current diagram is unchanged.`); return; } + // Opening a file replaces everything on the canvas, exactly like New, + // Load template and Load from library. It asked for none of their + // confirmation and, by resetting the history instead of committing + // the swap, left no way back: Ctrl+Z did nothing and the work was + // gone. Ask first, and keep the previous diagram one undo away. + // The guard only runs once the file has parsed, so a mistyped or + // cancelled pick never nags. + if (this.diagram.nodes.size || this.diagram.notes.size) { + if (!await this._confirmGuard( + `Open "${file.name}"? Your current diagram will be replaced (Ctrl+Z to undo).`, + 'Open file')) return; + } + const prev = this._snapshot(); this.diagram.loadJSON(data); this._applyMeta(); this.engine.reset(); @@ -1414,7 +1437,7 @@ class App { this.editor._select(null, null); this.renderer.render(); this.renderer.fitView(); - this._resetHistory(); + this._commitReplace(prev); }; reader.readAsText(file); }; diff --git a/test/smoke.js b/test/smoke.js index c7df6d1..b626c6f 100644 --- a/test/smoke.js +++ b/test/smoke.js @@ -2588,6 +2588,70 @@ const URL = process.env.SMOKE_URL || 'http://localhost:8080/'; ok(`components: save selection (${comp.compNodes} nodes, ${comp.compConns} conn), insert adds 2 nodes, undo reverts`); else fail('components: ' + JSON.stringify(comp)); + // Persistence: undo and redo have to reach autosave. They moved _lastState + // and repainted the canvas but never wrote it, so an accidental Delete looked + // repaired by Ctrl+Z and came back on the next reload with the deletion intact + // and the undo stack gone. + const undoSave = await page.evaluate(() => { + const app = window.app; + const saved = () => { + try { return (JSON.parse(localStorage.getItem('sim_autosave') || '{}').nodes || []).length; } + catch { return -1; } + }; + app._clearAll(); app._commit(); + app._loadDemo(); app._commit(); + const built = app.diagram.nodes.size; + for (const id of [...app.diagram.nodes.keys()]) app.diagram.removeNode(id); + app.renderer.render(); app._commit(); + const afterDelete = saved(); + app.undo(); + const onCanvas = app.diagram.nodes.size, inStorage = saved(); + app.redo(); + return { built, afterDelete, onCanvas, inStorage, afterRedo: saved() }; + }); + if (undoSave.built > 0 && undoSave.afterDelete === 0 + && undoSave.onCanvas === undoSave.built && undoSave.inStorage === undoSave.built + && undoSave.afterRedo === 0) + ok(`persistence: undo and redo write autosave (${undoSave.built} nodes back on canvas and in storage)`); + else fail('undo autosave: ' + JSON.stringify(undoSave)); + + // Persistence: File > Open replaces everything on the canvas, so it must ask + // first and stay undoable, like New / Load template / Load from library. It + // did neither, and reset the history, so the replaced work was unrecoverable. + const openFile = await (async () => { + await page.evaluate(() => { + const app = window.app; + app._clearAll(); app._loadDemo(); app._commit(); + // Record the guard instead of racing the overlay open: what matters is + // that Open goes through it at all, and with the file named. + window.__guard = null; + app.__realGuard = app._confirmGuard; + app._confirmGuard = (msg, title) => { window.__guard = { msg, title }; return Promise.resolve(true); }; + }); + const before = await page.evaluate(() => window.app.diagram.nodes.size); + const payload = JSON.stringify({ version: 1, nodes: [{ id: 'n1', type: 'pool', x: 100, y: 100, label: 'Imported' }], connections: [] }); + const [chooser] = await Promise.all([ + page.waitForEvent('filechooser'), + page.evaluate(() => document.getElementById('btn-load').click()), + ]); + await chooser.setFiles({ name: 'imported.json', mimeType: 'application/json', buffer: Buffer.from(payload) }); + await page.waitForFunction(() => window.app.diagram.nodes.size === 1, { timeout: 3000 }).catch(() => {}); + const r = await page.evaluate(() => { + const app = window.app; + const afterOpen = app.diagram.nodes.size; + const undoable = !document.getElementById('btn-undo').disabled; + app.undo(); + const afterUndo = app.diagram.nodes.size; + app._confirmGuard = app.__realGuard; delete app.__realGuard; + return { guard: window.__guard, afterOpen, undoable, afterUndo }; + }); + return { before, ...r }; + })(); + if (openFile.guard && /imported\.json/.test(openFile.guard.msg) && openFile.afterOpen === 1 + && openFile.undoable && openFile.afterUndo === openFile.before) + ok('persistence: File > Open confirms first and stays undoable'); + else fail('File > Open: ' + JSON.stringify(openFile)); + // UX pass: first-run welcome overlay shows for a brand-new user, dismisses, // and sets the seen flag (use a fresh context with no suppression). const welcome = await (async () => { From 1c6264a42999910ca4fd2f86bd87f53d29a56662 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 22:06:08 +0000 Subject: [PATCH 3/6] Keep a paused seeded run reproducible, and seed the sweep range from the parameter picked Round-4 review findings 9 and 10. - Batch Analysis stops the live run without resetting it, so the run resumes on the shared RNG. Every trial reseeds that RNG, and the batch cleared it back to Math.random when it finished, so a paused seeded run silently completed its remaining steps unseeded: the seed no longer reproduced the run. The batch now saves the stream position before its first trial and restores it after the last one, cancelled batches included. - The sweep's from/to were seeded once, from the first parameter in the list, and nothing re-ran that when the user chose a different parameter. Sweeping a capacity of 500 while the first parameter was a rate of 3 swept 1.5 to 4.5. The range now re-seeds on change, and a parameter sitting at zero gets a 0 to 1 range instead of a degenerate 0 to 0. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SH7ouoNymubArm7VEgALHg --- js/app-analysis.js | 22 ++++++++++++++++++---- js/app.js | 3 +++ js/engine.js | 8 +++++++- test/run.js | 34 ++++++++++++++++++++++++++++++++++ test/smoke.js | 29 +++++++++++++++++++++++++++++ 5 files changed, 91 insertions(+), 5 deletions(-) diff --git a/js/app-analysis.js b/js/app-analysis.js index 90ae3a1..8c79051 100644 --- a/js/app-analysis.js +++ b/js/app-analysis.js @@ -28,10 +28,7 @@ class AppAnalysis { sel.disabled = false; document.getElementById('mc-sweep-run').disabled = false; for (const n of names) sel.appendChild(new Option(n, n)); - // Seed the range around the parameter's current value. - const cur = this.diagram.params[names[0]]; - document.getElementById('mc-sweep-from').value = Math.round(cur * 0.5 * 100) / 100; - document.getElementById('mc-sweep-to').value = Math.round(cur * 1.5 * 100) / 100; + this._seedSweepRange(names[0]); } // Sensitivity needs at least one parameter with a non-zero value (a percent // perturbation of 0 is a no-op). @@ -44,6 +41,23 @@ class AppAnalysis { this._showModal('mc-overlay'); } + // Seed the sweep's from/to around a parameter's current value. This ran once, + // for the first parameter in the list, and nothing re-ran it when the user + // picked a different one: sweeping any other parameter silently used the + // first one's range, which for a rate of 3 against a capacity of 500 meant + // sweeping 1.5 to 4.5. + _seedSweepRange(name) { + const cur = this.diagram.params[name]; + if (!isFinite(cur)) return; + const round = v => Math.round(v * 100) / 100; + // A parameter sitting at 0 has no scale to spread around, so offer a small + // absolute range rather than 0 to 0. + const from = cur === 0 ? 0 : round(cur * 0.5); + const to = cur === 0 ? 1 : round(cur * 1.5); + document.getElementById('mc-sweep-from').value = from; + document.getElementById('mc-sweep-to').value = to; + } + _mcSeed() { return document.getElementById('mc-seed').value.trim(); } diff --git a/js/app.js b/js/app.js index 8585b71..0ddacd0 100644 --- a/js/app.js +++ b/js/app.js @@ -1570,6 +1570,9 @@ class App { this._modalize('mc-overlay'); document.getElementById('mc-run').addEventListener('click', () => this._runMonteCarlo()); document.getElementById('mc-sweep-run').addEventListener('click', () => this._runSweep()); + // Re-seed the sweep range when the parameter changes, not just on open. + document.getElementById('mc-sweep-param') + .addEventListener('change', (e) => this._seedSweepRange(e.target.value)); document.getElementById('mc-sens-run').addEventListener('click', () => this._runSensitivity()); // Touch layout ☰ overflow: the controls the collapsed topbar hides diff --git a/js/engine.js b/js/engine.js index 9432696..07452b1 100644 --- a/js/engine.js +++ b/js/engine.js @@ -1535,6 +1535,12 @@ class SimEngine { const endSteps = []; let endedCount = 0; const seeded = opts.seed != null && opts.seed !== ''; + // Every trial reseeds the shared RNG, so save the live run's stream position + // and put it back when the batch is done. Clearing it instead (the old + // `seed(null)`) dropped a paused seeded run onto Math.random: its remaining + // steps stopped being reproducible from its seed, with nothing on screen to + // say so. + const rngBefore = SimRandom.getState(); try { for (let r = 0; r < runs; r++) { @@ -1564,7 +1570,7 @@ class SimEngine { yield { done: r + 1, total: runs }; } } finally { - if (seeded) SimRandom.seed(null); // never leak a seeded RNG into live runs + SimRandom.setState(rngBefore); // never leak a batch's RNG into live runs } return { diff --git a/test/run.js b/test/run.js index 5874577..84df265 100644 --- a/test/run.js +++ b/test/run.js @@ -804,6 +804,40 @@ test('a trader cannot pay out of a delay or a queue', () => { } }); +test('a Monte Carlo batch leaves a paused seeded run where it found it', () => { + // Batch Analysis stops the live run but does not reset it, so the run resumes + // on the shared RNG. Every trial reseeds that RNG and the batch used to clear + // it back to Math.random, so a paused seeded run silently finished its + // remaining steps unseeded and unreproducible. + const build = () => { + const d = new Diagram(); + d.seed = 'live-42'; + const src = new MNode(NodeType.SOURCE, 0, 0); src.label = 'Mine'; + const pool = new MNode(NodeType.POOL, 200, 0); pool.label = 'Gold'; + d.addNode(src); d.addNode(pool); + const c = new MConnection(src.id, pool.id, ConnectionType.RESOURCE); + c.rateMode = RateMode.DICE; c.dice = '2d6'; + d.addConnection(c); + return { d, pool }; + }; + + // Reference: a seeded run of 20 steps, uninterrupted. + const a = build(); + const ea = new SimEngine(a.d); ea.reset(); + for (let i = 0; i < 20; i++) ea.doStep(); + const want = a.pool.resources; + + for (const batchSeed of ['mc-seed', null]) { + const b = build(); + const eb = new SimEngine(b.d); eb.reset(); + for (let i = 0; i < 10; i++) eb.doStep(); + eb.runMonteCarlo(5, 8, { seed: batchSeed }); + for (let i = 0; i < 10; i++) eb.doStep(); + eq(b.pool.resources, want, + `batch seed ${batchSeed}: the paused run resumes on its own seeded stream`); + } +}); + test('detectLoops finds a ring economy longer than ten nodes', () => { // The depth limit was 10, so a 12-stage production ring - one loop and // nothing else - enumerated no cycles at all and the Loops panel reported diff --git a/test/smoke.js b/test/smoke.js index b626c6f..afc289a 100644 --- a/test/smoke.js +++ b/test/smoke.js @@ -2588,6 +2588,35 @@ const URL = process.env.SMOKE_URL || 'http://localhost:8080/'; ok(`components: save selection (${comp.compNodes} nodes, ${comp.compConns} conn), insert adds 2 nodes, undo reverts`); else fail('components: ' + JSON.stringify(comp)); + // Analysis: the sweep range follows the chosen parameter. It was seeded once, + // from the first parameter in the list, and nothing re-ran it on change, so + // sweeping any other parameter ran over the first one's range. + const sweepRange = await page.evaluate(async () => { + const app = window.app; + app._clearAll(); + app.diagram.params = { mine_rate: 3, capacity: 500, upkeep: 0 }; + app._openMonteCarlo(); + const sel = document.getElementById('mc-sweep-param'); + const read = () => ({ + from: parseFloat(document.getElementById('mc-sweep-from').value), + to: parseFloat(document.getElementById('mc-sweep-to').value), + }); + const onOpen = read(); + sel.value = 'capacity'; + sel.dispatchEvent(new Event('change', { bubbles: true })); + const onCapacity = read(); + sel.value = 'upkeep'; + sel.dispatchEvent(new Event('change', { bubbles: true })); + const onZero = read(); + app._hideModal('mc-overlay'); + return { onOpen, onCapacity, onZero, options: [...sel.options].map(o => o.value) }; + }); + if (sweepRange.onOpen.from === 1.5 && sweepRange.onOpen.to === 4.5 + && sweepRange.onCapacity.from === 250 && sweepRange.onCapacity.to === 750 + && sweepRange.onZero.from === 0 && sweepRange.onZero.to === 1) + ok('analysis: the sweep range re-seeds from the parameter you pick'); + else fail('sweep range: ' + JSON.stringify(sweepRange)); + // Persistence: undo and redo have to reach autosave. They moved _lastState // and repainted the canvas but never wrote it, so an accidental Delete looked // repaired by Ctrl+Z and came back on the next reload with the deletion intact From 8bf31622d97f2d3a6045546cde7edd310ecef0aa Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 22:09:35 +0000 Subject: [PATCH 4/6] Make the CLI reject malformed numbers and stop printing source notes as help Round-4 review findings 3 and 4. - parseFloat and parseInt stop at the first character they cannot use, so --param carrying=1,000 ran with a carrying capacity of 1 and exited 0. On the Predator and Prey template that is the difference between a stable run and both populations dying, with nothing on stdout to say the value was not the one asked for. --param, --steps, --runs and --pass-rate now parse strictly and fail as usage errors (exit 1) on anything that is not a complete number. - --help printed every line in cli.js starting with //, so after the Examples block it trailed into implementation notes from the middle of the file. It now prints the header block only, stopping at the first non-comment line. The two em dashes in that block are gone as well: it is copy a user reads. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SH7ouoNymubArm7VEgALHg --- cli.js | 35 +++++++++++++++++++++++++++-------- test/run.js | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 8 deletions(-) diff --git a/cli.js b/cli.js index 5ec697e..971489a 100644 --- a/cli.js +++ b/cli.js @@ -1,5 +1,5 @@ #!/usr/bin/env node -// Headless CLI runner — simulate a diagram without a browser. +// Headless CLI runner. Simulate a diagram without a browser. // // node cli.js [options] // @@ -7,7 +7,7 @@ // --steps N steps to simulate (default 200) // --runs N Monte Carlo: run N isolated trials and print summary // stats instead of a single-run trace (default 1) -// --seed S seed the RNG — same seed, same results +// --seed S seed the RNG (same seed, same results) // --param name=val override a diagram parameter (repeatable) // --csv with --runs>1: print raw per-run final values as CSV // (one row per run) instead of the stats table @@ -78,6 +78,18 @@ function fail(msg) { process.exit(1); } +// Strict numeric option parse. parseFloat/parseInt stop at the first character +// they cannot use, so `--param carrying=1,000` quietly ran with a carrying +// capacity of 1 and `--steps 30x` with 30, both exiting 0 on a different +// simulation than the one asked for. A malformed value is a usage error. +function num(raw, what) { + const v = String(raw ?? '').trim(); + if (!/^[+-]?(\d+\.?\d*|\.\d+)([eE][+-]?\d+)?$/.test(v)) { + fail(`${what} expects a number, got "${raw ?? ''}"`); + } + return parseFloat(v); +} + function parseArgs(argv) { const opts = { steps: 200, runs: 1, seed: null, params: {}, csv: false, file: null, @@ -86,8 +98,8 @@ function parseArgs(argv) { }; for (let i = 0; i < argv.length; i++) { const a = argv[i]; - if (a === '--steps') opts.steps = parseInt(argv[++i], 10); - else if (a === '--runs') opts.runs = parseInt(argv[++i], 10); + if (a === '--steps') opts.steps = num(argv[++i], '--steps'); + else if (a === '--runs') opts.runs = num(argv[++i], '--runs'); else if (a === '--seed') opts.seed = argv[++i]; else if (a === '--csv') opts.csv = true; else if (a === '--assert') opts.asserts.push(argv[++i]); @@ -102,7 +114,7 @@ function parseArgs(argv) { } } else if (a === '--check') opts.check = true; - else if (a === '--pass-rate') opts.passRate = parseFloat(argv[++i]); + else if (a === '--pass-rate') opts.passRate = num(argv[++i], '--pass-rate'); else if (a === '--emit') opts.emit = argv[++i]; else if (a === '--to-dsl') opts.toDsl = true; else if (a === '--to-json') opts.toJson = true; @@ -111,10 +123,17 @@ function parseArgs(argv) { else if (a === '--param') { const m = String(argv[++i] || '').match(/^([^=]+)=(.+)$/); if (!m) fail(`--param expects name=value, got "${argv[i]}"`); - opts.params[m[1]] = parseFloat(m[2]); + opts.params[m[1]] = num(m[2], `--param ${m[1]}`); } else if (a === '--help' || a === '-h') { - process.stdout.write(fs.readFileSync(__filename, 'utf8').split('\n') - .filter(l => l.startsWith('//')).map(l => l.slice(3)).join('\n') + '\n'); + // The header block only. Filtering the whole file for `//` lines swept up + // implementation notes from the middle of the source and printed them as + // help ("Same loading trick as test/run.js", section rules, and so on). + const help = []; + for (const l of fs.readFileSync(__filename, 'utf8').split('\n').slice(1)) { + if (!l.startsWith('//')) break; + help.push(l.slice(3)); + } + process.stdout.write(help.join('\n') + '\n'); process.exit(0); } else if (!a.startsWith('-') && !opts.file) opts.file = a; else fail(`Unknown option: ${a}`); diff --git a/test/run.js b/test/run.js index 84df265..a815af0 100644 --- a/test/run.js +++ b/test/run.js @@ -3837,6 +3837,54 @@ test('cli --why prints an attribution table', () => { // ── Economy as code: CLI end-to-end ───────────────────────────────────────── console.log('\nEconomy as code: CLI'); +test('cli rejects a malformed numeric option instead of running a different economy', () => { + // parseFloat/parseInt stop at the first character they cannot use, so + // `--param carrying=1,000` ran with a carrying capacity of 1 and exited 0: + // a different simulation than the one asked for, with nothing to say so. + const { execFileSync } = require('child_process'); + const os = require('os'); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'econ-cli-num-')); + const econPath = path.join(dir, 'mine.econ'); + fs.writeFileSync(econPath, [ + 'param rate = 2', + 'source Mine @ 0,0', 'pool Gold @ 100,0', + 'Mine -> Gold : (rate)', + ].join('\n')); + const cli = path.join(__dirname, '..', 'cli.js'); + const run = (args) => { + try { return { out: execFileSync(process.execPath, [cli, ...args], { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }), err: '', code: 0 }; } + catch (e) { return { out: String(e.stdout || ''), err: String(e.stderr || ''), code: e.status }; } + }; + + for (const bad of ['rate=1,000', 'rate=abc', 'rate=3x', 'rate=']) { + const r = run([econPath, '--steps', '5', '--param', bad]); + eq(r.code, 1, `--param ${bad} is a usage error`); + assert(/expects a number|expects name=value/.test(r.err), `--param ${bad} says why`); + } + for (const good of ['rate=3', 'rate=2.5', 'rate=1e2', 'rate=-4', 'rate=.5']) { + eq(run([econPath, '--steps', '5', '--param', good]).code, 0, `--param ${good} still runs`); + } + eq(run([econPath, '--steps', '30x']).code, 1, '--steps 30x is a usage error'); + eq(run([econPath, '--steps', '5', '--runs', '2y']).code, 1, '--runs 2y is a usage error'); + eq(run([econPath, '--steps', '5', '--pass-rate', 'high']).code, 1, '--pass-rate high is a usage error'); + fs.rmSync(dir, { recursive: true, force: true }); +}); + +test('cli --help prints the header block, not the whole source comment set', () => { + // The help text was every `//` line in the file, so it trailed off into + // implementation notes from the middle of cli.js. + const { execFileSync } = require('child_process'); + const cli = path.join(__dirname, '..', 'cli.js'); + const out = execFileSync(process.execPath, [cli, '--help'], { encoding: 'utf8' }); + assert(/Options:/.test(out), 'options section is present'); + assert(/--param name=val/.test(out), 'options are documented'); + assert(/Examples:/.test(out), 'examples section is present'); + assert(!/loading trick|Exit quietly when the consumer/.test(out), + 'implementation notes stay out of the help text'); + assert(!/Load the diagram: JSON/.test(out), 'section rules stay out of the help text'); + assert(!out.includes('\u2014'), 'no em dashes in user-facing help copy'); +}); + test('cli runs .econ input, checks assertions and converts formats', () => { const { execFileSync } = require('child_process'); const os = require('os'); From fe8e8894b53528972a0428e30a0de0304475c07b Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 22:19:37 +0000 Subject: [PATCH 5/6] Clamp PNG export to the canvas limit, keep Monte Carlo responsive, and settle the mobile drawer Round-4 review findings 11, 12 and 2, plus a cancellation leak found while fixing 12. - PNG export sized the raster at 2x the diagram and never checked the result. Past a browser's canvas ceiling (268 Mpx in Chromium, and a longest-side cap besides) drawImage silently does nothing and toDataURL hands back a stub, so a large diagram downloaded a 0-byte file with no error anywhere. The scale now drops to whatever fits, an unrasterizable canvas says so instead of saving an empty file, and a scaled-down export tells the user and points at SVG. - runMonteCarloAsync is time-boxed at 14ms per chunk, but the generator yielded only after a whole trial, so one trial of a large model ran uninterruptible: measured on a 300-node model, the longest event-loop gap was 276ms, and Cancel went unanswered for that long. It now yields after every step as well, which brings the worst gap to 22ms and costs about 2% in total batch time. - Cancelling a batch dropped the generator instead of closing it, so its finally block never ran and the shared RNG stayed parked on the last trial's sub-seed. The driver now calls job.return() before resolving. - The mobile timeline drawer overshot the room it needed. #timeline-canvas is a , so its intrinsic height drove the drawer's content height whatever its flex basis said, and the drawer took 324px of a 1024px-tall screen with a 230px chart in it. Zeroing the canvas's CSS height lets the flex layout decide instead: a steady 190px from 320px wide to 768px, the scrub row still inside it at the legend's two-row cap, and #tl-resize still grows the chart. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SH7ouoNymubArm7VEgALHg --- css/style.css | 15 ++++++++--- js/app-export.js | 34 ++++++++++++++++++++---- js/engine.js | 23 +++++++++++++--- test/run.js | 44 +++++++++++++++++++++++++++++++ test/smoke.js | 68 ++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 172 insertions(+), 12 deletions(-) diff --git a/css/style.css b/css/style.css index 982f11c..2c25df2 100644 --- a/css/style.css +++ b/css/style.css @@ -1483,9 +1483,18 @@ input[type="range"] { so the whole footer (Replay, the slider, the step label, Back to live) sat below the fold with between zero and three usable pixels. html and body do not scroll and #tl-resize is mousedown-only, so there was no way to reach - it. Fit the content instead of cropping it, and let the drawer scroll if a - long legend still overflows. */ - #timeline { height: auto; min-height: 150px; max-height: 46vh; overflow-y: auto; } + it. + Sizing to content instead overshot the other way: #timeline-canvas is a + , so its intrinsic height (whatever it last rendered at) drives the + drawer's content height however small its flex basis is, and the drawer + took 324px of a 1024px screen with a 230px chart in it. Zero the canvas's + CSS height so the flex layout, not the bitmap, decides how tall it is; the + 90px min-height on the base rule is still its floor. That holds the drawer + at a steady 190px from 320px wide up to 768px, with the scrub row inside it + even at the legend's two-row cap, and dragging #tl-resize still grows the + chart rather than opening a gap under it. */ + #timeline-canvas { height: 0; } + #timeline { height: 190px; min-height: 0; max-height: 40vh; overflow-y: auto; } /* The scrub row is the reason the drawer exists on a phone; never let it shrink away under a tall legend. */ #tl-scrub { flex-shrink: 0; } diff --git a/js/app-export.js b/js/app-export.js index 9ef1ac0..47af40c 100644 --- a/js/app-export.js +++ b/js/app-export.js @@ -109,6 +109,18 @@ class AppExport { a.click(); } + // Largest raster scale that still fits inside the browser's canvas limits. + // Browsers cap both a canvas's longest side and its total area; past either, + // drawImage silently does nothing and toDataURL hands back a blank image, so + // a big diagram used to download a 0-byte PNG with no error anywhere. + // 2x (crisp on high-DPI screens) whenever it fits, less when it does not. + _pngScale(w, h) { + const MAX_DIM = 16384; // conservative across browsers + const MAX_AREA = 2.0e8; // Chromium's ceiling is 268,435,456 px + if (!(w > 0) || !(h > 0)) return 1; + return Math.max(0.02, Math.min(2, MAX_DIM / w, MAX_DIM / h, Math.sqrt(MAX_AREA / (w * h)))); + } + _exportPNG() { const built = this._buildExportSVG(); if (!built) return; @@ -118,19 +130,31 @@ class AppExport { const url = URL.createObjectURL(blob); const img = new Image(); img.onload = () => { - // 2x the diagram's natural size, for crispness on high-DPI screens. + const scale = this._pngScale(w, h); const canvas = document.createElement('canvas'); - canvas.width = w * 2; canvas.height = h * 2; + canvas.width = Math.max(1, Math.floor(w * scale)); + canvas.height = Math.max(1, Math.floor(h * scale)); const ctx = canvas.getContext('2d'); - ctx.scale(2, 2); + ctx.scale(scale, scale); ctx.fillStyle = bg; ctx.fillRect(0, 0, w, h); ctx.drawImage(img, 0, 0, w, h); + let href = ''; + try { href = canvas.toDataURL('image/png'); } catch { /* tainted or oversized */ } + URL.revokeObjectURL(url); + // A canvas the browser refused to rasterize returns a stub or nothing at + // all. Better to say so than to hand over an empty file. + if (!href || href.length < 128) { + this._toast('This diagram is too large to export as PNG. Export SVG instead, it has no size limit.'); + return; + } const a = Object.assign(document.createElement('a'), { - download: this._exportFilename('png'), href: canvas.toDataURL('image/png'), + download: this._exportFilename('png'), href, }); a.click(); - URL.revokeObjectURL(url); + if (scale < 2) { + this._toast(`This diagram is large, so the PNG was rendered at ${Math.round(scale * 100)}% to stay inside the browser's canvas limit. SVG exports at full size.`); + } }; img.onerror = () => URL.revokeObjectURL(url); img.src = url; diff --git a/js/engine.js b/js/engine.js index 07452b1..bc3d976 100644 --- a/js/engine.js +++ b/js/engine.js @@ -1498,7 +1498,7 @@ class SimEngine { } // Same batch, but yields to the event loop between time-boxed chunks of - // trials so the UI stays responsive; reports progress via opts.onProgress. + // work so the UI stays responsive; reports progress via opts.onProgress. runMonteCarloAsync(runs = 100, maxSteps = 200, opts = {}) { return new Promise(resolve => { const job = this._mcTrials(runs, maxSteps, opts); @@ -1506,21 +1506,30 @@ class SimEngine { // Cooperative cancellation: bail between chunks if the caller asks to // stop (e.g. a Cancel button on a long batch). Resolves to null so the // caller can distinguish a cancelled run from a completed one. - if (opts.shouldCancel && opts.shouldCancel()) { resolve(null); return; } + if (opts.shouldCancel && opts.shouldCancel()) { + // Close the generator so its finally block runs: abandoning it left + // the shared RNG parked on the last trial's sub-seed. + job.return(); + resolve(null); + return; + } const t0 = (typeof performance !== 'undefined' ? performance.now() : Date.now()); let r = job.next(); while (!r.done && ((typeof performance !== 'undefined' ? performance.now() : Date.now()) - t0) < 14) { r = job.next(); } if (r.done) { resolve(r.value); return; } - if (opts.onProgress) opts.onProgress(r.value.done, r.value.total); + // Mid-trial breaths carry no new progress; only completed trials do. + if (opts.onProgress && !r.value.partial) opts.onProgress(r.value.done, r.value.total); setTimeout(tick, 0); }; tick(); }); } - // Generator running one trial per yield. Shared by the sync and async paths. + // Generator yielding after every step, and again at the end of each trial + // (the unmarked yields, which carry progress). Shared by the sync and async + // paths. *_mcTrials(runs, maxSteps, opts = {}) { runs = Math.max(1, Math.round(runs)); maxSteps = Math.max(1, Math.round(maxSteps)); @@ -1560,6 +1569,12 @@ class SimEngine { while (s < maxSteps && !eng.ended) { eng.doStep(); s++; if (opts.perStep) opts.perStep(eng, r); + // Breathe inside the trial too. One trial of a large model can take + // most of a second by itself, and yielding only between whole trials + // put that entirely outside the async driver's 14 ms time box: the UI + // froze in second-long blocks and Cancel went unanswered until the + // trial finished. Marked partial so it does not report progress. + yield { done: r, total: runs, partial: true }; } if (opts.onTrialEnd) opts.onTrialEnd(eng, r); for (const [id, arr] of samples) { diff --git a/test/run.js b/test/run.js index a815af0..87c45fd 100644 --- a/test/run.js +++ b/test/run.js @@ -804,6 +804,50 @@ test('a trader cannot pay out of a delay or a queue', () => { } }); +test('a Monte Carlo batch yields inside a trial, not only between trials', () => { + // runMonteCarloAsync is time-boxed at 14ms per chunk, but the generator only + // yielded once a whole trial had finished, so a single long trial ran + // uninterruptible: the UI froze in blocks as long as one trial and Cancel + // went unanswered until it ended. + const d = new Diagram(); + const src = new MNode(NodeType.SOURCE, 0, 0); src.label = 'Mine'; + const pool = new MNode(NodeType.POOL, 200, 0); pool.label = 'Gold'; + d.addNode(src); d.addNode(pool); + d.addConnection(new MConnection(src.id, pool.id, ConnectionType.RESOURCE)); + + const e = new SimEngine(d); + const job = e._mcTrials(2, 25, {}); + let yields = 0, progressYields = 0; + for (let r = job.next(); !r.done; r = job.next()) { + yields++; + if (!r.value.partial) progressYields++; + } + eq(progressYields, 2, 'one progress yield per completed trial'); + assert(yields >= 2 * 25, `yields inside each trial too (got ${yields})`); +}); + +testAsync('a cancelled Monte Carlo batch still restores the RNG it borrowed', async () => { + // The driver dropped the generator on cancel instead of closing it, so the + // finally block never ran and the shared RNG stayed parked on the last + // trial's sub-seed. + const d = new Diagram(); + d.seed = 'live'; + const src = new MNode(NodeType.SOURCE, 0, 0); src.label = 'Mine'; + const pool = new MNode(NodeType.POOL, 200, 0); pool.label = 'Gold'; + const c = new MConnection(src.id, pool.id, ConnectionType.RESOURCE); + c.rateMode = RateMode.DICE; c.dice = '2d6'; + d.addNode(src); d.addNode(pool); d.addConnection(c); + + const e = new SimEngine(d); e.reset(); + for (let i = 0; i < 5; i++) e.doStep(); + const before = SimRandom.getState(); + let ticks = 0; + const res = await e.runMonteCarloAsync(2000, 500, { seed: 'batch', shouldCancel: () => (++ticks > 1) }); + eq(res, null, 'a cancelled batch resolves null'); + assert(ticks >= 2, 'the batch ran a chunk of trials before being cancelled'); + eq(SimRandom.getState(), before, 'the live run keeps its stream position'); +}); + test('a Monte Carlo batch leaves a paused seeded run where it found it', () => { // Batch Analysis stops the live run but does not reset it, so the run resumes // on the shared RNG. Every trial reseeds that RNG and the batch used to clear diff --git a/test/smoke.js b/test/smoke.js index afc289a..8e05c77 100644 --- a/test/smoke.js +++ b/test/smoke.js @@ -2588,6 +2588,74 @@ const URL = process.env.SMOKE_URL || 'http://localhost:8080/'; ok(`components: save selection (${comp.compNodes} nodes, ${comp.compConns} conn), insert adds 2 nodes, undo reverts`); else fail('components: ' + JSON.stringify(comp)); + // Export: a big diagram must not silently download a 0-byte PNG. Browsers cap + // both a canvas's longest side and its total area; past either, drawImage + // no-ops and toDataURL returns a stub. + const png = await page.evaluate(() => { + const app = window.app; + const scale = (w, h) => app._pngScale(w, h); + // 8260 x 8260 content: 2x would be 273 Mpx, past Chromium's 268 Mpx ceiling. + const big = scale(8260, 8260); + const bigCanvas = document.createElement('canvas'); + bigCanvas.width = Math.floor(8260 * big); bigCanvas.height = Math.floor(8260 * big); + const ctx = bigCanvas.getContext('2d'); + ctx.fillStyle = '#123456'; ctx.fillRect(0, 0, 40, 40); + let url = ''; + try { url = bigCanvas.toDataURL('image/png'); } catch { url = ''; } + return { + small: scale(800, 600), + big, + bigArea: Math.round(8260 * big) * Math.round(8260 * big), + wide: scale(40000, 300), + degenerate: scale(0, 0), + rasterized: url.length > 1000, + }; + }); + if (png.small === 2 && png.big < 2 && png.bigArea <= 268435456 && png.wide <= 16384 / 40000 * 1.001 + && png.degenerate === 1 && png.rasterized) + ok(`export: PNG scale clamps to the canvas limit (2x normally, ${png.big.toFixed(2)}x on an 8260px diagram) and still rasterizes`); + else fail('png scale: ' + JSON.stringify(png)); + + // Mobile: the timeline drawer must leave the canvas its room and still keep + // its own scrub row inside itself, at every small width. + const drawer = await (async () => { + const rows = []; + for (const vp of [{ width: 768, height: 1024 }, { width: 390, height: 844 }, { width: 320, height: 568 }]) { + const ctx = await browser.newContext({ viewport: vp }); + const mp = await ctx.newPage(); + await mp.route('https://fonts.googleapis.com/**', r => r.fulfill({ contentType: 'text/css', body: '' })); + await mp.addInitScript(() => { try { localStorage.setItem('sim_seen_welcome', '1'); } catch (e) {} }); + await mp.goto(URL, { waitUntil: 'networkidle' }); + rows.push(await mp.evaluate(async () => { + const app = window.app; + app._clearAll(); + // Enough tracked nodes to fill the legend's two-row cap: the worst case. + for (let i = 0; i < 14; i++) { + const n = new MNode(NodeType.POOL, 80 + i * 60, 100); + n.label = 'Resource ' + (i + 1); n.setCount(5 + i); + app.diagram.addNode(n); + } + app.renderer.render(); + for (let i = 0; i < 12; i++) app.engine.doStep(); + document.getElementById('btn-timeline').click(); + await new Promise(r => requestAnimationFrame(() => requestAnimationFrame(r))); + const tl = document.getElementById('timeline').getBoundingClientRect(); + const scrub = document.getElementById('tl-scrub').getBoundingClientRect(); + const cv = document.getElementById('timeline-canvas').getBoundingClientRect(); + return { + w: window.innerWidth, h: Math.round(tl.height), chart: Math.round(cv.height), + scrubInside: scrub.bottom <= tl.bottom + 1, + share: Math.round((tl.height / window.innerHeight) * 100), + }; + })); + await ctx.close(); + } + return rows; + })(); + if (drawer.every(r => r.h <= 200 && r.chart >= 90 && r.scrubInside)) + ok(`mobile: timeline drawer holds at ${drawer.map(r => r.h + 'px/' + r.share + '%').join(', ')} with the scrub row inside`); + else fail('mobile drawer: ' + JSON.stringify(drawer)); + // Analysis: the sweep range follows the chosen parameter. It was seeded once, // from the first parameter in the list, and nothing re-ran it on change, so // sweeping any other parameter ran over the first one's range. From cf44972ce2a581bdca635d8de07c39044317a54d Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 22:20:46 +0000 Subject: [PATCH 6/6] Tidy the PNG toast wording and document set()'s resource typing Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SH7ouoNymubArm7VEgALHg --- docs/ECONOMY_AS_CODE.md | 3 +++ js/app-export.js | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/ECONOMY_AS_CODE.md b/docs/ECONOMY_AS_CODE.md index 19129b5..eba162c 100644 --- a/docs/ECONOMY_AS_CODE.md +++ b/docs/ECONOMY_AS_CODE.md @@ -223,5 +223,8 @@ Notes: seeded economy at a time per process for bit-exact reproducibility. - `createEconomy()` parses a fresh copy of the embedded diagram each call, so instances never share mutable state. +- `set()` keeps the node's own resource type: topping a pool up adds units of + whatever it already holds (or of the type it was authored with, when empty), + so colour-filtered connections and converter recipes still accept them. - The module also exports `Diagram`, `SimEngine`, `SimRandom` and `NodeType` for power users who want to go under the hood. diff --git a/js/app-export.js b/js/app-export.js index 47af40c..867f015 100644 --- a/js/app-export.js +++ b/js/app-export.js @@ -145,7 +145,7 @@ class AppExport { // A canvas the browser refused to rasterize returns a stub or nothing at // all. Better to say so than to hand over an empty file. if (!href || href.length < 128) { - this._toast('This diagram is too large to export as PNG. Export SVG instead, it has no size limit.'); + this._toast('This diagram is too large to export as PNG. Export SVG instead, which has no size limit.'); return; } const a = Object.assign(document.createElement('a'), {