Harden markdown response parsing against formatting drift

src/coding_review_agent_loop/orchestrator.py

@@ -111,6 +111,11 @@
     re.I,
 )
 ROUND_RESUME_MARKER_RE = re.compile(r"<!--\s*AGENT_LOOP_META:\s*(?P<payload>[A-Za-z0-9+/=_-]+)\s*-->", re.I)
+ALL_RESOLVED_PROSE_RE = re.compile(
+    r"^all (?:prior items|listed items|carried-forward items) are resolved\.?$"
+    r"|^all prior unresolved items have been resolved\.?$",
+    re.I,
+)
 
 
 @dataclass(frozen=True)
@@ -121,6 +126,31 @@ class ValidatedAgentResponse:
     usage: UsageMetadata | None = None
 
 
+def _normalize_disposition_section_prose(text: str) -> str:
+    return " ".join(text.strip().split())
+
+
+def _maybe_fill_resolved_dispositions_from_prose(
+    parsed: ParsedReview | ParsedPlanReview,
+    *,
+    reviewer: str,
+    unresolved_items: Sequence[UnresolvedReviewItem],
+) -> tuple[ReviewItemDisposition, ...]:
+    if not unresolved_items or parsed.dispositions:
+        return parsed.dispositions
+    normalized = _normalize_disposition_section_prose(parsed.raw_dispositions_text)
+    if not normalized or not ALL_RESOLVED_PROSE_RE.fullmatch(normalized):
+        return parsed.dispositions
+    return tuple(
+        ReviewItemDisposition(
+            item_id=item.item_id,
+            reviewer=reviewer,
+            disposition="resolved",
+        )
+        for item in unresolved_items
+    )
+
+
 @dataclass(frozen=True)
 class PostedRoundMetadata:
     flow: str
