Fix the nine remaining defects found in review

js/app-export.js

@@ -10,9 +10,20 @@
 class AppExport {
   // ── Export ────────────────────────────────────────────────────────────────
 
+  // Backs every download in the app: SVG, PNG, CSV, .econ, the standalone
+  // module, File > Save, and the three analysis exports. The old ASCII-only
+  // class turned every character of a non-Latin name into an underscore and
+  // then stripped them, returning a bare ".svg" that the browser renames to
+  // "svg.svg", so a Chinese or Japanese diagram lost its name entirely and
+  // every export collided on one filename. Accented Latin fared little better
+  // ("Economie" losing its leading E). Unicode letters and digits are kept, and
+  // an empty stem falls back to "diagram" so the result can never start with a
+  // dot: a dotfile also cost .econ its extension, the browser rewriting it to
+  // "econ.txt" so it no longer matched the app's own file picker.
   _exportFilename(ext) {
     const raw = this.diagram.meta.name || 'diagram';
-    return raw.replace(/[^a-z0-9_\-]/gi, '_').replace(/_+/g, '_').replace(/^_|_$/g, '') + '.' + ext;
+    const stem = raw.replace(/[^\p{L}\p{N}_-]+/gu, '_').replace(/_+/g, '_').replace(/^_|_$/g, '');
+    return (stem || 'diagram') + '.' + ext;
   }
 
   // Build a standalone snapshot of the diagram, cloned from the live canvas so

js/app-fields.js

@@ -86,7 +86,7 @@ class AppFields {
     const val = document.createElement('div');
     val.className = 'hero-card-value';
     val.id = 'props-hero-value';
-    val.textContent = String(node.displayCount);
+    val.textContent = String(this._panelValueOf(node));
     card.appendChild(val);
 
     let subText = '';
@@ -407,6 +407,21 @@ class AppFields {
 
   // ── Live update helpers ───────────────────────────────────────────────────
 
+  // What the properties panel should display for a node right now. While the
+  // timeline is scrubbed that is the recorded value at the previewed step, not
+  // the live model. The canvas and the timeline both had a scrub-aware read
+  // path (Renderer._scrubSnap, Timeline.setScrub); the panel never did, so its
+  // hero card went on reporting the end of the run while the node beside it
+  // showed the replayed step. Two different numbers for the same node.
+  _panelValueOf(node) {
+    if (this._scrubIndex != null) {
+      const entry = this.engine.history[this._scrubIndex];
+      const v = entry && entry.snap ? entry.snap[node.id] : undefined;
+      if (v !== undefined) return v;
+    }
+    return node.displayCount;
+  }
+
   _refreshResourceCount() {
     // A diagram-rail feature (Parameters, Custom variables, Artificial player,
     // Design tests) borrows the properties panel without clearing the node
@@ -420,7 +435,7 @@ class AppFields {
 
     // Big hero readout tracks the live value for every node kind.
     const hero = document.getElementById('props-hero-value');
-    if (hero) hero.textContent = String(node.displayCount);
+    if (hero) hero.textContent = String(this._panelValueOf(node));
     // Same exclusions the amount field is built under (app-props.js). A Trader
     // has no amount row, so its first number input is the End/goal value, and
     // the refresh below was overwriting that goal with the trader's count.
@@ -429,7 +444,9 @@ class AppFields {
 
     // With a node owning the panel, the first number input is the Resources field.
     const inp = document.querySelector('#props-content input[type="number"]');
-    if (inp && document.activeElement !== inp) inp.value = node.resources;
+    // Not while scrubbing: the field is editable, and writing a replayed value
+    // into it invites the user to commit a past step as the current amount.
+    if (inp && document.activeElement !== inp && this._scrubIndex == null) inp.value = node.resources;
 
     // That field means two different things either side of step 0, and the
     // panel is not rebuilt as the run advances, so its label has to be patched
@@ -441,7 +458,12 @@ class AppFields {
     }
   }
 
-  _updateSparklines() { for (const sl of this._sparklines.values()) sl.update(); }
+  _updateSparklines() {
+    for (const sl of this._sparklines.values()) {
+      sl.scrubIndex = this._scrubIndex;
+      sl.update();
+    }
+  }
 
   _clearSparklines() { for (const sl of this._sparklines.values()) sl.destroy(); this._sparklines.clear(); }
 }

js/app-library.js

@@ -306,7 +306,7 @@ class AppLibrary {
           add('Export as JSON', 'download', () => {
             const a = Object.assign(document.createElement('a'), {
               href: URL.createObjectURL(new Blob([entry.json], { type: 'application/json' })),
-              download: `${(entry.name || 'diagram').replace(/[^\w\-]+/g, '_')}.json`,
+              download: `${(entry.name || 'diagram').replace(/[^\p{L}\p{N}_-]+/gu, '_') || 'diagram'}.json`,
             });
             a.click();
           });

js/app-props.js

@@ -624,6 +624,28 @@ class AppProps {
   }
 
   // Named constants available to all formulas.
+  // Every name that already resolves in the shared variable store, which the
+  // engine builds from parameters, custom variables, named state connections
+  // and register labels (SimEngine._updateVariables). Renaming has to consult
+  // all four, not just the editor being typed in: two rows agreeing on a name
+  // collapse to one value at simulation time whichever editor they came from.
+  // `except` is the name being edited, so a no-op rename is not a collision.
+  _nameTaken(name, except) {
+    const d = this.diagram;
+    if (name === except) return false;
+    for (const k of Object.keys(d.params || {})) if (k !== except && k === name) return true;
+    for (const rv of d.customVars || []) if (rv.name !== except && rv.name === name) return true;
+    for (const n of d.nodes.values()) {
+      if (n.type === NodeType.REGISTER && n.label !== except && n.label === name) return true;
+    }
+    for (const c of d.connections.values()) {
+      if (c.type !== ConnectionType.STATE) continue;
+      const vn = c.variableName || c.label;
+      if (vn && vn !== except && vn === name) return true;
+    }
+    return false;
+  }
+
   _paramsEditor(panel) {
     this._info(panel, 'Named constants available to all formulas (e.g. growth_rate * pool).');
     const params = this.diagram.params;
@@ -637,6 +659,17 @@ class AppProps {
         const nk = ki.value.trim();
         if (!nk || nk === key) { ki.value = key; return; }
         if (!VALID_IDENT.test(nk)) { ki.value = key; return; }
+        // Without this the assignment below overwrote the other parameter and
+        // deleted this one, so two rows silently became one: `gold` took this
+        // row's value, the original gold was gone, and every formula reading
+        // gold changed meaning with only undo to recover. The two checks above
+        // already reject a bad name by reverting the field, so a colliding name
+        // reverts the same way rather than destroying data.
+        if (this._nameTaken(nk, key)) {
+          ki.value = key;
+          this._toast(`"${nk}" is already used by another parameter or variable.`);
+          return;
+        }
         params[nk] = params[key];
         delete params[key];
         this._renderProps();
@@ -876,6 +909,13 @@ class AppProps {
       name.addEventListener('blur', () => {
         const nk = name.value.trim();
         if (!nk || !VALID_IDENT.test(nk)) { name.value = rv.name; return; }
+        // Both rows survive here, but the shared store keeps one value for the
+        // name, so the duplicate silently shadows the other at run time.
+        if (this._nameTaken(nk, rv.name)) {
+          name.value = rv.name;
+          this._toast(`"${nk}" is already used by another parameter or variable.`);
+          return;
+        }
         if (nk !== rv.name) { rv.name = nk; this._commit(); }
       });
 

js/app.js

@@ -647,6 +647,8 @@ class App {
     this.renderer.setScrub(entry.snap);
     if (this._timelineVisible) this.timeline.setScrub(entry.step);
     document.getElementById('step-counter').textContent = `Step ${entry.step} (replay)`;
+    this._refreshResourceCount();
+    this._updateSparklines();
     this._refreshScrubber();
   }
 
@@ -661,6 +663,8 @@ class App {
     if (wasScrubbing) {
       document.getElementById('step-counter').textContent = `Step ${this.engine.step}`;
       this.renderer.render();
+      this._refreshResourceCount();
+      this._updateSparklines();
       if (this._activeFeature === 'monitor') this._renderProps();
     }
     this._refreshScrubber();
@@ -721,6 +725,12 @@ class App {
     this.diagram.timeMode = 'sync';
     this.diagram.seed = '';
     this.diagram.aiPlayer = { enabled: false, rules: [] };
+    // Assertions are a serialized Diagram field like the rest, and were the one
+    // this missed. They survived File > New, every template load and the
+    // restored-session Discard, then went straight into autosave, Save as JSON,
+    // the .econ export and the share link, so cli.js --check on that file ran a
+    // previous model's tests against nodes that no longer exist.
+    this.diagram.assertions = [];
     this.diagram.meta = Diagram.defaultMeta();
     this._applyMeta();
     this._dropScenarioState();
@@ -1241,6 +1251,11 @@ class App {
     document.getElementById('btn-step').addEventListener('click', () => {
       this._exitScrub();
       this.engine.doStep();
+      // Run/Pause and Reset both refresh the scrubber; Step did not, so its
+      // range stayed one entry behind the history. While Live the thumb already
+      // sits at max, so pressing End or dragging it right emitted no input
+      // event and the newest step could not be reached at all.
+      this._refreshScrubber();
     });
 
     const runBtn = document.getElementById('btn-run');

js/attribution.js

@@ -42,12 +42,45 @@ function attributeChange(diagram, history, nodeId, index) {
   const entries = [];
   const nameOf = id => { const n = diagram.nodes.get(id); return (n && n.label) || id || '?'; };
 
+  // A trader holds nothing: it swaps between the two partners of a paired
+  // incoming/outgoing connection (SimEngine._fireTrader pairs ins[i] with
+  // outs[i], so the pairing is reconstructible here). Crucially the flow booked
+  // on the OUTGOING leg is what that partner PAID, not what it received, so
+  // reading direction off sourceId/targetId reported a payment as income, never
+  // credited either partner with what it got, and gave the trader itself two
+  // rows for resources that never touched it. Map each leg to its real payer
+  // and payee instead.
+  const traderLegs = new Map();   // connId -> { payer, payee, trader }
+  for (const t of diagram.nodes.values()) {
+    if (t.type !== NodeType.TRADER) continue;
+    const ins = diagram.incoming(t.id).filter(c => c.type === ConnectionType.RESOURCE);
+    const outs = diagram.outgoing(t.id).filter(c => c.type === ConnectionType.RESOURCE);
+    for (let i = 0; i < Math.min(ins.length, outs.length); i++) {
+      const cin = ins[i], cout = outs[i];
+      traderLegs.set(cin.id, { payer: cin.sourceId, payee: cout.targetId, trader: t.id });
+      traderLegs.set(cout.id, { payer: cout.targetId, payee: cin.sourceId, trader: t.id });
+    }
+  }
+
   const isRegister = node.type === NodeType.REGISTER;
   if (!isRegister) {
     for (const [connId, amt] of Object.entries(flows.conns)) {
       if (!amt) continue;
       const c = diagram.connections.get(connId);
       if (!c) continue;
+      const leg = traderLegs.get(connId);
+      if (leg) {
+        // The trader's own charted value is its trade count, not a balance, so
+        // these resources are none of its business.
+        if (nodeId === leg.trader) continue;
+        if (nodeId === leg.payer && node.type !== NodeType.DRAIN) {
+          entries.push({ kind: 'flow out', connId, amount: -amt, label: `to ${nameOf(leg.payee)}` });
+        }
+        if (nodeId === leg.payee && node.type !== NodeType.SOURCE) {
+          entries.push({ kind: 'flow in', connId, amount: amt, label: `from ${nameOf(leg.payer)}` });
+        }
+        continue;
+      }
       // A drain's charted value only ever grows with intake; a limited
       // source's stock only ever falls with output. Everything else counts
       // both directions. (A self-loop connection nets to zero via two rows.)

js/charts.js

@@ -7,11 +7,17 @@ class Sparkline {
     this.canvas.width = 260;
     this.canvas.height = 60;
     this.canvas.className = 'sparkline';
+    this.scrubIndex = null;   // history index being previewed, or null when live
     container.appendChild(this.canvas);
   }
 
   update() {
-    const history = this.engine.history;
+    // While the timeline is scrubbed the panel must show the replayed step, not
+    // the end of the run. scrubIndex is set by the app before each update and
+    // cleared on exit; null means live.
+    const history = this.scrubIndex == null
+      ? this.engine.history
+      : this.engine.history.slice(0, this.scrubIndex + 1);
     // One point draws nothing but an empty box and a stray midline, which read
     // as a broken chart at the top of the properties panel. Stay collapsed
     // until there are two points to join, then reveal.
@@ -23,14 +29,30 @@ class Sparkline {
     const w = this.canvas.width, h = this.canvas.height;
     ctx.clearRect(0, 0, w, h);
 
-    const max = Math.max(...values, 1);
+    // The scale has to span zero. Mapping v/max put anything below zero at
+    // y > h, off the bottom of the canvas: a register holding a net or deficit,
+    // which is most of what a register computes, drew nothing at all and was
+    // captioned "max: 1" because the old max floored at 1. A series crossing
+    // zero was worse, drawing only its positive half so the trace appeared to
+    // start mid-chart out of nowhere, looking like real data. Extending the
+    // range to include zero leaves a positive-only series scaled exactly as
+    // before (min lands on 0) and brings the rest onto the canvas.
+    const rawMax = Math.max(...values);
+    const rawMin = Math.min(...values);
+    const max = Math.max(rawMax, 0);
+    const min = Math.min(rawMin, 0);
+    const range = (max - min) || 1;
+    const yOf = (v) => h - ((v - min) / range) * (h - 4) - 2;
+
     ctx.fillStyle = '#0d0e11';
     ctx.fillRect(0, 0, w, h);
 
-    // Grid line at midpoint
+    // Reference line: the zero crossing once the series goes negative, where it
+    // actually means something, otherwise the midpoint as before.
+    const yRef = rawMin < 0 ? yOf(0) : h / 2;
     ctx.strokeStyle = '#22252e';
     ctx.lineWidth = 1;
-    ctx.beginPath(); ctx.moveTo(0, h / 2); ctx.lineTo(w, h / 2); ctx.stroke();
+    ctx.beginPath(); ctx.moveTo(0, yRef); ctx.lineTo(w, yRef); ctx.stroke();
 
     const step = w / (values.length - 1);
 
@@ -39,25 +61,29 @@ class Sparkline {
     ctx.lineWidth = 1.5;
     values.forEach((v, i) => {
       const x = i * step;
-      const y = h - (v / max) * (h - 4) - 2;
+      const y = yOf(v);
       if (i === 0) ctx.moveTo(x, y); else ctx.lineTo(x, y);
     });
     ctx.stroke();
 
-    // Fill under line
-    ctx.lineTo((values.length - 1) * step, h);
-    ctx.lineTo(0, h);
+    // Fill between the line and zero, so a dip below zero fills upward to the
+    // baseline rather than flooding the whole panel.
+    const yBase = yOf(0);
+    ctx.lineTo((values.length - 1) * step, yBase);
+    ctx.lineTo(0, yBase);
     ctx.closePath();
     ctx.fillStyle = 'rgba(182,233,77,0.10)';
     ctx.fill();
 
-    // Current value label
+    // Labels use the real extremes, not the zero-extended ones: an all-negative
+    // series reading "max: 0" would be as wrong as the old "max: 1".
+    const fmt = (n) => (Number.isInteger(n) ? String(n) : n.toFixed(2));
     ctx.fillStyle = '#b6e94d';
     ctx.font = "11px 'JetBrains Mono', monospace";
     const last = values[values.length - 1];
-    ctx.fillText(`${last}`, w - 30, 12);
+    ctx.fillText(`${fmt(last)}`, w - 30, 12);
     ctx.fillStyle = '#8a90a0';
-    ctx.fillText(`max: ${max}`, 4, 12);
+    ctx.fillText(rawMin < 0 ? `${fmt(rawMin)} to ${fmt(rawMax)}` : `max: ${fmt(rawMax)}`, 4, 12);
   }
 
   destroy() { this.canvas.remove(); }

js/dsl.js

@@ -150,14 +150,32 @@ function _econParseValue(raw, lineNo) {
 
 // Assign every node a unique, human-readable reference name derived from its
 // label, disambiguating duplicates with #2, #3, … in declaration order.
+// Every word that starts a statement. dslParse dispatches on tokens[0] before it
+// scans the line for an arrow, so a node whose label is one of these and which
+// is the SOURCE of a connection would emit `pool -> Gold : 2` and be read back
+// as a declaration. Seventeen of them throw on reload, which surfaces as a
+// failed File > Open; `economy` and `assert` are worse, matching the version
+// header and the assert handler so the connection silently disappears. Quoting
+// the reference keeps such a label round-tripping, since _econReadRef already
+// understands a quoted name. Case-sensitive on purpose: default labels are
+// capitalized (MNode sets `Pool`, `Queue`), so only a hand-typed lowercase
+// label is at risk. `name`, `desc`, `seed` and `timeMode` are absent because
+// their handler requires a literal colon, which a connection line never has.
+const ECON_RESERVED_HEADS = new Set([
+  'economy', 'meta', 'param', 'type', 'var', 'assert', 'player',
+  'group', 'note', 'chart',
+  ...ECON_NODE_KINDS,
+]);
+
 function _econRefNames(nodes) {
   const used = new Map(); // base label → count
   const refs = new Map(); // node id → ref string
   for (const n of nodes) {
     const base = n.label != null ? String(n.label) : '';
     const count = (used.get(base) || 0) + 1;
     used.set(base, count);
-    refs.set(n.id, _econName(base) + (count > 1 ? '#' + count : ''));
+    const name = ECON_RESERVED_HEADS.has(base) ? _econQuote(base) : _econName(base);
+    refs.set(n.id, name + (count > 1 ? '#' + count : ''));
   }
   return refs;
 }