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

[dborup-agent]

Relates to #270

Follow-ups N1, N2, N3 and N5 from the round-2 review of #247 (#245 M1). Tests and docs only: the estimator in cmd/server/advert_intervals.go is unchanged.

Plan

#	Item	Change	Mutant that must die
1	N1: the 1× gap guard in advertIntervalRaised	_RaisedInterval case: 12 h series ending in gaps 24, 12, 24, 24 h → 12 h high (17/16)	R2: a 1× gap among the newest 3 is skipped instead of breaking the run
2	N2: the repeated cut (for best > 0)	_RaisedInterval cases: zero-hop 60 → 120 → 240 min → 240 min, flood 12 → 24 → 47 h → 47 h (both high, 7/6)	R1: the raised rule cuts once (if instead of for)
3	N5: zero-hop 2-min candidate band	_CandidateBandEdges: 108 s and 132 s give 120 s; 107 s and 133 s give none. Also pins the 54 min zero-hop and 2.7 h flood edges on advertIntervalCandidateOK	C1: band ±10 % → ±20 %
4	N3: false-change trade-off	A "Known trade-off: false change" bullet in docs/api-spec.md, next to the sparse-coverage limitation. _FalseChange pins its examples (60 → 120 min, 47 → 94 h, 120 min with 3× gaps → 360 min, all medium 4/3). It also pins that an outage gap does not break the run and that the next 1× gap does	D2: the doc bullet removed
5	N5: OpenAPI text	NodeAdvertIntervals now says the median of gap/k, as the code and docs do. _DocsMatchCode checks the OpenAPI text and the docs section	D1: the OpenAPI text reverted

