fix(pr-management): verify dependency findings

[AlejandroMorgante]

Summary

• Require dependency-version findings to prove a supported failing resolution after intersecting every mandatory direct and transitive constraint.
• Require reviewers to read the applicable per-area AGENTS.md and repository dependency/release policy before prescribing a remediation.
• Add five regression evals: a transitively compatible graph, a genuinely installable incompatibility whose remediation follows repository policy, a repository release marker that already satisfies local policy, partial metadata that leaves runtime compatibility unknown while preserving an independently established policy finding, and an environment marker that changes the effective constraint intersection.

A real review on apache/airflow#70160 showed the impact. The finding treated common-compat 1.8–1.11 as installable even though the mandatory common-sql>=1.32.0 path requires common-compat>=1.12.0, then recommended replacing Airflow's # use next version release marker with a direct bound change. The result was published as blocking after human confirmation, so dependency findings need an explicit evidence gate before they can block contributor work.

Type of change

• Skill change (.claude/skills/<name>/) — eval fixtures updated below
• Tool / bridge contract (tools/<system>/*.md)
• Python package (tools/*/ with pyproject.toml)
• 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)
• Other:

Test plan

• prek run --all-files passes
• For Python packages touched: uv run pytest / ruff check / mypy passes
• For Groovy bridges touched: command-line invocation tested end-to-end
• For skill changes: eval suite passes for the affected skill
  (uv run --project tools/skill-evals skill-eval tools/skill-evals/evals/pr-management-code-review/ rendered all 115 cases successfully)
• For skill behaviour changes: a new or updated eval fixture is included in this PR
  (the dependency-compatibility suite passes 5/5 against claude -p)
• Other:

RFC-AI-0004 compliance

• HITL — any new mutation is gated on explicit user confirmation
• Sandbox — no new unrestricted host access; network reach declared in the adapter
• Vendor neutrality — the skill reads each adopter's policy and the regression fixtures use generic package names
• Conversational + correctable — agentic-override path documented if behaviour is adopter-tunable
• Write-access discipline — no autonomous outbound messages; drafts only, sent on confirmation
• Privacy LLM — private content does not reach a non-approved LLM; redactor invoked where needed

Linked issues

None. A deduplication search found no existing Magpie issue or PR for this defect.

Notes for reviewers

The Airflow link is evidence for the generic failure mode, not a project-specific rule in the skill. The production wording requires a complete constraint ledger and delegates remediation to the adopter's own AGENTS.md and dependency/release documentation.

[potiuk]

LGTM — this fixes a real class of false positive, and the reasoning behind it is more careful than the diff size suggests.

The rule is right

The core insight is the one that matters: "A direct lower bound by itself is not a failing resolution when another mandatory path narrows the range." Flagging foo>=1.0 as incompatible with an API added in foo 2.0 is wrong if some other mandatory dependency already forces foo>=2.1 — the failing version is not resolvable, so the runtime failure being claimed cannot occur. Requiring a constraint ledger and one concrete supported-but-failing version turns an inference into a demonstration.

Three things I would call out as better than the minimum:

• The three-state classification. Adding unknown alongside broken/compatible, and stating that partial metadata coverage yields unknown rather than compatible, is the honest modelling. The tempting shortcut is "no failing resolution found, therefore compatible", and the text explicitly rejects it: absence proves compatibility only when coverage is exhaustive, and the reviewer has to record what makes it exhaustive.
• Separating runtime findings from policy findings. unknown cannot support a runtime-incompatibility claim but does not suppress a policy finding backed by the project's own rules. Those are genuinely different claims with different evidence bars, and conflating them is how a reviewer ends up asserting a crash that never happens in order to justify a bound they think should change anyway.
• "Do not prescribe a direct version bump when the project's release process says contributors must not make one." Requiring the per-area AGENTS.md be read before prescribing remediation, rather than importing a convention from another repository, is the part most likely to save a contributor from being told to do something their project forbids.

The evals match the rule

Five cases spanning the interesting axes — transitive lower bound, an installable incompatibility, a release marker that already satisfies the constraint, partial metadata producing a policy finding, and an environment marker.

The assertions are the part I checked hardest, because assertions on a findings list are easy to get subtly wrong:

• The regex predicates match literal JSON double quotes ("dependency_evidence"\s*:\s*"[^"]+") against a list-valued field. That works because the runner JSON-serialises non-string values before text predicates run (text = value if isinstance(value, str) else json.dumps(...)), so the double quotes are really there. Worth stating explicitly since a str()-based implementation would have produced single quotes and the pattern would silently never match.
• has_empty_findings uses max_length: 0, which takes len() of the list rather than a string — the correct predicate for "no findings at all".
• has_complete_constraint_ledger is a judge predicate with a rubric spelling out all three admissible conclusions. Mixing deterministic predicates for the mechanical parts with a judge for the semantic one is exactly the right split: the regexes prove the field exists and is shaped correctly, the judge decides whether the ledger actually reasons.

Shipping assertions.json and grading-schema.json alongside the fixtures also means these cases grade automatically rather than falling back to MANUAL — the trap documented in #1043 and hit again in #982 this week. Good to see a new suite land already correct.

step-config.json extracts ## Step 4 — Examine the diff live from review-flow.md, so the eval exercises the shipped text rather than a copy that can drift.

No findings.

---

This review was drafted by an AI-assisted tool and
confirmed by an Apache Magpie maintainer. The maintainer
approving this PR has read the findings and signed off. If
something feels off, please reply on the PR and a maintainer
will follow up.

More on how Apache Magpie handles maintainer review:
CONTRIBUTING.md § Opening a pull request.