chore: update review skill details

.agents/skills/maintainer-review/SKILL.md

@@ -35,6 +35,7 @@ Lead with the current review state. Use `Preliminary assessment` while decision-
 - For a PR, inspect the current remote base and head, full patch, commit history when relevant, tests, linked issue, and review discussion. Do not substitute the current local checkout for the remote change under review.
 - State the claim in one falsifiable sentence. Distinguish the reported symptom from the reporter's proposed cause or fix.
 - Identify the released behavior boundary when compatibility or regression claims matter.
+- When a proposed change removes, reorders, or reinterprets an established observable or an explicit existing test expectation, inspect the introducing commit, blame, and original tests before any positive assessment. Intentional released coverage is compatibility-risk evidence even when it is not by itself a permanent public contract.
 - Verify whether linked evidence matches the PR's exact runtime variant, provider or tool type, triggering condition, and user outcome. A generic issue title, conceptual similarity, or wording such as `Related to` does not transfer evidence of need to an adjacent extension. If the reported scenario has already been fixed, treat additional variants as new needs requiring their own evidence.
 
 Respect repository instructions for remote access and mutation. A review does not authorize comments, labels, branch changes, pushes, or other remote writes.
@@ -43,7 +44,7 @@ Respect repository instructions for remote access and mutation. A review does no
 
 Complete this pass before deeply evaluating a proposed implementation and before any positive issue or PR assessment.
 
-First assign one `Need evidence` status:
+First assign one `Need status`:
 
 - **Demonstrated**: The exact scope has a concrete supported scenario, a real-path reproduction, a released compatibility requirement, repeated demand, or a broad invariant with a meaningful consequence.
 - **Plausible but unproven**: The path can exist, but realistic provider behavior, user reach, frequency, consequence, or demand is not established.
@@ -52,6 +53,15 @@ First assign one `Need evidence` status:
 
 Only `Demonstrated` need may receive `Merge-worthy as-is` or `Merge-worthy after focused changes`. For `Plausible but unproven`, prefer `Needs evidence` or `Not worth completing`; for `Already covered` or `Unsupported`, prefer closure or the relevant simpler alternative.
 
+Keep four decisions separate and record them before comparing implementations:
+
+1. **Observation validity**: whether the reported output, state, or code-path difference is real.
+2. **Downstream consequence**: what concrete user, operational, compatibility, or durable-state result changes because of it.
+3. **Need status**: one of the four evidence classifications above.
+4. **Issue action**: prioritize, accept, narrow, request evidence, or close.
+
+A real observation can still have no demonstrated need and a `Close` action. Do not describe an issue as simply "valid" when only the observation is confirmed. If the downstream consequence is missing, do not choose among proposed semantic contracts, select a competing PR, or draft implementation changes yet.
+
 Before assigning `Demonstrated`, require one of these evidence paths:
 
 1. **Observed impact**: A supported scenario, real-path reproduction, or credible user report shows a meaningful user-visible, operational, compatibility, or durable-state consequence.
@@ -59,6 +69,10 @@ Before assigning `Demonstrated`, require one of these evidence paths:
 
 For both paths, trace `realistic trigger -> supported execution path -> observable or durable effect`. A local intermediate inconsistency, constructible branch, redundant operation, defensive improvement, or theoretically cleaner invariant is not a demonstrated need without a meaningful downstream effect. A small diff, technically correct patch, or inexpensive test does not lower this threshold. Material preventive outcomes include security or privacy exposure, credential leakage, persistent data or state corruption, duplicate external side effects, unrecoverable compatibility breaks, deadlock or indefinite hangs, and realistically repeatable resource exhaustion.
 
