feat(merge-sweep): add failure history tracking and Track A-resolve [OMN-7573]

[jonahgabriel]

Summary

• Add FailureCategory enum with 11 standardized failure category strings
• Add FailureHistoryEntry and FailureHistorySummary dataclasses for cross-run tracking
• Add Track A-resolve for MERGEABLE+BLOCKED+GREEN PRs (unresolved review threads)
• Fix is_merge_ready() to exclude BLOCKED PRs so they fall through to A-resolve
• Add escalation logic: CHRONIC (≥5 consecutive failures) and RECIDIVIST (≥3 polishes still failing) PRs skip polish
• Bump contract version to 1.1.0
• Add 6 new tests covering failure history, escalation, and Track A-resolve

Test plan

• All 14 unit tests pass locally
• CI passes

Summary by CodeRabbit

• New Features
  • Added a new thread-resolution queue track for managing PRs requiring resolved review threads.
  • Introduced cross-run failure history tracking with escalation thresholds (STUCK, CHRONIC, RECIDIVIST).
  • Enhanced PR failure categorization to identify root causes (branch staleness, unresolved threads, conflicts, CI failures).
  • Implemented escalation-based skip logic to prevent repeated polish cycles on chronic failures.

[linear]

OMN-7573

[coderabbitai]

📝 Walkthrough

Walkthrough

The changes introduce cross-run failure tracking and a new PR classification track (A-resolve) for thread resolution. The contract version incremented to 1.1.0, failure history models were added, and MergeSweepRequest/Result extended to support failure categorization and escalation logic based on historical failure patterns.

Changes

Cohort / File(s)	Summary
Contract Definition src/omnimarket/nodes/node_merge_sweep/contract.yaml	Version bump from 1.0.0 to 1.1.0 for both contract_version and node_version. Description expanded to include thread-resolution queue (Track A-resolve) and failure tracking with escalation thresholds (STUCK, CHRONIC, RECIDIVIST).
Handler Implementation src/omnimarket/nodes/node_merge_sweep/handlers/handler_merge_sweep.py	Added new A_RESOLVE track for PRs blocked but mergeable with passing checks. Introduced failure history models (FailureCategory enum, FailureHistoryEntry, FailureHistorySummary). Extended MergeSweepRequest with failure_history dict and run_id. Extended MergeSweepResult with failure_history_summary and track_a_resolve property. Implemented _needs_thread_resolution(), updated _classify_pr() to track per-PR failure categories, added _should_skip_polish() escalation logic, and _compute_failure_summary() for cross-run history aggregation.
Test Coverage tests/test_golden_chain_merge_sweep.py	Expanded golden-chain assertions for PR failure-category attribution across Track A and B outcomes. Added new TestFailureHistory suite validating failure-history input, escalation logic (chronic/recidivist/stuck patterns triggering skip decisions), and failure_history_summary computation. Verified behavior with omitted/empty failure history.

Sequence Diagram

sequenceDiagram
    participant Req as MergeSweepRequest
    participant Handler as Handler
    participant Classify as Classifier
    participant History as FailureHistory
    participant Result as MergeSweepResult

    Req->>Handler: failure_history, run_id, PRs
    Handler->>Classify: _classify_pr() with history context
    Classify->>History: check failure_categories
    History-->>Classify: STUCK, CHRONIC, RECIDIVIST status
    Classify-->>Handler: (track, reason, failure_categories)
    
    alt Track A-resolve
        Handler->>Handler: _needs_thread_resolution()
        Handler->>Handler: mark A_RESOLVE track
    else Track B_POLISH
        Handler->>Handler: _should_skip_polish() via history
        Handler->>Handler: escalate to SKIP if threshold met
    else Track A
        Handler->>Handler: mergeable, proceed
    end
    
    Handler->>Handler: _compute_failure_summary()
    Handler->>Result: failure_history_summary
    Result-->>Req: MergeSweepResult

Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 munching carrot thoughtfully

A-resolve hops into sight,
With failure trails tracked through the night,
Threads untangled, stuck patterns named,
My sweep grows wise, cross-run tamed! 🧵✨

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/omnimarket/nodes/node_merge_sweep/handlers/handler_merge_sweep.py (1)

84-92: cleaned_merged is never populated.

FailureHistorySummary exposes this field, but _compute_failure_summary() has no current-run input and always returns the default 0. Either derive it from classified + history, or leave it out of the 1.1.0 response until it is actually wired up.

Also applies to: 307-329

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/omnimarket/nodes/node_merge_sweep/handlers/handler_merge_sweep.py` around
lines 84 - 92, FailureHistorySummary exposes cleaned_merged but it is never
populated; update _compute_failure_summary to compute cleaned_merged (or remove
it) — locate the FailureHistorySummary dataclass and the
_compute_failure_summary function and either: 1) derive cleaned_merged from the
current-run classified results combined with history (e.g., count items in
classified where label/state indicates a cleaned+merged outcome and/or where
history shows previously cleaned then merged), or 2) remove cleaned_merged from
FailureHistorySummary and any response construction until you wire it up; ensure
updates reference the FailureHistorySummary class and the
_compute_failure_summary method and adjust any callers that expect
cleaned_merged accordingly.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/omnimarket/nodes/node_merge_sweep/handlers/handler_merge_sweep.py`:
- Around line 219-240: The current ordering calls self._needs_thread_resolution
before self._needs_polish so PRs with review_decision == "CHANGES_REQUESTED" are
routed to PRTrack.A_RESOLVE instead of PRTrack.B_POLISH; fix by either moving
the polish block (the if self._needs_polish(pr, require_approval): ... that
returns PRTrack.B_POLISH) above the thread-resolution block (the if
self._needs_thread_resolution(pr): ... that returns PRTrack.A_RESOLVE) or adjust
self._needs_thread_resolution to explicitly ignore PRs where pr.review_decision
== "CHANGES_REQUESTED" so that CHANGES_REQUESTED PRs are evaluated by
self._needs_polish and routed to PRTrack.B_POLISH.

---

Nitpick comments:
In `@src/omnimarket/nodes/node_merge_sweep/handlers/handler_merge_sweep.py`:
- Around line 84-92: FailureHistorySummary exposes cleaned_merged but it is
never populated; update _compute_failure_summary to compute cleaned_merged (or
remove it) — locate the FailureHistorySummary dataclass and the
_compute_failure_summary function and either: 1) derive cleaned_merged from the
current-run classified results combined with history (e.g., count items in
classified where label/state indicates a cleaned+merged outcome and/or where
history shows previously cleaned then merged), or 2) remove cleaned_merged from
FailureHistorySummary and any response construction until you wire it up; ensure
updates reference the FailureHistorySummary class and the
_compute_failure_summary method and adjust any callers that expect
cleaned_merged accordingly.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Lite

Run ID: 3d0d63a3-9ce5-48a9-87d4-04528eea5851

📥 Commits

Reviewing files that changed from the base of the PR and between c74a41d and 5d82d90.

📒 Files selected for processing (3)

• src/omnimarket/nodes/node_merge_sweep/contract.yaml
• src/omnimarket/nodes/node_merge_sweep/handlers/handler_merge_sweep.py
• tests/test_golden_chain_merge_sweep.py