Files
EVOLV/.claude/refactor/OPEN_QUESTIONS.md
znetsixe 44ffae12f7 P11.6 wiki regen + Phase 10 private-test rewrites — bump pointers
All 11 nodes' wiki/Home.md regenerated with the Unit column +
per-topic descriptions. rotatingMachine + reactor private-method
test files rewritten to the public BaseNodeAdapter surface.

OPEN_QUESTIONS: rotatingMachine + reactor private-test entries
marked RESOLVED.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-11 19:44:19 +02:00

744 lines
38 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Open questions
Things deferred. Append, don't rewrite history. Add a date when you add
or resolve an entry. Anyone (human or agent) discovering an unclear
decision during refactor work writes it here rather than guessing.
---
## 2026-05-11 — Interview round — resolved decisions
| Topic | Decision |
|---|---|
| Ramp foot for run-zone curve (control/levelBased) | `inflowLevel` (current). startLevel is the 0% minimum, not the curve foot. |
| `overfillLevel` vs `highVolumeSafetyLevel` | **`highVolumeSafetyLevel` canonical**; drop the legacy alias. |
| measurement `isStable` tautology | Fix now with a config-driven absolute threshold (`stabilityThreshold` in scaling-units). Add to schema + editor UI. |
| monster cooldown-guard pre-existing fail | **RESOLVED 2026-05-11** — root cause was missing `nominalFlowMin`/`flowMax`/`maxRainRef`/`minSampleIntervalSec` in `monster.json`, stripped by `configUtils.initConfig` before reaching the domain. Added the four keys to the schema. |
| pumpingStation plain child dicts | Migrate to `declareChildGetter`; rewrite affected tests. |
| VGC custom `registerChild` overload | Adopt ChildRouter; rewrite disambiguation tests. |
| MGC inline dispatch gate vs LatestWinsGate | Extend LatestWinsGate with `fireAndWait(value)` returning the per-fire settlement promise. Migrate MGC. |
| measurement legacy `'mAbs'` event | Remove now. |
| ChildRouter wildcard emit-patch | Per-listener fan-out using canonical POSITIONS. No more emit patching. |
| commandRegistry payload schema | Add `'none'` type + per-command `description` field (wikiGen consumes). |
| UnitPolicy property-vs-method shape | Expose both. Frozen property bags alongside the methods. Drop `_unitView` workarounds. |
| rotatingMachine + reactor private-method-pinning tests (13 files) | Rewrite all to drive only the public BaseNodeAdapter surface. Phase 10. |
| **Unit-aware commands (new)** | Each numeric setter declares `units: { measure, default }`. commandRegistry normalises + warns + lists accepted units. `query.units` topic returns spec. Phase 11. |
Format:
```
## YYYY-MM-DD — Short title
**Context:** what we're trying to do
**Question:** what's unresolved
**Default chosen:** what we did meanwhile
**Decision needed by:** which phase or task
```
---
## 2026-05-10 — External Port-0 topic naming — RESOLVED
**Decision (2026-05-10):** Use canonical names (`set.*` / `cmd.*` /
`data.*` / `child.*` / `query.*` / `evt.*`) **from Phase 1 onwards**.
Each `commands/index.js` declares the canonical name as the topic and
lists legacy names in `aliases`. Aliases log a one-time deprecation
warning. Phase 7 shrinks to: remove aliases after one release cycle.
The full prefix glossary (with what each does and why) is now in
`CONTRACTS.md §1`. See it before naming a topic.
---
## 2026-05-10 — Parent EVOLV repo `development` branch lineage — RESOLVED
**Decision (2026-05-10):** Rebase parent `development` onto
`origin/main` before the refactor proceeds. Done at the start of
Phase 1.
---
## 2026-05-10 — `generalFunctions` deprecated paths — RESOLVED
**Decision (2026-05-10):** Tracked as Phase 8.5 in `TASKS.md`. Cleanup
runs after promotion to main. The list of paths to remove is captured
there so it isn't lost.
---
## 2026-05-10 — Two child-storage shapes — RESOLVED
**Decision (2026-05-10):** Registry-as-truth, **with named getters** that
read clearly in code. `domain.machines` keeps working — it's a getter
that returns the rotatingMachine slice of `this.child`. Same for
`domain.stations`, `domain.machineGroups`, etc. Domain code reads
naturally; the registry is the source of truth underneath.
Named getters are declared by the domain subclass in `configure()`:
```js
configure() {
Object.defineProperty(this, 'machines',
{ get: () => this.child?.machine?.centrifugal ?? {} });
}
```
(`BaseDomain` provides a helper for this pattern.)
---
## 2026-05-10 — Async vs sync `tick()` — RESOLVED with redesign
**Decision (2026-05-10):** Default is **event-driven**. Ticks are
opt-in.
`BaseNodeAdapter` exposes two timers:
- `static tickInterval = null` — opt-in periodic tick. Default null = no
tick. Domain emits `'output-changed'` on `this.emitter` instead, and
BaseNodeAdapter subscribes to that event to push outputs.
- `static statusInterval = 1000` — always-on status badge poll.
Required because Node-RED's editor refresh expects a heartbeat. Set
to 0 only in headless test environments.
When opting into ticks:
- Document **why** in a one-line comment above
`static tickInterval = ...` (e.g. "needs delta-time for predicted
volume integrator").
- A node should opt in only when truly time-driven. Examples that need
it: `pumpingStation` (predicted volume integrates over time),
`measurement` (when simulator is enabled — ticks the random walk).
- Examples that DO NOT need it: `MGC` (recomputes on pressure events),
`rotatingMachine` (recomputes on measurement events + state changes).
`tick()` is treated as fire-and-forget (no await). A node that needs
serialisation uses `LatestWinsGate` internally.
See `CONTRACTS.md §2` for the BaseNodeAdapter shape.
---
## 2026-05-10 — ChildRouter wildcard subscriptions monkey-patch `emit` — RESOLVED
**Resolution (2026-05-11):** Switched to per-listener fan-out using the
canonical `POSITIONS` list and a 19-type set (`MeasurementContainer.measureMap`
keys + synthetic EVOLV types). Each partial-filter subscription enumerates
every concrete `<type>.<variant>.<position>` event name and registers a
plain `emitter.on()` per combo. Multi-parent works without emit patching.
ChildRouter.js 184 → 164 lines; 12/12 tests pass including a new
multi-parent regression test.
### Original entry below
## 2026-05-10 — ChildRouter wildcard subscriptions monkey-patch `emit` (history)
**Context:** P1.2 implementation. EventEmitter has no native wildcard.
Subscriptions with a partial filter (`{type}`-only or `{position}`-only)
install a per-variant `emit` proxy on the child's emitter; concrete
`{type, position}` filters use plain `emitter.on`.
**Question:** Multi-parent children. `child.parent` is already an array
in `childRegistrationUtils`, so a child can be registered under several
parents. If two parents each install ChildRouter wildcard proxies on
the same `child.measurements.emitter`, the wraps stack — but
`tearDown` only unwraps when its own bookkeeping is empty. Is this
correct semantics for multi-parent teardown ordering? Or should we
switch to per-listener fan-out (subscribe to every known
`<type>.<variant>.<position>` enumerated from a registry)?
**Default chosen:** Stacked wrappers. The current `childRegistrationUtils`
multi-parent path is rarely exercised in production. Revisit if
Phase 2 / Phase 4 hits a real multi-parent case.
**Decision needed by:** Phase 4.
---
## 2026-05-10 — `predictionHealth` migration in rotatingMachine
**Context:** P1.4 implementation flagged that the existing
`rotatingMachine.predictionHealth` carries `quality` (string) +
`confidence` (0..1 numeric) on top of the new `HealthStatus` shape's
`{level, flags, message, source}`.
**Question:** Where does `confidence` live after migration?
**Default chosen:** Keep `confidence` on the per-metric drift
container as a sibling to a `health: HealthStatus` field. Drift
diagnostics (`nrmse`, `longTermNRMSD`, `immediateLevel`) stay as
siblings too. `HealthStatus` carries only the standardised five fields.
**Decision needed by:** Phase 5 (`rotatingMachine` refactor).
---
## 2026-05-10 — `dashboardAPI` basic test broken (pre-existing) — RESOLVED
**Context:** P1.12 sanity gate. `dashboardAPI/test/basic/structure-module-load.basic.test.js` uses Mocha-style `describe()` globals which don't exist under `node:test`. Reports 0 pass / 1 fail with `ReferenceError: describe is not defined`.
**Action:** Pre-existing — not caused by Phase 1. Convert to `node:test` form during Phase 6 when `dashboardAPI` gets its skeleton refactor. Tracked here so it isn't lost.
**Update (P6.7, 2026-05-10):** Converted to `node:test` form (`const test = require('node:test')` + `assert.doesNotThrow`). Basic test now reports 1 pass / 0 fail. The Mocha-style `test/dashboardapi.test.js`, `test/nodeClass.test.js`, `test/integration/`, and `test/edge/` files still use jest/Mocha globals — out of scope for P6.7; deferred to P10 test-suite refactor.
---
## 2026-05-10 — `dashboardAPI` skipped BaseNodeAdapter + BaseDomain
**Context:** P6.7. dashboardAPI is a passive HTTP-emitter utility node: no
`generalFunctions/src/configs/dashboardapi.json`, no periodic Port-0/1
telemetry stream, no parent registration, no measurements, no tick loop,
no status badge. `BaseDomain` constructor would throw on the missing
config file; `BaseNodeAdapter._scheduleRegistration` would emit a
spurious `child.register` for a node that has no parent; the
`outputUtils.formatMsg` pipeline assumes a measurement-shaped output
which dashboardAPI lacks.
**Default chosen:** `nodeClass` stays a plain class (does **not** extend
`BaseNodeAdapter`); `specificClass` (`DashboardApi`) stays a plain class
(does **not** extend `BaseDomain`). Only the shared `commandRegistry`
is adopted (canonical topic `child.register` with `registerChild` alias
+ deprecation warning). One handler module in `src/commands/`. nodeClass
shrunk from 134 → 73 lines.
**Decision needed by:** Phase 7 / Phase 8 — revisit if `BaseNodeAdapter`
grows a passive/HTTP-only mode (skip-registration + skip-output-stream
flags) or if a `dashboardapi.json` config gets added to generalFunctions.
Either makes adoption straightforward; until then the bespoke shape is
correct.
---
## 2026-05-10 — pumpingStation: plain dicts vs `declareChildGetter` — RESOLVED 2026-05-11
**Resolution (2026-05-11, B2.1):** Migrated. `this.machines / machineGroups
/ stations` are now BaseDomain `declareChildGetter` accessors over the
`childRegistrationUtils` registry; the `predictedFlowChildren` Map and
the dict-mutation lines in the router `onRegister` callbacks are gone.
The `context()` override installs **live getters** for the same three
names on the returned ctx so SafetyController (which captures ctx once
at construct-time) keeps reading the live registry across later
registrations. specificClass.js 316 → 314 lines.
Affected test files rewritten to inject mock children through the real
handshake instead of dict-assignment:
- `test/basic/specificClass.test.js` — added a `registerMockGroup(ps, id)`
helper that builds a mock with `config.functionality.softwareType =
'machinegroup'`, a stub `measurements.emitter.on`, and instrumented
`handleInput` / `turnOffAllMachines`. All 9 `ps.machineGroups['mgc1']
= {...}` blocks now call the helper; the 4 sub-tests that previously
asserted on a captured `turnOffCalls` / `demands` array assert on the
helper-returned `mock._calls` instead.
- `test/integration/shifted-ramp-end-to-end.test.js``buildHarness()`
now calls a local `registerMockGroup(ps, 'mgc1', demands)` helper that
pushes into the existing `demands` array via the registered mock's
`handleInput`. No assertion shape changed.
128/130 pumpingStation tests pass after the migration (the 2 remaining
failures — `canonical topics dispatch to their handlers` and `set.inflow
accepts number payload …` in `test/basic/commands.basic.test.js` — are
pre-existing and unrelated to child storage).
### Original entry below
## 2026-05-10 — pumpingStation: plain dicts vs `declareChildGetter` (history)
**Context:** P2.7+P2.8+P2.9. The 2026-05-10 "Two child-storage shapes"
decision says use `declareChildGetter` (registry-as-truth), but the
existing pumpingStation test suite mutates `ps.machineGroups['mgc1'] = {...}`
directly to inject mock children before driving `_controlLevelBased`.
A getter-backed `machineGroups` returns a fresh object per call, so the
mutation is on a throwaway and the orchestrator never sees the mock.
**Default chosen:** Keep `machines / stations / machineGroups` as plain
id-keyed dicts on `this`. ChildRouter `onRegister` handlers populate them
on real registration; tests can still assign directly. Registry remains
the upstream source of truth (handshake still flows through it), but the
flat dicts are also writable. Revisit if other domains can adopt
`declareChildGetter` cleanly without test rewrites.
**Decision needed by:** Phase 10 (test-suite refactor).
---
## 2026-05-10 — `reactor` test runtime is mathjs-bound (pre-existing)
**Context:** P1.12 sanity gate. Every reactor test file takes ~13 s because `require('mathjs')` alone is ~12.5 s on this machine (mathjs is huge and loads its full operator set eagerly). With basic tests parallelised by `node --test`, each subprocess pays the cost. A 90 s outer timeout doesn't accommodate the parallel load.
**Action:** Pre-existing — not caused by Phase 1. Two options to track for Phase 5/6 cleanup:
1. Switch to a tree-shaken mathjs subset (only ops actually used).
2. Cache the mathjs instance at module top and pass into Reactor classes.
Tracked; not blocking the refactor.
---
## 2026-05-10 — measurement `isStable` tautology (pre-existing bug) — RESOLVED
**Resolution (2026-05-11):** Replaced the tautological `stdDev < stdDev*2`
check with a config-driven absolute threshold. New schema field
`calibration.stabilityThreshold` (number, ≥ 0, default `0.01` in
scaling-units) added to `generalFunctions/src/configs/measurement.json` so
all callers see it. `Calibrator.isStable()` now returns `true` when
`stdDev === 0` or `stdDev <= threshold`, falling back to the default when
the config slot is missing or non-numeric. The two BUG-PRESERVED calibrator
tests were rewritten — high-variance buffers now correctly report unstable
under the default and only flip to stable when an explicit relaxed
threshold is supplied. Added edge tests for the relaxed-threshold path,
constant-buffer-with-zero-threshold path, just-above-threshold path, and
missing-config fallback. `nodeClass.buildDomainConfig` and
`measurement.html` (defaults + form field + oneditsave) propagate the UI
value through to the domain. 100/100 measurement tests pass; 70/70
generalFunctions basic tests pass.
### Original entry below
## 2026-05-10 — measurement `isStable` tautology (pre-existing bug) (history)
**Context:** P3.4. The existing `isStable` in `measurement/src/specificClass.js` does:
```js
stableThreshold = stdDev * marginFactor; // marginFactor = 2
return { isStable: (stdDev < stableThreshold || stdDev == 0), stdDev };
```
`stdDev < stdDev * 2` is always true for `stdDev > 0`, and the OR catches the
zero case. So `isStable` returns `true` for every non-empty buffer. That makes
`calibrate()` essentially un-gateable (it only aborts when there are < 2
samples) and `evaluateRepeatability()` happily reports a huge stdDev as
"repeatability".
**Action:** Preserved verbatim by the new `Calibrator` (additive). A
behavioural fix needs an external reference (config-driven absolute
threshold, or % of full scale). Two BUG-PRESERVED tests pin the current
shape so a follow-up behavioural PR is intentional.
**Decision needed by:** Phase 10 (test-suite refactor) — naturally
adjacent to the calibration test cleanup.
---
## 2026-05-10 — `commandRegistry` payload schema needs `'none'`/`'void'` type — RESOLVED
**Resolution (2026-05-11):** Added `'none'` to the payloadSchema.type
enum. Handler still fires; logs `warn` if `msg.payload` is non-empty
(catches accidental object payloads on trigger topics). Also added an
optional `description` field per descriptor for wikiGen consumption.
23/23 commandRegistry tests pass; CONTRACTS.md §4 updated.
### Original entry below
## 2026-05-10 — commandRegistry payload schema needs 'none'/'void' type (history)
**Context:** P3.7+P3.8. Trigger-only commands (`set.simulator`,
`set.outlier-detection`, `cmd.calibrate`) ignore their payload. The
current registry's `payloadSchema.type` enum is
`'string'|'number'|'object'|'boolean'|'any'`. Trigger commands fall
into `'any'`, which is too permissive (an object slipped past would
not be flagged).
**Default chosen:** Use `'any'` for now. Add `'none'`/`'void'` to the
registry schema enum during Phase 7 (topic-name standardisation).
**Decision needed by:** Phase 7.
---
## 2026-05-10 — measurement legacy `'mAbs'` emitter event — RESOLVED
**Resolution (2026-05-11):** Removed the on-emit subscription that
bridged the analog channel's `<type>.measured.<position>` event to
`source.emitter` as `'mAbs'`. No production consumer was reading it.
96/96 measurement tests pass.
### Original entry below
## 2026-05-10 — measurement legacy 'mAbs' emitter event (history)
**Context:** P3.7+P3.8 CONTRACT.md noted that the existing `Measurement`
class emits `'mAbs'` on `source.emitter` whenever the analog output
updates. This was a pre-MeasurementContainer broadcast. It's still
fired but no production consumer reads it (per the existing comment
"DEPRECATED: Use measurements container instead").
**Default chosen:** Keep firing it through Phase 3 (post-integration).
Remove in Phase 7 alongside the topic-rename cleanup, or in Phase 8.5
deprecated-path cleanup.
**Update (P3.2+P3.5+P3.6+P3.9, 2026-05-10):** Re-emitted from the analog
specificClass by subscribing to the MeasurementContainer's
`<type>.measured.<position>` event (position lowercased to match
container normalisation). Channel itself stays event-name-agnostic.
**Decision needed by:** Phase 7 / Phase 8.5.
---
## 2026-05-10 — measurement legacy property mirrors
**Context:** P3.2+P3.5+P3.6+P3.9. The analog pipeline now lives inside
`Channel`. The pre-refactor test suite pins many fields directly on the
Measurement instance: `outputAbs`, `outputPercent`, `storedValues`,
`totalMinValue`, `totalMaxValue`, `totalMinSmooth`, `totalMaxSmooth`,
`inputRange`, `processRange`. Some tests *write* `m.storedValues` /
`m.totalMinValue` directly before calling pipeline helpers.
**Default chosen:** Install getter/setter mirrors on the Measurement
instance (`_installChannelMirrors`) that forward read/write through
`this.analogChannel`. Storage stays single-sourced in Channel, the
legacy public surface stays writable, no test rewrites required.
**Decision needed by:** Phase 10 (test-suite refactor) — replace these
with direct `m.analogChannel.xxx` access in tests, then drop the mirrors.
---
## 2026-05-10 — measurement `handleScaling` mutates config.scaling
**Context:** P3.2+P3.5+P3.6+P3.9. Channel's `_applyScaling` resets its
*own* `scaling.inputMin/inputMax` to `[0,1]` when the input range
collapses (`inputMax <= inputMin`). The pre-refactor `handleScaling`
mutated `this.config.scaling.inputMin/inputMax` instead, and a basic
test pins that contract.
**Default chosen:** The Measurement-level `handleScaling` delegate
copies Channel's reset back to `config.scaling` after the call so the
visible behaviour is preserved. Long-term, the test should read the
new state from `m.analogChannel.scaling` and we drop the mirror write.
**Decision needed by:** Phase 10 (test-suite refactor).
---
## 2026-05-10 — measurement nodeClass routing tests pin private wiring
**Context:** P3.2+P3.5+P3.6+P3.9. The basic `nodeclass-routing` and
edge `invalid-payload` tests instantiated `NodeClass.prototype` and
called `_attachInputHandler()` / `_registerChild()` directly. The
BaseNodeAdapter superclass renamed these to `_attachInputHandler`
(unchanged) and `_scheduleRegistration` (was `_registerChild`), and
dispatch now goes through `this._commands` built in the constructor.
**Action:** Adjusted the two tests in-place to seed `inst._commands`
via `createRegistry(commands, …)` and to call `_scheduleRegistration`
instead of `_registerChild`. The on-the-wire payload topic also moved
from `'registerChild'``'child.register'` (BaseNodeAdapter
convention); the test assertion was updated accordingly.
**Decision needed by:** Phase 10 — these tests should be rewritten to
drive a full nodeClass through `new nodeClass(uiConfig, RED, node,
'measurement')` rather than poking at private members.
---
## 2026-05-10 — MGC `calcAbsoluteTotals` implicit pressure-key coupling
**Context:** P4.1/4.2 extracted `totals/totalsCalculator.js` preserving
original behaviour. `calcAbsoluteTotals` iterates
`machine.predictFlow.inputCurve` and re-uses the same pressure key to
index `machine.predictPower.inputCurve[pressure]`. If the two curves were
sampled at different pressures (legitimate when power was extrapolated
separately from flow), the lookup is `undefined` and the call throws.
**Question:** should the totals calculator defensively skip mismatched
pressure keys, or should the invariant "flow + power curves share pressure
keys" be enforced upstream in rotatingMachine's curveLoader/normalizer?
**Default chosen:** preserved the implicit coupling — no behavioural change.
**Decision needed by:** P5 (rotatingMachine refactor) — curveLoader/Normalizer
is the natural place to enforce or document the pairing.
---
## 2026-05-10 — MGC concern modules use legacy unitPolicy object shape — RESOLVED
**Resolution (2026-05-11):** UnitPolicy.declare() now exposes
canonical/output/curve as BOTH callable methods AND frozen property
bags. Both shapes work: `policy.canonical('flow')` and `policy.canonical.flow`.
Dropped the `_unitView`/`unitPolicyView` workaround in both MGC
(specificClass 336→318) and rotatingMachine (400→377). CONTRACTS.md §6
updated. All platform tests stay green.
### Original entry below
## 2026-05-10 — MGC concern modules use legacy unitPolicy object shape (history)
**Context:** The MGC concern modules (groupOps/groupOperatingPoint,
totals/totalsCalculator, combinatorics/pumpCombinations, control/strategies)
extracted in Wave 1 read units as `ctx.unitPolicy.canonical.flow` — the old
plain-object shape carried on the pre-refactor specificClass. `BaseDomain`
now wires `this.unitPolicy` to a `UnitPolicy` instance whose canonical/output
are methods (`canonical('flow')`).
**Question:** Should the concern modules be updated to call the methods, or
should we keep the object-shaped view long-term?
**Default chosen:** specificClass builds a frozen `this._unitView` ({
canonical: {flow,pressure,power,temperature}, output: {…} }) and passes it
to the modules. Two surface shapes live side-by-side in the same node.
**Decision needed by:** P5 (rotatingMachine) — the same concern-module
shape will likely repeat. Pick one and migrate before the second node lands
on the pattern.
---
## 2026-05-10 — rotatingMachine Machine constructor takes 3 positional args
**Context:** P5.9/5.10/5.12. The pre-refactor Machine class accepted
`(machineConfig, stateConfig, errorMetricsConfig)`. BaseDomain's
constructor only knows about the first slot. The whole test suite (~30
files) constructs Machines directly with two positional args, and
BaseNodeAdapter instantiates DomainClass with just `this.config`.
**Question:** Where do the extra positional configs travel? Schema
validation in `configUtils.initConfig` strips unknown top-level keys, so
embedding them in machineConfig doesn't work. Subclass-overriding
constructor before super() is blocked by ES6's pre-super `this` rule.
**Default chosen:** Static stash on the class itself
(`Machine._pendingExtras`) assigned just before `super()` (or by
nodeClass.buildDomainConfig before BaseNodeAdapter instantiates the
domain). `configure()` reads + clears it. Single-threaded JS makes the
hand-off race-free.
**Decision needed by:** P10 (test-suite refactor) — when tests get
rewritten to use the BaseNodeAdapter-built domain, drop the multi-arg
constructor and fold stateConfig/errorMetricsConfig into machineConfig
slices.
---
## 2026-05-10 — rotatingMachine private nodeClass tests (4 files adjusted) — RESOLVED 2026-05-11
**Resolution (2026-05-11, P10):** Rewritten to drive only the public
BaseNodeAdapter surface. Three test files were rewritten:
- `test/basic/nodeClass-config.basic.test.js``buildAdapter(ui)`
constructs a full `new nodeClass(ui, RED, node, 'rotatingMachine')`
and asserts against `inst.source.config.*` (the validated merged
shape from `configManager.buildConfig`) and observable state on the
domain. No `Object.create(NodeClass.prototype)` or direct
`buildDomainConfig` calls — `Machine._pendingExtras` is no longer
touched by tests.
- `test/edge/nodeClass-routing.edge.test.js` — dispatch is driven via
`node._handlers.input(msg, send, done)` (the handler the base
installs on `node.on('input', …)`). Assertions are against
`node._sent`, instrumented `source.handleInput` call lists, and the
`childRegistrationUtils.registerChild` side-effect. Status-badge
pressure-warn case calls `inst.source.getStatusBadge()` directly,
not `io.buildStatusBadge(source)`.
- `test/edge/error-paths.edge.test.js` — the error-on-status-badge
test now builds the adapter, forces `state.getCurrentState` to
throw, and asserts via `inst.source.getStatusBadge()`. The three
pre-existing Machine-direct-construction tests were untouched
(they never poked nodeClass privates).
Teardown of the always-on status-poll timer goes through the public
`node._handlers.close(() => {})` path (the BaseNodeAdapter close
handler) so the rewritten tests don't reach into `inst._statusUpdater`.
Verification: `npm test` reports 202 pass / 0 fail (up from 196 — net
+6 tests across the three rewritten files). No `inst._<private>`,
`_attachInputHandler`, `_commands = createRegistry`, `_pendingExtras`,
or `io.buildStatusBadge` references remain in the rewritten files.
### Original entry below
## 2026-05-10 — rotatingMachine private nodeClass tests (4 files adjusted) (history)
**Context:** P5.9/5.10/5.12. Four pre-refactor tests pinned private
nodeClass methods: `_loadConfig`, `_setupSpecificClass`,
`_updateNodeStatus`, and the inline `_attachInputHandler` switch. After
the BaseNodeAdapter migration those private methods are gone — config
build lives in `buildDomainConfig()`, dispatch in `commands/`, status
badge in `source.getStatusBadge()`.
**Default chosen:** Updated the four test files to drive the new
surface: `buildDomainConfig` returns the per-node slice (and stamps
`Machine._pendingExtras`); routing tests seed `inst._commands` via
`createRegistry(commands, …)` and assert through that path; status-badge
tests call `io.buildStatusBadge(source)` directly.
**Decision needed by:** P10 — these tests still poke private members
(`_commands`, `_attachInputHandler`). The right shape is constructing
a full `new nodeClass(uiConfig, RED, node, 'rotatingMachine')` and
asserting against the resulting `node._sent` / `node._statuses`.
---
## 2026-05-10 — monster schema strips command-line constraint keys — RESOLVED 2026-05-11
**Context:** P6.3. The monster JSON schema in `generalFunctions/src/configs/monster.json` defines `samplingtime`, `minVolume`, `maxWeight` and others under `constraints`, but NOT `nominalFlowMin`, `flowMax`, `maxRainRef`, `minSampleIntervalSec`. `configUtils.initConfig` strips these unknown keys with a `Unknown key … Removing it.` warning. The legacy code read them anyway — `Number.isFinite(undefined)` returns false, so guards naturally route into invalid-bounds territory and tests pass via the undefined cascade.
**Resolution (2026-05-11, B1.4):** Added the four missing fields (`nominalFlowMin`, `flowMax`, `maxRainRef`, `minSampleIntervalSec`) to the `constraints` section of `generalFunctions/src/configs/monster.json` with sensible defaults (0/0/10/60). The unknown-key warning disappears and user-supplied values now propagate through validation to the domain, restoring the documented sampling behaviour.
---
## 2026-05-10 — monster sampling-guards cooldown test fails on development (pre-existing) — RESOLVED 2026-05-11
**Context:** P6.3 baseline run. `test/edge/sampling-guards.edge.test.js` "cooldown guard blocks pulses when flow implies oversampling" already fails on `development` BEFORE the refactor (`assert.ok(monster.sumPuls > 0)` — sumPuls stays at 0 across 80 ticks). The legacy in-file equivalent in `test/monster.test.js` (Mocha-style wrapper, not picked up by `node:test`) appears to have passed in an earlier era.
**Resolution (2026-05-11, B1.4):** Root cause was the schema-stripping issue documented immediately above — `nominalFlowMin`/`flowMax`/`minSampleIntervalSec` were stripped by `configUtils.initConfig` before reaching the domain, so `validateFlowBounds` saw NaN/NaN and routed every `i_start` into the invalid-bounds early return, which prevented `_beginRun` from ever firing. With the four constraint keys now declared in `monster.json`, the test config propagates intact: `_beginRun` runs, the m3PerTick integrator accumulates ~0.056 m3/tick, `temp_pulse` crosses 1 at tick ~18, the first pulse fires, subsequent pulses are correctly blocked by the 60 s cooldown, and `sumPuls > 0` / `missedSamples > 0` / `bucketVol > 0` / `getSampleCooldownMs() > 0` all hold. Added a guard-site comment in `parameters/parameters.js#validateFlowBounds` pointing back at the schema contract. All 10/10 monster tests green.
---
## 2026-05-10 — MGC handleInput retained inline latest-wins (not DemandDispatcher) — RESOLVED
**Resolution (2026-05-11):** Extended `LatestWinsGate` with
`fireAndWait(value)` that returns a per-fire settlement promise. A
parked call superseded by a later fire resolves with the frozen
sentinel `LatestWinsGate.SUPERSEDED = { superseded: true }` (not a
reject) so callers branch on a value without try/catch. Dispatch
errors still resolve the promise (with `undefined`) and surface via
`gate.lastError`.
MGC's `handleInput` now delegates to `DemandDispatcher.fireAndWait`;
the inline `_dispatchInFlight` + `_delayedCall` block is gone.
`turnOffAllMachines` calls `cancelPending()` on the dispatcher
instead of zeroing `_delayedCall`. `LatestWinsGate.js` 75 → 116 lines
(under the 150 cap). MGC `specificClass.js` net 14 lines.
The `turnoff-deadlock` test that pinned `_delayedCall` was rewritten
to assert against the parked `fireAndWait` resolving as superseded.
Other awaiting tests (`ncog-distribution`, `idle-startup-deadlock`,
`demand-cycle-walkthrough`) needed no change since `fireAndWait`
preserves the "await waits for the call's dispatch" shape for
non-superseded calls. All 77/77 MGC tests pass; 12/12 LatestWinsGate
basic tests pass.
### Original entry below
## 2026-05-10 — MGC handleInput retained inline latest-wins (not DemandDispatcher) (history)
**Context:** Wave 1 added `src/dispatch/demandDispatcher.js` wrapping
`LatestWinsGate`. Tests (`turnoff-deadlock`, `idle-startup-deadlock`,
`ncog-distribution`) call `await mgc.handleInput(...)` and rely on the
awaited promise resolving after the dispatch completes; they also pin the
exact `_delayedCall` field. `LatestWinsGate.fire(value)` returns void.
**Question:** Should `handleInput` switch to the gate (changing the test
contract), or stay inline (keeping the awaitable shape)?
**Default chosen:** kept the inline `_dispatchInFlight + _delayedCall`
gate verbatim. `DemandDispatcher` remains exported but unused by the
orchestrator for now — its basic test still passes since it tests the
wrapper in isolation.
**Decision needed by:** P7 (topic-name standardisation) or P10 (test-suite
refactor) — adopting the gate requires either rewriting tests to drain
the gate or changing the gate to return a settle promise.
---
## 2026-05-10 — valveGroupControl registerChild overload (skipped ChildRouter)
**Context:** P6.2. `ValveGroupControl.registerChild(child, posOrType)` is
called from two distinct paths: (a) `childRegistrationUtils.registerChild`
delegates with the canonical softwareType as 2nd arg, and (b) tests + a
few in-process callers invoke it directly passing **either** a position
(`'atEquipment'`) **or** a softwareType (`'machine'`). The legacy code
disambiguated via a `KNOWN_POSITIONS` set lookup and returned a boolean
indicating registration success (used by `flow-distribution` regression
test to assert a non-valve payload yields `false`).
**Default chosen:** kept the legacy resolver in the domain — override
`this.registerChild` inside `configure()` so the boolean return + dual
semantics survive. `ChildRouter` is **not** used for VGC (no `onRegister`
/ `onMeasurement` handlers declared). Source-side event wiring still
lives in `src/sources/fluidContract.js` (raw emitter `.on` on each
`SOURCE_FLOW_EVENTS` name) because the source family includes mixed-case
event names (`flow.predicted.atEquipment` and lowercase variants both fire).
**RESOLVED 2026-05-11 (B2.2):** Migrated to `ChildRouter.onRegister`.
`configure()` now declares `router.onRegister('valve', …)` plus one
`onRegister(…)` per canonical source softwareType (`machine`,
`machinegroup`, `pumpingstation`, `valvegroupcontrol`); the custom
overloaded `registerChild` and `_resolveRegistrationContext` resolver
were removed and BaseDomain's default `registerChild` (which delegates
straight to `router.dispatchRegister`) is back in charge. Position now
comes from `child.positionVsParent` (set by `childRegistrationUtils`) or
`child.config.functionality.positionVsParent`, falling back to
`atEquipment`. The boolean-return regression test was rewritten to assert
via the side-effect (`Object.keys(group.valves).length === 0`) for the
non-valve-like payload, and a new test pins router dispatch through
`childRegistrationUtils.registerChild(valve, 'upstream')` honouring the
config's `positionVsParent`. Source-side measurement-event wiring still
lives in `sources/fluidContract.bindSource` — the mixed-case
`flow.{measured,predicted}.atEquipment` listeners remain raw `.on`
attachments until topic casing standardises platform-wide. specificClass
shrank 270→255 lines; tests 9→10, all green.
---
## 2026-05-10 — valveGroupControl `set.position` placeholder
**Context:** P6.2 command registry. Task spec required canonical name
`setpoint → set.position`, but VGC's pre-refactor input switch did not
implement a `setpoint` topic — valve position is driven by `data.totalFlow`
re-distribution, not direct per-valve setpoints. Registering `set.position`
with an empty handler keeps the canonical name reserved without breaking
the contract surface.
**Default chosen:** registered `set.position` with a no-op handler that
debug-logs the payload. `setpoint` listed as alias so a legacy emitter
gets the same no-op path.
**Decision needed by:** P7 — decide whether VGC actually needs a
per-valve setpoint topic (probably yes when virtualControl mode lands).
At that point promote the handler from no-op to real dispatch.
---
## 2026-05-10 — reactor private nodeClass tests (8 files adjusted) — RESOLVED 2026-05-11
**Resolution (2026-05-11, P10):** Rewrote all 8 reactor test files to drive
only the public BaseNodeAdapter surface — `new nodeClass(uiConfig, RED, node,
'reactor')`, then fire msgs through `node.handlers.input(...)` and observe
via `node.sends` / `node.statuses` / `inst.source.engine.*` /
`inst.source.tick(dt)`. The pre-refactor private methods (`_loadConfig`,
`_setupClass`, `_attachInputHandler`, `_updateNodeStatus`, `_registerChild`,
`_tick`, `_startTickLoop`, `_attachCloseHandler`) are no longer referenced.
`buildDomainConfig` is invoked on the real constructed instance (it's the
documented override hook in CONTRACTS.md §2). `_emitOutputs` is called on
the real instance for the tick-loop assertions (it's the reactor-specific
override for Port-0 emission, also documented). The "scheduled registration"
test now waits ~130 ms for the BaseNodeAdapter setTimeout to fire and
inspects the resulting Port-2 send. 46/46 reactor tests pass (was 39 pre-
rewrite — net +7 tests added covering canonical topic acceptance, alias
acceptance, child-with-no-source guard, empty-string reactor_type, missing-
topic guard, and the new Reactor.tick(dt) wrapper introduced in B2.3).
### Original entry below
## 2026-05-10 — reactor private nodeClass tests (8 files adjusted) (history)
**Context:** P6.5. Eight pre-refactor reactor tests pinned private
nodeClass methods (`_loadConfig`, `_setupClass`, `_registerChild`, inline
`_attachInputHandler` switch, `_tick`, `_startTickLoop`, `_attachCloseHandler`).
After the BaseNodeAdapter migration those private methods are gone — config
build lives in `buildDomainConfig()`, dispatch in `commands/`, registration in
`_scheduleRegistration` (renamed), and the periodic emit lives in
`_emitOutputs` (overridden so the Fluent / GridProfile Port-0 contract is
preserved — delta-compressed payloads can't carry the C-vector).
**Default chosen:** Adjusted in place: `test/basic/constructor.basic.test.js`,
`test/basic/input-routing.basic.test.js`, `test/basic/register-child.basic.test.js`,
`test/basic/speedup-factor.basic.test.js`, `test/edge/invalid-topic.edge.test.js`,
`test/edge/missing-child.edge.test.js`, `test/edge/invalid-reactor-type.edge.test.js`,
`test/integration/tick-loop.integration.test.js`. Routing tests seed
`inst._commands` via `createRegistry(commands, …)`; topic moved from
`'registerChild'``'child.register'`. The "unknown reactor_type throws"
edge case became "falls back to CSTR" — the legacy bottom-of-switch already
fell back to CSTR; only the surface changed (warning channel now via
domain logger, not `node.warn`).
**Decision needed by:** Phase 10 — same shape as the rotatingMachine /
measurement adjustments. The right fix is to drive a full
`new nodeClass(...)` and assert against `node._sent` / `node._statuses`
instead of poking private members.
---
## 2026-05-10 — reactor schema enum lowercases `reactor_type`
**Context:** P6.5. The reactor JSON schema defines `reactor.reactor_type`
as `type: 'enum'` with values `'CSTR'` / `'PFR'`. The shared enum validator
lowercases the user-supplied value before comparing, so an inbound `'PFR'`
ends up stored as `'pfr'` in the validated config. The pre-refactor
nodeClass switched on the raw uiConfig value and never saw the lowercased
form; after the BaseDomain migration the wrapper reads the validated
config and would always fall back to CSTR.
**Default chosen:** `Reactor._buildEngine` upper-cases the value before
switching. The schema is left intact so external Phase-7 enum-casing
work can decide whether to preserve original casing globally.
**Decision needed by:** Phase 7 (topic-name + schema standardisation) —
once enums standardise on a canonical casing, drop the `.toUpperCase()`
guard here.