diff --git a/CAPABILITIES.md b/CAPABILITIES.md index 2044d0ad9c..bad55f95f5 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 (it compares the contract's own tables too, so an excused column holds only on the rows the write declares, and the recheck sees a late change to a look-alike row), 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 (it compares the contract's own tables too, so an excused column holds only on the rows the write declares, and the recheck sees a late change to a look-alike row), 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) | diff --git a/packages/flow/docs/GENERATE_CONTRACTS.md b/packages/flow/docs/GENERATE_CONTRACTS.md index 3e07fcec29..46266ed338 100644 --- a/packages/flow/docs/GENERATE_CONTRACTS.md +++ b/packages/flow/docs/GENERATE_CONTRACTS.md @@ -147,7 +147,28 @@ 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 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 + 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. @@ -193,7 +214,16 @@ 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. 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: +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 @@ -205,6 +235,20 @@ 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, 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 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 3f54acd89f..db250ea0bd 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 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 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 own_key(child, col.name, key): + continue # the child's own key: equal by chance + walk( child, ch.after, @@ -462,11 +479,48 @@ 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, v) + } out[ch.table] = out[ch.table] & cols if ch.table in out else cols return out +#: 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 + 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 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) + + def best(locations: Sequence[Location]) -> tuple[Optional[Location], list[Location]]: """The single best location, or None and the tied ones.""" @@ -584,42 +638,78 @@ 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. + # 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() if values and all(v is not None and v == av for v in values) ] - if actor: + 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: + 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 = _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"} ) 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 @@ -638,6 +728,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 @@ -656,6 +751,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]: @@ -663,6 +781,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, @@ -715,9 +837,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 3febfebc7c..5fde292f34 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,23 @@ 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.""" + + +def _computed(role: ColumnRole) -> bool: + """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 + + class GenerateError(RuntimeError): """The generator can't go on; the message says why.""" @@ -268,6 +286,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 +319,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 +413,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 +461,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 +502,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 +784,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({id(d) for d, _ in 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 +827,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 +862,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, v) + } + 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 +885,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 +922,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 +943,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 +1027,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({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 + ) + 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 @@ -2123,6 +2259,13 @@ 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()) + 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()): # Empty in every demonstration and not named as unchecked: it stays # empty. @@ -2283,6 +2426,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 @@ -2993,21 +3146,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, group_roles in groups: + for col, role in group_roles.items(): if role.role in OPEN_ROLES: - own.setdefault(sd.table, {})[col] = role.role + own.setdefault(table, {})[col] = role.role + 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(): @@ -3017,7 +3175,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) @@ -3121,6 +3279,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", @@ -3293,9 +3459,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 0000000000..aeeb8cc766 --- /dev/null +++ b/packages/flow/tests/test_generate_coincidences.py @@ -0,0 +1,255 @@ +# 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'), ('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), + ('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: + # 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"], 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) " + "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_coincidences_r1.py b/packages/flow/tests/test_generate_coincidences_r1.py new file mode 100644 index 0000000000..f5f3bfc028 --- /dev/null +++ b/packages/flow/tests/test_generate_coincidences_r1.py @@ -0,0 +1,345 @@ +# 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); +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 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; +""" + + +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 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 job_audit SET ref=%s WHERE id=%s", (f"AUD-{aid}", aid)) + + 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 +] +BILL = [{"customer": "Acme", "ref": "B-1"}, {"customer": "Globex", "ref": "B-2"}] +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"}, +] +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_bill", "parameters": ["customer", "ref"], + "examples": [{"values": v} for v in BILL]}, + {"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]}, + ] +} # 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 BILL: + demo("create_bill", v, lambda v=v: app.bill(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"), + "--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 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, "first": first, + "questions": questions} # fmt: skip + + +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 + + +# -- 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 + + +# -- 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_job", v, lambda: shop["app"].job(v)) + 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 dd46c8b0c1..ef7ae8c7e7 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", + } + }, ], }, { @@ -264,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 d51e4a5df6..ee0590f66d 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