diff --git a/.github/skills/api-proposal/SKILL.md b/.github/skills/api-proposal/SKILL.md new file mode 100644 index 00000000000000..c0217780fc9162 --- /dev/null +++ b/.github/skills/api-proposal/SKILL.md @@ -0,0 +1,309 @@ +--- +name: api-proposal +description: Create prototype backed API proposals for dotnet/runtime. Use when asked to draft an API proposal or to refine a vague API idea into a complete proposal. +--- + +# API Proposal Skill + +Create complete, terse, and empirically grounded API proposals for dotnet/runtime. The output should have a high chance of passing the [API review process](https://github.com/dotnet/runtime/blob/main/docs/project/api-review-process.md). + +> 🚨 **NEVER** submit a proposal without a working prototype. The prototype is the evidence that the API works. Proposals without prototypes are speculative. + +> 🚨 **NEVER** use `gh pr review --approve` or `--request-changes`. Only `--comment` is allowed. + +## When to Use This Skill + +Use this skill when: +- Asked to propose a new API for dotnet/runtime +- Given a vague API idea or incomplete sketch that needs to be turned into a complete proposal +- Given an existing underdeveloped `api-suggestion` issue to refine +- Asked to prototype an API and draft a proposal +- Asked to "write an API proposal", "draft an api-suggestion", or "improve this proposal" + +## Core Principles + +1. **TERSENESS**: Proposals are reviewed live by humans during API review meetings who often lack prior context. Long text is counterproductive unless warranted by design complexity. Focus on WHAT problem and HOW to solve it. + +2. **Empirically grounded**: Build and test a working prototype BEFORE writing the proposal. The prototype validates the design, surfaces edge cases, and produces the exact API surface via ref source generation. + +3. **Claims backed by evidence**: Every motivating claim must have at least one concrete scenario. "This is useful" without showing *who* needs it and *how* they'd use it is the #1 reason proposals get sent back. + +4. **Context-driven depth**: The amount of supporting text should be proportional to how much **new information** the proposal introduces — not just API surface size. A small API introducing a novel concept needs more justification than a large API adding async counterparts to existing sync methods. + +## Modular Phases + +This skill has 6 phases. Each can run independently (e.g., "just draft the proposal from my existing prototype"). When running the full pipeline, execute in order. + +--- + +### Phase 0: Gather Input & Assess Context + +1. **Accept input** in any form: issue URL, text description, API sketch, or vague idea. + +2. **If the input is an existing GitHub issue**, read it in full and identify: + - What sections are missing or underdeveloped + - Whether the proposed API surface is concrete or still vague + - Any reviewer feedback in comments (especially if `api-needs-work` label is present) + +3. **Identify the target**: namespace, area label, affected types, target library under `src/libraries/`. + +4. **Assess novelty**: Is this a well-understood pattern extension (async variant, new overload, casing option) or something introducing a novel concept? This determines the depth of the proposal. + +5. **Evaluate existing workarounds**: Before proceeding, research what users can do TODAY without this API. + - Present the workaround(s) to the user for evaluation + - Explain trade-offs: performance penalty? excessive boilerplate? bad practices? + - **This is a checkpoint**: If a workaround is acceptable, the user may decide to shelve the proposal + - Only proceed to prototyping if workarounds are genuinely insufficient + +6. **Search for prior proposals** on the same topic: + - Search dotnet/runtime issues for related proposals (e.g., `api-suggestion`, `api-needs-work`, or closed issues) + - If duplicates exist, surface them — don't block work, but note them for linking later + - Look for clues in reviewer feedback: what caused a prior proposal to be marked `api-needs-work`? Why was it closed or stalled? Learn from that history to avoid repeating the same mistakes + +7. Ask clarifying questions if the proposal is too vague to prototype. + +--- + +### Phase 1: Research + +The skill contains baked-in examples and guidelines for writing good proposals (see [references/proposal-examples.md](references/proposal-examples.md) and [references/api-proposal-checklist.md](references/api-proposal-checklist.md)). The agent does NOT need to search for `api-approved` issues as templates — the baked-in references are sufficient. Phase 0's search for *related* issues on the same topic is a separate concern and is still required. + +**What the agent DOES at runtime:** + +1. **Read the Framework Design Guidelines digest** at `docs/coding-guidelines/framework-design-guidelines-digest.md`. Validate that proposed names follow the conventions. + +2. **Read existing APIs in the target namespace** to ensure consistency: + - Naming patterns (e.g., `TryX` pattern, `XAsync` pattern, overload shapes) + - Type hierarchies and interface implementations + - Parameter ordering conventions + +3. **Read the reference documentation for updating ref source** at `docs/coding-guidelines/updating-ref-source.md`. + +--- + +### Phase 2: Prototype + +> **If the user already has a prototype**, ask for the published branch link and/or commit hash. Skip steps 1–3 below, but still run all steps in **Prototype Validation** against the provided branch/commit before proceeding to Phase 3. + +1. Create a new branch: `api-proposal/`. The prototype must be kept as a **single commit** on this branch. + - Commit the initial prototype as a single commit. + - When addressing review feedback, amend the existing commit or squash locally and force-push so that the branch history remains a single commit. + +2. Implement the API surface with: + - Complete triple-slash XML documentation on all public members + - Proper `#if` guards for TFM-specific APIs + +3. Write comprehensive tests: + - Use `[Theory]` with `[InlineData]`/`[MemberData]` where applicable + - Cover edge cases, null inputs, boundary conditions + - Test any interaction with existing APIs + +#### Prototype Validation (all steps required) + +> **Prerequisite:** Follow the build and test workflow in [`copilot-instructions.md`](/.github/copilot-instructions.md) — complete the baseline build, configure the environment, and use the component-specific workflow for the target library. All build and test steps below assume the baseline build has already succeeded. + +**Step 1: Build and test** + +Build the src and test projects, then run all tests for the target library using the workflow described in `copilot-instructions.md`. All tests must pass with zero failures. + +Building the test project separately is critical for detecting **source breaking changes** that ApiCompat won't catch: +- New overloads/extension methods causing wrong method binding in existing code +- New generic overloads causing overload resolution ambiguity +- Pay attention to compilation **warnings**, not just errors + +**Step 2: Check TFM compatibility** + +Inspect the library's `.csproj` for `TargetFrameworks`. If it ships netstandard2.0 or net462 artifacts: +- Verify the prototype compiles for ALL target frameworks, not just `$(NetCoreAppCurrent)` +- Ensure .NET Core APIs form a **superset** of netstandard/netfx APIs +- Use `#if` guards where types like `DateOnly`, `IParsable` restrict parts of the surface to .NET Core +- Failure to maintain superset relationship risks breaking changes on upgrade/type-forward + +**Step 3: Generate reference assembly source** + +```bash +cd src/libraries//src +dotnet msbuild /t:GenerateReferenceAssemblySource +``` + +For System.Runtime, use `dotnet build --no-incremental /t:GenerateReferenceAssemblySource`. + +This: +- Produces the **exact public API diff** to use in the proposal +- Validates that only intended APIs were added (no accidental public surface leakage) +- The `ref/` folder changes **must be committed** as part of the prototype + +**The flow is**: vague input → working prototype → extract exact API surface from ref source → write the proposal. The prototype comes BEFORE the exact API proposal. + +--- + +### Phase 3: Review (encapsulates code-review skill) — BLOCKING + +1. Invoke the **code-review** skill against the prototype diff. + +2. **All errors and warnings must be fixed** before proceeding to the draft phase. + +3. If the API change could affect performance (hot paths, allocations, new collection types), suggest running the **performance-benchmark** skill. + +4. Re-run tests after any review-driven changes to confirm nothing regressed. + +--- + +### Phase 4: Draft Proposal + +**Core principle: TERSENESS.** Focus on WHAT problem and HOW to solve it. Do not generate long text unless the design complexity warrants it. + +Write the proposal matching the spirit of the [issue template](https://github.com/dotnet/runtime/blob/main/.github/ISSUE_TEMPLATE/02_api_proposal.yml). Skip inapplicable fields rather than filling them with "N/A". + +#### Proposal Structure + +**1. Background and motivation** + +- WHAT concrete user problem are we solving? Show scenario(s). +- Reference prior art in other ecosystems where relevant. +- Briefly summarize existing workarounds and why they are insufficient, but do not repeat the full Phase 0 analysis. Keep this section focused on the problem and the high-level rationale for a new API. + +**2. API Proposal** + +The exact API surface, extracted from the `GenerateReferenceAssemblySource` output: + +- **New self-contained types**: Clean declaration format (no diff markers). Example: + +```csharp +namespace System.Collections.Generic; + +public class PriorityQueue +{ + public PriorityQueue(); + public PriorityQueue(IComparer? comparer); + public int Count { get; } + public void Enqueue(TElement element, TPriority priority); + public TElement Dequeue(); + // ... +} +``` + +- **Additions to existing types**: Prefer `csharp` blocks when the proposal only adds new members and doesn't need to show existing APIs for context. Mark the containing type `partial` to emphasize it has other public members: + +```csharp +namespace System.Text.Json; + +public partial class JsonNamingPolicy +{ + public static JsonNamingPolicy SnakeLowerCase { get; } + public static JsonNamingPolicy SnakeUpperCase { get; } +} +``` + +When existing members ARE needed for context (e.g., to show sibling overloads), use `diff` blocks instead: + +```diff +namespace System.Text.Json; + +public partial class JsonNamingPolicy +{ + public static JsonNamingPolicy CamelCase { get; } ++ public static JsonNamingPolicy SnakeLowerCase { get; } ++ public static JsonNamingPolicy SnakeUpperCase { get; } +} +``` + +Rules: +- **No implementation code.** Ever. +- **No extensive XML docs.** Comments only as brief clarifications for the review board. +- Naming must follow the [Framework Design Guidelines](https://github.com/dotnet/runtime/blob/main/docs/coding-guidelines/framework-design-guidelines-digest.md). + +**3. API Usage** + +Realistic, compilable code examples demonstrating the primary scenarios. Number and depth should match the novelty of the API, not just its size. A simple new overload may need one example; a new collection type may need several showing different use patterns. + +**4. Design Decisions** (nontrivial only) + +For any decision where reasonable alternatives exist, briefly explain the reasoning. Omit for self-evident decisions. List format works well: + +- "Uses a quaternary heap instead of binary for better cache locality" +- "Does not implement `IEnumerable` because elements cannot be efficiently enumerated in priority order" + +**5. Alternative Designs** + +The agent has the burden of proof when claiming no viable alternatives exist. Show that alternatives were genuinely considered and explain why the proposed design is preferred. + +**6. Risks** + +The agent has the burden of proof when claiming absence of risks. Evaluate: +- Binary breaking changes (caught by ApiCompat) +- Source breaking changes (overload resolution, method binding) +- Performance implications +- TFM compatibility + +**7. Open Questions** (if any) + +List unresolved design questions with tentative answers. Surfacing uncertainty is a feature, not a weakness. Example from PriorityQueue: +- "Should we use `KeyValuePair` instead of tuples?" + +**8. Scope considerations** (if applicable) + +If the proposal could naturally extend to neighboring APIs (e.g., "should this also apply to `ToHashSet`?"), flag it as an open question. + +**9. Related issues** + +Link any related/duplicate proposals found during Phase 0 research. + +**10. Prototype** + +Link to the prototype commit (e.g., `https://github.com///commit/`). + +#### After Drafting + +Present the complete draft to the user for review. Iterate based on feedback before publishing. + +--- + +### Phase 5: Publish + +> **Agent disclaimer:** When publishing to GitHub on behalf of a user account, prepend the following disclaimer to the proposal body: +> +> ```markdown +> > [!NOTE] +> > This proposal was drafted with the help of an AI agent. Please review for accuracy and remove this notice once you're satisfied with the content. +> ``` + +#### Step 1: Push and capture commit URL + +Commit prototype changes and push the branch to the user's fork (default) or ask for an alternative remote. Capture the commit URL for inclusion in the proposal (e.g., `https://github.com///commit/`). + +#### Step 2: Non-interactive mode (Copilot Coding Agent) + +If the agent cannot prompt the user for input (e.g., running as Copilot Coding Agent), automatically post the API proposal as a comment on the associated pull request: + +```bash +gh pr comment --body-file proposal.md +``` + +Skip the interactive options below. + +#### Step 3: Interactive mode — offer publishing options + +Present the user with the following options. Which options appear depends on context: + +> **Note:** Always write the proposal text to a temporary file (e.g., `proposal.md`) and use `--body-file` instead of `--body` to avoid shell quoting/escaping issues with multi-line text. + +1. **Post as comment on existing issue/PR** — Only offer this when the user explicitly referenced an issue or PR in their original prompt. + ```bash + gh issue comment --body-file proposal.md + # or + gh pr comment --body-file proposal.md + ``` + +2. **Create a new issue** — Always offer this option. + ```bash + gh issue create --label api-suggestion --title "[API Proposal]: " --body-file proposal.md + ``` + No area label — repo automation handles that. + +3. **Create a new draft PR with proposal in OP** — Always offer this option. + ```bash + gh pr create --draft --title "[API Proposal]: <title>" --body-file proposal.md + ``` + +Include related issue links in the body for all options. diff --git a/.github/skills/api-proposal/references/api-proposal-checklist.md b/.github/skills/api-proposal/references/api-proposal-checklist.md new file mode 100644 index 00000000000000..272f368a2eba5b --- /dev/null +++ b/.github/skills/api-proposal/references/api-proposal-checklist.md @@ -0,0 +1,78 @@ +# API Proposal Quality Checklist + +Use this checklist to validate an API proposal before publishing. Items are ordered by importance. + +## Background and Motivation + +- [ ] **DO** state the concrete user problem with at least one real-world scenario +- [ ] **DO** evaluate existing workarounds and explain why they are insufficient (performance, boilerplate, bad practices) +- [ ] **DO** reference prior art in other ecosystems where relevant (e.g., Python, Java, Rust equivalents) +- [ ] **DO NOT** assume the reviewers are subject matter experts of the library being augmented (even though they're .NET experts) +- [ ] **DO NOT** make unsubstantiated claims (e.g., "this is commonly needed" without showing who needs it) +- [ ] **DO NOT** write motivation text that is longer than what the design complexity warrants + +## API Surface + +- [ ] **DO** extract the exact API surface from `GenerateReferenceAssemblySource` output +- [ ] **DO** use clean declaration format for new self-contained types +- [ ] **DO** use `diff` blocks showing relevant context (sibling overloads) for additions to existing types +- [ ] **DO** validate all names against the [Framework Design Guidelines](https://github.com/dotnet/runtime/blob/main/docs/coding-guidelines/framework-design-guidelines-digest.md) +- [ ] **DO** verify naming consistency with existing APIs in the target namespace +- [ ] **DO NOT** include implementation code in the API surface +- [ ] **DO NOT** include extensive XML documentation in the proposal diff — comments only as brief clarifications + +## Scope Completeness + +- [ ] **DO** consider whether neighboring APIs need the same treatment (e.g., adding to `ToDictionary`? what about `ToHashSet`?) +- [ ] **DO** consider whether APIs require common parameters (e.g., `CancellationToken`, `IEqualityComparer<T>`) +- [ ] **DO** consider async counterparts if adding sync APIs, and vice versa +- [ ] **DO** include `System.Linq.Queryable` and `System.Linq.AsyncEnumerable` equivalents when proposing new `System.Linq` methods +- [ ] **DO** consider overload consistency with existing method families +- [ ] **DO NOT** propose a narrow addition without evaluating the broader scope — reviewers will ask about it + +## Prototype Validation + +- [ ] **DO** verify the prototype builds for all target frameworks in the library's `.csproj` +- [ ] **DO** verify .NET Core APIs form a superset of netstandard/netfx APIs (if the library ships both) +- [ ] **DO** generate reference assembly source and commit the `ref/` changes +- [ ] **DO** build the test project separately to catch source breaking changes (overload resolution ambiguity, wrong method binding) +- [ ] **DO** verify all tests pass with zero failures +- [ ] **DO NOT** skip ApiCompat validation — binary compatibility must be maintained + +## Usage Examples + +- [ ] **DO** provide realistic, compilable code examples +- [ ] **DO** demonstrate the primary scenarios the API is designed to address +- [ ] **DO** match the number of examples to the novelty of the API, not just its size +- [ ] **DO NOT** provide only trivial/toy examples that don't demonstrate real usage + +## Design Decisions + +- [ ] **DO** explain the reasoning for nontrivial design choices +- [ ] **DO** list explicit trade-offs that were made (e.g., "no update support for 2-3x better performance") +- [ ] **DO NOT** explain self-evident decisions — this wastes reviewer time + +## Alternatives and Risks + +- [ ] **DO** demonstrate that alternatives were genuinely considered +- [ ] **DO** evaluate risks specifically: binary breaking changes, source breaking changes, performance, TFM compatibility +- [ ] **DO NOT** write "N/A" without demonstrating you've actually evaluated — it signals lack of research +- [ ] **DO NOT** dismiss risks with hand-wavy language ("low risk", "unlikely to cause issues") + +## Open Questions + +- [ ] **DO** list unresolved design questions with tentative answers +- [ ] **DO** surface uncertainty rather than hiding it — surprises during review are worse +- [ ] **DO NOT** leave fundamental design tensions unresolved without data to back a position + +## Common Reviewer Feedback Patterns (DO NOT) + +The following patterns consistently result in proposals being sent back for rework: + +- **DO NOT** state claims as facts without backing scenarios ("this is commonly needed" → show who and how) +- **DO NOT** propose a narrow scope without evaluating the full picture (reviewers will ask "what about X?") +- **DO NOT** use imprecise naming that violates Framework Design Guidelines conventions +- **DO NOT** propose APIs without checking for prior art ("has this been done before?" is a standard reviewer question) +- **DO NOT** oversimplify a complex design space — if the problem is nuanced, the proposal should acknowledge the nuance +- **DO NOT** hide open design questions — acknowledge them upfront with tentative answers +- **DO NOT** propose an API without a working prototype — speculation is not evidence diff --git a/.github/skills/api-proposal/references/proposal-examples.md b/.github/skills/api-proposal/references/proposal-examples.md new file mode 100644 index 00000000000000..200c1e94b7ab41 --- /dev/null +++ b/.github/skills/api-proposal/references/proposal-examples.md @@ -0,0 +1,118 @@ +# API Proposal Examples + +This document contains curated examples of successful API proposals to guide the drafting of new proposals. Study the structure, not just the content. + +## PriorityQueue (#43957) — Large Proposal + +> Source: https://github.com/dotnet/runtime/issues/43957 + +**Scale**: Major new collection type (`PriorityQueue<TElement, TPriority>`). + +**What made it succeed**: +- Backed by empirical research: surveyed .NET codebases for usage patterns, ran benchmarks across prototypes. Key finding: "90% of use cases do not require priority updates" and "implementations without update support are 2-3x faster." +- 8 explicit design decisions with rationale (e.g., "does not implement `IEnumerable` because elements cannot be efficiently enumerated in priority order") +- Open questions listed with tentative answers (e.g., "Should we use `KeyValuePair` instead of tuples? — We will use tuple types.") +- Link to a working prototype repo +- Implementation checklist (product code, benchmarks, property-based tests, API docs) +- Clean declaration format for the full API surface — no diff markers, no implementation code + +**Key lesson**: This proposal succeeded where the original PriorityQueue proposal (#14032) had stalled for years. The difference was doing the research first — empirical data (benchmarks, codebase surveys) resolved the design tensions that had blocked progress. The prototype was the evidence. + +--- + +## Annotated Summaries + +### PriorityQueue.DequeueEnqueue (#75070) — Small Proposal + +> Source: https://github.com/dotnet/runtime/issues/75070 + +**Scale**: Single method addition to an existing type. + +**What made it succeed**: +- Concise motivation: "extract-then-insert operations are generally more efficient than sequential extract/insert" +- Referenced prior art: Python's `heapq.heapreplace` +- Showed a concrete use case (linked list merge with priority queue) +- API surface was a single method with a clear `diff` block +- Risk section was brief but specific: "must be correctly optimized to outperform sequential Dequeue/Enqueue" + +**Key lesson**: A small proposal can be very short. The motivation was one paragraph. The usage example was one code block. This was sufficient because the concept (optimized pop-push) is well-understood. + +--- + +### snake_case Naming Policies (#782) — Medium Proposal + +> Source: https://github.com/dotnet/runtime/issues/782 + +**Scale**: Adding 4 static properties and 4 enum members across 2 types. + +**What made it succeed**: +- Clean `diff` format showing the additions alongside the existing `CamelCase` property +- Referenced Newtonsoft.Json behavior ("same behavior as Newtonsoft.Json") +- Pointed to GitHub's API as a concrete, widely-known use case for snake_case +- Scope was complete — covered both lower and upper variants for snake_case AND kebab-case + +**Key lesson**: The proposal succeeded because the reviewer could immediately understand it from the diff alone. The motivation was brief because snake_case support is a well-understood need. + +--- + +### Convert.ToHexString Lowercase (#60393) — Small Proposal + +> Source: https://github.com/dotnet/runtime/issues/60393 + +**Scale**: Adding a parameter to existing overloads. + +**What made it succeed**: +- Pointed to internal code that already supported the feature (`HexConverter.ToString` accepts casing) +- Motivation was brief: "in some scenarios lowercase is required" +- The implementation was trivial because the capability already existed internally + +**Key lesson**: When the internal infrastructure already supports a feature, the proposal can be minimal. The key evidence was "the internal class already supports this." + +--- + +### Async ZipFile APIs (#1541) — Large Pattern-Extension Proposal + +> Source: https://github.com/dotnet/runtime/issues/1541 + +**Scale**: Large API surface (30+ async counterparts across 3 types and 1 extension class), but following a well-understood pattern. + +**What made it succeed**: +- Motivation was one sentence: "All ZipFile APIs are currently synchronous. This means manipulations to zip files will always block a thread." +- Clean `diff` format showing each async method alongside its sync counterpart — the reviewer could immediately see the 1:1 mapping +- Explicitly addressed scope decisions: noted that `CreateEntry` had no async work inside, so `CreateEntryAsync` was proposed as optional +- Multiple usage examples covering different scenarios (read, update, create from directory) +- Implementation plan with a phased checklist + +**Key lesson**: A large API surface that follows a well-understood pattern (async counterparts for sync APIs) needs minimal motivation — the case is self-evident. The proposal's value was in being thorough about scope (which methods genuinely benefit from async) and providing clean, reviewable diffs. Depth of text was minimal despite the large surface area. + +--- + +### Options ValidateOnStart (#36391) — Medium Behavioral Proposal + +> Source: https://github.com/dotnet/runtime/issues/36391 + +**Scale**: Single extension method, but with significant behavioral implications for application startup. + +**What made it succeed**: +- Clear problem statement: "we want to get immediate feedback on validation problems — exceptions thrown on app startup rather than later" +- Terse API surface — just one extension method — but with detailed explanation of how it interacts with the existing `IOptions`/`IOptionsSnapshot`/`IOptionsMonitor` ecosystem +- Usage example was 3 lines of fluent builder code that immediately communicated the intent +- Explicitly scoped: "these APIs don't trigger for IOptionsSnapshot and IOptionsMonitor, where values may get recomputed on every request" + +**Key lesson**: Even a single method can require behavioral context when it changes when things happen (startup vs. lazy). The proposal was terse on API surface but precise on behavioral semantics — which aspects of the existing system it interacts with and which it doesn't. + +--- + +### JsonSerializerOptions.RespectNullableAnnotations (#74385) — Medium Proposal + +> Source: https://github.com/dotnet/runtime/issues/74385 + +**Scale**: Adding a boolean property with significant behavioral implications. + +**What made it succeed**: +- Strong community demand (83 reactions) established the motivation +- Detailed behavior specification covering edge cases +- Addressed the "what about source generators?" question proactively +- Backward compatibility was carefully designed (opt-in behavior) + +**Key lesson**: Even a single boolean property can require extensive proposal text when the behavioral implications are significant. The depth was proportional to the novelty of the concept, not the API surface size.