@@ -383,7 +413,12 @@ def _validate_review_response(
         return parsed
 
     unresolved_by_id = {item.item_id: item for item in unresolved_items}
-    disposition_ids = [item.item_id for item in parsed.dispositions]
+    dispositions = _maybe_fill_resolved_dispositions_from_prose(
+        parsed,
+        reviewer=reviewer,
+        unresolved_items=unresolved_items,
+    )
+    disposition_ids = [item.item_id for item in dispositions]
     duplicates = sorted({item_id for item_id in disposition_ids if disposition_ids.count(item_id) > 1})
     if duplicates:
         raise AgentLoopError(
@@ -399,7 +434,14 @@ def _validate_review_response(
         raise AgentLoopError(
             "Review did not evaluate all prior unresolved items: " + ", ".join(missing)
         )
-    return parsed
+    return ParsedReview(
+        state=parsed.state,
+        summary=parsed.summary,
+        blocking_items=parsed.blocking_items,
+        followups=parsed.followups,
+        dispositions=dispositions,
+        raw_dispositions_text=parsed.raw_dispositions_text,
+    )
 
 
 def _upsert_human_requirements_ack_item(
@@ -579,7 +621,12 @@ def _validate_plan_review_response(
         return parsed
 
     unresolved_by_id = {item.item_id: item for item in unresolved_items}
-    disposition_ids = [item.item_id for item in parsed.dispositions]
+    dispositions = _maybe_fill_resolved_dispositions_from_prose(
+        parsed,
+        reviewer=reviewer,
+        unresolved_items=unresolved_items,
+    )
+    disposition_ids = [item.item_id for item in dispositions]
     duplicates = sorted({item_id for item_id in disposition_ids if disposition_ids.count(item_id) > 1})
     if duplicates:
         raise AgentLoopError(
@@ -598,7 +645,13 @@ def _validate_plan_review_response(
             "Plan review did not evaluate all prior unresolved plan items: "
             + ", ".join(missing)
         )
-    return parsed
+    return ParsedPlanReview(
+        state=parsed.state,
+        summary=parsed.summary,
+        items=parsed.items,
+        dispositions=dispositions,
+        raw_dispositions_text=parsed.raw_dispositions_text,
+    )
 
 
 def _review_freeform_summary_text(text: str) -> str:

src/coding_review_agent_loop/protocol.py

@@ -68,7 +68,7 @@ def _followup_heading_re(title: str) -> re.Pattern[str]:
 
 def _empty_placeholder_re(*phrases: str) -> re.Pattern[str]:
     joined = "|".join(phrases)
-    return re.compile(rf"^(?:none|n/a|{joined})\.?$", re.I)
+    return re.compile(rf"^\(?\s*(?:none|n/a|{joined})\s*\)?\.?$", re.I)
 
 
 EMPTY_FOLLOWUP_RE = _empty_placeholder_re(
@@ -121,6 +121,7 @@ class ParsedReview:
     blocking_items: tuple[ApprovedFollowup, ...]
     followups: ApprovedFollowups
     dispositions: tuple[ReviewItemDisposition, ...]
+    raw_dispositions_text: str = ""
 
 
 @dataclass(frozen=True)
@@ -136,6 +137,7 @@ class ParsedPlanReview:
     summary: str
     items: PlanReviewItems
     dispositions: tuple[ReviewItemDisposition, ...]
+    raw_dispositions_text: str = ""
 
 
 @dataclass(frozen=True)
@@ -596,6 +598,7 @@ def _finalize_parsed_review(
     blocking_items: tuple[ApprovedFollowup, ...],
     followups: ApprovedFollowups,
     dispositions: tuple[ReviewItemDisposition, ...],
+    raw_dispositions_text: str = "",
 ) -> ParsedReview:
     if state == "blocking" and followups.future:
         followups = ApprovedFollowups(same_pr=followups.same_pr, future=())
@@ -619,6 +622,7 @@ def _finalize_parsed_review(
         blocking_items=blocking_items,
         followups=followups,
         dispositions=dispositions,
+        raw_dispositions_text=raw_dispositions_text,
     )
 
 
@@ -628,6 +632,7 @@ def _finalize_parsed_plan_review(
     summary: str,
     items: PlanReviewItems,
     dispositions: tuple[ReviewItemDisposition, ...],
+    raw_dispositions_text: str = "",
 ) -> ParsedPlanReview:
     if state == "blocking" and items.future:
         items = PlanReviewItems(blocking=items.blocking, same_plan=items.same_plan, future=())
@@ -650,6 +655,7 @@ def _finalize_parsed_plan_review(
         summary=summary,
         items=items,
         dispositions=dispositions,
+        raw_dispositions_text=raw_dispositions_text,
     )
 
 
@@ -1019,6 +1025,21 @@ def flush_current() -> None:
     flush_current()
 
 
+def _extract_section_text(text: str, *, heading_re: re.Pattern[str]) -> str:
+    active = False
+    lines: list[str] = []
+    for line in text.splitlines():
+        if heading_re.match(line):
+            active = True
+            continue
+        if not active:
+            continue
+        if ANY_HEADING_RE.match(line) or HTML_COMMENT_RE.match(line) or SIGNATURE_RE.match(line):
+            break
+        lines.append(line)
+    return "\n".join(lines).strip()
+
+
 def parse_approved_followups(text: str, *, reviewer: str) -> ApprovedFollowups:
     """Extract same-PR and future follow-ups from an approved review."""
     same_pr: list[ApprovedFollowup] = []
@@ -1141,10 +1162,12 @@ def _parse_unresolved_item_dispositions(
 ) -> tuple[ReviewItemDisposition, ...]:
     dispositions: list[ReviewItemDisposition] = []
     active = False
+    section_heading: str | None = None
 
-    for line in text.splitlines():
+    for line_number, line in enumerate(text.splitlines(), start=1):
         if heading_re.match(line):
             active = True
+            section_heading = line.strip()
             continue
         if not active:
             continue
@@ -1161,7 +1184,10 @@ def _parse_unresolved_item_dispositions(
             continue
         match = disposition_re.match(entry)
         if not match:
-            raise AgentLoopError(error_message)
+            raise AgentLoopError(
+                f"{error_message} In section `{section_heading or 'unknown section'}`, "
+                f"line {line_number}: `{entry}`."
+            )
         note = match.group("note")
         normalized_note = note.strip() if note else None
         disposition = _normalize_disposition(match.group("status"), same_status=same_status)
@@ -1171,7 +1197,10 @@ def _parse_unresolved_item_dispositions(
             same_status=same_status,
             is_plan_review=is_plan_review,
         ):
-            raise AgentLoopError(error_message)
+            raise AgentLoopError(
+                f"{error_message} In section `{section_heading or 'unknown section'}`, "
+                f"line {line_number}: `{entry}`."
+            )
         dispositions.append(
             ReviewItemDisposition(
                 item_id=match.group("item_id"),
@@ -1237,6 +1266,9 @@ def parse_review(text: str, *, reviewer: str) -> ParsedReview:
         blocking_items=blocking_items,
         followups=followups,
         dispositions=dispositions,
+        raw_dispositions_text=_extract_section_text(
+            text, heading_re=PRIOR_UNRESOLVED_ITEM_DISPOSITIONS_HEADING_RE
+        ),
     )
 
 
@@ -1258,6 +1290,9 @@ def parse_plan_review(text: str, *, reviewer: str) -> ParsedPlanReview:
         summary=summary,
         items=items,
         dispositions=dispositions,
+        raw_dispositions_text=_extract_section_text(
+            text, heading_re=PRIOR_UNRESOLVED_PLAN_ITEM_DISPOSITIONS_HEADING_RE
+        ),
     )