From 7baae8a74839647fc6c0aefb6cbfe8d3e2640748 Mon Sep 17 00:00:00 2001 From: Sebastian Date: Mon, 27 Jul 2026 19:32:04 +0200 Subject: [PATCH 1/4] fix(experiment): make run identity collision-safe ExperimentRunner decided whether a run had already completed using an identifier built only from model alias, regime, condition and seed. Changing the case count, the resolved model, the distributions configuration or a provider parameter produced the same identifier, so skip_completed=True could treat a materially different experiment as already done and return None without executing it. Separate the readable label from machine identity. run_label keeps the previous readable string; identity_sha256 is a SHA-256 over a versioned, allowlisted canonical payload and is the only thing that decides resume. run_id becomes "--<12 hex>" and stays an opaque string for existing consumers. Identity covers the case count, regime, condition, seed, a digest of the resolved model id, a digest of the parsed distributions configuration, provider generation parameters and the package version. The distributions fingerprint is semantic, so comment and formatting changes do not invalidate earlier runs while a changed parameter does. Runs that cannot prove equivalence -- no stable model id, undeclared provider parameters, unavailable package version -- are non-resumable and always execute with a warning. Failing open costs compute; failing closed silently corrupts a result set. Legacy records carrying only run_id are read but never rewritten and never treated as equivalent to a versioned identity. Unknown schemas, malformed digests and payload mismatches warn and execute. Warnings use stable codes MGI001-MGI007 on the mech_gov.experiment.identity logger and name the fields that differ without printing their values. Add LLMInterface.resume_identity_parameters(), returning None by default so existing custom providers keep working and are simply non-resumable. 95 new tests; no dependency added. --- CHANGELOG.md | 37 ++ README.md | 60 +++ src/mech_gov/experiment/identity.py | 245 ++++++++++ src/mech_gov/experiment/runner.py | 212 +++++++-- src/mech_gov/llm/base.py | 17 + src/mech_gov/llm/providers/bedrock.py | 5 + src/mech_gov/llm/providers/mock.py | 10 + .../llm/providers/openai_compatible.py | 5 + src/mech_gov/llm/providers/sagemaker.py | 5 + tests/test_identity.py | 296 ++++++++++++ tests/test_runner_identity.py | 436 ++++++++++++++++++ 11 files changed, 1296 insertions(+), 32 deletions(-) create mode 100644 src/mech_gov/experiment/identity.py create mode 100644 tests/test_identity.py create mode 100644 tests/test_runner_identity.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e34cde..14ee7ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,44 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- Experiment resume no longer skips materially different runs. `run_id` was + derived only from model alias, regime, condition and seed, so changing the + case count (or the resolved model, the distributions config, or provider + parameters) produced the same identifier and `skip_completed=True` treated the + new experiment as already completed. + ### Added +- `mech_gov.experiment.identity` — deterministic, versioned run identity. A + readable `run_label` is now separate from an `identity_sha256` digest computed + over a canonical, allowlisted payload (case count, regime, condition, seed, + hashed model ID, semantic distributions fingerprint, provider parameters, + package version). Only the full digest decides resume. +- `LLMInterface.resume_identity_parameters()` — optional hook for providers to + declare generation-affecting parameters. The default returns `None`, so + existing custom providers are unaffected and are treated as non-resumable. + Implemented for the bundled mock, OpenAI-compatible, Bedrock and SageMaker + providers. +- Results now include `run_label`, `identity_schema`, `identity_sha256` and + `identity` alongside the existing fields. +- Stable identity warning codes `MGI001`–`MGI007` on the + `mech_gov.experiment.identity` logger. Diagnostics name the fields that + differ and never print their values. +- README section documenting resume semantics, non-resumable runs and legacy + result files. + +### Changed +- `run_id` is now `--<12 hex>` for resumable runs. It remains an + opaque string prefixed by the previous readable label; non-resumable runs keep + the bare label. +- Runs whose identity cannot be established (no stable model ID, undeclared + provider parameters, missing package version) always execute rather than + risking a wrong skip. +- Legacy records without identity metadata are read, never rewritten, and never + treated as equivalent to a versioned identity. +- The distributions configuration is loaded before the completion check, since + its content participates in run identity. + - Open-source readiness scaffolding: - Apache 2.0 `NOTICE`, expanded `CONTRIBUTING.md` (CLA + issue/PR flow), `CODE_OF_CONDUCT.md`, `SECURITY.md`, `CODEOWNERS` diff --git a/README.md b/README.md index 8138a69..d4ce01c 100644 --- a/README.md +++ b/README.md @@ -138,6 +138,66 @@ mech_gov_framework/ └── tests/ # offline tests (mock provider) ``` +## Experiment resume and run identity + +`ExperimentRunner` is resumable: re-running an experiment skips work that is +already on disk. Each result therefore carries two different things. + +| Field | Purpose | +|---|---| +| `run_label` | Readable name, e.g. `mock-R2-S0-seed42`. Never decides resume. | +| `identity_sha256` | Full SHA-256 over the canonical identity payload. **Decides resume.** | +| `identity` | The canonical payload itself, so the digest is reproducible. | +| `run_id` | `run_label` plus the first 12 digest characters, for display and paths. | + +A run is skipped **only** when a stored record carries the exact same verified +`identity_sha256`. The identity covers the case count, regime, condition, seed, +a digest of the resolved model ID, a digest of the *loaded* distributions +configuration, the provider's declared generation parameters, and the package +version. Changing any of them re-runs the experiment instead of silently +reusing an older result. + +The distributions fingerprint is taken from the parsed configuration, not the +file bytes, so reformatting or editing comments does not invalidate previous +runs, while a changed parameter does. Equivalent content at a different path +fingerprints identically. + +### Non-resumable runs + +Some runs cannot prove equivalence. They always execute, and log why: + +- the provider cannot expose a stable, non-empty `model_id`; +- the provider does not declare its generation parameters (see below); +- the package version is unavailable. + +Failing open is deliberate: recomputing a run costs time, whereas wrongly +skipping one silently corrupts a result set. + +### Declaring provider parameters + +`LLMInterface.resume_identity_parameters()` returns the generation-affecting +values a provider wants included in run identity, or `None`. The base +implementation returns `None`, so existing custom providers keep working and are +simply treated as non-resumable. Return only public, result-affecting values — +never an API key, base URL, credential, profile name, endpoint, timeout or path. + +```python +class MyProvider(LLMInterface): + def resume_identity_parameters(self): + return {"top_p": self._top_p} +``` + +### Legacy result files + +Records written before versioned identity carry a `run_id` and nothing else. +They are never treated as equivalent to a versioned identity: a matching +readable label logs `MGI001` and the run executes again. Existing files are read +but never rewritten or deleted. + +Identity and resume warnings use the `mech_gov.experiment.identity` logger with +stable codes `MGI001`–`MGI007`. They name the configuration fields that differ +but never print their values. + ## CLI ```bash diff --git a/src/mech_gov/experiment/identity.py b/src/mech_gov/experiment/identity.py new file mode 100644 index 0000000..e470eeb --- /dev/null +++ b/src/mech_gov/experiment/identity.py @@ -0,0 +1,245 @@ +# Copyright (c) 2026 Santander Group +# SPDX-License-Identifier: Apache-2.0 +"""Deterministic, versioned identity for experiment runs. + +The readable run label (``mock-R2-S0-seed42``) is convenient for humans but too +coarse to decide whether a run has already completed: it omits the case count, +the resolved model identifier, the distribution configuration, and provider +generation parameters. Two materially different experiments can therefore share +a label, and the second one can be skipped silently. + +This module separates the two concerns: ``label`` stays the legacy readable +string, while ``digest`` is a SHA-256 over a versioned, allowlisted canonical +payload. Only the full digest decides equality — the truncated digest embedded +in ``run_id`` exists for display and filesystem convenience. + +The module is deliberately inert. It never reads environment variables, never +touches the filesystem, never invokes a model, and never accepts raw provider +configuration. Callers pass already-resolved, allowlisted values, so no secret, +endpoint, prompt, case, or path can reach an identity payload or a log line. +""" + +from __future__ import annotations + +import hashlib +import json +import logging +import math +import re +from collections.abc import Mapping, Sequence +from dataclasses import dataclass +from decimal import Decimal +from typing import Any + +logger = logging.getLogger("mech_gov.experiment.identity") + +#: Version of the canonical payload layout. Bump when fields change meaning. +IDENTITY_SCHEMA = 1 + +#: Payload versions this build is able to verify. +SUPPORTED_IDENTITY_SCHEMAS = frozenset({IDENTITY_SCHEMA}) + +#: Hex characters of the digest shown in ``run_id``. Display only. +DISPLAY_DIGEST_LENGTH = 12 + +#: Exact set of keys a schema-1 payload must contain. +PAYLOAD_FIELDS = frozenset( + "identity_schema model_name model_id_sha256 regime_name condition seed " + "cases_per_condition distributions_sha256 provider_parameters package_version".split() +) + +#: Stable warning codes. Every condition below resolves to "execute". +WARNING_SUMMARIES = { + "MGI001": "legacy result with the same run label carries no versioned identity", + "MGI002": "run label matches a different verified identity", + "MGI003": "stored identity uses an unsupported schema", + "MGI004": "stored identity payload or digest failed validation", + "MGI005": "identity digest is claimed by conflicting payloads", + "MGI006": "stable model or provider identity is unavailable", + "MGI007": "package provenance is unavailable", +} + +_FULL_DIGEST_RE = re.compile(r"\A[0-9a-f]{64}\Z") + + +class IdentityError(ValueError): + """A configuration value cannot take part in a deterministic identity.""" + + +def _canonical_number(value: int | float, path: str) -> str: + """Render a finite number so that ``1``, ``1.0`` and ``-0.0`` behave sanely.""" + if isinstance(value, float) and not math.isfinite(value): + raise IdentityError(f"non-finite numeric value at {path or ''}") + as_decimal = Decimal(value) if isinstance(value, int) else Decimal(repr(value)) + if as_decimal == 0: + return "0" + return str(as_decimal.normalize()) + + +def _normalize(value: Any, path: str = "") -> Any: + """Recursively rewrite a loaded value into a type-tagged canonical form. + + Scalars are tagged so that ``None``, ``False`` and the strings ``"z:"`` or + ``"b:false"`` can never collide. Mapping keys must be strings and are + ordered by the serializer; list order stays significant. + """ + if value is None: + return "z:" + if isinstance(value, bool): # bool is a subclass of int — check it first + return "b:true" if value else "b:false" + if isinstance(value, str): + return "s:" + value + if isinstance(value, (int, float)): + return "n:" + _canonical_number(value, path) + if isinstance(value, Mapping): + normalized = {} + for key, item in value.items(): + if not isinstance(key, str): + raise IdentityError(f"non-string mapping key at {path or ''}") + normalized[key] = _normalize(item, f"{path}.{key}") + return normalized + if isinstance(value, (list, tuple)): + return [_normalize(item, f"{path}[{index}]") for index, item in enumerate(value)] + raise IdentityError(f"unsupported {type(value).__name__} value at {path or ''}") + + +def _canonical_text(value: Any) -> str: + """Serialize a normalized value deterministically.""" + return json.dumps( + _normalize(value), + sort_keys=True, + separators=(",", ":"), + ensure_ascii=False, + allow_nan=False, + ) + + +def _sha256_text(text: str) -> str: + return hashlib.sha256(text.encode("utf-8")).hexdigest() + + +def canonical_bytes(payload: Mapping[str, Any]) -> bytes: + """Return the exact bytes a payload digest is computed over.""" + return _canonical_text(dict(payload)).encode("utf-8") + + +def digest_payload(payload: Mapping[str, Any]) -> str: + """Return the authoritative full SHA-256 digest of a payload.""" + return hashlib.sha256(canonical_bytes(payload)).hexdigest() + + +def distributions_digest(distributions_config: Mapping[str, Any]) -> str: + """Fingerprint the *semantics* of a loaded distributions configuration. + + The input is the mapping returned by ``load_distributions``, so comments, + formatting, key order and the source path are all excluded: identical + content at another path fingerprints identically, and changed content at + the same path does not. + """ + return _sha256_text(_canonical_text(dict(distributions_config))) + + +@dataclass(frozen=True) +class RunIdentity: + """A readable label paired with the digest that actually decides equality.""" + + label: str + payload: dict[str, Any] + digest: str + + @property + def run_id(self) -> str: + """Readable label plus a truncated digest. Never used for equality.""" + return f"{self.label}--{self.digest[:DISPLAY_DIGEST_LENGTH]}" + + +def build_run_identity( + *, + label: str, + model_name: str, + model_id: str, + regime_name: str, + condition: str, + seed: int, + cases_per_condition: int, + distributions_config: Mapping[str, Any], + provider_parameters: Mapping[str, Any], + package_version: str, +) -> RunIdentity: + """Build a versioned identity from already-resolved, allowlisted inputs. + + ``model_id`` is hashed in memory and never persisted in the payload. The + caller is responsible for establishing that it is stable and non-empty. + """ + if not model_id: + raise IdentityError("model_id must be a stable non-empty string") + if not package_version: + raise IdentityError("package_version must be a non-empty string") + payload: dict[str, Any] = { + "identity_schema": IDENTITY_SCHEMA, + "model_name": model_name, + "model_id_sha256": _sha256_text(model_id), + "regime_name": regime_name, + "condition": condition, + "seed": seed, + "cases_per_condition": cases_per_condition, + "distributions_sha256": distributions_digest(distributions_config), + "provider_parameters": dict(provider_parameters), + "package_version": package_version, + } + return RunIdentity(label=label, payload=payload, digest=digest_payload(payload)) + + +def verified_stored_digest(record: Mapping[str, Any]) -> tuple[str | None, str | None]: + """Validate one stored result record before it may cause a skip. + + Returns ``(digest, warning_code)``. A digest is returned only when the + schema is supported, the payload is complete, the stored digest is a + lowercase SHA-256, and re-canonicalizing the payload reproduces it. + ``(None, None)`` means the record is legacy and carries no identity at all. + """ + schema = record.get("identity_schema") + payload = record.get("identity") + stored = record.get("identity_sha256") + if schema is None and payload is None and stored is None: + return None, None + if isinstance(schema, bool) or not isinstance(schema, int): + return None, "MGI003" + if schema not in SUPPORTED_IDENTITY_SCHEMAS: + return None, "MGI003" + if not isinstance(stored, str) or not _FULL_DIGEST_RE.match(stored): + return None, "MGI004" + if not isinstance(payload, Mapping) or set(payload) != set(PAYLOAD_FIELDS): + return None, "MGI004" + if payload.get("identity_schema") != schema: + return None, "MGI004" + try: + recomputed = digest_payload(payload) + except IdentityError: + return None, "MGI004" + return (stored, None) if recomputed == stored else (None, "MGI004") + + +def changed_fields(current: Mapping[str, Any], stored: Mapping[str, Any]) -> list[str]: + """Return the sorted names of payload fields that differ. Never the values.""" + names = set(current) | set(stored) + return sorted(name for name in names if current.get(name) != stored.get(name)) + + +class IdentityWarnings: + """Emits each identity warning at most once per completion scan.""" + + def __init__(self) -> None: + self._seen: set[tuple[str, str]] = set() + + def emit(self, code: str, label: str, fields: Sequence[str] = ()) -> None: + """Log one deduplicated warning naming changed fields but no values.""" + if (code, label) in self._seen: + return + self._seen.add((code, label)) + logger.warning( + "[%s] %s; fields=%s; action=execute", + code, + WARNING_SUMMARIES[code], + ",".join(fields), + ) diff --git a/src/mech_gov/experiment/runner.py b/src/mech_gov/experiment/runner.py index 03d7e8a..3a56f78 100644 --- a/src/mech_gov/experiment/runner.py +++ b/src/mech_gov/experiment/runner.py @@ -15,12 +15,13 @@ import logging import subprocess import time -from collections.abc import Callable +from collections.abc import Callable, Iterator, Mapping from dataclasses import dataclass, field from datetime import datetime, timezone from pathlib import Path from typing import Any +import mech_gov from mech_gov.data.banking_case import ( BankingCase, Decision, @@ -30,6 +31,15 @@ from mech_gov.data.generator import generate_dataset from mech_gov.data.ground_truth import assign_ground_truth from mech_gov.data.stress import apply_stress +from mech_gov.experiment.identity import ( + IdentityError, + IdentityWarnings, + RunIdentity, + build_run_identity, + canonical_bytes, + changed_fields, + verified_stored_digest, +) from mech_gov.governance.r1_text_only import R1TextOnly from mech_gov.governance.r2_mechanical import R2Mechanical from mech_gov.governance.r3_adaptive import R3Adaptive @@ -97,6 +107,9 @@ class ExperimentResult: timestamp: str = "" code_version: str = "" hyperparameters: dict[str, Any] = field(default_factory=dict) + run_label: str = "" + identity_sha256: str = "" + identity: dict[str, Any] = field(default_factory=dict) def to_dict(self) -> dict[str, Any]: d = { @@ -118,6 +131,12 @@ def to_dict(self) -> dict[str, Any]: d["code_version"] = self.code_version if self.hyperparameters: d["hyperparameters"] = self.hyperparameters + if self.run_label: + d["run_label"] = self.run_label + if self.identity and self.identity_sha256: + d["identity_schema"] = self.identity.get("identity_schema") + d["identity_sha256"] = self.identity_sha256 + d["identity"] = self.identity return d @@ -134,9 +153,143 @@ def create_regime(regime_name: str, **kwargs) -> GovernanceRegime: def _make_run_id(model: str, regime: str, condition: str, seed: int) -> str: + """Build the human-readable run label. + + This string is intentionally coarse: it does **not** decide whether a run + has already completed. See :mod:`mech_gov.experiment.identity`. + """ return f"{model}-{regime}-{condition}-seed{seed}" +def _package_version() -> str: + """Package version used as code provenance inside run identity.""" + return getattr(mech_gov, "__version__", "") or "" + + +@dataclass +class _CompletionIndex: + """Verified identities and legacy labels found in stored result files.""" + + digests: set[str] = field(default_factory=set) + canonical: dict[str, bytes] = field(default_factory=dict) + conflicts: set[str] = field(default_factory=set) + payloads_by_label: dict[str, list[dict[str, Any]]] = field(default_factory=dict) + legacy_labels: set[str] = field(default_factory=set) + + def add(self, record: Mapping[str, Any], warnings: IdentityWarnings) -> None: + """Index one stored record, warning instead of trusting bad identities.""" + label = record.get("run_label") or record.get("run_id") or "" + if not isinstance(label, str): + return + digest, code = verified_stored_digest(record) + if code is not None: + warnings.emit(code, label) + return + if digest is None: + if label: # legacy record: readable label only, never an identity + self.legacy_labels.add(label) + return + blob = canonical_bytes(record["identity"]) + if digest in self.digests: + # Defensive: verified digests bind their payload, so a mismatch here + # means validation was bypassed. Duplicate copies are not conflicts. + if self.canonical[digest] != blob: + self.conflicts.add(digest) + warnings.emit("MGI005", label) + return + self.digests.add(digest) + self.canonical[digest] = blob + self.payloads_by_label.setdefault(label, []).append(dict(record["identity"])) + + +def _result_files(results_dir: Path) -> Iterator[Path]: + """Yield the legacy flat JSONL files and the per-model ones.""" + yield from results_dir.glob("*.jsonl") + for subdir in results_dir.iterdir(): + if subdir.is_dir(): + yield from subdir.glob("*.jsonl") + + +def _scan_completion_index(results_dir: Path, warnings: IdentityWarnings) -> _CompletionIndex: + """Build the completion index, tolerating malformed JSONL lines.""" + index = _CompletionIndex() + for path in _result_files(results_dir): + with open(path, encoding="utf-8") as handle: + for line in handle: + line = line.strip() + if not line: + continue + try: + record = json.loads(line) + except json.JSONDecodeError: + continue + if isinstance(record, Mapping): + index.add(record, warnings) + return index + + +def _resolve_identity( + config: RunConfig, + llm: LLMInterface, + label: str, + dist_config: Mapping[str, Any], + warnings: IdentityWarnings, +) -> RunIdentity | None: + """Build the versioned identity, or return None when the run is non-resumable.""" + try: + model_id = llm.model_id + except Exception: + model_id = "" + getter = getattr(llm, "resume_identity_parameters", None) + parameters = getter() if callable(getter) else None + if not model_id or parameters is None: + warnings.emit("MGI006", label) + return None + version = _package_version() + if not version: + warnings.emit("MGI007", label) + return None + try: + return build_run_identity( + label=label, + model_name=config.model_name, + model_id=model_id, + regime_name=config.regime_name, + condition=config.condition, + seed=config.seed, + cases_per_condition=config.cases_per_condition, + distributions_config=dist_config, + provider_parameters=parameters, + package_version=version, + ) + except IdentityError: + warnings.emit("MGI006", label) + return None + + +def _decide_skip( + identity: RunIdentity | None, + label: str, + index: _CompletionIndex, + warnings: IdentityWarnings, +) -> bool: + """Decide whether a requested run may be treated as already completed.""" + if identity is None: + return False + if identity.digest in index.conflicts: + warnings.emit("MGI005", label) + return False + if identity.digest in index.digests: + return True + stored = index.payloads_by_label.get(label) + if stored: + differing = {name for prior in stored for name in changed_fields(identity.payload, prior)} + warnings.emit("MGI002", label, sorted(differing)) + elif label in index.legacy_labels: + warnings.emit("MGI001", label) + return False + + class ExperimentRunner: """Orchestrates experimental runs with resumability. @@ -157,33 +310,25 @@ def __init__( self._dist_config_path = distributions_config_path self._on_run_complete = on_run_complete - def _completed_run_ids(self) -> set: - """Scan results dir (and model subdirs) for already-completed run IDs.""" - completed = set() - # Scan flat JSONL files in results_dir (legacy) - for jsonl_file in self._results_dir.glob("*.jsonl"): - completed.update(self._scan_jsonl(jsonl_file)) - # Scan model subdirectories - for subdir in self._results_dir.iterdir(): - if subdir.is_dir(): - for jsonl_file in subdir.glob("*.jsonl"): - completed.update(self._scan_jsonl(jsonl_file)) - return completed - - @staticmethod - def _scan_jsonl(path: Path) -> set: - """Extract run_ids from a JSONL file.""" - ids = set() - with open(path, encoding="utf-8") as f: - for line in f: - line = line.strip() - if not line: - continue - try: - ids.add(json.loads(line).get("run_id", "")) - except json.JSONDecodeError: - continue - return ids + def _completion_index(self, warnings: IdentityWarnings) -> _CompletionIndex: + """Scan results dir (and model subdirs) for verified completed identities.""" + return _scan_completion_index(self._results_dir, warnings) + + def _prepare_run( + self, + config: RunConfig, + llm: LLMInterface, + label: str, + skip_completed: bool, + ) -> tuple[dict[str, Any], RunIdentity | None, bool]: + """Resolve identity inputs before the completion check and decide on skipping.""" + dist_config = load_distributions(config.distributions_config_path) + warnings = IdentityWarnings() + identity = _resolve_identity(config, llm, label, dist_config, warnings) + skip = skip_completed and _decide_skip( + identity, label, self._completion_index(warnings), warnings + ) + return dist_config, identity, skip def _save_result(self, result: ExperimentResult, filename: str = "baseline.jsonl"): """Append a single run result to model-specific JSONL + legacy flat file.""" @@ -393,14 +538,16 @@ def run_single( Returns: ExperimentResult or None if skipped """ - run_id = _make_run_id( + label = _make_run_id( config.model_name, config.regime_name, config.condition, config.seed, ) - if skip_completed and run_id in self._completed_run_ids(): + dist_config, identity, skip = self._prepare_run(config, llm, label, skip_completed) + run_id = identity.run_id if identity is not None else label + if skip: logger.info("SKIP %s (already completed)", run_id) return None @@ -413,8 +560,6 @@ def run_single( config.cases_per_condition, ) - # Generate dataset - dist_config = load_distributions(config.distributions_config_path) try: condition_enum = StressCondition(config.condition) except ValueError: @@ -472,6 +617,9 @@ def run_single( model_id=llm_model_id, timestamp=datetime.now(timezone.utc).isoformat(), code_version=_get_code_version(), + run_label=label, + identity_sha256=identity.digest if identity is not None else "", + identity=dict(identity.payload) if identity is not None else {}, ) self._save_result(result) diff --git a/src/mech_gov/llm/base.py b/src/mech_gov/llm/base.py index d6ab204..e0fad36 100644 --- a/src/mech_gov/llm/base.py +++ b/src/mech_gov/llm/base.py @@ -11,6 +11,7 @@ from abc import ABC, abstractmethod from dataclasses import dataclass +from typing import Any @dataclass @@ -63,3 +64,19 @@ def model_id(self) -> str: def provider(self) -> str: """Return the provider name (e.g. 'bedrock', 'sagemaker').""" return "unknown" + + def resume_identity_parameters(self) -> dict[str, Any] | None: + """Return generation parameters that must take part in run identity. + + The experiment runner uses this to decide whether a previously stored + run is really equivalent to the one being requested. Return a mapping of + allowlisted, public, result-affecting values — never an API key, base + URL, credential, profile name, endpoint, timeout, or path. Opaque values + such as user-supplied callables cannot be fingerprinted; return ``None`` + for them. + + ``None`` (the default, so existing subclasses keep working unchanged) + means the provider cannot prove equivalence. Such runs are treated as + non-resumable: they always execute instead of being skipped. + """ + return None diff --git a/src/mech_gov/llm/providers/bedrock.py b/src/mech_gov/llm/providers/bedrock.py index 4855c93..a9534fd 100644 --- a/src/mech_gov/llm/providers/bedrock.py +++ b/src/mech_gov/llm/providers/bedrock.py @@ -17,6 +17,7 @@ import logging import random import time +from typing import Any from mech_gov.llm.base import LLMInterface, LLMResponse @@ -90,6 +91,10 @@ def model_id(self) -> str: def provider(self) -> str: return "bedrock" + def resume_identity_parameters(self) -> dict[str, Any] | None: + """Only ``top_p`` affects results; region, profile and retries do not.""" + return {"top_p": self._top_p} + def invoke( self, system_prompt: str, diff --git a/src/mech_gov/llm/providers/mock.py b/src/mech_gov/llm/providers/mock.py index da9bf11..a254914 100644 --- a/src/mech_gov/llm/providers/mock.py +++ b/src/mech_gov/llm/providers/mock.py @@ -8,9 +8,11 @@ from __future__ import annotations +import hashlib import json import time from collections.abc import Callable +from typing import Any from mech_gov.llm.base import LLMInterface, LLMResponse @@ -70,6 +72,14 @@ def model_id(self) -> str: def provider(self) -> str: return "mock" + def resume_identity_parameters(self) -> dict[str, Any] | None: + """Fingerprint the canned responses; a custom ``responder`` is opaque.""" + if self._responder is not None: + return None + canned = self._responses if self._responses else [self._response] + digest = hashlib.sha256("\0".join(canned).encode("utf-8")).hexdigest() + return {"responses_sha256": digest} + def invoke( self, system_prompt: str, diff --git a/src/mech_gov/llm/providers/openai_compatible.py b/src/mech_gov/llm/providers/openai_compatible.py index 0f66acf..b273008 100644 --- a/src/mech_gov/llm/providers/openai_compatible.py +++ b/src/mech_gov/llm/providers/openai_compatible.py @@ -20,6 +20,7 @@ import time import urllib.error import urllib.request +from typing import Any from mech_gov.llm.base import LLMInterface, LLMResponse @@ -62,6 +63,10 @@ def model_id(self) -> str: def provider(self) -> str: return "openai_compatible" + def resume_identity_parameters(self) -> dict[str, Any] | None: + """Only ``top_p`` affects results; base URL, key, path and timeout do not.""" + return {"top_p": self._top_p} + def invoke( self, system_prompt: str, diff --git a/src/mech_gov/llm/providers/sagemaker.py b/src/mech_gov/llm/providers/sagemaker.py index 61aab01..c5cefed 100644 --- a/src/mech_gov/llm/providers/sagemaker.py +++ b/src/mech_gov/llm/providers/sagemaker.py @@ -17,6 +17,7 @@ import json import logging import time +from typing import Any from mech_gov.llm.base import LLMInterface, LLMResponse @@ -82,6 +83,10 @@ def model_id(self) -> str: def provider(self) -> str: return "sagemaker" + def resume_identity_parameters(self) -> dict[str, Any] | None: + """Only ``top_p`` affects results; the endpoint is already in ``model_id``.""" + return {"top_p": self._top_p} + def invoke( self, system_prompt: str, diff --git a/tests/test_identity.py b/tests/test_identity.py new file mode 100644 index 0000000..6387c30 --- /dev/null +++ b/tests/test_identity.py @@ -0,0 +1,296 @@ +# Copyright (c) 2026 Santander Group +# SPDX-License-Identifier: Apache-2.0 +"""Unit tests for deterministic, versioned run identity.""" + +from __future__ import annotations + +import datetime +import json +import logging + +import pytest + +from mech_gov.experiment.identity import ( + DISPLAY_DIGEST_LENGTH, + PAYLOAD_FIELDS, + IdentityError, + IdentityWarnings, + build_run_identity, + canonical_bytes, + changed_fields, + digest_payload, + distributions_digest, + verified_stored_digest, +) + +BASE = { + "label": "mock-R2-S0-seed42", + "model_name": "mock", + "model_id": "vendor/model-v1", + "regime_name": "R2", + "condition": "S0", + "seed": 42, + "cases_per_condition": 10, + "distributions_config": {"credit_approval": {"amount": {"mu": 1.0}}}, + "provider_parameters": {"top_p": 1.0}, + "package_version": "0.1.0", +} + + +def identity(**overrides): + return build_run_identity(**{**BASE, **overrides}) + + +# -------------------------------------------------------------------------- +# Label / digest separation +# -------------------------------------------------------------------------- + + +def test_run_id_is_label_plus_truncated_digest(): + ident = identity() + assert ident.label == "mock-R2-S0-seed42" + assert len(ident.digest) == 64 + assert ident.run_id == f"mock-R2-S0-seed42--{ident.digest[:DISPLAY_DIGEST_LENGTH]}" + # The display suffix is strictly shorter than the authoritative digest. + assert len(ident.run_id.split("--")[1]) < len(ident.digest) + + +def test_identity_is_deterministic_across_instances(): + assert identity().digest == identity().digest + + +def test_payload_carries_exactly_the_declared_fields(): + assert set(identity().payload) == set(PAYLOAD_FIELDS) + + +@pytest.mark.parametrize( + "field, value", + [ + ("cases_per_condition", 1000), + ("model_id", "vendor/model-v2"), + ("model_name", "other"), + ("regime_name", "R1"), + ("condition", "S1"), + ("seed", 43), + ("distributions_config", {"credit_approval": {"amount": {"mu": 2.0}}}), + ("provider_parameters", {"top_p": 0.9}), + ("package_version", "0.2.0"), + ], +) +def test_every_result_affecting_input_changes_identity(field, value): + assert identity(**{field: value}).digest != identity().digest + + +def test_label_alone_does_not_change_identity(): + # The readable label is metadata, not part of the equivalence domain. + assert identity(label="something-else").digest == identity().digest + + +# -------------------------------------------------------------------------- +# Canonicalization +# -------------------------------------------------------------------------- + + +def test_mapping_insertion_order_is_irrelevant(): + first = distributions_digest({"a": 1, "b": {"x": 1, "y": 2}}) + second = distributions_digest({"b": {"y": 2, "x": 1}, "a": 1}) + assert first == second + + +def test_list_order_is_significant(): + assert distributions_digest({"k": [1, 2]}) != distributions_digest({"k": [2, 1]}) + + +def test_equivalent_numbers_normalize_identically(): + assert distributions_digest({"k": 1}) == distributions_digest({"k": 1.0}) + assert distributions_digest({"k": 0}) == distributions_digest({"k": -0.0}) + assert distributions_digest({"k": 100}) == distributions_digest({"k": 100.0}) + + +def test_scalar_types_never_collide(): + digests = {distributions_digest({"k": v}) for v in (None, False, True, 0, "z:", "b:false")} + assert len(digests) == 6 + + +def test_identical_content_at_another_path_is_identical(): + # distributions_digest consumes loaded content, so the source path is absent. + loaded = {"credit_approval": {"amount": {"mu": 1.0}}} + assert distributions_digest(dict(loaded)) == distributions_digest(loaded) + + +def test_utf8_content_is_preserved_not_escaped(): + payload = {"k": "café-Ω"} + assert "café-Ω".encode() in canonical_bytes(payload) + + +@pytest.mark.parametrize("bad", [float("nan"), float("inf"), float("-inf")]) +def test_non_finite_numbers_are_rejected(bad): + with pytest.raises(IdentityError, match="non-finite"): + distributions_digest({"k": bad}) + + +@pytest.mark.parametrize( + "bad", [datetime.date(2026, 1, 1), datetime.datetime(2026, 1, 1), b"bytes", {1, 2}, object()] +) +def test_unsupported_types_are_rejected(bad): + with pytest.raises(IdentityError, match="unsupported"): + distributions_digest({"k": bad}) + + +def test_non_string_mapping_keys_are_rejected(): + with pytest.raises(IdentityError, match="non-string mapping key"): + distributions_digest({1: "x"}) + + +def test_rejection_reports_the_field_path_without_the_value(): + with pytest.raises(IdentityError) as excinfo: + distributions_digest({"outer": [{"inner": float("nan")}]}) + assert ".outer[0].inner" in str(excinfo.value) + assert "nan" not in str(excinfo.value).replace("non-finite", "") + + +def test_empty_configuration_still_fingerprints(): + assert len(distributions_digest({})) == 64 + + +# -------------------------------------------------------------------------- +# Privacy +# -------------------------------------------------------------------------- + + +def test_raw_model_id_is_hashed_never_stored(): + ident = identity(model_id="arn:aws:bedrock:eu-west-1::secret-deployment") + serialized = json.dumps(ident.payload) + assert "arn:aws" not in serialized + assert "secret-deployment" not in serialized + assert len(ident.payload["model_id_sha256"]) == 64 + + +def test_missing_model_or_package_identity_is_refused(): + with pytest.raises(IdentityError, match="model_id"): + identity(model_id="") + with pytest.raises(IdentityError, match="package_version"): + identity(package_version="") + + +# -------------------------------------------------------------------------- +# Stored-record validation +# -------------------------------------------------------------------------- + + +def stored_record(ident=None): + ident = ident or identity() + return json.loads( + json.dumps( + { + "run_id": ident.run_id, + "run_label": ident.label, + "identity_schema": 1, + "identity_sha256": ident.digest, + "identity": ident.payload, + } + ) + ) + + +def test_valid_record_is_accepted(): + ident = identity() + assert verified_stored_digest(stored_record(ident)) == (ident.digest, None) + + +def test_legacy_record_reports_no_identity_and_no_warning(): + assert verified_stored_digest({"run_id": "mock-R2-S0-seed42"}) == (None, None) + + +@pytest.mark.parametrize("schema", [99, 0, "1", True, None]) +def test_unsupported_schema_is_refused(schema): + record = stored_record() + record["identity_schema"] = schema + digest, code = verified_stored_digest(record) + assert digest is None and code in {"MGI003", "MGI004"} + + +@pytest.mark.parametrize("digest", ["", "xyz", "A" * 64, "a" * 63, 12345]) +def test_malformed_digest_is_refused(digest): + record = stored_record() + record["identity_sha256"] = digest + assert verified_stored_digest(record) == (None, "MGI004") + + +def test_tampered_payload_is_refused(): + record = stored_record() + record["identity"]["cases_per_condition"] = 999 + assert verified_stored_digest(record) == (None, "MGI004") + + +def test_incomplete_payload_is_refused(): + record = stored_record() + del record["identity"]["seed"] + assert verified_stored_digest(record) == (None, "MGI004") + + +def test_extra_payload_field_is_refused(): + record = stored_record() + record["identity"]["injected"] = "x" + assert verified_stored_digest(record) == (None, "MGI004") + + +def test_payload_schema_must_match_record_schema(): + record = stored_record() + record["identity"]["identity_schema"] = 2 + assert verified_stored_digest(record) == (None, "MGI004") + + +def test_non_mapping_payload_is_refused(): + record = stored_record() + record["identity"] = ["not", "a", "mapping"] + assert verified_stored_digest(record) == (None, "MGI004") + + +def test_uncanonicalizable_payload_is_refused(): + ident = identity() + record = stored_record(ident) + record["identity"]["provider_parameters"] = {"top_p": float("nan")} + assert verified_stored_digest(record) == (None, "MGI004") + + +def test_digest_survives_json_round_trip(): + ident = identity() + assert digest_payload(json.loads(json.dumps(ident.payload))) == ident.digest + + +# -------------------------------------------------------------------------- +# Diagnostics +# -------------------------------------------------------------------------- + + +def test_changed_fields_reports_sorted_names_only(): + left = identity().payload + right = identity(cases_per_condition=1000, seed=1).payload + assert changed_fields(left, right) == ["cases_per_condition", "seed"] + + +def test_changed_fields_is_empty_for_equal_payloads(): + assert changed_fields(identity().payload, identity().payload) == [] + + +def test_warning_names_fields_but_never_values(caplog): + with caplog.at_level(logging.WARNING, logger="mech_gov.experiment.identity"): + IdentityWarnings().emit("MGI002", "mock-R2-S0-seed42", ["cases_per_condition"]) + record = caplog.records[0] + assert record.name == "mech_gov.experiment.identity" + assert record.levelno == logging.WARNING + assert "[MGI002]" in record.message + assert "fields=cases_per_condition" in record.message + assert "action=execute" in record.message + assert "1000" not in record.message + + +def test_warnings_are_deduplicated_per_code_and_label(caplog): + warnings = IdentityWarnings() + with caplog.at_level(logging.WARNING, logger="mech_gov.experiment.identity"): + warnings.emit("MGI001", "label-a") + warnings.emit("MGI001", "label-a") + warnings.emit("MGI001", "label-b") + warnings.emit("MGI003", "label-a") + assert len(caplog.records) == 3 diff --git a/tests/test_runner_identity.py b/tests/test_runner_identity.py new file mode 100644 index 0000000..e2448fd --- /dev/null +++ b/tests/test_runner_identity.py @@ -0,0 +1,436 @@ +# Copyright (c) 2026 Santander Group +# SPDX-License-Identifier: Apache-2.0 +"""Resume behaviour of the experiment runner under versioned run identity.""" + +from __future__ import annotations + +import json +import logging + +import pytest +import yaml + +import mech_gov +from mech_gov.experiment.identity import IdentityWarnings, canonical_bytes +from mech_gov.experiment.runner import ( + _DEFAULT_DIST_CONFIG, + ExperimentRunner, + RunConfig, + _CompletionIndex, + _decide_skip, + _resolve_identity, +) +from mech_gov.llm.providers.mock import MockLLM +from mech_gov.llm.registry import create_llm + +N = 5 # smallest count the generator accepts: one case per transaction type +M = 10 # a materially different count + + +def _llm(model_id: str = "mock"): + return MockLLM(model_id=model_id) + + +def _cfg(n: int = N, dist: str | None = None, **overrides): + kwargs = dict( + model_name="mock", regime_name="R2", condition="S0", seed=42, cases_per_condition=n + ) + kwargs.update(overrides) + if dist is not None: + kwargs["distributions_config_path"] = dist + return RunConfig(**kwargs) + + +def _identity_warnings(caplog): + return [r for r in caplog.records if r.name == "mech_gov.experiment.identity"] + + +def _codes(caplog): + return sorted({r.message.split("]")[0].lstrip("[") for r in _identity_warnings(caplog)}) + + +def _write_dist(tmp_path, mutate=None, name="dist.yaml"): + """Copy the bundled distributions config, optionally changing one number.""" + loaded = yaml.safe_load(open(_DEFAULT_DIST_CONFIG, encoding="utf-8")) + if mutate is not None: + mutate(loaded) + path = tmp_path / name + path.write_text(yaml.safe_dump(loaded), encoding="utf-8") + return str(path) + + +def _records(results_dir): + path = results_dir / "experiment_results.jsonl" + return [json.loads(line) for line in path.read_text(encoding="utf-8").splitlines() if line] + + +# -------------------------------------------------------------------------- +# The reported defect +# -------------------------------------------------------------------------- + + +def test_changed_case_count_executes_instead_of_skipping(tmp_path, caplog): + runner = ExperimentRunner(results_dir=str(tmp_path)) + first = runner.run_single(_cfg(N), _llm(), skip_completed=True) + with caplog.at_level(logging.WARNING, logger="mech_gov.experiment.identity"): + second = runner.run_single(_cfg(M), _llm(), skip_completed=True) + + assert first is not None and second is not None + assert first.n_cases == N and second.n_cases == M + assert first.run_label == second.run_label # same readable label + assert first.identity_sha256 != second.identity_sha256 # different identity + assert _codes(caplog) == ["MGI002"] + assert "fields=cases_per_condition" in _identity_warnings(caplog)[0].message + + +def test_exact_repeat_still_skips_without_warning(tmp_path, caplog): + runner = ExperimentRunner(results_dir=str(tmp_path)) + assert runner.run_single(_cfg(), _llm(), skip_completed=True) is not None + with caplog.at_level(logging.WARNING, logger="mech_gov.experiment.identity"): + assert runner.run_single(_cfg(), _llm(), skip_completed=True) is None + assert _identity_warnings(caplog) == [] + + +def test_changed_model_id_behind_same_alias_executes(tmp_path): + runner = ExperimentRunner(results_dir=str(tmp_path)) + first = runner.run_single(_cfg(), _llm("model-v1"), skip_completed=True) + second = runner.run_single(_cfg(), _llm("model-v2"), skip_completed=True) + assert second is not None + assert first.identity_sha256 != second.identity_sha256 + + +def test_changed_distribution_content_executes(tmp_path): + def bump(loaded): + loaded["credit_approval"]["risk_score"]["params"]["a"] += 1 + + original = _write_dist(tmp_path, name="a.yaml") + changed = _write_dist(tmp_path, mutate=bump, name="b.yaml") + runner = ExperimentRunner(results_dir=str(tmp_path / "results")) + assert runner.run_single(_cfg(dist=original), _llm(), skip_completed=True) is not None + assert runner.run_single(_cfg(dist=changed), _llm(), skip_completed=True) is not None + + +def test_same_distribution_content_at_another_path_still_skips(tmp_path): + first_path = _write_dist(tmp_path, name="one.yaml") + second_path = _write_dist(tmp_path, name="two.yaml") + runner = ExperimentRunner(results_dir=str(tmp_path / "results")) + assert runner.run_single(_cfg(dist=first_path), _llm(), skip_completed=True) is not None + assert runner.run_single(_cfg(dist=second_path), _llm(), skip_completed=True) is None + + +def test_changed_provider_parameters_execute(tmp_path): + runner = ExperimentRunner(results_dir=str(tmp_path)) + assert runner.run_single(_cfg(), MockLLM(response='{"decision": "APPROVE"}'), False) is not None + second = runner.run_single(_cfg(), MockLLM(response='{"decision": "DEFER"}'), True) + assert second is not None + + +# -------------------------------------------------------------------------- +# Non-resumable inputs +# -------------------------------------------------------------------------- + + +def test_opaque_provider_is_non_resumable(tmp_path, caplog): + def responder(_system, _user): + return '{"decision": "ESCALATE"}' + + runner = ExperimentRunner(results_dir=str(tmp_path)) + with caplog.at_level(logging.WARNING, logger="mech_gov.experiment.identity"): + first = runner.run_single(_cfg(), MockLLM(responder=responder), skip_completed=True) + second = runner.run_single(_cfg(), MockLLM(responder=responder), skip_completed=True) + + assert first is not None and second is not None # never skipped + assert first.identity == {} and first.identity_sha256 == "" + assert first.run_id == first.run_label # no digest suffix without identity + assert "MGI006" in _codes(caplog) + + +def test_unavailable_model_id_is_non_resumable(tmp_path, caplog): + class _NoModelId(MockLLM): + @property + def model_id(self): + raise RuntimeError("unavailable") + + runner = ExperimentRunner(results_dir=str(tmp_path)) + with caplog.at_level(logging.WARNING, logger="mech_gov.experiment.identity"): + result = runner.run_single(_cfg(), _NoModelId(), skip_completed=True) + assert result is not None and result.identity_sha256 == "" + assert "MGI006" in _codes(caplog) + + +def test_unresolvable_provider_parameters_are_non_resumable(tmp_path, caplog): + class _BadParams(MockLLM): + def resume_identity_parameters(self): + return {"temperature": float("nan")} + + runner = ExperimentRunner(results_dir=str(tmp_path)) + with caplog.at_level(logging.WARNING, logger="mech_gov.experiment.identity"): + result = runner.run_single(_cfg(), _BadParams(), skip_completed=True) + assert result is not None and result.identity_sha256 == "" + assert "MGI006" in _codes(caplog) + + +def test_missing_package_version_is_non_resumable(tmp_path, caplog, monkeypatch): + monkeypatch.setattr(mech_gov, "__version__", "", raising=False) + runner = ExperimentRunner(results_dir=str(tmp_path)) + with caplog.at_level(logging.WARNING, logger="mech_gov.experiment.identity"): + result = runner.run_single(_cfg(), _llm(), skip_completed=True) + assert result is not None and result.identity_sha256 == "" + assert "MGI007" in _codes(caplog) + + +def test_a_provider_that_never_overrides_the_contract_is_non_resumable(tmp_path): + warnings = IdentityWarnings() + llm = create_llm({"provider": "callable", "callable": lambda s, u: '{"decision": "DEFER"}'}) + identity = _resolve_identity(_cfg(), llm, "label", {}, warnings) + assert identity is None + + +# -------------------------------------------------------------------------- +# Legacy and malformed stored records +# -------------------------------------------------------------------------- + + +def _seed_legacy(tmp_path, **extra): + record = {"run_id": "mock-R2-S0-seed42", "model": "mock", "n_cases": N} + record.update(extra) + (tmp_path / "experiment_results.jsonl").write_text(json.dumps(record) + "\n", encoding="utf-8") + + +def test_legacy_label_warns_and_executes(tmp_path, caplog): + _seed_legacy(tmp_path) + runner = ExperimentRunner(results_dir=str(tmp_path)) + with caplog.at_level(logging.WARNING, logger="mech_gov.experiment.identity"): + result = runner.run_single(_cfg(), _llm(), skip_completed=True) + assert result is not None + assert _codes(caplog) == ["MGI001"] + + +def test_unknown_schema_warns_and_executes(tmp_path, caplog): + _seed_legacy(tmp_path, identity_schema=99, identity_sha256="a" * 64, identity={}) + runner = ExperimentRunner(results_dir=str(tmp_path)) + with caplog.at_level(logging.WARNING, logger="mech_gov.experiment.identity"): + assert runner.run_single(_cfg(), _llm(), skip_completed=True) is not None + assert "MGI003" in _codes(caplog) + + +def test_invalid_digest_warns_and_executes(tmp_path, caplog): + _seed_legacy(tmp_path, identity_schema=1, identity_sha256="not-a-digest", identity={}) + runner = ExperimentRunner(results_dir=str(tmp_path)) + with caplog.at_level(logging.WARNING, logger="mech_gov.experiment.identity"): + assert runner.run_single(_cfg(), _llm(), skip_completed=True) is not None + assert "MGI004" in _codes(caplog) + + +def test_tampered_payload_warns_and_executes(tmp_path, caplog): + runner = ExperimentRunner(results_dir=str(tmp_path)) + runner.run_single(_cfg(), _llm(), skip_completed=False) + path = tmp_path / "experiment_results.jsonl" + record = json.loads(path.read_text(encoding="utf-8").splitlines()[0]) + record["identity"]["cases_per_condition"] = 999 + path.write_text(json.dumps(record) + "\n", encoding="utf-8") + (tmp_path / "mock" / "baseline.jsonl").unlink() + + with caplog.at_level(logging.WARNING, logger="mech_gov.experiment.identity"): + assert runner.run_single(_cfg(), _llm(), skip_completed=True) is not None + assert "MGI004" in _codes(caplog) + + +@pytest.mark.parametrize( + "line", + [ + "{not json", # unparseable + "", # blank + " ", # whitespace only + "[1, 2, 3]", # valid JSON, not an object + '"a string"', # valid JSON, not an object + "42", # valid JSON, not an object + '{"run_id": 123}', # object with a non-string label + '{"run_id": null}', # object with an empty label + ], +) +def test_unusable_jsonl_lines_are_tolerated(tmp_path, line, caplog): + (tmp_path / "experiment_results.jsonl").write_text(line + "\n", encoding="utf-8") + runner = ExperimentRunner(results_dir=str(tmp_path)) + with caplog.at_level(logging.WARNING, logger="mech_gov.experiment.identity"): + assert runner.run_single(_cfg(), _llm(), skip_completed=True) is not None + assert _identity_warnings(caplog) == [] + + +def test_a_junk_line_does_not_hide_a_real_completed_identity(tmp_path): + runner = ExperimentRunner(results_dir=str(tmp_path)) + runner.run_single(_cfg(), _llm(), skip_completed=False) + path = tmp_path / "experiment_results.jsonl" + path.write_text("{not json\n\n" + path.read_text(encoding="utf-8"), encoding="utf-8") + assert runner.run_single(_cfg(), _llm(), skip_completed=True) is None + + +def test_record_without_run_id_creates_no_completed_identity(tmp_path, caplog): + (tmp_path / "experiment_results.jsonl").write_text('{"model": "mock"}\n', encoding="utf-8") + runner = ExperimentRunner(results_dir=str(tmp_path)) + with caplog.at_level(logging.WARNING, logger="mech_gov.experiment.identity"): + assert runner.run_single(_cfg(), _llm(), skip_completed=True) is not None + assert _identity_warnings(caplog) == [] + + +def test_conflicting_payloads_for_one_digest_disable_skipping(caplog): + """Defensive guard: verified digests bind their payload, so this needs seeding.""" + runner_warnings = IdentityWarnings() + identity = _resolve_identity(_cfg(), _llm(), "mock-R2-S0-seed42", {}, runner_warnings) + record = { + "run_label": identity.label, + "identity_schema": 1, + "identity_sha256": identity.digest, + "identity": identity.payload, + } + index = _CompletionIndex() + index.digests.add(identity.digest) + index.canonical[identity.digest] = b"{}" # a different canonical payload + + with caplog.at_level(logging.WARNING, logger="mech_gov.experiment.identity"): + index.add(record, IdentityWarnings()) + assert identity.digest in index.conflicts + assert _decide_skip(identity, identity.label, index, IdentityWarnings()) is False + assert "MGI005" in _codes(caplog) + + +def test_duplicate_output_copies_count_as_one_identity(tmp_path): + runner = ExperimentRunner(results_dir=str(tmp_path)) + runner.run_single(_cfg(), _llm(), skip_completed=False) + # The same result is written to both the flat file and the model subdir. + index = runner._completion_index(IdentityWarnings()) + assert len(index.digests) == 1 + assert len(index.payloads_by_label["mock-R2-S0-seed42"]) == 1 + + +def test_warning_is_emitted_once_per_scan(tmp_path, caplog): + _seed_legacy(tmp_path) + (tmp_path / "mock").mkdir() + (tmp_path / "mock" / "baseline.jsonl").write_text( + json.dumps({"run_id": "mock-R2-S0-seed42"}) + "\n", encoding="utf-8" + ) + runner = ExperimentRunner(results_dir=str(tmp_path)) + with caplog.at_level(logging.WARNING, logger="mech_gov.experiment.identity"): + runner.run_single(_cfg(), _llm(), skip_completed=True) + assert len(_identity_warnings(caplog)) == 1 + + +def test_multiple_prior_identities_report_the_union_of_changed_fields(tmp_path, caplog): + runner = ExperimentRunner(results_dir=str(tmp_path)) + runner.run_single(_cfg(N), _llm(), skip_completed=False) + runner.run_single(_cfg(N, seed=42, condition="S0"), _llm("model-v2"), skip_completed=False) + with caplog.at_level(logging.WARNING, logger="mech_gov.experiment.identity"): + assert runner.run_single(_cfg(M), _llm(), skip_completed=True) is not None + message = _identity_warnings(caplog)[0].message + assert "cases_per_condition" in message and "model_id_sha256" in message + + +# -------------------------------------------------------------------------- +# Compatibility and privacy +# -------------------------------------------------------------------------- + + +def test_historical_records_are_never_rewritten(tmp_path): + _seed_legacy(tmp_path) + path = tmp_path / "experiment_results.jsonl" + before = path.read_text(encoding="utf-8") + ExperimentRunner(results_dir=str(tmp_path)).run_single(_cfg(), _llm(), skip_completed=True) + assert path.read_text(encoding="utf-8").startswith(before) + + +def test_result_shape_and_output_paths_are_preserved(tmp_path): + seen: list = [] + runner = ExperimentRunner(results_dir=str(tmp_path), on_run_complete=seen.append) + result = runner.run_single(_cfg(), _llm(), skip_completed=False) + + assert (tmp_path / "experiment_results.jsonl").exists() + assert (tmp_path / "mock" / "baseline.jsonl").exists() + assert len(seen) == 1 and seen[0] is result + + record = _records(tmp_path)[0] + for key in ("run_id", "model", "regime", "condition", "seed", "n_cases", "metrics"): + assert key in record + assert record["model_id"] == "mock" # frozen: raw id still on the result + assert record["run_label"] == "mock-R2-S0-seed42" + assert record["run_id"].startswith("mock-R2-S0-seed42--") + assert record["identity_schema"] == 1 + assert len(record["identity_sha256"]) == 64 + + +def test_manifest_still_records_the_raw_model_id(tmp_path): + runner = ExperimentRunner(results_dir=str(tmp_path)) + runner.run_full_experiment( + models={"mock": _llm()}, regimes=["R2"], conditions=["S0"], seeds=[1], cases_per_condition=N + ) + manifest = json.loads((tmp_path / "mock" / "manifest.json").read_text(encoding="utf-8")) + assert manifest["model_id"] == "mock" + assert "identity" not in manifest + + +def test_full_experiment_still_skips_an_identical_repeat(tmp_path): + runner = ExperimentRunner(results_dir=str(tmp_path)) + args = dict( + models={"mock": _llm()}, + regimes=["R2"], + conditions=["S0"], + seeds=[1], + cases_per_condition=N, + ) + assert len(runner.run_full_experiment(**args)) == 1 + assert runner.run_full_experiment(**args) == [] + + +def test_full_experiment_reruns_when_the_case_count_changes(tmp_path): + runner = ExperimentRunner(results_dir=str(tmp_path)) + args = dict(models={"mock": _llm()}, regimes=["R2"], conditions=["S0"], seeds=[1]) + assert len(runner.run_full_experiment(cases_per_condition=N, **args)) == 1 + assert len(runner.run_full_experiment(cases_per_condition=M, **args)) == 1 + + +def test_identity_metadata_excludes_paths_and_raw_identifiers(tmp_path): + dist = _write_dist(tmp_path, name="secret-looking-path.yaml") + runner = ExperimentRunner(results_dir=str(tmp_path / "results")) + runner.run_single(_cfg(dist=dist), _llm("arn:aws:bedrock:eu-west-1::deployment"), False) + + identity = json.dumps(_records(tmp_path / "results")[0]["identity"]) + assert "secret-looking-path" not in identity + assert str(tmp_path) not in identity + assert "arn:aws" not in identity + assert "deployment" not in identity + + +def test_collision_diagnostics_never_print_values(tmp_path, caplog): + runner = ExperimentRunner(results_dir=str(tmp_path)) + runner.run_single(_cfg(N), _llm("arn:aws:bedrock:eu-west-1::deployment"), skip_completed=False) + with caplog.at_level(logging.WARNING, logger="mech_gov.experiment.identity"): + runner.run_single(_cfg(M), _llm("arn:aws:bedrock:eu-west-1::deployment"), True) + message = _identity_warnings(caplog)[0].message + assert "arn:aws" not in message + assert str(N) not in message and str(M) not in message + assert "cases_per_condition" in message + + +@pytest.mark.parametrize("provider", ["openai_compatible", "bedrock", "sagemaker"]) +def test_cloud_provider_identity_parameters_exclude_credentials(provider, monkeypatch): + monkeypatch.setenv("MECH_GOV_LLM_BASE_URL", "https://user:secret@internal.example/v1") + monkeypatch.setenv("MECH_GOV_LLM_API_KEY", "sk-super-secret") + monkeypatch.setenv("MECH_GOV_LLM_MODEL", "gpt-test") + if provider != "openai_compatible": + pytest.importorskip("boto3") + config = { + "openai_compatible": {"provider": "openai_compatible"}, + "bedrock": {"provider": "bedrock", "model_id": "anthropic.test"}, + "sagemaker": {"provider": "sagemaker", "endpoint_name": "internal-endpoint"}, + }[provider] + parameters = create_llm(config).resume_identity_parameters() + assert parameters == {"top_p": 1.0} + serialized = json.dumps(parameters) + for secret in ("secret", "sk-super-secret", "internal", "us-east-1"): + assert secret not in serialized + + +def test_canonical_bytes_of_a_stored_payload_match_the_live_one(tmp_path): + runner = ExperimentRunner(results_dir=str(tmp_path)) + result = runner.run_single(_cfg(), _llm(), skip_completed=False) + stored = _records(tmp_path)[0]["identity"] + assert canonical_bytes(stored) == canonical_bytes(result.identity) From bc9c8ca852e55d699ca48c3407d47500542bf49d Mon Sep 17 00:00:00 2001 From: rschumann Date: Mon, 27 Jul 2026 21:22:34 +0200 Subject: [PATCH 2/4] fix(experiment): harden collision-safe resume identity --- CHANGELOG.md | 23 +-- README.md | 52 ++++-- src/mech_gov/experiment/identity.py | 28 ++-- src/mech_gov/experiment/runner.py | 70 +++++--- src/mech_gov/llm/base.py | 11 +- src/mech_gov/llm/providers/bedrock.py | 10 +- src/mech_gov/llm/providers/mock.py | 5 +- .../llm/providers/openai_compatible.py | 42 ++++- src/mech_gov/llm/providers/sagemaker.py | 5 - tests/test_identity.py | 42 +++-- tests/test_runner_identity.py | 149 ++++++++++++++++-- 11 files changed, 345 insertions(+), 92 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 14ee7ee..5d0a6c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,10 +21,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 hashed model ID, semantic distributions fingerprint, provider parameters, package version). Only the full digest decides resume. - `LLMInterface.resume_identity_parameters()` — optional hook for providers to - declare generation-affecting parameters. The default returns `None`, so - existing custom providers are unaffected and are treated as non-resumable. - Implemented for the bundled mock, OpenAI-compatible, Bedrock and SageMaker - providers. + declare adapter/deployment identity plus generation-affecting parameters. + The default returns `None`, so existing custom providers remain + API-compatible and are treated as non-resumable. Implemented for the bundled + mock, OpenAI-compatible and Bedrock providers. SageMaker remains + non-resumable because endpoint name and region do not establish an + account-scoped deployment identity. - Results now include `run_label`, `identity_schema`, `identity_sha256` and `identity` alongside the existing fields. - Stable identity warning codes `MGI001`–`MGI007` on the @@ -34,16 +36,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 result files. ### Changed -- `run_id` is now `--<12 hex>` for resumable runs. It remains an - opaque string prefixed by the previous readable label; non-resumable runs keep - the bare label. +- `run_id` keeps the previous readable format for backward compatibility. + `identity_sha256`, not `run_id`, decides whether a run may be skipped. - Runs whose identity cannot be established (no stable model ID, undeclared - provider parameters, missing package version) always execute rather than - risking a wrong skip. + provider/deployment parameters, unreadable completion state, missing package + version) always execute rather than risking a wrong skip. - Legacy records without identity metadata are read, never rewritten, and never treated as equivalent to a versioned identity. - The distributions configuration is loaded before the completion check, since its content participates in run identity. +- OpenAI-compatible endpoint targets and Bedrock regions are represented only + by SHA-256 fingerprints. Raw endpoint URLs, credentials and private paths are + excluded. These fingerprints provide stable pseudonymous identity, not + confidentiality. - Open-source readiness scaffolding: - Apache 2.0 `NOTICE`, expanded `CONTRIBUTING.md` (CLA + issue/PR flow), diff --git a/README.md b/README.md index d4ce01c..eb43e3b 100644 --- a/README.md +++ b/README.md @@ -148,14 +148,20 @@ already on disk. Each result therefore carries two different things. | `run_label` | Readable name, e.g. `mock-R2-S0-seed42`. Never decides resume. | | `identity_sha256` | Full SHA-256 over the canonical identity payload. **Decides resume.** | | `identity` | The canonical payload itself, so the digest is reproducible. | -| `run_id` | `run_label` plus the first 12 digest characters, for display and paths. | +| `identity_schema` | Version of the canonical identity contract. | +| `run_id` | The previous readable identifier, unchanged for compatibility. | A run is skipped **only** when a stored record carries the exact same verified `identity_sha256`. The identity covers the case count, regime, condition, seed, a digest of the resolved model ID, a digest of the *loaded* distributions -configuration, the provider's declared generation parameters, and the package -version. Changing any of them re-runs the experiment instead of silently -reusing an older result. +configuration, the provider's declared adapter/deployment fingerprint and +generation parameters, and the package version. Changing any of them re-runs +the experiment instead of silently reusing an older result. + +`run_label` is present on every new result. `identity_schema`, +`identity_sha256`, and `identity` are present only when all required identity +producers are available; a non-resumable result retains the legacy `run_id` +without claiming a machine identity. The distributions fingerprint is taken from the parsed configuration, not the file bytes, so reformatting or editing comments does not invalidate previous @@ -167,32 +173,54 @@ fingerprints identically. Some runs cannot prove equivalence. They always execute, and log why: - the provider cannot expose a stable, non-empty `model_id`; -- the provider does not declare its generation parameters (see below); +- the provider cannot declare its adapter/deployment identity and generation + parameters (see below); - the package version is unavailable. Failing open is deliberate: recomputing a run costs time, whereas wrongly skipping one silently corrupts a result set. +Code provenance is bounded to the package version. Source edits made without a +version change are intentionally outside this identity contract; use +`skip_completed=False` while iterating on an editable checkout. + ### Declaring provider parameters -`LLMInterface.resume_identity_parameters()` returns the generation-affecting -values a provider wants included in run identity, or `None`. The base -implementation returns `None`, so existing custom providers keep working and are -simply treated as non-resumable. Return only public, result-affecting values — -never an API key, base URL, credential, profile name, endpoint, timeout or path. +`LLMInterface.resume_identity_parameters()` returns an allowlisted adapter +identifier, privacy-safe deployment fingerprint, and generation-affecting +values, or `None`. The base implementation returns `None`, so existing custom +providers remain API-compatible but always execute until they declare a stable +identity. The bundled callable and SageMaker providers are non-resumable: +arbitrary callables are opaque, and an endpoint name plus region does not prove +which account-scoped SageMaker deployment is selected. + +Return only public values and privacy-safe fingerprints — never an API key, +credential, raw private URL, profile name, endpoint, timeout, or path. ```python class MyProvider(LLMInterface): def resume_identity_parameters(self): - return {"top_p": self._top_p} + return { + "provider": "my_provider", + "deployment_sha256": self._safe_deployment_fingerprint, + "top_p": self._top_p, + } ``` +SHA-256 fingerprints prevent raw identifiers from appearing in the identity +payload; they are pseudonymous identifiers, not encryption. Use non-secret, +stable model and deployment identifiers. For backward compatibility, the +existing raw `model_id` fields in results and manifests are unchanged. +The readable model alias remains in `run_id`, `run_label`, and `identity` as +public metadata; do not put credentials or private identifiers in aliases. + ### Legacy result files Records written before versioned identity carry a `run_id` and nothing else. They are never treated as equivalent to a versioned identity: a matching readable label logs `MGI001` and the run executes again. Existing files are read -but never rewritten or deleted. +but never rewritten or deleted. The first post-upgrade sweep can therefore +repeat legacy provider calls and incur their normal cost. Identity and resume warnings use the `mech_gov.experiment.identity` logger with stable codes `MGI001`–`MGI007`. They name the configuration fields that differ diff --git a/src/mech_gov/experiment/identity.py b/src/mech_gov/experiment/identity.py index e470eeb..a9c6a93 100644 --- a/src/mech_gov/experiment/identity.py +++ b/src/mech_gov/experiment/identity.py @@ -10,13 +10,15 @@ This module separates the two concerns: ``label`` stays the legacy readable string, while ``digest`` is a SHA-256 over a versioned, allowlisted canonical -payload. Only the full digest decides equality — the truncated digest embedded -in ``run_id`` exists for display and filesystem convenience. +payload. Only the full digest decides equality. The existing result ``run_id`` +remains the readable label for compatibility; no truncated display value takes +part in resume decisions. The module is deliberately inert. It never reads environment variables, never touches the filesystem, never invokes a model, and never accepts raw provider -configuration. Callers pass already-resolved, allowlisted values, so no secret, -endpoint, prompt, case, or path can reach an identity payload or a log line. +configuration. Built-in callers pass already-resolved, allowlisted values. +Third-party providers must obey the +``LLMInterface.resume_identity_parameters()`` privacy contract. """ from __future__ import annotations @@ -39,9 +41,6 @@ #: Payload versions this build is able to verify. SUPPORTED_IDENTITY_SCHEMAS = frozenset({IDENTITY_SCHEMA}) -#: Hex characters of the digest shown in ``run_id``. Display only. -DISPLAY_DIGEST_LENGTH = 12 - #: Exact set of keys a schema-1 payload must contain. PAYLOAD_FIELDS = frozenset( "identity_schema model_name model_id_sha256 regime_name condition seed " @@ -147,11 +146,6 @@ class RunIdentity: payload: dict[str, Any] digest: str - @property - def run_id(self) -> str: - """Readable label plus a truncated digest. Never used for equality.""" - return f"{self.label}--{self.digest[:DISPLAY_DIGEST_LENGTH]}" - def build_run_identity( *, @@ -215,7 +209,7 @@ def verified_stored_digest(record: Mapping[str, Any]) -> tuple[str | None, str | return None, "MGI004" try: recomputed = digest_payload(payload) - except IdentityError: + except Exception: return None, "MGI004" return (stored, None) if recomputed == stored else (None, "MGI004") @@ -223,7 +217,13 @@ def verified_stored_digest(record: Mapping[str, Any]) -> tuple[str | None, str | def changed_fields(current: Mapping[str, Any], stored: Mapping[str, Any]) -> list[str]: """Return the sorted names of payload fields that differ. Never the values.""" names = set(current) | set(stored) - return sorted(name for name in names if current.get(name) != stored.get(name)) + return sorted( + name + for name in names + if name not in current + or name not in stored + or canonical_bytes({"value": current[name]}) != canonical_bytes({"value": stored[name]}) + ) class IdentityWarnings: diff --git a/src/mech_gov/experiment/runner.py b/src/mech_gov/experiment/runner.py index 3a56f78..fca208e 100644 --- a/src/mech_gov/experiment/runner.py +++ b/src/mech_gov/experiment/runner.py @@ -32,7 +32,6 @@ from mech_gov.data.ground_truth import assign_ground_truth from mech_gov.data.stress import apply_stress from mech_gov.experiment.identity import ( - IdentityError, IdentityWarnings, RunIdentity, build_run_identity, @@ -175,19 +174,21 @@ class _CompletionIndex: conflicts: set[str] = field(default_factory=set) payloads_by_label: dict[str, list[dict[str, Any]]] = field(default_factory=dict) legacy_labels: set[str] = field(default_factory=set) + invalid_labels: set[str] = field(default_factory=set) + scan_failed: bool = False def add(self, record: Mapping[str, Any], warnings: IdentityWarnings) -> None: """Index one stored record, warning instead of trusting bad identities.""" label = record.get("run_label") or record.get("run_id") or "" - if not isinstance(label, str): + if not isinstance(label, str) or not label: return digest, code = verified_stored_digest(record) if code is not None: + self.invalid_labels.add(label) warnings.emit(code, label) return if digest is None: - if label: # legacy record: readable label only, never an identity - self.legacy_labels.add(label) + self.legacy_labels.add(label) return blob = canonical_bytes(record["identity"]) if digest in self.digests: @@ -213,18 +214,38 @@ def _result_files(results_dir: Path) -> Iterator[Path]: def _scan_completion_index(results_dir: Path, warnings: IdentityWarnings) -> _CompletionIndex: """Build the completion index, tolerating malformed JSONL lines.""" index = _CompletionIndex() - for path in _result_files(results_dir): - with open(path, encoding="utf-8") as handle: - for line in handle: - line = line.strip() - if not line: - continue - try: - record = json.loads(line) - except json.JSONDecodeError: - continue - if isinstance(record, Mapping): - index.add(record, warnings) + try: + paths = list(_result_files(results_dir)) + except OSError: + index.scan_failed = True + warnings.emit("MGI004", "") + return index + for path in paths: + try: + with open(path, encoding="utf-8") as handle: + for line in handle: + line = line.strip() + if not line: + continue + try: + record = json.loads(line) + except (json.JSONDecodeError, RecursionError): + continue + if not isinstance(record, Mapping): + continue + try: + index.add(record, warnings) + except Exception: + label = record.get("run_label") or record.get("run_id") + if isinstance(label, str) and label: + index.invalid_labels.add(label) + warnings.emit("MGI004", label) + else: + index.scan_failed = True + warnings.emit("MGI004", str(path)) + except (OSError, UnicodeError): + index.scan_failed = True + warnings.emit("MGI004", str(path)) return index @@ -238,11 +259,12 @@ def _resolve_identity( """Build the versioned identity, or return None when the run is non-resumable.""" try: model_id = llm.model_id + getter = getattr(llm, "resume_identity_parameters", None) + parameters = getter() if callable(getter) else None except Exception: model_id = "" - getter = getattr(llm, "resume_identity_parameters", None) - parameters = getter() if callable(getter) else None - if not model_id or parameters is None: + parameters = None + if not isinstance(model_id, str) or not model_id or not isinstance(parameters, Mapping): warnings.emit("MGI006", label) return None version = _package_version() @@ -262,7 +284,7 @@ def _resolve_identity( provider_parameters=parameters, package_version=version, ) - except IdentityError: + except Exception: warnings.emit("MGI006", label) return None @@ -276,6 +298,8 @@ def _decide_skip( """Decide whether a requested run may be treated as already completed.""" if identity is None: return False + if index.scan_failed or label in index.invalid_labels: + return False if identity.digest in index.conflicts: warnings.emit("MGI005", label) return False @@ -533,7 +557,9 @@ def run_single( Args: config: Run configuration llm: LLM interface - skip_completed: If True, skip if run_id already in results + skip_completed: If True, skip only when a stored, verified machine + identity exactly matches the current run. Legacy run labels do + not establish completion. Returns: ExperimentResult or None if skipped @@ -546,7 +572,7 @@ def run_single( ) dist_config, identity, skip = self._prepare_run(config, llm, label, skip_completed) - run_id = identity.run_id if identity is not None else label + run_id = label if skip: logger.info("SKIP %s (already completed)", run_id) return None diff --git a/src/mech_gov/llm/base.py b/src/mech_gov/llm/base.py index e0fad36..91f68c4 100644 --- a/src/mech_gov/llm/base.py +++ b/src/mech_gov/llm/base.py @@ -66,14 +66,15 @@ def provider(self) -> str: return "unknown" def resume_identity_parameters(self) -> dict[str, Any] | None: - """Return generation parameters that must take part in run identity. + """Return adapter/deployment identity and result-affecting parameters. The experiment runner uses this to decide whether a previously stored run is really equivalent to the one being requested. Return a mapping of - allowlisted, public, result-affecting values — never an API key, base - URL, credential, profile name, endpoint, timeout, or path. Opaque values - such as user-supplied callables cannot be fingerprinted; return ``None`` - for them. + allowlisted public values and privacy-safe fingerprints. Include the + adapter kind and enough deployment identity to distinguish backends. + Never return an API key, credential, raw private URL, profile name, + endpoint, timeout, or path. Opaque values such as user-supplied + callables cannot be fingerprinted; return ``None`` for them. ``None`` (the default, so existing subclasses keep working unchanged) means the provider cannot prove equivalence. Such runs are treated as diff --git a/src/mech_gov/llm/providers/bedrock.py b/src/mech_gov/llm/providers/bedrock.py index a9534fd..cb314b5 100644 --- a/src/mech_gov/llm/providers/bedrock.py +++ b/src/mech_gov/llm/providers/bedrock.py @@ -14,6 +14,7 @@ from __future__ import annotations +import hashlib import logging import random import time @@ -92,8 +93,13 @@ def provider(self) -> str: return "bedrock" def resume_identity_parameters(self) -> dict[str, Any] | None: - """Only ``top_p`` affects results; region, profile and retries do not.""" - return {"top_p": self._top_p} + """Declare the adapter, privacy-safe region identity and ``top_p``.""" + region_sha256 = hashlib.sha256(self._region.encode("utf-8")).hexdigest() + return { + "provider": "bedrock", + "region_sha256": region_sha256, + "top_p": self._top_p, + } def invoke( self, diff --git a/src/mech_gov/llm/providers/mock.py b/src/mech_gov/llm/providers/mock.py index a254914..bd08503 100644 --- a/src/mech_gov/llm/providers/mock.py +++ b/src/mech_gov/llm/providers/mock.py @@ -77,8 +77,9 @@ def resume_identity_parameters(self) -> dict[str, Any] | None: if self._responder is not None: return None canned = self._responses if self._responses else [self._response] - digest = hashlib.sha256("\0".join(canned).encode("utf-8")).hexdigest() - return {"responses_sha256": digest} + canonical = json.dumps(canned, ensure_ascii=False, separators=(",", ":")) + digest = hashlib.sha256(canonical.encode("utf-8")).hexdigest() + return {"provider": "mock", "responses_sha256": digest} def invoke( self, diff --git a/src/mech_gov/llm/providers/openai_compatible.py b/src/mech_gov/llm/providers/openai_compatible.py index b273008..2880623 100644 --- a/src/mech_gov/llm/providers/openai_compatible.py +++ b/src/mech_gov/llm/providers/openai_compatible.py @@ -15,16 +15,47 @@ from __future__ import annotations +import hashlib import json import os import time import urllib.error +import urllib.parse import urllib.request from typing import Any from mech_gov.llm.base import LLMInterface, LLMResponse +def _endpoint_sha256(base_url: str, path: str) -> str | None: + """Fingerprint a credential-free endpoint target, or refuse ambiguous URLs.""" + try: + parsed = urllib.parse.urlsplit(base_url) + port = parsed.port + except ValueError: + return None + if ( + not parsed.scheme + or not parsed.hostname + or parsed.username is not None + or parsed.password is not None + or parsed.query + or parsed.fragment + ): + return None + if port == {"http": 80, "https": 443}.get(parsed.scheme.lower()): + port = None + target = [ + parsed.scheme.lower(), + parsed.hostname.lower(), + port, + parsed.path.rstrip("/"), + path, + ] + canonical = json.dumps(target, ensure_ascii=False, separators=(",", ":")) + return hashlib.sha256(canonical.encode("utf-8")).hexdigest() + + class OpenAICompatibleLLM(LLMInterface): """LLM client for any OpenAI-compatible ``/chat/completions`` endpoint.""" @@ -64,8 +95,15 @@ def provider(self) -> str: return "openai_compatible" def resume_identity_parameters(self) -> dict[str, Any] | None: - """Only ``top_p`` affects results; base URL, key, path and timeout do not.""" - return {"top_p": self._top_p} + """Declare adapter, endpoint fingerprint and result-affecting parameters.""" + endpoint_sha256 = _endpoint_sha256(self._base_url, self._path) + if endpoint_sha256 is None: + return None + return { + "provider": "openai_compatible", + "endpoint_sha256": endpoint_sha256, + "top_p": self._top_p, + } def invoke( self, diff --git a/src/mech_gov/llm/providers/sagemaker.py b/src/mech_gov/llm/providers/sagemaker.py index c5cefed..61aab01 100644 --- a/src/mech_gov/llm/providers/sagemaker.py +++ b/src/mech_gov/llm/providers/sagemaker.py @@ -17,7 +17,6 @@ import json import logging import time -from typing import Any from mech_gov.llm.base import LLMInterface, LLMResponse @@ -83,10 +82,6 @@ def model_id(self) -> str: def provider(self) -> str: return "sagemaker" - def resume_identity_parameters(self) -> dict[str, Any] | None: - """Only ``top_p`` affects results; the endpoint is already in ``model_id``.""" - return {"top_p": self._top_p} - def invoke( self, system_prompt: str, diff --git a/tests/test_identity.py b/tests/test_identity.py index 6387c30..37b7976 100644 --- a/tests/test_identity.py +++ b/tests/test_identity.py @@ -11,7 +11,6 @@ import pytest from mech_gov.experiment.identity import ( - DISPLAY_DIGEST_LENGTH, PAYLOAD_FIELDS, IdentityError, IdentityWarnings, @@ -46,19 +45,34 @@ def identity(**overrides): # -------------------------------------------------------------------------- -def test_run_id_is_label_plus_truncated_digest(): +def test_label_and_authoritative_digest_are_separate(): ident = identity() assert ident.label == "mock-R2-S0-seed42" assert len(ident.digest) == 64 - assert ident.run_id == f"mock-R2-S0-seed42--{ident.digest[:DISPLAY_DIGEST_LENGTH]}" - # The display suffix is strictly shorter than the authoritative digest. - assert len(ident.run_id.split("--")[1]) < len(ident.digest) def test_identity_is_deterministic_across_instances(): assert identity().digest == identity().digest +def test_schema_one_has_a_pinned_canonical_vector(): + expected = ( + '{"cases_per_condition":"n:1E+1","condition":"s:S0",' + '"distributions_sha256":"s:e0c57a20148c66a32df79a41b725c4fbf2525e30e585890b2bf9b65ed449367f",' + '"identity_schema":"n:1",' + '"model_id_sha256":"s:f0cb9b49bc405836585ef9d6785501b0685241788111424e03fba0f5d55e06b6",' + '"model_name":"s:mock","package_version":"s:0.1.0",' + '"provider_parameters":{"top_p":"n:1"},"regime_name":"s:R2","seed":"n:42"}' + ) + ident = identity() + assert canonical_bytes(ident.payload) == expected.encode("utf-8") + expected_digest = ( + "a5e7a917d45ead0077a43eacd286a96d" # pragma: allowlist secret + "2c30110d97dee330e1bb12dcc76108cd" # pragma: allowlist secret + ) + assert ident.digest == expected_digest + + def test_payload_carries_exactly_the_declared_fields(): assert set(identity().payload) == set(PAYLOAD_FIELDS) @@ -158,7 +172,7 @@ def test_empty_configuration_still_fingerprints(): # -------------------------------------------------------------------------- -def test_raw_model_id_is_hashed_never_stored(): +def test_raw_model_id_is_hashed_never_stored_in_identity(): ident = identity(model_id="arn:aws:bedrock:eu-west-1::secret-deployment") serialized = json.dumps(ident.payload) assert "arn:aws" not in serialized @@ -183,7 +197,7 @@ def stored_record(ident=None): return json.loads( json.dumps( { - "run_id": ident.run_id, + "run_id": ident.label, "run_label": ident.label, "identity_schema": 1, "identity_sha256": ident.digest, @@ -202,12 +216,15 @@ def test_legacy_record_reports_no_identity_and_no_warning(): assert verified_stored_digest({"run_id": "mock-R2-S0-seed42"}) == (None, None) -@pytest.mark.parametrize("schema", [99, 0, "1", True, None]) -def test_unsupported_schema_is_refused(schema): +@pytest.mark.parametrize( + "schema, expected_code", + [(99, "MGI003"), (0, "MGI003"), ("1", "MGI003"), (True, "MGI003"), (None, "MGI003")], +) +def test_unsupported_schema_is_refused(schema, expected_code): record = stored_record() record["identity_schema"] = schema digest, code = verified_stored_digest(record) - assert digest is None and code in {"MGI003", "MGI004"} + assert digest is None and code == expected_code @pytest.mark.parametrize("digest", ["", "xyz", "A" * 64, "a" * 63, 12345]) @@ -274,6 +291,11 @@ def test_changed_fields_is_empty_for_equal_payloads(): assert changed_fields(identity().payload, identity().payload) == [] +def test_changed_fields_uses_canonical_not_python_equality(): + assert changed_fields({"value": True}, {"value": 1}) == ["value"] + assert changed_fields({"value": (1, 2)}, {"value": [1.0, 2.0]}) == [] + + def test_warning_names_fields_but_never_values(caplog): with caplog.at_level(logging.WARNING, logger="mech_gov.experiment.identity"): IdentityWarnings().emit("MGI002", "mock-R2-S0-seed42", ["cases_per_condition"]) diff --git a/tests/test_runner_identity.py b/tests/test_runner_identity.py index e2448fd..7fa8c78 100644 --- a/tests/test_runner_identity.py +++ b/tests/test_runner_identity.py @@ -21,6 +21,7 @@ _resolve_identity, ) from mech_gov.llm.providers.mock import MockLLM +from mech_gov.llm.providers.openai_compatible import OpenAICompatibleLLM from mech_gov.llm.registry import create_llm N = 5 # smallest count the generator accepts: one case per transaction type @@ -51,7 +52,8 @@ def _codes(caplog): def _write_dist(tmp_path, mutate=None, name="dist.yaml"): """Copy the bundled distributions config, optionally changing one number.""" - loaded = yaml.safe_load(open(_DEFAULT_DIST_CONFIG, encoding="utf-8")) + with open(_DEFAULT_DIST_CONFIG, encoding="utf-8") as handle: + loaded = yaml.safe_load(handle) if mutate is not None: mutate(loaded) path = tmp_path / name @@ -125,6 +127,39 @@ def test_changed_provider_parameters_execute(tmp_path): assert second is not None +def test_mock_response_fingerprint_is_length_unambiguous(): + left = MockLLM(responses=["a\0b", "c"]).resume_identity_parameters() + right = MockLLM(responses=["a", "b\0c"]).resume_identity_parameters() + assert left != right + + +def test_openai_compatible_endpoint_changes_identity(): + first = OpenAICompatibleLLM(base_url="https://one.example/v1", model="shared-model") + second = OpenAICompatibleLLM(base_url="https://two.example/v1", model="shared-model") + warnings = IdentityWarnings() + first_identity = _resolve_identity(_cfg(), first, "label", {}, warnings) + second_identity = _resolve_identity(_cfg(), second, "label", {}, warnings) + assert first_identity is not None and second_identity is not None + assert first_identity.digest != second_identity.digest + assert "one.example" not in json.dumps(first_identity.payload) + assert "two.example" not in json.dumps(second_identity.payload) + + +def test_openai_compatible_default_port_is_semantically_stable(): + implicit = OpenAICompatibleLLM(base_url="https://example.test/v1", model="model") + explicit = OpenAICompatibleLLM(base_url="https://example.test:443/v1", model="model") + assert implicit.resume_identity_parameters() == explicit.resume_identity_parameters() + + +def test_openai_compatible_credentialed_endpoint_is_non_resumable(): + llm = OpenAICompatibleLLM( + base_url="https://user:secret@example.test/v1", # pragma: allowlist secret + api_key="sk-secret", # pragma: allowlist secret + model="model", + ) + assert llm.resume_identity_parameters() is None + + # -------------------------------------------------------------------------- # Non-resumable inputs # -------------------------------------------------------------------------- @@ -141,7 +176,7 @@ def responder(_system, _user): assert first is not None and second is not None # never skipped assert first.identity == {} and first.identity_sha256 == "" - assert first.run_id == first.run_label # no digest suffix without identity + assert first.run_id == first.run_label assert "MGI006" in _codes(caplog) @@ -170,6 +205,32 @@ def resume_identity_parameters(self): assert "MGI006" in _codes(caplog) +@pytest.mark.parametrize("bad_parameters", [["not-a-pair"], "not-a-mapping", 42]) +def test_invalid_provider_parameter_types_are_non_resumable(tmp_path, caplog, bad_parameters): + class _BadParams(MockLLM): + def resume_identity_parameters(self): + return bad_parameters + + runner = ExperimentRunner(results_dir=str(tmp_path)) + with caplog.at_level(logging.WARNING, logger="mech_gov.experiment.identity"): + result = runner.run_single(_cfg(), _BadParams(), skip_completed=True) + assert result is not None and result.identity_sha256 == "" + assert "MGI006" in _codes(caplog) + + +def test_provider_identity_hook_exception_is_non_resumable(tmp_path, caplog): + class _RaisingParams(MockLLM): + def resume_identity_parameters(self): + raise RuntimeError("secret exception detail") + + runner = ExperimentRunner(results_dir=str(tmp_path)) + with caplog.at_level(logging.WARNING, logger="mech_gov.experiment.identity"): + result = runner.run_single(_cfg(), _RaisingParams(), skip_completed=True) + assert result is not None and result.identity_sha256 == "" + assert "MGI006" in _codes(caplog) + assert "secret exception detail" not in caplog.text + + def test_missing_package_version_is_non_resumable(tmp_path, caplog, monkeypatch): monkeypatch.setattr(mech_gov, "__version__", "", raising=False) runner = ExperimentRunner(results_dir=str(tmp_path)) @@ -257,6 +318,50 @@ def test_unusable_jsonl_lines_are_tolerated(tmp_path, line, caplog): assert _identity_warnings(caplog) == [] +def test_invalid_utf8_result_file_warns_and_executes(tmp_path, caplog): + (tmp_path / "experiment_results.jsonl").write_bytes(b"\xff\xfe") + runner = ExperimentRunner(results_dir=str(tmp_path)) + with caplog.at_level(logging.WARNING, logger="mech_gov.experiment.identity"): + result = runner.run_single(_cfg(), _llm(), skip_completed=True) + assert result is not None + assert "MGI004" in _codes(caplog) + assert str(tmp_path) not in caplog.text + + +def test_jsonl_named_directory_warns_and_executes(tmp_path, caplog): + (tmp_path / "unreadable.jsonl").mkdir() + runner = ExperimentRunner(results_dir=str(tmp_path)) + with caplog.at_level(logging.WARNING, logger="mech_gov.experiment.identity"): + result = runner.run_single(_cfg(), _llm(), skip_completed=True) + assert result is not None + assert "MGI004" in _codes(caplog) + + +def test_scan_failure_disables_skip_even_with_a_valid_copy(tmp_path, caplog): + runner = ExperimentRunner(results_dir=str(tmp_path)) + assert runner.run_single(_cfg(), _llm(), skip_completed=False) is not None + (tmp_path / "unreadable.jsonl").write_bytes(b"\xff") + with caplog.at_level(logging.WARNING, logger="mech_gov.experiment.identity"): + assert runner.run_single(_cfg(), _llm(), skip_completed=True) is not None + assert "MGI004" in _codes(caplog) + + +def test_invalid_matching_record_disables_skip_even_with_a_valid_copy(tmp_path, caplog): + runner = ExperimentRunner(results_dir=str(tmp_path)) + assert runner.run_single(_cfg(), _llm(), skip_completed=False) is not None + path = tmp_path / "experiment_results.jsonl" + invalid = { + "run_id": "mock-R2-S0-seed42", + "identity_schema": 1, + "identity_sha256": "not-a-digest", + "identity": {}, + } + path.write_text(path.read_text(encoding="utf-8") + json.dumps(invalid) + "\n", encoding="utf-8") + with caplog.at_level(logging.WARNING, logger="mech_gov.experiment.identity"): + assert runner.run_single(_cfg(), _llm(), skip_completed=True) is not None + assert "MGI004" in _codes(caplog) + + def test_a_junk_line_does_not_hide_a_real_completed_identity(tmp_path): runner = ExperimentRunner(results_dir=str(tmp_path)) runner.run_single(_cfg(), _llm(), skip_completed=False) @@ -273,6 +378,23 @@ def test_record_without_run_id_creates_no_completed_identity(tmp_path, caplog): assert _identity_warnings(caplog) == [] +def test_valid_identity_without_any_label_cannot_skip(tmp_path): + producer = ExperimentRunner(results_dir=str(tmp_path / "producer")) + result = producer.run_single(_cfg(), _llm(), skip_completed=False) + record = { + "identity_schema": result.identity["identity_schema"], + "identity_sha256": result.identity_sha256, + "identity": result.identity, + } + results_dir = tmp_path / "consumer" + results_dir.mkdir() + (results_dir / "experiment_results.jsonl").write_text( + json.dumps(record) + "\n", encoding="utf-8" + ) + consumer = ExperimentRunner(results_dir=str(results_dir)) + assert consumer.run_single(_cfg(), _llm(), skip_completed=True) is not None + + def test_conflicting_payloads_for_one_digest_disable_skipping(caplog): """Defensive guard: verified digests bind their payload, so this needs seeding.""" runner_warnings = IdentityWarnings() @@ -352,7 +474,7 @@ def test_result_shape_and_output_paths_are_preserved(tmp_path): assert key in record assert record["model_id"] == "mock" # frozen: raw id still on the result assert record["run_label"] == "mock-R2-S0-seed42" - assert record["run_id"].startswith("mock-R2-S0-seed42--") + assert record["run_id"] == "mock-R2-S0-seed42" assert record["identity_schema"] == 1 assert len(record["identity_sha256"]) == 64 @@ -410,25 +532,34 @@ def test_collision_diagnostics_never_print_values(tmp_path, caplog): assert "cases_per_condition" in message -@pytest.mark.parametrize("provider", ["openai_compatible", "bedrock", "sagemaker"]) -def test_cloud_provider_identity_parameters_exclude_credentials(provider, monkeypatch): - monkeypatch.setenv("MECH_GOV_LLM_BASE_URL", "https://user:secret@internal.example/v1") +@pytest.mark.parametrize("provider", ["openai_compatible", "bedrock"]) +def test_resumable_cloud_provider_identity_excludes_credentials(provider, monkeypatch): + monkeypatch.setenv("MECH_GOV_LLM_BASE_URL", "https://internal.example/v1") monkeypatch.setenv("MECH_GOV_LLM_API_KEY", "sk-super-secret") monkeypatch.setenv("MECH_GOV_LLM_MODEL", "gpt-test") - if provider != "openai_compatible": + if provider == "bedrock": pytest.importorskip("boto3") config = { "openai_compatible": {"provider": "openai_compatible"}, "bedrock": {"provider": "bedrock", "model_id": "anthropic.test"}, - "sagemaker": {"provider": "sagemaker", "endpoint_name": "internal-endpoint"}, }[provider] parameters = create_llm(config).resume_identity_parameters() - assert parameters == {"top_p": 1.0} + assert parameters["provider"] == provider + assert parameters["top_p"] == 1.0 + assert any(key.endswith("_sha256") for key in parameters) serialized = json.dumps(parameters) for secret in ("secret", "sk-super-secret", "internal", "us-east-1"): assert secret not in serialized +def test_sagemaker_is_non_resumable_without_account_scoped_deployment_identity(monkeypatch): + pytest.importorskip("boto3") + llm = create_llm( + {"provider": "sagemaker", "endpoint_name": "internal-endpoint", "region": "eu-west-1"} + ) + assert llm.resume_identity_parameters() is None + + def test_canonical_bytes_of_a_stored_payload_match_the_live_one(tmp_path): runner = ExperimentRunner(results_dir=str(tmp_path)) result = runner.run_single(_cfg(), _llm(), skip_completed=False) From d58bba062dd30a52f7b2f03933651bf397ef4c2e Mon Sep 17 00:00:00 2001 From: rschumann Date: Wed, 29 Jul 2026 14:32:40 +0200 Subject: [PATCH 3/4] fix(experiment): make run identity context-independent --- CHANGELOG.md | 5 ++- README.md | 25 ++++++++++++- src/mech_gov/experiment/identity.py | 14 ++++--- src/mech_gov/experiment/runner.py | 23 +++++++++--- tests/test_identity.py | 14 +++++++ tests/test_runner_identity.py | 57 +++++++++++++++++++++++++++-- 6 files changed, 119 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d0a6c3..5ec795c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,9 +29,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 account-scoped deployment identity. - Results now include `run_label`, `identity_schema`, `identity_sha256` and `identity` alongside the existing fields. -- Stable identity warning codes `MGI001`–`MGI007` on the +- Stable identity warning codes `MGI001`–`MGI008` on the `mech_gov.experiment.identity` logger. Diagnostics name the fields that - differ and never print their values. + differ, use a pseudonymous reference for correlation, distinguish unreadable + completion state, and never print raw labels, paths, or parameter values. - README section documenting resume semantics, non-resumable runs and legacy result files. diff --git a/README.md b/README.md index eb43e3b..1a30823 100644 --- a/README.md +++ b/README.md @@ -168,6 +168,12 @@ file bytes, so reformatting or editing comments does not invalidate previous runs, while a changed parameter does. Equivalent content at a different path fingerprints identically. +Canonical numbers are type-tagged and derived exactly from integers or the +round-trip representation of finite floats. Decimal normalization uses explicit +precision equal to the operand's digit count, so a caller's mutable +`decimal` context cannot round two configurations into one identity. +Non-finite floats are rejected. + ### Non-resumable runs Some runs cannot prove equivalence. They always execute, and log why: @@ -194,6 +200,14 @@ identity. The bundled callable and SageMaker providers are non-resumable: arbitrary callables are opaque, and an endpoint name plus region does not prove which account-scoped SageMaker deployment is selected. +The bundled OpenAI-compatible and Bedrock adapters currently expose exactly one +constructor-configured generation parameter, `top_p`; both declare it alongside +their privacy-safe target fingerprint. Per-call `temperature` and `max_tokens` +come from governance-regime code and are therefore covered by the package +version. Connection, credential/profile, timeout, and retry settings affect +transport behavior rather than the successful response contract and are +deliberately excluded. + Return only public values and privacy-safe fingerprints — never an API key, credential, raw private URL, profile name, endpoint, timeout, or path. @@ -223,8 +237,15 @@ but never rewritten or deleted. The first post-upgrade sweep can therefore repeat legacy provider calls and incur their normal cost. Identity and resume warnings use the `mech_gov.experiment.identity` logger with -stable codes `MGI001`–`MGI007`. They name the configuration fields that differ -but never print their values. +stable codes `MGI001`–`MGI008`. They name the configuration fields that differ +and include a 12-hex SHA-256 reference for correlation, but never print the raw +run label, result path, or configuration values. `MGI008` distinguishes +unreadable completion state from a stored payload that failed validation. + +Distribution configuration is loaded before any resume decision because its +parsed content is part of identity. A missing or invalid distributions file +therefore raises even when an older readable label exists; silently skipping +without validating an identity input would be unsafe. ## CLI diff --git a/src/mech_gov/experiment/identity.py b/src/mech_gov/experiment/identity.py index a9c6a93..3628224 100644 --- a/src/mech_gov/experiment/identity.py +++ b/src/mech_gov/experiment/identity.py @@ -30,7 +30,7 @@ import re from collections.abc import Mapping, Sequence from dataclasses import dataclass -from decimal import Decimal +from decimal import MAX_EMAX, MIN_EMIN, Context, Decimal from typing import Any logger = logging.getLogger("mech_gov.experiment.identity") @@ -56,6 +56,7 @@ "MGI005": "identity digest is claimed by conflicting payloads", "MGI006": "stable model or provider identity is unavailable", "MGI007": "package provenance is unavailable", + "MGI008": "completion state could not be read safely", } _FULL_DIGEST_RE = re.compile(r"\A[0-9a-f]{64}\Z") @@ -72,12 +73,14 @@ def _canonical_number(value: int | float, path: str) -> str: as_decimal = Decimal(value) if isinstance(value, int) else Decimal(repr(value)) if as_decimal == 0: return "0" - return str(as_decimal.normalize()) + # Normalize under exact precision, never the mutable ambient context. + precision = max(1, len(as_decimal.as_tuple().digits)) + exact_context = Context(prec=precision, Emax=MAX_EMAX, Emin=MIN_EMIN) + return str(as_decimal.normalize(exact_context)) def _normalize(value: Any, path: str = "") -> Any: """Recursively rewrite a loaded value into a type-tagged canonical form. - Scalars are tagged so that ``None``, ``False`` and the strings ``"z:"`` or ``"b:false"`` can never collide. Mapping keys must be strings and are ordered by the serializer; list order stays significant. @@ -233,13 +236,14 @@ def __init__(self) -> None: self._seen: set[tuple[str, str]] = set() def emit(self, code: str, label: str, fields: Sequence[str] = ()) -> None: - """Log one deduplicated warning naming changed fields but no values.""" if (code, label) in self._seen: return self._seen.add((code, label)) + reference = hashlib.sha256(label.encode("utf-8")).hexdigest()[:12] logger.warning( - "[%s] %s; fields=%s; action=execute", + "[%s] %s; reference_sha256=%s; fields=%s; action=execute", code, WARNING_SUMMARIES[code], + reference, ",".join(fields), ) diff --git a/src/mech_gov/experiment/runner.py b/src/mech_gov/experiment/runner.py index fca208e..3b55793 100644 --- a/src/mech_gov/experiment/runner.py +++ b/src/mech_gov/experiment/runner.py @@ -218,7 +218,7 @@ def _scan_completion_index(results_dir: Path, warnings: IdentityWarnings) -> _Co paths = list(_result_files(results_dir)) except OSError: index.scan_failed = True - warnings.emit("MGI004", "") + warnings.emit("MGI008", "") return index for path in paths: try: @@ -245,7 +245,7 @@ def _scan_completion_index(results_dir: Path, warnings: IdentityWarnings) -> _Co warnings.emit("MGI004", str(path)) except (OSError, UnicodeError): index.scan_failed = True - warnings.emit("MGI004", str(path)) + warnings.emit("MGI008", str(path)) return index @@ -255,6 +255,7 @@ def _resolve_identity( label: str, dist_config: Mapping[str, Any], warnings: IdentityWarnings, + warn_non_resumable: bool = True, ) -> RunIdentity | None: """Build the versioned identity, or return None when the run is non-resumable.""" try: @@ -265,11 +266,13 @@ def _resolve_identity( model_id = "" parameters = None if not isinstance(model_id, str) or not model_id or not isinstance(parameters, Mapping): - warnings.emit("MGI006", label) + if warn_non_resumable: + warnings.emit("MGI006", label) return None version = _package_version() if not version: - warnings.emit("MGI007", label) + if warn_non_resumable: + warnings.emit("MGI007", label) return None try: return build_run_identity( @@ -285,7 +288,8 @@ def _resolve_identity( package_version=version, ) except Exception: - warnings.emit("MGI006", label) + if warn_non_resumable: + warnings.emit("MGI006", label) return None @@ -348,7 +352,14 @@ def _prepare_run( """Resolve identity inputs before the completion check and decide on skipping.""" dist_config = load_distributions(config.distributions_config_path) warnings = IdentityWarnings() - identity = _resolve_identity(config, llm, label, dist_config, warnings) + identity = _resolve_identity( + config, + llm, + label, + dist_config, + warnings, + warn_non_resumable=skip_completed, + ) skip = skip_completed and _decide_skip( identity, label, self._completion_index(warnings), warnings ) diff --git a/tests/test_identity.py b/tests/test_identity.py index 37b7976..00cda33 100644 --- a/tests/test_identity.py +++ b/tests/test_identity.py @@ -7,6 +7,7 @@ import datetime import json import logging +from decimal import localcontext import pytest @@ -73,6 +74,19 @@ def test_schema_one_has_a_pinned_canonical_vector(): assert ident.digest == expected_digest +def test_canonical_numbers_ignore_ambient_decimal_precision(): + expected = canonical_bytes(identity().payload) + with localcontext() as context: + context.prec = 3 + assert canonical_bytes(identity().payload) == expected + assert distributions_digest({"value": 1234.5678}) != distributions_digest( + {"value": 1234.5679} + ) + assert distributions_digest({"value": 10**30 + 1}) != distributions_digest( + {"value": 10**30 + 2} + ) + + def test_payload_carries_exactly_the_declared_fields(): assert set(identity().payload) == set(PAYLOAD_FIELDS) diff --git a/tests/test_runner_identity.py b/tests/test_runner_identity.py index 7fa8c78..a04af33 100644 --- a/tests/test_runner_identity.py +++ b/tests/test_runner_identity.py @@ -151,6 +151,35 @@ def test_openai_compatible_default_port_is_semantically_stable(): assert implicit.resume_identity_parameters() == explicit.resume_identity_parameters() +def test_openai_compatible_top_p_changes_identity(): + first = OpenAICompatibleLLM(base_url="https://example.test/v1", model="model", top_p=0.8) + second = OpenAICompatibleLLM(base_url="https://example.test/v1", model="model", top_p=0.9) + warnings = IdentityWarnings() + first_identity = _resolve_identity(_cfg(), first, "label", {}, warnings) + second_identity = _resolve_identity(_cfg(), second, "label", {}, warnings) + assert first_identity is not None and second_identity is not None + assert first_identity.digest != second_identity.digest + + +def test_bedrock_top_p_changes_identity(monkeypatch): + boto3 = pytest.importorskip("boto3") + + class _Session: + def client(self, *_args, **_kwargs): + return object() + + monkeypatch.setattr(boto3, "Session", lambda **_kwargs: _Session()) + from mech_gov.llm.providers.bedrock import BedrockLLM + + first = BedrockLLM("model", top_p=0.8) + second = BedrockLLM("model", top_p=0.9) + warnings = IdentityWarnings() + first_identity = _resolve_identity(_cfg(), first, "label", {}, warnings) + second_identity = _resolve_identity(_cfg(), second, "label", {}, warnings) + assert first_identity is not None and second_identity is not None + assert first_identity.digest != second_identity.digest + + def test_openai_compatible_credentialed_endpoint_is_non_resumable(): llm = OpenAICompatibleLLM( base_url="https://user:secret@example.test/v1", # pragma: allowlist secret @@ -180,6 +209,18 @@ def responder(_system, _user): assert "MGI006" in _codes(caplog) +def test_nonresumable_provider_is_quiet_when_resume_is_disabled(tmp_path, caplog): + runner = ExperimentRunner(results_dir=str(tmp_path)) + with caplog.at_level(logging.WARNING, logger="mech_gov.experiment.identity"): + result = runner.run_single( + _cfg(), + MockLLM(responder=lambda _system, _user: '{"decision": "ESCALATE"}'), + skip_completed=False, + ) + assert result is not None and result.identity_sha256 == "" + assert _identity_warnings(caplog) == [] + + def test_unavailable_model_id_is_non_resumable(tmp_path, caplog): class _NoModelId(MockLLM): @property @@ -324,7 +365,7 @@ def test_invalid_utf8_result_file_warns_and_executes(tmp_path, caplog): with caplog.at_level(logging.WARNING, logger="mech_gov.experiment.identity"): result = runner.run_single(_cfg(), _llm(), skip_completed=True) assert result is not None - assert "MGI004" in _codes(caplog) + assert "MGI008" in _codes(caplog) assert str(tmp_path) not in caplog.text @@ -334,7 +375,7 @@ def test_jsonl_named_directory_warns_and_executes(tmp_path, caplog): with caplog.at_level(logging.WARNING, logger="mech_gov.experiment.identity"): result = runner.run_single(_cfg(), _llm(), skip_completed=True) assert result is not None - assert "MGI004" in _codes(caplog) + assert "MGI008" in _codes(caplog) def test_scan_failure_disables_skip_even_with_a_valid_copy(tmp_path, caplog): @@ -343,7 +384,7 @@ def test_scan_failure_disables_skip_even_with_a_valid_copy(tmp_path, caplog): (tmp_path / "unreadable.jsonl").write_bytes(b"\xff") with caplog.at_level(logging.WARNING, logger="mech_gov.experiment.identity"): assert runner.run_single(_cfg(), _llm(), skip_completed=True) is not None - assert "MGI004" in _codes(caplog) + assert "MGI008" in _codes(caplog) def test_invalid_matching_record_disables_skip_even_with_a_valid_copy(tmp_path, caplog): @@ -528,7 +569,10 @@ def test_collision_diagnostics_never_print_values(tmp_path, caplog): runner.run_single(_cfg(M), _llm("arn:aws:bedrock:eu-west-1::deployment"), True) message = _identity_warnings(caplog)[0].message assert "arn:aws" not in message - assert str(N) not in message and str(M) not in message + assert f"cases_per_condition={N}" not in message + assert f"cases_per_condition={M}" not in message + assert "mock-R2-S0-seed42" not in message + assert "reference_sha256=" in message assert "cases_per_condition" in message @@ -547,6 +591,11 @@ def test_resumable_cloud_provider_identity_excludes_credentials(provider, monkey assert parameters["provider"] == provider assert parameters["top_p"] == 1.0 assert any(key.endswith("_sha256") for key in parameters) + assert set(parameters) == { + "provider", + "top_p", + "region_sha256" if provider == "bedrock" else "endpoint_sha256", + } serialized = json.dumps(parameters) for secret in ("secret", "sk-super-secret", "internal", "us-east-1"): assert secret not in serialized From 974d975e3d1e72d3cfcc35c23eb7ae9beb48847f Mon Sep 17 00:00:00 2001 From: rschumann Date: Wed, 29 Jul 2026 16:11:22 +0200 Subject: [PATCH 4/4] fix(experiment): tolerate oversized JSON integers --- src/mech_gov/experiment/runner.py | 2 +- src/mech_gov/llm/base.py | 2 ++ tests/test_runner_identity.py | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/mech_gov/experiment/runner.py b/src/mech_gov/experiment/runner.py index 3b55793..4314b67 100644 --- a/src/mech_gov/experiment/runner.py +++ b/src/mech_gov/experiment/runner.py @@ -229,7 +229,7 @@ def _scan_completion_index(results_dir: Path, warnings: IdentityWarnings) -> _Co continue try: record = json.loads(line) - except (json.JSONDecodeError, RecursionError): + except (ValueError, RecursionError): continue if not isinstance(record, Mapping): continue diff --git a/src/mech_gov/llm/base.py b/src/mech_gov/llm/base.py index 91f68c4..774f342 100644 --- a/src/mech_gov/llm/base.py +++ b/src/mech_gov/llm/base.py @@ -72,6 +72,8 @@ def resume_identity_parameters(self) -> dict[str, Any] | None: run is really equivalent to the one being requested. Return a mapping of allowlisted public values and privacy-safe fingerprints. Include the adapter kind and enough deployment identity to distinguish backends. + The mapping is persisted verbatim as ``identity.provider_parameters`` in + every result record, so every value must be safe for long-term storage. Never return an API key, credential, raw private URL, profile name, endpoint, timeout, or path. Opaque values such as user-supplied callables cannot be fingerprinted; return ``None`` for them. diff --git a/tests/test_runner_identity.py b/tests/test_runner_identity.py index a04af33..92f3dec 100644 --- a/tests/test_runner_identity.py +++ b/tests/test_runner_identity.py @@ -349,6 +349,7 @@ def test_tampered_payload_warns_and_executes(tmp_path, caplog): "42", # valid JSON, not an object '{"run_id": 123}', # object with a non-string label '{"run_id": null}', # object with an empty label + '{"run_id":"x","seed":' + "1" * 5000 + "}", # oversized integer ], ) def test_unusable_jsonl_lines_are_tolerated(tmp_path, line, caplog):