You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
PR #43273 added Section 6.5 (TSP-ARRAY-IDENTIFIERS) to the TypeSpec review instructions and a Section 5 anti-pattern entry stating that @extension(...) must not appear in TypeSpec source for any reason. The reasoning is consistent: whenever an author reaches for @extension, a dedicated TypeSpec decorator (or an Azure TypeSpec template) exists that does the right thing without bypassing the type system or emitter conventions.
That PR shipped the rule wording and one eval fixture for the x-ms-identifiers case (stimulus 070004). The remaining @extension(...) misuses, which are at least as common in practice, are not covered by the eval harness and have no per-case guidance in the instruction files. Without fixtures and rubric coverage, the agent''s behavior on those cases cannot be measured or regression-tested, and reviewers receive uneven feedback depending on which extension the author happened to choose.
The most common cases that need explicit coverage:
@extension("x-ms-mutability", ...). Replace with @visibility(...) (Lifecycle.Read, Lifecycle.Create, Lifecycle.Update).
@extension("x-ms-secret", true). Replace with @secret.
@extension("x-ms-client-flatten", true). For new APIs, do not flatten at all (this aligns with the existing Section 5 guidance that bans new @flattenProperty). For pre-existing flattening preserved for backward compatibility, use @flattenProperty on the property, not @extension.
@extension("x-ms-enum", { ... }). Replace with a TypeSpec union of string literals; the Azure emitter derives x-ms-enum (including name and modelAsString) automatically.
@extension("x-ms-discriminator-value", "..."). Replace with @discriminator(...) on the base model and a literal-typed discriminator property on each variant.
@extension("x-ms-long-running-operation", true) and @extension("x-ms-long-running-operation-options", ...). Replace with the ARM async operation templates (ArmResourceCreateOrReplaceAsync, ArmResourcePatchAsync, ArmResourceActionAsync, ArmResourceDeleteAsync, etc.).
@extension("x-ms-azure-resource", true). Replace with the ARM resource base templates (TrackedResource, ProxyResource, ExtensionResource).
@extension("x-ms-pageable", ...). Replace with the ARM list operation templates (ArmResourceListByParent, ArmListBySubscription) or Azure.Core paged operation templates, which set x-ms-pageable automatically.
References
PR #43273 (original TSP-ARRAY-IDENTIFIERS rule and the Section 5 anti-pattern entry).
For each @extension(...) case listed in the Background (and any further case discovered during the work), add per-case guidance to typespec-review.instructions.md:
State the forbidden extension and the dedicated decorator or template that replaces it.
Include a short correct-pattern code block, following the structure used in Section 6.5.
State whether a #suppress is ever acceptable for the underlying linter rule, and if so under what narrow conditions. Apply the existing Section 4.1 Warning Suppression Policy in every such note.
Add a single cross-cutting row to the "TypeSpec Review Checklist Summary" stating that @extension(...) must not appear in TypeSpec source for any reason, and add per-case rows for the most common extensions (mutability, secret, enum, discriminator-value, LRO, azure-resource, pageable, client-flatten).
Add eval fixtures under .github/skills/evals/arm-api-reviewer/fixtures/typespec/:
One fixture per @extension(...) case, each containing both a seeded violation and a positive control that shows the correct decorator or template in use.
Keep fixtures small and focused so a failure points at one rule.
Consolidate multiple cases into a single multi-case fixture only when the rubric grading remains unambiguous.
Add corresponding stimuli to .github/skills/evals/arm-api-reviewer/evaluate/eval-typespec.yaml:
Each stimulus must include output-matches graders for the extension name and the correct replacement decorator or template, plus a prompt grader rubric that requires the agent to:
Flag every seeded violation with the correct rule reference (Section 5 anti-pattern, plus the per-case section).
Recommend the correct dedicated decorator or template as the fix.
Run the full eval suite locally and confirm that all new stimuli pass before opening the PR.
Acceptance criteria
Every @extension(...) case listed in the Background has dedicated guidance in typespec-review.instructions.md with the correct replacement decorator or template and a worked example.
The "TypeSpec Review Checklist Summary" and the "ARM Review Checklist Summary" each contain a cross-cutting row stating that @extension(...) is forbidden in TypeSpec source for any reason.
The eval suite contains at least one stimulus per @extension(...) case, each with a fixture that includes both a seeded violation and a positive control. All new stimuli pass when the suite is run locally.
Stimulus and fixture counters in .vally.yaml, README.md, fixtures/README.md, and run-evals.ps1 are consistent with the new count after the change.
The PR introducing these changes does not modify any service specification/** files, language emitters, or eng/ content. Scope stays within agent instructions, eval fixtures, and eval configuration.
Background
PR #43273 added Section 6.5 (
TSP-ARRAY-IDENTIFIERS) to the TypeSpec review instructions and a Section 5 anti-pattern entry stating that@extension(...)must not appear in TypeSpec source for any reason. The reasoning is consistent: whenever an author reaches for@extension, a dedicated TypeSpec decorator (or an Azure TypeSpec template) exists that does the right thing without bypassing the type system or emitter conventions.That PR shipped the rule wording and one eval fixture for the
x-ms-identifierscase (stimulus070004). The remaining@extension(...)misuses, which are at least as common in practice, are not covered by the eval harness and have no per-case guidance in the instruction files. Without fixtures and rubric coverage, the agent''s behavior on those cases cannot be measured or regression-tested, and reviewers receive uneven feedback depending on which extension the author happened to choose.The most common cases that need explicit coverage:
@extension("x-ms-mutability", ...). Replace with@visibility(...)(Lifecycle.Read,Lifecycle.Create,Lifecycle.Update).@extension("x-ms-secret", true). Replace with@secret.@extension("x-ms-client-flatten", true). For new APIs, do not flatten at all (this aligns with the existing Section 5 guidance that bans new@flattenProperty). For pre-existing flattening preserved for backward compatibility, use@flattenPropertyon the property, not@extension.@extension("x-ms-enum", { ... }). Replace with a TypeSpecunionof string literals; the Azure emitter derivesx-ms-enum(includingnameandmodelAsString) automatically.@extension("x-ms-discriminator-value", "..."). Replace with@discriminator(...)on the base model and a literal-typed discriminator property on each variant.@extension("x-ms-long-running-operation", true)and@extension("x-ms-long-running-operation-options", ...). Replace with the ARM async operation templates (ArmResourceCreateOrReplaceAsync,ArmResourcePatchAsync,ArmResourceActionAsync,ArmResourceDeleteAsync, etc.).@extension("x-ms-azure-resource", true). Replace with the ARM resource base templates (TrackedResource,ProxyResource,ExtensionResource).@extension("x-ms-pageable", ...). Replace with the ARM list operation templates (ArmResourceListByParent,ArmListBySubscription) or Azure.Core paged operation templates, which setx-ms-pageableautomatically.References
TSP-ARRAY-IDENTIFIERSrule and the Section 5 anti-pattern entry)..github/agents/arm-api-reviewer.agent.md..github/instructions/typespec-review.instructions.md(Section 5 anti-patterns, Section 6 type rules, "TypeSpec Review Checklist Summary")..github/instructions/armapi-review.instructions.md("ARM Review Checklist Summary").TSP-ARRAY-IDENTIFIERSreview rule: ban@extension("x-ms-identifiers", ...)in TypeSpec; require@key/@identifiers#43273:.github/skills/evals/arm-api-reviewer/fixtures/typespec/x-ms-identifiers-violations.tsp..github/skills/evals/arm-api-reviewer/evaluate/eval-typespec.yaml(stimulus070004)..github/skills/evals/arm-api-reviewer/README.md,fixtures/README.md,.vally.yaml,run-evals.ps1.Requested work
@extension(...)case listed in the Background (and any further case discovered during the work), add per-case guidance totypespec-review.instructions.md:#suppressis ever acceptable for the underlying linter rule, and if so under what narrow conditions. Apply the existing Section 4.1 Warning Suppression Policy in every such note.@extension(...)must not appear in TypeSpec source for any reason, and add per-case rows for the most common extensions (mutability, secret, enum, discriminator-value, LRO, azure-resource, pageable, client-flatten).armapi-review.instructions.md, mirroring the row added by PR AddTSP-ARRAY-IDENTIFIERSreview rule: ban@extension("x-ms-identifiers", ...)in TypeSpec; require@key/@identifiers#43273 for the array identifiers case..github/skills/evals/arm-api-reviewer/fixtures/typespec/:@extension(...)case, each containing both a seeded violation and a positive control that shows the correct decorator or template in use..github/skills/evals/arm-api-reviewer/evaluate/eval-typespec.yaml:output-matchesgraders for the extension name and the correct replacement decorator or template, plus apromptgrader rubric that requires the agent to:@extension(...)as a fix..vally.yaml,README.md,fixtures/README.md, andrun-evals.ps1, following the bookkeeping pattern from PR AddTSP-ARRAY-IDENTIFIERSreview rule: ban@extension("x-ms-identifiers", ...)in TypeSpec; require@key/@identifiers#43273.Acceptance criteria
@extension(...)case listed in the Background has dedicated guidance intypespec-review.instructions.mdwith the correct replacement decorator or template and a worked example.@extension(...)is forbidden in TypeSpec source for any reason.@extension(...)case, each with a fixture that includes both a seeded violation and a positive control. All new stimuli pass when the suite is run locally..vally.yaml,README.md,fixtures/README.md, andrun-evals.ps1are consistent with the new count after the change.specification/**files, language emitters, oreng/content. Scope stays within agent instructions, eval fixtures, and eval configuration.