diff --git a/packages/agent/tests/test_approval_flow.py b/packages/agent/tests/test_approval_flow.py index e3ffa7e510..acc71b9e25 100644 --- a/packages/agent/tests/test_approval_flow.py +++ b/packages/agent/tests/test_approval_flow.py @@ -91,6 +91,8 @@ def _bridge( "web", "--effects-kind", "rest", + "--effects-records-path", + "/api/db", "--effects-base-url", mockmed["base_url"], ), diff --git a/packages/desktop/tests/test_engine/test_check_setup.py b/packages/desktop/tests/test_engine/test_check_setup.py index 12ca9ea5ad..c0a994b0be 100644 --- a/packages/desktop/tests/test_engine/test_check_setup.py +++ b/packages/desktop/tests/test_engine/test_check_setup.py @@ -279,7 +279,9 @@ def test_fixed_values_are_the_recorded_ones_and_follow_the_sealed_version(tmp_pa with pytest.raises(CheckSetupError, match="Unknown value"): answer_values(dispatcher, "wf1", ["member_id"], answered_by="person") - store.write_record_check_config("wf1", {"kind": "rest", "base_url": BASE}) + store.write_record_check_config( + "wf1", {"kind": "rest", "records_path": "/api/db", "base_url": BASE} + ) saved = store.load("wf1") saved["record_check_config"] = str(store.root / "wf1.record-check.yaml") store.save("wf1", saved) diff --git a/packages/flow/CHANGELOG.md b/packages/flow/CHANGELOG.md index 69f9426052..fea6cab853 100644 --- a/packages/flow/CHANGELOG.md +++ b/packages/flow/CHANGELOG.md @@ -43,6 +43,10 @@ ### Fixes +- A REST read names its own records path: `effects.kind: rest`, a reward recipe of kind `rest`, `RestRecordVerifier`, `RestCompensator` and the fault gate's in-memory copy require `records_path` (and `records_key`, `delete_path_template`) instead of defaulting to the demo app's `/api/db`; `run --effects-kind rest` takes `--effects-records-path`. Compile-time REST binding admission (`compile_recording(admit_api_bindings=True, admission_fixture=...)`) takes the app's read path, write path, server-assigned fields and idempotency key field from an `AdmissionFixture`, and proposes nothing when no write was captured and no write path is named. Effect mining and `scaffold-verifier` mine an idempotency key only from a field the caller names. Verifier identities and fault-gate check digests of existing configs are unchanged. + +- A FHIR read names its resource and field map: `effects.kind: fhir`, a reward recipe of kind `fhir` and `FhirEffectVerifier` require `resource_type` and `field_paths` instead of defaulting to one app's note-as-Observation layout (`DEFAULT_OBSERVATION_PATHS` stays importable for callers whose app stores notes that way). An app profile's `no_record_values` lists the record-field values its audit entries use for "no record" (OpenEMR's profile: `"0"`; profile version `openemr-frame-profile/2026-11-03.1`, so a config that pins the old profile digest is refused until it pins the new one); the frame and rollback no longer treat `0` as "no record" in every app. `generate ensure-contract` reads only the generic server-fact names from `facts.json`; the Medplum probe's translation moved to `benchmark/fhir_medplum/readpath/facts.py`. + - A qualification case can run on `--backend windows`. `WindowsBackend` now answers the qualification environment observer from one `/context/identity` call: the foreground executable's name, the SHA-256 of that executable file, the live logon-session digest, and a digest of the machine id and exact OS build. Before, `qualify run-case` on Windows refused with "qualification backend cannot observe the complete environment". The digest is withheld when the file changed after the process started (an updater replacing a running program), and it describes the host executable for an app run by an interpreter, browser or shared runtime. Guarded input now also binds the digest and the environment, so a same-named program from another path cannot take the foreground between the check and the input. The agent's session digest now accepts a connected RDP or Citrix session, not only the console session, so the agent can run inside a remote user session. An agent without the new `context_environment_v1` fields still refuses ([#1696](https://github.com/OpenAdaptAI/OpenAdapt/issues/1696)). - A covered-band dismiss step takes its dialog box from the window the backend proved covers the band, before any pixel search, so a native message box that the accessibility tree doesn't label as a dialog can be dismissed. Without a proved window the step is refused as before. diff --git a/packages/flow/benchmark/contract_autogen/blind.py b/packages/flow/benchmark/contract_autogen/blind.py index a3be44a130..f2fb98ac65 100644 --- a/packages/flow/benchmark/contract_autogen/blind.py +++ b/packages/flow/benchmark/contract_autogen/blind.py @@ -25,7 +25,6 @@ import hashlib import json import os -import shutil import sys import time from pathlib import Path @@ -33,6 +32,7 @@ from benchmark.fhir_medplum.fixture import FixtureError, MedplumFixture, _http from benchmark.fhir_medplum.readpath import suite as runner +from benchmark.fhir_medplum.readpath.facts import generic_facts HERE = Path(__file__).resolve().parent PROBE = HERE.parent / "fhir_medplum" / "readpath" / "probe.json" @@ -101,7 +101,7 @@ def cmd_teach(args: argparse.Namespace) -> int: out / "search_parameters.json", read_all(f"{fx.fhir_url}/SearchParameter?base={rtype}&_count=200", reader), ) - shutil.copyfile(PROBE, out / "facts.json") + _dump(out / "facts.json", generic_facts(json.loads(PROBE.read_text()))) _dump( out / "teach.json", { diff --git a/packages/flow/benchmark/contract_autogen/build_patient_teach.py b/packages/flow/benchmark/contract_autogen/build_patient_teach.py index 5cdfb085be..40c584cad5 100644 --- a/packages/flow/benchmark/contract_autogen/build_patient_teach.py +++ b/packages/flow/benchmark/contract_autogen/build_patient_teach.py @@ -18,6 +18,7 @@ import json from pathlib import Path +from benchmark.fhir_medplum.readpath.facts import generic_facts HERE = Path(__file__).resolve().parent SRC = HERE.parent / "fhir_medplum" / "readpath" @@ -123,7 +124,14 @@ def main() -> None: (OUT / "search_parameters.json").write_text( json.dumps(params_bundle, indent=1) + "\n" ) - (OUT / "facts.json").write_text((SRC / "probe.json").read_text()) + (OUT / "facts.json").write_text( + json.dumps( + generic_facts(json.loads((SRC / "probe.json").read_text())), + indent=2, + sort_keys=True, + ) + + "\n" + ) teach = { "resource_type": "Patient", "deployment": { diff --git a/packages/flow/benchmark/contract_autogen/teach/immunization/facts.json b/packages/flow/benchmark/contract_autogen/teach/immunization/facts.json index ed88066a65..a6c9b649c5 100644 --- a/packages/flow/benchmark/contract_autogen/teach/immunization/facts.json +++ b/packages/flow/benchmark/contract_autogen/teach/immunization/facts.json @@ -1,87 +1,8 @@ { - "audit_lag_s": { - "C": { - "max": 0.043, - "median": 0.036 - }, - "D": { - "max": 0.049, - "median": 0.036 - }, - "U": { - "max": 0.043, - "median": 0.031 - } - }, - "audit_names_the_user_profile": true, - "audit_seen_on_an_immediate_read": [ - "C by bot", - "C by bot", - "D by bot", - "U by other" - ], - "audit_writes_are_logged": false, - "bot_admin_endpoint": "HTTP 403", - "bot_is_project_admin": false, - "bot_is_super_admin": false, - "bundle_and_graphql_writes_logged": [ - "C Observation by bot", - "C Observation by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot" - ], - "date_header": true, - "delete_in_history": true, - "meta_author_returned": false, - "policy": "AccessPolicy/f348be14-df4d-47cc-a49b-1f45e08dd554", - "restricted_bot_audit_read_only": true, - "restricted_bot_audit_writes": { - "create": "HTTP 403", - "delete": "HTTP 403", - "update": "HTTP 403" - }, - "restricted_bot_can_write": { - "Observation": "allowed", - "Patient": "allowed", - "Task": "allowed" - }, - "sweep_sees_delete": false, - "type_history": "HTTP 404", - "unrestricted_bot_audit_writes": { - "create": null, - "delete": null, - "update": null - } + "agent_cannot_write_audit": true, + "audit_lag_max_s": 0.049, + "audit_logs_writes": true, + "audit_names_actor": true, + "history_lists_versions": true, + "meta_author_returned": false } diff --git a/packages/flow/benchmark/contract_autogen/teach/medplum_patient_rebuilt/facts.json b/packages/flow/benchmark/contract_autogen/teach/medplum_patient_rebuilt/facts.json index ed88066a65..a6c9b649c5 100644 --- a/packages/flow/benchmark/contract_autogen/teach/medplum_patient_rebuilt/facts.json +++ b/packages/flow/benchmark/contract_autogen/teach/medplum_patient_rebuilt/facts.json @@ -1,87 +1,8 @@ { - "audit_lag_s": { - "C": { - "max": 0.043, - "median": 0.036 - }, - "D": { - "max": 0.049, - "median": 0.036 - }, - "U": { - "max": 0.043, - "median": 0.031 - } - }, - "audit_names_the_user_profile": true, - "audit_seen_on_an_immediate_read": [ - "C by bot", - "C by bot", - "D by bot", - "U by other" - ], - "audit_writes_are_logged": false, - "bot_admin_endpoint": "HTTP 403", - "bot_is_project_admin": false, - "bot_is_super_admin": false, - "bundle_and_graphql_writes_logged": [ - "C Observation by bot", - "C Observation by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot" - ], - "date_header": true, - "delete_in_history": true, - "meta_author_returned": false, - "policy": "AccessPolicy/f348be14-df4d-47cc-a49b-1f45e08dd554", - "restricted_bot_audit_read_only": true, - "restricted_bot_audit_writes": { - "create": "HTTP 403", - "delete": "HTTP 403", - "update": "HTTP 403" - }, - "restricted_bot_can_write": { - "Observation": "allowed", - "Patient": "allowed", - "Task": "allowed" - }, - "sweep_sees_delete": false, - "type_history": "HTTP 404", - "unrestricted_bot_audit_writes": { - "create": null, - "delete": null, - "update": null - } + "agent_cannot_write_audit": true, + "audit_lag_max_s": 0.049, + "audit_logs_writes": true, + "audit_names_actor": true, + "history_lists_versions": true, + "meta_author_returned": false } diff --git a/packages/flow/benchmark/fhir_medplum/readpath/facts.py b/packages/flow/benchmark/fhir_medplum/readpath/facts.py new file mode 100644 index 0000000000..402a152d29 --- /dev/null +++ b/packages/flow/benchmark/fhir_medplum/readpath/facts.py @@ -0,0 +1,47 @@ +"""Translate this probe's output into the generic server facts. + +``openadapt_flow.generate.ensure_contract`` reads only the generic names in +``FACTS`` from a teach directory's ``facts.json``; what each Medplum probe +measured, and under which name, stays here with the probe. + + PYTHONPATH=. python -m benchmark.fhir_medplum.readpath.facts probe.json > facts.json +""" + +from __future__ import annotations + +import json +import sys +from typing import Any, Mapping + +from openadapt_flow.generate.ensure_contract import FACTS + + +def generic_facts(probe: Mapping[str, Any]) -> dict[str, Any]: + """The generic facts this probe's output supports, by their generic names.""" + + if all(k in probe for k in FACTS): + return {k: probe[k] for k in FACTS} + out: dict[str, Any] = {} + logged = probe.get("bundle_and_graphql_writes_logged") + if isinstance(logged, list): + out["audit_logs_writes"] = len(logged) > 0 + for name, key in ( + ("audit_names_actor", "audit_names_the_user_profile"), + ("agent_cannot_write_audit", "restricted_bot_audit_read_only"), + ("history_lists_versions", "delete_in_history"), + ("meta_author_returned", "meta_author_returned"), + ): + if key in probe: + out[name] = probe[key] + lags = probe.get("audit_lag_s") + if isinstance(lags, Mapping) and lags: + out["audit_lag_max_s"] = max( + float(v.get("max", 0)) for v in lags.values() if isinstance(v, Mapping) + ) + return out + + +if __name__ == "__main__": + with open(sys.argv[1], encoding="utf-8") as f: + json.dump(generic_facts(json.load(f)), sys.stdout, indent=2, sort_keys=True) + sys.stdout.write("\n") diff --git a/packages/flow/benchmark/fhir_medplum/run3/teach/allergy/facts.json b/packages/flow/benchmark/fhir_medplum/run3/teach/allergy/facts.json index ed88066a65..a6c9b649c5 100644 --- a/packages/flow/benchmark/fhir_medplum/run3/teach/allergy/facts.json +++ b/packages/flow/benchmark/fhir_medplum/run3/teach/allergy/facts.json @@ -1,87 +1,8 @@ { - "audit_lag_s": { - "C": { - "max": 0.043, - "median": 0.036 - }, - "D": { - "max": 0.049, - "median": 0.036 - }, - "U": { - "max": 0.043, - "median": 0.031 - } - }, - "audit_names_the_user_profile": true, - "audit_seen_on_an_immediate_read": [ - "C by bot", - "C by bot", - "D by bot", - "U by other" - ], - "audit_writes_are_logged": false, - "bot_admin_endpoint": "HTTP 403", - "bot_is_project_admin": false, - "bot_is_super_admin": false, - "bundle_and_graphql_writes_logged": [ - "C Observation by bot", - "C Observation by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot" - ], - "date_header": true, - "delete_in_history": true, - "meta_author_returned": false, - "policy": "AccessPolicy/f348be14-df4d-47cc-a49b-1f45e08dd554", - "restricted_bot_audit_read_only": true, - "restricted_bot_audit_writes": { - "create": "HTTP 403", - "delete": "HTTP 403", - "update": "HTTP 403" - }, - "restricted_bot_can_write": { - "Observation": "allowed", - "Patient": "allowed", - "Task": "allowed" - }, - "sweep_sees_delete": false, - "type_history": "HTTP 404", - "unrestricted_bot_audit_writes": { - "create": null, - "delete": null, - "update": null - } + "agent_cannot_write_audit": true, + "audit_lag_max_s": 0.049, + "audit_logs_writes": true, + "audit_names_actor": true, + "history_lists_versions": true, + "meta_author_returned": false } diff --git a/packages/flow/benchmark/fhir_medplum/run3/teach/appointment/facts.json b/packages/flow/benchmark/fhir_medplum/run3/teach/appointment/facts.json index ed88066a65..a6c9b649c5 100644 --- a/packages/flow/benchmark/fhir_medplum/run3/teach/appointment/facts.json +++ b/packages/flow/benchmark/fhir_medplum/run3/teach/appointment/facts.json @@ -1,87 +1,8 @@ { - "audit_lag_s": { - "C": { - "max": 0.043, - "median": 0.036 - }, - "D": { - "max": 0.049, - "median": 0.036 - }, - "U": { - "max": 0.043, - "median": 0.031 - } - }, - "audit_names_the_user_profile": true, - "audit_seen_on_an_immediate_read": [ - "C by bot", - "C by bot", - "D by bot", - "U by other" - ], - "audit_writes_are_logged": false, - "bot_admin_endpoint": "HTTP 403", - "bot_is_project_admin": false, - "bot_is_super_admin": false, - "bundle_and_graphql_writes_logged": [ - "C Observation by bot", - "C Observation by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot" - ], - "date_header": true, - "delete_in_history": true, - "meta_author_returned": false, - "policy": "AccessPolicy/f348be14-df4d-47cc-a49b-1f45e08dd554", - "restricted_bot_audit_read_only": true, - "restricted_bot_audit_writes": { - "create": "HTTP 403", - "delete": "HTTP 403", - "update": "HTTP 403" - }, - "restricted_bot_can_write": { - "Observation": "allowed", - "Patient": "allowed", - "Task": "allowed" - }, - "sweep_sees_delete": false, - "type_history": "HTTP 404", - "unrestricted_bot_audit_writes": { - "create": null, - "delete": null, - "update": null - } + "agent_cannot_write_audit": true, + "audit_lag_max_s": 0.049, + "audit_logs_writes": true, + "audit_names_actor": true, + "history_lists_versions": true, + "meta_author_returned": false } diff --git a/packages/flow/benchmark/fhir_medplum/run3/teach/patient/facts.json b/packages/flow/benchmark/fhir_medplum/run3/teach/patient/facts.json index ed88066a65..a6c9b649c5 100644 --- a/packages/flow/benchmark/fhir_medplum/run3/teach/patient/facts.json +++ b/packages/flow/benchmark/fhir_medplum/run3/teach/patient/facts.json @@ -1,87 +1,8 @@ { - "audit_lag_s": { - "C": { - "max": 0.043, - "median": 0.036 - }, - "D": { - "max": 0.049, - "median": 0.036 - }, - "U": { - "max": 0.043, - "median": 0.031 - } - }, - "audit_names_the_user_profile": true, - "audit_seen_on_an_immediate_read": [ - "C by bot", - "C by bot", - "D by bot", - "U by other" - ], - "audit_writes_are_logged": false, - "bot_admin_endpoint": "HTTP 403", - "bot_is_project_admin": false, - "bot_is_super_admin": false, - "bundle_and_graphql_writes_logged": [ - "C Observation by bot", - "C Observation by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "C Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "D Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot", - "U Patient by bot" - ], - "date_header": true, - "delete_in_history": true, - "meta_author_returned": false, - "policy": "AccessPolicy/f348be14-df4d-47cc-a49b-1f45e08dd554", - "restricted_bot_audit_read_only": true, - "restricted_bot_audit_writes": { - "create": "HTTP 403", - "delete": "HTTP 403", - "update": "HTTP 403" - }, - "restricted_bot_can_write": { - "Observation": "allowed", - "Patient": "allowed", - "Task": "allowed" - }, - "sweep_sees_delete": false, - "type_history": "HTTP 404", - "unrestricted_bot_audit_writes": { - "create": null, - "delete": null, - "update": null - } + "agent_cannot_write_audit": true, + "audit_lag_max_s": 0.049, + "audit_logs_writes": true, + "audit_names_actor": true, + "history_lists_versions": true, + "meta_author_returned": false } diff --git a/packages/flow/benchmark/harness/instrument/competitor_drift.py b/packages/flow/benchmark/harness/instrument/competitor_drift.py index 2abf9fe980..ffc2d2a069 100644 --- a/packages/flow/benchmark/harness/instrument/competitor_drift.py +++ b/packages/flow/benchmark/harness/instrument/competitor_drift.py @@ -416,7 +416,9 @@ def run_task( self, task: DriftTask, target_url: str, *, max_steps: int ) -> AgentRunResult: base = self._base_of(target_url) - verifier = RestRecordVerifier(base) + verifier = RestRecordVerifier( + base, records_path="/api/db", records_key="records" + ) # Observe the system of record BEFORE acting (an agent may inspect # state around its own action) so the honest policy can detect # collateral loss, not just its own row. @@ -736,7 +738,7 @@ def _run_one( db.reset(seed_concurrent=task.scenario.seed_concurrent) before_snapshot = db.snapshot()["records"] - verifier = RestRecordVerifier(base) + verifier = RestRecordVerifier(base, records_path="/api/db", records_key="records") before_state = verifier.capture_pre_state() target_url = f"{base}/?fault={task.scenario.name}" diff --git a/packages/flow/benchmark/harness/silent_wrong_action.py b/packages/flow/benchmark/harness/silent_wrong_action.py index daeb0c7c9f..1c3b954437 100644 --- a/packages/flow/benchmark/harness/silent_wrong_action.py +++ b/packages/flow/benchmark/harness/silent_wrong_action.py @@ -322,7 +322,7 @@ def run_one(base: str, db: Any, scenario: Scenario) -> dict[str, Any]: db.reset(seed_concurrent=scenario.seed_concurrent) before_snapshot = db.snapshot()["records"] - verifier = RestRecordVerifier(base) + verifier = RestRecordVerifier(base, records_path="/api/db", records_key="records") before_state = verifier.capture_pre_state() statuses = _drive(base, scenario) diff --git a/packages/flow/benchmark/openemr_e2e/harness.py b/packages/flow/benchmark/openemr_e2e/harness.py index 114e17d82d..5a496a9a6b 100644 --- a/packages/flow/benchmark/openemr_e2e/harness.py +++ b/packages/flow/benchmark/openemr_e2e/harness.py @@ -40,6 +40,8 @@ from __future__ import annotations +from openadapt_flow.runtime.effects.fhir import DEFAULT_OBSERVATION_PATHS + import json import os import time @@ -231,7 +233,9 @@ def _replay( replayer = Replayer( backend, vision=vision, - effect_verifier=RestRecordVerifier(url.rstrip("/")), + effect_verifier=RestRecordVerifier( + url.rstrip("/"), records_path="/api/db", records_key="records" + ), poll_interval_s=0.01, ) report = replayer.run( @@ -272,7 +276,12 @@ def _live_probe( from openadapt_flow.runtime.effects import FhirEffectVerifier token = os.environ.get("OPENEMR_FHIR_TOKEN") - verifier = FhirEffectVerifier(base.rstrip("/"), access_token=token) + verifier = FhirEffectVerifier( + base.rstrip("/"), + resource_type="Observation", + field_paths=dict(DEFAULT_OBSERVATION_PATHS), + access_token=token, + ) try: state = verifier.capture_pre_state() reachable = bool(state.reachable) diff --git a/packages/flow/benchmark/undo_proposal/run.py b/packages/flow/benchmark/undo_proposal/run.py index 2aa395db65..73af447bba 100644 --- a/packages/flow/benchmark/undo_proposal/run.py +++ b/packages/flow/benchmark/undo_proposal/run.py @@ -194,7 +194,7 @@ def _contract(directory: Path, base_url: str, fixture: Fixture, body: dict) -> P write_bundle( directory, contract_id=f"undo_propose_{fixture.name}", - oracle={"kind": "rest", "base_url": base_url}, + oracle={"kind": "rest", "base_url": base_url, "records_path": "/api/db"}, channel="api", key=Ed25519PrivateKey.generate(), issuer_key_id="self_signed:benchmark", @@ -219,7 +219,11 @@ def _deployment(path: Path, fixture: Fixture, base_url: str, contract: Path) -> { "name": f"{fixture.name}-test-copy", "backend": {"kind": "web", "url": base_url + "/" + fixture.query}, - "effects": {"kind": "rest", "base_url": base_url}, + "effects": { + "kind": "rest", + "base_url": base_url, + "records_path": "/api/db", + }, "actuation": {"api": True, "base_url": base_url}, "test_copy": { "declared": True, diff --git a/packages/flow/docs/EFFECT_KIT.md b/packages/flow/docs/EFFECT_KIT.md index cb02250ed9..1d16f7f373 100644 --- a/packages/flow/docs/EFFECT_KIT.md +++ b/packages/flow/docs/EFFECT_KIT.md @@ -1014,6 +1014,8 @@ Two kit-wide conventions: ```yaml effects: kind: rest + base_url: https://app.example + records_path: /api/records # required: the app's own records read auth: bearer_env: SOR_BEARER_TOKEN # or header+value_env, or basic_env ``` @@ -1094,6 +1096,7 @@ effects: kind: fhir base_url: https://fhir.example.org/r4 resource_type: Observation + field_paths: {id: id, patient: subject.reference, note: valueString} smart_backend: client_id: my-registered-client scope: system/Observation.rs diff --git a/packages/flow/docs/deployment.example.yaml b/packages/flow/docs/deployment.example.yaml index 52e088f96a..1afb43b39c 100644 --- a/packages/flow/docs/deployment.example.yaml +++ b/packages/flow/docs/deployment.example.yaml @@ -128,8 +128,12 @@ effects: # field: patient_id # added to every row for matching # fhir (used when kind: fhir) ---------------------------------------------- - # base_url: https://openemr.example.org/apis/default/fhir - # resource_type: Observation + # base_url: https://fhir.example.org/r4 + # resource_type: Observation # required: the app's resource + # field_paths: # required: where each field lives + # id: id + # patient: subject.reference + # note: valueString # search_params: { category: vital-signs } # search_param_exprs: # patient: { param: patient_ref } # bound per-run diff --git a/packages/flow/openadapt_flow/__main__.py b/packages/flow/openadapt_flow/__main__.py index 63af7ba7f3..bf260a735a 100644 --- a/packages/flow/openadapt_flow/__main__.py +++ b/packages/flow/openadapt_flow/__main__.py @@ -420,6 +420,8 @@ def _deployment_sections(args: argparse.Namespace): effects = effects.model_copy(update={"kind": args.effects_kind}) if getattr(args, "effects_base_url", None): effects = effects.model_copy(update={"base_url": args.effects_base_url}) + if getattr(args, "effects_records_path", None): + effects = effects.model_copy(update={"records_path": args.effects_records_path}) if getattr(args, "effects_root", None): effects = effects.model_copy(update={"root": args.effects_root}) @@ -8736,9 +8738,10 @@ def _add_deployment_flags( default=None, help=( "System-of-record EffectVerifier to wire so consequential writes " - "are verified against the real record (not the screen). The sql/" - "file kinds need their config fields (sql_query, root, ...) from " - "a --config deployment YAML" + "are verified against the real record (not the screen). rest " + "needs --effects-records-path; the fhir/sql/file kinds need their " + "config fields (resource_type and field_paths, sql_query, root, " + "...) from a --config deployment YAML" ), ) p.add_argument( @@ -8746,6 +8749,11 @@ def _add_deployment_flags( default=None, help="Base URL for the rest / fhir effect verifier", ) + p.add_argument( + "--effects-records-path", + default=None, + help="Path whose GET returns the app's records (rest effect verifier)", + ) p.add_argument( "--effects-root", default=None, diff --git a/packages/flow/openadapt_flow/app_profiles/__init__.py b/packages/flow/openadapt_flow/app_profiles/__init__.py index e559b6aeee..88747bef5f 100644 --- a/packages/flow/openadapt_flow/app_profiles/__init__.py +++ b/packages/flow/openadapt_flow/app_profiles/__init__.py @@ -10,7 +10,10 @@ rule's ``columns`` (none, by default) and may never go. With ``insert_entry_names_no_record``, the table is an id map or link row the app inserts with a save: the acting user's own insert entry for it that - names no record is the app's bookkeeping, not a write the run hid. + names no record is the app's bookkeeping, not a write the run hid. An entry + names no record when its record field is empty, or holds one of the + profile's ``no_record_values`` (an app may file such entries under record + ``0``). - ``columns``: an existing row may change in the named columns, and no row may be added or removed. Narrowed by ``fill_only`` (only from NULL or empty to a value: a backfill, never a swap, overwrite or clear), ``with_fill`` @@ -147,6 +150,9 @@ class AppProfile(BaseModel): system_columns: list[str] = Field(default_factory=list) #: Where the acting user's id is looked up from their sign-in name. users: Optional[UsersTable] = None + #: Record-field values with which the app's audit entries name no record + #: (besides an empty field), for ``insert_entry_names_no_record`` rules. + no_record_values: list[str] = Field(default_factory=list) rules: list[ProfileRule] @model_validator(mode="after") diff --git a/packages/flow/openadapt_flow/app_profiles/openemr.yaml b/packages/flow/openadapt_flow/app_profiles/openemr.yaml index 49c7fda99e..e72e319e4d 100644 --- a/packages/flow/openadapt_flow/app_profiles/openemr.yaml +++ b/packages/flow/openadapt_flow/app_profiles/openemr.yaml @@ -11,13 +11,15 @@ # excused. schema_version: openadapt.app-profile/v2 app: openemr -version: openemr-frame-profile/2026-10-29.5 +version: openemr-frame-profile/2026-11-03.1 bookkeeping_tables: [log, log_comment_encrypt, api_log, extended_log, audit_master, audit_details, clinical_rules_log, uuid_registry, uuid_mapping, contact, session_tracker, ip_tracking, background_services, sequences, users_secure, user_settings, recent_patients, users] system_columns: [uuid] users: {table: users, id_column: id, name_column: username} +# OpenEMR files an entry that names no chart under patient_id 0. +no_record_values: ["0"] rules: # Logs and id maps: rows are added, never changed or removed. On the four # marked insert_entry_names_no_record, OpenEMR inserts a row with a save or diff --git a/packages/flow/openadapt_flow/compiler/binding_admission.py b/packages/flow/openadapt_flow/compiler/binding_admission.py index 416eafd6c9..6c2f092903 100644 --- a/packages/flow/openadapt_flow/compiler/binding_admission.py +++ b/packages/flow/openadapt_flow/compiler/binding_admission.py @@ -16,8 +16,8 @@ is REST only. A placeholder effect is not a proposal: there is no real endpoint in the recording, and we will not invent one (:mod:`compiler.effect_mining`). -2. **Admit.** Execute the proposal against a *held-out* fixture (MockMed's - in-process REST store is the built-in one) with the same +2. **Admit.** Execute the proposal against a *held-out* fixture (a REST + store whose read and write paths the caller names) with the same :class:`~openadapt_flow.runtime.effects.EffectVerifier` that gates replay. The binding is copied onto the step only when every proposed :class:`~openadapt_flow.runtime.effects.Effect` CONFIRMs. Anything else @@ -68,9 +68,9 @@ #: Event key for an observed REST write (method / url_template / body_template). API_WRITE_KEY = "api_write" -#: Record fields assigned by MockMed (and typical REST stores) at write time, -#: never sent in the POST body and never used as a proposal identity. -_BODY_SKIP_FIELDS = SURROGATE_ID_FIELDS | frozenset({"source"}) +#: Record fields every REST store assigns at write time: never sent in the +#: POST body. An app's own server-assigned fields come from the fixture. +_BODY_SKIP_FIELDS = SURROGATE_ID_FIELDS #: The only ``ApiBinding.kind`` this module will propose or admit. _REST_KIND: Literal["rest"] = "rest" @@ -111,26 +111,38 @@ class AdmissionDecision: class AdmissionFixture: """Held-out REST system of record used to certify a proposed binding. - MockMed (``openadapt_flow.mockmed.fault_server``) is the built-in fixture: - ``POST /api/encounter`` writes, ``GET /api/db`` is the verifier read. + Every app keeps its records at its own paths, so the caller names them: + ``records_path`` / ``records_key`` are the verifier read, ``write_path`` + the write a proposal makes when the recording captured no ``api_write`` + (unset: such a step gets no proposal). ``server_fields`` are record fields + the app assigns itself, never sent in a proposed body; + ``idempotency_key_field`` is the field carrying its at-most-once key. """ base_url: str - write_path: str = "/api/encounter" + records_path: str + records_key: Optional[str] + write_path: Optional[str] = None write_method: str = "POST" + server_fields: frozenset[str] = frozenset() + idempotency_key_field: Optional[str] = None params: dict[str, str] = field(default_factory=dict) timeout_s: float = 2.0 session: Any = None def _param_placeholders( - record: dict[str, Any], params: dict[str, str] + record: dict[str, Any], + params: dict[str, str], + skip: frozenset[str] = frozenset(), ) -> dict[str, str]: """Map observed record fields to ``{param}`` leaves or literal strings.""" inverse = {str(value): name for name, value in params.items() if value} body: dict[str, str] = {} for key, value in record.items(): - if key in _BODY_SKIP_FIELDS or value is None or str(value) == "": + if key in _BODY_SKIP_FIELDS or key in skip: + continue + if value is None or str(value) == "": continue text = str(value) if text in inverse: @@ -180,7 +192,12 @@ def propose_rest_binding( effects = list(step.effects) source = "step.effects" if not effects or any(eff.needs_operator_confirmation for eff in effects): - mined = mine_step_effects(event, step, exclude_texts=exclude_texts) + mined = mine_step_effects( + event, + step, + exclude_texts=exclude_texts, + idempotency_key_field=fixture.idempotency_key_field, + ) if not mined.derived: logger.debug( "binding-propose %s: no derived SoR effect (%s)", @@ -199,11 +216,12 @@ def propose_rest_binding( return None method = fixture.write_method - url_template = fixture.write_path + url_template: Optional[str] = fixture.write_path body: dict[str, Any] = {} if captured is not None: method = str(captured.get("method") or method).upper() - url_template = str(captured.get("url_template") or url_template) + captured_url = captured.get("url_template") + url_template = str(captured_url) if captured_url else url_template raw_body = captured.get("body_template", captured.get("body")) if isinstance(raw_body, dict): body = dict(raw_body) @@ -216,7 +234,7 @@ def propose_rest_binding( new_records = _new_records(before, after) if len(new_records) != 1: return None - body = _param_placeholders(new_records[0], params) + body = _param_placeholders(new_records[0], params, fixture.server_fields) source = "sor_delta" if not url_template or not body: @@ -269,7 +287,12 @@ def admit_rest_binding( proposal=proposal, ) - verifier = RestRecordVerifier(fixture.base_url, timeout_s=fixture.timeout_s) + verifier = RestRecordVerifier( + fixture.base_url, + records_path=fixture.records_path, + records_key=fixture.records_key, + timeout_s=fixture.timeout_s, + ) actuator = ApiActuator( fixture.base_url, session=fixture.session, diff --git a/packages/flow/openadapt_flow/compiler/compile.py b/packages/flow/openadapt_flow/compiler/compile.py index d4f377a5bd..5a423898ca 100644 --- a/packages/flow/openadapt_flow/compiler/compile.py +++ b/packages/flow/openadapt_flow/compiler/compile.py @@ -8,6 +8,7 @@ from __future__ import annotations +import dataclasses import difflib import hashlib import json @@ -30,6 +31,7 @@ if TYPE_CHECKING: from openadapt_flow.compiler.annotate import StepAnnotator + from openadapt_flow.compiler.binding_admission import AdmissionFixture import numpy as np @@ -74,6 +76,7 @@ opener_explanation, presumed_commit_step_ids, ) +from openadapt_flow.runtime.effects.effect import DEFAULT_IDEMPOTENCY_KEY_FIELD from openadapt_flow.runtime.identity import ( band_region, context_from_lines, @@ -2117,7 +2120,8 @@ def _compile_recording( secret_param_steps: Optional[Iterable[str]] = None, mine_effects: bool = False, admit_api_bindings: bool = False, - admission_fixture_url: Optional[str] = None, + admission_fixture: Optional["AdmissionFixture"] = None, + idempotency_key_field: Optional[str] = DEFAULT_IDEMPOTENCY_KEY_FIELD, skill_library_root: Optional[Path | str] = None, annotate: bool = False, annotator: Optional["StepAnnotator"] = None, @@ -2216,13 +2220,20 @@ def _compile_recording( consequential write whose Effect was mined from an observed system-of-record delta, propose a REST binding and copy it onto the step only if the same Effect CONFIRMs on the held-out - fixture at ``admission_fixture_url``. A refused proposal leaves + ``admission_fixture``. A refused proposal leaves the step on the GUI ladder. FHIR / MCP / tool bindings are not synthesized. Default False keeps the bundle byte-identical. - admission_fixture_url: Base URL of the held-out REST fixture - (MockMed ``fault_server`` in tests). Required when - ``admit_api_bindings`` is True; without it the compiler logs - and leaves every step on the GUI ladder. + idempotency_key_field: With ``mine_effects``, the record field that + carries this app's at-most-once key (from its contract or + deployment config). Defaults to the ``key`` convention + (:data:`~openadapt_flow.compiler.effect_mining.DEFAULT_IDEMPOTENCY_KEY_FIELD`); + ``None``: no key is mined. + admission_fixture: The held-out REST fixture + (:class:`~openadapt_flow.compiler.binding_admission.AdmissionFixture`: + base URL, and the app's own read and write paths). Required when + ``admit_api_bindings`` is True; without it the compiler logs and + leaves every step on the GUI ladder. Its ``params`` default to the + recording's. skill_library_root: Optional :class:`~openadapt_flow.learning.library.SkillLibrary` root. When a binding is admitted, the lifted program is stored with @@ -3419,6 +3430,7 @@ def cached_lines(i: int, suffix: str, png: bytes) -> list[OcrLine]: param_bindings=bindings or None, store_unchanged=unchanged, write_requests=write_requests, + idempotency_key_field=idempotency_key_field, ) if mined.effects: step.effects = mined.effects @@ -3472,15 +3484,14 @@ def cached_lines(i: int, suffix: str, png: bytes) -> list[OcrLine]: if admit_api_bindings: from openadapt_flow.compiler.binding_admission import ( - AdmissionFixture, certify_steps_on_fixture, ) from openadapt_flow.learning.library import SkillLibrary - if not admission_fixture_url: + if admission_fixture is None: logger.info( - "admit_api_bindings requested but admission_fixture_url is " - "empty -- leaving every step on the GUI ladder" + "admit_api_bindings requested but admission_fixture is " + "unset -- leaving every step on the GUI ladder" ) else: library = ( @@ -3491,9 +3502,10 @@ def cached_lines(i: int, suffix: str, png: bytes) -> list[OcrLine]: decisions = certify_steps_on_fixture( steps, [event for _step, _sb, _sa, event in pending], - fixture=AdmissionFixture( - base_url=admission_fixture_url, - params=dict(params), + fixture=( + admission_fixture + if admission_fixture.params + else dataclasses.replace(admission_fixture, params=dict(params)) ), exclude_texts=exclude_texts, params=dict(params), @@ -3586,7 +3598,9 @@ def cached_lines(i: int, suffix: str, png: bytes) -> list[OcrLine]: ), "mine_effects": mine_effects, "admit_api_bindings": admit_api_bindings, - "admission_fixture_url": admission_fixture_url, + "admission_fixture_url": ( + admission_fixture.base_url if admission_fixture is not None else None + ), "name": name, "param_overrides": dict(sorted((param_overrides or {}).items())), "risk_overrides": dict(sorted((risk_overrides or {}).items())), @@ -3596,6 +3610,21 @@ def cached_lines(i: int, suffix: str, png: bytes) -> list[OcrLine]: if bind_effect_params: # Recorded only when set, so every existing compile keeps its digest. compiler_config["bind_effect_params"] = True + if idempotency_key_field != DEFAULT_IDEMPOTENCY_KEY_FIELD: + # Recorded only when not the default, so every existing compile keeps + # its digest. + compiler_config["idempotency_key_field"] = idempotency_key_field + if admission_fixture is not None: + # Added only when set, so a compile without one hashes as before. + admission: dict[str, Any] = { + "records_path": admission_fixture.records_path, + "records_key": admission_fixture.records_key, + "write_path": admission_fixture.write_path, + "write_method": admission_fixture.write_method, + "server_fields": sorted(admission_fixture.server_fields), + "idempotency_key_field": admission_fixture.idempotency_key_field, + } + compiler_config["admission_fixture"] = admission compiler_config_sha256 = hashlib.sha256( json.dumps( compiler_config, diff --git a/packages/flow/openadapt_flow/compiler/effect_mining.py b/packages/flow/openadapt_flow/compiler/effect_mining.py index d16b9d26c8..8bf6117e0f 100644 --- a/packages/flow/openadapt_flow/compiler/effect_mining.py +++ b/packages/flow/openadapt_flow/compiler/effect_mining.py @@ -55,6 +55,7 @@ from openadapt_flow.ir import ActionKind, PostconditionKind, Region, Step from openadapt_flow.runtime.effects.effect import ( + DEFAULT_IDEMPOTENCY_KEY_FIELD, MIN_RENAV_ACTIONS, Effect, EffectKind, @@ -96,11 +97,6 @@ def _vemap(selector: dict[str, str]) -> dict[str, ValueExpr]: # prune, rather than guess which app fields are "incidental". SURROGATE_ID_FIELDS = frozenset({"id"}) -# Field that commonly carries an at-most-once / idempotency key in a JSON -# system of record (the MockMed fault server uses ``key``). Only mined when the -# observed record actually carries a non-null value there — never invented. -IDEMPOTENCY_KEY_FIELD = "key" - # Sentinel selector for a PLACEHOLDER effect: a value no real record can carry, # so even if a misconfigured run tried to verify it (it must not — the replayer # HALTs on needs_operator_confirmation first), it could never falsely CONFIRM. @@ -288,6 +284,7 @@ def _mine_from_sor_delta( after: list[dict], exclude_texts: tuple[str, ...], param_bindings: Optional[Mapping[str, str]] = None, + idempotency_key_field: Optional[str] = DEFAULT_IDEMPOTENCY_KEY_FIELD, ) -> Optional[StepEffectMining]: """Mine real effects from an OBSERVED before/after system-of-record delta. @@ -342,12 +339,12 @@ def _mine_from_sor_delta( "system-of-record delta)" ), ) - # Idempotency key: derivable ONLY when the observed record actually carries - # one. Never invented — §7 names the idempotency key as app-specific. - key_val = record.get(IDEMPOTENCY_KEY_FIELD) - if key_val is not None and str(key_val) != "": + # Idempotency key: only from the field the caller names for this app (§7: + # the key is app-specific), and only when the observed record carries it. + key_val = record.get(idempotency_key_field) if idempotency_key_field else None + if idempotency_key_field and key_val is not None and str(key_val) != "": written.idempotency_key = _ve(str(key_val)) - written.key_field = IDEMPOTENCY_KEY_FIELD + written.key_field = idempotency_key_field effects.append(written) # field_equals per payload field: the typed value actually persisted (the @@ -380,7 +377,7 @@ def _mine_from_sor_delta( f"derived {len(effects)} effect(s) from an observed system-of-record " f"delta (1 new record; selector {selector}" + ( - f"; idempotency key from field {IDEMPOTENCY_KEY_FIELD!r}" + f"; idempotency key from field {idempotency_key_field!r}" if written.idempotency_key is not None else "; no idempotency key observed" ) @@ -900,6 +897,7 @@ def mine_step_effects( param_bindings: Optional[Mapping[str, str]] = None, store_unchanged: Optional[bool] = None, write_requests: Optional[bool] = None, + idempotency_key_field: Optional[str] = DEFAULT_IDEMPOTENCY_KEY_FIELD, ) -> StepEffectMining: """Derive candidate system-of-record effects for one compiled step. @@ -944,6 +942,11 @@ def mine_step_effects( ``None`` when the recording had no network capture, ``False`` when the capture showed no write request. Only used to word the reason. + idempotency_key_field: The record field that carries this app's + at-most-once key, from the contract or deployment config. Set, + and present on the observed record, the mined ``record_written`` + counts records bearing it. Defaults to + :data:`DEFAULT_IDEMPOTENCY_KEY_FIELD`; ``None``: no key is mined. Returns: A :class:`StepEffectMining` describing the decision. ``effects`` is @@ -952,7 +955,14 @@ def mine_step_effects( before = _as_records(event.get(SOR_BEFORE_KEY)) after = _as_records(event.get(SOR_AFTER_KEY)) if before is not None and after is not None: - mined = _mine_from_sor_delta(step, before, after, exclude_texts, param_bindings) + mined = _mine_from_sor_delta( + step, + before, + after, + exclude_texts, + param_bindings, + idempotency_key_field=idempotency_key_field, + ) if mined is not None: return mined # A snapshot existed but showed no single-record write. When the diff --git a/packages/flow/openadapt_flow/deployment.py b/packages/flow/openadapt_flow/deployment.py index a5f3d698e3..3a256e3cb0 100644 --- a/packages/flow/openadapt_flow/deployment.py +++ b/packages/flow/openadapt_flow/deployment.py @@ -26,7 +26,7 @@ import os import re from pathlib import Path -from typing import Any, Callable, Literal, Mapping, Optional +from typing import Any, Callable, Final, Literal, Mapping, Optional from pydantic import ( BaseModel, @@ -384,9 +384,11 @@ class EffectsConfig(BaseModel): #: ``runtime/effects/fresh_session.py`` and ``docs/READBACK_LADDER.md``. fresh_session: Optional[FreshSessionSpec] = None - # -- rest (JSON REST system of record, e.g. MockMed /api/db) ------------- + # -- rest (JSON REST system of record) ----------------------------------- base_url: Optional[str] = None - records_path: str = "/api/db" + #: ``rest`` only, and required there: the path whose GET returns the + #: records. Every app keeps them somewhere else, so there is no default. + records_path: Optional[str] = None records_key: Optional[str] = "records" #: ``rest`` only: dotted path in the response body to the next page's URL #: (``links.next``). Set, the read follows every page on the same origin @@ -445,8 +447,10 @@ class EffectsConfig(BaseModel): #: Allowlist variant: when set, every evidence field NOT named is redacted. evidence_keep_fields: Optional[list[str]] = None - # -- fhir (FHIR R4 search, e.g. OpenEMR) --------------------------------- - resource_type: str = "Observation" + # -- fhir (FHIR R4 search) ----------------------------------------------- + #: ``fhir`` only, and required there with ``field_paths``: which resource + #: the app keeps the write in, and where each field lives in it. + resource_type: Optional[str] = None search_params: dict[str, str] = Field(default_factory=dict) field_paths: Optional[dict[str, str]] = None #: OAuth2 bearer token literal. DEPRECATED for committed configs — prefer @@ -627,6 +631,8 @@ def _serialize_compatible(self, handler: Any) -> dict[str, Any]: "deferred_report", "records_next_path", "records_total_path", + "records_path", + "resource_type", ): if data.get(added, "") is None: data.pop(added, None) @@ -2043,6 +2049,38 @@ def build_effect_verifier( _PLUGIN_ONLY_FIELDS = ("options", "option_params", "secret_env") +#: What these fields held when they still had defaults. Verifier identities +#: and check digests issued before then hashed these values, so for hashing +#: only, an unset field still reads as its old value. Never used to read a +#: store: a read without its own records_path or resource_type is refused. +LEGACY_HASHED_DEFAULTS: Final[dict[str, str]] = { + "records_path": "/api/db", + "resource_type": "Observation", +} + + +def legacy_hash_dump(dumped: dict[str, Any], *, defaults_excluded: bool) -> None: + """Make a config dump hash as it did when those fields had defaults. + + ``defaults_excluded``: the dump left defaults out (``exclude_defaults``), + so a field holding its old default is dropped; otherwise an unset field + takes its old default. + """ + + for name, legacy in LEGACY_HASHED_DEFAULTS.items(): + if defaults_excluded: + if dumped.get(name) == legacy: + dumped.pop(name) + elif dumped.get(name) is None: + dumped[name] = legacy + for item in dumped.get("candidates") or []: + if isinstance(item, dict): + legacy_hash_dump(item, defaults_excluded=defaults_excluded) + for item in (dumped.get("surfaces") or {}).values(): + if isinstance(item, dict): + legacy_hash_dump(item, defaults_excluded=defaults_excluded) + + def _identity_config_dump(cfg: EffectsConfig) -> dict[str, Any]: """Serialize ``cfg`` for the verifier identity. @@ -2051,6 +2089,7 @@ def _identity_config_dump(cfg: EffectsConfig) -> dict[str, Any]: """ def prune(dumped: dict[str, Any]) -> dict[str, Any]: + legacy_hash_dump(dumped, defaults_excluded=False) for name in _PLUGIN_ONLY_FIELDS: if not dumped.get(name): dumped.pop(name, None) @@ -2223,6 +2262,11 @@ def _build_effect_verifier_unredacted( if kind == "rest": if not cfg.base_url: raise ValueError("effects.kind 'rest' requires effects.base_url") + if not cfg.records_path: + raise ValueError( + "effects.kind 'rest' requires effects.records_path (the path " + "whose GET returns the app's records)" + ) from openadapt_flow.runtime.effects import RestRecordVerifier headers = cfg.auth.resolve_headers() if cfg.auth is not None else None @@ -2258,6 +2302,12 @@ def _build_effect_verifier_unredacted( if kind == "fhir": if not cfg.base_url: raise ValueError("effects.kind 'fhir' requires effects.base_url") + if not cfg.resource_type or not cfg.field_paths: + raise ValueError( + "effects.kind 'fhir' requires effects.resource_type and " + "effects.field_paths (which resource the app keeps the write " + "in, and where each field lives in it)" + ) from openadapt_flow.runtime.effects import FhirEffectVerifier access_token = cfg.access_token diff --git a/packages/flow/openadapt_flow/fault_gate.py b/packages/flow/openadapt_flow/fault_gate.py index c07e5e2332..47a8f51dab 100644 --- a/packages/flow/openadapt_flow/fault_gate.py +++ b/packages/flow/openadapt_flow/fault_gate.py @@ -374,7 +374,10 @@ def check_digest(cfg: Any) -> str: check as production. """ + from openadapt_flow.deployment import legacy_hash_dump + verifier = cfg.verifier.model_dump(mode="json", exclude_defaults=True) + legacy_hash_dump(verifier, defaults_excluded=True) frame = cfg.frame.model_dump(mode="json", by_alias=True) if cfg.frame else None payload = { "effects": [e.model_dump(mode="json") for e in cfg.effects], @@ -719,8 +722,8 @@ def __init__( seed: Sequence[Mapping[str, Any]], target: Mapping[str, Any], *, - records_path: str = "/api/db", - records_key: Optional[str] = "records", + records_path: str, + records_key: Optional[str], key_field: str = "id", ) -> None: self.key_field = key_field diff --git a/packages/flow/openadapt_flow/fleet/simulator.py b/packages/flow/openadapt_flow/fleet/simulator.py index 642544ea26..a59e096c8d 100644 --- a/packages/flow/openadapt_flow/fleet/simulator.py +++ b/packages/flow/openadapt_flow/fleet/simulator.py @@ -621,7 +621,11 @@ def replay( ) workflow = Workflow.load(bundle_dir) verifier = RestRecordVerifier( - "http://sor.sim", session=_RecordSession(sor), poll_interval_s=0.01 + "http://sor.sim", + records_path="/records", + records_key="records", + session=_RecordSession(sor), + poll_interval_s=0.01, ) gate = evaluate_run_gate( workflow, diff --git a/packages/flow/openadapt_flow/generate/ensure_contract.py b/packages/flow/openadapt_flow/generate/ensure_contract.py index a34a3747bb..4ecabfbbaf 100644 --- a/packages/flow/openadapt_flow/generate/ensure_contract.py +++ b/packages/flow/openadapt_flow/generate/ensure_contract.py @@ -1,7 +1,7 @@ """Generate an ``openadapt.ensure-record/v1`` contract for a FHIR write. A person used to write about half of a FHIR ensure-record contract by hand -(``benchmark/fhir_medplum/readpath/MINUTES.md``): the ``telecom[system=...]`` +(for example, for a FHIR server's patient write): the ``telecom[system=...]`` paths, the comparison kinds, the state map, the identity rules, the search parameter per identity value, the closed record and the change feed. This module derives each of them from what a teach session already has: @@ -119,32 +119,16 @@ def _load(path: Path) -> Any: raise GenerateError(f"cannot read {path}: {exc}") from exc -def facts_from_probe(probe: Mapping[str, Any]) -> dict[str, Any]: - """Server facts from ``benchmark/fhir_medplum/readpath/probe.py`` output. +def server_facts(doc: Mapping[str, Any]) -> dict[str, Any]: + """The generic server facts in a teach directory's ``facts.json``. - A probe that already reports the generic names passes through. + Only the names in :data:`FACTS` are read. A probe written for one server + translates its own output to these names, beside the probe in the + benchmark tree. A fact that is missing is not measured, and the contract + it would back is refused. """ - if all(k in probe for k in FACTS): - return {k: probe[k] for k in FACTS} - out: dict[str, Any] = {} - logged = probe.get("bundle_and_graphql_writes_logged") - if isinstance(logged, list): - out["audit_logs_writes"] = len(logged) > 0 - for name, key in ( - ("audit_names_actor", "audit_names_the_user_profile"), - ("agent_cannot_write_audit", "restricted_bot_audit_read_only"), - ("history_lists_versions", "delete_in_history"), - ("meta_author_returned", "meta_author_returned"), - ): - if key in probe: - out[name] = probe[key] - lags = probe.get("audit_lag_s") - if isinstance(lags, Mapping) and lags: - out["audit_lag_max_s"] = max( - float(v.get("max", 0)) for v in lags.values() if isinstance(v, Mapping) - ) - return out + return {k: doc[k] for k in FACTS if k in doc} def load_teach(root: str | Path, confirmed: Sequence[str] = ()) -> Teach: @@ -152,7 +136,7 @@ def load_teach(root: str | Path, confirmed: Sequence[str] = ()) -> Teach: ``teach.json`` (resource type and deployment settings), ``capability.json``, ``search_parameters.json`` (Bundle), ``facts.json`` - (generic facts or a probe's output) and ``demos//{params,before, + (the generic facts in :data:`FACTS`) and ``demos//{params,before, after}.json``. """ @@ -174,7 +158,7 @@ def load_teach(root: str | Path, confirmed: Sequence[str] = ()) -> Teach: demos=demos, capability=_load(base / "capability.json"), search_parameters=_resources(_load(sp_path)) if sp_path.exists() else [], - facts=facts_from_probe(_load(base / "facts.json")), + facts=server_facts(_load(base / "facts.json")), deployment=dict(meta.get("deployment") or {}), # A confirmation is a person's, given to the command, never read # from the teach data. diff --git a/packages/flow/openadapt_flow/qualify/session.py b/packages/flow/openadapt_flow/qualify/session.py index a86e7dce8a..849ee5c562 100644 --- a/packages/flow/openadapt_flow/qualify/session.py +++ b/packages/flow/openadapt_flow/qualify/session.py @@ -159,11 +159,14 @@ def verifier_factory(config: VerifyConfig) -> Callable[[Mapping[str, object]], A if not cfg.base_url: raise QualifyConfigError("verifier.base_url is required for kind: rest") + if not cfg.records_path: + raise QualifyConfigError("verifier.records_path is required for kind: rest") + records_path = cfg.records_path def build_rest(params: Mapping[str, object]) -> Any: return RestRecordVerifier( cfg.base_url.rstrip("/"), # type: ignore[union-attr] - records_path=cfg.records_path, + records_path=records_path, records_key=cfg.records_key, session=hook(config), timeout_s=10.0, diff --git a/packages/flow/openadapt_flow/reward/models.py b/packages/flow/openadapt_flow/reward/models.py index 6223f843f0..4ebaa3ce2d 100644 --- a/packages/flow/openadapt_flow/reward/models.py +++ b/packages/flow/openadapt_flow/reward/models.py @@ -222,15 +222,15 @@ class OracleRecipeV1(_Strict): records_key: Optional[StrictStr] = "records" #: ``rest`` / ``fhir``: base URL of the read endpoint. base_url: Optional[StrictStr] = None - #: ``rest``: path of the records document. - records_path: StrictStr = "/api/db" + #: ``rest``: path of the records document. Required there. + records_path: Optional[StrictStr] = None #: ``rest``: environment variable holding a JSON object of headers. headers_env: Optional[StrictStr] = None #: ``sqlite``: database file. ``query``: one read-only SELECT. query: Optional[StrictStr] = None #: ``fhir``: resource type and search parameters; ``token_env`` names the #: bearer token variable. - resource_type: StrictStr = "Observation" + resource_type: Optional[StrictStr] = None search_params: dict[StrictStr, StrictStr] = Field(default_factory=dict) field_paths: Optional[dict[StrictStr, StrictStr]] = None token_env: Optional[StrictStr] = None @@ -245,6 +245,12 @@ def _kind_fields(self) -> "OracleRecipeV1": raise ValueError(f"oracle recipe {self.kind} requires path") if self.kind in {"rest", "fhir"} and not self.base_url: raise ValueError(f"oracle recipe {self.kind} requires base_url") + if self.kind == "rest" and not self.records_path: + raise ValueError("oracle recipe rest requires records_path") + if self.kind == "fhir" and not (self.resource_type and self.field_paths): + raise ValueError( + "oracle recipe fhir requires resource_type and field_paths" + ) if self.kind == "sqlite" and not self.query: raise ValueError("oracle recipe sqlite requires query") return self diff --git a/packages/flow/openadapt_flow/reward/oracles.py b/packages/flow/openadapt_flow/reward/oracles.py index 20cdb85221..90bdc31775 100644 --- a/packages/flow/openadapt_flow/reward/oracles.py +++ b/packages/flow/openadapt_flow/reward/oracles.py @@ -218,7 +218,7 @@ def _adapter_for(recipe: OracleRecipeV1, base_dir: Path) -> OracleAdapter: return VerifierOracle( RestRecordVerifier( str(recipe.base_url), - records_path=recipe.records_path, + records_path=str(recipe.records_path), records_key=recipe.records_key, headers=recipe.headers(), timeout_s=recipe.timeout_s, @@ -246,9 +246,9 @@ def connect() -> sqlite3.Connection: return VerifierOracle( FhirEffectVerifier( str(recipe.base_url), - resource_type=recipe.resource_type, + resource_type=str(recipe.resource_type), search_params=dict(recipe.search_params), - field_paths=recipe.field_paths, + field_paths=dict(recipe.field_paths or {}), access_token=recipe.token(), timeout_s=recipe.timeout_s, ), diff --git a/packages/flow/openadapt_flow/rollback.py b/packages/flow/openadapt_flow/rollback.py index 750e01f195..bc792ed4b4 100644 --- a/packages/flow/openadapt_flow/rollback.py +++ b/packages/flow/openadapt_flow/rollback.py @@ -345,6 +345,12 @@ def _bookkeeping_inserts(reader: Any) -> set[str]: return set(profile.no_record_insert_tables()) if profile is not None else set() +def _no_record_values(reader: Any) -> set[str]: + """Record-field values that name no record, from the app profile.""" + profile = _profile_of(reader) + return set(profile.no_record_values) if profile is not None else set() + + def _own_row_tables(reader: Any) -> set[str]: """Tables of the acting user's own rows the reader's app profile excuses.""" from openadapt_flow.app_profiles import table_name @@ -584,7 +590,11 @@ def hidden_writes( if attributor is None: return None return attributor.unaccounted_writes( - rows, window, _bookkeeping_inserts(reader), _own_row_tables(reader) + rows, + window, + _bookkeeping_inserts(reader), + _own_row_tables(reader), + _no_record_values(reader), ) record.after_stop_diff = landed.summary() diff --git a/packages/flow/openadapt_flow/rollback_state.py b/packages/flow/openadapt_flow/rollback_state.py index 84c58444d1..4f00e6fce8 100644 --- a/packages/flow/openadapt_flow/rollback_state.py +++ b/packages/flow/openadapt_flow/rollback_state.py @@ -759,11 +759,13 @@ def unaccounted_writes( window: Optional[list[dict[str, Any]]], bookkeeping_inserts: set[str] = frozenset(), # type: ignore[assignment] own_tables: set[str] = frozenset(), # type: ignore[assignment] + no_record_values: set[str] = frozenset(), # type: ignore[assignment] ) -> Optional[list[dict[str, Any]]]: """The run's own logged writes that no changed row accounts for (a write to a table the read keeps append-only, say), or None when there is no audit window to tell. As the frame does: an insert entry - that names no record (none, or patient 0) in a table the app profile + that names no record (none, or a profile ``no_record_values`` value) + in a table the app profile calls bookkeeping (``no_record_insert_tables``) is the app's own, and so is an entry for a table of the acting user's own rows the profile excuses (``own_rows``: UI settings, recent charts).""" @@ -783,7 +785,7 @@ def unaccounted_writes( ): continue if ( - record in (None, "", "0", 0) + (record in (None, "") or str(record) in no_record_values) and self._event(e) in set(self.spec.insert_events) and table not in (None, "") and any(_same_table(table, t) for t in bookkeeping_inserts) diff --git a/packages/flow/openadapt_flow/runtime/effects/compensation.py b/packages/flow/openadapt_flow/runtime/effects/compensation.py index 1572ff6184..9f1dcbf757 100644 --- a/packages/flow/openadapt_flow/runtime/effects/compensation.py +++ b/packages/flow/openadapt_flow/runtime/effects/compensation.py @@ -352,7 +352,7 @@ class RestCompensator: Args: base_url: Base URL of the system of record. delete_path_template: Path template with ``{id}`` for the per-record - DELETE (e.g. ``/api/encounter/{id}``). + DELETE (e.g. ``/records/{id}``). Required. session: Optional ``requests``-style session. timeout_s: Per-request timeout. """ @@ -361,7 +361,7 @@ def __init__( self, base_url: str, *, - delete_path_template: str = "/api/encounter/{id}", + delete_path_template: str, session: Any = None, timeout_s: float = 5.0, ) -> None: diff --git a/packages/flow/openadapt_flow/runtime/effects/effect.py b/packages/flow/openadapt_flow/runtime/effects/effect.py index 7ad9118006..6774989a7d 100644 --- a/packages/flow/openadapt_flow/runtime/effects/effect.py +++ b/packages/flow/openadapt_flow/runtime/effects/effect.py @@ -826,6 +826,12 @@ def _resolve_readback( # contract cannot claim stronger evidence than the compiler would emit. MIN_RENAV_ACTIONS = 2 +#: The record field read as an at-most-once key when nothing names another: +#: :attr:`Effect.key_field`'s default, and the field the effect miner reads +#: unless its caller names another (or ``None`` for no key). The miner sets a +#: key only when the observed record carries a value there. +DEFAULT_IDEMPOTENCY_KEY_FIELD = "key" + class Effect(BaseModel): """A typed system-of-record effect (RFC ``Effect``, concrete runtime form). @@ -897,9 +903,10 @@ class Effect(BaseModel): #: identity) so the key is PER-RUN, not the frozen demonstration literal #: that would collide across unrelated runs (P0-3). idempotency_key: Optional[ValueExpr] = None - #: Which record field carries the idempotency key (substrate-specific; - #: the MockMed system-of-record uses ``"key"``). - key_field: str = "key" + #: Which record field carries the idempotency key. The contract names it + #: beside ``idempotency_key``; the effect miner sets it from the field its + #: caller names (:data:`DEFAULT_IDEMPOTENCY_KEY_FIELD` unless told). + key_field: str = DEFAULT_IDEMPOTENCY_KEY_FIELD #: ``record_written`` only: a duplicate (idempotency) key made of several #: fields, such as OpenEMR's ``{lname, fname, DOB}``. Field -> value, each #: a :class:`ValueExpr` like :attr:`match`. The count reads only records diff --git a/packages/flow/openadapt_flow/runtime/effects/fhir.py b/packages/flow/openadapt_flow/runtime/effects/fhir.py index 2384806e9e..45b4a93e32 100644 --- a/packages/flow/openadapt_flow/runtime/effects/fhir.py +++ b/packages/flow/openadapt_flow/runtime/effects/fhir.py @@ -56,8 +56,9 @@ ) from openadapt_flow.verification import VerificationTier -#: OpenEMR's FHIR field paths for the canonical patient-note-as-Observation -#: write. Callers override for DocumentReference / Encounter / etc. +#: A field map for a note kept as a FHIR R4 Observation (the note in +#: ``valueString``). Not a default: a caller whose app stores its writes this +#: way passes it as ``field_paths``; any other resource or app names its own. DEFAULT_OBSERVATION_PATHS: dict[str, str] = { "id": "id", "patient": "subject.reference", @@ -188,18 +189,18 @@ def extract_path(resource: dict[str, Any], dotted: str) -> Any: class FhirEffectVerifier(VerifierAdapterBase): - """Verify effects against an OpenEMR FHIR R4 system of record. + """Verify effects against a FHIR R4 system of record. Args: - base_url: FHIR base URL (e.g. - ``https://demo.openemr.io/apis/default/fhir/R4``). + base_url: FHIR base URL (e.g. ``https://fhir.example.org/R4``). resource_type: FHIR resource type to search (``Observation``, - ``DocumentReference``, ``Encounter``, ...). + ``DocumentReference``, ``Encounter``, ...). Required. search_params: Query params identifying the candidate resource set (e.g. ``{"patient": "9", "category": "social-history"}``); used for both the pre-state snapshot and the post-action read. field_paths: Flat Effect key -> dotted FHIRPath-lite for flattening a - resource into the dict :func:`judge_records` matches on. + resource into the dict :func:`judge_records` matches on. Required: + where a field lives depends on the app and the resource. access_token: OAuth2 bearer token (``Authorization: Bearer ...``). token_source: Gets a token per read instead of ``access_token`` (mutually exclusive with it). @@ -218,9 +219,9 @@ def __init__( self, base_url: str, *, - resource_type: str = "Observation", + resource_type: str, + field_paths: dict[str, str], search_params: Optional[dict[str, str]] = None, - field_paths: Optional[dict[str, str]] = None, access_token: Optional[str] = None, token_source: Optional[AccessTokenSource] = None, session: Any = None, @@ -233,7 +234,9 @@ def __init__( self.drop_fields = tuple(drop_fields) self.resource_type = resource_type self.search_params = dict(search_params or {}) - self.field_paths = dict(field_paths or DEFAULT_OBSERVATION_PATHS) + if not resource_type or not field_paths: + raise ValueError("a FHIR read needs its resource_type and field_paths") + self.field_paths = dict(field_paths) if access_token and token_source is not None: raise ValueError("give access_token or token_source, not both") self.access_token = access_token diff --git a/packages/flow/openadapt_flow/runtime/effects/frame.py b/packages/flow/openadapt_flow/runtime/effects/frame.py index 35129df17f..d8f2cb6c3c 100644 --- a/packages/flow/openadapt_flow/runtime/effects/frame.py +++ b/packages/flow/openadapt_flow/runtime/effects/frame.py @@ -837,6 +837,7 @@ def judge_frame( new_row_defaults: Optional[Mapping[str, Mapping[str, Optional[str]]]] = None, unread_changes: Sequence[tuple[str, str]] = (), no_record_inserts: Collection[str] = (), + no_record_values: Collection[str] = (), hashers: Optional[Mapping[str, Callable[[str, Any], str]]] = None, clock: tuple[str, str] = ("", ""), database_evidence: Optional[dict[str, Any]] = None, @@ -875,6 +876,9 @@ def judge_frame( no_record_inserts: Tables both database reads read as append-only whose profile rule marks the acting user's insert entry naming no record as the app's bookkeeping (an id map, a link row). + no_record_values: Record-field values that name no record in such an + entry, besides an empty field (the app profile's + ``no_record_values``). hashers: Slice -> (column, value) -> the keyed MAC its baseline records hold instead of values (database tables). A post-save record is plain; it is hashed to compare with the baseline. @@ -1945,14 +1949,15 @@ def names_moved(entry_ref: Any) -> bool: else "" ) if ( - ref in (None, "", "0", 0) + (ref in (None, "") or str(ref) in no_record_values) and attribution is not None and event in attribution.insert_events and entry_table not in (None, "") and any(same_table(entry_table, t) for t in no_record_inserts) ): # The bot's insert into an id map or link row a save - # makes (OpenEMR's uuid_registry), filed under no chart: + # makes (an id map such as a uuid registry), filed under + # no record: # both reads read the table, which allows the row and # judges every existing row. An update or delete, an # entry naming a record, or any other table still counts. diff --git a/packages/flow/openadapt_flow/runtime/effects/rest.py b/packages/flow/openadapt_flow/runtime/effects/rest.py index 390d7ce6dc..4a02941934 100644 --- a/packages/flow/openadapt_flow/runtime/effects/rest.py +++ b/packages/flow/openadapt_flow/runtime/effects/rest.py @@ -74,9 +74,10 @@ class RestRecordVerifier(VerifierAdapterBase): Args: base_url: Base URL of the system of record (trailing slash optional). - records_path: Path whose GET returns the records document. + records_path: Path whose GET returns the records document. Required: + every app keeps its records somewhere else. records_key: Key in the JSON body holding the records list; when the - body is itself a JSON list, pass ``None``. + body is itself a JSON list, pass ``None``. Required. session: Optional ``requests``-style session (injectable for tests / auth headers); a module-level default is used when omitted. headers: Optional headers sent with every read (the kit's @@ -111,8 +112,8 @@ def __init__( self, base_url: str, *, - records_path: str = "/api/db", - records_key: Optional[str] = "records", + records_path: str, + records_key: Optional[str], session: Any = None, headers: Optional[dict[str, str]] = None, timeout_s: float = 5.0, diff --git a/packages/flow/openadapt_flow/scaffold_verifier.py b/packages/flow/openadapt_flow/scaffold_verifier.py index b52e5c8306..b917c9be8d 100644 --- a/packages/flow/openadapt_flow/scaffold_verifier.py +++ b/packages/flow/openadapt_flow/scaffold_verifier.py @@ -45,6 +45,7 @@ from openadapt_flow import artifacts as _artifacts from openadapt_flow import sealed_set as _sealed +from openadapt_flow.runtime.effects.effect import DEFAULT_IDEMPOTENCY_KEY_FIELD BUNDLE_MARKERS = ("workflow.json", "workflow.json.enc") RECORDING_MARKER = "events.jsonl" @@ -162,6 +163,7 @@ def _candidate_from_record( record: dict[str, Any], *, demonstrated_values: tuple[str, ...], + idempotency_key_field: Optional[str] = DEFAULT_IDEMPOTENCY_KEY_FIELD, ) -> WriteCandidate: """Split one observed new record into identity vs payload fields. @@ -170,7 +172,6 @@ def _candidate_from_record( exclusion, same selector rules, same demonstrated-value payload split). """ from openadapt_flow.compiler.effect_mining import ( - IDEMPOTENCY_KEY_FIELD, _match_selector, _param_value_fields, ) @@ -178,13 +179,14 @@ def _candidate_from_record( payload = _param_value_fields(record, demonstrated_values) selector = _match_selector(record, set(payload)) idempotency = None - raw_key = record.get(IDEMPOTENCY_KEY_FIELD) - if raw_key not in (None, ""): - idempotency = (IDEMPOTENCY_KEY_FIELD, str(raw_key)) + # The app's at-most-once key field, when the caller names one. + raw_key = record.get(idempotency_key_field) if idempotency_key_field else None + if idempotency_key_field and raw_key not in (None, ""): + idempotency = (idempotency_key_field, str(raw_key)) # The at-most-once key is surfaced separately (it must be bound to a # per-run param, never the frozen demo literal), so it never doubles # as an identity selector. - selector.pop(IDEMPOTENCY_KEY_FIELD, None) + selector.pop(idempotency_key_field, None) return WriteCandidate( step_id=step_id, action=action, @@ -196,7 +198,11 @@ def _candidate_from_record( ) -def candidates_from_recording(recording_dir: Path) -> list[WriteCandidate]: +def candidates_from_recording( + recording_dir: Path, + *, + idempotency_key_field: Optional[str] = DEFAULT_IDEMPOTENCY_KEY_FIELD, +) -> list[WriteCandidate]: """Write-shaped candidates straight from ``events.jsonl`` evidence.""" import json @@ -245,6 +251,7 @@ def candidates_from_recording(recording_dir: Path) -> list[WriteCandidate]: label, new_records[0], demonstrated_values=demonstrated_values, + idempotency_key_field=idempotency_key_field, ) ) continue diff --git a/packages/flow/openadapt_flow/verifier.py b/packages/flow/openadapt_flow/verifier.py index 6861fa994b..51e4672c3b 100644 --- a/packages/flow/openadapt_flow/verifier.py +++ b/packages/flow/openadapt_flow/verifier.py @@ -291,9 +291,11 @@ def from_dict(cls, data: Mapping[str, Any], *, name: str = "") -> "Contract": def digest(self) -> str: """SHA-256 of the contract as parsed: part of a write's identity.""" + config = self.config.model_dump(mode="json") + config["verifier"] = self._verifier_dump() body = json.dumps( { - "config": self.config.model_dump(mode="json"), + "config": config, "writes": list(self.writes), }, sort_keys=True, @@ -311,12 +313,20 @@ def store(self) -> str: return _digest( { "environment": self.config.environment_id, - "verifier": _strip( - self.config.verifier.model_dump(mode="json"), drop_secrets=True - ), + "verifier": _strip(self._verifier_dump(), drop_secrets=True), } ) + def _verifier_dump(self) -> dict[str, Any]: + """The verifier as hashed: fields that lost their defaults still hash + as their old values, so contracts written before keep their digests.""" + + from openadapt_flow.deployment import legacy_hash_dump + + dumped = self.config.verifier.model_dump(mode="json") + legacy_hash_dump(dumped, defaults_excluded=False) + return dumped + @property def shape(self) -> str: """SHA-256 of the effects, the verifier kind, the ``environment_id`` diff --git a/packages/flow/openadapt_flow/verify_run.py b/packages/flow/openadapt_flow/verify_run.py index 5952a8b1a4..61de488522 100644 --- a/packages/flow/openadapt_flow/verify_run.py +++ b/packages/flow/openadapt_flow/verify_run.py @@ -788,6 +788,8 @@ class _DatabaseFrame: #: Append-only tables in both reads whose profile rule excuses the acting #: user's insert entries that name no record. no_record_inserts: list[str] = field(default_factory=list) + #: The profile's record-field values that name no record. + no_record_values: list[str] = field(default_factory=list) #: The settled read after the save, and when it was taken: what a later #: recheck compares with (``recheck_after_s``). post: Any = None @@ -939,6 +941,7 @@ def row_key(table: str, record: Mapping[str, Any]) -> Optional[str]: profile = database.loaded_profile() no_record = profile.no_record_insert_tables() if profile is not None else set() + out.no_record_values = list(profile.no_record_values) if profile is not None else [] for name in sorted(set(old.tables) | set(new.tables)): a, b = old.tables.get(name), new.tables.get(name) try: @@ -1298,6 +1301,7 @@ def excuse(name: str, before: Any, after: Any, fields: list[str]) -> Any: new_row_defaults=None if db is None else db.defaults, unread_changes=() if db is None else db.unread, no_record_inserts=() if db is None else db.no_record_inserts, + no_record_values=() if db is None else db.no_record_values, hashers=None if db is None else db.hashers, clock=("", "") if db is None else db.clock, database_evidence=None if db is None else db.evidence, diff --git a/packages/flow/scripts/generality_baseline.json b/packages/flow/scripts/generality_baseline.json index 3fac4b495a..50ebd1a16c 100644 --- a/packages/flow/scripts/generality_baseline.json +++ b/packages/flow/scripts/generality_baseline.json @@ -31,18 +31,12 @@ "openemr": 1, "token_main": 1 }, - "compiler/binding_admission.py": { - "mockmed": 3, - "mockmed-records": 1, - "mockmed-write": 2 - }, "compiler/compile.py": { - "mockmed": 3, + "mockmed": 2, "mockmed-records": 1, "openemr": 1 }, "compiler/effect_mining.py": { - "mockmed": 1, "mockmed-records": 1 }, "compiler/fixup_passes.py": { @@ -55,9 +49,9 @@ "mockloan": 2 }, "deployment.py": { - "mockmed": 2, - "mockmed-records": 2, - "openemr": 2, + "mockmed": 1, + "mockmed-records": 1, + "openemr": 1, "textedit": 1 }, "eligibility/__init__.py": { @@ -91,15 +85,11 @@ "mockmed": 1 }, "fault_gate.py": { - "mockmed-records": 1, "openemr": 1 }, "generate/demo_analysis.py": { "redmine": 1 }, - "generate/ensure_contract.py": { - "medplum": 2 - }, "generate/verify_contract.py": { "dolibarr": 1, "espocrm": 2, @@ -190,9 +180,6 @@ "reward/bundle.py": { "mockmed": 3 }, - "reward/models.py": { - "mockmed-records": 1 - }, "reward/seed.py": { "mockmed": 34 }, @@ -218,36 +205,32 @@ "runtime/effects/auth.py": { "frappe": 1 }, - "runtime/effects/compensation.py": { - "mockmed-write": 2 - }, "runtime/effects/db_snapshot.py": { "dolibarr": 1, "dolibarr-table": 1 }, "runtime/effects/effect.py": { - "mockmed": 2, + "mockmed": 1, "openemr": 5, "redmine": 1 }, "runtime/effects/fhir.py": { "epic": 1, "mockmed": 2, - "openemr": 8, - "openemr-fhir-path": 2 + "openemr": 5, + "openemr-fhir-path": 1 }, "runtime/effects/frame.py": { - "openemr": 7, + "openemr": 6, "patient_data": 1, - "pnotes": 1, - "uuid_registry": 1 + "pnotes": 1 }, "runtime/effects/fresh_session.py": { "mockmed": 1 }, "runtime/effects/rest.py": { "mockmed": 1, - "mockmed-records": 2, + "mockmed-records": 1, "openemr": 1 }, "runtime/effects/sql.py": { diff --git a/packages/flow/tests/test_after_stop_mockmed.py b/packages/flow/tests/test_after_stop_mockmed.py index 248e764269..d03dc33ac5 100644 --- a/packages/flow/tests/test_after_stop_mockmed.py +++ b/packages/flow/tests/test_after_stop_mockmed.py @@ -186,7 +186,9 @@ def _halted_run(tmp_path, url, *, with_undo=True): replayer = Replayer( backend, vision=_vision(), - effect_verifier=RestRecordVerifier(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), poll_interval_s=0.01, ) run_dir = tmp_path / "run" @@ -199,7 +201,9 @@ def _undo_replayer(url): replayer = Replayer( backend, vision=_vision(), - effect_verifier=RestRecordVerifier(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), poll_interval_s=0.01, ) return replayer, backend @@ -317,7 +321,9 @@ def press(self, key): replayer = Replayer( DeleteIgnored(mockmed), vision=_vision(), - effect_verifier=RestRecordVerifier(mockmed), + effect_verifier=RestRecordVerifier( + mockmed, records_path="/api/db", records_key="records" + ), poll_interval_s=0.01, ) result = run_undo( @@ -607,7 +613,9 @@ def test_compiler_probe_text_never_reaches_the_ledger(tmp_path, mockmed): replayer = Replayer( MockMedBackend(mockmed), vision=_vision(), - effect_verifier=RestRecordVerifier(mockmed), + effect_verifier=RestRecordVerifier( + mockmed, records_path="/api/db", records_key="records" + ), poll_interval_s=0.01, ) report = replayer.run( @@ -731,7 +739,9 @@ def test_a_second_undo_declared_after_the_stop_still_matches(tmp_path, mockmed): replayer = Replayer( MockMedBackend(mockmed), vision=_vision(), - effect_verifier=RestRecordVerifier(mockmed), + effect_verifier=RestRecordVerifier( + mockmed, records_path="/api/db", records_key="records" + ), poll_interval_s=0.01, ) run_dir = tmp_path / "run" diff --git a/packages/flow/tests/test_api_step_approval.py b/packages/flow/tests/test_api_step_approval.py index 7fcdeee8be..09d727187c 100644 --- a/packages/flow/tests/test_api_step_approval.py +++ b/packages/flow/tests/test_api_step_approval.py @@ -163,7 +163,9 @@ def _run(tmp_path, person: Person, *, transport_setup=None): report = Replayer( backend, vision=FakeVision(), - effect_verifier=RestRecordVerifier(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), web_form_actuator=WebFormActuator(transport), approval_gate=gate, poll_interval_s=0.01, diff --git a/packages/flow/tests/test_api_step_durable.py b/packages/flow/tests/test_api_step_durable.py index f70743a07e..eae7c943cc 100644 --- a/packages/flow/tests/test_api_step_durable.py +++ b/packages/flow/tests/test_api_step_durable.py @@ -122,7 +122,7 @@ class SecondReadFails(RestRecordVerifier): """Reads the record check's pre-state once, then can't reach it.""" def __init__(self, base_url: str) -> None: - super().__init__(base_url) + super().__init__(base_url, records_path="/api/db", records_key="records") self.reads = 0 def capture_pre_state(self, context: Any = None): @@ -148,7 +148,10 @@ def replayer( return Replayer( self.backend, vision=FakeVision(), - effect_verifier=verifier or RestRecordVerifier(verifier_url or self.url), + effect_verifier=verifier + or RestRecordVerifier( + verifier_url or self.url, records_path="/api/db", records_key="records" + ), web_form_actuator=actuator, poll_interval_s=0.01, durable=True, diff --git a/packages/flow/tests/test_api_step_readback_repair.py b/packages/flow/tests/test_api_step_readback_repair.py index ea566f0c32..a5dec0ca33 100644 --- a/packages/flow/tests/test_api_step_readback_repair.py +++ b/packages/flow/tests/test_api_step_readback_repair.py @@ -43,7 +43,9 @@ def _run(tmp_path, *, pipeline, durable=False, verifier_url=None): report = Replayer( EnterWritesBackend(url), vision=FakeVision(), - effect_verifier=RestRecordVerifier(verifier_url or url), + effect_verifier=RestRecordVerifier( + verifier_url or url, records_path="/api/db", records_key="records" + ), web_form_actuator=actuator, poll_interval_s=0.01, readback_pipeline=pipeline, @@ -112,7 +114,9 @@ def test_a_probe_or_resumed_run_never_sends_the_api_step(tmp_path): backend = EnterWritesBackend(url) target = ReplayTarget( lambda _fault: _Session(backend), - effect_verifier=RestRecordVerifier(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), policy="permissive", profile="demo", replayer_kwargs={ @@ -146,7 +150,9 @@ def test_a_standard_change_test_record_run_is_gui_only(tmp_path): backend = EnterWritesBackend(url) target = ReplayTarget( lambda _fault: _Session(backend), - effect_verifier=RestRecordVerifier(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), policy="permissive", profile="demo", replayer_kwargs={ diff --git a/packages/flow/tests/test_api_tier_web_form.py b/packages/flow/tests/test_api_tier_web_form.py index 70c2a6c36e..f4d98310ed 100644 --- a/packages/flow/tests/test_api_tier_web_form.py +++ b/packages/flow/tests/test_api_tier_web_form.py @@ -599,7 +599,9 @@ def _run(tmp_path, wf, status, **kw): report = Replayer( backend, vision=FakeVision(), - effect_verifier=RestRecordVerifier(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), web_form_actuator=actuator, poll_interval_s=0.01, **kw, @@ -735,7 +737,9 @@ def test_an_uncertain_save_needs_reconciliation_and_is_never_redone(tmp_path): report = Replayer( backend, vision=FakeVision(), - effect_verifier=RestRecordVerifier(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), web_form_actuator=actuator, poll_interval_s=0.01, ).run(_span_workflow(), bundle_dir=bundle, run_dir=tmp_path / "run") diff --git a/packages/flow/tests/test_app_profile.py b/packages/flow/tests/test_app_profile.py index 505687d923..7a05385578 100644 --- a/packages/flow/tests/test_app_profile.py +++ b/packages/flow/tests/test_app_profile.py @@ -286,6 +286,12 @@ def test_a_colleagues_own_ui_state_is_theirs_only_by_their_entry_for_it( assert _frame(_run(tmp_path / "d", password)) == "refuted" +def test_the_openemr_profile_names_its_no_record_value() -> None: + """Record 0 names no chart in OpenEMR's data, not in the engine.""" + profile = load_profile("openemr") + assert profile.no_record_values == ["0"] + + def test_a_rule_never_judges_a_row_whose_columns_have_no_cell_macs( tmp_path: Path, ) -> None: diff --git a/packages/flow/tests/test_approval_mode_cli.py b/packages/flow/tests/test_approval_mode_cli.py index 481c71420f..dbcb5dc281 100644 --- a/packages/flow/tests/test_approval_mode_cli.py +++ b/packages/flow/tests/test_approval_mode_cli.py @@ -186,6 +186,8 @@ def _run_case_argv( str(tmp_path / "case-run"), "--effects-kind", "rest", + "--effects-records-path", + "/api/db", "--effects-base-url", "http://sor.local", "--backend", diff --git a/packages/flow/tests/test_at_rest_default.py b/packages/flow/tests/test_at_rest_default.py index a758a3fa4a..d4b2f291ec 100644 --- a/packages/flow/tests/test_at_rest_default.py +++ b/packages/flow/tests/test_at_rest_default.py @@ -276,7 +276,9 @@ def _mockmed_run(tmp_path: Path, bundle: Path, run_dir: Path, **kw): return Replayer( MockMedBackend(url), vision=_vision(), - effect_verifier=RestRecordVerifier(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), poll_interval_s=0.01, ).run(Workflow.load(bundle), bundle_dir=bundle, run_dir=run_dir, **kw) finally: diff --git a/packages/flow/tests/test_auto_repair_cli.py b/packages/flow/tests/test_auto_repair_cli.py index 2e34ecf672..43863dac73 100644 --- a/packages/flow/tests/test_auto_repair_cli.py +++ b/packages/flow/tests/test_auto_repair_cli.py @@ -41,6 +41,7 @@ def _config(tmp_path: Path, base_url: str, *, auto: bool = True) -> Path: config = tmp_path / "deploy.yaml" config.write_text( f"effects:\n kind: rest\n base_url: {base_url}\n" + " records_path: /api/db\n records_key: records\n" "policy:\n policy: clinical-write\n" f"{approvers_yaml()}" "repair:\n" diff --git a/packages/flow/tests/test_certified_binding_admission.py b/packages/flow/tests/test_certified_binding_admission.py index 480b11df92..8527763100 100644 --- a/packages/flow/tests/test_certified_binding_admission.py +++ b/packages/flow/tests/test_certified_binding_admission.py @@ -100,8 +100,13 @@ def _derived_event(record=None) -> dict: def _fixture(url: str, **over) -> AdmissionFixture: + # MockMed's own shape, named here: the engine assumes no app's paths. kw = dict( base_url=url, + records_path="/api/db", + records_key="records", + write_path="/api/encounter", + server_fields=frozenset({"source"}), params={"note": NOTE}, timeout_s=2.0, ) @@ -144,6 +149,68 @@ def test_propose_rest_binding_from_observed_sor_delta(): assert binding.on_unavailable == "gui" +def test_propose_needs_the_apps_write_path_when_none_was_captured(): + """No captured api_write and no write path named: no guessed endpoint.""" + fixture = AdmissionFixture( + base_url="http://held-out.example", + records_path="/api/db", + records_key="records", + params={"note": NOTE}, + ) + step = _save_step() + assert ( + propose_rest_binding( + step, _derived_event(), fixture=fixture, exclude_texts=(NOTE,) + ) + is None + ) + + +def test_propose_uses_another_apps_captured_write_and_server_fields(): + """A Frappe-shaped write: the captured endpoint, the app's own fields.""" + step = _save_step() + event = _derived_event() + event[API_WRITE_KEY] = { + "kind": "rest", + "method": "POST", + "url_template": "/api/resource/Loan Application", + "body": {"applicant": "p1", "description": "{note}"}, + } + fixture = AdmissionFixture( + base_url="http://held-out.example", + records_path="/api/resource/Loan Application", + records_key="data", + server_fields=frozenset({"owner", "creation", "modified"}), + params={"note": NOTE}, + ) + proposal = propose_rest_binding( + step, event, fixture=fixture, exclude_texts=(NOTE,), params={"note": NOTE} + ) + assert proposal is not None + assert proposal.binding.url_template == "/api/resource/Loan Application" + assert proposal.binding.body_template == { + "applicant": "p1", + "description": "{note}", + } + + # Without a capture, the body comes from the new record minus the fields + # this app assigns itself. + fixture = AdmissionFixture( + base_url="http://held-out.example", + records_path="/api/resource/Loan Application", + records_key="data", + write_path="/api/resource/Loan Application", + server_fields=frozenset({"source"}), + params={"note": NOTE}, + ) + proposal = propose_rest_binding( + step, _derived_event(), fixture=fixture, exclude_texts=(NOTE,) + ) + assert proposal is not None + assert "source" not in proposal.binding.body_template + assert "id" not in proposal.binding.body_template + + def test_propose_refuses_fhir_mcp_tool_captures(): step = _save_step() for kind in ("fhir", "mcp", "tool"): @@ -253,7 +320,9 @@ def test_refused_admission_leaves_gui_ladder(tmp_path): report = Replayer( backend, vision=_vision_that_confirms_saved(), - effect_verifier=RestRecordVerifier(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), poll_interval_s=0.01, ).run(_save_workflow(step), bundle_dir=bundle, run_dir=run_dir) @@ -306,7 +375,9 @@ def test_admitted_binding_halts_on_indeterminate_without_gui_retry(tmp_path): report = Replayer( backend, vision=_vision_that_confirms_saved(), - effect_verifier=_IndeterminateVerifier(url), + effect_verifier=_IndeterminateVerifier( + url, records_path="/api/db", records_key="records" + ), api_actuator=ApiActuator(url), poll_interval_s=0.01, ).run(_save_workflow(step), bundle_dir=bundle, run_dir=run_dir) @@ -339,7 +410,15 @@ def test_admit_requires_actuated_then_confirmed(): # build the request. Nothing is written; the step stays GUI-only. decision = admit_rest_binding( proposal, - fixture=AdmissionFixture(base_url=url, params={}, timeout_s=0.5), + fixture=AdmissionFixture( + records_path="/api/db", + records_key="records", + write_path="/api/encounter", + server_fields=frozenset({"source"}), + base_url=url, + params={}, + timeout_s=0.5, + ), ) assert decision.admitted is False assert decision.actuation is not None @@ -393,7 +472,7 @@ def test_compile_admits_rest_binding_on_held_out_fixture(tmp_path): name="certified-demo", risk_overrides={"step_001": "irreversible"}, admit_api_bindings=True, - admission_fixture_url=url, + admission_fixture=_fixture(url), skill_library_root=tmp_path / "skills", ) save = workflow.steps[1] diff --git a/packages/flow/tests/test_cli_deployment.py b/packages/flow/tests/test_cli_deployment.py index d5d3aedc63..d2b1932300 100644 --- a/packages/flow/tests/test_cli_deployment.py +++ b/packages/flow/tests/test_cli_deployment.py @@ -93,8 +93,20 @@ def test_load_non_mapping_raises(tmp_path: Path) -> None: @pytest.mark.parametrize( "kind,kwargs,expected", [ - ("rest", {"base_url": "http://sor"}, "RestRecordVerifier"), - ("fhir", {"base_url": "http://sor/fhir"}, "FhirEffectVerifier"), + ( + "rest", + {"base_url": "http://sor", "records_path": "/api/db"}, + "RestRecordVerifier", + ), + ( + "fhir", + { + "base_url": "http://sor/fhir", + "resource_type": "Observation", + "field_paths": {"value": "valueString"}, + }, + "FhirEffectVerifier", + ), ("document-hash", {"root": "/tmp/store"}, "DocumentHashVerifier"), ], ) @@ -109,7 +121,7 @@ def test_build_verifier_none_kind_is_none() -> None: def test_rest_requires_base_url() -> None: with pytest.raises(ValueError, match="base_url"): - build_effect_verifier(EffectsConfig(kind="rest")) + build_effect_verifier(EffectsConfig(kind="rest", records_path="/api/db")) def test_document_hash_requires_root() -> None: diff --git a/packages/flow/tests/test_cli_new_commands.py b/packages/flow/tests/test_cli_new_commands.py index dd9f06fdc3..620650efd8 100644 --- a/packages/flow/tests/test_cli_new_commands.py +++ b/packages/flow/tests/test_cli_new_commands.py @@ -126,6 +126,7 @@ def _replay_ns(**over) -> argparse.Namespace: config=None, effects_kind=None, effects_base_url=None, + effects_records_path=None, effects_root=None, api_actuator=False, api_base_url=None, @@ -145,6 +146,7 @@ def test_deployment_runtime_flags_wire_objects() -> None: ns = _replay_ns( effects_kind="rest", effects_base_url="http://sor", + effects_records_path="/api/db", api_base_url="http://api", durable=True, ) @@ -460,6 +462,8 @@ def test_replay_injects_effects_actuator_durable_and_worklist( "http://app.example", "--effects-kind", "rest", + "--effects-records-path", + "/api/db", "--effects-base-url", "http://sor", "--api-base-url", @@ -533,6 +537,7 @@ def _armed_click(sid, ocr, *, risk="reversible", effects=None): cfg.write_text( "backend:\n url: http://from-config\n" "effects:\n kind: rest\n base_url: http://sor\n" + " records_path: /api/db\n" "runtime:\n durable: true\n" "policy:\n policy: clinical-write\n" ) diff --git a/packages/flow/tests/test_compose_runtime.py b/packages/flow/tests/test_compose_runtime.py index 33d1eee6b0..0df6c75cf7 100644 --- a/packages/flow/tests/test_compose_runtime.py +++ b/packages/flow/tests/test_compose_runtime.py @@ -230,7 +230,9 @@ def child_run( report = Replayer( backend, vision=_vision_confirms_saved(), - effect_verifier=RestRecordVerifier(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), poll_interval_s=0.01, ).run( workflow, diff --git a/packages/flow/tests/test_create_form_wire.py b/packages/flow/tests/test_create_form_wire.py index f0265811af..9f2fc25113 100644 --- a/packages/flow/tests/test_create_form_wire.py +++ b/packages/flow/tests/test_create_form_wire.py @@ -1166,7 +1166,9 @@ def test_an_api_span_after_a_late_refusal_halts_before_sending( report = Replayer( backend, web_form_actuator=actuator, - effect_verifier=RestRecordVerifier(sor_url.rstrip("/")), + effect_verifier=RestRecordVerifier( + sor_url.rstrip("/"), records_path="/api/db", records_key="records" + ), poll_interval_s=0.01, ).run( _span_workflow(), @@ -1269,7 +1271,9 @@ def test_a_late_refusal_adds_to_an_earlier_halt_instead_of_replacing_it( replayer = Replayer( backend, web_form_actuator=actuator, - effect_verifier=RestRecordVerifier(sor_url.rstrip("/")), + effect_verifier=RestRecordVerifier( + sor_url.rstrip("/"), records_path="/api/db", records_key="records" + ), poll_interval_s=0.01, ) report = replayer.run( diff --git a/packages/flow/tests/test_decision_healthy_replay.py b/packages/flow/tests/test_decision_healthy_replay.py index c8e4d036e7..6a03c1ee0c 100644 --- a/packages/flow/tests/test_decision_healthy_replay.py +++ b/packages/flow/tests/test_decision_healthy_replay.py @@ -54,7 +54,9 @@ def _replay(tmp_path: Path): replayer = Replayer( WritingBackend(url), vision=_vision_that_confirms_saved(), - effect_verifier=RestRecordVerifier(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), poll_interval_s=0.01, ) return replayer.run(workflow, bundle_dir=bundle, run_dir=run_dir) diff --git a/packages/flow/tests/test_desktop_check_it_cli_mockmed.py b/packages/flow/tests/test_desktop_check_it_cli_mockmed.py index 47ac633a7d..874ca385ba 100644 --- a/packages/flow/tests/test_desktop_check_it_cli_mockmed.py +++ b/packages/flow/tests/test_desktop_check_it_cli_mockmed.py @@ -365,6 +365,8 @@ def run_case(case_id: str) -> SimpleNamespace: entry_url, "--effects-kind", "rest", + "--effects-records-path", + "/api/db", "--effects-base-url", base_url, "--approval-wait", @@ -580,6 +582,8 @@ def test_run_it_saves_once_after_approval_and_never_saves_a_form_nobody_sees( check_it.entry_url, "--effects-kind", "rest", + "--effects-records-path", + "/api/db", "--effects-base-url", base_url, "--params-file", diff --git a/packages/flow/tests/test_effect_fault_matrix.py b/packages/flow/tests/test_effect_fault_matrix.py index 4fc18606ed..756ec4b9e2 100644 --- a/packages/flow/tests/test_effect_fault_matrix.py +++ b/packages/flow/tests/test_effect_fault_matrix.py @@ -125,7 +125,7 @@ def test_screen_passes_but_effect_catches(sor, mode): seed = mode == "stale" db.reset(seed_concurrent=seed) - verifier = RestRecordVerifier(base) + verifier = RestRecordVerifier(base, records_path="/api/db", records_key="records") before = verifier.capture_pre_state() assert before.reachable @@ -148,7 +148,7 @@ def test_screen_passes_but_effect_catches(sor, mode): def test_control_clean_write_both_agree(sor): base, db = sor db.reset() - verifier = RestRecordVerifier(base) + verifier = RestRecordVerifier(base, records_path="/api/db", records_key="records") before = verifier.capture_pre_state() _drive_persistence_boundary(base, db, "ok") assert SCREEN_SHOWS_SUCCESS["ok"] is True @@ -161,7 +161,7 @@ def test_idempotency_key_neutralizes_duplicate(sor): # refuted -- proving the at-most-once check. base, db = sor db.reset() - verifier = RestRecordVerifier(base) + verifier = RestRecordVerifier(base, records_path="/api/db", records_key="records") before = verifier.capture_pre_state() _drive_persistence_boundary(base, db, "idempotent") written = Effect( @@ -181,7 +181,7 @@ def test_timeout_effect_verify_is_more_correct_than_screen(sor): # would double-write. base, db = sor db.reset() - verifier = RestRecordVerifier(base) + verifier = RestRecordVerifier(base, records_path="/api/db", records_key="records") before = verifier.capture_pre_state() _drive_persistence_boundary(base, db, "timeout") assert SCREEN_SHOWS_SUCCESS["timeout"] is False # screen: false-abort @@ -199,7 +199,7 @@ def test_session_expiry_both_safe(sor): # no banner -> the screen halts, and effect-verify agrees nothing landed. base, db = sor db.reset() - verifier = RestRecordVerifier(base) + verifier = RestRecordVerifier(base, records_path="/api/db", records_key="records") before = verifier.capture_pre_state() _drive_persistence_boundary(base, db, "session") assert SCREEN_SHOWS_SUCCESS["session"] is False diff --git a/packages/flow/tests/test_effect_fhir.py b/packages/flow/tests/test_effect_fhir.py index d49cc545d0..676e854ec4 100644 --- a/packages/flow/tests/test_effect_fhir.py +++ b/packages/flow/tests/test_effect_fhir.py @@ -9,6 +9,8 @@ from __future__ import annotations +from openadapt_flow.runtime.effects.fhir import DEFAULT_OBSERVATION_PATHS + import os import pytest @@ -38,6 +40,7 @@ def fhir(): def _verifier(base, *, token=None): return FhirEffectVerifier( base, + field_paths=dict(DEFAULT_OBSERVATION_PATHS), resource_type="Observation", search_params={"patient": PATIENT}, access_token=token, @@ -152,6 +155,7 @@ def test_fhir_expired_token_is_indeterminate_not_absent(fhir): try: v = FhirEffectVerifier( _base, + field_paths=dict(DEFAULT_OBSERVATION_PATHS), resource_type="Observation", search_params={"patient": PATIENT}, access_token="WRONG", @@ -172,6 +176,7 @@ def test_fhir_expired_token_is_indeterminate_not_absent(fhir): def test_fhir_unreachable_is_indeterminate(): v = FhirEffectVerifier( "http://127.0.0.1:1", + field_paths=dict(DEFAULT_OBSERVATION_PATHS), resource_type="Observation", search_params={"patient": PATIENT}, timeout_s=0.2, @@ -199,6 +204,7 @@ def test_live_openemr_fhir_reachable(): patient = os.environ.get("OPENEMR_FHIR_PATIENT", "1") v = FhirEffectVerifier( base, + field_paths=dict(DEFAULT_OBSERVATION_PATHS), resource_type="Observation", search_params={"patient": patient}, access_token=token, diff --git a/packages/flow/tests/test_effect_kit_config.py b/packages/flow/tests/test_effect_kit_config.py index 4a43a4bdbd..4cd0c4ccb2 100644 --- a/packages/flow/tests/test_effect_kit_config.py +++ b/packages/flow/tests/test_effect_kit_config.py @@ -8,6 +8,8 @@ from __future__ import annotations +from openadapt_flow.runtime.effects.fhir import DEFAULT_OBSERVATION_PATHS + import sqlite3 from pathlib import Path @@ -114,7 +116,11 @@ def json(): with_auth = _Session() RestRecordVerifier( - "http://x", session=with_auth, headers={"Authorization": "Bearer t"} + "http://x", + records_path="/api/db", + records_key="records", + session=with_auth, + headers={"Authorization": "Bearer t"}, ).capture_pre_state() assert with_auth.calls == [{"Authorization": "Bearer t"}] @@ -125,7 +131,9 @@ def get(self, url, *, timeout): # no headers kwarg at all return _Session.get(self, url, timeout=timeout) narrow = _NarrowSession() - state = RestRecordVerifier("http://x", session=narrow).capture_pre_state() + state = RestRecordVerifier( + "http://x", records_path="/api/db", records_key="records", session=narrow + ).capture_pre_state() assert state.reachable assert "narrow" in narrow.calls @@ -175,6 +183,8 @@ class TestFhirKitConfig: def test_search_param_exprs_and_token_env(self, monkeypatch): monkeypatch.setenv("KIT_FHIR_TOKEN", "oauth-token") cfg = EffectsConfig( + resource_type="Observation", + field_paths=dict(DEFAULT_OBSERVATION_PATHS), kind="fhir", base_url="https://emr.local/fhir", search_params={"category": "vital-signs"}, @@ -192,6 +202,8 @@ def test_search_param_exprs_and_token_env(self, monkeypatch): def test_token_env_missing_fails_loud(self, monkeypatch): monkeypatch.delenv("KIT_FHIR_ABSENT", raising=False) cfg = EffectsConfig( + resource_type="Observation", + field_paths=dict(DEFAULT_OBSERVATION_PATHS), kind="fhir", base_url="https://emr.local/fhir", access_token_env="KIT_FHIR_ABSENT", @@ -202,6 +214,8 @@ def test_token_env_missing_fails_loud(self, monkeypatch): def test_token_env_wins_over_literal(self, monkeypatch): monkeypatch.setenv("KIT_FHIR_TOKEN", "from-env") cfg = EffectsConfig( + resource_type="Observation", + field_paths=dict(DEFAULT_OBSERVATION_PATHS), kind="fhir", base_url="https://emr.local/fhir", access_token="literal-should-lose", @@ -329,6 +343,7 @@ def test_candidate_construction_never_skips_a_broken_stronger_proof( candidates=[ EffectsConfig( kind="rest", + records_path="/api/db", base_url="https://oracle.invalid", auth={"bearer_env": "MISSING_ORACLE_TOKEN"}, ), @@ -370,3 +385,53 @@ def test_onscreen_explicit_region_honored(self, tmp_path: Path): def test_onscreen_listed_in_unknown_kind_message(self): with pytest.raises(ValueError, match="onscreen"): build_effect_verifier(EffectsConfig(kind="bogus")) + + +def test_rest_needs_the_apps_records_path(): + """No app's records path is assumed: kind rest names its own.""" + cfg = EffectsConfig(kind="rest", base_url="http://app.local") + with pytest.raises(ValueError, match="records_path"): + build_effect_verifier(cfg) + # A config without a rest read does not carry the field at all. + assert "records_path" not in EffectsConfig(kind="none").model_dump() + + # MockMed's shape and Frappe's both work once named. + for path, key in ( + ("/api/db", "records"), + ("/api/resource/Loan Application", "data"), + ): + v = build_effect_verifier( + EffectsConfig( + kind="rest", + base_url="http://app.local", + records_path=path, + records_key=key, + ) + ) + assert isinstance(v, RestRecordVerifier) + assert (v.records_path, v.records_key) == (path, key) + + +def test_fhir_needs_the_apps_resource_and_field_paths(): + """No app's FHIR layout is assumed: kind fhir names its resource and paths.""" + with pytest.raises(ValueError, match="resource_type and"): + build_effect_verifier(EffectsConfig(kind="fhir", base_url="https://fhir.local")) + assert "resource_type" not in EffectsConfig(kind="none").model_dump() + # A note kept as an Observation (OpenEMR) and a Patient (Epic, Medplum) + # both work once named. + for resource, paths in ( + ("Observation", dict(DEFAULT_OBSERVATION_PATHS)), + ("Patient", {"id": "id", "family": "name.family", "birthDate": "birthDate"}), + ): + v = build_effect_verifier( + EffectsConfig( + kind="fhir", + base_url="https://fhir.local", + resource_type=resource, + field_paths=paths, + ) + ) + assert isinstance(v, FhirEffectVerifier) + assert (v.resource_type, v.field_paths) == (resource, paths) + with pytest.raises(TypeError): + FhirEffectVerifier("https://fhir.local") # type: ignore[call-arg] diff --git a/packages/flow/tests/test_effect_mining.py b/packages/flow/tests/test_effect_mining.py index 95f8db7bd9..d0d14c4468 100644 --- a/packages/flow/tests/test_effect_mining.py +++ b/packages/flow/tests/test_effect_mining.py @@ -112,20 +112,53 @@ def test_mines_record_written_and_field_equals_from_delta(): assert "note" not in field.match -def test_mines_idempotency_key_only_when_observed(): +def test_mines_idempotency_key_only_from_the_named_field_when_observed(): + # MockMed keeps its at-most-once key in "key". with_key = mine_step_effects( {SOR_BEFORE_KEY: [], SOR_AFTER_KEY: [_record(key="idem-abc")]}, _key_step(), exclude_texts=(NOTE,), + idempotency_key_field="key", ) written = next(e for e in with_key.effects if e.kind is EffectKind.RECORD_WRITTEN) assert written.idempotency_key == "idem-abc" assert written.key_field == "key" + # Another app names another field (a client request id, say). + other = mine_step_effects( + {SOR_BEFORE_KEY: [], SOR_AFTER_KEY: [_record(client_request_id="req-7")]}, + _key_step(), + exclude_texts=(NOTE,), + idempotency_key_field="client_request_id", + ) + written3 = next(e for e in other.effects if e.kind is EffectKind.RECORD_WRITTEN) + assert written3.idempotency_key == "req-7" + assert written3.key_field == "client_request_id" + + # No field named: the "key" convention, the default of Effect.key_field. + unnamed = mine_step_effects( + {SOR_BEFORE_KEY: [], SOR_AFTER_KEY: [_record(key="idem-abc")]}, + _key_step(), + exclude_texts=(NOTE,), + ) + written4 = next(e for e in unnamed.effects if e.kind is EffectKind.RECORD_WRITTEN) + assert written4.idempotency_key == "idem-abc" + + # None: no key mined, whatever the record holds. + none = mine_step_effects( + {SOR_BEFORE_KEY: [], SOR_AFTER_KEY: [_record(key="idem-abc")]}, + _key_step(), + exclude_texts=(NOTE,), + idempotency_key_field=None, + ) + written5 = next(e for e in none.effects if e.kind is EffectKind.RECORD_WRITTEN) + assert written5.idempotency_key is None + without = mine_step_effects( {SOR_BEFORE_KEY: [], SOR_AFTER_KEY: [_record()]}, _key_step(), exclude_texts=(NOTE,), + idempotency_key_field="key", ) written2 = next(e for e in without.effects if e.kind is EffectKind.RECORD_WRITTEN) # Never invented: §7 names the idempotency key as app-specific. @@ -196,7 +229,9 @@ def test_dom_field_delta_mines_flagged_field_equals(): def test_mined_effects_confirmed_by_verifier_on_real_sor(): base, db, stop = _fault_server() try: - verifier = RestRecordVerifier(base) + verifier = RestRecordVerifier( + base, records_path="/api/db", records_key="records" + ) before = db.snapshot()["records"] requests.post( f"{base}/api/encounter", @@ -220,7 +255,9 @@ def test_mined_effects_confirmed_by_verifier_on_real_sor(): def test_mined_record_written_refutes_a_duplicate_on_the_real_sor(): base, db, stop = _fault_server() try: - verifier = RestRecordVerifier(base) + verifier = RestRecordVerifier( + base, records_path="/api/db", records_key="records" + ) before = db.snapshot()["records"] requests.post( f"{base}/api/encounter", @@ -266,7 +303,9 @@ def test_placeholder_effect_halts_the_run(tmp_path): replayer = Replayer( backend, vision=vision, - effect_verifier=RestRecordVerifier(base), + effect_verifier=RestRecordVerifier( + base, records_path="/api/db", records_key="records" + ), poll_interval_s=0.01, ) report = replayer.run(workflow, bundle_dir=bundle, run_dir=run_dir) diff --git a/packages/flow/tests/test_effect_plugin_options.py b/packages/flow/tests/test_effect_plugin_options.py index cd2c78885a..035a51084f 100644 --- a/packages/flow/tests/test_effect_plugin_options.py +++ b/packages/flow/tests/test_effect_plugin_options.py @@ -250,7 +250,9 @@ def test_changed_options_change_the_verifier_identity(pack): def test_identity_of_configs_without_plugin_fields_is_unchanged(): """Existing deployments keep a byte-identical verifier identity.""" - cfg = EffectsConfig(kind="rest", base_url="http://127.0.0.1:9") + cfg = EffectsConfig( + kind="rest", records_path="/api/db", base_url="http://127.0.0.1:9" + ) verifier = build_effect_verifier(cfg) legacy_config = cfg.model_dump(mode="json") # Neither the plugin fields, ui_readback, smart_backend, surfaces, @@ -268,6 +270,8 @@ def test_identity_of_configs_without_plugin_fields_is_unchanged(): ): legacy_config.pop(name, None) legacy_config["candidates"] = [] + # resource_type had a default then and was always in the dump. + legacy_config.setdefault("resource_type", "Observation") def sanitized(value, *, key=""): # the identity's own redaction, verbatim lowered = key.lower() diff --git a/packages/flow/tests/test_effect_verifier.py b/packages/flow/tests/test_effect_verifier.py index 7229cda3ce..15542bd0a5 100644 --- a/packages/flow/tests/test_effect_verifier.py +++ b/packages/flow/tests/test_effect_verifier.py @@ -8,7 +8,6 @@ from __future__ import annotations - import pytest import requests @@ -59,11 +58,19 @@ def _post_encounter(base, *, note=NOTE, fault="", key=None): def test_all_verifiers_satisfy_protocol(tmp_path): - assert isinstance(RestRecordVerifier("http://x"), EffectVerifier) + assert isinstance( + RestRecordVerifier("http://x", records_path="/api/db", records_key="records"), + EffectVerifier, + ) assert isinstance(DocumentHashVerifier(tmp_path), EffectVerifier) from openadapt_flow.runtime.effects import FhirEffectVerifier - assert isinstance(FhirEffectVerifier("http://x"), EffectVerifier) + assert isinstance( + FhirEffectVerifier( + "http://x", resource_type="Observation", field_paths={"id": "id"} + ), + EffectVerifier, + ) # -- judge_records unit logic (no network) ---------------------------------- @@ -149,7 +156,7 @@ def test_record_matches_string_coercion(): def test_rest_verifier_confirms_real_write(sor): base, _db = sor - verifier = RestRecordVerifier(base) + verifier = RestRecordVerifier(base, records_path="/api/db", records_key="records") before = verifier.capture_pre_state() assert before.reachable and before.records == [] _post_encounter(base) @@ -164,7 +171,9 @@ def test_rest_verifier_confirms_real_write(sor): def test_rest_verifier_indeterminate_when_unreachable(): - verifier = RestRecordVerifier("http://127.0.0.1:1") # nothing listening + verifier = RestRecordVerifier( + "http://127.0.0.1:1", records_path="/api/db", records_key="records" + ) # nothing listening before = verifier.capture_pre_state() assert not before.reachable eff = Effect(kind=EffectKind.RECORD_WRITTEN, match=TARGET, timeout_s=0.1) @@ -175,7 +184,7 @@ def test_rest_verifier_indeterminate_when_unreachable(): def test_rest_idempotent_write_is_at_most_once(sor): base, _db = sor - verifier = RestRecordVerifier(base) + verifier = RestRecordVerifier(base, records_path="/api/db", records_key="records") before = verifier.capture_pre_state() # Two submissions carrying the SAME idempotency key -> server dedupes. _post_encounter(base, fault="idempotent", key="run-42") @@ -246,7 +255,7 @@ def test_document_hash_indeterminate_when_store_absent(tmp_path): def test_compensation_reconciles_detected_duplicate(sor): base, db = sor - verifier = RestRecordVerifier(base) + verifier = RestRecordVerifier(base, records_path="/api/db", records_key="records") before = verifier.capture_pre_state() _post_encounter(base) # two non-idempotent submissions -> two rows _post_encounter(base) @@ -260,7 +269,7 @@ def test_compensation_reconciles_detected_duplicate(sor): verdict = verifier.verify(eff, before) assert verdict.verdict is Verdict.REFUTED and verdict.observed_count == 2 - compensator = RestCompensator(base) + compensator = RestCompensator(base, delete_path_template="/api/encounter/{id}") result = reconcile_or_escalate( eff, verdict, verifier=verifier, before=before, compensator=compensator ) @@ -272,7 +281,7 @@ def test_compensation_reconciles_detected_duplicate(sor): def test_compensation_escalates_partial_save(sor): base, _db = sor - verifier = RestRecordVerifier(base) + verifier = RestRecordVerifier(base, records_path="/api/db", records_key="records") before = verifier.capture_pre_state() _post_encounter(base, fault="partial") # row persists, note dropped eff = Effect( @@ -290,7 +299,7 @@ def test_compensation_escalates_partial_save(sor): verdict, verifier=verifier, before=before, - compensator=RestCompensator(base), + compensator=RestCompensator(base, delete_path_template="/api/encounter/{id}"), ) assert result.outcome is CompensationOutcome.ESCALATED assert not result.proceed @@ -298,7 +307,9 @@ def test_compensation_escalates_partial_save(sor): def test_compensation_escalates_when_indeterminate(): - verifier = RestRecordVerifier("http://127.0.0.1:1") + verifier = RestRecordVerifier( + "http://127.0.0.1:1", records_path="/api/db", records_key="records" + ) before = EffectState(substrate="rest", reachable=False) eff = Effect( kind=EffectKind.RECORD_WRITTEN, @@ -313,7 +324,9 @@ def test_compensation_escalates_when_indeterminate(): verdict, verifier=verifier, before=before, - compensator=RestCompensator("http://127.0.0.1:1"), + compensator=RestCompensator( + "http://127.0.0.1:1", delete_path_template="/api/encounter/{id}" + ), ) assert result.outcome is CompensationOutcome.ESCALATED assert not result.proceed diff --git a/packages/flow/tests/test_effect_verifier_candidates.py b/packages/flow/tests/test_effect_verifier_candidates.py index 8a16767849..a01a1c0cdc 100644 --- a/packages/flow/tests/test_effect_verifier_candidates.py +++ b/packages/flow/tests/test_effect_verifier_candidates.py @@ -2,6 +2,8 @@ from __future__ import annotations +from openadapt_flow.runtime.effects.fhir import DEFAULT_OBSERVATION_PATHS + import pytest from openadapt_flow.deployment import ( @@ -368,17 +370,29 @@ def verify(self): def test_verifier_identity_binds_config_but_not_literal_secret(): first = build_effect_verifier( EffectsConfig( - kind="fhir", base_url="https://records.example/a", access_token="one" + resource_type="Observation", + field_paths=dict(DEFAULT_OBSERVATION_PATHS), + kind="fhir", + base_url="https://records.example/a", + access_token="one", ) ) rotated_secret = build_effect_verifier( EffectsConfig( - kind="fhir", base_url="https://records.example/a", access_token="two" + resource_type="Observation", + field_paths=dict(DEFAULT_OBSERVATION_PATHS), + kind="fhir", + base_url="https://records.example/a", + access_token="two", ) ) changed_boundary = build_effect_verifier( EffectsConfig( - kind="fhir", base_url="https://records.example/b", access_token="two" + resource_type="Observation", + field_paths=dict(DEFAULT_OBSERVATION_PATHS), + kind="fhir", + base_url="https://records.example/b", + access_token="two", ) ) diff --git a/packages/flow/tests/test_effect_verifier_transport.py b/packages/flow/tests/test_effect_verifier_transport.py index ee30982fef..de6176d57e 100644 --- a/packages/flow/tests/test_effect_verifier_transport.py +++ b/packages/flow/tests/test_effect_verifier_transport.py @@ -17,6 +17,8 @@ from __future__ import annotations +from openadapt_flow.runtime.effects.fhir import DEFAULT_OBSERVATION_PATHS + import builtins import json import threading @@ -95,12 +97,22 @@ def test_session_falls_back_to_httpx_without_requests( _hide(monkeypatch, "requests") assert isinstance( - FhirEffectVerifier("http://127.0.0.1:1")._get_session(), httpx.Client + FhirEffectVerifier( + "http://127.0.0.1:1", + resource_type="Observation", + field_paths=dict(DEFAULT_OBSERVATION_PATHS), + )._get_session(), + httpx.Client, ) def test_injected_session_is_always_preferred(self) -> None: sentinel = object() - verifier = FhirEffectVerifier("http://127.0.0.1:1", session=sentinel) + verifier = FhirEffectVerifier( + "http://127.0.0.1:1", + resource_type="Observation", + field_paths=dict(DEFAULT_OBSERVATION_PATHS), + session=sentinel, + ) assert verifier._get_session() is sentinel def test_records_are_read_over_the_fallback_transport( @@ -108,7 +120,11 @@ def test_records_are_read_over_the_fallback_transport( ) -> None: """The fallback is wired end to end, not merely constructible.""" _hide(monkeypatch, "requests") - state = FhirEffectVerifier(sor, resource_type="Observation").capture_pre_state() + state = FhirEffectVerifier( + sor, + field_paths=dict(DEFAULT_OBSERVATION_PATHS), + resource_type="Observation", + ).capture_pre_state() assert state.reachable is True assert [record["id"] for record in state.records] == ["obs-1"] @@ -121,7 +137,12 @@ def test_tls_verification_is_bound_on_the_fallback_client( the blanket except reads as an unreadable system of record. """ _hide(monkeypatch, "requests") - verifier = FhirEffectVerifier(sor, verify_tls=False) + verifier = FhirEffectVerifier( + sor, + resource_type="Observation", + field_paths=dict(DEFAULT_OBSERVATION_PATHS), + verify_tls=False, + ) assert verifier._get_session() is not None assert verifier._verify_per_request is False assert verifier.capture_pre_state().reachable is True diff --git a/packages/flow/tests/test_fault_gate.py b/packages/flow/tests/test_fault_gate.py index 1945c04a2a..1af8751c35 100644 --- a/packages/flow/tests/test_fault_gate.py +++ b/packages/flow/tests/test_fault_gate.py @@ -58,7 +58,7 @@ def _rest_cfg(base: str = "http://127.0.0.1:1"): @pytest.fixture def rest_copy(): - store = MemoryRestCopy(SEED, TARGET) + store = MemoryRestCopy(SEED, TARGET, records_path="/api/db", records_key="records") try: yield store finally: diff --git a/packages/flow/tests/test_frame_conditions.py b/packages/flow/tests/test_frame_conditions.py index a1d50ba519..9160af0ac8 100644 --- a/packages/flow/tests/test_frame_conditions.py +++ b/packages/flow/tests/test_frame_conditions.py @@ -282,6 +282,12 @@ def test_a_bot_log_entry_on_an_undeclared_record_is_collateral() -> None: # Both reads read uuid_registry as append-only, and the profile marks # it: the bot's insert there, filed under no chart, is bookkeeping. (["openemr.uuid_registry"], {}, "held"), + # An empty record field names no record in any app. + (["openemr.uuid_registry"], {"record": None, "values": ()}, "held"), + (["openemr.uuid_registry"], {"record": "", "values": ()}, "held"), + # Record 0 names no record only where the app profile says so; in an + # app whose profile doesn't, 0 is a record id and the entry counts. + (["openemr.uuid_registry"], {"values": ()}, "refuted"), # No read of it, or a read of another table: the entry still refutes. ([], {}, "refuted"), (["openemr.log"], {}, "refuted"), @@ -294,6 +300,9 @@ def test_a_bot_log_entry_on_an_undeclared_record_is_collateral() -> None: def test_a_bot_insert_entry_naming_no_record_needs_that_table_read( read: list[str], entry: dict, status: str ) -> None: + entry = dict(entry) + # OpenEMR's profile: an entry filed under patient 0 names no chart. + values = entry.pop("values", ("0",)) spec = _audit_spec() assert spec.attribution is not None spec.attribution.table_field = "tbl" @@ -308,6 +317,7 @@ def test_a_bot_insert_entry_naming_no_record_needs_that_table_read( audit_before=[], audit_after=[logged], no_record_inserts=read, + no_record_values=values, ) assert verdict.status == status, verdict.reason diff --git a/packages/flow/tests/test_generate_ensure_contract.py b/packages/flow/tests/test_generate_ensure_contract.py index f52da87ec8..ed32381f62 100644 --- a/packages/flow/tests/test_generate_ensure_contract.py +++ b/packages/flow/tests/test_generate_ensure_contract.py @@ -303,7 +303,7 @@ def test_an_unexplained_element_of_a_told_apart_list_refuses(teach): "bad", [{"agent_cannot_write_audit": "false"}, {"audit_lag_max_s": "0.1"}] ) def test_server_facts_of_the_wrong_type_refuse(teach, bad): - teach.facts = dict(gen.facts_from_probe(dict(teach.facts)), **bad) + teach.facts = dict(gen.server_facts(dict(teach.facts)), **bad) out = generate(teach) assert any("wrong type" in r for r in out.refusals) diff --git a/packages/flow/tests/test_loop_authoring.py b/packages/flow/tests/test_loop_authoring.py index 1e9c87ac03..bf7f5949bb 100644 --- a/packages/flow/tests/test_loop_authoring.py +++ b/packages/flow/tests/test_loop_authoring.py @@ -379,7 +379,9 @@ def test_healthy_loop_confirms_each_record_zero_model_calls(dirs): report = Replayer( backend, vision=_vision_confirms_saved(), - effect_verifier=RestRecordVerifier(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), poll_interval_s=0.01, ).run(looped, bundle_dir=bundle, run_dir=run_dir) @@ -425,7 +427,9 @@ def test_poisoned_record_effect_refute_halts_no_silent_skip(dirs): report = Replayer( backend, vision=_vision_confirms_saved(), - effect_verifier=RestRecordVerifier(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), poll_interval_s=0.01, ).run(looped, bundle_dir=bundle, run_dir=run_dir) @@ -500,7 +504,9 @@ def test_committed_encounter_showcase_loop_replays_through_the_interpreter(tmp_p report = Replayer( backend, vision=_vision_confirms_saved(), - effect_verifier=RestRecordVerifier(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), poll_interval_s=0.01, ).run(wf, bundle_dir=bundle, run_dir=run_dir) diff --git a/packages/flow/tests/test_openemr_e2e_harness.py b/packages/flow/tests/test_openemr_e2e_harness.py index 52062f0f4f..7348343989 100644 --- a/packages/flow/tests/test_openemr_e2e_harness.py +++ b/packages/flow/tests/test_openemr_e2e_harness.py @@ -40,7 +40,9 @@ def _replay(workflow, *, bundle_dir, run_dir, note, fault="", modal=None): replayer = Replayer( backend, vision=vision, - effect_verifier=RestRecordVerifier(url.rstrip("/")), + effect_verifier=RestRecordVerifier( + url.rstrip("/"), records_path="/api/db", records_key="records" + ), poll_interval_s=0.01, ) report = replayer.run( diff --git a/packages/flow/tests/test_probe_induction.py b/packages/flow/tests/test_probe_induction.py index 9457f96f31..6fe2940358 100644 --- a/packages/flow/tests/test_probe_induction.py +++ b/packages/flow/tests/test_probe_induction.py @@ -272,7 +272,11 @@ def _contract(directory: Path, base_url: str) -> Path: write_bundle( directory, contract_id="reward_contract_mockmed_probe", - oracle={"kind": "rest", "base_url": base_url.rstrip("/")}, + oracle={ + "kind": "rest", + "records_path": "/api/db", + "base_url": base_url.rstrip("/"), + }, channel="api", key=Ed25519PrivateKey.generate(), issuer_key_id="self_signed:test", diff --git a/packages/flow/tests/test_qualification_drills.py b/packages/flow/tests/test_qualification_drills.py index a3d9a19704..4f9a64d555 100644 --- a/packages/flow/tests/test_qualification_drills.py +++ b/packages/flow/tests/test_qualification_drills.py @@ -287,6 +287,8 @@ def _drill_run_case_argv(bundle, inputs_path, tmp_path, case_id, *extra): str(tmp_path / f"{case_id}-run"), "--effects-kind", "rest", + "--effects-records-path", + "/api/db", "--effects-base-url", "http://sor.local", "--backend", diff --git a/packages/flow/tests/test_readback_pipeline.py b/packages/flow/tests/test_readback_pipeline.py index 7c20079f88..1396249468 100644 --- a/packages/flow/tests/test_readback_pipeline.py +++ b/packages/flow/tests/test_readback_pipeline.py @@ -27,6 +27,11 @@ def dirs(tmp_path): return bundle, tmp_path / "run" +def _demo_verifier(url): + """The demo app's records read.""" + return RestRecordVerifier(url, records_path="/api/db", records_key="records") + + def _run(records, dirs, *, corrupt=None, pipeline=None, verifier_factory=None): bundle, run_dir = dirs looped = author_data_driven_loop(_encounter_body(), records) @@ -35,7 +40,7 @@ def _run(records, dirs, *, corrupt=None, pipeline=None, verifier_factory=None): replayer = Replayer( backend, vision=_vision_confirms_saved(), - effect_verifier=(verifier_factory or RestRecordVerifier)(url), + effect_verifier=(verifier_factory or _demo_verifier)(url), poll_interval_s=0.01, readback_pipeline=pipeline, ) @@ -132,10 +137,15 @@ class ScreenVerifier(RestRecordVerifier): def factory(url): if kind == "screen_attr": - return ScreenVerifier(url) + return ScreenVerifier(url, records_path="/api/db", records_key="records") if kind == "candidate": - return CandidateEffectVerifier([RestRecordVerifier(url)]) - return RedactingVerifier(RestRecordVerifier(url), policy=None) + return CandidateEffectVerifier( + [RestRecordVerifier(url, records_path="/api/db", records_key="records")] + ) + return RedactingVerifier( + RestRecordVerifier(url, records_path="/api/db", records_key="records"), + policy=None, + ) pipeline = ReadbackPipeline() report, settled, backend, _ = _run( @@ -172,7 +182,9 @@ def verify(self, *a, **k): report = Replayer( _RowWritingBackend(url), vision=_vision_confirms_saved(), - effect_verifier=SlowVerifier(url), + effect_verifier=SlowVerifier( + url, records_path="/api/db", records_key="records" + ), poll_interval_s=0.01, readback_pipeline=pipeline, ).run(looped, bundle_dir=bundle, run_dir=run_dir) diff --git a/packages/flow/tests/test_readback_volatile_fields.py b/packages/flow/tests/test_readback_volatile_fields.py index fb73ab01d8..769ecc4e38 100644 --- a/packages/flow/tests/test_readback_volatile_fields.py +++ b/packages/flow/tests/test_readback_volatile_fields.py @@ -98,6 +98,11 @@ def _run1_save(tmp_path: Path, base: str, store: _Store, **extra: Any) -> str: "kind": "fhir", "base_url": base, "resource_type": "Observation", + "field_paths": { + "id": "id", + "patient": "subject.reference", + "note": "valueString", + }, "timeout_s": 2.0, **extra, } @@ -171,6 +176,7 @@ def test_run1_refuses_mapped_server_metadata_before_recording( "verifier": { "kind": "fhir", "base_url": "https://example.test/fhir", + "resource_type": "Observation", "field_paths": {"revision": path}, } } @@ -201,6 +207,7 @@ def test_run1_allows_only_explicitly_dropped_metadata( section = { "kind": "fhir", "base_url": base, + "resource_type": "Observation", "field_paths": {"id": "id", "revision": path}, "records_drop_fields": ["revision"], } @@ -223,6 +230,7 @@ def test_run1_keeps_stable_metadata_mappings(tmp_path: Path) -> None: "verifier": { "kind": "fhir", "base_url": "https://example.test/fhir", + "resource_type": "Observation", "field_paths": {"profile": "meta.profile"}, } } diff --git a/packages/flow/tests/test_record_check_synthesis.py b/packages/flow/tests/test_record_check_synthesis.py index 409d8457dd..f2c846b4d2 100644 --- a/packages/flow/tests/test_record_check_synthesis.py +++ b/packages/flow/tests/test_record_check_synthesis.py @@ -534,6 +534,7 @@ def test_surfaces_config_validation_and_identity_unchanged(): EffectsConfig.model_validate( { "kind": "rest", + "records_path": "/api/db", "base_url": "http://x", "surfaces": {"a": {"kind": "rest"}}, } @@ -546,7 +547,9 @@ def test_surfaces_config_validation_and_identity_unchanged(): from openadapt_flow.deployment import _identity_config_dump dumped = _identity_config_dump( - EffectsConfig.model_validate({"kind": "rest", "base_url": "http://x"}) + EffectsConfig.model_validate( + {"kind": "rest", "records_path": "/api/db", "base_url": "http://x"} + ) ) assert "surfaces" not in dumped diff --git a/packages/flow/tests/test_replayer_api_actuator.py b/packages/flow/tests/test_replayer_api_actuator.py index fad040ea6b..388dfd5a2d 100644 --- a/packages/flow/tests/test_replayer_api_actuator.py +++ b/packages/flow/tests/test_replayer_api_actuator.py @@ -185,7 +185,7 @@ def __init__( fail_verify=False, tier_after_verify=None, ): - super().__init__(url) + super().__init__(url, records_path="/api/db", records_key="records") self.substrate = substrate self.verification_tier = tier self.fail_verify = fail_verify @@ -214,7 +214,7 @@ def __init__(self, reachable): class _LegacyReachabilityVerifier(RestRecordVerifier): def __init__(self, url, reachable): - super().__init__(url) + super().__init__(url, records_path="/api/db", records_key="records") self._reachable = reachable def capture_pre_state(self): @@ -236,7 +236,9 @@ def test_api_binding_actuates_and_confirms_skipping_gui(tmp_path): replayer = Replayer( backend, vision=vision, - effect_verifier=RestRecordVerifier(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), api_actuator=ApiActuator(url), poll_interval_s=0.01, ) @@ -281,7 +283,9 @@ def test_unreachable_api_halts_without_gui_fallback(tmp_path): replayer = Replayer( backend, vision=vision, - effect_verifier=RestRecordVerifier(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), api_actuator=ApiActuator(dead, timeout_s=1.0), poll_interval_s=0.01, ) @@ -307,7 +311,9 @@ def test_unreachable_effect_pre_state_refuses_api_actuation(tmp_path): replayer = Replayer( backend, vision=_vision_that_confirms_saved(), - effect_verifier=RestRecordVerifier("http://127.0.0.1:1"), + effect_verifier=RestRecordVerifier( + "http://127.0.0.1:1", records_path="/api/db", records_key="records" + ), api_actuator=ApiActuator(url), poll_interval_s=0.01, ) @@ -376,7 +382,9 @@ def test_post_send_protocol_error_is_proven_by_the_complete_contract(tmp_path): report = Replayer( backend, vision=_vision_that_confirms_saved(), - effect_verifier=RestRecordVerifier(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), api_actuator=ApiActuator(url, session=session), poll_interval_s=0.01, ).run(workflow, bundle_dir=bundle, run_dir=run_dir) @@ -502,7 +510,9 @@ def test_post_send_protocol_error_with_refuted_effect_requires_reconciliation( report = Replayer( backend, vision=_vision_that_confirms_saved(), - effect_verifier=RestRecordVerifier(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), api_actuator=ApiActuator(url, session=session), poll_interval_s=0.01, ).run(workflow, bundle_dir=bundle, run_dir=run_dir) @@ -533,7 +543,9 @@ def test_post_send_protocol_error_with_unavailable_verifier_requires_reconciliat report = Replayer( backend, vision=_vision_that_confirms_saved(), - effect_verifier=_UnavailableVerifier(url), + effect_verifier=_UnavailableVerifier( + url, records_path="/api/db", records_key="records" + ), api_actuator=ApiActuator(url, session=session), poll_interval_s=0.01, ).run(workflow, bundle_dir=bundle, run_dir=run_dir) @@ -574,7 +586,9 @@ def test_api_write_refuted_by_record_halts(tmp_path): replayer = Replayer( backend, vision=vision, - effect_verifier=RestRecordVerifier(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), api_actuator=ApiActuator(url), poll_interval_s=0.01, ) @@ -614,7 +628,9 @@ def test_api_non_2xx_halts_never_double_writes(tmp_path): replayer = Replayer( backend, vision=vision, - effect_verifier=RestRecordVerifier(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), api_actuator=ApiActuator(url), poll_interval_s=0.01, ) @@ -648,7 +664,9 @@ def test_api_binding_without_effects_is_config_error_halt(tmp_path): replayer = Replayer( backend, vision=vision, - effect_verifier=RestRecordVerifier(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), api_actuator=ApiActuator(url), poll_interval_s=0.01, ) @@ -710,7 +728,9 @@ def test_binding_present_but_no_actuator_uses_gui_unchanged(tmp_path): replayer = Replayer( backend, vision=vision, - effect_verifier=RestRecordVerifier(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), poll_interval_s=0.01, ) report = replayer.run(workflow, bundle_dir=bundle, run_dir=run_dir) diff --git a/packages/flow/tests/test_replayer_effect_param_binding.py b/packages/flow/tests/test_replayer_effect_param_binding.py index 498ac5c3a5..e62d0ff905 100644 --- a/packages/flow/tests/test_replayer_effect_param_binding.py +++ b/packages/flow/tests/test_replayer_effect_param_binding.py @@ -504,7 +504,9 @@ def test_parameterized_effect_confirmed_against_real_sor(tmp_path): replayer = Replayer( backend, vision=_vision_confirms_saved(), - effect_verifier=RestRecordVerifier(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), poll_interval_s=0.01, ) report = replayer.run( @@ -544,7 +546,9 @@ def test_demo_literal_effect_refutes_when_run_writes_different_patient(tmp_path) replayer = Replayer( backend, vision=_vision_confirms_saved(), - effect_verifier=RestRecordVerifier(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), poll_interval_s=0.01, ) report = replayer.run(workflow, bundle_dir=bundle, run_dir=run_dir) diff --git a/packages/flow/tests/test_replayer_effects.py b/packages/flow/tests/test_replayer_effects.py index 976f9ee005..c90989cede 100644 --- a/packages/flow/tests/test_replayer_effects.py +++ b/packages/flow/tests/test_replayer_effects.py @@ -162,7 +162,9 @@ def test_effect_confirmed_proceeds(tmp_path): replayer = Replayer( backend, vision=vision, - effect_verifier=RestRecordVerifier(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), poll_interval_s=0.01, ) report = replayer.run(workflow, bundle_dir=bundle, run_dir=run_dir) @@ -204,7 +206,9 @@ def test_effect_refuted_halts_despite_green_screen(tmp_path): replayer = Replayer( backend, vision=vision, - effect_verifier=RestRecordVerifier(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), poll_interval_s=0.01, ) report = replayer.run(workflow, bundle_dir=bundle, run_dir=run_dir) @@ -251,8 +255,12 @@ def test_effect_duplicate_irreversible_reconciles_as_rolled_back(tmp_path): replayer = Replayer( backend, vision=vision, - effect_verifier=RestRecordVerifier(url), - effect_compensator=RestCompensator(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), + effect_compensator=RestCompensator( + url, delete_path_template="/api/encounter/{id}" + ), poll_interval_s=0.01, ) report = replayer.run(workflow, bundle_dir=bundle, run_dir=run_dir) @@ -295,7 +303,9 @@ def test_effect_duplicate_irreversible_without_compensator_halts(tmp_path): replayer = Replayer( backend, vision=vision, - effect_verifier=RestRecordVerifier(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), # no compensator configured poll_interval_s=0.01, ) @@ -487,7 +497,9 @@ def test_effect_evidence_records_a_loopback_verifier_endpoint(tmp_path): report = Replayer( WritingBackend(url), vision=_vision_that_confirms_saved(), - effect_verifier=RestRecordVerifier(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), poll_interval_s=0.01, ).run(workflow, bundle_dir=bundle, run_dir=run_dir) @@ -503,15 +515,21 @@ def test_effect_evidence_records_a_loopback_verifier_endpoint(tmp_path): def test_effect_evidence_marks_a_remote_verifier_endpoint_external(): from openadapt_flow.runtime.replayer import effect_endpoint_scope - assert effect_endpoint_scope(RestRecordVerifier("https://emr.example.com")) == ( - "external" - ) - assert effect_endpoint_scope(RestRecordVerifier("http://[::1]:8080")) == ( - "loopback" - ) - assert effect_endpoint_scope(RestRecordVerifier("http://localhost:8080")) == ( - "loopback" - ) + assert effect_endpoint_scope( + RestRecordVerifier( + "https://emr.example.com", records_path="/api/db", records_key="records" + ) + ) == ("external") + assert effect_endpoint_scope( + RestRecordVerifier( + "http://[::1]:8080", records_path="/api/db", records_key="records" + ) + ) == ("loopback") + assert effect_endpoint_scope( + RestRecordVerifier( + "http://localhost:8080", records_path="/api/db", records_key="records" + ) + ) == ("loopback") assert effect_endpoint_scope(object()) == "unknown" diff --git a/packages/flow/tests/test_rest_verifier_transport.py b/packages/flow/tests/test_rest_verifier_transport.py index 00376c0cd3..d8a0bc623f 100644 --- a/packages/flow/tests/test_rest_verifier_transport.py +++ b/packages/flow/tests/test_rest_verifier_transport.py @@ -36,13 +36,20 @@ def test_session_falls_back_to_httpx_without_requests( import httpx _hide(monkeypatch, "requests") - session = RestRecordVerifier("http://127.0.0.1:1")._get_session() + session = RestRecordVerifier( + "http://127.0.0.1:1", records_path="/api/db", records_key="records" + )._get_session() assert isinstance(session, httpx.Client) def test_injected_session_is_always_preferred() -> None: sentinel = object() - verifier = RestRecordVerifier("http://127.0.0.1:1", session=sentinel) + verifier = RestRecordVerifier( + "http://127.0.0.1:1", + records_path="/api/db", + records_key="records", + session=sentinel, + ) assert verifier._get_session() is sentinel @@ -57,7 +64,9 @@ def test_records_are_read_over_the_fallback_transport( base_url, db, stop = serve() try: db.add("p1", "Triage", "note", key="k1") - state = RestRecordVerifier(base_url).capture_pre_state() + state = RestRecordVerifier( + base_url, records_path="/api/db", records_key="records" + ).capture_pre_state() assert state.reachable is True assert [record["patient_id"] for record in state.records] == ["p1"] finally: diff --git a/packages/flow/tests/test_reward_trust_boundary.py b/packages/flow/tests/test_reward_trust_boundary.py index 328887651d..bf3be7e4ec 100644 --- a/packages/flow/tests/test_reward_trust_boundary.py +++ b/packages/flow/tests/test_reward_trust_boundary.py @@ -531,3 +531,16 @@ def test_the_calibration_records_which_corpus_it_ran_on( calibration["calibration_corpus_digest"] == contract["certificate_policy"]["calibration_corpus_digest"] ) + + +def test_a_rest_recipe_names_its_records_path() -> None: + """No app's records path is assumed for a REST oracle.""" + with pytest.raises(ValueError, match="records_path"): + OracleRecipeV1(kind="rest", base_url="http://app.local") + recipe = OracleRecipeV1( + kind="rest", + base_url="http://app.local", + records_path="/api/resource/Loan Application", + records_key="data", + ) + assert recipe.records_path == "/api/resource/Loan Application" diff --git a/packages/flow/tests/test_rollback.py b/packages/flow/tests/test_rollback.py index b6d13d2990..acc067bd61 100644 --- a/packages/flow/tests/test_rollback.py +++ b/packages/flow/tests/test_rollback.py @@ -208,7 +208,9 @@ def _mockmed_replayer(url: str) -> Replayer: return Replayer( MockMedBackend(url), vision=_vision(), - effect_verifier=RestRecordVerifier(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), api_actuator=ApiActuator(url), poll_interval_s=0.01, ) @@ -220,7 +222,7 @@ def _run_with_rollback( bundle = tmp_path / "bundle" workflow.save(bundle) workflow = Workflow.load(bundle) - verifier = RestRecordVerifier(url) + verifier = RestRecordVerifier(url, records_path="/api/db", records_key="records") from openadapt_flow.after_stop import records_reader plan = RollbackPlan( @@ -1774,8 +1776,9 @@ def test_a_rollback_that_isnt_back_is_always_reconciliation(tmp_path): def test_the_apps_own_logged_bookkeeping_is_accounted_for(): - # Round 9, as the frame decides: an insert naming no record (none, or - # OpenEMR's patient 0) in a bookkeeping table, and the acting user's own + # Round 9, as the frame decides: an insert naming no record (none, or a + # value the app profile lists, such as OpenEMR's patient 0) in a + # bookkeeping table, and the acting user's own # per-user rows, are the app's; a write to a disclosure log isn't. from openadapt_flow.runtime.effects.frame import FrameAttribution @@ -1804,10 +1807,14 @@ def entry(event, table, record): ] assert ( attributor.unaccounted_writes( - [], window, {"uuid_registry"}, {"recent_patients"} + [], window, {"uuid_registry"}, {"recent_patients"}, {"0"} ) == [] ) + # Without that profile value, record 0 is a record like any other. + assert attributor.unaccounted_writes( + [], window, {"uuid_registry"}, {"recent_patients"} + ) == [window[0]] hidden = [entry("replace", "extended_log", "4")] assert ( attributor.unaccounted_writes( diff --git a/packages/flow/tests/test_rollback_qualify.py b/packages/flow/tests/test_rollback_qualify.py index e55515ac70..f3f4ca8818 100644 --- a/packages/flow/tests/test_rollback_qualify.py +++ b/packages/flow/tests/test_rollback_qualify.py @@ -55,7 +55,9 @@ def test_matrix_qualifies_a_compensation_for_every_save_on_mockmed(tmp_path, moc Workflow.load(bundle), bundle_dir=bundle, target=target, - reader=VerifierReader(RestRecordVerifier(mockmed)), + reader=VerifierReader( + RestRecordVerifier(mockmed, records_path="/api/db", records_key="records") + ), sources=Sources(network_lines=_network()), work_dir=tmp_path / "work", allow_delete=True, diff --git a/packages/flow/tests/test_run_gate.py b/packages/flow/tests/test_run_gate.py index 83be1a2e2b..39ad39aefe 100644 --- a/packages/flow/tests/test_run_gate.py +++ b/packages/flow/tests/test_run_gate.py @@ -206,7 +206,7 @@ def _deployment( *, verifier: bool = True, policy: str = "clinical-write" ) -> DeploymentConfig: effects = ( - EffectsConfig(kind="rest", base_url="http://sor.local") + EffectsConfig(kind="rest", records_path="/api/db", base_url="http://sor.local") if verifier else EffectsConfig(kind="none") ) @@ -810,6 +810,8 @@ def test_cli_run_dry_run_reports_without_executing(tmp_path, monkeypatch, capsys "clinical-write", "--effects-kind", "rest", + "--effects-records-path", + "/api/db", "--effects-base-url", "http://sor.local", "--dry-run", @@ -953,6 +955,8 @@ def capture(args): str(tmp_path / "case-run"), "--effects-kind", "rest", + "--effects-records-path", + "/api/db", "--effects-base-url", "http://sor.local", "--backend", @@ -1022,6 +1026,8 @@ def test_qualification_run_case_tightens_owned_legacy_state_root(tmp_path, monke str(tmp_path / "case-run"), "--effects-kind", "rest", + "--effects-records-path", + "/api/db", "--effects-base-url", "http://sor.local", "--backend", @@ -1063,6 +1069,8 @@ def test_qualification_run_case_refuses_changed_inputs_and_duplicate_attempt( str(tmp_path / "case-run"), "--effects-kind", "rest", + "--effects-records-path", + "/api/db", "--effects-base-url", "http://sor.local", "--backend", @@ -1144,6 +1152,8 @@ def run(self, *_args, **_kwargs): "http://127.0.0.1:5001", "--effects-kind", "rest", + "--effects-records-path", + "/api/db", "--effects-base-url", "http://sor.local", ] @@ -1471,6 +1481,8 @@ def test_cli_run_encryption_key_required_for_encrypted_bundle( "clinical-write", "--effects-kind", "rest", + "--effects-records-path", + "/api/db", "--effects-base-url", "http://sor.local", "--allow-unencrypted", # isolate the KEY-at-load behavior from gate 6 diff --git a/packages/flow/tests/test_scaffold_verifier.py b/packages/flow/tests/test_scaffold_verifier.py index cff4078781..72b84b6c26 100644 --- a/packages/flow/tests/test_scaffold_verifier.py +++ b/packages/flow/tests/test_scaffold_verifier.py @@ -175,13 +175,19 @@ def test_recording_candidates_split_identity_and_payload( self, tmp_path: Path ) -> None: rec = _recording_dir(tmp_path) - candidates = candidates_from_recording(rec) + # The demo app keeps its at-most-once key in "key". + candidates = candidates_from_recording(rec, idempotency_key_field="key") assert [candidate.step_id for candidate in candidates] == ["step_001"] candidate = candidates[0] assert candidate.observed_delta assert candidate.match == {"patient_id": "p2", "type": "Triage"} assert candidate.payload == {"note": "Synthetic follow-up in two weeks"} assert candidate.idempotency == ("key", "demo-key-1") + # None: no field is taken as the key, so it stays an identity field + # for a person to review. + (plain,) = candidates_from_recording(rec, idempotency_key_field=None) + assert plain.idempotency is None + assert plain.match["key"] == "demo-key-1" def test_write_draft_from_recording_is_todo_marked(self, tmp_path: Path) -> None: rec = _recording_dir(tmp_path) diff --git a/packages/flow/tests/test_sealed_at_rest.py b/packages/flow/tests/test_sealed_at_rest.py index d6cc063835..138d716a28 100644 --- a/packages/flow/tests/test_sealed_at_rest.py +++ b/packages/flow/tests/test_sealed_at_rest.py @@ -547,7 +547,9 @@ def test_halted_sealed_run_is_undone_without_plaintext( report = Replayer( MockMedBackend(url), vision=_vision(), - effect_verifier=RestRecordVerifier(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), poll_interval_s=0.01, ).run(workflow, bundle_dir=bundle, run_dir=run_dir) assert report.execution_outcome == "HALTED" diff --git a/packages/flow/tests/test_self_repair_cli.py b/packages/flow/tests/test_self_repair_cli.py index 143e7fe38c..8e2eda0c8c 100644 --- a/packages/flow/tests/test_self_repair_cli.py +++ b/packages/flow/tests/test_self_repair_cli.py @@ -57,6 +57,7 @@ def _config(tmp_path: Path, base_url: str) -> Path: config = tmp_path / "deploy.yaml" config.write_text( f"effects:\n kind: rest\n base_url: {base_url}\n" + " records_path: /api/db\n records_key: records\n" "policy:\n policy: clinical-write\n" + approvers_yaml() ) return config diff --git a/packages/flow/tests/test_smart_backend_auth.py b/packages/flow/tests/test_smart_backend_auth.py index 3bcbe2235a..11535daf9c 100644 --- a/packages/flow/tests/test_smart_backend_auth.py +++ b/packages/flow/tests/test_smart_backend_auth.py @@ -9,6 +9,8 @@ from __future__ import annotations +from openadapt_flow.runtime.effects.fhir import DEFAULT_OBSERVATION_PATHS + import base64 import json import logging @@ -282,6 +284,7 @@ def _fhir_cfg(server: Server, smart: dict) -> EffectsConfig: return EffectsConfig.model_validate( { "kind": "fhir", + "field_paths": dict(DEFAULT_OBSERVATION_PATHS), "base_url": server.fhir, "resource_type": "Observation", "timeout_s": 2.0, @@ -542,12 +545,19 @@ def test_bad_smart_config_is_refused(overrides, message): def test_smart_backend_needs_fhir_and_no_static_token(): with pytest.raises(ValueError, match="only to kind fhir"): EffectsConfig.model_validate( - {"kind": "rest", "base_url": "https://x.test", "smart_backend": _smart()} + { + "kind": "rest", + "records_path": "/api/db", + "base_url": "https://x.test", + "smart_backend": _smart(), + } ) with pytest.raises(ValueError, match="mutually exclusive"): EffectsConfig.model_validate( { "kind": "fhir", + "resource_type": "Observation", + "field_paths": dict(DEFAULT_OBSERVATION_PATHS), "base_url": "https://x.test/fhir", "access_token_env": "TOKEN", "smart_backend": _smart(), @@ -614,7 +624,13 @@ def test_static_token_and_token_source_are_exclusive(rsa_key): token_url="https://auth.example.test/token", ) with pytest.raises(ValueError, match="not both"): - FhirEffectVerifier("https://x.test/fhir", access_token="t", token_source=source) + FhirEffectVerifier( + "https://x.test/fhir", + resource_type="Observation", + field_paths=dict(DEFAULT_OBSERVATION_PATHS), + access_token="t", + token_source=source, + ) @pytest.mark.parametrize( diff --git a/packages/flow/tests/test_ui_readback_verifier.py b/packages/flow/tests/test_ui_readback_verifier.py index 70070f3d9c..8f594530b4 100644 --- a/packages/flow/tests/test_ui_readback_verifier.py +++ b/packages/flow/tests/test_ui_readback_verifier.py @@ -432,8 +432,10 @@ def test_existing_verifier_identities_do_not_change() -> None: # Digests computed with origin/main (before ``ui_readback`` existed). The # new optional field must not enter the identity of any verifier that # does not use it, or every issued admission bound to one would break. - rest = RestRecordVerifier("http://sor.test") - cfg = EffectsConfig(kind="rest", base_url="http://sor.test") + rest = RestRecordVerifier( + "http://sor.test", records_path="/api/db", records_key="records" + ) + cfg = EffectsConfig(kind="rest", records_path="/api/db", base_url="http://sor.test") assert _effect_verifier_config_identity(cfg, rest) == ( "sha256:9f288a5b15b66001565c6d5bbd0f0a09bf92485280fe5de37b421802c3e0e663" ) diff --git a/packages/flow/tests/test_undo_proposal.py b/packages/flow/tests/test_undo_proposal.py index a7f4f01a40..f06fcda719 100644 --- a/packages/flow/tests/test_undo_proposal.py +++ b/packages/flow/tests/test_undo_proposal.py @@ -178,7 +178,9 @@ def run(self, workflow, *, bundle_dir, run_dir, params): replayer = Replayer( SavingBackend(self.url), vision=_vision(), - effect_verifier=RestRecordVerifier(self.url), + effect_verifier=RestRecordVerifier( + self.url, records_path="/api/db", records_key="records" + ), api_actuator=ApiActuator(self.url), poll_interval_s=0.01, ) @@ -255,7 +257,9 @@ def test_network_undo_is_verified_written_as_a_proposal_and_waits_for_a_person( stopping = Replayer( _RejectSecond(mockmed), vision=_vision(), - effect_verifier=RestRecordVerifier(mockmed), + effect_verifier=RestRecordVerifier( + mockmed, records_path="/api/db", records_key="records" + ), poll_interval_s=0.01, ) stopped = stopping.run(run_wf, bundle_dir=bundle, run_dir=run_dir) @@ -323,7 +327,9 @@ def _undo_replayer(url: str) -> Replayer: return Replayer( SavingBackend(url), vision=_vision(), - effect_verifier=RestRecordVerifier(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), api_actuator=ApiActuator(url), poll_interval_s=0.01, ) @@ -581,7 +587,9 @@ def run(self, workflow, *, bundle_dir, run_dir, params): replayer = Replayer( EditingBackend(self.url, also_touch_p2=self.also_touch_p2), vision=_vision(), - effect_verifier=RestRecordVerifier(self.url), + effect_verifier=RestRecordVerifier( + self.url, records_path="/api/db", records_key="records" + ), poll_interval_s=0.01, ) return replayer.run( @@ -897,7 +905,7 @@ def test_deployment_test_copy_resets_and_reads_every_record(tmp_path, mockmed): write_bundle( tmp_path / "contract", contract_id="undo_copy", - oracle={"kind": "rest", "base_url": mockmed}, + oracle={"kind": "rest", "records_path": "/api/db", "base_url": mockmed}, channel="api", key=Ed25519PrivateKey.generate(), issuer_key_id="self_signed:test", @@ -917,6 +925,7 @@ def test_deployment_test_copy_resets_and_reads_every_record(tmp_path, mockmed): "name: copy\n" f"backend:\n kind: web\n url: {mockmed}/\n" f"effects:\n kind: rest\n base_url: {mockmed}\n" + " records_path: /api/db\n" "test_copy:\n declared: true\n" f" reset_url: {mockmed}/api/reset\n" " reward_contract: contract\n" diff --git a/packages/flow/tests/test_variant_router.py b/packages/flow/tests/test_variant_router.py index 3451656bd3..a98941ef31 100644 --- a/packages/flow/tests/test_variant_router.py +++ b/packages/flow/tests/test_variant_router.py @@ -513,7 +513,9 @@ def child_run( report = Replayer( _RowWritingBackend(url), vision=_vision_confirms_saved(), - effect_verifier=RestRecordVerifier(url), + effect_verifier=RestRecordVerifier( + url, records_path="/api/db", records_key="records" + ), poll_interval_s=0.01, ).run( workflow, diff --git a/packages/flow/tests/test_verifier_adapter_platform.py b/packages/flow/tests/test_verifier_adapter_platform.py index ee1d680766..2fe631156e 100644 --- a/packages/flow/tests/test_verifier_adapter_platform.py +++ b/packages/flow/tests/test_verifier_adapter_platform.py @@ -11,6 +11,8 @@ from __future__ import annotations +from openadapt_flow.runtime.effects.fhir import DEFAULT_OBSERVATION_PATHS + import importlib.metadata from datetime import datetime, timedelta, timezone @@ -104,9 +106,15 @@ def reachable_state(records=(), substrate="test"): class TestAdapterInterface: def test_builtin_adapters_conform_to_protocol(self, tmp_path): adapters = [ - RestRecordVerifier("http://sor"), + RestRecordVerifier( + "http://sor", records_path="/api/db", records_key="records" + ), GraphQLRecordVerifier("http://sor/graphql", query="query { r { id } }"), - FhirEffectVerifier("http://sor/fhir"), + FhirEffectVerifier( + "http://sor/fhir", + resource_type="Observation", + field_paths=dict(DEFAULT_OBSERVATION_PATHS), + ), SqlRecordVerifier(lambda: None, "SELECT id FROM t"), FileArrivalVerifier(str(tmp_path)), MaildirDeliveryVerifier(str(tmp_path)), @@ -127,7 +135,12 @@ def test_screen_readback_is_demoted_not_independent_proof(self): assert onscreen.verification_tier is VerificationTier.IMMEDIATE_SCREEN assert confidence_label(onscreen.verification_tier) == "screen-consistency" # Every system-of-record adapter IS independent proof. - assert RestRecordVerifier("http://sor").independent_system_of_record is True + assert ( + RestRecordVerifier( + "http://sor", records_path="/api/db", records_key="records" + ).independent_system_of_record + is True + ) def test_confidence_labels_cover_every_tier(self): labels = {confidence_label(tier) for tier in VerificationTier} @@ -140,7 +153,9 @@ def test_confidence_labels_cover_every_tier(self): def test_test_connection_ok(self): session = FakeSession([FakeResponse(200, {"records": []})]) - verifier = RestRecordVerifier("http://sor", session=session) + verifier = RestRecordVerifier( + "http://sor", records_path="/api/db", records_key="records", session=session + ) probe = verifier.test_connection() assert isinstance(probe, ConnectionProbe) assert probe.ok is True @@ -148,7 +163,9 @@ def test_test_connection_ok(self): def test_test_connection_credential_failure_not_ok(self): session = FakeSession([FakeResponse(401, {"error": "bad token"})]) - probe = RestRecordVerifier("http://sor", session=session).test_connection() + probe = RestRecordVerifier( + "http://sor", records_path="/api/db", records_key="records", session=session + ).test_connection() assert probe.ok is False def test_test_connection_never_raises(self): @@ -169,7 +186,9 @@ def test_capture_post_state_defaults_to_fresh_snapshot(self): FakeResponse(200, {"records": [{"id": 1}]}), ] ) - verifier = RestRecordVerifier("http://sor", session=session) + verifier = RestRecordVerifier( + "http://sor", records_path="/api/db", records_key="records", session=session + ) before = verifier.capture_pre_state() after = verifier.capture_post_state() assert before.records == [] @@ -244,7 +263,9 @@ def test_no_non_confirmed_result_maps_to_a_pass(self): def test_unreachable_sor_classifies_unavailable(self): session = FakeSession([FakeResponse(500)]) - verifier = RestRecordVerifier("http://sor", session=session) + verifier = RestRecordVerifier( + "http://sor", records_path="/api/db", records_key="records", session=session + ) before = verifier.capture_pre_state() verdict = verifier.verify(make_effect(), before) assert verdict.unavailable is True @@ -534,6 +555,6 @@ def test_builtin_kinds_are_not_shadowed_by_plugins(self, clean_registry, tmp_pat # first inside build_effect_verifier. register_verifier_factory("rest", build_csv_ledger_verifier) verifier = build_effect_verifier( - EffectsConfig(kind="rest", base_url="http://sor") + EffectsConfig(kind="rest", records_path="/api/db", base_url="http://sor") ) assert isinstance(verifier, RestRecordVerifier) diff --git a/packages/flow/tests/test_verifier_api.py b/packages/flow/tests/test_verifier_api.py index e3383a848e..0ce75a1f79 100644 --- a/packages/flow/tests/test_verifier_api.py +++ b/packages/flow/tests/test_verifier_api.py @@ -409,3 +409,72 @@ def test_a_pending_write_repr_shows_no_values(tmp_path: Path) -> None: text = repr(pending) assert "Ada" not in text and "example.test" not in text pending.release() + + +#: Contract hashes as openadapt-flow 2.5.0 computed them. A field that lost +#: its default (records_path, resource_type) must not move them: the write +#: ledger and reward certificates are keyed on these. +_PINNED_CONTRACT_HASHES = { + "fhir": { + "digest": "91c4a3ebe007fb29ff109ab420e004c69d18344776d91b833275cafa0764fd20", + "identity": "5035aab8eb04079931ced609318a886d463b270031edb967650e3966206919e3", + "store": "20d025f2007789ef2271b5cef933e38fd47d4114d559fc820aca9283d6d827fd", + }, + "rest": { + "digest": "5ab1d6ebc5bae6c5fba13a3f2ce60e2b65292c2126e6f858ef95190ee4b1104f", + "identity": "f287cf9ea6af74fd36dba0e42ae64a3024518c4c45ac7e063a1b4fee7dd2e678", + "store": "32521d4b64cce55ed24443f4d35ec1e73f472cd88ebc4d57e6a92d06161611a7", + }, + "sql": { + "digest": "45ea6035db9bf63b39e7bf31db07dab820e42a9a218ba39906ea54e283cb5b22", + "identity": "60cf99eaa0beed0335bff2e85e23ac1a29b90882075d728eb8952a71305b52a2", + "store": "f333364923dd35c0e086046de0494ee84df7c766bc5c9a53209e58cad4594a9c", + }, +} + + +def test_contract_hashes_do_not_change() -> None: + record = { + "name": "patient", + "match": ["email", "lname"], + "fields": {"fname": "fname", "lname": "lname", "email": "email"}, + } + verifiers = { + "sql": { + "kind": "sql", + "sqlite_database": "/data/app.db", + "sql_query": "SELECT * FROM patient", + }, + "rest": { + "kind": "rest", + "base_url": "http://app.local", + "records_path": "/api/db", + "records_key": "records", + }, + "fhir": { + "kind": "fhir", + "base_url": "https://fhir.local/r4", + "resource_type": "Observation", + "field_paths": { + "id": "id", + "email": "email", + "lname": "lname", + "fname": "fname", + }, + }, + } + for kind, verifier in verifiers.items(): + contract = Contract.from_dict( + { + "schema_version": "openadapt.verify/v1", + "verifier": verifier, + "record": record, + "frame": {"identity_field": "id"}, + } + ) + got = { + "digest": contract.digest, + "store": contract.store, + "identity": contract.identity, + } + assert got == _PINNED_CONTRACT_HASHES[kind], kind diff --git a/packages/flow/tests/test_windows_attempt_ledger.py b/packages/flow/tests/test_windows_attempt_ledger.py index ca01da449a..b217227ca6 100644 --- a/packages/flow/tests/test_windows_attempt_ledger.py +++ b/packages/flow/tests/test_windows_attempt_ledger.py @@ -134,6 +134,8 @@ def _run_case(main, bundle, inputs_path, run_dir): "rest", "--effects-base-url", "http://sor.local", + "--effects-records-path", + "/api/db", "--backend", "web", ]