Report no substrate time when there was no substrate reading - #635
Merged
Conversation
`ControlPortEnclosureObserver._unknown` stamped `clock.now()` onto the synthesized observation it emits on a PV disconnect or a clean stream end. That is a CORA time wearing a substrate label, and it is the same defect removed from the caproto adapter one layer up, in the commit that also wrote the rule it breaks: enclosure_observer.py, EnclosureObservation "An adapter with no substrate time MUST answer None rather than supply its own clock, because a synthesized time is indistinguishable from a reported one once it is written down." Writing a rule into a docstring does not enforce it. The absent-stamp pin added alongside that rule covers only the four ControlPort adapters and structurally cannot see EnclosureObserver implementors. Harmless today only because the seam discards the field: the next slice stops discarding it. At 2-BM the inversion would be total rather than occasional. Both PSS permit PVs report an undefined EPICS stamp, so every REAL reading already yields None there, while the monitor re-subscribes every 5 seconds and each cycle's stream-end would carry a real-looking time. The substrate-time column would be populated exactly when the substrate said nothing, and NULL exactly when it spoke. Removing the stamp makes the `clock` dependency dead, so it goes too, along with `clock=deps.clock` at the composition root. When CORA learned of the disconnect is still recorded: the event's `occurred_at` carries it. `AlwaysPermittedEnclosureObserver` had the same shape in sentinel form, a fixed 1970 date chosen for determinism across timezones. None is equally deterministic and cannot be mistaken for a reading. Its docstring also claimed a "first-boot no observer wired" code path; grep finds no such wiring, so that claim is corrected rather than preserved. Two tests pinned the old behaviour and were correct when written. They now pin absence, with the reasoning attached. Two more are added for the gap that let this through: no test anywhere drove a reading with no substrate time through the bridge. They are paired, absent and present, so neither direction can regress into a constant. Found by a four-lens gate review of the next slice's design. All four reviewers reported it independently, from different starting points, before any of them saw another's findings. Verified: 30561 unit + architecture pass, pyright clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||
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.
Found by a four-lens gate review of the next slice's design. All four reviewers
reported it independently, from different starting points, before any of them
saw another's findings.
ControlPortEnclosureObserver._unknownstampedclock.now()onto thesynthesized observation it emits on a PV disconnect or clean stream end. That is
a CORA time wearing a substrate label — the same defect removed from the caproto
adapter one layer up, in the very commit that wrote the rule it breaks:
Writing a rule into a docstring does not enforce it. The absent-stamp pin added
alongside that rule (
test_absent_timestamp_mapping.py) covers only the fourControlPort adapters and structurally cannot see
EnclosureObserverimplementors.Why it matters now
Harmless today only because the seam discards the field. The next slice stops
discarding it.
At 2-BM the inversion would be total rather than occasional. Both PSS permit PVs
report an undefined EPICS stamp, so every real reading already yields
Nonethere, while the monitor re-subscribes every 5 seconds and each cycle's
stream-end would carry a real-looking time. The substrate-time column would be
populated exactly when the substrate said nothing, and NULL exactly when it spoke.
Changes
_unknownreportsNone. This makes theclockdependency dead, so it goes,along with
clock=deps.clockat the composition root. When CORA learned of thedisconnect is still recorded, on the event's
occurred_at.AlwaysPermittedEnclosureObserverhad the same shape in sentinel form, a fixed1970 date chosen for determinism.
Noneis equally deterministic and cannot bemistaken for a reading.
finds no such wiring, so the claim is corrected rather than preserved.
Tests
Two pinned the old behaviour and were correct when written; they now pin absence
with the reasoning attached. Two are added for the gap that let this through: no
test anywhere drove a reading with no substrate time through the bridge. They are
paired, absent and present, so neither direction can regress into a constant.
Verified: 30561 unit + architecture pass, pyright clean.
🤖 Generated with Claude Code