Skip to content

Tidy-up after #1542: amendment-number note, bridge docstrings, bridge contract test - #1779

Merged
abrichr merged 1 commit into
mainfrom
claude/tidy-1542-notes
Nov 3, 2026
Merged

abrichr merged 1 commit into
mainfrom
claude/tidy-1542-notes

Conversation

@abrichr

@abrichr abrichr commented Nov 3, 2026

Copy link
Copy Markdown
Member

Non-blocking notes from the delta review of #1542. No behavior changes.

Changes

  • PROTOCOL.md: the renumbering note was inside the body of pre-registered Amendment 11. It is now its own short note before the amendments. It says old Amendment N is now N+1, and it names the three files that still use the old numbers: the two 2026-10-25-rb* run meta.json files and HAND_REVIEW.md. Those files are not edited. No amendment's rules changed.
  • BrowserTeachBridge.hold_needed and VerifyBridge.hold_needed: the docstrings said the tool could be teach_run. Neither bridge offers it. They now say the bridge has no person holds and tool is ignored.
  • BrowserTeachBridge class docstring: "none can change data" was stale since write contracts (feat: verified writes for any agent, and taught browser writes with receipts #1589). It now says that without a contract nothing can change data, and with one the wire check lets only the contract's writes out.
  • MCP server instructions: the fallback for a teach bridge without its own instructions now names teach_run (web apps) next to teach_act.
  • New tests/test_mcp_teach_bridge_contract.py: builds each real teach bridge (TeachBridge, BrowserTeachBridge, VerifyBridge) into build_server, calls teach_act and teach_run through an in-memory MCP client, and checks that hold_needed gets tool= with no internal failure. It also binds the signature for both tool names. With tool removed from BrowserTeachBridge.hold_needed, the test fails.

Finding: volatile fields in the read-back

New tests/test_readback_volatile_fields.py runs RecordReadBack on the real load_observer path against a local FHIR server whose resources get a new meta.lastUpdated and meta.versionId on every read.

  • With the default field_paths, meta fields are not mapped, so the read-back never sees them. The save reads back as landed.
  • If an operator maps meta.lastUpdated or meta.versionId in field_paths, run 1 stops as wrong_record, although only a read changed the starting record. This fails closed, but it is a false stop. records_drop_fields removes the field, and the save then reads back as landed.
  • db_snapshot is not a read-back source. load_observer has no kind for it; only the contract frame uses it, and it has its own ignore_columns. So the read-back can't give a false stop there.

Follow-up: the read-back can give a false wrong_record on FHIR when a mapped field changes on every read. A possible fix is to refuse, or warn about, meta.* paths in a run-1 verifier unless they are in records_drop_fields. Nothing in this PR changes that behavior.

Tests

pytest -n 2 on the two new files plus test_verify_tools.py, test_browser_teach.py and test_mcp_teach_cli.py: 44 passed. Ruff check and format pass on the changed files.

@abrichr

abrichr commented Nov 3, 2026

Copy link
Copy Markdown
Member Author

Coordinator review: diff checked. Engine changes are docstrings and MCP fallback text only (browser_teach.py, mcp.py, verify_tools.py); the rest is the PROTOCOL renumbering note and two new tests (bridge-contract build test, read-back volatile-field test). No behaviour change. CI green. Approve and merge.

@abrichr
abrichr merged commit fa15e1e into main Nov 3, 2026
45 checks passed
@abrichr
abrichr deleted the claude/tidy-1542-notes branch November 3, 2026 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant