Skip to content

FastSenseCompanion event viewer + dashboard widget polish#123

Open
HanSur94 wants to merge 27 commits intomainfrom
claude/recursing-bhaskara-eea418
Open

FastSenseCompanion event viewer + dashboard widget polish#123
HanSur94 wants to merge 27 commits intomainfrom
claude/recursing-bhaskara-eea418

Conversation

@HanSur94
Copy link
Copy Markdown
Owner

@HanSur94 HanSur94 commented May 8, 2026

Summary

Adds a tag-aware, time-filterable, Gantt-style event viewer that pops out of FastSenseCompanion, plus a handful of dashboard-widget rendering fixes uncovered while live-testing the result against the industrial-plant demo.

Event viewer (Tasks 1–14, commits 78628eb9151173)

Pop-out classic-figure window reachable from a new toolbar button on the companion. Filter bar (presets, From/To, severity I/W/A toggles, open-only, tag search, Refresh + Auto + interval), Gantt axes (one row per tag, severity-colored bars, dashed open-event right edges), and a TimeRangeSelector slider preview at the bottom. Coupled to the companion's Live mode — auto-refresh follows companion state; presets roll forward when live, snapshot when paused. Single-click a bar for editable details, double-click for SensorDetailPlot zoomed to the event window.

  • companionDiscoverEventStore — auto-discovers the EventStore from any registered MonitorTag, so the demo gets the viewer with zero config changes.
  • New 'EventStore' constructor option + getEventStore() accessor on FastSenseCompanion.
  • New LiveModeChanged event on the companion (used by the viewer for live-mode coupling).
  • EventGanttCanvas private helper handles bar drawing, click hit-testing, and severity coloring.
  • Single-instance viewer wiring: clicking the toolbar button when the viewer is open brings it to the front; closing the companion closes the viewer first via an ObjectBeingDestroyed listener.

Polish round (commits 1ee5ae8, 5b0e6fb)

After the first live test:

  • Tooltips on every filter-bar control.
  • Datetime tick labels on the Gantt X-axis (was raw datenums like 7.401e5).
  • Slider event-marker dots, severity-colored, fed via setEventMarkers.
  • Slider height trimmed 20% → 10%; Gantt area grew 65% → 75%.
  • Y-axis margin widened so long tag keys (feedline.pressure.high) render fully.
  • Recursion guard in updateSliderPreview_ (programmatic setSelection was triggering OnRangeChangedrefresh → infinite loop).

Pre-existing bug fix surfaced by the viewer (commit 5b0e6fb)

MonitorTag.fireEventsOnRisingEdges_ re-emitted every historical event on every recompute_ call. In live mode the parent's invalidate cascade triggered a fresh recompute_ on every tick, accumulating ~30× duplicates per closed event in the demo's EventStore. Fixed by deduping against existing entries via EventStore.getEventsForTag, with open→closed transitions handled by closeEvent rather than re-append.

Dashboard widget polish (commits 5ccd39a, 4e8384a)

Two unrelated visual issues uncovered during live testing of the demo dashboard:

  • Circle aspect ratio (ChipBarWidget, MultiStatusWidget): pre-existing pxH/pxW-based radius scaling went stale on panel resize, producing stretched ovals. Replaced with axes DataAspectRatio=[1 1 1] + equal radii in data units. MATLAB letterboxes the axes if needed; circles stay round at any window aspect.
  • Header-bar clipping (NumberWidget, StatusWidget, IconCardWidget): widgets rendered into the full panel area, but the WidgetButtonBar overlays the top 28 px. On shorter panels, large value digits got sliced. Added a panelMetrics_ helper to DashboardWidget returning the content area below the bar; affected widgets now base both font sizes and Position rectangles on it.

Test plan

  • TestCompanionEventViewer — 28/28 pass (new suite covering construction, filter pipeline, presets, refresh, live coupling, slider, click handlers).
  • TestEventGanttCanvas — 10/10 pass (new suite: row mapping, severity colors, draw, dashed open-edges).
  • TestFastSenseCompanion — 62 pass + 2 pre-existing fails (unrelated; same on base). New tests: EventStore option, auto-discovery, LiveModeChanged event, toolbar button enable/disable, single-instance viewer, ObjectBeingDestroyed cleanup.
  • TestIndustrialPlantDemoCompanion — 6 pass + 1 pre-existing fail (unrelated). New tests: EventStore exposed, button enabled, viewer opens.
  • MonitorTag/EventStore suites — 67/67 pass across all 6 suites; new regression test testNoDuplicateEventsOnRecomputeAfterInvalidate covers the dedup fix.
  • Affected widget suites — 55/55 pass across NumberWidget / StatusWidget / IconCardWidget(+Tag) / ChipBarWidget / MultiStatusWidget(+Tag).
  • Live demo verificationrun_demo + openEventViewer. Confirmed: 4 unique events / 4 unique tuples / max 1 duplicate per tuple at +45 s (vs. ~30× duplicates pre-fix). Circles render round; widget content visible below the header bar.

