Start a new epoch when a polled radar may be another box by Babissimo · Pull Request #534 · offworldlabs/retina-server

Babissimo · 2026-09-22T16:45:58Z

PR 2 of 3 for subtask 5 of the polled stock-blah2 radars (ClickUp 123zgec4baz, parent 123zgec4b91). PR 1 (#532) polls detections; this adds the config poll and the epoch triggers. PR 3 (poller-attested signing) follows.

What changes

Each poll session now starts by reading the radar's /api/config, compared against the polled_radars row and the active node_configs version.

Seen this session	Outcome
Fingerprint changed (sites, site names, fc)	Re-probe. A pass starts epoch + 1 on probation and writes a new config version if geometry or fc moved (a name alone writes none).
Name resolves into another network (/16 IPv4, /32 IPv6, same family)	Re-probe, and count the move on the row. The epoch and any graduation stand.
fs or CPI alone changed	New config version, same epoch, trust unchanged. A config that omits them keeps the values held.
Re-probe refused	After a fingerprint change: nothing written, no frames filed, retried next session. After a move: the radar keeps streaming and the move is still recorded.
Config no longer stock blah2, or refused by `validate_config`	No frames filed; reads as stalled, not unreachable.
Config request fails	A failed poll, as before.

last_config_at is written whenever the radar serves the configuration its epoch holds.

Why an address move does not start an epoch

Al's decision, 2026-09-23, replacing the earlier rule that a network move started one. An address is the operator's ISP or proxy moving them about: home connections and tunnels change network on their own, and an address says nothing about which box answers. Since graduation is manual (subtask 9), fencing on one would put exactly the operators this door is for back in the queue every time their ISP moved them.

So a move is re-probed and counted on the row (network_moves, last_network_move_at, migration 0017) as evidence for the trust layer to weigh (123zgec4bxx). The address is recorded whether or not the re-probe passes, so the same move is not probed again every session; only a probe that passed moves probe_passed_at.

Decisions to check

Network move compares against every address the name resolves to, not only the one that answered. So a hostname with records in two networks does not re-probe every session. A re-point of every record still counts.
The poller re-registers the radar with the pipeline, not the radar's task, whenever it restarts a radar for a new epoch or config version. The task's own commit changes its target, so any sync after that may cancel the task before it could re-register. That would leave the pipeline on the old geometry until a restart.

Reusable pieces

polled_radars.probed_config(config, base) turns a probe into a config version. Registration (subtask 4) can use it with a defaults base.
blah2_probe.read_config, Blah2Config.fingerprint, Blah2Probe.config.
node_config_store.active_config / config_fields.
PinnedClient.addresses.

Migration 0017 adds two columns to polled_radars and is additive; its downgrade is covered by a test, including that SQLite's table rewrite gives back the unique index on endpoint_key and the foreign key. No env or compose change. Inert on production until registration (subtask 4) exists, since the registry is empty.

Verified

Tests first; each new mechanism mutation-checked, meaning the matching test fails when that mechanism is broken.
pre-commit run --all-files is clean.
Full backend suite with CI's flags: 4695 passed.
tests/test_blah2_poller.py passed whole without xdist, repeatedly, while the full suite ran.
/code-review high clean, and the review bot's findings from three rounds are resolved (see the comments below).

Second commit

It fixes a flaky assertion from #532. The clock-offset test matched the substring clock offset -3, and a full-suite run logged -29.941 s when the wall clock stepped back about 60 ms under load. The test now parses the logged number and allows the same 2 s the test already allows for the poller's own reading.

🤖 Generated with Claude Code

Babissimo · 2026-09-22T17:11:21Z

All three review findings are fixed and folded into the first commit. Each has a test that failed first.

_same_epoch now checks the epoch-guarded update's rowcount. An epoch moved by another writer rolls back the new version and ends the task, as _next_epoch does.
The re-probe holds one MAX_IN_FLIGHT slot for the whole probe, including its wait between the two reads.
The missing-configuration error is logged once per transition, not every session.

The branch is also rebased onto main after #514. The full suite passes (4689), and a fresh review pass came back clean.

Babissimo · 2026-09-22T17:25:26Z

On the second review round:

Fixed. _next_epoch's missing-configuration hold and _check_config's now share one helper, so the error is logged once per transition from either path. It was worse than described: because the silent hold also set the reason, the next session's gated log stayed quiet as well, so the state was never logged. A test covers it and failed first.
Left as it is. _same() is reached in the same epoch only when the fingerprint matches, and the fingerprint covers lat, lon, altitude and fc. Those values are therefore the same floats parsed from the same config text, so there is no noise near zero to absorb. The tolerance exists for the metres-to-feet conversion, where a relative bound is the right one.
Raised with the ticket owner rather than changed. A name that answers one address per query, rotating across networks, would start an epoch whenever the answer changes network. The likelier case is an operator behind a tunnel or proxy service, where the "address" is the provider's and moves freely. This follows from the agreed /16 rule, so it is a design decision, not a bug in this change.

Babissimo · 2026-09-22T17:32:06Z

On _same() near zero, raised a second time, now for a sea-level site: this is left as it is, and the case is now covered by a test.

Conversion noise is relative, not absolute. _feet(m) is a multiplication, so a round trip moves the result by ULPs of its own magnitude, and rel_tol absorbs that at 0.05 m as it does at 120 ft. 0 m converts to exactly 0 ft. test_a_declaration_within_float_noise_of_the_held_value_keeps_it now runs at 12 m, 0.05 m and 0 m, both unchanged and with relative noise, and all three pass. In the same-epoch path the altitude text is identical anyway, because the fingerprint covers it.

The DNS point stays with the ticket owner, as noted above.

Babissimo · 2026-09-22T17:47:45Z

On the third round:

The 0 case is trivial, and that is intended. At zero there is no relative noise to absorb. It pins that 0 m converts to exactly 0 ft and compares equal, and the non-zero cases carry the noise check. An additive perturbation would test absolute noise, which a multiplication cannot produce.
The _next_epoch path does not need it either. A value whose text did not change parses to the same float as last time, and a value that did change is a real change. js-yaml serves blah2's numbers as shortest round-trip reprs, so no noise enters, and a version written there comes with a new epoch anyway.

Leaving both as they are. No code change this round.

Nothing on a stock blah2 box proves which radar is answering, so trust in a polled radar rides on its epoch. Until now nothing moved it: a radar could be re-sited, retuned or swapped behind the same id and keep its graduation and its old geometry. Each poll session now begins by reading /api/config. A changed fingerprint (sites, site names, fc) is re-probed. A pass starts a new epoch on probation, adopts the radar's declaration as a new node_configs version when geometry or fc moved, and records the probe's address. A refusal files nothing and is retried next session, since frames would otherwise be filed against a declaration the epoch does not hold. fs or CPI changing alone is a new version in the same epoch, and a config that omits them keeps the values held. A name that resolves into another network (/16 for IPv4, /32 for IPv6, within one family) is re-probed too, but keeps its epoch and its graduation. An address is the operator's ISP or proxy moving them about: home connections and tunnels change network on their own, and an address says nothing about which box answers, so spending a graduation on one would fence exactly the operators this door is for. The move is counted on the row instead (migration 0017), as evidence for the trust layer to weigh (123zgec4bxx), and the address is recorded whether or not the re-probe passes, so the same move is not probed again every session. The network check compares against every address the name resolves to, not only the one that answered, so a hostname with records in two networks does not re-probe every session. A config that no longer parses as stock blah2, or declares what the validator refuses, reads as stalled rather than unreachable: the radar is answering, just not with anything that can be filed. The poller, not the radar's task, hands the pipeline the new configuration when it restarts a radar for a new epoch or version. The task's own commit changes its target, so any sync after it may cancel the task before it could re-register, which would leave the pipeline on the old geometry until a restart. probed_config maps a probe onto a configuration version so the registration route (subtask 4) can reuse it. ClickUp 123zgec4baz (PR 2 of 3). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The clock-offset test looked for the substring "clock offset -3" in the stalled log line, assuming the offset reads as -30.x. A full-suite run logged -29.941 s: the wall clock stepped back about 60 ms between the stub stamping the frame and the poller reading it, which macOS does when it corrects the clock under load. The line was right and the check failed on its digits. It now parses the logged offset and allows the same 2 s the test already allows for the poller's own reading. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

claude · 2026-09-23T09:12:02Z

Claude finished @Babissimo's task in 1m 11s —— View job

Code Review — PR #534

Todo list

Gather context (PR diff, CLAUDE.md, comment history)
Run code review against current PR state (7365491 + 4b0d7f2)
Post findings

I independently re-traced the current state of feat/polled-radar-epochs (13 files, +1067/-86, matching the PR description exactly) rather than just re-reading the prior review's conclusions.

backend/services/blah2_poller.py

moved_network (line 134): filters current to previous's IP family before comparing, and returns False on an empty/unset previous — matches the "compared within family only" and "no previous means not moved" design.
_check_config's dispatch (declaration_changed → _next_epoch; else moved → _address_moved, then always _same_epoch) matches the PR's outcome table. When both a fingerprint change and a network move happen in the same session, only _next_epoch runs and no network_moves row update happens for that session — consistent with the table (a fingerprint change is unconditionally a re-probe/epoch event, and _next_epoch writes last_resolved_ip itself either way).
_next_epoch and _same_epoch both guard their epoch-scoped UPDATE with a rowcount check and roll back to SUPERSEDED if another writer moved the epoch first. _address_moved's update is unguarded, but that's harmless: the unconditional fall-through into _same_epoch re-validates the epoch and discards a stale write via its own guard, as the previous review round already established.
_unconfigured/_unholdable share the _held(reason) gate (self.reason != reason) so the "no configuration" / "invalid configuration" log lines fire once per transition, from either the _check_config or _next_epoch path.
Re-registration: Poller.sync() calls _rejoin (→ register_with_pipeline) from the stopping loop before the second loop starts a new task for the same node_id, so a sync racing the epoch-commit can't skip registration, matching the PR's stated reasoning.

backend/services/polled_radars.py

probed_config/_same: field set matches Blah2Config exactly; _same(None, value) treats a currently-unset base field as changed; _feet(0.0) stays exactly 0.0, so the multiplicative-noise argument for the rel_tol choice holds at the zero-altitude edge case raised earlier in review.

Migration 0017 — additive, and the downgrade batch-drops both columns in one pass (needed for SQLite's table-rewrite semantics); test_0017_downgrade_drops_the_network_move_columns covers that the rewrite preserves the unique endpoint_key index and the nodes FK.

node_config_store.py / blah2_probe.py / polled_endpoint.py — active_config/config_fields extraction and the .addresses rename are mechanical, no stale internal references found.

Second commit (4b0d7f2) — replaces the substring match "clock offset -3" with a parse of the logged value compared via pytest.approx(-30, abs=2), matching the tolerance the test already allows elsewhere. Minimal, addresses the flake directly (wall-clock step landing on -29.941 instead of -30.x).

No new findings — this matches the conclusion of the prior review rounds, and I did not find anything they missed.
(branch)