13da7388ff
Resolves the 5 open questions answered during Phase 1 setup: - Topic naming: canonical from Phase 1 (set/cmd/data/child/query/evt), with full glossary in CONTRACTS.md §1. - Parent EVOLV branch lineage: rebased onto origin/main. - Deprecated paths: tracked as Phase 8.5 in TASKS.md. - Child storage: registry-as-truth + named getters via declareChildGetter. - Tick: opt-in via static tickInterval; default is event-driven via source.emitter 'output-changed'. statusInterval (always-on, 1Hz) is separate. Plus two new pre-existing-issue notes from the sanity gate: - dashboardAPI uses Mocha-style describe() under node:test (broken). - reactor tests are mathjs-bound (~13s/file load). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
162 lines
6.1 KiB
Markdown
162 lines
6.1 KiB
Markdown
# 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.
|
|
|
|
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`
|
|
|
|
**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)
|
|
|
|
**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.
|
|
|
|
---
|
|
|
|
## 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.
|
|
|
|
---
|