API usability: resolve all audit blockers and friction (66 → ~88/100)#369
Open
HanSur94 wants to merge 12 commits into
Open
API usability: resolve all audit blockers and friction (66 → ~88/100)#369HanSur94 wants to merge 12 commits into
HanSur94 wants to merge 12 commits into
Conversation
Owner
Author
🤖 Automated advisory review — head
|
| File | Kind | Recommended resolution |
|---|---|---|
libs/FastSense/functionSignatures.json |
add/add | Take main's — it's a superset (also covers addBand/addMarker/addShaded/setScale) and uses the proper FastSense.FastSense constructor key form. |
libs/Dashboard/functionSignatures.json |
add/add | Take main's key form; port over any NV entries ours has that main's addWidget lacks (Tag, XData/YData, Thresholds). |
libs/Dashboard/DashboardEngine.m |
changed in both | Keep main's unknownType message (02909675 — lists types and points at widgetTypes() for descriptions; strictly better than this PR's). Keep this PR's other edits (Theme validation, addWidget/save/load help). |
docs/.api-usability/audit-2026-06-24.md |
add/add | Two different sessions wrote different baselines under the same filename. Keep main's; fold this branch's baseline content into audit-2026-06-29.md or rename. |
🟠 Findings
- Duplicated fixes / stale claims after rebase. F4 (widget-type list) and F8 for FastSense+Dashboard were fixed independently on main. After rebasing, drop those hunks and correct the audit reports + PR body so they don't claim credit for main's versions.
libs/SensorThreshold/functionSignatures.jsonis genuinely unique to this PR — keep it. - No new regression tests. The PR introduces 9 new error IDs/behaviors (
FastSense:positionalData,MonitorTag:invalidConditionArity,DerivedTag:invalidComputearity,DashboardEngine:unknownTheme,FastSense:invalidThreshold,SensorTag:sizeMismatch,set.Tagstring resolution,TagRegistry.keys(), positionaladdFillbaseline) with zero new test files. Existing suites pass (~200 runs verified during development), but nothing pins the new IDs/behaviors against regression. Suggest negative-case additions toTestMonitorTag,TestSensorTag,TestDerivedTag,TestDashboardWidget,TestTagRegistry,TestAddThreshold.
🟢 House-rule checks (pass)
- Toolbox-free: pure MATLAB throughout ✓ · No MEX sources touched (parity N/A) ✓
- Backward compat: no signature/default/serialization-key change;
fromStructload path resolves keys→objects before assignment, unaffected by the newset.Tag; repo-wide caller sweeps done for theme names, condition/compute arities, positional data ✓ - Naming: the 5 added "FastPlot" strings are audit-report prose describing the stale-name finding; code changes actually remove FastPlot debt ✓
- Hot path untouched;
set.Tagadds a trivialischarcheck per assignment ✓ - CI: no checks reported on this PR yet at review time — nothing to classify.
Next step
Rebase onto origin/main, resolving the 4 conflicts per the table (favor main for functionSignatures ×2 and the unknownType message; keep everything else), update the audit reports/PR body to credit main's parallel fixes, then add the regression tests. After that this is a strong merge candidate — run /merge-ready 369 to confirm.
A MonitorTag built with a wrong-arity condition (e.g. @() or @(x)) used to fail with a raw MATLAB:TooManyInputs deep inside getXY. The constructor now checks the handle's arity up front and throws the namespaced, actionable MonitorTag:invalidConditionArity, mirroring the existing invalidCondition guard. Backward-compatible / additive only: no signature, default, or serialization change. Only rejects already-broken 0/1-arg conditions (every condition handle in the repo is 2-arg); valid @(x,y) and varargin handles are unaffected. Proven: 108 Tag tests pass (MonitorTag 28, Events 14, Composite 31, Derived 35). Also documents the new error ID in the class + constructor help, and marks finding B3 resolved in docs/.api-usability/audit-2026-06-29.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Calling the constructor like plot(x, y) — a near-universal MATLAB reflex — used to fail with a cryptic MATLAB:mustBeFieldName from the option parser. The constructor now detects a non-string first argument up front and throws the namespaced FastSense:positionalData, pointing at the existing one-liner FastSense.plot(x, y) and the add-then-render builder. Backward-compatible / additive only: the guard runs before option parsing and only fires on a leading non-char/non-string arg — every valid call starts with a string option name, and FastSense.plot forwards options (never data) to the constructor. No signature, default, or serialization change. Proven: 18 FastSense/integration tests pass (TestAddLine 8, TestFastSenseAddTag 9, TestGoldenIntegration 1); valid name-value calls, unknownOption, and FastSense.plot(x,y) all verified unaffected. Marks finding B1 resolved in docs/.api-usability/audit-2026-06-29.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
addWidget(..., 'Tag', 'press_a') — passing a registry key string instead of
the Tag object, a natural shortcut since tags are registered by key — used to
fail with a raw MATLAB:structRefFromNonStruct deep in the widget title cascade.
The DashboardWidget base class now has a set.Tag that resolves a char/string
via TagRegistry.get, so the string form works as a shortcut for
'Tag', TagRegistry.get('press_a'). A missing key raises the namespaced
TagRegistry:unknownKey ("...Use TagRegistry.list()...") instead of a raw crash.
Backward-compatible / additive only: Tag objects and [] pass through unchanged,
the 'Sensor' alias inherits the same convenience, and the fromStruct load path
(which already resolves keys to objects before assigning) is unaffected — no
signature, default, or serialization-key change.
Proven: 35 widget/serializer/engine tests pass (TestFastSenseWidgetTag 7,
TestDashboardMSerializer 10, TestDashboardEngine 18); static analysis clean;
string/object/Sensor-alias/missing-key paths all verified live.
Marks finding B2 resolved in docs/.api-usability/audit-2026-06-29.md; all three
red blockers (B1, B2, B3) are now closed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
TagRegistry documented keys as part of its public surface, but the method did not exist — TagRegistry.keys() failed with a raw MATLAB:subscripting:classHasNoPropertyOrMethod, and list() only prints (its output cannot be captured). Callers reaching for the containers.Map.keys reflex had no programmatic way to enumerate the catalog. Adds a static keys() returning a sorted cellstr of all registered keys (1x0 cell when empty) — the read counterpart to list(); find/findByKind/ findByLabel still return Tag objects. Purely additive (new method); no existing signature or behavior changes. Proven: 26 TagRegistry tests pass; empty/populated/sorted/iteration paths verified live; static analysis clean at the new method. Marks finding F1 resolved in docs/.api-usability/audit-2026-06-29.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…y points
Six additive usability fixes, each turning a raw error or silent-accept into a
namespaced, actionable result. All only affect already-broken or already-
erroring input; every existing repo caller was verified unaffected.
- S2 DerivedTag: validate function-handle ComputeFn arity (@(parents)) up
front -> DerivedTag:invalidCompute, instead of a raw MATLAB:minrhs in
recompute_.
- V1 DashboardEngine: validate a string Theme preset at construction ->
DashboardEngine:unknownTheme (parity with FastSenseTheme:unknownPreset),
instead of silently falling back to 'light'. Legacy aliases
(default/industrial/scientific/ocean) still accepted.
- V2 FastSense.addFill: accept a positional baseline addFill(x,y,baseline)
as shorthand for addFill(x,y,'Baseline',baseline), instead of a raw
MATLAB:badsubscript from the name-value parser.
- F4 DashboardEngine.addWidget: list all valid widget types in the
DashboardEngine:unknownType message (parity with FastSense:unknownOption).
- F6a SensorTag: reject mismatched inline X/Y -> SensorTag:sizeMismatch.
- F6b FastSense.addThreshold: reject a non-numeric value ->
FastSense:invalidThreshold.
Backward-compatible: no signature, default, or serialization change.
Proven: 85 tests pass (TestDerivedTag 35, TestSensorTag 20, TestAddThreshold 7,
TestDashboardEngineWidgetTypes 5, TestDashboardEngine 18 in isolation); the lone
testTimerContinuesAfterError flake is the known environmental timer test (passes
clean in isolation). Static analysis clean at all edits.
Marks S2, V1, V2, F4, F6a, F6b resolved in docs/.api-usability/audit-2026-06-29.md.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Help-text only (no behavior change): - F2/F7 addWidget: full doc comment — the type form, the widget-object form, the valid types, and the FastSenseWidget data-binding modes (Tag / XData,YData / File,XVar,YVar / Thresholds). Was the misleading one-liner "Accept a pre-constructed widget object directly". - F3 DashboardEngine.save / load: real doc comments (format from extension, tags serialized by key, register-before-load). They previously had no help, so `help DashboardEngine/save` showed MATLAB's built-in save() docs. - S1 SensorTag.fromStruct: note that toStruct omits X/Y, so a round-tripped tag has no data until re-attached. - P2 DerivedTag: stale "FastPlot" -> "FastSense" (post-rename). - P3 CompositeTag: drop "implemented in Plan 02; Plan 01 ships core API only" framing — the merge-sort aggregation is implemented; describe current behavior. Verified: help renders correctly for all; built-in save/load leak gone; no "FastPlot"/"Plan 02" left in those headers. Marks F2, F3, S1, P2, P3 resolved in docs/.api-usability/audit-2026-06-29.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The DashboardWidget:unknownOption error correctly lists valid options, but the list mixed in internal graphics-handle properties (hPanel, hCellPanel, hValueText, hUnitText, ...) that are never set via name-value pairs, burying the real options. Filter out properties matching the h[A-Z] handle naming convention from the suggestion. Additive: error id and behavior unchanged; only the suggested-options string is cleaner. Real options (StaticValue, Units, Title, Tag, ...) all retained. Proven: 13 TestDashboardWidget tests pass; verified handles gone and real options present in the message; no test depends on the old message content. Marks P5 resolved in docs/.api-usability/audit-2026-06-29.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The public classes had no functionSignatures.json, so MATLAB offered no argument/name-value tab-completion — a discoverability gap. Add validated signature files for the three in-scope libraries: - FastSense: constructor options + addLine + addThreshold - Dashboard: DashboardEngine constructor + addWidget (type choices, data binding) - SensorThreshold: SensorTag / StateTag / MonitorTag / CompositeTag / DerivedTag constructors + TagRegistry.get/register Purely additive (new files; no code change). All three pass validateFunctionSignaturesJSON with no errors. Marks F8 resolved in docs/.api-usability/audit-2026-06-29.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lp (P1)
F5 — the FastSenseWidget 'Thresholds' option already accepts a numeric
scalar/vector (upper limit lines) and a cell of {Value|X-Y, Direction, Label,
Color, LineStyle} structs (applyThresholds_ handles them); only 'auto'|false
was documented, so the capability was undiscoverable. Document all accepted
forms on the Thresholds property and in addWidget help. Verified live: numeric
[52 30] draws 2 lines, struct form draws a labeled line. (The original
"silent no-op" audit finding was a false positive.)
P1 — strip internal process codes (Phase 1006/1007/1010, MONITOR-NN,
Pitfall NN, Plan 02, Pass-2, EVENT-01) from the MonitorTag public help and a
dev-only "do not write TagKeys" note, rephrasing to plain behavioral prose.
All behavioral facts (lazy/in-memory, Persist opt-in, event-level callbacks,
ZOH alignment, callback timing, lifecycle) are preserved.
Help-text only (no behavior change). Proven: 28 TestMonitorTag tests pass;
MonitorTag parses clean; header verified jargon-free.
Marks F5 and P1 (MonitorTag) resolved in docs/.api-usability/audit-2026-06-29.md.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nale Final live re-audit confirms every baseline raw-error/silent-accept probe now returns a namespaced error or a working path. Update the 2026-06-29 report: headline 66 -> ~88, record the full resolved set, and document P4 (example install bootstrap) as deferred-by-design (it is the path bootstrap; not replaceable by a helper without a .prj/toolbox packaging change). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The canonical two-sensor + threshold + 2-widget-dashboard task that the 66 baseline could not complete via documented API now runs end-to-end (string Tag keys, numeric widget thresholds, keys(), actionable ctor error). Logged as confirming evidence for the ~88 re-score. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds tests/suite/TestApiUsabilityErrors.m (19 tests) pinning the new namespaced error IDs and convenience behaviors from the api-usability batch so they cannot regress to raw MATLAB errors: FastSense:positionalData, MonitorTag:invalidConditionArity (+ varargin allowed), DerivedTag:invalidCompute arity (+ @(parents) allowed), DashboardEngine:unknownTheme (+ legacy aliases allowed), FastSense:invalidThreshold, SensorTag:sizeMismatch, widget 'Tag' string-key resolution (+ object passthrough + unknownKey), TagRegistry.keys() (empty-safe, sorted), and addFill positional baseline (+ name-value form). All 19 pass. Also adds a post-rebase erratum to docs/.api-usability/audit-2026-06-29.md: F4 and F8 (FastSense/Dashboard signatures) were independently fixed on main first (PR #212 + follow-ups); this branch's unique F8 contribution is the SensorThreshold signatures file plus the XData/YData/Thresholds addWidget entries. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
HanSur94
force-pushed
the
api-usability-polish
branch
from
afe8fa2 to
1162eac
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
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.
Summary
An empirical usability audit of the public FastSense API (
/api-audit, live-measured in MATLAB R2025b) scored the baseline at 66/100: excellent core plotting ergonomics, but three cryptic-error traps on a new user's most natural first guesses, uneven input validation, and several discoverability gaps. This PR resolves every actionable finding — all additive, fully backward-compatible — and re-scores at ~88/100. Full per-finding history lives indocs/.api-usability/audit-2026-06-29.md.🔴 Blockers — cryptic errors on natural first guesses
FastSense(x, y)(theplot(x,y)reflex) → namespacedFastSense:positionalDatapointing atFastSense.plot(x, y)and the add-then-render builder (was rawMATLAB:mustBeFieldName)addWidget(..., 'Tag', 'press_a')— a string key now works as a shortcut forTagRegistry.get(key); a missing key raisesTagRegistry:unknownKey(was rawstructRefFromNonStruct)MonitorTagcondition with wrong arity →MonitorTag:invalidConditionArityat construction (was rawTooManyInputsdeep ingetXY)🟠 Friction
TagRegistry.keys()— sorted cellstr, empty-safe (documented but previously missing)DerivedTagcompute arity,DashboardEngineTheme typos, positionaladdFill(x,y,baseline)now accepted,SensorTag:sizeMismatch,FastSense:invalidThresholdhelpforDashboardEngine.addWidget/save/load(previously near-empty or leaking MATLAB's built-insave/loaddocs); documented allThresholdsforms and data-binding modesfunctionSignatures.jsonfor SensorThreshold (unique to this PR: all five Tag constructors +TagRegistry.get/register), plusXData/YData/Thresholdsentries added to main'saddWidgetsignature🟢 Polish
SensorTag.fromStructnow documents that serialization omits X/Y dataMonitorTaghelp (Phase/Pitfall/MONITOR-NN codes → plain behavioral prose); fixed stale "FastPlot" name and "Plan 02" framingunknownOptionsuggestions no longer list internalh*graphics handlesDeferred (by design)
.prj/toolbox packaging. Recommendation only, documented in the audit report.Backward compatibility
No public signature, default, or serialization key changed. New validation only rejects input that already errored (raw) or silently misbehaved; every existing caller in the repo was verified unaffected (grep sweeps for theme names, condition/compute arities, positional data, Tag string bindings). The
fromStructload path resolves keys to objects before assignment and is untouched by the newset.Tag.Test plan
tests/suite/TestApiUsabilityErrors.m— 19 regression tests pinning every new error ID and convenience behavior (positive + negative cases). All pass.addThresholdnumeric check + main's unknown-option warning; Tag ctor arity checks + main's valid-key lists) — probed live, all correct.functionSignatures.jsonfiles passvalidateFunctionSignaturesJSON; static analysis clean at every edit; end-to-end fresh-user task re-simulated successfully.🤖 Generated with Claude Code