Score translations on the reactions they return - #981
Open
skearnes wants to merge 1 commit into
Open
Conversation
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) <noreply@anthropic.com>
Comment on lines
+141
to
+144
| except nl.MalformedQueryError as error: | ||
| return CaseResult( | ||
| case, passed=not case.compiles, detail=f"did not compile: {error}" | ||
| ) |
There was a problem hiding this comment.
Malformed output passes refusal case
When an inexpressible case produces an invalid build_query call instead of cannot_answer, run_case marks the resulting MalformedQueryError as passing, causing the evaluation to hide a failure to refuse.
Suggested change
| except nl.MalformedQueryError as error: | |
| return CaseResult( | |
| case, passed=not case.compiles, detail=f"did not compile: {error}" | |
| ) | |
| except nl.MalformedQueryError as error: | |
| return CaseResult( | |
| case, passed=False, detail=f"did not compile: {error}" | |
| ) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The eval harness (task 7), plus a gap it exposed and closed. A case states reactions any correct query returns and reactions a plausible wrong one returns — never the query it expects, since several spellings of a question are right.
Both
claude-haiku-4-5andclaude-opus-5pass 5/5 against the full corpus.Two things worth your attention
My first version of the cases was wrong, and it cost a round of prompt tuning. I built
must_not_returnby differencing a 200-row sample of the reference against a 400-row sample of the near-miss — set arithmetic on samples. It failed correct translations on both models, and I "fixed" the prompt for a problem that did not exist before checking the case itself. Counterexamples are now found by asking for them —near_miss AND NOT reference— which is exact, and the builder raises if a near-miss excludes nothing. The prompt example I added stands on its own merits but changed no measurement, and the commit says so.Forcing
build_queryleft the model no way to decline. Asked for something the grammar cannot express — comparing two columns — both models built a plausible query that means something else. That is the failure mode that looks most like an answer.cannot_answeris now offered besidebuild_querywithtool_choice: any, declining raisesUnanswerableErrorcarrying the model's reason, and a refusal is never repaired since nothing was wrong with its reasoning. With that, the inexpressible case passes on both models by being refused.Changes
nl_eval.py—EvalCase,score,run_case,report, and a command. Every case carries awhy, printed with any failure.nl_cases_build.py— how the IDs were derived, rerunnable when the corpus changes. IDs are only meaningful against the corpus they came from, which the module says.nl_cases.json— five cases: the scoping trap, a scalar comparison, correlation, aforall, and one the grammar cannot express.nl.py—REFUSAL_TOOL,UnanswerableError.--pivots-dirand--pivot-budget-bytes, defaulting the budget to 0: a run is a handful of queries, and building a pivot over 2.4M reactions costs minutes to save milliseconds. Without that the first run hung.Testing
uv run pytest -n auto— 1246 passed, none reaching the network. The harness tests cover scoring, the report, and that every shipped case has both a reason and something to assert; the refusal tests cover the new tool, the error, and that a refusal costs one request rather than two.Notes
Deviation from the plan: cases are JSON, not YAML. PyYAML is installed here but undeclared, and importing it would have been exactly what
dependencies_test.pyexists to catch. Each case carries awhyfield instead of a comment, which is better anyway — it prints with the failure.dependencies_test.pyalso caughtnl_evalimportinganthropicwhile belonging to thesearchprofile; its roots now name both modules.Stacked on #980 → #979; retarget as those land.
🤖 Generated with Claude Code
Greptile Summary
This PR adds explicit refusal support to natural-language query translation and an evaluation harness that scores translations by returned reaction IDs.
cannot_answeralongsidebuild_queryand exposes refusals throughUnanswerableError.Confidence Score: 4/5
The evaluation harness should be corrected before merging because it can report malformed model output as a successful refusal.
The production refusal path is coherently separated from malformed queries, but
run_casecollapses those outcomes and can therefore conceal a regression in the newly added evaluation.Files Needing Attention: ord_schema/search/nl_eval.py
Important Files Changed
run_caseexception semantics.Sequence Diagram
sequenceDiagram participant Eval as nl_eval.run_case participant NL as nl.translate participant Model as Anthropic model participant Corpus as Search corpus Eval->>NL: Translate case question NL->>Model: Offer build_query or cannot_answer alt cannot_answer Model-->>NL: Explicit refusal NL-->>Eval: UnanswerableError Eval-->>Eval: "Pass only if compiles=false" else valid build_query Model-->>NL: Query NL-->>Eval: Validated query Eval->>Corpus: Execute query Corpus-->>Eval: Reaction IDs Eval-->>Eval: Score required/forbidden IDs else malformed build_query Model-->>NL: Invalid query NL-->>Eval: MalformedQueryError Eval-->>Eval: "Currently passes when compiles=false" endReviews (1): Last reviewed commit: "Score translations on the reactions they..." | Re-trigger Greptile