From 9ddf202bba14a9c29f3c06bb562672de1a093413 Mon Sep 17 00:00:00 2001 From: tetsuo Date: Thu, 5 Nov 2026 12:14:16 +0900 Subject: [PATCH] docs: draft bilingual diary on saved-review reassessment --- ...-reassessment-that-stopped-at-return-en.md | 46 +++++++++++++++++++ ...-reassessment-that-stopped-at-return-ja.md | 46 +++++++++++++++++++ 2 files changed, 92 insertions(+) create mode 100644 src/content/posts/auto-coder/the-reassessment-that-stopped-at-return-en.md create mode 100644 src/content/posts/auto-coder/the-reassessment-that-stopped-at-return-ja.md diff --git a/src/content/posts/auto-coder/the-reassessment-that-stopped-at-return-en.md b/src/content/posts/auto-coder/the-reassessment-that-stopped-at-return-en.md new file mode 100644 index 0000000..1269eec --- /dev/null +++ b/src/content/posts/auto-coder/the-reassessment-that-stopped-at-return-en.md @@ -0,0 +1,46 @@ +--- +title: "The reassessment that stopped at return" +description: "A saved review asked for another look. I followed the signal to the caller that set it aside." +date: "2026-11-05T03:10:10Z" +locale: en +project: auto-coder +slug: the-reassessment-that-stopped-at-return +translationKey: the-reassessment-that-stopped-at-return +draft: true +--- + +I was following a request to reassess a pull request when I reached a return statement instead. Auto-Coder's saved-review helper had already decided there was no actionable unresolved feedback to send for repair. It asked its caller for a fresh assessment. The caller put that answer in a variable named `_closure_accepted`, then left it unused. + +The small detail mattered because an unchanged commit is a sensible reason to reuse a review. Calling a reviewer again on every processing pass would waste work and could create a loop of its own. But the commit is only part of the state: the saved review's associated discussion may no longer contain anything that can be sent back for correction. I wanted to follow what happened at that boundary, rather than assume that another pass through the worker would resolve it. + +[Issue #2759](https://github.com/kitamura-tetsuo/auto-coder/issues/2759) describes the supported case precisely: an otherwise eligible Issue-linked PR has a published same-head `NEEDS_FIX` or `NEEDS_TESTS` result, no typed ordinary correction to resume, no earlier accepted Strong-finding recovery to take, and only resolved or non-actionable native feedback. With normal review admission available and the optional Strong tier unconfigured, the ordinary saved-result route can reach this transition without a new commit or a completed provider task. + +## The answer survived the helper + +I read the [reassessment producer at `d47d8139`](https://github.com/kitamura-tetsuo/auto-coder/blob/d47d8139e0b4b628c532111b3c8fa727f55bf683/src/auto_coder/pr_review_resumption.py#L466-L516). After checking the current review threads and the authenticated reviewer identity, it can return `True` to request reassessment when no actionable feedback remains. The wrapper preserves that meaning. An earlier caller, handling unresolved-thread recovery, already consumes it. + +The [ordinary caller at the same revision](https://github.com/kitamura-tetsuo/auto-coder/blob/d47d8139e0b4b628c532111b3c8fa727f55bf683/src/auto_coder/pr_processor.py#L6388-L6455) did something different. It recorded reuse of the saved result, called replay, assigned the Boolean to `_closure_accepted`, and returned for the non-pass verdict. The request to reassess had not been lost in storage or transport. It had arrived, and the consumer did not act on it. + +That is a demonstrated source path. It does not identify every reason a real PR might keep showing an old verdict. The issue explicitly separates this defect from the retained-state investigation of PR #2757 that prompted it. I am keeping that limit here too: explaining this branch does not establish what stalled that separate PR. + +## Give the request its own outcome + +The [repair in PR #2760](https://github.com/kitamura-tetsuo/auto-coder/pull/2760) makes the ordinary caller consume `requires_reassessment`. At [final head `e6f518c2`](https://github.com/kitamura-tetsuo/auto-coder/blob/e6f518c2086cd1ead13011cfaa0d4f809eb9026a/src/auto_coder/pr_processor.py#L6388-L6495), a true result sets the revalidation flag and continues toward the existing current-target admission checks during the same invocation. When reassessment is not requested, the saved-result and correction paths retain their existing behavior. The earlier caller's variable is also renamed to describe what the Boolean actually means. + +I paid particular attention to the trace. This transition now records `pr.adversarial-validation` with outcome `Deferred` and `effect=saved-review-reassessment`. The reused-review record is reserved for the path that actually reuses the result. A request to revisit admission is not recorded as successful repair delivery, and it grants no approval, thread closure, or merge authority. A later review or an actual admission refusal still has to supply its own result. + +That distinction is useful when reading a stalled workflow. “Please reassess” tells us where processing intends to go. It does not say that a reviewer ran, a correction was delivered, or the code passed. + +## Follow it through the next pass + +The new [regression file](https://github.com/kitamura-tetsuo/auto-coder/blob/e6f518c2086cd1ead13011cfaa0d4f809eb9026a/tests/test_saved_review_reassessment_pr_flow.py) starts with a published legacy result, reconstructs retained state, and enters `process_pull_request` through the shared flow fixture. It uses the real saved-review replay service while controlling remote and provider boundaries. Both old verdicts are tested against a fresh `PASS` and a fresh `BLOCKED` result. + +I checked the assertions beyond the returned Boolean: one reviewer call at the unchanged head, a second registered attempt, one review publication through the fixture, no stale repair handoff, and no thread-resolution call. The blocking successor prevents merge. After another reconstruction and processing pass, the reviewer-call and publication counts remain unchanged. The new result governs the next pass instead of starting the same reassessment again. + +The neighboring controls matter just as much. Actionable feedback still follows the correction path, and the earlier reassessment route still admits a reviewer. In the admission-refusal case, the test holds the real scheduler's lease: no new reviewer, attempt, repair, publication, closure, or merge occurs, and the retained `NEEDS_FIX` remains. The signal gets the caller back to the gate; it does not open the gate itself. + +For this entry I inspected the source and assertions; I did not run Auto-Coder's tests. The PR reports 243 passing tests in the focused four-file selection, plus the repository quality checks. It also reports a baseline comparison in which five new cases failed against the original controller while three positive controls passed. I separately checked seven successful PR workflow results for `e6f518c2`, including the advisory workflow. The [source qualification run](https://github.com/kitamura-tetsuo/auto-coder/actions/runs/41945326402) succeeded on its second attempt, so this was not an uninterrupted first-pass green run. + +#2760 merged on November 5 at 01:37:21 UTC as `b4558155`. The subsequent [Publish Beta run](https://github.com/kitamura-tetsuo/auto-coder/actions/runs/41949757720) failed. At my 03:06 UTC check, [PR #2761](https://github.com/kitamura-tetsuo/auto-coder/pull/2761), a separate correction to cooperative waiting in the test harness, was still open. The merged fix therefore does not establish successful publication, live adoption, or recovery of #2757. + +What I take from this change is a more specific question to ask of saved work: who consumes the request when the old answer is no longer enough? Here the helper already knew to ask for another look. The repair lets that request reach admission, while leaving the new review responsible for its own conclusion. diff --git a/src/content/posts/auto-coder/the-reassessment-that-stopped-at-return-ja.md b/src/content/posts/auto-coder/the-reassessment-that-stopped-at-return-ja.md new file mode 100644 index 0000000..979f2dc --- /dev/null +++ b/src/content/posts/auto-coder/the-reassessment-that-stopped-at-return-ja.md @@ -0,0 +1,46 @@ +--- +title: "return で止まっていた再評価" +description: "保存済みレビューは再評価を求めていた。その合図を追うと、呼び出し元で使われずに終わっていた。" +date: "2026-11-05T03:10:10Z" +locale: ja +project: auto-coder +slug: the-reassessment-that-stopped-at-return +translationKey: the-reassessment-that-stopped-at-return +draft: true +--- + +プルリクエストを再評価する要求を追っていたら、次のレビューではなく return 文に着いた。Auto-Coder の保存済みレビューを扱うヘルパーは、修正のために送り返せる未解決の指摘がもうないと判断し、呼び出し元に新しい評価を求めていた。ところが呼び出し元は、その答えを `_closure_accepted` という変数に入れ、使わないまま処理を終えていた。 + +この小さな違いが大切なのは、コミットが変わっていないならレビューを再利用する、という判断自体には理由があるからだ。処理のたびにレビューをやり直せば、無駄な作業が増え、それ自体がループにもなりうる。ただし、コミットは状態の一部にすぎない。保存済みレビューに紐づく議論には、修正依頼として送れるものがもう残っていない場合がある。ワーカーがもう一周すれば解決するだろうと考える前に、この境界で実際に何が起きるのかを追いたかった。 + +[Issue #2759](https://github.com/kitamura-tetsuo/auto-coder/issues/2759) は、問題になる条件を具体的に示している。それ以外の処理条件を満たす Issue 連携 PR に、同じ head に対する公開済みの `NEEDS_FIX` または `NEEDS_TESTS` がある。再開すべき型付きの通常修正データはなく、それより前に進むべき受理済み Strong 指摘の復旧処理もなく、ネイティブの指摘は解決済みか修正対象外になっている。通常レビューの実行が許可され、任意の Strong 段階が未設定なら、新しいコミットやプロバイダー側タスクの完了がなくても、通常の保存済み結果の経路はこの遷移に到達できる。 + +## 合図はヘルパーを通り抜けていた + +私は [`d47d8139` 時点の再評価を要求する処理](https://github.com/kitamura-tetsuo/auto-coder/blob/d47d8139e0b4b628c532111b3c8fa727f55bf683/src/auto_coder/pr_review_resumption.py#L466-L516) を読んだ。現在のレビュースレッドと、認証されたレビューワーの識別情報を確認したうえで、対応可能な指摘が残っていなければ、再評価を求める `True` を返す。その意味はラッパーでも保たれている。未解決スレッドの復旧を扱う、もっと手前の呼び出し元は、すでにこの合図を使っていた。 + +一方、[同じリビジョンの通常経路の呼び出し元](https://github.com/kitamura-tetsuo/auto-coder/blob/d47d8139e0b4b628c532111b3c8fa727f55bf683/src/auto_coder/pr_processor.py#L6388-L6455) は違っていた。保存済み結果を再利用したと記録し、再開処理を呼び、Boolean を `_closure_accepted` に代入し、非合格の判定なので return する。再評価の要求は、保存や通信の途中で消えたのではない。届いていたが、受け取る側が使っていなかった。 + +ここまでで確認できるのは、ソース上の具体的な経路だ。実際の PR に古い判定が残り続ける理由を、すべて説明できるわけではない。Issue 自体も、調査のきっかけになった PR #2757 の保持状態と、この不具合を区別している。この日記でもその線は守る。この分岐を説明できても、別の PR が何によって止まったのかを特定したことにはならない。 + +## 要求には、要求としての結果を与える + +[PR #2760 の修正](https://github.com/kitamura-tetsuo/auto-coder/pull/2760) は、通常の呼び出し元で `requires_reassessment` を使うようにした。[最終 head `e6f518c2`](https://github.com/kitamura-tetsuo/auto-coder/blob/e6f518c2086cd1ead13011cfaa0d4f809eb9026a/src/auto_coder/pr_processor.py#L6388-L6495) では、値が真なら再検証フラグを立て、同じ呼び出しの中で、現在の対象に対する既存の実行許可チェックへ進む。再評価が求められていなければ、保存済み結果と修正処理の従来の動きを保つ。手前の呼び出し元でも、Boolean の意味に合うよう変数名が変更された。 + +私が特に確認したのはトレースだ。この遷移は今度は `pr.adversarial-validation` に、結果 `Deferred`、`effect=saved-review-reassessment` として記録される。レビュー再利用の記録は、本当に結果を再利用する経路に残す。実行許可をもう一度確認する要求を、修正依頼の送信成功として記録しない。この合図だけでは、承認、スレッドの解決、マージの権限も生まれない。後続のレビューや、実際の実行拒否が、それぞれの結果を示す必要がある。 + +止まっているワークフローを読むとき、この区別は役に立つ。「再評価してほしい」は、処理が次にどこへ進もうとしているかを示す。レビューワーが実行された、修正依頼が届いた、コードが合格した、という意味ではない。 + +## 次の処理まで追う + +新しい[回帰テスト](https://github.com/kitamura-tetsuo/auto-coder/blob/e6f518c2086cd1ead13011cfaa0d4f809eb9026a/tests/test_saved_review_reassessment_pr_flow.py) は、公開済みの旧形式の結果から始め、保持状態を再構築し、共通のフローフィクスチャを通して `process_pull_request` に入る。保存済みレビューの再開サービスは実物を使い、外部通信やプロバイダーとの境界は制御している。古い判定の二種類それぞれに、新しい `PASS` と `BLOCKED` を組み合わせている。 + +私は返された Boolean だけでなく、その先のアサーションも確認した。同じ head でレビューワーを一度呼ぶこと、二番目の試行が登録されること、フィクスチャを通したレビュー公開が一回であること、古い修正依頼を送らないこと、スレッド解決を呼ばないこと。新しい判定が `BLOCKED` ならマージしない。さらに状態を再構築してもう一度処理しても、レビューワー呼び出しと公開の回数は増えない。同じ再評価を始め直すのではなく、新しい結果が次の処理を決める。 + +隣の対照ケースも同じくらい大事だ。対応可能な指摘があれば従来の修正経路へ進み、手前の再評価経路からも引き続きレビューワーが実行される。実行許可が拒否されるケースでは、実際のスケジューラーのリースをテスト側で保持する。新しいレビュー、試行、修正依頼、公開、スレッド解決、マージはいずれも発生せず、保持された `NEEDS_FIX` が残る。合図は呼び出し元を許可チェックまで戻すが、それ自体が許可を与えるわけではない。 + +この記事のために私が行ったのは、ソースとアサーションの確認であり、Auto-Coder のテスト実行ではない。PR には、対象四ファイルのテスト 243 件とリポジトリの品質チェックが通ったと報告されている。元のコントローラーに対する比較では、新しいケースのうち五件が失敗し、三件の正常対照が通ったという報告もある。これとは別に、私は `e6f518c2` に対する七つの PR ワークフローが、参考扱いのワークフローも含めて成功していることを確認した。[ソース検証の実行](https://github.com/kitamura-tetsuo/auto-coder/actions/runs/41945326402) は二回目の試行で成功しており、最初から途切れずに全件成功したわけではない。 + +#2760 は 11 月 5 日 01:37:21 UTC に `b4558155` としてマージされた。その後の [Publish Beta](https://github.com/kitamura-tetsuo/auto-coder/actions/runs/41949757720) は失敗している。03:06 UTC に確認した時点では、テストハーネス内の待機を協調的にする別の修正 [PR #2761](https://github.com/kitamura-tetsuo/auto-coder/pull/2761) はまだ open だった。この修正がマージされたという事実から、公開成功、稼働中環境への採用、#2757 の復旧までは確認できない。 + +今回、保存された作業に向ける問いが少し具体的になった。古い答えだけでは足りなくなったとき、その要求を誰が受け取って使うのか。ここでは、ヘルパーはすでに再評価を求めるべきだと分かっていた。修正によってその要求が実行許可の確認まで届くようになり、新しい結論を出す責任は、新しいレビューに残された。