Skip to content

fix(#327): ChannelsPanePrototype's 81 bindings were all on reflection — three declared templates, and a drawer label bound to a member IChannel never declared - #394

Merged
tylerkron merged 1 commit into
mainfrom
issue-327-channelspaneprototype-compiled-bindings
Sep 19, 2026
Merged

tylerkron merged 1 commit into
mainfrom
issue-327-channelspaneprototype-compiled-bindings

Conversation

@tylerkron

@tylerkron tylerkron commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

refs #327 — not merging, for review.

What was wrong

ChannelsPanePrototype.axaml (the desktop Channels tab) declared no x:DataType, so all 81 of its bindings resolved by reflection: 72 attribute bindings plus the 9 <Binding> children of its 4 MultiBindings, counted by parsing the XAML (not grepping it). Rename or delete a member on ChannelsPaneViewModel, ChannelTileViewModel, IChannel or IStreamingDevice and a tile would render blank, or a drawer control would stop responding, while every head still built green.

The fix

Scope Declared or inferred Type
root declared, with x:CompileBindings="True" ChannelsPaneViewModel
ChannelTileTemplate (keyed, in UserControl.Resources) declared ChannelTileViewModel
the tile's 3 hops (IsLoggingActive, ToggleChannelCommand, OpenSettingsCommand) via $parent[UserControl].DataContext inferred, no cast needed ChannelsPaneViewModel
device-name chip ItemTemplate inferred from ConnectedDeviceNames string
palette ItemTemplate declared IBrush
its 2 hops (SelectedChannel.ChannelColorBrush, SetColorCommand) via $parent[UserControl] inferred ChannelsPaneViewModel
SwatchButton's ControlTemplate (Background="{Binding}") declared IBrush

Why each declaration is there:

  • The tile template is required. A resource-dictionary DataTemplate infers nothing. With the declaration removed, the build fails with 14 AVLN2000s on XamlPseudoType. All three lists that use the template (AnalogInputs, DigitalInputs, DigitalOutputs) are ObservableCollection<ChannelTileViewModel>.
  • The palette template and the swatch ControlTemplate compile without their declarations today, because the bindings inside them have no member path (<Binding/> and {Binding}). They are declared anyway because both would otherwise be typed wrongly. Palette is an array, and an array infers nothing. A ControlTemplate is not a new scope, so it would type-check against the root's ChannelsPaneViewModel, but at runtime its DataContext is the palette entry. The type is IBrush, not Brush. ChannelsPaneViewModel has using Brush = Avalonia.Media.IBrush;, so Palette is really an IBrush[] holding ImmutableSolidColorBrushes. My first draft declared Brush (the abstract Avalonia class). That compiled and was wrong, and I caught it only by reading the view-model's usings.

IChannel gains string ScaleExpressionError { get; }. Without it, the drawer's error label (SelectedChannel.ScaleExpressionError, where SelectedChannel is typed IChannel) is AVLN2000. This is not a runtime bug fix. On main the reflection binding resolves against the concrete AbstractChannel, which has always had the member, so the label worked. The interface simply didn't declare it. The hunk is byte-identical to the one in #392 (ChannelsMobileView), which needs the same member for the same reason. So the two PRs should merge in either order without a conflict in that file. AbstractChannel is the only IChannel implementer in the repo.

What DataContext each scope holds at runtime (read, not observed)

The one host is MainWindow.axaml's Channels tab: <ContentControl Content="{Binding}"> whose ContentTemplate holds <prototype:ChannelsPanePrototype/>. So the pane first inherits the window's DaqifiViewModel. Its code-behind then swaps in its own view-model in OnLoaded (if (DataContext is not ChannelsPaneViewModel) DataContext = new ChannelsPaneViewModel();) and nulls it in OnUnloaded. HeadlessBench builds a ChannelsPaneViewModel directly and never this view. ChannelsPaneViewModel is the only type these bindings are written for.

The one runtime difference: SelectedDevice.PwmFrequencyHz before OnLoaded (re-checked by a shepherd, 2026-09-18). Between the pane attaching and OnLoaded firing, the pane holds the window's DaqifiViewModel on the first visit and null on later visits (#397 measured this on the Devices pane, which has the same host and the same swap). On main, the reflection binding SelectedDevice.PwmFrequencyHz resolved against DaqifiViewModel.SelectedDevice during that window. It is the only root path whose name also exists on DaqifiViewModel. DaqifiViewModel.IsLoggingActive is an explicit interface implementation, so reflection never saw it. On this branch the compiled binding rejects the mismatched type and the box stays blank. This is not a regression, because main was showing the wrong data there:

  • DaqifiViewModel.SelectedDevice is the Devices pane's selection. This pane's SelectedDevice is a different property. OnLoaded creates a fresh ChannelsPaneViewModel whose SelectedDevice is null and whose IsSettingsOpen is false. It is set only by OpenSettings, which runs when the user clicks a tile's cog, to the device that owns that channel (FindOwningDevice), and it is cleared again by CloseSettings. So once the swap happens, both main and this branch show a blank box inside a closed drawer. The value never comes back after OnLoaded.
  • It lasted one to two frames, 16–37 ms by Devices pane paints one or two frames with the settings drawer forced open on every tab visit (pre-OnLoaded DataContext) #397's measurement, and only on the first visit to Channels. Later visits carry null, so main is blank there too. It also needs a device to be selected in Devices at that moment. The drawer is visible in that frame only because a failed IsSettingsOpen binding leaves IsVisible at its default true, on both main and this branch. That is Devices pane paints one or two frames with the settings drawer forced open on every tab visit (pre-OnLoaded DataContext) #397's defect, and this PR neither causes nor fixes it.
  • No write-back was possible. The binding is UpdateSourceTrigger=LostFocus, and nothing can take focus within that frame.

This conclusion is read from the code plus #397's frame measurement. I did not capture a frame of the Channels pane myself.

Verification

  • One-build positive control on main. I put a typo in the root scope (HasConnectedDeviceQ), a typo in the tile template (TileBackgroundQ) and a bogus property (BogusPropertyQ on the root Grid) in the same build. The result was exactly 1 error: the property AVLN2000 at (336,10). The compiler read the file and said nothing about either typo, which is the defect this PR closes.

  • Branch, one typo per scope, all 11 in one build from a fresh edit. Result: 11 AVLN2000s, each naming the right type:

    • ChannelsPaneViewModel via the tile's $parent hop (151), at the root (350), and via the palette's $parent hop (638)
    • ChannelTileViewModel (153)
    • IChannel through the tile's Channel. (177), through a MultiBinding child (650), and through SelectedChannel. (787)
    • IBrush in the swatch ControlTemplate (299) and in the palette template (629)
    • string in the chip template (404)
    • IStreamingDevice through SelectedDevice. (768)
  • Swatch ControlTemplate runtime-type control (shepherd, 2026-09-18, one build from a fresh edit). At runtime the template's Border gets its DataContext from the swatch Button. The Button sets no DataContext and no Content, so that DataContext is the palette item: an ImmutableSolidColorBrush from Palette, which is an IBrush[]. The type that must be checked is therefore IBrush. In one build, next to {Binding} I added Tag="{Binding OpacityQ}" (a typo) and Opacity="{Binding Opacity}" (a member of IBrush but not of ChannelsPaneViewModel). The result was exactly 1 AVLN2000: 'OpacityQ' on type 'Avalonia.Media.IBrush' at (300,29). Opacity was accepted. Then, from another fresh edit, I removed the template's x:DataType="IBrush". The result was 2 errors, both naming ChannelsPaneViewModel, one for OpacityQ and one for the valid Opacity. So without the declaration the template type-checks against the wrong type, and the declaration is what makes it correct. I reverted the edit afterwards, and the clean build exited 0.

  • Declaration-necessity control. I dropped all three inner declarations in one build. The result was 14 AVLN2000 on XamlPseudoType, all in the tile template. The two IBrush declarations are not needed to compile, as stated above.

  • Test: ChannelsPanePrototypeBindingTests, 28 lines, using only existing BindingFacts helpers (AssertRootDeclares ×2, AssertNoEscapeHatch). I ran two mutations against it:

    • Deleting the root x:CompileBindings line fails the CompileBindings row (1/3 red).
    • Adding x:CompileBindings = 'False' on the palette template fails the escape-hatch row (1/3 red).
  • Build gate is green: desktop, iOS restore, and iOS Release iossimulator-arm64 (the iOS leg was incremental). dotnet test passes 996/996.

  • Rendering: tools/parity-audit/run.sh --check-baseline, run alone (load avg ~1.5, no other capture running), gave 39/39 in one run. The denominator is git show origin/main:tools/parity-audit/baselines/macos-arm64.sha256 | wc -l = 39. desktop-3-channels and desktop-min-3-channels are byte-identical to main's baseline. Those screens show the no-device state, so they exercise the root scope and the empty-state gate. The tiles, the settings drawer, the palette and the swatch were not rendered; the compiler controls above cover them.

  • Warnings: delta 0, taken from CI's own logs at both ends: main 642caef, run 35377977069, against this head 4b13ea8, run 35394779286. These are all-project totals per job from cold CI builds:

    • Desktop-on-macOS 169 → 169
    • iOS 170 → 170
    • Android 175 → 175
    • Library+Desktop+capture 169 → 169

    The distinct (file, line, code) warning sets are identical for every job. CI tests went 993 → 996.

Coordination

Diffstat: +55 / −6 across 3 files (+28 test, +20/−6 view, +7 interface).

🤖 Generated with Claude Code

Root declares ChannelsPaneViewModel with x:CompileBindings; the resource-dictionary tile template declares ChannelTileViewModel (infers nothing otherwise); the palette ItemTemplate and SwatchButton's ControlTemplate declare IBrush (Palette is IBrush[] via the VM's Brush alias). IChannel gains ScaleExpressionError, identical to #392's hunk, so the drawer's error label type-checks through SelectedChannel.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tylerkron

Copy link
Copy Markdown
Contributor Author

/agentic_review

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Compile Channels pane bindings and expose channel scale errors

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Compile all 81 Channels pane bindings against explicit root and template data types.
• Expose scale-expression errors through IChannel for typed drawer bindings.
• Guard compiled-binding declarations and prohibit reflection escape hatches.
Diagram

graph TD
  Tests["Binding tests"] -->|validates declarations| View["Channels pane"] -->|root bindings| RootVM["Channels VM"] -->|selected channel| Channel["IChannel"]
  View -->|item scope| TileTemplate["Tile template"] -->|typed bindings| TileVM["Tile VM"]
  View -->|palette scope| PaletteTemplates["Palette templates"] -->|typed bindings| Brush["IBrush"]
Loading
High-Level Assessment

The scoped approach is appropriate: explicit root and template types make binding failures compile-time errors without changing binding behavior across unrelated views. Enabling compiled bindings project-wide was considered but would broaden risk substantially and should remain a separate migration.

Files changed (3) +55 / -6

Bug fix (2) +27 / -6
IChannel.csExpose scale-expression errors on IChannel +7/-0

Expose scale-expression errors on IChannel

• Adds the read-only ScaleExpressionError contract already provided by AbstractChannel. This lets the compiled settings-drawer binding resolve through the SelectedChannel interface type.

