feat(automation): radio-side display-stream inventory + leak detection (streams verb) (#3856)#3857
Conversation
…view of aethersdr#3856) A multi-agent review of aethersdr#3857 found five soundness defects, all in the new diagnostic (Layer A/B), all low severity, all fixed here: 1+2. Un-normalized id comparisons. m_radioDisplayPans/Waterfalls were keyed on the raw status-regex capture (uppercase hex possible via ToString("X")), while parentPanId and the owned sets were normalizePanadapterId'd (lowercase) — a hex-letter case divergence (e.g. 0x4200000A vs …a) could misclassify our own waterfall as orphan or, with the parent gone, a false leak. Now every inventory key and the owned waterfall ids are normalized, so all ours/foreign/orphan/parent-missing comparisons are case-insensitive. 3. Waterfall-removed prune only handled the bare "… removed" object form; the kv form "removed=1" would match wfRe and be RE-RECORDED as an add. Now gated on `kvs.contains("removed") || object.endsWith("removed")`, mirroring the pan branch. 4. Reconnect-reclaim window: displayInventoryReport() excluded m_stalePanadapters, so our own staged-but-not-yet-reclaimed pans/waterfalls transiently reported as orphan while the radio re-dumped them. Now the owned sets include staged objects too. 5. The orphan table never aged out and dropped NEW entries at the 32-cap, so a real leak could be lost behind transient entries. Now evicts the least-recently-seen entry (LRU by lastSeenMs) — a live leak keeps updating its lastSeenMs so it's never the stalest. Build clean; display_inventory_policy_test still 12/12. Changes are behavior-preserving for the canonical lowercase 8-digit stream ids this firmware emits (normalize is idempotent there), so the live FLEX-8400M proof in aethersdr#3857 stands; the fixes harden the edge cases (hex-letter ids, kv-form removal, mid-reconnect reads, cap pressure). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adversarial self-review passRan a 4-dimension multi-agent review (concurrency · Layer-B correctness · detector soundness · bridge/build), each finding refute-verified. It surfaced 5 real defects — all low severity, all in the new diagnostic, none in a code path that drives radio teardown — fixed in
Build clean, |
There was a problem hiding this comment.
Thanks for this @jensenpat — it's a clean, well-scoped, and unusually well-documented observability feature. The split into two independent radio-authoritative layers is the right design, the pure DisplayInventoryPolicy::classify() mirrors the existing RadioStatusOwnership/SliceRecreatePolicy pattern nicely, and it's properly unit-tested. CI is fully green (build, macOS, Windows, CodeQL, a11y). A few things I checked and one substantive note.
Verified good:
- Pan inventory is recorded before the ownership classification early-returns (
Defer/Ignore), so foreign and orphan pans are genuinely tracked — matches the PR claim. - Waterfall removal handles both wire forms (bare
removedinobject, andremoved=1inkvs), and correctly catches the kv form before it would matchwfReand be mis-recorded as an add. m_radioDisplayPans/Waterfallsare cleared ononDisconnected()so reconnect re-dumps cleanly.- Layer A's orphan eviction (evict least-recently-seen at capacity) correctly preserves an actively-streaming leak over transient tails.
PanadapterStreamaccess is mutex-guarded;OrphanStream.ageMsuses a consistent monotonic clock source.
Substantive — leaked-waterfall detection depends on panadapter= being present in the waterfall status. parentMissing (the headline #3843 fingerprint) only ever trips when RadioDisplayWf.parentPanId is non-empty, and that field is populated solely from kvs.value("panadapter") in the display waterfall branch. But the pre-existing ownership code treats panadapter= as a fallback — the primary pan→waterfall linkage is the pan status's waterfall= key, matched via pan->waterfallId() == wfId. That strongly implies the waterfall status doesn't always carry panadapter=. On firmware that conveys the parent link only through the pan side, parentPanId stays empty → parentMissing is never true → a real leaked waterfall reports clean. That's exactly the firmware class this feature targets (the FLEX-8400M you tested cascades, so it never exercised a live positive). Consider backfilling parentPanId from the owning pan's reverse mapping (the pan whose waterfall= referenced this id) when the waterfall status omits panadapter=, so Layer B's positive catch isn't gated on a field the radio may not echo. The unit test covers the logic, but the real-radio data path is the gap.
Minor — threading. displayInventoryReport() reads m_radioDisplayPans/Waterfalls without a lock, relying on the bridge running on the GUI thread (the header comment notes "Main-thread only"). That's consistent with how doPan already calls into RadioModel, so it's fine — just flagging it as the assumption to keep intact if the bridge ever moves off the main thread.
Neither blocks merge from my side (review-only). The parentPanId backfill is the one I'd genuinely encourage, since it's what makes Layer B's live leak detection actually fire on the #3843-class firmware. Nice work.
🤖 aethersdr-agent · cost: $5.3366 · model: claude-opus-4-8
aethersdr#3856) Adds a `streams` bridge verb that makes leaked/duplicate panadapter and waterfall streams on the radio directly observable — a class of bug `get pans` can never surface, because the client always cleans up its own view on the radio's "removed" echo. Two independent radio-authoritative layers: Layer A — `streams` (VITA-49 UDP truth). PanadapterStream records any FFT/waterfall packet for a stream id that was EVER registered this session but is no longer registered — a stream we once owned and let go of that the radio keeps transmitting. Keyed off "ever-registered AND not-now-registered" so it stays detectable after `pan close all` and never mis-flags a freshly created stream's registration-lag window. Catches continued-stream leaks (the aethersdr#268 class). `streams reset` re-baselines the tally. Layer B — `streams radio` (status-bookkeeping truth). RadioModel now accumulates the radio-authoritative display-object set (m_radioDisplayPans / m_radioDisplayWaterfalls) from ALL "display pan" / "display waterfall" status — ours, foreign, and orphan — pruned on the matching "removed", independent of m_panadapters (which only holds objects WE own). The pure DisplayInventoryPolicy::classify() then labels each object ours/foreign/orphan and flags leaked waterfalls (parent panadapter gone) — the aethersdr#3843 fingerprint. This catches resource-level lingering that emits no UDP, which Layer A cannot see. DisplayInventoryPolicy is a pure, header-only function (mirroring RadioStatusOwnership / SliceRecreatePolicy) with a unit test (display_inventory_policy_test) covering clean, leaked-parent-gone, foreign- vs-orphan, no-parent-reported, and unknown-own-handle cases. Proven on a live FLEX-8400M (firmware 4.2.18) via the bridge: - `streams radio` tracked the inventory exactly across add/close (1 pan/1 wf → 2/2 → 1/1), every object ownership=ours, no false-positive leaks. - Key firmware finding: on this radio `display pan remove` ALONE makes the radio also remove the waterfall — it echoes `display waterfall <id> removed`. So neither aethersdr#3843 symptom (continued UDP nor lingering resource) reproduces on 8000-series/4.2.18; the radio auto-cascades. The aethersdr#3855 close-path fix remains correct per FlexLib and matters for firmware that does NOT cascade (the aethersdr#268 class). Because this firmware never leaves an orphan, Layer B's positive leak detection is covered by the unit test; live it correctly reads clean with no false positives. All `streams` actions are RX/observe only — none sends a radio command or keys the transmitter. Docs in docs/automation-bridge.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…view of aethersdr#3856) A multi-agent review of aethersdr#3857 found five soundness defects, all in the new diagnostic (Layer A/B), all low severity, all fixed here: 1+2. Un-normalized id comparisons. m_radioDisplayPans/Waterfalls were keyed on the raw status-regex capture (uppercase hex possible via ToString("X")), while parentPanId and the owned sets were normalizePanadapterId'd (lowercase) — a hex-letter case divergence (e.g. 0x4200000A vs …a) could misclassify our own waterfall as orphan or, with the parent gone, a false leak. Now every inventory key and the owned waterfall ids are normalized, so all ours/foreign/orphan/parent-missing comparisons are case-insensitive. 3. Waterfall-removed prune only handled the bare "… removed" object form; the kv form "removed=1" would match wfRe and be RE-RECORDED as an add. Now gated on `kvs.contains("removed") || object.endsWith("removed")`, mirroring the pan branch. 4. Reconnect-reclaim window: displayInventoryReport() excluded m_stalePanadapters, so our own staged-but-not-yet-reclaimed pans/waterfalls transiently reported as orphan while the radio re-dumped them. Now the owned sets include staged objects too. 5. The orphan table never aged out and dropped NEW entries at the 32-cap, so a real leak could be lost behind transient entries. Now evicts the least-recently-seen entry (LRU by lastSeenMs) — a live leak keeps updating its lastSeenMs so it's never the stalest. Build clean; display_inventory_policy_test still 12/12. Changes are behavior-preserving for the canonical lowercase 8-digit stream ids this firmware emits (normalize is idempotent there), so the live FLEX-8400M proof in aethersdr#3857 stands; the fixes harden the edge cases (hex-letter ids, kv-form removal, mid-reconnect reads, cap pressure). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…thersdr#3856 review) Layer B's leaked-waterfall detection (`parentMissing`) only fired when the waterfall status carried `panadapter=`. But the pan↔waterfall link is primarily the pan status's `waterfall=` key; some firmware conveys it only there and omits `panadapter=` from the waterfall status — so on exactly the firmware this feature targets, a leaked waterfall would report clean. Stamp the parent onto the persistent waterfall entry from the pan side, at both record orderings: - pan status (`waterfall=<id>`): if that waterfall entry exists with no parent, set its parentPanId to this pan. - waterfall status without `panadapter=`: scan pans for one referencing this waterfall and adopt its id. Stamping the waterfall entry (which persists until the waterfall is removed) means the parent link survives the pan's removal — which is precisely when the leak shows. classify()/policy unchanged; the existing parent-gone unit test already covers the detection this now feeds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
02b5751 to
aff0e11
Compare
ten9876
left a comment
There was a problem hiding this comment.
Approved. Rebased onto current main (resolved the #3855 conflicts: verb-list help string now lists both connect/disconnect and streams; CHANGELOG + docs kept both sections), and added the parentPanId backfill addressing the one substantive review note.
- Read-only / observe only —
streams/streams radio/streams resetsend no radio command and never key TX (Principle VI N/A). - Two-layer design verified: Layer A (VITA-49 UDP orphan, ever-registered ∧ not-now-registered) + Layer B (pure
DisplayInventoryPolicy::classify, unit-tested, 12 assertions). - Backfill (#3856 review): Layer B's
parentMissingpreviously only fired when the waterfall status carriedpanadapter=. Now the parent link is stamped onto the persistent waterfall entry from the pan'swaterfall=link (both record orderings), so it survives the pan's removal — exactly when a leak shows — making the positive catch fire on firmware that omitspanadapter=. Policy + unit test unchanged;display_inventory_policy_test12/12 pass; app builds clean.
CI green on all 6. Thanks @jensenpat — excellent observability feature with a great live-hardware writeup.
What
Implements the radio-side display-stream inventory + leak detection feature requested in #3856. It makes leaked / duplicate / lingering panadapter & waterfall streams on the radio directly observable — a class of bug
get panscan never surface, because the client always cleans up its own view on the radio'sremovedecho.Two independent radio-authoritative layers behind one
streamsbridge verb:Layer A —
streams(VITA-49 UDP truth)PanadapterStreamrecords any FFT/waterfall packet for a stream id that was ever registered this session but is no longer registered — a stream we once owned and let go of that the radio keeps transmitting. Keyed off ever-registered ∧ not-now-registered (not "live set non-empty"), so it stays detectable afterpan close alland never mis-flags a freshly-created stream's registration-lag window. Catches continued-stream leaks (the #268 class).streams resetre-baselines.Layer B —
streams radio(status-bookkeeping truth)RadioModelnow accumulates the radio-authoritative display-object set (m_radioDisplayPans/m_radioDisplayWaterfalls) from alldisplay pan/display waterfallstatus — ours, foreign, and orphan — pruned on the matchingremoved, independent ofm_panadapters(which only holds objects we own). The pureDisplayInventory::classify()labels each objectours/foreign/orphanand flags leaked waterfalls (parent panadapter gone) — the #3843 fingerprint. This catches resource-level lingering that emits no UDP, which Layer A cannot see.DisplayInventoryPolicyis a pure, header-only function (mirroringRadioStatusOwnership/SliceRecreatePolicy), with a unit test (display_inventory_policy_test, 12 assertions) covering clean, leaked-parent-gone, foreign-vs-orphan, no-parent-reported, and unknown-own-handle cases.Closes #3856.
Bridge surface
streamsregisteredPanStreams,registeredWfStreams,orphanStreams(streamId,kind,packets,age_ms)streams radio(aliasinventory)leakedWaterfallsstreams resetAll
streamsactions are RX/observe only — none sends a radio command or keys TX. Docs indocs/automation-bridge.md.Proven on a live FLEX-8400M (firmware 4.2.18)
streams radiotracked the inventory exactly across an add/close cycle, every objectownership: ours, no false-positive leaks:Key firmware finding (answers the open #3843 question): on this radio,
display pan removealone makes the radio also remove the waterfall — it echoesdisplay waterfall <id> removed. A leak-repro build that deliberately omitteddisplay panafall removeproduced the same clean result (leakedWaterfalls=[], waterfall count 2→1, removal echo observed). So neither #3843 symptom — continued UDP nor lingering resource — reproduces on 8000-series / 4.2.18: the radio auto-cascades. The #3855 close-path fix remains correct per FlexLib and matters for firmware that does not cascade (the #268 class).Because this firmware never leaves an orphan, Layer B's positive leak detection is covered by the unit test (synthetic parent-gone waterfall → flagged); live, it correctly reads clean with no false positives. Layer A likewise reads
orphanCount:0here (this firmware stops the UDP), and is the guard for firmware that keeps streaming.Test plan
display_inventory_policy_test— 12/12 pass (pure classification logic, incl. the leaked-waterfall case).streams+streams radioinventory accurate across add/close; no false-positive leaks; firmware cascade behavior characterized.Notes / follow-ups
Related
💻 Generated with Claude Code (Opus 4.8) with architecture by @jensenpat