fix(gui): share one chart timeframe across the Runtime Monitor tabs (#5496)

[skerker]

Summary

Fixes #5496. The Runtime Monitor's Memory tab and Overview tab each built their own chart timeframe combo with the same four ranges, each read only by its own refresh (src/gui/SystemInfoDialog.cpp:583 and :772 on main 8a358c5f), so the two could disagree by construction: a range chosen on one tab never applied to the other. This moves the one control into a header row above the tab widget, the way the Network Diagnostics dialog keeps its networkDiagnosticsTimeframe in its page header (NetworkDiagnosticsDialog.cpp:327) and hides it on the pages with no chart (:958). Filed on the maintainer ruling in #5427 (design decision 3: accepted for that PR with this follow-up). Part of #2554.

What changed, by acceptance criterion:

1. One control in the dialog header, none in any tab — the combo and its label are built in the constructor on the dialog body, above the QTabWidget; both per-tab copies are deleted. m_memoryRange / m_overviewRange become one m_range.
2. One change redraws the Memory chart and all four Overview charts — one accessor selectedRangeSeconds() replaces the two; currentIndexChanged is connected to both refreshMemoryChart() and refreshOverview(), so the tab that is not current redraws too.
3. Hidden on Threads and Logs — the tab widget's currentChanged toggles label and combo by comparing the current page pointer against the Threads and Logs pages, not by tab index (the order changed once already in feat(gui): Runtime Monitor Overview tab — cards and the four charts (#2554) #5427).
4. Names carry over — object name systemInfoTimeframe and accessible name "Chart timeframe" are unchanged; the accessible description is now chart-neutral ("Choose how much recent history the charts display."). The Overview copy's object name systemInfoOverviewTimeframe (shipped in feat(gui): Runtime Monitor Overview tab — cards and the four charts (#2554) #5427 on 2026-09-08) no longer exists.

The two comments that justified the per-tab placement (:570-574, :766-767) are rewritten. Non-goals per the issue: no persistence of the choice (the Network dialog does not persist its own), no change to the ranges or bucket rules, no sidebar grouping.

Constitution principle honored

Principle XI — Fixes Are Demonstrated: every criterion is demonstrated through the agent automation bridge on the demo simulator, with the readings below. No test was added.

Test

Proof shape: bridge only. This is UI wiring — where a control lives and when it is shown — so the bridge readings and captures below are the proof, and no assertion was added. tests/system_info_dialog_test.cpp (existing target system_info_dialog_test) carries only the two edits the move forces: the Memory block's label no longer names the Memory tab (its lookup by systemInfoTimeframe on the dialog is unchanged and still finds the control), and the Overview block asserts that systemInfoOverviewTimeframe is gone instead of present. Local: the target passes at 4b2ce8ca (offscreen). It runs on full-suite.yml at merge and weekly on sanitizers.yml; it is not on the frozen per-PR gate (#5405).

Proof

Agent automation bridge, demo simulator DEMO-0001 (family sim), RX only, isolated settings store, AETHER_AUTOMATION_NO_TX=1. Readings at fae75daf (About v26.9.2 (fae75daf)); the Threads/Memory pair re-read at 390ba3d6. The head 4b2ce8ca differs from 390ba3d6 in tests/ only, so the app binary is the same.

• dump_tree filter "Timeframe": one QComboBox systemInfoTimeframe, parent = the dialog body, above the tab widget; filter systemInfoOverviewTimeframe: 0 matches.
• Set "1 hour" on Overview: all four charts read "Last 1 hour". Switch to Memory without touching the control: "Last 1 hour".
• Threads current: label and combo visible: false. Logs current: visible: false. Back on Overview and Memory: visible: true.
• Set "1 minute" while Memory is current, then switch to Overview: all four charts read "Last 1 minute" (the non-current tab redrew).
• Close and reopen the dialog: "5 minutes", visible — nothing persisted, per the non-goal.
• At 390ba3d6: the tab strip's geometry is the same with Memory current and with Threads current (the hidden row keeps its height).

No "before" build was run; the pre-fix state is the source cited above.

Evidence bundle index:

• fae75daf · About PNG, seven Runtime Monitor captures, the instance's whole log, ctest head and mutant outputs · aethersdr-pr5531-shared-timeframe-2026-09-09.zip (sha256 3bc2ee1f…0461f)
• 390ba3d6 · About PNG, Memory and Threads captures with the tab strip at the same y, ctest run at 4b2ce8ca · aethersdr-pr5531-retain-row-2026-09-09.zip (sha256 e9f4e793…3e6f1)

What I tried to break

• Changing the range while the control is hidden (Logs current): the bridge refuses a hidden control, so the case is unreachable by a user as well; the substitute attack (change on Memory, read Overview) held.
• Close and reopen (the dialog is delete-on-close): fresh default, visible, no stale state.
• Tab order: the visibility rule compares page pointers, so reordering tabs cannot break it; the test selects tabs by title for the same reason.
• Found by the adversarial pass at fae75daf: hiding the row let it collapse, so the tab strip jumped about 32 px between Memory and Threads. 390ba3d6 retains the widgets' size while hidden; the row stays put.

Not tested: a real radio (nothing in this change reads radio state); Linux and Windows builds (the code is plain Qt Widgets with no platform branch).

Test plan

• Local build passes (cmake --build build) — clean RelWithDebInfo builds at fae75daf and 390ba3d6 (4b2ce8ca is tests-only), 0 warnings in the touched files
• Behavior verified on a real radio if applicable — n/a, no radio-dependent behavior; verified on the demo simulator
• Existing tests pass (CI)
• Reproduction steps documented if user-reported bug — in the issue (two controls that can disagree)

Checklist

• Commits are signed (docs/COMMIT-SIGNING.md)
• No new flat-key AppSettings calls — n/a, no settings touched
• Code is clean-room
• All meter UI uses MeterSmoother — n/a, no meters
• Documentation updated if user-visible behavior changed — n/a, docs/ has no mention of the Runtime Monitor timeframe; CHANGELOG.md untouched per AGENTS.md
• Security-sensitive changes reference a GHSA if applicable — n/a

— authored by agent (Claude Code) on behalf of @skerker

[aethersdr-agent]

Issue fit

Yes, on three of the four acceptance criteria, verified by reading the head checkout at 4b2ce8ca:

• One control in the header, none inside any tab — SystemInfoDialog.cpp:227-262 builds it above the QTabWidget; both per-tab combos are gone, and systemInfoOverviewTimeframe no longer occurs anywhere in the tree (grepped: only the test's absence assertion).
• Both refreshes read it — selectedRangeSeconds() (:699) is the single reader, called from refreshMemoryChart and refreshOverview.
• Hidden on Threads and Logs — :272-279, page-identity comparison rather than index.
• Object/accessible names carry over — systemInfoTimeframe and "Chart timeframe" preserved; the Memory test finds it unchanged.

The non-goals hold: nothing persists the choice, the four ranges and bucket rules are untouched, and the tab structure is unchanged. The shape matches NetworkDiagnosticsDialog.cpp:326-341 and :957-960 closely enough that I diffed them side by side — same label text, same object-name convention, same accessible description phrasing, same page != logsTab && page != tciTab idiom. The one deliberate divergence is setRetainSizeWhenHidden (commit 2), which the network dialog does not do; that is an improvement, not a drift.

Scope

File	What it changes	Claimed?	Verdict
src/gui/SystemInfoDialog.cpp	Adds the header row + visibility lambda; deletes both per-tab combos; renames selectedMemoryRangeSeconds → selectedRangeSeconds	Yes	In scope
src/gui/SystemInfoDialog.h	m_overviewRange/m_memoryRange → m_rangeLabel/m_range; one accessor instead of two	Yes	In scope
tests/system_info_dialog_test.cpp	Retargets the Memory lookup; converts the Overview assertion to an absence check	Yes	In scope, but see nit 1

Everything in the diff is explained by the issue. No new settings key, no protocol surface, no CHANGELOG entry, no unrelated files, no formatting churn. The deleted - lines are the two combos and their comments; neither comment names a symptom that can recur — they document the per-tab rationale that #5496 explicitly overrules. No sibling implementation left behind: NetworkDiagnosticsDialog already had the window-level control, and no third dialog carries a chart timeframe.

Preference check: this is a fix, not smuggled preference — the issue exists, carries a maintainer ruling from #5427, and names the reference behavior (the network dialog) the PR moves toward. No default changed: both old combos already defaulted to index 1, and the new one does too.

Socket tests: none added, modified, or removed. tests/system_info_dialog_test.cpp opens no socket and constructs no fake peer.

Blockers

None.

Nits (non-blocking)

1. The test would pass with the two connect lines deleted. The Overview assertion is now purely negative (findChild(...) == nullptr), and the Memory case's range->setCurrentIndex(3) only asserts the readouts did not change — which is true whether or not refreshOverview is wired. So the PR's central claim, "changing the shared control redraws the Overview charts too," has no coverage at any layer. I checked whether it could: TimeSeriesGraphWidget (src/gui/TimeSeriesGraphWidget.h:60) stores m_rangeSeconds privately with no accessor, so asserting the redraw range would need a new getter — fair to leave out. But acceptance criterion 3 is cheap here and untested: findChild<QTabWidget*>(), show(), setCurrentIndex() to Threads, then m_range->isVisible() == false. That is a socket-free assertion in an already-registered target.

2. Both m_rangeLabel and m_range carry the accessible name "Chart timeframe" (:231, :234). A screen reader passing over the row announces it twice. Copied verbatim from the network dialog, so this is a pre-existing convention rather than something this PR introduced — flagging only in case it's worth fixing in both places later.

What I tried to break

• Construction order. refreshOverview and refreshMemoryChart are now reachable from the combo. I checked that m_range is fully built at :227-262 before buildOverviewTab()/buildMemoryTab() run at :264-267, and that setCurrentIndex(1) at :243 precedes both connect calls at :254-258 — so no refresh fires against half-built graph pointers. selectedRangeSeconds() also keeps the old null guard returning 300.
• Null pointers on the new path. refreshOverview dereferences four m_overview*Graph pointers; every one is behind an if (... != nullptr), and the ring accessors are guarded by cpu != nullptr / mem != nullptr. Changing the combo before any sample has arrived draws empty series rather than crashing.
• Dangling capture at teardown. The visibility lambda captures raw tabs, threadsTab, logsTab and touches m_rangeLabel/m_range. If QTabWidget::currentChanged could fire while children are being destroyed, those would be stale. Qt 6's ~QObject tears down connections before deleteChildren(), and the receiver context is this, so the connection is gone before tabs dies. Same shape the network dialog already ships.
• Stale external references. Grepped the whole head checkout for systemInfoOverviewTimeframe, m_overviewRange, selectedOverviewRangeSeconds and selectedMemoryRangeSeconds across sources, tests, docs, resources/help and the touchpoint manifests — the only survivor is the test's deliberate absence check. Nothing in the automation bridge or SupportBundle addresses these widgets.
• Tab identity vs index. The comparison is by page pointer, so the Painters tab that the :263 comment anticipates can be inserted anywhere without silently flipping the rule. buildOverviewTab() returns its QScrollArea wrapper, and that same pointer is what addTab receives, so currentWidget() compares consistently.
• CI. All five checks green on 4b2ce8ca (build, check-macos, check-windows, Static checks, Sanitizer). Noting the standing caveat that ctest in ci.yml is -R-filtered, so green is not the full suite.

All of the above is reasoned from source in /tmp/aetherclaude/pr-5531; I am headless and did not build, run the dialog test, or drive the GUI, so no runtime behavior here was observed.

Recommendation

Approve with nits. The change does exactly what the issue asks, matches the reference dialog it cites, and I could not construct an ordering, lifetime, or null case that breaks it. The one thing worth asking for before merge is a visibility assertion in system_info_dialog_test.cpp — the criterion the PR's own commit message calls out is the one nothing currently pins, and it costs about four lines in a test that already exists.

---

_{🤖 aethersdr-agent · cost: $6.4786 · model: claude-opus-5}

[aethersdr-agent]

This is now the only Overview-side assertion about the timeframe, and it is purely negative — it passes as long as the old object name is absent. Combined with the Memory case (which asserts only that the readouts are undisturbed by setCurrentIndex(3)), deleting both connect lines in SystemInfoDialog.cpp:254-258 would leave the whole file green.

The redraw itself is genuinely hard to assert — TimeSeriesGraphWidget keeps m_rangeSeconds private with no getter — so I'm not asking for that. But the issue's third acceptance criterion is cheap in this already-registered socket-free target:

        report("the Overview tab has no timeframe selector of its own (#5496)",
               ov.findChild<QComboBox*>(QStringLiteral("systemInfoOverviewTimeframe")) == nullptr);
        // ...and the shared one hides on the tabs with no chart (#5496).
        auto* ovTabs  = ov.findChild<QTabWidget*>();
        auto* ovRange = ov.findChild<QComboBox*>(QStringLiteral("systemInfoTimeframe"));
        if (ovTabs != nullptr && ovRange != nullptr) {
            ov.show();
            QCoreApplication::processEvents();
            report("the timeframe shows on Overview", ovRange->isVisible());
            ovTabs->setCurrentIndex(1);   // Threads: fixed 60 s window
            report("the timeframe hides on Threads", !ovRange->isVisible());
            ovTabs->setCurrentIndex(2);   // Memory: charted again
            report("the timeframe returns on Memory", ovRange->isVisible());
            ov.hide();
        }

Non-blocking.

[aethersdr-agent]

Checked the ordering hazard this introduces and it holds: m_range is fully populated and setCurrentIndex(1) has already run by line 243, both connects come after it, and buildOverviewTab()/buildMemoryTab() only run at 264-267 — so neither slot can fire against null graph pointers during construction, and the default selection raises no spurious refresh. refreshOverview is null-guarded on all four m_overview*Graph members besides.

No change requested; recording it because "one combo now drives two refreshes" is the place this would have gone wrong.