Daqifi.Avalonia/Daqifi.Desktop/Channel/IChannel.cs

ChannelsPanePrototype.axamlEnable typed compiled bindings throughout the Channels pane +20/-6

Enable typed compiled bindings throughout the Channels pane

• Declares ChannelsPaneViewModel as the root binding type and enables compiled bindings. It explicitly types the keyed channel-tile template as ChannelTileViewModel and both palette-related template scopes as IBrush, allowing all 81 bindings to be compiler-checked.

Daqifi.Avalonia/Daqifi.Desktop/View/Prototype/ChannelsPanePrototype.axaml

Tests (1) +28 / -0
ChannelsPanePrototypeBindingTests.csGuard Channels pane compiled-binding configuration +28/-0

Guard Channels pane compiled-binding configuration

• Adds parsed-XAML tests verifying the root ChannelsPaneViewModel data type and compiled-binding flag. It also prevents nested escape hatches from silently restoring reflection bindings.

Daqifi.Avalonia.Tests/Views/ChannelsPanePrototypeBindingTests.cs

@qodo-code-review

qodo-code-review Bot commented Sep 18, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can type 'qodo, fix this' on a finding and the fix lands right on your PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Previous reviews

Review updated until commit 4b13ea8 ⚖️ Balanced

Results up to commit 4b13ea8 ⚖️ Balanced


No changes from previous review

Grey Divider

Qodo Logo

@tylerkron

Copy link
Copy Markdown
Contributor Author

/agentic_review

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 4b13ea8

@tylerkron

