Skip to content

fix: coerce judge score drift#756

Merged
nabinchha merged 5 commits into
NVIDIA-NeMo:mainfrom
schultzjack:codex/569-coerce-judge-scores
Jul 7, 2026
Merged

fix: coerce judge score drift#756
nabinchha merged 5 commits into
NVIDIA-NeMo:mainfrom
schultzjack:codex/569-coerce-judge-scores

Conversation

@schultzjack

@schultzjack schultzjack commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • normalize LLM-judge score values before enum validation in generated judge response models
  • accept numeric/string drift and simple case/whitespace drift when it maps unambiguously to a configured score option
  • keep unmatched or malformed scores on the existing Pydantic validation path

Scope

This addresses the LLM-judge validation path discussed in #569. It intentionally leaves the broader LLM-structured schema coercion path unchanged.

Testing

  • uv run --group dev pytest packages/data-designer-engine/tests/engine/column_generators/utils/test_judge_score_factory.py -q
  • uv run --group dev pytest packages/data-designer-engine/tests/engine/column_generators/utils/test_judge_score_factory.py packages/data-designer-engine/tests/engine/column_generators/utils/test_prompt_renderer.py packages/data-designer-engine/tests/engine/column_generators/generators/test_llm_completion_generators.py packages/data-designer-engine/tests/engine/models/recipes/test_response_recipes.py -q
  • make check-engine
  • make test-engine

Fixes #569

Signed-off-by: schultzjack <schultzjack@users.noreply.github.com>
@schultzjack schultzjack requested a review from a team as a code owner June 17, 2026 00:31
@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

All contributors have signed the DCO ✍️ ✅
Posted by the DCO Assistant Lite bot.

@schultzjack

Copy link
Copy Markdown
Contributor Author

I have read the DCO document and I hereby sign the DCO.

@schultzjack

Copy link
Copy Markdown
Contributor Author

recheck

@greptile-apps

greptile-apps Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR introduces score-drift coercion for LLM-judge response models, normalizing common type/case/whitespace mismatches (e.g., int 1"1", " good ""Good") before Pydantic enum validation runs, while leaving unmatched values on the existing Pydantic error path.

  • Adds _normalize_score_value and _coerce_score_value helpers, with an explicit bool-vs-int guard to prevent True/False from silently matching 1/0 enum members.
  • Adds a model_validator(mode="before") on BaseJudgeResponse that only activates when the subclass has a typed Enum score field, so the base class itself is unaffected.
  • New tests cover numeric→string coercion, float→int coercion, string→int coercion, case/whitespace folding, and validation-error fallthrough for unhashable and out-of-range inputs.

Confidence Score: 5/5

The change is narrowly scoped to pre-validation coercion in judge response models and is safe to merge.

The two-stage coercion logic (exact match with bool guard, then normalized single-match) is correct and well-tested. Unmatched values fall through to Pydantic's own validation path unchanged, so no regressions in error handling. The model_validator only activates on concrete subclasses with an Enum-typed score field, leaving BaseJudgeResponse itself unaffected.

No files require special attention.

Important Files Changed

Filename Overview
packages/data-designer-engine/src/data_designer/engine/column_generators/utils/judge_score_factory.py Adds two normalization helpers and a Pydantic before-mode model_validator on BaseJudgeResponse; logic is sound, bool guard is correctly scoped to the exact-match loop only, and the normalized path is consistent.
packages/data-designer-engine/tests/engine/column_generators/utils/test_judge_score_factory.py Adds parametrized coercion tests plus fallthrough tests for invalid/out-of-range inputs; covers the primary drift scenarios described in the PR.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["model_validator(mode='before')\ncoerce_score"] --> B{data is dict\nwith 'score' key?}
    B -- No --> Z[Return data unchanged\nPydantic validates normally]
    B -- Yes --> C{score field\nexists in model_fields?}
    C -- No --> Z
    C -- Yes --> D{score annotation\nis an Enum subclass?}
    D -- No --> Z
    D -- Yes --> E["_coerce_score_value(value, enum_type)"]
    E --> F{For each member:\nbool-ness match AND\nvalue == member.value?}
    F -- Yes --> G[Return value as-is\nexact match]
    F -- No\nall members --> H["_normalize_score_value(value)\n→ strip, casefold, int-float"]
    H --> I{Exactly one\nnormalized member match?}
    I -- Yes --> J[Return canonical\nmember.value]
    I -- No / Zero --> K[Return original value\nPydantic validates / rejects]
    G --> L[Pydantic enum validation + use_enum_values]
    J --> L
    K --> L
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["model_validator(mode='before')\ncoerce_score"] --> B{data is dict\nwith 'score' key?}
    B -- No --> Z[Return data unchanged\nPydantic validates normally]
    B -- Yes --> C{score field\nexists in model_fields?}
    C -- No --> Z
    C -- Yes --> D{score annotation\nis an Enum subclass?}
    D -- No --> Z
    D -- Yes --> E["_coerce_score_value(value, enum_type)"]
    E --> F{For each member:\nbool-ness match AND\nvalue == member.value?}
    F -- Yes --> G[Return value as-is\nexact match]
    F -- No\nall members --> H["_normalize_score_value(value)\n→ strip, casefold, int-float"]
    H --> I{Exactly one\nnormalized member match?}
    I -- Yes --> J[Return canonical\nmember.value]
    I -- No / Zero --> K[Return original value\nPydantic validates / rejects]
    G --> L[Pydantic enum validation + use_enum_values]
    J --> L
    K --> L
