External results importer: Stage 1 freeze import, authorized overlays, evaluation - #2
Open
cotenthusiast wants to merge 47 commits into
Open
cotenthusiast wants to merge 47 commits into
cotenthusiast wants to merge 47 commits into
Conversation
Eighth-cycle review found six residual identity gaps: - importer-core code identity was never bound into importer_implementation_identity; only adapter/validator were, so an identity.py change was invisible to realization identity. - declared sources and lineage components could be unreachable: nothing required every source to be referenced by a lineage component's source_digests, or every lineage component to own a result-origin row. - non-derived realizations (native_execution/external_import) could carry lineage components whose operation_type didn't match the declared derivation_origin, smuggling repair/transformation-labeled lineage past the authorization/overlay gates that only trigger off the top-level derivation_origin. - source provenance (source_run_id/source_repository/source_commit) could be null with no explicit reason, unlike every other nullable provenance field in this module. - runtime callable identity hashed only source/defaults, so a behavior-affecting module-level global the callable reads could change without changing the callable's identity. - ExpectedDataset derivation revision/fingerprint were accepted unvalidated, letting audit paths, timestamps, or machine-local values enter stable dataset identity. Adds one failing regression test per confirmed gap, then the narrowest production fix: bind importer-core via _runtime_callable_record; require every declared source/lineage component to be reachable from a result-origin row; reject non-matching operation_type on non-derived realizations; require unknown_reasons for null source provenance fields (reusing the existing _validate_unknown_reason_contract); bind resolved non-callable globals into the runtime callable digest; validate revision/fingerprint with the existing _validate_stable_values guard. Focused gate (tests/test_condition_grid.py + tests/importing/test_identity.py): 159 passed. Full suite: 1195 passed, only the pre-existing Python 3.14 google.genai.types deprecation warning. Claude-Session: https://claude.ai/code/session_017nAzkbNH4HiJeZnojCGhBj
An adversarial re-review of ee2108b found that the resolved-globals fix (finding #5) still had a gap: it skipped every callable global on the theory that a helper's behavior is "already covered by the callable's own source digest." That's false whenever the helper lives in a different file than the target: implementation_identity() (and therefore _runtime_callable_record) only hashes the target's own defining file, so a cross-module helper's source was never hashed by anything. Verified with a live repro: an adapter calling a helper imported from another module produced a byte-identical identity record across two genuinely different helper implementations. Fix: function/class globals are now bound via a shallow implementation_identity() call (their own defining-file digest), not expanded further, so mutually-referencing helpers can't cause unbounded or cyclic recursion. Also fail closed (raise ImportIdentityError) instead of silently dropping a resolved global that can't be represented or is an uninspectable stateful callable, matching how this module already treats unsupported callable defaults elsewhere. Removes a redundant assigned_lineage_ids recomputation the same review flagged as a trivial duplication (Minor). Adds tests/importing/_cross_module_helpers.py (two helper functions in a separate module) plus a regression test proving the callable identity now changes when the referenced cross-module helper changes, and a test proving an uninspectable resolved global is rejected rather than ignored. Focused gate (tests/test_condition_grid.py + tests/importing/test_identity.py): 161 passed. Full suite: 1197 passed, only the pre-existing Python 3.14 google.genai.types deprecation warning. Claude-Session: https://claude.ai/code/session_017nAzkbNH4HiJeZnojCGhBj
The final adversarial re-review of the cross-module-helper fix (1c6625c) found an Important performance liability: implementation_identity() walks and hashes a non-choicebench package's entire file tree, and the resolved- globals binding called it uncached for every function/class-valued global a callable references. A live repro showed a bare `from pandas import isna` costing ~0.3s per identity computation, repeated with zero memoization on every call. Fix: memoize implementation_identity's validated output per referenced object via functools.lru_cache. Functions/classes hash by identity, and their defining files don't change within a process, so caching by object identity is safe. Verified the exact reproducer now costs ~0.3s on the first call and ~9 microseconds on the second. No behavior change: same identity records, same rejection paths, just memoized. Focused gate (tests/test_condition_grid.py + tests/importing/test_identity.py): 161 passed in ~5m20s (down from ~9m24s). Full suite: 1197 passed in ~6m54s (down from ~10m44s), only the pre-existing Python 3.14 google.genai.types deprecation warning. Claude-Session: https://claude.ai/code/session_017nAzkbNH4HiJeZnojCGhBj
Reduced-scope Task 6: adds PROTOCOL_V3_VERSION/MANIFEST_V3_SCHEMA_VERSION/ RUN_STATE_V3_SCHEMA_VERSION and make_manifest_v3/validate_manifest_v3/ initial_run_state_v3/validate_run_state_v3/load_run_state_v3, dispatching strictly on exact schema_version. Every existing v2 function/constant is untouched. Deliberately narrower than the original plan: no ManifestView cross-version abstraction and no normalize_manifest() legacy-v2 synthesis. Every manifest this module builds is a fresh v3 import (a base Stage 1 import or a repair overlay import); nothing here ever needs to read an existing v2 run through a unified interface. That unification would only matter for migrating ChoiceBench's own native runner to v3, which is out of scope for finishing the importer. v3 payload holds only semantic_conditions/realizations tables (keyed by condition_id/realization_id, built by the Task 5 identity module) plus protocol/canonicalization versions; audit is a separate top-level field, excluded from the experiment digest. validate_manifest_v3 recomputes every condition/realization ID and digest from its own identity payload rather than trusting the stored value, and requires each realization's declared condition_digest to match its owning semantic condition. Focused: 13 new tests passed. Full suite: 1210 passed (1197 baseline + 13), only the pre-existing Python 3.14 google.genai.types deprecation warning. Claude-Session: https://claude.ai/code/session_017nAzkbNH4HiJeZnojCGhBj
Reduced-scope Task 7: adds PreparedResultArtifact, prepare_manifest_result, publish_manifest_result to src/choicebench/io/writers.py, and extends the existing validate_result_artifact with optional manifest/realization kwargs that dispatch to a new v2 path. Calling it with no kwargs (the only way any existing caller does) hits the exact unchanged v1 branch. prepare_manifest_result computes result identity strictly after the manifest/realization are already fixed (result_artifact_id/digest are never part of the manifest), validates that every result row matches its declared result_origin row_assignment (question_id, prediction_origin, prediction_lineage_id) with no missing/extra/mismatched rows, orders rows by the declared assignment order regardless of input order, and requires (never fabricates) the mandatory per-row identity columns (condition, realization, experiment, dataset/model/method/prompt, split) already injected upstream by row normalization. publish_manifest_result writes only to an absent result/sidecar pair; an existing pair is a byte-for-byte no-op or a hard refusal on any divergence, never a silent overwrite. Focused: 14 new tests in tests/importing/test_result_artifact.py, plus the existing tests/io/test_writers.py, tests/io/test_readers.py, tests/importing/test_manifest_v2_compat.py, and tests/test_publication_identity.py all still pass unchanged (71 total). Full suite: 1224 passed (1210 baseline + 14), only the pre-existing Python 3.14 google.genai.types deprecation warning. Claude-Session: https://claude.ai/code/session_017nAzkbNH4HiJeZnojCGhBj
…e status Collapsed Tasks 8+9 into one module (they share the same file in the original plan; Task 9 only extends Task 8's types). Reduced scope: exact option-count/method-specific-extension richness is dropped in favor of what the repair-import workflow concretely needs. validate_source_rows joins by exact string question_id via a caller- supplied semantic-field mapping (needed since the plan's own interface didn't specify how raw CSV columns map to semantic fields; this module needs that explicitly to be self-contained). No row is dropped, padded, deduplicated, reordered, or repaired: every input row is kept (with findings) except when its question_id is genuinely ambiguous (duplicated within the source). Missing/unexpected/duplicate question IDs, question- text and correct_option mismatches against the expected-dataset snapshot, and missing/invalid parsed predictions (checked against the question's real choices_json-derived option-letter range, not an invented choice_a/choice_b column shape) all accumulate as ValidationFinding records with a findings_digest. normalize_realization_rows recomputes evidence_status from exact coverage (present vs expected question IDs) and defects (findings tied to a present question_id), and raises ImportValidationError on any mismatch against the condition's declared evidence_status -- never silently trusting the caller's declaration. Only complete/qualified + scope included realizations produce evaluable_rows; partial/malformed/ recoverable/failed/excluded/held realizations produce none, matching the plan's evidence-only rule. recoverable is computed (not just copied from a declared status): coverage gaps must be an exact subset of the condition's declared recoverable_question_ids with no other defects. prepare/write/validate_realization_validation_artifact publish one self-digested JSON sidecar per realization at artifacts/imports/validation/<realization_id>.json: byte-identical existing content is a no-op, any divergence is refused, and the realization_id/digest binding is checked on read. Focused: 18 new tests, all passing in 0.15s (no identity.py-style file hashing in this module). Full suite: 1242 passed (1224 baseline + 18), only the pre-existing Python 3.14 google.genai.types deprecation warning. Claude-Session: https://claude.ai/code/session_017nAzkbNH4HiJeZnojCGhBj
Task 10, reduced scope. This is the core of the repair-queueing objective: authorization.py types a repair/offline-transformation grant as either inference_repair (executable=true, a real model may run) or offline_transformation (executable=false, non-executing semantic rematching only), refusing any type/executable mismatch. Every grant is checked against the caller's own known condition digests and expected dataset selections -- an authorization cannot grant a question outside its condition's selection, cannot have an empty reason, and cannot self-assign an authorization_id (it must equal the bundle's own recomputed digest, so tampering with the source bytes used to build it is caught by ID mismatch, not a separate checksum field the schema doesn't carry). authorization_for_condition slices one condition's grant out of a bundle while keeping the whole bundle's ID/digest, so a slice can never borrow another condition's grants or diverge from its source bundle. overlays.py's derive_overlay is a pure function over an already-verified VerifiedBaseRealization (never a bare path or unverified dict): it checks the overlay's base/authorization binding, requires exact evidence-digest agreement across the overlay declaration, the authorization's granted evidence, and the base's own evidence sources, and refuses replacing an unauthorized or out-of-selection question. Reuses OverlaySpec's existing result_origin field (ResultOriginSpec) for per-question origin assignment rather than inventing new parameters: inference_repair requires an explicit, valid repair origin for every replaced question (no bare "mixed", no missing/extra assignments); offline_transformation forbids declaring a new origin and retains the base response's own underlying prediction_origin. Builds each replaced row's lineage component via the existing Task 5 identity.py helpers, so the narrower "declared_external" identity/source-ownership checks already built into make_lineage_component apply here unchanged. Scope boundary (documented in the module docstring): derive_overlay covers only the transformation's own replaced rows and their lineage; it does not merge them with the base's retained rows or construct the final realization -- that full-graph assembly belongs to the import engine (Unit G), which alone has the base run directory and destination for the new run. Focused: 21 new tests (11 authorization, 10 overlays), all passing in under 0.2s. Full suite: 1263 passed (1242 baseline + 21), only the pre-existing Python 3.14 google.genai.types deprecation warning. Claude-Session: https://claude.ai/code/session_017nAzkbNH4HiJeZnojCGhBj
Task 11, reduced scope. evidence.py: open_verified_source reads a declared regular file exactly once and hashes those same bytes forward (never re-reads or re-derives the digest separately), refuses symlinks and paths outside an optional containment root, and requires the read bytes match the declared expected_sha256. write_evidence_blob stores referenced row-level bytes at a run-local sha256-addressed path; identical bytes already staged are reused, any divergent existing blob or sidecar is refused. write_evidence_index/validate_evidence_index publish and verify one self-digested index over all evidence records, transitively validating every referenced blob. transaction.py: fsync_tree flushes every file and directory under a staged tree before publication. atomic_publish_directory_no_replace uses a real renameat2(RENAME_NOREPLACE) syscall wrapper (isolated so tests can mock its absence) and fails closed if the primitive is unavailable -- never falls back to os.replace or an existence-check race. ImportTransaction holds the run's manifest lock for its lifetime, stages new runs under a same-filesystem sibling directory marked with an owner file (so cleanup only ever touches its own staging directory, never an unrelated leftover), and for an existing run skips staging entirely -- publish() calls the caller's validator against the already-published final directory instead. Focused: 19 new tests (11 evidence, 8 transaction), all passing in under 4s together with tests/test_release_adversarial.py and tests/infra/. Full suite: 1282 passed (1263 baseline + 19), only the pre-existing Python 3.14 google.genai.types deprecation warning. Claude-Session: https://claude.ai/code/session_017nAzkbNH4HiJeZnojCGhBj
Task 12, base (non-overlay) import path. build_import_plan resolves sources/datasets/models/methods/prompts, opens and parses each condition's declared CSV(s), validates rows against the expected dataset snapshot, computes evidence status, builds one lineage component per evaluable row, and assembles a v3 manifest -- performing every read/validation/identity step without writing anything. execute_import(dry_run=True) returns the same report a real import would, with wrote_artifacts=False and no workspace directories created. A real import stages evidence blobs/index, the manifest, run state, per-realization validation artifacts, and result CSVs through ImportTransaction, publishing once atomically; an existing run is verified in place (idempotent no-op) rather than restaged. verify_import_run is the shared full-graph verifier: it validates the manifest, run state, evidence index, every validation artifact against its recorded checksum, and every result artifact, returning immutable VerifiedBaseRealization values only after the whole graph passes -- the foundation the not-yet-wired overlay path (see below) will build on. Report schema is reduced from the plan: counts (conditions/evidence_status/ scope_disposition), a defect summary, and condition/realization digests -- enough to confirm exact status/queue reproduction and reject held/excluded work, not the full historical checksum-report/column-disposition breakdown. Two supporting fixes surfaced by wiring real code through Task 5's identity layer: - identity.py's canonicalize() gained bytes/bytearray support (hex-encoded). Real functions reference plain data constants (e.g. csv_adapter.py's UTF-8 BOM marker) that a resolved-globals check now inspects; canonicalize previously had no representation for bytes at all. Narrow and additive: the only previous behavior for bytes was an unconditional TypeError, and the full suite (1289 tests, up from 1282) confirms no existing caller relied on that. - The engine binds importer/lineage runtime-callable identity to small local marker functions rather than the real parse_csv_source/ validate_source_rows: those reference many stdlib globals (hashlib.sha256, a C builtin; re; etc.) that are neither plain data nor inspectable user-level code, which Task 5's runtime-callable identity correctly refuses rather than silently ignoring. Binding to engine.py-local markers still captures "did this engine's own orchestration change" (identity hashes the whole defining file); the installed package version already carried in importer_implementation_identity is the primary signal for "did the installed release change." Documented in engine.py. Scope boundary (unchanged from the module docstring, now proven out by a working base-import path): applying an authorized repair/offline- transformation overlay on top of a published base run is the next increment. Every piece it needs -- identity, validation, authorization, overlay derivation, evidence storage, atomic transactions, and this module's verify_import_run -- is built and tested; the merge-and-publish orchestration itself is not yet wired into execute_import. Focused: 7 new engine tests (plan validity, dry-run no-op, real import, idempotence, verify, failure reporting, report serialization), all passing together with every prior importing/* + schema test (223 total). Full suite: 1289 passed (1282 baseline + 7), only the pre-existing Python 3.14 google.genai.types deprecation warning. Claude-Session: https://claude.ai/code/session_017nAzkbNH4HiJeZnojCGhBj
Task 13, reduced scope. Adds read_manifest_result_set/ build_evaluation_report_for_result_set to io/readers.py; the existing read_manifest_results() v2 wrapper is untouched, and cli/evaluate_run.py (the pre-existing v1-only evaluator CLI) is not modified -- direct Python API access is sufficient for the research workflow this session targets, consistent with deferring the installed-CLI work (Task 17). read_manifest_result_set never reads a manifest/state/result directly; it goes through engine.py's verify_import_run (the shared full-graph verifier), so an evaluated result set can never be built from an unverified or tampered run. Auto-selection picks the single eligible (result-bearing) realization per condition and refuses when a condition has more than one, matching the plan's ambiguity-is-a-refusal rule; explicit realization_ids selection validates every ID is both declared and eligible before reading any CSV. build_evaluation_report_for_result_set accounts for every realization in the manifest, not just selected ones: each carries its evidence_status, scope_disposition, prediction_origins, and a selected flag; only selected realizations get computed accuracy metrics, so partial/malformed/held/ excluded realizations are visible in the report with empty metrics rather than silently dropped. Focused: 5 new tests, all passing together with the existing v2 reader/ evaluator tests (36 total) with zero changes to their behavior. Full suite: 1294 passed (1289 baseline + 5), only the pre-existing Python 3.14 google.genai.types deprecation warning. Claude-Session: https://claude.ai/code/session_017nAzkbNH4HiJeZnojCGhBj
Task 15, reduced scope. translate_stage1_paper_freeze() verifies checksums for the canonical manifest, cell-status/expected matrices, and all four queue files against checksums/checksums.sha256, parses the canonical manifest into per-cell records, and explodes the queue files into (cell_id, question_id) pairs -- reproducing the exact counts the research objective needs to confirm. Verified directly against the real, read-only Stage 1 freeze at /home/cotenthusiast/Projects/model-generalization/paper_data_freeze during development (never copied into the repo or committed): 138 approved / 687 held / 3 excluded pairs, all pairwise disjoint; the 100/84/4/12/2 paper- matrix breakdown; and the 6-cell/18-pair offline-recoverable authority (exact cell IDs and question IDs matching) all reproduce exactly. One real correction this verification surfaced: an earlier assumption (from this session's plan-based review, before real freeze access) that rerun_queue.csv was a disjoint "776 forensic" authority tier was wrong. Running the checksum-verified parser against the real file showed all 776 of its pairs are already a subset of the 828 classified (approved/held/ excluded) pairs -- and rerun_queue.csv carries no queue_disposition/ execution_authority/executable columns at all, unlike the three classification files. Fixed to verify "rerun candidates are a subset of classified" (a real, meaningful invariant) rather than a disjointness check that would reject the real data. Fail-closed invariants enforced: approved rows must declare queue_disposition=approved/execution_authority=authoritative/ executable=true; held and excluded rows must declare executable=false; approved/held/excluded must be pairwise disjoint; every rerun-queue candidate pair must already be classified; all cells claiming recoverable_question_ids must authorize the exact same question set. Scope boundary (module docstring): this does not yet build ImportSpec/ AuthorizationSpec objects for the generic engine -- that needs the ARC/MMLU ExpectedDataset trust chain (Task 16/Unit J, next) to validate authorization grants against real selected question IDs, plus per-cell SourceArtifactSpec construction across the freeze's distinct method CSV schemas. Focused: 10 new tests against a small synthetic fixture (never real historical data, per the plan's global constraint), all passing. Full suite: 1304 passed (1294 baseline + 10), only the pre-existing Python 3.14 google.genai.types deprecation warning. Claude-Session: https://claude.ai/code/session_017nAzkbNH4HiJeZnojCGhBj
Task 16, reduced scope. build_stage1_expected_datasets() verifies checksums for each benchmark's raw/normalized/robustness_ids/ robustness_metadata files, then independently recomputes question_text/ correct_option/correct_answer_text/choices from each raw row and compares them fieldwise against the archived normalized row at the same file position. Raw and normalized files have identical row counts and order in the real freeze, so a positional join is sufficient and avoids needing to reproduce the archived normalizer's own question_id hash algorithm -- this is a ChoiceBench revalidation of internal freeze consistency, recorded as a limitation on the returned ExpectedDataset, not a claim that the archived normalizer or upstream publisher was authentic. MMLU duplicate question_ids are handled with drop_duplicates(keep="first") semantics, but only after requiring every duplicate occurrence to agree on all parsed fields -- a disagreeing duplicate fails closed rather than silently picking one. Selected question IDs (from robustness_ids.json, preserving its own order, which differs from source file order) must all exist in the deduplicated normalized set or the whole benchmark is refused. Deduplicated rows are canonicalized into fresh CSV bytes and handed to the existing Task 4 build_expected_dataset()/DatasetReferenceSpec machinery, which independently re-validates and re-derives all identity digests. Verified directly against the real, read-only Stage 1 freeze (never copied into the repo) during development: both ExpectedDataset objects build successfully with exactly 1000 selected questions each, every one of 1172 ARC and 14042 MMLU rows revalidates, and all 27 real MMLU duplicate groups are confirmed field-identical before deduplication. This surfaced one genuine, verified freeze quirk: 3 raw ARC questions have a 5th option that the archived normalizer silently drops (all 3 correct answers fall within the first 4, so truncation never removes the correct option) -- modeled explicitly with a fail-closed bound check rather than either ignoring the option or crashing on an honest schema mismatch. Scope boundary (unchanged from the module docstring, now fully proven out): this and Unit I (queue/status/matrix accounting) are the two verified building blocks a full ImportSpec/AuthorizationSpec assembly for the actual repair-import workflow would combine; that combination -- plus per-cell SourceArtifactSpec construction across the freeze's method CSV schemas -- is not done here. Focused: 8 new tests (ARC variable/five-option handling, MMLU duplicate dedup and disagreement rejection, checksum/revalidation/row-count/ missing-selected-ID failures) against a synthetic fixture verified to match the real freeze's exact file formats; 18 total in this file, all passing. Full suite: 1312 passed (1304 baseline + 8), only the pre-existing Python 3.14 google.genai.types deprecation warning. Claude-Session: https://claude.ai/code/session_017nAzkbNH4HiJeZnojCGhBj
…erride - engine.py: execute_overlay_import/_merge_overlay_realization merge an authorized repair/offline-transformation overlay with a verified base run's retained rows into a new, separate, immutable run. Row/lineage assignments are reassembled in the expected dataset's question order (not retained-then-replaced concatenation order), matching make_realization's ordered-subset identity requirement. Divergent overwrite of an existing overlay run_id now raises (matching the base import path's verify_import_run(expected=...) contract) instead of silently treating a different experiment as an idempotent no-op. - engine.py: ImportRequest gains an optional expected_datasets override so a profile needing dataset-specific revalidation/dedup (e.g. Stage 1's ARC/MMLU trust chain) can supply pre-built ExpectedDataset objects instead of the generic per-source rebuild, which cannot reproduce that logic (needed for MMLU's real duplicate question_ids). - validation.py: extract compute_evidence_status as a reusable pure function from normalize_realization_rows (no behavior change), so a profile assembling declarations can discover the correct evidence_status to declare instead of guessing against the declaration-mismatch check. - tests/importing/test_engine_overlay.py (new): merge, offline- transformation, dry-run, idempotence, divergent-overwrite refusal, unknown-base-realization, forged-source rejection. - tests/importing/test_engine.py: expected_datasets override coverage. 1321 full tests passed. Claude-Session: https://claude.ai/code/session_017nAzkbNH4HiJeZnojCGhBj
- profiles/stage1_paper_freeze.py: build_stage1_import_spec assembles all 102 real cells into one ImportSpec (104 sources: 102 per-cell canonical CSVs + 2 documentation-only ARC/MMLU normalized-reference sources bound to ImportRequest.expected_datasets, never opened by build_import_plan). Each condition's declared evidence_status is reconciled via an actual probe (validate_source_rows + compute_evidence_status) rather than trusted blindly from the freeze's own STATUS_MAP-mapped status. build_stage1_cell_authorization builds one AuthorizationSpec scoped to exactly one cell (a multi-cell bundle can never satisfy derive_overlay's evidence-digest cross-check for any single cell's overlay -- verified by reading overlays.py). VERIFIED DIRECTLY against the real freeze (never committed): assembly runs cleanly end-to-end and reconciles to complete=49/malformed=45/ qualified=8 (vs. the freeze's own 57/28/5/4/6/2 STATUS_MAP breakdown). Confirmed by inspecting finding codes that the entire divergence is MISSING_PREDICTION (zero CORRECT_OPTION_MISMATCH/INVALID_PREDICTION/ UNEXPECTED_QUESTION_ID anywhere, i.e. no column-mapping bug): 20 of 28 "malformed_requires_inference" cells have zero row-level defects (their defect, e.g. a phantom/NaN rendered option D, is parse_ok/valid-answer and invisible to generic validation) and reconcile to "complete"; 28 of 57 "canonical_complete" cells have MISSING_PREDICTION rows the freeze's own classification tolerates (one MMLU cell has 152/1000 parse_missing rows, its own score_status="score_unscorable", yet is declared "canonical_complete") that ChoiceBench's fail-closed generic validator does not -- a genuine, intentional strictness difference, not a bug. All 102 conditions' build_import_semantic_identity calls succeed with 102 unique condition_digests. - tests/importing/test_stage1_import_spec.py (new): synthetic-fixture coverage for assembly shape, the recoverable-cell and malformed-cell reconciliations, excluded-cell scope_disposition, the per-cell checksum-ledger cross-check, and a full build_stage1_cell_authorization -> validate_authorization_bundle round trip. Also fixes a real bug this surfaced: build_stage1_import_spec's ImportConditionSpec left unknown_reasons empty despite declaring calibration_identity/preflight_identity as None, violating identity.py's unknown-reason contract. 520 focused / 520 targeted tests passed (tests/importing/). Claude-Session: https://claude.ai/code/session_017nAzkbNH4HiJeZnojCGhBj
…y gap) Discovered and fixed while running Task 19's full-workflow validation of the importer against the real, read-only Stage 1 freeze: - engine.py: ImportRequest/OverlayImportRequest gain an optional source_containment_root, separate from workspace_root, for open_verified_source's path-escape check. Sources previously had to live under workspace_root itself; importing a real, independent, read-only data root into an isolated temporary CHOICEBENCH_HOME needs these to differ. Defaults to workspace_root, so all existing behavior/tests are unaffected. 2 new tests cover the rejection and the override. - provenance.py: implementation_identity() called importlib.metadata.packages_distributions() -- an uncached full scan of every installed distribution's metadata -- once per lineage component (i.e. potentially once per row). At 102 conditions x up to 1000 rows, that's ~100,000 calls of something that cannot change within a process's lifetime, and it made a real import run take 30+ minutes instead of ~4. Fixed with a simple process-wide cache (_cached_packages_distributions). This also cut the full test suite from ~8-9 minutes to ~64 seconds, since many tests exercise make_realization/implementation_identity too. - identity.py: _validate_realization_identity required "complete"/ "qualified" realizations to have full result-origin coverage of the expected dataset regardless of scope_disposition, but normalize_realization_rows's own evaluable computation already treats scope_disposition != "included" as non-evaluable regardless of evidence_status. This combination (individually row-complete/qualified, but excluded_from_paper_matrix for an unrelated scientific reason) was never exercised before real data -- the real freeze has exactly 2 such cells (both "pride"/qwen-2.5-7b-instruct-turbo). Widened the check to match normalize_realization_rows's own definition. 2 new tests: one regression (full coverage still required when included), one for the newly-allowed case. Full Task 19 validation (assemble all 102 real cells, dry run, real import into an isolated CHOICEBENCH_HOME, idempotent re-import, verify representative complete/qualified/malformed/variable-option-ARC cases, apply one synthetic inference-repair overlay and one offline- transformation-mechanism overlay into new immutable derived runs, read + evaluate all three runs, confirm divergent-overwrite refusal, confirm the freeze remains byte-identical) now passes end-to-end against the real freeze. Finding for the record (not a bug): of the 102 real cells, ChoiceBench's independently-computed evidence_status (complete=49/qualified=8/ malformed=45) disagrees substantially with the freeze's own STATUS_MAP- mapped classification (57/5/28/4/6/2) -- confirmed via finding codes to be entirely MISSING_PREDICTION (no mapping bug). Also: none of the 6 real offline-transformation-authority cells currently have an evaluable base realization (each has 112-229/1000 rows with a parse failure unrelated to their 3 authorized recoverable questions), so the offline-transformation overlay demo above uses a different, evaluable real cell to prove the mechanism, not one of the 6 official recoverable cells. 1332 full tests passed (up from 1321; +2 engine, +2 identity, no regressions). Claude-Session: https://claude.ai/code/session_017nAzkbNH4HiJeZnojCGhBj
…cesses Two independent bounded reviews (spec-compliance and adversarial code quality) of commits 965dc77..79fffee both found the same Critical bug: _merge_overlay_realization built retained_lineage_components by iterating retained_lineage_ids, a set, so its order depended on Python's per-process string hash randomization. That order flowed unchanged into realization_identity["lineage_components"], which make_realization hashes in list order -- so the merged realization/experiment digest was not reproducible across processes whenever more than one base row was retained (the normal case for a real repair overlay: replace a handful of questions, retain the rest). A fresh-process idempotent re-import could then spuriously compute a different experiment_digest and raise "belongs to a different experiment" instead of the intended no-op. Fix: build lineage_components in the same deterministic order already used for row_assignments (expected_dataset question order), via a lineage_id -> component lookup keyed off the already-ordered row_assignments list, instead of iterating a set directly. retained_lineage_ids itself is unchanged (still a set) since its only remaining use, lineage_runtime_callables, is a dict keyed by lineage_id where iteration order doesn't matter. Added a deterministic (non-flaky) regression test: a 3-question overlay retaining 2 rows and replacing 1, asserting lineage_components matches row_assignments' order exactly. Verified the test fails against the pre-fix code (order came out ['q2','q3','q1'], the old "retained then replaced" order) and passes with the fix (['q1','q2','q3'], matching expected_dataset's own order) -- confirming it actually catches the bug, not just probabilistically via hash-seed comparison. Also extended test_engine.py's _raw_spec/_spec fixtures with an optional question_ids parameter (default unchanged) to support building larger synthetic fixtures. 1333 full tests passed. Claude-Session: https://claude.ai/code/session_017nAzkbNH4HiJeZnojCGhBj
Backport of ac949ed from analysis/final-paper-authoritative-run. _merge_overlay_realization computed transformation_input_digest and preownership_output_digest from derive_overlay's sorted(replacement_ids) order, but identity.py recomputes them from the realization's own dataset-ordered lineage_components. When >=2 replaced IDs' dataset order differs from their sorted order, integrity_digest (which hashes list order) diverged and identity validation raised a false-positive 'offline_transformation overlay input, output, or implementation digest conflicts with its lineage components.' Compute the digests from the same dataset-ordered, operation_type-filtered derived components the verifier uses. Adds a real-lineage regression test (reverse dataset-order IDs). (cherry picked from commit ac949ed)
…se realizations Backport of 0f1c0a6 from analysis/final-paper-authoritative-run. The overlay-merge path previously gated inference_repair/offline_transformation overlays on the base realization being evaluable (a published, scoreable result). This wrongly refused overlays against a malformed-but-structurally- complete base -- e.g. a cell whose repair-target rows are fine but which has unrelated, benign parse-missing rows elsewhere -- even though such a base has a full, publishable canonical row set safe to patch. Introduces a distinct overlayable gate: the base must have published a full result CSV whose row-identity is exactly the expected complete question set (verified via verify_import_run reading results for a new 'published_unscored' run status, alongside the existing 'completed'). Structurally broken bases (missing/duplicate/unexpected rows, or no published result) are still refused fail-closed with the same behavior as before. After a merge, the derived realization's evidence_status is recomputed from the actual merged row content (never forced to the overlay's declared status), while the overlay's declared/intended historical status is preserved separately as declared_evidence_status. The base realization remains immutable throughout. (cherry picked from commit 0f1c0a6)
…r portion) Backport of 2477625 from analysis/final-paper-authoritative-run, importer portion only. verify_import_run's per-row dict conversion re-coerced a blank (unparseable) predicted_option cell to a float NaN even after DataFrame-level NA cleanup (pandas' per-row Series dtype inference during iterrows() collapses a real None back to NaN -- confirmed empirically). The manifest writer's canonicalize() rejects NaN outright, so a malformed-but-structurally-complete base with a genuinely blank prediction in a retained (non-overlaid) row previously blew up mid-overlay. Normalize per-value at the read boundary instead. A 'completed' realization's rows never contain NaN (evaluable requires a valid prediction in every row), so this is a no-op on the existing path. Adds a real-engine regression test in test_engine_overlay.py. The original commit's other half -- import_orchestration.py's authorized_question_ids cross-check against a patch's declared question IDs -- lives in experiments/final_paper_analysis/, which does not exist on this branch (that's analysis-branch-specific orchestration code built on top of this importer, not part of the importer itself). Only the importer-side fix applies here; nothing in this branch currently calls the overlay-request builders that half of the original commit hardened. (cherry picked from commit 2477625)
…rter portion) Backport of daed183 from analysis/final-paper-authoritative-run, importer portion only. csv_adapter.py's _validate_structured_choices required every choice object to carry an explicit string 'label' key; real fourth-cell data only carries an integer 'source_index' (no label at all). Now accepts either an explicit string label (existing behavior, unchanged) or a positional integer index (0 -> 'A', 1 -> 'B', ...), letterized before the existing duplicate-label check. Added regression tests: positional labels accepted, out-of-range index rejected, duplicate positional index rejected. The original commit's other half -- import_orchestration.py's build_fourth_cell_source switching to mode='structured_json' against the real choices_json/source_index shape -- lives in experiments/final_paper_analysis/, which does not exist on this branch (analysis-branch-specific orchestration built on top of this importer). Only the importer-side format-handling fix applies here. (cherry picked from commit daed183)
…kages test_built_wheel_and_sdist_run_outside_repository's functional CLI venv was created with --system-site-packages and installed the wheel with --no-deps, assuming the runtime deps (pandas, etc.) would be inherited from context. Under a genuinely clean venv/container --system-site-packages inherits an empty or unrelated site-packages instead, so choicebench-run etc. crash on missing imports. Reproduced with PYTHONNOUSERSITE=1: the nested venv had no pandas. Fixed by making the venv fully isolated and installing the wheel with its full dependency set instead of relying on inheritance. Verified green under PYTHONNOUSERSITE=1 (full suite, 810 passed) to confirm the fix holds without this machine's incidental ~/.local package leak.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an external-results importer to ChoiceBench, scoped to the actual research objective (import the immutable Stage 1 data freeze, authorize and apply repair/offline-transformation overlays, evaluate) rather than the original, larger ~19-task plan. Built additively on top of the existing manifest v2 system — no v2 code path is touched.
make_manifest_v3/validate_manifest_v3/initial_run_state_v3/validate_run_state_v3, alongside the untouched v2 schema.inference_repair(executable) vsoffline_transformation(non-executable) authorization bundles; pure derivation of replaced-row lineage/content from an authorized overlay.renameat2(RENAME_NOREPLACE)-backed no-clobber publication.build_import_plan/execute_import/verify_import_runfor the base (non-overlay) path, andexecute_overlay_import/_merge_overlay_realizationto merge an authorized overlay with a verified base run's retained rows into a new, separate, immutable run — the base run is never mutated.read_manifest_result_set/build_evaluation_report_for_result_set, built on the same full-graph verifier as everything else.profiles/stage1_paper_freeze.py— the only module allowed to know paper-specific facts): verified queue/status/matrix translation, an ARC/MMLU expected-dataset trust chain with real-freeze-verified revalidation and dedup, fullImportSpecassembly across all 102 real cells with per-condition evidence-status reconciliation, and a per-cellAuthorizationSpecbuilder.Explicitly out of scope per the reduced objective: CLI, docs, experiment methods/configs, repair execution (running new inference), the final 2×2 analysis, job submission, native-run v3 migration.
Validation
Ran the complete workflow against the real, read-only Stage 1 freeze (a 3.9GB, 102-cell research data snapshot) via a one-off validation script (not committed):
ImportSpec.CHOICEBENCH_HOME.This surfaced and fixed 4 real bugs (none previously caught by synthetic-fixture tests alone): overlay row/lineage order not following the expected dataset's own question order; no way to declare a source containment root independent from the run workspace; an uncached, O(all-installed-packages) call in provenance identity hashing that made a 102-condition/100k-row run take 30+ minutes instead of ~4 (also cut the full test suite from ~8–9 min to ~64s); and an identity check that required full row coverage for "qualified" realizations regardless of scope disposition, missed because "qualified but excluded from the paper's matrix" was never exercised before real data.
Two independent bounded reviews (spec-compliance, adversarial/code-quality) of everything built this session both found the same additional bug — overlay
lineage_componentsbuilt from set iteration, making the merged realization digest nondeterministic across processes — fixed with a deterministic regression test verified to fail against the pre-fix code.Finding for the record (not a bug): ChoiceBench's independently-computed evidence status for the 102 real cells (complete=49/qualified=8/malformed=45) disagrees substantially with the freeze's own classification (57/5/28/4/6/2) — confirmed via finding codes to be entirely
MISSING_PREDICTION, not a mapping bug. Also, none of the 6 real offline-transformation-authority cells currently have an evaluable base realization (each has 112–229/1000 rows with an unrelated parse failure), so the offline-transformation overlay demo used a different, evaluable real cell to prove the mechanism.Test plan
pytest -q)https://claude.ai/code/session_017nAzkbNH4HiJeZnojCGhBj