Skip to content

Clear the gauge reading when a gauge is re-pointed at another path - #577

Merged
mairas merged 3 commits into
mainfrom
fix/gauge-reset-on-path-change
Aug 13, 2026
Merged

Clear the gauge reading when a gauge is re-pointed at another path#577
mairas merged 3 commits into
mainfrom
fix/gauge-reset-on-path-change

Conversation

@mairas

@mairas mairas commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Why

Re-point a Radial, Linear or Compass gauge at a path that reports nothing — edit gaugePath from the options dialog on a live dashboard — and the gauge kept the previous path's needle position and numeric value, presented as a live reading of the new path.

dataAvailable had exactly one writer, inside the streams.observe() callback. All three gauges set suppressBootstrapNull: true, and WidgetStreamsDirective.buildAndSubscribe recreates the suppression closure with a fresh seenNonNull = false on every rebuild, so a rebuild against a silent path filters the replayed leading null and the callback never runs. widget-host2 reconfigures in place, so the component instance and its signals survive the save.

What

Each gauge now clears its stream-derived state — dataAvailable, value, textValue, the tagged measure, and the zone state — when the configured path changes identity.

The reset is gated on a path signature rather than run unconditionally: the data effect also re-runs on theme changes, and clearing there would blink the needle off and back on at every switch. widgetPathSignature is extracted from the directive's existing computePathSignature, so the widget and the subscription diff agree on what "the same reading" means (path, pathType, convertUnitTo, source, bootstrap-null policy) instead of drifting apart.

One behaviour change beyond the report: the Linear gauge used to reset effectiveUnit on every run of the effect, so a theme switch briefly redrew its scale in the widget's stored unit instead of the server-resolved one. That reset moved under the same gate.

Tests

Two per gauge — re-point to a silent path clears the reading, and an unrelated reconfigure (theme change) does not. All four fail on main (the linear "keeps the reading" case fails there on the effectiveUnit wipe described above).

Full suite: 2035 passing, three consecutive clean runs. One earlier local run had a spec file abort during teardown (the documented EnvironmentTeardownError class, unrelated to these files); it did not reproduce.

Fixes #534

@mairas

mairas commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

Code review — correctness + testing, findings applied

Two reviewers, same defect

Clearing a widget's path left its reading on the dial. The no-path early return ran before clearReadingOnRepoint, so widgetPathSignature's deliberate null-for-no-path branch was unreachable. Meanwhile applyStreamsConfigDiff takes the opposite branch for the same config and unsubscribes — so the gauge kept a number with nothing feeding it. Same lie as #534, reached a different way, and the path control is not a required field.

Fixed: the signature is computed first and gates the observe, so a cleared path clears the reading.

That fix exposed a second one the correctness reviewer had already flagged: null doubled as both the first-run sentinel and "no usable path", so a null signature reset the guard and suppressed the next re-point's clear. The sentinel is now undefined.

The highest-value missing test

No test covers re-pointing at a path that DOES report — the clear-then-observe ordering is unpinned

All six original tests stubbed observe with a fake that never calls back, so every one exercised only the silent-path branch. But the real directive's base is a BehaviorSubject: a path holding a value replays it synchronously, inside the same block as the clear. That ordering is the whole reason the fix does not blank every gauge on every re-point — and moving the clear into its own effect, or deferring it a microtask, would have kept all six green.

Added per gauge: a fake that replays synchronously, asserting the new path's reading is on screen. That is the only test separating "clears stale data" from "clears all data".

Applied

Finding Reviewers Fix
Clearing the path bypasses the reset entirely correctness P3, testing P2 Signature computed before the bail-out and gates the observe
null is both the first-run sentinel and "no usable path" correctness P3 Sentinel is undefined
Re-point to a live path untested — the ordering that makes this safe testing P2 Test per gauge, replaying synchronously
Theme tests have no positive control; they pass even if the effect never re-ran testing P3 observe count asserted to have incremented
Zone/alarm state reset is asserted by nothing testing P3 Test that an old path's Alarm does not carry over
Linear asserts a signal where the rendered header is available testing P3 Asserts the rendered .gaugeUnit
widgetPathSignature is new exported API with no direct test testing P3 Unit spec: null/whitespace paths, source: null'default', each field separating identities

Tests went 6 → 15 across the three gauges, plus 5 for the extracted helper. Suite: 2046 passing.

Confirmed by review, no change needed

The reviewer checked several things I had reasoned about but not verified, and they hold: every reset target equals its initializer in all three components (so skipping the first run is safe); the radial's computed textValue follows dataAvailable to the same '--' the other two write; gaugeBootstrapped staying set is right, because the BehaviorSubject replay means the template only ever sees old → new; and computePathSignature's ?? '' fallback is unreachable from the directive, so the refactor is behaviour-preserving.

One decision the tests now record rather than leave implicit: convertUnitTo is part of the signature, so a unit-only edit clears the reading. For a live path the replay repaints it at once; for a quiet path it drops to --. That is intended — the old number was in the old unit.

@mairas
mairas force-pushed the fix/gauge-reset-on-path-change branch from fe5e848 to 359b9ad Compare August 12, 2026 21:41
@mairas

mairas commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Code review — 5 personas, findings applied

