werkrevisor 0.20.5: the result comment holds the findings until the line comments exist #36
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: mi/werkrevisor#36
Reference in New Issue
Block a user
See the PR doc.
🤖 Generated with Claude Code
🟠 werkrevisor ORANGE for
6a4d136, superseded by the result for408d97c🟠 werkrevisor: ORANGE / FINDINGS
Change kind:
mixed.Reasons
Findings
[medium] Failed review leaves a stale "announced" state and the hold-note edit can drop findings at
werkrevisor/cli.py:743-752, ORANGE, as a line comment[medium] Test does not cover the held-merge edit after a failed shortening at
tests/test_line_comments.py:109-118, hint, as a line commentClassifier
8 of 8 units passed all questions (1 PR document(s)).
Usage: model `openrouter/anthropic/claude-sonnet-5.5`, LLM done, tokens counted before the call 58608, calls 1, tokens in 57225 / out 552 (reasoning included in out), duration 17.2 s, cost 0.1200 USD (reported), classifier `jev-1.13.0` calls 8, tokens in 6280, cost 0.0003 USD, finding judge `jev-1.13.0` calls 2, tokens in 3510, cost 0.0001 USD, build file judge `jev-1.13.0` calls 1, tokens in 560, cost 0.0000 USD, config: defaults + host + repository (.werkrevisor.toml at base4 answers ignored because of a confidence below [classifier] min_confidence.
93b3979).Reviewed head `6a4d136e8ee9da17dcb3c00aa3aa35a8ba66fb06` against base `93b3979350ce0b3887b4480367a46c7fdc2f26a1`, werkrevisor 0.20.4.
werkrevisor: ORANGE / FINDINGS for
6a4d136e8ee9da17dcb3c00aa3aa35a8ba66fb06.The result comment of werkrevisor holds the details.
@@ -93,6 +106,17 @@ class FailureTest(unittest.TestCase):finding_run([APP], header="[low] Naming", gitea=gitea)self.assertEqual([r[0] for r in gitea.created_reviews], ["APPROVED"])def test_the_result_comment_keeps_the_findings_if_gitea_fails_twice(self):[medium] Test does not cover the held-merge edit after a failed shortening (werkrevisor)
hint; Jev: real 0.77, severity 2.0 of 3
No test covers
_note_holdre-rendering the result comment after_shorten_to_line_commentsfailed. The new tests cover failure of both the review and the edit, but not the case where the review succeeds, the shortening fails and a later edit happens.Test added in
408d97c:test_a_later_edit_shortens_the_result_once_the_line_comments_existlets the shortening fail and checks that the hold note then shortens the comment while the line comment exists.@@ -740,2 +740,4 @@fresh = existing is None and result.verdict is not Verdict.ERRORcomments = _plan_line_comments(gitea, pr, result, bot_login, redactor) if fresh else []# Until Gitea has accepted the line comments, the result comment is the only place of their findings.attached, result.attached = result.attached, set()[medium] Failed review leaves a stale "announced" state and the hold-note edit can drop findings (werkrevisor)
ORANGE; Jev: real 0.66, severity 2.8 of 3
_publishclearsresult.attachedbefore posting. If_publish_reviewraises,result.attachedstays empty, which is intended. But_note_holdinexecutelater re-renders the comment withresult.attachedrestored to the full set when the review was accepted. If the "shortening" edit failed in_shorten_to_line_comments,_note_holdre-renders withresult.attachedset and overwrites the full-text comment with the short version. The line comments exist, so the findings are not lost, but the stated guarantee "a failed edit leaves the full text" no longer holds when a held merge is noted afterwards.Behaviour unchanged, doc made precise in
408d97c:_note_holdruns only after Gitea accepted the review, so the line comments exist and the short form loses nothing. The guarantee is about findings, not about the full text: the full text stays as long as no line comment holds it.Answer to the review of
6a4d136(ORANGE):werkrevisor/cli.py:743-752(hold note shortens after a failed shortening): intended, since the line comments exist then; the doc states it precisely in408d97c, thread resolved.408d97c, thread resolved.statistics/build.gradle.kts: only the version bump.🟠 werkrevisor ORANGE for
408d97c, superseded by the result forb8ea578🟠 werkrevisor: ORANGE / FINDINGS
Change kind:
mixed.Reasons
Findings
[medium] Failed shortening edit is never retried on a rerun at
werkrevisor/cli.py:745-752, ORANGE, as a line comment[medium] Fold test weakened to a set comparison at
tests/test_line_comments.py:225-226, hint, as a line commentClassifier
8 of 8 units passed all questions (1 PR document(s)).
Usage: model `openrouter/anthropic/claude-sonnet-5.5`, LLM done, tokens counted before the call 63874, calls 1, tokens in 62425 / out 660 (reasoning included in out), duration 14.0 s, cost 0.1315 USD (reported), classifier `jev-1.13.0` calls 8, tokens in 6311, cost 0.0003 USD, finding judge `jev-1.13.0` calls 2, tokens in 3473, cost 0.0001 USD, build file judge `jev-1.13.0` calls 1, tokens in 560, cost 0.0000 USD, config: defaults + host + repository (.werkrevisor.toml at base4 answers ignored because of a confidence below [classifier] min_confidence.
93b3979).Reviewed head `408d97c7a6f4cbac63c80e9211854881e2c985e1` against base `93b3979350ce0b3887b4480367a46c7fdc2f26a1`, werkrevisor 0.20.4.
werkrevisor: ORANGE / FINDINGS for
408d97c7a6f4cbac63c80e9211854881e2c985e1.The result comment of werkrevisor holds the details.
@@ -200,3 +224,2 @@finding_run([APP], gitea=gitea)[(comment_id, body)] = gitea.edited_commentsself.assertEqual(comment_id, 3)[body] = [body for comment_id, body in gitea.edited_comments if comment_id == 3][medium] Fold test weakened to a set comparison (werkrevisor)
hint; Jev: real 0.64, severity 1.9 of 3
test_new_result_folds_the_earlier_oneswas[(comment_id, body)] = gitea.edited_comments, which required exactly one edit. It now accepts any edits whose ids are in {3, 5} and picks the body by id. Likewisetest_a_mention_of_the_head_...changed from an exact list[3]to a sorted[3, 4]. These asserts are justified by the new shortening edit but no longer ensure the earlier comment is edited only once (duplicate fold edits would pass the set check). Consider asserting the exact edit list in order.Fixed in
b8ea578: both fold tests assert the exact list of edits in order,[5, 3]and[4, 3].@@ -742,2 +742,4 @@# Until Gitea has accepted the line comments, the result comment is the only place of their findings.attached, result.attached = result.attached, set()created = _post_comment(config, pr, gitea, result, existing, bot_login, redactor, gate_note)if result.verdict is not Verdict.ERROR:[medium] Failed shortening edit is never retried on a rerun (werkrevisor)
ORANGE; Jev: real 0.78, severity 2.8 of 3
If
_shorten_to_line_commentsfails and no later edit happens, the result comment keeps the full findings text even though the line comments exist, duplicating the text. A rerun of the same head reuses the existing comment (existing is not None) and_publishskips line-comment planning, soresult.attachedstays empty and the comment is never shortened. The code comment says this loses nothing, but the duplicate stays permanently. Also, the README states the comment lists such findings in one line; this breaks it after a transient 502. This is a trade-off, but the PR doc should state that the duplicate is permanent, and no test covers the permanent case on rerun beyond the fold test comment.Documented in
b8ea578: without a later edit in the same run, the repetition stays, since a rerun reuses the result comment as it is. That is the chosen trade-off: a repeated finding after a rare failed edit, instead of a lost one.test_a_failed_fold_is_retried_by_a_rerunshows the rerun keeping the comment.Answer to the review of
408d97c(ORANGE):werkrevisor/cli.py:745-752(failed shortening is never retried): intended trade-off, now stated in the PR doc in b8ea578; thread resolved.b8ea578, thread resolved.statistics/build.gradle.kts: only the version bump.🟢 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] Exception in review leaves the result comment with full findings, but
_publish_reviewfailure is no longer handled atwerkrevisor/cli.py:745-752, hint, as a line comment[low] Scenario#36.03 mixes unrelated change at
docs/prs/2026-10-06-PR#36-result-comment-holds-the-findings-first.md:52-72, hint, as a line commentClassifier
8 of 8 units passed all questions (1 PR document(s)).
Usage: model `openrouter/anthropic/claude-sonnet-5.5`, LLM done, tokens counted before the call 63912, calls 1, tokens in 62453 / out 525 (reasoning included in out), duration 11.7 s, cost 0.1302 USD (reported), classifier `jev-1.13.0` calls 8, tokens in 6333, cost 0.0003 USD, finding judge `jev-1.13.0` calls 1, tokens in 1638, cost 0.0001 USD, build file judge `jev-1.13.0` calls 1, tokens in 560, cost 0.0000 USD, config: defaults + host + repository (.werkrevisor.toml at base3 answers ignored because of a confidence below [classifier] min_confidence.
93b3979).Reviewed head `b8ea578210cd7b1346b5d7a73b9e4e2a15addda9` against base `93b3979350ce0b3887b4480367a46c7fdc2f26a1`, werkrevisor 0.20.4.
werkrevisor: GREEN / PASS for
b8ea578210cd7b1346b5d7a73b9e4e2a15addda9.The result comment of werkrevisor holds the details.
@@ -0,0 +49,4 @@- [test_line_comments: test_finding_with_a_line_becomes_a_line_comment](../../tests/test_line_comments.py)- [test_hints_hold_merge: test_a_later_edit_shortens_the_result_once_the_line_comments_exist](../../tests/test_hints_hold_merge.py)### Feature: the kind of the bot review follows the gate[low] Scenario#36.03 mixes unrelated change (werkrevisor)
hint
Scenario#36.03 documents only a missing test, not a behavior change of this PR. The Problem section mentions it in one line, but the PR title and the Solution do not. This blurs the scope of the PR document.
Not changed: the Problem names the missing test as the second follow-up of PR#20, and the user decided to put both into one PR; Scenario#36.03 is the place where the doc says what that test shows.
@@ -742,2 +742,4 @@# Until Gitea has accepted the line comments, the result comment is the only place of their findings.attached, result.attached = result.attached, set()created = _post_comment(config, pr, gitea, result, existing, bot_login, redactor, gate_note)if result.verdict is not Verdict.ERROR:[medium] Exception in review leaves the result comment with full findings, but
_publish_reviewfailure is no longer handled (werkrevisor)hint; Jev: real 0.51, severity 2.0 of 3
The removed
try/exceptaround_publish_reviewused to move the findings into the comment when the review call raised. Now the comment keeps the full text from the start, which covers the loss. However,result.attachedstays empty after an exception, andreusedfindings and the laterrender_commentcalls (for example_note_hold) then render the findings in full. This is acceptable, but the exception path has no test that checks the comment is not shortened when the review fails and a later edit succeeds. Only the "fails twice" case is tested.Not changed: if
_publish_reviewraises, the exception ends the run with ERROR, so no later edit of the run follows, and the comment keeps the full text.test_failed_review_moves_the_findings_into_the_comment_and_a_rerun_submits_itcovers a failing review with working edits and checks the full text in the comment.Answer to the review of
b8ea578(GREEN, merge held for 2 hints):werkrevisor/cli.py:745-752(exception path): the run ends with ERROR, no later edit follows; covered by an existing test. Resolved.Restarting the review to release the merge.
werkrevisor, review restarted for
b8ea578210cd7b1346b5d7a73b9e4e2a15addda9: approved by werkrevisor; merge scheduled for when the checks succeed; review request of mi withdrawn.