test(nodes): pin raised-interval guard and repeated cut, document false change (#270)

cmd/server/advert_intervals_test.go

@@ -3,6 +3,7 @@ package main
 import (
 	"encoding/json"
 	"fmt"
+	"os"
 	"strings"
 	"testing"
 	"time"
@@ -204,6 +205,15 @@ func TestEstimateAdvertInterval_RaisedInterval(t *testing.T) {
 		// Lowered (47 h -> 12 h): 12 h explains the old 47 h gaps as 4x
 		// once it is a candidate, as before.
 		{"flood 47 h -> 12 h", advertIntervalFlood, series(aiRepeat(47*H, 14), aiRepeat(12*H, 5)), 12 * 3600, advertConfidenceHigh, 20, 19},
+		// A gap at the interval itself among the newest 3 that fit breaks
+		// the run (review N1 on #247): 24, 12, 24, 24 h is missed adverts
+		// on 12 h, not a raised setting since the 12 h gap.
+		{"flood 12 h, last 4 gaps 2x 1x 2x 2x", advertIntervalFlood, series(aiRepeat(12*H, 12), []time.Duration{24 * H, 12 * H, 24 * H, 24 * H}), 12 * 3600, advertConfidenceHigh, 17, 16},
+		// Raised twice (review N2 on #247): the cut repeats on the adverts
+		// since the first change until no run is left, so the estimate is
+		// the final setting, not the one in between.
+		{"zero-hop 60 -> 120 -> 240 min", advertIntervalZeroHop, series(aiRepeat(60*M, 6), aiRepeat(120*M, 6), aiRepeat(240*M, 6)), 240 * 60, advertConfidenceHigh, 7, 6},
+		{"flood 12 h -> 24 h -> 47 h", advertIntervalFlood, series(aiRepeat(12*H, 6), aiRepeat(24*H, 6), aiRepeat(47*H, 6)), 47 * 3600, advertConfidenceHigh, 7, 6},
 	}
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
@@ -228,6 +238,77 @@ func TestEstimateAdvertInterval_TimerCandidates(t *testing.T) {
 	aiCheck(t, estimateAdvertInterval(aiGaps(aiRepeat(60*M, 8)...), advertIntervalFlood), 0, advertConfidenceNone, 9, 0)
 }
 
+// The zero-hop 2 min candidate band is 120 s +- 10 % (review N5 on #247),
+// the same tolerance the snap uses: gaps of 108-132 s give the 2 min
+// default, gaps just outside it are no timer interval. The other edges:
+// zero-hop 54 min (60 less 10 %), flood 2.7 h (3 h less 10 %).
+func TestEstimateAdvertInterval_CandidateBandEdges(t *testing.T) {
+	S, M := time.Second, time.Minute
+	for _, c := range []struct {
+		gap  time.Duration
+		want int64
+		conf string
+		gaps int
+	}{
+		{108 * S, 120, advertConfidenceHigh, 19},
+		{132 * S, 120, advertConfidenceHigh, 19},
+		{107 * S, 0, advertConfidenceNone, 0},
+		{133 * S, 0, advertConfidenceNone, 0},
+	} {
+		t.Run(c.gap.String(), func(t *testing.T) {
+			aiCheck(t, estimateAdvertInterval(aiGaps(aiRepeat(c.gap, 19)...), advertIntervalZeroHop), c.want, c.conf, 20, c.gaps)
+		})
+	}
+	for _, c := range []struct {
+		class advertIntervalClass
+		s     float64
+		want  bool
+	}{
+		{advertIntervalZeroHop, 108, true},
+		{advertIntervalZeroHop, 132, true},
+		{advertIntervalZeroHop, 107, false},
+		{advertIntervalZeroHop, 133, false},
+		{advertIntervalZeroHop, (54 * M).Seconds(), true},
+		{advertIntervalZeroHop, (54 * M).Seconds() - 1, false},
+		{advertIntervalFlood, 2.7 * 3600, true},
+		{advertIntervalFlood, 2.7*3600 - 1, false},
+	} {
+		if got := advertIntervalCandidateOK(c.s, c.class); got != c.want {
+			t.Errorf("advertIntervalCandidateOK(%v s, class %d) = %v, want %v", c.s, c.class, got, c.want)
+		}
+	}
+}
+
+// Known trade-off of the raised-interval rule (review N3 on #247): with an
+// unchanged interval, when the newest 3 heard gaps are the same multiple k
+// (every 2nd or 3rd advert lost), the estimate is k x at medium confidence
+// on 4 adverts, until the next gap at the interval itself breaks the run.
+// docs/api-spec.md documents it next to the sparse-coverage limitation.
+func TestEstimateAdvertInterval_FalseChange(t *testing.T) {
+	H, M := time.Hour, time.Minute
+	cases := []struct {
+		name     string
+		class    advertIntervalClass
+		gaps     []time.Duration
+		interval int64
+	}{
+		{"zero-hop 60 min, last 3 gaps 2x", advertIntervalZeroHop, append(aiRepeat(60*M, 10), aiRepeat(120*M, 3)...), 120 * 60},
+		{"flood 47 h, last 3 gaps 2x", advertIntervalFlood, append(aiRepeat(47*H, 10), aiRepeat(94*H, 3)...), 94 * 3600},
+		{"zero-hop 120 min, last 3 gaps 3x", advertIntervalZeroHop, append(aiRepeat(120*M, 10), aiRepeat(360*M, 3)...), 360 * 60},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			aiCheck(t, estimateAdvertInterval(aiGaps(c.gaps...), c.class), c.interval, advertConfidenceMedium, 4, 3)
+		})
+	}
+	// A gap that fits no multiple (an outage over 4x) does not break the run.
+	s := aiGaps(append(aiRepeat(60*M, 10), 120*M, 120*M, 300*M, 120*M)...)
+	aiCheck(t, estimateAdvertInterval(s, advertIntervalZeroHop), 120*60, advertConfidenceMedium, 5, 3)
+	// One more gap at the interval itself ends it.
+	s = aiGaps(append(append(aiRepeat(60*M, 10), aiRepeat(120*M, 3)...), 60*M)...)
+	aiCheck(t, estimateAdvertInterval(s, advertIntervalZeroHop), 3600, advertConfidenceHigh, 15, 14)
+}
+
 // Known limitation (review F2 on #247): when coverage is so sparse that the
 // interval itself is never heard twice in a row, it is no candidate and a
 // multiple of it is estimated. A 47 h flood heard only 2x and 3x apart
