20260811 - Antenna geometry optional, cpi_s withdrawn, Q8 closed - #11
Merged
Merged
Conversation
Q3 asked the server author to add cpi_s to NodeConfig on two grounds. Neither survived checking. The one this document leaned on hardest was gap detection: t[n+1] - t[n] > cpi_s means capture was dropped in between, which nothing else reveals since seq stays contiguous through capture loss. The statement is true and the detector is useless. Processing exceeds the CPI on every node measured, so the condition holds for every frame — 27 of 27 gaps in a captured run, minimum 0.866 s against a 0.5 s CPI. It describes the permanent state of the fleet rather than an event, and would only discriminate once processing fits inside the CPI, at which point there would be nothing left to detect. The other was correcting the window-edge offset. cpi is 0.5 across the whole fleet, so the offset is a constant the server can be told once in prose, worth ±0.25 s inside a 4 s association window. The two-nodes-on-different-cpi case that motivated it is hypothetical; we have no such nodes. What remains is that cpi_s would let the server compute coverage fraction — 56% mean over that run, ranging 46-58%. Real, but a want for them to express rather than a field for us to lobby for. Same discipline as "the spec is the scope", applied outbound. Q3 keeps the half that costs nothing: their description of t is ambiguous, we send the end, they confirm. And it gains the fact that prompted the review — "2 Hz, fixed" cannot be honoured, and not merely because processing is slow today. There is no send cadence in this service at all: one POST per frame blah2 produces, no timer, no batching, at most one in flight. The arrival rate at the server therefore is the radar's frame rate, ~1.13 Hz here and varying with load and hardware. That is structural rather than a current condition, so it belongs in the contract discussion rather than in a list of node quirks. Also corrects three things found while tracing the claim: - wire/__init__.py's provenance table still listed the singular `agreement`. That table is the one place claiming to be exhaustive, which makes it the worst place to be stale. It now carries the three records, plus the array truncation and the adsb_hex pattern the revision added. - CLAUDE.md justified out-of-spec collection with two examples that no longer exist: cpi_s seeded a staleness window since removed, and truth.adsb.enabled turned out redundant because the presence of the adsb key is the flag. The rule stands; the examples are replaced with delay_max_bins, which is the only thing that currently qualifies, since max_range_km derives from it. - wire/detection.py repeated the "invisible any other way" claim in a docstring. Correspondence with the server author is gitignored. Those drafts get rewritten between sends and would otherwise read as a second, competing spec; the durable record is docs/open-questions.md, which is where an answer should land. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Josh's decision, and withdrawn from the outbound draft before it was ever sent. retina-tracker keeps producing track lifecycle events as JSONL for GUI preview and Auto-Calibrate, and they stay on the node. Recorded rather than deleted, because the question was a real one and its answer shapes the architecture. A second, lower-rate, must-not-lose stream is the expensive thing to retrofit — it is a different transport discipline from latest-wins, and would have meant a spool. Leaving this container detections-only keeps one discipline throughout, so a later reader should be able to see that was chosen rather than overlooked. Moves Q8 from WIRE into Answered, and corrects the three places that described it as pending: CLAUDE.md's sibling-repo table, the plan's "deliberately not doing" list, and the data-sources pointer at retina-gui's JSONL tailer, which is now framed as a starting point for whoever reopens it rather than as a plan. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ise.py
Both beam fields leave NodeConfig.required in docs/node-ingest-v1.yml, agreed
with the server author and relayed by Josh. That is the second time the spec has
been edited on our side and the exception is now named in CLAUDE.md's working
agreements: the change lives in our copy, not theirs, so the next revision they
send will not carry it unless they made it upstream too.
An uncharacterised antenna now omits both keys. Nothing is substituted, which is
the same discipline as the consent records — no value the node did not give us
reaches the server. retina-gui is not collecting the geometry from owners for the
foreseeable future, so absent is the fleet-wide steady state rather than a gap
awaiting cleanup, and the docstrings and tests say so rather than treating it as
temporary.
Two earlier designs are recorded in Q1 so they are not re-derived. The first
raised IncompleteConfig, on the grounds that a guessed beam width is worse than a
node that will not register; correct while the field was mandatory. The second
defaulted an unset width to 360, on the grounds that refusing traded a silent
misreport for a silent node; still a claim the node had not made.
wire/serialise.py goes with it. It existed for exactly one field:
beam_azimuth_deg was required AND nullable, so null was a value the server
expected rather than an absence, and exclude_none=True dropped the key and
produced a payload the server rejects. The spec now has no such field, which made
to_wire and exclude_none=True the same function. Payloads go out as
model_dump(mode="json", exclude_none=True) at every call site.
The accepted risk is that a future revision reintroducing a required-nullable
field would break payloads silently. tests/wire/test_payload_encoding.py turns
that into a loud failure, and both canaries were verified to fire against the
regression they guard rather than merely passing today:
- no payload field is both required and nullable, which is what makes
exclude_none=True safe. The check compares against the union's members rather
than the annotation's text, because adsb_hex is list[AdsbHexItem | None] —
nullable items inside a list, which exclude_none never touches.
- every payload survives json.dumps. mode="json" is load-bearing and no longer
owned by a helper: without it the acceptance timestamps stay as datetime
objects and registration fails at send time with nothing having validated it.
IncompleteConfig is deleted rather than whitelisted, since nothing raised it once
the refusal went and the dead-code gate would have caught it. Its guard in
_send_config becomes "except ValueError", which is wider than what it replaces:
pydantic's ValidationError is a ValueError, so a config violating the spec's own
bounds — a latitude past 90, an fc_hz below the minimum — is now reported instead
of killing the config loop.
Verified against a tracked-files-only tree, since the working tree carries
gitignored drafts CI never sees: tests, ruff, the dead-code gate and the model
drift check all pass, and all four beam-field combinations round-trip 200 through
the mock on both register and config.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three commits, each a decision that reduced scope rather than added it.
6307e8a— Withdraw thecpi_srequestQ3 was about to ask the server author to add
cpi_stoNodeConfig. Josh pushedback — "cpi_s is 0.5s on the whole fleet, what does it buy?" — and neither
justification survived checking.
The one it leaned on hardest was gap detection:
t[n+1] − t[n] > cpi_srevealsdropped capture, which nothing else shows because
seqstays contiguous throughit. Tested against real captured frames, it fires on 27 of 27 gaps — minimum
0.866 s against a 0.5 s CPI. Processing exceeds the CPI on every node, so it
describes the fleet's permanent state rather than an event.
The offset-correction argument was thin too:
cpiis 0.5 fleet-wide, so it is aconstant you state once in prose, worth ±0.25 s inside a 4 s association window.
What Q3 keeps is the half that costs nothing — which edge is
t? — plus thefact the question surfaced: the node has no send cadence at all. One POST per
frame blah2 produces, no timer, no batching, at most one in flight. The arrival
rate at the server therefore is the radar's frame rate, so "2 Hz, fixed" cannot
be honoured structurally, not merely because processing is slow today.
2455304— Close Q8This service does not communicate tracks. Moved to Answered rather than deleted,
because the answer shapes the architecture: a second must-not-lose stream is a
different transport discipline from latest-wins and would have meant a spool.
Worth a later reader seeing it was chosen rather than overlooked.
5f14d1c— Antenna geometry optional, andwire/serialise.pyretiredBoth beam fields leave
NodeConfig.required, agreed with the server author.An uncharacterised antenna omits both keys and nothing is substituted — the
same discipline as the consent records. retina-gui is not collecting the geometry
for the foreseeable future, so absent is the fleet-wide steady state and the
docstrings say so rather than treating it as temporary.
Two earlier designs are recorded in Q1 so they are not re-derived: raising
IncompleteConfig(correct while the field was mandatory) and defaulting to 360(still a claim the node had not made).
wire/serialise.pygoes with it. It existed for exactly one field —beam_azimuth_degwas required and nullable, sonullwas a value the serverexpected rather than an absence. With no such field left,
to_wireandexclude_none=Truebecame the same function.The accepted risk, made loud
A future revision reintroducing a required-nullable field would break payloads
silently.
tests/wire/test_payload_encoding.pyguards it, and both canarieswere verified to fire against the regression they guard, not merely to pass:
members rather than the annotation's text, because
adsb_hexislist[AdsbHexItem | None]— nullable items inside a list, whichexclude_nonenever touches. That distinction fooled a first attempt, so ithas its own test.
json.dumps.mode="json"is load-bearing and nolonger owned by a helper: without it the acceptance timestamps stay as
datetimeobjects and registration fails at send time.One guard widened, not removed
_send_configcaughtIncompleteConfigto report a bad config instead ofkilling the config loop. It is now
except ValueError— and since pydantic'sValidationErroris aValueError, it also catches a config violating thespec's own bounds. A latitude past 90 or an
fc_hzbelow the minimum previouslytook the loop down.
Verification
Run against a tracked-files-only tree, because the working tree carries
gitignored drafts CI never sees — the check that would have caught two earlier
false passes on this branch:
200through the mock on bothregisterandconfig— neither field, width only, azimuth only, bothNote for the next spec revision
docs/node-ingest-v1.ymlnow carries a change that is ours, not theirs. Ifthe server author sends another revision it will not include it unless they
applied it upstream too — check
NodeConfig.requiredwhen adopting one. Theexception is named in CLAUDE.md's working agreements and recorded in Q1.
🤖 Generated with Claude Code