Reviewers: correctness, testing, maintainability, project-standards, adversarial. Fresh contexts, isolated worktree.

The core fix holds

Correctness traced the diff, the streams directive, Host2's reconfigure ordering and DataService's replay semantics: the '' → null signature change is behaviour-preserving (the signature is computed only at guarded call sites), the clear runs before the resubscribe in all three components, and subscribePath returns a BehaviorSubject so a live path re-delivers synchronously inside the same block — no transient blank is observable. Host2 calls applyStreamsConfigDiff synchronously before the effect flushes, which is the safe order.

Adversarial could not break it on the scenarios that would matter most: theme switch, unit-preference change, profile reload and dashboard duplication produce no signature change; a same-content config from a new object still compares equal because the signature is a string; data arriving between the clear and the new subscription is impossible synchronously.

Three branches the specs did not hold

Testing mutation-checked each one:

Mutation Result
Move the clear below the no-path bail-out (linear) 16/16 still green
Same (compass) 6/6 still green
Delete currentState.set(States.Normal) (linear + compass) 22/22 still green

Radial had the equivalent cases; linear and compass got the main re-point scenario copied but not the rest. currentState drives colorValueText independently of dataAvailable, so without the reset an alarm-red readout from the old path survives onto the new one. Four tests added; the suite is 2057 now.

The dependency the fix rests on, written down

Two reviewers independently found that the reading comes back only because each effect run passes a new closure to observe(): the directive compares callback identity as well as the signature, so it rebuilds and replays a second time. Hoist that callback to a stable reference — which the directive's own docs recommend — and the gauge stays blank on a live path until the next delta. widget-numeric already sits on that side.

Nothing recorded this, and no test can catch a regression: all three specs fake the streams directive with an observe that replays regardless of callback identity. It is now documented at each clearReadingOnRepoint, and the seam question is #585.

Also applied

  • The lastPathSignature doc comment said "null until the first subscription" in all three copies. The initializer is undefined, and null is a distinct real identity — a cleared path that must still compare unequal to the path that follows. Corrected.
  • The compass matched its negative-to-port list against the raw configured path while the signature and the subscription both normalized it. pathRequiredValidator rejects only null and '', so a padded path subscribed fine and then clamped a −30° apparent wind angle to 0 instead of converting it to 330°. Pre-existing, but the PR normalizes on the adjacent line.
  • CLAUDE.md now states the widget-side obligation; it previously described WidgetStreamsDirective as owning subscriptions and said nothing about who clears derived state.

Not actioned

Three findings are real and out of this PR's scope, filed as #585:

  • 13 other streams.observe callers have the same defect, and the widget schematic still scaffolds the vulnerable shape.
  • The reset belongs in WidgetStreamsDirective, which already computes the signature and knows the moment it rebuilds — that would fix every consumer at once and remove the need to export the helper.
  • Clearing a path entirely leaves the previous path's zone bands painted: WidgetMetadataDirective.observe() returns early on a falsy path without resetting _meta, and highlights never gates on whether a path is configured. Verified against the real directive — zones().length stayed at 2 after applyMetaConfigDiff with path: null. That directive has no spec file at all.

Also left: gaugeBootstrapped is not reset on a re-point, so the first reading on the new path tweens up from the scale minimum rather than from the old value. Cosmetic, one animation.

Gate on the head commit: 2057 tests pass, snc clean, lint clean.

mairas added 3 commits August 13, 2026 10:55
dataAvailable, value and textValue were written only inside the stream
callback, and a rebuilt subscription against a silent path replays
nothing -- the leading null is suppressed with a fresh closure -- so the
previous path's needle stayed on the dial as a live reading of the new
one. Gate a reset on the path signature so a theme change, which rebuilds
the same subscription, leaves the reading alone.

Fixes #534
…inel

Review found the no-path early return ran before the clear, so emptying a
widget's path tore the subscription down and left its reading on the dial
-- the same lie, reached another way. The signature is now computed first
and gates the observe, and `undefined` marks "never run" so a null
signature is a real identity rather than a reset of that guard.

Tests: re-point to a path that DOES report (the case separating "clears
stale data" from "clears all data"), a cleared path, the re-point after
one, zone-state reset, positive controls on the theme tests, and direct
coverage for widgetPathSignature.

Fixes #534
…-point

Mutation testing found three branches the specs did not hold. Moving the clear
below the no-path bail-out left linear and compass green; deleting their
currentState reset left them green too. Both are now pinned, matching the radial.

The compass matched its negative-to-port list against the raw configured path
while the signature and the subscription both used the normalized one, so a
padded path subscribed correctly and then clamped a negative apparent wind angle
to 0 instead of converting it to a bearing.

The sentinel's doc comment named the wrong value for the first-run state and
left null's meaning unstated, in all three copies. The dependency on observe()
receiving a fresh closure each run is now written down where it can be read.
@mairas
mairas force-pushed the fix/gauge-reset-on-path-change branch from 619a524 to 80bc27f Compare August 13, 2026 07:56
@mairas
mairas merged commit f0ad575 into main Aug 13, 2026
4 checks passed
@mairas
mairas deleted the fix/gauge-reset-on-path-change branch August 13, 2026 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Gauges keep the old reading after being re-pointed at a silent path

1 participant