fix(experiment): make run identity collision-safe - #20
Open
rschumann wants to merge 4 commits into
Open
Conversation
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 "<run_label>--<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.
|
All contributors have signed the CLA ✍️ ✅ |
Author
|
I have read the CLA Document and I hereby sign the CLA |
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
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.
What this fixes
Closes #19.
ExperimentRunner.run_single()decided whether a run had already completed froman identifier built only from model alias, regime, condition and seed. Changing
the case count — or the resolved model, the distributions configuration, or a
provider parameter — produced the same identifier, so
skip_completed=Truecould treat a materially different experiment as already done and return
Nonewithout executing it.
Approach
The readable label and machine identity are now separate:
run_labelidentity_sha256identityrun_idIdentity covers the case count, regime, condition, seed, a digest of the
resolved model ID, a digest of the parsed distributions configuration,
declared adapter/deployment identity, constructor-configured provider generation
parameters (
top_pfor the current OpenAI-compatible and Bedrock adapters), andthe package version. Per-call
temperatureandmax_tokensare selected bygovernance-regime code and covered by that package version. The distributions
fingerprint is semantic, so comments and formatting do not invalidate earlier
runs while changing a parameter does.
New standard-library-only module
src/mech_gov/experiment/identity.pyownscanonicalization, digests, stored-record validation and safe diffs.
It never reads the environment, touches the filesystem, invokes a model, or
accepts raw provider configuration.
Failing open
Runs that cannot prove equivalence always execute rather than risk a wrong skip:
no stable non-empty
model_id, undeclared or invalid provider identity,ambiguous credentialed endpoint, unavailable package version, or unreadable
completion state. Malformed JSONL records, including integers rejected by
Python's conversion-size limit, are ignored rather than aborting the run.
Provider identity hook failures are isolated from experiment execution. A
sentinel never makes two unknown configurations compare equal.
Recomputing a run costs time; wrongly skipping one silently corrupts a result
set.
Backward compatibility
run_idare read, never rewritten ordeleted, and never treated as equivalent to a versioned identity — a
matching label logs
MGI001and the run executes.model_id, the manifest, both JSONL output paths, and theon_run_completecallback are unchanged.run_idand CLI display retain the legacy readable value. New machineidentity is additive and never parsed from
run_id.LLMInterface.resume_identity_parameters()is a concrete method returningNone, so existing custom providers keep working; they are simply treated asnon-resumable.
Warnings use stable codes
MGI001–MGI008on themech_gov.experiment.identitylogger, naming the fields that differ and using a12-hex SHA-256 reference for correlation. They never print raw labels, paths, or
configuration values;
MGI008distinguishes unreadable completion state frominvalid stored identity.
OpenAI-compatible endpoint targets and Bedrock regions are represented only by
SHA-256 fingerprints. These are pseudonymous identifiers, not encryption.
SageMaker stays non-resumable because endpoint name plus region does not prove
which account-scoped deployment is selected. Existing raw
model_idfieldsremain unchanged for compatibility. The readable model alias also remains
public in
run_id,run_labelandidentity; it must not contain credentialsor private identifiers.
Verification
100% coverage on
experiment/identity.py.regression pass on all three supported interpreters.
pass.
internal-pattern scan, changed-code Bandit comparison, tracked-tree secret
scan and private-project/path scan pass.
Windows was not executed; no platform-specific code is introduced.
Known provenance boundary: identity uses the package version. Source edits made
without a version change are outside the equivalence contract; editable-checkout
development should use
skip_completed=False.Questions for maintainers
traceability-only in the existing
code_versionfield, because_get_code_version()runsgit rev-parsein the caller's working directoryand therefore reports whatever repository the user happens to run from. Would
you prefer a stricter contract, accepting that runs from a source checkout
become non-resumable?
resume_identity_parameters()widensLLMInterface. If you would rather not, I can drop it and narrow the v1guarantee to
RunConfigplus distributions and model identity, documentingthe residual collision risk.
configuration lacks an account-scoped immutable deployment identifier. Would
you prefer an explicit safe deployment-ID field in a later change?
label-only legacy records because they cannot prove equivalence. Would you
prefer a clearly named opt-in flag that restores label-based legacy resume
while explicitly accepting collision risk?