fix(validate): auto-populate omitted locked_parameters instead of hard-failing (#298)

[namasl]

Problem

The DESIGN gate required the design agent to copy every campaign.locked_parameters key verbatim into bundle.experiment_spec.verified_parameters. A single omitted key hard-failed the whole iteration before any experiment ran — pure friction, since the campaign already declares the value. In one recent 6-iteration campaign this killed 5 of 6 iterations, none of which reached the experiment phase.

_validate_locked_parameters conflated two very different cases:

• Missing key — the agent just didn't echo a value the campaign already specifies. No information is lost.
• Deviating value — the agent wrote a different value than the campaign locked. This is the real spec-fidelity violation ([F1] Add locked_parameters schema field to enforce campaign spec-fidelity in bundles #246: DESIGN silently rewriting locked workload params).

Both hard-failed. Only the second should.

Change

• _validate_locked_parameters — the missing-key branch now auto-populates verified_parameters[k] from campaign.locked_parameters[k] (mutating the bundle in place) and continues. The value-mismatch branch is unchanged and still hard-fails, listing every deviation.
• validate_design — snapshots the bundle before validation and, if auto-fill mutated it, persists the filled verified_parameters back to bundle.yaml (via atomic_write) so downstream phases read the campaign's canonical values.
• compute_campaign_spec_diff (soft-audit mirror) — skips omitted keys so it no longer re-flags auto-filled values as violations.
• A present-but-wrong-type experiment_spec / verified_parameters (not a mapping) still hard-fails — we auto-fill omissions, not structurally broken bundles.

Why this is safe

The #246 anti-mirage protection is fully preserved: a bundle that sets a different value than the campaign still hard-fails. We only stop punishing bundles that leave a locked value unspecified, where the campaign is unambiguously the source of truth.

Acceptance criteria

• A bundle that omits some locked_parameters keys passes DESIGN, with the omitted values populated from the campaign (and persisted to bundle.yaml).
• A bundle that sets a verified_parameters value different from the campaign's still hard-fails, listing every deviation.
• compute_campaign_spec_diff reports no violation for auto-populated (omitted) keys.
• Unit tests cover both the auto-populate and the still-fail-on-mismatch paths.

Tests

The two test_f1_* tests that encoded the old hard-fail-on-omission behavior are reworked into test_298_*, covering: auto-fill of an omitted key, missing verified_parameters block, missing experiment_spec block, the mixed omit+deviate case, malformed-type still-fails, end-to-end validate_design pass + on-disk persistence, still-fail-on-deviation, and compute_campaign_spec_diff ignoring omitted keys.

tests/test_friction_245.py — 38 passed. Full suite — 1339 passed.

Docs updated: CLAUDE.md spec-fidelity section, docs/campaign-authoring-guide.md, docs/friction-245-resolution.md.

Closes #298

🤖 Generated with Claude Code

[susiejojo]

Review notes

Thanks for this — the omission-vs-deviation distinction is the right call, and the PR is well-documented (CLAUDE.md, authoring guide, and the F1 resolution row all tell a consistent story) with clean, behavioral tests. The anti-mirage deviation path is genuinely preserved and covered at both the unit and end-to-end levels. A few things stood out that I think are worth addressing before merge; happy to be wrong on any of these.

The common thread in the two bigger items is that a previously pure validator (validate_design) now has an unconditional, unsignalled disk side-effect.

Would recommend fixing before merge

1. bundle.yaml is persisted even when validation ultimately fails. In validate_design the atomic_write runs immediately after _validate_locked_parameters, but before _validate_locked_workload, _validate_depth_overrides, the physical-realism / ground-truth checks, and _check_unexpected_files. Any of those can append to errors, and the mixed omit+deviate case (_validate_locked_parameters auto-fills the omitted key and returns a deviation error) also mutates the bundle. So a run that returns status: "fail" can still have rewritten bundle.yaml on disk with auto-filled values the agent never authored — a partially-mutated artifact left behind by a failed gate. Suggestion: only persist when the final verdict is pass (capture the locked-param errors and move the atomic_write to just before the final return, gated on not errors and mutated).

2. Auto-population is silent. _validate_locked_parameters returns [] on auto-fill and appends nothing to the warnings list that validate_design already threads through to the design gate. compute_campaign_spec_diff then re-reads the persisted bundle and continues past the now-present keys, so the gate summary looks clean too. The behavior is defensible, but there's no record of which verified_parameters were agent-authored vs gate-injected — which is a bit at odds with the spirit of #246. Suggestion: append a WARN:-prefixed entry naming each auto-filled key (same convention used elsewhere in this file), so the human gate sees it.

Worth considering

3. atomic_write OSError escapes the structured-error contract. The write sits inside a try whose only handlers are yaml.YAMLError / jsonschema.ValidationError, so a read-only FS / ENOSPC / permission error would propagate out of validate_design as a raw traceback — surprising for a function whose contract is {"status", "errors"} and in a path not obviously associated with disk I/O. Consider catching OSError around the write and converting it to a structured error.

4. Test gap on the new malformed-experiment_spec branch. The PR adds elif not isinstance(spec, dict): return [...] for experiment_spec, but the only malformed test covers the sibling verified_parameters branch. A regression reintroducing bundle.get("experiment_spec") or {} would silently coerce a non-dict to {} with no failing test. A test_298_malformed_experiment_spec_still_fails would close that.

Minor / optional

• json.dumps(bundle, sort_keys=True, default=str) compares string renderings (so 1 and "1" look equal). Safe today since auto-fill only adds keys, but a copy.deepcopy + != structural compare would be exact and future-proof.
• A few tests would nail down the new contract: no-spurious-write when all keys are already present (assert the file is untouched), idempotency (two runs → identical bytes), and compute_campaign_spec_diff when experiment_spec is entirely absent.
• The compute_campaign_spec_diff comment credits the DESIGN-phase ordering for correctness, but the safety actually comes from the local if k not in verified: continue guard (its own direct-call test relies on the guard, not ordering). Minor reword would make it robust to a future out-of-order caller.

Nice work overall — items 1 and 2 are the ones I'd prioritize. 🤖 Reviewed with Claude Code.

[namasl]

Addressed in 87760c5. Walking through the items:

1. Persist even when validation fails — fixed. You're right that the write sat before the other design validators and would leave a gate-rewritten bundle.yaml behind a status: "fail" (including the mixed omit+deviate case). The atomic_write now happens once, just before the final return, gated on not errors and bundle_mutated. A failing gate leaves the file byte-for-byte as the agent authored it. New test_298_no_persist_when_gate_fails asserts exactly that (omitted key that would auto-fill + a deviation → file unchanged, no injected concurrency).

2. Auto-population is silent — fixed. _validate_locked_parameters now returns a WARN:-prefixed advisory naming every injected key, and validate_design routes it into the warnings list it already threads to the gate (same convention as _validate_physical_realism / #85). One consolidated WARN lists all keys, mirroring the one-message-lists-all-deviations format. compute_campaign_spec_diff still reports clean.

3. atomic_write OSError escapes the contract — fixed. The persist write is wrapped; a read-only FS / ENOSPC / permission error now returns {"status": "fail", "errors": ["failed to persist auto-filled bundle.yaml: ..."]} instead of a raw traceback. Verified by driving it against a read-only iter-dir.

4. Test gap on malformed experiment_spec — fixed. Added test_298_malformed_experiment_spec_still_fails covering the elif not isinstance(spec, dict) branch directly.

Minors — all taken:

• Switched mutation detection to copy.deepcopy + structural != (drops the json.dumps(..., default=str) string-rendering compare that conflated 1/"1").
• Added test_298_no_spurious_write_when_all_present (all keys present → file untouched), test_298_persist_is_idempotent (two runs → identical bytes), and test_298_compute_campaign_spec_diff_no_experiment_spec_is_all_omitted (absent experiment_spec).
• Reworded the compute_campaign_spec_diff comment to credit the local if k not in verified: continue guard rather than DESIGN-phase call ordering, so it's robust to a direct/out-of-order caller.

Full suite: 1344 passed.

🤖 Reviewed with Claude Code

[susiejojo]

Thanks for the thorough turnaround — I went through 87760c5 and everything checks out. Confirming each item:

• 1. Persist-on-failure — fixed cleanly. The write is now deferred to a single point after all validators run, gated on not errors and bundle_to_persist != bundle_snapshot, so a failed gate leaves the artifact exactly as authored. test_298_no_persist_when_gate_fails pins the mixed omit+deviate case (file byte-for-byte unchanged, no injected concurrency). This effectively makes validate_design transactional — mutate in-memory early, commit once on success — which is the right shape.
• 2. Silent auto-fill — fixed. The consolidated WARN: advisory naming every injected key, routed through the existing warnings channel, is exactly the visibility that was missing. test_298_missing_key_autofills_and_warns covers it.
• 3. atomic_write OSError — fixed; the structured {"status": "fail", "errors": [...]} return keeps the contract intact instead of leaking a traceback.
• 4. Malformed experiment_spec — test_298_malformed_experiment_spec_still_fails closes the branch gap.

The minors are all in too — copy.deepcopy structural compare, the no-spurious-write / idempotency / absent-experiment_spec tests, and the compute_campaign_spec_diff comment reworded to credit the local guard rather than call ordering.

One small optional follow-up: item 3's OSError path is the only fix without an automated regression test (the note mentions manual verification against a read-only dir). A monkeypatched-atomic_write or chmod read-only test would lock it in, but I don't consider it blocking.

LGTM. 🤖 Reviewed with Claude Code