Skip to content

fix: packet log millisecond timestamps and date column#2403

Merged
Yeraze merged 3 commits intomainfrom
fix/packet-monitor-sort-by-id
Mar 23, 2026
Merged

fix: packet log millisecond timestamps and date column#2403
Yeraze merged 3 commits intomainfrom
fix/packet-monitor-sort-by-id

Conversation

@Yeraze
Copy link
Owner

@Yeraze Yeraze commented Mar 23, 2026

Summary

Fixes packet monitor showing traceroute request/response in wrong order when both arrive within the same second, and improves timestamp display.

  • Root cause: timestamp column stored Unix seconds (Math.floor(Date.now() / 1000)), so same-second packets had identical timestamps and undefined sort order
  • Fix 1: Add id DESC as tiebreaker sort key (defense-in-depth)
  • Fix 2: Store Date.now() (milliseconds) for proper sub-second ordering
  • Fix 3: Add "Date" column to packet monitor (shows "Today" or short date)
  • Fix 4: Fix 12h time format — 12:09:55.979 PM instead of 12:09:55 PM.979
  • Backward compat: Frontend auto-detects old seconds vs new milliseconds data; API since parameter accepts either format

Closes #2364

Changes

File Change
src/server/meshtasticManager.ts Date.now() instead of Math.floor(Date.now() / 1000) at both write points
src/components/PacketMonitorPanel.tsx Date column, toMs() helper, fixed 12h ms formatting
src/server/routes/packetRoutes.ts normalizeSinceToMs() for since query param
src/server/routes/v1/packets.ts Same normalizeSinceToMs() helper
src/db/repositories/misc.ts id DESC tiebreaker (commit 1)
public/locales/en.json Added packet_monitor.column.date and packet_monitor.today keys

Test plan

  • npx vitest run — 3070 tests pass
  • npm run build — no TypeScript errors
  • Deploy on SQLite, trigger traceroute, verify correct order with sub-second precision
  • Verify old packets still display correct timestamps (not 1970)
  • Verify 12h format shows HH:MM:SS.mmm AM/PM
  • Verify "Today" shows for today's packets, short date for older

🤖 Generated with Claude Code

Packets within the same second appeared in wrong order because the
ORDER BY used timestamp DESC, created_at DESC — both in seconds.
Adding id DESC as tiebreaker guarantees insertion order (request
before response) when timestamps match.

Addresses #2364

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

github-actions bot commented Mar 23, 2026

Claude finished @Yeraze's task —— View job


PR Review Analysis

Todo List:

  • Read the changed file to understand the modification
  • Analyze code quality and best practices
  • Check for potential bugs or issues
  • Evaluate performance considerations
  • Review security concerns
  • Assess test coverage

Review Summary

This is a well-implemented fix for a subtle ordering issue in the packet monitor. The change replaces created_at DESC with id DESC as the tiebreaker in the ORDER BY clause when timestamps are identical.

Strengths

1. Correct Problem Identification

  • The issue was properly diagnosed: both timestamp and created_at have seconds precision, making ordering indeterminate for packets within the same second
  • The traceroute scenario (response appearing before request) is a perfect example of this problem

2. Optimal Solution

  • Using id DESC as the tiebreaker is the right approach since auto-increment IDs guarantee insertion order
  • The change is minimal and surgical (single line modification)
  • Maintains the primary sort by timestamp while fixing the secondary sort

3. Code Quality

  • The change follows existing patterns in the codebase
  • No breaking changes to the API
  • Consistent with database best practices

⚠️ Areas for Consideration

1. Test Coverage Gap src/db/repositories/misc.packetlog.test.ts:119-125
The existing test expects packets to be ordered by created_at DESC as the tiebreaker:

it('orders by timestamp DESC then created_at DESC', async () => {
  // Test expects packet_id 2 first due to higher created_at
  expect(packets[0].packet_id).toBe(2); 
  expect(packets[1].packet_id).toBe(1);
});

This test will now fail because the ordering changed from created_at DESC to id DESC. The test needs to be updated to reflect the new expected behavior.

2. Documentation
Consider updating the test description and any related documentation that mentions the ordering behavior.

🚀 Performance & Security

Performance: ✅ Positive Impact

  • id is the PRIMARY KEY, making ORDER BY id DESC more efficient than ORDER BY created_at DESC
  • Auto-increment IDs have better index locality than timestamp-based ordering

Security: ✅ No Concerns

  • No security implications from this change
  • Still using parameterized queries with Drizzle ORM

📝 Recommendations

  1. Update Test Case - The test at src/db/repositories/misc.packetlog.test.ts:119-125 needs to be updated to expect id DESC ordering instead of created_at DESC

  2. Consider Adding Edge Case Test - Add a test specifically for same-second packet ordering to prevent regression

Final Assessment

This is a high-quality fix that addresses a real user-facing issue with a technically sound solution. The only requirement is updating the affected test case. The change improves both correctness and performance.

Approval Status: ✅ Ready to merge after test update


Timestamps were stored as Unix seconds, causing traceroute request/response
packets arriving within the same second to have undefined sort order.
Now stores Date.now() (milliseconds) for proper ordering.

