Skip to content

fix(scorers): a digit inside a name is not a count - #505

Open
arthapraha wants to merge 1 commit into
IBM:mainfrom
arthapraha:fix/static-json-name-digit-fallthrough
Open

fix(scorers): a digit inside a name is not a count#505
arthapraha wants to merge 1 commit into
IBM:mainfrom
arthapraha:fix/static-json-name-digit-fallthrough

Conversation

@arthapraha

Copy link
Copy Markdown

Description

_extract_count_from_text accepted any answer containing exactly one number,
wherever that number sat. "Chiller 6" therefore parsed as 6 — and so did
"Boiler 6", so two differently-named assets compared equal on a digit that
happened 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 (which parse_structured_answer reaches first).

input parse_structured_answer before after
"Chiller 6" 6 'Chiller 6'
"Boiler 6" 6 'Boiler 6'
"The count is 6" 6 6
"There are 5 assets" 5 5
"Answer: 6" 6 6
"6" 6 6

Impact on Benchmarking

  • Baseline change: This fix corrects a scoring error.

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_execute runner: the answer "The MAIN site has five assets: Chiller 6,
Motor_01, PUMP3, hyd_1, and mp_1"
reduced to {'answer': '6'} before, and is
now 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

Verification Steps

  1. tests/integration does not exist at e11d1c1, so the template's command
    has nothing to run. What was run instead, from src/:

    python -m pytest evaluation/tests/ -q
    

    main at e11d1c1: 6 failed / 90 passed
    this branch: 6 failed / 92 passed

    The 6 failures are pre-existing and untouched by this change —
    test_car_metadata_* and test_static_json_scorer_uses_car_metadata_score,
    all failing on KeyError: 'car_score'. Flagging so a red run here is not
    read as this PR's doing; happy to file them separately if useful.

  2. Manual verification, from src/:

    from evaluation.scorers.static_json import parse_structured_answer as p
    p("Chiller 6")        # main -> 6      this branch -> 'Chiller 6'
    p("The count is 6")   # main -> 6      this branch -> 6

    Both extractor frames were instrumented to confirm the fix sits on the path
    parse_structured_answer actually takes.

Checklist

  • I have added tests that prove my fix is effective. (+2 tests)
  • My code follows the project's Ruff formatting and linting rules.
    Precisely: this change introduces no new findings. ruff check reports
    the same 3 findings on main and on this branch (SIM118, ISC004,
    I001), and ruff format does not touch a single added line — the format
    drift in both files is pre-existing at e11d1c1. I have deliberately not
    reformatted the files, to keep the diff to the fix.
  • I have signed off my commits (DCO).

_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>
@DhavalRepo18

Copy link
Copy Markdown
Collaborator

@ChathurangiShyalika Pls check if we need this PR, or if it is some misunderstanding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

static_json scores differently-named assets as perfect matches when their names share a digit

2 participants