werkrevisor 0.21.1: MiniMax M3 advises in the reviews on git.javagil.de, a failed advising model leaves the verdict to Jev #40
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: mi/werkrevisor#40
Reference in New Issue
Block a user
See docs/prs/2026-10-07-PR#40-economy-model.md.
🤖 Generated with Claude Code
WIP: MiniMax M3 advises in the reviews on git.javagil.deto werkrevisor 0.21.1: MiniMax M3 advises in the reviews on git.javagil.de, a failed advising model leaves the verdict to Jev🟠 werkrevisor ORANGE for
3de4a38, superseded by the result for8202fe0🟠 werkrevisor: ORANGE / FINDINGS
Change kind:
mixed.Reasons
Findings
[medium] Catch-all swallows Superseded and other non-model failures at
werkrevisor/cli.py:207-213, ORANGE, as a line comment[medium] Test no longer checks the factor for
werkator.tomlattests/test_mechanical.py:290-296, hint, as a line commentClassifier
5 of 5 units passed all questions (1 PR document(s)).
Usage: model `openrouter/anthropic/claude-sonnet-5.5`, LLM done, tokens counted before the call 86794, calls 1, tokens in 86152 / out 598 (reasoning included in out), duration 17.8 s, cost 0.1783 USD (reported), classifier `jev-1.13.0` calls 5, tokens in 4109, cost 0.0002 USD, finding judge `jev-1.13.0` calls 2, tokens in 2862, cost 0.0001 USD, excerpt selection `openrouter/openai/gpt-6-luna` calls 1, tokens in 13447 / out 147, cost 0.0018 USD (reported), build file judge `jev-1.13.0` calls 11, tokens in 7789, cost 0.0003 USD, config: defaults + host + repository (.werkrevisor.toml at main1 answer ignored because of a confidence below [classifier] min_confidence.
32a13f7).Reviewed head `3de4a387f55dcf08c5320ab410192f7c4c8a36d4` against base `32a13f7b07f1d78800c5c4d033fb4aec99f3efb9`, werkrevisor 0.21.0.
werkrevisor: ORANGE / FINDINGS for
3de4a387f55dcf08c5320ab410192f7c4c8a36d4.The result comment of werkrevisor holds the details.
@@ -288,3 +288,3 @@def test_example_configs(self):for name in ("werkprobe.toml", "hsadmin-ng.toml", "werkrevisor.toml"):# MiniMax M3 counts close to o200k_base, Anthropic counts more.[medium] Test no longer checks the factor for
werkator.toml(werkrevisor)hint; Jev: real 0.63, severity 2.0 of 3
test_example_configspreviously assertedtoken_count_factor == 1.55for the listed configs. It now uses per-file factors. The newwerkator.toml(factor 1.05) is not in the list, so its changed factor is not covered by this test, and the 1.05 asserted forwerkrevisor.tomlis the only guard for it. The loop also never checkswerkator.tomlfor mechanical or engine settings, as before. The test is not weaker per entry, but the changedwerkator.tomlmodel settings have no test.Fixed in
8202fe0: the test now also checkswerkator.toml(factor 1.05); it was not in the list before this PR either.@@ -204,6 +204,13 @@ def _review(config, pr, deps, redactor, use_llm, result, pr_data):if not (reductions and e.status == "skipped (too large)" and not e.details.get("num_ai_calls")):stop = ebreakexcept Exception as e:[medium] Catch-all swallows Superseded and other non-model failures (werkrevisor)
ORANGE; Jev: real 0.85, severity 3.0 of 3
The new
except Exceptionclause wraps the wholedeps.model_reviewcall. Withmodel_role = "advises"it also swallowsSupersededif it is raised insidemodel_revieworadmit. It also swallows other bugs, and the review then continues with a hint, so a GREEN verdict is possible.Supersededderives fromExceptionand must reachexecute, which turns it into ERROR.Supersededand any other non-model error should be re-raised, so that only model call failures becomemodel_failed.Fixed in
8202fe0:Supersededis re-raised before the catch-all, tested by test_a_superseded_head_still_ends_the_review_when_the_model_only_advises (red without the fix).Other failures of the model step stay caught on purpose when the model only advises: Jev has judged every file, and the reason
model_failednames the exception, so a bug is still visible as a hint.Review round of
3de4a38answered in8202fe0: both findings fixed and resolved; 538 tests pass.🟠 werkrevisor ORANGE for
8202fe0, superseded by the result for72e9d8b🟠 werkrevisor: ORANGE / FINDINGS
Change kind:
mixed.Reasons
Findings
[medium] Model failure after a real model call loses the cost and usage at
werkrevisor/cli.py:209-215, ORANGE, as a line comment[medium] PR document contradicts the code on the timeout test at
docs/prs/2026-10-07-PR#40-economy-model.md:31-44, hint, as a line commentClassifier
5 of 5 units passed all questions (1 PR document(s)).
Usage: model `openrouter/anthropic/claude-sonnet-5.5`, LLM done, tokens counted before the call 88199, calls 1, tokens in 87558 / out 616 (reasoning included in out), duration 18.2 s, cost 0.1813 USD (reported), classifier `jev-1.13.0` calls 5, tokens in 4118, cost 0.0002 USD, finding judge `jev-1.13.0` calls 2, tokens in 3384, cost 0.0001 USD, excerpt selection `openrouter/openai/gpt-6-luna` calls 1, tokens in 13447 / out 191, cost 0.0002 USD (reported), build file judge `jev-1.13.0` calls 11, tokens in 8003, cost 0.0003 USD, config: defaults + host + repository (.werkrevisor.toml at main1 answer ignored because of a confidence below [classifier] min_confidence.
32a13f7).Reviewed head `8202fe0583da7b9807dce2f9c2541a823980c8a6` against base `32a13f7b07f1d78800c5c4d033fb4aec99f3efb9`, werkrevisor 0.21.0.
werkrevisor: ORANGE / FINDINGS for
8202fe0583da7b9807dce2f9c2541a823980c8a6.The result comment of werkrevisor holds the details.
@@ -0,0 +28,4 @@### Feature: an advising model never ends a review#### Scenario#40.01: a failed advising model leaves the verdict to Jev[medium] PR document contradicts the code on the timeout test (werkrevisor)
hint; Jev: real 0.70, severity 1.8 of 3
The document and the test message say a call "exceeded 720 seconds", while the configured
timeout_secondsis 300 withmax_retries = 1. The test error text is therefore inconsistent with the documented 2 calls of up to 300 s. The "Scenario#40.01" table says "not every changed file" gives YELLOW. In the code, YELLOW arises only because the reasonmodel_failedis not downgraded whentriage_completeis false. The document does not say thatmodel_failedhas the verdict GREEN by default, so the hint-or-YELLOW logic is hard to follow.Clarified in
72e9d8b: the scenario now says thatmodel_failedis YELLOW and becomes a hint only when Jev judged every changed file.The test text "review exceeded 720 seconds" is the real message of the whole review timeout in
review.py: 2 calls × 300 s + 120 s; the PR document speaks of the per-call timeout of 300 s, so both fit.@@ -206,1 +206,4 @@breakexcept Superseded:raiseexcept Exception as e:[medium] Model failure after a real model call loses the cost and usage (werkrevisor)
ORANGE; Jev: real 0.87, severity 3.0 of 3
When the advising model fails, the handler sets
llm_statusand returns without ever settingresult.cost_usdandresult.cost_status. These were set toNoneand"unknown"before the loop, so the usage record shows an unknown cost even though the failed call may have been billed. A model that times out at 300 s after 32000 reasoning tokens costs money, and the retry logic can make it two calls. This also bypasses the budget accounting that theNoModelCallbranch keeps.Fixed in
72e9d8bas far as possible: PR-Agent returns no usage for a call that raises, so the cost stays "unknown" (as it did before, when the same failure ended the review as ERROR).The handler now records the remaining key limit after the failure, like a successful review does, so the spend shows as the difference to
budget_remaining_before; the test asserts it (red without the fix).Review round of
8202fe0answered in72e9d8b: both findings fixed or clarified and resolved; 538 tests pass.🟢 werkrevisor GREEN for
72e9d8b, superseded by the result foraba8254🟢 werkrevisor: GREEN / PASS
Gate: approved by werkrevisor; merge waits for 2 open hint(s): reply to each line comment and resolve it, then restart the review or push a commit.
Change kind:
mixed.Reasons
Findings
[medium] Failed model call skips usage and cost record at
werkrevisor/cli.py:209-220, hint, as a line comment[low] PR document contradicts the code on when a failure is a hint at
docs/prs/2026-10-07-PR#40-economy-model.md:44-45, hint, as a line commentClassifier
5 of 5 units passed all questions (1 PR document(s)).
Usage: model `openrouter/anthropic/claude-sonnet-5.5`, LLM done, tokens counted before the call 88820, calls 1, tokens in 88189 / out 651 (reasoning included in out), duration 18.5 s, cost 0.1829 USD (reported), classifier `jev-1.13.0` calls 5, tokens in 4164, cost 0.0002 USD, finding judge `jev-1.13.0` calls 1, tokens in 1652, cost 0.0001 USD, excerpt selection `openrouter/openai/gpt-6-luna` calls 1, tokens in 13447 / out 193, cost 0.0002 USD (reported), build file judge `jev-1.13.0` calls 11, tokens in 8087, cost 0.0003 USD, config: defaults + host + repository (.werkrevisor.toml at main1 answer ignored because of a confidence below [classifier] min_confidence.
32a13f7).Reviewed head `72e9d8b74cd058a47272bad532b677ef812c459f` against base `32a13f7b07f1d78800c5c4d033fb4aec99f3efb9`, werkrevisor 0.21.0.
werkrevisor: GREEN / PASS for
72e9d8b74cd058a47272bad532b677ef812c459f.The result comment of werkrevisor holds the details.
@@ -0,0 +41,4 @@| every changed file and found nothing | GREEN, a hint || not every changed file | YELLOW |The reason `model_failed` is YELLOW and becomes a hint only when Jev judged every changed file.[low] PR document contradicts the code on when a failure is a hint (werkrevisor)
hint
The scenario table says a failure with every changed file judged gives GREEN, and one without gives YELLOW. In the code the failure only becomes a hint when
triage_completeholds. That also requiresuse_llm, Jev available and no secrets. The document says only "Jev judged every changed file", so the condition is stated more narrowly than the code applies it. Separately, "Withmodel_role = "decides", the failure stays an ERROR, and a newer head still ends the review" reads as if the superseded case is specific todecides. In the code,Supersededends the review for both roles.Clarified in
aba8254: a newer head ends the review with either role; that sentence now stands apart from thedecidesone."Jev judged every changed file" is the definition of
triage_completeas PR#39 introduced it; a secret or a skipped Jev already gives YELLOW or RED on its own, so the scenario does not repeat these conditions.@@ -206,1 +206,4 @@breakexcept Superseded:raiseexcept Exception as e:[medium] Failed model call skips usage and cost record (werkrevisor)
hint; Jev: real 0.78, severity 2.3 of 3
On a failed advising model call, the new
except Exceptionbranch returns early. It setsresult.llm_statusbut never setsresult.cost_usd/cost_status, which stay at theNone, "unknown"set before the loop. The call may still have cost money, because a timeout after partial generation is billed. The budget fields only partly cover this:budget_remaining_beforeis set inadmit, andbudget_remaining_afteris set here. Nothing computes a cost from them, so the failed call is probably missing from the statistics service's LLM cost. This is a limited reporting gap, not a verdict bug.Intended, no change: the key is shared by all reviews of the host, so the difference of the two key limits is no reliable cost of this one review, and werkrevisor does not book it as such.
The record keeps
cost_status = "unknown", which the statistics show as unknown cost, as for any other failed call; the two limits stay in the record for a manual check.Review round of
72e9d8banswered inaba8254: one hint clarified, one intended; both resolved.New commits pushed, approval review dismissed automatically according to repository settings
🟠 werkrevisor: ORANGE / FINDINGS
Change kind:
mixed.Reasons
Findings
[medium] Failed model call leaves partial state and skips the cost record at
werkrevisor/cli.py:209-220, ORANGE, as a line comment[low] Document claims "Jev judged every file" gives GREEN, but the scenario is incomplete at
docs/prs/2026-10-07-PR#40-economy-model.md:33-46, hint, as a line commentClassifier
5 of 5 units passed all questions (1 PR document(s)).
Usage: model `openrouter/anthropic/claude-sonnet-5.5`, LLM done, tokens counted before the call 88343, calls 1, tokens in 87787 / out 620 (reasoning included in out), duration 17.3 s, cost 0.1818 USD (reported), classifier `jev-1.13.0` calls 5, tokens in 4166, cost 0.0002 USD, finding judge `jev-1.13.0` calls 1, tokens in 1664, cost 0.0001 USD, excerpt selection `openrouter/openai/gpt-6-luna` calls 1, tokens in 13447 / out 229, cost 0.0002 USD (reported), build file judge `jev-1.13.0` calls 11, tokens in 8087, cost 0.0003 USD, config: defaults + host + repository (.werkrevisor.toml at main32a13f7).Reviewed head `aba82543c4eaafa5ed76959c5be84c5268db39d4` against base `32a13f7b07f1d78800c5c4d033fb4aec99f3efb9`, werkrevisor 0.21.0.
werkrevisor: ORANGE / FINDINGS for
aba82543c4eaafa5ed76959c5be84c5268db39d4.The result comment of werkrevisor holds the details.
@@ -0,0 +30,4 @@#### Scenario#40.01: a failed advising model leaves the verdict to JevSo that a slow economy model does not hold a PR that Jev found harmless.[low] Document claims "Jev judged every file" gives GREEN, but the scenario is incomplete (werkrevisor)
hint
Scenario 40.01 says a failure is a reason of verdict
<verdict>, with GREEN only when Jev judged every changed file. The code's_advisoryturnsmodel_failedGREEN only whentriage_completeholds. That also requires no secrets,use_llm, anddeps.jevset. The table does not mention these conditions, so a reader may expect GREEN where YELLOW results.Same point as 1769, no change: the scenario names the condition by its meaning, Jev judged every changed file; the other inputs (no LLM skip, Jev configured) are preconditions of that judgment, not extra rules of this PR.
@@ -206,1 +206,4 @@breakexcept Superseded:raiseexcept Exception as e:[medium] Failed model call leaves partial state and skips the cost record (werkrevisor)
ORANGE; Jev: real 0.77, severity 2.8 of 3
On a generic exception with
model_role == "advises", the code appendsmodel_failedand returns. It leavesresult.token_countfromadmitandresult.budget_remaining_beforeset, but never setsresult.cost_usdorcost_status. They stay atNoneand"unknown", so the usage record lacks the cost of a failed call that may have used tokens (the 0.02 USD in the PR doc). The statistics service therefore under-reports LLM cost for exactly the failures this PR expects with a slow model. A caveat: the usage-record consumers are not visible here, so the impact is uncertain.Intended, no change (same point as 1768): the statistics do not under-report a failed call silently.
They count it in
unknownCostReviews("N with unknown cost") and mark the LLM sum as estimate (≈), and the review row shows "+ LLM unknown".The record also keeps
token_countandworst_case_usdfromadmit, the upper bound of the call, and the key limits before and after it; inventing a cost from them would overstate or misattribute it.Review round of
aba8254answered: both findings repeat 1768/1769 and are intended; resolved without a code change. The ORANGE needs a human decision.