Skip to content

fix(#327): ChannelsMobileView's 76 bindings were all on reflection — two declared templates, three cast hops, and a drawer label bound to a member IChannel never had - #392

Merged
tylerkron merged 2 commits into
mainfrom
issue-327-channelsmobileview
Sep 19, 2026
Merged

tylerkron merged 2 commits into
mainfrom
issue-327-channelsmobileview

Conversation

@tylerkron

@tylerkron tylerkron commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

refs #327 — not merging, for review.

What was wrong

All 76 bindings in ChannelsMobileView.axaml (the mobile Channels tab) resolved by reflection. If a member on ChannelsPaneViewModel, ChannelTileViewModel or IChannel were renamed or removed, the tile, the settings card or its error label would just go blank, and desktop and iOS would still build green.

Turning compile checking on found one binding that only worked because reflection doesn't check types. The scaling-error label binds SelectedChannel.ScaleExpressionError (#311). SelectedChannel is typed IChannel, and that member was declared only on AbstractChannel. It worked at runtime because every channel is an AbstractChannel. Against the declared type, the compiler rejects it (AVLN2000 … 'ScaleExpressionError' on type 'IChannel').

This was not a user-visible bug on main, and this PR does not fix one. Reflection resolves the path on the runtime object, and every concrete IChannel in the repo (AnalogChannel, DigitalChannel, and the four test doubles) derives from AbstractChannel, which declares the member. AbstractChannel is the only direct implementer of IChannel. A shepherd rendered the view with two channels on both main 642caef and this branch, opened the drawer through the tile's own gear button, and entered a 40-deep parenthesised expression. Both builds showed TOO MANY NESTED PARENTHESES (LIMIT 8) in the label, and the two PNGs are byte-identical. The interface change is a contract correction that compile checking needs. It is not a behaviour fix.

The fix

The runtime DataContext for each scope was traced, not guessed. MobileMainView.OnChannels is the only place that constructs the view: new ChannelsMobileView { DataContext = new ChannelsPaneViewModel() }. Nothing else in the repo creates it, and the capture harness gets to it through that same route.

Scope Declaration Why
root x:DataType="vm:ChannelsPaneViewModel" + x:CompileBindings="True" the only construction site hands it a ChannelsPaneViewModel. CompileBindings is needed because AvaloniaUseCompiledBindingsByDefault is false (#326)
ChannelTileTemplate (resource) x:DataType="vm:ChannelTileViewModel" a resource-dictionary template infers no type (rule b). All three lists that use it (AnalogInputs, DigitalInputs, DigitalOutputs) are ObservableCollection<ChannelTileViewModel>
└ its 3 $parent[ItemsControl] hops ((vm:ChannelsPaneViewModel)DataContext) cast in a resource template the compiler can't know which ItemsControl the tile will be placed in, so it types DataContext as System.Object (measured: 3 × AVLN2000). The traversal itself is unchanged: $parent walks the logical tree, and the nearest ItemsControl is still the section list whose DataContext is the pane VM
device-name ItemTemplate none inferred string from ConnectedDeviceNames (measured)
palette ItemTemplate x:DataType="IBrush" an array ItemsSource infers no type. Palette is IBrush[] (declared Brush[] under the file's using Brush = Avalonia.Media.IBrush alias, and holding ImmutableSolidColorBrush values at runtime), and a member path in that template is AVLN2000 … on type 'XamlPseudoType' (measured). Its two bindings are path-less {Binding}, so they compiled without the declaration, but only because they had nothing to check. IBrush is the element type exactly. Its $parent[ItemsControl].DataContext hop resolves against the root scope without a cast

IChannel gains string ScaleExpressionError { get; }. AbstractChannel is the only implementer, and its [ObservableProperty] already generates a public get/set that satisfies the getter-only member, so runtime behaviour is unchanged. The alternative was a ((AbstractChannel)SelectedChannel) cast in the binding. It would also compile, but it ties the view to the base class, and the desktop drawer (ChannelsPanePrototype.axaml:807, still on reflection) binds the same path and would need the same cast when it is converted. Declaring the member on the interface fixes both. The test that pins the label binding text on both views (ChannelScalingExpressionTests.The_scaling_error_label_shows_the_reason_on_both_heads) still passes unchanged; only its comment was updated.

No escape hatches: no x:CompileBindings="False" subtree and no {ReflectionBinding} in any spelling. A test guards this.

Verification

Positive control, one build per side (fresh edits each time, exit codes read, no re-runs trusted):

  • Branch: one build with a typo in each of 7 scopes gave exactly 7 × AVLN2000, each naming the right type: ChannelsPaneViewModel (root DeviceNameZZ), ChannelTileViewModel (TypeLabelZZ), ChannelsPaneViewModel (tile cast hop IsLoggingActiveZZ), System.String (device-name template), Avalonia.Media.IBrush (palette template), ChannelsPaneViewModel (palette $parent hop), IChannel (SelectedChannel.ScaleExpressionErrorZZ).
  • main: one build with a bogus property BogusPropertyZZ="1" and a binding typo {Binding TypeLabelZZ} on the same element gave exactly 1 error, the property (AVLN2000 … BogusPropertyZZ on type TextBlock), and nothing for the typo. This shows the compiler read the file and skipped the binding, rather than that no build ran.

Gate: desktop build, iOS restore, and iOS Release iossimulator-arm64 build all exit 0. dotnet test: 996 passed, 0 failed.

Capture baseline: mobile-portrait-2-channels and mobile-landscape-2-channels are captured screens. ./run.sh --check-baseline, single run: baseline OK, 39/39 screens (the manifest on origin/main has 39 lines). Limitation: both channels captures show the no-device empty state, so the tile template, the palette and the settings card compile but no capture renders them. The empty-state gate (!HasConnectedDevice) and the root scope are what the pixels confirm. (Shepherd, later: this limitation is now covered by a separate render. A temporary HeadlessBench probe, since reverted, connected a ScriptedDevice with two analog inputs through ConnectionManager.Connect. It built the view the way MobileMainView.OnChannels does and drove it through the tile controls. Both tiles rendered. The gear's cast hop opened the drawer. The toggle's cast hop flipped IsActive from false to true. All 8 palette swatches rendered through the IBrush template, and the error label showed the specific reason. The PNG is attached in a comment.)

Warnings (from CI logs, both ends): delta 0. Read from CI’s own logs at both ends, main 642caef (run 35377977069, this branch’s merge base) against this branch 01764b9 (run 35388501986). Printed all-project totals are identical on every head that prints one: iOS 170/170, Android 175/175, Desktop-on-macOS 169/169. The de-duplicated warning sets extracted from all four build jobs (Library+Desktop, iOS, Android, macOS) are identical, with nothing added or removed.

Test: ChannelsMobileViewBindingTests uses only the existing BindingFacts helpers: AssertRootDeclares ×2 and AssertNoEscapeHatch. The compiler already enforces the tile template's declaration: drop it and the build fails (XamlPseudoType on the first member path). Dropping the palette's declaration would still build, because its bindings have no path. That is the case the declaration covers, and it is not tested (no existing helper states it, and #387 owns BindingFacts.cs).

Scope

One view (#327 is a checklist). BindingFacts.cs is untouched (#387 edits it). Diff: 4 files, +68/−13 (--numstat: view +30/−11, test +28/−0, IChannel +7/−0, scaling-test comment +3/−2).

🤖 Generated with Claude Code

tylerkron and others added 2 commits September 18, 2026 09:46
…one declared root, one declared resource template, and a drawer label bound to a member IChannel never had

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… infers nothing

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-check ChannelsMobileView bindings and fix IChannel contract

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Compile all mobile Channels bindings with explicit root and template data types.
• Expose scaling errors through IChannel so typed drawer bindings validate.
• Add regression coverage preventing reflection-binding escape hatches.
Diagram

graph TD
  Main["Mobile Main"] -->|"creates"| Pane["Channels Pane VM"] -->|"DataContext"| View["Mobile Channels View"] -->|"compile checks"| Compiler["XAML Compiler"]
  View -->|"typed template"| Tile["Channel Tile VM"]
  View -->|"typed palette"| Brush["IBrush"]
  Pane -->|"selected channel"| Contract["IChannel Contract"] -->|"error property"| View
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Enable compiled bindings project-wide

Recommendation: Keep the PR's scoped approach. Explicit data types on the root and non-inferable templates provide compile-time safety for this view without creating a project-wide migration, while adding ScaleExpressionError to IChannel correctly aligns the declared contract with existing runtime behavior.

Files changed (4) +68 / -13

Bug fix (2) +37 / -11
IChannel.csExpose scaling-expression errors through IChannel +7/-0

Expose scaling-expression errors through IChannel

• Adds the existing ScaleExpressionError property to the channel interface so bindings through the SelectedChannel interface type compile correctly. This aligns the declared contract with all current AbstractChannel-backed runtime instances.

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

ChannelsMobileView.axamlCompile-check all ChannelsMobileView binding scopes +30/-11

Compile-check all ChannelsMobileView binding scopes

• Enables compiled bindings against ChannelsPaneViewModel and declares explicit ChannelTileViewModel and IBrush template types. Parent ItemsControl bindings now cast their otherwise object-typed DataContext back to the pane view model without changing logical-tree traversal.

Daqifi.Avalonia/Views/Mobile/ChannelsMobileView.axaml

Tests (2) +31 / -2
ChannelScalingExpressionTests.csClarify compiled coverage for the mobile scaling-error binding +3/-2

Clarify compiled coverage for the mobile scaling-error binding

• Updates the dual-head binding test commentary to distinguish the desktop reflection binding from the newly compiled mobile binding. The existing assertion continues preventing the two views from drifting.

Daqifi.Avalonia.Tests/Channels/ChannelScalingExpressionTests.cs

ChannelsMobileViewBindingTests.csGuard the mobile view's compiled-binding declarations +28/-0

Guard the mobile view's compiled-binding declarations

• Adds regression tests requiring the root ChannelsPaneViewModel data type, explicit compiled bindings, and the absence of reflection-binding escape hatches.

Daqifi.Avalonia.Tests/Views/ChannelsMobileViewBindingTests.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 01764b9 ⚖️ Balanced

Results up to commit 01764b9 ⚖️ 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 01764b9

@tylerkron

Copy link
Copy Markdown
Contributor Author

Qodo-clean, CI green — ready for review. Head 01764b9: two Qodo rounds with 0 findings and 0 threads (round 1 graded Balanced, round 2 fired on the unchanged head and reported no changes). The settle re-check after the second round was also clean. All CI jobs are green on this same SHA (run 35388501986). One disclosed gap: the channels captures show only the no-device state, so the tile, palette and settings-card bindings are compile-checked but have not been rendered.

@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 01764b9

@tylerkron

Copy link
Copy Markdown
Contributor Author

Qodo-clean, CI green, ready for review (shepherd). CI's green covers head 01764b9, the current head, in run 35388501986 (all jobs). The shepherd pushed no commits, and the body edit below creates none.

Review. I fired a third /agentic_review on the unchanged head at 20:12:40Z. The summary re-rendered at 20:14:10Z, anchored to 01764b9, with "no material issues" and 0 bugs, 0 rule violations and 0 gaps. It shows no new grade line. The standing graded review is round 1's ⚖️ Balanced on this same SHA. After the settle wait (4+ min), both surfaces were rechecked on the same head: the summary had not changed, and there are 0 review threads. The thread query was checked against #196, where it found 3 qodo threads.

The "latent bug" question: no user-visible bug existed on main. AbstractChannel is the only direct implementer of IChannel. Every concrete channel (AnalogChannel, DigitalChannel, and the four test doubles) derives from it, so the reflection binding SelectedChannel.ScaleExpressionError always resolved. I checked the result, not just the reasoning. A temporary HeadlessBench probe (reverted, nothing committed) connected a ScriptedDevice with two analog inputs through ConnectionManager.Connect and built new ChannelsMobileView { DataContext = new ChannelsPaneViewModel() }. It opened the drawer through the tile's own gear button and entered (((…x…))) 40 deep. On both main 642caef and the merged tree carrying this PR, the label read TOO MANY NESTED PARENTHESES (LIMIT 8), and the two PNGs are byte-identical (sha256 9051793e…). So the IChannel member is a contract correction that compile checking needs, not a bug fix. The body now says so. Adding it to the interface is the right minimal change: a (AbstractChannel) cast in the binding would couple the view to the base class, and the desktop drawer binds the same path.

Hand review of the rest.

  • Tile template. The runtime DataContext is ChannelTileViewModel, observed on the rendered gear button. The gear hop opened the drawer (IsSettingsOpen=True, SelectedChannel is an AnalogChannel), and the toggle hop flipped IsActive from false to true. So the $parent[ItemsControl] casts land on the section list, whose DataContext is the pane VM.
  • Palette. Palette is Brush[] under using Brush = Avalonia.Media.IBrush, so its declared type is exactly IBrush[]. The values are ImmutableSolidColorBrush, and all 8 swatches render.
  • Other checks. The view has no ControlTemplate, so rule (e) does not apply. There are no escape hatches in any spelling. The binding count is 76, parsed with a comment-proof XML parse and matching on both sides.

Body corrections (no commit):

  • It now states plainly that main was not broken, with the render evidence.
  • It says why the interface member beats a cast.
  • It gives the palette's alias and runtime element type.
  • The "no capture renders the tiles, palette or drawer" limitation is now marked as covered by the render above.

The attached image is the branch render. main's render is byte-identical.

ChannelsMobileView with two channels: tiles, palette, drawer open, error label reading TOO MANY NESTED PARENTHESES (LIMIT 8)

@tylerkron

Copy link
Copy Markdown
Contributor Author

Merge build (shepherd, throwaway local branch, nothing pushed): main 642caef (checked with ls-remote) + #387 259a829 + #389 808752a + #391 4370e80 + #392 01764b9. The octopus merge applied cleanly with no conflicts. Desktop build, iOS restore, and iOS Release iossimulator-arm64 build all exited 0 under a build.2 lock. dotnet test: 1002 passed, 0 failed. That reconciles as 993 (main) + 0 (#387, which replaces a guard without changing the test count) + 3 (#389) + 3 (#391) + 3 (#392: The_view_declares_its_data_type ×2 plus The_view_opens_no_escape_hatch). The branch's own 996 = 993 + 3 agrees. #392 compiles against #387's reworked BindingFacts (AssertRootDeclares and AssertNoEscapeHatch still resolve). No interaction found.

tylerkron added a commit that referenced this pull request Sep 18, 2026
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

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

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

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.

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