You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
feat(repo): record the model that answered each cell - #119
repo run recorded the model it was asked for, not the model that answered. A harness alias can silently pin an older model: we hit an alias haiku that still pointed at Haiku 4.5 while Haiku 5.5 was current, and nothing in the batch showed it (#41, #65).
claude-stream-json trace
system/init.model -> init_model (the harness repeats the request)
assistant message.model -> answered_by (main thread only; subagents, "<synthetic>" skipped)
Transcript.model_resolved() = answered_by or [init_model] -> str | sorted list | None (text trace)
meta.json / result.json
+ "model_resolved": "anthropic.claude-haiku-4-5-20251001-v1:0"
repo report warning: cells run with model X were answered by Y; check the harness's model aliases
repo compare same warning for the compared cells
+ "T vs B: model M was answered by ... in baseline, ... in treatment"
model_matches() does not treat a provider prefix (us.anthropic., org/) or a Bedrock -v1:0 suffix as a different model, because Bedrock runs report the request with the prefix and the reply without it.
Evidence
Before: a run with --model haiku records "model": "haiku". repo report prints no warning. After: checked against real Claude Code 2.1.296 traces from earlier oneDAL batches.
requested
init.model
model_resolved
warning
us.anthropic.claude-haiku-4-5-20251001-v1:0
same
anthropic.claude-haiku-4-5-20251001-v1:0
none
us.anthropic.claude-haiku-5-5
same
claude-haiku-5-5
none
claude-sonnet-5
same
claude-sonnet-5
none
haiku (alias; the first row's reply checked against this request)
—
anthropic.claude-haiku-4-5-20251001-v1:0
yes
Tests:
model extraction in test_repo_reach.py;
end-to-end fields, the report warning and the prefix rule in test_repo_run.py;
the arm-mismatch warning in test_repo_compare.py.
The fake harness in repo_eval_fixture.py now puts --model in its init and assistant events.
ruff check . and mypy (212 files) are clean.
make ci-local: the first run failed once in the test step (176 s against about 88 s normally; output not kept). Every run since has passed: 19 passed, 4 skipped, 5186 tests, plus three more pytest -n auto runs.
Merge Danger
Door: two-way
The change adds fields and warnings. Nothing that existed before is removed or renamed.
Blast Radius: small
Consumers of repo report --json see model_resolved on rows and model_mismatch in the summary. repo compare batch stamps gain model_resolved.
An undated request answered by a dated ID warns, for example claude-sonnet-4-5 answered as claude-sonnet-4-5-20250929. That is alias resolution too, but it may be noisy.
Vertex-style @date IDs are not normalized and would warn.
A stale alias used only by subagents is not flagged.
`repo run` recorded only the requested `--model`, so a harness alias that
maps to an older model went unnoticed. From a claude-stream-json trace,
meta.json and result.json now carry `model_resolved`: the main thread's
`message.model` (subagents and "<synthetic>" replies excluded), else the
init event's `model`; one name, a sorted list, or null for a text trace.
`repo report` and `repo compare` warn when a cell was answered by another
model than requested (a provider prefix such as `us.anthropic.` and a
Bedrock `-v1:0` suffix are not a difference), and `repo compare` also when
the two arms of a comparison were answered by different models.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.15%. Comparing base (5c70cbf) to head (03a286c). ⚠️ Report is 4 commits behind head on main.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Apply model_matches() equivalence logic when comparing resolved models
between baseline and treatment arms, not raw set comparison. Bedrock
model IDs like us.anthropic.claude-x, anthropic.claude-x, and claude-x
are equivalent but previously triggered a false mismatch warning.
Adds _models_equivalent() to check bidirectional equivalence and a
regression test covering Bedrock alias variants.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…ne model
Drop the set-size check; the two coverage checks decide. An arm whose
repetitions resolve to us.anthropic.claude-x and claude-x matches an arm
that saw only claude-x.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Equivalent model IDs with crossed provider-prefix and version-suffix variants still trigger a false comparison warning.
No open issues 🎉
1 previously missed issue
In code that hasn't changed since last review
Normalize model IDs before bidirectional equivalence comparison
dokimi/dokimi/repos/compare.py:178
The bidirectional check still rejects equivalent IDs when the provider prefix and version suffix occur on opposite sides. For request us.anthropic.claude-haiku-4-5-20251001-v1:0, baseline anthropic.claude-haiku-4-5-20251001 and treatment claude-haiku-4-5-20251001-v1:0 both pass model_matches(requested, resolved), but _models_equivalent() returns False. This produces a false different-model warning in repo compare, despite both arms matching the requested model. Normalize provider prefixes and Bedrock suffixes consistently before comparing identities, and add a regression for this crossed case; the existing tests cover variants that a directional match can handle.
The reason will be displayed to describe this comment to others. Learn more.
Review at 7d2a303. The extraction in events.py is sound: the main-thread filter, the <synthetic> skip and the reply-over-init precedence are each pinned by a test that fails when the line is removed. The problems are at the edges of the model-equivalence rule. Two of the inline comments I'd fix before merge.
Reproductions for the equivalence findings, with no model called.
Mutation runs of _models_equivalent and model_matches against test_repo_compare.py and test_repo_run.py (100 tests).
I agree with @copilot-pull-request-reviewer on the crossed prefix and suffix in _models_equivalent. New evidence: a region swap is also a false warning. _models_equivalent({"eu.anthropic.claude-x"}, {"us.anthropic.claude-x"}) is False, and {"anthropic.claude-x-v1:0"} against {"us.anthropic.claude-x"} is False. Comparing the two sides through one normalizer (strip the prefix up to the last . or /, strip -vN:M) fixes all three.
Checked, no issues
Extraction: dropping the parent_tool_use_id filter, dropping the <synthetic> skip, and putting init_model ahead of the replies each fail test_claude_stream_json_names_the_models_that_answered.
Old cells: a record with model_resolved missing, null, a string or a list goes through resolved_models without error, so batches written before this PR still report and compare.
Schemas:schemas/ has no schema for meta.json or result.json, so no SCHEMA_VERSIONS entry is needed. The changelog fragment is in changelog.d/ and no generated files are in the diff.
The model_matches() function only checked if requested could be resolved,
but not the reverse. Now checks both directions so that a dated/aliased
request like us.anthropic.claude-x answered by claude-x (shorter form)
doesn't incorrectly warn. Add regression test that would fail if the
symmetric half of _models_equivalent is removed.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The reason will be displayed to describe this comment to others. Learn more.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Normalize prefixes and suffixes independently in model equivalence checks
dokimi/dokimi/repos/report.py:135
The matcher still fails when decorations differ on both IDs. For request us.anthropic.claude-haiku-4-5-20251001-v1:0, resolutions anthropic.claude-haiku-4-5-20251001 and claude-haiku-4-5-20251001-v1:0 each match the request, but do not match each other. _models_equivalent() therefore produces a false different-model warning in repo compare. Likewise, otherwise identical IDs with us.anthropic. and eu.anthropic. prefixes fail to match. Normalize supported provider prefixes and Bedrock suffixes independently on both IDs before comparing, preserving version numbers within model names. Add regressions for crossed prefix/suffix omissions and different region prefixes.
…hreads
- Thread 4314952362: Replace regex-based model matching with explicit normalization+boundary checks.
Added _normalize_model_id() to strip Bedrock version suffixes (-v1:0), and _model_prefix_matches()
for clearer prefix checking. Reduces regex opacity and improves readability/maintainability.
- Thread 4314952438: Guard fixture against missing/incomplete --model argument.
Added try/except to handle ValueError/IndexError when parsing args, defaulting to 'unknown' model.
Prevents fixture crashes in malformed test invocations.
- Thread 4314952485: Remove duplicate bidirectional checks in _models_equivalent.
model_matches() is already bidirectional, so simplified both loops to call it once per pair,
removing redundant or statements. Improves code clarity and avoids unnecessary comparisons.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
repo compare ... --model m-2 still emits both new model warnings for excluded m-1 cells. compare() filters comparison rows by models but passes all cells to _provenance(), and both warning paths filter only by arm. Pass the selected models through to these checks and exclude unselected models from both warnings. Add a regression with matching m-2 cells and mismatching m-1 cells so the selected comparison does not produce unrelated model diagnostics.
The reason will be displayed to describe this comment to others. Learn more.
Review at d6f4cb1. Neither of my two earlier threads is fixed. The rewrite of model_matches into _normalize_model_id and _model_prefix_matches changed how it is written, not what it accepts, and the new compare test passes without the code it names. One nit inline.
make ci-local: 19 passed, 0 failed, 4 skipped; the changed-line coverage step passed.
Reproductions for the findings below, with no model called.
Mutation runs against test_repo_compare.py (61 tests).
Still open from my earlier review:
dokimi/dokimi/repos/report.py:145: a dated or aliased answer to an undated request still warns. model_matches("claude-sonnet-4-5", "claude-sonnet-4-5-20250929") is False at this head, so model_mismatch still reports it as an alias problem. Edf068d made the check bidirectional, which covers the prefix-on-the-request case only. Add the -YYYYMMDD / @YYYYMMDD rule, or say in the contract that this warns.
dokimi/tests/test_repo_compare.py:628: test_compare_models_equivalent_pins_symmetric_coverage does not pin the reverse loop. Replacing the second loop in _models_equivalent with for treat_model in []: leaves all 61 tests passing. The test has one cell per arm, so it never reaches that loop; the docstring describes a {a} against {a, b} case that the body does not run. _models_equivalent({"m"}, {"m", "other"}) is correctly False, so only the test is weak. Add the case the docstring describes, with a treatment arm that holds a model no baseline model matches.
I agree with @copilot-pull-request-reviewer on -vN being stripped from ordinary ids: model_matches("deepseek-v2", "deepseek-v3") is True at this head, so a real generation change raises no warning, for any family named -vN.
I agree with @copilot-pull-request-reviewer that compare() passes every cell to _provenance. New evidence: with m-2 cells answered by m-2 and m-1 cells answered by old-model, compare([b], "raw", None, models=["m-2"]) warns "cells run with model m-1 were answered by old-model; check the harness's model aliases", and the stamp reports model_resolved: ['m-2', 'old-model'].
Checked, no issues
Extraction: the main-thread filter, the <synthetic> skip and the reply-over-init precedence behave as the changelog says.
Old cells: a record without model_resolved still goes through resolved_models.
Real alias pin:model_matches("haiku", "anthropic.claude-haiku-4-5-20251001-v1:0") is False, so the case the PR targets still warns.
Rules: no schema for meta.json or result.json, the changelog fragment is in place, no generated files, tests are offline.
Stripping every -vN suffix made deepseek-v2 match deepseek-v3. Only
Bedrock's -v1:0 transport suffix is stripped now. A dated snapshot of the
requested id (-20250929 or @20250929) is the same model, so it no longer
warns. The compare test now covers the treatment-to-baseline direction,
and the contract documents the mismatch warnings.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
repo compare --model m-2 warned about m-1 cells it did not compare:
the arm loop and the per-cell alias check ran over every loaded cell.
Both now follow the selected models; the batch stamp stays batch-wide.
Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
The contract says a snapshot of the requested id, as -DATE or @Date,
or its provider-prefixed form is the same model. An @Date request
answered as -DATE, and a bare request answered with a prefixed dated
id, still warned. Both sides now map @Date to -DATE before matching.
Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
No test failed with the baseline-to-treatment loop of _models_equivalent
removed, nor with the org/ prefix or the 8-digit snapshot rule dropped.
A baseline snapshot the treatment never got must warn.
Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
Ran on Linux: full suite (-n 4), ruff 0.17.0, mypy 2.4.0 strict, bandit -ll, the repo compare CLI, and mutants of the new model checks.
repo compare b1 --baseline guidance:none --model m-2, where the m-1 cells were answered by another model -> prints two m-1 warnings -> expected none (the open point from the Copilot and antonwolfy review bodies).
model_matches('claude-sonnet-4-5@20250929', 'claude-sonnet-4-5-20250929') and ('claude-sonnet-4-5', 'us.anthropic.claude-sonnet-4-5-20250929-v1:0') -> False, so the cells warn -> expected True per contract §9.
No test fails with the baseline->treatment loop of _models_equivalent removed, or with the org/ prefix or the 8-digit date rule dropped.
Commits:
1d584f2: the compare model warnings follow --model; the stamp stays batch-wide like harness_versions.
996de3c: both sides map @DATE to -DATE before matching.
47498b9: tests that pin the forward loop, org/ and \d{8}.
Left for the author: three sentences in the PR body no longer match the code (undated vs dated "warns", "@Date not normalized", init.model "repeats the request").
The reason will be displayed to describe this comment to others. Learn more.
Approved at 47498b9. No must-fix or mid item remains after our commits, each red without its fix; full suite, ruff, mypy and bandit clean on Linux; CI green.
Thanks @xaleryb. I checked your three commits on fork/feat/repo-resolved-model (head 47498b9) and agree with each point:
repo compare --model m-2 warning scope: fixed in 1d584f2. Model warnings now follow --model; the stamp stays batch-wide. test_model_warnings_follow_the_selected_model fails on the parent 1559045 and passes at 47498b9.
@DATE vs -DATE and the prefixed-dated case: fixed in 996de3c. model_matches('claude-sonnet-4-5@20250929', 'claude-sonnet-4-5-20250929') and ('claude-sonnet-4-5', 'us.anthropic.claude-sonnet-4-5-20250929-v1:0') are now True. Both parametrized cases fail on 1559045.
Pinned tests: 47498b9 adds the baseline-to-treatment check, the org/ prefix and the 8-digit snapshot rule coverage you listed.
The PR body sentences you listed (undated vs dated "warns", "@Date not normalized", init.model "repeats the request") are not updated in this reply; I will correct them in the body separately.
The reason will be displayed to describe this comment to others. Learn more.
Review at 47498b9. The equivalence rule now does what the contract says, and the earlier threads on it are fixed: the snapshot rule in either spelling, -v2 against -v3, the crossed prefix and suffix, and --model scoping of the compare warnings. One test gap is worth closing before merge, and one nit.
make ci-local: 19 passed, 0 failed, 4 skipped; the changed-line coverage step passed.
Reproductions for the findings, with no model called.
Mutation runs: 52 mutants of report.py, compare.py, events.py and runner.py against test_repo_compare.py, test_repo_run.py and test_repo_reach.py (183 tests); 46 killed, 6 survived, 2 of them real behaviour changes.
Checked, no issues
Extraction: the main-thread filter, the <synthetic> skip, reply-over-init precedence and the null for a text trace are each pinned by test_claude_stream_json_names_the_models_that_answered.
Equivalence rule:model_matches is True for the crossed us.anthropic.claude-haiku-4-5-20251001-v1:0 / anthropic.claude-haiku-4-5-20251001 pair and for claude-sonnet-4-5 / us.anthropic.claude-sonnet-4-5-20250929-v1:0. It is False for deepseek-v2 / deepseek-v3, two different snapshot dates, and haiku / anthropic.claude-haiku-4-5-20251001-v1:0, so the real alias pin still warns.
Old cells:resolved_models takes a missing, null, string or list model_resolved.
Rules: no schema for meta.json or result.json, so no SCHEMA_VERSIONS entry; the changelog fragment is in changelog.d/; no generated files; no new docs page; tests are offline.
The same pair across arms in _models_equivalent is False. I raised this in my first review. The anthropic. / us.anthropic. and -v1:0 crossings are now fine, so only the region swap is left.
Suggestion: compare the part after the last . or / of both sides, or say in §9 that a different region profile warns.
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 385e9d9. model_matches now strips a Bedrock cross-region inference prefix (us., eu., apac., us-gov., global., jp., au., ca.) from both ids before comparing, so us.anthropic.claude-x vs eu.anthropic.claude-x no longer warns, in report and in the compare arm check. A bare dot split is not used, so gpt-4.1 vs gpt-4.2 still warns. Rows added to test_a_provider_prefix_is_not_another_model; the us/eu rows fail on the previous code.
The fake harness echoed the requested model, so replacing the record's
model_resolved with the request went unnoticed. The fixture can now
answer with FAKE_ANSWER_MODEL, and the run test pins that the record
and the model_mismatch summary carry the answering model.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The reason will be displayed to describe this comment to others. Learn more.
LGTM. The run test now pins model_resolved against a different answering model, and the extraction, persistence and scoping of the warnings hold up. One nit inline.
Mutation runs: 28 mutants of runner.py, events.py, report.py and compare.py against test_repo_compare.py, test_repo_run.py and test_repo_reach.py (189 tests); 27 killed, 1 is equivalent (union instead of intersection of the arms' models, skipped by the base and treat guard).
Still open from my earlier review:
dokimi/dokimi/repos/report.py:141: a region swap still warns. model_matches("eu.anthropic.claude-x", "us.anthropic.claude-x") is False at this head, and _models_equivalent gives the same for the pair across arms.
Checked, no issues
Run test: replacing meta.get("model_resolved") with opts.model in runner.py now fails test_a_run_answered_by_another_model_is_recorded_and_reported.
Extraction: the <synthetic> skip, the parent_tool_use_id filter and reply-over-init precedence each fail a test when removed.
Old cells: a record with model_resolved missing, null, a string or a list goes through resolved_models.
Rules: no schema covers meta.json or result.json, the changelog fragment is in changelog.d/, no generated files, tests are offline.
The reason will be displayed to describe this comment to others. Learn more.
Nit: the docstring says only -v1:0 is stripped, the regex strips every Bedrock -vN:M.
model_matches("m-v1:0", "m-v3:0") is True at this head, so a request and a reply that differ only in the Bedrock revision raise no model_mismatch warning. The docstring describes the suffix as -v1:0.
Suggestion: either narrow the regex to -v1:0, or reword the docstring to say any -vN:M revision is treated as the same model.
The reason will be displayed to describe this comment to others. Learn more.
Reworded rather than narrowed in 03a286c: a -vN:M suffix is the Bedrock revision of one model id, so _normalize_model_id and model_matches now say any -vN:M counts as the same model. A ("m-v1:0", "m-v3:0", True) row in test_a_provider_prefix_is_not_another_model pins it.
The regex strips every -vN:M suffix; the docstrings said -v1:0. Pin the
behavior with a test row.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
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
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.
antonwolfy
left a comment
Summary
repo runrecorded the model it was asked for, not the model that answered. A harness alias can silently pin an older model: we hit an aliashaikuthat still pointed at Haiku 4.5 while Haiku 5.5 was current, and nothing in the batch showed it (#41, #65).model_matches()does not treat a provider prefix (us.anthropic.,org/) or a Bedrock-v1:0suffix as a different model, because Bedrock runs report the request with the prefix and the reply without it.Evidence
Before: a run with
--model haikurecords"model": "haiku".repo reportprints no warning.After: checked against real Claude Code 2.1.296 traces from earlier oneDAL batches.
init.modelmodel_resolvedus.anthropic.claude-haiku-4-5-20251001-v1:0anthropic.claude-haiku-4-5-20251001-v1:0us.anthropic.claude-haiku-5-5claude-haiku-5-5claude-sonnet-5claude-sonnet-5haiku(alias; the first row's reply checked against this request)anthropic.claude-haiku-4-5-20251001-v1:0Tests:
test_repo_reach.py;test_repo_run.py;test_repo_compare.py.The fake harness in
repo_eval_fixture.pynow puts--modelin its init and assistant events.ruff check .andmypy(212 files) are clean.make ci-local: the first run failed once in the test step (176 s against about 88 s normally; output not kept). Every run since has passed: 19 passed, 4 skipped, 5186 tests, plus three morepytest -n autoruns.Merge Danger
Door: two-way
The change adds fields and warnings. Nothing that existed before is removed or renamed.
Blast Radius: small
repo report --jsonseemodel_resolvedon rows andmodel_mismatchin the summary.repo comparebatch stamps gainmodel_resolved.claude-sonnet-4-5answered asclaude-sonnet-4-5-20250929. That is alias resolution too, but it may be noisy.@dateIDs are not normalized and would warn.🤖 Generated with Claude Code