+For a representation-only change, identify one concrete consumer computation, decision, or persisted interpretation that differs before and after the patch, then establish why the current result is wrong. If the patch only changes list shape, placeholder presence, metadata, ordering, or terminology without recovering information or changing a meaningful consumer outcome, the need is not `Demonstrated`.
+
+An ambiguous contract is not itself evidence that the contract should change. When multiple current shapes are released or intentionally test-covered, prefer no code change until a demonstrated outcome justifies selecting a different semantic contract. Do not choose one shape only because it is more symmetric or easier to explain.
+
 When a report establishes only a harmless or speculative logic-level improvement, prefer `Not worth completing` or `Close` rather than requesting implementation refinements. Use `Needs evidence` only when a specific missing reproduction or consequence trace could realistically change the practical-impact decision.
 
 1. Restate the desired user outcome without naming the requested API, class, file, option, or implementation. Separate the actual constraint from the reporter's preferred mechanism.
@@ -168,7 +182,7 @@ Do not over-investigate. Stop when additional evidence is unlikely to change val
 
 Use `references/evaluation-framework.md` to assess claim validity, realistic reach, consequence, breadth, frequency, recoverability, compatibility, and severity. Keep observed facts separate from inference and state any missing evidence that could change the decision.
 
-Report the `Need evidence` status before classifying the need as a capability gap, ergonomics or discoverability gap, unsupported use case, or no demonstrated gap. Do not assign practical impact to the absence of the requested mechanism when an existing supported workflow already produces the requested outcome. Do not infer practical importance merely from reachability, API asymmetry, or a technically successful patch.
+Report the `Need status` before classifying the need as a capability gap, ergonomics or discoverability gap, unsupported use case, or no demonstrated gap. Do not assign practical impact to the absence of the requested mechanism when an existing supported workflow already produces the requested outcome. Do not infer practical importance merely from reachability, API asymmetry, or a technically successful patch.
 
 For a PR, make `Severity` describe the underlying issue or user need only. Do not combine it with the risk created by the proposed patch. Report a meaningful patch-induced regression, compatibility, lifecycle, or maintenance risk separately as `Patch risk`.
 
@@ -185,7 +199,7 @@ Use one code recommendation:
 - **Supersede with a simpler alternative**: real need, but a smaller or more coherent fix is preferable.
 - **Not worth completing**: negligible or unsupported impact, no-op behavior, wrong abstraction, or excessive completion cost.
 
-`Merge-worthy as-is` and `Merge-worthy after focused changes` are invalid unless `Need evidence` is `Demonstrated`. A bounded set of implementation fixes cannot promote a `Plausible but unproven` need into a merge-worthy recommendation.
+`Merge-worthy as-is` and `Merge-worthy after focused changes` are invalid unless `Need status` is `Demonstrated`. A bounded set of implementation fixes cannot promote a `Plausible but unproven` need into a merge-worthy recommendation.
 
 For `Merge-worthy as-is` and `Merge-worthy after focused changes`, use one repository-readiness status when it helps communicate the integration state:
 
@@ -217,7 +231,7 @@ Do not infer the assessment language from the GitHub URL, contributor, code, or
 
 Use the matching compact report variant in `references/evaluation-framework.md`. While decision-relevant evidence is pending, use its preliminary-assessment variant and end with the evidence limitation and optional suggestion for a separate runtime investigation instead of presenting a final recommendation. Collapse sections for simple cases rather than padding the answer. Put unexpected or negative runtime findings first, and name the preferred PR or approach explicitly when candidates compete.
 
-For PRs, put `Need evidence` before code recommendation. When the need is not `Demonstrated`, lead with that result, omit repository readiness, and avoid presenting patch fixes as the primary maintainer action.
+For PRs, put `Need status` before code recommendation. When the need is not `Demonstrated`, lead with that result, omit repository readiness, and avoid presenting patch fixes as the primary maintainer action.
 
 When existing functionality or a better alternative materially affects the decision, state it explicitly in the evidence and recommendation. Name the exact supported path, what it does and does not cover, and why it is preferable. Do not bury a `Not worth completing` or `Supersede with a simpler alternative` conclusion beneath praise for implementation quality.
 