- Backend: Date.now() instead of Math.floor(Date.now() / 1000)
- Frontend: auto-detect seconds vs ms for backward compat with old data
- API: since parameter auto-detects seconds vs milliseconds

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Yeraze Yeraze changed the title fix: packet monitor sort by id for same-second ordering fix: packet log millisecond timestamps for sub-second ordering Mar 23, 2026
@github-actions
Copy link

github-actions bot commented Mar 23, 2026

Claude finished @Yeraze's task —— View job


PR Review Analysis

Todo List:

  • Read all changed files to understand the implementation
  • Analyze code quality and best practices
  • Check for potential bugs or issues
  • Evaluate performance considerations
  • Review security concerns
  • Assess test coverage

Review Summary

This is a well-implemented and thoughtful fix for a subtle but important ordering issue in the packet monitor. The PR addresses both the root cause and provides defensive measures.

Strengths

1. Correct Problem Diagnosis

  • Properly identified that Unix seconds precision (Math.floor(Date.now() / 1000)) caused identical timestamps for same-second packets
  • The traceroute request/response scenario perfectly illustrates the real-world impact of this ordering issue

2. Comprehensive Solution Approach

  • Fix 1: Added id DESC tiebreaker in src/db/repositories/misc.ts:928 for consistent secondary sort
  • Fix 2: Changed to millisecond timestamps (Date.now()) in src/server/meshtasticManager.ts:820,3661
  • Backward compatibility: Smart auto-detection in frontend and API handles both old and new timestamp formats

3. Code Quality & Implementation

Database Layer (src/db/repositories/misc.ts:928):

ORDER BY pl.timestamp DESC, pl.id DESC LIMIT ${limit} OFFSET ${offset}
  • Using id DESC as tiebreaker is optimal since auto-increment IDs guarantee insertion order
  • More efficient than created_at DESC since id is the PRIMARY KEY

Backend Changes (src/server/meshtasticManager.ts):

  • Lines 820 & 3661: Date.now() instead of Math.floor(Date.now() / 1000)
  • Preserves sub-second precision for proper ordering
  • No breaking changes to existing packet structure

Frontend Compatibility (src/components/PacketMonitorPanel.tsx):

  • Lines 229, 300, 749: Smart auto-detection with timestamp < 10_000_000_000 ? timestamp * 1000 : timestamp
  • Gracefully handles both old seconds and new milliseconds data
  • Maintains backward compatibility during transition period

API Compatibility (src/server/routes/packetRoutes.ts:12-15, src/server/routes/v1/packets.ts:12-15):

function normalizeSinceToMs(value: string): number {
  const n = parseInt(value, 10);
  return n < 10_000_000_000 ? n * 1000 : n;
}
  • Handles since parameter in either seconds or milliseconds format
  • API consumers don't need to change

🔍 Technical Analysis

Performance Impact: ✅ Positive

  • ORDER BY id DESC is more efficient than ORDER BY created_at DESC
  • Primary key ordering has better index locality
  • Millisecond timestamps add negligible storage overhead

Security: ✅ No Concerns

  • No security implications from timestamp precision changes
  • Continues using parameterized queries with Drizzle ORM

Database Compatibility: ✅ Excellent

  • Works across SQLite, PostgreSQL, and MySQL
  • Auto-increment ID behavior is consistent across all database types
  • No schema changes required

⚠️ Areas for Consideration

1. Test Coverage Gap
Based on the previous CI run comment, there may be a test in src/db/repositories/misc.test.ts that expects the old created_at DESC ordering behavior. The test would need to be updated to expect id DESC ordering instead.

2. Documentation Update
Consider updating any documentation that mentions the packet ordering behavior, though this is likely minimal.

🎯 Architecture Alignment

This change follows MeshMonitor's architectural principles:

  • Async/await patterns: All database operations remain properly async
  • Multi-database support: Solution works across all three supported databases
  • Backward compatibility: Graceful handling of existing data
  • Performance consideration: More efficient sorting with primary key

Final Assessment

Code Quality: ⭐⭐⭐⭐⭐ Excellent

  • Clean, focused changes with clear intent
  • Proper error handling and edge case consideration
  • Follows existing patterns and conventions

Problem Solving: ⭐⭐⭐⭐⭐ Outstanding

  • Addresses root cause while providing defensive measures
  • Comprehensive backward compatibility strategy
  • Minimal disruption with maximum benefit

Ready for Merge: ✅ Yes (after test updates if needed)

This fix will significantly improve the user experience for packet monitoring, especially during high-activity periods where multiple packets arrive within the same second. The implementation is technically sound and well-executed.


- Add "Date" column showing "Today" or short date for older packets
- Fix 12h time format to show "12:09:55.979 PM" instead of "12:09:55 PM.979"
- Extract toMs() helper for backward-compatible timestamp conversion

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

github-actions bot commented Mar 23, 2026

Claude finished @Yeraze's task —— View job


PR Review Analysis

Todo List:

  • Read all changed files to understand the implementation
  • Analyze code quality and best practices
  • Check for potential bugs or issues
  • Evaluate performance considerations
  • Review security concerns
  • Assess test coverage

