chore: update maintainer-review skill

This commit is contained in:
Kazuhiro Sera
2026-06-30 15:11:31 +09:00
parent fea17ef542
commit 7713342bed
3 changed files with 53 additions and 10 deletions
+26 -6
View File
@@ -15,10 +15,11 @@ Make a maintainer decision, not a generic code-review summary. Separate these qu
4. If a gap remains, is the proposed solution the best design and implementation layer?
5. Can normal users plausibly reach the gap, and what happens when they do?
6. Is it important enough to act on now?
7. For a PR, is this solution worth merging and maintaining?
8. Can overlapping or stale operations corrupt shared state or clean up resources owned by surviving work?
9. If competing PRs exist, which single implementation path should maintainers pursue?
10. What concise maintainer message should communicate a closure or change request clearly and politely?
7. If this PR did not already exist, would maintainers choose to open and implement the same work?
8. For a PR, is this solution worth merging and maintaining?
9. Can overlapping or stale operations corrupt shared state or clean up resources owned by surviving work?
10. If competing PRs exist, which single implementation path should maintainers pursue?
11. What concise maintainer message should communicate a closure or change request clearly and politely?
Treat an issue's requested field, callback, flag, class, or implementation strategy as a proposed mechanism, not as the accepted requirement. Do not begin by asking how to implement it. First prove that a concrete user outcome is not already supported and that the proposed mechanism is better than the available alternatives.
@@ -33,6 +34,7 @@ Lead with the current review state. Use `Preliminary assessment` while runtime a
- 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.
- 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.
@@ -40,13 +42,26 @@ 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:
- **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.
- **Already covered**: A reasonable supported workflow already satisfies the outcome.
- **Unsupported**: The outcome belongs outside the SDK contract or at a provider, adapter, or caller-owned layer.
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.
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.
2. Trace the closest supported ways to achieve that outcome in the current release and current target. Inspect the owning code path, public API, tests, and relevant docs rather than assuming that an unfamiliar capability is missing. Consider configuration, composition, cloning, callbacks, extension points, provider adapters, and doing the work at a caller-owned layer.
3. Determine whether the report shows a capability gap, an ergonomics or discoverability problem, an unsupported use case, or no demonstrated problem. A more convenient spelling is not automatically a missing capability.
4. Compare the proposed solution against the strongest existing approach and at least one better-design candidate: no code change, clearer documentation or validation, a narrower fix, reuse of an existing abstraction, or enforcement at a more coherent shared boundary.
5. For each viable approach, compare whether it satisfies the concrete scenario, what new public or internal contract it creates, cross-path consistency, compatibility, and permanent maintenance cost.
Do not treat a test proving that new code can work as evidence that the feature is needed. If the report provides no concrete scenario, the existing functionality appears sufficient, or the requested mechanism solves only a hypothetical convenience problem, prefer `Needs evidence`, `Close`, `Supersede with a simpler alternative`, or `Not worth completing` over designing the requested feature on the reporter's behalf.
Do not treat a test proving that new code can work as evidence that the feature is needed. A `FakeModel` response, manually constructed provider item, mock, or new regression test can establish code-path reachability and implementation correctness; it does not by itself establish realistic provider behavior, user reach, frequency, practical consequence, or demand.
API symmetry, naming consistency, and parity with an adjacent tool, provider, or output type are design arguments, not evidence of need. Parity may justify work when it removes existing complexity or enforces a broad demonstrated invariant, but adding branches, tests, documentation, or public behavior requires independent practical justification.
If the need is not `Demonstrated`, inspect the patch only far enough to understand its contract, risk, and maintenance cost. Do not turn implementation defects, missing tests, or documentation gaps into a request-changes recommendation, because those questions become merge-blocking only after the need gate passes. If the report provides no concrete scenario, the existing functionality appears sufficient, or the requested mechanism solves only a hypothetical convenience problem, prefer `Needs evidence`, `Close`, `Supersede with a simpler alternative`, or `Not worth completing` over designing the requested feature on the reporter's behalf.
### 3. Discover competing open PRs proportionally
@@ -88,6 +103,7 @@ Before a positive assessment, complete the pass in step 2 and be able to state a
3. Why the proposed behavior belongs at the chosen abstraction layer instead of a caller, adapter, validation, documentation, or existing extension point.
4. Why the proposed permanent contract is better than no code change and the strongest narrower alternative.
5. What real scenario, compatibility requirement, or repeated demand justifies the new maintenance surface.
6. Whether maintainers would choose to pursue the same work if no contributor had already supplied a patch.
If any answer is missing and could change whether code should exist at all, do not call the issue actionable or the PR merge-worthy. Request only the evidence needed to distinguish a genuine capability gap from a usage, discoverability, or solution-design problem. This is a product and architecture evidence gap, not a runtime-probe trigger by itself.
@@ -130,7 +146,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.
Classify the need explicitly 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.
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.
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`.
@@ -147,6 +163,8 @@ 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.
For `Merge-worthy as-is` and `Merge-worthy after focused changes`, use one repository-readiness status when it helps communicate the integration state:
- **Ready**: current head is reviewable and required checks are green.
@@ -175,6 +193,8 @@ 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 runtime approval is pending, use its preliminary-assessment variant and end with the approval request 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.
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.
When recommending closure, requesting more evidence, requesting code changes, or superseding a PR, append the English, copy-paste-ready maintainer comment defined by the framework. If multiple PRs need different actions, label one draft for each affected PR. Include only merge-blocking requests in the main action paragraph; keep optional documentation or polish clearly non-blocking or omit it.
@@ -1,4 +1,4 @@
interface:
display_name: "Maintainer Review"
short_description: "Assess unmet need, design alternatives, and PR value"
default_prompt: "Use $maintainer-review with this GitHub issue or PR URL. Before evaluating the requested solution, identify the underlying user outcome, trace whether supported functionality already satisfies it, and compare the proposal with no code change and a better or narrower design. Then complete the desk review, including required lifecycle interleaving and cleanup-ownership checks, request approval before any decision-relevant runtime probe, compare credible competing PRs, recommend the best maintainer action, and include an English comment draft when closure or changes are needed."
short_description: "Gate PR value on demonstrated user need"
default_prompt: "Use $maintainer-review with this GitHub issue or PR URL. Before evaluating implementation quality, verify that linked evidence matches the exact runtime variant and assign Need evidence as Demonstrated, Plausible but unproven, Already covered, or Unsupported. Only a Demonstrated need may receive a merge-worthy recommendation; synthetic tests, API parity, and contributor effort do not establish need. Then compare existing and alternative approaches, complete the desk review and required lifecycle ownership checks, request approval before any decision-relevant runtime probe, compare credible competing PRs, recommend the best maintainer action, and include an English comment draft when closure or changes are needed."
@@ -29,6 +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? | Same-scope user scenario, real-path reproduction, released compatibility requirement, repeated demand, or broad consequential invariant |
| 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 |
@@ -65,6 +66,21 @@ Use `partially confirmed` when the symptom is real but the cause, reach, or clai
Issue reports often combine a desired outcome with a proposed API or implementation. Treat the proposed mechanism as a hypothesis. Confirm the unmet outcome before evaluating how well the patch implements that mechanism.
### Linked-evidence scope
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
Assign one status before deep implementation review:
- **Demonstrated**: The exact scope has a concrete supported scenario, real-path reproduction, released compatibility requirement, repeated demand, or broad invariant with meaningful consequence.
- **Plausible but unproven**: The code path is possible, but realistic reach, frequency, consequence, provider behavior, or demand is missing.
- **Already covered**: A reasonable supported workflow already satisfies the outcome.
- **Unsupported**: The outcome is outside the SDK contract or belongs at a provider, adapter, or caller-owned layer.
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.
Before accepting an issue or recommending a PR, record:
| Question | Required evidence |
@@ -82,7 +98,11 @@ Classify the result:
- **Unsupported use case**: the desired outcome lies outside the SDK contract or belongs at a provider, adapter, application, or other caller-owned layer. Do not expand the core API merely to make it possible.
- **No demonstrated gap**: no concrete scenario proves that existing functionality is insufficient. Request evidence or close rather than designing from the proposed mechanism.
Passing tests for a new implementation establish feasibility and correctness, not need. A technically coherent patch can still be `Not worth completing` when the motivating scenario is hypothetical, already supported, or better solved elsewhere.
Passing tests for a new implementation establish feasibility and correctness, not need. A `FakeModel` response, manually constructed provider item, mock, or synthetic fixture does not establish realistic provider behavior, user reach, frequency, consequence, or demand. API symmetry and parity with an adjacent runtime are design arguments, not need evidence. A technically coherent patch can still be `Not worth completing` when the motivating scenario is hypothetical, already supported, or better solved elsewhere.
Use the counterfactual maintainer test: if the PR did not already exist, would maintainers choose to file and implement the same work from the available evidence? Contributor effort lowers implementation cost but does not create product need or remove permanent maintenance cost.
When the need is not `Demonstrated`, inspect implementation only far enough to estimate contract, risk, and maintenance cost. Do not convert patch defects, missing tests, or documentation gaps into a request-changes disposition; those become merge blockers only after the need gate passes.
## Issue disposition
@@ -100,7 +120,7 @@ When requesting evidence, ask only for information that could change the disposi
Assess these independently:
1. **Need**: The linked issue or runtime evidence demonstrates a concrete unmet user outcome that the closest supported capability cannot reasonably satisfy.
1. **Need**: Same-scope issue or runtime evidence demonstrates a concrete unmet user outcome that the closest supported capability cannot reasonably satisfy. Do not inherit evidence from an adjacent variant or already-fixed scenario.
2. **Correctness**: The fix works for the reported case and meaningful boundaries.
3. **Placement**: The invariant is enforced once at the right layer instead of duplicating existing functionality, patching locally, or moving caller- or provider-owned policy into the core SDK.
4. **Consistency**: Equivalent sync/async, streaming/non-streaming, provider, serialization, and resume paths remain aligned where applicable.
@@ -111,6 +131,8 @@ Assess these independently:
A PR can be correct but not merge-worthy. Typical reasons include a nonexistent or negligible need, an outcome already supported through a reasonable existing mechanism, a no-op on the actual runtime path, incomplete cross-path semantics, an abstraction cost larger than the benefit, or a simpler design at another layer.
Do not use implementation correctness, bounded remaining work, CI status, or contributor effort to upgrade a need that is only `Plausible but unproven`. Merge-worthiness is gated by demonstrated need, not by how close the patch is to completion.
Keep issue impact and patch risk separate. `Severity` describes the underlying issue or user need. A regression, compatibility break, lifecycle leak, or maintenance hazard introduced by the proposed patch belongs under `Patch risk` and must not inflate or obscure the issue severity.
## Documentation threshold
@@ -288,6 +310,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>
- Code recommendation: <code disposition>
- Repository readiness: <integration status; include only for a merge-worthy recommendation when material>