Skip to content

fix(dashboard): stabilize Unified source node count (#2805)#2806

Merged
Yeraze merged 1 commit intomainfrom
fix/unified-source-node-count-2805
Apr 26, 2026
Merged

fix(dashboard): stabilize Unified source node count (#2805)#2806
Yeraze merged 1 commit intomainfrom
fix/unified-source-node-count-2805

Conversation

@Yeraze
Copy link
Copy Markdown
Owner

@Yeraze Yeraze commented Apr 26, 2026

Summary

Fixes #2805. The Unified source card showed an inconsistent node count that drifted depending on which source was currently selected.

The card was computing its count two different ways:

  • Selected: deduped merged count (unifiedSourceData.nodes.length).
  • Not selected: raw sum of per-source statusMap.nodeCount — over-counts any node present in multiple sources.

Plus, anonymous viewers always saw "disconnected" because the connection check fell back to a statusMap that's gated by sources:read.

Fix

  • New GET /api/unified/status returns { nodeCount, connected }.
    • nodeCount is COUNT(DISTINCT nodeNum) across the user's readable sources (permission-scoped).
    • connected reflects whether any source is currently up — not permission-scoped, matching the existing /api/unified/sources-status (operational signal, not user data).
  • New useUnifiedStatus() hook polls on the standard 15s dashboard cadence.
  • DashboardPage and DashboardSidebar consume this single source of truth for the Unified card. The previous live-merged length is kept only as a pre-poll fallback when Unified is selected.

Files

  • src/db/repositories/nodes.ts — added getDistinctNodeCount(sourceIds) using Drizzle countDistinct.
  • src/server/routes/unifiedRoutes.ts — added GET /status.
  • src/hooks/useDashboardData.ts — added useUnifiedStatus() + UnifiedStatus type.
  • src/components/Dashboard/DashboardSidebar.tsx — Unified dot prefers unifiedStatus.connected, falls back to statusMap.
  • src/pages/DashboardPage.tsx — single formula for Unified count, threads unifiedStatus to the sidebar.

Test plan

  • Unit: nodes.getDistinctNodeCount dedupes shared nodes across sources, handles empty list.
  • Route: /api/unified/status admin all-sources, regular-user permission filter, no-readable-sources case.
  • Hook + page tests updated to mock useUnifiedStatus.
  • All existing unifiedRoutes (42), useDashboardData (16), DashboardSidebar, and DashboardPage tests pass.
  • Manual: SQLite dev container at port 8081 — Unified card stays at deduped count regardless of selection; anonymous viewer sees "Connected" dot when at least one source is up.

🤖 Generated with Claude Code

The Unified source card used two different formulas for its node count: the
deduped merged count when Unified was selected, and a raw sum of per-source
counts when it was not. Sum over-counted nodes shared between sources, and
the fan-out hook is disabled when Unified is not the active source, so the
displayed value drifted as the user clicked between individual sources.

Add GET /api/unified/status returning a server-computed deduped count
(COUNT(DISTINCT nodeNum) across readable sources) plus an aggregate
connected flag. The dashboard now polls this on the same cadence as the
per-source statuses and uses it as a single source of truth for the Unified
card. The connected flag is computed across all sources (not permission
scoped) so anonymous viewers see the correct connection dot — same approach
as /api/unified/sources-status.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 26, 2026

Claude finished @Yeraze's task —— View job


Comprehensive Code Review

I've thoroughly reviewed this PR which fixes the inconsistent Unified source node count (#2805). The implementation is solid with good architectural decisions and comprehensive testing. Here's my detailed analysis:

Strengths

Database Layer (nodes.ts)

API Endpoint (unifiedRoutes.ts)

  • Clean separation of concerns: nodeCount is permission-scoped, connected is operational
  • Follows existing patterns in the unified routes file
  • Proper error handling with 500 status codes
  • Good permission checking with admin bypass logic
  • Comprehensive test coverage including admin vs regular user scenarios

React Integration

  • New useUnifiedStatus() hook follows existing hook patterns with consistent polling interval
  • Proper integration in DashboardPage.tsx with fallback logic for pre-poll state
  • DashboardSidebar.tsx correctly prioritizes unifiedStatus.connected with statusMap fallback
  • Test mocks properly updated to include the new hook

🔍 Technical Analysis

Performance

  • Database query is efficient using countDistinct() - single query vs N separate counts
  • Hook polling at 15s intervals matches existing dashboard cadence
  • No unnecessary API calls or data fetching

Security

  • Proper permission scoping for nodeCount (user can only see count for readable sources)
  • Connection status appropriately not permission-gated (operational signal)
  • Follows established auth patterns in the codebase

Maintainability

  • Clear code structure following existing conventions
  • Good error handling and logging
  • Comprehensive test coverage prevents regressions
  • Documentation explains the issue being solved

📋 Code Quality Assessment

Database Best Practices

  • Uses Drizzle ORM correctly for database-agnostic queries
  • Follows async naming convention (getDistinctNodeCount)
  • Proper BIGINT handling and type coercion
  • Consistent with repository architecture patterns

API Design

  • RESTful endpoint design (GET /api/unified/status)
  • Appropriate response structure with clear semantics
  • Consistent error handling patterns
  • Follows authentication middleware patterns

React/TypeScript

  • Proper TypeScript interfaces (UnifiedStatus)
  • Consistent hook patterns with TanStack Query
  • Good separation of concerns in components
  • Proper state management and fallback handling

Testing

  • Unit tests for database repository layer
  • Integration tests for API endpoints
  • React component test updates
  • Good test coverage of edge cases (empty sources, permission scenarios)

🎯 Architectural Alignment

This PR follows MeshMonitor's architectural principles:

  • Multi-database support: Uses Drizzle ORM for database agnosticism
  • Permission system: Proper resource scoping with admin bypass
  • Async patterns: All database methods follow async conventions
  • Testing requirements: Comprehensive test coverage maintained

🏆 Overall Assessment

This is a well-executed fix that:

  • Solves the core issue (inconsistent node count display) definitively
  • Uses the right architectural patterns and tools
  • Maintains backward compatibility
  • Includes proper testing to prevent regression
  • Follows established code conventions

Verdict: ✅ Ready to merge

The implementation correctly addresses issue #2805 with a clean, maintainable solution that integrates well with the existing codebase architecture.

@github-actions
Copy link
Copy Markdown

System Test Results

MeshMonitor System Test Results

Test Run: 2026-04-26 11:23:52 EDT

Test Summary

Test Suite Result
Configuration Import ✅ PASSED
Quick Start Test ✅ PASSED
Security Test ✅ PASSED
V1 API Test ✅ PASSED
Reverse Proxy Test ✅ PASSED
Reverse Proxy + OIDC ✅ PASSED
Virtual Node CLI Test ✅ PASSED
Backup & Restore Test ✅ PASSED
Database Migration Test ✅ PASSED
DB Backing Consistency ✅ PASSED
API Exercise (3 DBs) ✅ PASSED

✅ Overall Result: PASSED

All deployment configurations are working correctly!

Test Details

Configuration Import:

  • Tests configuration import and device reboot cycle
  • Verifies channel roles, PSKs, and LoRa configuration
  • Note: Channel name verification skipped due to architectural limitation

Quick Start Test:

  • Zero-config deployment (no SESSION_SECRET or COOKIE_SECURE required)
  • HTTP access without HSTS
  • Auto-generated admin user with default credentials
  • Session cookies work over HTTP
  • Meshtastic node connection and message exchange verified

Security Test:

  • Verifies Node IP address hidden from anonymous users in API responses
  • Verifies MQTT configuration hidden from anonymous users
  • Verifies Node IP address visible to authenticated users
  • Verifies MQTT configuration visible to authenticated users
  • Verifies protected endpoints require authentication

V1 API Test:

  • Tests v1 REST API endpoints with Bearer token authentication
  • Verifies Bearer token requests bypass CSRF protection
  • Verifies POST/PUT/DELETE work without CSRF token when using Bearer auth
  • Verifies session-based requests still require CSRF token

Reverse Proxy Test:

  • Production deployment with COOKIE_SECURE=true
  • HTTPS-ready configuration
  • Trust proxy enabled for reverse proxy compatibility
  • CORS configured for HTTPS domain
  • Meshtastic node connection and message exchange verified

Reverse Proxy + OIDC Test:

  • OIDC authentication integration
  • Mock OIDC provider health checks
  • Authorization flow and session creation
  • Hybrid mode (OIDC + local auth)
  • Meshtastic node connection verified

Virtual Node CLI Test:

  • Virtual Node Server enabled on TCP port 4404
  • Meshtastic Python client successfully connects
  • Node data download and synchronization verified
  • Test message sent on gauntlet channel (index 3)
  • Message delivery confirmed via Web UI API
  • Virtual Node Server connection logging verified

Backup & Restore Test:

  • System backup created from running dev container
  • New container spun up with RESTORE_FROM_BACKUP env var
  • Data integrity verified (node count, message count, settings)
  • Restore event logged in audit log
  • Dev container unaffected by restore test

Database Migration Test:

  • SQLite to PostgreSQL migration verified
  • SQLite to MySQL migration verified
  • Data integrity confirmed for both target databases
  • Row counts match between source and target

DB Backing Consistency Test:

  • SQLite, PostgreSQL, and MySQL backends tested with same device
  • Node counts within ±10 across all three backends
  • Favorite counts identical across all backends
  • Key station verified as favorite on all backends

@Yeraze Yeraze merged commit 1c74cb9 into main Apr 26, 2026
20 checks passed
@Yeraze Yeraze deleted the fix/unified-source-node-count-2805 branch April 26, 2026 20:28
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.

[BUG] Unified source node count changes incorrectly based on selected source

1 participant