@@ -384,7 +465,7 @@ func TestEstimateAdvertInterval_ConfidenceTiers(t *testing.T) {
 	}
 }
 
-// The interval is the median of the fitting gaps, not their mean: one late
+// The interval is the median of gap/k over the fitting gaps, not their mean: one late
 // first_seen (no sender clock) moves a mean, not the median.
 func TestEstimateAdvertInterval_MedianNotMean(t *testing.T) {
 	s := aiSeries(12*time.Hour, aiInts(0, 7)...)
@@ -557,3 +638,40 @@ func TestNodeDetail_AdvertIntervals(t *testing.T) {
 		t.Fatalf("window = %d", body.Intervals.Window)
 	}
 }
+
+// The docs describe what the estimator does (review N3 and N5 on #247): the
+// value is the median of gap/k, not of the raw fitting gaps, and the
+// false-change trade-off pinned by _FalseChange sits next to the
+// sparse-coverage limitation in docs/api-spec.md.
+func TestAdvertIntervals_DocsMatchCode(t *testing.T) {
+	schemas := asMap(t, asMap(t, fetchSpec(t)["components"], "components")["schemas"], "schemas")
+	desc := fmt.Sprint(asMap(t, schemas["NodeAdvertIntervals"], "NodeAdvertIntervals")["description"])
+	if !strings.Contains(desc, "median of gap/k") || strings.Contains(desc, "median of the fitting gaps") {
+		t.Errorf("OpenAPI NodeAdvertIntervals must say the median of gap/k: %s", desc)
+	}
+
+	raw, err := os.ReadFile("../../docs/api-spec.md")
+	if err != nil {
+		t.Fatal(err)
+	}
+	doc := string(raw)
+	start := strings.Index(doc, "#### Estimated advert intervals")
+	if start < 0 {
+		t.Fatal("docs/api-spec.md: no Estimated advert intervals section")
+	}
+	section := doc[start:]
+	if end := strings.Index(section[1:], "\n#"); end >= 0 {
+		section = section[:end+1]
+	}
+	sparse := strings.Index(section, "**Known limitation: sparse coverage.**")
+	falseChange := strings.Index(section, "**Known trade-off: false change.**")
+	if sparse < 0 || falseChange < 0 {
+		t.Fatalf("docs/api-spec.md must document sparse coverage and the false change (sparse %d, false change %d)", sparse, falseChange)
+	}
+	// The examples are the _FalseChange cases.
+	for _, want := range []string{"60 min reads as 120 min", "47 h as 94 h", "120 min with 3× gaps as 360 min", "medium confidence on 4 adverts"} {
+		if !strings.Contains(section[falseChange:], want) {
+			t.Errorf("false-change limitation lacks %q", want)
+		}
+	}
+}

