From 4a88c5684a28e982e2ffdb9fd45cc416e958f7c2 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 2 Sep 2026 11:00:05 -0400 Subject: [PATCH 01/22] Add one shared chronicle-first env helper Chronicle's operational stores migrate by dual-run (chronicle#143, mechanism 3). Every env read was ledger-first with no chronicle-named alternative, and each of the three modules that read configuration had grown its own helper. Add chronicle/env.py: env_value/env_flag expand a name into CHRONICLE_, LEDGER_, POLICYENGINE_LEDGER_ and return the first set value, warning once per process with ChronicleEnvDeprecationWarning when a ledger-era name supplied it. Names outside those three prefixes, such as POLICYENGINE_SUPABASE_URL, are read literally so the helper renames the ledger-era surface only. The warning subclasses FutureWarning, not DeprecationWarning, so operators running the CLI actually see it. The Supabase schema name stays "ledger"; only the env var that overrides it moves to CHRONICLE_SCHEMA. Renaming the schema is a later slice. Co-Authored-By: Claude Fable 5.1 --- PROGRESS.md | 74 ++++++++++------------ chronicle/__init__.py | 1 + chronicle/env.py | 120 ++++++++++++++++++++++++++++++++++++ chronicle/source_package.py | 25 ++------ db/pe_source_inventory.py | 21 +++---- db/supabase_client.py | 23 +++---- 6 files changed, 174 insertions(+), 90 deletions(-) create mode 100644 chronicle/env.py diff --git a/PROGRESS.md b/PROGRESS.md index 24773009..e46b096c 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -1,51 +1,41 @@ -# Lane C5 progress +# Operational rename, slice 1 (chronicle#143, mechanism 3) + +Lane C5's handoff notes previously lived here; its durable record is +`LANE_C5_REPORT.md`. This file now tracks the active lane on this branch. ## State -- Branch: `be-2025-vintages` from `origin/main` at `5c15bfd`. -- Worktree inputs are staged under `.lane-raw/` and must remain uncommitted. -- Lane C5 is complete, validated, independently reviewed, and ready for handoff. -- The requested staged C2 report is absent, but root `LANE_C2_REPORT.md` is byte-identical - to the sibling lane's staged copy (SHA-256 `4590e0dc...50f06e7`) and is the pattern used. +- Branch: `ops-rename-slice1`, cut from `origin/main` at `ff3efd3`. +- Scope: env names, R2 bucket configurability, `ledger.db` -> `chronicle.db`, + and the docs for all three. Code and docs only; no infrastructure changes. +- Out of scope and deliberately untouched: the `ledger` console-script alias, + the Supabase `"ledger"` schema and mirror table names, governance role ids and + concept authorities, hash domains and schema ids, anything under `releases/`. ## Done -- Read the repository Chronicle boundary rules in `AGENTS.md`. -- Read `.lane-raw/SOURCES.md` and confirmed all five named publisher artifacts are present. -- Confirmed the worktree is otherwise clean apart from `.lane-raw/` and the shared `.venv` link. -- Verified all five staged artifact SHA-256 pins exactly. -- Mapped FPB workbook cells: 990 facts across T01/T06/T07/T11/T17/T24, with - 2022–2025 observations and 2026–2031 `source_projection` facts. -- Confirmed PDF boundary evidence: printed page 19 calls 2026 the first projection year; - annex table units appear on printed pages 45, 48, 49, 53, 58, and 65. -- Chosen Eurostat layout: two vintage-specific source-package aliases share new manifest - entries, preserving the prior package YAMLs, raw bytes, and fact outputs unchanged. -- Reproduced the Statbel curator logic: 18 NUTS1 × sex × age-band cells totaling 11,825,551. -- Added the hash-pinned FPB workbook and publication PDF plus the - `fpb-economic-outlook-2026-2031-june-2026` package alias. -- Built 990 line-specific publisher facts (99 per year): 396 observations for - 2022–2025 and 594 `source_projection` facts for 2026–2031. -- Passed FPB `validate-package` and `build-suite`: 990 facts, full cell lineage, - zero acceptance errors, and pinned 2025 cells 320578 / 77771 / 5602 million euro. -- Re-ran the Statbel 2026 curator logic on the 2025 ZIP and added the hash-pinned - raw capture plus its deterministic 18-row curated CSV. -- Passed Statbel 2025 `validate-package` and `build-suite`: 18 facts totaling - 11,825,551, 66 constraints, full lineage, and zero acceptance errors. -- Added the Eurostat `gov_10a_taxag` 2025 and `spr_exp_func` 2024 manifest - entries plus vintage-specific package aliases, without modifying either - prior artifact or prior package specification. -- Passed both new Eurostat package validations and suite builds: 12 tax facts - and 9 ESSPROS facts, full lineage, and zero acceptance errors. -- Extended Belgium and Eurostat regressions for FPB table counts/cells and - assertion boundary, vintage non-overlap, prior-output digests, Statbel pins, - and the declared 0.25% Statbel/FPB population comparison tolerance. -- Passed 43 focused tests and the full merged-bundle regression: 157,177 facts, - 148 packages, zero aggregate-key duplicates, and expected goldens throughout. -- Recorded pins, counts, boundary evidence, curator commands, validation tails, - and consumer fact families in `LANE_C5_REPORT.md`. -- Passed independent `ledger-source-fidelity` and `ledger-boundary` reviews with - no required corrections. +- Read `AGENTS.md`, `docs/storage-architecture.md`, + `docs/agent-source-package-harness.md`, and the mechanism-3 migration spec in + the first comment of PolicyEngine/chronicle#143. +- Enumerated every ledger-named env read in tracked Python: the four real + variables (`LEDGER_SOURCE_ARTIFACT_CACHE_DIR`, `LEDGER_SOURCE_ARTIFACT_FETCH`, + `LEDGER_PE_US_DATA_ROOT`, `LEDGER_PE_UK_DATA_ROOT`) plus + `POLICYENGINE_LEDGER_SCHEMA`. `LEDGER_MIRROR_TABLES`, + `LEDGER_MIRROR_PRIMARY_KEYS`, and `LEDGER_DB_SCHEMA_VERSION` are module + constants, not env reads, and name out-of-scope surfaces. +- Added `chronicle/env.py`: one shared `env_value`/`env_flag`/`env_names` + helper reading `CHRONICLE_` first, then `LEDGER_` and + `POLICYENGINE_LEDGER_` with a once-per-process + `ChronicleEnvDeprecationWarning` naming the preferred variable. +- Replaced all three ad-hoc helpers (`db/supabase_client._env`, + `chronicle/source_package._env_value`/`_truthy_env`, + `db/pe_source_inventory._env_value`) with the shared helper. ## Next -- None; ready for handoff. No push was performed. +- Make R2 bucket names configurable with unchanged `ledger-*` defaults. +- Emit `chronicle.db` for new suite outputs; keep reading `ledger.db`. +- Fix the backwards fallback statement in the docs and sweep every env-name, + bucket-name, and db-filename mention in README/AGENTS/docs. +- Add the hermetic dual-read tests; run pytest, ruff check, ruff format --check. +- Push and open the PR. Do not merge. diff --git a/chronicle/__init__.py b/chronicle/__init__.py index 04d50943..e3ddfb06 100644 --- a/chronicle/__init__.py +++ b/chronicle/__init__.py @@ -13,6 +13,7 @@ "consumer_contract", "core", "database", + "env", "facts", "harness", "jurisdictions", diff --git a/chronicle/env.py b/chronicle/env.py new file mode 100644 index 00000000..80667f4d --- /dev/null +++ b/chronicle/env.py @@ -0,0 +1,120 @@ +"""Environment configuration for the Chronicle rename window. + +Chronicle's operational stores migrate by dual-run (PolicyEngine/chronicle#143, +mechanism 3): every configuration variable gets a ``CHRONICLE_``-prefixed name +that is read first, while the ledger-era ``LEDGER_`` and +``POLICYENGINE_LEDGER_`` names keep working behind a deprecation warning. That +window lets downstream publish flows migrate on their own schedule instead of +breaking the moment Chronicle ships a rename. + +Names that carry none of those three prefixes are read literally: this helper +renames the ledger-era surface, not every PolicyEngine variable. +""" + +from __future__ import annotations + +import os +from typing import TypeVar +import warnings + +__all__ = [ + "CHRONICLE_ENV_PREFIX", + "ChronicleEnvDeprecationWarning", + "LEGACY_ENV_PREFIXES", + "env_flag", + "env_names", + "env_value", + "reset_env_deprecation_state", +] + +CHRONICLE_ENV_PREFIX = "CHRONICLE_" + +# Ordered most specific first so prefix stripping is unambiguous. +LEGACY_ENV_PREFIXES = ("POLICYENGINE_LEDGER_", "LEDGER_") + +TRUTHY_ENV_VALUES = frozenset({"1", "true", "yes", "on"}) + + +class ChronicleEnvDeprecationWarning(FutureWarning): + """A ledger-era environment variable supplied a Chronicle setting. + + Subclasses :class:`FutureWarning` rather than :class:`DeprecationWarning` + so the notice reaches operators running the CLI, who are the people who + have to move the variable. ``DeprecationWarning`` is silenced by default + outside ``__main__``. + """ + + +_Default = TypeVar("_Default") + +_WARNED_LEGACY_NAMES: set[str] = set() + + +def _env_suffix(name: str) -> str | None: + """Return the rename-window suffix of ``name``, or None if it has none.""" + for prefix in (CHRONICLE_ENV_PREFIX, *LEGACY_ENV_PREFIXES): + if name.startswith(prefix) and len(name) > len(prefix): + return name[len(prefix) :] + return None + + +def env_names(name: str) -> tuple[str, ...]: + """Return the lookup order for ``name``. + + The chronicle-preferred name comes first, then the ledger-era names that + remain accepted during the migration window. A name outside the rename + window is returned unchanged, as its own single-element lookup order. + """ + suffix = _env_suffix(name) + if suffix is None: + return (name,) + return ( + f"{CHRONICLE_ENV_PREFIX}{suffix}", + *(f"{prefix}{suffix}" for prefix in LEGACY_ENV_PREFIXES), + ) + + +def _warn_legacy(found: str, preferred: str) -> None: + """Warn once per process that a ledger-era variable supplied a value.""" + if found in _WARNED_LEGACY_NAMES: + return + _WARNED_LEGACY_NAMES.add(found) + warnings.warn( + f"{found} is a ledger-era Chronicle environment variable; " + f"set {preferred} instead. The old name is still honored during the " + "Chronicle rename window and will be removed once consumers migrate.", + ChronicleEnvDeprecationWarning, + stacklevel=3, + ) + + +def reset_env_deprecation_state() -> None: + """Forget which legacy names have already warned. Test-support hook.""" + _WARNED_LEGACY_NAMES.clear() + + +def env_value(*names: str, default: _Default = None) -> str | _Default: + """Read the first set value across ``names``, chronicle-preferred first. + + Each name is expanded through :func:`env_names`, so a caller can pass the + chronicle name and still pick up a value set under a ledger-era name. + Empty values are treated as unset, matching the helpers this replaces. + """ + for name in names: + candidates = env_names(name) + preferred = candidates[0] + for candidate in candidates: + value = os.environ.get(candidate) + if value: + if candidate != preferred: + _warn_legacy(candidate, preferred) + return value + return default + + +def env_flag(*names: str) -> bool: + """Return whether the first set value across ``names`` reads as true.""" + value = env_value(*names) + if value is None: + return False + return value.strip().lower() in TRUTHY_ENV_VALUES diff --git a/chronicle/source_package.py b/chronicle/source_package.py index 7234621b..d28fd641 100644 --- a/chronicle/source_package.py +++ b/chronicle/source_package.py @@ -3,7 +3,6 @@ from __future__ import annotations import hashlib -import os from dataclasses import dataclass, replace from importlib.resources import files from io import BytesIO @@ -32,6 +31,7 @@ AggregateFact, build_label, ) +from chronicle.env import env_flag, env_value from chronicle.sources.cells import ( SourceArtifactMetadata, SourceCell, @@ -364,8 +364,8 @@ "usda_snap/fy2025_monthly_state_caseloads" ), } -SOURCE_ARTIFACT_CACHE_ENV = "LEDGER_SOURCE_ARTIFACT_CACHE_DIR" -SOURCE_ARTIFACT_FETCH_ENV = "LEDGER_SOURCE_ARTIFACT_FETCH" +SOURCE_ARTIFACT_CACHE_ENV = "CHRONICLE_SOURCE_ARTIFACT_CACHE_DIR" +SOURCE_ARTIFACT_FETCH_ENV = "CHRONICLE_SOURCE_ARTIFACT_FETCH" DEFAULT_SOURCE_ARTIFACT_CACHE_DIR = ( Path.home() / ".cache" / "policyengine-chronicle" / "source-artifacts" ) @@ -2218,7 +2218,7 @@ def _read_source_artifact_content( if cache_path.exists(): return cache_path.read_bytes() - if not _truthy_env(SOURCE_ARTIFACT_FETCH_ENV): + if not env_flag(SOURCE_ARTIFACT_FETCH_ENV): raise FileNotFoundError( f"Source artifact {spec['filename']} is not packaged and was not " f"found in {cache_path}. Set {SOURCE_ARTIFACT_FETCH_ENV}=1 to fetch " @@ -2240,7 +2240,7 @@ def _read_source_artifact_content( def _source_artifact_cache_path(spec: dict[str, Any]) -> Path: cache_root = Path( - _env_value( + env_value( SOURCE_ARTIFACT_CACHE_ENV, default=DEFAULT_SOURCE_ARTIFACT_CACHE_DIR, ) @@ -2277,21 +2277,6 @@ def _validate_source_artifact_sha( ) -def _env_value(*names: str, default: str | Path) -> str | Path: - for name in names: - value = os.environ.get(name) - if value: - return value - return default - - -def _truthy_env(*names: str) -> bool: - return any( - os.environ.get(name, "").strip().lower() in {"1", "true", "yes", "on"} - for name in names - ) - - def _single_archive_member(archive: ZipFile, *, suffixes: tuple[str, ...]) -> str: members = [ name diff --git a/db/pe_source_inventory.py b/db/pe_source_inventory.py index ceac5bdd..a4a61691 100644 --- a/db/pe_source_inventory.py +++ b/db/pe_source_inventory.py @@ -2,14 +2,15 @@ from __future__ import annotations -import os from pathlib import Path +from chronicle.env import env_value + from .schema import Jurisdiction from .source_files import SourceArtifactSpec, make_slug, make_url_slug -PE_US_DATA_ROOT_ENV = "LEDGER_PE_US_DATA_ROOT" -PE_UK_DATA_ROOT_ENV = "LEDGER_PE_UK_DATA_ROOT" +PE_US_DATA_ROOT_ENV = "CHRONICLE_PE_US_DATA_ROOT" +PE_UK_DATA_ROOT_ENV = "CHRONICLE_PE_UK_DATA_ROOT" SOURCE_SUFFIXES = { ".csv", @@ -271,21 +272,13 @@ ] -def _env_value(*names: str) -> str | None: - for name in names: - value = os.environ.get(name) - if value: - return value - return None - - def _resolve_required_root( root: Path | None, *, flag: str, env_var: str, ) -> Path: - value = root if root is not None else _env_value(env_var) + value = root if root is not None else env_value(env_var) if value is None: raise ValueError(f"{flag} or {env_var} is required.") path = Path(value).expanduser() @@ -611,8 +604,8 @@ def pe_source_specs( ) -> list[SourceArtifactSpec]: """Return source files used by the PE-US and PE-UK calibration pipelines.""" specs: list[SourceArtifactSpec] = [] - us_configured = pe_us_root is not None or _env_value(PE_US_DATA_ROOT_ENV) - uk_configured = pe_uk_root is not None or _env_value(PE_UK_DATA_ROOT_ENV) + us_configured = pe_us_root is not None or env_value(PE_US_DATA_ROOT_ENV) + uk_configured = pe_uk_root is not None or env_value(PE_UK_DATA_ROOT_ENV) if include_us and (us_configured or not include_uk or not uk_configured): specs.extend(pe_us_source_specs(pe_us_root)) if include_uk and (uk_configured or not include_us or not us_configured): diff --git a/db/supabase_client.py b/db/supabase_client.py index e14bb5bb..50ce4f30 100644 --- a/db/supabase_client.py +++ b/db/supabase_client.py @@ -8,7 +8,6 @@ from __future__ import annotations -import os from dataclasses import dataclass from functools import lru_cache from typing import Any, Dict, List, Optional @@ -16,18 +15,14 @@ from supabase import create_client, Client +from chronicle.env import env_value -def _env(*names: str) -> str | None: - """Read PolicyEngine-owned storage config.""" - for name in names: - value = os.environ.get(name) - if value: - return value - return None - - -LEDGER_SCHEMA = _env("POLICYENGINE_LEDGER_SCHEMA") or "ledger" -TARGETS_SCHEMA = _env("POLICYENGINE_TARGETS_SCHEMA") or "targets" +# The hosted Postgres schema is still named "ledger"; only the environment +# variable that overrides it has moved to the chronicle prefix. Renaming the +# schema itself is a later slice of PolicyEngine/chronicle#143, coordinated +# with the CI writers that already target the ledger schema. +LEDGER_SCHEMA = env_value("CHRONICLE_SCHEMA") or "ledger" +TARGETS_SCHEMA = env_value("POLICYENGINE_TARGETS_SCHEMA") or "targets" @dataclass @@ -49,14 +44,14 @@ def from_env(cls) -> "SupabaseConfig": Raises: ValueError: If required environment variables are missing """ - url = _env("POLICYENGINE_SUPABASE_URL") + url = env_value("POLICYENGINE_SUPABASE_URL") if not url: raise ValueError( "POLICYENGINE_SUPABASE_URL not set. " "Set this to your Supabase project URL." ) - secret_key = _env( + secret_key = env_value( "POLICYENGINE_SUPABASE_SERVICE_KEY", "POLICYENGINE_SUPABASE_SECRET_KEY", ) From cf07e0dd6c9d9228d8ba3c678317dc77cd29adbf Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 2 Sep 2026 11:06:52 -0400 Subject: [PATCH 02/22] Make R2 buckets configurable and emit chronicle.db Bucket names: add CHRONICLE_R2_RAW_BUCKET / CHRONICLE_R2_DERIVED_BUCKET, resolved in the function body rather than bound as keyword defaults so the setting reaches long-lived processes. Defaults are unchanged at ledger-raw and ledger-derived; only the follow-up cutover PR flips them. Plumbed through fetch-artifact, publish-raw, publish-derived and bootstrap-r2, whose --r2-bucket/--raw-bucket/--derived-bucket now default to the resolver. Because the bucket can now vary, two manifest write paths could restate recorded storage.r2 blocks. Archived witness records pin raw R2 URLs by hash, so both now preserve history: publish-raw reports recorded_r2_bucket_is_preserved_history instead of uploading, and fetch-artifact keeps an already-recorded r2 block rather than overwriting it with a different bucket. The consumer-fact boundary guard matched the literal strings ledger-derived: and r2://ledger-derived/, so a renamed bucket would have made it silently stop firing. It now matches on shape (bucket ends in -derived, or key starts with derived/) and parses the r2:// URI, which also closes the uri-only hole where a fact carrying no bucket or key slipped past. Database artifact: new suite outputs write chronicle.db. infer_build_id reads chronicle.db then ledger.db (both branches were byte-identical, so the existing fallback was a no-op), and _derived_artifact_kind classifies both names (its set literal held one element twice). The sidecar resource list moves with the write site because _resource_descriptor stats every listed path. Fix db/cli.py, which imported the private env reader that the shared helper replaced. That ImportError broke every db CLI subcommand, including the three the CI job runs, while pytest stayed green. Co-Authored-By: Claude Fable 5.1 --- chronicle/artifacts.py | 106 ++++++++++++++++++++++++++++----- chronicle/consumer_contract.py | 37 ++++++++---- chronicle/database.py | 9 +++ chronicle/env.py | 38 ++++++++---- chronicle/harness.py | 56 +++++++++++------ chronicle/suite.py | 10 +++- db/cli.py | 5 +- tests/test_chronicle_mirror.py | 8 +-- tests/test_chronicle_suite.py | 10 ++-- 9 files changed, 212 insertions(+), 67 deletions(-) diff --git a/chronicle/artifacts.py b/chronicle/artifacts.py index d9620761..001d6994 100644 --- a/chronicle/artifacts.py +++ b/chronicle/artifacts.py @@ -18,12 +18,39 @@ import httpx import yaml - +from chronicle.database import ( + CHRONICLE_DB_FILENAME, + CHRONICLE_DB_FILENAMES, + LEGACY_CHRONICLE_DB_FILENAME, +) +from chronicle.env import env_value + + +R2_RAW_BUCKET_ENV = "CHRONICLE_R2_RAW_BUCKET" +R2_DERIVED_BUCKET_ENV = "CHRONICLE_R2_DERIVED_BUCKET" + +# The bucket defaults stay at their ledger-era names. Archived witness records +# pin raw R2 URLs by hash, so ledger-raw and ledger-derived are preserved +# read-only forever and no recorded manifest URI is ever rewritten. The env +# vars exist so the cutover in docs/storage-architecture.md can be rehearsed, +# and so flipping to chronicle-raw/chronicle-derived is a default change rather +# than a code change (PolicyEngine/chronicle#143, mechanism 3). DEFAULT_R2_RAW_BUCKET = "ledger-raw" DEFAULT_R2_DERIVED_BUCKET = "ledger-derived" DEFAULT_R2_PREFIX = "raw" DEFAULT_R2_DERIVED_PREFIX = "derived" + +def default_r2_raw_bucket() -> str: + """Resolve the raw bucket: ``$CHRONICLE_R2_RAW_BUCKET`` or the default.""" + return env_value(R2_RAW_BUCKET_ENV, default=DEFAULT_R2_RAW_BUCKET) + + +def default_r2_derived_bucket() -> str: + """Resolve the derived bucket: ``$CHRONICLE_R2_DERIVED_BUCKET`` or default.""" + return env_value(R2_DERIVED_BUCKET_ENV, default=DEFAULT_R2_DERIVED_BUCKET) + + # New UK and New Zealand uploads are namespaced by country. US objects predate # the country segment and deliberately keep their legacy ``raw/{source_id}`` # and ``derived/{source_id}`` shapes. Publisher directories are the stable @@ -390,11 +417,12 @@ def fetch_source_artifact( table: str | None = None, filename: str | None = None, upload_r2: bool = False, - r2_bucket: str = DEFAULT_R2_RAW_BUCKET, + r2_bucket: str | None = None, r2_prefix: str | None = None, wrangler_command: str = "npx wrangler", ) -> ArtifactFetchReport: """Fetch/register a source artifact and optionally upload it to R2.""" + r2_bucket = r2_bucket or default_r2_raw_bucket() output = Path(output_dir) resolved_r2_prefix = resolve_r2_prefix( prefix=r2_prefix, @@ -480,12 +508,13 @@ def publish_derived_artifacts( package_id: str, year: int, build_id: str | None = None, - r2_bucket: str = DEFAULT_R2_DERIVED_BUCKET, + r2_bucket: str | None = None, r2_prefix: str | None = None, wrangler_command: str = "npx wrangler", build_artifacts_output: str | Path | None = None, ) -> DerivedArtifactPublishReport: """Upload a deterministic build output directory to the derived R2 bucket.""" + r2_bucket = r2_bucket or default_r2_derived_bucket() input_path = Path(input_dir) if not input_path.exists(): return DerivedArtifactPublishReport( @@ -596,11 +625,12 @@ def publish_source_artifacts( manifest_filename: str = "manifest.yaml", source_id: str | None = None, package_id: str | None = None, - r2_bucket: str = DEFAULT_R2_RAW_BUCKET, + r2_bucket: str | None = None, r2_prefix: str | None = None, wrangler_command: str = "npx wrangler", ) -> RawArtifactPublishReport: """Upload manifest-declared raw source artifacts and record R2 locations.""" + r2_bucket = r2_bucket or default_r2_raw_bucket() root_path = Path(root) if not root_path.exists(): return RawArtifactPublishReport( @@ -770,12 +800,15 @@ def inventory_source_artifacts( def bootstrap_r2_buckets( *, - raw_bucket: str = DEFAULT_R2_RAW_BUCKET, - derived_bucket: str = DEFAULT_R2_DERIVED_BUCKET, + raw_bucket: str | None = None, + derived_bucket: str | None = None, wrangler_command: str = "npx wrangler", ) -> R2BootstrapReport: """Create the R2 buckets Chronicle expects, if Wrangler is authenticated.""" - buckets = (raw_bucket, derived_bucket) + buckets = ( + raw_bucket or default_r2_raw_bucket(), + derived_bucket or default_r2_derived_bucket(), + ) commands: list[ArtifactCommandResult] = [] errors: list[str] = [] @@ -987,9 +1020,9 @@ def infer_build_id(input_dir: str | Path) -> str | None: if build_id: return str(build_id) - db_path = input_path / "ledger.db" + db_path = input_path / CHRONICLE_DB_FILENAME if not db_path.exists(): - db_path = input_path / "ledger.db" + db_path = input_path / LEGACY_CHRONICLE_DB_FILENAME if db_path.exists(): with sqlite3.connect(db_path) as connection: row = connection.execute( @@ -1020,6 +1053,17 @@ def _filename_from_url(source_url: str) -> str: return Path(unquote(parsed.path)).name +def _recorded_r2(spec: Any) -> dict[str, Any]: + """Return a manifest file spec's recorded ``storage.r2`` block, if any.""" + if not isinstance(spec, dict): + return {} + storage = spec.get("storage") + if not isinstance(storage, dict): + return {} + recorded = storage.get("r2") + return recorded if isinstance(recorded, dict) else {} + + def _upsert_manifest( manifest_path: Path, *, @@ -1053,8 +1097,18 @@ def _upsert_manifest( "size_bytes": size_bytes, "fetched_at": fetched_at, } - if r2_location is not None: - file_entry["storage"] = {"r2": r2_location.to_dict()} + recorded_r2 = _recorded_r2(payload["files"].get(year)) + new_r2 = r2_location.to_dict() if r2_location is not None else None + if recorded_r2 and ( + new_r2 is None or recorded_r2.get("bucket") != new_r2.get("bucket") + ): + # A recorded storage.r2 block is historical truth: archived witness + # records pin raw R2 URLs by hash. Re-fetching under a renamed bucket + # copies bytes; it does not restate where the bytes were first + # published (PolicyEngine/chronicle#143, mechanism 3). + file_entry["storage"] = {"r2": recorded_r2} + elif new_r2 is not None: + file_entry["storage"] = {"r2": new_r2} payload["files"][year] = file_entry manifest_path.write_text( yaml.safe_dump(payload, sort_keys=False), @@ -1146,8 +1200,32 @@ def _publish_raw_manifest_entry( package_path=manifest_path, ), ) - storage = spec.get("storage") if isinstance(spec.get("storage"), dict) else {} - recorded_r2 = storage.get("r2") if isinstance(storage.get("r2"), dict) else {} + recorded_r2 = _recorded_r2(spec) + recorded_bucket = recorded_r2.get("bucket") + if recorded_bucket and recorded_bucket != location.bucket: + # The recorded bucket is preserved history. Publishing the same bytes + # into a renamed bucket is a backfill copy, not a restatement, so the + # manifest must not be rewritten to point at the new bucket. + errors.append( + "recorded_r2_bucket_is_preserved_history:" + f"recorded={recorded_bucket}:requested={location.bucket}" + ) + return ( + RawArtifactPublishEntry( + manifest_path=str(manifest_path), + source_id=source_id, + package_id=package_id, + year=str(year), + filename=filename, + local_path=str(artifact_path), + sha256=sha256_actual, + size_bytes=size_bytes, + r2_location=None, + upload=None, + errors=tuple(errors), + ), + None, + ) recorded_key = recorded_r2.get("key") if recorded_key and recorded_key != location.key: errors.append( @@ -1252,7 +1330,7 @@ def _inventory_entry( def _derived_artifact_kind(artifact_name: str) -> str: - if artifact_name in {"ledger.db", "ledger.db"}: + if artifact_name in CHRONICLE_DB_FILENAMES: return "sqlite_database" if artifact_name.endswith(".jsonl"): return "jsonl" diff --git a/chronicle/consumer_contract.py b/chronicle/consumer_contract.py index 08276b8a..31f90912 100644 --- a/chronicle/consumer_contract.py +++ b/chronicle/consumer_contract.py @@ -390,6 +390,24 @@ def validate_consumer_fact_contract( ) +def _r2_uri_parts(uri: str) -> tuple[str, str]: + """Split an ``r2://bucket/key`` URI into its bucket and key.""" + if not uri.startswith("r2://"): + return "", "" + bucket, _, key = uri[len("r2://") :].partition("/") + return bucket, key + + +def _points_at_derived(bucket: str, key: str) -> bool: + """Whether an R2 bucket/key pair addresses derived build output. + + Matched on shape rather than on the ledger-era bucket names, so the guard + keeps firing once the buckets are renamed (PolicyEngine/chronicle#143, + mechanism 3). + """ + return bucket.endswith("-derived") or key.startswith("derived/") + + def _derived_source_provenance_issue(fact: AggregateFact) -> str | None: """Return a boundary error if a fact is a downstream target derivation.""" source = fact.source @@ -411,20 +429,14 @@ def _derived_source_provenance_issue(fact: AggregateFact) -> str | None: "itself. Target construction, aging, and reconciliation belong in " "Microcosm." ) - if source_file.startswith("ledger-derived:"): + source_file_bucket, bucket_separator, _ = source_file.partition(":") + if bucket_separator and source_file_bucket.endswith("-derived"): return ( "Chronicle consumer facts must cite raw publisher artifacts. Derived " "target-construction artifacts belong in Microcosm." ) - if ( - raw_r2_bucket.endswith("-derived") - or raw_r2_key.startswith("derived/") - or raw_r2_uri.startswith( - ( - "r2://ledger-derived/", - "r2://ledger-raw/derived/", - ) - ) + if _points_at_derived(raw_r2_bucket, raw_r2_key) or _points_at_derived( + *_r2_uri_parts(raw_r2_uri) ): return ( "Chronicle consumer facts must point at raw source artifacts, not " @@ -450,7 +462,10 @@ def _fact_provenance_class_issue( "provenance_class", ) if fact.provenance_class == "survey_aggregate": - if type(fact.survey_instrument) is not str or not fact.survey_instrument.strip(): + if ( + type(fact.survey_instrument) is not str + or not fact.survey_instrument.strip() + ): return ( "missing_survey_instrument", "Survey aggregates need a non-empty survey instrument.", diff --git a/chronicle/database.py b/chronicle/database.py index 9c9aaedd..73f8f93f 100644 --- a/chronicle/database.py +++ b/chronicle/database.py @@ -35,6 +35,15 @@ LEDGER_DB_SCHEMA_VERSION = "ledger.relational.v1" +# New suite outputs write chronicle.db. Existing builds wrote ledger.db and are +# still read and published unchanged, so the legacy name stays accepted for +# inference and artifact classification (PolicyEngine/chronicle#143, +# mechanism 3). The relational schema id above is a frozen machine surface that +# migrates with the epoch lane, not with this rename. +CHRONICLE_DB_FILENAME = "chronicle.db" +LEGACY_CHRONICLE_DB_FILENAME = "ledger.db" +CHRONICLE_DB_FILENAMES = (CHRONICLE_DB_FILENAME, LEGACY_CHRONICLE_DB_FILENAME) + @dataclass(frozen=True) class ChronicleDbBuildReport: diff --git a/chronicle/env.py b/chronicle/env.py index 80667f4d..3a2f7af6 100644 --- a/chronicle/env.py +++ b/chronicle/env.py @@ -75,7 +75,11 @@ def env_names(name: str) -> tuple[str, ...]: def _warn_legacy(found: str, preferred: str) -> None: - """Warn once per process that a ledger-era variable supplied a value.""" + """Warn once per process that a ledger-era variable supplied a value. + + ``stacklevel=4`` walks out through :func:`_first_set` and its public + wrapper so the notice points at the code that asked for the setting. + """ if found in _WARNED_LEGACY_NAMES: return _WARNED_LEGACY_NAMES.add(found) @@ -84,7 +88,7 @@ def _warn_legacy(found: str, preferred: str) -> None: f"set {preferred} instead. The old name is still honored during the " "Chronicle rename window and will be removed once consumers migrate.", ChronicleEnvDeprecationWarning, - stacklevel=3, + stacklevel=4, ) @@ -93,12 +97,11 @@ def reset_env_deprecation_state() -> None: _WARNED_LEGACY_NAMES.clear() -def env_value(*names: str, default: _Default = None) -> str | _Default: - """Read the first set value across ``names``, chronicle-preferred first. +def _first_set(names: tuple[str, ...]) -> str | None: + """Return the first set value across ``names``, warning on a legacy hit. - Each name is expanded through :func:`env_names`, so a caller can pass the - chronicle name and still pick up a value set under a ledger-era name. - Empty values are treated as unset, matching the helpers this replaces. + Both public readers call this at the same stack depth so the deprecation + warning is always attributed to their caller, not to this module. """ for name in names: candidates = env_names(name) @@ -109,12 +112,27 @@ def env_value(*names: str, default: _Default = None) -> str | _Default: if candidate != preferred: _warn_legacy(candidate, preferred) return value - return default + return None + + +def env_value(*names: str, default: _Default = None) -> str | _Default: + """Read the first set value across ``names``, chronicle-preferred first. + + Each name is expanded through :func:`env_names`, so a caller can pass the + chronicle name and still pick up a value set under a ledger-era name. + Empty values are treated as unset, matching the helpers this replaces. + """ + value = _first_set(names) + return default if value is None else value def env_flag(*names: str) -> bool: - """Return whether the first set value across ``names`` reads as true.""" - value = env_value(*names) + """Return whether the first set value across ``names`` reads as true. + + The chronicle-preferred name wins even when it reads false, so an operator + who has migrated can turn a flag off without unsetting the legacy name. + """ + value = _first_set(names) if value is None: return False return value.strip().lower() in TRUTHY_ENV_VALUES diff --git a/chronicle/harness.py b/chronicle/harness.py index 1d96f093..19b7a253 100644 --- a/chronicle/harness.py +++ b/chronicle/harness.py @@ -8,6 +8,8 @@ from pathlib import Path from chronicle.artifacts import ( + DEFAULT_R2_DERIVED_BUCKET, + DEFAULT_R2_RAW_BUCKET, ArtifactFetchReport, ArtifactInventoryReport, DerivedArtifactPublishReport, @@ -335,7 +337,7 @@ def fetch_artifact_file( table: str | None = None, filename: str | None = None, upload_r2: bool = False, - r2_bucket: str = "ledger-raw", + r2_bucket: str | None = None, r2_prefix: str | None = None, wrangler_command: str = "npx wrangler", ) -> ArtifactFetchReport: @@ -372,7 +374,7 @@ def publish_raw_artifact_files( manifest_filename: str = "manifest.yaml", source_id: str | None = None, package_id: str | None = None, - r2_bucket: str = "ledger-raw", + r2_bucket: str | None = None, r2_prefix: str | None = None, wrangler_command: str = "npx wrangler", ) -> RawArtifactPublishReport: @@ -390,8 +392,8 @@ def publish_raw_artifact_files( def bootstrap_r2_storage( *, - raw_bucket: str = "ledger-raw", - derived_bucket: str = "ledger-derived", + raw_bucket: str | None = None, + derived_bucket: str | None = None, wrangler_command: str = "npx wrangler", ) -> R2BootstrapReport: """Create Chronicle R2 buckets when Wrangler is authenticated.""" @@ -409,7 +411,7 @@ def publish_derived_artifact_files( package_id: str, year: int, build_id: str | None = None, - r2_bucket: str = "ledger-derived", + r2_bucket: str | None = None, r2_prefix: str | None = None, wrangler_command: str = "npx wrangler", build_artifacts_output: str | Path | None = None, @@ -888,8 +890,11 @@ def main(argv: list[str] | None = None) -> int: ) artifact_parser.add_argument( "--r2-bucket", - default="ledger-raw", - help="R2 bucket for raw artifacts when --upload-r2 is set.", + default=None, + help=( + "R2 bucket for raw artifacts when --upload-r2 is set. Defaults to " + f"$CHRONICLE_R2_RAW_BUCKET, else {DEFAULT_R2_RAW_BUCKET}." + ), ) artifact_parser.add_argument( "--r2-prefix", @@ -923,7 +928,7 @@ def main(argv: list[str] | None = None) -> int: raw_publish_parser = subparsers.add_parser( "publish-raw", - help="Upload manifest-declared raw source artifacts to ledger-raw R2", + help="Upload manifest-declared raw source artifacts to the raw R2 bucket", ) raw_publish_parser.add_argument( "--root", @@ -946,8 +951,11 @@ def main(argv: list[str] | None = None) -> int: ) raw_publish_parser.add_argument( "--r2-bucket", - default="ledger-raw", - help="R2 bucket for immutable raw artifacts.", + default=None, + help=( + "R2 bucket for immutable raw artifacts. Defaults to " + f"$CHRONICLE_R2_RAW_BUCKET, else {DEFAULT_R2_RAW_BUCKET}." + ), ) raw_publish_parser.add_argument( "--r2-prefix", @@ -969,13 +977,19 @@ def main(argv: list[str] | None = None) -> int: ) r2_parser.add_argument( "--raw-bucket", - default="ledger-raw", - help="R2 bucket name for immutable raw source artifacts.", + default=None, + help=( + "R2 bucket name for immutable raw source artifacts. Defaults to " + f"$CHRONICLE_R2_RAW_BUCKET, else {DEFAULT_R2_RAW_BUCKET}." + ), ) r2_parser.add_argument( "--derived-bucket", - default="ledger-derived", - help="R2 bucket name for derived Chronicle build artifacts.", + default=None, + help=( + "R2 bucket name for derived Chronicle build artifacts. Defaults to " + f"$CHRONICLE_R2_DERIVED_BUCKET, else {DEFAULT_R2_DERIVED_BUCKET}." + ), ) r2_parser.add_argument( "--wrangler-command", @@ -985,7 +999,7 @@ def main(argv: list[str] | None = None) -> int: derived_publish_parser = subparsers.add_parser( "publish-derived", - help="Upload deterministic Chronicle build outputs to ledger-derived R2", + help="Upload deterministic Chronicle build outputs to the derived R2 bucket", ) derived_publish_parser.add_argument( "--dir", @@ -1011,12 +1025,18 @@ def main(argv: list[str] | None = None) -> int: ) derived_publish_parser.add_argument( "--build-id", - help="Build ID. Defaults to the ID inferred from reports or ledger.db.", + help=( + "Build ID. Defaults to the ID inferred from reports, chronicle.db, " + "or a legacy ledger.db." + ), ) derived_publish_parser.add_argument( "--r2-bucket", - default="ledger-derived", - help="R2 bucket for derived build artifacts.", + default=None, + help=( + "R2 bucket for derived build artifacts. Defaults to " + f"$CHRONICLE_R2_DERIVED_BUCKET, else {DEFAULT_R2_DERIVED_BUCKET}." + ), ) derived_publish_parser.add_argument( "--r2-prefix", diff --git a/chronicle/suite.py b/chronicle/suite.py index 07b9551e..bf5b45db 100644 --- a/chronicle/suite.py +++ b/chronicle/suite.py @@ -22,7 +22,11 @@ build_fact_key, validate_facts, ) -from chronicle.database import ChronicleDbBuildReport, build_chronicle_db +from chronicle.database import ( + CHRONICLE_DB_FILENAME, + ChronicleDbBuildReport, + build_chronicle_db, +) from chronicle.sources.cells import ( SourceCell, SourceCellReport, @@ -371,7 +375,7 @@ def build_source_suite( concept_report.to_dict(), ) - db_path = output_path / "ledger.db" + db_path = output_path / CHRONICLE_DB_FILENAME db_report = build_chronicle_db( facts, db_path, @@ -1646,7 +1650,7 @@ def _write_package_sidecars(output_path: Path, *, source: str, year: int) -> Non output_path / "source_regions.jsonl", output_path / "facts.jsonl", output_path / "consumer_facts.jsonl", - output_path / "ledger.db", + output_path / CHRONICLE_DB_FILENAME, output_path / "reports" / "source_rows.json", output_path / "reports" / "source_cells.json", output_path / "reports" / "source_regions.json", diff --git a/db/cli.py b/db/cli.py index 52257240..66538149 100644 --- a/db/cli.py +++ b/db/cli.py @@ -276,14 +276,15 @@ def cmd_query(args): def _pe_source_root_env_default(jurisdiction: str) -> str | None: + from chronicle.env import env_value + from .pe_source_inventory import ( PE_UK_DATA_ROOT_ENV, PE_US_DATA_ROOT_ENV, - _env_value, ) env_var = PE_US_DATA_ROOT_ENV if jurisdiction == "us" else PE_UK_DATA_ROOT_ENV - return _env_value(env_var) + return env_value(env_var) def main(): diff --git a/tests/test_chronicle_mirror.py b/tests/test_chronicle_mirror.py index 787429c1..c06a33bd 100644 --- a/tests/test_chronicle_mirror.py +++ b/tests/test_chronicle_mirror.py @@ -18,7 +18,7 @@ def test_export_chronicle_db_tables_writes_jsonl_and_manifest(tmp_path): - db_path = tmp_path / "ledger.db" + db_path = tmp_path / "chronicle.db" output_dir = tmp_path / "mirror" build_chronicle_db( build_soi_table_1_1_facts(2023), @@ -53,7 +53,7 @@ def test_export_chronicle_db_tables_writes_jsonl_and_manifest(tmp_path): def test_export_chronicle_db_tables_orders_rows_deterministically(tmp_path): - db_path = tmp_path / "ledger.db" + db_path = tmp_path / "chronicle.db" first_output_dir = tmp_path / "mirror-first" second_output_dir = tmp_path / "mirror-second" build_chronicle_db( @@ -72,7 +72,7 @@ def test_export_chronicle_db_tables_orders_rows_deterministically(tmp_path): def test_export_db_tables_cli_emits_manifest_summary(tmp_path, capsys): - db_path = tmp_path / "ledger.db" + db_path = tmp_path / "chronicle.db" output_dir = tmp_path / "mirror" build_chronicle_db( build_soi_table_1_1_facts(2023), @@ -99,7 +99,7 @@ def test_export_db_tables_cli_emits_manifest_summary(tmp_path, capsys): def test_load_supabase_mirror_dry_run_counts_exported_rows(tmp_path): - db_path = tmp_path / "ledger.db" + db_path = tmp_path / "chronicle.db" output_dir = tmp_path / "mirror" build_chronicle_db( build_soi_table_1_1_facts(2023), diff --git a/tests/test_chronicle_suite.py b/tests/test_chronicle_suite.py index 3f418137..ae9b7654 100644 --- a/tests/test_chronicle_suite.py +++ b/tests/test_chronicle_suite.py @@ -94,7 +94,7 @@ def test_build_source_suite_writes_artifacts_and_reports(tmp_path): assert (output_dir / "source_regions.jsonl").exists() assert (output_dir / "facts.jsonl").exists() assert (output_dir / "consumer_facts.jsonl").exists() - assert (output_dir / "ledger.db").exists() + assert (output_dir / "chronicle.db").exists() assert (output_dir / "datapackage.json").exists() assert (output_dir / "ro-crate-metadata.json").exists() assert (output_dir / "reports" / "source_regions.json").exists() @@ -122,7 +122,7 @@ def test_build_source_suite_writes_artifacts_and_reports(tmp_path): "source_regions.jsonl", "facts.jsonl", "consumer_facts.jsonl", - "ledger.db", + "chronicle.db", "reports/build_summary.json", "reports/source_regions.json", "reports/selectors.json", @@ -148,7 +148,7 @@ def test_build_source_suite_writes_artifacts_and_reports(tmp_path): "concept_alignment_validation_skipped" ] - with sqlite3.connect(output_dir / "ledger.db") as connection: + with sqlite3.connect(output_dir / "chronicle.db") as connection: facts_count = connection.execute( "SELECT COUNT(*) FROM aggregate_facts" ).fetchone()[0] @@ -182,7 +182,7 @@ def test_build_source_suite_supports_soi_table_1_4(tmp_path): "concept_alignment_validation_skipped" ) assert (output_dir / "source_regions.jsonl").exists() - assert (output_dir / "ledger.db").exists() + assert (output_dir / "chronicle.db").exists() def test_agent_acceptance_accepts_aggregate_income_range_source_rows(): @@ -676,7 +676,7 @@ def test_build_suite_cli_emits_json_summary(tmp_path, capsys): assert payload["outputs"]["source_regions"] == str( output_dir / "source_regions.jsonl" ) - assert payload["outputs"]["database"] == str(output_dir / "ledger.db") + assert payload["outputs"]["database"] == str(output_dir / "chronicle.db") assert payload["outputs"]["consumer_facts"] == str( output_dir / "consumer_facts.jsonl" ) From 26b6da881a82302249c73730f05f6c44192d2f9e Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 2 Sep 2026 11:25:23 -0400 Subject: [PATCH 03/22] Add hermetic tests for the rename window tests/test_chronicle_env.py covers the shared helper end to end: the lookup ladder, CHRONICLE_* winning over both ledger-era spellings without a warning, each legacy spelling still working with a once-per-process warning attributed to the caller, empty values counting as unset, and a migrated operator being able to turn a flag off without unsetting the stale legacy name. It then exercises the real call sites -- source-package artifact cache and fetch flag, the db CLI's --pe-us-root default, the Supabase schema override, and R2 bucket resolution -- so a helper regression cannot pass by only testing the helper. An autouse fixture strips every rename-window variable from the ambient environment, so the file is hermetic under any shell. The artifacts tests cover the chronicle.db write and ledger.db read fallback, both database names classifying as sqlite_database, publish-derived following the configured bucket, and the two manifest-preservation paths: publish-raw refusing to restate a recorded bucket and fetch-artifact keeping the recorded r2 block while still uploading the backfill copy. Co-Authored-By: Claude Fable 5.1 --- tests/test_chronicle_artifacts.py | 169 +++++++++++++++ tests/test_chronicle_env.py | 334 ++++++++++++++++++++++++++++++ 2 files changed, 503 insertions(+) create mode 100644 tests/test_chronicle_env.py diff --git a/tests/test_chronicle_artifacts.py b/tests/test_chronicle_artifacts.py index e08beabc..759e3157 100644 --- a/tests/test_chronicle_artifacts.py +++ b/tests/test_chronicle_artifacts.py @@ -4,6 +4,7 @@ import hashlib import json +import sqlite3 import pytest import yaml @@ -587,3 +588,171 @@ def test_top_level_cli_dispatches_publish_derived(tmp_path, capsys, monkeypatch) assert exc.value.code == 0 assert payload["valid"] + + +def _sqlite_build(path, build_id): + """Write a minimal build database carrying one ledger_builds row.""" + with sqlite3.connect(path) as connection: + connection.execute("CREATE TABLE ledger_builds (build_id TEXT PRIMARY KEY)") + connection.execute("INSERT INTO ledger_builds VALUES (?)", (build_id,)) + + +@pytest.mark.parametrize("db_name", ["chronicle.db", "ledger.db"]) +def test_infer_build_id_reads_new_and_legacy_database_names(tmp_path, db_name): + suite = tmp_path / "suite" + suite.mkdir() + _sqlite_build(suite / db_name, "ledger.build.v1:from-db") + + assert infer_build_id(suite) == "ledger.build.v1:from-db" + + +def test_infer_build_id_prefers_the_chronicle_database(tmp_path): + suite = tmp_path / "suite" + suite.mkdir() + _sqlite_build(suite / "chronicle.db", "ledger.build.v1:chronicle") + _sqlite_build(suite / "ledger.db", "ledger.build.v1:legacy") + + assert infer_build_id(suite) == "ledger.build.v1:chronicle" + + +@pytest.mark.parametrize("db_name", ["chronicle.db", "ledger.db"]) +def test_publish_derived_classifies_both_database_names(tmp_path, db_name): + suite = tmp_path / "suite" + reports = suite / "reports" + reports.mkdir(parents=True) + build_id = "ledger.build.v1:kind" + (reports / "database.json").write_text(json.dumps({"build_id": build_id})) + (suite / db_name).write_bytes(b"db") + wrangler = tmp_path / "wrangler" + wrangler.write_text("#!/bin/sh\necho ok\n") + wrangler.chmod(0o755) + + report = publish_derived_artifacts( + suite, + source_id="irs_soi", + package_id="soi-table-1-1", + year=2023, + wrangler_command=str(wrangler), + ) + rows = {row["artifact_name"]: row for row in build_artifact_rows(report)} + + assert rows[db_name]["artifact_kind"] == "sqlite_database" + + +def test_publish_derived_uses_the_configured_bucket(tmp_path, monkeypatch): + monkeypatch.setenv("CHRONICLE_R2_DERIVED_BUCKET", "chronicle-derived") + suite = tmp_path / "suite" + reports = suite / "reports" + reports.mkdir(parents=True) + (reports / "database.json").write_text( + json.dumps({"build_id": "ledger.build.v1:bucket"}) + ) + (suite / "facts.jsonl").write_text("{}\n") + log = tmp_path / "wrangler.log" + wrangler = tmp_path / "wrangler" + wrangler.write_text(f"#!/bin/sh\nprintf '%s\\n' \"$*\" >> {log}\necho ok\n") + wrangler.chmod(0o755) + + report = publish_derived_artifacts( + suite, + source_id="irs_soi", + package_id="soi-table-1-1", + year=2023, + wrangler_command=str(wrangler), + ) + + assert report.valid + assert report.entries[0].r2_location.bucket == "chronicle-derived" + assert "chronicle-derived/derived/irs_soi/" in log.read_text() + + +def test_publish_raw_refuses_to_restate_a_recorded_bucket(tmp_path, monkeypatch): + """A recorded storage.r2 bucket is preserved history, not a publish target. + + Archived witness records pin raw R2 URLs by hash, so backfilling the same + bytes into a renamed bucket must not rewrite the manifest. + """ + monkeypatch.setenv("CHRONICLE_R2_RAW_BUCKET", "chronicle-raw") + output_dir = tmp_path / "db" / "data" / "irs_soi" / "soi-table-1-1" + source = tmp_path / "soi.xlsx" + source.write_bytes(b"official SOI workbook") + fetch_source_artifact( + str(source), + source_id="irs_soi", + package_id="soi-table-1-1", + year=2023, + output_dir=output_dir, + ) + manifest_path = output_dir / "manifest.yaml" + manifest = yaml.safe_load(manifest_path.read_text()) + artifact = manifest["files"][2023] + recorded_key = ( + f"raw/irs_soi/soi-table-1-1/2023/{artifact['sha256']}/{artifact['filename']}" + ) + artifact["storage"] = { + "r2": { + "provider": "r2", + "bucket": "ledger-raw", + "key": recorded_key, + "uri": f"r2://ledger-raw/{recorded_key}", + } + } + manifest_path.write_text(yaml.safe_dump(manifest, sort_keys=False)) + log = tmp_path / "wrangler.log" + wrangler = tmp_path / "wrangler" + wrangler.write_text(f"#!/bin/sh\nprintf '%s\\n' \"$*\" >> {log}\necho ok\n") + wrangler.chmod(0o755) + + report = publish_source_artifacts(output_dir, wrangler_command=str(wrangler)) + unchanged = yaml.safe_load(manifest_path.read_text()) + + assert not report.valid + assert ( + report.entries[0] + .errors[0] + .startswith("recorded_r2_bucket_is_preserved_history:") + ) + assert not log.exists() + assert unchanged["files"][2023]["storage"]["r2"]["bucket"] == "ledger-raw" + + +def test_fetch_artifact_keeps_an_already_recorded_bucket(tmp_path, monkeypatch): + output_dir = tmp_path / "db" / "data" / "irs_soi" / "soi-table-1-1" + source = tmp_path / "soi.xlsx" + source.write_bytes(b"official SOI workbook") + log = tmp_path / "wrangler.log" + wrangler = tmp_path / "wrangler" + wrangler.write_text(f"#!/bin/sh\nprintf '%s\\n' \"$*\" >> {log}\necho ok\n") + wrangler.chmod(0o755) + fetch_source_artifact( + str(source), + source_id="irs_soi", + package_id="soi-table-1-1", + year=2023, + output_dir=output_dir, + upload_r2=True, + wrangler_command=str(wrangler), + ) + manifest_path = output_dir / "manifest.yaml" + first = yaml.safe_load(manifest_path.read_text()) + assert first["files"][2023]["storage"]["r2"]["bucket"] == "ledger-raw" + + monkeypatch.setenv("CHRONICLE_R2_RAW_BUCKET", "chronicle-raw") + report = fetch_source_artifact( + str(source), + source_id="irs_soi", + package_id="soi-table-1-1", + year=2023, + output_dir=output_dir, + upload_r2=True, + wrangler_command=str(wrangler), + ) + second = yaml.safe_load(manifest_path.read_text()) + + # The backfill copy really is uploaded to the new bucket, but the manifest + # keeps recording where the bytes were first published. + assert report.r2_location.bucket == "chronicle-raw" + assert "chronicle-raw" in log.read_text() + assert ( + second["files"][2023]["storage"]["r2"] == first["files"][2023]["storage"]["r2"] + ) diff --git a/tests/test_chronicle_env.py b/tests/test_chronicle_env.py new file mode 100644 index 00000000..4ffe0206 --- /dev/null +++ b/tests/test_chronicle_env.py @@ -0,0 +1,334 @@ +"""Tests for the chronicle-first environment read window. + +Chronicle's operational stores migrate by dual-run (PolicyEngine/chronicle#143, +mechanism 3): ``CHRONICLE_*`` names win, ledger-era names keep working behind a +deprecation warning. Every test here is hermetic — the fixture strips every +variable in the rename window from the ambient environment first. +""" + +from __future__ import annotations + +import importlib +import os +from pathlib import Path + +import pytest + +from chronicle.artifacts import ( + DEFAULT_R2_DERIVED_BUCKET, + DEFAULT_R2_RAW_BUCKET, + default_r2_derived_bucket, + default_r2_raw_bucket, +) +from chronicle.env import ( + CHRONICLE_ENV_PREFIX, + ChronicleEnvDeprecationWarning, + LEGACY_ENV_PREFIXES, + env_flag, + env_names, + env_value, + reset_env_deprecation_state, +) +from chronicle.harness import main as harness_main +from chronicle.source_package import ( + SOURCE_ARTIFACT_CACHE_ENV, + SOURCE_ARTIFACT_FETCH_ENV, +) + +RENAME_WINDOW_PREFIXES = (CHRONICLE_ENV_PREFIX, *LEGACY_ENV_PREFIXES) + + +@pytest.fixture(autouse=True) +def isolated_rename_window_env(monkeypatch): + """Run each test with no rename-window variable inherited from the shell.""" + for name in list(os.environ): + if name.startswith(RENAME_WINDOW_PREFIXES): + monkeypatch.delenv(name, raising=False) + reset_env_deprecation_state() + yield + reset_env_deprecation_state() + + +def _fake_wrangler(tmp_path, log): + wrangler = tmp_path / "wrangler" + wrangler.write_text(f"#!/bin/sh\nprintf '%s\\n' \"$*\" >> {log}\necho ok\n") + wrangler.chmod(0o755) + return wrangler + + +# --------------------------------------------------------------------------- +# Lookup order +# --------------------------------------------------------------------------- + + +def test_env_names_puts_chronicle_first_then_ledger_era_names(): + assert env_names("CHRONICLE_SOURCE_ARTIFACT_FETCH") == ( + "CHRONICLE_SOURCE_ARTIFACT_FETCH", + "POLICYENGINE_LEDGER_SOURCE_ARTIFACT_FETCH", + "LEDGER_SOURCE_ARTIFACT_FETCH", + ) + + +def test_env_names_expands_a_ledger_era_name_to_the_same_ladder(): + assert env_names("LEDGER_PE_US_DATA_ROOT") == env_names("CHRONICLE_PE_US_DATA_ROOT") + + +def test_env_names_leaves_variables_outside_the_rename_window_alone(): + assert env_names("POLICYENGINE_SUPABASE_URL") == ("POLICYENGINE_SUPABASE_URL",) + assert env_names("POLICYENGINE_TARGETS_SCHEMA") == ("POLICYENGINE_TARGETS_SCHEMA",) + + +def test_bare_prefix_is_not_treated_as_a_renamed_variable(): + assert env_names("LEDGER_") == ("LEDGER_",) + + +# --------------------------------------------------------------------------- +# Precedence and the deprecation warning +# --------------------------------------------------------------------------- + + +def test_chronicle_name_wins_over_both_ledger_era_names(monkeypatch, recwarn): + monkeypatch.setenv("CHRONICLE_PE_US_DATA_ROOT", "/chronicle") + monkeypatch.setenv("LEDGER_PE_US_DATA_ROOT", "/ledger") + monkeypatch.setenv("POLICYENGINE_LEDGER_PE_US_DATA_ROOT", "/policyengine-ledger") + + assert env_value("CHRONICLE_PE_US_DATA_ROOT") == "/chronicle" + assert not [ + warning + for warning in recwarn.list + if issubclass(warning.category, ChronicleEnvDeprecationWarning) + ] + + +def test_ledger_name_alone_still_works_and_warns(monkeypatch): + monkeypatch.setenv("LEDGER_PE_US_DATA_ROOT", "/ledger") + + with pytest.warns(ChronicleEnvDeprecationWarning) as warnings_raised: + assert env_value("CHRONICLE_PE_US_DATA_ROOT") == "/ledger" + + message = str(warnings_raised[0].message) + assert "LEDGER_PE_US_DATA_ROOT" in message + assert "CHRONICLE_PE_US_DATA_ROOT" in message + + +def test_policyengine_ledger_name_alone_still_works_and_warns(monkeypatch): + monkeypatch.setenv("POLICYENGINE_LEDGER_SCHEMA", "ledger") + + with pytest.warns(ChronicleEnvDeprecationWarning) as warnings_raised: + assert env_value("CHRONICLE_SCHEMA") == "ledger" + + message = str(warnings_raised[0].message) + assert "POLICYENGINE_LEDGER_SCHEMA" in message + assert "CHRONICLE_SCHEMA" in message + + +def test_deprecation_warning_is_raised_once_per_process(monkeypatch, recwarn): + monkeypatch.setenv("LEDGER_PE_UK_DATA_ROOT", "/ledger") + + for _ in range(3): + assert env_value("CHRONICLE_PE_UK_DATA_ROOT") == "/ledger" + + deprecations = [ + warning + for warning in recwarn.list + if issubclass(warning.category, ChronicleEnvDeprecationWarning) + ] + assert len(deprecations) == 1 + + +def test_deprecation_warning_is_attributed_to_the_calling_module(monkeypatch): + monkeypatch.setenv("LEDGER_SOURCE_ARTIFACT_FETCH", "1") + + with pytest.warns(ChronicleEnvDeprecationWarning) as warnings_raised: + assert env_flag(SOURCE_ARTIFACT_FETCH_ENV) + + # env_flag and env_value must report at the same depth, or operators get a + # notice pointing at Chronicle's own source instead of their call site. + assert Path(warnings_raised[0].filename).name == "test_chronicle_env.py" + + +def test_unset_variables_fall_back_to_the_default(): + assert env_value("CHRONICLE_PE_US_DATA_ROOT") is None + assert env_value("CHRONICLE_PE_US_DATA_ROOT", default="/fallback") == "/fallback" + + +def test_empty_values_count_as_unset(monkeypatch): + monkeypatch.setenv("CHRONICLE_PE_US_DATA_ROOT", "") + monkeypatch.setenv("LEDGER_PE_US_DATA_ROOT", "/ledger") + + with pytest.warns(ChronicleEnvDeprecationWarning): + assert env_value("CHRONICLE_PE_US_DATA_ROOT") == "/ledger" + + +# --------------------------------------------------------------------------- +# Flags +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize("value", ["1", "true", "TRUE", "yes", "on", " on "]) +def test_env_flag_accepts_truthy_spellings(monkeypatch, value): + monkeypatch.setenv("CHRONICLE_SOURCE_ARTIFACT_FETCH", value) + assert env_flag(SOURCE_ARTIFACT_FETCH_ENV) + + +@pytest.mark.parametrize("value", ["0", "false", "no", "off", "maybe"]) +def test_env_flag_rejects_other_values(monkeypatch, value): + monkeypatch.setenv("CHRONICLE_SOURCE_ARTIFACT_FETCH", value) + assert not env_flag(SOURCE_ARTIFACT_FETCH_ENV) + + +def test_env_flag_lets_the_chronicle_name_turn_a_legacy_flag_off(monkeypatch): + monkeypatch.setenv("CHRONICLE_SOURCE_ARTIFACT_FETCH", "0") + monkeypatch.setenv("LEDGER_SOURCE_ARTIFACT_FETCH", "1") + + # An operator who has migrated must be able to turn the flag off without + # first hunting down the stale ledger-era variable. + assert not env_flag(SOURCE_ARTIFACT_FETCH_ENV) + + +# --------------------------------------------------------------------------- +# Real call sites +# --------------------------------------------------------------------------- + + +def test_source_artifact_env_constants_are_chronicle_named(): + assert SOURCE_ARTIFACT_CACHE_ENV == "CHRONICLE_SOURCE_ARTIFACT_CACHE_DIR" + assert SOURCE_ARTIFACT_FETCH_ENV == "CHRONICLE_SOURCE_ARTIFACT_FETCH" + + +@pytest.mark.parametrize( + "name", + ["CHRONICLE_SOURCE_ARTIFACT_CACHE_DIR", "LEDGER_SOURCE_ARTIFACT_CACHE_DIR"], +) +def test_source_artifact_cache_dir_honors_both_names(monkeypatch, tmp_path, name): + from chronicle.source_package import _source_artifact_cache_path + + monkeypatch.setenv(name, str(tmp_path)) + + cache_path = _source_artifact_cache_path( + {"filename": "table.xlsx", "sha256": "abc123"} + ) + + assert cache_path == tmp_path / "abc123" / "table.xlsx" + + +@pytest.mark.parametrize( + "name", ["CHRONICLE_PE_US_DATA_ROOT", "LEDGER_PE_US_DATA_ROOT"] +) +def test_pe_source_root_cli_default_honors_both_names(monkeypatch, name): + from db.cli import _pe_source_root_env_default + + monkeypatch.setenv(name, "/pe-us") + + assert _pe_source_root_env_default("us") == "/pe-us" + + +def test_pe_source_inventory_env_constants_are_chronicle_named(): + from db.pe_source_inventory import PE_UK_DATA_ROOT_ENV, PE_US_DATA_ROOT_ENV + + assert PE_US_DATA_ROOT_ENV == "CHRONICLE_PE_US_DATA_ROOT" + assert PE_UK_DATA_ROOT_ENV == "CHRONICLE_PE_UK_DATA_ROOT" + + +def test_db_cli_parser_builds_with_the_env_backed_defaults(monkeypatch, capsys): + """The db CLI builds its parser before dispatching any subcommand. + + Its --pe-us-root/--pe-uk-root defaults call into the env helper, so an + import error there breaks `chronicle init`, `load` and `stats` alike while + the rest of the test suite stays green. + """ + import db.cli + + monkeypatch.setenv("CHRONICLE_PE_US_DATA_ROOT", "/pe-us") + monkeypatch.setattr("sys.argv", ["chronicle", "--help"]) + + with pytest.raises(SystemExit) as exit_info: + db.cli.main() + + assert exit_info.value.code == 0 + assert "Manage Chronicle target input data" in capsys.readouterr().out + + +@pytest.mark.parametrize("name", ["CHRONICLE_SCHEMA", "POLICYENGINE_LEDGER_SCHEMA"]) +def test_supabase_schema_honors_both_names(monkeypatch, name): + import db.supabase_client + + monkeypatch.setenv(name, "chronicle_probe") + try: + reloaded = importlib.reload(db.supabase_client) + assert reloaded.LEDGER_SCHEMA == "chronicle_probe" + finally: + monkeypatch.delenv(name, raising=False) + importlib.reload(db.supabase_client) + + +def test_supabase_schema_default_is_unchanged(): + import db.supabase_client + + # The hosted schema name itself is out of this slice; only the variable + # that overrides it moved. + assert db.supabase_client.LEDGER_SCHEMA == "ledger" + + +# --------------------------------------------------------------------------- +# R2 bucket configuration +# --------------------------------------------------------------------------- + + +def test_r2_bucket_defaults_are_still_the_ledger_era_names(): + assert DEFAULT_R2_RAW_BUCKET == "ledger-raw" + assert DEFAULT_R2_DERIVED_BUCKET == "ledger-derived" + assert default_r2_raw_bucket() == "ledger-raw" + assert default_r2_derived_bucket() == "ledger-derived" + + +def test_r2_buckets_follow_the_chronicle_env_vars(monkeypatch): + monkeypatch.setenv("CHRONICLE_R2_RAW_BUCKET", "chronicle-raw") + monkeypatch.setenv("CHRONICLE_R2_DERIVED_BUCKET", "chronicle-derived") + + assert default_r2_raw_bucket() == "chronicle-raw" + assert default_r2_derived_bucket() == "chronicle-derived" + + +def test_r2_buckets_honor_ledger_era_names_with_a_warning(monkeypatch): + monkeypatch.setenv("LEDGER_R2_RAW_BUCKET", "legacy-raw") + + with pytest.warns(ChronicleEnvDeprecationWarning): + assert default_r2_raw_bucket() == "legacy-raw" + + +def test_bootstrap_r2_cli_creates_the_configured_buckets(monkeypatch, tmp_path): + log = tmp_path / "wrangler.log" + wrangler = _fake_wrangler(tmp_path, log) + monkeypatch.setenv("CHRONICLE_R2_RAW_BUCKET", "chronicle-raw") + monkeypatch.setenv("CHRONICLE_R2_DERIVED_BUCKET", "chronicle-derived") + + exit_code = harness_main(["bootstrap-r2", "--wrangler-command", str(wrangler)]) + + commands = log.read_text() + assert exit_code == 0 + assert "r2 bucket create chronicle-raw" in commands + assert "r2 bucket create chronicle-derived" in commands + + +def test_bootstrap_r2_cli_flags_still_override_the_environment(monkeypatch, tmp_path): + log = tmp_path / "wrangler.log" + wrangler = _fake_wrangler(tmp_path, log) + monkeypatch.setenv("CHRONICLE_R2_RAW_BUCKET", "chronicle-raw") + + harness_main( + [ + "bootstrap-r2", + "--raw-bucket", + "explicit-raw", + "--derived-bucket", + "explicit-derived", + "--wrangler-command", + str(wrangler), + ] + ) + + commands = log.read_text() + assert "r2 bucket create explicit-raw" in commands + assert "r2 bucket create explicit-derived" in commands + assert "chronicle-raw" not in commands From 944e1d295db3d56925a68748dfc2dba13c5c931c Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 2 Sep 2026 11:31:20 -0400 Subject: [PATCH 04/22] Document the rename window and the bucket cutover docs/storage-architecture.md stated the fallback direction backwards: it claimed the CHRONICLE_-prefixed variables were the old names kept as migration fallbacks, when they are the names to migrate to. Replaced with an "Environment Variable Rename Window" section that gives the actual lookup order, the once-per-process warning, and a table of all seven variables. Two behaviors get stated outright because both invert a naive fallback: the CHRONICLE_ name wins even when it reads false, so a migrated operator can turn a flag off without hunting down a stale legacy export; and an empty value counts as unset. The section also records what is deliberately NOT aliased -- POLICYENGINE_SUPABASE_* and POLICYENGINE_TARGETS_SCHEMA carry no ledger prefix and are read literally -- and that CHRONICLE_SCHEMA renames the override variable, not the schema value, which moves in a later slice. Added a "Bucket Cutover" section covering the six steps: create the buckets on the account wrangler.toml pins, enumerate, backfill-copy, verify, flip the defaults in a follow-up, and leave the ledger-era buckets read-only forever. The enumeration and verification are given as commands rather than prose. Every raw key ends {sha256}/{filename}, so the key is its own checksum witness and verification needs no manifest lookup. Counted rather than assumed: 186 distinct ledger-raw objects across 154 tracked manifest files, and the doc says to recount rather than trust that number, since source packages land continuously. The derived bucket needs no backfill -- derived artifacts are reproducible and already keyed by {build_id}, so a rebuild republishes them wherever configured. README and the harness doc follow: bucket references become archive roles with the default named, ledger.db becomes chronicle.db with the legacy name noted as still readable, and bootstrap-r2's example drops the hardcoded bucket flags now that they default to the resolver. Left alone: LEDGER_EXPLORER_DATA_DIRS in the README. It configures the explorer app, which is not in this repository, so renaming it here would document a name no shipped code honors -- the same defect this commit fixes in the other direction. Co-Authored-By: Claude Fable 5.1 --- PROGRESS.md | 42 ++++-- README.md | 31 +++-- docs/agent-source-package-harness.md | 27 ++-- docs/storage-architecture.md | 191 +++++++++++++++++++++++++-- 4 files changed, 249 insertions(+), 42 deletions(-) diff --git a/PROGRESS.md b/PROGRESS.md index e46b096c..da5d13e5 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -11,6 +11,8 @@ Lane C5's handoff notes previously lived here; its durable record is - Out of scope and deliberately untouched: the `ledger` console-script alias, the Supabase `"ledger"` schema and mirror table names, governance role ids and concept authorities, hash domains and schema ids, anything under `releases/`. +- This PR does not touch the source-data boundary. No package spec, parser, + selector, manifest, or fact value changes. ## Done @@ -24,18 +26,40 @@ Lane C5's handoff notes previously lived here; its durable record is `LEDGER_MIRROR_PRIMARY_KEYS`, and `LEDGER_DB_SCHEMA_VERSION` are module constants, not env reads, and name out-of-scope surfaces. - Added `chronicle/env.py`: one shared `env_value`/`env_flag`/`env_names` - helper reading `CHRONICLE_` first, then `LEDGER_` and - `POLICYENGINE_LEDGER_` with a once-per-process - `ChronicleEnvDeprecationWarning` naming the preferred variable. + helper reading `CHRONICLE_` first, then `POLICYENGINE_LEDGER_` and + `LEDGER_` with a once-per-process `ChronicleEnvDeprecationWarning` naming + the preferred variable. - Replaced all three ad-hoc helpers (`db/supabase_client._env`, `chronicle/source_package._env_value`/`_truthy_env`, `db/pe_source_inventory._env_value`) with the shared helper. +- Made the R2 bucket names configurable via `CHRONICLE_R2_RAW_BUCKET` and + `CHRONICLE_R2_DERIVED_BUCKET`, plumbed through fetch-artifact, publish-raw, + publish-derived and bootstrap-r2. Defaults unchanged at `ledger-raw` and + `ledger-derived`. Both manifest write paths now preserve a recorded + `storage.r2` block instead of restating it under a renamed bucket. +- Emitted `chronicle.db` for new suite outputs, with `ledger.db` still accepted + on read and on derived-artifact kind inference. +- Added `tests/test_chronicle_env.py` plus artifact tests: 75 hermetic tests + covering the lookup ladder, precedence, the once-per-process warning, and every + real call site. +- Swept the docs. `docs/storage-architecture.md` gained an "Environment Variable + Rename Window" section (the old text stated the fallback direction backwards) + and a "Bucket Cutover" section; `docs/agent-source-package-harness.md` and + `README.md` follow. Verified 186 distinct `ledger-raw` objects across 154 + tracked manifest files, every key content-addressed by sha256. + +## Verification + +- `uv run pytest -q`: green. +- `uv run ruff check .`: clean. +- `uv run ruff format --check .`: clean for every file this branch touches. 13 + files are unformatted on `main` already and are byte-identical here; CI runs + `ruff check` only, so they are pre-existing and out of scope. +- CI's db CLI gate (`chronicle init` / `load all` / `stats`): passes. ## Next -- Make R2 bucket names configurable with unchanged `ledger-*` defaults. -- Emit `chronicle.db` for new suite outputs; keep reading `ledger.db`. -- Fix the backwards fallback statement in the docs and sweep every env-name, - bucket-name, and db-filename mention in README/AGENTS/docs. -- Add the hermetic dual-read tests; run pytest, ruff check, ruff format --check. -- Push and open the PR. Do not merge. +- Push and open the PR against `main`. Do not merge. +- Follow-up PR, after Max creates and backfills the new buckets: flip + `DEFAULT_R2_RAW_BUCKET` / `DEFAULT_R2_DERIVED_BUCKET` to `chronicle-raw` / + `chronicle-derived`. diff --git a/README.md b/README.md index 576919cd..1cdb4ceb 100644 --- a/README.md +++ b/README.md @@ -65,10 +65,12 @@ contract that aligns it to another period (see | Microcosm Target Contracts | Selection, measurement bindings, and active subset | Period alignment, support-aware activation, solver inputs, diagnostics | The storage split is documented in -[`docs/storage-architecture.md`](docs/storage-architecture.md): `ledger-raw` -stores immutable source bytes, `ledger-derived` stores reproducible build +[`docs/storage-architecture.md`](docs/storage-architecture.md): a raw R2 archive +stores immutable source bytes, a derived R2 archive stores reproducible build artifacts, and Supabase/Postgres hosts the queryable relational Chronicle registry -mirrored from accepted builds. +mirrored from accepted builds. The bucket names are configuration +(`$CHRONICLE_R2_RAW_BUCKET` and `$CHRONICLE_R2_DERIVED_BUCKET`), still defaulting +to the ledger-era `ledger-raw` and `ledger-derived`. ## Repository Model @@ -247,7 +249,7 @@ This writes: source_regions.jsonl facts.jsonl consumer_facts.jsonl - ledger.db + chronicle.db reports/ source_rows.json source_cells.json @@ -349,8 +351,9 @@ needed, even when your Cloudflare user belongs to several accounts: # One-time per machine (opens a browser consent page): bunx wrangler login -# One-time per account (already done for the PolicyEngine account): -uv run chronicle bootstrap-r2 --raw-bucket ledger-raw --derived-bucket ledger-derived +# One-time per account (already done for the PolicyEngine account). The bucket +# flags default to $CHRONICLE_R2_RAW_BUCKET / $CHRONICLE_R2_DERIVED_BUCKET: +uv run chronicle bootstrap-r2 # Fetch/register a source artifact, write db/data/.../manifest.yaml, and upload # the exact bytes to R2 when Wrangler is authenticated: @@ -367,8 +370,8 @@ uv run chronicle fetch-artifact \ # Audit local manifests and checksums: uv run chronicle inventory-artifacts --root db/data -# Upload all existing manifest-declared local artifacts to ledger-raw and write -# storage.r2 metadata back into the manifests: +# Upload all existing manifest-declared local artifacts to the raw archive and +# write storage.r2 metadata back into the manifests: uv run chronicle publish-raw --root db/data ``` @@ -406,10 +409,10 @@ To prepare the deterministic SQLite artifact for a hosted Supabase/Postgres mirror, export each relational table to JSONL plus a manifest: ```bash -uv run chronicle export-db-tables --db /tmp/chronicle-suite/ledger.db --out /tmp/chronicle-mirror --replace +uv run chronicle export-db-tables --db /tmp/chronicle-suite/chronicle.db --out /tmp/chronicle-mirror --replace ``` -To publish the deterministic build outputs to the `ledger-derived` R2 bucket: +To publish the deterministic build outputs to the derived R2 archive: ```bash uv run chronicle publish-derived \ @@ -437,6 +440,14 @@ uv run chronicle load-supabase-mirror \ Use `--dry-run` first to validate JSONL row counts and file coverage without writing to Supabase. +Chronicle settings are read chronicle-first: `CHRONICLE_X` wins, and the +ledger-era `POLICYENGINE_LEDGER_X` and `LEDGER_X` spellings still work behind a +one-time deprecation warning naming the variable to move to. +[`docs/storage-architecture.md`](docs/storage-architecture.md#environment-variable-rename-window) +lists every variable in that window, and +[Bucket Cutover](docs/storage-architecture.md#bucket-cutover) covers the R2 +bucket rename. + Chronicle facts keep source concepts and canonical concepts separately. For example, the SOI Table 1.1 adjusted gross income column is preserved as `irs_soi.adjusted_gross_income`, while the canonical concept is diff --git a/docs/agent-source-package-harness.md b/docs/agent-source-package-harness.md index 5816ec47..afeb98f4 100644 --- a/docs/agent-source-package-harness.md +++ b/docs/agent-source-package-harness.md @@ -14,8 +14,9 @@ lineage, provenance, constraints, and a passing `build-suite` report. The first gate for a new package is source-artifact acquisition. Agents should register raw source files with `uv run chronicle fetch-artifact` before authoring selectors. This writes the local artifact, captures checksum and retrieval -metadata in `manifest.yaml`, and can upload the exact bytes to the private -`ledger-raw` R2 bucket when Wrangler is authenticated. Agents can audit the local +metadata in `manifest.yaml`, and can upload the exact bytes to the private raw +R2 bucket (`ledger-raw` today; overridable with `CHRONICLE_R2_RAW_BUCKET`) when +Wrangler is authenticated. Agents can audit the local artifact registry with `uv run chronicle inventory-artifacts --root db/data`. For already-downloaded manifest artifacts, agents should run `uv run chronicle publish-raw --root db/data` to upload checksum-verified bytes to @@ -23,12 +24,15 @@ R2 and write `storage.r2` metadata back into each manifest entry. Builds do not require production raw bytes to be committed to Git. Source packages first read packaged fixture bytes, then -`LEDGER_SOURCE_ARTIFACT_CACHE_DIR` (defaulting to +`CHRONICLE_SOURCE_ARTIFACT_CACHE_DIR` (defaulting to `~/.cache/policyengine-chronicle/source-artifacts`). If a manifest artifact is -missing locally, set `LEDGER_SOURCE_ARTIFACT_FETCH=1` to fetch it from the +missing locally, set `CHRONICLE_SOURCE_ARTIFACT_FETCH=1` to fetch it from the manifest `source_url`, verify the declared SHA-256, and write it to that cache. -The old `CHRONICLE_`-prefixed environment variables remain accepted only as -migration fallbacks. +The ledger-era spellings `LEDGER_SOURCE_ARTIFACT_CACHE_DIR` and +`LEDGER_SOURCE_ARTIFACT_FETCH` are still honored during the rename window and +emit a one-time deprecation warning naming the `CHRONICLE_` variable to set +instead; see "Environment Variable Rename Window" in +[`docs/storage-architecture.md`](storage-architecture.md#environment-variable-rename-window). For broad PE source migration, generate the agent queue from the manifest before assigning work: @@ -603,16 +607,19 @@ uv run chronicle build-suite packages/irs_soi/table_1_1 \ --require-axiom-validation ``` -The SQLite `ledger.db` is the source of hosted mirrors. To prepare tables for +The SQLite `chronicle.db` is the source of hosted mirrors. To prepare tables for Supabase/Postgres bulk loading, export the DB artifact rather than inserting cells through the Supabase client: ```bash -uv run chronicle export-db-tables --db /tmp/chronicle-suite/ledger.db --out /tmp/chronicle-mirror --replace +uv run chronicle export-db-tables --db /tmp/chronicle-suite/chronicle.db --out /tmp/chronicle-mirror --replace ``` -Accepted build-suite outputs can be published to the private `ledger-derived` R2 -bucket after validation: +Builds produced before the rename wrote `ledger.db`. That name is still read and +published unchanged, so point `--db` at whichever file the build emitted. + +Accepted build-suite outputs can be published to the private derived R2 bucket +after validation: ```bash uv run chronicle publish-derived \ diff --git a/docs/storage-architecture.md b/docs/storage-architecture.md index cf898933..e381353f 100644 --- a/docs/storage-architecture.md +++ b/docs/storage-architecture.md @@ -9,17 +9,24 @@ class of Chronicle data belongs. Chronicle uses three storage layers with different jobs. -`ledger-raw` is the immutable source-byte archive. It stores exact publisher +The raw archive is the immutable source-byte store. It holds exact publisher artifacts as fetched: workbooks, CSVs, PDFs, ZIPs, HTML snapshots, and similar government-statistics release files. Raw objects are content-addressed by checksum and should never be overwritten in place. -`ledger-derived` is the reproducible artifact archive. It stores build outputs +The derived archive is the reproducible artifact store. It holds build outputs that Chronicle can regenerate from raw bytes, package specs, parser code, and build configuration. Examples include parsed-cell or parsed-row Parquet/JSONL files, -source record outputs, `ledger.db`, mirror JSONL exports, QA reports, Data +source record outputs, `chronicle.db`, mirror JSONL exports, QA reports, Data Package metadata, and RO-Crate metadata. +Both bucket names are configuration, not constants. The raw archive is +`$CHRONICLE_R2_RAW_BUCKET` and the derived archive is +`$CHRONICLE_R2_DERIVED_BUCKET`; the shipped defaults are still the ledger-era +`ledger-raw` and `ledger-derived`. [Bucket Cutover](#bucket-cutover) records how +those defaults move to `chronicle-raw` and `chronicle-derived` and why the +ledger-era buckets are preserved read-only rather than retired. + Supabase/Postgres is the queryable relational registry for accepted Chronicle builds. It stores rows that applications, agents, and downstream systems need to search and join: source artifacts, source rows/cells, source records, @@ -32,8 +39,8 @@ Hosted tables mirror accepted build outputs and provide a shared query surface. ## Ownership Matrix -| Data class | Git/local package | `ledger-raw` R2 | `ledger-derived` R2 | SQLite `ledger.db` | Supabase/Postgres | -|------------|-------------------|---------------|-------------------|------------------|-------------------| +| Data class | Git/local package | Raw R2 | Derived R2 | SQLite `chronicle.db` | Supabase/Postgres | +|------------|-------------------|--------|------------|-----------------------|-------------------| | Source package specs | Authoritative YAML and parser code | No | Optional packaged snapshot | No | Metadata only | | Raw publisher files | Tiny fixtures only | Authoritative bytes | No | Metadata only | Metadata plus R2 pointer | | Source manifests | Authoritative checked metadata | No | Optional snapshot | Metadata loaded into tables | Queryable artifact registry | @@ -76,7 +83,7 @@ Examples: ```text derived/uk/ons/ons-mye-2024-uk/2024/{build_id}/source_cells.jsonl -derived/nz/ird/ird-working-for-families-statistics-sept-2025/2024/{build_id}/ledger.db +derived/nz/ird/ird-working-for-families-statistics-sept-2025/2024/{build_id}/chronicle.db ``` Legacy US derived keys likewise remain `derived/{source_id}/...`. @@ -114,20 +121,21 @@ The intended flow is: 1. Register raw source artifacts with `uv run chronicle fetch-artifact`, which writes local bytes, records checksums in `manifest.yaml`, and can upload the - exact bytes to `ledger-raw`. Existing manifest-declared artifacts can be + exact bytes to the raw archive. Existing manifest-declared artifacts can be checksum-validated, uploaded, and linked with `uv run chronicle publish-raw`. Production package specs may omit raw bytes from Git as long as the manifest keeps `source_url` and SHA-256 metadata; builds can fill - `LEDGER_SOURCE_ARTIFACT_CACHE_DIR` by setting - `LEDGER_SOURCE_ARTIFACT_FETCH=1`. The old `CHRONICLE_`-prefixed environment - variables remain accepted only as migration fallbacks. + `CHRONICLE_SOURCE_ARTIFACT_CACHE_DIR` by setting + `CHRONICLE_SOURCE_ARTIFACT_FETCH=1`. Ledger-era spellings of both still work; + see [Environment Variable Rename Window](#environment-variable-rename-window). 2. Validate and build a source package with `uv run chronicle validate-package` and `uv run chronicle build-suite`. 3. Produce local deterministic outputs: parsed rows/cells, source records, - aggregate facts, `ledger.db`, QA reports, Data Package metadata, and RO-Crate - metadata. + aggregate facts, `chronicle.db`, QA reports, Data Package metadata, and + RO-Crate metadata. Builds before this rename wrote `ledger.db`; every reader + still accepts that name. 4. Export relational mirror files with `uv run chronicle export-db-tables`. -5. Publish derived build outputs to `ledger-derived`: +5. Publish derived build outputs to the derived archive: ```bash uv run chronicle publish-derived \ @@ -150,6 +158,163 @@ The Supabase project must have the checked migration applied and the `chronicle` schema exposed in PostgREST/Data API settings before the REST loader can write to it. Use `--dry-run` to verify local JSONL files without writing. +## Environment Variable Rename Window + +Every Chronicle setting is read chronicle-first by one shared helper, +`chronicle/env.py`. For a setting `X`, the lookup order is: + +1. `CHRONICLE_X` +2. `POLICYENGINE_LEDGER_X` +3. `LEDGER_X` + +The first name that holds a non-empty value wins. When that name is a ledger-era +one, the process emits a single `ChronicleEnvDeprecationWarning` naming the +`CHRONICLE_`-prefixed variable to set instead. The warning fires once per legacy +name per process, and it subclasses `FutureWarning` rather than +`DeprecationWarning` so it actually reaches operators running the CLI. + +Two consequences are worth stating outright, because both are the reverse of +what a naive fallback would do: + +- The `CHRONICLE_` name wins even when its value reads false. An operator who + has migrated can set `CHRONICLE_SOURCE_ARTIFACT_FETCH=0` and have the flag + turn off, without first hunting down a stale `LEDGER_SOURCE_ARTIFACT_FETCH=1` + somewhere in their profile. +- An empty value counts as unset, so exporting an empty `CHRONICLE_` name does + not mask a set legacy name. + +| Chronicle name | Ledger-era names still accepted | Meaning | +|----------------|--------------------------------|---------| +| `CHRONICLE_SOURCE_ARTIFACT_CACHE_DIR` | `LEDGER_SOURCE_ARTIFACT_CACHE_DIR` | Where fetched raw bytes are cached; defaults to `~/.cache/policyengine-chronicle/source-artifacts` | +| `CHRONICLE_SOURCE_ARTIFACT_FETCH` | `LEDGER_SOURCE_ARTIFACT_FETCH` | Fetch a missing manifest artifact from its `source_url` during a build | +| `CHRONICLE_PE_US_DATA_ROOT` | `LEDGER_PE_US_DATA_ROOT` | Local checkout root for PE US source inventory | +| `CHRONICLE_PE_UK_DATA_ROOT` | `LEDGER_PE_UK_DATA_ROOT` | Local checkout root for PE UK source inventory | +| `CHRONICLE_SCHEMA` | `POLICYENGINE_LEDGER_SCHEMA` | Postgres schema the Supabase client reads and writes | +| `CHRONICLE_R2_RAW_BUCKET` | `LEDGER_R2_RAW_BUCKET` | Raw R2 archive bucket; defaults to `ledger-raw` | +| `CHRONICLE_R2_DERIVED_BUCKET` | `LEDGER_R2_DERIVED_BUCKET` | Derived R2 archive bucket; defaults to `ledger-derived` | + +The two R2 rows are new in this window rather than renamed: those buckets were +hardcoded before, so the ledger-era spellings are accepted for consistency, not +because anything ever set them. + +Variables carrying none of the three prefixes are read literally. This helper +renames the ledger-era surface, not every PolicyEngine variable, so +`POLICYENGINE_SUPABASE_URL`, `POLICYENGINE_SUPABASE_SERVICE_KEY` and +`POLICYENGINE_TARGETS_SCHEMA` keep their names and gain no aliases. + +The hosted schema *value* is a separate migration. `CHRONICLE_SCHEMA` renames +the variable that overrides the schema; the schema still defaults to `ledger`, +and the mirror table names are unchanged. Those move in a later slice +coordinated with the CI writers. + +## Bucket Cutover + +Chronicle's operational stores migrate by dual-run +(PolicyEngine/chronicle#143, mechanism 3): stand up the chronicle-named home, +backfill it, repoint writers, retire the old home. The R2 buckets take one +exception to the last step. Archived witness records pin raw R2 URLs by hash, so +`ledger-raw` and `ledger-derived` are preserved read-only forever rather than +deleted, and manifests keep the `storage.r2` URIs they already recorded as +historical truth. A backfill copies bytes into the new bucket; it never rewrites +where those bytes were first published. `publish-raw` and `fetch-artifact` +enforce that: both refuse to restate a recorded `storage.r2` block under a +different bucket. + +The cutover therefore has one irreversible-looking step that is in fact additive +(creating and filling the new buckets), one cheap reversible step (flipping the +defaults, which is a one-line change in `chronicle/artifacts.py`), and no +deletion step at all. + +### 1. Create the new buckets + +Bucket creation needs a Cloudflare login carrying R2 permissions, so it is an +operator step rather than something CI can do. `wrangler.toml` already pins the +PolicyEngine account (`account_id = "20d90f557651969925eece96e58e24dc"`), so no +`CLOUDFLARE_ACCOUNT_ID` is needed even for a user who belongs to several +accounts: + +```bash +bunx wrangler login +uv run chronicle bootstrap-r2 --raw-bucket chronicle-raw --derived-bucket chronicle-derived +``` + +`bootstrap-r2` verifies authentication with `wrangler whoami` before creating +anything, and creating a bucket that already exists is not an error. + +### 2. Enumerate what has to be copied + +Tracked manifests are the authoritative registry of raw objects. Every one of +them points at `ledger-raw` today: + +```bash +git ls-files '*manifest*.yaml' '*manifest*.yml' \ + | xargs grep -ho 'r2://ledger-raw/[^"'"'"' ]*' | sort -u > /tmp/chronicle-raw-objects.txt +wc -l < /tmp/chronicle-raw-objects.txt +``` + +That is 186 distinct objects at `ff3efd3`, spread over 154 manifest files. Recount +rather than trusting the number: source packages land continuously, and each new +package adds objects. + +The derived bucket needs no backfill. Derived artifacts are reproducible by +definition and are already keyed by `{build_id}`, so a rebuild republishes them +into whichever bucket is configured. + +### 3. Backfill-copy the raw objects + +Keys are content-addressed and identical across buckets, so the copy is a +straight get/put per object: + +```bash +mkdir -p /tmp/chronicle-r2-backfill +while read -r uri; do + key=${uri#r2://ledger-raw/} + dest=/tmp/chronicle-r2-backfill/$key + mkdir -p "$(dirname "$dest")" + bunx wrangler r2 object get "ledger-raw/$key" --file "$dest" --remote + bunx wrangler r2 object put "chronicle-raw/$key" --file "$dest" --remote +done < /tmp/chronicle-raw-objects.txt +``` + +### 4. Verify the copy against the keys themselves + +Every raw key ends `.../{sha256}/{filename}`, so the key is its own checksum +witness and verification needs no manifest lookup: + +```bash +while read -r uri; do + key=${uri#r2://ledger-raw/} + expected=$(printf '%s\n' "$key" | awk -F/ '{print $(NF-1)}') + actual=$(shasum -a 256 "/tmp/chronicle-r2-backfill/$key" | cut -d' ' -f1) + [ "$expected" = "$actual" ] || echo "MISMATCH $key" +done < /tmp/chronicle-raw-objects.txt +``` + +Silence means every downloaded object hashes to the checksum its key claims. +That covers the read from `ledger-raw`; to cover the write to `chronicle-raw`, +re-download each key from the new bucket into a second directory and rerun the +same loop against it. + +### 5. Flip the defaults, in a follow-up PR + +Once the new buckets are filled and verified, change `DEFAULT_R2_RAW_BUCKET` and +`DEFAULT_R2_DERIVED_BUCKET` in `chronicle/artifacts.py` to `chronicle-raw` and +`chronicle-derived`. Until then, operators can opt in per-shell: + +```bash +export CHRONICLE_R2_RAW_BUCKET=chronicle-raw +export CHRONICLE_R2_DERIVED_BUCKET=chronicle-derived +``` + +New raw publications land in the new bucket from that point. Manifests written +before the flip keep pointing at `ledger-raw`, which is why the old bucket stays +readable. + +### 6. Set the ledger-era buckets read-only + +`ledger-raw` and `ledger-derived` keep serving archived witness records after the +flip. They should accept no further writes and should never be deleted. + ## Non-Goals Supabase should not store large raw binary artifacts. It should point to R2. From b703c165c398d592f58f3a028ed4dc5ebb4141cb Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 2 Sep 2026 11:34:42 -0400 Subject: [PATCH 05/22] Pin the boundary guard against the bucket rename The consumer-fact boundary guard now matches derived artifacts on shape, but nothing tested the cases that motivated the change. Both of these pass every check the old literal matching applied, so they would have slipped through silently once the buckets are renamed: - a fact carrying no bucket and no key, only a `r2://chronicle-derived/derived/source/fact.json` URI. The old code compared the URI against two hardcoded prefixes, `r2://ledger-derived/` and `r2://ledger-raw/derived/`, and matched neither. - a `source_file` prefixed `chronicle-derived:`. The old code matched the literal prefix `ledger-derived:`. Co-Authored-By: Claude Fable 5.1 --- tests/test_chronicle_consumer_contract.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/test_chronicle_consumer_contract.py b/tests/test_chronicle_consumer_contract.py index 8dd13e46..a34f3bd4 100644 --- a/tests/test_chronicle_consumer_contract.py +++ b/tests/test_chronicle_consumer_contract.py @@ -727,6 +727,29 @@ def test_export_consumer_facts_cli_rejects_contract_invalid_facts(tmp_path, caps }, "irs_soi.ty2024.table.us.taxable_interest_amount.ledger_derived", ), + # The guard used to match two hardcoded URI prefixes, so a URI naming + # any derived bucket other than `ledger-derived` did not match. Once the + # buckets are renamed (PolicyEngine/chronicle#143, mechanism 3) that is + # every derived URI, so the guard has to match on shape. + ( + { + "source_name": "irs_soi", + "source_file": "publisher.xlsx", + "raw_r2_bucket": None, + "raw_r2_key": None, + "raw_r2_uri": "r2://chronicle-derived/derived/source/fact.json", + }, + "publisher.raw.fact", + ), + ( + { + "source_name": "irs_soi", + "source_file": "chronicle-derived:taxable_interest.json", + "raw_r2_bucket": "ledger-raw", + "raw_r2_uri": "r2://ledger-raw/raw/source/publisher.xlsx", + }, + "publisher.raw.fact", + ), ], ) def test_consumer_contract_rejects_downstream_derived_target_facts( From d2360276410ab0cf608698bfe1aa12156c2e4712 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 2 Sep 2026 12:29:41 -0400 Subject: [PATCH 06/22] Isolate the rename window for every test, not one module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The env-isolation fixture lived in tests/test_chronicle_env.py, so only that module ran with CHRONICLE_/POLICYENGINE_LEDGER_/LEDGER_ variables stripped. Tests elsewhere assert the defaults those variables override — the raw and derived bucket names, the Supabase schema — and failed when an operator's shell had them set: CHRONICLE_R2_RAW_BUCKET=zzz CHRONICLE_SCHEMA=zzz uv run pytest -q FAILED tests/test_chronicle_artifacts.py::test_publish_source_artifacts_uploads_manifest_entries The fixture moves to tests/conftest.py as a suite-wide autouse fixture, and tests/test_chronicle_env.py gains an assertion that no rename-window variable reaches a test. db.supabase_client resolves LEDGER_SCHEMA and TARGETS_SCHEMA at import, which happens during collection — before any fixture runs — so the namespace test re-imports the module under the cleared environment instead of asserting on the constant it bound at collection time. Co-Authored-By: Claude Fable 5.1 --- tests/conftest.py | 33 +++++++++++++++++++++++++++++++ tests/test_chronicle_env.py | 33 +++++++++++++++++-------------- tests/test_chronicle_namespace.py | 24 +++++++++++++++------- 3 files changed, 68 insertions(+), 22 deletions(-) create mode 100644 tests/conftest.py diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 00000000..378fa0b3 --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,33 @@ +"""Shared fixtures for the Chronicle test suite. + +Chronicle is mid-rename (PolicyEngine/chronicle#143, mechanism 3), so its +settings answer to three prefixes at once: ``CHRONICLE_``, and the ledger-era +``POLICYENGINE_LEDGER_`` and ``LEDGER_``. Any of them can be set in an +operator's shell, and many tests assert the defaults those variables override. +Isolation therefore belongs to the whole suite, not to one module. +""" + +from __future__ import annotations + +import os + +import pytest + +from chronicle.env import ( + CHRONICLE_ENV_PREFIX, + LEGACY_ENV_PREFIXES, + reset_env_deprecation_state, +) + +RENAME_WINDOW_PREFIXES = (CHRONICLE_ENV_PREFIX, *LEGACY_ENV_PREFIXES) + + +@pytest.fixture(autouse=True) +def isolated_rename_window_env(monkeypatch): + """Run every test with no rename-window variable inherited from the shell.""" + for name in list(os.environ): + if name.startswith(RENAME_WINDOW_PREFIXES): + monkeypatch.delenv(name, raising=False) + reset_env_deprecation_state() + yield + reset_env_deprecation_state() diff --git a/tests/test_chronicle_env.py b/tests/test_chronicle_env.py index 4ffe0206..65029d14 100644 --- a/tests/test_chronicle_env.py +++ b/tests/test_chronicle_env.py @@ -2,7 +2,8 @@ Chronicle's operational stores migrate by dual-run (PolicyEngine/chronicle#143, mechanism 3): ``CHRONICLE_*`` names win, ledger-era names keep working behind a -deprecation warning. Every test here is hermetic — the fixture strips every +deprecation warning. Every test here is hermetic — the suite-wide +``isolated_rename_window_env`` fixture in ``tests/conftest.py`` strips every variable in the rename window from the ambient environment first. """ @@ -27,7 +28,6 @@ env_flag, env_names, env_value, - reset_env_deprecation_state, ) from chronicle.harness import main as harness_main from chronicle.source_package import ( @@ -35,19 +35,6 @@ SOURCE_ARTIFACT_FETCH_ENV, ) -RENAME_WINDOW_PREFIXES = (CHRONICLE_ENV_PREFIX, *LEGACY_ENV_PREFIXES) - - -@pytest.fixture(autouse=True) -def isolated_rename_window_env(monkeypatch): - """Run each test with no rename-window variable inherited from the shell.""" - for name in list(os.environ): - if name.startswith(RENAME_WINDOW_PREFIXES): - monkeypatch.delenv(name, raising=False) - reset_env_deprecation_state() - yield - reset_env_deprecation_state() - def _fake_wrangler(tmp_path, log): wrangler = tmp_path / "wrangler" @@ -61,6 +48,22 @@ def _fake_wrangler(tmp_path, log): # --------------------------------------------------------------------------- +def test_every_test_runs_with_the_rename_window_cleared(): + """Isolation is suite-wide (tests/conftest.py), not module-scoped. + + Modules well outside this one assert the defaults these variables override + — the raw and derived bucket names, the Supabase schema — so an operator's + shell must not reach any test. + """ + leaked = sorted( + name + for name in os.environ + if name.startswith((CHRONICLE_ENV_PREFIX, *LEGACY_ENV_PREFIXES)) + ) + + assert leaked == [] + + def test_env_names_puts_chronicle_first_then_ledger_era_names(): assert env_names("CHRONICLE_SOURCE_ARTIFACT_FETCH") == ( "CHRONICLE_SOURCE_ARTIFACT_FETCH", diff --git a/tests/test_chronicle_namespace.py b/tests/test_chronicle_namespace.py index 9b0ffd41..e8890948 100644 --- a/tests/test_chronicle_namespace.py +++ b/tests/test_chronicle_namespace.py @@ -1,5 +1,7 @@ """Tests for the Chronicle namespace.""" +import importlib + from chronicle.client import get_supabase_client from chronicle.normalization import convert_units from chronicle.targets import ( @@ -8,11 +10,7 @@ query_targets, ) from db.schema import Target as DbTarget -from db.supabase_client import ( - LEDGER_SCHEMA, - TARGETS_SCHEMA, - query_targets as db_query_targets, -) +from db.supabase_client import query_targets as db_query_targets def test_chronicle_targets_reexport_schema_objects(): @@ -29,8 +27,20 @@ def test_chronicle_client_reexports_supabase_client(): def test_chronicle_supabase_schema_boundaries_are_defaulted(): - assert LEDGER_SCHEMA == "ledger" - assert TARGETS_SCHEMA == "targets" + """The schema names are import-time constants, so re-read them here. + + ``db.supabase_client`` resolves them from the environment when it is first + imported, which happens at collection — before the suite-wide + ``isolated_rename_window_env`` fixture clears an operator's + ``CHRONICLE_SCHEMA``. Reloading under the cleared environment is what makes + this a test of the defaults rather than of the shell. + """ + import db.supabase_client + + supabase_client = importlib.reload(db.supabase_client) + + assert supabase_client.LEDGER_SCHEMA == "ledger" + assert supabase_client.TARGETS_SCHEMA == "targets" def test_chronicle_normalization_exports_helpers(): From 77c9fe9cb9f722cd83e5401bf37b9a58d59259ac Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 2 Sep 2026 12:29:53 -0400 Subject: [PATCH 07/22] Refuse to attach a recorded R2 URI to different bytes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A raw R2 key is content-addressed, so a recorded storage.r2 block is a claim about specific bytes. fetch-artifact preserved that block whenever one existed and the fetch did not re-upload into the same bucket, then rewrote the entry's sha256 and size_bytes from the newly fetched bytes. When a publisher re-publishes under the same URL and vintage — the IRS did exactly this to the 2022 IRA tables today (PolicyEngine/chronicle#225) — the manifest ended up describing the new bytes while pointing at the old object's key and URI. Verified against this branch's parent: a second fetch of different bytes leaves sha256=109dcf49… in an entry whose storage.r2 key is addressed by c63744a4…, both when the fetch only registers the bytes and when the bucket default has moved. Identity now decides, not the bucket: - the recorded key's last two segments (sha256, filename) are compared with the fetched bytes. Identical: the recorded block is preserved exactly, whichever bucket is configured now. Different: SourceArtifactRevisionError, raised before the cached artifact or its manifest entry is touched, naming recorded and fetched sha256/size_bytes and the ADR rule that the same vintage with new bytes is a new release revision. - --record-revision opts in: the fetched bytes get their own content-addressed key under the configured bucket, never the old key, and the superseded block moves to storage.previous_r2 with its sha256, size_bytes and fetched_at so the earlier bytes stay addressable. - publish-raw applies the same check before treating a recorded block as history, so a local file the recorded object does not hold is refused rather than uploaded. storage.previous_r2 is a sibling of storage.r2: every reader (inventory-artifacts, publish-raw, source_package._artifact_content and the suite's raw-R2-link acceptance check) reads storage.r2 alone, and publish-raw already spreads the existing storage block when it rewrites, so a revision survives publication untouched. Co-Authored-By: Claude Fable 5.1 --- chronicle/artifacts.py | 330 ++++++++++++++++++++++++------ chronicle/harness.py | 57 ++++-- tests/test_chronicle_artifacts.py | 287 ++++++++++++++++++++++++++ 3 files changed, 599 insertions(+), 75 deletions(-) diff --git a/chronicle/artifacts.py b/chronicle/artifacts.py index 001d6994..edb9752f 100644 --- a/chronicle/artifacts.py +++ b/chronicle/artifacts.py @@ -51,6 +51,18 @@ def default_r2_derived_bucket() -> str: return env_value(R2_DERIVED_BUCKET_ENV, default=DEFAULT_R2_DERIVED_BUCKET) +class SourceArtifactRevisionError(RuntimeError): + """Fetched bytes are not the bytes the recorded R2 object holds. + + Raw R2 keys are content-addressed, so a recorded ``storage.r2`` block is a + claim about specific bytes. When a publisher re-publishes under the same + URL and vintage, keeping that block would attach its provenance to bytes it + never described. Chronicle refuses instead: same vintage plus new bytes is a + new release revision (docs/adr-chronicle-fact-identity-v2.md), registered + with ``fetch-artifact --record-revision``. + """ + + # New UK and New Zealand uploads are namespaced by country. US objects predate # the country segment and deliberately keep their legacy ``raw/{source_id}`` # and ``derived/{source_id}`` shapes. Publisher directories are the stable @@ -417,11 +429,19 @@ def fetch_source_artifact( table: str | None = None, filename: str | None = None, upload_r2: bool = False, + record_revision: bool = False, r2_bucket: str | None = None, r2_prefix: str | None = None, wrangler_command: str = "npx wrangler", ) -> ArtifactFetchReport: - """Fetch/register a source artifact and optionally upload it to R2.""" + """Fetch/register a source artifact and optionally upload it to R2. + + ``record_revision`` opts into registering a publisher revision: the fetched + bytes get their own content-addressed key under the configured bucket and + the superseded object moves to ``storage.previous_r2``. Without it, bytes + that disagree with the recorded object raise + :class:`SourceArtifactRevisionError` before anything is overwritten. + """ r2_bucket = r2_bucket or default_r2_raw_bucket() output = Path(output_dir) resolved_r2_prefix = resolve_r2_prefix( @@ -436,14 +456,26 @@ def fetch_source_artifact( if not artifact_filename: raise ValueError("Could not infer artifact filename; pass --filename.") - output.mkdir(parents=True, exist_ok=True) - local_path = output / artifact_filename - local_path.write_bytes(content) - sha256 = hashlib.sha256(content).hexdigest() size_bytes = len(content) manifest_path = output / "manifest.yaml" + # Guard before the cached artifact is touched. A rejected fetch must leave + # the recorded bytes and their manifest entry exactly as they were. + _assert_recorded_object_holds_these_bytes( + manifest_path, + year=year, + filename=artifact_filename, + sha256=sha256, + size_bytes=size_bytes, + r2_bucket=r2_bucket, + record_revision=record_revision, + ) + + output.mkdir(parents=True, exist_ok=True) + local_path = output / artifact_filename + local_path.write_bytes(content) + r2_location = ArtifactStorageLocation( provider="r2", bucket=r2_bucket, @@ -482,6 +514,7 @@ def fetch_source_artifact( size_bytes=size_bytes, fetched_at=fetched_at, r2_location=(r2_location if upload_r2 and r2_upload and r2_upload.ok else None), + record_revision=record_revision, ) return ArtifactFetchReport( @@ -1053,17 +1086,181 @@ def _filename_from_url(source_url: str) -> str: return Path(unquote(parsed.path)).name -def _recorded_r2(spec: Any) -> dict[str, Any]: - """Return a manifest file spec's recorded ``storage.r2`` block, if any.""" +def _read_manifest(manifest_path: Path) -> dict[str, Any]: + """Return a manifest's parsed payload, or an empty mapping.""" + if not manifest_path.exists(): + return {} + payload = yaml.safe_load(manifest_path.read_text(encoding="utf-8")) or {} + return payload if isinstance(payload, dict) else {} + + +def _manifest_file_spec(payload: dict[str, Any], year: Any) -> dict[str, Any]: + """Return one manifest ``files`` entry, or an empty mapping.""" + files = payload.get("files") + if not isinstance(files, dict): + return {} + spec = files.get(year) + return spec if isinstance(spec, dict) else {} + + +def _recorded_storage(spec: Any) -> dict[str, Any]: + """Return a manifest file spec's recorded ``storage`` block, if any.""" if not isinstance(spec, dict): return {} storage = spec.get("storage") - if not isinstance(storage, dict): - return {} - recorded = storage.get("r2") + return storage if isinstance(storage, dict) else {} + + +def _recorded_r2(spec: Any) -> dict[str, Any]: + """Return a manifest file spec's recorded ``storage.r2`` block, if any.""" + recorded = _recorded_storage(spec).get("r2") return recorded if isinstance(recorded, dict) else {} +def _r2_key_identity(key: Any) -> tuple[str, str]: + """Return the ``(sha256, filename)`` a raw R2 key is addressed by. + + Raw keys are ``{prefix}/{source_id}/{package_id}/{year}/{sha256}/{filename}`` + (see :func:`build_r2_key`), so the last two segments say which bytes the + object holds. A key in any other shape yields empty strings and therefore + never matches fetched bytes. + """ + if not isinstance(key, str): + return ("", "") + parts = [part for part in key.split("/") if part] + if len(parts) < 2: + return ("", "") + return (parts[-2], parts[-1]) + + +def _r2_holds_these_bytes( + recorded_r2: dict[str, Any], + *, + sha256: str, + filename: str, +) -> bool: + """Whether a recorded ``storage.r2`` block addresses exactly these bytes.""" + recorded_sha256, recorded_filename = _r2_key_identity(recorded_r2.get("key")) + return bool(recorded_sha256) and (recorded_sha256, recorded_filename) == ( + sha256, + Path(filename).name, + ) + + +def _revision_error_message( + *, + manifest_path: Path, + year: Any, + filename: str, + recorded_spec: dict[str, Any], + recorded_r2: dict[str, Any], + sha256: str, + size_bytes: int, + r2_bucket: str, +) -> str: + """Explain a refused fetch: recorded identity, fetched identity, next step.""" + recorded_sha256, recorded_filename = _r2_key_identity(recorded_r2.get("key")) + declared_sha256 = recorded_spec.get("sha256") + recorded_size = ( + recorded_spec.get("size_bytes") if declared_sha256 == recorded_sha256 else None + ) + message = ( + f"{manifest_path} entry {year!r} already records the R2 object " + f"{recorded_r2.get('uri') or recorded_r2.get('key')}, which holds " + f"sha256={recorded_sha256 or 'unknown'} " + f"filename={recorded_filename or 'unknown'} " + f"size_bytes={recorded_size if recorded_size is not None else 'unknown'}. " + f"The fetched bytes are sha256={sha256} filename={Path(filename).name} " + f"size_bytes={size_bytes}. Chronicle will not attach a recorded, " + "content-addressed R2 URI to bytes it does not describe." + ) + if declared_sha256 and declared_sha256 != recorded_sha256: + message += ( + f" (The entry also declares sha256={declared_sha256}, which its own " + "R2 key contradicts: an earlier fetch rewrote the hash without " + "moving the object.)" + ) + return message + ( + " The same vintage with new bytes is a new release revision " + "(docs/adr-chronicle-fact-identity-v2.md). Re-run with " + "--record-revision to store the fetched bytes under their own " + f"content-addressed key in {r2_bucket} and keep the superseded object " + "in storage.previous_r2." + ) + + +def _assert_recorded_object_holds_these_bytes( + manifest_path: Path, + *, + year: Any, + filename: str, + sha256: str, + size_bytes: int, + r2_bucket: str, + record_revision: bool, +) -> None: + """Refuse a publisher revision that has not been opted into.""" + if record_revision: + return + recorded_spec = _manifest_file_spec(_read_manifest(manifest_path), year) + recorded_r2 = _recorded_r2(recorded_spec) + if not recorded_r2: + return + if _r2_holds_these_bytes(recorded_r2, sha256=sha256, filename=filename): + return + raise SourceArtifactRevisionError( + _revision_error_message( + manifest_path=manifest_path, + year=year, + filename=filename, + recorded_spec=recorded_spec, + recorded_r2=recorded_r2, + sha256=sha256, + size_bytes=size_bytes, + r2_bucket=r2_bucket, + ) + ) + + +def _superseding_storage( + recorded_spec: dict[str, Any], + *, + new_r2: dict[str, Any] | None, + superseded_at: str, +) -> dict[str, Any]: + """Return a storage block in which the recorded object becomes history. + + ``storage.r2`` only ever names the object that holds the entry's current + bytes. The superseded block is appended, oldest first, to + ``storage.previous_r2`` so the earlier bytes stay addressable by the URI + archived witness records already pin. + """ + storage = dict(_recorded_storage(recorded_spec)) + previous = storage.get("previous_r2") + entries = list(previous) if isinstance(previous, list) else [] + recorded_r2 = _recorded_r2(recorded_spec) + if recorded_r2: + entry = dict(recorded_r2) + recorded_sha256, _recorded_filename = _r2_key_identity(recorded_r2.get("key")) + if recorded_sha256: + entry["sha256"] = recorded_sha256 + if recorded_spec.get("sha256") == recorded_sha256: + # Only carry metadata the superseded key agrees with: a manifest + # can arrive here already describing the new bytes. + for field in ("size_bytes", "fetched_at", "source_url"): + value = recorded_spec.get(field) + if value is not None: + entry[field] = value + entry["superseded_at"] = superseded_at + entries.append(entry) + storage["previous_r2"] = entries + if new_r2 is None: + storage.pop("r2", None) + else: + storage["r2"] = new_r2 + return storage + + def _upsert_manifest( manifest_path: Path, *, @@ -1079,11 +1276,9 @@ def _upsert_manifest( size_bytes: int, fetched_at: str, r2_location: ArtifactStorageLocation | None, + record_revision: bool = False, ) -> None: - if manifest_path.exists(): - payload = yaml.safe_load(manifest_path.read_text(encoding="utf-8")) or {} - else: - payload = {} + payload = _read_manifest(manifest_path) payload.setdefault("source_id", source_id) payload.setdefault("package_id", package_id) payload.setdefault("dataset", dataset) @@ -1097,18 +1292,44 @@ def _upsert_manifest( "size_bytes": size_bytes, "fetched_at": fetched_at, } - recorded_r2 = _recorded_r2(payload["files"].get(year)) + recorded_spec = _manifest_file_spec(payload, year) + recorded_storage = _recorded_storage(recorded_spec) + recorded_r2 = _recorded_r2(recorded_spec) new_r2 = r2_location.to_dict() if r2_location is not None else None - if recorded_r2 and ( - new_r2 is None or recorded_r2.get("bucket") != new_r2.get("bucket") + if recorded_r2 and _r2_holds_these_bytes( + recorded_r2, sha256=sha256, filename=filename ): - # A recorded storage.r2 block is historical truth: archived witness - # records pin raw R2 URLs by hash. Re-fetching under a renamed bucket - # copies bytes; it does not restate where the bytes were first - # published (PolicyEngine/chronicle#143, mechanism 3). - file_entry["storage"] = {"r2": recorded_r2} + # A recorded storage.r2 block for these exact bytes is historical + # truth: archived witness records pin raw R2 URLs by hash. Re-fetching + # under a renamed bucket copies bytes; it does not restate where the + # bytes were first published (PolicyEngine/chronicle#143, mechanism 3). + file_entry["storage"] = {**recorded_storage, "r2": recorded_r2} + elif recorded_r2: + # Different bytes under the same vintage. The guard in + # fetch_source_artifact refuses this without --record-revision; repeat + # the check here so no caller can reach a false-provenance write. + if not record_revision: + raise SourceArtifactRevisionError( + _revision_error_message( + manifest_path=manifest_path, + year=year, + filename=filename, + recorded_spec=recorded_spec, + recorded_r2=recorded_r2, + sha256=sha256, + size_bytes=size_bytes, + r2_bucket=(new_r2 or {}).get("bucket") or default_r2_raw_bucket(), + ) + ) + file_entry["storage"] = _superseding_storage( + recorded_spec, + new_r2=new_r2, + superseded_at=fetched_at, + ) elif new_r2 is not None: - file_entry["storage"] = {"r2": new_r2} + file_entry["storage"] = {**recorded_storage, "r2": new_r2} + elif recorded_storage: + file_entry["storage"] = dict(recorded_storage) payload["files"][year] = file_entry manifest_path.write_text( yaml.safe_dump(payload, sort_keys=False), @@ -1169,7 +1390,10 @@ def _publish_raw_manifest_entry( if sha256_expected and sha256_actual != sha256_expected: errors.append("checksum_mismatch") - if errors: + def refuse(reason: str | None = None) -> tuple[RawArtifactPublishEntry, None]: + """Report the entry unpublished, with nothing uploaded or rewritten.""" + if reason is not None: + errors.append(reason) return ( RawArtifactPublishEntry( manifest_path=str(manifest_path), @@ -1187,6 +1411,27 @@ def _publish_raw_manifest_entry( None, ) + if errors: + return refuse() + + recorded_r2 = _recorded_r2(spec) + if recorded_r2 and not _r2_holds_these_bytes( + recorded_r2, sha256=sha256_actual or "", filename=filename + ): + # The recorded object is addressed by different bytes, so it is not + # this file's history. Uploading anyway would either publish under a + # key that misdescribes its content or restate a URI that belongs to + # the superseded bytes. Registering a publisher revision is + # `fetch-artifact --record-revision`, not a publish-time rewrite. + recorded_sha256, recorded_filename = _r2_key_identity(recorded_r2.get("key")) + return refuse( + "recorded_r2_identity_mismatch:" + f"recorded_sha256={recorded_sha256 or 'unknown'}:" + f"recorded_filename={recorded_filename or 'unknown'}:" + f"local_sha256={sha256_actual}:" + f"local_filename={Path(filename).name}" + ) + location = ArtifactStorageLocation( provider="r2", bucket=r2_bucket, @@ -1200,54 +1445,21 @@ def _publish_raw_manifest_entry( package_path=manifest_path, ), ) - recorded_r2 = _recorded_r2(spec) recorded_bucket = recorded_r2.get("bucket") if recorded_bucket and recorded_bucket != location.bucket: # The recorded bucket is preserved history. Publishing the same bytes # into a renamed bucket is a backfill copy, not a restatement, so the # manifest must not be rewritten to point at the new bucket. - errors.append( + return refuse( "recorded_r2_bucket_is_preserved_history:" f"recorded={recorded_bucket}:requested={location.bucket}" ) - return ( - RawArtifactPublishEntry( - manifest_path=str(manifest_path), - source_id=source_id, - package_id=package_id, - year=str(year), - filename=filename, - local_path=str(artifact_path), - sha256=sha256_actual, - size_bytes=size_bytes, - r2_location=None, - upload=None, - errors=tuple(errors), - ), - None, - ) recorded_key = recorded_r2.get("key") if recorded_key and recorded_key != location.key: - errors.append( + return refuse( "recorded_r2_key_disagrees_with_country_prefix:" f"recorded={recorded_key}:expected={location.key}" ) - return ( - RawArtifactPublishEntry( - manifest_path=str(manifest_path), - source_id=source_id, - package_id=package_id, - year=str(year), - filename=filename, - local_path=str(artifact_path), - sha256=sha256_actual, - size_bytes=size_bytes, - r2_location=None, - upload=None, - errors=tuple(errors), - ), - None, - ) upload = _upload_r2_object( location, artifact_path, diff --git a/chronicle/harness.py b/chronicle/harness.py index 19b7a253..528d4f77 100644 --- a/chronicle/harness.py +++ b/chronicle/harness.py @@ -5,6 +5,7 @@ import argparse import json import shlex +import sys from pathlib import Path from chronicle.artifacts import ( @@ -15,6 +16,7 @@ DerivedArtifactPublishReport, R2BootstrapReport, RawArtifactPublishReport, + SourceArtifactRevisionError, bootstrap_r2_buckets, fetch_source_artifact, inventory_source_artifacts, @@ -337,11 +339,17 @@ def fetch_artifact_file( table: str | None = None, filename: str | None = None, upload_r2: bool = False, + record_revision: bool = False, r2_bucket: str | None = None, r2_prefix: str | None = None, wrangler_command: str = "npx wrangler", ) -> ArtifactFetchReport: - """Fetch/register a raw source artifact and optionally upload it to R2.""" + """Fetch/register a raw source artifact and optionally upload it to R2. + + Raises :class:`SourceArtifactRevisionError` when the fetched bytes are not + the bytes the manifest's recorded R2 object holds, unless + ``record_revision`` opts into registering the publisher revision. + """ return fetch_source_artifact( source_url, source_id=source_id, @@ -353,6 +361,7 @@ def fetch_artifact_file( table=table, filename=filename, upload_r2=upload_r2, + record_revision=record_revision, r2_bucket=r2_bucket, r2_prefix=r2_prefix, wrangler_command=wrangler_command, @@ -888,6 +897,17 @@ def main(argv: list[str] | None = None) -> int: action="store_true", help="Upload the artifact to R2 after local checksum capture.", ) + artifact_parser.add_argument( + "--record-revision", + action="store_true", + help=( + "Register a publisher revision: the fetched bytes get their own " + "content-addressed key under the configured bucket and the " + "superseded object moves to storage.previous_r2. Without this " + "flag, bytes that disagree with the recorded R2 object are " + "refused." + ), + ) artifact_parser.add_argument( "--r2-bucket", default=None, @@ -1321,21 +1341,26 @@ def main(argv: list[str] | None = None) -> int: print(json.dumps(report.to_dict(), indent=2, sort_keys=True)) return 0 if args.command == "fetch-artifact": - report = fetch_artifact_file( - args.url, - source_id=args.source_id, - package_id=args.package_id, - year=args.year, - output_dir=args.out_dir, - dataset=args.dataset, - source_page=args.source_page, - table=args.table, - filename=args.filename, - upload_r2=args.upload_r2, - r2_bucket=args.r2_bucket, - r2_prefix=args.r2_prefix, - wrangler_command=args.wrangler_command, - ) + try: + report = fetch_artifact_file( + args.url, + source_id=args.source_id, + package_id=args.package_id, + year=args.year, + output_dir=args.out_dir, + dataset=args.dataset, + source_page=args.source_page, + table=args.table, + filename=args.filename, + upload_r2=args.upload_r2, + record_revision=args.record_revision, + r2_bucket=args.r2_bucket, + r2_prefix=args.r2_prefix, + wrangler_command=args.wrangler_command, + ) + except SourceArtifactRevisionError as error: + print(f"error: {error}", file=sys.stderr) + return 1 print(json.dumps(report.to_dict(), indent=2, sort_keys=True)) return 0 if report.valid else 1 if args.command == "inventory-artifacts": diff --git a/tests/test_chronicle_artifacts.py b/tests/test_chronicle_artifacts.py index 759e3157..6a3f7e5b 100644 --- a/tests/test_chronicle_artifacts.py +++ b/tests/test_chronicle_artifacts.py @@ -11,6 +11,7 @@ from chronicle.cli import main as cli_main from chronicle.artifacts import ( + SourceArtifactRevisionError, build_artifact_rows, build_derived_r2_key, bootstrap_r2_buckets, @@ -756,3 +757,289 @@ def test_fetch_artifact_keeps_an_already_recorded_bucket(tmp_path, monkeypatch): assert ( second["files"][2023]["storage"]["r2"] == first["files"][2023]["storage"]["r2"] ) + + +# --------------------------------------------------------------------------- +# Publisher revisions +# +# A raw R2 key is content-addressed, so a recorded storage.r2 block is a claim +# about specific bytes. On 2026-09-02 the IRS re-published 22in05ira.xlsx and +# 22in06ira.xlsx under their existing URLs (PolicyEngine/chronicle#225): a +# repeated fetch must never pair those new bytes with the old object's URI. +# --------------------------------------------------------------------------- + +REPUBLISHED_URL = "https://www.irs.gov/pub/irs-soi/22in05ira.xlsx" +REPUBLISHED_FILENAME = "22in05ira.xlsx" +FIRST_PUBLICATION = b"IRA table 5, first publication" +SECOND_PUBLICATION = b"IRA table 5, silently re-published with revised rows" + + +def _wrangler_stub(tmp_path, log): + wrangler = tmp_path / "wrangler" + wrangler.write_text(f"#!/bin/sh\nprintf '%s\\n' \"$*\" >> {log}\necho ok\n") + wrangler.chmod(0o755) + return wrangler + + +def _serve(monkeypatch, content): + """Serve ``content`` from the publisher URL, without touching the network.""" + + def _fake_read_artifact(source_url): + assert source_url == REPUBLISHED_URL + return content, REPUBLISHED_FILENAME + + monkeypatch.setattr("chronicle.artifacts._read_artifact", _fake_read_artifact) + + +def _fetch_republished(output_dir, wrangler, *, upload_r2=True, **kwargs): + return fetch_source_artifact( + REPUBLISHED_URL, + source_id="irs_soi", + package_id="soi-table-5", + year=2022, + output_dir=output_dir, + upload_r2=upload_r2, + wrangler_command=str(wrangler), + **kwargs, + ) + + +def test_repeated_fetch_of_identical_bytes_preserves_the_recorded_block( + tmp_path, monkeypatch +): + """Same bytes: the recorded block survives whatever bucket is configured.""" + output_dir = tmp_path / "db" / "data" / "irs_soi" / "soi-table-5" + log = tmp_path / "wrangler.log" + wrangler = _wrangler_stub(tmp_path, log) + manifest_path = output_dir / "manifest.yaml" + _serve(monkeypatch, FIRST_PUBLICATION) + _fetch_republished(output_dir, wrangler) + first = yaml.safe_load(manifest_path.read_text())["files"][2022] + + monkeypatch.setenv("CHRONICLE_R2_RAW_BUCKET", "chronicle-raw") + report = _fetch_republished(output_dir, wrangler) + second = yaml.safe_load(manifest_path.read_text())["files"][2022] + + assert report.valid + # The backfill copy really goes to the renamed bucket, but the manifest + # keeps recording where these bytes were first published. + assert report.r2_location.bucket == "chronicle-raw" + assert "chronicle-raw" in log.read_text() + assert second["storage"] == first["storage"] + assert second["storage"]["r2"]["bucket"] == "ledger-raw" + assert "previous_r2" not in second["storage"] + assert second["sha256"] == first["sha256"] + + +@pytest.mark.parametrize( + ("upload_r2", "configured_bucket"), + [ + pytest.param(True, None, id="reuploaded"), + # The two routes that reached a manifest in the wild: a fetch that only + # registers the bytes, and a fetch once the bucket default has moved. + # Both preserved the recorded block while rewriting sha256/size_bytes. + pytest.param(False, None, id="registered-without-upload"), + pytest.param(True, "chronicle-raw", id="after-the-bucket-rename"), + ], +) +def test_repeated_fetch_of_different_bytes_is_refused( + tmp_path, monkeypatch, upload_r2, configured_bucket +): + """A publisher revision must not inherit the recorded object's provenance.""" + output_dir = tmp_path / "db" / "data" / "irs_soi" / "soi-table-5" + log = tmp_path / "wrangler.log" + wrangler = _wrangler_stub(tmp_path, log) + manifest_path = output_dir / "manifest.yaml" + artifact_path = output_dir / REPUBLISHED_FILENAME + _serve(monkeypatch, FIRST_PUBLICATION) + first_report = _fetch_republished(output_dir, wrangler) + manifest_before = manifest_path.read_bytes() + uploads_before = log.read_text() + + if configured_bucket: + monkeypatch.setenv("CHRONICLE_R2_RAW_BUCKET", configured_bucket) + _serve(monkeypatch, SECOND_PUBLICATION) + with pytest.raises(SourceArtifactRevisionError) as raised: + _fetch_republished(output_dir, wrangler, upload_r2=upload_r2) + + message = str(raised.value) + assert first_report.sha256 in message + assert hashlib.sha256(SECOND_PUBLICATION).hexdigest() in message + assert f"size_bytes={len(FIRST_PUBLICATION)}" in message + assert f"size_bytes={len(SECOND_PUBLICATION)}" in message + assert "release revision" in message + assert "--record-revision" in message + # Nothing was overwritten, copied or uploaded on the way to the refusal. + assert manifest_path.read_bytes() == manifest_before + assert artifact_path.read_bytes() == FIRST_PUBLICATION + assert log.read_text() == uploads_before + + +def test_record_revision_writes_a_new_key_and_keeps_the_previous_object( + tmp_path, monkeypatch +): + """The opt-in records the new bytes' own key under the configured bucket.""" + output_dir = tmp_path / "db" / "data" / "irs_soi" / "soi-table-5" + log = tmp_path / "wrangler.log" + wrangler = _wrangler_stub(tmp_path, log) + manifest_path = output_dir / "manifest.yaml" + _serve(monkeypatch, FIRST_PUBLICATION) + first_report = _fetch_republished(output_dir, wrangler) + superseded = yaml.safe_load(manifest_path.read_text())["files"][2022] + + monkeypatch.setenv("CHRONICLE_R2_RAW_BUCKET", "chronicle-raw") + _serve(monkeypatch, SECOND_PUBLICATION) + report = _fetch_republished(output_dir, wrangler, record_revision=True) + revised = yaml.safe_load(manifest_path.read_text())["files"][2022] + revised_sha256 = hashlib.sha256(SECOND_PUBLICATION).hexdigest() + + assert report.valid + # storage.r2 names the object that holds the entry's current bytes... + assert revised["sha256"] == revised_sha256 + assert revised["size_bytes"] == len(SECOND_PUBLICATION) + assert revised["storage"]["r2"]["bucket"] == "chronicle-raw" + assert revised["storage"]["r2"]["key"] == ( + f"raw/irs_soi/soi-table-5/2022/{revised_sha256}/{REPUBLISHED_FILENAME}" + ) + assert revised["storage"]["r2"]["uri"] == ( + f"r2://chronicle-raw/{revised['storage']['r2']['key']}" + ) + # ...and never the superseded key, which stays addressable as history. + previous = revised["storage"]["previous_r2"] + assert [entry["uri"] for entry in previous] == [superseded["storage"]["r2"]["uri"]] + assert previous[0]["bucket"] == "ledger-raw" + assert previous[0]["sha256"] == first_report.sha256 + assert previous[0]["size_bytes"] == len(FIRST_PUBLICATION) + assert previous[0]["fetched_at"] == superseded["fetched_at"] + assert previous[0]["superseded_at"] == revised["fetched_at"] + assert (output_dir / REPUBLISHED_FILENAME).read_bytes() == SECOND_PUBLICATION + assert f"chronicle-raw/{revised['storage']['r2']['key']}" in log.read_text() + + +def test_a_revised_manifest_still_reads_as_one_r2_linked_artifact( + tmp_path, monkeypatch +): + """storage.previous_r2 is a sibling key, so every storage.r2 reader is intact.""" + output_dir = tmp_path / "db" / "data" / "irs_soi" / "soi-table-5" + wrangler = _wrangler_stub(tmp_path, tmp_path / "wrangler.log") + _serve(monkeypatch, FIRST_PUBLICATION) + _fetch_republished(output_dir, wrangler) + _serve(monkeypatch, SECOND_PUBLICATION) + _fetch_republished(output_dir, wrangler, record_revision=True) + + inventory = inventory_source_artifacts(output_dir) + + assert inventory.valid + assert inventory.counts["r2_link_count"] == 1 + assert inventory.counts["checksum_mismatch_count"] == 0 + assert inventory.entries[0].r2["bucket"] == "ledger-raw" + assert inventory.entries[0].sha256_actual == ( + hashlib.sha256(SECOND_PUBLICATION).hexdigest() + ) + + +def test_publish_raw_refuses_a_file_the_recorded_object_does_not_hold( + tmp_path, monkeypatch +): + """Recorded sha256 != local sha256 is a revision, not a backfill.""" + output_dir = tmp_path / "db" / "data" / "irs_soi" / "soi-table-5" + log = tmp_path / "wrangler.log" + wrangler = _wrangler_stub(tmp_path, log) + manifest_path = output_dir / "manifest.yaml" + _serve(monkeypatch, FIRST_PUBLICATION) + first_report = _fetch_republished(output_dir, wrangler) + + # Reproduce the state a pre-fix fetch left behind: new bytes on disk, the + # entry's own hash rewritten, the recorded key still addressing the old + # bytes. + revised_sha256 = hashlib.sha256(SECOND_PUBLICATION).hexdigest() + (output_dir / REPUBLISHED_FILENAME).write_bytes(SECOND_PUBLICATION) + manifest = yaml.safe_load(manifest_path.read_text()) + manifest["files"][2022]["sha256"] = revised_sha256 + manifest["files"][2022]["size_bytes"] = len(SECOND_PUBLICATION) + manifest_path.write_text(yaml.safe_dump(manifest, sort_keys=False)) + manifest_before = manifest_path.read_bytes() + uploads_before = log.read_text() + + report = publish_source_artifacts(output_dir, wrangler_command=str(wrangler)) + + assert not report.valid + assert report.entries[0].upload is None + assert report.entries[0].r2_location is None + assert report.entries[0].errors == ( + "recorded_r2_identity_mismatch:" + f"recorded_sha256={first_report.sha256}:" + f"recorded_filename={REPUBLISHED_FILENAME}:" + f"local_sha256={revised_sha256}:" + f"local_filename={REPUBLISHED_FILENAME}", + ) + assert log.read_text() == uploads_before + assert manifest_path.read_bytes() == manifest_before + + +def test_publish_raw_uploads_a_registered_revision_and_keeps_its_history( + tmp_path, monkeypatch +): + """Once the revision is registered, publishing it is ordinary work.""" + output_dir = tmp_path / "db" / "data" / "irs_soi" / "soi-table-5" + log = tmp_path / "wrangler.log" + wrangler = _wrangler_stub(tmp_path, log) + manifest_path = output_dir / "manifest.yaml" + _serve(monkeypatch, FIRST_PUBLICATION) + first_report = _fetch_republished(output_dir, wrangler) + _serve(monkeypatch, SECOND_PUBLICATION) + _fetch_republished(output_dir, wrangler, record_revision=True) + revised_sha256 = hashlib.sha256(SECOND_PUBLICATION).hexdigest() + + report = publish_source_artifacts(output_dir, wrangler_command=str(wrangler)) + published = yaml.safe_load(manifest_path.read_text())["files"][2022] + + assert report.valid + assert published["storage"]["r2"]["key"].endswith( + f"/{revised_sha256}/{REPUBLISHED_FILENAME}" + ) + assert [entry["sha256"] for entry in published["storage"]["previous_r2"]] == [ + first_report.sha256 + ] + + +def test_fetch_artifact_cli_refuses_a_revision_then_records_it_on_request( + tmp_path, monkeypatch, capsys +): + output_dir = tmp_path / "db" / "data" / "irs_soi" / "soi-table-5" + wrangler = _wrangler_stub(tmp_path, tmp_path / "wrangler.log") + argv = [ + "fetch-artifact", + "--url", + REPUBLISHED_URL, + "--source-id", + "irs_soi", + "--package-id", + "soi-table-5", + "--year", + "2022", + "--out-dir", + str(output_dir), + "--upload-r2", + "--wrangler-command", + str(wrangler), + ] + _serve(monkeypatch, FIRST_PUBLICATION) + assert harness_main(argv) == 0 + capsys.readouterr() + + _serve(monkeypatch, SECOND_PUBLICATION) + refused = harness_main(argv) + refusal = capsys.readouterr() + + assert refused == 1 + assert "--record-revision" in refusal.err + assert refusal.out == "" + + assert harness_main([*argv, "--record-revision"]) == 0 + recorded = json.loads(capsys.readouterr().out) + + assert recorded["sha256"] == hashlib.sha256(SECOND_PUBLICATION).hexdigest() + assert recorded["r2_location"]["key"].endswith( + f"/{recorded['sha256']}/{REPUBLISHED_FILENAME}" + ) From 13ef40dbef3759cafe71e83e51187ecf6cbb8de2 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 2 Sep 2026 12:30:52 -0400 Subject: [PATCH 08/22] Document publisher revisions alongside the bucket rename The rename-window text said a recorded storage.r2 block is preserved because the bucket may have moved. That is the weaker half of the rule: the block is preserved because it addresses the bytes in hand. Adds a Publisher Revisions section covering the identity check, the refusal, --record-revision, the storage.previous_r2 shape and why every existing storage.r2 reader is unaffected, and points the bucket-cutover, publish-flow and harness passages at it. Co-Authored-By: Claude Fable 5.1 --- README.md | 7 +++ docs/agent-source-package-harness.md | 9 ++++ docs/storage-architecture.md | 73 ++++++++++++++++++++++++++-- 3 files changed, 86 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 1cdb4ceb..4397663b 100644 --- a/README.md +++ b/README.md @@ -367,6 +367,13 @@ uv run chronicle fetch-artifact \ --table "Publication 1304 Table 1.2" \ --upload-r2 +# Re-fetching is safe: identical bytes keep the recorded storage.r2 block, and +# bytes that disagree with it are refused. When a publisher has re-published +# under the same URL and vintage, register the revision explicitly — the new +# bytes get their own content-addressed key and the superseded object is kept +# in storage.previous_r2: +uv run chronicle fetch-artifact ... --record-revision + # Audit local manifests and checksums: uv run chronicle inventory-artifacts --root db/data diff --git a/docs/agent-source-package-harness.md b/docs/agent-source-package-harness.md index afeb98f4..11200ebb 100644 --- a/docs/agent-source-package-harness.md +++ b/docs/agent-source-package-harness.md @@ -22,6 +22,15 @@ For already-downloaded manifest artifacts, agents should run `uv run chronicle publish-raw --root db/data` to upload checksum-verified bytes to R2 and write `storage.r2` metadata back into each manifest entry. +Both commands treat a recorded `storage.r2` block as a claim about specific +bytes, because raw keys are content-addressed. Re-fetching or publishing bytes +the recorded object does not hold is refused; when a publisher has re-published +under the same URL and vintage, register the revision with +`uv run chronicle fetch-artifact ... --record-revision`, which stores the new +bytes under their own key and keeps the superseded object in +`storage.previous_r2`. See +[Publisher Revisions](storage-architecture.md#publisher-revisions). + Builds do not require production raw bytes to be committed to Git. Source packages first read packaged fixture bytes, then `CHRONICLE_SOURCE_ARTIFACT_CACHE_DIR` (defaulting to diff --git a/docs/storage-architecture.md b/docs/storage-architecture.md index e381353f..6f4cfb00 100644 --- a/docs/storage-architecture.md +++ b/docs/storage-architecture.md @@ -91,6 +91,68 @@ Legacy US derived keys likewise remain `derived/{source_id}/...`. Derived artifacts are reproducible and may be replaced by a new build, but a specific `{build_id}` path should be immutable once published. +## Publisher Revisions + +A raw key embeds the sha256 of the bytes it holds, so a manifest's recorded +`storage.r2` block is a claim about specific bytes, not a pointer to a file +name. Publishers do not always honor that: on 2026-09-02 the IRS re-published +`22in05ira.xlsx` and `22in06ira.xlsx` under their existing URLs +(PolicyEngine/chronicle#225). + +`fetch-artifact` therefore compares the recorded key's `{sha256}/{filename}` +tail with the bytes it just fetched, before it writes anything: + +- **Identical** — the recorded block is preserved exactly, whichever bucket is + configured now. Re-fetching after the bucket rename copies bytes; it does not + restate where they were first published. +- **Different** — the fetch is refused. Nothing is overwritten: not the cached + artifact, not the manifest entry, and no object is uploaded. The error names + the recorded and the fetched `sha256`/`size_bytes`. Per + `docs/adr-chronicle-fact-identity-v2.md`, the same vintage with new bytes is a + new release revision, so registering it is a decision an operator makes, not a + silent rewrite. + +`--record-revision` makes that decision explicit. The fetched bytes get their +own content-addressed key under the configured bucket — never the recorded key — +and the superseded block moves to `storage.previous_r2`: + +```yaml +files: + 2022: + filename: 22in05ira.xlsx + sha256: + size_bytes: + fetched_at: "2026-09-02T17:04:11+00:00" + storage: + r2: + provider: r2 + bucket: chronicle-raw + key: raw/irs_soi/soi-table-5/2022//22in05ira.xlsx + uri: r2://chronicle-raw/raw/irs_soi/soi-table-5/2022//22in05ira.xlsx + previous_r2: + - provider: r2 + bucket: ledger-raw + key: raw/irs_soi/soi-table-5/2022//22in05ira.xlsx + uri: r2://ledger-raw/raw/irs_soi/soi-table-5/2022//22in05ira.xlsx + sha256: + size_bytes: + fetched_at: "2026-06-11T14:22:05+00:00" + superseded_at: "2026-09-02T17:04:11+00:00" +``` + +`storage.r2` only ever names the object that holds the entry's current bytes, +and `previous_r2` lists superseded objects oldest first, so the bytes an +archived witness record pinned stay addressable at the URI it pinned. Every +reader — `inventory-artifacts`, `publish-raw`, source-package artifact loading, +and the suite's raw-R2-link acceptance check — reads `storage.r2` alone, so a +revised entry reads exactly like an unrevised one; `publish-raw` preserves the +rest of the `storage` block when it writes back. + +`publish-raw` applies the same identity check before treating a recorded block +as history. A local file the recorded object does not hold is reported as +`recorded_r2_identity_mismatch` and nothing is uploaded: registering a revision +is a fetch-time decision, not a publish-time rewrite. + ## Relational Registry Contract The hosted `chronicle` schema should be the lookup surface for Chronicle, not the place @@ -121,7 +183,10 @@ The intended flow is: 1. Register raw source artifacts with `uv run chronicle fetch-artifact`, which writes local bytes, records checksums in `manifest.yaml`, and can upload the - exact bytes to the raw archive. Existing manifest-declared artifacts can be + exact bytes to the raw archive. Re-fetching an entry whose bytes the + publisher has changed is refused unless the revision is registered with + `--record-revision`; see [Publisher Revisions](#publisher-revisions). + Existing manifest-declared artifacts can be checksum-validated, uploaded, and linked with `uv run chronicle publish-raw`. Production package specs may omit raw bytes from Git as long as the manifest keeps `source_url` and SHA-256 metadata; builds can fill @@ -217,8 +282,10 @@ exception to the last step. Archived witness records pin raw R2 URLs by hash, so deleted, and manifests keep the `storage.r2` URIs they already recorded as historical truth. A backfill copies bytes into the new bucket; it never rewrites where those bytes were first published. `publish-raw` and `fetch-artifact` -enforce that: both refuse to restate a recorded `storage.r2` block under a -different bucket. +enforce that: a recorded block that addresses the bytes in hand is preserved +whichever bucket is configured, and `publish-raw` refuses to restate it under a +different one. Bytes that the recorded object does not hold are not that +object's history at all; see [Publisher Revisions](#publisher-revisions). The cutover therefore has one irreversible-looking step that is in fact additive (creating and filling the new buckets), one cheap reversible step (flipping the From 479aefbc32ae0748e45d8c2907948f36912a5f3e Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 2 Sep 2026 12:32:09 -0400 Subject: [PATCH 09/22] Record the gate round-1 fixes in PROGRESS.md Co-Authored-By: Claude Fable 5.1 --- PROGRESS.md | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/PROGRESS.md b/PROGRESS.md index da5d13e5..27b1babe 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -48,6 +48,42 @@ Lane C5's handoff notes previously lived here; its durable record is `README.md` follow. Verified 186 distinct `ledger-raw` objects across 154 tracked manifest files, every key content-addressed by sha256. +## Review fixes (gate round 1) + +The Fable+Sol gate requested changes; both findings are applied on this branch. + +- **[high] `fetch-artifact` could attach a recorded R2 URI to new bytes.** The + preserve rule keyed on the bucket, so a repeated fetch that did not re-upload + into the same bucket kept the recorded `storage.r2` block while rewriting the + entry's `sha256`/`size_bytes`. Reproduced against this branch's parent: the + entry ends up declaring `109dcf49…` with a key addressed by `c63744a4…`, both + when the fetch only registers the bytes and when the bucket default has moved. + The rule now keys on identity — the recorded key's `{sha256}/{filename}` tail + against the fetched bytes. Identical preserves the block exactly; different + raises `SourceArtifactRevisionError` before the cached artifact or its + manifest entry is touched, naming recorded and fetched `sha256`/`size_bytes` + and the ADR rule that same vintage plus new bytes is a new release revision. + `--record-revision` opts in: the new bytes get their own content-addressed key + under the configured bucket, never the old key, and the superseded block moves + to `storage.previous_r2`. `publish-raw` applies the same check before treating + a recorded block as history (`recorded_r2_identity_mismatch`, nothing + uploaded). +- **[low] Env isolation was scoped to one module.** The autouse fixture moved to + `tests/conftest.py` and now clears all three prefixes for every test. + `db.supabase_client` resolves `LEDGER_SCHEMA` at import — during collection, + before any fixture — so `tests/test_chronicle_namespace.py` re-imports it + under the cleared environment instead of asserting the constant it bound at + collection time. + +`storage.previous_r2` is a sibling key, chosen because every reader +(`inventory-artifacts`, `publish-raw`, `source_package._artifact_content`, the +suite's raw-R2-link acceptance check) reads `storage.r2` alone, and +`publish-raw` already spreads the rest of the `storage` block when it writes +back, so a revision survives publication untouched. All 180 tracked manifest +entries that carry a `storage.r2` block are content-addressed and agree with +their declared `sha256` and `filename`, so the identity check never fires on +tracked data. + ## Verification - `uv run pytest -q`: green. @@ -56,6 +92,10 @@ Lane C5's handoff notes previously lived here; its durable record is files are unformatted on `main` already and are byte-identical here; CI runs `ruff check` only, so they are pre-existing and out of scope. - CI's db CLI gate (`chronicle init` / `load all` / `stats`): passes. +- `CHRONICLE_R2_RAW_BUCKET=zzz CHRONICLE_SCHEMA=zzz uv run pytest -q`: green. + Before the shared fixture it failed five tests — four bucket-default + assertions in `tests/test_chronicle_artifacts.py` and the collection-time + schema constant in `tests/test_chronicle_namespace.py`. ## Next From 37ee3fafe9382b239172fa9051d918d58a2f0a78 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 2 Sep 2026 12:33:18 -0400 Subject: [PATCH 10/22] Read the recorded object's identity from its URI too A storage.r2 block written by Chronicle always carries both key and uri, but a hand-written one need not. The identity read now falls back to the uri, whose last two segments are the same {sha256}/{filename}, so a block that records only where the object lives is still recognized as history for the bytes it holds instead of being refused as a revision. Also covers registering a revision without an upload: the entry is left with no storage.r2 at all rather than a pointer to bytes R2 does not hold, the superseded object stays in storage.previous_r2, and publish-raw completes the registration. Co-Authored-By: Claude Fable 5.1 --- chronicle/artifacts.py | 22 +++++----- tests/test_chronicle_artifacts.py | 69 +++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 10 deletions(-) diff --git a/chronicle/artifacts.py b/chronicle/artifacts.py index edb9752f..ff9b2b48 100644 --- a/chronicle/artifacts.py +++ b/chronicle/artifacts.py @@ -1117,17 +1117,19 @@ def _recorded_r2(spec: Any) -> dict[str, Any]: return recorded if isinstance(recorded, dict) else {} -def _r2_key_identity(key: Any) -> tuple[str, str]: - """Return the ``(sha256, filename)`` a raw R2 key is addressed by. +def _r2_key_identity(recorded_r2: dict[str, Any]) -> tuple[str, str]: + """Return the ``(sha256, filename)`` a recorded R2 object is addressed by. Raw keys are ``{prefix}/{source_id}/{package_id}/{year}/{sha256}/{filename}`` (see :func:`build_r2_key`), so the last two segments say which bytes the - object holds. A key in any other shape yields empty strings and therefore - never matches fetched bytes. + object holds; the URI ends in the same two segments and stands in for a + block that records only that. A locator in any other shape yields empty + strings and therefore never matches fetched bytes. """ - if not isinstance(key, str): + locator = recorded_r2.get("key") or recorded_r2.get("uri") + if not isinstance(locator, str): return ("", "") - parts = [part for part in key.split("/") if part] + parts = [part for part in locator.split("/") if part] if len(parts) < 2: return ("", "") return (parts[-2], parts[-1]) @@ -1140,7 +1142,7 @@ def _r2_holds_these_bytes( filename: str, ) -> bool: """Whether a recorded ``storage.r2`` block addresses exactly these bytes.""" - recorded_sha256, recorded_filename = _r2_key_identity(recorded_r2.get("key")) + recorded_sha256, recorded_filename = _r2_key_identity(recorded_r2) return bool(recorded_sha256) and (recorded_sha256, recorded_filename) == ( sha256, Path(filename).name, @@ -1159,7 +1161,7 @@ def _revision_error_message( r2_bucket: str, ) -> str: """Explain a refused fetch: recorded identity, fetched identity, next step.""" - recorded_sha256, recorded_filename = _r2_key_identity(recorded_r2.get("key")) + recorded_sha256, recorded_filename = _r2_key_identity(recorded_r2) declared_sha256 = recorded_spec.get("sha256") recorded_size = ( recorded_spec.get("size_bytes") if declared_sha256 == recorded_sha256 else None @@ -1241,7 +1243,7 @@ def _superseding_storage( recorded_r2 = _recorded_r2(recorded_spec) if recorded_r2: entry = dict(recorded_r2) - recorded_sha256, _recorded_filename = _r2_key_identity(recorded_r2.get("key")) + recorded_sha256, _recorded_filename = _r2_key_identity(recorded_r2) if recorded_sha256: entry["sha256"] = recorded_sha256 if recorded_spec.get("sha256") == recorded_sha256: @@ -1423,7 +1425,7 @@ def refuse(reason: str | None = None) -> tuple[RawArtifactPublishEntry, None]: # key that misdescribes its content or restate a URI that belongs to # the superseded bytes. Registering a publisher revision is # `fetch-artifact --record-revision`, not a publish-time rewrite. - recorded_sha256, recorded_filename = _r2_key_identity(recorded_r2.get("key")) + recorded_sha256, recorded_filename = _r2_key_identity(recorded_r2) return refuse( "recorded_r2_identity_mismatch:" f"recorded_sha256={recorded_sha256 or 'unknown'}:" diff --git a/tests/test_chronicle_artifacts.py b/tests/test_chronicle_artifacts.py index 6a3f7e5b..7487f8b4 100644 --- a/tests/test_chronicle_artifacts.py +++ b/tests/test_chronicle_artifacts.py @@ -1043,3 +1043,72 @@ def test_fetch_artifact_cli_refuses_a_revision_then_records_it_on_request( assert recorded["r2_location"]["key"].endswith( f"/{recorded['sha256']}/{REPUBLISHED_FILENAME}" ) + + +def test_record_revision_without_an_upload_records_no_current_object( + tmp_path, monkeypatch +): + """An offline revision keeps history without claiming the new bytes exist. + + Registering a revision without ``--upload-r2`` leaves the entry with no + ``storage.r2`` at all rather than a pointer to bytes R2 does not hold. The + superseded object stays addressable, and a later publish-raw completes the + registration. + """ + output_dir = tmp_path / "db" / "data" / "irs_soi" / "soi-table-5" + log = tmp_path / "wrangler.log" + wrangler = _wrangler_stub(tmp_path, log) + manifest_path = output_dir / "manifest.yaml" + _serve(monkeypatch, FIRST_PUBLICATION) + first_report = _fetch_republished(output_dir, wrangler) + + _serve(monkeypatch, SECOND_PUBLICATION) + _fetch_republished(output_dir, wrangler, upload_r2=False, record_revision=True) + registered = yaml.safe_load(manifest_path.read_text())["files"][2022] + + assert "r2" not in registered["storage"] + assert [entry["sha256"] for entry in registered["storage"]["previous_r2"]] == [ + first_report.sha256 + ] + + report = publish_source_artifacts(output_dir, wrangler_command=str(wrangler)) + published = yaml.safe_load(manifest_path.read_text())["files"][2022] + revised_sha256 = hashlib.sha256(SECOND_PUBLICATION).hexdigest() + + assert report.valid + assert published["storage"]["r2"]["key"].endswith( + f"/{revised_sha256}/{REPUBLISHED_FILENAME}" + ) + assert [entry["sha256"] for entry in published["storage"]["previous_r2"]] == [ + first_report.sha256 + ] + + +def test_a_recorded_block_that_only_carries_a_uri_is_still_recognized( + tmp_path, monkeypatch +): + """Identity reads the URI when a hand-written block records no key.""" + output_dir = tmp_path / "db" / "data" / "irs_soi" / "soi-table-5" + wrangler = _wrangler_stub(tmp_path, tmp_path / "wrangler.log") + manifest_path = output_dir / "manifest.yaml" + _serve(monkeypatch, FIRST_PUBLICATION) + _fetch_republished(output_dir, wrangler) + manifest = yaml.safe_load(manifest_path.read_text()) + recorded = manifest["files"][2022]["storage"]["r2"] + manifest["files"][2022]["storage"]["r2"] = { + "provider": recorded["provider"], + "bucket": recorded["bucket"], + "uri": recorded["uri"], + } + manifest_path.write_text(yaml.safe_dump(manifest, sort_keys=False)) + uri_only = manifest["files"][2022]["storage"]["r2"] + + report = _fetch_republished(output_dir, wrangler) + preserved = yaml.safe_load(manifest_path.read_text())["files"][2022] + + assert report.valid + assert preserved["storage"]["r2"] == uri_only + + _serve(monkeypatch, SECOND_PUBLICATION) + with pytest.raises(SourceArtifactRevisionError): + _fetch_republished(output_dir, wrangler) From 14077a7a17acfc98c286e317c35d085f0ffe4767 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 2 Sep 2026 12:35:35 -0400 Subject: [PATCH 11/22] Pin the preserved block's field order too Dict equality would pass on a block that was rebuilt with the same values in a different order, which is not what "preserved" means for a manifest that gets dumped back to YAML. Co-Authored-By: Claude Fable 5.1 --- tests/test_chronicle_artifacts.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_chronicle_artifacts.py b/tests/test_chronicle_artifacts.py index 7487f8b4..f5715055 100644 --- a/tests/test_chronicle_artifacts.py +++ b/tests/test_chronicle_artifacts.py @@ -826,6 +826,8 @@ def test_repeated_fetch_of_identical_bytes_preserves_the_recorded_block( assert report.r2_location.bucket == "chronicle-raw" assert "chronicle-raw" in log.read_text() assert second["storage"] == first["storage"] + # Field order too, so the block is byte-for-byte identical once dumped. + assert list(second["storage"]["r2"].items()) == list(first["storage"]["r2"].items()) assert second["storage"]["r2"]["bucket"] == "ledger-raw" assert "previous_r2" not in second["storage"] assert second["sha256"] == first["sha256"] From 34d1d0fbe4bc854450b2e2b424f0e1e3c62a44a1 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 2 Sep 2026 12:37:48 -0400 Subject: [PATCH 12/22] Say what the reproduction actually ran The hashes named the fix lane's two-body fixture, not IRS bytes. Co-Authored-By: Claude Fable 5.1 --- PROGRESS.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/PROGRESS.md b/PROGRESS.md index 27b1babe..38e669e2 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -55,9 +55,11 @@ The Fable+Sol gate requested changes; both findings are applied on this branch. - **[high] `fetch-artifact` could attach a recorded R2 URI to new bytes.** The preserve rule keyed on the bucket, so a repeated fetch that did not re-upload into the same bucket kept the recorded `storage.r2` block while rewriting the - entry's `sha256`/`size_bytes`. Reproduced against this branch's parent: the - entry ends up declaring `109dcf49…` with a key addressed by `c63744a4…`, both - when the fetch only registers the bytes and when the bucket default has moved. + entry's `sha256`/`size_bytes`. Reproduced against this branch's parent by + serving two different bodies from one URL: the entry ends up declaring the + fetched bytes' `sha256` under a key addressed by the superseded bytes' one, + both when the fetch only registers the bytes and when the bucket default has + moved. The rule now keys on identity — the recorded key's `{sha256}/{filename}` tail against the fetched bytes. Identical preserves the block exactly; different raises `SourceArtifactRevisionError` before the cached artifact or its From cbeca55469eee626ac71d51bb56a0eee13cbb5a3 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 2 Sep 2026 13:09:03 -0400 Subject: [PATCH 13/22] Record the round-2 gate findings and the corpus scan The seven findings, in the order the fixes depend on each other, plus what a scan of every tracked manifest says about how strict the locator check can be: 187 entries, all content-addressed, no contradictory field. Co-Authored-By: Claude Fable 5.1 --- PROGRESS.md | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/PROGRESS.md b/PROGRESS.md index 38e669e2..e51ea2ca 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -86,6 +86,39 @@ entries that carry a `storage.r2` block are content-addressed and agree with their declared `sha256` and `filename`, so the identity check never fires on tracked data. +## Review fixes (gate round 2) + +The second Fable+Sol gate requested changes again. Seven findings, each fixed +with a regression test on this branch. Plan, in dependency order: + +1. **[high] `CHRONICLE_SCHEMA` does not reach the Supabase mirror writer.** + `chronicle/harness.py` and `chronicle/mirror.py` default the schema to the + literal `"ledger"`; only `db.supabase_client` reads the renamed variable. + Resolve through the shared helper whenever no explicit `--schema` is given. +2. **[high] The derived-fact boundary check is not rename-safe.** + `chronicle/consumer_contract.py` matches the `.ledger_derived` suffix only. +3. **[high] `fetch-artifact` cannot address a package's non-default manifest.** + Seven tracked packages keep a `manifest_*_source_package.yaml`; three + directories keep two. A fetch into one of them writes a third manifest and + never sees the recorded block. +4. **[high] Revision protection vanishes when the entry has no `storage.r2`.** +5. **[medium] Recorded-R2 locator fields must be cross-checked**, not read as + key-or-URI, before a block is preserved or published. +6. **[medium] `_read_manifest` must reject a malformed document**, not treat a + non-mapping YAML payload as an absent manifest. +7. **[low] Schema resolution must be lazy** so no legacy variable is read at + collection, before the autouse isolation fixture runs. + +## State (round 2) + +- Read both gate rounds on PolicyEngine/chronicle#226 and the code each finding + names. +- Scanned all 154 tracked manifest files (187 `files` entries, every one + carrying `storage.r2`): every recorded block supplies provider, bucket, key + and uri; every key is content-addressed; every declared `sha256`/`filename` + agrees with its key tail; no `uri` contradicts its `key`. Strict locator + validation therefore refuses nothing that is tracked today. + ## Verification - `uv run pytest -q`: green. From eedff5854ca2be4e4ba43b7df249c87686d53366 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 2 Sep 2026 13:09:55 -0400 Subject: [PATCH 14/22] Reject the chronicle spelling of the derived-row marker The consumer boundary matched the `.ledger_derived` suffix literally, so a producer that renamed its derived rows with everything else would have walked a downstream target fact straight through the guard. Match the whole final dot-segment against both spellings instead. Without the fix the two new cases fail: the chronicle-spelled record id validates clean. Co-Authored-By: Claude Fable 5.1 --- chronicle/consumer_contract.py | 16 ++++++- tests/test_chronicle_consumer_contract.py | 52 +++++++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/chronicle/consumer_contract.py b/chronicle/consumer_contract.py index 31f90912..01fa4eb5 100644 --- a/chronicle/consumer_contract.py +++ b/chronicle/consumer_contract.py @@ -398,6 +398,20 @@ def _r2_uri_parts(uri: str) -> tuple[str, str]: return bucket, key +# The marker a downstream target row carries in its source_record_id. It moves +# with the rename window: producers write `ledger_derived` today and +# `chronicle_derived` once they migrate (PolicyEngine/chronicle#143, mechanism +# 3), so the boundary has to reject both spellings identically or the guard +# stops firing the moment a producer renames. +DERIVED_SOURCE_RECORD_SUFFIXES = frozenset({"ledger_derived", "chronicle_derived"}) + + +def _is_derived_source_record_id(source_record_id: str) -> bool: + """Whether a source_record_id marks a downstream derived target row.""" + _, separator, suffix = source_record_id.rpartition(".") + return bool(separator) and suffix in DERIVED_SOURCE_RECORD_SUFFIXES + + def _points_at_derived(bucket: str, key: str) -> bool: """Whether an R2 bucket/key pair addresses derived build output. @@ -442,7 +456,7 @@ def _derived_source_provenance_issue(fact: AggregateFact) -> str | None: "Chronicle consumer facts must point at raw source artifacts, not " "derived build artifacts." ) - if source_record_id.endswith(".ledger_derived"): + if _is_derived_source_record_id(source_record_id): return ( "Chronicle source_record_id must identify a publisher-backed row, not " "a downstream derived target row." diff --git a/tests/test_chronicle_consumer_contract.py b/tests/test_chronicle_consumer_contract.py index a34f3bd4..8e667d1c 100644 --- a/tests/test_chronicle_consumer_contract.py +++ b/tests/test_chronicle_consumer_contract.py @@ -750,6 +750,18 @@ def test_export_consumer_facts_cli_rejects_contract_invalid_facts(tmp_path, caps }, "publisher.raw.fact", ), + # The derived-row marker renames with everything else, so the guard has + # to reject the chronicle spelling the same way it rejects the ledger + # one (PolicyEngine/chronicle#143, mechanism 3). + ( + { + "source_name": "irs_soi", + "source_file": "publisher.xlsx", + "raw_r2_bucket": "ledger-raw", + "raw_r2_uri": "r2://ledger-raw/raw/source/publisher.xlsx", + }, + "irs_soi.ty2024.table.us.taxable_interest_amount.chronicle_derived", + ), ], ) def test_consumer_contract_rejects_downstream_derived_target_facts( @@ -769,6 +781,46 @@ def test_consumer_contract_rejects_downstream_derived_target_facts( assert "derived_fact_provenance" in {error.code for error in report.errors} +def test_derived_record_marker_is_rejected_in_either_spelling(): + """Both rename-window spellings produce the identical boundary error.""" + fact = _soi_agi_fact() + base = "irs_soi.ty2024.table.us.taxable_interest_amount" + + reports = { + suffix: validate_consumer_fact_contract( + [replace(fact, source_record_id=f"{base}.{suffix}")] + ) + for suffix in ("ledger_derived", "chronicle_derived") + } + + ledger_errors = [ + (error.code, error.message) for error in reports["ledger_derived"].errors + ] + chronicle_errors = [ + (error.code, error.message) for error in reports["chronicle_derived"].errors + ] + assert ledger_errors == chronicle_errors + assert "derived_fact_provenance" in {code for code, _ in ledger_errors} + + +@pytest.mark.parametrize( + "source_record_id", + [ + # A publisher-backed row that merely contains the marker as a word, or + # carries it without the separating dot, is not a derived target row. + "irs_soi.ty2024.table.us.chronicle_derived_totals", + "irs_soi.ty2024.table.us.ledger_derived_totals", + "chronicle_derived", + ], +) +def test_derived_record_marker_matches_the_whole_final_segment(source_record_id): + fact = replace(_soi_agi_fact(), source_record_id=source_record_id) + + report = validate_consumer_fact_contract([fact]) + + assert report.valid + + def test_export_consumer_facts_cli_preserves_decimal_values(tmp_path, capsys): input_path = tmp_path / "facts.jsonl" output_path = tmp_path / "consumer_facts.jsonl" From fb0bcac0352423f49df5f686bec8ccf3812407bd Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 2 Sep 2026 13:15:03 -0400 Subject: [PATCH 15/22] Let CHRONICLE_SCHEMA configure the mirror writer, at call time The hosted mirror loader is the primary writer into the schema, and both it and its CLI defaulted to the literal "ledger": setting CHRONICLE_SCHEMA to rehearse a cutover moved the read-side client and left the writer pointed at production. The default now resolves through the shared helper -- one home for the CHRONICLE_SCHEMA -> POLICYENGINE_LEDGER_SCHEMA -> LEDGER_SCHEMA -> "ledger" ladder -- whenever no explicit --schema is supplied. Defaults are unchanged. Resolution is a function rather than a module constant, so db.supabase_client no longer binds the schema while being imported. That import happens at collection, before any fixture, which is why the namespace test needed a reload dance to assert the defaults; it now just calls the resolver. A pytest_configure hook clears the rename window before collection too, so no module can read (or warn from) an operator's shell on the way in. Six tests fail against the previous code: the loader and its CLI ignore the variable, and the namespace assertions see the collection-time constant. Co-Authored-By: Claude Fable 5.1 --- chronicle/env.py | 23 ++++++ chronicle/harness.py | 16 +++- chronicle/mirror.py | 13 +++- db/supabase_client.py | 43 ++++++++--- tests/conftest.py | 17 ++++ tests/test_chronicle_env.py | 45 ++++++++--- tests/test_chronicle_mirror.py | 124 ++++++++++++++++++++++++++++++ tests/test_chronicle_namespace.py | 34 ++++---- 8 files changed, 276 insertions(+), 39 deletions(-) diff --git a/chronicle/env.py b/chronicle/env.py index 3a2f7af6..0d4bbbad 100644 --- a/chronicle/env.py +++ b/chronicle/env.py @@ -19,8 +19,11 @@ __all__ = [ "CHRONICLE_ENV_PREFIX", + "CHRONICLE_SCHEMA_ENV", "ChronicleEnvDeprecationWarning", + "DEFAULT_CHRONICLE_SCHEMA", "LEGACY_ENV_PREFIXES", + "default_chronicle_schema", "env_flag", "env_names", "env_value", @@ -34,6 +37,14 @@ TRUTHY_ENV_VALUES = frozenset({"1", "true", "yes", "on"}) +CHRONICLE_SCHEMA_ENV = "CHRONICLE_SCHEMA" + +# The hosted Postgres schema is still named "ledger". Renaming the schema value +# is a later slice of PolicyEngine/chronicle#143, coordinated with the CI +# writers that already target it; only the variable that overrides the name has +# moved to the chronicle prefix. +DEFAULT_CHRONICLE_SCHEMA = "ledger" + class ChronicleEnvDeprecationWarning(FutureWarning): """A ledger-era environment variable supplied a Chronicle setting. @@ -126,6 +137,18 @@ def env_value(*names: str, default: _Default = None) -> str | _Default: return default if value is None else value +def default_chronicle_schema() -> str: + """Resolve the Chronicle schema: ``$CHRONICLE_SCHEMA``, else the default. + + Every reader of the setting goes through this function so the lookup ladder + and the default have one home. It resolves at call time rather than at + import: a module-level constant binds whatever the shell held when the + module was first imported, which for a library means an arbitrary moment + the caller cannot control, and for the test suite means collection. + """ + return env_value(CHRONICLE_SCHEMA_ENV, default=DEFAULT_CHRONICLE_SCHEMA) + + def env_flag(*names: str) -> bool: """Return whether the first set value across ``names`` reads as true. diff --git a/chronicle/harness.py b/chronicle/harness.py index 528d4f77..bf3d5ebe 100644 --- a/chronicle/harness.py +++ b/chronicle/harness.py @@ -37,6 +37,7 @@ ) from chronicle.core import AggregateFact, ValidationReport, validate_facts from chronicle.database import ChronicleDbBuildReport, build_chronicle_db +from chronicle.env import DEFAULT_CHRONICLE_SCHEMA from chronicle.mirror import ( ChronicleMirrorExportReport, SupabaseMirrorLoadReport, @@ -452,12 +453,16 @@ def export_chronicle_db_table_files( def load_supabase_mirror_files( input_dir: str | Path, *, - schema: str = "ledger", + schema: str | None = None, batch_size: int = 500, dry_run: bool = False, build_artifacts_path: str | Path | None = None, ) -> SupabaseMirrorLoadReport: - """Load exported Chronicle JSONL mirror files into Supabase/Postgres.""" + """Load exported Chronicle JSONL mirror files into Supabase/Postgres. + + ``schema`` of None resolves to ``$CHRONICLE_SCHEMA``, else the default + schema, so the hosted mirror writer answers to the renamed variable. + """ table_paths = ( {"build_artifacts": Path(build_artifacts_path)} if build_artifacts_path is not None @@ -1111,8 +1116,11 @@ def main(argv: list[str] | None = None) -> int: ) mirror_load_parser.add_argument( "--schema", - default="ledger", - help="Supabase/Postgres schema to load into.", + default=None, + help=( + "Supabase/Postgres schema to load into. Defaults to " + f"$CHRONICLE_SCHEMA, else {DEFAULT_CHRONICLE_SCHEMA}." + ), ) mirror_load_parser.add_argument( "--batch-size", diff --git a/chronicle/mirror.py b/chronicle/mirror.py index 7a3ccaf3..f9c95dc2 100644 --- a/chronicle/mirror.py +++ b/chronicle/mirror.py @@ -11,6 +11,8 @@ from pathlib import Path from typing import Any +from chronicle.env import default_chronicle_schema + LEDGER_MIRROR_TABLES = ( "ledger_builds", "build_artifacts", @@ -200,15 +202,22 @@ def export_chronicle_db_tables( def load_supabase_mirror( input_dir: str | Path, *, - schema: str = "ledger", + schema: str | None = None, batch_size: int = 500, dry_run: bool = False, table_paths: dict[str, str | Path] | None = None, client: Any | None = None, ) -> SupabaseMirrorLoadReport: - """Load exported Chronicle JSONL mirror files into Supabase/Postgres.""" + """Load exported Chronicle JSONL mirror files into Supabase/Postgres. + + ``schema`` defaults to :func:`chronicle.env.default_chronicle_schema`, so + the writer that owns the hosted mirror answers to ``CHRONICLE_SCHEMA`` (and + the ledger-era names behind it) exactly like every other reader of the + setting. The resolved name is reported back in the load report. + """ if batch_size < 1: raise ValueError("batch_size must be at least 1.") + schema = schema or default_chronicle_schema() input_path = Path(input_dir) tables: list[SupabaseTableLoad] = [] errors: list[str] = [] diff --git a/db/supabase_client.py b/db/supabase_client.py index 50ce4f30..190d8b7e 100644 --- a/db/supabase_client.py +++ b/db/supabase_client.py @@ -15,14 +15,33 @@ from supabase import create_client, Client -from chronicle.env import env_value +from chronicle.env import default_chronicle_schema, env_value -# The hosted Postgres schema is still named "ledger"; only the environment -# variable that overrides it has moved to the chronicle prefix. Renaming the -# schema itself is a later slice of PolicyEngine/chronicle#143, coordinated -# with the CI writers that already target the ledger schema. -LEDGER_SCHEMA = env_value("CHRONICLE_SCHEMA") or "ledger" -TARGETS_SCHEMA = env_value("POLICYENGINE_TARGETS_SCHEMA") or "targets" +TARGETS_SCHEMA_ENV = "POLICYENGINE_TARGETS_SCHEMA" +DEFAULT_TARGETS_SCHEMA = "targets" + + +def chronicle_schema() -> str: + """Resolve the hosted Chronicle schema for a query. + + Read at call time, not bound at import: an import-time constant fixes the + schema at whatever the environment held when this module was first + imported, which the caller does not control (in the test suite that moment + is collection, before any fixture has isolated the environment). The + hosted schema is still named "ledger" -- only the variable that overrides + it has moved to the chronicle prefix, and renaming the schema value is a + later slice of PolicyEngine/chronicle#143. + """ + return default_chronicle_schema() + + +def targets_schema() -> str: + """Resolve the hosted targets schema. Read at call time, as above. + + ``POLICYENGINE_TARGETS_SCHEMA`` names a surface outside the ledger rename + window, so it is read literally. + """ + return env_value(TARGETS_SCHEMA_ENV, default=DEFAULT_TARGETS_SCHEMA) @dataclass @@ -109,7 +128,7 @@ def query_sources( List of source records """ client = get_supabase_client() - query = _table(client, LEDGER_SCHEMA, "sources").select("*") + query = _table(client, chronicle_schema(), "sources").select("*") if jurisdiction: query = query.eq("jurisdiction", jurisdiction) @@ -138,7 +157,9 @@ def query_strata( List of strata records with nested constraints """ client = get_supabase_client() - query = _table(client, TARGETS_SCHEMA, "strata").select("*, stratum_constraints(*)") + query = _table(client, targets_schema(), "strata").select( + "*, stratum_constraints(*)" + ) if jurisdiction: query = query.eq("jurisdiction", jurisdiction) @@ -167,7 +188,7 @@ def query_targets( """ client = get_supabase_client() # Nested join: strata with their stratum_constraints - query = _table(client, TARGETS_SCHEMA, "targets").select( + query = _table(client, targets_schema(), "targets").select( "*, strata(*, stratum_constraints(*)), sources(*)" ) @@ -214,7 +235,7 @@ def insert_targets_batch( for i in range(0, len(targets), chunk_size): chunk = targets[i : i + chunk_size] - _table(client, TARGETS_SCHEMA, "targets").insert(chunk).execute() + _table(client, targets_schema(), "targets").insert(chunk).execute() total += len(chunk) return total diff --git a/tests/conftest.py b/tests/conftest.py index 378fa0b3..3d315441 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -22,6 +22,23 @@ RENAME_WINDOW_PREFIXES = (CHRONICLE_ENV_PREFIX, *LEGACY_ENV_PREFIXES) +def pytest_configure(config): + """Strip the rename window before collection imports a single module. + + The autouse fixture below runs per test, which is too late for anything a + module does while being imported. Collection happens after this hook, so + clearing here means no module can read an operator's ``LEDGER_SCHEMA`` + (warning as it goes, or freezing it into a constant) before a fixture has + had the chance to isolate it. Modules should resolve settings at call time + rather than at import; this hook makes that property testable instead of + depending on which shell ran pytest. + """ + for name in list(os.environ): + if name.startswith(RENAME_WINDOW_PREFIXES): + del os.environ[name] + reset_env_deprecation_state() + + @pytest.fixture(autouse=True) def isolated_rename_window_env(monkeypatch): """Run every test with no rename-window variable inherited from the shell.""" diff --git a/tests/test_chronicle_env.py b/tests/test_chronicle_env.py index 65029d14..202d86f5 100644 --- a/tests/test_chronicle_env.py +++ b/tests/test_chronicle_env.py @@ -24,7 +24,9 @@ from chronicle.env import ( CHRONICLE_ENV_PREFIX, ChronicleEnvDeprecationWarning, + DEFAULT_CHRONICLE_SCHEMA, LEGACY_ENV_PREFIXES, + default_chronicle_schema, env_flag, env_names, env_value, @@ -252,17 +254,18 @@ def test_db_cli_parser_builds_with_the_env_backed_defaults(monkeypatch, capsys): assert "Manage Chronicle target input data" in capsys.readouterr().out -@pytest.mark.parametrize("name", ["CHRONICLE_SCHEMA", "POLICYENGINE_LEDGER_SCHEMA"]) -def test_supabase_schema_honors_both_names(monkeypatch, name): +@pytest.mark.parametrize( + "name", + ["CHRONICLE_SCHEMA", "POLICYENGINE_LEDGER_SCHEMA", "LEDGER_SCHEMA"], +) +def test_supabase_schema_honors_every_name_in_the_window(monkeypatch, name): + """Set after import and still honored: the schema is read at call time.""" import db.supabase_client monkeypatch.setenv(name, "chronicle_probe") - try: - reloaded = importlib.reload(db.supabase_client) - assert reloaded.LEDGER_SCHEMA == "chronicle_probe" - finally: - monkeypatch.delenv(name, raising=False) - importlib.reload(db.supabase_client) + + assert db.supabase_client.chronicle_schema() == "chronicle_probe" + assert default_chronicle_schema() == "chronicle_probe" def test_supabase_schema_default_is_unchanged(): @@ -270,7 +273,31 @@ def test_supabase_schema_default_is_unchanged(): # The hosted schema name itself is out of this slice; only the variable # that overrides it moved. - assert db.supabase_client.LEDGER_SCHEMA == "ledger" + assert DEFAULT_CHRONICLE_SCHEMA == "ledger" + assert default_chronicle_schema() == "ledger" + assert db.supabase_client.chronicle_schema() == "ledger" + assert db.supabase_client.targets_schema() == "targets" + + +def test_supabase_schema_is_not_bound_at_import(monkeypatch): + """No module-level constant may freeze the schema at import time. + + A reload under a set variable is the pre-fix behavior this guards against: + it proves nothing about a module that resolved the value once, at + collection, and answers with the stale constant forever after. + """ + import db.supabase_client + + assert not [ + name + for name, value in vars(db.supabase_client).items() + if name.isupper() and value == "ledger" + ] + + monkeypatch.setenv("CHRONICLE_SCHEMA", "chronicle_probe") + unreloaded = importlib.import_module("db.supabase_client") + + assert unreloaded.chronicle_schema() == "chronicle_probe" # --------------------------------------------------------------------------- diff --git a/tests/test_chronicle_mirror.py b/tests/test_chronicle_mirror.py index c06a33bd..6b2e694d 100644 --- a/tests/test_chronicle_mirror.py +++ b/tests/test_chronicle_mirror.py @@ -4,6 +4,9 @@ import json +import pytest + +from chronicle.env import ChronicleEnvDeprecationWarning from chronicle.harness import main as harness_main from chronicle.mirror import ( LEDGER_MIRROR_TABLES, @@ -189,6 +192,127 @@ def test_load_supabase_mirror_cli_dry_run(tmp_path, capsys): assert payload["table_count"] == len(LEDGER_MIRROR_TABLES) +# --------------------------------------------------------------------------- +# Schema configuration +# +# The mirror loader is the primary writer into the hosted schema, so it is the +# call site CHRONICLE_SCHEMA has to reach (PolicyEngine/chronicle#143, +# mechanism 3). It defaulted to the literal "ledger" while only the read-side +# client honored the renamed variable, which would have sent a rehearsal load +# into production the moment an operator set it. +# --------------------------------------------------------------------------- + + +def _empty_mirror(tmp_path): + mirror_dir = tmp_path / "mirror" + mirror_dir.mkdir() + for table in LEDGER_MIRROR_TABLES: + (mirror_dir / f"{table}.jsonl").write_text("") + return mirror_dir + + +def _one_build_artifact(tmp_path): + path = tmp_path / "build_artifacts.jsonl" + path.write_text( + json.dumps( + { + "build_artifact_key": "ledger.build_artifact.v1:test", + "build_id": "ledger.build.v1:test", + "artifact_kind": "json", + "artifact_name": "reports/build_summary.json", + "sha256": "abc", + "size_bytes": 3, + "r2_bucket": "ledger-derived", + "r2_key": "derived/test", + "r2_uri": "r2://ledger-derived/derived/test", + }, + sort_keys=True, + ) + + "\n" + ) + return path + + +def _load_into_fake_client(tmp_path, **kwargs): + client = _FakeSupabaseClient() + report = load_supabase_mirror( + _empty_mirror(tmp_path), + table_paths={"build_artifacts": _one_build_artifact(tmp_path)}, + client=client, + **kwargs, + ) + return report, client + + +def test_load_supabase_mirror_defaults_to_the_ledger_schema(tmp_path): + report, client = _load_into_fake_client(tmp_path) + + assert report.schema == "ledger" + assert [upsert[0] for upsert in client.upserts] == ["ledger"] + + +def test_load_supabase_mirror_writes_to_the_chronicle_schema(tmp_path, monkeypatch): + """The renamed variable configures the writer, not just the reader.""" + monkeypatch.setenv("CHRONICLE_SCHEMA", "chronicle_probe") + + report, client = _load_into_fake_client(tmp_path) + + assert report.schema == "chronicle_probe" + assert [upsert[0] for upsert in client.upserts] == ["chronicle_probe"] + + +@pytest.mark.parametrize("name", ["POLICYENGINE_LEDGER_SCHEMA", "LEDGER_SCHEMA"]) +def test_load_supabase_mirror_honors_a_ledger_era_schema_name( + tmp_path, monkeypatch, name +): + monkeypatch.setenv(name, "legacy_probe") + + with pytest.warns(ChronicleEnvDeprecationWarning): + report, client = _load_into_fake_client(tmp_path) + + assert report.schema == "legacy_probe" + assert [upsert[0] for upsert in client.upserts] == ["legacy_probe"] + + +def test_an_explicit_schema_still_wins_over_the_environment(tmp_path, monkeypatch): + monkeypatch.setenv("CHRONICLE_SCHEMA", "chronicle_probe") + + report, client = _load_into_fake_client(tmp_path, schema="explicit_probe") + + assert report.schema == "explicit_probe" + assert [upsert[0] for upsert in client.upserts] == ["explicit_probe"] + + +def test_load_supabase_mirror_cli_writes_to_the_configured_schema( + tmp_path, monkeypatch, capsys +): + """The CLI resolves the same way when no --schema is supplied.""" + client = _FakeSupabaseClient() + monkeypatch.setattr("chronicle.mirror._get_supabase_client", lambda: client) + monkeypatch.setenv("CHRONICLE_SCHEMA", "chronicle_probe") + argv = [ + "load-supabase-mirror", + "--dir", + str(_empty_mirror(tmp_path)), + "--build-artifacts", + str(_one_build_artifact(tmp_path)), + ] + + exit_code = harness_main(argv) + payload = json.loads(capsys.readouterr().out) + + assert exit_code == 0 + assert payload["schema"] == "chronicle_probe" + assert [upsert[0] for upsert in client.upserts] == ["chronicle_probe"] + + assert harness_main([*argv, "--schema", "explicit_probe"]) == 0 + assert json.loads(capsys.readouterr().out)["schema"] == "explicit_probe" + assert [upsert[0] for upsert in client.upserts] == [ + "chronicle_probe", + "explicit_probe", + ] + + class _FakeSupabaseClient: def __init__(self): self.upserts = [] diff --git a/tests/test_chronicle_namespace.py b/tests/test_chronicle_namespace.py index e8890948..5dad695e 100644 --- a/tests/test_chronicle_namespace.py +++ b/tests/test_chronicle_namespace.py @@ -1,7 +1,5 @@ """Tests for the Chronicle namespace.""" -import importlib - from chronicle.client import get_supabase_client from chronicle.normalization import convert_units from chronicle.targets import ( @@ -27,20 +25,30 @@ def test_chronicle_client_reexports_supabase_client(): def test_chronicle_supabase_schema_boundaries_are_defaulted(): - """The schema names are import-time constants, so re-read them here. - - ``db.supabase_client`` resolves them from the environment when it is first - imported, which happens at collection — before the suite-wide - ``isolated_rename_window_env`` fixture clears an operator's - ``CHRONICLE_SCHEMA``. Reloading under the cleared environment is what makes - this a test of the defaults rather than of the shell. + """The schema names resolve per call, so this reads the cleared window. + + ``db.supabase_client`` is imported at collection, before any fixture runs. + Resolving the schema there — as an import-time constant — would bind an + operator's ``CHRONICLE_SCHEMA`` (or a ledger-era name, warning as it went) + into the module for the whole session, and no fixture could take it back. + Reading at call time is what makes this a test of the defaults rather than + of the shell. """ - import db.supabase_client + from db import supabase_client + + assert supabase_client.chronicle_schema() == "ledger" + assert supabase_client.targets_schema() == "targets" + + +def test_chronicle_supabase_schema_follows_the_environment(monkeypatch): + """The renamed variable reaches the client after it has been imported.""" + from db import supabase_client - supabase_client = importlib.reload(db.supabase_client) + monkeypatch.setenv("CHRONICLE_SCHEMA", "chronicle_probe") + monkeypatch.setenv("POLICYENGINE_TARGETS_SCHEMA", "targets_probe") - assert supabase_client.LEDGER_SCHEMA == "ledger" - assert supabase_client.TARGETS_SCHEMA == "targets" + assert supabase_client.chronicle_schema() == "chronicle_probe" + assert supabase_client.targets_schema() == "targets_probe" def test_chronicle_normalization_exports_helpers(): From 6ca5f650ab3cdb49c0a3c90880bfd28b70973569 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 2 Sep 2026 13:26:41 -0400 Subject: [PATCH 16/22] Address the manifest a fetch is actually revising, and read it strictly Four defects in the state fetch-artifact reads before it writes: - It always targeted manifest.yaml. Seven tracked packages keep a manifest_*_source_package.yaml instead, and three publisher directories keep two of them, so the IRA revision workflow the docs cite would have written a third manifest beside the real ones and never seen the recorded block. A --manifest filename now selects it; the name has to stay inside the package. - Revision protection vanished when the entry had no storage.r2 -- a fetch that only registered bytes, or one whose upload failed, which is the state #225 landed in. A manifest entry identifies its bytes by its declared sha256 whether or not it has been published, and a fetch of different bytes over either identity is refused unless --record-revision opts in. - Recorded R2 validation read the key or the uri, whichever came first. A block whose key and uri named different objects was preserved verbatim, so the entry kept publishing a URI for bytes it no longer described. Every supplied locator field is now cross-checked against every other and against the content-addressed key shape; a contradiction is an error at fetch time and a refusal at publish time, never a silent preserve. - A manifest that parsed as anything but a mapping was treated as absent, so the fetch would replace it with a single entry. It is now refused before the publisher is read at all, and inventory-artifacts and publish-raw report it rather than crashing on it. The refusals share a SourceArtifactManifestError base, so the CLI reports all of them as an exit-1 message with nothing written. Co-Authored-By: Claude Fable 5.1 --- chronicle/artifacts.py | 525 +++++++++++++++++++++++------- chronicle/harness.py | 29 +- tests/test_chronicle_artifacts.py | 416 +++++++++++++++++++++++ 3 files changed, 846 insertions(+), 124 deletions(-) diff --git a/chronicle/artifacts.py b/chronicle/artifacts.py index ff9b2b48..514acaa2 100644 --- a/chronicle/artifacts.py +++ b/chronicle/artifacts.py @@ -9,6 +9,7 @@ from datetime import UTC, datetime from pathlib import Path import posixpath +import re import shlex import sqlite3 import subprocess @@ -40,6 +41,27 @@ DEFAULT_R2_PREFIX = "raw" DEFAULT_R2_DERIVED_PREFIX = "derived" +# Most packages keep one manifest.yaml. Publisher directories that feed several +# source packages keep one manifest each -- db/data/irs_soi/ira_contributions +# holds manifest_traditional_source_package.yaml beside the Roth one -- so the +# name is an input, not a constant, wherever a caller addresses a package. +DEFAULT_MANIFEST_FILENAME = "manifest.yaml" + + +def _manifest_path(output: Path, manifest_filename: str) -> Path: + """Return the named manifest inside ``output``. + + The name is a filename, not a path: it selects among the manifests a + package directory keeps, and must not reach outside it. + """ + name = manifest_filename.strip() + if not name or name in (".", "..") or name != Path(name).name: + raise ValueError( + "Manifest must name a file inside the package directory, not " + f"{manifest_filename!r}." + ) + return output / name + def default_r2_raw_bucket() -> str: """Resolve the raw bucket: ``$CHRONICLE_R2_RAW_BUCKET`` or the default.""" @@ -51,15 +73,44 @@ def default_r2_derived_bucket() -> str: return env_value(R2_DERIVED_BUCKET_ENV, default=DEFAULT_R2_DERIVED_BUCKET) -class SourceArtifactRevisionError(RuntimeError): - """Fetched bytes are not the bytes the recorded R2 object holds. +class SourceArtifactManifestError(RuntimeError): + """A manifest refuses the write a fetch is about to make. + + Every subclass is raised before anything is downloaded, cached, uploaded or + rewritten, so a refusal leaves the package exactly as it was. + """ + + +class SourceArtifactRevisionError(SourceArtifactManifestError): + """Fetched bytes are not the bytes the manifest entry identifies. + + A manifest entry identifies specific bytes: by its declared ``sha256``, and + -- once published -- by a content-addressed R2 key that repeats them. When + a publisher re-publishes under the same URL and vintage, rewriting that + entry would attach its provenance, and any recorded URI, to bytes it never + described. Chronicle refuses instead: same vintage plus new bytes is a new + release revision (docs/adr-chronicle-fact-identity-v2.md), registered with + ``fetch-artifact --record-revision``. + """ + - Raw R2 keys are content-addressed, so a recorded ``storage.r2`` block is a - claim about specific bytes. When a publisher re-publishes under the same - URL and vintage, keeping that block would attach its provenance to bytes it - never described. Chronicle refuses instead: same vintage plus new bytes is a - new release revision (docs/adr-chronicle-fact-identity-v2.md), registered - with ``fetch-artifact --record-revision``. +class MalformedManifestError(SourceArtifactManifestError): + """A manifest document, or a block inside one, is not a mapping. + + Reading such a file as an absent manifest would let a fetch replace it with + a single entry, dropping whatever the unreadable document recorded. + """ + + +class RecordedR2LocatorError(SourceArtifactManifestError): + """A recorded ``storage.r2`` block does not locate exactly one object. + + ``provider``, ``bucket``, ``key`` and ``uri`` all describe the same object, + so any that are supplied have to agree, and the key has to carry the + ``{sha256}/{filename}`` tail that says which bytes it holds. A block whose + fields contradict each other has no single answer to "which bytes does this + entry claim R2 holds", and preserving or publishing under it would ship + whichever field the reader happened to consult. """ @@ -112,6 +163,60 @@ def to_dict(self) -> dict[str, str]: } +# A raw key ends in {sha256}/{filename} (see build_r2_key), so the segment +# before the filename is what says which bytes the object holds. +_SHA256_KEY_SEGMENT = re.compile(r"[0-9a-f]{64}") + + +@dataclass(frozen=True) +class RecordedR2Object: + """The R2 object a manifest entry's ``storage.r2`` block claims exists. + + Built only by :func:`_validated_recorded_r2`, so every instance names one + object whose locator fields agree with each other. + """ + + provider: str + bucket: str + key: str + sha256: str + filename: str + + @property + def uri(self) -> str: + """Return the storage URI the recorded fields spell out.""" + return f"{self.provider}://{self.bucket}/{self.key}" + + +@dataclass(frozen=True) +class RecordedIdentity: + """The bytes a manifest entry says its vintage currently holds. + + From the recorded object's content-addressed key once the entry has been + published, and from the entry's own declared ``sha256``/``filename`` before + that. ``r2`` is None in the second case: protection does not wait for an + upload to have happened. + """ + + sha256: str + filename: str + size_bytes: int | None + declared_sha256: str | None + r2: RecordedR2Object | None + + def holds(self, *, sha256: str, filename: str) -> bool: + """Whether this identity is exactly the given bytes under that name. + + The filename participates only when the entry records one: a published + key always carries it, an entry that declares bytes and no name does + not, and inventing a mismatch there would refuse a re-fetch of the very + bytes the entry describes. + """ + if self.sha256 != sha256: + return False + return not self.filename or self.filename == Path(filename).name + + @dataclass(frozen=True) class ArtifactCommandResult: """Result from a storage command.""" @@ -428,6 +533,7 @@ def fetch_source_artifact( source_page: str | None = None, table: str | None = None, filename: str | None = None, + manifest_filename: str = DEFAULT_MANIFEST_FILENAME, upload_r2: bool = False, record_revision: bool = False, r2_bucket: str | None = None, @@ -436,20 +542,36 @@ def fetch_source_artifact( ) -> ArtifactFetchReport: """Fetch/register a source artifact and optionally upload it to R2. + ``manifest_filename`` names the manifest inside ``output_dir`` the entry + belongs to. Packages that split one publisher directory across several + source packages keep one manifest each, so a fetch that always wrote + ``manifest.yaml`` would write a fresh manifest beside the real ones and + never see the entry it is revising. + ``record_revision`` opts into registering a publisher revision: the fetched bytes get their own content-addressed key under the configured bucket and the superseded object moves to ``storage.previous_r2``. Without it, bytes - that disagree with the recorded object raise + that disagree with the entry's recorded identity raise :class:`SourceArtifactRevisionError` before anything is overwritten. """ r2_bucket = r2_bucket or default_r2_raw_bucket() output = Path(output_dir) + manifest_path = _manifest_path(output, manifest_filename) resolved_r2_prefix = resolve_r2_prefix( prefix=r2_prefix, default_prefix=DEFAULT_R2_PREFIX, source_id=source_id, package_path=output, ) + # Read and validate the entry being written before anything is fetched: a + # manifest Chronicle cannot read, or a recorded block that names two + # different objects, is a refusal that need not touch the publisher. + recorded_identity = _recorded_identity( + _manifest_file_spec(_read_manifest(manifest_path), year), + manifest_path=manifest_path, + year=year, + ) + fetched_at = datetime.now(UTC).replace(microsecond=0).isoformat() content, inferred_filename = _read_artifact(source_url) artifact_filename = filename or inferred_filename @@ -458,12 +580,12 @@ def fetch_source_artifact( sha256 = hashlib.sha256(content).hexdigest() size_bytes = len(content) - manifest_path = output / "manifest.yaml" # Guard before the cached artifact is touched. A rejected fetch must leave # the recorded bytes and their manifest entry exactly as they were. - _assert_recorded_object_holds_these_bytes( - manifest_path, + _assert_recorded_identity_holds_these_bytes( + recorded_identity, + manifest_path=manifest_path, year=year, filename=artifact_filename, sha256=sha256, @@ -655,7 +777,7 @@ def publish_derived_artifacts( def publish_source_artifacts( root: str | Path, *, - manifest_filename: str = "manifest.yaml", + manifest_filename: str = DEFAULT_MANIFEST_FILENAME, source_id: str | None = None, package_id: str | None = None, r2_bucket: str | None = None, @@ -676,8 +798,8 @@ def publish_source_artifacts( errors: list[str] = [] for manifest_path in sorted(root_path.rglob(manifest_filename)): try: - manifest = yaml.safe_load(manifest_path.read_text(encoding="utf-8")) or {} - except (OSError, yaml.YAMLError) as exc: + manifest = _read_manifest(manifest_path) + except (OSError, MalformedManifestError) as exc: errors.append(f"Could not read {manifest_path}: {exc}") continue @@ -780,7 +902,7 @@ def write_build_artifacts_jsonl( def inventory_source_artifacts( root: str | Path, *, - manifest_filename: str = "manifest.yaml", + manifest_filename: str = DEFAULT_MANIFEST_FILENAME, ) -> ArtifactInventoryReport: """Inventory manifest-declared source artifacts under a root directory.""" root_path = Path(root) @@ -803,9 +925,8 @@ def inventory_source_artifacts( manifests = sorted(root_path.rglob(manifest_filename)) for manifest_path in manifests: try: - manifest = yaml.safe_load(manifest_path.read_text(encoding="utf-8")) or {} - files = manifest.get("files") or {} - except (OSError, yaml.YAMLError) as exc: + files = _read_manifest(manifest_path).get("files") or {} + except (OSError, MalformedManifestError) as exc: errors.append(f"Could not read {manifest_path}: {exc}") continue if not isinstance(files, dict): @@ -1087,11 +1208,31 @@ def _filename_from_url(source_url: str) -> str: def _read_manifest(manifest_path: Path) -> dict[str, Any]: - """Return a manifest's parsed payload, or an empty mapping.""" + """Return a manifest's parsed payload, refusing a document it cannot read. + + An absent or empty manifest reads as an empty mapping: ``fetch-artifact`` + writes the first entry into a package that has none. A document that parses + as anything else -- a list, a scalar, a truncated or half-merged file -- is + not an absent manifest, and treating it as one would let the fetch replace + it with a single entry and drop everything it recorded. + """ if not manifest_path.exists(): return {} - payload = yaml.safe_load(manifest_path.read_text(encoding="utf-8")) or {} - return payload if isinstance(payload, dict) else {} + try: + payload = yaml.safe_load(manifest_path.read_text(encoding="utf-8")) + except yaml.YAMLError as exc: + raise MalformedManifestError(f"{manifest_path} is not valid YAML: {exc}") from ( + exc + ) + if payload is None: + return {} + if not isinstance(payload, dict): + raise MalformedManifestError( + f"{manifest_path} must be a YAML mapping; it parses as a " + f"{type(payload).__name__}. Chronicle will not overwrite a manifest " + "it cannot read." + ) + return payload def _manifest_file_spec(payload: dict[str, Any], year: Any) -> dict[str, Any]: @@ -1112,40 +1253,182 @@ def _recorded_storage(spec: Any) -> dict[str, Any]: def _recorded_r2(spec: Any) -> dict[str, Any]: - """Return a manifest file spec's recorded ``storage.r2`` block, if any.""" + """Return the recorded ``storage.r2`` block verbatim, if any. + + Raw access, for callers that carry the block forward as history. Callers + that reason about which object it names go through + :func:`_validated_recorded_r2` instead. + """ recorded = _recorded_storage(spec).get("r2") return recorded if isinstance(recorded, dict) else {} -def _r2_key_identity(recorded_r2: dict[str, Any]) -> tuple[str, str]: - """Return the ``(sha256, filename)`` a recorded R2 object is addressed by. +def _split_r2_uri(uri: str) -> tuple[str, str, str] | None: + """Split ``provider://bucket/key`` into its three parts, or None.""" + provider, separator, remainder = uri.partition("://") + if not separator or not provider: + return None + bucket, separator, key = remainder.partition("/") + if not separator or not bucket or not key: + return None + return (provider, bucket, key) - Raw keys are ``{prefix}/{source_id}/{package_id}/{year}/{sha256}/{filename}`` - (see :func:`build_r2_key`), so the last two segments say which bytes the - object holds; the URI ends in the same two segments and stands in for a - block that records only that. A locator in any other shape yields empty - strings and therefore never matches fetched bytes. + +def _validated_recorded_r2( + spec: Any, + *, + manifest_path: Path, + year: Any, +) -> RecordedR2Object | None: + """Return the object a recorded ``storage.r2`` block names, or None. + + Every locator field the block supplies is cross-checked against every + other: ``key`` against the URI's path, ``bucket`` against its authority, + ``provider`` against its scheme, and the resulting key against the + canonical content-addressed shape :func:`build_r2_key` writes. Reading one + field and trusting the rest is what lets a block that says two different + things survive a preserve or a publish. """ - locator = recorded_r2.get("key") or recorded_r2.get("uri") - if not isinstance(locator, str): - return ("", "") - parts = [part for part in locator.split("/") if part] - if len(parts) < 2: - return ("", "") - return (parts[-2], parts[-1]) + storage = _recorded_storage_block(spec, manifest_path=manifest_path, year=year) + if "r2" not in storage: + return None + block = storage["r2"] + where = f"{manifest_path} entry {year!r} storage.r2" + if not isinstance(block, dict): + raise MalformedManifestError( + f"{where} must be a mapping; it is a {type(block).__name__}." + ) + + supplied: dict[str, str] = {} + for field in ("provider", "bucket", "key", "uri"): + value = block.get(field) + if value is None: + continue + if not isinstance(value, str) or not value.strip(): + raise RecordedR2LocatorError( + f"{where}: {field} must be a non-empty string, not {value!r}." + ) + supplied[field] = value + + provider = supplied.get("provider") + bucket = supplied.get("bucket") + key = supplied.get("key") + uri = supplied.get("uri") + if uri is not None: + parts = _split_r2_uri(uri) + if parts is None: + raise RecordedR2LocatorError( + f"{where}: uri {uri!r} is not provider://bucket/key." + ) + for field, value, from_uri in zip( + ("provider", "bucket", "key"), (provider, bucket, key), parts + ): + if value is not None and value != from_uri: + raise RecordedR2LocatorError( + f"{where}: {field}={value!r} contradicts uri {uri!r}, which " + f"names {from_uri!r}. The block records two different " + "objects, so Chronicle cannot say which bytes it claims." + ) + provider, bucket, key = ( + provider or parts[0], + bucket or parts[1], + key or parts[2], + ) + + missing = [ + field + for field, value in ( + ("provider", provider), + ("bucket", bucket), + ("key", key), + ) + if not value + ] + if missing: + raise RecordedR2LocatorError( + f"{where}: records no {', '.join(missing)}. A recorded block has to " + "locate its object, by key and bucket or by uri." + ) + segments = key.split("/") + if ( + len(segments) < 2 + or not all(segments) + or not _SHA256_KEY_SEGMENT.fullmatch(segments[-2]) + ): + raise RecordedR2LocatorError( + f"{where}: key {key!r} is not content-addressed. A raw key ends in " + "{sha256}/{filename}, which is what says the object holds the " + "entry's bytes; Chronicle will not guess for a key that does not." + ) + return RecordedR2Object( + provider=provider, + bucket=bucket, + key=key, + sha256=segments[-2], + filename=segments[-1], + ) -def _r2_holds_these_bytes( - recorded_r2: dict[str, Any], + +def _recorded_storage_block( + spec: Any, *, - sha256: str, - filename: str, -) -> bool: - """Whether a recorded ``storage.r2`` block addresses exactly these bytes.""" - recorded_sha256, recorded_filename = _r2_key_identity(recorded_r2) - return bool(recorded_sha256) and (recorded_sha256, recorded_filename) == ( - sha256, - Path(filename).name, + manifest_path: Path, + year: Any, +) -> dict[str, Any]: + """Return the entry's ``storage`` mapping, refusing a malformed one.""" + if not isinstance(spec, dict) or "storage" not in spec: + return {} + storage = spec["storage"] + if not isinstance(storage, dict): + raise MalformedManifestError( + f"{manifest_path} entry {year!r} storage must be a mapping; it is a " + f"{type(storage).__name__}." + ) + return storage + + +def _recorded_identity( + spec: Any, + *, + manifest_path: Path, + year: Any, +) -> RecordedIdentity | None: + """Return what a manifest entry says its vintage holds, if anything. + + A published entry is identified by its recorded object's content-addressed + key. An entry that has not been published yet -- registered without an + upload, or left behind by a failed one -- is identified by its own declared + ``sha256`` and ``filename``. Both are recorded identities, and a fetch of + different bytes over either one is a publisher revision. + """ + recorded_r2 = _validated_recorded_r2(spec, manifest_path=manifest_path, year=year) + declared_sha256 = spec.get("sha256") if isinstance(spec, dict) else None + declared_sha256 = declared_sha256 if isinstance(declared_sha256, str) else None + declared_filename = spec.get("filename") if isinstance(spec, dict) else None + declared_filename = ( + declared_filename if isinstance(declared_filename, str) else None + ) + size_bytes = spec.get("size_bytes") if isinstance(spec, dict) else None + size_bytes = size_bytes if isinstance(size_bytes, int) else None + if recorded_r2 is not None: + return RecordedIdentity( + sha256=recorded_r2.sha256, + filename=recorded_r2.filename, + # Only report a size the recorded key agrees with: an entry can + # arrive here already describing the new bytes. + size_bytes=size_bytes if declared_sha256 == recorded_r2.sha256 else None, + declared_sha256=declared_sha256, + r2=recorded_r2, + ) + if not declared_sha256: + return None + return RecordedIdentity( + sha256=declared_sha256, + filename=Path(declared_filename).name if declared_filename else "", + size_bytes=size_bytes, + declared_sha256=declared_sha256, + r2=None, ) @@ -1154,33 +1437,35 @@ def _revision_error_message( manifest_path: Path, year: Any, filename: str, - recorded_spec: dict[str, Any], - recorded_r2: dict[str, Any], + identity: RecordedIdentity, sha256: str, size_bytes: int, r2_bucket: str, ) -> str: """Explain a refused fetch: recorded identity, fetched identity, next step.""" - recorded_sha256, recorded_filename = _r2_key_identity(recorded_r2) - declared_sha256 = recorded_spec.get("sha256") - recorded_size = ( - recorded_spec.get("size_bytes") if declared_sha256 == recorded_sha256 else None + records = ( + f"already records the R2 object {identity.r2.uri}, which holds" + if identity.r2 is not None + else "already records" ) message = ( - f"{manifest_path} entry {year!r} already records the R2 object " - f"{recorded_r2.get('uri') or recorded_r2.get('key')}, which holds " - f"sha256={recorded_sha256 or 'unknown'} " - f"filename={recorded_filename or 'unknown'} " - f"size_bytes={recorded_size if recorded_size is not None else 'unknown'}. " + f"{manifest_path} entry {year!r} {records} " + f"sha256={identity.sha256} " + f"filename={identity.filename or 'unknown'} " + f"size_bytes=" + f"{identity.size_bytes if identity.size_bytes is not None else 'unknown'}. " f"The fetched bytes are sha256={sha256} filename={Path(filename).name} " - f"size_bytes={size_bytes}. Chronicle will not attach a recorded, " - "content-addressed R2 URI to bytes it does not describe." + f"size_bytes={size_bytes}. Chronicle will not rewrite a vintage that " + "identifies specific bytes to describe bytes it never identified." ) - if declared_sha256 and declared_sha256 != recorded_sha256: + if identity.r2 is not None and identity.declared_sha256 not in ( + None, + identity.sha256, + ): message += ( - f" (The entry also declares sha256={declared_sha256}, which its own " - "R2 key contradicts: an earlier fetch rewrote the hash without " - "moving the object.)" + f" (The entry also declares sha256={identity.declared_sha256}, which " + "its own R2 key contradicts: an earlier fetch rewrote the hash " + "without moving the object.)" ) return message + ( " The same vintage with new bytes is a new release revision " @@ -1191,9 +1476,10 @@ def _revision_error_message( ) -def _assert_recorded_object_holds_these_bytes( - manifest_path: Path, +def _assert_recorded_identity_holds_these_bytes( + identity: RecordedIdentity | None, *, + manifest_path: Path, year: Any, filename: str, sha256: str, @@ -1202,21 +1488,16 @@ def _assert_recorded_object_holds_these_bytes( record_revision: bool, ) -> None: """Refuse a publisher revision that has not been opted into.""" - if record_revision: + if record_revision or identity is None: return - recorded_spec = _manifest_file_spec(_read_manifest(manifest_path), year) - recorded_r2 = _recorded_r2(recorded_spec) - if not recorded_r2: - return - if _r2_holds_these_bytes(recorded_r2, sha256=sha256, filename=filename): + if identity.holds(sha256=sha256, filename=filename): return raise SourceArtifactRevisionError( _revision_error_message( manifest_path=manifest_path, year=year, filename=filename, - recorded_spec=recorded_spec, - recorded_r2=recorded_r2, + identity=identity, sha256=sha256, size_bytes=size_bytes, r2_bucket=r2_bucket, @@ -1227,6 +1508,7 @@ def _assert_recorded_object_holds_these_bytes( def _superseding_storage( recorded_spec: dict[str, Any], *, + recorded_r2: RecordedR2Object | None, new_r2: dict[str, Any] | None, superseded_at: str, ) -> dict[str, Any]: @@ -1235,18 +1517,16 @@ def _superseding_storage( ``storage.r2`` only ever names the object that holds the entry's current bytes. The superseded block is appended, oldest first, to ``storage.previous_r2`` so the earlier bytes stay addressable by the URI - archived witness records already pin. + archived witness records already pin. An entry that was never published has + no object to supersede, and gets no ``previous_r2`` key. """ storage = dict(_recorded_storage(recorded_spec)) previous = storage.get("previous_r2") entries = list(previous) if isinstance(previous, list) else [] - recorded_r2 = _recorded_r2(recorded_spec) - if recorded_r2: - entry = dict(recorded_r2) - recorded_sha256, _recorded_filename = _r2_key_identity(recorded_r2) - if recorded_sha256: - entry["sha256"] = recorded_sha256 - if recorded_spec.get("sha256") == recorded_sha256: + if recorded_r2 is not None: + entry = dict(_recorded_r2(recorded_spec)) + entry["sha256"] = recorded_r2.sha256 + if recorded_spec.get("sha256") == recorded_r2.sha256: # Only carry metadata the superseded key agrees with: a manifest # can arrive here already describing the new bytes. for field in ("size_bytes", "fetched_at", "source_url"): @@ -1255,7 +1535,8 @@ def _superseding_storage( entry[field] = value entry["superseded_at"] = superseded_at entries.append(entry) - storage["previous_r2"] = entries + if entries: + storage["previous_r2"] = entries if new_r2 is None: storage.pop("r2", None) else: @@ -1296,42 +1577,45 @@ def _upsert_manifest( } recorded_spec = _manifest_file_spec(payload, year) recorded_storage = _recorded_storage(recorded_spec) - recorded_r2 = _recorded_r2(recorded_spec) + identity = _recorded_identity(recorded_spec, manifest_path=manifest_path, year=year) new_r2 = r2_location.to_dict() if r2_location is not None else None - if recorded_r2 and _r2_holds_these_bytes( - recorded_r2, sha256=sha256, filename=filename - ): - # A recorded storage.r2 block for these exact bytes is historical - # truth: archived witness records pin raw R2 URLs by hash. Re-fetching - # under a renamed bucket copies bytes; it does not restate where the - # bytes were first published (PolicyEngine/chronicle#143, mechanism 3). - file_entry["storage"] = {**recorded_storage, "r2": recorded_r2} - elif recorded_r2: + holds = identity is not None and identity.holds(sha256=sha256, filename=filename) + if identity is not None and not holds and not record_revision: # Different bytes under the same vintage. The guard in # fetch_source_artifact refuses this without --record-revision; repeat # the check here so no caller can reach a false-provenance write. - if not record_revision: - raise SourceArtifactRevisionError( - _revision_error_message( - manifest_path=manifest_path, - year=year, - filename=filename, - recorded_spec=recorded_spec, - recorded_r2=recorded_r2, - sha256=sha256, - size_bytes=size_bytes, - r2_bucket=(new_r2 or {}).get("bucket") or default_r2_raw_bucket(), - ) + raise SourceArtifactRevisionError( + _revision_error_message( + manifest_path=manifest_path, + year=year, + filename=filename, + identity=identity, + sha256=sha256, + size_bytes=size_bytes, + r2_bucket=(new_r2 or {}).get("bucket") or default_r2_raw_bucket(), ) - file_entry["storage"] = _superseding_storage( + ) + if holds and identity.r2 is not None: + # A recorded storage.r2 block for these exact bytes is historical + # truth: archived witness records pin raw R2 URLs by hash. Re-fetching + # under a renamed bucket copies bytes; it does not restate where the + # bytes were first published (PolicyEngine/chronicle#143, mechanism 3). + storage = {**recorded_storage, "r2": _recorded_r2(recorded_spec)} + elif identity is not None and not holds: + storage = _superseding_storage( recorded_spec, + recorded_r2=identity.r2, new_r2=new_r2, superseded_at=fetched_at, ) elif new_r2 is not None: - file_entry["storage"] = {**recorded_storage, "r2": new_r2} - elif recorded_storage: - file_entry["storage"] = dict(recorded_storage) + storage = {**recorded_storage, "r2": new_r2} + else: + storage = dict(recorded_storage) + # An entry that has no storage to record carries no empty block: a + # revision over a never-published entry supersedes nothing. + if storage: + file_entry["storage"] = storage payload["files"][year] = file_entry manifest_path.write_text( yaml.safe_dump(payload, sort_keys=False), @@ -1416,20 +1700,27 @@ def refuse(reason: str | None = None) -> tuple[RawArtifactPublishEntry, None]: if errors: return refuse() - recorded_r2 = _recorded_r2(spec) - if recorded_r2 and not _r2_holds_these_bytes( - recorded_r2, sha256=sha256_actual or "", filename=filename + try: + recorded_r2 = _validated_recorded_r2( + spec, manifest_path=manifest_path, year=year + ) + except SourceArtifactManifestError as error: + # A block that does not name one object cannot be treated as history, + # and publishing under it would ship whichever field was read. + return refuse(f"recorded_r2_locator_invalid:{error}") + if recorded_r2 is not None and (recorded_r2.sha256, recorded_r2.filename) != ( + sha256_actual or "", + Path(filename).name, ): # The recorded object is addressed by different bytes, so it is not # this file's history. Uploading anyway would either publish under a # key that misdescribes its content or restate a URI that belongs to # the superseded bytes. Registering a publisher revision is # `fetch-artifact --record-revision`, not a publish-time rewrite. - recorded_sha256, recorded_filename = _r2_key_identity(recorded_r2) return refuse( "recorded_r2_identity_mismatch:" - f"recorded_sha256={recorded_sha256 or 'unknown'}:" - f"recorded_filename={recorded_filename or 'unknown'}:" + f"recorded_sha256={recorded_r2.sha256}:" + f"recorded_filename={recorded_r2.filename}:" f"local_sha256={sha256_actual}:" f"local_filename={Path(filename).name}" ) @@ -1447,7 +1738,7 @@ def refuse(reason: str | None = None) -> tuple[RawArtifactPublishEntry, None]: package_path=manifest_path, ), ) - recorded_bucket = recorded_r2.get("bucket") + recorded_bucket = recorded_r2.bucket if recorded_r2 is not None else None if recorded_bucket and recorded_bucket != location.bucket: # The recorded bucket is preserved history. Publishing the same bytes # into a renamed bucket is a backfill copy, not a restatement, so the @@ -1456,7 +1747,7 @@ def refuse(reason: str | None = None) -> tuple[RawArtifactPublishEntry, None]: "recorded_r2_bucket_is_preserved_history:" f"recorded={recorded_bucket}:requested={location.bucket}" ) - recorded_key = recorded_r2.get("key") + recorded_key = recorded_r2.key if recorded_r2 is not None else None if recorded_key and recorded_key != location.key: return refuse( "recorded_r2_key_disagrees_with_country_prefix:" diff --git a/chronicle/harness.py b/chronicle/harness.py index bf3d5ebe..979c04d7 100644 --- a/chronicle/harness.py +++ b/chronicle/harness.py @@ -9,6 +9,7 @@ from pathlib import Path from chronicle.artifacts import ( + DEFAULT_MANIFEST_FILENAME, DEFAULT_R2_DERIVED_BUCKET, DEFAULT_R2_RAW_BUCKET, ArtifactFetchReport, @@ -16,7 +17,7 @@ DerivedArtifactPublishReport, R2BootstrapReport, RawArtifactPublishReport, - SourceArtifactRevisionError, + SourceArtifactManifestError, bootstrap_r2_buckets, fetch_source_artifact, inventory_source_artifacts, @@ -339,6 +340,7 @@ def fetch_artifact_file( source_page: str | None = None, table: str | None = None, filename: str | None = None, + manifest_filename: str = DEFAULT_MANIFEST_FILENAME, upload_r2: bool = False, record_revision: bool = False, r2_bucket: str | None = None, @@ -347,8 +349,9 @@ def fetch_artifact_file( ) -> ArtifactFetchReport: """Fetch/register a raw source artifact and optionally upload it to R2. - Raises :class:`SourceArtifactRevisionError` when the fetched bytes are not - the bytes the manifest's recorded R2 object holds, unless + ``manifest_filename`` selects which of the package directory's manifests + the entry belongs to. Raises :class:`SourceArtifactRevisionError` when the + fetched bytes are not the bytes that manifest's entry identifies, unless ``record_revision`` opts into registering the publisher revision. """ return fetch_source_artifact( @@ -361,6 +364,7 @@ def fetch_artifact_file( source_page=source_page, table=table, filename=filename, + manifest_filename=manifest_filename, upload_r2=upload_r2, record_revision=record_revision, r2_bucket=r2_bucket, @@ -852,7 +856,7 @@ def main(argv: list[str] | None = None) -> int: artifact_parser = subparsers.add_parser( "fetch-artifact", - help="Fetch/register a raw source artifact and update manifest.yaml", + help="Fetch/register a raw source artifact and update its manifest", ) artifact_parser.add_argument( "--url", @@ -873,13 +877,23 @@ def main(argv: list[str] | None = None) -> int: "--year", type=int, required=True, - help="Artifact vintage year to record in manifest.yaml", + help="Artifact vintage year to record in the manifest", ) artifact_parser.add_argument( "--out-dir", type=Path, required=True, - help="Directory where the raw artifact and manifest.yaml should live", + help="Directory where the raw artifact and its manifest should live", + ) + artifact_parser.add_argument( + "--manifest", + default=DEFAULT_MANIFEST_FILENAME, + help=( + "Manifest filename inside --out-dir. A publisher directory that " + "feeds several source packages keeps one manifest each, and the " + "entry being revised lives in exactly one of them. Defaults to " + f"{DEFAULT_MANIFEST_FILENAME}." + ), ) artifact_parser.add_argument( "--dataset", @@ -1360,13 +1374,14 @@ def main(argv: list[str] | None = None) -> int: source_page=args.source_page, table=args.table, filename=args.filename, + manifest_filename=args.manifest, upload_r2=args.upload_r2, record_revision=args.record_revision, r2_bucket=args.r2_bucket, r2_prefix=args.r2_prefix, wrangler_command=args.wrangler_command, ) - except SourceArtifactRevisionError as error: + except SourceArtifactManifestError as error: print(f"error: {error}", file=sys.stderr) return 1 print(json.dumps(report.to_dict(), indent=2, sort_keys=True)) diff --git a/tests/test_chronicle_artifacts.py b/tests/test_chronicle_artifacts.py index f5715055..0a4b10f9 100644 --- a/tests/test_chronicle_artifacts.py +++ b/tests/test_chronicle_artifacts.py @@ -11,6 +11,8 @@ from chronicle.cli import main as cli_main from chronicle.artifacts import ( + MalformedManifestError, + RecordedR2LocatorError, SourceArtifactRevisionError, build_artifact_rows, build_derived_r2_key, @@ -1114,3 +1116,417 @@ def test_a_recorded_block_that_only_carries_a_uri_is_still_recognized( _serve(monkeypatch, SECOND_PUBLICATION) with pytest.raises(SourceArtifactRevisionError): _fetch_republished(output_dir, wrangler) + + +# --------------------------------------------------------------------------- +# Manifest addressing, entry identity, and recorded locators +# +# Everything below concerns the state a fetch reads before it writes: which +# manifest it reads, what that manifest's entry says its vintage holds, and +# whether the recorded R2 block names one object or two. +# --------------------------------------------------------------------------- + +TRADITIONAL_MANIFEST = "manifest_traditional_source_package.yaml" +ROTH_MANIFEST = "manifest_roth_source_package.yaml" + + +def _publish(tmp_path, name, content): + """Write bytes a fetch can read as a local publisher path.""" + path = tmp_path / "publisher" / name + path.parent.mkdir(parents=True, exist_ok=True) + path.write_bytes(content) + return path + + +def _fetch_local(output_dir, source_path, *, package_id="soi-table-5", **kwargs): + return fetch_source_artifact( + str(source_path), + source_id="irs_soi", + package_id=package_id, + year=2022, + output_dir=output_dir, + **kwargs, + ) + + +def _entry(manifest_path): + return yaml.safe_load(manifest_path.read_text())["files"][2022] + + +def test_fetch_artifact_writes_the_manifest_it_was_given(tmp_path): + """One publisher directory, two source packages, two manifests. + + db/data/irs_soi/ira_contributions keeps the traditional and Roth IRA + packages side by side. A fetch that always wrote manifest.yaml would write + a third manifest neither package reads. + """ + package = tmp_path / "db" / "data" / "irs_soi" / "ira_contributions" + traditional = _publish(tmp_path, "22in05ira.xlsx", b"traditional IRA table") + roth = _publish(tmp_path, "22in06ira.xlsx", b"roth IRA table") + + _fetch_local( + package, + traditional, + package_id="soi-ira-traditional-contributions-2022", + manifest_filename=TRADITIONAL_MANIFEST, + ) + _fetch_local( + package, + roth, + package_id="soi-ira-roth-contributions-2022", + manifest_filename=ROTH_MANIFEST, + ) + + assert sorted(path.name for path in package.glob("manifest*.yaml")) == [ + ROTH_MANIFEST, + TRADITIONAL_MANIFEST, + ] + assert not (package / "manifest.yaml").exists() + assert _entry(package / TRADITIONAL_MANIFEST)["filename"] == "22in05ira.xlsx" + assert _entry(package / ROTH_MANIFEST)["filename"] == "22in06ira.xlsx" + assert _entry(package / TRADITIONAL_MANIFEST)["sha256"] == ( + hashlib.sha256(b"traditional IRA table").hexdigest() + ) + + +def test_a_revision_is_refused_in_the_manifest_that_records_it(tmp_path): + """The IRA revision workflow the docs cite, on a two-manifest package.""" + package = tmp_path / "db" / "data" / "irs_soi" / "ira_contributions" + traditional = _publish(tmp_path, "22in05ira.xlsx", b"traditional IRA table") + _fetch_local( + package, + traditional, + package_id="soi-ira-traditional-contributions-2022", + manifest_filename=TRADITIONAL_MANIFEST, + ) + recorded = (package / TRADITIONAL_MANIFEST).read_bytes() + + # The IRS re-publishes under the same URL and vintage. + traditional.write_bytes(b"traditional IRA table, revised rows") + + with pytest.raises(SourceArtifactRevisionError) as raised: + _fetch_local( + package, + traditional, + package_id="soi-ira-traditional-contributions-2022", + manifest_filename=TRADITIONAL_MANIFEST, + ) + + assert TRADITIONAL_MANIFEST in str(raised.value) + assert (package / TRADITIONAL_MANIFEST).read_bytes() == recorded + assert (package / "22in05ira.xlsx").read_bytes() == b"traditional IRA table" + + # Without the flag the same fetch addresses a manifest that has no entry to + # protect -- which is exactly why the flag exists. + report = _fetch_local( + package, + traditional, + package_id="soi-ira-traditional-contributions-2022", + ) + + assert report.valid + assert report.manifest_path.endswith("manifest.yaml") + assert (package / TRADITIONAL_MANIFEST).read_bytes() == recorded + + +@pytest.mark.parametrize( + "manifest_filename", + ["../manifest.yaml", "nested/manifest.yaml", "", " ", ".", ".."], +) +def test_a_manifest_name_must_stay_inside_the_package(tmp_path, manifest_filename): + package = tmp_path / "db" / "data" / "irs_soi" / "soi-table-5" + source = _publish(tmp_path, "table.xlsx", b"table") + + with pytest.raises(ValueError, match="inside the package directory"): + _fetch_local(package, source, manifest_filename=manifest_filename) + + assert not package.exists() + + +def test_fetch_artifact_cli_targets_the_named_manifest(tmp_path, capsys): + package = tmp_path / "db" / "data" / "irs_soi" / "ira_contributions" + traditional = _publish(tmp_path, "22in05ira.xlsx", b"traditional IRA table") + argv = [ + "fetch-artifact", + "--url", + str(traditional), + "--source-id", + "irs_soi", + "--package-id", + "soi-ira-traditional-contributions-2022", + "--year", + "2022", + "--out-dir", + str(package), + "--manifest", + TRADITIONAL_MANIFEST, + ] + + assert harness_main(argv) == 0 + payload = json.loads(capsys.readouterr().out) + + assert payload["manifest_path"].endswith(TRADITIONAL_MANIFEST) + assert not (package / "manifest.yaml").exists() + + traditional.write_bytes(b"traditional IRA table, revised rows") + + assert harness_main(argv) == 1 + assert TRADITIONAL_MANIFEST in capsys.readouterr().err + + +# --------------------------------------------------------------------------- +# Identity without a recorded R2 object +# --------------------------------------------------------------------------- + + +def _failing_wrangler(tmp_path, log): + wrangler = tmp_path / "failing-wrangler" + wrangler.write_text(f"#!/bin/sh\nprintf '%s\\n' \"$*\" >> {log}\nexit 1\n") + wrangler.chmod(0o755) + return wrangler + + +def test_a_registered_entry_is_protected_before_it_is_ever_published(tmp_path): + """No storage.r2 yet is not no identity: the entry declares its bytes.""" + package = tmp_path / "db" / "data" / "irs_soi" / "soi-table-5" + source = _publish(tmp_path, "22in05ira.xlsx", b"IRA table 5, first publication") + first = _fetch_local(package, source, upload_r2=False) + recorded = (package / "manifest.yaml").read_bytes() + + assert "storage" not in _entry(package / "manifest.yaml") + + # Same bytes: an ordinary repeated fetch, not a revision. + assert _fetch_local(package, source, upload_r2=False).sha256 == first.sha256 + + source.write_bytes(b"IRA table 5, silently re-published") + with pytest.raises(SourceArtifactRevisionError) as raised: + _fetch_local(package, source, upload_r2=False) + + message = str(raised.value) + assert first.sha256 in message + assert hashlib.sha256(b"IRA table 5, silently re-published").hexdigest() in message + assert "size_bytes=30" in message + assert "--record-revision" in message + assert (package / "manifest.yaml").read_bytes() == recorded + assert (package / "22in05ira.xlsx").read_bytes() == ( + b"IRA table 5, first publication" + ) + + +def test_a_failed_upload_does_not_disable_revision_protection(tmp_path): + """The state #225 hit: bytes registered, upload failed, no storage.r2.""" + package = tmp_path / "db" / "data" / "irs_soi" / "soi-table-5" + log = tmp_path / "wrangler.log" + wrangler = _failing_wrangler(tmp_path, log) + source = _publish(tmp_path, "22in05ira.xlsx", b"IRA table 5, first publication") + + report = _fetch_local( + package, source, upload_r2=True, wrangler_command=str(wrangler) + ) + recorded = (package / "manifest.yaml").read_bytes() + + assert report.errors == ("r2_upload_failed",) + assert "storage" not in _entry(package / "manifest.yaml") + + source.write_bytes(b"IRA table 5, silently re-published") + with pytest.raises(SourceArtifactRevisionError): + _fetch_local(package, source, upload_r2=True, wrangler_command=str(wrangler)) + + assert (package / "manifest.yaml").read_bytes() == recorded + + +def test_record_revision_over_an_unpublished_entry_supersedes_nothing(tmp_path): + """There is no object to keep, so the entry gets no previous_r2 key.""" + package = tmp_path / "db" / "data" / "irs_soi" / "soi-table-5" + source = _publish(tmp_path, "22in05ira.xlsx", b"IRA table 5, first publication") + _fetch_local(package, source, upload_r2=False) + + source.write_bytes(b"IRA table 5, silently re-published") + report = _fetch_local(package, source, upload_r2=False, record_revision=True) + revised = _entry(package / "manifest.yaml") + + assert report.valid + assert revised["sha256"] == ( + hashlib.sha256(b"IRA table 5, silently re-published").hexdigest() + ) + assert "storage" not in revised + + +# --------------------------------------------------------------------------- +# Recorded locator cross-checks +# --------------------------------------------------------------------------- + + +def _recorded_package(tmp_path, content=b"IRA table 5, first publication"): + """A package whose entry records a published, content-addressed object.""" + package = tmp_path / "db" / "data" / "irs_soi" / "soi-table-5" + wrangler = _wrangler_stub(tmp_path, tmp_path / "wrangler.log") + source = _publish(tmp_path, "22in05ira.xlsx", content) + report = _fetch_local( + package, source, upload_r2=True, wrangler_command=str(wrangler) + ) + return package, source, report + + +def _rewrite_recorded_r2(package, mutate, manifest="manifest.yaml"): + manifest_path = package / manifest + payload = yaml.safe_load(manifest_path.read_text()) + mutate(payload["files"][2022]["storage"]) + manifest_path.write_text(yaml.safe_dump(payload, sort_keys=False)) + return manifest_path + + +def _other_sha256(): + return hashlib.sha256(b"some other object entirely").hexdigest() + + +def _contradict_key(storage): + key = storage["r2"]["key"] + storage["r2"]["key"] = key.replace(key.split("/")[-2], _other_sha256()) + + +def _contradict_bucket(storage): + storage["r2"]["bucket"] = "some-other-bucket" + + +def _contradict_provider(storage): + storage["r2"]["provider"] = "s3" + + +def _mangle_uri(storage): + storage["r2"]["uri"] = "r2:/ledger-raw-missing-a-slash" + + +def _drop_the_locator(storage): + storage["r2"] = {"provider": "r2", "bucket": "ledger-raw"} + + +def _flatten_the_key(storage): + storage["r2"]["key"] = "raw/irs_soi/22in05ira.xlsx" + storage["r2"]["uri"] = f"r2://ledger-raw/{storage['r2']['key']}" + + +@pytest.mark.parametrize( + ("mutate", "expected"), + [ + pytest.param(_contradict_key, "contradicts uri", id="key-vs-uri"), + pytest.param(_contradict_bucket, "contradicts uri", id="bucket-vs-uri"), + pytest.param(_contradict_provider, "contradicts uri", id="provider-vs-uri"), + pytest.param(_mangle_uri, "is not provider://bucket/key", id="uri-shape"), + pytest.param(_drop_the_locator, "records no key", id="no-locator"), + pytest.param( + _flatten_the_key, "is not content-addressed", id="not-content-addressed" + ), + ], +) +def test_a_recorded_block_that_names_two_objects_is_refused(tmp_path, mutate, expected): + """A contradictory locator is an error, never a silently preserved block. + + The key-vs-uri case is the one that used to pass: identity was read from + the key alone, so a block whose uri named different bytes was carried + forward verbatim, and the manifest kept publishing a URI for an object it + no longer described. + """ + package, source, _ = _recorded_package(tmp_path) + manifest_path = _rewrite_recorded_r2(package, mutate) + recorded = manifest_path.read_bytes() + + # Identical bytes: the fetch would otherwise preserve the recorded block. + with pytest.raises(RecordedR2LocatorError) as raised: + _fetch_local(package, source, upload_r2=False) + + assert expected in str(raised.value) + assert manifest_path.read_bytes() == recorded + + +def test_a_malformed_storage_block_is_not_treated_as_absent(tmp_path): + package, source, _ = _recorded_package(tmp_path) + manifest_path = _rewrite_recorded_r2( + package, lambda storage: storage.update({"r2": ["r2://ledger-raw/raw/key"]}) + ) + recorded = manifest_path.read_bytes() + + with pytest.raises(MalformedManifestError, match="must be a mapping"): + _fetch_local(package, source, upload_r2=False) + + assert manifest_path.read_bytes() == recorded + + +def test_publish_raw_refuses_a_contradictory_recorded_block(tmp_path): + """Nothing is uploaded under a block that does not name one object.""" + package, _, _ = _recorded_package(tmp_path) + log = tmp_path / "publish.log" + wrangler = _wrangler_stub(tmp_path, log) + manifest_path = _rewrite_recorded_r2(package, _contradict_key) + recorded = manifest_path.read_bytes() + + report = publish_source_artifacts(package, wrangler_command=str(wrangler)) + + assert not report.valid + assert report.entries[0].upload is None + assert report.entries[0].errors[0].startswith("recorded_r2_locator_invalid:") + assert "contradicts uri" in report.entries[0].errors[0] + assert not log.exists() + assert manifest_path.read_bytes() == recorded + + +# --------------------------------------------------------------------------- +# Malformed manifests +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "document", + [ + pytest.param("- one entry\n- another\n", id="list"), + pytest.param("a bare scalar\n", id="scalar"), + pytest.param("files: [\n", id="unparseable"), + ], +) +def test_a_malformed_manifest_is_refused_before_anything_is_fetched(tmp_path, document): + """Not an absent manifest: refusing it protects what it still records. + + The publisher path does not exist, so reaching the fetch at all would raise + FileNotFoundError instead. Getting MalformedManifestError is what says the + manifest was read and refused first. + """ + package = tmp_path / "db" / "data" / "irs_soi" / "soi-table-5" + package.mkdir(parents=True) + manifest_path = package / "manifest.yaml" + manifest_path.write_text(document) + + with pytest.raises(MalformedManifestError): + _fetch_local(package, tmp_path / "publisher" / "never-read.xlsx") + + assert manifest_path.read_text() == document + assert list(package.iterdir()) == [manifest_path] + + +@pytest.mark.parametrize("document", ["", "\n", "{}\n", "# only a comment\n"]) +def test_an_empty_manifest_still_reads_as_absent(tmp_path, document): + package = tmp_path / "db" / "data" / "irs_soi" / "soi-table-5" + package.mkdir(parents=True) + (package / "manifest.yaml").write_text(document) + source = _publish(tmp_path, "22in05ira.xlsx", b"IRA table 5") + + report = _fetch_local(package, source, upload_r2=False) + + assert report.valid + assert _entry(package / "manifest.yaml")["filename"] == "22in05ira.xlsx" + + +def test_a_malformed_manifest_is_reported_by_inventory_and_publish(tmp_path): + """Neither sweep may crash on, or silently skip, a document it cannot read.""" + package = tmp_path / "db" / "data" / "irs_soi" / "soi-table-5" + package.mkdir(parents=True) + (package / "manifest.yaml").write_text("- not a mapping\n") + + inventory = inventory_source_artifacts(package) + published = publish_source_artifacts(package) + + assert not inventory.valid + assert inventory.entries == () + assert "must be a YAML mapping" in inventory.errors[0] + assert not published.valid + assert published.entries == () + assert "must be a YAML mapping" in published.errors[0] From 75d9d4a3f6acf2d5cc0b0472b4823fb4767b1d13 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 2 Sep 2026 13:38:08 -0400 Subject: [PATCH 17/22] Document identity, manifest selection and locator checks What a manifest entry identifies (declared sha256 from registration, the content-addressed key once published), which manifest a fetch addresses in a package that keeps more than one, what a recorded storage.r2 block has to say for itself, and that load-supabase-mirror takes its schema default from CHRONICLE_SCHEMA like every other reader. Co-Authored-By: Claude Fable 5.1 --- README.md | 8 ++++-- docs/agent-source-package-harness.md | 11 +++++--- docs/storage-architecture.md | 41 ++++++++++++++++++++++++++-- 3 files changed, 51 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 4397663b..ad4bb5ff 100644 --- a/README.md +++ b/README.md @@ -356,7 +356,9 @@ bunx wrangler login uv run chronicle bootstrap-r2 # Fetch/register a source artifact, write db/data/.../manifest.yaml, and upload -# the exact bytes to R2 when Wrangler is authenticated: +# the exact bytes to R2 when Wrangler is authenticated. Pass --manifest when the +# package directory keeps more than one manifest (ira_contributions keeps a +# traditional and a Roth one): uv run chronicle fetch-artifact \ --url https://www.irs.gov/pub/irs-soi/23in12ms.xls \ --source-id irs_soi \ @@ -368,7 +370,9 @@ uv run chronicle fetch-artifact \ --upload-r2 # Re-fetching is safe: identical bytes keep the recorded storage.r2 block, and -# bytes that disagree with it are refused. When a publisher has re-published +# bytes that disagree with what the entry identifies -- its declared sha256, or +# its recorded content-addressed key once published -- are refused. When a +# publisher has re-published # under the same URL and vintage, register the revision explicitly — the new # bytes get their own content-addressed key and the superseded object is kept # in storage.previous_r2: diff --git a/docs/agent-source-package-harness.md b/docs/agent-source-package-harness.md index 11200ebb..56d65bb7 100644 --- a/docs/agent-source-package-harness.md +++ b/docs/agent-source-package-harness.md @@ -16,15 +16,18 @@ register raw source files with `uv run chronicle fetch-artifact` before authorin selectors. This writes the local artifact, captures checksum and retrieval metadata in `manifest.yaml`, and can upload the exact bytes to the private raw R2 bucket (`ledger-raw` today; overridable with `CHRONICLE_R2_RAW_BUCKET`) when -Wrangler is authenticated. Agents can audit the local +Wrangler is authenticated. A publisher directory that feeds several source +packages keeps one manifest each, so pass `--manifest ` to address +the right one. Agents can audit the local artifact registry with `uv run chronicle inventory-artifacts --root db/data`. For already-downloaded manifest artifacts, agents should run `uv run chronicle publish-raw --root db/data` to upload checksum-verified bytes to R2 and write `storage.r2` metadata back into each manifest entry. -Both commands treat a recorded `storage.r2` block as a claim about specific -bytes, because raw keys are content-addressed. Re-fetching or publishing bytes -the recorded object does not hold is refused; when a publisher has re-published +Both commands treat a manifest entry as a claim about specific bytes: by its +declared `sha256` from the moment it is registered, and by the content-addressed +key of its recorded `storage.r2` block once it is published. Re-fetching or +publishing bytes the entry does not identify is refused; when a publisher has re-published under the same URL and vintage, register the revision with `uv run chronicle fetch-artifact ... --record-revision`, which stores the new bytes under their own key and keeps the superseded object in diff --git a/docs/storage-architecture.md b/docs/storage-architecture.md index 6f4cfb00..3607e82b 100644 --- a/docs/storage-architecture.md +++ b/docs/storage-architecture.md @@ -99,8 +99,12 @@ name. Publishers do not always honor that: on 2026-09-02 the IRS re-published `22in05ira.xlsx` and `22in06ira.xlsx` under their existing URLs (PolicyEngine/chronicle#225). -`fetch-artifact` therefore compares the recorded key's `{sha256}/{filename}` -tail with the bytes it just fetched, before it writes anything: +`fetch-artifact` therefore compares the entry's recorded identity with the +bytes it just fetched, before it writes anything. That identity is the recorded +key's `{sha256}/{filename}` tail once the entry has been published, and the +entry's own declared `sha256` before then — an entry that was registered +without an upload, or whose upload failed, still identifies its bytes, and gets +the same protection: - **Identical** — the recorded block is preserved exactly, whichever bucket is configured now. Re-fetching after the bucket rename copies bytes; it does not @@ -153,6 +157,31 @@ as history. A local file the recorded object does not hold is reported as `recorded_r2_identity_mismatch` and nothing is uploaded: registering a revision is a fetch-time decision, not a publish-time rewrite. +### Which manifest + +Most packages keep one `manifest.yaml`. A publisher directory that feeds +several source packages keeps one manifest each — +`db/data/irs_soi/ira_contributions/` holds +`manifest_traditional_source_package.yaml` beside +`manifest_roth_source_package.yaml` — and the entry being revised lives in +exactly one of them. `fetch-artifact --manifest ` selects it; +defaulting to `manifest.yaml` there would write a third manifest neither +package reads, and the recorded block would never be compared at all. The name +must be a filename inside `--out-dir`, not a path. + +### What a recorded block has to say + +A `storage.r2` block's `provider`, `bucket`, `key` and `uri` all describe one +object, so every field that is present is cross-checked against every other: +the key against the URI's path, the bucket against its authority, the provider +against its scheme, and the resulting key against the content-addressed +`{sha256}/{filename}` shape. A block whose fields disagree does not answer +"which bytes does this entry claim R2 holds", so it is an error rather than +something to preserve or publish under. Likewise a manifest that parses as +anything other than a mapping is refused rather than treated as absent — +reading it as absent would let the next fetch replace the file with a single +entry. + ## Relational Registry Contract The hosted `chronicle` schema should be the lookup surface for Chronicle, not the place @@ -254,7 +283,7 @@ what a naive fallback would do: | `CHRONICLE_SOURCE_ARTIFACT_FETCH` | `LEDGER_SOURCE_ARTIFACT_FETCH` | Fetch a missing manifest artifact from its `source_url` during a build | | `CHRONICLE_PE_US_DATA_ROOT` | `LEDGER_PE_US_DATA_ROOT` | Local checkout root for PE US source inventory | | `CHRONICLE_PE_UK_DATA_ROOT` | `LEDGER_PE_UK_DATA_ROOT` | Local checkout root for PE UK source inventory | -| `CHRONICLE_SCHEMA` | `POLICYENGINE_LEDGER_SCHEMA` | Postgres schema the Supabase client reads and writes | +| `CHRONICLE_SCHEMA` | `POLICYENGINE_LEDGER_SCHEMA`, `LEDGER_SCHEMA` | Postgres schema the Supabase client reads and `load-supabase-mirror` writes; defaults to `ledger` | | `CHRONICLE_R2_RAW_BUCKET` | `LEDGER_R2_RAW_BUCKET` | Raw R2 archive bucket; defaults to `ledger-raw` | | `CHRONICLE_R2_DERIVED_BUCKET` | `LEDGER_R2_DERIVED_BUCKET` | Derived R2 archive bucket; defaults to `ledger-derived` | @@ -272,6 +301,12 @@ the variable that overrides the schema; the schema still defaults to `ledger`, and the mirror table names are unchanged. Those move in a later slice coordinated with the CI writers. +Every reader of the setting resolves it the same way, at call time. That +includes the writer: `load-supabase-mirror` takes its `--schema` default from +`CHRONICLE_SCHEMA`, so setting the variable to rehearse a cutover moves the +mirror load with the client rather than leaving it pointed at `ledger`. An +explicit `--schema` still wins. + ## Bucket Cutover Chronicle's operational stores migrate by dual-run From d4ece34d1d1478a2428f9edbfc1b180179f56476 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 2 Sep 2026 13:39:25 -0400 Subject: [PATCH 18/22] Name the strict storage reader for the pair it belongs to _validated_recorded_storage sits next to _validated_recorded_r2 and above its only caller, instead of a _recorded_storage_block that read like a variant of the lenient accessor. Co-Authored-By: Claude Fable 5.1 --- chronicle/artifacts.py | 44 +++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/chronicle/artifacts.py b/chronicle/artifacts.py index 514acaa2..6c6c6719 100644 --- a/chronicle/artifacts.py +++ b/chronicle/artifacts.py @@ -1221,9 +1221,9 @@ def _read_manifest(manifest_path: Path) -> dict[str, Any]: try: payload = yaml.safe_load(manifest_path.read_text(encoding="utf-8")) except yaml.YAMLError as exc: - raise MalformedManifestError(f"{manifest_path} is not valid YAML: {exc}") from ( - exc - ) + raise MalformedManifestError( + f"{manifest_path} is not valid YAML: {exc}" + ) from exc if payload is None: return {} if not isinstance(payload, dict): @@ -1274,6 +1274,24 @@ def _split_r2_uri(uri: str) -> tuple[str, str, str] | None: return (provider, bucket, key) +def _validated_recorded_storage( + spec: Any, + *, + manifest_path: Path, + year: Any, +) -> dict[str, Any]: + """Return the entry's ``storage`` mapping, refusing a malformed one.""" + if not isinstance(spec, dict) or "storage" not in spec: + return {} + storage = spec["storage"] + if not isinstance(storage, dict): + raise MalformedManifestError( + f"{manifest_path} entry {year!r} storage must be a mapping; it is a " + f"{type(storage).__name__}." + ) + return storage + + def _validated_recorded_r2( spec: Any, *, @@ -1289,7 +1307,7 @@ def _validated_recorded_r2( field and trusting the rest is what lets a block that says two different things survive a preserve or a publish. """ - storage = _recorded_storage_block(spec, manifest_path=manifest_path, year=year) + storage = _validated_recorded_storage(spec, manifest_path=manifest_path, year=year) if "r2" not in storage: return None block = storage["r2"] @@ -1370,24 +1388,6 @@ def _validated_recorded_r2( ) -def _recorded_storage_block( - spec: Any, - *, - manifest_path: Path, - year: Any, -) -> dict[str, Any]: - """Return the entry's ``storage`` mapping, refusing a malformed one.""" - if not isinstance(spec, dict) or "storage" not in spec: - return {} - storage = spec["storage"] - if not isinstance(storage, dict): - raise MalformedManifestError( - f"{manifest_path} entry {year!r} storage must be a mapping; it is a " - f"{type(storage).__name__}." - ) - return storage - - def _recorded_identity( spec: Any, *, From 3d7327673ff0cc7b0422a63e43094043db7b8fa4 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 2 Sep 2026 13:40:10 -0400 Subject: [PATCH 19/22] Record what each round-2 fix does and how it was reproduced Co-Authored-By: Claude Fable 5.1 --- PROGRESS.md | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/PROGRESS.md b/PROGRESS.md index e51ea2ca..79bafcd9 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -118,6 +118,56 @@ with a regression test on this branch. Plan, in dependency order: and uri; every key is content-addressed; every declared `sha256`/`filename` agrees with its key tail; no `uri` contradicts its `key`. Strict locator validation therefore refuses nothing that is tracked today. +- All seven findings are applied, each with a regression test, and each + reproduced against this branch's previous head (`34d1d0f`) first. + +### What each fix does + +1. `chronicle/env.py` gains `default_chronicle_schema()`: one home for the + `CHRONICLE_SCHEMA` -> `POLICYENGINE_LEDGER_SCHEMA` -> `LEDGER_SCHEMA` -> + `"ledger"` ladder. `load_supabase_mirror`, its harness wrapper and the + `--schema` CLI default all resolve through it when no schema is supplied; + an explicit `--schema` still wins. Defaults unchanged. +2. `chronicle/consumer_contract.py` matches the whole final dot-segment of a + `source_record_id` against both `ledger_derived` and `chronicle_derived`. +3. `fetch-artifact --manifest ` selects which of a package's + manifests the entry belongs to (default `manifest.yaml`); the name must be + a filename inside `--out-dir`. +4. Revision protection now compares against the entry's recorded identity -- + the recorded key's `{sha256}/{filename}` once published, the declared + `sha256` before that -- so a registered-but-unpublished entry, or one whose + upload failed, is protected exactly like a published one. +5. `_validated_recorded_r2` cross-checks every supplied locator field against + every other and against the content-addressed key shape. A contradiction is + `RecordedR2LocatorError` at fetch time and `recorded_r2_locator_invalid` at + publish time, never a preserved block. +6. `_read_manifest` refuses a non-mapping or unparseable document + (`MalformedManifestError`) before the publisher is read at all; + `inventory-artifacts` and `publish-raw` report it instead of crashing. +7. `db.supabase_client` resolves both schemas per call rather than at import, + and `tests/conftest.py` strips the rename window in `pytest_configure`, so + no module can read or warn from an operator's shell during collection. + +All four refusals share a `SourceArtifactManifestError` base, so the +`fetch-artifact` CLI reports every one as exit 1 with nothing written. + +### Reproduced against `34d1d0f` (the round-1 head) + +Running the same operations against a checkout of the previous head: + +1. `load_supabase_mirror` default `schema='ledger'`; with + `CHRONICLE_SCHEMA=chronicle_probe` the load still reports `schema='ledger'`. +2. `'.chronicle_derived'.endswith('.ledger_derived')` is False: the boundary + never fired for the chronicle spelling. +3. `fetch_source_artifact()` rejects `manifest_filename` as an unexpected + keyword; a fetch into `ira_contributions/` writes `manifest.yaml`. +4. A fetch of different bytes over a registered (unpublished) entry was + accepted silently: the entry's `sha256` was rewritten with no refusal. +5. A block whose `key` and `uri` named different objects was preserved + verbatim, key sha `c63744a4...` beside uri sha `1e9b3fdb...`. +6. A list-valued `manifest.yaml` was overwritten by the fetch. +7. Importing `db.supabase_client` under `LEDGER_SCHEMA=zzz` bound + `LEDGER_SCHEMA='zzz'` and emitted a `FutureWarning` at collection. ## Verification From b1d86c125aa8dfe69887dad450ca5cff70befa3f Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 2 Sep 2026 14:15:39 -0400 Subject: [PATCH 20/22] Say which fields a recorded block actually needs The message named key and bucket while the check also requires provider, so a hand-written block missing it read as a contradiction between the error and the rule. Co-Authored-By: Claude Fable 5.1 --- chronicle/artifacts.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/chronicle/artifacts.py b/chronicle/artifacts.py index 6c6c6719..feb7c9cb 100644 --- a/chronicle/artifacts.py +++ b/chronicle/artifacts.py @@ -1365,7 +1365,8 @@ def _validated_recorded_r2( if missing: raise RecordedR2LocatorError( f"{where}: records no {', '.join(missing)}. A recorded block has to " - "locate its object, by key and bucket or by uri." + "locate its object: provider, bucket and key, or a uri that " + "supplies them." ) segments = key.split("/") From e16d8aa152087aa2fcbfc271e7f5f5f3b6055685 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 2 Sep 2026 14:16:15 -0400 Subject: [PATCH 21/22] Say what --record-revision is an opt-in over The flag's help still described the recorded R2 object alone, which is no longer the only identity a fetch is refused against. Co-Authored-By: Claude Fable 5.1 --- chronicle/harness.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/chronicle/harness.py b/chronicle/harness.py index 979c04d7..f8ce6f01 100644 --- a/chronicle/harness.py +++ b/chronicle/harness.py @@ -923,8 +923,9 @@ def main(argv: list[str] | None = None) -> int: "Register a publisher revision: the fetched bytes get their own " "content-addressed key under the configured bucket and the " "superseded object moves to storage.previous_r2. Without this " - "flag, bytes that disagree with the recorded R2 object are " - "refused." + "flag, bytes that disagree with what the entry identifies -- its " + "declared sha256, or its recorded content-addressed key once " + "published -- are refused." ), ) artifact_parser.add_argument( From ea67f0a066c1a1c15fab47225549419286d45d34 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 2 Sep 2026 14:16:41 -0400 Subject: [PATCH 22/22] Say when the manifest guards actually run They run before the publisher is read, and again before the manifest is rewritten; the base class claimed the second one also predated the cache write. Co-Authored-By: Claude Fable 5.1 --- chronicle/artifacts.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/chronicle/artifacts.py b/chronicle/artifacts.py index feb7c9cb..32858d8b 100644 --- a/chronicle/artifacts.py +++ b/chronicle/artifacts.py @@ -76,8 +76,10 @@ def default_r2_derived_bucket() -> str: class SourceArtifactManifestError(RuntimeError): """A manifest refuses the write a fetch is about to make. - Every subclass is raised before anything is downloaded, cached, uploaded or - rewritten, so a refusal leaves the package exactly as it was. + The checks that raise these run before the publisher is read, so an + ordinary refusal costs nothing and leaves the package exactly as it was. + They are repeated immediately before the manifest is rewritten, so no + caller can reach a false-provenance write by another route. """