diff --git a/apps/api/openapi.json b/apps/api/openapi.json index a05c75bad02..b3e33402455 100644 --- a/apps/api/openapi.json +++ b/apps/api/openapi.json @@ -11660,7 +11660,7 @@ }, "evidence": { "additionalProperties": true, - "description": "Capture-specific evidence (freeform placeholder today; shape-only validated). Per-Family evidence schemas are a future slice.", + "description": "Capture-specific evidence: frame accounting and provenance (reader_kind, checksum_computer_kind, captured_at_source, frame counts by role, the angle range). Validated against AcquisitionEvidence's known keys; unknown keys are rejected. Empty means no evidence supplied.", "title": "Evidence", "type": "object" }, diff --git a/apps/api/src/cora/data/aggregates/acquisition/__init__.py b/apps/api/src/cora/data/aggregates/acquisition/__init__.py index e2d967478fd..802c05a9872 100644 --- a/apps/api/src/cora/data/aggregates/acquisition/__init__.py +++ b/apps/api/src/cora/data/aggregates/acquisition/__init__.py @@ -27,8 +27,10 @@ AcquisitionAlreadyExistsError, AcquisitionAssetNotFoundError, AcquisitionCannotRecordWithoutCapturingError, + AcquisitionEvidence, AcquisitionRunNotFoundError, AcquisitionStatus, + CapturedAtSource, InvalidAcquisitionCapturedAtError, InvalidAcquisitionEvidenceError, InvalidAcquisitionSettingsError, @@ -42,9 +44,11 @@ "AcquisitionAssetNotFoundError", "AcquisitionCannotRecordWithoutCapturingError", "AcquisitionEvent", + "AcquisitionEvidence", "AcquisitionRecorded", "AcquisitionRunNotFoundError", "AcquisitionStatus", + "CapturedAtSource", "InvalidAcquisitionCapturedAtError", "InvalidAcquisitionEvidenceError", "InvalidAcquisitionSettingsError", diff --git a/apps/api/src/cora/data/aggregates/acquisition/events.py b/apps/api/src/cora/data/aggregates/acquisition/events.py index 567eadcbfa7..3d145585c8f 100644 --- a/apps/api/src/cora/data/aggregates/acquisition/events.py +++ b/apps/api/src/cora/data/aggregates/acquisition/events.py @@ -25,7 +25,10 @@ - UUIDs serialize as strings; the optional `producing_run_id` serializes as null when None. - `settings` and `evidence` are JSON objects on disk (may be `{}` - empty but never None). + empty but never None). `evidence` is `AcquisitionEvidence` + in-memory, not a bare dict: an evidence field left None is + OMITTED from the object, never nulled, same as + `ingest_scan.EVIDENCE_SCHEMA` before this VO replaced it. - Datetimes serialize via `.isoformat()`. - Status is NOT carried in the payload; the event type encodes it (AcquisitionRecorded -> RECORDED), same precedent as the rest of @@ -38,11 +41,16 @@ `occurred_at`, `recorded_by`. """ -from dataclasses import dataclass +from dataclasses import dataclass, fields from datetime import datetime from typing import Any, assert_never from uuid import UUID +from cora.data.aggregates.acquisition.state import ( + AcquisitionEvidence, + CapturedAtSource, + validate_evidence, +) from cora.infrastructure.event_payload import deserialize_or_raise from cora.infrastructure.ports.event_store import StoredEvent from cora.shared.identity import ActorId @@ -56,11 +64,17 @@ class AcquisitionRecorded: Status is implicit (`Recorded`); the evolver sets it. This is the only event the Acquisition aggregate ever emits. - Per CONTRIBUTING.md "Primitives in event payloads": every field - here is a primitive (str, int, UUID, datetime) or a JSON-object - carrier dict (`settings`, `evidence`). The dual-time pair carries - `captured_at` (instrument wall-clock) alongside `occurred_at` - (CORA-side wall-clock; the in-memory state field is `recorded_at`). + Per `docs/reference/modeling.md`'s event-VO carve-out: `evidence` + is `AcquisitionEvidence`, not degraded to a bare `dict`, because + the record exporter's generator recurses into a declared VO field + by field but must drop an untyped `dict` opaque whole; see that VO + for why it still recurses (not `ClosedValueObject`) rather than + being kept whole. Every other field is a primitive (str, int, UUID, + datetime) or `settings`, a JSON-object carrier dict with no known + shape yet (see `Acquisition`'s "Settings and evidence" docstring). + The dual-time pair carries `captured_at` (instrument wall-clock) + alongside `occurred_at` (CORA-side wall-clock; the in-memory state + field is `recorded_at`). Fold-symmetry attribution (every-fact-has-an-actor): - `recorded_by: ActorId`: the envelope `principal_id` of the @@ -75,7 +89,7 @@ class AcquisitionRecorded: producing_run_id: UUID | None captured_at: datetime settings: dict[str, Any] - evidence: dict[str, Any] + evidence: AcquisitionEvidence occurred_at: datetime recorded_by: ActorId @@ -114,7 +128,7 @@ def to_payload(event: AcquisitionEvent) -> dict[str, Any]: ), "captured_at": captured_at.isoformat(), "settings": settings, - "evidence": evidence, + "evidence": _evidence_to_payload(evidence), "occurred_at": occurred_at.isoformat(), "recorded_by": str(recorded_by), } @@ -122,6 +136,27 @@ def to_payload(event: AcquisitionEvent) -> dict[str, Any]: assert_never(event) +def _evidence_to_payload(evidence: AcquisitionEvidence) -> dict[str, Any]: + """Wire shape for `AcquisitionEvidence`: a JSON object with a key + per non-None field, omitted (never nulled) when absent, matching + the `ingest_scan.EVIDENCE_SCHEMA` convention this VO replaced. + + Iterates `dataclasses.fields` rather than a hand-maintained field + list: a second, separately-maintained list here previously could + silently drop a field from every published record if it fell out + of sync with the dataclass (an addition to `AcquisitionEvidence` + forgotten here), with no test able to catch it short of asserting + every field by name. + """ + payload: dict[str, Any] = {} + for f in fields(evidence): + raw = getattr(evidence, f.name) + if raw is None: + continue + payload[f.name] = raw.value if isinstance(raw, CapturedAtSource) else raw + return payload + + def from_stored(stored: StoredEvent) -> AcquisitionEvent: """Rebuild an Acquisition event from a StoredEvent loaded from the store. @@ -144,12 +179,12 @@ def _build_recorded() -> AcquisitionRecorded: ), captured_at=datetime.fromisoformat(payload["captured_at"]), settings=dict(payload["settings"]), - evidence=dict(payload["evidence"]), + evidence=validate_evidence(payload["evidence"]), occurred_at=datetime.fromisoformat(payload["occurred_at"]), recorded_by=ActorId(UUID(payload["recorded_by"])), ) - return deserialize_or_raise("AcquisitionRecorded", _build_recorded) + return deserialize_or_raise("AcquisitionRecorded", _build_recorded, extra=(ValueError,)) case _: msg = f"Unknown AcquisitionEvent event_type: {stored.event_type!r}" raise ValueError(msg) diff --git a/apps/api/src/cora/data/aggregates/acquisition/evolver.py b/apps/api/src/cora/data/aggregates/acquisition/evolver.py index b1a268f7730..6401ef0e36e 100644 --- a/apps/api/src/cora/data/aggregates/acquisition/evolver.py +++ b/apps/api/src/cora/data/aggregates/acquisition/evolver.py @@ -46,9 +46,10 @@ def evolve(state: Acquisition | None, event: AcquisitionEvent) -> Acquisition: producing_asset_id=producing_asset_id, producing_run_id=producing_run_id, captured_at=captured_at, - # Defensive copies so mutating either side cannot alias. + # Defensive copy so mutating one side cannot alias the + # other; evidence is a frozen VO, already immune to this. settings=dict(settings), - evidence=dict(evidence), + evidence=evidence, recorded_at=occurred_at, recorded_by=recorded_by, status=AcquisitionStatus.RECORDED, diff --git a/apps/api/src/cora/data/aggregates/acquisition/state.py b/apps/api/src/cora/data/aggregates/acquisition/state.py index 6ef24a6f938..017acdda02a 100644 --- a/apps/api/src/cora/data/aggregates/acquisition/state.py +++ b/apps/api/src/cora/data/aggregates/acquisition/state.py @@ -54,9 +54,21 @@ `settings` is a carrier dict validated for primitive-leaf shape today; per-Family schema validation against the producing Asset's -Family.settings_schema is deferred. `evidence` is a freeform -placeholder dict (primitive-leaf shape only); per-Family evidence -schemas are deferred until operator demand surfaces a distinct shape. +Family.settings_schema is deferred. No real writer populates it with +anything beyond ad hoc test fixtures (`ingest_scan` always sends +`{}`), so there is no real shape yet to type; forcing a VO onto zero +production data would be inventing structure ahead of the operator +demand the module already says it is waiting for. Revisit when a +Family.settings_schema exists to type against. + +`evidence` is `AcquisitionEvidence`, not a freeform dict: it carries +the one real shape a writer produces today (`ingest_scan`'s frame +accounting), so the record exporter's generated disposition table can +resolve `projection_count`, the angle range, and `captured_at_source` +field by field instead of dropping the whole carrier opaque. See +`AcquisitionEvidence` and `validate_evidence` for the shape and the +writer-unification this replaced (`ingest_scan.EVIDENCE_SCHEMA`, a +JSON-schema copy of the same rules enforced a second time). """ from collections.abc import Mapping @@ -125,12 +137,11 @@ def __init__(self, reason: str) -> None: class InvalidAcquisitionEvidenceError(ValueError): - """The supplied evidence dict has a malformed shape. + """The supplied evidence dict does not fit `AcquisitionEvidence`. - Shape-only check today: evidence must be a mapping whose leaves - are JSON-primitives (or nested lists / dicts of them). Per-Family - evidence schemas are deferred. Symmetric pair with the settings - shape check. + Raised for an unknown key, a wrong-typed value, or a + `captured_at_source` outside `CapturedAtSource`. See + `validate_evidence`, the sole declarer of the shape. Mapped to HTTP 400. """ @@ -210,10 +221,121 @@ def __init__(self, asset_id: UUID) -> None: self.asset_id = asset_id +class CapturedAtSource(StrEnum): + """Which of the file's candidate timestamps `captured_at` came from. + + A layout can offer several timestamps and a deployment's writer can + be wrong about one of them: 2-BM's `start_date` is measurably the + PREVIOUS scan's end, while its `end_date` is correct to within + seconds (see `ScanReader.Description.captured_at_source`'s own + docstring). The published record needs to say which one it + believed rather than leave a reader to assume. + + `Description.captured_at_source` is deliberately a plain `str` at + the port so a future layout can name a timestamp no reader has + produced yet; that is a port-level extensibility promise this enum + does not honor. It does not narrow anything new, though: + `ingest_scan.EVIDENCE_SCHEMA` already closed this same set via a + JSON-schema enum before this VO existed, so a layout naming a + fourth source already refused evidence before this change, just + with a JSON-schema error instead of this enum's ValueError. A + layout that legitimately needs a fourth name must widen this enum + in the same change, or ingest keeps refusing it. + """ + + START_DATE = "start_date" + END_DATE = "end_date" + OPERATOR = "operator" + + +@dataclass(frozen=True) +class AcquisitionEvidence: + """Frame accounting and provenance for one recorded capture. + + Declares the one real shape a writer produces today (`ingest_scan`, + the 2-BM pilot's scan ingest): frame counts by role, the projection + angle range, and which of the file's timestamps was believed. + Supersedes `ingest_scan.EVIDENCE_SCHEMA`, a JSON-schema copy of the + same rules enforced a second time at the ingest boundary; this VO + is now the sole declarer, checked once by `validate_evidence` on + both the `ingest_scan` and direct `record_acquisition` write paths. + + Every field is independently optional: 0 means verified-none, None + means the source cannot know, and a consumer must not collapse the + two (mirrors `ScanReader.Description`'s own zero-versus-None + convention). `AcquisitionEvidence()`, every field None, is the "no + evidence supplied" state, replacing the old empty-dict `{}` + sentinel; the wire rendering is the same empty JSON object either + way (see `to_payload`). + + Declared directly on `AcquisitionRecorded` (not degraded to `dict` + first) so the record exporter's generator recurses into it field by + field instead of dropping the whole carrier opaque; see + `docs/reference/modeling.md`'s event-VO carve-out. This does NOT + make it a `ClosedValueObject`: `reader_kind`, `checksum_computer_kind` + and `captured_at_raw` are open-vocabulary strings with no closed + range (a new adapter, or a new file layout, can introduce a value + this VO has never seen), so they recurse and drop like any other + free text, the same posture as `DatasetEncoding`'s `media_type`. + Only `captured_at_source` closes, via `CapturedAtSource`. + + Unlike `DatasetChecksum` / `DatasetEncoding`, this VO has no + `__post_init__`: it is a plain typed carrier, not self-validating. + `validate_evidence` is what actually enforces the shape (unknown + keys, wrong types, a closed `captured_at_source`); constructing + `AcquisitionEvidence(...)` directly bypasses all of it, the same + way constructing a dataclass always bypasses a validator that + lives outside `__init__`. Every real call site goes through + `validate_evidence`; direct construction is for tests only. + + `projection_angle_first` / `projection_angle_last` are degrees + (canonical unit; see `ScanReader.Description.projection_angles_deg`), + a fact the deleted `ingest_scan.EVIDENCE_SCHEMA` carried as a + `"unit"` JSON-schema annotation that was never functionally + enforced (no validator read it) and has no equivalent here beyond + this sentence. + """ + + reader_kind: str | None = None + checksum_computer_kind: str | None = None + captured_at_source: CapturedAtSource | None = None + captured_at_raw: str | None = None + projection_count: int | None = None + flat_count: int | None = None + dark_count: int | None = None + invalid_count: int | None = None + commanded_projection_count: int | None = None + commanded_flat_count: int | None = None + commanded_dark_count: int | None = None + dropped_frame_count: int | None = None + projection_angle_count: int | None = None + projection_angle_first: float | None = None + projection_angle_last: float | None = None + + +_EVIDENCE_STR_FIELDS = ("reader_kind", "checksum_computer_kind", "captured_at_raw") +_EVIDENCE_INT_FIELDS = ( + "projection_count", + "flat_count", + "dark_count", + "invalid_count", + "commanded_projection_count", + "commanded_flat_count", + "commanded_dark_count", + "dropped_frame_count", + "projection_angle_count", +) +_EVIDENCE_FLOAT_FIELDS = ("projection_angle_first", "projection_angle_last") +_EVIDENCE_KNOWN_KEYS = frozenset( + {"captured_at_source", *_EVIDENCE_STR_FIELDS, *_EVIDENCE_INT_FIELDS, *_EVIDENCE_FLOAT_FIELDS} +) + + def _validate_carrier_shape(value: Any, *, label: str, depth: int = 0) -> None: """Recursively check that a carrier dict has only primitive leaves. - Used by both settings and evidence. Raises ValueError with a + Used by settings only (evidence has its own `AcquisitionEvidence` + shape, see `validate_evidence`). Raises ValueError with a `label`-prefixed reason on the first malformed leaf; the caller wraps that into the field-specific Invalid* error class. """ @@ -252,20 +374,83 @@ def validate_settings(value: dict[str, Any]) -> dict[str, Any]: return value -def validate_evidence(value: dict[str, Any]) -> dict[str, Any]: - """Validate the evidence carrier dict for primitive-leaf shape. +def _validate_evidence_field(value: Any, expected: type, key: str) -> Any: + """Type-check one evidence leaf, rejecting `bool` where `int` is expected + (JSON's `integer` and `boolean` are distinct types; Python's is not).""" + if not isinstance(value, expected) or (expected is int and isinstance(value, bool)): + raise InvalidAcquisitionEvidenceError( + f"{key} must be a {expected.__name__} (got {type(value).__name__})" + ) + return value + - Shape-only today (per-Family evidence schemas are deferred). The - top level must be a dict keyed by strings; leaves must be - JSON-primitives or nested containers of them. +def validate_evidence(value: dict[str, Any]) -> AcquisitionEvidence: + """Validate a caller-supplied dict and build the `AcquisitionEvidence` VO. + + Sole declarer of evidence's shape (see `AcquisitionEvidence`). + Every key is optional; an unknown key, a wrong-typed value, or a + `captured_at_source` outside `CapturedAtSource` all raise + `InvalidAcquisitionEvidenceError`. An empty dict is valid and + returns `AcquisitionEvidence()` ("no evidence supplied"). + + Used on both write paths: `record_acquisition`'s decider calls this + on the caller-supplied dict; `from_stored` calls it again on the + same dict shape read back off the wire, so a corrupted stored + payload surfaces as `Malformed AcquisitionRecorded` rather than + silently reconstructing something the aggregate never actually + validated. Matches `DatasetChecksum` / `DatasetEncoding`'s pattern + of reusing one validator on both paths, though unlike them the + validation lives here rather than in `AcquisitionEvidence.__post_init__` + (see that class's docstring). + + A rejection here (e.g. an unrecognized `captured_at_source`) embeds + the specific bad key or value in its message. Raised from the + decider that is the caller's own just-submitted value echoed back + to them, not a leak. Raised from `from_stored` on an already-stored + payload, this reproduces the same pattern `InvalidDatasetChecksumError` + already has on that path (`f"...value={value!r}"`, wrapped into + `Malformed DatasetRegistered payload` by the identical + `deserialize_or_raise(..., extra=(ValueError,))` mechanism) rather + than introducing a new one; `event_payload.py`'s no-payload-echo + convention protects the RAW PAYLOAD DICT from appearing in the + wrapper message, not the wrapped exception's own text. Scrubbing + that residual (across every VO reusing a validator this way, not + only this one) is an open campaign-wide question, not something to + special-case here. """ if not isinstance(value, dict): # pyright: ignore[reportUnnecessaryIsInstance] raise InvalidAcquisitionEvidenceError(f"must be a dict (got {type(value).__name__})") - try: - _validate_carrier_shape(value, label="evidence") - except ValueError as exc: - raise InvalidAcquisitionEvidenceError(str(exc)) from exc - return value + unknown = set(value) - _EVIDENCE_KNOWN_KEYS + if unknown: + raise InvalidAcquisitionEvidenceError(f"unknown key(s): {sorted(unknown)}") + + kwargs: dict[str, Any] = {} + for key in _EVIDENCE_STR_FIELDS: + if key in value: + kwargs[key] = _validate_evidence_field(value[key], str, key) + for key in _EVIDENCE_INT_FIELDS: + if key in value: + kwargs[key] = _validate_evidence_field(value[key], int, key) + for key in _EVIDENCE_FLOAT_FIELDS: + if key in value: + raw = value[key] + if isinstance(raw, bool) or not isinstance(raw, (int, float)): + raise InvalidAcquisitionEvidenceError( + f"{key} must be a number (got {type(raw).__name__})" + ) + kwargs[key] = float(raw) + if "captured_at_source" in value: + raw_source = _validate_evidence_field( + value["captured_at_source"], str, "captured_at_source" + ) + try: + kwargs["captured_at_source"] = CapturedAtSource(raw_source) + except ValueError as exc: + allowed = sorted(source.value for source in CapturedAtSource) + raise InvalidAcquisitionEvidenceError( + f"captured_at_source must be one of {allowed} (got {raw_source!r})" + ) from exc + return AcquisitionEvidence(**kwargs) @dataclass(frozen=True, slots=True) @@ -287,7 +472,7 @@ class Acquisition: producing_run_id: UUID | None captured_at: datetime settings: dict[str, Any] - evidence: dict[str, Any] + evidence: AcquisitionEvidence recorded_at: datetime recorded_by: ActorId status: AcquisitionStatus = field(default=AcquisitionStatus.RECORDED) diff --git a/apps/api/src/cora/data/aggregates/dataset/events.py b/apps/api/src/cora/data/aggregates/dataset/events.py index 32e29742c48..38b5d00c8ca 100644 --- a/apps/api/src/cora/data/aggregates/dataset/events.py +++ b/apps/api/src/cora/data/aggregates/dataset/events.py @@ -38,6 +38,7 @@ from typing import Any, assert_never from uuid import UUID +from cora.data.aggregates.dataset.state import DatasetChecksum, DatasetEncoding, Intent from cora.infrastructure.event_payload import deserialize_or_raise from cora.infrastructure.ports.event_store import StoredEvent from cora.shared.identity import ActorId @@ -51,19 +52,45 @@ class DatasetRegistered: cross-aggregate refs (`producing_run_id`, `subject_id`, `derived_from`) are eventual-consistency primitives. - Per CONTRIBUTING.md "Primitives in event payloads": every field - here is a primitive (str, int, UUID, datetime, frozenset of - primitives). VOs (`DatasetChecksum`, `DatasetEncoding`) are - reconstructed by the evolver on fold; the decider unwraps VOs - before constructing the event. + `checksum: DatasetChecksum` and `intent: Intent` are declared as + their real value object / enum, not unwrapped to primitives, per + the closed-vocabulary carve-out in `docs/reference/modeling.md`'s + "Primitives in events" rule: the record exporter's generated + redaction table resolves a field's disposition from its DECLARED + TYPE, so a field wrapped down to `str` on the event is unpublishable + by construction even when its own constructor already closes its + range. `DatasetChecksum` also marks itself `ClosedValueObject` (see + that module), so the exporter keeps it whole; `checksum.value` is + the record's own hex digest, the field that makes a published + record checkable against the data it describes. + + `encoding: DatasetEncoding` moved onto the event alongside + `checksum` for shape symmetry, not because it independently + qualifies for the carve-out: `DatasetEncoding` is NOT a + `ClosedValueObject` (its `media_type` is a loose, unvalidated + string), so the exporter still recurses into it field by field and + still drops `media_type`/`conforms_to` exactly as it did when they + were flat primitives. Do not cite this field as carve-out precedent. + + `to_payload`/`from_stored` still serialize the SAME on-disk shape + (`{"algorithm": str, "value": str}` / + `{"media_type": str, "conforms_to": list[str]}`) as before this + change; only the dataclass's declared shape moved to match it. Trust-level additions (additive, forward-compat): - `producing_run_end_state: str | None`: Run's terminal status captured at registration when producing_run_id is set; None otherwise. Legacy events fold cleanly with this defaulting - to None (payload.get). - - `intent: str`: trust level (Intent.value); defaults to "Trial" - on register. Legacy events fold cleanly with default "Trial". + to None (payload.get). Stays a bare `str` deliberately: the + Run BC's own `RunStatus` enum cannot be imported into + `cora.data.aggregates` (only the wider `cora.data` feature + layer may reach `cora.run.aggregates`), and mirroring it with a + Data-BC-local enum risks a decider-time `ValueError` on any + future `RunStatus` member this file has not caught up to. Left + dropping as a named, deferred decision, not an oversight. + - `intent: Intent`: trust level; defaults to `Intent.TRIAL` on + register. Legacy events fold cleanly via `payload.get("intent", + "Trial")` in `from_stored`, wrapped into the enum there. Calibration-citation addition (additive, forward-compat): - `used_calibration_ids: tuple[UUID, ...]`: revision-cited atomic @@ -87,11 +114,9 @@ class DatasetRegistered: dataset_id: UUID name: str uri: str - checksum_algorithm: str - checksum_value: str + checksum: DatasetChecksum byte_size: int - media_type: str - conforms_to: frozenset[str] + encoding: DatasetEncoding producing_run_id: UUID | None subject_id: UUID | None derived_from: frozenset[UUID] @@ -112,8 +137,18 @@ class DatasetRegistered: # upload, or a conduct with no routing table to consult). Powers the # `promote_dataset` simulator-origin guard. Forward-compat via # `payload.get("producing_actuation_kind")` for legacy streams. + # + # Stays a bare `str | None` for the same layering reason as + # `producing_run_end_state`: `cora.operation.ports.control_port` + # (where `ActuationKind` lives) is not reachable from + # `cora.data.aggregates`, and a Data-BC-local mirror enum would raise + # at the decider on any future `ActuationKind` member it has not + # caught up to. This is the record's real rehearsal-versus-live + # carrier, so it dropping is a named, deferred decision, not an + # oversight -- see `is_simulated`'s own history for what an + # unexamined default costs. producing_actuation_kind: str | None = None - intent: str = "Trial" + intent: Intent = Intent.TRIAL # Calibration BC AsShot citation; revision-cited # atomic-ID model per [[project_calibration_design]]. See state.py # for the full rationale. NO cross-BC existence check at the @@ -231,11 +266,9 @@ def to_payload(event: DatasetEvent) -> dict[str, Any]: dataset_id=dataset_id, name=name, uri=uri, - checksum_algorithm=checksum_algorithm, - checksum_value=checksum_value, + checksum=checksum, byte_size=byte_size, - media_type=media_type, - conforms_to=conforms_to, + encoding=encoding, producing_run_id=producing_run_id, subject_id=subject_id, derived_from=derived_from, @@ -252,13 +285,13 @@ def to_payload(event: DatasetEvent) -> dict[str, Any]: "name": name, "uri": uri, "checksum": { - "algorithm": checksum_algorithm, - "value": checksum_value, + "algorithm": checksum.algorithm, + "value": checksum.value, }, "byte_size": byte_size, "encoding": { - "media_type": media_type, - "conforms_to": sorted(conforms_to), + "media_type": encoding.media_type, + "conforms_to": sorted(encoding.conforms_to), }, "producing_run_id": ( str(producing_run_id) if producing_run_id is not None else None @@ -273,7 +306,7 @@ def to_payload(event: DatasetEvent) -> dict[str, Any]: str(producing_procedure_id) if producing_procedure_id is not None else None ), "producing_actuation_kind": producing_actuation_kind, - "intent": intent, + "intent": intent.value, # addition (sorted for deterministic jsonb bytes, # mirrors derived_from + Run.pinned_calibration_ids precedent). "used_calibration_ids": sorted(str(c) for c in used_calibration_ids), @@ -339,11 +372,14 @@ def _build_registered() -> DatasetRegistered: dataset_id=UUID(payload["dataset_id"]), name=payload["name"], uri=payload["uri"], - checksum_algorithm=raw_checksum["algorithm"], - checksum_value=raw_checksum["value"], + checksum=DatasetChecksum( + algorithm=raw_checksum["algorithm"], value=raw_checksum["value"] + ), byte_size=int(payload["byte_size"]), - media_type=raw_encoding["media_type"], - conforms_to=frozenset(raw_encoding["conforms_to"]), + encoding=DatasetEncoding( + media_type=raw_encoding["media_type"], + conforms_to=frozenset(raw_encoding["conforms_to"]), + ), producing_run_id=( UUID(raw_producing_run_id) if raw_producing_run_id is not None else None ), @@ -358,13 +394,13 @@ def _build_registered() -> DatasetRegistered: else None ), producing_actuation_kind=payload.get("producing_actuation_kind"), - intent=payload.get("intent", "Trial"), + intent=Intent(payload.get("intent", "Trial")), used_calibration_ids=tuple( UUID(c) for c in payload.get("used_calibration_ids", []) ), ) - return deserialize_or_raise("DatasetRegistered", _build_registered) + return deserialize_or_raise("DatasetRegistered", _build_registered, extra=(ValueError,)) case "DatasetDiscarded": return deserialize_or_raise( "DatasetDiscarded", diff --git a/apps/api/src/cora/data/aggregates/dataset/evolver.py b/apps/api/src/cora/data/aggregates/dataset/evolver.py index 5f7268ce754..79c8d730d4b 100644 --- a/apps/api/src/cora/data/aggregates/dataset/evolver.py +++ b/apps/api/src/cora/data/aggregates/dataset/evolver.py @@ -48,8 +48,6 @@ ) from cora.data.aggregates.dataset.state import ( Dataset, - DatasetChecksum, - DatasetEncoding, DatasetName, DatasetStatus, DatasetUri, @@ -65,11 +63,9 @@ def evolve(state: Dataset | None, event: DatasetEvent) -> Dataset: dataset_id=dataset_id, name=name, uri=uri, - checksum_algorithm=checksum_algorithm, - checksum_value=checksum_value, + checksum=checksum, byte_size=byte_size, - media_type=media_type, - conforms_to=conforms_to, + encoding=encoding, producing_run_id=producing_run_id, producing_procedure_id=producing_procedure_id, subject_id=subject_id, @@ -84,15 +80,9 @@ def evolve(state: Dataset | None, event: DatasetEvent) -> Dataset: id=dataset_id, name=DatasetName(name), uri=DatasetUri(uri), - checksum=DatasetChecksum( - algorithm=checksum_algorithm, - value=checksum_value, - ), + checksum=checksum, byte_size=byte_size, - encoding=DatasetEncoding( - media_type=media_type, - conforms_to=conforms_to, - ), + encoding=encoding, producing_run_id=producing_run_id, producing_procedure_id=producing_procedure_id, subject_id=subject_id, @@ -102,7 +92,7 @@ def evolve(state: Dataset | None, event: DatasetEvent) -> Dataset: # via payload.get for legacy events in from_stored). producing_run_end_state=producing_run_end_state, producing_actuation_kind=producing_actuation_kind, - intent=Intent(intent), + intent=intent, # AsShot citation set at genesis (frozenset for # in-memory equality semantics; the event carries a tuple # for deterministic wire byte ordering). diff --git a/apps/api/src/cora/data/aggregates/dataset/state.py b/apps/api/src/cora/data/aggregates/dataset/state.py index a2b78a75e4e..4005861e1aa 100644 --- a/apps/api/src/cora/data/aggregates/dataset/state.py +++ b/apps/api/src/cora/data/aggregates/dataset/state.py @@ -128,6 +128,7 @@ from uuid import UUID from cora.shared.bounded_text import bounded_name, validate_bounded_text +from cora.shared.closed_value import ClosedValueObject from cora.shared.text_bounds import REASON_MAX_LENGTH DATASET_NAME_MAX_LENGTH = 200 @@ -823,7 +824,7 @@ def __post_init__(self) -> None: @dataclass(frozen=True) -class DatasetChecksum: +class DatasetChecksum(ClosedValueObject): """Bulk-content integrity hash. Algorithm + canonical value. Deviation from Identifier VO: strict 64-char lowercase-hex value @@ -836,6 +837,14 @@ class DatasetChecksum: are identical; only the algorithm tag distinguishes them. The `(algorithm, value)` shape stays forward-compatible for adding BLAKE3 / SHA3 / etc. when a real consumer asks. + + `ClosedValueObject`: both fields are closed by construction + (`algorithm` to a 2-member set, `value` to 64 lowercase-hex chars), + so the record exporter's generated disposition table KEEPS this VO + whole rather than resolving `value` as an unbounded string. A hex + digest of file bytes discloses nothing about a person, and it is the + field that makes a published record checkable against the data it + describes. """ algorithm: str diff --git a/apps/api/src/cora/data/aggregates/distribution/events.py b/apps/api/src/cora/data/aggregates/distribution/events.py index 288d8d8bc9b..7c8257ddcc4 100644 --- a/apps/api/src/cora/data/aggregates/distribution/events.py +++ b/apps/api/src/cora/data/aggregates/distribution/events.py @@ -47,6 +47,8 @@ from typing import Any, assert_never from uuid import UUID +from cora.data.aggregates.dataset.state import DatasetChecksum, DatasetEncoding +from cora.data.aggregates.distribution.state import AccessProtocol from cora.infrastructure.event_payload import deserialize_or_raise from cora.infrastructure.ports.event_store import StoredEvent from cora.shared.identity import ActorId @@ -61,11 +63,28 @@ class DistributionRegistered: eventual-consistency primitives; the handler pre-loads each on the write path, the evolver does NOT re-verify on fold. - Per CONTRIBUTING.md "Primitives in event payloads": every field here - is a primitive (str, int, UUID, datetime). VOs (``DistributionUri``, - ``DatasetChecksum``, ``DatasetEncoding``, ``AccessProtocol``) are - reconstructed by the evolver on fold; the decider unwraps VOs before - constructing the event. + ``checksum: DatasetChecksum`` and ``access_protocol: AccessProtocol`` + are declared as their real value object / enum, not unwrapped to + primitives, for the same closed-vocabulary carve-out as + ``DatasetRegistered`` (see that event's docstring): the record + exporter's generated redaction table resolves a field's disposition + from its DECLARED TYPE, and ``DatasetChecksum`` marks itself + ``ClosedValueObject`` so the exporter keeps it whole, digest + included. + + ``encoding: DatasetEncoding`` moved onto the event alongside + ``checksum`` for shape symmetry with ``DatasetRegistered``, not + because it independently qualifies for the carve-out: it is NOT a + ``ClosedValueObject`` and the exporter still recurses into it and + still drops ``media_type``/``conforms_to`` exactly as before. Do not + cite this field as carve-out precedent. + + ``to_payload`` / ``from_stored`` still serialize the SAME on-disk + shapes as before this change (``checksum``/``encoding`` as nested + objects, ``access_protocol`` as ``AccessProtocol.value``'s bare + string, per [[project-facility-aggregate-design]] cryptographic-chain + immutability discipline); only the dataclass's declared shape moved + to match what was already on disk. Fold-symmetry attribution per [[project-fold-symmetry-design]]: ``registered_by: ActorId`` carries the envelope ``principal_id`` of @@ -78,12 +97,10 @@ class DistributionRegistered: dataset_id: UUID supply_id: UUID uri: str - checksum_algorithm: str - checksum_value: str + checksum: DatasetChecksum byte_size: int - media_type: str - conforms_to: frozenset[str] - access_protocol: str + encoding: DatasetEncoding + access_protocol: AccessProtocol occurred_at: datetime registered_by: ActorId @@ -139,11 +156,9 @@ def to_payload(event: DistributionEvent) -> dict[str, Any]: dataset_id=dataset_id, supply_id=supply_id, uri=uri, - checksum_algorithm=checksum_algorithm, - checksum_value=checksum_value, + checksum=checksum, byte_size=byte_size, - media_type=media_type, - conforms_to=conforms_to, + encoding=encoding, access_protocol=access_protocol, occurred_at=occurred_at, registered_by=registered_by, @@ -154,15 +169,15 @@ def to_payload(event: DistributionEvent) -> dict[str, Any]: "supply_id": str(supply_id), "uri": uri, "checksum": { - "algorithm": checksum_algorithm, - "value": checksum_value, + "algorithm": checksum.algorithm, + "value": checksum.value, }, "byte_size": byte_size, "encoding": { - "media_type": media_type, - "conforms_to": sorted(conforms_to), + "media_type": encoding.media_type, + "conforms_to": sorted(encoding.conforms_to), }, - "access_protocol": access_protocol, + "access_protocol": access_protocol.value, "occurred_at": occurred_at.isoformat(), "registered_by": str(registered_by), } @@ -205,17 +220,22 @@ def _build_registered() -> DistributionRegistered: dataset_id=UUID(payload["dataset_id"]), supply_id=UUID(payload["supply_id"]), uri=payload["uri"], - checksum_algorithm=raw_checksum["algorithm"], - checksum_value=raw_checksum["value"], + checksum=DatasetChecksum( + algorithm=raw_checksum["algorithm"], value=raw_checksum["value"] + ), byte_size=int(payload["byte_size"]), - media_type=raw_encoding["media_type"], - conforms_to=frozenset(raw_encoding["conforms_to"]), - access_protocol=payload["access_protocol"], + encoding=DatasetEncoding( + media_type=raw_encoding["media_type"], + conforms_to=frozenset(raw_encoding["conforms_to"]), + ), + access_protocol=AccessProtocol(payload["access_protocol"]), occurred_at=datetime.fromisoformat(payload["occurred_at"]), registered_by=ActorId(UUID(payload["registered_by"])), ) - return deserialize_or_raise("DistributionRegistered", _build_registered) + return deserialize_or_raise( + "DistributionRegistered", _build_registered, extra=(ValueError,) + ) case "DistributionDiscarded": return deserialize_or_raise( "DistributionDiscarded", diff --git a/apps/api/src/cora/data/aggregates/distribution/evolver.py b/apps/api/src/cora/data/aggregates/distribution/evolver.py index 16bcf585e45..dbbedeadbe6 100644 --- a/apps/api/src/cora/data/aggregates/distribution/evolver.py +++ b/apps/api/src/cora/data/aggregates/distribution/evolver.py @@ -32,17 +32,12 @@ from dataclasses import replace from typing import assert_never -from cora.data.aggregates.dataset.state import ( - DatasetChecksum, - DatasetEncoding, -) from cora.data.aggregates.distribution.events import ( DistributionDiscarded, DistributionEvent, DistributionRegistered, ) from cora.data.aggregates.distribution.state import ( - AccessProtocol, Distribution, DistributionStatus, DistributionUri, @@ -58,11 +53,9 @@ def evolve(state: Distribution | None, event: DistributionEvent) -> Distribution dataset_id=dataset_id, supply_id=supply_id, uri=uri, - checksum_algorithm=checksum_algorithm, - checksum_value=checksum_value, + checksum=checksum, byte_size=byte_size, - media_type=media_type, - conforms_to=conforms_to, + encoding=encoding, access_protocol=access_protocol, occurred_at=occurred_at, registered_by=registered_by, @@ -73,10 +66,10 @@ def evolve(state: Distribution | None, event: DistributionEvent) -> Distribution dataset_id=dataset_id, supply_id=supply_id, uri=DistributionUri(uri), - checksum=DatasetChecksum(algorithm=checksum_algorithm, value=checksum_value), + checksum=checksum, byte_size=byte_size, - encoding=DatasetEncoding(media_type=media_type, conforms_to=conforms_to), - access_protocol=AccessProtocol(access_protocol), + encoding=encoding, + access_protocol=access_protocol, registered_at=occurred_at, registered_by=registered_by, status=DistributionStatus.REGISTERED, diff --git a/apps/api/src/cora/data/features/ingest_scan/handler.py b/apps/api/src/cora/data/features/ingest_scan/handler.py index 02f0b4dc0a4..08f8a8b7d8f 100644 --- a/apps/api/src/cora/data/features/ingest_scan/handler.py +++ b/apps/api/src/cora/data/features/ingest_scan/handler.py @@ -22,8 +22,16 @@ policy, the digest pass, the changed-under-read guard, the natural-key duplicate check, and the cross-aggregate pre-loads. A refusal at any point leaves zero events. Decider rejections (Capturing gate, future -captured_at, non-Storage supply) then fire inside the composition and -also leave zero events, because nothing has been appended yet. +captured_at, non-Storage supply, and now evidence shape -- +`AcquisitionEvidence` validation moved from a pre-decider check here to +`record_acquisition.decide`'s `validate_evidence` call, reached last +inside the composed decider) then fire inside the composition and +also leave zero events, because nothing has been appended yet. The +only observable effect of that move: a request whose evidence AND a +dataset/distribution invariant are both violated now surfaces the +dataset/distribution error first, not the evidence one; the +all-or-nothing guarantee and the resulting HTTP 400 either way are +unaffected. ## The timestamp policy @@ -71,7 +79,6 @@ from cora.infrastructure.routing import NIL_SENTINEL_ID from cora.run.aggregates.run import load_run from cora.shared.identity import ActorId -from cora.shared.json_schema_validation import validate_values_against_schema _COMMAND_NAME = "IngestScan" _DATASET_STREAM = "Dataset" @@ -86,42 +93,6 @@ #: CORA's assertion, pinned to the DXfile paper's DOI. DATA_EXCHANGE_PROFILE = "https://doi.org/10.1107/S160057751401604X" -#: Declared shape of the frame-accounting evidence this slice records -#: (declarer-owns-schema: the ingest slice declares, the Acquisition -#: carries). Optional keys are OMITTED when the source cannot know -#: them, never nulled: absence means source-cannot-know, 0 means -#: verified-none, and a consumer must not collapse the two. -EVIDENCE_SCHEMA: dict[str, Any] = { - "type": "object", - "additionalProperties": False, - "properties": { - "reader_kind": {"type": "string"}, - "checksum_computer_kind": {"type": "string"}, - "captured_at_source": { - "type": "string", - "enum": ["start_date", "end_date", "operator"], - }, - "captured_at_raw": {"type": "string"}, - "projection_count": {"type": "integer"}, - "flat_count": {"type": "integer"}, - "dark_count": {"type": "integer"}, - "invalid_count": {"type": "integer"}, - "commanded_projection_count": {"type": "integer"}, - "commanded_flat_count": {"type": "integer"}, - "commanded_dark_count": {"type": "integer"}, - "dropped_frame_count": {"type": "integer"}, - "projection_angle_count": {"type": "integer"}, - "projection_angle_first": { - "type": "number", - "unit": {"system": "udunits", "code": "degree"}, - }, - "projection_angle_last": { - "type": "number", - "unit": {"system": "udunits", "code": "degree"}, - }, - }, -} - class DatasetByChecksumLookup(Protocol): """Digest-equality probe against the dataset read model. @@ -274,12 +245,12 @@ async def handler( # context; surface the same error its handler would. raise ProducingRunNotFoundError(command.producing_run_id) + # Built as a plain dict and validated where every RecordAcquisition + # writer's evidence is validated, `record_acquisition.decide`'s + # `validate_evidence` call inside the composed decider below (see + # `AcquisitionEvidence`); this slice no longer keeps a second, + # ingest-local copy of the same schema. evidence = _build_evidence(described, captured_at_source, scan_reader, checksum_computer) - validate_values_against_schema( - evidence, - EVIDENCE_SCHEMA, - error_class=InvalidScanFileError, - ) now = deps.clock.now() dataset_id = deps.id_generator.new_id() @@ -403,8 +374,8 @@ def _build_evidence( scan_reader: ScanReader, checksum_computer: ChecksumComputer, ) -> dict[str, Any]: - """Frame accounting per EVIDENCE_SCHEMA: omit what the source cannot - know, record who produced each fact.""" + """Frame accounting per `AcquisitionEvidence`: omit what the source + cannot know, record who produced each fact.""" evidence: dict[str, Any] = { "reader_kind": scan_reader.kind, "checksum_computer_kind": checksum_computer.kind, @@ -443,7 +414,6 @@ def _filename_of(locator: str) -> str: __all__ = [ "DATA_EXCHANGE_PROFILE", - "EVIDENCE_SCHEMA", "DatasetByChecksumLookup", "Handler", "IdempotentHandler", diff --git a/apps/api/src/cora/data/features/record_acquisition/decider.py b/apps/api/src/cora/data/features/record_acquisition/decider.py index 5121d902a1a..3d7f379084b 100644 --- a/apps/api/src/cora/data/features/record_acquisition/decider.py +++ b/apps/api/src/cora/data/features/record_acquisition/decider.py @@ -79,7 +79,7 @@ def decide( - State must be None (genesis-only) -> AcquisitionAlreadyExistsError - settings must be primitive-leaf shaped -> InvalidAcquisitionSettingsError - - evidence must be primitive-leaf shaped + - evidence must fit AcquisitionEvidence's known keys/types -> InvalidAcquisitionEvidenceError - captured_at must be tz-aware and not in the future beyond now + skew_tolerance -> InvalidAcquisitionCapturedAtError diff --git a/apps/api/src/cora/data/features/record_acquisition/route.py b/apps/api/src/cora/data/features/record_acquisition/route.py index c2776177fff..83f7e0c34a6 100644 --- a/apps/api/src/cora/data/features/record_acquisition/route.py +++ b/apps/api/src/cora/data/features/record_acquisition/route.py @@ -72,9 +72,11 @@ class RecordAcquisitionRequest(BaseModel): evidence: dict[str, Any] = Field( default_factory=dict, description=( - "Capture-specific evidence (freeform placeholder today; " - "shape-only validated). Per-Family evidence schemas are a " - "future slice." + "Capture-specific evidence: frame accounting and provenance " + "(reader_kind, checksum_computer_kind, captured_at_source, " + "frame counts by role, the angle range). Validated against " + "AcquisitionEvidence's known keys; unknown keys are rejected. " + "Empty means no evidence supplied." ), ) diff --git a/apps/api/src/cora/data/features/record_acquisition/tool.py b/apps/api/src/cora/data/features/record_acquisition/tool.py index 03196b621a8..3123c25287f 100644 --- a/apps/api/src/cora/data/features/record_acquisition/tool.py +++ b/apps/api/src/cora/data/features/record_acquisition/tool.py @@ -90,8 +90,10 @@ async def record_acquisition_tool( # pyright: ignore[reportUnusedFunction] Field( default=None, description=( - "Capture-specific evidence (freeform placeholder). Shape-" - "only validated today. Defaults to empty." + "Capture-specific evidence: frame accounting and " + "provenance. Validated against AcquisitionEvidence's " + "known keys; unknown keys are rejected. Defaults to empty " + "(no evidence supplied)." ), ), ] = None, diff --git a/apps/api/src/cora/data/features/register_dataset/decider.py b/apps/api/src/cora/data/features/register_dataset/decider.py index 3cb21e5c765..f080aa0aa6d 100644 --- a/apps/api/src/cora/data/features/register_dataset/decider.py +++ b/apps/api/src/cora/data/features/register_dataset/decider.py @@ -221,11 +221,9 @@ def decide( dataset_id=new_id, name=name.value, uri=uri.value, - checksum_algorithm=checksum.algorithm, - checksum_value=checksum.value, + checksum=checksum, byte_size=byte_size, - media_type=encoding.media_type, - conforms_to=encoding.conforms_to, + encoding=encoding, producing_run_id=command.producing_run_id, producing_procedure_id=command.producing_procedure_id, subject_id=command.subject_id, diff --git a/apps/api/src/cora/data/features/register_distribution/decider.py b/apps/api/src/cora/data/features/register_distribution/decider.py index e77a2162869..e8f960e3db0 100644 --- a/apps/api/src/cora/data/features/register_distribution/decider.py +++ b/apps/api/src/cora/data/features/register_distribution/decider.py @@ -199,21 +199,20 @@ def decide( actual_byte_size=byte_size, ) - # Step 11: emit DistributionRegistered. The event payload carries - # primitive types only per CONTRIBUTING.md "Primitives in event - # payloads"; VOs reconstructed by the evolver on fold. + # Step 11: emit DistributionRegistered. checksum / encoding / + # access_protocol are the real VOs / enum already validated above + # (see DatasetRegistered's docstring for why the event declares + # them directly rather than unwrapping to primitives). return [ DistributionRegistered( distribution_id=new_id, dataset_id=command.dataset_id, supply_id=command.supply_id, uri=uri.value, - checksum_algorithm=checksum.algorithm, - checksum_value=checksum.value, + checksum=checksum, byte_size=byte_size, - media_type=encoding.media_type, - conforms_to=encoding.conforms_to, - access_protocol=access_protocol.value, + encoding=encoding, + access_protocol=access_protocol, occurred_at=now, registered_by=registered_by, ) diff --git a/apps/api/src/cora/infrastructure/record_export/__init__.py b/apps/api/src/cora/infrastructure/record_export/__init__.py index 47ddca25f47..be48bb8421f 100644 --- a/apps/api/src/cora/infrastructure/record_export/__init__.py +++ b/apps/api/src/cora/infrastructure/record_export/__init__.py @@ -30,6 +30,7 @@ STREAMS_NAME, BundleDestinationNotEmptyError, MalformedBundleError, + ManifestRecordMismatchError, read_bundle_body, write_bundle, ) @@ -54,6 +55,9 @@ ) from cora.infrastructure.record_export._manifest import Manifest, build_manifest, capture_git_commit from cora.infrastructure.record_export._redact_tier1 import ( + FIXED_DROP_COLUMNS, + FIXED_KEEP_COLUMNS, + FIXED_TOKEN_COLUMNS, Tier1Redactor, UnknownEventTypeError, redact_tier1_payload, @@ -92,6 +96,9 @@ from cora.infrastructure.record_export._tokens import TokenMap __all__ = [ + "FIXED_DROP_COLUMNS", + "FIXED_KEEP_COLUMNS", + "FIXED_TOKEN_COLUMNS", "KNOWN_STREAM_TYPES", "LOGBOOKS_DIR", "LOGBOOKS_PAYLOAD_TYPE", @@ -111,6 +118,7 @@ "ExportedRecord", "MalformedBundleError", "Manifest", + "ManifestRecordMismatchError", "RedactedRecord", "RedactionProfileMismatchError", "RedactionResult", diff --git a/apps/api/src/cora/infrastructure/record_export/_bundle.py b/apps/api/src/cora/infrastructure/record_export/_bundle.py index 652c661b240..d2cb9f7c6cb 100644 --- a/apps/api/src/cora/infrastructure/record_export/_bundle.py +++ b/apps/api/src/cora/infrastructure/record_export/_bundle.py @@ -35,7 +35,11 @@ from pathlib import Path from typing import cast -from cora.infrastructure.record_export._hashing import TwoTierRecord +from cora.infrastructure.record_export._hashing import ( + TwoTierRecord, + hash_record, + hash_redacted_record, +) from cora.infrastructure.record_export._manifest import Manifest MANIFEST_NAME = "manifest.json" @@ -48,6 +52,7 @@ "STREAMS_NAME", "BundleDestinationNotEmptyError", "MalformedBundleError", + "ManifestRecordMismatchError", "read_bundle_body", "write_bundle", ] @@ -80,6 +85,55 @@ class MalformedBundleError(RuntimeError): """ +class ManifestRecordMismatchError(RuntimeError): + """The manifest handed to `write_bundle` does not describe the record + handed alongside it. + + `record` and `manifest` are two independent arguments with nothing + structurally binding them together: nothing before this check + verified that `manifest` was built FROM `record`. Reproduced + concretely: passing the unredacted record next to a manifest whose + `published_record_hash` was computed from the real redacted record + wrote a self-consistently-formatted, fully unredacted bundle under a + `--published` label, and the default verifier printed `OK`. + `write_bundle` recomputes whichever of H1 or H3 the manifest claims + over the record it was actually handed, before writing a single + byte, and refuses on disagreement. + """ + + def __init__(self, *, expected: str, actual: str, published: bool) -> None: + field = "published_record_hash (H3)" if published else "record_hash (H1)" + super().__init__( + f"refusing to write a bundle: the manifest's {field} is {expected!r}, but " + f"the record handed to write_bundle hashes to {actual!r}. A bundle's " + "manifest must describe the exact record written beside it." + ) + self.expected = expected + self.actual = actual + + +def _ensure_manifest_describes_record(record: TwoTierRecord, manifest: Manifest) -> None: + """Which hash applies follows `manifest.published_record_hash`: + present means this claims to be a published projection, so `record` + must hash to H3; absent means an unredacted bundle, so `record` must + hash to H1. Either branch also catches the mixed case (an unredacted + `record` beside a manifest that carries H3, or vice versa), because + the wrong-shaped record cannot reproduce the hash the manifest names. + """ + if manifest.published_record_hash is not None: + actual = hash_redacted_record(record) + if actual != manifest.published_record_hash: + raise ManifestRecordMismatchError( + expected=manifest.published_record_hash, actual=actual, published=True + ) + return + actual = hash_record(record) + if actual != manifest.record_hash: + raise ManifestRecordMismatchError( + expected=manifest.record_hash, actual=actual, published=False + ) + + def _kind_filename(kind: str) -> str: """`logbooks/.jsonl`, with `kind` rejected if it could escape. @@ -136,7 +190,11 @@ def write_bundle(record: TwoTierRecord, manifest: Manifest, destination: Path) - a directory with no `manifest.json`, which `read_bundle_body` refuses, rather than a complete-looking bundle whose row files are truncated. + + Raises `ManifestRecordMismatchError` before writing anything if + `manifest` does not describe `record`: see that error's docstring. """ + _ensure_manifest_describes_record(record, manifest) destination.mkdir(parents=True, exist_ok=True) existing = sorted(p.name for p in destination.iterdir()) if existing: diff --git a/apps/api/src/cora/infrastructure/record_export/_dispositions.py b/apps/api/src/cora/infrastructure/record_export/_dispositions.py index 066542efd54..61be2a414df 100644 --- a/apps/api/src/cora/infrastructure/record_export/_dispositions.py +++ b/apps/api/src/cora/infrastructure/record_export/_dispositions.py @@ -7,18 +7,26 @@ from the field's real type by `tools/gen_record_dispositions.py`. The vocabulary: - keep:enum: closed value set, provably reviewable. The enum is - NAMED because a human signs off the value set, and - swapping one enum for another must read as drift. - keep:number int / float / bool - keep:time datetime - token:uuid replaced with a per-export random surrogate - drop:text free text, no finite range, dropped by default - drop:opaque a dict with no declared keys, nothing to allowlist - by-value the slot is polymorphic across scalars and objects, - so no static answer exists. Apply the tier-2 leaf - rule at export time: numbers and booleans keep, - UUID-shaped strings token, other strings drop. + keep:enum: closed value set, provably reviewable. The + enum is NAMED because a human signs off the + value set, and swapping one enum for another + must read as drift. + keep:closed: a value object every one of whose fields is + closed by construction (a fixed charset and + length, a closed literal set), kept WHOLE + rather than resolved field by field. See + `cora.shared.closed_value.ClosedValueObject`. + keep:number int / float / bool + keep:time datetime + token:uuid replaced with a per-export random surrogate + drop:text free text, no finite range, dropped by default + drop:opaque a dict with no declared keys, nothing to + allowlist + by-value the slot is polymorphic across scalars and + objects, so no static answer exists. Apply the + tier-2 leaf rule at export time: numbers and + booleans keep, UUID-shaped strings token, + other strings drop. A nested mapping is a value object recursed into. A mapping whose sole key is `[]` is a fixed-length heterogeneous tuple, and its value lists @@ -28,6 +36,12 @@ key absent from its event's entry is dropped; an event type absent from this table aborts the export. The canonical hash of this mapping is the redaction profile hash recorded in the export manifest. + +A handful of entries are keyed on the WIRE key a field is actually +stored under rather than its dataclass field name, per +`gen_record_dispositions.py`'s `_OVERRIDE_WIRE_KEYS`: the two never +disagree about what ships, only about which name this table's lookup +uses to find it. """ from typing import Any @@ -37,7 +51,23 @@ "acquisition_id": "token:uuid", "captured_at": "keep:time", "dataset_id": "token:uuid", - "evidence": "drop:opaque", + "evidence": { + "captured_at_raw": "drop:text", + "captured_at_source": "keep:enum:CapturedAtSource", + "checksum_computer_kind": "drop:text", + "commanded_dark_count": "keep:number", + "commanded_flat_count": "keep:number", + "commanded_projection_count": "keep:number", + "dark_count": "keep:number", + "dropped_frame_count": "keep:number", + "flat_count": "keep:number", + "invalid_count": "keep:number", + "projection_angle_count": "keep:number", + "projection_angle_first": "keep:number", + "projection_angle_last": "keep:number", + "projection_count": "keep:number", + "reader_kind": "drop:text", + }, "occurred_at": "keep:time", "producing_asset_id": "token:uuid", "producing_run_id": "token:uuid", @@ -673,7 +703,7 @@ "audience": "drop:text", "credential_id": "token:uuid", "expires_at": "keep:time", - "facility_code": {"value": "drop:text"}, + "facility_id": {"value": "drop:text"}, "occurred_at": "keep:time", "public_material_ref": "drop:text", "purpose": "keep:enum:CredentialPurpose", @@ -724,13 +754,11 @@ }, "DatasetRegistered": { "byte_size": "keep:number", - "checksum_algorithm": "drop:text", - "checksum_value": "drop:text", - "conforms_to": "drop:text", + "checksum": "keep:closed:DatasetChecksum", "dataset_id": "token:uuid", "derived_from": "token:uuid", - "intent": "drop:text", - "media_type": "drop:text", + "encoding": {"conforms_to": "drop:text", "media_type": "drop:text"}, + "intent": "keep:enum:Intent", "name": "drop:text", "occurred_at": "keep:time", "producing_actuation_kind": "drop:text", @@ -793,14 +821,12 @@ "reason": "drop:text", }, "DistributionRegistered": { - "access_protocol": "drop:text", + "access_protocol": "keep:enum:AccessProtocol", "byte_size": "keep:number", - "checksum_algorithm": "drop:text", - "checksum_value": "drop:text", - "conforms_to": "drop:text", + "checksum": "keep:closed:DatasetChecksum", "dataset_id": "token:uuid", "distribution_id": "token:uuid", - "media_type": "drop:text", + "encoding": {"conforms_to": "drop:text", "media_type": "drop:text"}, "occurred_at": "keep:time", "registered_by": "token:uuid", "supply_id": "token:uuid", @@ -1235,7 +1261,7 @@ "direction": "keep:enum:Direction", "expires_at": "keep:time", "occurred_at": "keep:time", - "peer_facility_code": {"value": "drop:text"}, + "peer_facility_id": {"value": "drop:text"}, "permit_id": "token:uuid", "terms": { "accepted_canonicalization_versions": "drop:text", @@ -1620,21 +1646,21 @@ "run_id": "token:uuid", }, "SealInitialized": { - "facility_code": {"value": "drop:text"}, + "facility_id": {"value": "drop:text"}, "initialized_by": "token:uuid", "occurred_at": "keep:time", "offline_credential_id": "token:uuid", "online_credential_id": "token:uuid", }, "SealOnlineKeyRotated": { - "facility_code": {"value": "drop:text"}, + "facility_id": {"value": "drop:text"}, "new_online_credential_id": "token:uuid", "occurred_at": "keep:time", "rotated_by": "token:uuid", "signed_by_offline_root": "keep:number", }, "SealPointerSigned": { - "facility_code": {"value": "drop:text"}, + "facility_id": {"value": "drop:text"}, "head_hash": "drop:text", "occurred_at": "keep:time", "sequence_number": "keep:number", @@ -1643,13 +1669,13 @@ }, "SealRepublishingCompleted": { "completed_by": "token:uuid", - "facility_code": {"value": "drop:text"}, + "facility_id": {"value": "drop:text"}, "new_head_hash": "drop:text", "new_sequence_number": "keep:number", "occurred_at": "keep:time", }, "SealRepublishingStarted": { - "facility_code": {"value": "drop:text"}, + "facility_id": {"value": "drop:text"}, "occurred_at": "keep:time", "reason": "drop:text", "started_by": "token:uuid", diff --git a/apps/api/src/cora/infrastructure/record_export/_export.py b/apps/api/src/cora/infrastructure/record_export/_export.py index fb505187a89..ab472b3a191 100644 --- a/apps/api/src/cora/infrastructure/record_export/_export.py +++ b/apps/api/src/cora/infrastructure/record_export/_export.py @@ -67,10 +67,21 @@ class ExportedRecord: entries-tier row pulled via an envelope by `kind`, in each kind's own registry order-by order; rows are UNFOLDED (one dict per row), never aggregated. + + `watermark` is the SAME xmin value `capture_watermark` produced and + the stream query above was bounded by, carried on the record itself + so `build_manifest` can read it here rather than take it as an + independent parameter: a caller wanting "the watermark this export + used" for the manifest had no way to obtain it other than by calling + `capture_watermark` a SECOND time, which returns a different snapshot + than the one the rows were actually bounded by. Defaults to 0 for + hand-built test fixtures that do not exercise watermark plumbing; + every real export sets it from the same call `export_record` used. """ streams: tuple[dict[str, object], ...] logbooks: dict[str, tuple[dict[str, object], ...]] + watermark: int = 0 async def capture_watermark(conn: asyncpg.Connection) -> int: @@ -82,10 +93,10 @@ async def capture_watermark(conn: asyncpg.Connection) -> int: cast to `text` on the way out here and to `int` on the way back in, then bound as `$1::xid8` by the caller -- never compared as a string. - Callers must pass the SAME returned value to `export_record`'s - underlying query exactly once; capturing it here rather than letting - the stream query re-evaluate `pg_snapshot_xmin` per row is what makes - one export see one snapshot instead of a moving target. + `export_record` is the sole caller: it binds this value into the + stream query AND carries it on the returned `ExportedRecord`, so + there is exactly one snapshot per export and one place that reads it + back. """ value = await conn.fetchval(_WATERMARK_SQL) assert value is not None, "pg_snapshot_xmin(pg_current_snapshot()) returned NULL" @@ -130,4 +141,5 @@ async def export_record(conn: asyncpg.Connection) -> ExportedRecord: return ExportedRecord( streams=tuple(streams), logbooks={kind: tuple(entries) for kind, entries in logbooks.items()}, + watermark=watermark, ) diff --git a/apps/api/src/cora/infrastructure/record_export/_hashing.py b/apps/api/src/cora/infrastructure/record_export/_hashing.py index 89c52fbdc79..efb0582d1a1 100644 --- a/apps/api/src/cora/infrastructure/record_export/_hashing.py +++ b/apps/api/src/cora/infrastructure/record_export/_hashing.py @@ -30,7 +30,11 @@ from typing import Protocol from cora.infrastructure.record_export._dispositions import DISPOSITIONS -from cora.infrastructure.record_export._export import ExportedRecord +from cora.infrastructure.record_export._redact_tier1 import ( + FIXED_DROP_COLUMNS, + FIXED_KEEP_COLUMNS, + FIXED_TOKEN_COLUMNS, +) from cora.infrastructure.record_export._redact_tier2 import ( TIER2_DISPOSITIONS, TIER2_JSONB_CLEARED_POINTERS, @@ -85,13 +89,18 @@ def hash_logbooks(logbooks: dict[str, tuple[dict[str, object], ...]]) -> str: return compute_content_hash(LOGBOOKS_PAYLOAD_TYPE, body) -def hash_record(record: ExportedRecord) -> str: +def hash_record(record: TwoTierRecord) -> str: """SHA-256 content hash over the whole bundle, both tiers, no exclusions. This is THE record hash (H1): per F2, it covers everything `export_record` produced. Re-running the export against an unchanged database reproduces this value exactly; any single differing byte anywhere in either tier changes it. + + Takes the structural `TwoTierRecord`, not `ExportedRecord` by name, + so `_bundle.write_bundle`'s binding check can call this on whatever + it was actually handed and compare against what a manifest claims, + without needing to know which concrete type that is. """ return compute_content_hash(RECORD_PAYLOAD_TYPE, _two_tier_body(record)) @@ -116,26 +125,38 @@ def hash_redacted_record(record: TwoTierRecord) -> str: Takes the structural `TwoTierRecord` rather than `RedactedRecord` only because importing `_redaction` here would invert this module's dependency; callers pass `RedactionResult.redacted_record`. Passing - an unredacted `ExportedRecord` is a caller error this signature - cannot catch, and the reason `write_bundle` derives H3 itself from - the record it is handed rather than accepting one as a parameter. + an unredacted `ExportedRecord` alongside a manifest whose + `published_record_hash` was computed from the real redacted record + is a caller error this function's signature cannot catch by itself + -- `write_bundle` is what catches it, by recomputing this same hash + over whatever record it was actually handed and refusing on + disagreement (`ManifestRecordMismatchError`) before writing a single + byte. """ return compute_content_hash(PUBLISHED_RECORD_PAYLOAD_TYPE, _two_tier_body(record)) def hash_redaction_profile() -> str: """SHA-256 content hash over every table that decides what a - published record discloses: tier 1's generated `DISPOSITIONS` AND + published record discloses: tier 1's generated per-field + `DISPOSITIONS` AND its hand-authored fixed-column dispositions + (`FIXED_KEEP_COLUMNS` / `FIXED_TOKEN_COLUMNS` / `FIXED_DROP_COLUMNS` + in `_redact_tier1.py`, which govern `stream_id`, `principal_id`, + `signature`, and every other `events` column outside `payload`), AND tier 2's hand-authored `TIER2_DISPOSITIONS` / `TIER2_JSONB_CLEARED_POINTERS` / `TIER2_JSONB_DROPPED_COLUMNS`. This IS the redaction profile hash (H2). Step 7's security re-review - found the tier-2 tables missing from this hash: the fail-closed - switch (`redact_record`'s `expected_redaction_profile_hash` check) - was fail-closed for tier 1 only, silently blind to a tier-2 table - edit that weakened a disposition (e.g. `conduit_verdicts.reason` - `DROP` -> `KEEP`) or dropped a jsonb clearance restriction. Both - tiers must be in H2, or "the hash matches" does not mean what + found the tier-2 tables missing from this hash and fixed that; this + fixes the same class of gap one seam over. `_redact_tier1.py`'s three + fixed-column tuples decide, unconditionally for every event, whether + `principal_id` tokens or `signature` drops -- editing either was + invisible to `redact_record`'s `expected_redaction_profile_hash` + check, so moving `principal_id` from TOKEN to KEEP, or `signature` + from DROP to KEEP (republishing a signature beside a redacted payload, + the exact confirmation oracle F5's anti-hooks forbid), would not have + moved H2 at all. Every table that decides a disposition must be in + H2, or "the hash matches" does not mean what `RedactionProfileMismatchError`'s docstring claims it means. Tuple-keyed dicts (`TIER2_JSONB_CLEARED_POINTERS` / @@ -146,8 +167,8 @@ def hash_redaction_profile() -> str: reviewable string. Regenerating tier 1's table via `make record-dispositions` after a - real event-model change, or hand-editing tier 2's tables, is - expected to change this value; `test_record_dispositions_drift.py` + real event-model change, or hand-editing either tier's fixed tables, + is expected to change this value; `test_record_dispositions_drift.py` guards tier 1's generator output specifically, and `test_redact_tier2.py`'s live-schema drift test guards tier 2's column coverage, but only THIS hash is what a caller's @@ -155,6 +176,9 @@ def hash_redaction_profile() -> str: """ body = { "tier1": DISPOSITIONS, + "tier1_fixed_keep_columns": sorted(FIXED_KEEP_COLUMNS), + "tier1_fixed_token_columns": sorted(FIXED_TOKEN_COLUMNS), + "tier1_fixed_drop_columns": sorted(FIXED_DROP_COLUMNS), "tier2_dispositions": TIER2_DISPOSITIONS, "tier2_jsonb_cleared_pointers": { f"{kind}/{column}": sorted(pointers) diff --git a/apps/api/src/cora/infrastructure/record_export/_manifest.py b/apps/api/src/cora/infrastructure/record_export/_manifest.py index 7a9049cbb41..b9b77a1f2a5 100644 --- a/apps/api/src/cora/infrastructure/record_export/_manifest.py +++ b/apps/api/src/cora/infrastructure/record_export/_manifest.py @@ -12,10 +12,10 @@ bundle genuinely has none; see the field's own docstring for why its absence is a signal rather than a default. -`build_manifest` is pure: every input it needs (`git_commit`, -`watermark`) is captured by the caller first and passed in, so the -function itself does no I/O and is trivial to test with synthetic -`ExportedRecord`s. +`build_manifest` is pure: every input it needs (`git_commit`; the +watermark comes off `record` itself) is captured by the caller first and +passed in, so the function itself does no I/O and is trivial to test +with synthetic `ExportedRecord`s. """ import subprocess @@ -30,6 +30,8 @@ hash_redacted_record, hash_redaction_profile, ) +from cora.infrastructure.record_export._redaction import RedactionResult +from cora.infrastructure.record_export._tokens import TokenMap @dataclass(frozen=True, slots=True) @@ -79,6 +81,22 @@ class Manifest: every rule the profile declares", which is a caveat about coverage, not a leak. """ + unfired_tier1_fields: tuple[str, ...] | None = None + """The same completeness fact as `unfired_tier2_clearances`, one tier + up: `"event_type/field"` pairs declared in the generated disposition + table for an event type this export carried, whose field never + appeared on any row of that type. Same `None`/empty-tuple convention. + + Almost always empty in practice: tier 1's table is exhaustively + generated, and a declared field is normally present (even as `null`) + on every row of an event type the current dataclass produces. A + field appearing here means EVERY row of that event type in this + export predates the `schema_version` that added the field -- one + surviving row with the key present would have marked it fired -- a + narrowness caveat about THIS export, not a leak; see + `RedactionResult.unfired_tier1_fields`'s docstring for why a + build-time guard cannot see this and a per-export field can. + """ def capture_git_commit(*, cwd: Path | str | None = None) -> str: @@ -126,23 +144,44 @@ def _max_schema_version_by_event_type(record: ExportedRecord) -> dict[str, int]: def _is_simulated(record: ExportedRecord) -> bool: - """True unless an observation row explicitly says otherwise. - - Vacuously True when the export carries no observation rows at all: - nothing in the bundle contradicts "this is a simulated record". A - mixed result (some True, some False) reports as False rather than - raising -- the manifest's job is to report the fact, not gate on it. + """False unless an observation row explicitly says otherwise. + + Matches the Run BC's own fold of this exact column + (`postgres_run_channel_lookup.py`'s `coalesce(bool_or(is_simulated), false)`): + an observation asserts simulated by being present and True, so the + identity element for "no observations at all" is False, the same as + `bool_or` over an empty set. The manifest previously used `all(...)`, + whose empty-set identity is True, so a record with zero observation + rows -- including the pilot's first genuine beamline-attached export, + which had none -- was reported as simulated. A published record + cannot carry that flag by an accident of aggregation identity. + + A mixed result (some True, some False) reports as True: ANY row + asserting simulated is enough to call the whole export simulated, + mirroring `bool_or`'s semantics exactly rather than requiring + unanimity in either direction. """ observations = record.logbooks.get("observation", ()) - return all(row["is_simulated"] is True for row in observations) + return any(row["is_simulated"] is True for row in observations) -def _expansion_digest_presence_by_run(record: ExportedRecord) -> dict[str, bool]: +def _expansion_digest_presence_by_run( + record: ExportedRecord, *, token_map: TokenMap | None = None +) -> dict[str, bool]: """Per F8: a run has a pinned expansion digest iff at least one of its child Procedures was registered via `register_procedure_from_recipe` (carries a `RecipeExpansionRecorded` on its own stream). A Procedure registered directly, or a run recorded by observing an external scan, has no digest to compare against; that is correct, not a gap. + + Without `token_map`, this dict is keyed by the RAW `stream_id` values + pulled straight from the unredacted `record` -- harmless on a full + bundle, but on a published one it would republish in plaintext + exactly the Run identifiers tier-1 redaction already replaced with + per-export surrogates in the streams body. Pass the SAME + `RedactionResult.token_map` tier-1 redaction used (via `token_uuid`, + memoized by source) so a run's key here always equals the surrogate + a reader finds on that run's rows. """ run_ids = { _require_str(row["stream_id"]) for row in record.streams if row["stream_type"] == "Run" @@ -159,57 +198,91 @@ def _expansion_digest_presence_by_run(record: ExportedRecord) -> dict[str, bool] ) elif row["event_type"] == "RecipeExpansionRecorded": expanded_procedures.add(_require_str(_payload(row)["procedure_id"])) - return { + by_raw_run_id = { run_id: any( parent_run_id == run_id and procedure_id in expanded_procedures for procedure_id, parent_run_id in parent_run_by_procedure.items() ) for run_id in run_ids } + if token_map is None: + return by_raw_run_id + return { + _require_str(token_map.token_uuid(run_id)): value for run_id, value in by_raw_run_id.items() + } def _render_unfired_clearances(unfired: frozenset[tuple[str, str, str]]) -> tuple[str, ...]: return tuple(sorted(f"{kind}/{column}/{pointer}" for kind, column, pointer in unfired)) +def _render_unfired_tier1_fields(unfired: frozenset[tuple[str, str]]) -> tuple[str, ...]: + return tuple(sorted(f"{event_type}/{field}" for event_type, field in unfired)) + + def build_manifest( record: ExportedRecord, *, - watermark: int, git_commit: str, - redacted: TwoTierRecord | None = None, - unfired_tier2_clearances: frozenset[tuple[str, str, str]] | None = None, + redaction: RedactionResult | None = None, ) -> Manifest: """Assemble the manifest for one already-exported, already-rendered record. - Pass `redacted` (a `RedactionResult.redacted_record`) when the bundle - being written is the published projection, so the manifest carries - H3. The shape counts stay derived from the UNREDACTED `record`: - redaction never adds or removes a row, only rewrites values within - one, so the counts describe both, and deriving them from the - unredacted side keeps a reader's recomputation honest if redaction - ever does start dropping rows. - - Pass `unfired_tier2_clearances` (a `RedactionResult.unfired_tier2_clearances`) - alongside `redacted` so the manifest carries the completeness caveat - described on `Manifest.unfired_tier2_clearances`. Meaningless without - `redacted` and ignored if `redacted` is `None`, matching that field's - same "no redaction happened" absence. + `watermark` is read from `record.watermark`, the value `export_record` + itself captured and bounded its query by, rather than taken as a + separate parameter: no caller could otherwise produce "the SAME value + the query used" without calling `capture_watermark` a second time, + which returns a different snapshot. + + Pass `redaction` (the `RedactionResult` `redact_record` returned) when + the bundle being written is the published projection, so the manifest + carries H3 and its per-run map is keyed by the same surrogates tier-1 + redaction already put on the streams body. The shape counts stay + derived from the UNREDACTED `record` regardless: redaction never adds + or removes a row, only rewrites values within one, so the counts + describe both, and deriving them from the unredacted side keeps a + reader's recomputation honest if redaction ever does start dropping + rows. + + `redaction` carries `redacted_record`, `token_map`, + `unfired_tier2_clearances` and `unfired_tier1_fields` together as one + object, deliberately, not as four independently-omittable parameters: + a caller could otherwise supply a `token_map` from an unrelated + redaction (or none at all) alongside a genuinely redacted record, + producing a manifest whose per-run keys disagree with the surrogates + actually on the streams body; or supply `redacted` while omitting one + of the completeness fields, silently reporting a false "everything + fired" instead of the true count. All become structurally impossible + once every one of them comes from the one `RedactionResult` a real + redaction pass produced. """ + if redaction is None: + redacted: TwoTierRecord | None = None + token_map: TokenMap | None = None + unfired_tier2: frozenset[tuple[str, str, str]] = frozenset() + unfired_tier1: frozenset[tuple[str, str]] = frozenset() + else: + redacted = redaction.redacted_record + token_map = redaction.token_map + unfired_tier2 = redaction.unfired_tier2_clearances + unfired_tier1 = redaction.unfired_tier1_fields return Manifest( git_commit=git_commit, - watermark=watermark, + watermark=record.watermark, record_hash=hash_record(record), redaction_profile_hash=hash_redaction_profile(), row_count_by_logbook_kind=_row_count_by_logbook_kind(record), max_schema_version_by_event_type=_max_schema_version_by_event_type(record), is_simulated=_is_simulated(record), - expansion_digest_presence_by_run=_expansion_digest_presence_by_run(record), + expansion_digest_presence_by_run=_expansion_digest_presence_by_run( + record, token_map=token_map + ), published_record_hash=None if redacted is None else hash_redacted_record(redacted), unfired_tier2_clearances=( - None - if redacted is None - else _render_unfired_clearances(unfired_tier2_clearances or frozenset()) + None if redacted is None else _render_unfired_clearances(unfired_tier2) + ), + unfired_tier1_fields=( + None if redacted is None else _render_unfired_tier1_fields(unfired_tier1) ), ) diff --git a/apps/api/src/cora/infrastructure/record_export/_redact_tier1.py b/apps/api/src/cora/infrastructure/record_export/_redact_tier1.py index 5d95da780ef..439855a628b 100644 --- a/apps/api/src/cora/infrastructure/record_export/_redact_tier1.py +++ b/apps/api/src/cora/infrastructure/record_export/_redact_tier1.py @@ -36,21 +36,21 @@ from cora.infrastructure.record_export._leaf_rule import OMITTED, apply_leaf_rule from cora.infrastructure.record_export._tokens import TokenMap -_FIXED_KEEP_COLUMNS = ( +FIXED_KEEP_COLUMNS = ( "schema_version", "stream_type", "event_type", "occurred_at", "recorded_at", ) -_FIXED_TOKEN_COLUMNS = ( +FIXED_TOKEN_COLUMNS = ( "stream_id", "correlation_id", "causation_id", "event_id", "principal_id", ) -_FIXED_DROP_COLUMNS = ("metadata", "signature", "signature_kid", "signature_version") +FIXED_DROP_COLUMNS = ("metadata", "signature", "signature_kid", "signature_version") class UnknownEventTypeError(LookupError): @@ -109,10 +109,24 @@ def _apply_field_disposition(disposition: Any, value: Any, *, token_map: TokenMa def redact_tier1_payload( - event_type: str, payload: dict[str, Any], *, token_map: TokenMap + event_type: str, + payload: dict[str, Any], + *, + token_map: TokenMap, + fired_fields: dict[str, set[str]] | None = None, ) -> dict[str, Any]: """Redact one event's `payload`, iterating the STORED payload's own - keys (never the disposition table's), per F5's fail-closed property.""" + keys (never the disposition table's), per F5's fail-closed property. + + `fired_fields`, when given, records which of `DISPOSITIONS[event_type]`'s + DECLARED field keys were actually present on this row -- the tier-1 + completeness twin to tier-2's `fired_pointers`. A field can be + declared but never fire within one export's event types if every row + of that type in this export happens to come from an older + `schema_version` that predates the field; `redact_record` uses this + to report the fact on the manifest, mirroring + `Manifest.unfired_tier2_clearances`. + """ if event_type not in DISPOSITIONS: raise UnknownEventTypeError(event_type) field_dispositions = DISPOSITIONS[event_type] @@ -122,6 +136,8 @@ def redact_tier1_payload( disposition = field_dispositions.get(key) if disposition is None: continue # known event type, unlisted field: schema-evolution DROP + if fired_fields is not None: + fired_fields.setdefault(event_type, set()).add(key) redacted = _apply_field_disposition(disposition, value, token_map=token_map) if redacted is not OMITTED: result[key] = redacted @@ -144,6 +160,14 @@ def __init__(self, token_map: TokenMap) -> None: self._next_version_by_stream: dict[str, int] = {} self._transaction_id_index: dict[str, int] = {} self._next_transaction_id = 1 + self._fired_fields: dict[str, set[str]] = {} + + @property + def fired_fields(self) -> dict[str, frozenset[str]]: + """Per event type redacted so far, the declared field keys that + actually appeared on at least one row. A copy; callers cannot + mutate the accumulator this instance still writes to.""" + return {event_type: frozenset(keys) for event_type, keys in self._fired_fields.items()} def _dense_version(self, raw_stream_id: str) -> int: version = self._next_version_by_stream.get(raw_stream_id, 1) @@ -166,16 +190,26 @@ def redact_row(self, row: dict[str, Any]) -> dict[str, Any]: } self._next_position += 1 - for column in _FIXED_KEEP_COLUMNS: + for column in FIXED_KEEP_COLUMNS: redacted[column] = row[column] - for column in _FIXED_TOKEN_COLUMNS: + for column in FIXED_TOKEN_COLUMNS: redacted[column] = self._token_map.token_uuid(row[column]) - # _FIXED_DROP_COLUMNS (metadata, signature*) intentionally absent. + # FIXED_DROP_COLUMNS (metadata, signature*) intentionally absent. redacted["payload"] = redact_tier1_payload( - row["event_type"], row["payload"], token_map=self._token_map + row["event_type"], + row["payload"], + token_map=self._token_map, + fired_fields=self._fired_fields, ) return redacted -__all__ = ["Tier1Redactor", "UnknownEventTypeError", "redact_tier1_payload"] +__all__ = [ + "FIXED_DROP_COLUMNS", + "FIXED_KEEP_COLUMNS", + "FIXED_TOKEN_COLUMNS", + "Tier1Redactor", + "UnknownEventTypeError", + "redact_tier1_payload", +] diff --git a/apps/api/src/cora/infrastructure/record_export/_redact_tier2.py b/apps/api/src/cora/infrastructure/record_export/_redact_tier2.py index 7d85b9376fa..bbcac1a4643 100644 --- a/apps/api/src/cora/infrastructure/record_export/_redact_tier2.py +++ b/apps/api/src/cora/infrastructure/record_export/_redact_tier2.py @@ -161,11 +161,113 @@ # that jsonb column. Absent entries default to an empty set (every # string leaf drops). Pointers use "*" for "any list element", matching # F5's own notation (`outcomes.measurements/*/name`). +# +# `activity/payload`'s set below replaces one transcribed from a route +# docstring that named no key the real Conductor ever writes (slice 6 of +# project_record_publishing_campaign.md). Every pointer matches a real, +# closed-in-practice string leaf `conductor.py` writes on some step kind; +# see the detailed rationale AND the scope note (a second, un-addressed +# `activity/payload` writer exists) in the comment following this dict. TIER2_JSONB_CLEARED_POINTERS: dict[tuple[str, str], frozenset[str]] = { - ("activity", "payload"): frozenset({"channel", "action_name", "units"}), + ("activity", "payload"): frozenset( + { + "address", + "result", + "error_class", + "criterion/kind", + "reading/kind", + "reading/quality", + "post_reading/kind", + "post_reading/quality", + "post_read_error/error_class", + "measurements/*/name", + "measurements/*/units", + "measurements/*/kind", + "measurements/*/quality", + } + ), ("outcome", "measurements"): frozenset({"*/name", "*/units", "*/kind", "*/quality"}), } +# `activity/payload`'s clearance rationale, per pointer. `address` is a +# facility-fixed PV, same posture as `observation.channel_name`. `result` +# is closed to 3 module constants (verified against every `result=` call +# site). `error_class`/`post_read_error/error_class` are `type(exc).__name__` +# for exceptions this module catches by tuple membership or explicit +# subclass (e.g. `ComputeExecutableNotPermittedError`); CORA-defined +# literals either way, never third-party or input-varying. `criterion/kind` +# is closed to "equals"/"within_tolerance". The `kind`/`quality` fields of +# every `Measurement` projection (`reading`, `post_reading`, +# `measurements/*`) match the outcome/measurements precedent above; unlike +# the control-path readings (closed by a real ACL, +# `epics_ca_control_port.py`'s quality translation), `measurements/*` from +# a COMPUTE step is closed only by there being no real value-arm +# ComputePort adapter yet (`Measurement` itself has no runtime validation +# on these fields) -- re-verify this clearance the day one lands. +# +# `criterion/expected`, `criterion/tolerance`, and the uncleared setpoint +# `value` need no pointer entry ONLY when their leaf is numeric, which is +# the common case but not the type: all three are typed as +# `int | float | bool | str | tuple[Any, ...]` unions, so a categorical +# check/setpoint (a string or tuple expected/value) silently drops today. +# Fails closed, not a leak, but means "what was checked/written" is +# incomplete for exactly the non-numeric case this slice is partly about; +# a real per-kind typed payload (Step 3, project_record_export_build_brief.md) +# is what would let this clear correctly instead of silently. +# +# Deliberately NOT cleared: `message`/`post_read_error/message` (free +# text, same shape as `verdict.reason`'s DROP); `quality_detail` and any +# `sampled_at`/`produced_at` timestamp (same non-clearance as +# outcome/measurements, and the timestamp-linkage lesson in +# feedback-claims-need-a-threat-model); `command`/`input_uris`/ +# `output_uri`/`input_refs`/`artifacts` (locator-shaped; a 2-BM path +# carries a PI surname and a proposal number); `parameters`/`params`/ +# `result_data`/compute `job_id`/`status` (arbitrary or unverified-closed +# shapes, watch items rather than asserted safe); and, DECIDED AND +# REVERSED during this slice's own gate review, `name` (ActionStep) plus +# `capture_name`/`capture_ref`/`steering_ref`/`output_ref_name` (Setpoint/ +# Capture/Compute ref-and-slot names). The first draft cleared these five +# by analogy to `command_name`/`tool_name` ("code-literal, not a +# person"). Both the analogy and the closure claim were wrong: none of +# the five is type- or registry-closed (`capture_name`/`output_ref_name`/ +# the ref names are plain `str` fields on Setpoint/Capture/Compute steps +# with no character-class validation anywhere in the recipe/body +# machinery), and `ActionStep.name` specifically is recorded on the +# PRE-LOOKUP in-flight marker and on the `UnknownActionError` failure arm +# -- both before or instead of the registry check that would have closed +# it, so an unregistered, arbitrary operator-authored string reaches the +# payload. The correct precedent in this same file is `agent_name`/ +# `agent_description`: DROP, "operator-authored free text". Per +# feedback-claims-need-a-threat-model, withdrawing this overclaim costs +# nothing: `address`+`result`+`criterion`+`reading` already carry the +# core "what did the Conductor do" evidence this slice exists for. +# +# SCOPE: `redact_tier2_row` dispatches on `kind` alone (`redact_tier2_row`, +# below) -- it does not know or care which code path wrote a row, so +# EVERY pointer above governs EVERY `("activity", "payload")` row +# regardless of writer. `entries_operation_procedure_activities.payload` +# has a SECOND real writer besides the Conductor: `append_activities`'s +# route and MCP tool accept an arbitrary caller-submitted payload +# (`payload: dict[str, Any]`, Pydantic does not constrain its shape), and +# 16 `tests/integration/scenarios/test_2bm_*.py` files modeling genuine +# 2-BM procedures submit activities this way directly, using the OLDER +# `channel`/`target_value`/`units`/`ramp_rate` (setpoint), `action_name`/ +# `params` (action), `channel`/`passed`/`expected`/`actual`/`tolerance` +# (check) shape this file used to (uselessly) clear. This slice's real, +# observable effect on that writer: the three old dead pointers +# (`channel`, `action_name`, `units`) tighten to DROP for it too, and any +# of its rows that happen to use `address`/`result`/`criterion`/`reading`- +# shaped keys would newly clear under the pointers above -- neither +# effect was decided FOR that writer, both are a side effect of one +# shared dispatch-on-`kind` mechanism with no writer discrimination. +# Verified no scenario test's fixture currently collides with the new +# pointer names, so nothing changes in practice today; that is +# incidental, not structural. That writer's own threat-modeled pass is a +# separate, un-briefed follow-up slice; do not assume it is covered by +# the pointers above just because they live in the same dict. This is +# the ONE authoritative copy of this note -- do not restate it in a +# docstring elsewhere, only cross-reference it. + # (kind, column) pairs whose jsonb value drops WHOLE rather than recursing. TIER2_JSONB_DROPPED_COLUMNS: frozenset[tuple[str, str]] = frozenset({("inference", "messages")}) @@ -223,13 +325,13 @@ def unfired_clearances( anything, so treating it as fatal was importing a denylist-shaped fear into an allowlist-shaped mechanism. - The practical failure this produced: `activity/payload`'s three - cleared pointers (`channel`, `action_name`, `units`) live on - different step kinds, two of them optional, so no small export -- - including a first rehearsal bundle -- reliably fires all three. The - export would abort with an error reading like a broken disposition - table rather than "this export was too narrow to exercise every - clearance." + The practical failure this produced: `activity/payload`'s cleared + pointers live on different step kinds (setpoint, action, check, + compute), several of them optional or failure-arm-only, so no small + export -- including a first rehearsal bundle -- reliably fires every + one. The export would abort with an error reading like a broken + disposition table rather than "this export was too narrow to + exercise every clearance." Callers now record the result on the manifest (`Manifest.unfired_tier2_clearances`) instead of treating it as a diff --git a/apps/api/src/cora/infrastructure/record_export/_redaction.py b/apps/api/src/cora/infrastructure/record_export/_redaction.py index f0342c0eae0..97ae100eac5 100644 --- a/apps/api/src/cora/infrastructure/record_export/_redaction.py +++ b/apps/api/src/cora/infrastructure/record_export/_redaction.py @@ -11,6 +11,7 @@ from dataclasses import dataclass +from cora.infrastructure.record_export._dispositions import DISPOSITIONS from cora.infrastructure.record_export._export import ExportedRecord from cora.infrastructure.record_export._hashing import hash_redaction_profile from cora.infrastructure.record_export._redact_tier1 import Tier1Redactor, UnknownEventTypeError @@ -71,11 +72,30 @@ class RedactionResult: reader can see, from the artifact itself, which parts of the redaction profile this particular export was too narrow to exercise. + + `unfired_tier1_fields` is the same completeness fact one tier up: + every `(event_type, field)` pair DECLARED in `DISPOSITIONS` for an + event type THIS export carried, whose field never actually appeared + on any row of that type. Unlike tier 2's hand-curated allowlist, + tier 1 is exhaustively generated by Step 0's generator, so most of + the time every declared field fires (a payload key is present, even + as `null`, on every schema version that still declares it); the case + this catches is an older `schema_version` row whose payload predates + a field the CURRENT dataclass declares. This is a narrowness caveat, + not a leak: a field that never fired was never published either way. + A rule that is dead in EVERY export (one whose declared key can never + match anything the store actually writes, as opposed to one export's + narrow coverage) is a different problem -- a mismatch between what + the generator resolved from the dataclass and what `to_payload` + actually serializes -- and needs a build-time check comparing the two + directly; this per-export field cannot see that case and does not + claim to. """ redacted_record: RedactedRecord token_map: TokenMap unfired_tier2_clearances: frozenset[tuple[str, str, str]] + unfired_tier1_fields: frozenset[tuple[str, str]] def redact_record( @@ -108,10 +128,23 @@ def redact_record( ) for kind, rows in record.logbooks.items() } - unfired = unfired_clearances(fired_pointers, kinds_present=frozenset(record.logbooks)) + unfired_tier2 = unfired_clearances(fired_pointers, kinds_present=frozenset(record.logbooks)) + + event_types_present = {str(row["event_type"]) for row in record.streams} + fired_fields = tier1.fired_fields + unfired_tier1 = frozenset( + (event_type, field) + for event_type in event_types_present + # `tier1.redact_row` above already raised UnknownEventTypeError for + # any event_type not in DISPOSITIONS, so every member of + # event_types_present is guaranteed present here. + for field in DISPOSITIONS[event_type] + if field not in fired_fields.get(event_type, frozenset()) + ) return RedactionResult( redacted_record=RedactedRecord(streams=redacted_streams, logbooks=redacted_logbooks), token_map=token_map, - unfired_tier2_clearances=unfired, + unfired_tier2_clearances=unfired_tier2, + unfired_tier1_fields=unfired_tier1, ) diff --git a/apps/api/src/cora/shared/closed_value.py b/apps/api/src/cora/shared/closed_value.py new file mode 100644 index 00000000000..4033e310438 --- /dev/null +++ b/apps/api/src/cora/shared/closed_value.py @@ -0,0 +1,58 @@ +"""`ClosedValueObject`: marker for a frozen VO the record exporter may +KEEP whole rather than resolve field by field. + +Per `project_record_export_v3.md` F5, the generated redaction +disposition table (`tools/gen_record_dispositions.py`) resolves a field +by its DECLARED TYPE. A frozen value object normally RECURSES: each of +its own fields gets classified in turn, and a bare `str` field inside it +drops by the same fail-closed default as everywhere else. That is +correct for a VO like `DatasetEncoding`, whose `media_type` is a loose, +unvalidated string with no closed range. + +It is the WRONG answer for a VO whose constructor closes every field's +range completely: a hex digest is a fixed-length, fixed-charset string, +and a checksum algorithm tag is drawn from a short closed set. Neither +can carry free text, so dropping them is not caution, it is the specific +defect this marker exists to fix: the record's own checksum, dropped by +the same rule that correctly protects an operator's free-text comment. + +Subclass `ClosedValueObject` ONLY when EVERY field of the VO is closed +by construction: a fixed-length charset check (a hex digest), a closed +literal set, a bounded number, another `ClosedValueObject`. If any field +could carry unconstrained text (a name, a free-form reason, a URI), do +not use this marker; let the field recurse and drop like any other. + +The generator checks this marker BEFORE recursing into a value object's +fields (see `_classify` in `tools/gen_record_dispositions.py`) and, when +it matches, emits `keep:closed:` for the whole VO. At +export time this keeps the VO's rendered form (already a dict of JSON +primitives) verbatim, exactly like any other `keep:*` disposition. + +This is a marker only: it adds no fields, no methods, and no runtime +behavior. It exists purely so a build-time tool can ask a type object +"does this VO close its own range?" without hand-maintaining a list of +class names to keep. + +Nothing checks the claim mechanically. Whether a subclass truly closes +every field is enforced today by code review at subclass-creation time +plus review of the generated table's diff, not by a fitness test: +`DatasetChecksum`, the only subclass as of this writing, is covered by +direct unit tests of its own `__post_init__` rejection paths (see +`tests/unit/data/test_dataset.py`), which is a proportionate check for +one instance. Rule of three: once a SECOND subclass exists, add a +fitness test that enumerates every `ClosedValueObject` subclass and +verifies each field's own validation actually closes it (a property +test over arbitrary strings/numbers is one way), rather than trusting +the marker and this docstring indefinitely. +""" + +__all__ = ["ClosedValueObject"] + + +class ClosedValueObject: + """Marker base for a frozen VO whose constructor closes every + field's range. See module docstring for the criterion and the + consequence of getting it wrong. + """ + + __slots__ = () diff --git a/apps/api/tests/architecture/conftest.py b/apps/api/tests/architecture/conftest.py index a8e364dc54d..4f42233b8f9 100644 --- a/apps/api/tests/architecture/conftest.py +++ b/apps/api/tests/architecture/conftest.py @@ -29,6 +29,7 @@ """ import os +import re import subprocess from functools import cache from pathlib import Path @@ -161,3 +162,60 @@ def tracked_migration_files() -> tuple[Path, ...]: return tuple( sorted(_REPO_ROOT / line for line in result.stdout.splitlines() if line.endswith(".sql")) ) + + +_CREATE_TABLE_RE = re.compile( + r"CREATE\s+TABLE\s+(?:IF\s+NOT\s+EXISTS\s+)?([a-zA-Z_][a-zA-Z0-9_]*)", + re.IGNORECASE, +) +_RENAME_TABLE_RE = re.compile( + r"ALTER\s+TABLE\s+([a-zA-Z_][a-zA-Z0-9_]*)\s+RENAME\s+TO\s+([a-zA-Z_][a-zA-Z0-9_]*)", + re.IGNORECASE, +) + + +def append_only_table_lineage() -> dict[str, frozenset[str]]: + """Every CURRENTLY append-only (`entries_*` / `events`) table, keyed + by its CURRENT identifier and mapped to every name it has ever held. + + Moved here from `test_entries_table_grants.py` (slice 6 of + project_record_publishing_campaign.md) once a second test + (`test_entries_tables_registered_in_record_export.py`) needed the + same rename-following lineage: single-sourced rather than a second + SQL parser. + + Follows `ALTER TABLE ... RENAME TO ...` across ALL tables' migration + history, not just tables already matching `entries_`/`events`, then + filters to that prefix only on the final (current) name. A table can + enter the append-only family through a rename whose OLD name never + matched the prefix: `entries_conduit_verdicts` was created as + `observations_conduit_traversals`, renamed to + `entries_conduit_traversals`, then renamed again to its current name. + Gating the rename-follow on "old name already tracked as entries_/ + events" would silently drop that chain the moment the origin name + fell outside the prefix, exactly the same blind spot this function + exists to close for the `entries_run_readings` case (see + `test_entries_table_grants.py`), just one hop earlier. + + The full lineage (not just the current name) matters to callers that + search for a fact attached to an OLD name (e.g. a GRANT issued before + a rename); callers that only care about the current schema should + read just this dict's keys. + """ + lineage: dict[str, set[str]] = {} + for path in tracked_migration_files(): + text = path.read_text() + for match in _CREATE_TABLE_RE.finditer(text): + name = match.group(1) + lineage.setdefault(name, {name}) + for match in _RENAME_TABLE_RE.finditer(text): + old_name, new_name = match.group(1), match.group(2) + if old_name in lineage: + names = lineage.pop(old_name) + names.add(new_name) + lineage[new_name] = names + return { + name: frozenset(names) + for name, names in lineage.items() + if name == "events" or name.startswith("entries_") + } diff --git a/apps/api/tests/architecture/test_entries_table_grants.py b/apps/api/tests/architecture/test_entries_table_grants.py index 0f0d06f6aea..eef9671b242 100644 --- a/apps/api/tests/architecture/test_entries_table_grants.py +++ b/apps/api/tests/architecture/test_entries_table_grants.py @@ -22,16 +22,7 @@ import pytest -from tests.architecture.conftest import tracked_migration_files - -_CREATE_TABLE_RE = re.compile( - r"CREATE\s+TABLE\s+(?:IF\s+NOT\s+EXISTS\s+)?([a-zA-Z_][a-zA-Z0-9_]*)", - re.IGNORECASE, -) -_RENAME_TABLE_RE = re.compile( - r"ALTER\s+TABLE\s+([a-zA-Z_][a-zA-Z0-9_]*)\s+RENAME\s+TO\s+([a-zA-Z_][a-zA-Z0-9_]*)", - re.IGNORECASE, -) +from tests.architecture.conftest import append_only_table_lineage, tracked_migration_files # Closed: the five tables that relied on the false ALTER DEFAULT # PRIVILEGES claim all got an explicit GRANT in @@ -45,64 +36,16 @@ def _all_migration_text() -> str: return "\n".join(f.read_text() for f in tracked_migration_files()) -def _append_only_tables_created() -> dict[str, frozenset[str]]: - """Every CURRENTLY append-only table, keyed by its CURRENT identifier - and mapped to every name it has ever held. - - Follows `ALTER TABLE ... RENAME TO ...` across ALL tables' migration - history, not just tables already matching `entries_`/`events`, then - filters to that prefix only on the final (current) name. A table can - enter the append-only family through a rename whose OLD name never - matched the prefix: `entries_conduit_verdicts` was created as - `observations_conduit_traversals`, renamed to - `entries_conduit_traversals`, then renamed again to its current name. - Gating the rename-follow on "old name already tracked as entries_/ - events" would silently drop that chain the moment the origin name - fell outside the prefix, exactly the same blind spot this function - exists to close for the `entries_run_readings` case (see below), just - one hop earlier. - - The same walk gives the full lineage, not just the current name, - which matters for the GRANT search: a privilege attaches to the - table's OID, not its name, so a GRANT issued under an OLD name (e.g. - `entries_decision_reasonings`, before it became - `entries_decision_inferences`) remains valid forever and a rename - never needs it re-issued under the new name. And the current-name - requirement matters for correctness in the other direction: a GRANT - written TODAY must target the table's current name (e.g. - `entries_run_observations`, not the dead `entries_run_readings`), the - only name that actually exists in the database by the time a later - migration runs. - """ - lineage: dict[str, set[str]] = {} - for path in tracked_migration_files(): - text = path.read_text() - for match in _CREATE_TABLE_RE.finditer(text): - name = match.group(1) - lineage.setdefault(name, {name}) - for match in _RENAME_TABLE_RE.finditer(text): - old_name, new_name = match.group(1), match.group(2) - if old_name in lineage: - names = lineage.pop(old_name) - names.add(new_name) - lineage[new_name] = names - return { - name: frozenset(names) - for name, names in lineage.items() - if name == "events" or name.startswith("entries_") - } - - @pytest.mark.architecture def test_every_new_entries_table_has_cora_app_grant() -> None: """Pattern accepted: `GRANT ... ON [TABLE] ... TO cora_app`, where `` is any name in the table's rename lineage (see - `_append_only_tables_created`), not just its current one. Tables on + `append_only_table_lineage`), not just its current one. Tables on `_GRANDFATHERED` are skipped: they predate this test and fixing them is a separate production migration, not a test change. """ haystack = _all_migration_text() - lineages = _append_only_tables_created() + lineages = append_only_table_lineage() tables = set(lineages) - _GRANDFATHERED assert tables, ( "No non-grandfathered append-only tables found; either the schema " diff --git a/apps/api/tests/architecture/test_entries_tables_registered_in_record_export.py b/apps/api/tests/architecture/test_entries_tables_registered_in_record_export.py new file mode 100644 index 00000000000..bc0a1b65937 --- /dev/null +++ b/apps/api/tests/architecture/test_entries_tables_registered_in_record_export.py @@ -0,0 +1,56 @@ +"""Every `entries_*` table a migration creates is known to record export's +entries-tier registry, and vice versa. + +Sibling of `test_record_export_registry_completeness.py`, which +cross-checks `cora.infrastructure.record_export._registry` against +`*LogbookOpened` EVENT CLASSES (AST-discovered under `src/cora`). That +check has nothing to say about a table that ships with no envelope at +all -- `entries_run_feed_heartbeats` and `entries_enclosure_permit_probes` +are declared in the registry by hand for exactly that reason +(`_registry.py`'s module docstring). Nothing today cross-checks the +registry against the OTHER source of truth, migration SQL: a migration +can add an `entries_*` table with no registry entry and nothing fails. +That gap is real -- an envelope-less table has already shipped twice +before `_registry.py`'s explicit declarations closed it -- and this +file is slice 6 of project_record_publishing_campaign.md's fix. + +Reuses `tests.architecture.conftest.append_only_table_lineage`'s +rename-following migration scan (moved there from +`test_entries_table_grants.py` for this reuse) rather than writing a +second SQL parser. +""" + +import pytest + +from cora.infrastructure.record_export import all_specs +from tests.architecture.conftest import append_only_table_lineage + + +@pytest.mark.architecture +def test_every_migration_created_entries_table_is_in_the_record_export_registry() -> None: + discovered = {name for name in append_only_table_lineage() if name != "events"} + registered = {spec.table for spec in all_specs()} + + unregistered = discovered - registered + assert not unregistered, ( + f"{sorted(unregistered)} are created by a migration but have no " + "cora.infrastructure.record_export._registry entry. An envelope-" + "less entries table silently narrows what the exporter can reach " + "until it is declared (see _registry.py's module docstring for " + "the heartbeat/probe precedent) -- add an EntriesTableSpec, with " + "envelope_class=None if the table genuinely has no *LogbookOpened " + "envelope, or the real envelope class name otherwise." + ) + + +@pytest.mark.architecture +def test_no_record_export_registry_entry_names_a_table_no_migration_created() -> None: + discovered = {name for name in append_only_table_lineage() if name != "events"} + registered = {spec.table for spec in all_specs()} + + stale = registered - discovered + assert not stale, ( + f"{sorted(stale)} are named by a record_export EntriesTableSpec.table " + "but no migration creates (or renames a table to) that name. " + "Renamed in a migration without updating _registry.py?" + ) diff --git a/apps/api/tests/architecture/test_record_disposition_keys_match_stored_payload.py b/apps/api/tests/architecture/test_record_disposition_keys_match_stored_payload.py new file mode 100644 index 00000000000..2dec1102a1b --- /dev/null +++ b/apps/api/tests/architecture/test_record_disposition_keys_match_stored_payload.py @@ -0,0 +1,208 @@ +"""Pin: the disposition table's keys are the keys `to_payload` writes. + +The generated redaction table (`_dispositions.py`) is built from each +event's DATACLASS FIELD names (`tools/gen_record_dispositions.py`'s +`_resolve_fields`, absent a `_OVERRIDE_WIRE_KEYS` entry). Redaction +(`redact_tier1_payload`) looks a field up by iterating the STORED +PAYLOAD's own keys -- the literal strings `to_payload` writes into the +dict jsonb actually holds. When a field's dataclass name and its wire +key disagree, the table carries a rule under a key redaction will never +see, and the actual stored key has no rule at all: the field drops by +the ordinary "unlisted key" default, silently, forever, for every event +of that type any export will ever carry. + +This is F6's root cause measured directly: `DatasetRegistered` declared +`checksum_algorithm` / `checksum_value` while `to_payload` nested them +under `"checksum"`, so the checksum -- the one field that makes a +published record checkable against the data it describes -- dropped by +a rule that could never fire, not by a redaction DECISION. Nine event +classes had this shape before it was fixed; this test is what keeps a +tenth from arriving unnoticed. + +## Scope + +Only event classes whose `to_payload` `case ClassName(...):` arm returns +a PLAIN dict literal with string-constant keys are checked: that covers +the AST shape most event classes in this codebase use, but not all of +them. A class using some other shape does not silently pass, it does +not appear in the comparison at all (see `_KEYS_BY_CLASS`'s docstring). + +As of this writing, 18 committed event classes build their payload as +`payload: dict[str, Any] = {...}` followed by conditional +`payload["x"] = ...` mutation rather than a single `return {...}` +literal, and are therefore NOT checked by this test: `AssetRegistered` +and all six Supply lifecycle events (`SupplyRegistered`, +`SupplyDegraded`, `SupplyDeregistered`, `SupplyMarkedAvailable`, +`SupplyMarkedRecovering`, `SupplyMarkedUnavailable`, `SupplyRestored`), +plus `ActorRegisteredV2`, `CalibrationRevisionAppended`, +`CautionAcknowledgement`, `EnclosurePermitObserved`, +`MethodRequiredRoleAdded`, `MethodVersioned`, `ModelDefined`, +`PlanVersioned`, `VisitCheckedOut`, `VisitPresenceClosed`. Notably this +includes Equipment and Supply, the two BCs already carrying VOs +directly on events, so a future field-name/wire-key mismatch in exactly +that shape would stay undetected. Widening `_dict_literal_keys` to read +the assign-then-mutate shape closes this gap; it is not attempted here +because it is a second AST case, not a fix to this one. If a future +event ever builds its payload some third way, this test needs a +matching AST case too, not a bigger blind spot. + +## Why this stays a static AST check, not an import + +Resolving `DISPOSITIONS` is a normal import (it lives in +`cora.infrastructure.record_export`, not the generator). Resolving +`to_payload`'s actual keys is done by parsing SOURCE, deliberately not +by calling `to_payload` on a constructed instance: constructing a real +instance of all 235 event classes would need valid values for every +field (including cross-BC value objects with their own validation), +which is exactly the generator-internal cost the campaign chose not to +pay for a stronger, structural-parity-only guarantee (see +`RedactionResult.unfired_tier1_fields`'s docstring for the tradeoff +argument). A static AST read has no such cost and needs no live data. +""" + +from __future__ import annotations + +import ast +from typing import TYPE_CHECKING + +import pytest + +from cora.infrastructure.record_export._dispositions import DISPOSITIONS +from tests.architecture.conftest import CORA_ROOT, tracked_python_files + +if TYPE_CHECKING: + from pathlib import Path + + +def _event_files() -> list[Path]: + return sorted( + f + for f in tracked_python_files() + if f.name == "events.py" + and f.parent.parent.name == "aggregates" + and f.parent.parent.parent.parent == CORA_ROOT + ) + + +def _find_to_payload(tree: ast.Module) -> ast.FunctionDef | None: + for node in tree.body: + if isinstance(node, ast.FunctionDef) and node.name == "to_payload": + return node + return None + + +def _match_class_name(pattern: ast.pattern) -> str | None: + """The class name a `case ClassName(...):` arm dispatches on. + + `ast.MatchClass.cls` is an `ast.Name` for a bare `ClassName(...)` + pattern (every event class in this codebase) or an `ast.Attribute` + for a qualified `module.ClassName(...)` pattern (unused today, kept + so a future qualified pattern does not silently fall through). + """ + if not isinstance(pattern, ast.MatchClass): + return None + if isinstance(pattern.cls, ast.Name): + return pattern.cls.id + if isinstance(pattern.cls, ast.Attribute): + return pattern.cls.attr + return None + + +def _dict_literal_keys(case: ast.match_case) -> frozenset[str] | None: + """String-literal keys of this arm's `return {...}`, or `None` if + the arm's return value is not a plain dict literal with every key a + string constant (out of scope; see module docstring).""" + for node in ast.walk(case): + if isinstance(node, ast.Return) and isinstance(node.value, ast.Dict): + string_keys: list[str] = [] + for k in node.value.keys: + if not (isinstance(k, ast.Constant) and isinstance(k.value, str)): + return None + string_keys.append(k.value) + return frozenset(string_keys) + return None + + +def _keys_by_class(func: ast.FunctionDef) -> dict[str, frozenset[str]]: + """`{ClassName: {stored keys}}` for every arm whose payload is a + plain dict literal. An arm this cannot read (see `_dict_literal_keys`) + is simply absent from the result, not recorded as empty.""" + out: dict[str, frozenset[str]] = {} + for node in ast.walk(func): + if not isinstance(node, ast.Match): + continue + for case in node.cases: + class_name = _match_class_name(case.pattern) + if class_name is None: + continue + keys = _dict_literal_keys(case) + if keys is not None: + out[class_name] = keys + return out + + +def _stored_keys_by_class() -> dict[str, tuple[Path, frozenset[str]]]: + """`{ClassName: (defining file, stored keys)}` across every tracked + `events.py`, for classes whose `to_payload` arm is AST-readable.""" + out: dict[str, tuple[Path, frozenset[str]]] = {} + for path in _event_files(): + tree = ast.parse(path.read_text(encoding="utf-8")) + func = _find_to_payload(tree) + if func is None: + continue + for class_name, keys in _keys_by_class(func).items(): + out[class_name] = (path, keys) + return out + + +def _cases() -> list[tuple[str, Path, frozenset[str]]]: + """One case per event type this test can actually check: present in + BOTH the committed table and an AST-readable `to_payload` arm.""" + stored = _stored_keys_by_class() + return sorted( + (event_type, path, keys) + for event_type, (path, keys) in stored.items() + if event_type in DISPOSITIONS + ) + + +_CASES = _cases() + + +@pytest.mark.architecture +@pytest.mark.parametrize("case", _CASES, ids=lambda c: c[0]) +def test_disposition_keys_match_stored_payload_keys(case: tuple[str, Path, frozenset[str]]) -> None: + event_type, path, stored_keys = case + table_keys = frozenset(DISPOSITIONS[event_type]) + + dead_rules = table_keys - stored_keys + unruled_keys = stored_keys - table_keys + + assert not dead_rules and not unruled_keys, ( + f"{event_type} ({path}): the generated disposition table and " + f"to_payload's actual stored keys disagree. Rules that can never " + f"fire (declared in DISPOSITIONS, absent from the stored payload): " + f"{sorted(dead_rules)}. Stored keys with no rule (redacted by the " + f"unlisted-key default, silently, forever): {sorted(unruled_keys)}. " + "If the field's dataclass name deliberately differs from its wire " + "key, add an entry to `_OVERRIDE_WIRE_KEYS` in " + "tools/gen_record_dispositions.py and regenerate with " + "`make record-dispositions`; otherwise retype the field so its " + "declared shape matches what it is actually stored as." + ) + + +@pytest.mark.architecture +def test_at_least_the_known_checksum_carrying_events_are_checked() -> None: + """Canary: if the AST walk above ever matches nothing (a refactor of + `to_payload`'s shape broke `_dict_literal_keys`'s assumptions), this + test says so specifically rather than the parametrized suite quietly + collecting zero cases and reporting all green.""" + checked = {event_type for event_type, _, _ in _CASES} + expected = {"DatasetRegistered", "DistributionRegistered"} + missing = expected - checked + assert not missing, ( + f"{sorted(missing)} should be AST-readable by this test's " + "to_payload walk and are not; the walk's assumptions about the " + "match-arm shape have drifted from the real event modules." + ) diff --git a/apps/api/tests/architecture/test_tier2_jsonb_clearances_are_real_keys.py b/apps/api/tests/architecture/test_tier2_jsonb_clearances_are_real_keys.py index 6ef7a2c359e..6fab6fdf3a5 100644 --- a/apps/api/tests/architecture/test_tier2_jsonb_clearances_are_real_keys.py +++ b/apps/api/tests/architecture/test_tier2_jsonb_clearances_are_real_keys.py @@ -17,79 +17,180 @@ (`cora.operation.ports.measurement.Measurement`), so this file INTROSPECTS it. A misspelled pointer here fails for the same reason a misspelled attribute access would. -- `activity.payload` has NO typed contract. Its shape lives in - `append_activities/route.py`'s docstring (three of the five - `STEP_KIND_VALUES` -- setpoint/action/check; capture/compute are - undocumented) plus in `conductor.py`'s `_append_step`, which merges in - `step_index` / `result` / `error_class?` / `message?` on EVERY kind, - none of which the docstring mentions. This file therefore - HAND-ENCODES the known keys from both sources and checks against - their union. That is a stopgap, not a fix: it catches a pointer that - matches NO known key, but it cannot detect a typo that happens to - collide with a different real key, and it says nothing about whether - the CLEARED set is complete (see the note on `result` below). Step 3 - in `project_record_export_build_brief.md` -- per-kind typed payloads - -- is what would let this become a real introspection check like the - measurements one; it is not built. - -FOUND while writing this, and NOT acted on here, because changing what -gets published is a content decision, not a typo check: `result` is -written on every conductor-driven activity row -(`conductor.py:3956` / `_append_step`) and is, in practice, drawn from -exactly three module-level string constants (`_RESULT_OK = "ok"`, -`_RESULT_FAILED = "failed"`, `_RESULT_IN_FLIGHT = "in_flight"`, -verified by grepping every `result=` call site in `conductor.py`), the -same "closed in practice, declared as bare `str`" shape as several -existing JUDGED LOW RISK tier-2 clearances. It is NOT in -`TIER2_JSONB_CLEARED_POINTERS`, so it drops on every export today, -which means the published record cannot currently distinguish a step -that succeeded from one that failed. `_KNOWN_ACTIVITY_PAYLOAD_KEYS` -below lists `result` as a known key precisely so this file's own -completeness gap is visible in its source rather than silently absent. +- `activity.payload` has NO typed contract; its shape is built + imperatively across several `Conductor` methods + (`_run_setpoint`/`_run_action`/`_run_check`/`_run_capture`/ + `_run_compute*`) with no single dataclass or dict-literal `return` to + introspect the way `test_record_disposition_keys_match_stored_payload.py` + does for an event's `to_payload`. This file therefore AST-SCANS those + methods (plus their free-function helpers) for every string dict-key + literal actually written into source, and checks each cleared + pointer's segments against that discovered set. + + CORRECTED 2026-08-12 (slice 6 of project_record_publishing_campaign.md). + The PREVIOUS version of this check hand-encoded its reference set from + `append_activities/route.py`'s payload docstring plus a manual list of + `_record`'s envelope keys. That docstring is free text (the field is + `payload: dict[str, Any]`, unvalidated) and, measured against + `conductor.py`, simply describes a payload shape the Conductor has + never written: `channel`/`target_value`/`units`/`ramp_rate` for + setpoint (the real keys are `address`/`value`), `action_name` for + action (the real key is `name`), `channel`/`passed`/`actual` for check + (the real keys are `address`/`criterion`, with the verdict living only + in `result`). Because `TIER2_JSONB_CLEARED_POINTERS` had ALSO been + transcribed from that same docstring, the old test was checking the + clearance list's copy of the wrong source against its own copy of the + same wrong source: both sides agreed, so it stayed green over three + cleared pointers -- `channel`, `action_name`, `units` -- that could + never fire against any payload the Conductor actually writes, while + the real fields (`address`, `name`, `criterion`, `reading`, and + `result` itself) dropped on every export. See + `_redact_tier2.py`'s `TIER2_JSONB_CLEARED_POINTERS` comment for the + corrected clearance list and the disclosure rationale (threat model) + behind each pointer. + + This AST scan is still not full introspection: it collects every + string key used ANYWHERE inside the whitelisted functions, flattened, + not pointer-shaped, so it cannot tell a `kind` that belongs under + `reading` from a `kind` that belongs under `criterion`. It fails, + loudly, on a pointer whose segment names no key the Conductor writes + ANYWHERE -- which is precisely the failure mode that let + `channel`/`action_name`/`units` through. A pointer that reuses a real + key name from an unrelated part of the payload is a residual blind + spot, the same kind `test_record_disposition_keys_match_stored_payload.py` + documents for its own AST case. Per-kind typed payloads (Step 3 in + `project_record_export_build_brief.md`) is what would close it for + good. + + SCOPE: this file (and the clearance list it checks) covers ONLY the + Conductor-driven shape. `activity.payload` has a second real writer, + not addressed here -- see `_redact_tier2.py`'s + `TIER2_JSONB_CLEARED_POINTERS` comment for the full account (the + authoritative copy; do not restate it here, it drifts). """ +import ast import dataclasses +from pathlib import Path from cora.infrastructure.record_export._redact_tier2 import TIER2_JSONB_CLEARED_POINTERS from cora.operation.ports.measurement import Measurement -# Per `append_activities/route.py`'s payload docstring (setpoint/action/ -# check only) plus the envelope keys `conductor.py`'s `_append_step` -# merges into EVERY kind's payload (`step_index`, `result`, and -# `error_class` / `message` on failure). `capture` and `compute` are two -# of `STEP_KIND_VALUES`'s five members and are NOT represented here: no -# docstring or call site documents their payload shape, which is itself -# evidence for project_record_export_build_brief.md's step 3. -_KNOWN_ACTIVITY_PAYLOAD_KEYS = frozenset( +# Every Conductor method (or free-function helper) that builds part of a +# conducted step's `activity.payload`. Kept as an explicit whitelist, +# not "every function in the file", so an unrelated dict literal +# elsewhere in this 4000+ line module (diagnostics, outcomes, the +# decide/convergence loops) cannot loosen this check by donating a +# same-named key that happens to make a bad pointer look real. +_ACTIVITY_PAYLOAD_FUNCTIONS = frozenset( { - "channel", - "target_value", - "units", - "ramp_rate", - "action_name", - "params", - "passed", - "expected", - "actual", - "tolerance", - "step_index", - "result", - "error_class", - "message", + "_record", + "_run_setpoint", + "_post_read_evidence", + "_run_action", + "_run_check", + "_run_capture", + "_run_compute", + "_run_compute_artifact_arm", + "_record_compute_capture_failure", + "_record_compute_output_failure", + "_record_compute_failure", + "_criterion_to_dict", + "_measurement_to_dict", + "_compute_measurement_to_dict", + "_compute_artifact_to_dict", } ) -def test_activity_payload_clearances_are_within_the_known_key_space() -> None: - cleared = TIER2_JSONB_CLEARED_POINTERS[("activity", "payload")] - unknown = cleared - _KNOWN_ACTIVITY_PAYLOAD_KEYS - assert not unknown, ( - f"TIER2_JSONB_CLEARED_POINTERS[('activity', 'payload')] clears {sorted(unknown)}, " - "which names no key documented in append_activities/route.py or written by " - "conductor.py's _append_step. Likely a typo; see this file's module docstring." +def _conductor_source() -> ast.Module: + import cora.operation.conductor as conductor_module + + return ast.parse(Path(conductor_module.__file__).read_text(encoding="utf-8")) + + +def _discovered_functions(tree: ast.Module) -> dict[str, ast.AST]: + return { + node.name: node + for node in ast.walk(tree) + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) + and node.name in _ACTIVITY_PAYLOAD_FUNCTIONS + } + + +def _activity_payload_key_space(tree: ast.Module) -> frozenset[str]: + """Every string dict-key literal written anywhere inside the + whitelisted functions, flattened across nesting. See module + docstring for what this can and cannot catch. + + Two AST shapes, both collected: a `{"key": ...}` dict literal (every + step-body builder uses this), and a `payload["key"] = ...` subscript + assignment (`_record` itself uses this for `error_class`/`message`). + Today the subscript-assigned keys also happen to appear as dict + literals elsewhere in the whitelist (`_post_read_evidence` nests + `error_class` under `post_read_error`), so collecting only dict + literals would still pass by coincidence -- collecting both shapes + removes that coincidence rather than relying on it. + """ + keys: set[str] = set() + for node in _discovered_functions(tree).values(): + for inner in ast.walk(node): + if isinstance(inner, ast.Dict): + for k in inner.keys: + if isinstance(k, ast.Constant) and isinstance(k.value, str): + keys.add(k.value) + elif ( + isinstance(inner, ast.Subscript) + and isinstance(inner.slice, ast.Constant) + and isinstance(inner.slice.value, str) + ): + keys.add(inner.slice.value) + return frozenset(keys) + + +def test_every_whitelisted_activity_payload_function_still_exists() -> None: + """A rename or removal of a whitelisted function must fail loudly + here rather than silently shrinking the discovered key space (and + with it, this test's ability to catch a bad pointer).""" + tree = _conductor_source() + discovered = set(_discovered_functions(tree)) + missing = _ACTIVITY_PAYLOAD_FUNCTIONS - discovered + assert not missing, ( + f"{sorted(missing)} not found in conductor.py (renamed or removed). " + "Update _ACTIVITY_PAYLOAD_FUNCTIONS in this file, or the real key " + "space this test checks against silently narrows." + ) + + +def test_activity_payload_key_space_is_not_suspiciously_small() -> None: + """A parser bug or an over-narrow whitelist would silently shrink the + discovered key space toward empty rather than raise; this pins a + floor so that failure mode is visible. Not a precise count -- see + [[feedback-narrow-edits-verify-the-count]] on why a floor beats + nothing, and [[project-record-publishing-campaign]] on why an exact + count would just be one more number to re-measure and forget.""" + real_keys = _activity_payload_key_space(_conductor_source()) + assert len(real_keys) >= 20, ( + f"only {len(real_keys)} keys discovered across " + f"{sorted(_ACTIVITY_PAYLOAD_FUNCTIONS)}; the AST scan may be broken." ) +def test_activity_payload_clearances_are_real_conductor_keys() -> None: + real_keys = _activity_payload_key_space(_conductor_source()) + cleared = TIER2_JSONB_CLEARED_POINTERS[("activity", "payload")] + for pointer in cleared: + segments = [segment for segment in pointer.split("/") if segment != "*"] + unknown = [segment for segment in segments if segment not in real_keys] + assert not unknown, ( + f"TIER2_JSONB_CLEARED_POINTERS[('activity', 'payload')] clears " + f"{pointer!r}, whose segment(s) {unknown} name no dict key found " + "anywhere in conductor.py's step-recording methods " + f"({sorted(_ACTIVITY_PAYLOAD_FUNCTIONS)}). Likely a typo, or a " + "stale pointer describing a key the Conductor no longer writes." + ) + + def test_outcome_measurements_clearances_are_real_measurement_fields() -> None: """`*/name`, `*/units`, `*/kind`, `*/quality` per element of the `measurements` list: strip the `*/` list-element marker and check diff --git a/apps/api/tests/integration/test_conductor_record_publishing_postgres.py b/apps/api/tests/integration/test_conductor_record_publishing_postgres.py new file mode 100644 index 00000000000..648f00e5c92 --- /dev/null +++ b/apps/api/tests/integration/test_conductor_record_publishing_postgres.py @@ -0,0 +1,320 @@ +"""End-to-end: a real Conductor run, published, and read back honest. + +Slice 6 of project_record_publishing_campaign.md's deliverable 4. Every +other end-to-end record-export test +(`test_record_export_bundle_postgres.py`) seeds `entries_operation_procedure_activities` +rows by hand with a route-shaped payload that never matched what the +Conductor actually writes -- that mismatch is exactly what let +`TIER2_JSONB_CLEARED_POINTERS[("activity", "payload")]` clear three +keys (`channel`, `action_name`, `units`) the real code never wrote. This +test closes that gap by splicing the two already-proven halves that +were never run back to back: + +- the CONDUCTING half, reused near-verbatim from + `test_conductor_against_softioc_postgres.py`: a real `Conductor` + driving `EpicsCaControlPort` against the shared softIOC subprocess and + a real `PostgresActivityStore`. +- the EXPORT/REDACT/BUNDLE/VERIFY half, reused near-verbatim from + `test_record_export_bundle_postgres.py`: `export_record` -> + `redact_record` -> `build_manifest` -> `write_bundle`, verified by the + standalone `scripts/verify_record_hash.py` in a subprocess that never + imports `cora` (the isolation is structural -- the script simply never + does `import cora` -- not an explicit `PYTHONPATH` strip, matching + every other test that shells out to it). + +The point of this test is the CONTENT assertion, not merely that the +bundle verifies (every other test here already proves that): a +published record of a real conducted run must be able to say what +address was set, what was checked, what was read, and whether the step +passed. Before slice 6 none of that survived redaction. +""" + +# pyright: reportUnknownMemberType=false, reportUnknownVariableType=false, reportUnknownArgumentType=false + +import json +import subprocess +import sys +from datetime import UTC, datetime +from pathlib import Path +from typing import Any +from uuid import UUID + +import asyncpg +import pytest + +from cora.infrastructure.event_envelope import to_new_event +from cora.infrastructure.record_export import ( + build_manifest, + capture_git_commit, + export_record, + hash_redaction_profile, + redact_record, + write_bundle, +) +from cora.operation.adapters.control_port_registry import ControlPortRegistry +from cora.operation.adapters.epics_ca_control_port import EpicsCaControlPort +from cora.operation.aggregates.procedure import ( + PostgresActivityStore, + ProcedureRegistered, + event_type_name, + to_payload, +) +from cora.operation.conductor import CheckStep, Conductor, SetpointStep, WithinToleranceCriterion +from cora.operation.features.abort_procedure import bind as bind_abort +from cora.operation.features.append_activities import bind as bind_append +from cora.operation.features.complete_procedure import bind as bind_complete +from cora.operation.features.start_procedure import bind as bind_start +from tests.integration._helpers import build_postgres_deps + +_NOW = datetime(2026, 8, 12, 12, 0, 0, tzinfo=UTC) +_PRINCIPAL_ID = UUID("01900000-0000-7000-8000-0000030d0099") +_CORRELATION_ID = UUID("01900000-0000-7000-8000-0000030d00aa") +_REPO_ROOT = Path(__file__).resolve().parents[4] +_VERIFIER = _REPO_ROOT / "scripts" / "verify_record_hash.py" + + +def _control_port(softioc: str) -> ControlPortRegistry: + """Same wiring as `test_conductor_against_softioc_postgres.py`'s + helper of the same name: the Conductor takes the registry, not the + bare substrate adapter.""" + registry = ControlPortRegistry() + registry.register_substrate_port(softioc, EpicsCaControlPort(), "epics_ca") + return registry + + +async def _seed_defined_procedure(deps_event_store: object, procedure_id: UUID) -> None: + """Seed a single ProcedureRegistered event so the Procedure exists in + `Defined`, bypassing `register_procedure`'s cross-aggregate + validation the same way the softIOC test does.""" + registered = ProcedureRegistered( + procedure_id=procedure_id, + name="2-BM bakeout, published", + kind="bakeout", + target_asset_ids=(), + parent_run_id=None, + occurred_at=_NOW, + ) + stored = to_new_event( + event_type=event_type_name(registered), + payload=to_payload(registered), + occurred_at=registered.occurred_at, + event_id=UUID("01900000-0000-7000-8000-0000030d0001"), + command_name="RegisterProcedure", + correlation_id=_CORRELATION_ID, + principal_id=_PRINCIPAL_ID, + ) + await deps_event_store.append( # type: ignore[attr-defined] + stream_type="Procedure", + stream_id=procedure_id, + expected_version=0, + events=[stored], + ) + + +def _verify(bundle: Path, *, published: bool = False) -> subprocess.CompletedProcess[str]: + argv = [sys.executable, str(_VERIFIER), "verify-bundle", str(bundle)] + if published: + argv.append("--published") + return subprocess.run(argv, capture_output=True, text=True) + + +def _activity_rows(bundle: Path) -> list[dict[str, Any]]: + path = bundle / "logbooks" / "activity.jsonl" + return [json.loads(line) for line in path.read_text(encoding="utf-8").splitlines() if line] + + +@pytest.mark.integration +async def test_a_real_conducted_run_publishes_what_it_did( + db_pool: asyncpg.Pool, softioc: str, tmp_path: Path +) -> None: + procedure_id = UUID("01900000-0000-7000-8000-0000030d0100") + logbook_id = UUID("01900000-0000-7000-8000-0000030d0101") + open_event_id = UUID("01900000-0000-7000-8000-0000030d0102") + started_event_id = UUID("01900000-0000-7000-8000-0000030d0103") + setpoint_marker_id = UUID("01900000-0000-7000-8000-0000030d0104") + setpoint_outcome_id = UUID("01900000-0000-7000-8000-0000030d0105") + check_step_id = UUID("01900000-0000-7000-8000-0000030d0106") + completed_event_id = UUID("01900000-0000-7000-8000-0000030d0107") + + deps = build_postgres_deps( + db_pool, + now=_NOW, + ids=[ + started_event_id, + logbook_id, + open_event_id, + setpoint_marker_id, + setpoint_outcome_id, + check_step_id, + completed_event_id, + ], + ) + await _seed_defined_procedure(deps.event_store, procedure_id) + step_store = PostgresActivityStore(db_pool) + control_port = _control_port(softioc) + conductor = Conductor( + control_port=control_port, + append_step=bind_append(deps, step_store=step_store), + clock=deps.clock, + id_generator=deps.id_generator, + start_procedure=bind_start(deps), + complete_procedure=bind_complete(deps), + abort_procedure=bind_abort(deps), + ) + + address = f"{softioc}double_value" + try: + await control_port.write(address, 42.0, wait=True) + result = await conductor.conduct( + procedure_id=procedure_id, + principal_id=_PRINCIPAL_ID, + correlation_id=_CORRELATION_ID, + steps=( + SetpointStep(address=address, value=7.5, verify=True), + CheckStep( + address=address, + criterion=WithinToleranceCriterion(expected=7.5, tolerance=0.01), + ), + ), + ) + finally: + await control_port.aclose() + + assert result.succeeded is True + + async with db_pool.acquire() as conn: + pg_conn: asyncpg.Connection = conn # type: ignore[assignment] + exported = await export_record(pg_conn) + + redaction = redact_record(exported, expected_redaction_profile_hash=hash_redaction_profile()) + manifest = build_manifest(exported, git_commit=capture_git_commit(), redaction=redaction) + bundle = write_bundle(redaction.redacted_record, manifest, tmp_path / "published") + + verified = _verify(bundle, published=True) + assert verified.returncode == 0, verified.stderr + + rows = _activity_rows(bundle) + by_result: dict[tuple[str, str], dict[str, Any]] = { + (row["step_kind"], row["payload"]["result"]): row["payload"] for row in rows + } + + setpoint_ok = by_result[("setpoint", "ok")] + assert setpoint_ok["address"] == address + assert setpoint_ok["result"] == "ok" + assert setpoint_ok["post_reading"]["value"] == 7.5 + assert setpoint_ok["post_reading"]["kind"] is not None + assert setpoint_ok["post_reading"]["quality"] == "Good" + # Not cleared: a full-precision substrate timestamp drops even when + # its parent object is published (feedback-claims-need-a-threat-model). + assert "sampled_at" not in setpoint_ok["post_reading"] + + check_ok = by_result[("check", "ok")] + assert check_ok["address"] == address + assert check_ok["result"] == "ok" + assert check_ok["criterion"] == { + "kind": "within_tolerance", + "expected": 7.5, + "tolerance": 0.01, + } + assert check_ok["reading"]["value"] == 7.5 + assert check_ok["reading"]["quality"] == "Good" + assert "sampled_at" not in check_ok["reading"] + + +@pytest.mark.integration +async def test_a_failed_check_publishes_the_failure_without_leaking_the_message( + db_pool: asyncpg.Pool, softioc: str, tmp_path: Path +) -> None: + """The happy-path test above proves cleared fields survive; this + proves the deliberately-uncleared ones actually drop on a REAL + conducted failure, not just in the unit-level fixtures in + `test_redact_tier2.py`. A criterion mismatch is the cheapest real + failure to provoke: no port-level error injection needed, just an + `expected` the softIOC's actual value won't satisfy. + """ + procedure_id = UUID("01900000-0000-7000-8000-0000030d0200") + logbook_id = UUID("01900000-0000-7000-8000-0000030d0201") + open_event_id = UUID("01900000-0000-7000-8000-0000030d0202") + started_event_id = UUID("01900000-0000-7000-8000-0000030d0203") + setpoint_marker_id = UUID("01900000-0000-7000-8000-0000030d0204") + setpoint_outcome_id = UUID("01900000-0000-7000-8000-0000030d0205") + check_step_id = UUID("01900000-0000-7000-8000-0000030d0206") + aborted_event_id = UUID("01900000-0000-7000-8000-0000030d0207") + + deps = build_postgres_deps( + db_pool, + now=_NOW, + ids=[ + started_event_id, + logbook_id, + open_event_id, + setpoint_marker_id, + setpoint_outcome_id, + check_step_id, + aborted_event_id, + ], + ) + await _seed_defined_procedure(deps.event_store, procedure_id) + step_store = PostgresActivityStore(db_pool) + control_port = _control_port(softioc) + conductor = Conductor( + control_port=control_port, + append_step=bind_append(deps, step_store=step_store), + clock=deps.clock, + id_generator=deps.id_generator, + start_procedure=bind_start(deps), + complete_procedure=bind_complete(deps), + abort_procedure=bind_abort(deps), + ) + + address = f"{softioc}double_value" + try: + await control_port.write(address, 7.5, wait=True) + result = await conductor.conduct( + procedure_id=procedure_id, + principal_id=_PRINCIPAL_ID, + correlation_id=_CORRELATION_ID, + steps=( + SetpointStep(address=address, value=7.5), + CheckStep( + address=address, + criterion=WithinToleranceCriterion(expected=999.0, tolerance=0.01), + ), + ), + ) + finally: + await control_port.aclose() + + assert result.succeeded is False + + async with db_pool.acquire() as conn: + pg_conn: asyncpg.Connection = conn # type: ignore[assignment] + exported = await export_record(pg_conn) + + redaction = redact_record(exported, expected_redaction_profile_hash=hash_redaction_profile()) + manifest = build_manifest(exported, git_commit=capture_git_commit(), redaction=redaction) + bundle = write_bundle(redaction.redacted_record, manifest, tmp_path / "published") + + verified = _verify(bundle, published=True) + assert verified.returncode == 0, verified.stderr + + rows = _activity_rows(bundle) + by_result: dict[tuple[str, str], dict[str, Any]] = { + (row["step_kind"], row["payload"]["result"]): row["payload"] for row in rows + } + + check_failed = by_result[("check", "failed")] + assert check_failed["address"] == address + assert check_failed["result"] == "failed" + assert check_failed["error_class"] == "CheckFailedError" + assert check_failed["criterion"] == { + "kind": "within_tolerance", + "expected": 999.0, + "tolerance": 0.01, + } + assert check_failed["reading"]["value"] == 7.5 + assert check_failed["reading"]["quality"] == "Good" + # The core negative-path proof: conductor.py's `message=str(exc)` for + # this failure includes the mismatch reason as free text, and it must + # not survive redaction even though its sibling `error_class` does. + assert "message" not in check_failed diff --git a/apps/api/tests/integration/test_record_acquisition_handler_postgres.py b/apps/api/tests/integration/test_record_acquisition_handler_postgres.py index 234c4673a7f..ca4f9b297f4 100644 --- a/apps/api/tests/integration/test_record_acquisition_handler_postgres.py +++ b/apps/api/tests/integration/test_record_acquisition_handler_postgres.py @@ -27,6 +27,7 @@ from cora.data.aggregates.acquisition import ( AcquisitionAssetNotFoundError, AcquisitionCannotRecordWithoutCapturingError, + AcquisitionEvidence, AcquisitionStatus, load_acquisition, ) @@ -126,7 +127,7 @@ async def test_record_acquisition_happy_path_round_trip(db_pool: asyncpg.Pool) - producing_asset_id=asset_id, captured_at=_CAPTURED_AT, settings={"exposure_ms": 200}, - evidence={"frames": 1801}, + evidence={"projection_count": 1801}, ), principal_id=_PRINCIPAL_ID, correlation_id=_CORRELATION_ID, @@ -142,7 +143,7 @@ async def test_record_acquisition_happy_path_round_trip(db_pool: asyncpg.Pool) - assert acq.captured_at == _CAPTURED_AT assert acq.recorded_at == _NOW assert acq.settings == {"exposure_ms": 200} - assert acq.evidence == {"frames": 1801} + assert acq.evidence == AcquisitionEvidence(projection_count=1801) assert acq.status is AcquisitionStatus.RECORDED # Persisted event payload preserves dual-time + bindings. diff --git a/apps/api/tests/integration/test_record_export_bundle_postgres.py b/apps/api/tests/integration/test_record_export_bundle_postgres.py index 41034e08395..c0d66b16b8d 100644 --- a/apps/api/tests/integration/test_record_export_bundle_postgres.py +++ b/apps/api/tests/integration/test_record_export_bundle_postgres.py @@ -14,18 +14,24 @@ FOUND WHILE WRITING THIS, and FIXED separately (same session, next commit): `redact_record` used to raise unless EVERY declared -`activity/payload` clearance fired, and those three keys (`channel`, -`action_name`, `units`) live on different step kinds, two of them -optional per `append_activities/route.py:86-94`, so a narrow export -(a single setpoint, say) aborted instead of exporting. The check -reasoned from a denylist's threat model (an unfired rule that should -have hidden something is a leak) applied backwards to tier 2's +`activity/payload` clearance fired, and those clearances live on +different step kinds, several of them optional or failure-arm-only, so +a narrow export (a single setpoint, say) aborted instead of exporting. +The check reasoned from a denylist's threat model (an unfired rule that +should have hidden something is a leak) applied backwards to tier 2's allowlist (an unfired rule here means something was published LESS than the profile permits, never more). `unfired_clearances` now reports the fact on the manifest instead of aborting; see its docstring in `_redact_tier2.py` for the full argument. This test's fixture still seeds three step kinds, not to dodge an abort that no longer exists, but because it is better coverage of the redaction path than one kind. + +Payload shapes below are the real ones `conductor.py` writes +(`address`/`value`, `name`/`params`, `address`/`criterion`), not the +route-docstring shape slice 6 of project_record_publishing_campaign.md +found was fictional -- see `_redact_tier2.py`'s +`TIER2_JSONB_CLEARED_POINTERS` comment and +`test_tier2_jsonb_clearances_are_real_keys.py` for that fix. """ # pyright: reportUnknownMemberType=false, reportUnknownVariableType=false, reportUnknownArgumentType=false @@ -45,6 +51,8 @@ MANIFEST_NAME, RECORD_PAYLOAD_TYPE, STREAMS_NAME, + TIER2_JSONB_CLEARED_POINTERS, + ManifestRecordMismatchError, build_manifest, capture_git_commit, export_record, @@ -116,19 +124,22 @@ async def _seed_a_procedure_with_one_activity(db_pool: asyncpg.Pool) -> None: ActivityInput( event_id=uuid4(), step_kind="setpoint", - payload={"channel": "T_oven", "target_value": 423.0, "units": "K"}, + payload={"address": "T_oven", "value": 423.0}, sampled_at=_NOW, ), ActivityInput( event_id=uuid4(), step_kind="action", - payload={"action_name": "open_valve", "params": {"valve": "V12"}}, + payload={"name": "open_valve", "params": {"valve": "V12"}}, sampled_at=_NOW, ), ActivityInput( event_id=uuid4(), step_kind="check", - payload={"channel": "T_oven", "passed": True}, + payload={ + "address": "T_oven", + "criterion": {"kind": "equals", "expected": 423.0}, + }, sampled_at=_NOW, ), ), @@ -172,7 +183,7 @@ async def test_a_real_export_writes_a_bundle_a_stranger_can_verify( pg_conn: asyncpg.Connection = conn # type: ignore[assignment] exported = await export_record(pg_conn) - manifest = build_manifest(exported, watermark=1, git_commit=capture_git_commit()) + manifest = build_manifest(exported, git_commit=capture_git_commit()) bundle = write_bundle(exported, manifest, tmp_path / "bundle") assert (bundle / STREAMS_NAME).is_file() @@ -195,13 +206,7 @@ async def test_a_real_redacted_export_verifies_against_h3( exported = await export_record(pg_conn) redaction = redact_record(exported, expected_redaction_profile_hash=hash_redaction_profile()) - manifest = build_manifest( - exported, - watermark=1, - git_commit=capture_git_commit(), - redacted=redaction.redacted_record, - unfired_tier2_clearances=redaction.unfired_tier2_clearances, - ) + manifest = build_manifest(exported, git_commit=capture_git_commit(), redaction=redaction) bundle = write_bundle(redaction.redacted_record, manifest, tmp_path / "published") result = _verify(bundle, published=True) @@ -213,12 +218,18 @@ async def test_a_narrow_export_redacts_and_reports_what_it_could_not_exercise( db_pool: asyncpg.Pool, tmp_path: Path ) -> None: """The regression test for the defect this module's docstring - describes. A single setpoint, no `units`, no `action`, no `check`: - exactly the shape that used to abort `redact_record` outright. + describes. A single setpoint, no action, no check: exactly the + shape that used to abort `redact_record` outright. It must now redact successfully, verify against H3, AND the - manifest must name the two clearances this narrow export could not - exercise -- proving the fact is surfaced, not just silently dropped. + manifest must name every clearance this narrow export could not + exercise -- proving the fact is surfaced, not just silently + dropped. The expected set is DERIVED from + `TIER2_JSONB_CLEARED_POINTERS` itself (every declared pointer this + fixture's one `address`-only payload doesn't fire), not + re-transcribed by hand: hand-transcribing it is exactly the mistake + slice 6 of project_record_publishing_campaign.md fixed for the + clearance list itself. """ procedure_id = uuid4() logbook_id = uuid4() @@ -259,7 +270,7 @@ async def test_a_narrow_export_redacts_and_reports_what_it_could_not_exercise( ActivityInput( event_id=uuid4(), step_kind="setpoint", - payload={"channel": "T_oven", "target_value": 423.0}, # no units + payload={"address": "T_oven", "value": 423.0}, # no criterion, no name sampled_at=_NOW, ), ), @@ -276,23 +287,44 @@ async def test_a_narrow_export_redacts_and_reports_what_it_could_not_exercise( # UnfiredClearanceError for exactly this fixture. redaction = redact_record(exported, expected_redaction_profile_hash=hash_redaction_profile()) - manifest = build_manifest( - exported, - watermark=1, - git_commit=capture_git_commit(), - redacted=redaction.redacted_record, - unfired_tier2_clearances=redaction.unfired_tier2_clearances, - ) - assert manifest.unfired_tier2_clearances == ( - "activity/payload/action_name", - "activity/payload/units", + manifest = build_manifest(exported, git_commit=capture_git_commit(), redaction=redaction) + expected_unfired_clearances = tuple( + sorted( + f"activity/payload/{pointer}" + for pointer in TIER2_JSONB_CLEARED_POINTERS[("activity", "payload")] + if pointer != "address" + ) ) + assert manifest.unfired_tier2_clearances == expected_unfired_clearances bundle = write_bundle(redaction.redacted_record, manifest, tmp_path / "narrow") result = _verify(bundle, published=True) assert result.returncode == 0, result.stderr +@pytest.mark.integration +async def test_write_bundle_refuses_a_real_unredacted_record_beside_an_h3_manifest( + db_pool: asyncpg.Pool, tmp_path: Path +) -> None: + """Against real Postgres-shaped rows, not the synthetic unit fixture: + passing the UNREDACTED export beside a manifest whose H3 was computed + from the real redacted record must refuse. Before this guard existed + this wrote a fully unredacted bundle under a --published label that + the default verifier printed OK for.""" + await _seed_a_procedure_with_one_activity(db_pool) + + async with db_pool.acquire() as conn: + pg_conn: asyncpg.Connection = conn # type: ignore[assignment] + exported = await export_record(pg_conn) + + redaction = redact_record(exported, expected_redaction_profile_hash=hash_redaction_profile()) + manifest = build_manifest(exported, git_commit=capture_git_commit(), redaction=redaction) + + with pytest.raises(ManifestRecordMismatchError): + write_bundle(exported, manifest, tmp_path / "should_not_exist") + assert not (tmp_path / "should_not_exist").exists() + + @pytest.mark.integration async def test_a_real_bundle_fails_verification_after_one_edited_digit( db_pool: asyncpg.Pool, tmp_path: Path @@ -304,7 +336,7 @@ async def test_a_real_bundle_fails_verification_after_one_edited_digit( pg_conn: asyncpg.Connection = conn # type: ignore[assignment] exported = await export_record(pg_conn) - manifest = build_manifest(exported, watermark=1, git_commit=capture_git_commit()) + manifest = build_manifest(exported, git_commit=capture_git_commit()) bundle = write_bundle(exported, manifest, tmp_path / "bundle") assert _verify(bundle).returncode == 0 @@ -330,7 +362,7 @@ async def test_both_reassembly_implementations_agree_on_a_real_bundle( pg_conn: asyncpg.Connection = conn # type: ignore[assignment] exported = await export_record(pg_conn) - manifest = build_manifest(exported, watermark=1, git_commit=capture_git_commit()) + manifest = build_manifest(exported, git_commit=capture_git_commit()) bundle = write_bundle(exported, manifest, tmp_path / "bundle") # CORA's reader reassembles the body; the script's reader then has to diff --git a/apps/api/tests/integration/test_record_export_checksum_survives_postgres.py b/apps/api/tests/integration/test_record_export_checksum_survives_postgres.py new file mode 100644 index 00000000000..16b8dc2f791 --- /dev/null +++ b/apps/api/tests/integration/test_record_export_checksum_survives_postgres.py @@ -0,0 +1,203 @@ +"""The campaign's proof: a real checksum, registered through the real +command path, survives redaction and lands in the published bundle. + +F6 found the published record of a real scan scientifically empty: +`checksum_value` dropped, along with `uri`, `name`, `media_type`, +`intent`, `conforms_to`, and `evidence` entire. Without the checksum a +published record cannot be checked against the data it describes, +which is the whole proposition. This test is that proposition, proven +against a real database through the real `register_dataset` and +`register_distribution` handlers -- not a synthetic `ExportedRecord` +built to already have the right shape. + +Deliberately standalone (`producing_run_id=None`, `subject_id=None`, +`derived_from=frozenset()`): this is the exact shape of the first real +2-BM ingest (`project_2bm_first_scan_record.md`), a commissioning scan +with no Run context, so this test does not carry the cost of seeding +Family -> Asset -> Method -> Practice -> Plan -> Subject -> Run just to +prove the checksum survives. +""" + +# pyright: reportUnknownMemberType=false, reportUnknownVariableType=false, reportUnknownArgumentType=false + +from dataclasses import replace +from datetime import UTC, datetime +from pathlib import Path +from uuid import UUID, uuid4 + +import asyncpg +import pytest + +from cora.data.aggregates.dataset import DATASET_CHECKSUM_SHA256_HEX_LENGTH +from cora.data.features import register_dataset, register_distribution +from cora.data.features.register_dataset import RegisterDataset +from cora.data.features.register_distribution import RegisterDistribution +from cora.infrastructure.projection import ProjectionRegistry, drain_projections +from cora.infrastructure.record_export import ( + build_manifest, + capture_git_commit, + export_record, + hash_redaction_profile, + read_bundle_body, + redact_record, + write_bundle, +) +from cora.supply._projections import register_supply_projections +from cora.supply.adapters import PostgresSupplyLookup +from cora.supply.features import register_supply +from cora.supply.features.register_supply import RegisterSupply +from tests._drain import drain_deadline_s +from tests.integration._helpers import build_postgres_deps + +_NOW = datetime(2026, 8, 12, 6, 21, 17, tzinfo=UTC) +_PRINCIPAL_ID = UUID("01900000-0000-7000-8000-000000000099") +_CORRELATION_ID = UUID("01900000-0000-7000-8000-0000000000aa") + +# A fabricated digest (sha256 of an arbitrary literal, not any real +# file's bytes), not the live 2-BM scan's own value: this test proves +# the MECHANISM survives redaction, and pinning a real production digest +# into test source would be a second, unrelated way to leak it. CAUGHT +# DRIFTING during the campaign's live-verification pass on 2026-08-13: +# this constant had silently become the real test_005.h5 digest at some +# point (confirmed against a direct, authorized read of the live +# database), exactly the promise this comment makes and had stopped +# keeping. Regenerate with a fresh literal if this ever needs to change +# again; never copy a value observed from a live export. +_CHECKSUM_VALUE = "3244f0175ea7d0107cb39a37cd000c313dedf1610b45fced8b5edbad99616c39" +assert len(_CHECKSUM_VALUE) == DATASET_CHECKSUM_SHA256_HEX_LENGTH + + +async def _drain_supply(db_pool: asyncpg.Pool) -> None: + """`register_distribution` pre-loads the Supply via a projection-backed + `SupplyLookup` port, not by folding the Supply's own event stream, so + a freshly-registered Supply is invisible until its projection catches + up.""" + registry = ProjectionRegistry() + register_supply_projections(registry) + await drain_projections(db_pool, registry, deadline_seconds=drain_deadline_s()) + + +async def _register_dataset_and_distribution_standalone( + db_pool: asyncpg.Pool, *, dataset_id: UUID, distribution_id: UUID, supply_id: UUID +) -> None: + """The 2-BM commissioning shape: no Run, no Subject, no lineage.""" + supply_deps = build_postgres_deps(db_pool, now=_NOW, ids=[supply_id, uuid4()]) + await register_supply.bind(supply_deps)( + RegisterSupply(kind="Storage", name="checksum-survives-test-supply", facility_code="cora"), + principal_id=_PRINCIPAL_ID, + correlation_id=_CORRELATION_ID, + ) + await _drain_supply(db_pool) + + dataset_deps = build_postgres_deps(db_pool, now=_NOW, ids=[dataset_id, uuid4()]) + await register_dataset.bind(dataset_deps)( + RegisterDataset( + name="test_005.h5", + uri="file:///local/cora-scans/test_005.h5", + checksum_algorithm="sha256", + checksum_value=_CHECKSUM_VALUE, + byte_size=24_504_057_268, + media_type="application/x-hdf5", + conforms_to=frozenset({"https://www.aps.anl.gov/DataExchange"}), + ), + principal_id=_PRINCIPAL_ID, + correlation_id=_CORRELATION_ID, + ) + + distribution_deps = build_postgres_deps(db_pool, now=_NOW, ids=[distribution_id, uuid4()]) + # `supply_lookup` defaults to the AllSatisfiedSupplyLookup synthetic + # stub; register_distribution's cross-BC Supply pre-load needs the + # real projection-backed port to see a Supply this test just wrote. + distribution_deps = replace(distribution_deps, supply_lookup=PostgresSupplyLookup(db_pool)) + await register_distribution.bind(distribution_deps)( + RegisterDistribution( + dataset_id=dataset_id, + supply_id=supply_id, + uri="file:///local/cora-scans/test_005.h5", + checksum_algorithm="sha256", + checksum_value=_CHECKSUM_VALUE, + byte_size=24_504_057_268, + media_type="application/x-hdf5", + access_protocol="POSIX", + conforms_to=frozenset({"https://www.aps.anl.gov/DataExchange"}), + ), + principal_id=_PRINCIPAL_ID, + correlation_id=_CORRELATION_ID, + ) + + +def _all_string_leaves(record: object) -> set[str]: + leaves: set[str] = set() + + def _walk(value: object) -> None: + if isinstance(value, dict): + for sub in value.values(): + _walk(sub) + elif isinstance(value, (list, tuple)): + for item in value: + _walk(item) + elif isinstance(value, str): + leaves.add(value) + + _walk(record) + return leaves + + +@pytest.mark.integration +async def test_checksum_survives_redaction_through_the_real_command_path( + db_pool: asyncpg.Pool, +) -> None: + dataset_id, distribution_id, supply_id = uuid4(), uuid4(), uuid4() + await _register_dataset_and_distribution_standalone( + db_pool, dataset_id=dataset_id, distribution_id=distribution_id, supply_id=supply_id + ) + + async with db_pool.acquire() as conn: + pg_conn: asyncpg.Connection = conn # type: ignore[assignment] + exported = await export_record(pg_conn) + + redaction = redact_record(exported, expected_redaction_profile_hash=hash_redaction_profile()) + + redacted = redaction.redacted_record + redacted_leaves = _all_string_leaves( + {"streams": redacted.streams, "logbooks": redacted.logbooks} + ) + assert _CHECKSUM_VALUE in redacted_leaves, ( + "the checksum did not survive redaction: a published record of this " + "Dataset would be unable to be checked against the data it describes" + ) + + # Named F6 drops, confirmed still dropping by decision, not oversight: + # uri and name never publish. A 2-BM experiment folder is + # `/local2/2BM/2026-08-DeCarlo-1015116`, so the locator carries a PI + # surname and a proposal number. A publishable locator is its own + # decision with its own threat model, deliberately not taken here. + assert "test_005.h5" not in redacted_leaves + assert "file:///local/cora-scans/test_005.h5" not in redacted_leaves + + +@pytest.mark.integration +async def test_checksum_survives_redaction_in_the_published_bundle_on_disk( + db_pool: asyncpg.Pool, tmp_path: Path +) -> None: + """The literal proof: the digest string, findable by a stranger reading + the bundle files, no CORA required.""" + dataset_id, distribution_id, supply_id = uuid4(), uuid4(), uuid4() + await _register_dataset_and_distribution_standalone( + db_pool, dataset_id=dataset_id, distribution_id=distribution_id, supply_id=supply_id + ) + + async with db_pool.acquire() as conn: + pg_conn: asyncpg.Connection = conn # type: ignore[assignment] + exported = await export_record(pg_conn) + + redaction = redact_record(exported, expected_redaction_profile_hash=hash_redaction_profile()) + manifest = build_manifest(exported, git_commit=capture_git_commit(), redaction=redaction) + bundle = write_bundle(redaction.redacted_record, manifest, tmp_path / "published") + + body = read_bundle_body(bundle) + assert _CHECKSUM_VALUE in _all_string_leaves(body), ( + "the checksum is not readable in the published bundle on disk -- " + "the one fact that makes a published record checkable against the " + "data it describes" + ) diff --git a/apps/api/tests/integration/test_record_export_manifest_postgres.py b/apps/api/tests/integration/test_record_export_manifest_postgres.py index d562aee1363..52758a8bba4 100644 --- a/apps/api/tests/integration/test_record_export_manifest_postgres.py +++ b/apps/api/tests/integration/test_record_export_manifest_postgres.py @@ -92,7 +92,7 @@ async def test_manifest_built_from_a_real_export(db_pool: asyncpg.Pool) -> None: pg_conn: asyncpg.Connection = conn # type: ignore[assignment] exported = await export_record(pg_conn) - manifest = build_manifest(exported, watermark=1, git_commit=capture_git_commit()) + manifest = build_manifest(exported, git_commit=capture_git_commit()) assert manifest.record_hash == hash_record(exported) assert len(manifest.redaction_profile_hash) == 64 @@ -101,5 +101,53 @@ async def test_manifest_built_from_a_real_export(db_pool: asyncpg.Pool) -> None: # No Run stream in this fixture (parent_run_id=None, no RunStarted # seeded), so the per-run map must be empty, not crash. assert manifest.expansion_digest_presence_by_run == {} - # No observation rows in this fixture: vacuously simulated. - assert manifest.is_simulated is True + # No observation rows in this fixture: vacuously NOT simulated, + # matching the Run BC's bool_or(is_simulated) identity on an empty set. + assert manifest.is_simulated is False + + +@pytest.mark.integration +async def test_watermark_is_the_real_captured_xmin_not_the_zero_default( + db_pool: asyncpg.Pool, +) -> None: + """`ExportedRecord.watermark` defaults to 0 for hand-built test + fixtures; a real export must never rely on that default. This is the + end-to-end proof the unit tests (which only check that `build_manifest` + reads back whatever a synthetic `ExportedRecord` carries) cannot give: + a real `capture_watermark()` call against live Postgres returns a + genuine, large `xid8`-derived integer, and it must reach the manifest + unchanged.""" + procedure_id = uuid4() + registered = ProcedureRegistered( + procedure_id=procedure_id, + name="Watermark probe", + kind="bakeout", + target_asset_ids=(), + parent_run_id=None, + occurred_at=_NOW, + ) + new_event = to_new_event( + event_type=event_type_name(registered), + payload=to_payload(registered), + occurred_at=registered.occurred_at, + event_id=uuid4(), + command_name="RegisterProcedure", + correlation_id=_CORRELATION_ID, + principal_id=_PRINCIPAL_ID, + ) + deps = build_postgres_deps(db_pool, now=_NOW, ids=[uuid4(), uuid4()]) + await deps.event_store.append( + stream_type="Procedure", stream_id=procedure_id, expected_version=0, events=[new_event] + ) + + async with db_pool.acquire() as conn: + pg_conn: asyncpg.Connection = conn # type: ignore[assignment] + exported = await export_record(pg_conn) + + assert exported.watermark > 0, ( + "a real export's watermark must be a genuine captured xmin, never the " + "0 default hand-built ExportedRecord fixtures fall back to" + ) + + manifest = build_manifest(exported, git_commit=capture_git_commit()) + assert manifest.watermark == exported.watermark diff --git a/apps/api/tests/integration/test_record_export_redaction_postgres.py b/apps/api/tests/integration/test_record_export_redaction_postgres.py index 48b4cc62e50..8387b9dfae4 100644 --- a/apps/api/tests/integration/test_record_export_redaction_postgres.py +++ b/apps/api/tests/integration/test_record_export_redaction_postgres.py @@ -225,6 +225,55 @@ async def test_wrong_redaction_profile_hash_refuses_before_redacting(db_pool: as redact_record(exported, expected_redaction_profile_hash="0" * 64) +@pytest.mark.integration +async def test_unfired_tier1_fields_is_empty_for_a_realistic_export(db_pool: asyncpg.Pool) -> None: + """A real export naturally exercises every declared field of every + event type it carries: a stored payload includes a key, even as + `null`, on any `schema_version` that still declares it. The tier-1 + completeness twin to tier-2's `unfired_tier2_clearances` should be + empty for a normal fixture, not merely small.""" + procedure_id = uuid4() + await _seed_running_procedure_with_activity(db_pool, procedure_id) + + async with db_pool.acquire() as conn: + pg_conn: asyncpg.Connection = conn # type: ignore[assignment] + exported = await export_record(pg_conn) + + result = redact_record(exported, expected_redaction_profile_hash=hash_redaction_profile()) + assert result.unfired_tier1_fields == frozenset() + + +@pytest.mark.integration +async def test_unfired_tier1_fields_names_a_declared_field_missing_from_every_row_of_its_type( + db_pool: asyncpg.Pool, +) -> None: + """Simulates an older `schema_version` row: a declared field + (`ProcedureRegistered.kind`, a real `DISPOSITIONS` key) removed from + the only `ProcedureRegistered` row this export carries must be + reported as unfired for that event type -- the narrowness caveat a + build-time guard cannot see, because the field is not missing from + the TABLE, only from every row THIS export happens to carry.""" + procedure_id = uuid4() + await _seed_running_procedure_with_activity(db_pool, procedure_id) + + async with db_pool.acquire() as conn: + pg_conn: asyncpg.Connection = conn # type: ignore[assignment] + exported = await export_record(pg_conn) + + streams = list(exported.streams) + for index, row in enumerate(streams): + if row["event_type"] == "ProcedureRegistered": + raw_payload = row["payload"] + assert isinstance(raw_payload, dict) + payload = dict(raw_payload) + del payload["kind"] + streams[index] = {**row, "payload": payload} + tampered = dataclasses.replace(exported, streams=tuple(streams)) + + result = redact_record(tampered, expected_redaction_profile_hash=hash_redaction_profile()) + assert ("ProcedureRegistered", "kind") in result.unfired_tier1_fields + + @pytest.mark.integration @pytest.mark.parametrize("spec", all_specs(), ids=lambda spec: spec.kind) async def test_tier2_disposition_table_columns_match_live_schema( diff --git a/apps/api/tests/unit/data/test_acquisition_evolver.py b/apps/api/tests/unit/data/test_acquisition_evolver.py index ae98cef2997..c6d3795290c 100644 --- a/apps/api/tests/unit/data/test_acquisition_evolver.py +++ b/apps/api/tests/unit/data/test_acquisition_evolver.py @@ -3,8 +3,8 @@ The aggregate ships one event arm (AcquisitionRecorded -> RECORDED), terminal at genesis. Tests lock the genesis fold (including the dual-time mapping occurred_at -> recorded_at) and the from_stored / -to_payload round-trip including JSON serialization of the settings / -evidence carrier dicts. +to_payload round-trip, including JSON serialization of the settings +carrier dict and the AcquisitionEvidence VO. """ from datetime import UTC, datetime @@ -13,8 +13,10 @@ import pytest from cora.data.aggregates.acquisition import ( + AcquisitionEvidence, AcquisitionRecorded, AcquisitionStatus, + CapturedAtSource, evolve, fold, from_stored, @@ -26,13 +28,14 @@ _CAPTURED_AT = datetime(2026, 6, 10, 9, 0, 0, tzinfo=UTC) _OCCURRED_AT = datetime(2026, 6, 11, 12, 0, 0, tzinfo=UTC) _RECORDED_BY = ActorId(UUID("01900000-0000-7000-8000-0000000000c1")) +_DEFAULT_EVIDENCE = AcquisitionEvidence(projection_count=1801) def _event( *, producing_run_id: UUID | None = None, settings: dict[str, object] | None = None, - evidence: dict[str, object] | None = None, + evidence: AcquisitionEvidence | None = None, ) -> AcquisitionRecorded: return AcquisitionRecorded( acquisition_id=uuid4(), @@ -41,7 +44,7 @@ def _event( producing_run_id=producing_run_id, captured_at=_CAPTURED_AT, settings=settings if settings is not None else {"exposure_ms": 200}, - evidence=evidence if evidence is not None else {"frames": 1801}, + evidence=evidence if evidence is not None else _DEFAULT_EVIDENCE, occurred_at=_OCCURRED_AT, recorded_by=_RECORDED_BY, ) @@ -76,7 +79,7 @@ def test_evolve_preserves_producing_run_id_when_set() -> None: @pytest.mark.unit -def test_evolve_copies_carrier_dicts_defensively() -> None: +def test_evolve_copies_settings_dict_defensively() -> None: settings: dict[str, object] = {"a": 1} state = evolve(state=None, event=_event(settings=settings)) settings["b"] = 2 @@ -99,7 +102,23 @@ def test_to_payload_from_stored_round_trip() -> None: event = _event( producing_run_id=uuid4(), settings={"exposure_ms": 200, "roi": {"w": 1024}}, - evidence={"frames": 1801, "ok": True}, + evidence=AcquisitionEvidence( + reader_kind="DataExchange", + checksum_computer_kind="PosixChecksum", + captured_at_source=CapturedAtSource.END_DATE, + captured_at_raw="2026-06-10T09:00:00", + projection_count=1501, + flat_count=40, + dark_count=20, + invalid_count=0, + commanded_projection_count=1501, + commanded_flat_count=40, + commanded_dark_count=20, + dropped_frame_count=0, + projection_angle_count=1501, + projection_angle_first=0.0, + projection_angle_last=180.0, + ), ) payload = to_payload(event) stored = StoredEvent( @@ -126,6 +145,18 @@ def test_to_payload_serializes_none_run_id_as_null() -> None: assert payload["producing_run_id"] is None +@pytest.mark.unit +def test_to_payload_omits_none_evidence_fields_rather_than_nulling() -> None: + payload = to_payload(_event(evidence=AcquisitionEvidence(projection_count=5))) + assert payload["evidence"] == {"projection_count": 5} + + +@pytest.mark.unit +def test_to_payload_no_evidence_supplied_serializes_as_empty_object() -> None: + payload = to_payload(_event(evidence=AcquisitionEvidence())) + assert payload["evidence"] == {} + + @pytest.mark.unit def test_to_payload_key_ordering_is_pinned() -> None: payload = to_payload(_event()) @@ -180,3 +211,53 @@ def test_from_stored_malformed_payload_raises_wrapped() -> None: ) with pytest.raises(ValueError, match="Malformed AcquisitionRecorded payload"): from_stored(stored) + + +@pytest.mark.unit +def test_from_stored_evidence_with_unknown_key_raises_wrapped() -> None: + """A stored payload whose nested evidence no longer validates (an + old-shape carrier from before this VO existed, or a hand-written + store mutation) is Malformed, not silently reconstructed or a raw + InvalidAcquisitionEvidenceError leaking past the aggregate boundary.""" + payload = to_payload(_event()) + payload["evidence"] = {"frames": 1801} + stored = StoredEvent( + position=1, + event_id=uuid4(), + stream_type="Acquisition", + stream_id=uuid4(), + version=1, + event_type="AcquisitionRecorded", + schema_version=1, + payload=payload, + correlation_id=uuid4(), + causation_id=None, + occurred_at=_OCCURRED_AT, + recorded_at=_OCCURRED_AT, + ) + with pytest.raises(ValueError, match="Malformed AcquisitionRecorded payload"): + from_stored(stored) + + +@pytest.mark.unit +def test_from_stored_evidence_with_unknown_captured_at_source_raises_wrapped() -> None: + """A future layout naming a fourth captured_at_source (see + CapturedAtSource's docstring) fails loud on replay, not silently.""" + payload = to_payload(_event()) + payload["evidence"] = {"captured_at_source": "acquisition_time"} + stored = StoredEvent( + position=1, + event_id=uuid4(), + stream_type="Acquisition", + stream_id=uuid4(), + version=1, + event_type="AcquisitionRecorded", + schema_version=1, + payload=payload, + correlation_id=uuid4(), + causation_id=None, + occurred_at=_OCCURRED_AT, + recorded_at=_OCCURRED_AT, + ) + with pytest.raises(ValueError, match="Malformed AcquisitionRecorded payload"): + from_stored(stored) diff --git a/apps/api/tests/unit/data/test_acquisition_state.py b/apps/api/tests/unit/data/test_acquisition_state.py index d5ef332f2bb..cf6279243e2 100644 --- a/apps/api/tests/unit/data/test_acquisition_state.py +++ b/apps/api/tests/unit/data/test_acquisition_state.py @@ -1,7 +1,8 @@ """Unit tests for Acquisition state: status enum, errors, carrier-shape VOs. Pins the single-value AcquisitionStatus, the don't-hoist error -family, and the shape-only settings / evidence validators. +family, the shape-only settings validator, and evidence's +AcquisitionEvidence validator/builder. """ from datetime import UTC, datetime @@ -14,8 +15,10 @@ AcquisitionAlreadyExistsError, AcquisitionAssetNotFoundError, AcquisitionCannotRecordWithoutCapturingError, + AcquisitionEvidence, AcquisitionRunNotFoundError, AcquisitionStatus, + CapturedAtSource, InvalidAcquisitionEvidenceError, InvalidAcquisitionSettingsError, validate_evidence, @@ -50,7 +53,7 @@ def test_acquisition_state_defaults_to_recorded() -> None: producing_run_id=None, captured_at=_NOW, settings={}, - evidence={}, + evidence=AcquisitionEvidence(), recorded_at=_NOW, recorded_by=_RECORDED_BY, ) @@ -93,9 +96,42 @@ def test_validate_settings_rejects_non_string_key() -> None: @pytest.mark.unit -def test_validate_evidence_accepts_primitive_leaves() -> None: - value = {"checksum": "abc", "verified": True} - assert validate_evidence(value) is value +def test_validate_evidence_accepts_empty_dict() -> None: + """No evidence supplied: every AcquisitionEvidence field is None.""" + assert validate_evidence({}) == AcquisitionEvidence() + + +@pytest.mark.unit +def test_validate_evidence_accepts_known_shape() -> None: + value = { + "reader_kind": "DataExchange", + "checksum_computer_kind": "PosixChecksum", + "captured_at_source": "end_date", + "captured_at_raw": "2026-06-10T09:00:00", + "projection_count": 1501, + "flat_count": 40, + "dark_count": 20, + "invalid_count": 0, + "commanded_projection_count": 1501, + "commanded_flat_count": 40, + "commanded_dark_count": 20, + "dropped_frame_count": 0, + "projection_angle_count": 1501, + "projection_angle_first": 0.0, + "projection_angle_last": 180.0, + } + evidence = validate_evidence(value) + assert evidence.reader_kind == "DataExchange" + assert evidence.captured_at_source is CapturedAtSource.END_DATE + assert evidence.projection_count == 1501 + assert evidence.projection_angle_first == 0.0 + + +@pytest.mark.unit +def test_validate_evidence_accepts_sparse_subset() -> None: + """No key is required; a caller may report only what it knows.""" + evidence = validate_evidence({"projection_count": 5}) + assert evidence == AcquisitionEvidence(projection_count=5) @pytest.mark.unit @@ -105,9 +141,52 @@ def test_validate_evidence_rejects_non_dict() -> None: @pytest.mark.unit -def test_validate_evidence_rejects_non_primitive_leaf() -> None: - with pytest.raises(InvalidAcquisitionEvidenceError, match="non-primitive leaf"): - validate_evidence({"bad": object()}) +def test_validate_evidence_rejects_unknown_key() -> None: + with pytest.raises(InvalidAcquisitionEvidenceError, match="unknown key"): + validate_evidence({"frames": 1801}) + + +@pytest.mark.unit +def test_validate_evidence_rejects_wrong_typed_value() -> None: + with pytest.raises(InvalidAcquisitionEvidenceError, match="projection_count"): + validate_evidence({"projection_count": "lots"}) + + +@pytest.mark.unit +def test_validate_evidence_rejects_bool_for_int_field() -> None: + """A Python bool is an int subclass, but JSON's integer and boolean + are not the same type: a boolean here is a shape violation, not a 0/1.""" + with pytest.raises(InvalidAcquisitionEvidenceError, match="projection_count"): + validate_evidence({"projection_count": True}) + + +@pytest.mark.unit +def test_validate_evidence_rejects_wrong_typed_float_field() -> None: + with pytest.raises(InvalidAcquisitionEvidenceError, match="projection_angle_first"): + validate_evidence({"projection_angle_first": "zero"}) + + +@pytest.mark.unit +def test_validate_evidence_rejects_bool_for_float_field() -> None: + """Same shape-violation rule as the int fields: a bool is not a number + here even though Python's bool is an int subclass.""" + with pytest.raises(InvalidAcquisitionEvidenceError, match="projection_angle_last"): + validate_evidence({"projection_angle_last": False}) + + +@pytest.mark.unit +def test_validate_evidence_accepts_int_for_float_field() -> None: + """A whole-number angle (e.g. `180`) is valid JSON for a float field; + the stored value coerces to float rather than being rejected.""" + evidence = validate_evidence({"projection_angle_first": 0}) + assert evidence.projection_angle_first == 0.0 + assert isinstance(evidence.projection_angle_first, float) + + +@pytest.mark.unit +def test_validate_evidence_rejects_unknown_captured_at_source() -> None: + with pytest.raises(InvalidAcquisitionEvidenceError, match="captured_at_source"): + validate_evidence({"captured_at_source": "acquisition_time"}) @pytest.mark.unit @@ -140,3 +219,34 @@ def test_acquisition_asset_missing_capturing_affordance_error_carries_id() -> No err = AcquisitionCannotRecordWithoutCapturingError(asset_id) assert err.asset_id == asset_id assert "Capturing" in str(err) + + +@pytest.mark.unit +def test_acquisition_recorded_evidence_disposition_pins_the_disclosure_split() -> None: + """Pins F6's whole point for Acquisition: the actual keep/drop split + the generator produces for evidence's fields, independent of the + generator itself (test_record_dispositions_drift.py only proves the + committed table equals a FRESH run of the SAME generator, so a + classification bug that is wrong-but-self-consistent would still + pass it). A future change that regresses any of these back to + drop:opaque, or promotes an open-vocabulary string to keep:, should + fail here first.""" + from cora.infrastructure.record_export._dispositions import DISPOSITIONS + + assert DISPOSITIONS["AcquisitionRecorded"]["evidence"] == { + "reader_kind": "drop:text", + "checksum_computer_kind": "drop:text", + "captured_at_source": "keep:enum:CapturedAtSource", + "captured_at_raw": "drop:text", + "projection_count": "keep:number", + "flat_count": "keep:number", + "dark_count": "keep:number", + "invalid_count": "keep:number", + "commanded_projection_count": "keep:number", + "commanded_flat_count": "keep:number", + "commanded_dark_count": "keep:number", + "dropped_frame_count": "keep:number", + "projection_angle_count": "keep:number", + "projection_angle_first": "keep:number", + "projection_angle_last": "keep:number", + } diff --git a/apps/api/tests/unit/data/test_add_dataset_to_edition_handler.py b/apps/api/tests/unit/data/test_add_dataset_to_edition_handler.py index d89b6491c30..521a9bf84ce 100644 --- a/apps/api/tests/unit/data/test_add_dataset_to_edition_handler.py +++ b/apps/api/tests/unit/data/test_add_dataset_to_edition_handler.py @@ -19,6 +19,7 @@ from cora.data.aggregates.dataset.events import ( to_payload as dataset_to_payload, ) +from cora.data.aggregates.dataset.state import DatasetChecksum, DatasetEncoding from cora.data.aggregates.edition import ( EditionCannotBindToDiscardedDatasetError, EditionDatasetAlreadyMemberError, @@ -63,11 +64,9 @@ async def _seed_dataset( dataset_id=dataset_id, name="seed", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=1024, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=None, subject_id=None, derived_from=frozenset(), diff --git a/apps/api/tests/unit/data/test_dataset_events.py b/apps/api/tests/unit/data/test_dataset_events.py index cc507eba0c1..a0c977dbadd 100644 --- a/apps/api/tests/unit/data/test_dataset_events.py +++ b/apps/api/tests/unit/data/test_dataset_events.py @@ -8,9 +8,12 @@ from cora.data.aggregates.dataset import ( DATASET_CHECKSUM_SHA256_HEX_LENGTH, + DatasetChecksum, DatasetDemoted, + DatasetEncoding, DatasetPromoted, DatasetRegistered, + Intent, event_type_name, from_stored, to_payload, @@ -50,11 +53,9 @@ def test_event_type_name_returns_class_name() -> None: dataset_id=uuid4(), name="D", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=0, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=None, subject_id=None, derived_from=frozenset(), @@ -71,11 +72,9 @@ def test_to_payload_serializes_all_fields_with_nulls_and_empties() -> None: dataset_id=dataset_id, name="D", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=0, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=None, subject_id=None, derived_from=frozenset(), @@ -115,11 +114,12 @@ def test_to_payload_sorts_set_semantic_fields_deterministically() -> None: dataset_id=uuid4(), name="D", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=1, - media_type="application/x-hdf5", - conforms_to=frozenset({"https://b.example/", "https://a.example/"}), + encoding=DatasetEncoding( + media_type="application/x-hdf5", + conforms_to=frozenset({"https://b.example/", "https://a.example/"}), + ), producing_run_id=None, subject_id=None, derived_from=frozenset({derived_a, derived_b, derived_c}), @@ -139,11 +139,9 @@ def test_to_payload_serializes_optional_refs_when_set() -> None: dataset_id=uuid4(), name="D", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=0, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=run_id, subject_id=subject_id, derived_from=frozenset(), @@ -166,11 +164,12 @@ def test_round_trip_through_stored_envelope() -> None: dataset_id=dataset_id, name="32-ID Recon", uri="s3://aps-32id/runs/abc/recon.h5", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=1_073_741_824, - media_type="application/x-hdf5", - conforms_to=frozenset({"https://manual.nexusformat.org/"}), + encoding=DatasetEncoding( + media_type="application/x-hdf5", + conforms_to=frozenset({"https://manual.nexusformat.org/"}), + ), producing_run_id=run_id, subject_id=subject_id, derived_from=frozenset({derived_id}), @@ -215,11 +214,9 @@ def test_to_payload_serializes_producing_procedure_id() -> None: dataset_id=uuid4(), name="D", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=0, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=None, subject_id=None, derived_from=frozenset(), @@ -309,17 +306,15 @@ def test_to_payload_includes_producing_run_end_state_and_intent_when_set() -> No dataset_id=uuid4(), name="D", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=0, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=uuid4(), subject_id=None, derived_from=frozenset(), occurred_at=_NOW, producing_run_end_state="Completed", - intent="Trial", + intent=Intent.TRIAL, registered_by=_REGISTERED_BY, ) payload = to_payload(event) @@ -334,11 +329,9 @@ def test_producing_actuation_kind_round_trips_through_stored_envelope() -> None: dataset_id=uuid4(), name="D", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=0, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=uuid4(), subject_id=None, derived_from=frozenset(), @@ -583,11 +576,9 @@ def test_to_payload_serializes_used_calibration_ids_as_sorted_string_list() -> N dataset_id=uuid4(), name="D", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=0, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=None, subject_id=None, derived_from=frozenset(), @@ -607,11 +598,9 @@ def test_to_payload_serializes_empty_used_calibration_ids_as_empty_list() -> Non dataset_id=uuid4(), name="D", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=0, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=None, subject_id=None, derived_from=frozenset(), @@ -662,11 +651,9 @@ def test_used_calibration_ids_round_trip_through_stored_envelope() -> None: dataset_id=uuid4(), name="D", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=0, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=None, subject_id=None, derived_from=frozenset(), @@ -698,11 +685,9 @@ def _build(used_calibration_ids: tuple[UUID, ...]) -> DatasetRegistered: dataset_id=dataset_id, name="D", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=0, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=None, subject_id=None, derived_from=frozenset(), diff --git a/apps/api/tests/unit/data/test_dataset_evolver.py b/apps/api/tests/unit/data/test_dataset_evolver.py index a7cc2f2a394..5b900d1e681 100644 --- a/apps/api/tests/unit/data/test_dataset_evolver.py +++ b/apps/api/tests/unit/data/test_dataset_evolver.py @@ -12,8 +12,10 @@ from cora.data.aggregates.dataset import ( DATASET_CHECKSUM_SHA256_HEX_LENGTH, + DatasetChecksum, DatasetDemoted, DatasetDiscarded, + DatasetEncoding, DatasetPromoted, DatasetRegistered, DatasetStatus, @@ -38,11 +40,9 @@ def test_evolve_registered_creates_dataset_with_registered_status() -> None: dataset_id=dataset_id, name="D", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=42, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=None, subject_id=None, derived_from=frozenset(), @@ -73,11 +73,12 @@ def test_evolve_preserves_optional_refs() -> None: dataset_id=uuid4(), name="D", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=0, - media_type="application/x-hdf5", - conforms_to=frozenset({"https://manual.nexusformat.org/"}), + encoding=DatasetEncoding( + media_type="application/x-hdf5", + conforms_to=frozenset({"https://manual.nexusformat.org/"}), + ), producing_run_id=run_id, subject_id=subject_id, derived_from=frozenset({derived}), @@ -102,11 +103,9 @@ def test_fold_single_register_event_returns_dataset() -> None: dataset_id=uuid4(), name="D", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=0, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=None, subject_id=None, derived_from=frozenset(), @@ -128,11 +127,9 @@ def test_evolve_registered_defaults_intent_to_trial() -> None: dataset_id=uuid4(), name="D", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=0, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=None, subject_id=None, derived_from=frozenset(), @@ -152,11 +149,9 @@ def test_evolve_registered_captures_producing_run_end_state_when_provided() -> N dataset_id=uuid4(), name="D", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=0, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=uuid4(), subject_id=None, derived_from=frozenset(), @@ -176,11 +171,9 @@ def _registered_event() -> DatasetRegistered: dataset_id=uuid4(), name="D", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=0, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=None, subject_id=None, derived_from=frozenset(), @@ -320,17 +313,15 @@ def test_demote_preserves_used_calibration_ids_asshot_invariant() -> None: dataset_id=uuid4(), name="seed", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=0, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=None, subject_id=None, derived_from=frozenset(), occurred_at=_NOW, producing_run_end_state=None, - intent="Trial", + intent=Intent.TRIAL, used_calibration_ids=(revision_id,), registered_by=_REGISTERED_BY, ) @@ -362,11 +353,9 @@ def test_evolve_discarded_preserves_producing_run_end_state() -> None: dataset_id=uuid4(), name="D", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=0, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=uuid4(), subject_id=None, derived_from=frozenset(), @@ -397,11 +386,9 @@ def test_evolve_promoted_preserves_producing_run_end_state() -> None: dataset_id=uuid4(), name="D", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=0, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=uuid4(), subject_id=None, derived_from=frozenset(), @@ -438,11 +425,9 @@ def test_register_genesis_populates_used_calibration_ids_as_frozenset() -> None: dataset_id=uuid4(), name="D", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=0, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=None, subject_id=None, derived_from=frozenset(), @@ -465,11 +450,9 @@ def test_legacy_dataset_registered_without_used_calibration_ids_folds_to_empty_f dataset_id=uuid4(), name="D", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=0, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=None, subject_id=None, derived_from=frozenset(), @@ -492,11 +475,9 @@ def test_discard_preserves_used_calibration_ids_asshot_invariant() -> None: dataset_id=uuid4(), name="D", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=0, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=None, subject_id=None, derived_from=frozenset(), @@ -527,11 +508,9 @@ def test_promote_preserves_used_calibration_ids_asshot_invariant() -> None: dataset_id=uuid4(), name="D", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=0, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=None, subject_id=None, derived_from=frozenset(), @@ -559,11 +538,9 @@ def _registered_with_kind(kind: str) -> DatasetRegistered: dataset_id=uuid4(), name="D", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=0, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=None, subject_id=None, derived_from=frozenset(), diff --git a/apps/api/tests/unit/data/test_demote_dataset_handler.py b/apps/api/tests/unit/data/test_demote_dataset_handler.py index d8f1fa74103..758b4cb60b8 100644 --- a/apps/api/tests/unit/data/test_demote_dataset_handler.py +++ b/apps/api/tests/unit/data/test_demote_dataset_handler.py @@ -27,6 +27,7 @@ event_type_name, to_payload, ) +from cora.data.aggregates.dataset.state import DatasetChecksum, DatasetEncoding, Intent from cora.data.features import demote_dataset from cora.data.features.demote_dataset import DemoteDataset from cora.infrastructure.adapters.in_memory_event_store import InMemoryEventStore @@ -47,17 +48,15 @@ async def _seed_registered( store: InMemoryEventStore, dataset_id: UUID, *, - intent: str = "Trial", + intent: Intent = Intent.TRIAL, ) -> None: event = DatasetRegistered( dataset_id=dataset_id, name="seed", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=0, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=None, subject_id=None, derived_from=frozenset(), diff --git a/apps/api/tests/unit/data/test_discard_dataset_handler.py b/apps/api/tests/unit/data/test_discard_dataset_handler.py index d7ee85d4781..72fd6d7ab00 100644 --- a/apps/api/tests/unit/data/test_discard_dataset_handler.py +++ b/apps/api/tests/unit/data/test_discard_dataset_handler.py @@ -17,6 +17,7 @@ event_type_name, to_payload, ) +from cora.data.aggregates.dataset.state import DatasetChecksum, DatasetEncoding from cora.data.features import discard_dataset from cora.data.features.discard_dataset import DiscardDataset from cora.infrastructure.adapters.in_memory_event_store import InMemoryEventStore @@ -38,11 +39,9 @@ async def _seed_registered(store: InMemoryEventStore, dataset_id: UUID) -> None: dataset_id=dataset_id, name="seed", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=0, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=None, subject_id=None, derived_from=frozenset(), diff --git a/apps/api/tests/unit/data/test_discard_distribution_handler.py b/apps/api/tests/unit/data/test_discard_distribution_handler.py index 6f1ef8759d4..d530e235fdc 100644 --- a/apps/api/tests/unit/data/test_discard_distribution_handler.py +++ b/apps/api/tests/unit/data/test_discard_distribution_handler.py @@ -23,6 +23,7 @@ from cora.data.aggregates.dataset.events import ( to_payload as dataset_to_payload, ) +from cora.data.aggregates.dataset.state import DatasetChecksum, DatasetEncoding from cora.data.aggregates.distribution import ( DistributionCannotDiscardError, DistributionCannotDiscardLastVerifiedError, @@ -31,6 +32,7 @@ event_type_name, to_payload, ) +from cora.data.aggregates.distribution.state import AccessProtocol from cora.data.features import discard_distribution from cora.data.features.discard_distribution import DiscardDistribution from cora.infrastructure.adapters.in_memory_event_store import InMemoryEventStore @@ -61,11 +63,9 @@ async def _seed_dataset(store: InMemoryEventStore, dataset_id: UUID) -> None: dataset_id=dataset_id, name="seed", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=0, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=None, subject_id=None, derived_from=frozenset(), @@ -98,12 +98,10 @@ async def _seed_distribution( dataset_id=_DATASET_ID, supply_id=supply_id, uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=0, - media_type="application/x-hdf5", - conforms_to=frozenset(), - access_protocol="S3", + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), + access_protocol=AccessProtocol.S3, occurred_at=_NOW, registered_by=_SEED_ACTOR_ID, ) diff --git a/apps/api/tests/unit/data/test_distribution_events.py b/apps/api/tests/unit/data/test_distribution_events.py index 8d7158368df..90da0359a6b 100644 --- a/apps/api/tests/unit/data/test_distribution_events.py +++ b/apps/api/tests/unit/data/test_distribution_events.py @@ -13,6 +13,7 @@ import pytest from cora.data.aggregates.dataset import DATASET_CHECKSUM_SHA256_HEX_LENGTH +from cora.data.aggregates.dataset.state import DatasetChecksum, DatasetEncoding from cora.data.aggregates.distribution import ( DistributionDiscarded, DistributionRegistered, @@ -22,6 +23,7 @@ from_stored, to_payload, ) +from cora.data.aggregates.distribution.state import AccessProtocol from cora.infrastructure.ports.event_store import StoredEvent from cora.shared.identity import ActorId @@ -57,12 +59,13 @@ def _registered() -> DistributionRegistered: dataset_id=_DATASET_ID, supply_id=_SUPPLY_ID, uri="s3://bucket/key.h5", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=1024, - media_type="application/x-hdf5", - conforms_to=frozenset({"https://manual.nexusformat.org/"}), - access_protocol="S3", + encoding=DatasetEncoding( + media_type="application/x-hdf5", + conforms_to=frozenset({"https://manual.nexusformat.org/"}), + ), + access_protocol=AccessProtocol.S3, occurred_at=_NOW, registered_by=_REGISTERED_BY, ) diff --git a/apps/api/tests/unit/data/test_get_dataset_handler.py b/apps/api/tests/unit/data/test_get_dataset_handler.py index ef1834e5770..b84368da628 100644 --- a/apps/api/tests/unit/data/test_get_dataset_handler.py +++ b/apps/api/tests/unit/data/test_get_dataset_handler.py @@ -14,6 +14,7 @@ event_type_name, to_payload, ) +from cora.data.aggregates.dataset.state import DatasetChecksum, DatasetEncoding from cora.data.features import get_dataset from cora.data.features.get_dataset import GetDataset from cora.infrastructure.adapters.in_memory_event_store import InMemoryEventStore @@ -33,11 +34,9 @@ async def _seed_dataset(store: InMemoryEventStore, dataset_id: UUID) -> None: dataset_id=dataset_id, name="32-ID FlyScan recon", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=1024, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=None, subject_id=None, derived_from=frozenset(), diff --git a/apps/api/tests/unit/data/test_ingest_scan_handler.py b/apps/api/tests/unit/data/test_ingest_scan_handler.py index 229ac94f725..866ce3e5584 100644 --- a/apps/api/tests/unit/data/test_ingest_scan_handler.py +++ b/apps/api/tests/unit/data/test_ingest_scan_handler.py @@ -13,7 +13,10 @@ import pytest -from cora.data.aggregates.acquisition import AcquisitionCannotRecordWithoutCapturingError +from cora.data.aggregates.acquisition import ( + AcquisitionCannotRecordWithoutCapturingError, + InvalidAcquisitionEvidenceError, +) from cora.data.aggregates.dataset import DatasetAlreadyIngestedError from cora.data.aggregates.distribution import DistributionCannotRegisterOnNonStorageSupplyError from cora.data.errors import InvalidScanFileError, UnauthorizedError @@ -199,9 +202,26 @@ async def test_ingest_records_file_timestamp_with_source_marker() -> None: events, _ = await store.load("Acquisition", _ACQUISITION_ID) payload = events[0].payload assert payload["captured_at"] == datetime.fromisoformat(_AWARE_RAW).isoformat() - assert payload["evidence"]["captured_at_source"] == "start_date" - assert payload["evidence"]["projection_count"] == 5 - assert payload["evidence"]["reader_kind"] == "Configured" + # Every field _description() populates survives the real + # _build_evidence -> decide_ingest -> record_acquisition.decide -> + # validate_evidence -> to_payload path, not just a hand-picked few. + assert payload["evidence"] == { + "reader_kind": "Configured", + "checksum_computer_kind": "Configured", + "captured_at_source": "start_date", + "captured_at_raw": _AWARE_RAW, + "projection_count": 5, + "flat_count": 2, + "dark_count": 2, + "invalid_count": 0, + "commanded_projection_count": 5, + "commanded_flat_count": 2, + "commanded_dark_count": 2, + "dropped_frame_count": 0, + "projection_angle_count": 5, + "projection_angle_first": 0.0, + "projection_angle_last": 180.0, + } async def test_ingest_unreadable_file_refusal_leaves_zero_events() -> None: @@ -227,6 +247,27 @@ async def test_ingest_incomplete_file_refusal_leaves_zero_events() -> None: assert await _stream_counts(store) == (0, 0, 0) +async def test_ingest_reader_names_an_unrecognized_captured_at_source_refuses() -> None: + """`Description.captured_at_source` is a plain str so a future layout + can name a timestamp no reader has produced yet (its own docstring); + `CapturedAtSource` has not caught up to that hypothetical layout yet. + This is now an InvalidAcquisitionEvidenceError from the composed + decider's validate_evidence call, not the InvalidScanFileError every + other refusal in this file raises (EVIDENCE_SCHEMA's pre-decider + check, which raised InvalidScanFileError for the same case, is gone; + see the handler module docstring's "Refusal order" section).""" + store = InMemoryEventStore() + handler = _bind( + _deps(store), + described=_description(captured_at_source="acquisition_time"), + ) + + with pytest.raises(InvalidAcquisitionEvidenceError, match="captured_at_source"): + await handler(_command(), principal_id=_PRINCIPAL_ID, correlation_id=_CORRELATION_ID) + + assert await _stream_counts(store) == (0, 0, 0) + + async def test_ingest_timestampless_file_without_operator_value_refuses() -> None: store = InMemoryEventStore() handler = _bind( diff --git a/apps/api/tests/unit/data/test_promote_dataset_handler.py b/apps/api/tests/unit/data/test_promote_dataset_handler.py index ea25d0452f8..037aedc3c98 100644 --- a/apps/api/tests/unit/data/test_promote_dataset_handler.py +++ b/apps/api/tests/unit/data/test_promote_dataset_handler.py @@ -22,6 +22,7 @@ event_type_name, to_payload, ) +from cora.data.aggregates.dataset.state import DatasetChecksum, DatasetEncoding, Intent from cora.data.features import promote_dataset from cora.data.features.promote_dataset import PromoteDataset from cora.infrastructure.adapters.in_memory_event_store import InMemoryEventStore @@ -44,7 +45,7 @@ async def _seed_registered( producing_run_id: UUID | None = None, producing_run_end_state: str | None = None, derived_from: frozenset[UUID] = frozenset(), - intent: str = "Trial", + intent: Intent = Intent.TRIAL, ) -> None: from cora.shared.identity import ActorId @@ -52,11 +53,9 @@ async def _seed_registered( dataset_id=dataset_id, name="seed", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=0, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=producing_run_id, subject_id=None, derived_from=derived_from, diff --git a/apps/api/tests/unit/data/test_publish_edition_handler.py b/apps/api/tests/unit/data/test_publish_edition_handler.py index f9f59bb1676..d1bd7707600 100644 --- a/apps/api/tests/unit/data/test_publish_edition_handler.py +++ b/apps/api/tests/unit/data/test_publish_edition_handler.py @@ -108,11 +108,9 @@ async def _seed_dataset_production( dataset_id=dataset_id, name="seed", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=1024, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=None, subject_id=None, derived_from=frozenset(), diff --git a/apps/api/tests/unit/data/test_record_acquisition_decider.py b/apps/api/tests/unit/data/test_record_acquisition_decider.py index 52ff9484e33..3ebbcc69464 100644 --- a/apps/api/tests/unit/data/test_record_acquisition_decider.py +++ b/apps/api/tests/unit/data/test_record_acquisition_decider.py @@ -14,6 +14,7 @@ from cora.data.aggregates.acquisition import ( AcquisitionAlreadyExistsError, AcquisitionCannotRecordWithoutCapturingError, + AcquisitionEvidence, AcquisitionStatus, InvalidAcquisitionCapturedAtError, InvalidAcquisitionEvidenceError, @@ -53,7 +54,7 @@ def _command(**overrides: object) -> RecordAcquisition: "captured_at": _CAPTURED_AT, "producing_run_id": None, "settings": {"exposure_ms": 200}, - "evidence": {"frames": 1801}, + "evidence": {"projection_count": 1801}, } base.update(overrides) return RecordAcquisition(**base) # type: ignore[arg-type] @@ -125,7 +126,7 @@ def test_decide_emits_acquisition_recorded_on_valid_command() -> None: assert event.occurred_at == _NOW assert event.recorded_by == _RECORDED_BY assert event.settings == {"exposure_ms": 200} - assert event.evidence == {"frames": 1801} + assert event.evidence == AcquisitionEvidence(projection_count=1801) @pytest.mark.unit @@ -169,7 +170,7 @@ def test_decide_raises_already_exists_on_non_none_state() -> None: producing_run_id=None, captured_at=_CAPTURED_AT, settings={}, - evidence={}, + evidence=AcquisitionEvidence(), recorded_at=_NOW, recorded_by=_RECORDED_BY, status=AcquisitionStatus.RECORDED, @@ -274,7 +275,7 @@ def test_decide_raises_on_malformed_evidence() -> None: with pytest.raises(InvalidAcquisitionEvidenceError): record_acquisition.decide( state=None, - command=_command(evidence={"bad": object()}), + command=_command(evidence={"frames": 1801}), context=_context(), now=_NOW, new_id=_NEW_ID, diff --git a/apps/api/tests/unit/data/test_record_acquisition_decider_properties.py b/apps/api/tests/unit/data/test_record_acquisition_decider_properties.py index 81447318c51..efc26d8b600 100644 --- a/apps/api/tests/unit/data/test_record_acquisition_decider_properties.py +++ b/apps/api/tests/unit/data/test_record_acquisition_decider_properties.py @@ -27,7 +27,9 @@ from cora.data.aggregates.acquisition import ( AcquisitionAlreadyExistsError, AcquisitionCannotRecordWithoutCapturingError, + AcquisitionEvidence, AcquisitionStatus, + validate_evidence, ) from cora.data.aggregates.acquisition.state import Acquisition from cora.data.aggregates.dataset import ( @@ -56,13 +58,41 @@ # rejected, covered by the example-based decider test). _BACKFILL_DELTA = st.timedeltas(min_value=timedelta(0), max_value=timedelta(days=365)) -# Primitive-leaf carrier dicts (settings / evidence shape today). +# settings has no declared shape today: primitive-leaf carrier dict. _CARRIER = st.dictionaries( keys=st.text(min_size=1, max_size=12), values=st.one_of(st.integers(), st.text(max_size=12), st.booleans(), st.none()), max_size=4, ) +# evidence DOES have a declared shape (AcquisitionEvidence): a random +# subset of its known keys, each independently present or absent, per +# the "0 vs None vs source-cannot-know" convention. +_EVIDENCE_CARRIER = st.fixed_dictionaries( + {}, + optional={ + "reader_kind": st.text(max_size=20), + "checksum_computer_kind": st.text(max_size=20), + "captured_at_source": st.sampled_from(["start_date", "end_date", "operator"]), + "captured_at_raw": st.text(max_size=40), + "projection_count": st.integers(min_value=0, max_value=100_000), + "flat_count": st.integers(min_value=0, max_value=100_000), + "dark_count": st.integers(min_value=0, max_value=100_000), + "invalid_count": st.integers(min_value=0, max_value=100_000), + "commanded_projection_count": st.integers(min_value=0, max_value=100_000), + "commanded_flat_count": st.integers(min_value=0, max_value=100_000), + "commanded_dark_count": st.integers(min_value=0, max_value=100_000), + "dropped_frame_count": st.integers(min_value=0, max_value=100_000), + "projection_angle_count": st.integers(min_value=0, max_value=100_000), + "projection_angle_first": st.floats( + min_value=-360.0, max_value=360.0, allow_nan=False, allow_infinity=False + ), + "projection_angle_last": st.floats( + min_value=-360.0, max_value=360.0, allow_nan=False, allow_infinity=False + ), + }, +) + def _dataset(dataset_id: UUID) -> Dataset: return Dataset( @@ -107,7 +137,7 @@ def _context( asset_id=st.uuids(), actor_id=st.uuids(), settings=_CARRIER, - evidence=_CARRIER, + evidence=_EVIDENCE_CARRIER, ) def test_genesis_emits_single_event_with_injected_fields_and_dual_time( now: datetime, @@ -146,7 +176,7 @@ def test_genesis_emits_single_event_with_injected_fields_and_dual_time( assert event.captured_at == captured_at assert event.occurred_at == now assert event.settings == settings - assert event.evidence == evidence + assert event.evidence == validate_evidence(evidence) @pytest.mark.unit @@ -175,7 +205,7 @@ def test_non_none_state_always_raises_already_exists( producing_run_id=None, captured_at=now - backfill, settings={}, - evidence={}, + evidence=AcquisitionEvidence(), recorded_at=now, recorded_by=ActorId(actor_id), status=AcquisitionStatus.RECORDED, diff --git a/apps/api/tests/unit/data/test_record_acquisition_handler.py b/apps/api/tests/unit/data/test_record_acquisition_handler.py index 4855f749f32..e35877da097 100644 --- a/apps/api/tests/unit/data/test_record_acquisition_handler.py +++ b/apps/api/tests/unit/data/test_record_acquisition_handler.py @@ -17,6 +17,8 @@ ) from cora.data.aggregates.dataset import ( DATASET_CHECKSUM_SHA256_HEX_LENGTH, + DatasetChecksum, + DatasetEncoding, DatasetNotFoundError, DatasetRegistered, ) @@ -78,11 +80,9 @@ async def _seed_dataset(store: InMemoryEventStore, dataset_id: UUID) -> None: dataset_id=dataset_id, name="recon.h5", uri="s3://b/recon.h5", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=1024, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=None, subject_id=None, derived_from=frozenset(), diff --git a/apps/api/tests/unit/data/test_record_attestation_handler.py b/apps/api/tests/unit/data/test_record_attestation_handler.py index ff7ba631dd5..8eea2c700f8 100644 --- a/apps/api/tests/unit/data/test_record_attestation_handler.py +++ b/apps/api/tests/unit/data/test_record_attestation_handler.py @@ -30,6 +30,7 @@ from cora.data.aggregates.dataset.events import ( to_payload as dataset_to_payload, ) +from cora.data.aggregates.dataset.state import DatasetChecksum, DatasetEncoding from cora.data.aggregates.distribution.events import ( DistributionRegistered, ) @@ -39,6 +40,7 @@ from cora.data.aggregates.distribution.events import ( to_payload as distribution_to_payload, ) +from cora.data.aggregates.distribution.state import AccessProtocol from cora.data.features import record_attestation from cora.data.features.record_attestation import RecordAttestation from cora.data.ports.checksum_verifier import ( @@ -105,11 +107,9 @@ async def _seed_dataset( dataset_id=dataset_id, name="seed", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=checksum_value, + checksum=DatasetChecksum(algorithm="sha256", value=checksum_value), byte_size=byte_size, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=None, subject_id=None, derived_from=frozenset(), @@ -145,12 +145,10 @@ async def _seed_distribution( dataset_id=dataset_id, supply_id=_SUPPLY_ID, uri=uri, - checksum_algorithm=checksum_algorithm, - checksum_value=checksum_value, + checksum=DatasetChecksum(algorithm=checksum_algorithm, value=checksum_value), byte_size=1024, - media_type="application/x-hdf5", - conforms_to=frozenset(), - access_protocol=access_protocol, + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), + access_protocol=AccessProtocol(access_protocol), occurred_at=_NOW, registered_by=ActorId(_PRINCIPAL_ID), ) diff --git a/apps/api/tests/unit/data/test_register_dataset_decider.py b/apps/api/tests/unit/data/test_register_dataset_decider.py index 913ad5b17de..0dcd26f926f 100644 --- a/apps/api/tests/unit/data/test_register_dataset_decider.py +++ b/apps/api/tests/unit/data/test_register_dataset_decider.py @@ -124,11 +124,11 @@ def test_decide_emits_dataset_registered_with_minimum_fields() -> None: assert event.dataset_id == new_id assert event.name == "32-ID FlyScan recon" assert event.uri == "s3://aps-32id/runs/abc/recon.h5" - assert event.checksum_algorithm == "sha256" - assert event.checksum_value == _GOOD_SHA256 + assert event.checksum.algorithm == "sha256" + assert event.checksum.value == _GOOD_SHA256 assert event.byte_size == 1024 - assert event.media_type == "application/x-hdf5" - assert event.conforms_to == frozenset() + assert event.encoding.media_type == "application/x-hdf5" + assert event.encoding.conforms_to == frozenset() assert event.producing_run_id is None assert event.subject_id is None assert event.derived_from == frozenset() @@ -188,7 +188,7 @@ def test_decide_accepts_encoding_conforms_to_set() -> None: new_id=uuid4(), registered_by=_REGISTERED_BY, ) - assert events[0].conforms_to == frozenset({"https://manual.nexusformat.org/"}) + assert events[0].encoding.conforms_to == frozenset({"https://manual.nexusformat.org/"}) # ---------- Field validation ---------- diff --git a/apps/api/tests/unit/data/test_register_dataset_handler.py b/apps/api/tests/unit/data/test_register_dataset_handler.py index 4cb8bac6454..c6de88c8d70 100644 --- a/apps/api/tests/unit/data/test_register_dataset_handler.py +++ b/apps/api/tests/unit/data/test_register_dataset_handler.py @@ -24,6 +24,7 @@ event_type_name, to_payload, ) +from cora.data.aggregates.dataset.state import DatasetChecksum, DatasetEncoding from cora.data.features import register_dataset from cora.data.features.register_dataset import RegisterDataset from cora.infrastructure.adapters.in_memory_event_store import InMemoryEventStore @@ -200,11 +201,9 @@ async def _seed_dataset(store: InMemoryEventStore, dataset_id: UUID) -> None: dataset_id=dataset_id, name="seed", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=0, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=None, subject_id=None, derived_from=frozenset(), diff --git a/apps/api/tests/unit/data/test_register_distribution_decider.py b/apps/api/tests/unit/data/test_register_distribution_decider.py index 38b98b7321d..b4eb027a2a7 100644 --- a/apps/api/tests/unit/data/test_register_distribution_decider.py +++ b/apps/api/tests/unit/data/test_register_distribution_decider.py @@ -146,11 +146,11 @@ def test_decide_emits_distribution_registered_with_all_fields() -> None: assert event.dataset_id == cmd.dataset_id assert event.supply_id == cmd.supply_id assert event.uri == cmd.uri - assert event.checksum_algorithm == "sha256" - assert event.checksum_value == _GOOD_SHA256 + assert event.checksum.algorithm == "sha256" + assert event.checksum.value == _GOOD_SHA256 assert event.byte_size == 1024 - assert event.media_type == "application/x-hdf5" - assert event.conforms_to == frozenset() + assert event.encoding.media_type == "application/x-hdf5" + assert event.encoding.conforms_to == frozenset() assert event.access_protocol == "S3" assert event.occurred_at == _NOW assert event.registered_by == _REGISTERED_BY @@ -199,7 +199,7 @@ def test_decide_passes_conforms_to_through() -> None: new_id=uuid4(), registered_by=_REGISTERED_BY, ) - assert events[0].conforms_to == frozenset({"https://manual.nexusformat.org/"}) + assert events[0].encoding.conforms_to == frozenset({"https://manual.nexusformat.org/"}) @pytest.mark.unit @@ -417,7 +417,7 @@ def test_decide_accepts_matching_sha256_tree_distribution() -> None: new_id=uuid4(), registered_by=_REGISTERED_BY, ) - assert events[0].checksum_algorithm == "sha256-tree" + assert events[0].checksum.algorithm == "sha256-tree" # ---------- Strict-not-idempotent ---------- diff --git a/apps/api/tests/unit/data/test_register_distribution_handler.py b/apps/api/tests/unit/data/test_register_distribution_handler.py index 25b0e80796f..1a9acfd6dee 100644 --- a/apps/api/tests/unit/data/test_register_distribution_handler.py +++ b/apps/api/tests/unit/data/test_register_distribution_handler.py @@ -21,6 +21,7 @@ event_type_name, to_payload, ) +from cora.data.aggregates.dataset.state import DatasetChecksum, DatasetEncoding from cora.data.aggregates.distribution import ( DistributionCannotRegisterOnNonStorageSupplyError, DistributionChecksumMismatchError, @@ -91,11 +92,9 @@ async def _seed_dataset( dataset_id=dataset_id, name="seed", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=checksum_value, + checksum=DatasetChecksum(algorithm="sha256", value=checksum_value), byte_size=byte_size, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=None, subject_id=None, derived_from=frozenset(), diff --git a/apps/api/tests/unit/data/test_register_edition_handler.py b/apps/api/tests/unit/data/test_register_edition_handler.py index 2a877f1640d..4a8fe9cd42d 100644 --- a/apps/api/tests/unit/data/test_register_edition_handler.py +++ b/apps/api/tests/unit/data/test_register_edition_handler.py @@ -22,6 +22,7 @@ event_type_name, to_payload, ) +from cora.data.aggregates.dataset.state import DatasetChecksum, DatasetEncoding from cora.data.aggregates.edition import EditionCannotBindToDiscardedDatasetError from cora.data.features import register_edition from cora.data.features.register_edition import CreatorEntry, RegisterEdition @@ -64,11 +65,9 @@ async def _seed_dataset( dataset_id=dataset_id, name="seed", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=1024, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=None, subject_id=None, derived_from=frozenset(), diff --git a/apps/api/tests/unit/data/test_seal_edition_handler.py b/apps/api/tests/unit/data/test_seal_edition_handler.py index d4abd523183..2f7fad6b5f4 100644 --- a/apps/api/tests/unit/data/test_seal_edition_handler.py +++ b/apps/api/tests/unit/data/test_seal_edition_handler.py @@ -87,11 +87,9 @@ async def _seed_dataset_production( dataset_id=dataset_id, name="seed", uri="s3://b/k", - checksum_algorithm="sha256", - checksum_value=_GOOD_SHA256, + checksum=DatasetChecksum(algorithm="sha256", value=_GOOD_SHA256), byte_size=1024, - media_type="application/x-hdf5", - conforms_to=frozenset(), + encoding=DatasetEncoding(media_type="application/x-hdf5", conforms_to=frozenset()), producing_run_id=None, subject_id=None, derived_from=frozenset(), diff --git a/apps/api/tests/unit/infrastructure/record_export/test_bundle.py b/apps/api/tests/unit/infrastructure/record_export/test_bundle.py index 863548b5e60..630eeb30e63 100644 --- a/apps/api/tests/unit/infrastructure/record_export/test_bundle.py +++ b/apps/api/tests/unit/infrastructure/record_export/test_bundle.py @@ -24,10 +24,15 @@ BundleDestinationNotEmptyError, ExportedRecord, MalformedBundleError, + ManifestRecordMismatchError, + RedactionResult, + TokenMap, build_manifest, hash_record, hash_redacted_record, + hash_redaction_profile, read_bundle_body, + redact_record, write_bundle, ) from cora.infrastructure.record_export._redaction import RedactedRecord @@ -65,7 +70,7 @@ def _record() -> ExportedRecord: def _manifest(record: ExportedRecord) -> object: - return build_manifest(record, watermark=42, git_commit=_COMMIT) + return build_manifest(record, git_commit=_COMMIT) def test_write_bundle_lays_out_the_names_the_design_fixed(tmp_path: Path) -> None: @@ -162,12 +167,18 @@ def test_non_object_line_refuses_rather_than_reading_partially(tmp_path: Path) - read_bundle_body(tmp_path / "b") -def test_manifest_carries_h3_only_when_a_redacted_record_is_supplied(tmp_path: Path) -> None: +def test_manifest_carries_h3_only_when_a_redaction_is_supplied(tmp_path: Path) -> None: record = _record() redacted = RedactedRecord(streams=record.streams, logbooks=record.logbooks) + redaction = RedactionResult( + redacted_record=redacted, + token_map=TokenMap(), + unfired_tier2_clearances=frozenset(), + unfired_tier1_fields=frozenset(), + ) - without = build_manifest(record, watermark=42, git_commit=_COMMIT) - with_h3 = build_manifest(record, watermark=42, git_commit=_COMMIT, redacted=redacted) + without = build_manifest(record, git_commit=_COMMIT) + with_h3 = build_manifest(record, git_commit=_COMMIT, redaction=redaction) assert without.published_record_hash is None assert with_h3.published_record_hash == hash_redacted_record(redacted) @@ -180,3 +191,116 @@ def test_h1_and_h3_differ_even_when_redaction_changed_nothing() -> None: redacted = RedactedRecord(streams=record.streams, logbooks=record.logbooks) assert hash_record(record) != hash_redacted_record(redacted) + + +def _other_record() -> ExportedRecord: + """A record with different content from `_record()`, so its H1/H3 + cannot coincidentally match a manifest built for the other one.""" + return ExportedRecord( + streams=( + { + "stream_type": "Run", + "stream_id": "01900000-0000-7000-8000-0000000000ff", + "event_type": "RunStarted", + "schema_version": 1, + "payload": {"note": "a different run entirely"}, + }, + ), + logbooks={}, + ) + + +def test_write_bundle_refuses_when_the_manifest_describes_a_different_record( + tmp_path: Path, +) -> None: + """Unredacted case: `write_bundle` must not accept a manifest built + from one record next to a different record. Before this guard + existed, neither argument was checked against the other at all.""" + manifest = build_manifest(_record(), git_commit=_COMMIT) + + with pytest.raises(ManifestRecordMismatchError): + write_bundle(_other_record(), manifest, tmp_path / "b") # pyright: ignore[reportArgumentType] + + assert not (tmp_path / "b").exists() + + +def _record_redactable_by_the_real_pipeline() -> ExportedRecord: + """A record whose stream rows carry every fixed column + `Tier1Redactor.redact_row` reads directly (`_record()`'s rows are + minimal and lack `transaction_id` / `event_id` / etc.), so it can go + through the REAL `redact_record` rather than an aliased + `RedactedRecord` copy. Tokenizing `stream_id` to a random surrogate + changes the body content, which is what makes H3 actually differ + from hashing the unredacted record -- an aliased copy is + byte-identical and cannot reproduce a real mismatch at all.""" + return ExportedRecord( + streams=( + { + "stream_type": "Run", + "stream_id": "01900000-0000-7000-8000-0000000000a1", + "event_type": "RunStarted", + "schema_version": 1, + "occurred_at": "2026-05-15T12:00:00+00:00", + "recorded_at": "2026-05-15T12:00:00+00:00", + "transaction_id": 1, + "event_id": "01900000-0000-7000-8000-0000000000e1", + "correlation_id": None, + "causation_id": None, + "principal_id": None, + "payload": {"note": "first"}, + }, + ), + logbooks={}, + ) + + +def _redact(record: ExportedRecord) -> RedactionResult: + return redact_record(record, expected_redaction_profile_hash=hash_redaction_profile()) + + +def test_write_bundle_refuses_an_unredacted_record_beside_a_manifest_carrying_h3( + tmp_path: Path, +) -> None: + """The exact reproduction: a manifest whose `published_record_hash` + (H3) was computed from the REAL redacted record, handed to + `write_bundle` alongside the UNREDACTED record instead. Before this + guard existed, this wrote a fully unredacted bundle that the default + verifier printed `OK` for under a `--published` label.""" + record = _record_redactable_by_the_real_pipeline() + redaction = _redact(record) + manifest = build_manifest(record, git_commit=_COMMIT, redaction=redaction) + + with pytest.raises(ManifestRecordMismatchError): + write_bundle(record, manifest, tmp_path / "b") # pyright: ignore[reportArgumentType] + + assert not (tmp_path / "b").exists() + + +def test_write_bundle_refuses_a_redacted_record_beside_an_h1_only_manifest( + tmp_path: Path, +) -> None: + """The mirror direction: a manifest with NO `published_record_hash` + (an unredacted-bundle manifest) handed the REDACTED record instead of + the one it was actually built from. Tokenized `stream_id`s and + dropped columns mean the redacted body cannot reproduce H1.""" + record = _record_redactable_by_the_real_pipeline() + redaction = _redact(record) + manifest = build_manifest(record, git_commit=_COMMIT) + + with pytest.raises(ManifestRecordMismatchError): + write_bundle(redaction.redacted_record, manifest, tmp_path / "b") + + assert not (tmp_path / "b").exists() + + +def test_write_bundle_accepts_the_redacted_record_beside_its_own_h3_manifest( + tmp_path: Path, +) -> None: + """The positive case: the record `build_manifest` actually hashed for + H3 is exactly what `write_bundle` was handed, so it must proceed.""" + record = _record_redactable_by_the_real_pipeline() + redaction = _redact(record) + manifest = build_manifest(record, git_commit=_COMMIT, redaction=redaction) + + bundle = write_bundle(redaction.redacted_record, manifest, tmp_path / "b") + assert (bundle / MANIFEST_NAME).is_file() diff --git a/apps/api/tests/unit/infrastructure/record_export/test_hash_redaction_profile.py b/apps/api/tests/unit/infrastructure/record_export/test_hash_redaction_profile.py index da73751e88f..cbb5f05a12a 100644 --- a/apps/api/tests/unit/infrastructure/record_export/test_hash_redaction_profile.py +++ b/apps/api/tests/unit/infrastructure/record_export/test_hash_redaction_profile.py @@ -5,14 +5,24 @@ `TIER2_JSONB_DROPPED_COLUMNS`) missing from H2: the hash covered only tier 1's generated `DISPOSITIONS`, so `redact_record`'s fail-closed switch could not detect a tier-2 table edit that weakened a -disposition. These tests pin the fix: every one of the four tables H2 -is supposed to cover must actually move the hash. +disposition. A later pass found the SAME gap one seam over: tier 1's +own hand-authored fixed-column tables +(`FIXED_KEEP_COLUMNS`/`FIXED_TOKEN_COLUMNS`/`FIXED_DROP_COLUMNS` in +`_redact_tier1.py`, which decide `principal_id`/`signature`/etc. for +every event) were ALSO outside H2 -- moving `signature` from DROP to +KEEP would not have moved the hash. These tests pin both fixes: every +table H2 is supposed to cover must actually move the hash. """ import pytest from cora.infrastructure.record_export import hash_redaction_profile from cora.infrastructure.record_export._dispositions import DISPOSITIONS +from cora.infrastructure.record_export._redact_tier1 import ( + FIXED_DROP_COLUMNS, + FIXED_KEEP_COLUMNS, + FIXED_TOKEN_COLUMNS, +) from cora.infrastructure.record_export._redact_tier2 import ( TIER2_DISPOSITIONS, TIER2_JSONB_CLEARED_POINTERS, @@ -48,3 +58,45 @@ def test_widening_a_tier2_jsonb_clearance_changes_the_hash( def test_hash_redaction_profile_is_stable_across_repeated_calls() -> None: assert hash_redaction_profile() == hash_redaction_profile() + + +def test_widening_a_tier1_fixed_drop_column_to_keep_changes_the_hash( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The gap the second pass found: `signature` moving from DROP to KEEP + (republishing a signature beside a redacted payload) used to be a + no-op on the hash, because these three tuples are hand-authored in + `_redact_tier1.py` and were never part of H2's body. Patched on + `_hashing`, where `hash_redaction_profile` actually reads the name + it imported, not on `_redact_tier1` (rebinding a tuple there would + not be visible through `_hashing`'s own already-bound import).""" + baseline = hash_redaction_profile() + monkeypatch.setattr( + "cora.infrastructure.record_export._hashing.FIXED_DROP_COLUMNS", + tuple(c for c in FIXED_DROP_COLUMNS if c != "signature"), + ) + assert hash_redaction_profile() != baseline + + +def test_narrowing_tier1_fixed_keep_columns_changes_the_hash( + monkeypatch: pytest.MonkeyPatch, +) -> None: + baseline = hash_redaction_profile() + monkeypatch.setattr( + "cora.infrastructure.record_export._hashing.FIXED_KEEP_COLUMNS", + FIXED_KEEP_COLUMNS[:-1], + ) + assert hash_redaction_profile() != baseline + + +def test_narrowing_tier1_fixed_token_columns_changes_the_hash( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The column this fixed table tokens is `principal_id`; moving it out + of TOKEN would be the attribution leak F5's threat model names.""" + baseline = hash_redaction_profile() + monkeypatch.setattr( + "cora.infrastructure.record_export._hashing.FIXED_TOKEN_COLUMNS", + tuple(c for c in FIXED_TOKEN_COLUMNS if c != "principal_id"), + ) + assert hash_redaction_profile() != baseline diff --git a/apps/api/tests/unit/infrastructure/record_export/test_manifest.py b/apps/api/tests/unit/infrastructure/record_export/test_manifest.py index 2add776afe2..a4fe6e27ef4 100644 --- a/apps/api/tests/unit/infrastructure/record_export/test_manifest.py +++ b/apps/api/tests/unit/infrastructure/record_export/test_manifest.py @@ -11,6 +11,9 @@ from cora.infrastructure.record_export import ( ExportedRecord, + RedactedRecord, + RedactionResult, + TokenMap, build_manifest, capture_git_commit, hash_record, @@ -59,7 +62,7 @@ def _run_started(run_id: str) -> dict[str, object]: return _stream_row(stream_type="Run", stream_id=run_id, event_type="RunStarted", payload={}) -def _record() -> ExportedRecord: +def _record(*, watermark: int = 100) -> ExportedRecord: streams = ( _run_started(_RUN_A), _run_started(_RUN_B), @@ -73,16 +76,16 @@ def _record() -> ExportedRecord: "activity": ({"step_kind": "setpoint"}, {"step_kind": "check"}), "observation": ({"is_simulated": True}, {"is_simulated": True}), } - return ExportedRecord(streams=streams, logbooks=logbooks) + return ExportedRecord(streams=streams, logbooks=logbooks, watermark=watermark) def test_logbook_row_counts_match_each_kinds_length() -> None: - manifest = build_manifest(_record(), watermark=100, git_commit="deadbeef") + manifest = build_manifest(_record(), git_commit="deadbeef") assert manifest.row_count_by_logbook_kind == {"activity": 2, "observation": 2} def test_max_schema_version_takes_the_max_per_event_type() -> None: - manifest = build_manifest(_record(), watermark=100, git_commit="deadbeef") + manifest = build_manifest(_record(), git_commit="deadbeef") # Two ProcedureRegistered rows in the fixture: schema_version 1 (the # three _procedure_registered() calls) and 2 (the last row). assert manifest.max_schema_version_by_event_type["ProcedureRegistered"] == 2 @@ -90,46 +93,65 @@ def test_max_schema_version_takes_the_max_per_event_type() -> None: def test_is_simulated_true_when_every_observation_says_so() -> None: - manifest = build_manifest(_record(), watermark=100, git_commit="deadbeef") + manifest = build_manifest(_record(), git_commit="deadbeef") assert manifest.is_simulated is True -def test_is_simulated_false_on_a_single_dissenting_observation() -> None: +def test_is_simulated_true_on_a_single_asserting_observation() -> None: + """Matches `bool_or`: ANY row asserting simulated is enough, mixed or + not. Renamed and flipped from `..._false_on_a_single_dissenting...`, + which pinned the `all(...)` inversion this fix corrects.""" record = ExportedRecord( streams=(), logbooks={"observation": ({"is_simulated": True}, {"is_simulated": False})}, ) - manifest = build_manifest(record, watermark=100, git_commit="deadbeef") + manifest = build_manifest(record, git_commit="deadbeef") + assert manifest.is_simulated is True + + +def test_is_simulated_false_when_every_observation_says_otherwise() -> None: + record = ExportedRecord( + streams=(), + logbooks={"observation": ({"is_simulated": False}, {"is_simulated": False})}, + ) + manifest = build_manifest(record, git_commit="deadbeef") assert manifest.is_simulated is False -def test_is_simulated_vacuously_true_with_no_observations() -> None: +def test_is_simulated_vacuously_false_with_no_observations() -> None: + """Matches the Run BC's `coalesce(bool_or(is_simulated), false)` + identity for an empty window. Renamed and flipped from + `..._vacuously_true_...`, which pinned the `all(...)` inversion this + fix corrects: the first genuine beamline-attached export had zero + observation rows and was reported simulated by that bug.""" record = ExportedRecord(streams=(), logbooks={}) - manifest = build_manifest(record, watermark=100, git_commit="deadbeef") - assert manifest.is_simulated is True + manifest = build_manifest(record, git_commit="deadbeef") + assert manifest.is_simulated is False def test_expansion_digest_present_only_for_the_run_whose_child_was_expanded() -> None: - manifest = build_manifest(_record(), watermark=100, git_commit="deadbeef") + manifest = build_manifest(_record(), git_commit="deadbeef") assert manifest.expansion_digest_presence_by_run == {_RUN_A: True, _RUN_B: False} def test_expansion_digest_ignores_procedures_with_no_parent_run() -> None: """_PROC_NO_RUN has parent_run_id=None; it must not create a phantom run entry or affect either real run's result.""" - manifest = build_manifest(_record(), watermark=100, git_commit="deadbeef") + manifest = build_manifest(_record(), git_commit="deadbeef") assert set(manifest.expansion_digest_presence_by_run) == {_RUN_A, _RUN_B} def test_manifest_hashes_match_calling_the_hash_functions_directly() -> None: record = _record() - manifest = build_manifest(record, watermark=100, git_commit="deadbeef") + manifest = build_manifest(record, git_commit="deadbeef") assert manifest.record_hash == hash_record(record) assert manifest.redaction_profile_hash == hash_redaction_profile() def test_manifest_carries_the_watermark_and_commit_verbatim() -> None: - manifest = build_manifest(_record(), watermark=4242, git_commit="cafef00d") + """`watermark` comes off `record.watermark`, not a separate parameter: + it is the value `export_record` itself captured its query with.""" + manifest = build_manifest(_record(watermark=4242), git_commit="cafef00d") assert manifest.watermark == 4242 assert manifest.git_commit == "cafef00d" @@ -139,21 +161,38 @@ def test_capture_git_commit_returns_a_full_sha() -> None: assert re.fullmatch(r"[0-9a-f]{40}", commit) +def _redaction_result( + record: ExportedRecord, + *, + unfired: frozenset[tuple[str, str, str]] = frozenset(), + unfired_tier1: frozenset[tuple[str, str]] = frozenset(), +) -> RedactionResult: + """A `RedactionResult` wrapping `record`'s own content unchanged, for + tests that only exercise `build_manifest`'s H3 / unfired-clearances + plumbing and do not need genuinely redacted (tokenized) content.""" + return RedactionResult( + redacted_record=RedactedRecord(streams=record.streams, logbooks=record.logbooks), + token_map=TokenMap(), + unfired_tier2_clearances=unfired, + unfired_tier1_fields=unfired_tier1, + ) + + def test_unfired_tier2_clearances_absent_without_redaction() -> None: """`None` means no redaction happened, the same convention as `published_record_hash`; unrelated to whether any clearance would have fired.""" - manifest = build_manifest(_record(), watermark=1, git_commit="deadbeef") + manifest = build_manifest(_record(), git_commit="deadbeef") assert manifest.unfired_tier2_clearances is None -def test_unfired_tier2_clearances_empty_when_none_supplied_but_redacted() -> None: - """Passing `redacted` without `unfired_tier2_clearances` reports an - empty tuple, not `None`: redaction DID happen, so absence-as-signal - no longer applies, and "empty" correctly reads as "nothing to - report" rather than "not tracked".""" +def test_unfired_tier2_clearances_empty_when_none_fired() -> None: + """A `RedactionResult` whose `unfired_tier2_clearances` is empty + reports an empty tuple, not `None`: redaction DID happen, so + absence-as-signal no longer applies, and "empty" correctly reads as + "nothing to report" rather than "not tracked".""" record = _record() - manifest = build_manifest(record, watermark=1, git_commit="deadbeef", redacted=record) + manifest = build_manifest(record, git_commit="deadbeef", redaction=_redaction_result(record)) assert manifest.unfired_tier2_clearances == () @@ -161,17 +200,70 @@ def test_unfired_tier2_clearances_renders_sorted_kind_column_pointer() -> None: record = _record() manifest = build_manifest( record, - watermark=1, git_commit="deadbeef", - redacted=record, - unfired_tier2_clearances=frozenset( - { - ("activity", "payload", "units"), - ("activity", "payload", "channel"), - } + redaction=_redaction_result( + record, + unfired=frozenset( + { + ("activity", "payload", "units"), + ("activity", "payload", "channel"), + } + ), ), ) assert manifest.unfired_tier2_clearances == ( "activity/payload/channel", "activity/payload/units", ) + + +def test_unfired_tier1_fields_absent_without_redaction() -> None: + """Same `None`-means-no-redaction convention as `unfired_tier2_clearances`.""" + manifest = build_manifest(_record(), git_commit="deadbeef") + assert manifest.unfired_tier1_fields is None + + +def test_unfired_tier1_fields_empty_when_none_unfired() -> None: + record = _record() + manifest = build_manifest(record, git_commit="deadbeef", redaction=_redaction_result(record)) + assert manifest.unfired_tier1_fields == () + + +def test_unfired_tier1_fields_renders_sorted_event_type_field() -> None: + record = _record() + manifest = build_manifest( + record, + git_commit="deadbeef", + redaction=_redaction_result( + record, + unfired_tier1=frozenset( + { + ("ProcedureRegistered", "kind"), + ("ProcedureRegistered", "capability_id"), + } + ), + ), + ) + assert manifest.unfired_tier1_fields == ( + "ProcedureRegistered/capability_id", + "ProcedureRegistered/kind", + ) + + +def test_expansion_digest_presence_by_run_is_keyed_by_the_redactions_own_surrogate() -> None: + """The published manifest must not carry the raw Run `stream_id` as a + dict key: it must be the SAME surrogate `TokenMap.token_uuid` would + hand back for that source, i.e. what tier-1 redaction already put on + the run's own rows. Threading `token_map` through `redaction` rather + than as an independent parameter makes it impossible to key by a + DIFFERENT redaction's surrogates than the one that produced the + streams body beside this manifest.""" + record = _record() + redaction = _redaction_result(record) + manifest = build_manifest(record, git_commit="deadbeef", redaction=redaction) + + assert _RUN_A not in manifest.expansion_digest_presence_by_run + assert _RUN_B not in manifest.expansion_digest_presence_by_run + surrogate_a = redaction.token_map.token_uuid(_RUN_A) + surrogate_b = redaction.token_map.token_uuid(_RUN_B) + assert manifest.expansion_digest_presence_by_run == {surrogate_a: True, surrogate_b: False} diff --git a/apps/api/tests/unit/infrastructure/record_export/test_redact_tier1.py b/apps/api/tests/unit/infrastructure/record_export/test_redact_tier1.py index 20e06b8ff35..7f45a88d8b1 100644 --- a/apps/api/tests/unit/infrastructure/record_export/test_redact_tier1.py +++ b/apps/api/tests/unit/infrastructure/record_export/test_redact_tier1.py @@ -161,3 +161,49 @@ def test_stream_id_correlation_id_and_event_id_are_tokened() -> None: def test_causation_id_none_stays_none() -> None: redacted = Tier1Redactor(TokenMap()).redact_row(_stream_row(causation_id=None)) assert redacted["causation_id"] is None + + +def test_fired_fields_records_declared_keys_actually_present_on_a_row() -> None: + """The tier-1 completeness twin to tier-2's `fired_pointers`: every + declared field key on the payload that had a real disposition entry, + not just a snapshot of the payload's own keys.""" + fired: dict[str, set[str]] = {} + redact_tier1_payload( + "AgentDefined", _agent_defined_payload(), token_map=TokenMap(), fired_fields=fired + ) + assert "agent_id" in fired["AgentDefined"] + assert "daily_token_cap" in fired["AgentDefined"] + + +def test_fired_fields_excludes_an_unlisted_key_that_only_dropped_by_omission() -> None: + """A key with no table entry (schema-evolution drop) is not a fired + RULE: nothing in the disposition table was exercised by it.""" + fired: dict[str, set[str]] = {} + payload = _agent_defined_payload() + payload["a_field_removed_in_a_later_schema_version"] = "still in an old row" + redact_tier1_payload("AgentDefined", payload, token_map=TokenMap(), fired_fields=fired) + assert "a_field_removed_in_a_later_schema_version" not in fired["AgentDefined"] + + +def test_fired_fields_defaults_to_none_and_costs_nothing_when_omitted() -> None: + """Existing callers that never pass `fired_fields` keep working.""" + redacted = redact_tier1_payload("AgentDefined", _agent_defined_payload(), token_map=TokenMap()) + assert redacted["daily_token_cap"] == 1000 + + +def test_tier1_redactor_exposes_fired_fields_per_event_type_accumulated_across_rows() -> None: + redactor = Tier1Redactor(TokenMap()) + redactor.redact_row(_stream_row(event_type="AgentDefined")) + fired = redactor.fired_fields + assert "AgentDefined" in fired + assert "agent_id" in fired["AgentDefined"] + + +def test_tier1_redactor_fired_fields_is_a_copy_not_a_live_view() -> None: + redactor = Tier1Redactor(TokenMap()) + redactor.redact_row(_stream_row()) + snapshot = redactor.fired_fields + redactor.redact_row( + _stream_row(event_type="AgentSuspended", payload={"agent_id": str(uuid4())}) + ) + assert "AgentSuspended" not in snapshot diff --git a/apps/api/tests/unit/infrastructure/record_export/test_redact_tier2.py b/apps/api/tests/unit/infrastructure/record_export/test_redact_tier2.py index fb8c4d912b2..6110317461f 100644 --- a/apps/api/tests/unit/infrastructure/record_export/test_redact_tier2.py +++ b/apps/api/tests/unit/infrastructure/record_export/test_redact_tier2.py @@ -9,6 +9,7 @@ from cora.infrastructure.record_export import TokenMap from cora.infrastructure.record_export._redact_tier2 import ( TIER2_DISPOSITIONS, + TIER2_JSONB_CLEARED_POINTERS, redact_tier2_row, unfired_clearances, ) @@ -58,24 +59,35 @@ def test_a_column_absent_from_the_disposition_table_is_omitted() -> None: def test_activities_payload_keeps_cleared_string_leaves_and_drops_others() -> None: + """`address`/`result`/`error_class` are real, closed-in-practice keys + `conductor.py` writes for a conducted step (slice 6 of + project_record_publishing_campaign.md); `message` and `name` are + deliberately NOT cleared -- `message` is free text (same posture as + `verdict.reason`), and `name` was cleared in this slice's first draft + then reverted during its own gate review: `ActionStep.name` reaches + the payload on the pre-lookup in-flight marker and the + `UnknownActionError` failure arm, both before or instead of the + registry check that would have closed it.""" row = { "event_id": str(uuid4()), "payload": { - "channel": "T_oven", - "target_value": 423.0, - "units": "K", - "action_name": "open_valve", - "an_uncleared_free_text_field": "should drop", + "address": "T_oven", + "value": 423.0, + "name": "open_valve", + "result": "ok", + "error_class": "ControlNotConnectedError", + "message": "should drop", }, } fired: dict[tuple[str, str], set[str]] = {} redacted = redact_tier2_row("activity", row, token_map=TokenMap(), fired_pointers=fired) payload = redacted["payload"] - assert payload["channel"] == "T_oven" - assert payload["units"] == "K" - assert payload["action_name"] == "open_valve" - assert payload["target_value"] == 423.0 - assert "an_uncleared_free_text_field" not in payload + assert payload["address"] == "T_oven" + assert payload["result"] == "ok" + assert payload["error_class"] == "ControlNotConnectedError" + assert payload["value"] == 423.0 + assert "name" not in payload + assert "message" not in payload def test_activities_payload_tokens_a_uuid_shaped_string_leaf() -> None: @@ -128,22 +140,29 @@ def test_every_declared_kind_has_at_least_one_uuid_scope_column(kind: str) -> No def test_unfired_clearance_names_the_pointer_that_never_matched() -> None: - """A narrow export (one setpoint, no units) is a normal export, not - an error: CORRECTED 2026-08-12, this used to raise. See + """A narrow export (one setpoint, address only) is a normal export, + not an error: CORRECTED 2026-08-12, this used to raise. See `unfired_clearances`'s own docstring for why raising here was a - denylist-shaped mistake applied to an allowlist mechanism.""" - # No channel/action_name/units in this payload. - row = {"event_id": str(uuid4()), "payload": {"target_value": 423.0}} + denylist-shaped mistake applied to an allowlist mechanism. + + Expected unfired set is DERIVED from `TIER2_JSONB_CLEARED_POINTERS` + (every declared pointer besides `address`, which this payload does + fire), not re-transcribed by hand -- hand-transcribing it is exactly + the mistake slice 6 of project_record_publishing_campaign.md fixed + for the clearance list itself. + """ + row = {"event_id": str(uuid4()), "payload": {"address": "T_oven", "value": 423.0}} fired: dict[tuple[str, str], set[str]] = {} redact_tier2_row("activity", row, token_map=TokenMap(), fired_pointers=fired) unfired = unfired_clearances(fired, kinds_present=frozenset({"activity"})) - assert unfired == { - ("activity", "payload", "channel"), - ("activity", "payload", "action_name"), - ("activity", "payload", "units"), + expected = { + ("activity", "payload", pointer) + for pointer in TIER2_JSONB_CLEARED_POINTERS[("activity", "payload")] + if pointer != "address" } + assert unfired == expected def test_unfired_clearance_for_a_kind_not_present_reports_empty() -> None: @@ -153,9 +172,23 @@ def test_unfired_clearance_for_a_kind_not_present_reports_empty() -> None: def test_all_declared_clearances_fired_reports_empty() -> None: + """Not a realistic single conducted step -- no real payload carries + every step kind's fields at once -- but every declared pointer needs + SOME payload shape that fires it, and this is the compact way to + prove each one still matches a real leaf position after any future + edit to `TIER2_JSONB_CLEARED_POINTERS`.""" row = { "event_id": str(uuid4()), - "payload": {"channel": "T_oven", "action_name": "open_valve", "units": "K"}, + "payload": { + "address": "T_oven", + "result": "ok", + "error_class": "ControlNotConnectedError", + "criterion": {"kind": "equals"}, + "reading": {"kind": "Scalar", "quality": "Good"}, + "post_reading": {"kind": "Scalar", "quality": "Good"}, + "post_read_error": {"error_class": "ControlNotConnectedError"}, + "measurements": [{"name": "flux", "units": "cps", "kind": "Scalar", "quality": "Good"}], + }, } fired: dict[tuple[str, str], set[str]] = {} redact_tier2_row("activity", row, token_map=TokenMap(), fired_pointers=fired) diff --git a/apps/api/tests/unit/infrastructure/record_export/test_standalone_verifier.py b/apps/api/tests/unit/infrastructure/record_export/test_standalone_verifier.py index aa9e6878072..59c19f8f756 100644 --- a/apps/api/tests/unit/infrastructure/record_export/test_standalone_verifier.py +++ b/apps/api/tests/unit/infrastructure/record_export/test_standalone_verifier.py @@ -196,9 +196,22 @@ def _write_bundle_for_cli(tmp_path: Path, *, published: bool) -> Path: Imports `cora` only to BUILD the fixture. The verification itself runs as a subprocess that never imports `cora`, which is the property under test. + + The `published=True` path goes through the REAL `redact_record`, not + an aliased `RedactedRecord` copy of the unredacted streams: an + aliased copy is byte-identical content, so H1 and H3 differ only by + payload type and the default (non-`--published`) check would happen + to still pass against it, unable to reproduce the mode-confusion bug + at all. Real redaction tokenizes `stream_id`, which actually changes + the body. """ - from cora.infrastructure.record_export import ExportedRecord, build_manifest, write_bundle - from cora.infrastructure.record_export._redaction import RedactedRecord + from cora.infrastructure.record_export import ( + ExportedRecord, + build_manifest, + hash_redaction_profile, + redact_record, + write_bundle, + ) record = ExportedRecord( streams=( @@ -207,18 +220,28 @@ def _write_bundle_for_cli(tmp_path: Path, *, published: bool) -> Path: "stream_id": "01900000-0000-7000-8000-0000000000a1", "event_type": "RunStarted", "schema_version": 1, + "occurred_at": "2026-05-15T12:00:00+00:00", + "recorded_at": "2026-05-15T12:00:00+00:00", + "transaction_id": 1, + "event_id": "01900000-0000-7000-8000-0000000000e1", + "correlation_id": None, + "causation_id": None, + "principal_id": None, "payload": {"note": _PRECOMPOSED_E_ACUTE, "target_value": 423.0}, }, ), logbooks={"activity": ({"event_id": "a1", "payload": {"channel": "2bma:x"}},)}, ) - redacted = ( - RedactedRecord(streams=record.streams, logbooks=record.logbooks) if published else None - ) - manifest = build_manifest(record, watermark=7, git_commit="0" * 40, redacted=redacted) bundle = tmp_path / "bundle" - write_bundle(redacted if redacted is not None else record, manifest, bundle) + if not published: + manifest = build_manifest(record, git_commit="0" * 40) + write_bundle(record, manifest, bundle) + return bundle + + redaction = redact_record(record, expected_redaction_profile_hash=hash_redaction_profile()) + manifest = build_manifest(record, git_commit="0" * 40, redaction=redaction) + write_bundle(redaction.redacted_record, manifest, bundle) return bundle @@ -270,6 +293,18 @@ def test_cli_published_flag_refuses_a_bundle_carrying_no_h3(tmp_path: Path) -> N assert "not a published projection" in result.stderr +def test_cli_default_verify_bundle_on_a_published_bundle_asks_for_the_flag( + tmp_path: Path, +) -> None: + """Forgetting `--published` on a genuinely published bundle must + not read as tampering. Before the fix this printed MISMATCH and + exited 1, byte-identical to the tampered-row case above.""" + result = _run_bundle_cli(_write_bundle_for_cli(tmp_path, published=True)) + assert result.returncode == 2 + assert "--published" in result.stderr + assert "MISMATCH:" not in result.stderr # the tamper-signal prefix, exit 1's format + + def test_cli_verify_bundle_refuses_a_directory_missing_its_manifest(tmp_path: Path) -> None: bundle = _write_bundle_for_cli(tmp_path, published=False) (bundle / "manifest.json").unlink() diff --git a/apps/api/tools/gen_record_dispositions.py b/apps/api/tools/gen_record_dispositions.py index f7d7018e206..4c028a85ce0 100644 --- a/apps/api/tools/gen_record_dispositions.py +++ b/apps/api/tools/gen_record_dispositions.py @@ -42,10 +42,13 @@ from typing import Any, Literal, NewType, Union, get_args, get_origin from uuid import UUID +from cora.shared.closed_value import ClosedValueObject + _API_ROOT = Path(__file__).resolve().parents[1] _SRC = _API_ROOT / "src" _OUT = _SRC / "cora" / "infrastructure" / "record_export" / "_dispositions.py" +KEEP_CLOSED = "keep:closed" KEEP_ENUM = "keep:enum" KEEP_NUMBER = "keep:number" KEEP_TIME = "keep:time" @@ -54,6 +57,28 @@ DROP_OPAQUE = "drop:opaque" BY_VALUE = "by-value" +# A field's dataclass name occasionally differs, DELIBERATELY, from the +# key `to_payload` actually writes it under. Every entry here is a +# documented design decision in the event module itself, not a typo: +# `to_payload` and `from_stored` already agree with EACH OTHER on the +# wire key, so nothing about the stored bytes changes; only the +# generated table's lookup key does, so redaction can find the field it +# already knows the wire calls something else. Renaming the dataclass +# field instead was rejected for the Seal events specifically because +# the wire key is under a cryptographic-chain immutability lock (Seal +# events.py's own module docstring, `project_slice6_design` L7); the +# other two follow the same convention for consistency across +# Federation's identity-bearing events. +_OVERRIDE_WIRE_KEYS: dict[tuple[str, str], str] = { + ("CredentialRegistered", "facility_code"): "facility_id", + ("PermitDefined", "peer_facility_code"): "peer_facility_id", + ("SealInitialized", "facility_code"): "facility_id", + ("SealPointerSigned", "facility_code"): "facility_id", + ("SealOnlineKeyRotated", "facility_code"): "facility_id", + ("SealRepublishingStarted", "facility_code"): "facility_id", + ("SealRepublishingCompleted", "facility_code"): "facility_id", +} + _SCALAR_KEEP: Mapping[type, str] = { bool: KEEP_NUMBER, int: KEEP_NUMBER, @@ -158,6 +183,14 @@ def _classify(annotation: Any, event: str, field: str) -> str | dict[str, Any]: if annotation is scalar: return disposition if _is_value_object(annotation): + if issubclass(annotation, ClosedValueObject): + # The whole VO closes its own range by construction (see + # `ClosedValueObject`'s docstring for the criterion); KEEP + # it whole rather than resolving field by field, so a + # bare `str` field inside it (a hex digest) does not fall + # through the generic drop-by-default rule that field + # would otherwise get on its own. + return f"{KEEP_CLOSED}:{annotation.__name__}" return _resolve_fields(annotation) origin = get_origin(annotation) @@ -182,11 +215,22 @@ def _classify(annotation: Any, event: str, field: str) -> str | dict[str, Any]: def _resolve_fields(cls: type) -> dict[str, Any]: - """Disposition per field of one dataclass, recursing into value objects.""" + """Disposition per field of one dataclass, recursing into value objects. + + The table is keyed on the STORED key, per `_OVERRIDE_WIRE_KEYS`, when + a field's dataclass name deliberately differs from what `to_payload` + writes it under; every other field's key is just its own name. Only + ever consulted with `cls.__name__` as the class actually being + resolved, so an override keyed on an EVENT class name (e.g. + `("CredentialRegistered", "facility_code")`) cannot accidentally + apply while recursing into an unrelated nested value object that + happens to share a field name. + """ hints = typing.get_type_hints(cls) out: dict[str, Any] = {} for spec in dataclasses.fields(cls): - out[spec.name] = _classify(hints[spec.name], cls.__name__, spec.name) + wire_key = _OVERRIDE_WIRE_KEYS.get((cls.__name__, spec.name), spec.name) + out[wire_key] = _classify(hints[spec.name], cls.__name__, spec.name) return out @@ -304,18 +348,26 @@ def render(table: Mapping[str, Mapping[str, Any]]) -> str: from the field's real type by `tools/gen_record_dispositions.py`. The vocabulary: - keep:enum: closed value set, provably reviewable. The enum is - NAMED because a human signs off the value set, and - swapping one enum for another must read as drift. - keep:number int / float / bool - keep:time datetime - token:uuid replaced with a per-export random surrogate - drop:text free text, no finite range, dropped by default - drop:opaque a dict with no declared keys, nothing to allowlist - by-value the slot is polymorphic across scalars and objects, - so no static answer exists. Apply the tier-2 leaf - rule at export time: numbers and booleans keep, - UUID-shaped strings token, other strings drop. + keep:enum: closed value set, provably reviewable. The + enum is NAMED because a human signs off the + value set, and swapping one enum for another + must read as drift. + keep:closed: a value object every one of whose fields is + closed by construction (a fixed charset and + length, a closed literal set), kept WHOLE + rather than resolved field by field. See + `cora.shared.closed_value.ClosedValueObject`. + keep:number int / float / bool + keep:time datetime + token:uuid replaced with a per-export random surrogate + drop:text free text, no finite range, dropped by default + drop:opaque a dict with no declared keys, nothing to + allowlist + by-value the slot is polymorphic across scalars and + objects, so no static answer exists. Apply the + tier-2 leaf rule at export time: numbers and + booleans keep, UUID-shaped strings token, + other strings drop. A nested mapping is a value object recursed into. A mapping whose sole key is `[]` is a fixed-length heterogeneous tuple, and its value lists @@ -325,6 +377,12 @@ def render(table: Mapping[str, Mapping[str, Any]]) -> str: key absent from its event's entry is dropped; an event type absent from this table aborts the export. The canonical hash of this mapping is the redaction profile hash recorded in the export manifest. + +A handful of entries are keyed on the WIRE key a field is actually +stored under rather than its dataclass field name, per +`gen_record_dispositions.py`'s `_OVERRIDE_WIRE_KEYS`: the two never +disagree about what ships, only about which name this table's lookup +uses to find it. """ from typing import Any diff --git a/docs/reference/modeling.md b/docs/reference/modeling.md index e87396ce5a3..d94388521c7 100644 --- a/docs/reference/modeling.md +++ b/docs/reference/modeling.md @@ -55,7 +55,18 @@ class ActorName: Each VO keeps its own frozen dataclass type, per-aggregate error class, and `MAX_LENGTH`. A shared base class would couple aggregates; a class factory would weaken `isinstance`. A free function avoids both. -**Primitives in events, VOs at state and decider boundaries.** Events carry primitives (str, int, UUID, datetime, dict), never VOs. Decider unwraps: `ActorRegistered(name=actor_name.value)`. Evolver re-validates: `Actor(name=ActorName(event.name))`. The round-trip test at `tests/unit//test_evolver.py` verifies this per aggregate. +**Primitives in events, VOs at state and decider boundaries, EXCEPT a closed vocabulary.** Events carry primitives (str, int, UUID, datetime, dict), never VOs. Decider unwraps: `ActorRegistered(name=actor_name.value)`. Evolver re-validates: `Actor(name=ActorName(event.name))`. The round-trip test at `tests/unit//test_evolver.py` verifies this per aggregate. + +The carve-out: a field whose VALUE SET is closed, a `StrEnum`, or a frozen VO every one of whose fields is closed by construction (a fixed charset and length, a closed literal set), may be declared on the event as that type directly, unwrapped-and-rewrapped ceremony skipped. Two independent forces created this exception and both must hold before using it: + +1. `tools/gen_record_dispositions.py`, the record exporter's redaction-profile generator, resolves a field's publishability from its DECLARED TYPE. A field wrapped down to bare `str` on the event is unpublishable by construction even when its own constructor already closes its range: this is why `DatasetRegistered.checksum: DatasetChecksum` (not `checksum_algorithm: str` + `checksum_value: str`) and `.intent: Intent` (not `str`) are declared as their real types. A hex digest and a closed trust-level tag disclose nothing a redaction reviewer needs to withhold, and wrapping them to `str` first only cost the record its own checksum for a release cycle (see `project_2bm_first_scan_record.md` F6, the published record of the first real 2-BM scan). +2. The type must be reachable from wherever the event class lives. `cora.data.aggregates` may depend on `cora.infrastructure` and `cora.shared` only (`tach.toml`), narrower than the feature layer above it (`cora.data`) that a decider runs in. `DatasetRegistered.producing_run_end_state` stays a bare `str`, deliberately, because the Run BC's `RunStatus` enum is reachable from `cora.data`'s deciders but not from `cora.data.aggregates`'s events, and a Data-BC-local mirror enum would raise at the decider on any future `RunStatus` member the mirror has not caught up to. A closed type that is not SAFELY reachable stays a primitive; that is the ordinary rule, not an exception to it. + +A frozen VO that opts into this carve-out for the record exporter's benefit marks itself with `cora.shared.closed_value.ClosedValueObject`, so the generator can ask a type object "does this VO close its own range?" without a hand-maintained list of class names. See that module's docstring for the exact criterion (every field closed, none free text) before subclassing it. + +A second, narrower reason to declare a VO directly on an event even when it is NOT closed: a `dict`/`Mapping`-typed field always resolves to `drop:opaque` whole, so a structured carrier with a MIX of closed and open leaves loses the closed ones too unless the generator can see the mix. `AcquisitionRecorded.evidence: AcquisitionEvidence` is this case: `reader_kind` and `checksum_computer_kind` are open-vocabulary strings that correctly recurse to `drop:text`, but `projection_count`, the angle range, and (via the `StrEnum` carve-out above) `captured_at_source` would otherwise be dropped along with them by the same all-or-nothing rule that made `evidence: dict[str, Any]` unpublishable by construction. `DatasetRegistered.encoding: DatasetEncoding` predates this reasoning and was justified ad hoc as "shape symmetry" with the closed `checksum` field on the same event; treat that docstring and this one as the same pattern, not two. + +When a genesis command carries TWO sibling freeform carrier dicts and only one gets this treatment (`AcquisitionRecorded.settings` stayed `dict[str, Any]` / `drop:opaque` while `evidence` was typed), the dividing line is real writer content, not a coin flip: type the one a production writer actually populates with a stable shape today; leave the other opaque until one does, rather than inventing a shape ahead of demand. This is a narrower, cheaper bar than a full `Capability.settings_schema` (`project_capability_settings_schema.md`'s per-Family JSON Schema mechanism, which several aggregates' `settings` fields are already deferred to): it only asks "does anything real write more than `{}` into this field," not "is there an operator-declared schema for it." A carrier that clears the Capability-schema bar (e.g. `Asset.settings`, populated in production and validated against a per-Family schema union) is a STRONGER candidate for this same treatment than `AcquisitionRecorded.settings` ever was; that gap is a known, unscoped follow-up, not evidence the rule is wrong. ## Field grouping diff --git a/scripts/verify_record_hash.py b/scripts/verify_record_hash.py index 059d961a99d..3f4105a3b4e 100644 --- a/scripts/verify_record_hash.py +++ b/scripts/verify_record_hash.py @@ -39,7 +39,10 @@ DOI landing page) rather than from a file the same tamperer could edit. Exit codes: 0 success (hash printed, or verify matched); 1 verify -mismatch; 2 the input could not be read or parsed. +mismatch; 2 the input could not be read or parsed, OR the wrong mode was +used for this bundle (`verify-bundle` with no `--published` against a +manifest that carries `published_record_hash`, or `--published` against +one that does not). """ # pyright: reportUnknownMemberType=false, reportUnknownVariableType=false, reportUnknownArgumentType=false @@ -189,6 +192,24 @@ def _verify_bundle(bundle: Path, *, published: bool) -> int: print(f"{MANIFEST_NAME} is not a JSON object", file=sys.stderr) return 2 + # The symmetric guard to the one four lines below. A bundle + # whose manifest carries `published_record_hash` (H3) is structurally + # a published projection -- checking it against `record_hash` (H1) + # instead recomputes over the redacted body with the wrong payload + # type, which mismatches for two independent reasons and prints + # MISMATCH: byte-identical, on this CLI, to genuine tampering. Refuse + # by the manifest's own shape, before ever comparing a digest. + if not published and isinstance(manifest.get("published_record_hash"), str): + print( + "cannot verify: this bundle's manifest carries published_record_hash " + "(H3), so it is a published projection. Checking it against " + "record_hash (H1) would compare the redacted body to the unredacted " + "record's hash and print MISMATCH, indistinguishable from tampering. " + "Re-run with --published.", + file=sys.stderr, + ) + return 2 + field = "published_record_hash" if published else "record_hash" payload_type = PUBLISHED_RECORD_PAYLOAD_TYPE if published else RECORD_PAYLOAD_TYPE expected = manifest.get(field)