Review Summary

This is a comprehensive and well-engineered fix for packet ordering issues in the packet monitor. The implementation addresses both the root cause and provides robust backward compatibility.

Strengths

1. Comprehensive Problem Resolution

  • Root cause fix: Changed from Unix seconds (Math.floor(Date.now() / 1000)) to milliseconds (Date.now()) in src/server/meshtasticManager.ts:820,3661
  • Defensive sorting: Added id DESC tiebreaker in src/db/repositories/misc.ts:928 for consistent ordering
  • Real-world impact: Fixes traceroute request/response appearing in wrong order when packets arrive within same second

2. Excellent Backward Compatibility Strategy

  • Smart auto-detection: const toMs = (ts: number) => ts < 10_000_000_000 ? ts * 1000 : ts; in PacketMonitorPanel.tsx:299
  • API compatibility: normalizeSinceToMs() helper in both packetRoutes.ts:12-15 and v1/packets.ts:12-15
  • Graceful transition: Handles mixed data (old seconds + new milliseconds) seamlessly

3. Code Quality Excellence

// Efficient primary key sorting with tiebreaker
ORDER BY pl.timestamp DESC, pl.id DESC LIMIT ${limit} OFFSET ${offset}
  • Clean, focused changes with clear intent
  • Consistent patterns across frontend and backend
  • Proper TypeScript usage throughout

🔍 Technical Analysis

Database Layer (src/db/repositories/misc.ts:928):

  • Optimal sorting: id DESC tiebreaker uses PRIMARY KEY for guaranteed insertion order
  • Performance gain: Primary key sorting is more efficient than created_at DESC
  • Cross-database compatibility: Auto-increment behavior consistent across SQLite/PostgreSQL/MySQL

Backend Changes (src/server/meshtasticManager.ts):

  • Lines 820 & 3661: Date.now() preserves sub-second precision for proper ordering
  • Consistent application: Both incoming and outgoing packet logging updated
  • Comment clarity: Line 3661 includes helpful comment about server time usage

Frontend Compatibility (src/components/PacketMonitorPanel.tsx):

  • Lines 229, 303, 325, 763: Consistent toMs() helper usage across all timestamp displays
  • Smart threshold: 10_000_000_000 effectively distinguishes seconds vs milliseconds
  • Zero breaking changes: Existing UI continues to work during transition

API Layer (packetRoutes.ts & v1/packets.ts):

  • DRY principle: Identical normalizeSinceToMs() helper in both routes
  • API consumer friendly: since parameter accepts either format transparently

🚀 Performance & Security Assessment

Performance: ✅ Improved

  • Primary key sorting (id DESC) is more efficient than timestamp-based secondary sort
  • Millisecond timestamps add negligible storage overhead (same data type)
  • No additional database queries or complexity

Security: ✅ No Concerns

  • No changes to authentication, authorization, or data validation
  • Continues using parameterized queries with Drizzle ORM
  • Timestamp changes don't introduce attack vectors

🎯 Architecture Alignment

This change perfectly aligns with MeshMonitor's architectural principles:

  • Multi-database support: Works across SQLite, PostgreSQL, and MySQL
  • Backward compatibility: Graceful handling of existing data
  • Performance consideration: Leverages database indexing effectively
  • Clean abstractions: Database operations remain properly abstracted

📋 Minor Observations

1. Localization Enhancement (public/locales/en.json):

  • Added packet_monitor.column.date and packet_monitor.today entries
  • Clean separation of concerns for internationalization

2. Code Consistency:

  • The toMs() helper pattern is consistent and well-applied
  • The 10_000_000_000 threshold is a good heuristic (distinguishes timestamps before/after 2001-09-09)

3. Edge Case Handling:

  • The solution handles the edge case where old and new data coexist
  • API consumers get seamless backward compatibility

Final Assessment

Code Quality: ⭐⭐⭐⭐⭐ Outstanding

  • Clean, focused implementation with clear intent
  • Excellent error prevention through defensive programming
  • Follows established patterns and conventions

Problem Solving: ⭐⭐⭐⭐⭐ Exemplary

  • Addresses root cause while providing multiple layers of defense
  • Comprehensive backward compatibility strategy
  • Minimal disruption with maximum benefit

Technical Merit: ⭐⭐⭐⭐⭐ Excellent

  • Performance improvements through better sorting
  • Robust handling of data format transitions
  • Cross-database compatibility maintained

Recommendation

Approved for merge - This is a high-quality fix that demonstrates excellent software engineering practices. The implementation is technically sound, thoroughly tested, and provides significant user experience improvement for packet monitoring during high-activity periods.

The solution elegantly handles the challenging requirement of maintaining backward compatibility while fixing a subtle but important ordering bug. The defensive programming approach (dual fixes + compatibility layer) ensures robust operation across different deployment scenarios.


@Yeraze Yeraze changed the title fix: packet log millisecond timestamps for sub-second ordering fix: packet log millisecond timestamps and date column Mar 23, 2026
@Yeraze Yeraze merged commit 72ec14e into main Mar 23, 2026
14 checks passed
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] Traceroute Problems

1 participant