fix(evaluators): omit default model=None from to_dict instead of pinning to default model id

[pdebjyot]

What

Evaluator.to_dict() serialized a default model=None as model_id=DEFAULT_BEDROCK_MODEL_ID. Combined with Experiment.from_dict() restoring model_id as an explicit model=, every judge that relied on the runtime default got pinned to whatever DEFAULT_BEDROCK_MODEL_ID happened to be at serialization time, silently, on every to_file/from_file round-trip.

This changes to_dict() to omit model entirely when it is None — the same way every other default-valued field is omitted — so a reloaded evaluator restores model=None and re-resolves the default at runtime.

Why it matters

• The default-model sentinel (model is None) is how the library expresses "use the current default judge." Persisting it as a concrete ID means upgrading DEFAULT_BEDROCK_MODEL_ID in a future release no longer takes effect for any saved experiment.
• It's silent: no warning, and the reloaded experiment looks correct.
• It's on the CLI's load path, so anyone using save/load is affected.

Change

if k == "model":
    if isinstance(v, Model):
        _dict["model_id"] = self._get_model_id(v)
    elif v is None:
        # model=None means "resolve the default at runtime". Omit it (like any
        # other default-valued field) so reload restores None rather than pinning
        # the judge to whatever DEFAULT_BEDROCK_MODEL_ID happens to be.
        pass
    else:
        # Explicit string model ID, include as-is
        _dict[k] = v

_get_model_id(None) still returns DEFAULT_BEDROCK_MODEL_ID (unchanged) — this only affects serialization of the default.

Tests

• test_to_dict_omits_default_none_model — a model=None evaluator serializes with no model/model_id key.
• test_none_model_survives_round_trip_as_none — Experiment.to_dict() → from_dict() restores model is None.
• Updated three existing to_dict assertions (Output/Trajectory/Interactions defaults) that were pinning the buggy model_id output.

Full suite green; ruff clean.

[poshinchen]

@strandly-the-agent review the changes carefully

[strandly-the-agent]

Approve — the fix is correct and minimal; needs a rebase before merge (conflicts with main).

Reproduced the bug on main (a default-model OutputEvaluator round-trips as model='global.anthropic.claude-sonnet-4-6') and confirmed the branch restores model is None. Old saved files that already contain model_id still load unchanged; only newly written files drop the key. No other consumer depends on the key being present.

Action: rebase — tests/strands_evals/test_experiment.py conflicts with main on the import block (this PR removes the DEFAULT_BEDROCK_MODEL_ID import; #330 added SkillSelectionScore / _get_label_from_score imports right next to it). Resolving as "keep main's new imports, drop DEFAULT_BEDROCK_MODEL_ID" is all it takes; the full suite passes on that resolution.

Question (non-blocking, for poshinchen as #40 author): #40 deliberately wrote the default out as model_id, presumably so a saved experiment records which judge ran. This PR trades that provenance for "follow the current default". I think the trade is right — evaluators build the judge via Agent(model=self.model) (output_evaluator.py:133), so None is resolved by the SDK's default, not by evals' own DEFAULT_BEDROCK_MODEL_ID; pinning to the evals constant was never guaranteed to match what actually ran. If provenance matters, it belongs in the run report, not in the experiment definition. Worth a one-line confirmation that this reversal is intended.

Verification, appendix

✅ Verified (PR head 9162a99 rebased onto main abf7b91, conflict resolved as above):

• Repro (Experiment.to_dict() → from_dict() with OutputEvaluator(rubric="r")): main → {'evaluator_type': 'OutputEvaluator', 'rubric': 'r', 'model_id': 'global.anthropic.claude-sonnet-4-6'}, reloaded model='global.anthropic.claude-sonnet-4-6'; PR → {'evaluator_type': 'OutputEvaluator', 'rubric': 'r'}, reloaded model=None.
• pytest tests/strands_evals/evaluators/test_evaluator.py tests/strands_evals/test_experiment.py → 152 passed.
• Full pytest tests/ → 2083 passed; 9 failed / 49 errors are all langfuse_provider / telemetry/test_config / cli/test_fetch failing on missing optional deps (langfuse, opentelemetry.exporter) — identical on main, unrelated.
• Consumers of model_id: only experiment.py:836 (guarded if "model_id" in evaluator_args) and a guarded test helper. Redteam serializes its model via its own _serialize_model path (already omits None) — unaffected.
• All 25+ evaluator subclasses default model=None and store the arg verbatim (self.model = model), so the new elif v is None: pass is behaviour-identical to the old branch in every case except the targeted one.
• New tests use real objects, and test_none_model_survives_round_trip_as_none fails on old code — a genuine regression test.

Reading order: evaluator.py hunk → test_evaluator.py::test_none_model_survives_round_trip_as_none → test_experiment.py expectation updates.

Appendix — non-blocking (1)

• ⚪ _get_model_id's None → DEFAULT_BEDROCK_MODEL_ID branch (evaluator.py:75-76) is now unreachable from src/ (its only caller passes a Model); kept alive by test_evaluator.py:103-107. Optional cleanup, not for this PR.