cmd/server/openapi.go

@@ -261,7 +261,7 @@ func nodeAdvertRouteSchemas() map[string]*openAPISchema {
 		},
 		"NodeAdvertIntervals": {
 			Type:        "object",
-			Description: "Node detail with include=advertRoutes only (#245): the node's estimated flood and zero-hop advert intervals, from the gaps between the adverts listed in recentAdvertsByRoute.flood / .zero_hop (mixed and unknown adverts are not used). A gap uses the adverts' own (sender) timestamps when both are plausible - not ahead of first_seen by more than 10 min, positive, and within max(10 min, 10 %) of the first_seen gap - else first_seen. The interval must be seen directly in at least two gaps and a quarter of them, and be one the class's timer can run at (flood 3 h or more, zero-hop 2 min or 60 min or more, each less 10 %); gaps of 2-4x it count as missed adverts, shorter gaps (manual adverts, reboots) are dropped, longer gaps that are no multiple are irregular. When the newest 3 gaps that fit are all the same multiple k > 1 the interval was raised, and the estimate is redone on the adverts since the change. It is the median of the fitting gaps, snapped to the firmware's settable values: flood.advert.interval whole hours 3-168, advert.interval even minutes 60-240 or the 2-minute new-install default. No zero-hop adverts can mean the node's zero-hop interval is 0 (off) or that no observer hears it directly. Absent without include=advertRoutes and when the node's identity is hidden; cached with recentAdvertsByRoute.",
+			Description: "Node detail with include=advertRoutes only (#245): the node's estimated flood and zero-hop advert intervals, from the gaps between the adverts listed in recentAdvertsByRoute.flood / .zero_hop (mixed and unknown adverts are not used). A gap uses the adverts' own (sender) timestamps when both are plausible - not ahead of first_seen by more than 10 min, positive, and within max(10 min, 10 %) of the first_seen gap - else first_seen. The interval must be seen directly in at least two gaps and a quarter of them, and be one the class's timer can run at (flood 3 h or more, zero-hop 2 min or 60 min or more, each less 10 %); gaps of 2-4x it count as missed adverts, shorter gaps (manual adverts, reboots) are dropped, longer gaps that are no multiple are irregular. When the newest 3 gaps that fit are all the same multiple k > 1 the interval was raised, and the estimate is redone on the adverts since the change. It is the median of gap/k over the gaps that fit k x the interval, snapped to the firmware's settable values: flood.advert.interval whole hours 3-168, advert.interval even minutes 60-240 or the 2-minute new-install default. No zero-hop adverts can mean the node's zero-hop interval is 0 (off) or that no observer hears it directly. Absent without include=advertRoutes and when the node's identity is hidden; cached with recentAdvertsByRoute.",
 			Properties: map[string]*openAPISchema{
 				"window":   {Type: "integer", Description: "Most adverts per class considered (the recentAdvertsByRoute limit, 20)."},
 				"flood":    openAPIRef("AdvertIntervalEstimate"),

docs/api-spec.md

@@ -636,6 +636,15 @@ The firmware settings it maps to (MeshCore `src/helpers/CommonCLI.cpp`):
   heard twice in a row (a distant node heard every second or third time),
   it is no candidate, and a multiple of it is reported: a 47 h flood heard
   94 h and 141 h apart reads as 94 h or 141 h at medium confidence.
+- **Known trade-off: false change.** The raised-interval rule can read an
+  unchanged interval as raised. When the newest 3 heard gaps are all the
+  same multiple k (every 2nd or 3rd advert lost), the result is k× at
+  medium confidence on 4 adverts: 60 min reads as 120 min, 47 h as 94 h,
+  and 120 min with 3× gaps as 360 min. Gaps that fit no multiple (an
+  outage over 4×, a reboot advert between the 2× gaps) do not break the
+  run. The next gap heard at the interval itself does, and the estimate
+  returns to the interval. The opposite choice kept a raised interval at the
+  old value, at high confidence, for weeks (review F1 on #247).
 - **No zero-hop adverts.** A zero-hop advert is only recorded when an
   observer hears the node directly. "None observed" can therefore mean that
   the interval is 0 (off), or that no observer is in direct range.