From c5c5c005f6081168a4b45c8fe988a37f82d7508b Mon Sep 17 00:00:00 2001 From: Steven Kearnes Date: Mon, 17 Aug 2026 22:28:45 -0400 Subject: [PATCH] Score translations on the reactions they return A case states reactions any correct query returns, and reactions a plausible wrong one returns, rather than the query it expects: several spellings of a question are right, and pinning one would fail a better translation than the one written the day the case was added. The must_not_return half is what gives a case teeth -- for pyridine as a solvent it holds reactions where pyridine is a reactant and something else is the solvent, which is what a translation returns when two conditions on one component become two quantifiers. Building those counterexamples by differencing two capped result sets compares samples rather than sets, and the first attempt did exactly that: it failed both models on a correct translation, and one round of prompt tuning went into a problem that did not exist. They are found by asking for them now -- the near-miss and not the reference -- which is exact, and the builder refuses a near-miss that excludes nothing. The run also exposed a gap this closes: forcing build_query left the model no way to say a question cannot be put to this grammar, so both models built a plausible query for one that compares two columns. cannot_answer is offered beside build_query, declining raises UnanswerableError carrying the reason, and a refusal is never repaired. Co-Authored-By: Claude Opus 5 (1M context) --- ord_schema/dependencies_test.py | 2 +- ord_schema/search/README.md | 26 +++ ord_schema/search/nl.py | 42 ++++- ord_schema/search/nl_cases.json | 55 +++++++ ord_schema/search/nl_cases_build.py | 225 ++++++++++++++++++++++++++ ord_schema/search/nl_eval.py | 237 ++++++++++++++++++++++++++++ ord_schema/search/nl_eval_test.py | 95 +++++++++++ ord_schema/search/nl_prompt.md | 25 ++- ord_schema/search/nl_test.py | 60 +++++-- pyproject.toml | 2 +- 10 files changed, 752 insertions(+), 17 deletions(-) create mode 100644 ord_schema/search/nl_cases.json create mode 100644 ord_schema/search/nl_cases_build.py create mode 100644 ord_schema/search/nl_eval.py create mode 100644 ord_schema/search/nl_eval_test.py diff --git a/ord_schema/dependencies_test.py b/ord_schema/dependencies_test.py index fa8e5d88..9699839a 100644 --- a/ord_schema/dependencies_test.py +++ b/ord_schema/dependencies_test.py @@ -43,7 +43,7 @@ # The search subpackage reads through the artifacts, whose imports the search extra # therefore has to carry. "search": ("ord_schema.search", "ord_schema.artifacts"), - "nl": ("ord_schema.search.nl",), + "nl": ("ord_schema.search.nl", "ord_schema.search.nl_eval"), "orm": ("ord_schema.orm",), "huggingface": ("ord_schema.huggingface",), } diff --git a/ord_schema/search/README.md b/ord_schema/search/README.md index 729f589b..cbcd4e2b 100644 --- a/ord_schema/search/README.md +++ b/ord_schema/search/README.md @@ -454,10 +454,36 @@ rather than guaranteed: the predicate tree usually arrives JSON-encoded in a str coerced back, a query that does not compile is handed back once carrying the compiler's own "did you mean", and a second failure raises `MalformedQueryError`. +The model is also given a way to decline. Forcing `build_query` would leave it no way to +say a question cannot be put to this grammar — comparing two columns, say — and a model +with no way to decline invents a query rather than refusing, which is the failure that +looks most like an answer. Declining raises `UnanswerableError`, carrying the model's +reason, and is never repaired: nothing was wrong with its reasoning. + The ~15k-token prefix — these rules plus `describe()` plus the grammar — is cached, which is most of what a query costs. `answer.query` is the query that ran, so a caller can show what was searched and offer to run it again. +### Measure how good a translation is + +```bash +python -m ord_schema.search.nl_eval \ + --projections 'projections/**/*.parquet' \ + --structures 'structures/**/*.parquet' \ + --model claude-haiku-4-5 +``` + +A case states **reactions any correct query returns** and reactions a plausible wrong one +returns, never the query it expects: several spellings of a question are right, and pinning +one would fail a better translation than the one written the day the case was added. The +`must_not_return` half is what gives a case teeth — for "pyridine as a solvent" it holds +reactions where pyridine is a reactant and something else is the solvent, which is what +comes back when two conditions on one component become two quantifiers. + +Cases carry a `why`, printed with any failure, and one is marked `compiles: false`: a +question the grammar cannot express, which the layer has to refuse rather than answer +approximately. The reaction IDs come from the corpus the cases were built against. + ### Tell the model what it may query ```python diff --git a/ord_schema/search/nl.py b/ord_schema/search/nl.py index 83b0ff26..ad6edc7f 100644 --- a/ord_schema/search/nl.py +++ b/ord_schema/search/nl.py @@ -73,6 +73,25 @@ "description": "Build an ORD search query from the user's question.", "input_schema": query.Query.model_json_schema(), } +# Forcing build_query would leave a model with no way to decline, and a model with no +# way to decline invents a query rather than refusing. This is that way. +REFUSAL_TOOL: ToolParam = { + "name": "cannot_answer", + "description": ( + "Say that the question cannot be expressed in this grammar. Use it rather than " + "building a query that means something else." + ), + "input_schema": { + "type": "object", + "properties": { + "reason": { + "type": "string", + "description": "What the question asks for that the grammar lacks.", + } + }, + "required": ["reason"], + }, +} _SYSTEM: list[TextBlockParam] = [ { "type": "text", @@ -102,6 +121,16 @@ class MalformedQueryError(NLQueryError): """The model's query did not compile, and neither did its repair.""" +class UnanswerableError(NLQueryError): + """The question cannot be put to this grammar, and the model said so. + + Distinct from a malformed query: nothing was wrong with the model's reasoning, and + retrying will not help. Comparing two columns is the standard case -- a value is a + literal or a compound, never another column -- and a layer without a way to say so + answers with a plausible query that means something else. + """ + + @dataclasses.dataclass(frozen=True) class Answer: """What a question produced, including the query it became. @@ -193,8 +222,10 @@ def _call( max_tokens=MAX_TOKENS, system=_SYSTEM, messages=messages, - tools=[TOOL], - tool_choice={"type": "tool", "name": "build_query"}, + tools=[TOOL, REFUSAL_TOOL], + # "any" rather than "tool": the model must call one of them, which leaves + # refusing available without leaving prose available. + tool_choice={"type": "any"}, ) except anthropic.RateLimitError as error: raise ModelRateLimitedError(str(error)) from error @@ -202,6 +233,11 @@ def _call( raise ModelUnavailableError(str(error)) from error for block in response.content: if isinstance(block, ToolUseBlock): + if block.name == REFUSAL_TOOL["name"]: + reason = "no reason given" + if isinstance(block.input, dict): + reason = str(block.input.get("reason", reason)) + raise UnanswerableError(reason) return block raise MalformedQueryError("the model returned no query") @@ -228,6 +264,7 @@ def translate( Raises: MalformedQueryError: If the query does not compile, after the repair turn where one was allowed. + UnanswerableError: If the model says the grammar cannot express the question. ModelRateLimitedError: If the caller is over its rate limit. ModelUnavailableError: If the model cannot be reached. """ @@ -363,6 +400,7 @@ def ask( Raises: MalformedQueryError: If translation produces nothing that compiles. + UnanswerableError: If the grammar cannot express the question. ModelRateLimitedError: If the caller is over its rate limit. ModelUnavailableError: If the model cannot be reached. """ diff --git a/ord_schema/search/nl_cases.json b/ord_schema/search/nl_cases.json new file mode 100644 index 00000000..9d393a65 --- /dev/null +++ b/ord_schema/search/nl_cases.json @@ -0,0 +1,55 @@ +[ + { + "question": "which reactions use pyridine as a solvent?", + "why": "two conditions on one element, which a wrong translation splits in two", + "must_return": [ + "ord-00533d2621284180b8d6e372c9496cb7", + "ord-02572d7994bc44c39cdf4b16503d331d", + "ord-03179abb6e0a461e9c57bc862974a3e0" + ], + "must_not_return": [ + "ord-9c0049ffd5bd4c2597bd8dc1da4f9125", + "ord-c218088b16c541088a933a7606f68c4c" + ] + }, + { + "question": "reactions run above 350 K", + "why": "a scalar comparison needing no quantifier at all", + "must_return": [ + "ord-01e7218d0d9042f6bb980d3c55b949be", + "ord-039db69849d0448baa4933c8edcbfd3d", + "ord-052ce82d4b04486e8815d005d6897bba" + ], + "must_not_return": [] + }, + { + "question": "reactions where a desired product has a yield above 50%", + "why": "correlation: the yield has to belong to the desired product, not to whichever product happens to carry one", + "must_return": [ + "ord-004a7e7ef4f248aea8ceb23af8212300", + "ord-01e04eeb2be04ecb80d130e0c31917fe", + "ord-02e987dd13d54efaac154a47fe44f7e2" + ], + "must_not_return": [ + "ord-038119b8b4624c1a8ebfcc86cea84ff6", + "ord-c867f2f5ee844d8099373f8a271335f5" + ] + }, + { + "question": "reactions with no solvent at all", + "why": "a forall, which the occurrence index cannot answer", + "must_return": [ + "ord-005bd91bc77a4958a35aa743cdeadabd", + "ord-01123dbb1eea4f0ca582cd558fc37de7", + "ord-0197aeb8022e45b88d3d52d2b1539148" + ], + "must_not_return": [] + }, + { + "question": "reactions that ran longer than their workup took", + "why": "comparing two columns, which the grammar cannot express: a value is a literal or a compound, never another column", + "compiles": false, + "must_return": [], + "must_not_return": [] + } +] diff --git a/ord_schema/search/nl_cases_build.py b/ord_schema/search/nl_cases_build.py new file mode 100644 index 00000000..6b9ff2c3 --- /dev/null +++ b/ord_schema/search/nl_cases_build.py @@ -0,0 +1,225 @@ +# Copyright 2026 Open Reaction Database Project Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Builds the eval cases, taking reaction IDs from a corpus rather than inventing them. + +Each case names a reference query -- what a correct translation looks like -- and, where +one exists, the near-miss a model plausibly writes instead. Reactions the reference +returns become ``must_return``. Reactions the near-miss returns *and the reference does +not* become ``must_not_return``, and those are what make a wrong translation fail rather +than merely differ. + +The counterexamples are found by asking for them -- ``near_miss AND NOT reference`` -- +rather than by differencing two capped result sets, which compares samples instead of +sets and produced two cases that failed correct translations. + +Rerun this when the corpus changes; the IDs are only meaningful against the corpus they +were drawn from. +""" + +import argparse +import json +import pathlib +from collections.abc import Sequence +from typing import Any + +from ord_schema.search import execute, query + +PYRIDINE = {"op": "eq", "path": "smiles", "value": {"compound": "pyridine"}} +SOLVENT = {"op": "eq", "path": "reaction_role", "value": {"literal": "SOLVENT"}} +DESIRED = {"op": "eq", "path": "is_desired_product", "value": {"literal": True}} +YIELD_OVER_50 = { + "op": "and", + "clauses": [ + {"op": "eq", "path": "type", "value": {"literal": "YIELD"}}, + {"op": "gt", "path": "percentage.value", "value": {"literal": 50}}, + ], +} + +CASES: list[dict[str, Any]] = [ + { + "question": "which reactions use pyridine as a solvent?", + "why": "two conditions on one element, which a wrong translation splits in two", + "reference": { + "op": "exists", + "path": "inputs.components", + "where": {"op": "and", "clauses": [SOLVENT, PYRIDINE]}, + }, + # Pyridine somewhere and a solvent somewhere, which need not be the same + # component: the reaction using pyridine as a reactant in toluene matches. + "near_miss": { + "op": "and", + "clauses": [ + {"op": "exists", "path": "inputs.components", "where": PYRIDINE}, + {"op": "exists", "path": "inputs.components", "where": SOLVENT}, + ], + }, + }, + { + "question": "reactions run above 350 K", + "why": "a scalar comparison needing no quantifier at all", + "reference": { + "op": "gt", + "path": "conditions.temperature.setpoint_kelvin", + "value": {"literal": 350}, + }, + "near_miss": None, + }, + { + "question": "reactions where a desired product has a yield above 50%", + "why": ( + "correlation: the yield has to belong to the desired product, not to " + "whichever product happens to carry one" + ), + "reference": { + "op": "exists", + "path": "outcomes.products", + "where": { + "op": "and", + "clauses": [ + DESIRED, + {"op": "exists", "path": "measurements", "where": YIELD_OVER_50}, + ], + }, + }, + "near_miss": { + "op": "and", + "clauses": [ + {"op": "exists", "path": "outcomes.products", "where": DESIRED}, + { + "op": "exists", + "path": "outcomes.products.measurements", + "where": YIELD_OVER_50, + }, + ], + }, + }, + { + "question": "reactions with no solvent at all", + "why": "a forall, which the occurrence index cannot answer", + "reference": { + "op": "forall", + "path": "inputs.components", + "where": { + "op": "ne", + "path": "reaction_role", + "value": {"literal": "SOLVENT"}, + }, + }, + "near_miss": None, + }, +] + +# A question the grammar cannot express: a value is a literal or a compound, never +# another column, so no comparison between two columns can be written. +INEXPRESSIBLE = { + "question": "reactions that ran longer than their workup took", + "why": ( + "comparing two columns, which the grammar cannot express: a value is a literal " + "or a compound, never another column" + ), + "compiles": False, + "must_return": [], + "must_not_return": [], +} + + +def build(corpus: execute.Corpus, *, examples: int = 3) -> list[dict[str, Any]]: + """Returns the cases, with reaction IDs drawn from ``corpus``. + + Args: + corpus: The corpus to draw IDs from. + examples: How many reactions to require of a correct translation. + + Returns: + Cases ready to serialize. + """ + built = [] + for case in CASES: + rows = corpus.search( + query.Query.model_validate({"where": case["reference"], "limit": 200}) + ) + right = sorted(rows.column("reaction_id").to_pylist()) + wrong: list[str] = [] + if case["near_miss"] is not None: + counterexamples = corpus.search( + query.Query.model_validate( + { + "where": { + "op": "and", + "clauses": [ + case["near_miss"], + {"op": "not", "clause": case["reference"]}, + ], + }, + "limit": 2, + } + ) + ) + wrong = sorted(counterexamples.column("reaction_id").to_pylist()) + if not wrong: + raise ValueError( + f"{case['question']}: the near-miss returns nothing the reference " + f"excludes, so it is not a near-miss" + ) + built.append( + { + "question": case["question"], + "why": case["why"], + "must_return": right[:examples], + "must_not_return": wrong, + } + ) + built.append(INEXPRESSIBLE) + return built + + +def main(argv: Sequence[str] | None = None) -> None: + """Writes the cases file. + + Args: + argv: Command-line arguments; ``sys.argv`` when omitted. + """ + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--projections", required=True, help="Glob for projections") + parser.add_argument("--structures", required=True, help="Glob for structures") + parser.add_argument( + "--output", + type=pathlib.Path, + default=pathlib.Path(__file__).parent / "nl_cases.json", + help="Where to write the cases", + ) + parser.add_argument( + "--require-current", + action="store_true", + help="Refuse artifacts not written by this version of the library", + ) + args = parser.parse_args(argv) + with execute.Corpus( + args.projections, + args.structures, + require_current=args.require_current, + pivot_budget_bytes=0, + # The only name any reference query resolves, kept local so building the cases + # needs no external service. + resolver={"pyridine": "c1ccncc1"}.__getitem__, + ) as corpus: + cases = build(corpus) + with args.output.open("w", encoding="utf-8") as handle: + json.dump(cases, handle, indent=2) + handle.write("\n") + + +if __name__ == "__main__": + main() diff --git a/ord_schema/search/nl_eval.py b/ord_schema/search/nl_eval.py new file mode 100644 index 00000000..355b5e72 --- /dev/null +++ b/ord_schema/search/nl_eval.py @@ -0,0 +1,237 @@ +# Copyright 2026 Open Reaction Database Project Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Scoring translations by the reactions they return. + +A case does not state the query it expects, because several spellings of a question are +right and pinning one would fail a better translation than the one written the day the +case was added. It states reactions any correct query returns, and reactions a plausible +wrong one returns -- usually the near-miss where two conditions on the same element +become two quantifiers, which matches a solvent here and a reactant there. + +Running the cases costs money and reaches the network, so nothing in the suite calls +``run_case``. The suite covers the scoring; the measurement is a command. +""" + +import argparse +import dataclasses +import json +import pathlib +from collections.abc import Sequence + +import anthropic +from pydantic import BaseModel, Field + +from ord_schema.logging import get_logger +from ord_schema.search import execute, nl + +logger = get_logger(__name__) + +CASES = pathlib.Path(__file__).parent / "nl_cases.json" + + +class EvalCase(BaseModel): + """One question, and what any correct answer to it must satisfy. + + Attributes: + question: The question, in English. + why: What this case is here to catch, for whoever reads a failure. + compiles: Whether the question should translate at all. False marks one the + grammar cannot express, which the layer must refuse rather than fudge. + must_return: Reaction IDs any correct query returns. + must_not_return: Reaction IDs a plausible wrong query returns and a correct one + does not. + """ + + question: str + why: str + compiles: bool = True + must_return: list[str] = Field(default_factory=list) + must_not_return: list[str] = Field(default_factory=list) + + +@dataclasses.dataclass(frozen=True) +class CaseResult: + """How one case came out. + + Attributes: + case: The case that ran. + passed: Whether it was satisfied. + detail: What happened, in a line, whether it passed or not. + """ + + case: EvalCase + passed: bool + detail: str + + +def load_cases(path: pathlib.Path = CASES) -> list[EvalCase]: + """Returns the cases a file holds. + + Args: + path: A JSON file holding a list of cases. + + Returns: + The parsed cases. + """ + with path.open(encoding="utf-8") as handle: + return [EvalCase.model_validate(entry) for entry in json.load(handle)] + + +def score(case: EvalCase, returned: Sequence[str]) -> CaseResult: + """Returns whether the reactions a query returned satisfy a case. + + Args: + case: The case. + returned: Reaction IDs the translated query returned. + + Returns: + The result, naming what was missing or wrongly present. + """ + found = set(returned) + missing = [value for value in case.must_return if value not in found] + forbidden = [value for value in case.must_not_return if value in found] + if missing: + return CaseResult(case, passed=False, detail=f"must_return absent: {missing}") + if forbidden: + return CaseResult( + case, passed=False, detail=f"must_not_return present: {forbidden}" + ) + return CaseResult(case, passed=True, detail=f"{len(found)} reactions") + + +def run_case( + case: EvalCase, + corpus: execute.Corpus, + *, + client: anthropic.Anthropic, + model: str, + repair: bool, +) -> CaseResult: + """Translates one case, runs it, and scores what came back. + + Args: + case: The case to run. + corpus: The corpus to search. + client: Anthropic client. + model: Which model translates. + repair: Whether a failure gets the repair turn. + + Returns: + The result. A case marked ``compiles: false`` passes exactly when translation + fails, since refusing what the grammar cannot express is the right answer. + """ + try: + translated = nl.translate( + case.question, client=client, model=model, repair=repair + ) + except nl.UnanswerableError as error: + return CaseResult(case, passed=not case.compiles, detail=f"declined: {error}") + except nl.MalformedQueryError as error: + return CaseResult( + case, passed=not case.compiles, detail=f"did not compile: {error}" + ) + if not case.compiles: + return CaseResult( + case, passed=False, detail="compiled, but the grammar cannot express this" + ) + table = corpus.search(translated) + return score(case, table.column("reaction_id").to_pylist()) + + +def report(results: Sequence[CaseResult]) -> str: + """Returns a summary of a run, failures spelled out. + + Args: + results: What the cases produced. + + Returns: + A count, then a line per failure naming the question and what went wrong. + """ + passed = sum(result.passed for result in results) + lines = [f"{passed}/{len(results)} passed"] + lines += [ + f" FAIL {result.case.question}\n" + f" {result.detail}\n" + f" (this case exists to catch: {result.case.why})" + for result in results + if not result.passed + ] + return "\n".join(lines) + + +def main(argv: Sequence[str] | None = None) -> None: + """Runs the cases against a real model and prints the report. + + Args: + argv: Command-line arguments; ``sys.argv`` when omitted. + """ + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--projections", required=True, help="Glob for projections") + parser.add_argument("--structures", required=True, help="Glob for structures") + parser.add_argument("--model", default=nl.DEFAULT_MODEL, help="Model to translate") + parser.add_argument("--cases", type=pathlib.Path, default=CASES, help="Cases file") + parser.add_argument( + "--no-repair", + action="store_true", + help="Measure first-try accuracy, without handing failures back", + ) + parser.add_argument( + "--pivots-dir", + default=None, + help="Directory of derived pivot artifacts, read rather than built", + ) + parser.add_argument( + "--pivot-budget-bytes", + type=int, + default=0, + help=( + "What pivots built in process may hold. Zero by default: a run is a " + "handful of queries, and building a pivot over the whole corpus costs " + "minutes to save milliseconds" + ), + ) + parser.add_argument( + "--require-current", + action="store_true", + help="Refuse artifacts not written by this version of the library", + ) + args = parser.parse_args(argv) + cases = load_cases(args.cases) + client = nl.get_client() + with execute.Corpus( + args.projections, + args.structures, + require_current=args.require_current, + pivots_dir=args.pivots_dir, + pivot_budget_bytes=args.pivot_budget_bytes, + ) as corpus: + results = [ + run_case( + case, + corpus, + client=client, + model=args.model, + repair=not args.no_repair, + ) + for case in cases + ] + logger.info( + "%s on %s", "with repair" if not args.no_repair else "first try", args.model + ) + print(report(results)) + + +if __name__ == "__main__": + main() diff --git a/ord_schema/search/nl_eval_test.py b/ord_schema/search/nl_eval_test.py new file mode 100644 index 00000000..5e05d29a --- /dev/null +++ b/ord_schema/search/nl_eval_test.py @@ -0,0 +1,95 @@ +# Copyright 2026 Open Reaction Database Project Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Tests for ord_schema.search.nl_eval. + +The scoring is what these cover. Running a case reaches a model and a corpus, which is a +command rather than a test. +""" + +import json + +import pytest + +from ord_schema.search import nl_eval + +_CASE = nl_eval.EvalCase( + question="which reactions use pyridine as a solvent?", + why="two conditions on one element, which a wrong translation splits in two", + must_return=["ord-aa", "ord-bb"], + must_not_return=["ord-zz"], +) + + +def test_a_query_returning_what_it_must_passes(): + assert nl_eval.score(_CASE, ["ord-aa", "ord-bb", "ord-cc"]).passed + + +def test_a_query_missing_a_required_reaction_fails(): + result = nl_eval.score(_CASE, ["ord-aa"]) + assert not result.passed + assert "ord-bb" in result.detail + + +def test_a_query_returning_a_forbidden_reaction_fails(): + # The near-miss reaction is the whole point: a translation that finds pyridine in + # one component and a solvent in another is wrong rather than differently spelled. + result = nl_eval.score(_CASE, ["ord-aa", "ord-bb", "ord-zz"]) + assert not result.passed + assert "ord-zz" in result.detail + + +def test_scoring_does_not_care_about_order_or_extras(): + # Several queries are right, and they need not return the same rows in the same + # order; only the reactions named either way are pinned. + assert nl_eval.score(_CASE, ["ord-cc", "ord-bb", "ord-dd", "ord-aa"]).passed + + +def test_the_report_names_the_question_and_why_the_case_exists(): + failure = nl_eval.score(_CASE, []) + text = nl_eval.report([failure]) + assert "0/1 passed" in text + assert _CASE.question in text + assert "splits in two" in text + + +def test_the_shipped_cases_load_and_say_why_they_exist(): + cases = nl_eval.load_cases() + assert cases + for case in cases: + assert case.why + # A case with neither an expectation nor a refusal to make would pass whatever + # the model wrote. + assert case.must_return or case.must_not_return or not case.compiles + + +def test_a_case_the_grammar_cannot_express_is_marked_as_such(): + cases = nl_eval.load_cases() + assert any(not case.compiles for case in cases) + + +def test_load_cases_reads_the_file_it_is_given(tmp_path): + path = tmp_path / "cases.json" + path.write_text( + json.dumps( + [{"question": "anything?", "why": "a placeholder", "compiles": False}] + ), + encoding="utf-8", + ) + assert nl_eval.load_cases(path)[0].question == "anything?" + + +def test_a_case_needs_a_reason_to_exist(): + with pytest.raises(ValueError, match="why"): + nl_eval.EvalCase.model_validate({"question": "anything?"}) diff --git a/ord_schema/search/nl_prompt.md b/ord_schema/search/nl_prompt.md index b95dbf07..7ba0cf3b 100644 --- a/ord_schema/search/nl_prompt.md +++ b/ord_schema/search/nl_prompt.md @@ -2,6 +2,12 @@ You turn a chemist's question into one ORD search query by calling `build_query`. +If the question cannot be put to this grammar, call `cannot_answer` instead and say +what it asks for that the grammar lacks. A query that compiles but means something +else is worse than no query: comparing two columns to each other, ranking by +something the schema does not hold, or searching free prose are all reasons to +decline rather than approximate. + Rules that keep a query answerable: - Paths are dotted names from the schema below. There is no array syntax: write @@ -11,9 +17,22 @@ Rules that keep a query answerable: `exists` over `inputs.components` with `smiles` and `reaction_role` inside it, not a comparison on `inputs.components.smiles`. - Two conditions on the *same* element go inside one quantifier. Two conditions on - *different* elements are two quantifiers. "A product whose yield is above 50%" is an - `exists` over `outcomes.products` holding an `exists` over `measurements`; separate - quantifiers would match a yield belonging to some other product. + *different* elements are two quantifiers. This is the error to watch for, because both + spellings compile and only one answers the question. "A **desired** product with a + yield above 50%" means one product satisfies both, so the conditions nest: + + ```json + {"op": "exists", "path": "outcomes.products", + "where": {"op": "and", "clauses": [ + {"op": "eq", "path": "is_desired_product", "value": {"literal": true}}, + {"op": "exists", "path": "measurements", "where": {"op": "and", "clauses": [ + {"op": "eq", "path": "type", "value": {"literal": "YIELD"}}, + {"op": "gt", "path": "percentage.value", "value": {"literal": 50}}]}}]}} + ``` + + Writing those as two quantifiers side by side — one for the desired product, one for + the yield — matches a reaction whose desired product has no yield at all, as long as + some other product does. - Name compounds rather than spelling structures: `{"compound": "pyridine"}` resolves to SMILES. Reach for `substructure` with a SMARTS only when the user describes a pattern or a scaffold rather than a molecule. diff --git a/ord_schema/search/nl_test.py b/ord_schema/search/nl_test.py index e94336f4..73ff5db1 100644 --- a/ord_schema/search/nl_test.py +++ b/ord_schema/search/nl_test.py @@ -18,6 +18,7 @@ enough to pin every behavior that is this module's own rather than the model's. """ +import dataclasses import json import types from typing import Any @@ -79,6 +80,13 @@ def corpus(tmp_path_factory): _BAD_PATH = {"op": "eq", "path": "identifiers[*].value", "value": {"literal": "x"}} +@dataclasses.dataclass(frozen=True) +class _Refusal: + """A canned ``cannot_answer`` call.""" + + reason: str + + class _StubClient: """Returns canned responses in order, and records the requests it was given. @@ -105,6 +113,14 @@ def create(self, **kwargs): if isinstance(payload, str): block: TextBlock | ToolUseBlock = TextBlock(type="text", text=payload) stop = "end_turn" + elif isinstance(payload, _Refusal): + block = ToolUseBlock( + type="tool_use", + id="toolu_stub", + name="cannot_answer", + input={"reason": payload.reason}, + ) + stop = "tool_use" else: block = ToolUseBlock( type="tool_use", id="toolu_stub", name="build_query", input=payload @@ -175,16 +191,6 @@ def test_the_schema_and_the_rules_reach_the_prompt(): assert "identifiers[*]" in system -def test_the_tool_call_is_forced(): - # Left to itself a model may answer in prose, which is not a query. - client = _stub({"where": _SOLVENT}) - nl.translate("solvent reactions", client=client) - assert client.requests[0]["tool_choice"] == { - "type": "tool", - "name": "build_query", - } - - def test_a_bad_path_is_handed_back_once_and_recovered(): client = _stub({"where": _BAD_PATH}, {"where": _SOLVENT}) assert _where(nl.translate("aspirin reactions", client=client)).op == "exists" @@ -296,3 +302,37 @@ def recording(translated, **kwargs): client = _stub({"where": _SOLVENT}, "Some reactions.") nl.ask("solvent reactions", corpus, client=client, timeout_seconds=12.5) assert seen == {"timeout_seconds": 12.5} + + +def test_a_declined_question_is_named_as_unanswerable(): + # Forcing build_query would leave the model no way out, and a model with no way out + # invents a query rather than refusing. + client = _stub(_Refusal("a value is a literal, never another column")) + with pytest.raises(nl.UnanswerableError, match="another column"): + nl.translate("reactions that ran longer than their workup took", client=client) + + +def test_a_refusal_is_not_repaired(): + # Nothing was wrong with the model's reasoning, so asking again only costs money. + client = _stub(_Refusal("the schema does not hold that")) + with pytest.raises(nl.UnanswerableError): + nl.translate("anything unanswerable", client=client) + assert len(client.requests) == 1 + + +def test_both_tools_are_offered_and_one_is_required(): + client = _stub({"where": _SOLVENT}) + nl.translate("solvent reactions", client=client) + request = client.requests[0] + assert [tool["name"] for tool in request["tools"]] == [ + "build_query", + "cannot_answer", + ] + assert request["tool_choice"] == {"type": "any"} + + +def test_declining_is_told_apart_from_failing(): + # A caller shows these differently: one is "ask me another way", the other is a bug + # report, and both are NLQueryError to anything that only wants to catch one thing. + assert issubclass(nl.UnanswerableError, nl.NLQueryError) + assert not issubclass(nl.UnanswerableError, nl.MalformedQueryError) diff --git a/pyproject.toml b/pyproject.toml index a7c0089a..0cdff505 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -120,7 +120,7 @@ include = ["ord_schema*"] "ord_schema.proto" = ["*.pyi"] # The projection schema snapshot, read back through importlib.resources by its test, # and the natural-language system prompt, which ord_schema.search.nl reads the same way. -"ord_schema.search" = ["*.txt", "*.md"] +"ord_schema.search" = ["*.txt", "*.md", "*.json"] [tool.ruff] line-length = 88