From 9e2d62d4ebf0d95131874446e911a4b6bda5f6b9 Mon Sep 17 00:00:00 2001 From: abrichr Date: Sat, 31 Oct 2026 13:43:02 -0400 Subject: [PATCH 1/7] Generator refuses joins and pins the demonstrations can't tell apart Equal ids on a fresh install were taken as joins, and a value one write happened to store was pinned: - a row's own primary key is never a key to another table, and a new row's own key never links it to its record, without a declared foreign key; - a value is never placed in the column a path arrived by; - a list item's link must hold the record's key in every item row, and two that always agree are refused unless a declared foreign key picks one; - a place through a key only equal values support, with a supported rival that fits every demonstration, becomes a locations question; - a pin needs two rows that agree; one row is listed in the bookkeeping question, left out of the proposal, and a field set that leaves such a column open gets no contract; - child rows see the acting user, and a row beside an update may link to the target instead of pinning a demonstration's id. --- packages/flow/docs/GENERATE_CONTRACTS.md | 34 ++- .../openadapt_flow/generate/demo_analysis.py | 48 +++- .../generate/verify_contract.py | 199 ++++++++++++-- .../flow/tests/test_generate_coincidences.py | 253 ++++++++++++++++++ .../tests/test_generate_verify_contract.py | 14 +- 5 files changed, 524 insertions(+), 24 deletions(-) create mode 100644 packages/flow/tests/test_generate_coincidences.py diff --git a/packages/flow/docs/GENERATE_CONTRACTS.md b/packages/flow/docs/GENERATE_CONTRACTS.md index b4d12cc9a..bf93bec0a 100644 --- a/packages/flow/docs/GENERATE_CONTRACTS.md +++ b/packages/flow/docs/GENERATE_CONTRACTS.md @@ -147,7 +147,26 @@ keeps, in this order: the table defines first when they hold the same value in every row; 3. the place another kind's demonstrations used for the same table. -If places are still tied, that is a question. A value that is also in a row +If places are still tied, that is a question. + +Equal ids are not a join. Demonstrations made on a fresh install often give +a new row the same id as an unrelated row, so: + +- A row's own primary key is never a key to another table, and a new row's + own key never links it to its record, unless a declared foreign key says + so. +- A value is never placed in the column a path arrived by (a child row's + link column holds the record's key by construction). +- A list item's link is the column that holds the record's key in every + item row of every demonstration. When two always agree and no declared + foreign key picks one, the list is refused until a demonstration tells + them apart. +- When the best place goes through a key only equal values support (no + declared foreign key, no column named after its table) and a place whose + keys are all supported fits every demonstration too, that is a question + (`locations.`), never a silent pick. + +A value that is also in a row the write added beside the record (a call's lead is its parent and an attendee row) is checked in both places. @@ -205,6 +224,19 @@ columns that vary or are time stamps, so accepting it is explicit, and leaves free text out: a person adds a free-text column to `unchecked` only on purpose. +A pin needs two rows that agree. A value seen beside one write only (a field +set one demonstration alone gives, a row only one demonstration added) is +pinned only when every row of that table the kind's demonstrations wrote +holds it. Otherwise the bookkeeping question lists it as one the +demonstrations can't tell, it is left out of the proposed answer, and the +kind has no contract until a person names it under `unchecked` or records a +demonstration where it differs. A field set whose own demonstrations leave +such a column open, while the kind's full set pins it, gets no contract +(`rejected` in `report.json`). A column that holds the acting user's id in +every row is the actor, even when it also equals a column of a row it points +to. A row the app adds beside an update may hold the target's key: that is a +link to the record, never a pinned id. + **Rows the app adds on its own.** A row type the app added the same number of times in every demonstration (an agenda event, a stream note, a price-history row) is declared with that count and its constant columns. A table whose diff --git a/packages/flow/openadapt_flow/generate/demo_analysis.py b/packages/flow/openadapt_flow/generate/demo_analysis.py index 3f54acd89..a071b9f92 100644 --- a/packages/flow/openadapt_flow/generate/demo_analysis.py +++ b/packages/flow/openadapt_flow/generate/demo_analysis.py @@ -376,9 +376,19 @@ def locate( def walk( t: Table, r: Mapping[str, Optional[str]], hops: tuple[Hop, ...], depth: int ) -> None: + # The column a hop arrived by holds the other row's key by + # construction (a child's link, the key an fk names): a value equal + # to it there can't be told from the link itself. + via = ( + (hops[-1].column if hops[-1].kind == "child" else hops[-1].ref_column) + if hops + else None + ) for cols, rule in _display_matches(t, r, value): if depth == 0 and changed_only and not (set(cols) & changed): continue + if via is not None and via in cols: + continue found.append(Location(hops, cols, rule)) if depth >= max_hops: return @@ -389,6 +399,10 @@ def walk( if depth == 0 and changed_only and col.name not in changed: continue for target, ref, declared in _fk_targets(schema, idx, t, col.name, v): + if col.name in t.primary_key and not declared: + # A row's own key names that row: equal to another + # table's key only by chance, unless a foreign key says so. + continue target_row = idx.row(target, v) if target_row is None or (target == anchor.table and hops): continue # back to the main record: not a location @@ -426,6 +440,9 @@ def walk( ) if not declared and not _IDLIKE.search(col.name): continue + if not declared and col.name in child.primary_key: + continue # the child's own key: equal by chance + walk( child, ch.after, @@ -462,11 +479,26 @@ def link_columns( for ch in demo.diff.changes: if ch.kind != "added" or ch.table == anchor.table or ch.after is None: continue - cols = {c for c, v in ch.after.items() if v == key} + cols = { + c + for c, v in ch.after.items() + if v == key and is_link_column(schema, ch.table, c, anchor.table) + } out[ch.table] = out[ch.table] & cols if ch.table in out else cols return out +def is_link_column(schema: Schema, table: str, column: str, parent: str) -> bool: + """Can ``table.column`` link a row to a ``parent`` row? Not the row's own + primary key, unless a foreign key declares it (a one-to-one extension + table): a new row's key equals its parent's key only by chance.""" + + t = schema.tables.get(table) + if t is None or column not in t.primary_key: + return True + return any(fk.column == column and fk.table == parent for fk in t.foreign_keys) + + def best(locations: Sequence[Location]) -> tuple[Optional[Location], list[Location]]: """The single best location, or None and the tied ones.""" @@ -584,12 +616,19 @@ def column_roles( ): out[name] = ColumnRole("timestamp") continue + # One row can't tell a pinned value from what that one write + # happened to store (a reference number, a currency, an id equal to + # the bot's): never pinned from one row. + one = len(rows) < 2 actor = [ p for p, av in actor_values.items() if values and all(v is not None and v == av for v in values) ] - if actor: + if actor and not one: + # The acting user's id in every row: the person declared it + # (``--actor-param``), so it wins over a copy of another row's + # column that happens to hold the same id. out[name] = ColumnRole("actor", detail={"param": actor[0]}) continue links = link_values or [set() for _ in rows] @@ -638,6 +677,11 @@ def column_roles( ) out[name] = ColumnRole("free", detail={"empty": empty}) continue + if one: + out[name] = ColumnRole( + "varies", detail={"why": "seen in one demonstration only"} + ) + continue if len(distinct) == 1: v = values[0] default = col.default diff --git a/packages/flow/openadapt_flow/generate/verify_contract.py b/packages/flow/openadapt_flow/generate/verify_contract.py index 2b472d69d..38292ab5d 100644 --- a/packages/flow/openadapt_flow/generate/verify_contract.py +++ b/packages/flow/openadapt_flow/generate/verify_contract.py @@ -56,6 +56,7 @@ best, column_roles, is_identifier, + is_link_column, link_columns, locate, pick_anchor, @@ -71,6 +72,13 @@ DEFAULT_RECHECK_S = 300 +class Unpinned(ValueError): + """A column of a new row that no rule checks, in one field set only (seen + in one demonstration, or two explanations the demonstrations can't tell + apart), and that the answer doesn't name under ``unchecked``: that field + set gets no contract.""" + + class GenerateError(RuntimeError): """The generator can't go on; the message says why.""" @@ -268,6 +276,8 @@ class KindPlan: #: More places a value is in: rows the write added beside the record #: (a call's lead is its parent and also an attendee row). extra: dict[str, list[Location]] = field(default_factory=dict) + #: The acting user's ids (``--actor-param``), for the roles of new rows. + actor_values: dict[str, str] = field(default_factory=dict) def _scalars(values: Mapping[str, Any]) -> dict[str, Any]: @@ -299,6 +309,7 @@ def analyze( actor_values: Mapping[str, str], ) -> KindPlan: plan = KindPlan(spec.kind, params=list(spec.parameters)) + plan.actor_values = dict(actor_values) mine = [d for d in demos if d.kind == spec.kind and d.diff.changes] if not mine: plan.refused.append( @@ -392,6 +403,17 @@ def analyze( continue all_found = _intersect(found[p]) loc, tied = _pick(p, all_found, plan.demos, schema) + if ( + loc is not None + and _by_value_only(loc) + and any(x != loc and not _by_value_only(x) for x in all_found) + ): + # The best place reaches the value through a key that only equal + # values support (no declared foreign key, no column named after + # its table), and a place whose every key is supported fits every + # demonstration too: the demonstrations can't tell a real join + # from ids that happened to be equal. A person picks. + loc, tied = None, sorted(all_found, key=_rank) if loc is not None: plan.extra[p] = [ x @@ -429,10 +451,14 @@ def analyze( for c in loc.columns } for name, tied in list(plan.unresolved.items()): + # A place one key away through a column another value already + # holds is that value's row, not this one's. A longer path through + # it (a customer behind the project) still counts. left = [ loc for loc in tied - if (loc.hops[0].column if loc.hops else loc.columns[0]) not in claimed + if len(loc.hops) > 1 + or (loc.hops[0].column if loc.hops else loc.columns[0]) not in claimed ] if len(left) == 1 and name in plan.params: plan.locations[name] = left[0] @@ -466,6 +492,13 @@ def analyze( return plan +def _by_value_only(loc: Location) -> bool: + """Does the place go through a hop that only a value match supports (no + declared foreign key, no column named after the table)?""" + + return any(_hop_cost(h) >= 2 for h in loc.hops) + + def _share_locations( schema: Schema, plans: Sequence[KindPlan], kinds: Sequence[KindSpec] ) -> None: @@ -741,11 +774,37 @@ def _children( ): co.append(q) explained |= {h[0].hops[0].column for h in hits} + roles = column_roles( + schema, loc.hops[0].table, rows, explained, plan.actor_values + ) + if len(rows) < 2: + # One demonstration gives this value. Every row of the table the + # kind's demonstrations added for their record is evidence for a + # column that holds the same in all of them (a journal's type). + linked = [ + (d, ch.after) + for d, a in zip(plan.demos, plan.anchors) + for ch in d.diff.of(loc.hops[0].table, "added") + if ch.after is not None + and ch.after.get(loc.hops[0].column) + == (a.after or {}).get(loc.hops[0].ref_column) + ] + if len(linked) >= 2: + wide = column_roles( + schema, loc.hops[0].table, linked, explained, plan.actor_values + ) + for col, role in roles.items(): + if role.detail.get( + "why" + ) == "seen in one demonstration only" and wide.get( + col, role + ).role in ("constant", "actor", "copied"): + roles[col] = wide[col] group = ChildGroup( loc.hops[0].table, loc.hops[0], {p: rel}, - column_roles(schema, loc.hops[0].table, rows, explained, {}), + roles, 1, co_owners=co, param=p, @@ -758,6 +817,8 @@ def _children( per_key: dict[str, list[list[Location]]] = {} table = link = None rows = [] + #: Columns that hold the record's key in every list row so far. + link_cols: Optional[set[str]] = None for d, a in zip(plan.demos, plan.anchors): items = d.values.get(p) if not isinstance(items, list): @@ -791,12 +852,18 @@ def _children( } if all(found_at.values()): used.add(i) - link_col = next(c for c, v in after.items() if v == key) + here = { + c + for c, v in after.items() + if v == key + and is_link_column(schema, ch.table, c, plan.anchor) + } + link_cols = here if link_cols is None else link_cols & here table = ch.table link = Hop( "child", ch.table, - link_col, + "", schema.tables[plan.anchor].primary_key[0], ) for k, ls in found_at.items(): @@ -808,6 +875,30 @@ def _children( if table is None or link is None: plan.unresolved.setdefault(p, []) continue + # The column that links an item to its record: the one that holds + # the record's key in every item row. Two that always agree (an + # item's own id equal to the record's) can't be told apart, unless + # exactly one is a declared foreign key to the record. + cols = sorted(link_cols or []) + declared = [ + c + for c in cols + if any( + fk.column == c and fk.table == plan.anchor + for fk in schema.tables[table].foreign_keys + ) + ] + if len(cols) != 1 and len(declared) == 1: + cols = declared + if len(cols) != 1: + plan.refused.append( + f"{p}: the demonstrations can't tell which column of {table} " + "links an item to its record (" + + (", ".join(cols) or "none holds the record's key in every item") + + "); record a demonstration where they differ" + ) + return + link = Hop("child", table, cols[0], link.ref_column) item_locs: dict[str, Location] = {} for k, per in per_key.items(): item_loc, tied = best(_intersect(per)) @@ -821,7 +912,7 @@ def _children( table, link, item_locs, - column_roles(schema, table, rows, explained, {}), + column_roles(schema, table, rows, explained, plan.actor_values), list_param=p, ) @@ -842,6 +933,19 @@ def _demo_groups(plan: KindPlan, d: Demo) -> list[ChildGroup]: return _all_groups(plan, dv) if dv is not None else list(plan.children.values()) +def _link_keys(schema: Schema, plan: KindPlan, d: Demo) -> set[str]: + """Keys a row added beside a write may link to: the rows that write + added, and its main record (an update's journal names the issue).""" + + keys = set(getattr(d, "_added_keys", set())) + if d in plan.demos: + a = plan.anchors[plan.demos.index(d)] + key = (a.after or a.before or {}).get(plan_pk(schema, plan.anchor)) + if key is not None: + keys.add(key) + return keys + + def _sides( schema: Schema, plan: KindPlan, @@ -913,8 +1017,30 @@ def _sides( for t, cs in sorted(counts.items()): rows = [r for group in pure.get(t, []) for r in group] if len(set(cs)) == 1 and cs[0] > 0 and rows: - links: list[set[str]] = [getattr(d, "_added_keys", set()) for d, _ in rows] + links: list[set[str]] = [_link_keys(schema, plan, d) for d, _ in rows] roles = column_roles(schema, t, rows, set(), actor_values, links) + if len(rows) < 2: + # Seen beside one write only (the other writes gave the + # value this row holds): every row the kind's writes added + # here is evidence for a column that holds the same in all. + wide = [ + (d, ch.after) + for d in plan.demos + for ch in d.diff.of(t, "added") + if ch.after is not None + ] + if len(wide) >= 2: + wide_links = [_link_keys(schema, plan, d) for d, _ in wide] + more = column_roles( + schema, t, wide, set(), actor_values, wide_links + ) + for col, role in roles.items(): + if role.detail.get( + "why" + ) == "seen in one demonstration only" and more.get( + col, role + ).role in ("constant", "actor", "copied", "link"): + roles[col] = more[col] plan.sides.append(SideSpec(t, cs[0], roles, unless=child_tables.get(t))) elif rows: # The number of added rows differs between demonstrations: a log @@ -2106,6 +2232,9 @@ def _pin( actor_params: Mapping[str, str], unchecked: Mapping[str, set[str]] = {}, ) -> None: + if role.role == "varies" and col not in unchecked.get(table, set()): + why = role.detail.get("why") or "it differs between demonstrations" + raise Unpinned(f"{table}.{col} is not checked ({why})") if role.role == "free" and col not in unchecked.get(table, set()): # Empty in every demonstration and not named as unchecked: it stays # empty. @@ -2266,6 +2395,16 @@ def _anchor_roles( v is not None and _TOKEN.match(v) for v in values ): roles[col] = role + elif ( + len(support) < 2 + and roles.get(col, role).detail.get("why") + == "seen in one demonstration only" + and role.role in ("constant", "actor", "copied") + ): + # One demonstration gives this field set, but every + # demonstration of the kind stored the same there: that + # agreement is the evidence for the pin. + roles[col] = role return roles @@ -2976,21 +3115,26 @@ def table_cols(key: str) -> dict[str, set[str]]: excused[t] = (took | columns_add.get(t, set())) - columns_remove.get(t, set()) # Columns of a new row no rule checks, per kind and table. open_by_kind: dict[str, dict[str, dict[str, str]]] = {} + #: Why a column can't be pinned, where the demonstrations can't tell + #: (one demonstration, or two explanations that agree). + why_open: dict[str, str] = {} for p in plans: own: dict[str, dict[str, str]] = {} + groups: list[tuple[str, dict[str, ColumnRole]]] = [] if p.mode == "create" and p.demos: for v in [Variant(p.kind, sorted(p.locations))]: - for col, role in _anchor_roles(schema, p, v, actor_params).items(): - if role.role in OPEN_ROLES: - own.setdefault(p.anchor, {})[col] = role.role - for g in [*p.children.values(), *p.lists.values()]: - for col, role in g.roles.items(): - if role.role in OPEN_ROLES: - own.setdefault(g.table, {})[col] = role.role - for sd in p.sides: - for col, role in sd.roles.items(): + groups.append((p.anchor, _anchor_roles(schema, p, v, actor_params))) + groups += [ + (g.table, g.roles) for g in [*p.children.values(), *p.lists.values()] + ] + groups += [(sd.table, sd.roles) for sd in p.sides] + for table, roles_of in groups: + for col, role in roles_of.items(): if role.role in OPEN_ROLES: - own.setdefault(sd.table, {})[col] = role.role + own.setdefault(table, {})[col] = role.role + why = str(role.detail.get("why") or "") + if "one demonstration" in why or "can't tell" in why: + why_open[f"{table}.{col}"] = why open_by_kind[p.kind] = own unchecked: dict[str, set[str]] = {} for own_cols in open_by_kind.values(): @@ -3000,7 +3144,7 @@ def table_cols(key: str) -> dict[str, set[str]]: for own_cols in open_by_kind.values(): for t, roles_of in own_cols.items(): for c, role_name in roles_of.items(): - if role_name != "free": + if role_name != "free" and f"{t}.{c}" not in why_open: proposed_open.setdefault(t, []) if c not in proposed_open[t]: proposed_open[t].append(c) @@ -3104,6 +3248,14 @@ def table_cols(key: str) -> dict[str, set[str]]: ) + "." ) + if why_open: + lines.append( + "The demonstrations can't tell what these hold, so they are not " + "pinned: " + + "; ".join(f"{c} ({w})" for c, w in sorted(why_open.items())) + + ". Record a demonstration where they differ, or name them " + "under unchecked." + ) questions.append( Question( "bookkeeping", @@ -3276,9 +3428,16 @@ def table_cols(key: str) -> dict[str, set[str]]: continue ok = True for flavor in flavors: - doc = build_contract( - schema, p, v, flavor, settings, excused, set(p.logs) - ) + try: + doc = build_contract( + schema, p, v, flavor, settings, excused, set(p.logs) + ) + except Unpinned as exc: + rep["rejected"].append( + {"variant": v.slug, "flavor": flavor, "why": str(exc)} + ) + ok = False + break rel = Path("contracts") / p.kind / f"{v.slug}.{flavor}.yaml" path = out / rel path.parent.mkdir(parents=True, exist_ok=True) diff --git a/packages/flow/tests/test_generate_coincidences.py b/packages/flow/tests/test_generate_coincidences.py new file mode 100644 index 000000000..b09f69ccb --- /dev/null +++ b/packages/flow/tests/test_generate_coincidences.py @@ -0,0 +1,253 @@ +# ruff: noqa: F811 (pytest fixtures imported from the generator test, then used as arguments) +"""Joins and pins two demonstrations can't tell apart (#1677, item 4). + +A small time tracker and order book on a throwaway PostgreSQL, with no +declared foreign keys, seeded so that ids coincide in every demonstration: + +- a new time entry's own id equals its customer's id; +- a new ticket's ``ext_id`` equals its customer's id, while the real path is + the ticket's project and that project's customer; +- an order line's own id equals its order's id; +- a value one demonstration alone stored (an order's currency). + +The generator must never take a row's own key for a join, must ask when a +join only equal ids support has a supported rival, and must not pin a value +seen beside one write only. Correct writes are then CONFIRMED and wrong ones +stopped. Records are made up. +""" + +from __future__ import annotations + +import json +import os +import tempfile +import time +from typing import Any, Callable + +import pytest +import yaml + +psycopg2 = pytest.importorskip("psycopg2") + +from openadapt_flow.generate import verify_contract as gen # noqa: E402 +from tests.test_generate_verify_contract import BOT, App, pg # noqa: E402,F401 + +SCHEMA = """ +CREATE TABLE users (id serial PRIMARY KEY, login text); +CREATE TABLE customers (id serial PRIMARY KEY, name text NOT NULL); +CREATE TABLE projects (id serial PRIMARY KEY, name text NOT NULL, customer_id integer); +CREATE TABLE timesheets (id serial PRIMARY KEY, project_id integer NOT NULL, + hours integer NOT NULL, user_id integer NOT NULL, created_at timestamp); +CREATE TABLE tickets (id serial PRIMARY KEY, project_id integer NOT NULL, + title text NOT NULL, ext_id integer, user_id integer NOT NULL, created_at timestamp); +CREATE TABLE orders (id serial PRIMARY KEY, supplier text NOT NULL, note text, + currency text NOT NULL, user_id integer NOT NULL, created_at timestamp); +CREATE TABLE order_lines (id serial PRIMARY KEY, order_id integer NOT NULL, + part text NOT NULL, qty integer NOT NULL); +INSERT INTO users (login) VALUES ('bot'); +INSERT INTO customers (name) VALUES ('Acme'), ('Globex'), ('Initech'), ('Umbrella'), + ('Hooli'), ('Vandelay'), ('Stark'), ('Wayne'); +INSERT INTO projects (name, customer_id) VALUES ('Apollo', 3), ('Gemini', 4), + ('Mercury', 6), ('Skylab', 3), ('Ranger', 7); +SELECT setval('timesheets_id_seq', 2); +SELECT setval('tickets_id_seq', 40); +SELECT setval('orders_id_seq', 2); +SELECT setval('order_lines_id_seq', 2); +GRANT SELECT ON ALL TABLES IN SCHEMA public TO oa_reader; +GRANT SELECT ON ALL SEQUENCES IN SCHEMA public TO oa_reader; +""" + + +class Book(App): + def project(self, name: str) -> int: + return self.one("SELECT id FROM projects WHERE name=%s", (name,)) + + def log_time(self, v: dict[str, Any]) -> None: + self.q("INSERT INTO timesheets (project_id, hours, user_id, created_at) " + "VALUES (%s,%s,%s,now())", (self.project(v["project"]), v["hours"], BOT)) # fmt: skip + + def open_ticket(self, v: dict[str, Any], ext: Any) -> None: + self.q("INSERT INTO tickets (project_id, title, ext_id, user_id, created_at) " + "VALUES (%s,%s,%s,%s,now())", (self.project(v["project"]), v["title"], ext, BOT)) # fmt: skip + + def order(self, v: dict[str, Any], currency: str) -> None: + oid = self.one("INSERT INTO orders (supplier, note, currency, user_id, created_at) " + "VALUES (%s,%s,%s,%s,now()) RETURNING id", + (v["supplier"], v.get("note"), currency, BOT)) # fmt: skip + for line in v["lines"]: + self.q("INSERT INTO order_lines (order_id, part, qty) VALUES (%s,%s,%s)", + (oid, line["part"], line["qty"])) # fmt: skip + + +LOG = [ + {"project": "Apollo", "customer": "Initech", "hours": 3}, # entry 3, customer 3 + {"project": "Gemini", "customer": "Umbrella", "hours": 5}, # entry 4, customer 4 +] +TICKET = [ + {"project": "Apollo", "customer": "Initech", "title": "Printer"}, # ext 3 + {"project": "Gemini", "customer": "Umbrella", "title": "VPN"}, # ext 4 +] +ORDER = [ + {"supplier": "Bolt Co", "note": "Rush", "lines": [{"part": "M4 bolt", "qty": 10}]}, + {"supplier": "Nut Co", "note": None, "lines": [{"part": "M4 nut", "qty": 20}]}, +] +KINDS = { + "kinds": [ + {"kind": "log_time", "parameters": ["project", "customer", "hours"], + "examples": [{"values": v} for v in LOG]}, + {"kind": "open_ticket", "parameters": ["project", "customer", "title"], + "examples": [{"values": v} for v in TICKET]}, + {"kind": "create_order", "parameters": ["supplier", "note", "lines"], + "examples": [{"values": v} for v in ORDER]}, + ] +} # fmt: skip + + +@pytest.fixture(scope="module") +def book(pg: Any) -> dict[str, Any]: + root, port = pg["root"], pg["port"] + admin = psycopg2.connect( + host="127.0.0.1", port=port, user="admin", dbname="postgres" + ) + admin.autocommit = True + admin.cursor().execute("CREATE DATABASE book") + admin.close() + app = Book(port, "book") + with app.con() as c, c.cursor() as cur: + cur.execute( + "DO $$BEGIN IF NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname='oa_reader') " + "THEN CREATE ROLE oa_reader LOGIN PASSWORD 'reader-pw'; END IF; END$$" + ) + cur.execute(SCHEMA) + os.environ["OA_GEN_TEST_PW"] = "reader-pw" + work = root / "work-coincidences" + work.mkdir() + read = {"sql_driver": "psycopg2", + "sql_connect_args": {"host": "127.0.0.1", "port": port, "user": "oa_reader", "dbname": "book"}, + "sql_password_env": "OA_GEN_TEST_PW"} # fmt: skip + (work / "read.yaml").write_text(yaml.safe_dump(read)) + (work / "kinds.yaml").write_text(yaml.safe_dump(KINDS)) + common = ["--read", str(work / "read.yaml")] + + def demo(kind: str, values: dict[str, Any], write: Callable[[], Any]) -> None: + assert gen.main(["snapshot", *common, "--out", str(work / "before.json")]) == 0 + write() + assert gen.main(["record", *common, "--before", str(work / "before.json"), "--kind", kind, + "--values", json.dumps(values), "--demos", str(work / "demos")]) == 0 # fmt: skip + + for v in LOG: + demo("log_time", v, lambda v=v: app.log_time(v)) + for v, ext in zip(TICKET, (3, 4)): + demo("open_ticket", v, lambda v=v, ext=ext: app.open_ticket(v, ext)) + for v, cur in zip(ORDER, ("SEK", "USD")): + demo("create_order", v, lambda v=v, cur=cur: app.order(v, cur)) + argv = ["propose", *common, "--kinds", str(work / "kinds.yaml"), "--demos", str(work / "demos"), + "--out", str(work / "out"), "--actor-param", f"bot_user_id={BOT}"] # fmt: skip + assert gen.main(argv) == 0 + first = json.loads((work / "out" / "report.json").read_text()) + questions = (work / "out" / "QUESTIONS.md").read_text() + answers = {q["key"]: q["proposal"] for q in first["questions"]} + # The person picks the ticket's customer behind its project. + tied = first["kinds"]["open_ticket"]["unresolved"].get("customer") or [] + if tied: + answers["locations.open_ticket"] = { + "customer": next( + i for i, t in enumerate(tied) if "projects.customer_id" in t + ) + } + # The order's currency was seen beside one write only: the person + # names it as unchecked (else the kind has no contract). + bk = answers["bookkeeping"] + bk = json.loads(json.dumps(bk)) if isinstance(bk, dict) else {"accept": True} + bk.setdefault("unchecked", {}).setdefault("orders", []).append("currency") + answers["bookkeeping"] = bk + (work / "answers.yaml").write_text(yaml.safe_dump(answers)) + assert gen.main([*argv, "--answers", str(work / "answers.yaml")]) == 0 + report = json.loads((work / "out" / "report.json").read_text()) + return {"app": app, "out": work / "out", "first": first, "report": report, + "questions": questions} # fmt: skip + + +def _run( + book: dict[str, Any], kind: str, values: dict[str, Any], write: Callable[[], Any] +) -> str: + from openadapt_flow.verifier import Actor, begin_write, verify_write + + contract, params = gen.choose(book["out"] / "manifest.yaml", kind, values, + {"bot_user_id": str(BOT)}) # fmt: skip + with tempfile.TemporaryDirectory() as tmp: + pending = begin_write(contract, params, actor=Actor("agent", "ops-bot"), ledger=None, + write_key=f"t-{time.time_ns()}", workdir=tmp) # fmt: skip + write() + result = verify_write(pending) + return ( + str(getattr(result.outcome, "value", result.outcome)) + + " " + + "; ".join(result.reasons)[:300] + ) + + +def test_a_rows_own_key_is_never_a_join(book: dict[str, Any]) -> None: + """Each new time entry's id equalled its customer's id. The customer is + placed behind the project, never at the entry's own key.""" + + where = book["report"]["kinds"]["log_time"]["locations"]["customer"] + assert where.startswith( + "timesheets.project_id -> projects.customer_id -> customers" + ), where + # A correct entry on a project whose customer's id is not the entry's. + v = {"project": "Mercury", "customer": "Vandelay", "hours": 2} + assert _run(book, "log_time", v, lambda: book["app"].log_time(v)).startswith( + "CONFIRMED" + ) + wrong = { + "project": "Ranger", + "customer": "Vandelay", + "hours": 2, + } # Ranger is Stark's + assert not _run( + book, "log_time", v, lambda: book["app"].log_time(wrong) + ).startswith("CONFIRMED") + + +def test_a_join_only_equal_ids_support_is_asked(book: dict[str, Any]) -> None: + """The ticket's ext_id equalled its customer's id twice. That join and + the project's customer both fit; the generator asks.""" + + first = book["first"]["kinds"]["open_ticket"] + tied = first["unresolved"]["customer"] + assert any("tickets.ext_id -> customers" in t for t in tied), tied + assert any("projects.customer_id -> customers" in t for t in tied), tied + assert "locations.open_ticket" in book["questions"] + v = {"project": "Mercury", "customer": "Vandelay", "title": "Fax"} + assert _run( + book, "open_ticket", v, lambda: book["app"].open_ticket(v, 6) + ).startswith("CONFIRMED") + + +def test_a_list_items_own_key_is_never_its_link(book: dict[str, Any]) -> None: + """Each order's one line had the order's id as its own id. The line is + linked by order_id, so a correct order with two lines is confirmed.""" + + v = {"supplier": "Bolt Co", "note": None, + "lines": [{"part": "M5 bolt", "qty": 5}, {"part": "M6 bolt", "qty": 6}]} # fmt: skip + app = book["app"] + out = _run(book, "create_order", v, lambda: app.order(v, "USD")) + assert out.startswith("CONFIRMED"), out + + +def test_a_value_seen_beside_one_write_is_not_pinned(book: dict[str, Any]) -> None: + """Only the order with a note stored SEK. The generator doesn't pin SEK + to orders with a note: it asks, and leaves it out of the proposal.""" + + assert "orders.currency (seen in one demonstration only)" in book["questions"] + proposal = book["first"]["questions"][0]["proposal"] + assert "currency" not in ((proposal or {}).get("unchecked") or {}).get("orders", []) + assert any( + "orders.currency" in r + for r in book["first"]["kinds"]["create_order"]["refused"] + ) + # Named as unchecked, an order with a note in another currency passes. + v = {"supplier": "Nut Co", "note": "Rush", "lines": [{"part": "M8 nut", "qty": 3}]} + out = _run(book, "create_order", v, lambda: book["app"].order(v, "EUR")) + assert out.startswith("CONFIRMED"), out diff --git a/packages/flow/tests/test_generate_verify_contract.py b/packages/flow/tests/test_generate_verify_contract.py index 3806f1815..b7d0ac104 100644 --- a/packages/flow/tests/test_generate_verify_contract.py +++ b/packages/flow/tests/test_generate_verify_contract.py @@ -138,7 +138,7 @@ def create_issue(self, v: dict[str, Any], *, project_id: Any = None) -> int: ) if v.get("customer_ref") is not None: self.q("INSERT INTO custom_values (customized_type, customized_id, custom_field_id, value) " - "VALUES ('Issue', %s, 1, %s)", (iid, v["customer_ref"])) # fmt: skip + "VALUES ('Issue', %s, 4, %s)", (iid, v["customer_ref"])) # fmt: skip return iid def find(self, project: str, subject: str) -> int: @@ -210,6 +210,18 @@ def update_issue(self, iid: int, v: dict[str, Any], *, user: int = BOT) -> None: "customer_ref": "007341", } }, + # A second write with a custom field: one row can't tell a + # pinned value from what one write happened to store. + { + "values": { + "project": "Billing Portal Legacy", + "subject": "Ledger export", + "status": "In Progress", + "assignee": "Jon Park", + "due_date": None, + "customer_ref": "008812", + } + }, ], }, { From 6b530de3ddba9f58056b7cbee8644275d9185cd8 Mon Sep 17 00:00:00 2001 From: abrichr Date: Sat, 31 Oct 2026 13:50:20 -0400 Subject: [PATCH 2/7] CAPABILITIES: joins and pins the demonstrations can't tell apart --- CAPABILITIES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CAPABILITIES.md b/CAPABILITIES.md index e9353b06d..1cb81332c 100644 --- a/CAPABILITIES.md +++ b/CAPABILITIES.md @@ -147,7 +147,7 @@ improvising or retrying when something is off. | Judge an "ensure the record exists" write (found, created once with every value, or needs review) through a FHIR server's read path only: searches, AuditEvent and `_history`, never the database; the coverage claim names that read path. An agent's own correction of the record it created counts as one write when the audit trail and history show only that agent, no other reader, and the same record in every version | On main ([#1640](https://github.com/OpenAdaptAI/OpenAdapt/pull/1640), corrections [#1655](https://github.com/OpenAdaptAI/OpenAdapt/pull/1655)) | Real app (Medplum 5.2.8; three blind fault suites: 17/18, 17/20, 19/20; no fault passed in the last two. Corrections, on 15 blind cases and the 37 public ones: false stops 8 to 2 and 9 to 4 for Claude, no harm passed that the old rule caught) | `openadapt_flow/ensure_record.py`, [ENSURE_RECORD.md](packages/flow/docs/ENSURE_RECORD.md) | | Generate an ensure-record contract from three demonstrations, the server's CapabilityStatement and its SearchParameter definitions: field paths and `[system=…]` filters, comparison kinds, value maps, identity, search parameters, the closed record and the change feed. It refuses when unsure, asks a person to confirm an identity for a type with no reviewed template, and self-checks every demonstration plus a perturbation per field | On main ([#1653](https://github.com/OpenAdaptAI/OpenAdapt/pull/1653)) | Real app (Medplum 5.2.8: all 16 hand-written Patient lines regenerated; one blind write type, Immunization: 78 s of person time, 2 lines edited after the person declined the proposed identity; blind suite 23/24, no fault passed) | `openadapt_flow/generate/ensure_contract.py`, [RESULTS.md](packages/flow/benchmark/contract_autogen/results/RESULTS.md) | | A stop card for a stopped write: the exact record difference and who else wrote, with Undo or Fix that reads back, or Open in the app; cost modelled from published operators and timed machine steps, as pre-registered | On main ([#1653](https://github.com/OpenAdaptAI/OpenAdapt/pull/1653)) | Real app (Medplum 5.2.8, blind suite: 16 stops, 14 one-tap with no failed read-back; modelled 42 s per caught harm against 420; one wrong category on a false stop, gone in the re-measure with `void`: 24/24, 15 stops, no wrong category, no failed read-back) | `benchmark/stop_card/`, [PREREG_REVIEW.md](packages/flow/benchmark/stop_card/PREREG_REVIEW.md) | -| Generate `openadapt.verify/v1` contracts from a MySQL, MariaDB or PostgreSQL schema, the kinds of write with their parameter names, and one demonstration write per example: one contract per field set (an update gets one for each set of fields it may change), list parameters by length, a `found` twin for a record already in place, the whole-database frame on every dialect, rows the app adds on its own declared by count, a recheck window the person confirms, and `frame.versions` (a lost update) from a version column or the app's audit trail of saves. Every undecided point is one numbered question; every contract is loaded and dry-read against the live store before it is written; a needed `TRIGGER` grant or view is applied with a flag or the kind is refused. Not yet: OpenAPI metadata | [#1672](https://github.com/OpenAdaptAI/OpenAdapt/pull/1672) | Real app (first study, instance copies; questions for a person: Dolibarr 4, Redmine 2, EspoCRM 3, against 11 hand decisions each; every in-scope planted fault stopped (22/22, 19/19, 21/21); correct writes confirmed 32/43, 32/38, 27/40; evidence in the private study repository) | `python -m openadapt_flow.generate.verify_contract`, [GENERATE_CONTRACTS.md](packages/flow/docs/GENERATE_CONTRACTS.md) | +| Generate `openadapt.verify/v1` contracts from a MySQL, MariaDB or PostgreSQL schema, the kinds of write with their parameter names, and one demonstration write per example: one contract per field set (an update gets one for each set of fields it may change), list parameters by length, a `found` twin for a record already in place, the whole-database frame on every dialect, rows the app adds on its own declared by count, a recheck window the person confirms, and `frame.versions` (a lost update) from a version column or the app's audit trail of saves. Equal ids are never taken for a join (a row's own key, the column a path arrived by), a join only equal ids support is a question when a supported place also fits, and a pin needs two rows that agree. Every undecided point is one numbered question; every contract is loaded and dry-read against the live store before it is written; a needed `TRIGGER` grant or view is applied with a flag or the kind is refused. Not yet: OpenAPI metadata | [#1672](https://github.com/OpenAdaptAI/OpenAdapt/pull/1672) | Real app (first study, instance copies; questions for a person: Dolibarr 4, Redmine 2, EspoCRM 3, against 11 hand decisions each; every in-scope planted fault stopped (22/22, 19/19, 21/21); correct writes confirmed 32/43, 32/38, 27/40; evidence in the private study repository) | `python -m openadapt_flow.generate.verify_contract`, [GENERATE_CONTRACTS.md](packages/flow/docs/GENERATE_CONTRACTS.md) | | `void` in an ensure-record contract: values that make a record assert nothing (`status="entered-in-error"`). A record holding one is never the person, is not a duplicate, and a created one needs review; it names values to leave out, never values to keep, and can't name an identity field | On main ([#1657](https://github.com/OpenAdaptAI/OpenAdapt/pull/1657)) | Real app (Medplum 5.2.8: 8 live cases written in advance, 8/8 with the line, 5/8 and one silent pass without it; frozen blind Immunization suite 24/24); adversarial review recorded | `openadapt_flow/ensure_record.py`, [RESULTS.md](packages/flow/benchmark/fhir_medplum/void/results/RESULTS.md), [REVIEW_VOID.md](packages/flow/benchmark/fhir_medplum/void/REVIEW_VOID.md) | | Check a patient registration on a commercial EHR through its public FHIR sandbox: Epic (open.epic, R4), read through FHIR search and read with SMART backend auth; the frame is the search set, since the sandbox has no AuditEvent or `_history` | On main ([#1656](https://github.com/OpenAdaptAI/OpenAdapt/pull/1656)) | Real app (Epic public sandbox, August 2026 release; blind round 12/13 with none passed as verified, the miss a false refusal fixed in the contract; caught Epic dropping the phone on a 201 create) | `benchmark/commercial_ehr/`, [FINDINGS.md](packages/flow/benchmark/commercial_ehr/FINDINGS.md) | | A public benchmark a vendor reruns with their own agent: `openadapt bench medplum --agent ` runs 37 written-for-it cases (Patient, AllergyIntolerance, Appointment) against a local Medplum through a fault proxy, and scores the agent's claim, a read-back and OpenAdapt's verifier against an end-state oracle | On main ([#1652](https://github.com/OpenAdaptAI/OpenAdapt/pull/1652)) | Real app (Medplum 5.2.8; Claude Sonnet 5.5: OpenAdapt missed 1 of 9 harms with 4 false stops, all corrections that changed the record's identity, against 8 missed by the claim and the read-back; cold run from clone to scorecard 5 min) | [README](packages/flow/benchmark/fhir_medplum/public/README.md), [RESULTS.md](packages/flow/benchmark/fhir_medplum/public/RESULTS.md) | From deeed8062aa7d30a58c987923b43aa8f60e3cc12 Mon Sep 17 00:00:00 2001 From: abrichr Date: Sat, 31 Oct 2026 14:02:33 -0400 Subject: [PATCH 3/7] Generator: distinct names for the open-role loop (mypy) --- .../flow/openadapt_flow/generate/verify_contract.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/flow/openadapt_flow/generate/verify_contract.py b/packages/flow/openadapt_flow/generate/verify_contract.py index 38292ab5d..b580106d0 100644 --- a/packages/flow/openadapt_flow/generate/verify_contract.py +++ b/packages/flow/openadapt_flow/generate/verify_contract.py @@ -3128,13 +3128,13 @@ def table_cols(key: str) -> dict[str, set[str]]: (g.table, g.roles) for g in [*p.children.values(), *p.lists.values()] ] groups += [(sd.table, sd.roles) for sd in p.sides] - for table, roles_of in groups: - for col, role in roles_of.items(): + for table, group_roles in groups: + for col, role in group_roles.items(): if role.role in OPEN_ROLES: own.setdefault(table, {})[col] = role.role - why = str(role.detail.get("why") or "") - if "one demonstration" in why or "can't tell" in why: - why_open[f"{table}.{col}"] = why + reason = str(role.detail.get("why") or "") + if "one demonstration" in reason or "can't tell" in reason: + why_open[f"{table}.{col}"] = reason open_by_kind[p.kind] = own unchecked: dict[str, set[str]] = {} for own_cols in open_by_kind.values(): From c3a9df0815ed95e3345427c1e92db0638efbdf59 Mon Sep 17 00:00:00 2001 From: abrichr Date: Sat, 31 Oct 2026 14:51:48 -0400 Subject: [PATCH 4/7] A date part needs two rows; a column the app computes is not refused as unpinned --- .../openadapt_flow/generate/demo_analysis.py | 2 +- .../openadapt_flow/generate/verify_contract.py | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/packages/flow/openadapt_flow/generate/demo_analysis.py b/packages/flow/openadapt_flow/generate/demo_analysis.py index a071b9f92..8c779159e 100644 --- a/packages/flow/openadapt_flow/generate/demo_analysis.py +++ b/packages/flow/openadapt_flow/generate/demo_analysis.py @@ -652,7 +652,7 @@ def column_roles( ): out[name] = ColumnRole("varies", detail={"why": "holds the row's own key"}) continue - part = _date_part(values, [d for d, _ in rows]) + part = None if one else _date_part(values, [d for d, _ in rows]) if part is not None: out[name] = ColumnRole( "varies", detail={"why": f"the {part} of a date the task gives"} diff --git a/packages/flow/openadapt_flow/generate/verify_contract.py b/packages/flow/openadapt_flow/generate/verify_contract.py index b580106d0..afc2fbfe0 100644 --- a/packages/flow/openadapt_flow/generate/verify_contract.py +++ b/packages/flow/openadapt_flow/generate/verify_contract.py @@ -79,6 +79,17 @@ class Unpinned(ValueError): set gets no contract.""" +def _computed(role: ColumnRole) -> bool: + """A column the app computes from the row (the day of a date the task + gives, a text holding the row's own key): recognized, so not pinned and + not asked about.""" + + why = str(role.detail.get("why") or "") + return why == "holds the row's own key" or ( + why.startswith("the ") and "of a date the task gives" in why + ) + + class GenerateError(RuntimeError): """The generator can't go on; the message says why.""" @@ -2232,7 +2243,11 @@ def _pin( actor_params: Mapping[str, str], unchecked: Mapping[str, set[str]] = {}, ) -> None: - if role.role == "varies" and col not in unchecked.get(table, set()): + if ( + role.role == "varies" + and col not in unchecked.get(table, set()) + and not _computed(role) + ): why = role.detail.get("why") or "it differs between demonstrations" raise Unpinned(f"{table}.{col} is not checked ({why})") if role.role == "free" and col not in unchecked.get(table, set()): From aac319cbf9bd2a16cb26510fd33c421e458fcba8 Mon Sep 17 00:00:00 2001 From: abrichr Date: Mon, 2 Nov 2026 19:03:04 -0500 Subject: [PATCH 5/7] Generator coincidences: review round 1 - B1: the own-key rule applies only to a whole single-column key holding a generated number. A composite key member and a copied token (UUID) link again (line tables, one-to-one extension tables). - B2: a link and 'holds the row's own key' need two demonstrations, and the own key must sit in one template in every row (INV-{key}), never a substring. - An actor id that is also a copy or a link is a question, not a pick. - A date part must differ between demonstrations; a constant equal to every demonstration's month is a constant. - A copy goes only through a declared key or a column named after its table. - Widening counts demonstrations, not rows. Each attack is a test (tests/test_generate_coincidences_r1.py). --- packages/flow/docs/GENERATE_CONTRACTS.md | 21 +- .../openadapt_flow/generate/demo_analysis.py | 127 +++++++-- .../generate/verify_contract.py | 6 +- .../flow/tests/test_generate_coincidences.py | 6 +- .../tests/test_generate_coincidences_r1.py | 240 ++++++++++++++++++ .../tests/test_generate_verify_contract.py | 12 + ...est_generate_verify_contract_attacks_r2.py | 9 +- 7 files changed, 381 insertions(+), 40 deletions(-) create mode 100644 packages/flow/tests/test_generate_coincidences_r1.py diff --git a/packages/flow/docs/GENERATE_CONTRACTS.md b/packages/flow/docs/GENERATE_CONTRACTS.md index bf93bec0a..525e08765 100644 --- a/packages/flow/docs/GENERATE_CONTRACTS.md +++ b/packages/flow/docs/GENERATE_CONTRACTS.md @@ -152,9 +152,11 @@ If places are still tied, that is a question. Equal ids are not a join. Demonstrations made on a fresh install often give a new row the same id as an unrelated row, so: -- A row's own primary key is never a key to another table, and a new row's - own key never links it to its record, unless a declared foreign key says - so. +- A row's own whole single-column key holding a generated number is never + a key to another table, and a new row's such key never links it to its + record, unless a declared foreign key says so. A member of a composite key + (an order line's `order_id` in `(order_id, line_no)`) and a copied token + (an extension table keyed by its parent's UUID) still link. - A value is never placed in the column a path arrived by (a child row's link column holds the record's key by construction). - A list item's link is the column that holds the record's key in every @@ -212,7 +214,13 @@ too. A list parameter gets a contract per length, from 1 to 10 items. user, to the value every demonstration held, or to a copy of a column of the row a key points to (an invoice line's price from its product). A key, a link to a row the write added, the year, month, week or day of a date the -task gives, and a generated token (a UUID) are recognized as such. Columns +task gives, and a generated token (a UUID) are recognized as such. A link +and a column that holds the row's own key need two demonstrations; the own +key must sit in the same text in every row (`INV-{key}`), never as a mere +substring. A date part must differ between the demonstrations: a constant +that happens to equal the month of every demonstration's date is a +constant. A copy is taken only through a declared key or a column named +after its table. Columns that differ between demonstrations, free text every demonstration left empty, and the write's time stamps are listed in the bookkeeping question, and each is left unchecked only if the answer names it (`unchecked`). Free @@ -233,8 +241,9 @@ kind has no contract until a person names it under `unchecked` or records a demonstration where it differs. A field set whose own demonstrations leave such a column open, while the kind's full set pins it, gets no contract (`rejected` in `report.json`). A column that holds the acting user's id in -every row is the actor, even when it also equals a column of a row it points -to. A row the app adds beside an update may hold the target's key: that is a +every row is the actor, unless it is also a copy of a column of a row it +points to, or a link: then the demonstrations can't tell, and the question +lists it. Pins and widening count demonstrations, not rows. A row the app adds beside an update may hold the target's key: that is a link to the record, never a pinned id. **Rows the app adds on its own.** A row type the app added the same number of diff --git a/packages/flow/openadapt_flow/generate/demo_analysis.py b/packages/flow/openadapt_flow/generate/demo_analysis.py index 8c779159e..b64f33e88 100644 --- a/packages/flow/openadapt_flow/generate/demo_analysis.py +++ b/packages/flow/openadapt_flow/generate/demo_analysis.py @@ -399,7 +399,7 @@ def walk( if depth == 0 and changed_only and col.name not in changed: continue for target, ref, declared in _fk_targets(schema, idx, t, col.name, v): - if col.name in t.primary_key and not declared: + if not declared and own_key(t, col.name, v): # A row's own key names that row: equal to another # table's key only by chance, unless a foreign key says so. continue @@ -440,7 +440,7 @@ def walk( ) if not declared and not _IDLIKE.search(col.name): continue - if not declared and col.name in child.primary_key: + if not declared and own_key(child, col.name, key): continue # the child's own key: equal by chance walk( @@ -482,19 +482,41 @@ def link_columns( cols = { c for c, v in ch.after.items() - if v == key and is_link_column(schema, ch.table, c, anchor.table) + if v == key and is_link_column(schema, ch.table, c, anchor.table, v) } out[ch.table] = out[ch.table] & cols if ch.table in out else cols return out -def is_link_column(schema: Schema, table: str, column: str, parent: str) -> bool: +#: A generated token (a UUID, a long hex id): equal in two rows only when +#: one copied it from the other, never by chance. +TOKEN = re.compile( + r"^([0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}" + r"|[0-9a-fA-F]{16,})$" +) + + +def own_key(t: Table, column: str, value: Any) -> bool: + """Is ``t.column`` the row's own whole single-column key, holding a value + that can equal another table's key by chance (a small generated number)? + A member of a composite key (an order line's ``order_id``) and a copied + token (an extension table's UUID) can link rows.""" + + return ( + t.primary_key == [column] and value is not None and not TOKEN.match(str(value)) + ) + + +def is_link_column( + schema: Schema, table: str, column: str, parent: str, value: Any = None +) -> bool: """Can ``table.column`` link a row to a ``parent`` row? Not the row's own - primary key, unless a foreign key declares it (a one-to-one extension - table): a new row's key equals its parent's key only by chance.""" + whole key holding a small generated number, unless a foreign key + declares it (a one-to-one extension table): a new row's key equals its + parent's key only by chance.""" t = schema.tables.get(table) - if t is None or column not in t.primary_key: + if t is None or not own_key(t, column, "0" if value is None else value): return True return any(fk.column == column and fk.table == parent for fk in t.foreign_keys) @@ -625,32 +647,49 @@ def column_roles( for p, av in actor_values.items() if values and all(v is not None and v == av for v in values) ] + links = link_values or [set() for _ in rows] + is_link = ( + not one + and bool(values) + and all( + v is not None + and ( + v in lv + or (len(t.primary_key) == 1 and v == r.get(t.primary_key[0])) + ) + for v, lv, (_, r) in zip(values, links, rows) + ) + ) if actor and not one: - # The acting user's id in every row: the person declared it - # (``--actor-param``), so it wins over a copy of another row's - # column that happens to hold the same id. + copy = _copied(schema, t, name, rows) + if copy is not None or is_link: + # The acting user's id in every row, and also a copy of + # another row's column or a link: the demonstrations can't + # tell which the app does. + other = ( + f"a copy of {copy['table']}.{copy['column']}" + if copy is not None + else "a link to a row the write added" + ) + out[name] = ColumnRole( + "varies", + detail={ + "why": f"the acting user's id or {other}: the " + "demonstrations can't tell" + }, + ) + continue out[name] = ColumnRole("actor", detail={"param": actor[0]}) continue - links = link_values or [set() for _ in rows] - if values and all( - v is not None - and (v in lv or (len(t.primary_key) == 1 and v == r.get(t.primary_key[0]))) - for v, lv, (_, r) in zip(values, links, rows) - ): + if is_link: out[name] = ColumnRole("link") continue - if ( - len(t.primary_key) == 1 - and values - and all( - v is not None - and r.get(t.primary_key[0]) is not None - and str(r.get(t.primary_key[0])) in v - and v != r.get(t.primary_key[0]) - for v, (_, r) in zip(values, rows) + template = None if one else _key_template(t, values, rows) + if template is not None: + out[name] = ColumnRole( + "varies", + detail={"why": "holds the row's own key", "template": template}, ) - ): - out[name] = ColumnRole("varies", detail={"why": "holds the row's own key"}) continue part = None if one else _date_part(values, [d for d, _ in rows]) if part is not None: @@ -700,6 +739,29 @@ def column_roles( return out +def _key_template( + t: Table, + values: Sequence[Optional[str]], + rows: Sequence[tuple["Demo", Mapping[str, Optional[str]]]], +) -> Optional[str]: + """``prefix{key}suffix`` when every row holds its own single-column key + between the same prefix and suffix (``INV-{key}``), else None.""" + + if len(t.primary_key) != 1 or not values: + return None + shapes = set() + for v, (_, r) in zip(values, rows): + key = r.get(t.primary_key[0]) + if v is None or key is None or v == key or str(key) not in v: + return None + i = v.index(str(key)) + shapes.add((v[:i], v[i + len(str(key)) :])) + if len(shapes) != 1: + return None + prefix, suffix = next(iter(shapes)) + return prefix + "{key}" + suffix + + def _date_part( values: Sequence[Optional[str]], demos: Sequence["Demo"] ) -> Optional[str]: @@ -707,6 +769,10 @@ def _date_part( task gives, in every row? (Redmine stores a time entry's ``tyear``, ``tmonth`` and ``tweek``.) The app computes it, so it isn't pinned.""" + if len({v for v in values}) < 2: + # The same value in every row: a constant, not a date part (two + # demonstrations in one month can't tell them apart). + return None parts = { "year": lambda d: d.year, "month": lambda d: d.month, @@ -759,9 +825,14 @@ def _copied( or r.get(other.name) is None ): continue - for target, ref, _declared in _fk_targets( + for target, ref, declared in _fk_targets( schema, idx, t, other.name, str(r.get(other.name)) ): + if _hop_cost(Hop("fk", target, other.name, ref, declared)) > 1: + # Only a declared key, or a column named after its table: + # a value equal to some row's id by chance (an issue id + # that is also an audit row's id) points to nothing. + continue trow = idx.row(target, str(r.get(other.name))) if trow is None: continue diff --git a/packages/flow/openadapt_flow/generate/verify_contract.py b/packages/flow/openadapt_flow/generate/verify_contract.py index afc2fbfe0..0ec302414 100644 --- a/packages/flow/openadapt_flow/generate/verify_contract.py +++ b/packages/flow/openadapt_flow/generate/verify_contract.py @@ -800,7 +800,7 @@ def _children( and ch.after.get(loc.hops[0].column) == (a.after or {}).get(loc.hops[0].ref_column) ] - if len(linked) >= 2: + if len({id(d) for d, _ in linked}) >= 2: wide = column_roles( schema, loc.hops[0].table, linked, explained, plan.actor_values ) @@ -867,7 +867,7 @@ def _children( c for c, v in after.items() if v == key - and is_link_column(schema, ch.table, c, plan.anchor) + and is_link_column(schema, ch.table, c, plan.anchor, v) } link_cols = here if link_cols is None else link_cols & here table = ch.table @@ -1040,7 +1040,7 @@ def _sides( for ch in d.diff.of(t, "added") if ch.after is not None ] - if len(wide) >= 2: + if len({id(d) for d, _ in wide}) >= 2: wide_links = [_link_keys(schema, plan, d) for d, _ in wide] more = column_roles( schema, t, wide, set(), actor_values, wide_links diff --git a/packages/flow/tests/test_generate_coincidences.py b/packages/flow/tests/test_generate_coincidences.py index b09f69ccb..aeeb8cc76 100644 --- a/packages/flow/tests/test_generate_coincidences.py +++ b/packages/flow/tests/test_generate_coincidences.py @@ -44,7 +44,7 @@ currency text NOT NULL, user_id integer NOT NULL, created_at timestamp); CREATE TABLE order_lines (id serial PRIMARY KEY, order_id integer NOT NULL, part text NOT NULL, qty integer NOT NULL); -INSERT INTO users (login) VALUES ('bot'); +INSERT INTO users (login) VALUES ('bot'), ('ana'); INSERT INTO customers (name) VALUES ('Acme'), ('Globex'), ('Initech'), ('Umbrella'), ('Hooli'), ('Vandelay'), ('Stark'), ('Wayne'); INSERT INTO projects (name, customer_id) VALUES ('Apollo', 3), ('Gemini', 4), @@ -63,8 +63,10 @@ def project(self, name: str) -> int: return self.one("SELECT id FROM projects WHERE name=%s", (name,)) def log_time(self, v: dict[str, Any]) -> None: + # Logged for user 2: were every row the bot's, a ticket's user could + # also be read as a copy of the user of the entry its ext_id names. self.q("INSERT INTO timesheets (project_id, hours, user_id, created_at) " - "VALUES (%s,%s,%s,now())", (self.project(v["project"]), v["hours"], BOT)) # fmt: skip + "VALUES (%s,%s,%s,now())", (self.project(v["project"]), v["hours"], 2)) # fmt: skip def open_ticket(self, v: dict[str, Any], ext: Any) -> None: self.q("INSERT INTO tickets (project_id, title, ext_id, user_id, created_at) " diff --git a/packages/flow/tests/test_generate_coincidences_r1.py b/packages/flow/tests/test_generate_coincidences_r1.py new file mode 100644 index 000000000..3a80d69c8 --- /dev/null +++ b/packages/flow/tests/test_generate_coincidences_r1.py @@ -0,0 +1,240 @@ +# ruff: noqa: F811 (pytest fixtures imported from the generator test, then used as arguments) +"""Review round 1 of #1682: each attack, as a test. + +On a throwaway PostgreSQL with no declared foreign keys: + +- a line table whose key is (order_id, line_no): the order's key is part of + the line's key, and links them; +- a one-to-one extension table keyed by its parent's UUID; +- a column equal to "13" while the row's own key is 3 (a substring, not a + template), seen beside one write; +- a constant equal to the month of the dates every demonstration gave; +- a column that holds the acting user's id and also a copy of the project's + manager, who is the acting user in every demonstration. + +Records are made up. +""" + +from __future__ import annotations + +import json +import os +import tempfile +import time +import uuid +from typing import Any, Callable + +import pytest +import yaml + +psycopg2 = pytest.importorskip("psycopg2") + +from openadapt_flow.generate import verify_contract as gen # noqa: E402 +from tests.test_generate_verify_contract import BOT, App, pg # noqa: E402,F401 + +SCHEMA = """ +CREATE TABLE users (id serial PRIMARY KEY, login text); +CREATE TABLE po (id serial PRIMARY KEY, supplier text NOT NULL); +CREATE TABLE po_items (order_id integer NOT NULL, line_no integer NOT NULL, + part text NOT NULL, qty integer NOT NULL, PRIMARY KEY (order_id, line_no)); +CREATE TABLE accounts (id text PRIMARY KEY, name text NOT NULL); +CREATE TABLE accounts_cstm (id_c text PRIMARY KEY, industry_c text); +CREATE TABLE invoices (id serial PRIMARY KEY, customer text NOT NULL, note text, + tag text, due date NOT NULL, discount integer NOT NULL, priority integer NOT NULL); +CREATE TABLE projects (id serial PRIMARY KEY, name text NOT NULL, manager_id integer); +CREATE TABLE tasks (id serial PRIMARY KEY, project_id integer NOT NULL, + title text NOT NULL, assignee_id integer); +INSERT INTO users (login) VALUES ('bot'), ('ana'), ('raj'); +INSERT INTO projects (name, manager_id) VALUES ('Apollo', 1), ('Gemini', 1), ('Mercury', 3); +SELECT setval('invoices_id_seq', 2); +SELECT setval('po_id_seq', 5); +GRANT SELECT ON ALL TABLES IN SCHEMA public TO oa_reader; +GRANT SELECT ON ALL SEQUENCES IN SCHEMA public TO oa_reader; +""" + + +class Shop(App): + def po(self, v: dict[str, Any]) -> None: + oid = self.one( + "INSERT INTO po (supplier) VALUES (%s) RETURNING id", (v["supplier"],) + ) + for n, item in enumerate(v["items"], 1): + self.q( + "INSERT INTO po_items VALUES (%s,%s,%s,%s)", + (oid, n, item["part"], item["qty"]), + ) + + def account(self, v: dict[str, Any], industry: Any = "same") -> None: + key = str(uuid.uuid4()) + self.q("INSERT INTO accounts VALUES (%s,%s)", (key, v["name"])) + ind = v.get("industry") if industry == "same" else industry + self.q("INSERT INTO accounts_cstm VALUES (%s,%s)", (key, ind)) + + def invoice( + self, v: dict[str, Any], discount: int = 13, priority: int = 11 + ) -> None: + self.q("INSERT INTO invoices (customer, note, tag, due, discount, priority) " + "VALUES (%s,%s,%s,%s,%s,%s)", + (v["customer"], v.get("note"), v.get("tag"), v["due"], discount, priority)) # fmt: skip + + def task(self, v: dict[str, Any]) -> None: + pid = self.one("SELECT id FROM projects WHERE name=%s", (v["project"],)) + mgr = self.one("SELECT manager_id FROM projects WHERE id=%s", (pid,)) + self.q("INSERT INTO tasks (project_id, title, assignee_id) VALUES (%s,%s,%s)", + (pid, v["title"], mgr)) # fmt: skip + + +PO = [ + {"supplier": "Bolt Co", "items": [{"part": "M4 bolt", "qty": 10}]}, + { + "supplier": "Nut Co", + "items": [{"part": "M4 nut", "qty": 20}, {"part": "M5 nut", "qty": 5}], + }, +] +ACCOUNT = [ + {"name": "Acme", "industry": "Retail"}, + {"name": "Globex", "industry": "Energy"}, +] +INVOICE = [ + {"customer": "Acme", "note": "Net 30", "tag": None, "due": "2026-11-05"}, # id 3 + {"customer": "Globex", "note": None, "tag": "q4", "due": "2026-11-20"}, # id 4 +] +TASK = [ + {"project": "Apollo", "title": "Kickoff"}, + {"project": "Gemini", "title": "Review"}, +] +KINDS = { + "kinds": [ + {"kind": "create_po", "parameters": ["supplier", "items"], + "examples": [{"values": v} for v in PO]}, + {"kind": "create_account", "parameters": ["name", "industry"], + "examples": [{"values": v} for v in ACCOUNT]}, + {"kind": "create_invoice", "parameters": ["customer", "note", "tag", "due"], + "examples": [{"values": v} for v in INVOICE]}, + {"kind": "create_task", "parameters": ["project", "title"], + "examples": [{"values": v} for v in TASK]}, + ] +} # fmt: skip + + +@pytest.fixture(scope="module") +def shop(pg: Any) -> dict[str, Any]: + root, port = pg["root"], pg["port"] + admin = psycopg2.connect( + host="127.0.0.1", port=port, user="admin", dbname="postgres" + ) + admin.autocommit = True + admin.cursor().execute("CREATE DATABASE shop") + admin.close() + app = Shop(port, "shop") + with app.con() as c, c.cursor() as cur: + cur.execute( + "DO $$BEGIN IF NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname='oa_reader') " + "THEN CREATE ROLE oa_reader LOGIN PASSWORD 'reader-pw'; END IF; END$$" + ) + cur.execute(SCHEMA) + os.environ["OA_GEN_TEST_PW"] = "reader-pw" + work = root / "work-coincidences-r1" + work.mkdir() + read = {"sql_driver": "psycopg2", + "sql_connect_args": {"host": "127.0.0.1", "port": port, "user": "oa_reader", "dbname": "shop"}, + "sql_password_env": "OA_GEN_TEST_PW"} # fmt: skip + (work / "read.yaml").write_text(yaml.safe_dump(read)) + (work / "kinds.yaml").write_text(yaml.safe_dump(KINDS)) + common = ["--read", str(work / "read.yaml")] + + def demo(kind: str, values: dict[str, Any], write: Callable[[], Any]) -> None: + assert gen.main(["snapshot", *common, "--out", str(work / "before.json")]) == 0 + write() + assert gen.main(["record", *common, "--before", str(work / "before.json"), "--kind", kind, + "--values", json.dumps(values), "--demos", str(work / "demos")]) == 0 # fmt: skip + + for v in PO: + demo("create_po", v, lambda v=v: app.po(v)) + for v in ACCOUNT: + demo("create_account", v, lambda v=v: app.account(v)) + for v in INVOICE: + demo("create_invoice", v, lambda v=v: app.invoice(v)) + for v in TASK: + demo("create_task", v, lambda v=v: app.task(v)) + argv = ["propose", *common, "--kinds", str(work / "kinds.yaml"), "--demos", str(work / "demos"), + "--out", str(work / "out"), "--actor-param", f"bot_user_id={BOT}"] # fmt: skip + assert gen.main(argv) == 0 + first = json.loads((work / "out" / "report.json").read_text()) + questions = (work / "out" / "QUESTIONS.md").read_text() + answers = {q["key"]: q["proposal"] for q in first["questions"]} + (work / "answers.yaml").write_text(yaml.safe_dump(answers)) + assert gen.main([*argv, "--answers", str(work / "answers.yaml")]) == 0 + report = json.loads((work / "out" / "report.json").read_text()) + return {"app": app, "out": work / "out", "report": report, "questions": questions} + + +def _run( + shop: dict[str, Any], kind: str, values: dict[str, Any], write: Callable[[], Any] +) -> str: + from openadapt_flow.verifier import Actor, begin_write, verify_write + + try: + contract, params = gen.choose(shop["out"] / "manifest.yaml", kind, values, + {"bot_user_id": str(BOT)}) # fmt: skip + except gen.GenerateError as exc: + return f"NO CONTRACT {exc}" + with tempfile.TemporaryDirectory() as tmp: + pending = begin_write(contract, params, actor=Actor("agent", "ops-bot"), ledger=None, + write_key=f"t-{time.time_ns()}", workdir=tmp) # fmt: skip + write() + result = verify_write(pending) + return ( + str(getattr(result.outcome, "value", result.outcome)) + + " " + + "; ".join(result.reasons)[:300] + ) + + +def test_b1_a_composite_key_member_links_line_items(shop: dict[str, Any]) -> None: + v = { + "supplier": "Bolt Co", + "items": [{"part": "M6 bolt", "qty": 3}, {"part": "M8 bolt", "qty": 4}], + } + out = _run(shop, "create_po", v, lambda: shop["app"].po(v)) + assert out.startswith("CONFIRMED"), out + + +def test_b1_a_uuid_extension_row_is_a_link(shop: dict[str, Any]) -> None: + where = shop["report"]["kinds"]["create_account"]["locations"]["industry"] + assert "accounts_cstm" in where, where + v = {"name": "Hooli", "industry": "Media"} + assert _run(shop, "create_account", v, lambda: shop["app"].account(v)).startswith( + "CONFIRMED" + ) + v = {"name": "Initech", "industry": "Media"} + wrong = _run(shop, "create_account", v, lambda: shop["app"].account(v, "Retail")) + assert not wrong.startswith("CONFIRMED"), wrong + + +@pytest.mark.parametrize("discount,priority", [(99, 11), (3, 11), (13, 99)]) +def test_b2_a_substring_or_a_month_never_leaves_a_column_open( + shop: dict[str, Any], discount: int, priority: int +) -> None: + """``discount`` 13 holds the key 3 only as a substring, and ``priority`` + 11 equals the month of both due dates: both are constants, pinned.""" + + v = {"customer": "Umbrella", "note": "Net 60", "tag": None, "due": "2026-12-01"} + out = _run(shop, "create_invoice", v, + lambda: shop["app"].invoice(v, discount=discount, priority=priority)) # fmt: skip + assert not out.startswith("CONFIRMED"), out + + +def test_b2_the_constants_still_pass(shop: dict[str, Any]) -> None: + v = {"customer": "Umbrella", "note": "Net 60", "tag": None, "due": "2026-12-02"} + out = _run(shop, "create_invoice", v, lambda: shop["app"].invoice(v)) + assert out.startswith("CONFIRMED"), out + + +def test_an_actor_id_that_is_also_a_copy_is_asked(shop: dict[str, Any]) -> None: + """The bot manages both demonstrated projects, so the task's assignee is + the bot's id and a copy of the manager. A question, never a silent pick.""" + + assert "tasks.assignee_id (the acting user's id or a copy of" in shop["questions"] + refused = shop["report"]["kinds"]["create_task"]["refused"] + assert any("tasks.assignee_id" in r for r in refused), refused diff --git a/packages/flow/tests/test_generate_verify_contract.py b/packages/flow/tests/test_generate_verify_contract.py index b7d0ac104..215bcaa5d 100644 --- a/packages/flow/tests/test_generate_verify_contract.py +++ b/packages/flow/tests/test_generate_verify_contract.py @@ -276,6 +276,18 @@ def generated(pg: Any) -> dict[str, Any]: app = App(port, "tracker") with app.con() as c, c.cursor() as cur: cur.execute(SCHEMA) + # Saved before: a version of 1 after a save would equal the bot's + # id, and the generator asks when a column is both the acting + # user's id and a copy of another row's column. + cur.execute("UPDATE issues SET lock_version = 4") + # Likewise new journal rows numbered from 1 would equal the bot's id + # (a link to a row the write added, or the acting user?). + for seq in ( + "journals_id_seq", + "journal_details_id_seq", + "custom_values_id_seq", + ): + cur.execute(f"SELECT setval('{seq}', 100)") os.environ["OA_GEN_TEST_PW"] = "reader-pw" work = root / "work" work.mkdir() diff --git a/packages/flow/tests/test_generate_verify_contract_attacks_r2.py b/packages/flow/tests/test_generate_verify_contract_attacks_r2.py index d51e4a5df..ee0590f66 100644 --- a/packages/flow/tests/test_generate_verify_contract_attacks_r2.py +++ b/packages/flow/tests/test_generate_verify_contract_attacks_r2.py @@ -356,7 +356,14 @@ def test_a_mapping_without_accept_takes_no_proposed_column(gv: dict[str, Any]) - app = gv["app"] iid = _fresh(app, "No accept") v = {"project": "Help Desk", "subject": "No accept", "status": "Resolved"} - out = _run(gv, "set_status", v, lambda: app.save(iid, {"status": "Resolved"})) + try: + out = _run( + gv, "set_status", v, lambda: app.save(iid, {"status": "Resolved"}) + ) + except gen.GenerateError as exc: + # No contract (an open column the answer doesn't name): stopped + # at begin, which is also not a pass. + out = f"NO CONTRACT {exc}" assert not out.startswith("CONFIRMED"), out finally: assert _propose(gv["work"], gv["argv"], gv["answers"]) == 0 From a0cf20a6ca9bb850e5de6f5098d689f4e8f9d989 Mon Sep 17 00:00:00 2001 From: abrichr Date: Mon, 2 Nov 2026 19:39:04 -0500 Subject: [PATCH 6/7] Generator coincidences: review round 2 - A column and its copy source that hold one value in every demonstration are a question (a constant or a copy), never a silent copy. - Two demonstrations means two demonstrations, not two rows. - The own-key template is checked by the contract, byte-exact on MySQL; such a column is no longer left open. --- packages/flow/docs/GENERATE_CONTRACTS.md | 6 +- .../openadapt_flow/generate/demo_analysis.py | 14 ++++- .../generate/verify_contract.py | 22 +++++-- .../tests/test_generate_coincidences_r1.py | 63 +++++++++++++++++++ 4 files changed, 98 insertions(+), 7 deletions(-) diff --git a/packages/flow/docs/GENERATE_CONTRACTS.md b/packages/flow/docs/GENERATE_CONTRACTS.md index 525e08765..6a57f0cf0 100644 --- a/packages/flow/docs/GENERATE_CONTRACTS.md +++ b/packages/flow/docs/GENERATE_CONTRACTS.md @@ -217,10 +217,12 @@ link to a row the write added, the year, month, week or day of a date the task gives, and a generated token (a UUID) are recognized as such. A link and a column that holds the row's own key need two demonstrations; the own key must sit in the same text in every row (`INV-{key}`), never as a mere -substring. A date part must differ between the demonstrations: a constant +substring, and the contract checks that text exactly. A date part must differ between the demonstrations: a constant that happens to equal the month of every demonstration's date is a constant. A copy is taken only through a declared key or a column named -after its table. Columns +after its table, and only when the source differs between demonstrations: +a column and its source that hold one value in every demonstration fit a +constant and a copy alike, so the question lists the column. Columns that differ between demonstrations, free text every demonstration left empty, and the write's time stamps are listed in the bookkeeping question, and each is left unchecked only if the answer names it (`unchecked`). Free diff --git a/packages/flow/openadapt_flow/generate/demo_analysis.py b/packages/flow/openadapt_flow/generate/demo_analysis.py index b64f33e88..db250ea0b 100644 --- a/packages/flow/openadapt_flow/generate/demo_analysis.py +++ b/packages/flow/openadapt_flow/generate/demo_analysis.py @@ -641,7 +641,8 @@ def column_roles( # One row can't tell a pinned value from what that one write # happened to store (a reference number, a currency, an id equal to # the bot's): never pinned from one row. - one = len(rows) < 2 + # Two rows one demonstration added (two lines) are one example. + one = len({id(d) for d, _ in rows}) < 2 actor = [ p for p, av in actor_values.items() @@ -698,6 +699,17 @@ def column_roles( ) continue copied = _copied(schema, t, name, rows) + if copied is not None and len({v for v in values}) == 1: + # The same value in every row, and so in its source: a constant + # and a copy both fit, and the demonstrations can't tell. + out[name] = ColumnRole( + "varies", + detail={ + "why": f"a constant or a copy of {copied['table']}." + f"{copied['column']}: the demonstrations can't tell" + }, + ) + continue if copied is not None: out[name] = ColumnRole("copied", detail=copied) continue diff --git a/packages/flow/openadapt_flow/generate/verify_contract.py b/packages/flow/openadapt_flow/generate/verify_contract.py index 0ec302414..397bfff6d 100644 --- a/packages/flow/openadapt_flow/generate/verify_contract.py +++ b/packages/flow/openadapt_flow/generate/verify_contract.py @@ -85,9 +85,7 @@ def _computed(role: ColumnRole) -> bool: not asked about.""" why = str(role.detail.get("why") or "") - return why == "holds the row's own key" or ( - why.startswith("the ") and "of a date the task gives" in why - ) + return why.startswith("the ") and "of a date the task gives" in why class GenerateError(RuntimeError): @@ -2243,6 +2241,22 @@ def _pin( actor_params: Mapping[str, str], unchecked: Mapping[str, set[str]] = {}, ) -> None: + template = role.detail.get("template") if role.role == "varies" else None + if template and col not in unchecked.get(table, set()): + # The row's own key in its exact text (``INV-{key}``): checked. + prefix, suffix = str(template).split("{key}", 1) + key = b.text("t", table, b.schema.tables[table].primary_key[0]) + parts = [b.literal(prefix), key, b.literal(suffix)] + whole = ( + "CONCAT(" + ", ".join(parts) + ")" if b.d == "mysql" else " || ".join(parts) + ) + stored = b.text("t", table, col) + if b.d == "mysql": # compare bytes: the collation may fold case + stored, whole = f"CAST({stored} AS BINARY)", f"CAST({whole} AS BINARY)" + expr = f"CASE WHEN {stored} = {whole} THEN '1' ELSE '0' END" + alias = _add_column(surf, f"pin__{_alias(col)}", expr) + match[alias] = {"literal": "1"} + return if ( role.role == "varies" and col not in unchecked.get(table, set()) @@ -3145,7 +3159,7 @@ def table_cols(key: str) -> dict[str, set[str]]: groups += [(sd.table, sd.roles) for sd in p.sides] for table, group_roles in groups: for col, role in group_roles.items(): - if role.role in OPEN_ROLES: + if role.role in OPEN_ROLES and not role.detail.get("template"): own.setdefault(table, {})[col] = role.role reason = str(role.detail.get("why") or "") if "one demonstration" in reason or "can't tell" in reason: diff --git a/packages/flow/tests/test_generate_coincidences_r1.py b/packages/flow/tests/test_generate_coincidences_r1.py index 3a80d69c8..8d5e9d965 100644 --- a/packages/flow/tests/test_generate_coincidences_r1.py +++ b/packages/flow/tests/test_generate_coincidences_r1.py @@ -48,12 +48,31 @@ INSERT INTO projects (name, manager_id) VALUES ('Apollo', 1), ('Gemini', 1), ('Mercury', 3); SELECT setval('invoices_id_seq', 2); SELECT setval('po_id_seq', 5); +CREATE TABLE customers (id serial PRIMARY KEY, name text NOT NULL, status integer); +CREATE TABLE bills (id serial PRIMARY KEY, customer_id integer NOT NULL, + ref text NOT NULL, status integer NOT NULL); +INSERT INTO customers (name, status) VALUES ('Acme', 5), ('Globex', 5), + ('Initech', 6), ('Umbrella', 6); +CREATE TABLE notes (id serial PRIMARY KEY, title text NOT NULL, code text NOT NULL); +SELECT setval('notes_id_seq', 30); GRANT SELECT ON ALL TABLES IN SCHEMA public TO oa_reader; GRANT SELECT ON ALL SEQUENCES IN SCHEMA public TO oa_reader; """ class Shop(App): + def bill(self, v: dict[str, Any], status: int = 5) -> None: + cid = self.one("SELECT id FROM customers WHERE name=%s", (v["customer"],)) + self.q("INSERT INTO bills (customer_id, ref, status) VALUES (%s,%s,%s)", + (cid, v["ref"], status)) # fmt: skip + + def note(self, v: dict[str, Any], code: Any = None) -> None: + nid = self.one( + "INSERT INTO notes (title, code) VALUES (%s, '') RETURNING id", + (v["title"],), + ) + self.q("UPDATE notes SET code=%s WHERE id=%s", (code or f"NT-{nid}", nid)) + def po(self, v: dict[str, Any]) -> None: oid = self.one( "INSERT INTO po (supplier) VALUES (%s) RETURNING id", (v["supplier"],) @@ -99,6 +118,8 @@ def task(self, v: dict[str, Any]) -> None: {"customer": "Acme", "note": "Net 30", "tag": None, "due": "2026-11-05"}, # id 3 {"customer": "Globex", "note": None, "tag": "q4", "due": "2026-11-20"}, # id 4 ] +BILL = [{"customer": "Acme", "ref": "B-1"}, {"customer": "Globex", "ref": "B-2"}] +NOTE = [{"title": "Kickoff"}, {"title": "Review"}] TASK = [ {"project": "Apollo", "title": "Kickoff"}, {"project": "Gemini", "title": "Review"}, @@ -111,6 +132,10 @@ def task(self, v: dict[str, Any]) -> None: "examples": [{"values": v} for v in ACCOUNT]}, {"kind": "create_invoice", "parameters": ["customer", "note", "tag", "due"], "examples": [{"values": v} for v in INVOICE]}, + {"kind": "create_bill", "parameters": ["customer", "ref"], + "examples": [{"values": v} for v in BILL]}, + {"kind": "create_note", "parameters": ["title"], + "examples": [{"values": v} for v in NOTE]}, {"kind": "create_task", "parameters": ["project", "title"], "examples": [{"values": v} for v in TASK]}, ] @@ -155,6 +180,10 @@ def demo(kind: str, values: dict[str, Any], write: Callable[[], Any]) -> None: demo("create_account", v, lambda v=v: app.account(v)) for v in INVOICE: demo("create_invoice", v, lambda v=v: app.invoice(v)) + for v in BILL: + demo("create_bill", v, lambda v=v: app.bill(v)) + for v in NOTE: + demo("create_note", v, lambda v=v: app.note(v)) for v in TASK: demo("create_task", v, lambda v=v: app.task(v)) argv = ["propose", *common, "--kinds", str(work / "kinds.yaml"), "--demos", str(work / "demos"), @@ -238,3 +267,37 @@ def test_an_actor_id_that_is_also_a_copy_is_asked(shop: dict[str, Any]) -> None: assert "tasks.assignee_id (the acting user's id or a copy of" in shop["questions"] refused = shop["report"]["kinds"]["create_task"]["refused"] assert any("tasks.assignee_id" in r for r in refused), refused + + +# -- review round 2 ------------------------------------------------------------ + + +def test_a_constant_equal_to_its_source_in_every_demo_is_asked( + shop: dict[str, Any], +) -> None: + """The app stores status 5 on every bill; both demonstrated customers + happen to have status 5. A constant and a copy both fit: a question, + and no contract until the person answers (never a silent copy).""" + + assert "bills.status (a constant or a copy of customers.status" in shop["questions"] + refused = shop["report"]["kinds"]["create_bill"]["refused"] + assert any("bills.status" in r for r in refused), refused + # Customer 4's status is 6: under a copy, a wrong 6 would have passed. + v = {"customer": "Umbrella", "ref": "B-9"} + out = _run(shop, "create_bill", v, lambda: shop["app"].bill(v, status=6)) + assert not out.startswith("CONFIRMED"), out + + +def test_the_own_key_template_is_checked(shop: dict[str, Any]) -> None: + """``code`` holds the row's own key as ``NT-{key}`` in both + demonstrations: the contract checks that text exactly.""" + + v = {"title": "Retro"} + out = _run(shop, "create_note", v, lambda: shop["app"].note(v)) + assert out.startswith("CONFIRMED"), out + v = {"title": "Plan"} + wrong = _run(shop, "create_note", v, lambda: shop["app"].note(v, code="NT-999")) + assert not wrong.startswith("CONFIRMED"), wrong + v = {"title": "Plan B"} + case = _run(shop, "create_note", v, lambda: shop["app"].note(v, code="nt-0")) + assert not case.startswith("CONFIRMED"), case From b6066de5e4e0d92fd67794928c3afa17c215c035 Mon Sep 17 00:00:00 2001 From: abrichr Date: Mon, 2 Nov 2026 20:00:54 -0500 Subject: [PATCH 7/7] Generator coincidences: review round 3, drop the own-key template check Revert the round-2 template check: a column that holds the row's own key (or reads like it) is listed in the question again, never checked or left open on its own. It is no longer exempt from the field-set guard either. The constant-or-copy question and counting demonstrations stay. Tests: prices 15 and 25 that read as {key}5 are asked, not templated, and the app's real price passes once named; an audit row AUD-{id} beside the write can be named under unchecked, and a correct write is CONFIRMED. --- packages/flow/docs/GENERATE_CONTRACTS.md | 3 +- .../generate/verify_contract.py | 25 ++---- .../tests/test_generate_coincidences_r1.py | 88 ++++++++++++++----- 3 files changed, 72 insertions(+), 44 deletions(-) diff --git a/packages/flow/docs/GENERATE_CONTRACTS.md b/packages/flow/docs/GENERATE_CONTRACTS.md index 6a57f0cf0..ef9ec54e6 100644 --- a/packages/flow/docs/GENERATE_CONTRACTS.md +++ b/packages/flow/docs/GENERATE_CONTRACTS.md @@ -217,7 +217,8 @@ link to a row the write added, the year, month, week or day of a date the task gives, and a generated token (a UUID) are recognized as such. A link and a column that holds the row's own key need two demonstrations; the own key must sit in the same text in every row (`INV-{key}`), never as a mere -substring, and the contract checks that text exactly. A date part must differ between the demonstrations: a constant +substring. Such a column is listed in the question, never checked or +left open on its own. A date part must differ between the demonstrations: a constant that happens to equal the month of every demonstration's date is a constant. A copy is taken only through a declared key or a column named after its table, and only when the source differs between demonstrations: diff --git a/packages/flow/openadapt_flow/generate/verify_contract.py b/packages/flow/openadapt_flow/generate/verify_contract.py index 397bfff6d..a997d2867 100644 --- a/packages/flow/openadapt_flow/generate/verify_contract.py +++ b/packages/flow/openadapt_flow/generate/verify_contract.py @@ -80,9 +80,10 @@ class Unpinned(ValueError): def _computed(role: ColumnRole) -> bool: - """A column the app computes from the row (the day of a date the task - gives, a text holding the row's own key): recognized, so not pinned and - not asked about.""" + """A column the app computes from a date the task gives (its day, its + month): recognized, so a field set whose own demonstrations leave it open + still gets a contract. A text holding the row's own key is not exempt: + it is listed in the question like any other open column.""" why = str(role.detail.get("why") or "") return why.startswith("the ") and "of a date the task gives" in why @@ -2241,22 +2242,6 @@ def _pin( actor_params: Mapping[str, str], unchecked: Mapping[str, set[str]] = {}, ) -> None: - template = role.detail.get("template") if role.role == "varies" else None - if template and col not in unchecked.get(table, set()): - # The row's own key in its exact text (``INV-{key}``): checked. - prefix, suffix = str(template).split("{key}", 1) - key = b.text("t", table, b.schema.tables[table].primary_key[0]) - parts = [b.literal(prefix), key, b.literal(suffix)] - whole = ( - "CONCAT(" + ", ".join(parts) + ")" if b.d == "mysql" else " || ".join(parts) - ) - stored = b.text("t", table, col) - if b.d == "mysql": # compare bytes: the collation may fold case - stored, whole = f"CAST({stored} AS BINARY)", f"CAST({whole} AS BINARY)" - expr = f"CASE WHEN {stored} = {whole} THEN '1' ELSE '0' END" - alias = _add_column(surf, f"pin__{_alias(col)}", expr) - match[alias] = {"literal": "1"} - return if ( role.role == "varies" and col not in unchecked.get(table, set()) @@ -3159,7 +3144,7 @@ def table_cols(key: str) -> dict[str, set[str]]: groups += [(sd.table, sd.roles) for sd in p.sides] for table, group_roles in groups: for col, role in group_roles.items(): - if role.role in OPEN_ROLES and not role.detail.get("template"): + if role.role in OPEN_ROLES: own.setdefault(table, {})[col] = role.role reason = str(role.detail.get("why") or "") if "one demonstration" in reason or "can't tell" in reason: diff --git a/packages/flow/tests/test_generate_coincidences_r1.py b/packages/flow/tests/test_generate_coincidences_r1.py index 8d5e9d965..f5f3bfc02 100644 --- a/packages/flow/tests/test_generate_coincidences_r1.py +++ b/packages/flow/tests/test_generate_coincidences_r1.py @@ -53,8 +53,13 @@ ref text NOT NULL, status integer NOT NULL); INSERT INTO customers (name, status) VALUES ('Acme', 5), ('Globex', 5), ('Initech', 6), ('Umbrella', 6); -CREATE TABLE notes (id serial PRIMARY KEY, title text NOT NULL, code text NOT NULL); -SELECT setval('notes_id_seq', 30); +CREATE TABLE products (id serial PRIMARY KEY, name text NOT NULL, price integer NOT NULL); +INSERT INTO products (name, price) VALUES ('Widget', 15), ('Gadget', 25), ('Doohickey', 40); +CREATE TABLE sales (id serial PRIMARY KEY, product_id integer NOT NULL, + ref text NOT NULL, price integer NOT NULL); +CREATE TABLE jobs (id serial PRIMARY KEY, title text NOT NULL); +CREATE TABLE job_audit (id serial PRIMARY KEY, job_id integer NOT NULL, ref text NOT NULL); +SELECT setval('jobs_id_seq', 50); GRANT SELECT ON ALL TABLES IN SCHEMA public TO oa_reader; GRANT SELECT ON ALL SEQUENCES IN SCHEMA public TO oa_reader; """ @@ -66,12 +71,21 @@ def bill(self, v: dict[str, Any], status: int = 5) -> None: self.q("INSERT INTO bills (customer_id, ref, status) VALUES (%s,%s,%s)", (cid, v["ref"], status)) # fmt: skip - def note(self, v: dict[str, Any], code: Any = None) -> None: - nid = self.one( - "INSERT INTO notes (title, code) VALUES (%s, '') RETURNING id", - (v["title"],), + def sale(self, v: dict[str, Any], price: Any = None) -> None: + pid, cost = self.q( + "SELECT id, price FROM products WHERE name=%s", (v["product"],) + )[0] + self.q("INSERT INTO sales (product_id, ref, price) VALUES (%s,%s,%s)", + (pid, v["ref"], cost if price is None else price)) # fmt: skip + + def job(self, v: dict[str, Any]) -> None: + jid = self.one( + "INSERT INTO jobs (title) VALUES (%s) RETURNING id", (v["title"],) + ) + aid = self.one( + "INSERT INTO job_audit (job_id, ref) VALUES (%s, '') RETURNING id", (jid,) ) - self.q("UPDATE notes SET code=%s WHERE id=%s", (code or f"NT-{nid}", nid)) + self.q("UPDATE job_audit SET ref=%s WHERE id=%s", (f"AUD-{aid}", aid)) def po(self, v: dict[str, Any]) -> None: oid = self.one( @@ -119,7 +133,8 @@ def task(self, v: dict[str, Any]) -> None: {"customer": "Globex", "note": None, "tag": "q4", "due": "2026-11-20"}, # id 4 ] BILL = [{"customer": "Acme", "ref": "B-1"}, {"customer": "Globex", "ref": "B-2"}] -NOTE = [{"title": "Kickoff"}, {"title": "Review"}] +SALE = [{"product": "Widget", "ref": "S-1"}, {"product": "Gadget", "ref": "S-2"}] +JOB = [{"title": "Kickoff"}, {"title": "Review"}] TASK = [ {"project": "Apollo", "title": "Kickoff"}, {"project": "Gemini", "title": "Review"}, @@ -134,8 +149,10 @@ def task(self, v: dict[str, Any]) -> None: "examples": [{"values": v} for v in INVOICE]}, {"kind": "create_bill", "parameters": ["customer", "ref"], "examples": [{"values": v} for v in BILL]}, - {"kind": "create_note", "parameters": ["title"], - "examples": [{"values": v} for v in NOTE]}, + {"kind": "create_sale", "parameters": ["product", "ref"], + "examples": [{"values": v} for v in SALE]}, + {"kind": "create_job", "parameters": ["title"], + "examples": [{"values": v} for v in JOB]}, {"kind": "create_task", "parameters": ["project", "title"], "examples": [{"values": v} for v in TASK]}, ] @@ -182,8 +199,10 @@ def demo(kind: str, values: dict[str, Any], write: Callable[[], Any]) -> None: demo("create_invoice", v, lambda v=v: app.invoice(v)) for v in BILL: demo("create_bill", v, lambda v=v: app.bill(v)) - for v in NOTE: - demo("create_note", v, lambda v=v: app.note(v)) + for v in SALE: + demo("create_sale", v, lambda v=v: app.sale(v)) + for v in JOB: + demo("create_job", v, lambda v=v: app.job(v)) for v in TASK: demo("create_task", v, lambda v=v: app.task(v)) argv = ["propose", *common, "--kinds", str(work / "kinds.yaml"), "--demos", str(work / "demos"), @@ -192,10 +211,20 @@ def demo(kind: str, values: dict[str, Any], write: Callable[[], Any]) -> None: first = json.loads((work / "out" / "report.json").read_text()) questions = (work / "out" / "QUESTIONS.md").read_text() answers = {q["key"]: q["proposal"] for q in first["questions"]} + # The person names the two columns that hold a row's own key, or what + # reads like one (a sale's price, an audit row's reference). + bk = answers["bookkeeping"] + bk = json.loads(json.dumps(bk)) if isinstance(bk, dict) else {"accept": True} + named = bk.setdefault("unchecked", {}) + for table, col in (("sales", "price"), ("job_audit", "ref")): + if col not in named.setdefault(table, []): + named[table].append(col) + answers["bookkeeping"] = bk (work / "answers.yaml").write_text(yaml.safe_dump(answers)) assert gen.main([*argv, "--answers", str(work / "answers.yaml")]) == 0 report = json.loads((work / "out" / "report.json").read_text()) - return {"app": app, "out": work / "out", "report": report, "questions": questions} + return {"app": app, "out": work / "out", "report": report, "first": first, + "questions": questions} # fmt: skip def _run( @@ -288,16 +317,29 @@ def test_a_constant_equal_to_its_source_in_every_demo_is_asked( assert not out.startswith("CONFIRMED"), out -def test_the_own_key_template_is_checked(shop: dict[str, Any]) -> None: - """``code`` holds the row's own key as ``NT-{key}`` in both - demonstrations: the contract checks that text exactly.""" +# -- review round 3 ------------------------------------------------------------ + + +def test_a_value_that_reads_as_the_own_key_is_asked_never_templated( + shop: dict[str, Any], +) -> None: + """Sales 1 and 2 copied prices 15 and 25, which read as ``{key}5``. The + column is listed in the question; nothing checks that template, so the + app's real price for sale 3 (40) passes once the person names it.""" + + assert "sales: " in shop["questions"] and "price" in shop["questions"] + refused_first = shop["first"]["kinds"]["create_sale"]["refused"] + assert any("sales.price" in r for r in refused_first), refused_first + v = {"product": "Doohickey", "ref": "S-3"} + out = _run(shop, "create_sale", v, lambda: shop["app"].sale(v)) + assert out.startswith("CONFIRMED"), out + +def test_an_audit_row_holding_its_own_key_can_be_named(shop: dict[str, Any]) -> None: + """The app adds an audit row ``AUD-{id}`` beside each job. The question + lists it, the answer names it, and a correct job is CONFIRMED.""" + + assert "job_audit" in shop["questions"] v = {"title": "Retro"} - out = _run(shop, "create_note", v, lambda: shop["app"].note(v)) + out = _run(shop, "create_job", v, lambda: shop["app"].job(v)) assert out.startswith("CONFIRMED"), out - v = {"title": "Plan"} - wrong = _run(shop, "create_note", v, lambda: shop["app"].note(v, code="NT-999")) - assert not wrong.startswith("CONFIRMED"), wrong - v = {"title": "Plan B"} - case = _run(shop, "create_note", v, lambda: shop["app"].note(v, code="nt-0")) - assert not case.startswith("CONFIRMED"), case