🤖 Generated with Claude Code

HanSur94 and others added 23 commits May 8, 2026 18:17
Walk TagRegistry for the first MonitorTag carrying a non-empty
EventStore. Private helper companionDiscoverEventStore lives in
libs/FastSenseCompanion/private/; runDiscoverEventStoreTests bridges
the private-directory access gap for the class-based test suite.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n + flat test

Rewrote runDiscoverEventStoreTests to use the void/assert-based pattern
matching all sibling runners (runFilterTagsTests, etc.), fixed H1 typos
(RUNDISCOVEREVENTRESTORETESTS -> RUNDISCOVEREVENTSTORETESTS, SKIPSTAGSWITHOUSTORE_
-> SKIPSTAGSWITHOUTSTORE_), initialized storePath before the try block in
test 2, added the flat Octave-compatible test file
tests/test_companion_discover_event_store.m, and replaced the three
struct-return delegate methods in TestFastSenseCompanion with a single
testDiscoverEventStoreSuite that wraps the runner in verifyWarningFree.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…llback

Adds 'EventStore' name-value constructor option to FastSenseCompanion.
When supplied, the explicit handle is stored verbatim; when absent or [],
the private companionDiscoverEventStore() helper is called to walk the
TagRegistry and return the first MonitorTag's non-empty EventStore.
Adds getEventStore() public accessor and 5 new passing tests.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Code-review polish on top of fb77912 — surface the new EventStore
option and the long-standing LivePeriod option in the classdef
help block + inline constructor comment.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Declare LiveModeChanged in the events block, notify after IsLive is
mutated in startLiveMode and stopLiveMode, and add the helper class
LiveModeCapture plus the testLiveModeChangedFiresOnStartAndStop test.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…event end)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ed edges

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Creates the CompanionEventViewer classic-figure shell with three uipanels
(filter 15%, axes 65%, slider 20%), EventGanttCanvas-hosted axes, idempotent
close/bringToFront, and constructor validation for store/registry/companion.
Adds 7 unit tests in TestCompanionEventViewer.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…t/custom

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…raws

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Wire CompanionEventViewer to FastSenseCompanion's LiveModeChanged event:
starts/stops an auto-refresh timer on live-mode transitions, advances
the TimeRange window each tick when in 'roll' mode, and demotes
'roll' → 'snapshot' when live mode is turned off. AutoEnabled_ flag
gates timer creation for future Task 11 checkbox control.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Implements Task 11: populates the FilterPanel_ with preset buttons (1h/24h/7d/All),
From/To datetime edits, severity toggles, open-only checkbox, tag search, and
refresh/auto controls; instantiates TimeRangeSelector in SliderPanel_; adds
onSliderRangeChanged_ and other private callbacks; adds getSliderForTest_ and
onSliderRangeChanged_internalForTest test accessors; applyPreset_ now calls
refresh() at the end; applyPreset_ handles both 'all' and 'All' case variants.
26/26 viewer tests pass.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…lot drill-down)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add a fixed-width col-1 [Events ↗] button to the companion toolbar that
opens a CompanionEventViewer (idempotent: second click brings it to front).
Wire ObjectBeingDestroyed listener to clear the handle on viewer close.
Companion close() tears down the viewer first, before the live timer and
listeners, preventing stale callbacks into a half-deleted companion.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… preview, layout)

Five fixes after first round of live testing:

* Tooltips on every filter-bar control (presets, From/To, severity I/W/A,
  open-only, search, Refresh, Auto, interval).
* Datetime tick labels on the Gantt X-axis via datetick(ax, 'x',
  'keeplimits'). Replaces the raw datenum scaffold (7.401e5...) with
  HH:MM:SS-style labels MATLAB picks based on the visible range.
* Event-marker dots on the TimeRangeSelector slider, severity-colored.
  Feeds via Selector_.setEventMarkers(times, colors) from a new private
  updateSliderPreview_(allEvents) called on every refresh. The slider
  now shows the full unfiltered event distribution while the Gantt
  above shows the filtered slice.
* Slider panel reduced 20% -> 10% of figure height; Gantt area grew
  65% -> 75%. Filter bar unchanged.
* Axes left margin widened 0.10 -> 0.18 inside its panel so long tag
  keys (e.g. feedline.pressure.high) render fully instead of clipping
  to "edline.pressure".

Programmatic Selector_.setSelection calls suppress OnRangeChanged via
a save/clear/restore guard to break a recursion loop discovered in
testing: refresh -> setSelection -> OnRangeChanged ->
onSliderRangeChanged_ -> refresh.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…es against EventStore

Pre-existing bug surfaced by the new event viewer. In live mode the
industrial-plant demo accumulated ~30x duplicates of every closed event
in the EventStore over a single minute.

Root cause: SensorTag.updateData fires invalidate on its MonitorTag
listener, which wipes cache_ and sets dirty_=true. The next getXY()
falls into recompute_, which calls fireEventsOnRisingEdges_ over the
parent's full history and unconditionally appends every closed run to
the EventStore. Every refresh tick = one full re-emission of every
historical event. Existing tests caught the cache-hit case (second
getXY after a fresh recompute) but not the dirty/recompute case.

Fix at the emit call site: query EventStore.getEventsForTag(obj.Key) at
the start of fireEventsOnRisingEdges_, build a StartTime-keyed dedup
index, and skip emission for any candidate whose StartTime matches an
existing entry. For the open->closed transition (run previously stored
open, now closes), close the existing event in place via
EventStore.closeEvent rather than appending a duplicate. Also re-seed
cache_.openEventId_ from the existing open event so the streaming
appendData hot path can still close it later.

Regression test:
testNoDuplicateEventsOnRecomputeAfterInvalidate. Pre-fix: 1 -> 2 events
after one invalidate, 1 -> 7 after six cycles. Post-fix: 1 -> 1 -> 1.

Verified live: industrial-plant demo at +45s shows 4 unique events / 4
unique tuples / max 1 duplicate per tuple. All 67 tests across the 6
MonitorTag/EventStore suites pass; zero regressions.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the pxH/pxW-based rx scaling (computed once per render, stale
after panel resize) with axes DataAspectRatio=[1 1 1] plus equal x/y
radii in data units. Circles now render as circles regardless of how
the panel resizes; MATLAB letterboxes the axes when needed and the
indicators stay centered at their assigned slots.

Affects ChipBarWidget (the 4-chip Subsystem Health row in the
industrial-plant demo) and MultiStatusWidget (the 4-circle All Monitors
panel) — both showed clearly stretched ovals in the live demo.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… top

Widgets rendered text/icons into the full panel area (normalized
[0, 1]) and computed adaptive font sizes from full panel height. The
WidgetButtonBar (added by DashboardLayout, hosting the i + detach
buttons) is a 28-px-tall opaque strip overlaying the top of the panel,
so on shorter panels widget content was clipped under the bar — most
visible on NumberWidget where large value digits were sliced in half.

Adds DashboardWidget.panelMetrics_(parentPanel) returning pxW, pxH,
and a content area (pxHContent = pxH - 32) plus its normalized top
edge. NumberWidget, StatusWidget, and IconCardWidget now base both
font sizes and uicontrol Position rectangles on this content area.
ChipBarWidget and MultiStatusWidget already render in axes far enough
below the bar that they don't clip; left untouched here to keep the
diff small. Existing relayout_ SizeChangedFcn hooks pick up resize
without changes.

Verified live in the industrial-plant demo: temperature reads as
"163.7 °C" with full vertical extent visible; the threshold-formula
StatusWidget's "active when pressure>18bar" no longer collides with
the bar; reactor-critical IconCardWidget's value sits below its
header.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…le line_length

The 195-char inline comment introduced in d3478d2 (260508-llw) exceeds
the 160-char project limit and broke MISS_HIT mh_style on every PR
opened against main. Lifts the explanation to a 3-line block comment
above the property declaration; same content, identical semantics, no
runtime impact.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented May 8, 2026

Codecov Report

❌ Patch coverage is 85.71429% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
libs/SensorThreshold/MonitorTag.m 84.37% 5 Missing ⚠️

📢 Thoughts on this report? Let us know!

…e-portable

Octave doesn't auto-define eq (==) on user-defined handle classes the
way MATLAB does, so `found == es` errored with "eq method not defined
for EventStore class" on the Octave CI job. Replace with a mutation-
based identity check: set a sentinel on `es` and confirm `found` sees
the same change. Proves both references point at the same object
without relying on overloaded operators.

Re-verified: runDiscoverEventStoreTests passes 3/3 locally on MATLAB.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'FastSense Performance'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.10.

Benchmark suite Current: e2fe8b7 Previous: dc895b0 Ratio
Render mean std(1M) 2.819 ms 1.489 ms 1.89
Zoom cycle mean (1M) 17.735 ms 15.37 ms 1.15
Instantiation mean std(5M) 0.85 ms 0.401 ms 2.12
Render mean std(5M) 2.393 ms 1.022 ms 2.34
Zoom cycle mean (5M) 17.305 ms 15.677 ms 1.10
Zoom cycle mean std(5M) 0.692 ms 0.558 ms 1.24
Downsample mean std10M) 0.078 ms 0.056 ms 1.39
Render mean std10M) 4.684 ms 0.736 ms 6.36
Zoom cycle mean (10M) 17.635 ms 15.394 ms 1.15
Downsample mean std50M) 0.291 ms 0.195 ms 1.49
Render mean std50M) 12.057 ms 3.268 ms 3.69
Zoom cycle mean (50M) 17.226 ms 15.377 ms 1.12
Zoom cycle mean std50M) 0.799 ms 0.684 ms 1.17
Downsample mean ( std00M) 0.286 ms 0.073 ms 3.92
Instantiation mean ( std00M) 80.213 ms 52.146 ms 1.54
Zoom cycle mean (100M) 17.474 ms 15.054 ms 1.16
Zoom cycle mean ( std00M) 1.03 ms 0.749 ms 1.38
Downsample mean ( std00M) 3.776 ms 0.073 ms 51.73
Render mean ( std00M) 2.369 ms 2.014 ms 1.18
Zoom cycle mean (500M) 18.896 ms 16.929 ms 1.12
Zoom cycle mean ( std00M) 0.98 ms 0.749 ms 1.31
Dashboard create+render mean 311.332 ms 277.053 ms 1.12
Dashboard create+render stdmean 31.939 ms 22.076 ms 1.45
Dashboard page switch mean 0.174 ms 0.094 ms 1.85

This comment was automatically generated by workflow using github-action-benchmark.

CC: @HanSur94

HanSur94 and others added 3 commits May 8, 2026 22:27
Adds two regression tests addressing codecov's 50% patch coverage on the
fireEventsOnRisingEdges_ dedup change (PR #123 codecov comment, 16
uncovered lines).

* testRecomputeWithOpenRunDoesNotDuplicate — trailing IsOpen=true run
  must not be re-emitted on recompute after invalidate. The original
  dedup test only exercised the closed-run path; this covers the
  parallel "skip already-emitted open" branch in the trailing-open-run
  emission block.

* testRecomputeClosesExistingOpenEventInPlace — when a recompute_ sees
  a run that was previously stored as open but has now closed (parent
  grew with samples ending the run), the existing open event must be
  closed in place via EventStore.closeEvent — same Id preserved,
  EndTime set, IsOpen flipped to false — instead of being appended as
  a separate duplicate. Exercises the "open->closed transition" branch
  + the cache_.openEventId_ re-seeding path.

Note SensorTag.updateData REPLACES X/Y wholesale (not append-only); the
test passes the full extended grid [1..10] with the historical run
preserved and trailing samples that drop below threshold, mirroring how
the live pipeline re-publishes data files after each tick.

All 14 tests in TestMonitorTagEvents pass; 67/67 across the 6 monitor
+ event-store suites; 62/64 in TestFastSenseCompanion (2 pre-existing
fails unrelated to this PR).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pre-existing Octave failure inherited from main commit d3478d2
(260508-llw). The cross-page time-broadcast path eventually calls
xlim(), and Octave's __axis_limits__ wraps that in
addlistener(..., 'PostSet', ...), which it doesn't implement —
the suite errors with `'PostSet' undefined` on every case.

The implementation under test is MATLAB-only by virtue of its
xlim() dependency; nothing in the test logic is genuinely Octave-
hostile. Mirror the existing pattern from
test_dashboard_builder_interaction.m and other Octave-skipped
flat tests: print SKIPPED and return cleanly. Class-based
TestDashboardTimeSync (MATLAB-only suite) keeps full coverage.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tentPanel split

Pre-existing failure inherited from main commits d3478d2/d2a8594
(260508-mhv full-width-widget-bar series). When DashboardLayout
introduced WidgetContentPanel, widget.hPanel was redefined to point
at the content sub-panel BELOW the chrome bar — the bar itself,
hosting DetachButton + InfoIconButton, lives in the new outer
hCellPanel. The test files weren't updated when that split landed,
so findobj(w.hPanel, 'Tag', 'DetachButton'/'InfoIconButton') searches
the wrong subtree and returns 0×0 GraphicsPlaceholder.

Updates 7 findobj sites across TestDashboardDetach.m (1) and
TestInfoTooltip.m (6) to search hCellPanel instead. No production
behavior changes; same coverage, pointed at the correct panel.

Verified locally: TestDashboardDetach 10/10 + TestInfoTooltip 13/13
pass on MATLAB after the fix. Same fix is needed on main; main's
CI has been red since 939d902.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

1 participant