fix(scorers): a digit inside a name is not a count - #505
Open
arthapraha wants to merge 1 commit into
Open
Conversation
_extract_count_from_text accepted any answer containing exactly one number, so "Chiller 6" parsed as 6 -- and so did "Boiler 6". Two differently-named assets then compared equal on a digit that happened to be inside a name, and a wrong asset scored a perfect strict match while a different wrong asset scored zero. The docstring already states the intent: count-only or nearly count-only. This enforces it. Once the number and the usual count wording are removed, any remaining word means the number was part of a phrase -- most importantly, part of a name. "The count is 6" and "There are 5 assets" still parse. "Chiller 6" no longer does. Note for reviewers: 6 tests in evaluation/tests/ already fail on main at e11d1c1 (test_car_metadata_*, test_static_json_scorer_uses_car_metadata_score). This change does not touch them; the suite goes from 6 failed / 90 passed to 6 failed / 92 passed. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Attila <29815676+arthapraha@users.noreply.github.com>
Collaborator
|
@ChathurangiShyalika Pls check if we need this PR, or if it is some misunderstanding. |
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.
Description
_extract_count_from_textaccepted any answer containing exactly one number,wherever that number sat.
"Chiller 6"therefore parsed as6— and so did"Boiler 6", so two differently-named assets compared equal on a digit thathappened to be inside a name.
A wrong asset scored a perfect strict match while a different wrong asset
scored zero. Full report in #504.
Fix Details
The docstring already states the intent — "count-only or nearly count-only" —
and this enforces it rather than changing it. After the number and the usual
count wording are removed, any remaining word means the number was part of a
phrase, most importantly part of a name.
The change lands in
_extract_count_from_text, which covers both entry points:the direct call in
parse_structured_answer, and the delegation from_extract_final_count_from_text(whichparse_structured_answerreaches first).parse_structured_answerbefore"Chiller 6"6'Chiller 6'"Boiler 6"6'Boiler 6'"The count is 6"66"There are 5 assets"55"Answer: 6"66"6"66Impact on Benchmarking
Before vs. After. The correction only ever removes false positives, so
scores can fall but not rise. Observed on scenario 3 via this repository's own
plan_executerunner: the answer "The MAIN site has five assets: Chiller 6,Motor_01, PUMP3, hyd_1, and mp_1" reduced to
{'answer': '6'}before, and isnow compared as the string it is.
Any published number computed against a list-shaped gold whose entries contain
digits may have been inflated. Golds that are genuine counts are unaffected —
every count form in the table above still parses.
Related Issues
static_jsonscores differently-named assets as perfect matches when their names share a digit #504losing correct answers. This one accepts wrong ones. A fix for either that
treats the extractor as "harvest a number from the text" will leave the other
in place. Happy to fold this into whatever Fix static JSON evaluator parsing for noisy final numeric answers #494's assignee is already
doing, or to close it in favour of a combined change — please just say which
you prefer.
Verification Steps
tests/integrationdoes not exist ate11d1c1, so the template's commandhas nothing to run. What was run instead, from
src/:mainate11d1c1: 6 failed / 90 passedthis branch: 6 failed / 92 passed
The 6 failures are pre-existing and untouched by this change —
test_car_metadata_*andtest_static_json_scorer_uses_car_metadata_score,all failing on
KeyError: 'car_score'. Flagging so a red run here is notread as this PR's doing; happy to file them separately if useful.
Manual verification, from
src/:Both extractor frames were instrumented to confirm the fix sits on the path
parse_structured_answeractually takes.Checklist
Precisely: this change introduces no new findings.
ruff checkreportsthe same 3 findings on
mainand on this branch (SIM118,ISC004,I001), andruff formatdoes not touch a single added line — the formatdrift in both files is pre-existing at
e11d1c1. I have deliberately notreformatted the files, to keep the diff to the fix.