Repository navigation
fix(evaluation): validate FMEA threshold, fix scorer-default drift - #650
Merged
DhavalRepo18 merged 3 commits intoOct 19, 2026
Merged
Conversation
…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
marked this pull request as ready for review
October 19, 2026 04:09
DhavalRepo18
approved these changes
Oct 19, 2026
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
Follow-up to #646, #648 and #649, from a cross-check of the recent eval changes against each other.
fmea.pytook any int, float or bool fromscenario_meta.jsonasf1_pass_thresholdand silently used 0.80 for anything else (-1passed every answer,80none,truemeant 1.0,"0.8"was replaced by the default). Thefccandstatic_jsonscorers fail bad metadata with a rationale;fmeanow does too.static_jsonthe Evaluator/CLI default, butevaluation.runner.evaluatestill defaulted tollm_judge.docs/fmea-evaluation.mdstill said "strictly greater than 0.70".docs/evaluation.md, the Harbor README, QUICKSTART and thetest.shtemplate still describedllm_judgeas the CLI default.docs/static-json-evaluation.mddidn't mentionrelative_tolerance(feat(evaluation): add relative_tolerance to static_json scoring #649).Fix Details
fmea.py:_f1_pass_thresholdreturns the 0.80 default when the key is missing or null, andNonefor a bool, a non-number, NaN, or a value outside [0, 1]. The scenario then fails witherror: "invalid_scenario_meta".runner.py:default_scoring_methodis nowstatic_json.fmea/fccrows in the scorer table; Harbor routing; the real cause ofunknown scorer 'llm_judge'; a static_json "Numeric tolerance" section.Impact on Benchmarking
scenario_meta.jsonfiles set a valid threshold (0.8 or 0.9). Re-scoring the 304 recorded FMEA trials raises noinvalid_scenario_meta, so every verdict is unchanged. Therunner.pydefault only applies to scenarios with noscoring_methodwhen called throughevaluation.runner.evaluate, which nothing in the repo calls outside tests.Related Issues
Verification Steps
uv run pytest src/ -v -k "not integration": 853 passed. The 2 failures insrc/observability/tests/test_file_exporter.pyalso fail onaafeedback_changesand are unrelated.test_fmea_scorer.py: a scenario threshold overrides the default, a missing/null threshold uses 0.80, andtrue,-0.1,1.5,80,"0.8"and NaN fail with a rationale.Checklist
ruff checkpasses.fmea.pyand its test were already offruff formaton the base branch, so the new lines follow the surrounding style instead of reformatting the file.)