@@ -225,6 +239,8 @@ When recommending closure, requesting more evidence, requesting code changes, or
 
 Before returning any maintainer comment draft, perform a GitHub paste-readiness pass using the repository-wide rule in `AGENTS.md` and the detailed guidance in `references/evaluation-framework.md`. In the draft, use `#123` for same-repository issues or PRs and `owner/repo#123` for cross-repository references. Remove Markdown-linked issue or PR labels, Codex navigation links, local file links, Codex-only citation markers or footnotes, and app directives from the copy-ready draft. Preserve ordinary descriptive links to API docs, design notes, and other targets without native GitHub issue or pull-request syntax.
 
+Also perform an action-delta pass. Every imperative sentence must correspond to a concrete difference between the current remote head and the desired state. Remove requests to "keep", "preserve", document, test, or change behavior that the current head already satisfies or that is not merge-blocking. A `Merge-worthy as-is` result must not contain change-request language. Keep portfolio comparisons out of a contributor-facing draft unless duplicate or supersession handling is the action for that target.
+
 For request-changes comments, phrase maintainer-owned semantic decisions as a directive, not as a menu. It is fine to mention the rejected alternative briefly in the rationale, but the requested action must identify the chosen behavior, scope, or compatibility boundary. Use "please do X because..." instead of "either do X or Y" when X versus Y changes the SDK contract or user-visible semantics.
 
 Do not produce a line-by-line review unless requested. Do not equate passing tests with merge-worthiness, or a logically correct patch with practical value.

.agents/skills/maintainer-review/references/evaluation-framework.md

@@ -29,7 +29,7 @@ Treat validity, severity, and merge-worthiness as separate results. Also disting
 | Consequence | What fails, and is the result silent or recoverable? | Observed output/error/state plus downstream effect |
 | Breadth | Who is affected? | Supported providers, platforms, versions, and configurations identified precisely |
 | Frequency | Is this normal, intermittent, or pathological? | Repeat runs, telemetry or reports when available, deterministic preconditions |
-| Need evidence | Is the exact scope demonstrated, merely plausible, already covered, or unsupported? | Observed impact or a complete realistic trigger-to-material-consequence trace for prevention |
+| Need status | Is the exact scope demonstrated, merely plausible, already covered, or unsupported? | Observed impact or a complete realistic trigger-to-material-consequence trace for prevention |
 | Unmet need | What user outcome cannot be achieved through supported behavior today? | Concrete scenario plus a trace showing why the closest existing path is insufficient |
 | Existing capability | Can configuration, composition, cloning, callbacks, extension points, or a caller-owned layer already satisfy the outcome? | Current release code, tests, docs, and an exact supported workflow |
 | Compatibility | Is released behavior or durable state changed? | Latest release comparison and explicit contract inspection |
@@ -58,6 +58,7 @@ Before calling a claim confirmed, answer:
 - Are setup failures, stale builds, environment leakage, proxies, caches, or unsupported options excluded?
 - Does an adjacent helper or equivalent path follow different semantics?
 - Is the observed behavior prohibited by an actual contract, or merely surprising?
+- If the patch removes or reinterprets an established observable or explicit test expectation, what did the introducing commit and original tests intend? Treat that history as compatibility-risk evidence, not automatic proof that the behavior must never change.
 - For latency, timeout, buffering, backpressure, or cleanup claims, was observable elapsed time or a real state transition measured when feasible rather than inferred only from mocks?
 - For shared asynchronous state, do tests control completion order and prove that stale failure or cleanup cannot affect the surviving operation?
 
@@ -71,7 +72,7 @@ Issue reports often combine a desired outcome with a proposed API or implementat
 
 Evidence from a linked issue applies only when the issue and PR share the same runtime variant, provider or tool type, trigger, supported configuration, and user outcome. A broad title, ordinary reference, `Related to` statement, or conceptual similarity is not enough. If an earlier change already resolved the concrete reported scenario, an adjacent extension starts with no inherited evidence of need.
 
-### Need evidence status
+### Need status
 
 Assign one status before deep implementation review:
 
