Complete #13: metamorphic checks, source pins, connector pilot, and the OpenMapBench interface - #21
Conversation
…ddressing contract Three no-golden-answer relations -- input permutation invariance, duplicate resistance, and positive-buffer monotonicity -- each declare the precondition that makes them valid, execute the project's own canonical entrypoint in an isolated variant workspace, and report not_testable with the reason when the precondition does not hold on the data. Unknown relations, non-growing variants, and duplicate resistance declared for a count or sum are declaration failures rather than skipped checks. runtime.implementation.parameters (openmapstack-parameters/v1) addresses one knob of the pipeline through an argument or environment binding; verify fails when the declared canonical value drifts from the bound step. The fixture generator gains the parameter plus three pipeline-logic break modes that its copied pipeline.py reproduces on rerun; cases 015 and 923-925 prove the healthy, deliberate-defect, and isolation paths. Refs #13 (B4). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TMMy6A73zi7yGjK3wHBhju
… sources A source whose bytes come from a query must say which of two pin classes froze them: a hash-matched local snapshot under data/source/, or a backend snapshot with an identifier, capture time, and retention limit. A snapshot that cannot deliver its bytes again -- missing, edited, expired, or last verified inaccessible -- is reported as not_reproducible rather than pinned because a string is present. A mutable alias such as 'latest' in version.identifier stays unpinned whatever else is declared. Secrets never enter project.yaml: access.connection must be a reference (env:, service:, keyring:, or a file outside the project) and every source block is scanned for password fragments, credentialed URLs, and well-known key shapes. Findings name the path and pattern, never the secret. The fixture generator pins all three sources as local snapshots and gains two break modes; cases 926 and 927 prove the failure paths. Refs #13 (B5a). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TMMy6A73zi7yGjK3wHBhju
…ts (pilot) openmapstack source discover/snapshot implements the connector pilot for the reference pair, DuckDB local files and PostGIS. Credentials are resolved from a reference (env:, service:, file:) and never recorded; sessions are read-only with a statement timeout; only a single SELECT is accepted; a snapshot is a dry run until --approve, is bounded by row and byte limits, lands only under data/source/, never overwrites, and returns the local_snapshot pin plus query and schema digests for the manifest. The DuckDB connector confines file access to its root and exposes files as views. PostGIS has no durable time travel, so its pin is the local snapshot; pg_current_snapshot() is recorded as non-durable retrieval metadata. Unverified backends are refused rather than approximated. references/user-data-sources.md documents the four rules and the manual path for other warehouses; a scheduled workflow runs the live PostGIS round trip against a service container. Refs #13 (B5b). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TMMy6A73zi7yGjK3wHBhju
openmapstack-check-api/v1 is the surface OpenMapBench consumes without vendoring a check: api_info/negotiate for compatibility, list_checks for the catalogue with parameters, oracle_free flags and reporting dimensions, run_check returning an openmapstack-check-result/v1 record, and the packaged verify-result schema. A check that raises is not_testable with check_error, never a pass or a graded failure. Reporting dimensions are owned by openmapstack.api and imported by the eval runner; visual checks now report under their own visual_judgement bucket. CLI: openmapstack checks, check NAME WORKSPACE --arg k=v, api-info [--require-api --min-version --require-check]. The consumer fixture test proves the contract from the CLI and JSON schemas alone. Refs #13 (C1). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TMMy6A73zi7yGjK3wHBhju
…nd task export openmapstack skill-snapshot --out DIR creates the controlled copy of SKILL.md, references/, and templates/ with a per-file inventory and content hash (openmapstack-skill-snapshot/v1); --inspect re-verifies one. Symlinks and escaping inventory paths are refused. The eval runner now records the same snapshot for the oms arm. A live run records one openmapstack-benchmark-arm/v1 record per arm: skill hash and commit, task-set hash, checker and check-API versions, harness commit, runtime, adapter and agent version, model, sampling, and --price-catalog-date; unknown values are null, never omitted. --arms paired runs plain and oms over identical cases, trials, and seeds and reports them side by side with task parity, per-arm quality with a Wilson interval, per-arm cost, and trajectory diagnostics; no combined score is published. --export-tasks writes vendor-neutral openmapstack-benchmark-task/v1 bundles, marking cases 070-073 as OpenMapBench-owned. Refs #13 (C2, C3). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TMMy6A73zi7yGjK3wHBhju
Two synthetic projects built by pure-Python pipelines over invented geodata (trail/river crossings with a metamorphic relation and a declared parameter; facilities per district with an override, an unverified expectation, and a warning) are verified with --metamorphic, and their text and JSON reports are compared with committed goldens after path and digest normalisation. The JSON also validates against the packaged verify-result schema. Writing the first project surfaced a genuine finding: GeoJSON without a crs member reads as WGS84, so declaring EPSG:3301 for it fails geodata.dataset_crs_is correctly. The project now writes the crs member; the debugging notes record that trap and the pipeline-copy requirement for metamorphic mutations. Refs #13 (definition of done). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TMMy6A73zi7yGjK3wHBhju
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f55cea52f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| def _write_parquet(duck, destination: Path, columns, geometry_columns, srids, rows) -> None: | ||
| duck_types = { | ||
| "int2": "SMALLINT", "int4": "INTEGER", "int8": "BIGINT", "float4": "FLOAT", "float8": "DOUBLE", | ||
| "numeric": "DOUBLE", "bool": "BOOLEAN", "date": "DATE", "timestamp": "TIMESTAMP", "timestamptz": "TIMESTAMPTZ", |
There was a problem hiding this comment.
When a PostGIS query returns a numeric column with more precision than an IEEE-754 double can represent—common for identifiers and high-precision measurements—this mapping silently rounds the value while materializing the supposedly immutable snapshot. The resulting Parquet file is then hashed and accepted as reproducible even though it no longer contains the source value; map numeric to a lossless decimal representation instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 5af4cda: NUMERIC is now materialised as DECIMAL(p, s) with the widest precision and scale present in the result, or as exact text beyond DECIMAL(38). The live PostGIS test seeds numeric(24, 9) and asserts an exact round trip; a unit test covers the beyond-38 fallback.
| for case_dir in sorted(case_dirs): | ||
| case_def = _load_case(case_dir) | ||
| ids.append(case_def.get("id", case_dir.name)) |
There was a problem hiding this comment.
When live mode uses the default unfiltered case selection, case_dirs includes fixture-only cases that run_case later marks skipped, but this loop still adds all of their IDs and files to the arm's task_set. Published arm provenance therefore claims an exact task set containing many tasks that were never run, so comparisons cannot reliably identify the workload; filter this input to live-capable cases before building the IDs and digest.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 5af4cda: the task-set hash now covers only live-capable cases, so fixture-only cases that live mode skips no longer enter an arm's workload identity. The paired-arm test includes a fixture-only neighbour and asserts it is absent.
| "tool_surface": {"adapter": agent_name, "agent_version": agent_version}, | ||
| "model": {"provider": provider, "id": model, "revision": metadata.get("model_revision")}, |
There was a problem hiding this comment.
When --agent is omitted, the runner still resolves a per-case adapter, but this record emits a null adapter/provider because it uses the CLI override rather than agent_run.agent. It also records the requested model instead of agent_run.model, even though the Claude adapter deliberately reports the model actually observed. Thus otherwise successful benchmark arms can publish missing or incorrect execution identity despite that information being present in the normalized trial records.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 5af4cda: arm provenance takes the adapter and model from the normalized trial records (what actually ran), falling back to the CLI values only when no trial reported them. Test runs without --agent and with a requested alias, and asserts the resolved adapter and observed model.
| for entry in sorted(entries, key=lambda item: item["path"]): | ||
| digest.update(entry["path"].encode("utf-8")) | ||
| digest.update(b"\0") | ||
| digest.update(bytes.fromhex(entry["sha256"].removeprefix("sha256:"))) | ||
| digest.update(b"\0") |
There was a problem hiding this comment.
The previous benchmark runner hashed each relative path followed by the raw file bytes, whereas this implementation hashes the path followed by the file's SHA-256 digest bytes. Consequently, an unchanged skill receives a different content_sha256 after upgrading to this commit, contradicting the stated historical-comparability guarantee and making old and new benchmark arms appear to use different skill content. Preserve the prior raw-byte algorithm or explicitly version the digest scheme.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 5af4cda: the content hash is the historical raw-byte algorithm again (path, NUL, bytes, NUL per file in path order), so an unchanged skill keeps the hash recorded by earlier arms. A test pins the algorithm inline.
| if scheme == "file": | ||
| target = project_path(root, remainder.strip()) | ||
| if target is not None: | ||
| return "access.connection file references must point outside the project directory" |
There was a problem hiding this comment.
For a reference such as file:../secrets/postgis.dsn, project_path returns None because the path resolves outside the project, so this validator accepts it. The connector subsequently rejects the same reference because resolve_connection_reference requires file: paths to be absolute, meaning preflight and no_inline_credentials can pass a manifest that every source operation will reject. Enforce the same absolute-path rule here.
AGENTS.md reference: AGENTS.md:L139-L142
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 5af4cda: validation now applies the connector's rule (absolute path, outside the project) to file: references, so preflight and provenance.no_inline_credentials reject file:../secrets/postgis.dsn the same way source discover/snapshot would.
… references - PostGIS NUMERIC is materialised as DECIMAL with the widest precision and scale present, or as exact text beyond DECIMAL(38); a DOUBLE mapping rounded identifiers and high-precision measurements inside a snapshot that was then hashed and pinned as immutable. Live and unit tests round trip numeric(24, 9) exactly. - The arm's task-set hash covers only live-capable cases; fixture-only cases in the selection are skipped in live mode and no longer inflate the workload identity. - Arm provenance records the adapter and model that actually ran, taken from the normalized trial records, rather than the CLI override and the requested alias. - The skill snapshot content hash is the historical raw-byte algorithm (path, NUL, bytes, NUL), so an unchanged skill keeps the hash recorded in earlier benchmark arms; a test pins the algorithm. - file: connection references must be absolute and outside the project at validation time, matching the connector, so preflight cannot accept a reference every source operation refuses. Refs #13, PR #21 review. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TMMy6A73zi7yGjK3wHBhju
The package gains substantial new CLI surface in this release: openmapstack source discover/snapshot, checks, check, api-info, skill-snapshot, and verify --metamorphic, plus the openmapstack-check-api/v1 interface external harnesses consume. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TMMy6A73zi7yGjK3wHBhju
1 participant
Completes the remaining workstreams of #13: B4 (conditional metamorphic checks), B5a (source pin contract), B5b (safe connector pilot), C1 (versioned check API), C2 (skill snapshots and arm provenance), and C3 (paired arms and task export), plus the definition-of-done evidence that
verifyreports stably on projects whose data this repository has never seen.What changed
B4 — conditional metamorphic checks (
openmapstack/metamorphic.py,openmapstack/parameters.py)validation.metamorphic[]declares three relations, each with the precondition that makes it valid:input_permutation_invariance(declared tie-break, unique key),duplicate_resistance(declared dedup key, set semantics only; counts and sums are rejected),positive_buffer_monotonicity(declared inclusion predicate, strictly growing variant).not_testablewith the reason; unknown relations and invalid declarations fail rather than skip.runtime.implementation.parameters(openmapstack-parameters/v1) is the versioned parameter-addressing contract;project.parameters_match_stepscatches drift between the declared canonical value and the bound step.openmapstack verify --metamorphic; eval assertionmetamorphic.relation_holds; cases 015 and 923–925 prove healthy, deliberate-defect, and isolation paths; unit tests cover positive, defect, and invalid-precondition paths for every relation.B5a — source pin contract (
openmapstack/sources.py)pin.class: local_snapshot(hash-matched file underdata/source/) orbackend_snapshot(identifier, capture time, retention limit, optional verification). A snapshot that cannot deliver its bytes again isnot_reproducible; a mutable alias inversion.identifierstayssource_unpinnedwhatever else is declared.project.yaml:access.connectionmust be a reference and every source is scanned for embedded credentials (source.credentials,provenance.no_inline_credentials); findings name the path and pattern, never the secret. Cases 926 and 927.B5b — connector pilot (
openmapstack/connectors/,openmapstack source discover|snapshot)SELECTonly; dry run until--approve; row and byte limits; never overwrites; returns the pin block and query/schema digests. DuckDB file access is confined to the source root; PostGIS recordspg_current_snapshot()as non-durable retrieval metadata because PostgreSQL has no durable time travel. Unverified backends are refused.references/user-data-sources.mddocuments the four rules and the manual path for other warehouses. A new scheduled workflow runs the live PostGIS round trip against a service container; the same test ran green locally againstpostgis/postgis:16-3.4.C1 — versioned check API (
openmapstack/api.py,docs/openmapbench-interop.md, ADR 0004)openmapstack-check-api/v1:api-info/negotiate,checks,check NAME WORKSPACE --arg k=v, packagedcheck-resultandverify-resultschemas. Reporting dimensions are owned byopenmapstack.api.DIMENSIONSand imported by the eval runner; visual checks now report undervisual_judgement, metamorphic undermetamorphic_evidence. The consumer fixture test uses only the CLI and schemas.C2 / C3 — snapshots, arm provenance, paired arms, task export (
openmapstack/snapshot.py,evals/run.py)openmapstack skill-snapshot --out DIR | --inspect DIR(openmapstack-skill-snapshot/v1), symlink- and escape-safe; the eval runner records the same snapshot.openmapstack-benchmark-arm/v1tuple per arm (skill hash, task-set hash, checker and API versions, harness commit, runtime, adapter and agent version, model, sampling,--price-catalog-date).--arms pairedrunsplainandomsover identical cases, trials, and seeds and reports them side by side with task parity and trajectory diagnostics; no combined score.--export-tasks DIRwritesopenmapstack-benchmark-task/v1bundles and marks 070–073 as OpenMapBench-owned.Evidence
python evals/run.py --mode fixture: all contract and mutation cases pass; new mutations 923–927 are detected and isolated.tests/test_verify_foreign.pyverifies two projects absent from the fixtures against committed goldens.Not in this PR
openmapstackgains new CLI surface (source,checks,check,api-info,skill-snapshot,verify --metamorphic), so the next release should be 0.3.0.Closes #13.
🤖 Generated with Claude Code
https://claude.ai/code/session_01TMMy6A73zi7yGjK3wHBhju