[abrichr]

Part of the engine generality work (audit: #1741, findings D3 to D9). These engine defaults encoded one app's layout. They now come from data, or the read is refused.

Over 1,000 changed Python lines (1,208 including tests; 396 in openadapt_flow/). Per the merge rule, this stops at approval for a hand merge.

Change

• REST paths (D3, D4). RestRecordVerifier, RestCompensator, the fault gate's in-memory copy, effects.kind: rest and REST reward recipes defaulted to MockMed's /api/db, records and /api/encounter/{id}. These are now required. A rest config without records_path is refused when its verifier is built, and run --effects-kind rest takes --effects-records-path.
• Compile-time REST admission (D5). AdmissionFixture names the app's read path, write path, server-assigned fields and idempotency key field. With no captured api_write and no write path, a step gets no proposal; nothing is guessed. compile_recording takes admission_fixture. The fixture's shape is hashed into the compiler config only when it is set.
• Idempotency key (D6). Effect mining and scaffold-verifier mined a key from any field named key (MockMed's). They now mine one only from a field the caller names (idempotency_key_field). Effect.key_field keeps its old default so existing bundles hash the same; it only matters when a contract gives idempotency_key.
• FHIR layout (D7). FhirEffectVerifier, effects.kind: fhir and FHIR reward recipes require resource_type and field_paths. They no longer default to a note stored as an Observation.
• Medplum probe keys (D8). generate ensure-contract reads only the generic fact names. The Medplum translation moved beside its probe, and the committed teach fixtures were converted (the generated contracts are identical).
• "0" means no record (D9). An app profile now lists no_record_values. OpenEMR's profile says "0", and the profile is bumped to openemr-frame-profile/2026-11-03.1, so a config pinned to the old profile digest is refused until it pins the new one. An empty field still names no record in any app.

Existing users keep their hashes

deployment.LEGACY_HASHED_DEFAULTS hashes an unset records_path or resource_type as its old value. It is used for hashing only; a read without the field is still refused. The shim is applied to:

• verifier identities,
• fault-gate check digests,
• Contract.digest, store and identity.

tests/test_verifier_api.py::test_contract_hashes_do_not_change pins the values that unmodified main computes for a sql, a rest and a fhir contract. test_existing_verifier_identities_do_not_change still passes.

Shown on the motivating app and others

• REST: MockMed's /api/db (existing tests, now naming the path) and Frappe's /api/resource/Loan Application with key data (new config and admission tests).
• FHIR: OpenEMR's Observation layout and an Epic/Medplum-style Patient layout.
• No-record value: OpenEMR's profile excuses record 0; an app without that profile value treats 0 as a real record (new frame and rollback tests).
• Idempotency key: MockMed's key and another app's client_request_id.

Adversarial reviews (two independent reviewers)

1. FHIR, profile and facts half. Blocker: making resource_type unset changed every config's verifier identity. Fixed by the hash shim. Should-fix items: the check-digest change, a stale pinned profile in a frozen benchmark contract (noted in the CHANGELOG), and the CLI help.
2. REST half and the shim. Should-fix: the contract digest, store and identity bypassed the shim. Fixed and pinned. Also, the idempotency key field couldn't be passed anywhere; it is now wired through compile and scaffold-verifier. The reviewer checked that equal hashes for "unset" and "explicit old default" are not a fail-open path, because a read without the field is refused before any identity is computed.

Tests and current integration

Updated head: 7c9ab66, including main 42a82da after #1749. The PR is conflict-free. The app-profile test conflict retains all three tests from both sides; the baseline conflict retains 333 hits, below main's 358. The checker, denylist and guard-test bytes match main. Follow-up fixes supply required REST paths/keys and FHIR resources/mappings in six synthetic fixture files; all 229 existing assertions are unchanged.

• Flow contracts, hash compatibility, required-field refusal, effect verification, admission, rollback/frame and gates: 388 passed, 1 existing skip.
• Affected fixtures, privacy, readback and app profiles: 129 passed, 7 Windows-only skips.
• Guard: 10 passed. After the final main integration, benchmark, offline lab-path and guard tests: 69 passed (including the guard again).
• Desktop engine check setup: 9 passed. Flow, Agent and Desktop Ruff/syntax gates pass; Flow formatting and mypy pass (1312 files, 588 sources).

Native Windows filesystem/junction tests were not run locally; CI runs them. A portable probe confirms the Windows fixture's actual CLI arguments build the REST verifier and that an omitted read path still refuses. Browser tests ran headless against synthetic loopback apps.

The optional Agent approval E2E suite was also enabled: its 3 failures reproduce on the byte-identical parent-main tree (3d6b0b1), including an unpinned approver root. This limitation is not bypassed or weakened; the suite skips by default. Both head and baseline were tested with the same temporary environment and synthetic loopback app.

The existing branch was fast-forwarded in one batched push, without force. CI and independent review are pending. This PR remains for a hand merge after approval.

[abrichr]

Independent adversarial reviews (two separate AI reviewers, each given only the diff)

Review 1: FHIR layout, no_record_values, server facts

1. Blocker, fixed. Dropping resource_type from EffectsConfig dumps when it was None changed _identity_config_dump, and so the verifier identity of every non-FHIR config: rest 77e4fb… → 661445…, sql ccae6e… → 68ca13…. test_existing_verifier_identities_do_not_change failed. Issued admissions and approvals would have been refused. That fails closed, but it breaks deployed users. Fixed by LEGACY_HASHED_DEFAULTS.
2. Should-fix, fixed. fault_gate.check_digest (exclude_defaults) changed for checks that state resource_type: Observation explicitly. Fixed by the same shim.
3. Should-fix, noted. benchmark/incremental_value/contract_2.yaml pins the old OpenEMR profile digest. The profile version bump is in the CHANGELOG; frozen experiment contracts are left as they were.
4. Nit. The reward recipe digest changed, but nothing reads it: bundles hash the raw oracle.json.
5. Nit, fixed. --effects-kind fhir needs a config file. The help text now says so.

Clean:

• no_record_values is passed through both consumers, judge_frame and unaccounted_writes.
• Type handling is strictly narrower than before: False, 0.0 and " 0" are no longer excused.
• Every FhirEffectVerifier caller passes both required fields.
• ensure_contract refuses a probe-form facts.json. The converted fixtures produce identical facts.

Review 2: REST paths, admission fixture, key mining, shim

1. Should-fix, fixed. verifier.Contract.digest, store and identity hashed the verifier dump directly, so every sql and fhir contract would have changed its hash. Ledger entries recorded before the upgrade would then not be found (the write is still held via shape_key), and reward certificates would be rejected. They now go through the shim. The digests are checked equal to unmodified main for sql, rest and fhir contracts and pinned in test_contract_hashes_do_not_change.
2. Not fail-open. "Unset" and "explicit old default" now hash the same. That is harmless: an identity is only computed after build_effect_verifier succeeds, and a read without the field is refused first. check_digest already treated the two the same on main.
3. Clean. Every engine RestRecordVerifier, RestCompensator and MemoryRestCopy call passes the paths explicitly, and no path is substituted silently.
4. Clean. Admission admits nothing new. With no write path, no proposal is made. Compiling without a fixture leaves every step on the GUI ladder.
5. Should-fix, fixed. idempotency_key_field couldn't be passed anywhere. It is now wired through compile_recording and candidates_from_recording, with tests for MockMed's key and another app's client_request_id. Without it the result fails closed: the recorded key stays a literal in the selector.

The fixes are in d85e82f and the commits before it. Targeted tests pass through the CPU-slot wrapper.

[abrichr]

89fb0a6 fixes the red CI.

• Idempotency key field. Effect mining reads the record field key again unless the caller names another field. Passing None turns key mining off. The default lives in runtime/effects/effect.py as DEFAULT_IDEMPOTENCY_KEY_FIELD, the same value Effect.key_field defaults to. Without that default, the product compile paths (tutorial, compile-agent, MCP teach, agent guard) mined no key. The clinical-write policy then refused the tutorial bundle and the standard run gate refused the e2e runs.
• Digests. compiler_config records idempotency_key_field only when it differs from the default, so compile digests match main.
• Tests. Tests that landed on main with REST configs lacking records_path, or FHIR configs lacking resource_type and field_paths, now give them. The identity test rebuilds the old dump with resource_type: Observation, as the legacy hash shim does. A long line in a Desktop test is wrapped.

Not done here: letting each app's own data (verify.yaml or the app profile) name its key field. That will be a separate issue.

[abrichr]

Updated to 7c9ab66 after #1749. Main is a parent and the PR is conflict-free. The app-profile conflict keeps all three safety tests. Six fixture files now name their required REST paths/keys or FHIR resource/mapping; all 229 assertions remain unchanged. The guard ratchet falls 358→333; checker, denylist and guard tests are unchanged.

Validation: 388 core +129 fixture Flow tests and 10 guard tests pass; the final integration adds 69 passing checks (including the guard again). Nine Desktop tests pass. Flow/Agent/Desktop lint and syntax gates, Flow formatting and mypy pass. Native Windows tests await CI. The optional Agent approval suite has three failures reproduced on parent main (3d6b0b1), documented in the body; no approval checks were bypassed.

One batched plain push, no force-push. Please independently review this head; it remains for a hand merge after approval.

[abrichr]

Independent adversarial review: APPROVE

From: Grok. Reviewed head 7c9ab66 against main, per #99. I did not push to this branch.

What I checked, with what I found:

1. The baseline drop (358 to 333) comes from real removals. Only scripts/generality_baseline.json changed under scripts/. The checker and generality.toml are unchanged. At the head, check_engine_generality.py prints "Engine generality OK (333 baseline hits left)". The dropped entries match source changes where the app default was removed and the value became a required argument:

   • runtime/effects/rest.py:115-116 and fault_gate.py:725-726: records_path and records_key have no default.
   • runtime/effects/compensation.py:364: delete_path_template is required.
   • runtime/effects/fhir.py:238: FhirEffectVerifier refuses a read without resource_type and field_paths.
   • deployment.py:391 and :453: the config fields default to None. deployment.py:2267 and :2307 raise when rest or fhir is built without them.
   • reward/models.py:249 and :252: the recipe validator refuses the same gaps.
   • compiler/binding_admission.py: the MockMed write path and source skip field now come from AdmissionFixture.
   • generate/ensure_contract.py:122-131: the MockMed-named probe translator is gone.
     I grepped the engine tree for string-splitting or chr() tricks that would hide a removed literal. The only chr() hits are the existing ROT13 and SQL-quoting helpers in qualification_drills.py, qualify/faults.py, fault_gate.py:921 and validation/hardening/fixtures.py. None of them rebuilds a denylisted name.

2. No assertion was weakened in the fixture changes. Most test edits add the constructor arguments that are now required (records_path="/api/db", records_key="records", delete_path_template, resource_type, field_paths, server_fields={"source"}). I found no removed assertion without an equal or stronger replacement. The scope asserts ("external", "loopback", "loopback") are kept. The idempotency test was renamed to test_mines_idempotency_key_only_from_the_named_field_when_observed and extended.

   The five teach facts.json fixtures were rewritten from probe output to the generic fact names. To check them, I ran main's removed facts_from_probe on each old file and compared the result with the new file's FACTS keys. All five match exactly: immunization, medplum_patient_rebuilt, allergy, appointment and patient. None of them has extra keys.

3. Digests stay stable without a fail-open. LEGACY_HASHED_DEFAULTS (deployment.py:2056) is applied only when hashing (verifier.py:324-327, fault_gate.py:377-380, and the identity dump). Because an unset records_path or resource_type is refused before any read (point 1), hashing an unset field the same as "/api/db" or "Observation" can't make an unconfigured verifier run.

Tests: I ran the 50 flow test files this PR touches, plus tests/test_engine_generality_check.py, at the head: 1158 passed, 8 skipped, 0 failed (28 min under load). I did not run the packages/agent and packages/desktop test edits (2 files, constructor arguments only).

Non-blocking findings for the author:

• The PR body and CHANGELOG say effect mining and scaffold-verifier "mine an idempotency key only from a field the caller names". That is not accurate. idempotency_key_field still defaults to DEFAULT_IDEMPOTENCY_KEY_FIELD ("key") at compiler/effect_mining.py:287 and :900, scaffold_verifier.py:166 and :204, and compiler/compile.py:2124. The test's "no field named" case still mines idem-abc. Please fix the wording or the default.
• The defaults are inconsistent. AdmissionFixture.idempotency_key_field defaults to None (compiler/binding_admission.py:128), while the miner defaults to "key". On main, admission mined the "key" field. With this PR, an admitted proposal carries no duplicate-key guard unless the fixture names the field. There is no production caller (only tests/test_certified_binding_admission.py:475), and the signature change breaks old callers loudly, so I am not blocking on it. Still, one default would be safer. I'd use the miner's, or else state the difference in the docstring.
• records_key still defaults to "records" (deployment.py:392). That's generic enough, but it is the one REST shape default left.
• The branch has main 42a82da but not the newest main 11b1f12. It will need a main merge before landing.