Commit 2b8c2747b4399933577a56e68e9706646773e0a5
1 parent
2717f8c8
fix: record the final attempt's own rejection feedback when a loop_back exhausts its retry budget
Real incident (2026-08-13, task b243dfe8), diagnosed live with the user after they asked "what's the actual bug behind this block": the task's benchmark regression-check node kept looping back to itself, then hit "exceeded max_attempts=3" -- but the DB had no record of WHY the final (3rd) attempt was rejected, only attempts 1 and 2's feedback. This made the block impossible to properly diagnose: reading attempt 3's actual content (now possible thanks to the artifact-promotion fix) showed it had correctly grounded itself in the task's own approved SDD, unlike attempts 1/2 -- but whether the judge agreed was unknowable from the DB alone. Root cause: _run_layers()'s exceeded-limit branch raised ConsistencyCheckFailedError immediately, skipping the _record_loop_back_event() call a few lines below entirely -- sig.feedback (the judge's real, specific reason for rejecting THIS FINAL attempt, the one that actually exhausted the budget) was silently discarded. Every other rejected attempt's feedback gets persisted via that same call; only the budget-exhausting one didn't. Fixed by recording it before raising, same as every other rejection. New test drives a real 3-attempt exhaustion (max_attempts=2 -> attempts 1, 2, 3) and asserts all three loop_back_triggered events exist with attempt 3's own distinct feedback text -- confirmed it reproduces the exact real gap (2 events, not 3) on the pre-fix code. 823/823 tests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Showing
2 changed files
with
49 additions
and
0 deletions
src/test_work_graph_orchestrate.py
| ... | ... | @@ -1151,6 +1151,41 @@ class RunGraphLoopBackTest(RunGraphTestBase): |
| 1151 | 1151 | status = self.conn.execute("SELECT status FROM work_graph_run WHERE run_id = 'run-1'").fetchone()[0] |
| 1152 | 1152 | self.assertEqual(status, "blocked") |
| 1153 | 1153 | |
| 1154 | + def test_exceeding_limit_still_records_the_final_rejections_own_feedback(self): | |
| 1155 | + # Real incident (2026-08-13, task b243dfe8, diagnosed live with the | |
| 1156 | + # user): the exceeded-limit branch used to raise | |
| 1157 | + # ConsistencyCheckFailedError WITHOUT ever calling | |
| 1158 | + # _record_loop_back_event() -- the judge's real reason for | |
| 1159 | + # rejecting the FINAL attempt (the one that actually exhausted the | |
| 1160 | + # budget) was silently discarded. Only a human/arbiter reading | |
| 1161 | + # loop_back_triggered execution_events, expecting one per real | |
| 1162 | + # rejection, would notice the last one is simply missing. | |
| 1163 | + manifest = { | |
| 1164 | + "nodes": [{ | |
| 1165 | + "node_id": "benchmark", | |
| 1166 | + "judgment": {"kind": "mechanical", "check_ref": "compiles"}, | |
| 1167 | + "on_fail": {"action": "retry_self", "max_attempts": 2}, | |
| 1168 | + }], | |
| 1169 | + "edges": [], | |
| 1170 | + } | |
| 1171 | + registry = {"compiles": lambda content: (False, "specific reason attempt 3 was rejected")} | |
| 1172 | + with patch.object(org_node_trigger, "trigger_org_node_structured", return_value={"content": "always bad"}): | |
| 1173 | + with self.assertRaises(wgo.ConsistencyCheckFailedError): | |
| 1174 | + wgo.run_graph( | |
| 1175 | + self.conn, "run-1", "planner", "planner-doc-writing", "/tmp/proj-1", manifest, | |
| 1176 | + lambda node, outputs: "p", _IN_SCHEMA, _OUT_SCHEMA, mechanical_check_registry=registry, | |
| 1177 | + ) | |
| 1178 | + events = self.conn.execute( | |
| 1179 | + "SELECT payload FROM execution_event WHERE run_id = 'run-1' AND event_type = 'loop_back_triggered' " | |
| 1180 | + "ORDER BY created_at" | |
| 1181 | + ).fetchall() | |
| 1182 | + # on_fail.max_attempts=2 -> attempts 1 and 2 loop back normally, | |
| 1183 | + # attempt 3 exhausts the budget -- all THREE must be recorded, not | |
| 1184 | + # just the first two. | |
| 1185 | + self.assertEqual(len(events), 3) | |
| 1186 | + last_feedback = json.loads(events[-1][0])["feedback"] | |
| 1187 | + self.assertIn("specific reason attempt 3 was rejected", str(last_feedback)) | |
| 1188 | + | |
| 1154 | 1189 | def test_hard_fail_raises_immediately_with_no_retry(self): |
| 1155 | 1190 | manifest = { |
| 1156 | 1191 | "nodes": [{ | ... | ... |
src/work_graph_orchestrate.py
| ... | ... | @@ -1164,6 +1164,20 @@ def _run_layers( |
| 1164 | 1164 | total_attempts = _loop_attempt_count_from_events_total(conn, run_id, sig.source_node_id) + 1 |
| 1165 | 1165 | total_limit = _resolve_total_loop_limit(conn, org_node, sig.source_node_id) |
| 1166 | 1166 | if attempt_number > limit or total_attempts > total_limit: |
| 1167 | + # 2026-08-13 (real incident, task b243dfe8, diagnosed live | |
| 1168 | + # with the user): this branch used to jump straight to | |
| 1169 | + # raise, skipping _record_loop_back_event() below entirely | |
| 1170 | + # -- sig.feedback (the judge's REAL reason THIS FINAL | |
| 1171 | + # attempt was rejected, the one that actually exhausted the | |
| 1172 | + # budget) was silently discarded. Nothing in the DB could | |
| 1173 | + # answer "what was specifically wrong with the last try", | |
| 1174 | + # only "it happened N times" (this exception's own summary | |
| 1175 | + # message). The attempt that exhausted the budget is | |
| 1176 | + # recorded the same way every other rejected attempt already | |
| 1177 | + # is, before giving up -- so a human (or a future arbiter) | |
| 1178 | + # diagnosing the block can see the real judgment, not just a | |
| 1179 | + # count. | |
| 1180 | + _record_loop_back_event(conn, run_id, sig, attempt_number) | |
| 1167 | 1181 | conn.execute("UPDATE work_graph_run SET status = 'blocked' WHERE run_id = ?", (run_id,)) |
| 1168 | 1182 | conn.commit() |
| 1169 | 1183 | raise ConsistencyCheckFailedError( | ... | ... |