Skip to content

Update the Components.Platform porting skill with rules settled by Cookie Store - #69989

Open
jsakamotoIGJP wants to merge 7 commits into
dotnet:mainfrom
jsakamotoIGJP:components-platform-skill-followup
Open

jsakamotoIGJP wants to merge 7 commits into
dotnet:mainfrom
jsakamotoIGJP:components-platform-skill-followup

Conversation

@jsakamotoIGJP

Copy link
Copy Markdown
Contributor
  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable. (Not applicable. This PR changes only the skill and its eval.)
  • You've included inline docs for your change, where applicable. (Not applicable.)
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Update the porting skill with rules settled by Cookie Store

Description

This PR updates the port-components-platform-api skill (#69829). It does not change any product code.

When the skill was merged, two rules were still open, so the skill told the agent to stop and ask. Both were settled in the review of the Cookie Store API proposal (#69844), and Cookie Store (#69845) is now in main. This PR turns them into fixed rules and points the agent at Cookie Store as a second example.

  • [Experimental] for members that are not Baseline. The skill said that the package had no diagnostic ID yet. Now it tells the agent to use ASP0040 with the same UrlFormat as ASP0039, never to allocate a new ID, and to suppress the warning with #pragma in tests and demo pages. The CookieInit example in type-mapping.md now has the attribute on Partitioned, as in the real code.
  • NotSupportedException for a missing browser object. The skill called it a proposed default and told the agent to raise it as an open question. Now it is a fixed rule.
  • E2E result marker. The skill now describes the data-run counter that CookieStoreTests uses, so a step cannot pass on the text of the previous step.
  • Cookie Store as a second reference. Web Storage stays the reference implementation. The skill now also names Cookie Store for what Web Storage does not have, such as a missing browser object, dictionaries, an enum, and JSON assertions in unit tests.

Skill eval

I added three stimuli to eng/skill-evals/port-components-platform-api/eval.vally.yaml, one for each new rule. They cover a member that is not Baseline, a missing browser object, and E2E steps that expect the same text as an earlier step. I ran the smoke experiment locally (one trial per stimulus). The skilled variant passed all 8 stimuli, and the baseline passed none.

Contributes to #69987

@jsakamotoIGJP
jsakamotoIGJP requested review from a team and wtgodbe as code owners October 23, 2026 07:06
Copilot AI balanced review requested due to automatic review settings October 23, 2026 07:06
@github-actions github-actions Bot added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Oct 23, 2026
@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Oct 23, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Thanks for your PR, @jsakamotoIGJP. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Hey @dotnet/aspnet-build, looks like this PR is something you want to take a look at.

@jsakamotoIGJP

Copy link
Copy Markdown
Contributor Author

@ilonatommy This is the follow-up to the porting skill (#69829). It turns the two rules you settled on the Cookie Store proposal ([Experimental] with ASP0040, and NotSupportedException for a missing browser object) into fixed rules, and adds eval stimuli for them.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The required frozen review bundle was unavailable, so the prescribed approval checks remain incomplete.

No open issues 🎉

What changed in this PR

This PR updates the Components.Platform porting skill to reflect decisions settled by Cookie Store, without changing product code.

Changes:

  • Establishes ASP0040 and NotSupportedException as fixed porting rules.
  • Adds Cookie Store references and completion-counter guidance for E2E steps.
  • Adds three evaluation stimuli covering the newly documented rules.
File Description
eng/​skill-evals/​port-components-platform-api/​eval.vally.yaml Adds evaluations for experimental members, missing browser objects, and E2E result tracking.
.github/​skills/​port-components-platform-api/​SKILL.md Codifies settled rules and adds Cookie Store reference patterns.
.github/​skills/​port-components-platform-api/​references/​type-mapping.md Marks the example Partitioned property experimental with ASP0040.

Review effort: Balanced

- The PublicAPI.Unshipped.txt lines for the experimental member have no diagnostic ID prefix.
- The ASP0040 warning is suppressed only around the code that uses the experimental member, not for a whole project with NoWarn.
- The answer does not treat the experimental marking itself as an unresolved open question that blocks the design.
- Callers reach the family through a synchronous Screen extension property on Window, and the other attributes become GetWidthAsync, GetHeightAsync, and GetColorDepthAsync with int, int, and uint results.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agent review: Could we add Web IDL long → C# int to references/type-mapping.md? This rubric requires that mapping, but the reference currently documents only unsigned long and long long. Without it, this case partly tests prior Web IDL knowledge rather than the skill.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PureWeen Thanks, good catch. I added long to int to references/type-mapping.md in b479756, so the Screen rubric no longer depends on prior Web IDL knowledge.

This branch was previously deployed

1 inactive (outdated) deployment
copilot-pat-pool — 5577e747 Deployed Oct 23, 2026 by dotnet-policy-service[bot] via pat_pool #1934
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants