fix(devices): show node display name in Node filter, not model (#1157)

[AustinChangLinksys]

Summary

The Node filter (device list page) showed the router model name (e.g. MR7500) instead of the user-assigned display name (e.g. Living Room). Fixes #1157.

Root cause

The filter options were built directly from meshTopology.nodes (DataElements), whose NodeEntity entries carry empty friendlyName/hostName — so displayName fell back to model. The friendlyName only lives on the fully-built mesh nodes (data.nodes), sourced from the Hosts table.

An earlier attempt to swap n.model → n.displayName in the view alone did not work, because the underlying data source still had no name to resolve.

Fix

• Introduce a dedicated NodeFilterOption { id, label } view model — mirroring the other filter dimensions (ssids/bands) so the view never needs to know how a node name is derived.
• In deviceFilterOptionsProvider, resolve the label by matching meshTopology.nodes[i].deviceId back to the built node's displayName via dataElementsId, falling back to model when no display name exists.
• The selection key stays the DataElements node id (NodeFilterOption.id == meshTopology.nodes[i].deviceId), which is exactly what ClientDevice.parentNodeId matches against. Only the display label changed — filtering behavior is unchanged.

Testing

• flutter analyze on the 4 changed files — no issues.
• fvm dart format — no changes needed.
• Added 2 provider tests:
  • label resolves to friendlyName while id stays the DataElements node id.
  • label falls back to model when no display name exists.
• Full functional suite (run_tests.sh) — 3332 tests passed.

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[HankYuLinksys]

Code Review

Reviewed against the PR head (ba65c27). The fix is correct and well-reasoned — root-cause analysis is accurate and the implementation holds up under tracing.

✅ Core logic verified

1. Root cause is correct. meshTopology.nodes (DataElements) carry empty friendlyName/hostName, so displayName falls back to model. The real name lives on the fully-built nodes (data.nodes, sourced from the Hosts table).

2. The dataElementsId back-mapping actually holds. In mesh_network_builder.dart, _buildMasterNode / _buildSlaveNode set dataElementsId: masterMeshInfo?.deviceId / slaveMeshInfo?.deviceId. So a built node's dataElementsId is the corresponding meshTopology.nodes[].deviceId — the labelByDataElementsId[n.deviceId] lookup keys off the right value, not a coincidence.

3. Selection key unchanged, filtering behavior unchanged. NodeFilterOption.id == meshTopology.nodes[].deviceId, which is what device filtering matches against via device.parentNodeId. _reconcile was also updated from n.deviceId to n.id consistently. Only the display label changed.

4. Type-change blast radius is clean. DeviceFilterOptions.nodes changed from List<NodeEntity> to List<NodeFilterOption>; all consumers (the 6 panel usages, provider reconcile, chip bar) migrated to .id/.label. The golden mock uses default empty options and state_test doesn't build .nodes, so neither breaks.

5. Tests are meaningful. The two new tests cover both paths — label resolves to friendlyName while id stays the DataElements id, and label falls back to model when no display name exists. The fixture sets dataElementsId on the built nodes to exercise the back-mapping.

🟢 Informational (non-blocking)

• NodeFilterOption design is the right call. A dedicated view model that lets the provider resolve the name once — mirroring ssids/bands — is more correct than the earlier attempt of swapping n.model → n.displayName in the view alone, which couldn't work because the data source had no name to resolve.
• Dangling doc reference. device_filter_state.dart dropped the node_entity.dart import but the doc comment still references [NodeEntity], so that dartdoc link no longer resolves. Harmless (the comment_references lint isn't enabled, so it doesn't affect analyze/compile) — optional cleanup.
• Duplicate dataElementsId. labelByDataElementsId is a map, so if two built nodes shared a dataElementsId the later one would win. DataElements ids should be unique, and even on collision the ?? n.displayName fallback keeps it safe — just noting.

LGTM.

[HankYuLinksys]

LGTM. Verified against PR head — root cause is correct, the dataElementsId back-mapping holds in the builder, the selection key is unchanged so filtering behavior is preserved, and the type change is fully migrated across all consumers. Tests cover both the friendlyName and model-fallback paths.

[AustinChangLinksys]

🤖 Automated Review — Round 1 · c45ee12..ba65c27 (full)

Verdict: 💬 Self-review (comment only) — Austin's own PR; automated review provided for reference only (no self-approve). 0 Critical · 2 Warning · 4 Suggestion.

Standards (Reviewer A — Security/Correctness + Reviewer B — Architecture/Maintainability)

	Conf.	Where	Issue (one-liner)
⚠️	🟡Med	device_filter_provider.dart:244	Cross-map key contract not type-enforced: if dataElementsId is absent on any data.nodes entry, lookup silently misses and falls back to meshTopology node's displayName (= model) — silently re-introducing #1157
⚠️	🟡Med	test/.../device_filter_provider_test.dart	No test covers the dataElementsId == null partial-miss case (some nodes enriched, some not) — silent degradation path untested
💡	🟢High	usp_device_filter_panel.dart:132–141	[Duplicated Code] options.nodes.map((n) => n.id).toList() computed twice in same build; extract to local final nodeIds
💡	🟡Med	device_filter_state.dart:120	NodeFilterOption (view model) lives in device_filter_state.dart (provider-state file); consider moving to _shared/models/ for cleaner layering (no 3-layer violation, just clarity)

Confidence: 🟢High = code-verified · 🟡Med = located + reasoned, not fully confirmed · ⚪Low = speculative, please double-check.
[X] = 1 agent · [XX] = 2 agents flagged independently → higher confidence (Critical only).

Spec (#1157 — "Connected via/to should show node name, not model")

	Conf.	Where	Issue (one-liner)
⚠️	🟡Med	usp_device_detail_view.dart:252, usp_device_list_tile.dart:183	Spec step: "Select a client" path — device detail/list-tile "Connected to/via" uses device.parentNodeName which has its own fallback-to-model in MeshNetworkBuilder; PR doesn't touch this path, may still show model on certain firmware
💡	🟡Med	usp_node_detail_view.dart:316	Topology "Connected to" shows parentNode.model directly — not in spec steps but spec title says broadly "Connected via/to info"
💡	⚪Low	device_filter_state.dart	Scope creep (minor): new NodeFilterOption model is an API-surface change not required by spec; benign given 3332 tests passing

Confidence: 🟢High = code-verified · 🟡Med = located + reasoned, not fully confirmed · ⚪Low = speculative, please double-check.

🔴 Critical findings — none

No Critical issues found in this round.

⚠️ Warning details

W-1 · 🟡Med — Silent dataElementsId miss in cross-map lookup
lib/page/devices/providers/device_filter_provider.dart:237–244

final labelByDataElementsId = {
  for (final n in data.nodes)
    if (n.dataElementsId != null) n.dataElementsId!: n.displayName,
};
final nodeOptions = data.meshTopology.nodes
    .map((n) => NodeFilterOption(
          id: n.deviceId,
          label: labelByDataElementsId[n.deviceId] ?? n.displayName,  // fallback
        ))
    .toList();

The mapping relies on the invariant that meshTopology.nodes[i].deviceId == data.nodes[j].dataElementsId. If any data.nodes entry has dataElementsId == null (filtered out of the map at line 239), the lookup at line 244 returns null and falls back to n.displayName on the meshTopology.nodes entry — which only has model, not friendlyName. This silently re-introduces #1157 for that node with no error signal.

No evidence this currently happens in production (the PR + tests show the invariant holds for supported firmware). Recommend adding a debug-mode assertion:

assert(
  data.meshTopology.nodes.every((n) => labelByDataElementsId.containsKey(n.deviceId)),
  'Some topology nodes have no enriched counterpart — label will fall back to model.',
);

Fix: add assertion in debug builds; optionally log a warning in release.

---

W-2 · 🟡Med — Missing test for partial-miss degradation path
test/page/devices/providers/device_filter_provider_test.dart

Existing tests cover:

• ✅ All nodes: dataElementsId set → labels resolve to friendlyName
• ✅ All nodes: no dataElementsId → labels fall back to model

Missing:

• ❌ Mixed: node A has dataElementsId set, node B has dataElementsId == null — the per-node fallback behavior is not independently verified.

---

W-3 (Spec) · 🟡Med — "Select a client" detail/list-tile path not verified
lib/page/devices/views/usp_device_detail_view.dart:252, lib/page/devices/views/components/usp_device_list_tile.dart:183

These display device.parentNodeName, assigned in MeshNetworkBuilder._buildClientDevice() via a MAC-embedded Hosts↔DataElements cross-reference. If that lookup fails (firmware variation), the builder falls back to matchingNode?.model. This PR does not touch that path. PR description says it was already correct; worth a manual confirmation on 3-node hardware given the spec reproduces on specific firmware.

✅ What looks good

• Correct root-cause fix: deviceFilterOptionsProvider previously fed raw meshTopology.nodes (DataElements, no friendlyName) directly to the filter; now resolves labels via data.nodes (Hosts-enriched). The data-flow logic is sound.
• Key identity preserved: filtering still uses NodeFilterOption.id (= DataElements node ID = ClientDevice.parentNodeId), so filter selection/reconciliation logic is unchanged.
• View properly decoupled: usp_device_filter_panel.dart now only knows {id, label} — consistent with ssids/bands pattern; removed leaked NodeEntity dependency from view.
• Fallback handled: ?? n.displayName gracefully covers nodes with no enriched match.
• Tests well-structured: the two new provider tests explicitly document the dataElementsId-based cross-reference invariant and the model-name fallback; full DevicesData fixture is written out (not stubbed).
• 3332 tests passing, flutter analyze clean.

Cross-reviewed by two independent agents (security+correctness / architecture+maintainability). Automated — please sanity-check before merge.