@@ -82,6 +83,8 @@ Assign one status before deep implementation review:
 
 Only `Demonstrated` need can support a merge-worthy code recommendation. `Plausible but unproven` maps to `Needs evidence` or `Not worth completing`, even when the patch is technically correct and its remaining fixes are bounded. `Already covered` and `Unsupported` normally map to closure or a simpler non-core alternative.
 
+The need status is an evidence classification, not the issue action. Record observation validity, downstream consequence, need status, and issue action separately. A reported shape or branch difference can be confirmed while the need remains `Plausible but unproven` and the correct issue action is `Close`.
+
 ### Practical-impact gate
 
 Do not accept a change merely because desk review identifies a local logical flaw, defensive improvement, or constructible edge case. Trace the complete consequence chain:
@@ -90,6 +93,8 @@ Do not accept a change merely because desk review identifies a local logical fla
 
 A local intermediate inconsistency, redundant operation, surprising branch, or theoretically cleaner invariant is not a demonstrated need when it has no meaningful downstream effect. Reachability, a passing new test, a small diff, and low implementation cost establish neither practical impact nor maintenance value.
 
+For representation-only changes, name the concrete consumer computation, decision, or persisted interpretation that changes before and after the patch. If the patch only changes list shape, placeholder presence, metadata, ordering, or terminology without recovering information or changing a meaningful result, it does not establish practical impact. Contract ambiguity and API symmetry are insufficient by themselves, especially when the current shapes are released or intentionally test-covered.
+
 Use one of these evidence paths:
 
 | Evidence path | Required proof | Insufficient proof |
@@ -136,6 +141,8 @@ Choose one primary action:
 
 When requesting evidence, ask only for information that could change the disposition.
 
+For external contribution triage, default a `Plausible but unproven` need to `Close` when the current report shows only a logic-level or representation-level inconsistency. Use the `Needs evidence` issue action only when maintainers intentionally want to keep the issue open and can name one bounded piece of evidence likely to change the decision. A closed issue may still state the concrete evidence that would justify reconsideration.
+
 ## PR quality and value
 
 Assess these independently:
@@ -269,6 +276,13 @@ Use GitHub-native references in every draft:
 
 Before returning the draft, normalize any same-repository URL or qualified reference to `#<number>`, normalize any cross-repository issue or pull-request URL to `owner/repo#<number>`, and rescan the draft. Do not return it while a Markdown-linked issue or pull-request label, `openai/openai-agents-python#<number>`, or bare GitHub issue or pull-request URL remains.
 
+Perform an action-delta pass after the paste-readiness pass:
+
+- Map every imperative sentence to a concrete difference between the current remote head and the desired state.
+- Remove requests for behavior, tests, documentation, or scope that the current head already satisfies or that the recommendation does not require.
+- Do not use change-request language for `Merge-worthy as-is`.
+- Keep portfolio comparisons out of a contributor-facing draft unless that target is being closed or redirected as a duplicate or superseded implementation.
+
 Do not include internal labels such as `severity: low`, speculate about AI authorship or contributor intent, repeat the full review, or soften the message until the requested action becomes unclear.
 
 Do not ask contributors to choose maintainer-owned semantics. If two implementations are technically possible but one changes the SDK contract, decide the contract in the review and make the comment actionable. Use a short rationale such as "This keeps the new handler scoped to the existing raise site" or "This makes the handler name match all invalid final messages", then request the exact code and tests for that decision.
@@ -347,7 +361,7 @@ Use `Maintainer decision` for a concluded review. Use `Preliminary assessment` w
 ```markdown
 ## Maintainer decision
 <Need, practical impact, and merge-worthiness.>
-- Need evidence: <Demonstrated / Plausible but unproven / Already covered / Unsupported>
+- Need status: <Demonstrated / Plausible but unproven / Already covered / Unsupported>
 - Code recommendation: <code disposition>
 - Repository readiness: <integration status; include only for a merge-worthy recommendation when material>