diff --git a/Client/udpscope/TimeBase.cpp b/Client/udpscope/TimeBase.cpp index a1c87aa..43bf780 100644 --- a/Client/udpscope/TimeBase.cpp +++ b/Client/udpscope/TimeBase.cpp @@ -13,11 +13,13 @@ double TimeSignalScale(uint8_t typeCode) { } double ClockOffset::map(double producerSec, double wallSec) { - /* Recalibrate only when wall is significantly ahead of the prediction. - * Early-arriving packets (wall < prediction) are normal network jitter and - * must not reset the offset — doing so would wobble the trace. A large - * positive error means the producer clock jumped back or restarted. */ - if (!valid_ || (wallSec - (offset_ + producerSec)) > kRecalibThresholdS) { + /* Symmetric on purpose. Delivery jitter of a few tens of ms either side of + * the prediction must not reset the offset or the whole trace wobbles, but a + * producer clock that steps in EITHER direction has to be picked up: a + * restart leaves the prediction behind the wall clock, an NTP correction on + * the producer's host leaves it ahead. A one-sided test silently never fires + * for the second case and the trace sits in the future for the whole run. */ + if (!valid_ || std::fabs(wallSec - (offset_ + producerSec)) > kRecalibThresholdS) { offset_ = wallSec - producerSec; valid_ = true; } diff --git a/Client/udpscope/tests/TimeBaseTest.cpp b/Client/udpscope/tests/TimeBaseTest.cpp index a06b9cc..53bd6f1 100644 --- a/Client/udpscope/tests/TimeBaseTest.cpp +++ b/Client/udpscope/tests/TimeBaseTest.cpp @@ -20,8 +20,24 @@ TEST(ClockOffset, HoldsTheOffsetThroughSmallArrivalJitter) { ClockOffset off; off.map(10.0, 1000.0); // offset = 990 - EXPECT_DOUBLE_EQ(off.map(11.0, 1001.02), 1001.0); - EXPECT_DOUBLE_EQ(off.map(12.0, 1000.97), 1002.0); + // Arrival wanders either side of the prediction. wallSec is a local receive + // timestamp, so it only ever advances — jitter shows up as the gap growing + // and shrinking, never as the clock going backwards. + EXPECT_DOUBLE_EQ(off.map(11.0, 1001.02), 1001.0); // +0.02 late + EXPECT_DOUBLE_EQ(off.map(12.0, 1001.97), 1002.0); // -0.03 early +} + +// The threshold has to be symmetric. A producer whose clock steps FORWARD (an +// NTP correction on the producer's host, say) puts the prediction permanently +// ahead of the wall clock — a one-sided "recalibrate only when wall is ahead" +// test never fires for it, and the trace sits in the future for the rest of the +// run. +TEST(ClockOffset, RecalibratesWhenTheProducerClockJumpsForward) { + ClockOffset off; + off.map(10.0, 1000.0); // offset = 990 + + // Producer leaps 100 s ahead while only 1 s of wall time passes. + EXPECT_DOUBLE_EQ(off.map(111.0, 1001.0), 1001.0); } TEST(ClockOffset, RecalibratesWhenDriftExceedsTheThreshold) { diff --git a/docs/superpowers/plans/2026-08-27-udpscope.md b/docs/superpowers/plans/2026-08-27-udpscope.md index 6350d39..add1d89 100644 --- a/docs/superpowers/plans/2026-08-27-udpscope.md +++ b/docs/superpowers/plans/2026-08-27-udpscope.md @@ -1043,8 +1043,24 @@ TEST(ClockOffset, HoldsTheOffsetThroughSmallArrivalJitter) { ClockOffset off; off.map(10.0, 1000.0); // offset = 990 - EXPECT_DOUBLE_EQ(off.map(11.0, 1001.02), 1001.0); - EXPECT_DOUBLE_EQ(off.map(12.0, 1000.97), 1002.0); + // Arrival wanders either side of the prediction. wallSec is a local receive + // timestamp, so it only ever advances — jitter shows up as the gap growing + // and shrinking, never as the clock going backwards. + EXPECT_DOUBLE_EQ(off.map(11.0, 1001.02), 1001.0); // +0.02 late + EXPECT_DOUBLE_EQ(off.map(12.0, 1001.97), 1002.0); // -0.03 early +} + +// The threshold has to be symmetric. A producer whose clock steps FORWARD (an +// NTP correction on the producer's host, say) puts the prediction permanently +// ahead of the wall clock — a one-sided "recalibrate only when wall is ahead" +// test never fires for it, and the trace sits in the future for the rest of the +// run. +TEST(ClockOffset, RecalibratesWhenTheProducerClockJumpsForward) { + ClockOffset off; + off.map(10.0, 1000.0); // offset = 990 + + // Producer leaps 100 s ahead while only 1 s of wall time passes. + EXPECT_DOUBLE_EQ(off.map(111.0, 1001.0), 1001.0); } TEST(ClockOffset, RecalibratesWhenDriftExceedsTheThreshold) {