Loading

Reviews (4): Last reviewed commit: "Merge branch 'main' into codex/569-coerc..." | Re-trigger Greptile

@github-actions

Copy link
Copy Markdown
Contributor

Stale PR reminder

This PR has had failing checks for 7 days without activity.

Failing checks: check

Please push an update or leave a comment if you're still working on this.
Otherwise, this PR will be automatically closed in 7 days.

To prevent auto-close, add the keep-open label.

@andreatgretel

Copy link
Copy Markdown
Contributor

Thanks for the contribution, this is a useful bit of tolerance around judge outputs. I reviewed the score coercion path and the generated Pydantic models. The implementation is nicely scoped and I don't see major blockers, but I'd like a small polish pass before merge.

A couple of test cases would make the new fallback behavior clearer:

  • Add a case for float drift into string score options, e.g. options {"1": "Low quality"} with model output 1.0 should coerce to "1".
  • Add a case for an out-of-range scalar like 99 to confirm it still falls through to Pydantic validation rather than being coerced.

Also, please add a short comment above the bool guard in _coerce_score_value(). It's there because bool is a subclass of int in Python, so True == 1 and False == 0; that context will help keep the guard from looking accidental.

Focused tests and smoke checks passed locally. Once those small coverage/readability items are in, this looks good to merge from my side.

@nabinchha

nabinchha commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Nice work on this one, @schultzjack — this is a focused fix with good coverage for the judge-score validation path.

Summary

This PR normalizes LLM-judge score values before enum validation so small-model drift like numeric/string mismatches, casing, and surrounding whitespace can still resolve to configured score options when the match is unambiguous. The implementation matches the stated scope by keeping the broader LLM-structured schema coercion path unchanged.

Findings

No findings from my pass. Andre has a few asks that should be addressed before merge.

What Looks Good

  • The coercion is scoped to BaseJudgeResponse, so it affects only the internal judge response models and does not broaden user-defined structured output validation.
  • The matcher preserves Pydantic's existing validation path for malformed or ambiguous values instead of swallowing errors silently.
  • The tests cover direct response models, nested structured output models, numeric/string drift, case/whitespace drift, and a malformed-score failure path.

Verdict

Looks good to merge after Andre's asks are addressed.


This review was generated by an AI assistant.

nabinchha and others added 3 commits June 29, 2026 14:39
Address review feedback: add coverage for float drift into string
score options and out-of-range scalars falling through to Pydantic
validation, and document why the bool guard exists in
_coerce_score_value().

Signed-off-by: schultzjack <schultzjack@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@schultzjack

Copy link
Copy Markdown
Contributor Author

Thanks — good catches, the float-drift and out-of-range paths were real gaps in the coverage. All three items addressed in 857dcc8.

  • Added the float-drift case to the parametrized coercion test: options {"1": ..., "2": ...} with model output 1.0 coerces to "1".
  • Added an out-of-range case: 99 falls through to Pydantic validation and raises ValidationError rather than being coerced.
  • Added a comment above the bool guard in _coerce_score_value() noting that bool is a subclass of int (True == 1, False == 0), so bool-ness must match to keep True/False from silently matching 1/0 options.

Branch is synced with main.

Appreciate the second pass as well.

@andreatgretel andreatgretel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the follow-up. The float-drift and out-of-range cases are covered, and the bool guard rationale is documented. The coercion remains scoped to judge responses, while invalid or ambiguous values still fall through to Pydantic validation. I don't see any remaining blockers. Approving.

@nabinchha

Copy link
Copy Markdown
Contributor

Took another pass — clean, well-scoped fix. Verified locally: all 13 tests pass and ruff is clean. Only optional nits (string-encoded floats like "1.0" don't get numeric normalization, and minor helper placement), nothing blocking. Since we've already reviewed and approved this, good to ship.

@nabinchha nabinchha merged commit bff09d8 into NVIDIA-NeMo:main Jul 7, 2026
60 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.

Schema validation rejects small-model output before coercion can normalize it

3 participants