|
| 1 | +# API Usability Audit — 2026-06-24 |
| 2 | + |
| 3 | +Scope: public surface only — `FastSense` (`addLine`/`addThreshold`/`addTag`/…/`render`/`updateData`), |
| 4 | +`DashboardEngine` (`addWidget`/`Theme`/`render`/`save`/`load`), Tag constructors |
| 5 | +(`SensorTag`/`StateTag`/`MonitorTag`/`CompositeTag`/`DerivedTag`), `TagRegistry`, |
| 6 | +`toStruct`/`fromStruct`. Internals out of scope. |
| 7 | + |
| 8 | +Runtime: headless `matlab -batch` (R2025b) — the live matlab MCP was not connected this session. |
| 9 | +Friction was reproduced by running real code through the public API, not by reading internals. |
| 10 | + |
| 11 | +## Score |
| 12 | + |
| 13 | +| Metric | Value | |
| 14 | +|---|---| |
| 15 | +| Entry points with helpful "valid options" validation | DashboardEngine ctor ✅, attachPlantLog ✅, `addWidget` type ✅; Tag ctors ✅ (all six fixed) | |
| 16 | +| Confusing / dead-end errors (no recovery path) | **0** open (was 1) | |
| 17 | +| Methods that silently discard unknown options | **0** (was 4: addThreshold/Band/Marker/Shaded — now warn) | |
| 18 | +| Discoverability: `functionSignatures.json` for tab-completion | **FastSense** (ctor + add* + setScale) + **Dashboard** (ctor + addWidget type enum); Tag pending | |
| 19 | +| Lines-to-first-plot (example_basic) | ~4 (`FastSense` → `addLine` → `addThreshold` → `render`) — good | |
| 20 | + |
| 21 | +## Findings (severity-ordered) |
| 22 | + |
| 23 | +### 🔴 F-1 — `addWidget` unknown type was a dead-end — ✅ RESOLVED (this run) |
| 24 | +- **Where:** `DashboardEngine.addWidget` → `error('DashboardEngine:unknownType', ...)`. |
| 25 | +- **What a user hit:** any wrong guess (`'linechart'`, `'line'`, `'plot'`, `'numbers'`, `'bar'`) |
| 26 | + returned `Unknown widget type: <guess>` with **no list of valid types and no pointer** to how to |
| 27 | + find them — on the #1 dashboard-building entry point. It was also inconsistent with the *same |
| 28 | + method's* unknown-name-value error (line 164: `... Valid options: %s`). |
| 29 | +- **Fix (additive, message-only — ID `DashboardEngine:unknownType` unchanged):** |
| 30 | + now `Unknown widget type 'linechart'. Valid types: barchart, chipbar, divider, fastsense, gauge, |
| 31 | + group, heatmap, histogram, iconcard, image, multistatus, number, rawaxes, scatter, sparkline, |
| 32 | + status, table, text, timeline. Call DashboardEngine.widgetTypes() for descriptions.` |
| 33 | + List is derived from `DashboardWidgetRegistry.types()` (single source of truth — custom-registered |
| 34 | + types appear automatically). |
| 35 | +- **Proof:** 18/18 passed across `TestDashboardEngineWidgetTypes` + `TestDashboardWidgetRegistry` |
| 36 | + (incl. `testUnknownTypeStillErrors` which asserts the ID); valid `number` add and the `kpi` |
| 37 | + deprecation alias both still work. |
| 38 | + |
| 39 | +### 🟠 F-2 — Tag constructors' `unknownOption` didn't list valid keys — ✅ RESOLVED (2nd run) |
| 40 | +- **Where:** `SensorTag`, `StateTag`, `MonitorTag`, `CompositeTag`, `DerivedTag`, and `Tag` base all |
| 41 | + threw `Unknown option '<key>'.` (DerivedTag: `Unknown NV key '<key>'.`) with **no list of valid |
| 42 | + name-value keys** — contradicting the project's own convention (CLAUDE.md: *"unknown keys throw |
| 43 | + immediately with helpful message listing valid options"*) and inconsistent with `DashboardEngine`'s |
| 44 | + `Valid options: %s`. A user who mistyped `'Unit'` for `'Units'` got no hint of the correct spelling. |
| 45 | +- **Fix (additive, message-only — all six `*:unknownOption` IDs unchanged):** each unknown-option |
| 46 | + error now appends `Valid options: %s`, built via `strjoin` from the key arrays already local to each |
| 47 | + `splitArgs_` (so the list cannot drift from what the constructor accepts). Examples: |
| 48 | + - `SensorTag('s', 'Unit', 'bar')` → `Unknown option 'Unit'. Valid options: Name, Units, Description, |
| 49 | + Labels, Metadata, Criticality, SourceRef, ID, Source, MatFile, KeyName, RawSource, X, Y.` |
| 50 | + - `MonitorTag(...,'NotARealKey',5)` → lists `... AlarmOffConditionFn, MinDuration, EventStore, |
| 51 | + OnEventStart, OnEventEnd, Persist, DataStore, EventLog.` |
| 52 | + DerivedTag's wording was normalized from `Unknown NV key` to `Unknown option` for family consistency. |
| 53 | +- **Proof:** 156/156 passed across `TestTag`, `TestSensorTag`, `TestStateTag`, `TestMonitorTag`, |
| 54 | + `TestCompositeTag`, `TestDerivedTag` (incl. every `unknownOption` ID assertion); before/after |
| 55 | + reproduced for all five concrete subclasses. The unreachable defensive `otherwise` branches at |
| 56 | + `MonitorTag:199` / `DerivedTag:218` were intentionally left untouched (they never see a user key — |
| 57 | + `monArgs`/`ownArgs` only hold already-recognized keys). |
| 58 | + |
| 59 | +### 🟠 F-5 — `FastSense` add* methods silently discarded unknown options — ✅ RESOLVED (3rd run) |
| 60 | +- **Where:** `FastSense.addThreshold`, `addBand`, `addMarker`, `addShaded` — each has a *closed* |
| 61 | + option set but called `parseOpts(defaults, args, obj.Verbose)` and **discarded** the unmatched |
| 62 | + output (`[parsed, ~]`). A misspelled option (`addThreshold(5, 'Colour', 'r')`, `addMarker(.., |
| 63 | + 'Symbol','x')`) was silently dropped: no error, no warning (unless `Verbose`), and the intended |
| 64 | + styling silently did nothing. (Found during this run — not in the original F-1..F-4 list.) |
| 65 | +- **Why it matters:** silent failure is worse than a loud one, and it directly violated the project's |
| 66 | + own *house convention* — the `FastSense` constructor's comment: *"closed option set, so reject |
| 67 | + unknown/misspelled keys immediately rather than silently discarding them."* `addLine` and the |
| 68 | + constructor already reject unknown keys; these four were the inconsistent holdouts. |
| 69 | +- **Fix (additive, non-breaking):** new private helper `warnUnknownOpts_` warns (does **not** error) |
| 70 | + for each unrecognized option, naming the method and listing valid options. Wired into the four |
| 71 | + sites; scripts still run identically (the option was already being ignored — now the mistake is |
| 72 | + visible). Error would have been breaking (silent→throw); a **warning** keeps existing scripts |
| 73 | + working while surfacing the bug. `addFill` was left alone — it legitimately forwards its unmatched |
| 74 | + options to `addShaded`. |
| 75 | +- **Proof:** all four typos now warn (e.g. `addMarker: unknown option 'Symbol' ignored. Valid |
| 76 | + options: Marker, MarkerSize, Color, Label`); **0 false positives** on valid + case-insensitive |
| 77 | + calls; 48/48 pass across `TestAddThreshold`, `TestAddBand`, `TestAddMarker`, `TestAddShaded`, |
| 78 | + `TestThresholdLabels`, `TestRender`, plus 5/5 flat add-* tests. |
| 79 | +- **Future (breaking, report-only):** a major version could promote the warning to a hard error to |
| 80 | + fully match the constructor's reject-immediately convention. |
| 81 | + |
| 82 | +### 🟢 F-3 — No `functionSignatures.json` (tab-completion discoverability) — ✅ RESOLVED (4th run, FastSense scope) |
| 83 | +- **Where:** library roots had none — MATLAB editor tab-completion couldn't suggest any public-API |
| 84 | + name-value option. |
| 85 | +- **Fix (additive, new file — zero runtime change, ignored by Octave):** added |
| 86 | + `libs/FastSense/functionSignatures.json` covering the core plotting surface — the `FastSense` |
| 87 | + constructor plus `addLine`, `addThreshold`, `addBand`, `addMarker`, `addShaded`, `setScale` — with |
| 88 | + enumerated `choices` for the discrete options (`Direction` upper/lower, `XScale`/`YScale` |
| 89 | + linear/log, `DownsampleMethod` minmax/lttb, `Marker`, `LineStyle`). The editor now offers these as |
| 90 | + you type. |
| 91 | +- **Proof:** `validateFunctionSignaturesJSON` returns **0 messages** (schema-valid, all method/type |
| 92 | + names resolve against the real class metadata with FastSense on the path); runtime smoke |
| 93 | + (construct + add* + setScale) clean — no error, no warning; no `.m` changed so existing |
| 94 | + tests/examples are inherently unaffected. |
| 95 | +- **Remaining (additive follow-ups):** `libs/Dashboard/functionSignatures.json` for |
| 96 | + `DashboardEngine.addWidget` (the `type` arg as a `choices` enum — the single highest-value |
| 97 | + completion, directly preventing the run-1 wrong-type trap at typing time) and |
| 98 | + `libs/SensorThreshold/functionSignatures.json` for the Tag constructors. |
| 99 | + |
| 100 | +### 🟢 F-4 — Example bootstrap boilerplate *(minor / informational)* |
| 101 | +- Every example repeats `projectRoot = fileparts(fileparts(fileparts(mfilename('fullpath'))));` |
| 102 | + + `run(fullfile(projectRoot,'install.m'))`. This is example-script bootstrap, not public-API |
| 103 | + friction, so it is informational only — no API change warranted. |
| 104 | + |
| 105 | +### 🟢 F-3b — `DashboardEngine.addWidget` had no type tab-completion — ✅ RESOLVED (5th run) |
| 106 | +- **Fix (additive, new file — zero runtime change):** added `libs/Dashboard/functionSignatures.json` |
| 107 | + covering the `DashboardEngine` constructor (Name + Theme/LiveInterval/InfoFile/ProgressMode/ |
| 108 | + ShowTimePanel) and `addWidget`, with the `type` argument modeled as a `choices` enum of all 19 |
| 109 | + built-in widget types — so the editor now lists the valid types *as you type*, complementing the |
| 110 | + run-1 error-message fix at the point of entry. Common `addWidget` options (Position, Title, Tag, |
| 111 | + Label) are offered too. |
| 112 | +- **Proof:** `validateFunctionSignaturesJSON` returns **0 messages** (after switching the optional |
| 113 | + positional `Name` from the deprecated `"optional"` kind to `"ordered"`, which R2025b now requires); |
| 114 | + runtime smoke confirms `addWidget('number', ...)` still returns a `NumberWidget` and an unknown |
| 115 | + type still errors `DashboardEngine:unknownType` — the metadata file is runtime-inert. |
| 116 | + |
| 117 | +## Next-worst additive finding for `/api-polish` |
| 118 | +**F-3c** — add `libs/SensorThreshold/functionSignatures.json` for the Tag constructors |
| 119 | +(`SensorTag`, `StateTag`, `MonitorTag`, `CompositeTag`, `DerivedTag`), completing the |
| 120 | +tab-completion coverage for the third core public surface. The valid name-value keys are already |
| 121 | +enumerated per class (from the run-2 work) — `Name`/`Units`/`Description`/`Labels`/`Metadata`/ |
| 122 | +`Criticality`/`SourceRef` universals plus each class's extras (e.g. SensorTag `ID`/`Source`/`X`/`Y`, |
| 123 | +MonitorTag `MinDuration`/`Persist`/…). Purely additive (new file). |
0 commit comments