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