Dashboard API hardening: correctness fixes + widget-type registry + nested-layout ergonomics#178
Merged
Merged
Conversation
…ist (P0-1)
setWidgetPosition/getWidgetByTitle/markAllDirty/preview read obj.Widgets
directly, so they silently broke once addPage() was used — widgets then live
under obj.Pages{ActivePage}.Widgets and obj.Widgets is empty {}. getWidgetByTitle
returned [] for widgets that exist, setWidgetPosition threw invalidIndex for valid
indices, markAllDirty flagged nothing, and preview printed "(empty)". Route all
four through the existing activePageWidgets()/allPageWidgets() accessors, mirroring
removeWidget. Single-page behaviour is byte-identical.
Adds multi-page + single-page regression tests to TestDashboardMultiPage.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
(cherry picked from commit 3882536d8ab739770b223815b9d074073b5dc41f)
BarChart/Histogram/Heatmap/Scatter read obj.Sensor.Y (and SensorX/SensorY.Y) directly. DerivedTag and CompositeTag expose no public .Y, so binding one to these widgets threw MATLAB:noSuchMethodOrField at refresh — even though they are first-class members of the documented Tag hierarchy and getXY() is the contract data accessor (FastSenseWidget already uses it). Route all four widgets' refresh and asciiRender data reads through Tag.getXY(), which works uniformly across SensorTag/StateTag/DerivedTag/CompositeTag. SensorTag and DataFcn paths unchanged. Adds a DerivedTag-refresh regression test to each of the four widget suites. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit 557a0315c36d726a1c83703ce50813f8e5316816)
…to linesForWidget (P0-3) Two silent data-loss bugs on the serialized-dashboard backward-compat path: 1. BarChart/Histogram/Heatmap/Scatter toStruct wrote their data source (callback function / dual-sensor keys) but fromStruct never read it, so save/load returned charts with no binding — they rendered empty with no warning. Each fromStruct now restores its source (str2func for callbacks, TagRegistry for Scatter sensors), warns with a namespaced *:sourceUnresolved id when a key can't be resolved, and stays silent for old structs that have no source field. 2. DashboardSerializer.save() open-coded a per-type switch that was a lossy duplicate of linesForWidget — number/status/gauge .m export dropped the ValueFcn/StaticValue/StatusFcn bindings that exportScript preserves. save() now delegates per-widget emission to linesForWidget (keeping the group case, which needs a captured gN variable for addChild), so all bindings survive. Adds chart-callback, Scatter-sensor, old-struct-quiet, and save-binding tests to TestDashboardSerializerRoundTrip. Group/.m-export round-trips (MSerializer) stay green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit 262a8ca9b1837d2a82fbc98e9abac521f95e4b7a)
… constructors (P0-4)
The house convention is that unknown name-value keys throw immediately listing
valid options, but the two biggest entry points violated it:
- FastSense's constructor called parseOpts WITHOUT the verbose flag and discarded
the unmatched output, so FastSense('Themee','dark') silently built a fully
defaulted object — the hardest class of config bug to diagnose. It now captures
the unmatched struct and throws FastSense:unknownOption (the closed ctor option
set makes strict rejection safe; addLine/addBand etc. keep their line() pass-through).
- DashboardWidget's base constructor (shared by all ~20 widgets) accepted unknown
keys, surfacing only MATLAB's generic 'no public property' error. It now throws
DashboardWidget:unknownOption listing valid options. The legacy 'Sensor'->'Tag'
remap runs first, so old scripts and serialized dashboards still construct.
Adds throw + good-option + legacy-alias tests. Verified the base-ctor guard breaks
no legitimate widget construction across all widget suites; the 14 unrelated
failures in that sweep (companion-uifigure / timer-recovery env tests) are
pre-existing and reproduce with this change reverted.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
(cherry picked from commit a85c06b23bc5bad141bcd5adb48eb5192e39b4cf)
…h for widget types (P1) The widget type->class mapping was hand-maintained in four out-of-sync places (DashboardEngine.WidgetTypeMap_, widgetTypes(), DashboardSerializer .createWidgetFromStruct, DetachedMirror.cloneWidget) — adding a widget required editing all of them or it silently failed to deserialize/detach/list. Introduces DashboardWidgetRegistry, a static-method class mirroring TagRegistry (persistent containers.Map, namespaced error ids, reset() for test isolation), seeded with the 19 built-in types and the 'kpi'->'number' alias. API: types/isRegistered/resolveAlias/constructorFor/fromStruct/register/registerAlias/reset. The next commits route the four drift sites through it. This commit only adds the registry + its tests; no consumer wired yet, so behaviour is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit 383f2e65e7bd6eb48cd24bcebbf61433c734070f)
…e registry (P1) addWidget now dispatches via DashboardWidgetRegistry.constructorFor instead of the instance-level WidgetTypeMap_ (removed), and widgetTypes() derives its list from DashboardWidgetRegistry.types() — so it can no longer drift from what addWidget accepts (it now correctly includes iconcard/chipbar/sparkline, which the stale 16-entry static list omitted). Adds the documented extension point DashboardEngine.registerWidgetType(type, ctor). The kpi deprecation warning, timeline no-store warning, GroupWidget ReflowCallback injection, multi-page routing, overlap resolution and pre-constructed-widget passthrough are all preserved unchanged. New TestDashboardEngineWidgetTypes (figure-free): widgetTypes() includes the drifted types, kpi still warns + returns NumberWidget, registerWidgetType makes a custom type work through addWidget, unknown type still errors DashboardEngine:unknownType. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit 35b6cda64fda926f4e7d12fa23d49262edb4b8e3)
…istry (P1) DashboardSerializer.createWidgetFromStruct (21-case switch) and DetachedMirror.cloneWidget (19-case switch) now both dispatch through DashboardWidgetRegistry.fromStruct — the last two drift sites are gone, so a registerWidgetType'd custom type now round-trips through save/load AND detach with no core edits. 'kpi' resolves to NumberWidget via the registry alias on the load path. Preserved exactly: the serializer's warn-and-skip for unknown types (DashboardSerializer:unknownType), the mirror's DetachedMirror:unknownType error, and the 'mock' test-widget special-case (kept serializer-only and guarded by exist() so the library keeps zero dependency on tests/). Tests: custom type deserializes via the registry (was dropped before); kpi struct loads as NumberWidget; unknown type still warns+returns []; a 'mock' widget still errors DetachedMirror:unknownType through the mirror. Existing all-types round-trip and detach suites stay green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit 0a38060378ff0038c87825cf94aa41aecd6b61d4)
…liases on load (P1) Saved dashboard configs now carry a top-level schemaVersion (=1), stamped by widgetsToConfig and widgetsPagesToConfig via a single currentSchemaVersion() constant, so both .json and .m save paths are covered. loadJSON now warns DashboardSerializer:schemaVersionNewer when it reads a config whose schemaVersion exceeds what this build supports, and treats a MISSING field as v1 — so every existing (pre-versioning) dashboard still loads silently. Combined with the registry alias from earlier in this refactor, a legacy 'kpi' widget now survives the full JSON load path (loadJSON -> configToWidgets) as a NumberWidget instead of being dropped. The .m export path is intentionally unchanged (it reconstructs via addWidget, so it needs no version field). Tests: config + JSON carry schemaVersion==1; an old config without the field loads with no warning; a schemaVersion=999 file warns; a 'kpi' JSON widget loads as NumberWidget through the full path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit 3f17ed412144f2a09b9ce5596a315cd6ad904ada)
…Name)
removeChild only ever removed from obj.Children, so a tabbed GroupWidget (whose
children live in obj.Tabs{}.widgets) could not be edited programmatically — the
headline nested-layout feature was a one-way street. removeChild now takes an
optional tabName mirroring addChild: with a tab name it removes from that tab
(GroupWidget:unknownTab if absent, GroupWidget:invalidIndex if out of range); with
none it keeps the exact prior Children behaviour, so existing removeChild(idx)
calls are byte-identical.
Adds tabbed removeChild + unknown-tab + out-of-range tests; panel-mode test unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
(cherry picked from commit 049634812a59c4febad288049c69a76fd519c370)
Completes the tabbed-group editing API begun in the previous commit: removeTab drops an entire tab and its widgets, errors GroupWidget:unknownTab for an unknown name, and moves ActiveTab to the first surviving tab (or '' when none remain) when the removed tab was active. Purely additive. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit 9c5695bf8f2213f4b50c24181902e20e4a882bb8)
…rty doc GroupWidget (the class behind the headline collapsible/tabbed nested layouts) had no class header, so `help GroupWidget` was empty. Adds a standard header documenting the three Modes, the add/remove/switch/collapse contract, the 2-level nesting cap, key properties, and a usage example. Also corrects DashboardWidget.Description: it is shown in a click-to-open info popup via the widget's (i) button, not a hover tooltip as the comment claimed. Docs only — no behaviour change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit 70324122b9a1bcd6c39ee8eec4df1ce6f43c6b62)
… (P1 symmetry) Pages could be added but never removed — the add/remove asymmetry the API review flagged. removePage drops the page at idx (DashboardEngine:invalidIndex on a bad index), deletes the page's widgets and the DashboardPage handle, keeps ActivePage valid (decrement when removing a page before it, clamp when removing the active page, reset to 0 when none remain), and re-renders when a figure is live — exactly mirroring removeWidget. Purely additive. Adds drop / before-active / active-clamp / last-page / invalid-index tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit 99dac88ec3a5c5b3fd0ab33df2fef49f154715d8)
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
…getXY (codecov patch) Adds non-rendering coverage for the P0-2/P0-3 chart-widget changes (render-based tests don't contribute coverage in headless CI): - Histogram/Heatmap DataFcn survives toStruct/fromStruct round-trip. - BarChart fromStruct warns BarChartWidget:sourceUnresolved on an unknown source type. - Scatter fromStruct warns ScatterWidget:sourceUnresolved when a sensor key is absent from TagRegistry (exercises resolveTag_'s catch path). - Histogram/Scatter asciiRender probe Tag data via getXY() (DerivedTag bound). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit 7cf87544037f3d4c1cb11c2887b8b3a985dbf01b)
Owner
Author
|
Pushed
All 21 chart-widget tests green locally on R2025b. The remaining uncovered lines are refresh/render branches that headless CI can't instrument, plus a GroupWidget patch-length-parity artifact (the methods are fully exercised by TestGroupWidget). |
…s inclusive bug The threshold-violation predicate was copy-pasted into ~8 loops across 3 widgets with inconsistent boundary semantics: MultiStatusWidget used inclusive >= / <= while StatusWidget, ChipBarWidget, IconCardWidget and GaugeWidget all used strict > / <. A sensor sitting exactly on a threshold limit appeared red in a MultiStatus tile but green in every other widget. Fixes: - New shared isThresholdViolated(threshold, value) helper (strict > / < throughout, matching the SensorThreshold engine convention). Returns false on empty inputs and for CompositeThresholds with no values. - MultiStatusWidget's 3 first-violation loops (2 static-state, 1 legacy-Sensor) routed through the helper, fixing all 3 inclusive comparisons. - ChipBarWidget's 2 and IconCardWidget's 2 first-violation loops also routed through the helper — behaviour-preserving (they were already strict), but they can no longer drift back. - StatusWidget and GaugeWidget left as-is: they rank violations by distance to colour the worst threshold, which a boolean predicate can't replace. New: MockThreshold stub (IsUpper + allValues) for pure-logic testing without the SensorThreshold class graph; TestIsThresholdViolated (4 tests: upper/lower strict, multi-value, empty guards); regression test in TestMultiStatusWidget documenting the exact on-limit fix. Regression sweep: 628 tests, 614 pass, 14 fail = all known pre-existing env/flaky. New failures: 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit 37329135aefca47cc91ac039372721dd226a2ecc)
HanSur94
added a commit
that referenced
this pull request
Jun 3, 2026
Resolve conflicts from main's Dashboard API hardening (#178) and crosshair-link toggle (#184): - FastSenseWidget.m: keep all three methods -- 1041's setTimeWindow + isShowingEmptyState alongside main's setCrosshairLink (independent additions at the same location; added the missing end for isShowingEmptyState). All three backing properties coexist. - .planning/STATE.md: status/activity lines reconciled to current state. Merge verified locally: TestFastSenseWidget 16/16, test_dashboard_time_window 8/8, TestCompanionTimeBar 12/12. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hardening pass on the Dashboard / FastSense API, driven by a multi-agent design review of the codebase. Three arcs, 12 commits, every change TDD'd (test proves the bug, then the fix) against the live MATLAB session. Backward compatibility preserved throughout — existing scripts, serialized dashboards, and the documented widget contract all keep working. (GSD
.planning/artifacts are filtered out of this branch.)1 — P0 correctness fixes
Real user-facing defects hiding in the public API:
fix: route widget-addressing methods through active-page list—getWidgetByTitle/setWidgetPosition/markAllDirty/previewreadobj.Widgetsdirectly and silently broke onceaddPage()was used; now routed through the page-aware accessors (single-page byte-identical).fix: chart widgets read Tag data via getXY()— Bar/Histogram/Heatmap/Scatter readTag.Ydirectly and crashed onDerivedTag/CompositeTag; now use the documentedgetXY()contract.fix: restore chart data bindings on load; delegate save() to linesForWidget— chartfromStructdropped the data source (charts reloaded empty) andsave()droppedValueFcn/StatusFcnon.mexport; both fixed, violating-the-backward-compat-guarantee bug closed.fix: strict unknown-option guards on FastSense + DashboardWidget ctors—FastSense('Themee',…)silently swallowed typos; now throws namespaced:unknownOption(legacySensor→Tagalias preserved).2 — Widget-type registry (P1 structural root cause)
The widget type→class map was hand-maintained in four out-of-sync places (engine map,
widgetTypes(), serializer switch, detach switch), so adding a widget silently failed to deserialize/detach/list.DashboardWidgetRegistry(static-method class mirroringTagRegistry) is the single source of truth; all four sites now dispatch through it.DashboardEngine.registerWidgetType(type, ctor)— a custom widget now works throughaddWidget, serialization, and detach with zero core edits.widgetTypes()is self-updating (fixed a stale list that omittediconcard/chipbar/sparkline).schemaVersionstamp + newer-file warning; legacykpiresolves toNumberWidgeton load.3 — Nested-layout & navigation ergonomics
Closes the add-only asymmetry the review flagged:
GroupWidget.removeChild(idx, tabName)makes tabbed groups editable (children live inTabs{}.widgets; previously unreachable). Panel/collapsible path byte-identical.GroupWidget.removeTab(tabName)andDashboardEngine.removePage(idx)(mirrorsremoveWidget) — additive.DashboardWidget.Description(click-to-open info popup, not hover tooltip).Verification
check_matlab_codeclean on all changed code.TestDashboardListPane"companion uifigure not found" in headless runs + a flakytestTimerContinuesAfterError), confirmed independent of these changes via stash-revert.🤖 Generated with Claude Code