Record when the hutch changed and when the substrate said so - #637
Merged
Conversation
`proj_enclosure_summary.last_observed_at` never held an observation
time. The projection writes the event's `occurred_at` into it, which
is CORA's clock at handler-append. Its name promised the substrate's
time and delivered CORA's, and a reader could not tell which. At 2-BM
the two are far apart: both PSS permit PVs report no time at all.
Two facts now get two columns, and the substrate's time reaches them
for the first time. The seam has been computing it and dropping it.
CORA's record of the transition
last_permit_status_changed_at, last_permit_status_reason,
last_trigger
the substrate's attribution
last_source_kind, last_source_id, last_source_observed_at
Naming is not invented here. Five sibling projections already share
one skeleton for an occurred_at-sourced timestamp,
`last_<axis>_changed_at` + `last_<axis>_reason` + `last_trigger`, and
Supply is the declared precedent of this very decider. Enclosure was
the sole outlier. The axis is spelled out because Enclosure has two
status axes and `decommissioned_at` is a change on the other one.
`sampled_at` was rejected: `source` is already this BC's word for the
substrate, so the new column completes that trio rather than importing
Run's vocabulary. `recorded_at` was rejected: it means Postgres write
time and is the trust anchor.
The rename reaches four names, not two. `EnclosureLookupResult
.observed_at` is a cross-BC field Run and Operation read, fed from the
renamed column, so leaving it alone would have kept substrate-sounding
ingest time at the one place other BCs look. It becomes
`permit_status_changed_at` and its docstring now says what it is not.
`observed_at` on the command and the event has NO default. That is
deliberate: the value crosses four hops and a default is how a field
gets silently dropped at one of them. Making it required turned into
43 compile errors across 16 test files, which is 43 sites that would
otherwise have defaulted quietly. One was a test that bypasses
`__init__` with `object.__setattr__` and sets each field by hand; a
default would have left it constructing an event missing the field.
On the wire it is present-as-null, deviating from `monitor_ref`
one field over. Omitting the key would make an event written before
this field existed byte-identical to one saying the substrate gave no
time, conflating "never captured" with "nothing to capture" forever.
Both re-read sites guard with `.get()`. This is not a rebuild-only
concern: `_monitor.record_observation` re-folds the entire stream
before every decision, so an unguarded read would break the live 2-BM
permit monitor on its next transition.
DEPLOY ORDER IS THE OPPOSITE OF THE PROJECT DEFAULT, and
`docs/stack/deployment.md` said so wrongly. "Apply migrations first,
then roll the image" is right for additive DDL; for a rename the old
image keeps naming the old column, so lookups raise
UndefinedColumnError, the run-gate pre-flight fails closed, and the
projection rolls back without advancing its bookmark. Stop, apply,
start. CI cannot warn: the destructive-DDL scan matches DROP and ALTER
COLUMN TYPE, not RENAME COLUMN. The doc now carries the caveat for
every future rename, not just this one.
No backfill. Every existing row already holds ingest time, so the
rename relabels rather than moves. `last_source_observed_at` is NULL
for historic rows and that is the true value: the payloads were
written before the field existed, so there is nothing to recover and
nothing to fake.
Shaped by a four-lens gate review plus a naming review. Both author
candidate names were rejected on precedent, the "repoint" half of the
design turned out to describe work that did not exist, and the
deploy-doc defect was found by a reviewer looking somewhere I was not.
Verified: no-IO lane 46692 passed, 99 2-BM scenarios, 22 enclosure
Postgres integration, pyright clean, atlas.sum rechained with no
cascade.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Coverage report
Click 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
1 participant
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.
Stacked on #635, now rebased onto main (#635 landed as
8e36634d22). This is P1 of [[project-source-timestamp-design]]; #631 shipped P2.proj_enclosure_summary.last_observed_atnever held an observation time. The projection writes the event'soccurred_atinto it, which is CORA's clock at handler-append. The name promised the substrate's time and delivered CORA's, and a reader could not tell which. At 2-BM the two are far apart: both PSS permit PVs report no time at all.Two facts now get two columns, and the substrate's time reaches them for the first time. The seam has been computing it and dropping it.
Naming is precedent, not invention
Five sibling projections already share one skeleton for an
occurred_at-sourced timestamp (last_<axis>_changed_at+last_<axis>_reason+last_trigger), and Supply is the declared precedent of this very decider. Enclosure was the sole outlier. The axis is spelled out because Enclosure has two status axes anddecommissioned_atis a change on the other one.sampled_atwas rejected becausesourceis already this BC's word for the substrate;recorded_atwas rejected because it means Postgres write time and is the trust anchor.The rename reaches four names, not two.
EnclosureLookupResult.observed_atis a cross-BC field Run and Operation read, fed from the renamed column, so leaving it alone would have kept substrate-sounding ingest time at the one place other BCs look.Why the new field has no default
observed_atcrosses four hops, and a default is how a field gets silently dropped at one of them. Making it required surfaced 43 compile errors across 16 test files, which is 43 sites that would otherwise have defaulted quietly. One was a test that bypasses__init__withobject.__setattr__; a default would have left it constructing an event missing the field.On the wire it is present-as-null, deviating from
monitor_refone field over. Omitting the key would make an event written before this field existed byte-identical to one saying the substrate gave no time, conflating "never captured" with "nothing to capture" forever. Both re-read sites guard with.get(), and that is not a rebuild-only concern:_monitor.record_observationre-folds the entire stream before every decision, so an unguarded read would break the live 2-BM permit monitor on its next transition.Deploy order is the opposite of the project default
docs/stack/deployment.mdsaid so wrongly. "Apply migrations first, then roll the image" is right for additive DDL; for a rename the old image keeps naming the old column, so lookups raiseUndefinedColumnError, the run-gate pre-flight fails closed, and the projection rolls back without advancing its bookmark. Stop, apply, start. CI cannot warn here: the destructive-DDL scan matchesDROPandALTER COLUMN TYPE, notRENAME COLUMN. The doc now carries the caveat for every future rename.No backfill. Every existing row already holds ingest time, so the rename relabels rather than moves.
last_source_observed_atis NULL for historic rows and that is the true value.Review and verification
Shaped by a four-lens gate review plus a naming review. Both author candidate names were rejected on precedent, the "repoint" half of the design turned out to describe work that did not exist, and the deploy-doc defect was found by a reviewer looking somewhere the author was not.
Verified: no-IO lane 46692 passed, 99 2-BM scenarios, 22 enclosure Postgres integration, pyright clean,
atlas.sumrechained with no cascade. Re-verified after the rebase onto current main; zero file overlap with #632/#633/#634/#635.🤖 Generated with Claude Code