Skip to content

fix(evaluation): validate FMEA threshold, fix scorer-default drift - #650

Merged
DhavalRepo18 merged 3 commits into
aafeedback_changesfrom
fix/eval-defaults-fmea-threshold
Oct 19, 2026
Merged

DhavalRepo18 merged 3 commits into
aafeedback_changesfrom
fix/eval-defaults-fmea-threshold

Conversation

@ShuxinLin

Copy link
Copy Markdown
Collaborator

Description

Follow-up to #646, #648 and #649, from a cross-check of the recent eval changes against each other.

  1. FMEA threshold validation. fmea.py took any int, float or bool from scenario_meta.json as f1_pass_threshold and silently used 0.80 for anything else (-1 passed every answer, 80 none, true meant 1.0, "0.8" was replaced by the default). The fcc and static_json scorers fail bad metadata with a rationale; fmea now does too.
  2. Default scorer drift. fix(evaluation): let scenario_meta.json route scoring, and make the F… #646 made static_json the Evaluator/CLI default, but evaluation.runner.evaluate still defaulted to llm_judge.
  3. Stale docs. docs/fmea-evaluation.md still said "strictly greater than 0.70". docs/evaluation.md, the Harbor README, QUICKSTART and the test.sh template still described llm_judge as the CLI default. docs/static-json-evaluation.md didn't mention relative_tolerance (feat(evaluation): add relative_tolerance to static_json scoring #649).

Fix Details

  • fmea.py: _f1_pass_threshold returns the 0.80 default when the key is missing or null, and None for a bool, a non-number, NaN, or a value outside [0, 1]. The scenario then fails with error: "invalid_scenario_meta".
  • runner.py: default_scoring_method is now static_json.
  • Docs: inclusive per-scenario FMEA threshold; the CLI default; fmea/fcc rows in the scorer table; Harbor routing; the real cause of unknown scorer 'llm_judge'; a static_json "Numeric tolerance" section.

Impact on Benchmarking

  • No change to baselines: All 13 FMEA scenario_meta.json files set a valid threshold (0.8 or 0.9). Re-scoring the 304 recorded FMEA trials raises no invalid_scenario_meta, so every verdict is unchanged. The runner.py default only applies to scenarios with no scoring_method when called through evaluation.runner.evaluate, which nothing in the repo calls outside tests.

Related Issues

Verification Steps

  1. uv run pytest src/ -v -k "not integration": 853 passed. The 2 failures in src/observability/tests/test_file_exporter.py also fail on aafeedback_changes and are unrelated.
  2. New tests in test_fmea_scorer.py: a scenario threshold overrides the default, a missing/null threshold uses 0.80, and true, -0.1, 1.5, 80, "0.8" and NaN fail with a rationale.

Checklist

  • I have added tests that prove my fix is effective.
  • My code follows the project's Ruff formatting and linting rules. (ruff check passes. fmea.py and its test were already off ruff format on the base branch, so the new lines follow the surrounding style instead of reformatting the file.)
  • I have signed off my commits (DCO).

…n [0, 1]

fmea.py took any int, float or bool from scenario_meta.json as the pass
threshold and fell back to 0.80 for anything else. A threshold of -1 passed
every answer, 80 passed none, true meant 1.0, and the string "0.8" was
silently replaced by the default. The fcc and static_json scorers fail such
metadata with a rationale instead.

fmea.py now does the same: a threshold that is a bool, a non-number, NaN,
or outside [0, 1] fails the scenario with error "invalid_scenario_meta". A
missing or null threshold still uses the 0.80 default.

All 13 FMEA scenario_meta.json files set 0.8 or 0.9, and re-scoring the 304
recorded FMEA trials gives no invalid_scenario_meta, so no verdict changes.

docs/fmea-evaluation.md still said a scenario passes when F1 is strictly
greater than 0.70; it now states the inclusive per-scenario threshold.

Signed-off-by: Shuxin Lin <linshuhsin@gmail.com>
…ng docs

#646 moved the Evaluator and CLI default from llm_judge to static_json so a
scenario without metadata lands on a deterministic scorer, but
evaluation.runner.evaluate still defaulted to llm_judge. It now matches.

The docs still described the old routing:

- docs/evaluation.md gave the CLI default as llm_judge and listed neither
  the fmea nor the fcc scorer.
- The Harbor README said the loader defaults to static_json and the CLI to
  llm_judge; scenario_meta.json routes, and the evaluator falls back to
  --scorer-default, which is static_json.
- The QUICKSTART entry for "unknown scorer 'llm_judge'" blamed a missing
  --scorer-default, which can no longer cause it. It now names the real
  cause: an llm_judge scenario, or --scorer-default llm_judge, without a
  judge model.
- The test.sh template comment gave the old default as its reason for
  passing --scorer-default static_json.

Signed-off-by: Shuxin Lin <linshuhsin@gmail.com>
#649 added the scenario_meta.json opt-in {"match": "numeric",
"relative_tolerance": t} to static_json, but the doc still said static_json
passes only on a strict exact match, with CAR as the only exception.

Signed-off-by: Shuxin Lin <linshuhsin@gmail.com>
@ShuxinLin
ShuxinLin marked this pull request as ready for review October 19, 2026 04:09
@DhavalRepo18
DhavalRepo18 merged commit 566b929 into aafeedback_changes Oct 19, 2026
5 checks passed
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.

2 participants