Copy link
Copy Markdown
Contributor Author

Qodo-clean, CI green — ready for review. Two Qodo rounds on head 4b13ea8, both graded Balanced with 0 findings. Each was confirmed to have fired after its trigger, and after each I waited 3+ minutes and re-checked: 0 Qodo threads. CI run 35394779286 on that same SHA is green on all heads, including Android and iOS. Not merging.

@tylerkron

Copy link
Copy Markdown
Contributor Author

/agentic_review

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 4b13ea8

@tylerkron

Copy link
Copy Markdown
Contributor Author

N-way merge check: all seven open #327-family PRs together (built locally from the #394 shepherd, nothing pushed)

Set: main 642caef + #387 (259a829) + #389 (808752a) + #390 (a7c89ec) + #391 (4370e80) + #392 (01764b9) + #393 (f1599e0) + #394 (4b13ea8).

@tylerkron

Copy link
Copy Markdown
Contributor Author

Shepherd: Qodo-clean, CI green, ready for review. CI run 35394779286 is green on 4b13ea8, which is the current head (it checked out Merge 4b13ea8 into 642caef, today's main). I fired an independent round at 21:23:10Z on that unchanged head. It ran rather than being refused: the summary updated at 21:25:22Z, anchored on 4b13ea8, with 0 bugs, 0 rule violations and 0 requirement gaps. After a 200 s settle I re-checked both surfaces: still 0, and 0 qodo threads, with the author filter validated as 3 on control #196. The standing grade is Balanced, from the builder's round on this head. The new render shows no grade of its own. I also checked by hand: the host route and DataContext for each scope (MainWindow's tab hands DaqifiViewModel and OnLoaded swaps in ChannelsPaneViewModel), and the tile, palette and swatch template types against the VM (ObservableCollection x3, IBrush[] Palette, ObservableCollection device names). All 5 undeclared ControlTemplates carry no {Binding}. Parsing the XAML gives 81 bindings (72 markup + 9 element), matching the body. I pushed nothing and edited nothing in the body.

@tylerkron

Copy link
Copy Markdown
Contributor Author

9-way merge build, 2026-09-18 (sweep via #396's shepherd). I merged main 642caef with #387 259a829, #389 808752a, #390 a7c89ec, #391 4370e80, #392 01764b9, #393 f1599e0, #394 4b13ea8, #395 9a0bfa4 and #396 8f4f29e on a throwaway branch. Nothing was pushed.

@tylerkron

Copy link
Copy Markdown
Contributor Author

/agentic_review

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 4b13ea8

@tylerkron

Copy link
Copy Markdown
Contributor Author

Sweep (13-way merge build): all 13 PRs are clean together on main 642caef

Covered: #387 259a829, #389 808752a, #390 a7c89ec, #391 4370e80, #392 01764b9, #393 f1599e0, #394 4b13ea8, #395 9a0bfa4, #396 8f4f29e, #398 8e2bb7e, #399 df20ae1, #400 c810ddf, #401 998da59.

Per PR: every head matches its ready note. Every PR is MERGEABLE/CLEAN, and its Build run on that exact SHA passed. No PR has an unresolved qodo review thread. I validated the thread-author filter against #196 first: it finds 3 qodo threads there, as expected. None of the green runs are stale: the last workflow change on main is 94fcac3 (09-08), older than all of them.

Merge build: I merged 642caef ← 387, 389–396 first, as a checkpoint. It produced tree 94b85fe, the same tree the previous sweep reported, so that result now holds up. Adding #398, #399, #400 and #401 gave tree b0f2033. Merging all 13 in reverse order gives the same b0f2033. Neither order had a text conflict.

  • Desktop build: exit 0.
  • iOS Release build (iossimulator-arm64, --no-restore): exit 0.
  • Tests: 1039/1039 passed. No lock-file drift.

Test count reconciles exactly: 993 on main + #387 0 + #389–#394 +3 each (+18) + #395 +3 + #396 +4 + #398 +13 + #399 0 + #400 +8 + #401 0 = 1039. Each delta comes from that PR's own CI total minus its base's total. For #387 that is 989 − 989 on 25df7b8.

Cross-PR hazards checked:

No merge was performed and nothing was pushed to any PR branch.

@tylerkron
tylerkron deleted the issue-327-channelspaneprototype-compiled-bindings branch September 19, 2026 18:35
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