Validate appended experiment versions before they are stored - #16
Merged
Conversation
…ored Appending a user assertion built the next version with `model_copy`, which does not run model validators. The aggregate rule that assertion identities are unique therefore never ran on the append path. An assertion identity covers its content, so declaring the same value with the same evidence note against the same supplemented assertion produced a second assertion with an identical identity. The resulting version was persisted and could not be validated again, so every later read of that experiment failed: reopening the demo fixture returned HTTP 422 from POST /experiments carrying a raw validator dump, and the experiment could not be recovered. The next version is now constructed through validation, and a repeat is refused with a stable message before anything is written. A correction still supersedes the active assertion, which was already the append-only path for changing a declared value. Recorded as F-067 in the failure matrix; no existing row covered an append that would violate an aggregate invariant.
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.
The defect
Reopening the demo fixture a second time against the same database returned HTTP 422 from
POST /experiments, carrying a raw validator dump:The experiment could not be recovered: every later read failed the same way.
Root cause
add_user_assertionbuilt the next version withExperiment.model_copy(update=...). Pydantic does not run model validators onmodel_copy, so the aggregate rule that assertion identities are unique never ran on the append path.An assertion identity is content-derived (
content_id("assertion", body)), so declaring the same value with the same evidence note against the same supplemented assertion yields a second assertion with an identical identity. That version was written toexperiment_versions, and_load_versionthen failed to validate it on every subsequent read — including thePOST /experimentsreplay path, which is why creating an experiment reported a validation error about an experiment it had not been asked to create.The change
Experiment(...)so the aggregate validator always runs. An invalid version can no longer reach the repository.Changing a declared value still goes through the existing append-only path: supersede the active assertion with a corrected one.
Deliberately not changed
The refusal maps to HTTP 422 through the existing
_errorhelper.409 Conflictis arguably a better fit for "this already exists", but that would mean a new typed error class and a separate API-semantics decision, so it is left for review rather than folded into a root-cause fix.Already-corrupted rows in an existing database stay unreadable; this prevents new ones. No migration or repair command is included.
Validation
pytestoffline suitepytest tests/integrationmypy --strictruff format --check,ruff checkBoth new tests were confirmed to fail before the change:
tests/domain/test_experiment_passport.py::test_repeating_an_identical_declaration_is_refused_before_it_is_stored— did not raise.tests/test_experiment_adapter_parity.py::test_repeated_declaration_reports_an_actionable_message_not_a_validator_dump— the corrupt aggregate escaped the domain and raised inside the response model, reproducing the production symptom.A second domain test asserts that an appended version validates as an aggregate, so an append that cannot be read back fails at the layer that produces it.
Verified end to end against a running stack
Driving the documented demo flow twice against
docker compose --profile demo:POST /experimentsthe experiment already records this assertionGET /experiments/{id}Failure matrix
Added F-067: appending a user assertion that would produce a version violating an aggregate invariant is rejected before persistence, with all prior versions retained. No existing row covered this; F-056 covers stale expected versions and idempotency-key reuse.
SHA256SUMS.txtregenerated for the documentation change.Related
The interface changes in #15 report this response cleanly and attribute it to the stage that issued the request. That branch does not depend on this one.