F7/N4 belong to M2 (#245) and need no code here. The optional "report low while the estimate rests on a short post-change segment" from N3 is not done. It would change the confidence rules, so it is better decided with M2.

Commits

1. 082c26b2 test: pins for N1, N2, N5 and the N3 behaviour (green on master code: these are behaviour pins).
2. e6209c41 test: _DocsMatchCode, red until the docs are fixed.
3. 785792bb docs: the api-spec bullet and the OpenAPI text. _DocsMatchCode turns green.

Note: the commit 3 message says it changes the MedianNotMean test comment ("median of gap/k"). That edit actually landed in commit 1. Commit 1 also left a trailing blank line in the test file, which commit 2 removes. History is not rewritten.

Tests

• Go: cd cmd/server && go test ./... and cd cmd/ingestor && go test ./....
• sh test-all.sh: 221/221 files. node test-frontend-helpers.js: 707/0.
• E2E against a local Go server on e2e-fixture.db, prepared as in CI (freshen, inline seed, migrate, seeds 2073/199/245): test-issue-245-advert-intervals-e2e.js 7/7, test-issue-2073-recent-adverts-e2e.js 10/10.
• Every mutant was run against TestEstimateAdvertInterval*, TestSnapAdvertInterval, TestNodeAdvertIntervals*, TestAdvertIntervalSamples*, TestNodeDetail_AdvertIntervals, TestOpenAPI* and TestAdvertIntervals_*. Each survives the master test file and dies with this branch's.

Invariants

• cmd/server stays read-only (no write in the diff), and there is no new map[string]interface{}.
• No frontend change: scripts/check-xss-sinks.sh --diff origin/master has nothing to scan, and no colours change.
• .github/ is unchanged: fork guards are 9 in deploy.yml and 1 in release-fast-path.yml.
• Performance: there is no runtime change.

🤖 Generated with Claude Code

[dborup-agent]

Rapport — CS-pve-agent2 PR#292 #270 — head 785792b

Status: All five items from #270 are done (N1, N2, N5 tests; N3 docs; N5 OpenAPI text). Each mutant survives the master tests and dies on this branch, and CI is green on 785792bb on the first attempt. The PR is a draft; the estimator code is unchanged.

Evidence tags: [T] test or run output in this session, [A] analysis or reading of source, [K] known context from earlier issues and PRs.

Requirements

#	Requirement	Test	Red before → green after	Mutant: master tests → this branch
1	N1: a 1× gap among the newest 3 fitting gaps breaks the raised run	TestEstimateAdvertInterval_RaisedInterval/flood_12_h,_last_4_gaps_2x_1x_2x_2x: 24, 12, 24, 24 h on 12 h → 12 h high, 17/16	Behaviour pin: green on unchanged code, red under R2 [T]	R2 (skip the 1× gap instead of returning 0): survives → killed (_RaisedInterval, _FalseChange) [T]
2	N2: the repeated cut	_RaisedInterval/zero-hop_60_->_120_->_240_min → 240 min, and /flood_12_h_->_24_h_->_47_h → 47 h, both high 7/6	Behaviour pin, red under R1 [T]	R1 (if instead of for, one cut): survives → killed (both double-raise cases) [T]
3	N5: zero-hop 2-min band edges	TestEstimateAdvertInterval_CandidateBandEdges: 108 s and 132 s → 120 s high 20/19; 107 s and 133 s → none. Plus advertIntervalCandidateOK at 54 min and 2.7 h ± 1 s	Behaviour pin, red under C1 [T]	C1 (band ±10 % → ±20 %): survives → killed (/1m47s, /2m13s) [T]
4	N3: document the false-change trade-off	TestAdvertIntervals_DocsMatchCode checks for the "Known trade-off: false change" bullet next to the sparse-coverage one, with the examples. TestEstimateAdvertInterval_FalseChange pins those examples: 60 → 120 min, 47 → 94 h, 120 min with 3× gaps → 360 min, all medium 4/3. It also pins that an outage gap (5×) does not break the run (120 min medium 5/3) and that the next 1× gap does (60 min high 15/14)	Red at e6209c41, green at 785792bb [T]	D2 (bullet removed): survives → killed (_DocsMatchCode) [T]
5	N5: OpenAPI says median of gap/k	TestAdvertIntervals_DocsMatchCode reads /api/spec NodeAdvertIntervals.description	Red at e6209c41, green at 785792bb [T]	D1 (text reverted to "median of the fitting gaps"): survives → killed [T]

The mutant runs used TestEstimateAdvertInterval*, TestSnapAdvertInterval, TestNodeAdvertIntervals*, TestAdvertIntervalSamples*, TestNodeDetail_AdvertIntervals, TestOpenAPI* and TestAdvertIntervals_*, run once with the master advert_intervals_test.go and once with this branch's. [T]

The docs examples and the outage claim come from the round-2 review on #247 [K], and _FalseChange reproduces them [T].

F7/N4 belong to M2 on #245, with no code change here [K]. The optional N3 idea, "low while the estimate rests on a short post-change segment", is not done [A]. It changes the confidence rules, so it fits better with M2.

Local runs [T]

Run	Result
cd cmd/server && go test -timeout 30m ./...	ok (705 s)
cd cmd/ingestor && go test -timeout 20m ./...	ok (1129 s)
sh test-all.sh	221 passed, 0 failed
node test-frontend-helpers.js	707 passed, 0 failed
test-issue-245-advert-intervals-e2e.js on a local Go server, e2e-fixture.db prepared as in CI (freshen, inline seed, migrate, seeds 2073/199/245)	7/7
test-issue-2073-recent-adverts-e2e.js, same server	10/10
bash scripts/check-xss-sinks.sh --diff origin/master	clean (no public/ changes to scan)

The server was stopped by pid, and its port was confirmed free.

A first local run with the default 10 min go test timeout timed out in both modules. They passed with the CI timeouts above. [T]

CI on 785792bb (run attempt 1) [T]

Job	Result
Go Build & Test	pass (23m10s)
Playwright E2E Tests	pass (20m58s)
Build & Publish Docker Image	pass
Release Artifacts	skipping
Publish Badges & Summary	skipping
Deploy Staging	skipping

There were no re-runs, and the known flaky test (#271) did not fail.

Invariants [T][A]

• cmd/server stays read-only: no write statement and no RW connection in the diff.
• No new map[string]interface{} in the added lines.
• No frontend or colour change.
• .github/ is unchanged: fork guards are 9 in deploy.yml and 1 in release-fast-path.yml.
• No runtime or performance change, since only tests, the OpenAPI text and the docs changed.

Remaining issues

• Commit message slip [T]. The 785792bb message says it changes the MedianNotMean test comment, but that edit is in 082c26b2. Also, 082c26b2 left a trailing blank line, which e6209c41 removes. History is not rewritten, and the PR description notes both.
• Local full-suite timing [T]. I created the commits while the local full Go run was in progress, after its compile step. The full suites on the exact head ran in CI, and they are green.
• Not merged or marked ready, as instructed.

[dborup-agent]

Review — CS-MacBook PR#292 — head 785792b

Dom: APPROVE with nits

Independent, read-only review. Verified on the merged tree git merge-tree --write-tree origin/master 785792bb (base origin/master 4f1de04; head branched from an older master, so the merged tree carries today's master + this PR — a combination CI did not test). The estimator cmd/server/advert_intervals.go is byte-identical across head, the merged tree and master: this PR is tests + OpenAPI text + docs only. Head SHA unchanged before and after. Evidence: [T] run this session, [A] source reading, [K] earlier context.

Findings

#	Sev	Finding
1	nit	Commit-message slip, self-disclosed in the PR body: 785792bb's message claims the MedianNotMean comment edit, but that edit landed in 082c26b2; 082c26b2 also left a trailing blank line that e6209c41 removes. Cosmetic, history not rewritten. [T][A]
2	nit	The reworded MedianNotMean comment line is now ~100 cols (// The interval is the median of gap/k over the fitting gaps, not their mean: one late). Accurate, just long. [A]

No correctness, scope or safety issue found.

Point-by-point

1. Requirements each have a test, red-before → green-after. [T][A]
Because the estimator is unchanged, the N1/N2/N5 cases are behaviour pins (green on real code, red under the review's mutants), and the N3/N5 docs test is a true red-at-e6209c41 → green-at-785792bb. The issue's acceptance criterion is "kill mutants R1, R2, C1" plus "docs/OpenAPI match the code" — both met.

Issue item	Test	Verified
N1: a 1× gap among the newest 3 fitting gaps breaks the run	_RaisedInterval/flood 12 h, last 4 gaps 2x 1x 2x 2x (24,12,24,24 h → 12 h high)	killed by R2 [T]
N2: the repeated cut (for best > 0)	_RaisedInterval/zero-hop 60→120→240 min, /flood 12→24→47 h	killed by R1 [T]
N5: zero-hop 2-min band edges	_CandidateBandEdges (108/132 s → 120 s; 107/133 s → none; advertIntervalCandidateOK 54 min / 2.7 h ±1 s)	killed by C1 [T]
N3: false-change doc bullet	_DocsMatchCode + _FalseChange	killed by D2 [T]
N5: OpenAPI "median of gap/k"	_DocsMatchCode	killed by D1 [T]

Optional N3 ("report low on a short post-change segment") is explicitly deferred to M2 — the issue marks it optional, so not a gap. F7/N4 are M2 analysis, no code here. [A][K]

2. Scope. [T][A] Diff touches only cmd/server/advert_intervals_test.go, cmd/server/openapi.go (a Description string), docs/api-spec.md. cmd/server stays read-only (no write statement, no RW handle added); no new map[string]interface{} added anywhere; no write added, so the ingestor rule is N/A. No frontend/colour change. .github/ unchanged — fork guards remain 9 in deploy.yml and 1 in release-fast-path.yml. No closing keyword (PR uses "Relates to #270").

3. Docs match code. [T] _DocsMatchCode reads live /api/spec and the docs/api-spec.md section; the bullet's examples (60→120 min, 47→94 h, 120 min w/ 3× → 360 min, medium on 4 adverts) line up with the _FalseChange pins.

Tests I ran [T]

Run (merged tree)	Result
go test ./ in cmd/server (full package, -count=1, 30m)	ok, 41.6 s
advert suite verbose (TestEstimateAdvertInterval*, TestAdvertIntervals_DocsMatchCode, TestSnapAdvertInterval, TestNodeAdvertIntervals*, TestAdvertIntervalSamples*, TestNodeDetail_AdvertIntervals)	all PASS
go vet ./ (cmd/server)	clean
node test-frontend-helpers.js	709 passed, 0 failed
scripts/check-xss-sinks.sh --diff origin/master	no public/** changes to scan

Mutants I ran [T] (own, against the merged tree)

Mutant	Edit	Result
R1 (single cut)	for best > 0 → single-pass if (compiling variant; the naive for→if won't build — the break is load-bearing)	killed: _RaisedInterval/…→240 min and /…→47 h fail
R2 (skip the 1× gap)	in advertIntervalRaised, continue on k==1 before the mismatch check (the issue's described R2)	killed: _RaisedInterval/…2x 1x 2x 2x (got 24 h, want 12 h) and _FalseChange "next 1× ends the run" (got 2 h, want 1 h) fail
C1 (band ±10 %→±20 %)	advertIntervalTol*120 → 0.20*120	killed: _CandidateBandEdges/1m47s, /2m13s fail
D1 (OpenAPI reverted)	"median of gap/k…" → "median of the fitting gaps"	killed: _DocsMatchCode fails
D2 (doc bullet removed)	drop the false-change bullet	killed: _DocsMatchCode fails

Note: my first R2 attempt (dropping k==1 from the OR) did not kill the N1 pin — the runK>0 && k!=runK branch still catches a k==1 gap when runK==2. Only the "skip-before-mismatch" variant (the issue's actual R2) reproduces the false-raise and the pin then fires. The pin is sound.

Edge case I added and ran [T]

The docs claim "a reboot advert between the 2× gaps" does not break the run, but _FalseChange only pins the long outage (5×, k=0). I exercised the short extra-advert case: zero-hop 60 min base with gaps …,120,30,120,120 min → stays 120 min medium (the 30 min k=0 gap is skipped), while …,120,60,120,120 → drops to 60 min high (the 1× gap breaks). Behaviour matches the doc on the untested variant. No bug.

Not verified / relied on CI

• Playwright E2E and the advert/recent-adverts E2E: not independently re-driven this session. The HTTP surface is unchanged except the OpenAPI Description string, and CI is green on the exact head (Go Build & Test 23m10s, Playwright 20m58s, Docker build). The author reports 7/7 and 10/10 locally, and sh test-all.sh 221/221. [K][A]
• cmd/ingestor: untouched by the diff, not re-run. [A]
• The red-at-e6209c41 state for _DocsMatchCode I took from the author's report + the D1/D2 mutants rather than checking out the intermediate commit. [K][T]

CI on 785792bb: all required jobs pass; no re-runs. Known flakies (#256 hash-sort, #267 backfill write-hold) did not fire.