fix(evals): name the model of a spec hash rather than stringifying it - #430
Merged
Conversation
parse_all called to_s on each value, so a Hash arrived at the provider as its
inspected form and every scenario failed with '{"label" => ...} is not a
valid model ID'. The dashboard persists a run's models as specs.map(&:to_h)
and hands that back on a re-run, so any dashboard-initiated run hit this.
There was a problem hiding this comment.
🟢 Approval recommended
The focused implementation resolves the reported round-trip failure and is adequately covered by regression tests.
Pull request overview
Fixes dashboard evaluation reruns by correctly parsing persisted model-spec hashes.
Changes:
- Extracts
label, falling back tomodel, from hash-like values. - Adds regression coverage for string-keyed, symbol-keyed, and label-less hashes.
File summaries
| File | Description |
|---|---|
lib/active_agent/evals/model_spec.rb |
Coerces persisted model specs before parsing. |
test/evals/model_spec_test.rb |
Tests supported hash representations. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This was referenced Sep 11, 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.
A scenario evaluation started from the dashboard failed every scenario with HTTP 400 before reaching the model.
Root cause
ModelSpec.parse_allcallsto_son each value:A
Hashstringifies to its inspected form, which then travels as the model ID. OpenRouter answers:This is reachable from the dashboard by design, not by misuse:
ScenarioEvaluationRunner#selection_summarypersists a run's models asspecs.map(&:to_h), and re-running that selection hands the hashes straight back toparse_all. A caller passing a plain string (test_execute) was unaffected, which is why single runs worked while every batch run failed.Fix
parse_allnames the model of a hash —labelfirst, thenmodel— and leaves strings alone.Verification
Against a host application whose agent calls MCP tools, the same three-scenario run before and after:
list_filters,search_providersThe remaining failures are genuine quality results rather than infrastructure.
Three regression tests cover the label-bearing hash, a symbol-keyed hash, and a hash carrying only
model.