Skip to content

Signal viewer: channel_scope — a panel that actually owns its channels#19

Merged
RaulSimpetru merged 1 commit into
mainfrom
feat/viewer-channel-scope
Jul 25, 2026
Merged

Signal viewer: channel_scope — a panel that actually owns its channels#19
RaulSimpetru merged 1 commit into
mainfrom
feat/viewer-channel-scope

Conversation

@RaulSimpetru

Copy link
Copy Markdown
Member

Stacked on #18channel_scope builds on the widget_id multi-panel work, so this targets
that branch rather than main. Review/merge #18 first.

Closes the known gap #18 called out: initial_channels seeds a selection, it does not restrict
one.
A per-electrode-grid panel (Quattrocento IN1IN5, 64 ch each of a 320-ch stream) was
therefore scoped in appearance only — until the user touched it:

  • All enabled every channel in the stream
  • [Edit…] listed every grid, not just the panel's
  • the count read N/320 rather than N/64

SignalViewer(channel_scope=…) is the hard restriction: the columns a panel may ever show. It
bounds the seed, All/None/Invert, the count, the grid selector, shift-click ranges and rubber-band
drags, and it forms part of the selection cache key so a differently-scoped viewer sharing a
widget_id can't inherit another's selection. None (default) is unrestricted — nothing changes
for a single-panel viewer.

Bugs found while reviewing the plan

Codex reviewed the design and blocked it. Each finding was verified in source before being
accepted; four were real and would otherwise have shipped:

  • resolve_enabled returns early on an unchanged key, so enforcing the scope only while
    seeding/restoring would let anything that mutated v.channels in between escape it. It now
    clamps on the fast path too.
  • Invert went through reduce_selection's XOR, which preserves out-of-scope members (they
    aren't in the target set). It is now a complement within the scope.
  • Shift-click selected a raw numeric range — a sparse scope {0,2,4} shift-clicked 0→4 pulled
    in 1 and 3. The span is filtered through the allowed set.
  • An active-key change reset last_clicked but not the armed drag, so a drag spanning a
    stream/scope switch applied a stale snapshot.

Design notes

  • Fail closed. An explicit scope matching no valid column renders "no channels in scope"
    rather than falling back to every channel — a silent widening would defeat the point of scoping
    a panel. (normalize_layout's forgiving fallback stays right for malformed stream metadata;
    it's wrong for an explicit caller constraint.)
  • resolve_initial keeps its signature and takes a keyword-only scope, so the default policy
    runs inside the scope. Without that, a 256‥319 scope would seed "the first 16 of 320" and
    intersect to the empty set — a dead panel.
  • Out-of-scope grid cells are nulled, not dropped, so a rectangular grid keeps its physical
    shape and — since rect_to_channels skips None — drag selection is structurally incapable of
    reaching outside the scope.
  • Scoped columns that no grid covers are collected into a trailing "other" grid, so
    everything All can select is also individually toggleable.
  • The constructor snapshots the scope: Iterable admits a generator, which per-frame
    resolution would exhaust.
  • channel_scope also drives the default selection, so a 64-channel scope opens on its first 16
    unless initial_channels is passed too — the multi-grid example passes both.

Notes

  • 705 tests pass (9 new), ruff clean. New coverage: sparse/ordered/duplicate/empty/wholly-invalid
    scopes, the 256‥319 high-offset seed, fast-path clamping, scope-fingerprint cache keys,
    sparse shift-click, nulled-grid drags, partial metadata coverage, generator stability, and a
    channel_scope=None regression guard.
  • tests/test_stream_lsl.py::test_stream_reconnect_swaps_buffers_atomically remains flaky under
    full-suite load (LSL multicast contention) and passes in isolation — pre-existing.

`initial_channels` only ever seeded a selection, so a per-electrode-grid panel
was scoped in appearance only: All enabled every channel in the stream, [Edit…]
listed every grid, and the count read N/320 rather than N/64. `channel_scope`
is the hard restriction — the columns a panel may EVER show.

It bounds the seed, All/None/Invert, the N/total count, the grid selector,
shift-click ranges and rubber-band drags, and it forms part of the selection
cache key so a differently-scoped viewer sharing a widget_id cannot inherit
another's selection. `None` stays unrestricted, so nothing changes for a
single-panel viewer.

Codex reviewed the plan and blocked it; each of its findings was verified in
source before being accepted, and four were real:

- resolve_enabled returns early on an unchanged key, so enforcing the scope
  only while seeding/restoring would let anything that mutated v.channels in
  between escape it. It now clamps on the fast path too.
- Invert went through reduce_selection's XOR, which PRESERVES out-of-scope
  members (they aren't in the target set). It is now a complement within scope.
- Shift-click selected a raw numeric range, so a sparse scope {0,2,4}
  shift-clicked 0->4 pulled in 1 and 3. The span is filtered through the
  allowed set.
- An active-key change reset last_clicked but not the armed drag, so a drag
  spanning a stream/scope switch applied a stale snapshot.

Also from that review: an explicit scope that matches nothing now renders "no
channels in scope" instead of falling back to every channel — a silent widening
would defeat the point of scoping a panel. resolve_initial keeps its signature
and takes a keyword-only scope, so the policy runs *inside* the scope; without
that, a 256..319 scope would seed "the first 16 of 320" and intersect to the
empty set, leaving a dead panel. Out-of-scope grid cells are nulled rather than
dropped, which preserves a grid's shape and — since rect_to_channels skips None
— makes drag selection incapable of reaching outside the scope. Scoped columns
no grid covers are collected into a trailing "other" grid so everything All can
select is also individually toggleable. The constructor snapshots the scope,
since an Iterable may be a generator that per-frame resolution would exhaust.
@RaulSimpetru
RaulSimpetru changed the base branch from feat/viewer-scaling-and-panels to main July 25, 2026 05:27
@RaulSimpetru
RaulSimpetru merged commit 87649af into main Jul 25, 2026
4 checks passed
RaulSimpetru added a commit that referenced this pull request Jul 25, 2026
Cut 2.4.0 — the signal-viewer scaling/detail/multi-panel work (#18) and the
channel_scope follow-up (#19). New user-facing capability with no breaking
changes, hence a minor bump.

Highlights: the y-axis eases instead of jittering and ignores artifacts by
duration; "Point cap" becomes the width-relative "Detail" control; one stream
can be shown through several panels (widget_id) each owning its own channels
(channel_scope); Heatmap gained a shared colour range. The visual language that
had only ever lived in the code is now written down in
docs/concepts/visual-language.md and two of its rules are enforced by tests —
which caught RawSignalViewer rendering as stock ImPlot and 19 hardcoded colours
that could not follow the light theme.
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