refactor(telemetry): unify localStats/host/trafficManagement onto buildCanonicalMetrics (#3515) by Yeraze · Pull Request #3518 · Yeraze/meshmonitor

Yeraze · 2026-06-17T11:28:51Z

Summary

Closes #3515. Follow-up to #3506 — converts the last three serial telemetry branches (localStats, hostMetrics, trafficManagementStats) from hand-maintained { type, value, unit } lists onto the shared buildCanonicalMetrics() path, so all seven metric groups now go through one normalizer.

Not a bug fix — a consistency/maintainability item (these three have no _<digit> fields). From the #3514 review.

How the three differing conventions are preserved

Group	Stored as	Mechanism
`localStats`	bare leaf (`uptimeSeconds`, `heapFreeBytes`)	joins `STRIP_GROUPS`
`host`	`host`+PascalCase (`hostUptimeSeconds`, `hostLoad1`)	new `PREFIX_GROUPS`
`trafficManagement`	`tm`+PascalCase (`tmPacketsInspected`)	new `PREFIX_GROUPS`

Their type→unit entries are added to CANONICAL_TELEMETRY_UNITS (new LOCAL_STATS_UNITS / HOST_UNITS / TRAFFIC_MGMT_UNITS sub-maps) — required before converting, since buildCanonicalMetrics only emits leaves with a known unit (the parity caveat from the issue).

Safety

MQTT unaffected: mqttIngestion.ts only processes device/environment/airQuality/power/health — it never calls canonicalTelemetryType with these three group names, so extending the normalizer for them is serial-only.
Side-effect preserved: the localStats checkAutoHeapManagement(heapFreeBytes) call is kept.
Behavior-preserving: parity tests assert each branch stores the same types/units/values as the old static lists, including genuine 0s.

Tests

telemetryKeys.test.ts: STRIP for localStats, PREFIX for host/tm, and units for all new canonical types.
meshtasticManager.telemetryCanonical.test.ts: end-to-end parity for all three branches.

Full suite: 6576 passed, 0 failed.

🤖 Generated with Claude Code

…ldCanonicalMetrics (#3515) Follow-up to #3506: convert the last three serial telemetry branches (localStats, hostMetrics, trafficManagementStats) from hand-maintained { type, value, unit } lists onto the shared buildCanonicalMetrics() path, so all seven metric groups go through one normalizer. These three use distinct naming conventions, so canonicalTelemetryType is extended to reproduce them exactly: - localStats joins STRIP_GROUPS → bare leaf names (uptimeSeconds, heapFreeBytes…) - host/trafficManagement use a new PREFIX_GROUPS map → host/tm + PascalCase leaf (uptimeSeconds → hostUptimeSeconds, packetsInspected → tmPacketsInspected) Their type→unit entries are added to CANONICAL_TELEMETRY_UNITS (new LOCAL_STATS_UNITS / HOST_UNITS / TRAFFIC_MGMT_UNITS sub-maps), which is required before converting — buildCanonicalMetrics only emits leaves with a known unit. MQTT ingestion only processes device/environment/airQuality/power/health, so extending canonicalTelemetryType for these serial-only groups does not affect MQTT. The localStats checkAutoHeapManagement side-effect is preserved. Behavior-preserving; adds parity tests asserting each branch stores the same types/units/values as before (incl. genuine 0s). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

github-actions · 2026-06-17T11:29:13Z

Claude finished @Yeraze's task —— View job

PR Review: refactor(telemetry): unify localStats/host/trafficManagement onto buildCanonicalMetrics

This is a clean, well-scoped refactor. The approach is sound and the PR achieves its goal of unifying all seven telemetry groups onto buildCanonicalMetrics. Here are my observations:

Positive

Consistent pattern: The three newly-converted branches (localStats, host, trafficManagement) now match the same 3-line calling convention as the four that were already converted. The code is noticeably simpler at the call site.
Design table in the PR description clearly explains how the three different naming conventions are preserved — this is good documentation for future contributors.
Side-effects preserved: The checkAutoHeapManagement(localStats.heapFreeBytes, fromNum) call at meshtasticManager.ts:6526 is correctly kept after the saveTelemetryMetrics call.
Parity tests for all three branches cover genuine 0 values, which is the classic footgun when 0 is falsy — that's the right thing to pin.

Minor observations

telemetryKeys.ts:164-167 — PREFIX_GROUPS is partially self-documenting but the key/value symmetry for host could confuse readers:

const PREFIX_GROUPS: Record<string, string> = {
  host: 'host',           // group name = prefix
  trafficManagement: 'tm', // group name ≠ prefix
};

The existing comment covers this, so it's not a blocker, but a brief inline note on the host: 'host' line would make the asymmetry clearer at a glance.

telemetryKeys.ts:97-110 — LOCAL_STATS_UNITS omits a few fields that DEVICE_UNITS already covers.

The comment on line 95-96 correctly notes that uptimeSeconds, channelUtilization, and airUtilTx are covered by DEVICE_UNITS. Those fields arriving from a localStats packet will still resolve a unit because buildCanonicalMetrics calls canonicalTelemetryUnit(type) and the canonical type is the bare leaf name (same as DEVICE_UNITS), so lookup succeeds. This is correct. However, numPacketsRxBad and numTxDropped aren't tested in the parity test — they're unit-tested via canonicalTelemetryUnit in telemetryKeys.test.ts:78-87, but not exercised end-to-end. Not a bug, but a note for completeness.

meshtasticManager.ts:6513 — telemetry.localStats vs telemetry.local_stats asymmetry

At the packet-log preview path (meshtasticManager.ts:5221), both telemetry.localStats || telemetry.local_stats are checked. But in processTelemetryMessageProtobuf (line 6513), only telemetry.localStats is checked. This is unchanged from before this PR — the old code had the same asymmetry — so it's out of scope here, but worth noting as a pre-existing inconsistency.

meshtasticManager.telemetryCanonical.test.ts:113 — comment typo:

humidity: -Infinity, // relativeHumidity canonical is 'humidity'; this is an unknown leaf anyway

humidity isn't an unknown leaf — it's the canonical form of relativeHumidity, and ENVIRONMENT_UNITS has it. The comment is misleading; the field being passed here as key humidity is NOT the same as the protobuf field relativeHumidity, so it won't hit the rename and won't be in units. The test still passes because humidity as a raw key is not in ENVIRONMENT_UNITS directly (it would need to come in as relativeHumidity). The behavior is correct — the comment should just be cleaner.

Summary

No blocking issues. The refactor correctly unifies the three serial telemetry branches, maintains behavioral parity, and the test coverage is solid. The minor observations above are all non-blocking nits or pre-existing issues outside the PR's scope.

…s fields (review #3518) - Correct the NaN/Infinity test comment: humidity IS a known canonical type, excluded by the Number.isFinite guard, not for being unknown. - Note that host's prefix coincidentally equals its group name. - Exercise numPacketsRxBad / numTxDropped end-to-end in the localStats parity test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

github-actions · 2026-06-17T11:55:12Z

Claude finished @Yeraze's task —— View job

PR Review: refactor(telemetry): unify localStats/host/trafficManagement onto buildCanonicalMetrics

This is a clean, well-scoped refactor. The approach is sound and the PR achieves its goal of unifying all seven telemetry groups onto buildCanonicalMetrics. Here are my observations:

Positive

Consistent pattern: The three newly-converted branches (localStats, host, trafficManagement) now match the same 3-line calling convention as the four that were already converted. The code is noticeably simpler at the call site.
Design table in the PR description clearly explains how the three different naming conventions are preserved — this is good documentation for future contributors.
Side-effects preserved: The checkAutoHeapManagement(localStats.heapFreeBytes, fromNum) call at meshtasticManager.ts:6526 is correctly kept after the saveTelemetryMetrics call.
Parity tests for all three branches cover genuine 0 values, which is the classic footgun when 0 is falsy — that's the right thing to pin.

Minor observations

telemetryKeys.ts:164-167 — PREFIX_GROUPS is partially self-documenting but the key/value symmetry for host could confuse readers:

const PREFIX_GROUPS: Record<string, string> = {
  host: 'host',           // group name = prefix
  trafficManagement: 'tm', // group name ≠ prefix
};

The existing comment covers this, so it's not a blocker, but a brief inline note on the host: 'host' line would make the asymmetry clearer at a glance.

telemetryKeys.ts:97-110 — LOCAL_STATS_UNITS omits a few fields that DEVICE_UNITS already covers.

The comment on line 95-96 correctly notes that uptimeSeconds, channelUtilization, and airUtilTx are covered by DEVICE_UNITS. Those fields arriving from a localStats packet will still resolve a unit because buildCanonicalMetrics calls canonicalTelemetryUnit(type) and the canonical type is the bare leaf name (same as DEVICE_UNITS), so lookup succeeds. This is correct. However, numPacketsRxBad and numTxDropped aren't tested in the parity test — they're unit-tested via canonicalTelemetryUnit in telemetryKeys.test.ts:78-87, but not exercised end-to-end. Not a bug, but a note for completeness.

meshtasticManager.ts:6513 — telemetry.localStats vs telemetry.local_stats asymmetry

At the packet-log preview path (meshtasticManager.ts:5221), both telemetry.localStats || telemetry.local_stats are checked. But in processTelemetryMessageProtobuf (line 6513), only telemetry.localStats is checked. This is unchanged from before this PR — the old code had the same asymmetry — so it's out of scope here, but worth noting as a pre-existing inconsistency.

meshtasticManager.telemetryCanonical.test.ts:113 — comment could be clearer:

humidity: -Infinity, // relativeHumidity canonical is 'humidity'; this is an unknown leaf anyway

The comment says humidity is "unknown" but that's only because the key arrives here as the raw string 'humidity' (not via the group 'environment' rename path for relativeHumidity). The canonical type humidity is in ENVIRONMENT_UNITS — the leaf is excluded purely by Number.isFinite(-Infinity) returning false, not because of an unknown unit. A small comment clarification would avoid confusion.

Summary

No blocking issues. The refactor correctly unifies the three serial telemetry branches, maintains behavioral parity, and the test coverage is solid. The minor observations above are all non-blocking nits or pre-existing issues outside the PR's scope.

Yeraze merged commit 2105574 into main Jun 17, 2026
20 checks passed

Yeraze deleted the fix/3515-canonical-localstats-host-tm branch June 17, 2026 12:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor(telemetry): unify localStats/host/trafficManagement onto buildCanonicalMetrics (#3515)#3518

refactor(telemetry): unify localStats/host/trafficManagement onto buildCanonicalMetrics (#3515)#3518
Yeraze merged 2 commits into
mainfrom
fix/3515-canonical-localstats-host-tm

Yeraze commented Jun 17, 2026

Uh oh!

github-actions Bot commented Jun 17, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jun 17, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

Yeraze commented Jun 17, 2026

Summary

How the three differing conventions are preserved

Safety

Tests

Uh oh!

github-actions Bot commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review: refactor(telemetry): unify localStats/host/trafficManagement onto buildCanonicalMetrics

Positive

Minor observations

Summary

Uh oh!

github-actions Bot commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review: refactor(telemetry): unify localStats/host/trafficManagement onto buildCanonicalMetrics

Positive

Minor observations

Summary

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

github-actions Bot commented Jun 17, 2026 •

edited

Loading

github-actions Bot commented Jun 17, 2026 •

edited

Loading