[PR] feat(code-review): hand addressed outdated threads to the maintainer to resolve [magpie]

["shahar1 (via GitHub)" <gi...@apache.org>]

shahar1 opened a new pull request, #1758:
URL: https://github.com/apache/magpie/pull/1758

   ## Summary
   
   - A review thread GitHub marks **outdated** blocked `APPROVE` until someone resolved it, even when a later push had already fixed what it asked for. On a base branch with `required_conversation_resolution`, the PR then sat blocked and the skill never said why. This came up for real on apache/airflow#72542: an outdated `kubernetes_conn_id` thread, fixed and answered "Done", was one of the two threads that held the merge.
   - `code-review context` now lists unresolved outdated threads (`outdated_threads`). The agent checks each one against the current diff (new `classifications/outdated-thread.md`). For each thread the code addresses, `disposition --addressed-thread <id>` removes it from the thread gate and returns it in `resolve_first`, and the skill tells the maintainer: *"resolve it to unblock the merge"*.
   - Guard rails: only threads that are both unresolved and outdated can be passed; any other id exits with an error. The author saying "Done" does not count; the current diff has to show the fix. The skill never resolves a thread itself, since that stays the maintainer's sign-off. Addressed threads never appear in the review body. Threads that are not outdated still gate `APPROVE` as before.
   
   ## Type of change
   
   - [x] Skill change (`.claude/skills/<name>/`) — eval fixtures updated below
   - [x] Python package (`tools/*/` with `pyproject.toml`)
   - [ ] Tool / bridge contract (`tools/<system>/*.md`)
   - [ ] Groovy reference impl
   - [ ] Cross-cutting (RFC, AGENTS.md, sandbox, privacy-LLM)
   - [ ] Documentation (`docs/`, `README.md`, `CONTRIBUTING.md`)
   - [ ] Project template (`projects/_template/`)
   - [ ] CI / dev loop (`prek`, workflows, validators)
   - [x] Other: `cr-pr.graphql` reads thread `id` / `isOutdated` and comment `url` (vetted-ops query, plugin copy synced)
   
   ## Test plan
   
   - [x] `prek run --all-files` passes
   - [x] For Python packages touched: `uv run pytest` / `ruff check` / `mypy` passes — `tools/pr-management/tests/code_review`: 141 passed, including 3 new tests: `test_step6_addressed_outdated_threads_go_to_the_maintainer`, `test_outdated_thread_the_code_addresses_unblocks_approve`, and `test_addressed_thread_must_be_outdated_and_unresolved`
   - [ ] For Groovy bridges touched: command-line invocation tested end-to-end
   - [x] For skill changes: eval suite passes for the affected skill — new `step-6-outdated-thread` suite, 3/3 with `claude -p`
   - [x] For skill *behaviour* changes: a new or updated eval fixture is included in this PR (`step-6-outdated-thread`: fixed as asked / code moved but the concern remains / author says "Done" but the diff disagrees)
   - [x] Other: ran the edited `cr-pr.graphql` live against apache/airflow#72542. With the thread state from before it was resolved, `context` listed the outdated `kubernetes_conn_id` thread, and `--addressed-thread` dropped the thread gate from 2 to 1 and returned it in `resolve_first`. The remaining thread is not outdated, so it still blocked `APPROVE`.
   
   ## RFC-AI-0004 compliance
   
   - [x] **HITL** — no new mutation; resolving a thread is left to the maintainer, the skill only tells them
   - [x] **Vendor neutrality** — placeholders kept; eval fixtures are synthetic (`acme`)
   - [x] **Write-access discipline** — no autonomous outbound messages; `resolve_first` is maintainer-facing only, never in the review body
   
   ## Linked issues
   
   Refs #1585 (the single `APPROVE` definition this narrows).
   
   ## Notes for reviewers (optional)
   
   - Scope is outdated threads only. A thread that is not outdated but already answered (for example, a reviewer withdrew their concern in a reply) still gates `APPROVE`. Extending the same flow to those is a separate decision, because nothing in the diff can show it is addressed.
   - `resolve_first` is returned even when another gate keeps the result at `COMMENT`, so the maintainer still learns which threads they can resolve.
   - `SKILL.md` Golden rule 7 gained one sentence; `measured_tokens` was restamped (3770 → 3826); `surface_hash` is unchanged.
   
   ---
   
   Generative AI: drafted with Claude Code (Claude Opus 5.5), reviewed by the author.
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@magpie.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat(code-review): hand addressed outdated threads to the maintainer to resolve [magpie]

["potiuk (via GitHub)" <gi...@apache.org>]

potiuk merged PR #1758:
URL: https://github.com/apache/magpie/pull/1758


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@magpie.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org