fix(wrapper-generator): make the parity-data refresh runnable, and refresh it - #3745
Merged
Conversation
Graph declares Edm.Int32/Int64 as "number" with the real type in the format; mapping by type alone emitted double? against Kiota's int? and did not compile. An explicit format now decides the CLR type, mirroring Kiota's own mapping.
…racle audit Auditing every v1.0 GET command in MgCommandMetadata.json against the singularizer surfaced four words where the rules disagree with shipped cmdlet names: Cookies -> "Cooky" (ships as ...HostCookie), Skus kept as-is (ships as Get-MgSubscribedSku), Dns -> "Dn" (ships as Get-MgDomainVerificationDnsRecord), Ios -> "Io" (ships as Get-MgDeviceAppManagementIosManagedAppProtection). Adds two irregulars and two invariants, each with a pinned test, and refreshes the README test count. 82 tests passing. Full-inventory match after fix: 796 of 870 noun segments; the remaining 74 are action/function segments and AutoRest hand renames, tracked separately.
…nt, start edge-case catalog Only 2 of 30 shipped whois-family commands truncate "Whois" to "Whoi"; per review decision the generator emits the corrected ...HostWhois (no alias for the old name), and the parity gate reports it as [CORRECTED] instead of failing. "Statistics" joins the invariants, found via the DEVX Humanizer exception list. edge-cases/naming-edge-cases.md starts the per-class catalog of naming defects. 88 tests passing.
Build-WrapperModule.ps1 turns one OpenAPI doc into an importable module (kiota client + wrappers + csproj + dll + PSD1 manifest), reading the Kiota-compatible docs by default with a hard kiota timeout and per-module doc fallback. Test-WrapperModule.ps1 imports each build in a fresh pwsh and verifies exports, worker pairing, and the sessionless NoGraphSession path. All 35 cmdlet-producing v1.0 modules build and pass.
Compiling all v1.0 modules against freshly generated kiota clients surfaced eight alignment defects, each fixed and pinned by a test: dispatchers re-wrapped worker errors (NoGraphSession was lost); body properties colliding with path ids (published convention: -DeviceId1); bare model types colliding with namespaces and BCL types (now fully qualified, mirroring kiota's move-inside and reserved-name renames at root and in sub-namespaces); collection responses resolved from their own $ref; underscore members (riskEventTypes_v2 -> RiskEventTypesV2); $select/$expand emitted only where declared; re-fetch only where a GET exists; media/content endpoints skipped like $value.
…ant, catalog kiota edge cases
The full-module parity sweep found two shipped-name issues: AutoRest
truncated /places/{id}/checkIns at the preposition (8 commands ship as
*-MgPlaceCheck while Get-MgPlaceCheckInCount keeps "In") - corrected per
policy with gate rows and pinned tests; and "Rights" needs to be an
inflection invariant (Get-MgPrivacySubjectRightsRequest, 42 cmdlets) -
Compliance now matches 23 of 23. New edge-cases/kiota-alignment file
catalogs the compile-found defect classes; README refreshed. 103 tests.
A second operation resolving to an already-written cmdlet file now fails generation with the full collision list instead of silently overwriting it, which is the silent-drop failure mode AutoRest had. OData cast list/item pairs (owners/graph.user) now merge like plain pairs, and the sweep's collisions land as cited NamingOverrides entries: termStore and agreement-file stitches, default-singleton renames (SubSite, DefaultDrive, DefaultCalendarEvent), and nested navs the SDK never shipped. Remaining families are tracked on #3704.
… data Derive-CollisionResolutions.ps1 replays the checked-in collision inventory (212 lines, 365 contested routes) against MgCommandMetadata and emits exact-match resolution data: 191 suppressions (routes the published SDK prunes) and 64 renames (published nouns), each entry carrying its oracle evidence. The files embed into the generator and apply only when UseCollisionData is set; -Validate fails on drift, and a new xunit test runs it on every `dotnet test` so staleness fails the suite instead of depending on someone remembering to run the script by hand. Derivation itself fails on any unclassified or ambiguous route. Only 2 cross-path variant merges exist in all of v1.0 (GroupPhoto, ShareListItem) - deferred with the singleton side kept, cataloged in crosspath-merge-edge-cases.md. Full 39-module v1.0 generation now produces zero collisions; 20 published commands that lost filename races are recovered; exact-name matches rise 5,042 -> 5,098. Also: cmdlets emit into a per-module namespace derived from the client namespace instead of the leftover MgPoC placeholder; Build-WrapperModule's generated csproj references Authentication by a relative path instead of an absolute one; its -Configuration parameter now actually reaches the wrapper generator's own build, not just the final module build; and a pre-existing nullable warning in the list/item pairing check is fixed. 121 tests pass.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
fix(wrapper-generator): fail loudly on cmdlet file collisions
Request bodies bound only top-level primitives, so 4,466 property occurrences across the v1.0 specs had no parameter. Every shape the classifier reaches now binds: referenced models and enums, formatted strings, schema-less UntypedNode values (converted on assignment, nulls dropped to match the published SDK's AddIf), and the numeric INF/NaN union. The invented -Password pair is replaced by the published -PasswordProfile. New gates verify it - omission oracle, coverage sweep, inventory diff, runtime conversions: 0 unbound across all 38 specs, 35 modules build and import, 148 tests. The pre-existing naming-parity gap is tracked separately.
Both lines carried the same singularizer-era fixes as different commits, so the five conflicts resolve to identical rules plus each side's real additions: powershell-v3's docs/edge-cases layout and lean comments win; the packaging line's Rights invariant, ResolveParameterNameCollisions, and PlaceCheckIn corrections survive. README test count set to the measured 121; its no-compile-test claim was already false here and is corrected.
…t-body-properties
- Emit media/content downloads (78 routes), completing the shape list in #3709 - Stamp each cmdlet with a [GraphRoute] attribute so the parity gate reads the operation's route from the compiled assembly rather than reconstructing it from generated C#, removing the cast and parameterized-function exclusions that left 1,669 cmdlets unverified - Order Count before the cast suffix on /$count routes (126 cmdlets) - Drop the unusable -OutFile parameter from content writes returning an entity - Collapse the three parallel OData segment tables into one - Refresh generator docs against measured figures 38/38 modules generate and build, 184/184 tests.
…s' into feat/wrapper-operation-shapes
…e}/wrapper
All 38 modules' Kiota clients and generated cmdlets are committed - 11,719
cmdlet files, 9,051 public names - so a clean checkout builds with only the
.NET SDK. The API version nests under wrapper/ because AutoRest clears
src/{Module}/{version} on regeneration; both projects target netstandard2.0
so one dll path serves Core and Desktop. DirectoryObjects no longer
double-declares publicKeyInfrastructure. Gates and docs re-measured against
this corpus: parity 9,548 of 10,385 joinable, omission oracle 0 failures.
…cation binding Manifests now declare RequiredModules on Microsoft.Graph.Authentication (minimum read from its csproj) and packages no longer embed the Authentication assemblies: a module-local copy at a different version splits the GraphSession static under Windows PowerShell's loader, reporting NoGraphSession while connected. ModuleVersion now equals the package version - the hard-coded placeholder made every versioned-folder install refuse to import. Proven by marker-client probes; package gate passes.
…e base class Microsoft.Graph.Wrapper.Runtime hosts what every generated cmdlet duplicated: GraphClientCmdlet owns -AccessToken/-Headers and transport acquisition - the session path reuses one request adapter keyed to the session HttpClient's identity (rebuilt on reconnect), the token path shares one HttpClient with per-request auth headers instead of leaking a connection pool per call. GraphRouteAttribute, UntypedValue and the bearer provider move here from the per-module Shared.g.cs. 22 pinned tests; emitted code unchanged until the emitter adopts the base class in the follow-up.
Every generated cmdlet now derives from GraphClientCmdlet: transport, the -AccessToken/-Headers surface and Graph error translation come from Microsoft.Graph.Wrapper.Runtime instead of being repeated per file, and the per-module Shared.g.cs is gone. The corpus shrinks ~417k lines (30%) with zero behavioral change: operation inventory unchanged (11,719 = 11,719), parity identical (9,548 of 10,385), omission oracle 0 failures, 38/38 build from a clean index snapshot. Also fixes the parity gate picking the runtime dll for modules sorting after R, and repo-root discovery in git worktrees.
…d Authentication module The first live-tenant run of a wrapper cmdlet crashed casting kiota's ObservabilityOptions: module bins carried the whole Authentication dependency closure (~60 dlls), loading a second identity of each assembly into the default load context beside the one the installed Microsoft.Graph.Authentication serves from its isolated context. Bins now carry only the three wrapper assemblies plus the two kiota dlls Authentication does not ship - Std.UriTemplate (preloaded via RequiredAssemblies: its requester lives in the isolated context and cannot probe the module folder) and Serialization.Multipart. PruneModuleBin removes package assets by package id so kept versions stay derived from the restore graph; DisableTransitiveProjectReferences and Private="false" stop the Authentication project copies. Cmdlet discovery and the parity gate now import Authentication before loading wrapper assemblies - nine cmdlets typed with kiota's Date/Time structs force those loads at class load, and both probes silently degraded without the resolver. Proven live against Graph: session, dispatcher, adapter reuse and 16-way parallel all pass; tools/Test-WrapperLive.ps1 is that gate, kept in the repo because offline gates are structurally blind to resolution faults that only manifest on a real request. Inventory unchanged (11,719 = 11,719), parity identical (9,548 of 10,385), oracle 0 failures, package gate PASS.
The #3712 evidence sweep found four defects between "packages exist" and "testers can install them": the nuspec declared no dependencies, so a clean machine got the wrapper without Authentication and import failed (Install-Module resolves from NuGet metadata, not the manifest); packages versioned as stable 2.39.0, colliding number-for-number with the real SDK's release train; the module GUID was random every build, breaking Update-Module identity across handout iterations; and both checkouts held stale pre-fix nupkgs under shipping ids. Packages now declare Microsoft.Graph.Authentication as an open-floor dependency (matching the manifest minimum and the use-latest ruling), always carry a prerelease label (-Prerelease, default wrapperpreview01), and derive a stable RFC 4122 name-based GUID from the module name - no lookup table, identical across builds. Stale artifacts deleted. Proven three rounds each: 38 of 38 packages carry the dependency, label and distinct stable GUIDs; Save-Module from a folder repository auto-resolves Authentication, and the saved layout imports and completes a live Graph call in a host that can see nothing else; package gate PASS. Also corrects the Std.UriTemplate comment - the AssemblyRef lives in Microsoft.Kiota.Abstractions, not the HTTP library - and the live gate's committed name (tools/Test-WrapperLive.ps1).
Joywambui-maina
requested review from
Peter Ombwa (peombwa) and
Ramses Sanchez-Hernandez (ramsessanchez)
and
a lite review from Copilot
August 25, 2026 19:26
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the wrapper-generator tooling so parity/collision-derived data can be regenerated against the committed wrapper corpus layout (src/<Module>/wrapper/<ApiVersion>/...) and adds an end-to-end refresh script that runs generation + derivation in an isolated git worktree.
Changes:
- Make cmdlet-name collisions fatal only when collision data is enabled, and non-fatal (logged) under
--no-collision-datato allow inventory capture runs. - Update wrapper gate + parity derivation scripts to read from the committed corpus layout and avoid hard-coded framework/output paths.
- Add
Update-WrapperParityData.ps1plus newBuild-WrapperModule.ps1switches (-NoCollisionData,-GenerateOnly) to orchestrate a clean refresh.
Reviewed changes
Copilot reviewed 5 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tools/WrapperGenerator/PowerShellWrapperGenerationService.cs | Makes collisions non-fatal when collision data is disabled; adds structured logging for raw collision inventory. |
| tools/Update-WrapperParityData.ps1 | New orchestrator script: snapshots current tracked state, generates unresolved corpus in an isolated worktree, derives parity data, and publishes outputs. |
| tools/Invoke-WrapperGates.ps1 | Updates gates to read committed corpus layout, adds -ApiVersion, and removes invalid -Configuration pass-through to Test-WrapperModule.ps1. |
| tools/Derive-ParityResolutions.ps1 | Updates default/capture paths to the committed corpus layout (src/<Module>/wrapper/<ApiVersion>/Cmdlets). |
| tools/Build-WrapperModule.ps1 | Adds -NoCollisionData and -GenerateOnly; supports parity/collision capture workflows and prevents -GenerateOnly + -Pack. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…dlets List cmdlets emitted only the first page and dropped @odata.nextLink, so scripts silently received partial data. EmitListGet - the single template behind every list-shaped cmdlet - now emits an -All switch that follows each non-empty nextLink to exhaustion, streaming every page to the pipeline before the next request is issued, and the public dispatcher declares the switch so it reaches the worker. A bound -Top caps the total under -All at whole-page granularity, matching the published ListCmdlet's shipped semantics. Without -All a surviving nextLink writes one short warning and costs no extra request - deliberately stronger than the published SDK, which truncates silently; approved in the design spec. Continuations go through kiota's WithUrl and re-apply headers only: the link already carries the query state, and a raw-URL builder ignores query bindings. A pipeline stop passes through the shared catch instead of being re-branded a Graph failure. Decisions and evidence in tools/WrapperGenerator/docs/pagination.md. Proven: 189 generator tests including five pagination pins, and tools/Test-WrapperPaging.ps1 drives a real compiled cmdlet and public dispatcher across a stubbed two-page collection - ten assertions, three rounds, all pass. The regenerated corpus follows in a separate pull request.
Output of the pagination emitter change, with no hand edits: 1,479 list-shaped cmdlets gain -All and the nextLink loop, 1,334 dispatchers declare the switch, and every cmdlet's shared catch gains the pipeline-stop filter. Proven against the sealed pre-change baseline: operation inventory unchanged (11,719 = 11,719), name parity identical (9,548 of 10,385), body-binding oracle 0 failures, package and live gates PASS, and -All walks a real Graph page boundary - 182 items across two server pages, matching a separately calibrated total.
…erations Delta cmdlets returned a single page and dropped both @odata.nextLink and @odata.deltaLink, so callers silently received partial change sets and had no way to resume. The token form of the same operation was emitted as a separate command the published SDK does not have - it folds each into the canonical delta command. Delta now has its own emit shape rather than the function template: items enumerate to the pipeline, -All follows nextLink through the change set, and the terminal deltaLink is published to -DeltaLinkVariable (alias DLV) in the caller's scope, cleared at entry so a run that stops early cannot leave the previous link readable. A response carrying both links is refused rather than guessed at; one carrying neither stops without publishing. -DeltaLink resumes any delta operation through the raw-URL builder, which is why no -Token is exposed: a token argument exists on five routes, a link resumes all of them. The pairing is derived from route shape, never a list of names, and an orphaned, ambiguous or duplicated resume form fails generation naming the route. Path ids are scoped to the initial-sync set, since the resume link already carries them. Contract and evidence in docs/edge-cases/delta-edge-cases.md. Proven: 194 generator tests including five delta pins, and tools/Test-WrapperDelta.ps1 drives a real compiled cmdlet across two nextLink pages to a terminal deltaLink and back through a resume - eleven assertions, three rounds, all pass. The regenerated corpus follows in a separate pull request.
…tion Output of the delta emitter change, with no hand edits: 67 delta commands gain -All, -DeltaLink and -DeltaLinkVariable, and the five delta(token=...) commands disappear, folded into their parameterless siblings as a resume parameter set. Proven against the sealed pre-change baseline: operation inventory 11,719 to 11,714 with exactly five removals and no additions, name parity matched unchanged at 9,548 with no-oracle down by the same five, body-binding oracle 0 failures, and the package gate PASS.
Refreshing the derived parity data was impossible for three independent reasons. Derive-ParityResolutions.ps1 and Invoke-WrapperGates.ps1 both read modules from <root>/<Module>/src/Cmdlets under artifacts/wrapper-modules, a layout superseded when the corpus moved under src/<Module>/wrapper/<version>, so neither found a single module. The capture the derivation documents needs a --no-collision-data tree, but the generator threw on cmdlet name collisions unconditionally - and those collisions are that mode's entire output, so the procedure asked for a tree the generator refused to produce. And the runtime gate passed -Configuration to Test-WrapperModule.ps1, which has no such parameter, so it errored out once it could reach the call at all. Collisions are now fatal only while the derived data is in use; the gates read the committed corpus through one shared discovery helper instead of four globs that drifted apart; Build-WrapperModule.ps1 gains -NoCollisionData and -GenerateOnly for isolated capture; and Update-WrapperParityData.ps1 orchestrates the refresh end to end. Verified: discovery finds 38 modules where it found 0, and --no-collision-data exits 0 where it exited 127 while still reporting every collision.
The checked-in data described a corpus several generator changes old: 14 rows referenced files that no longer generate, one generated file had no row, and five sat in a disposition the gate can no longer produce. Recaptured from an isolated --no-collision-data worktree so the raw, unresolved command surface is what gets swept - sweeping the resolved tree re-derives "nothing to fix" and would have discarded most of the data. Input ledger 13,946 to 14,001 rows, renames 1,726 to 2,126, suppressions 2,268 to 2,658, resolution ledger 10,928 to 12,588, with 27 deferred (17 cross-path, 10 cross-path merge). Byte-for-byte derivation validation passes and the runtime gate passes on 38 of 38 freshly packed modules with no stale packages.
Joywambui-maina
force-pushed
the
feat/wrapper-delta
branch
from
August 25, 2026 19:38
69b51b4 to
37bed0d
Compare
Joywambui-maina
force-pushed
the
feat/wrapper-parity-data
branch
from
August 25, 2026 19:38
b43ead7 to
6ac6a61
Compare
Peter Ombwa (peombwa)
left a comment
Peter Ombwa (peombwa)
left a comment
Member
There was a problem hiding this comment.
Please add a tools/README.md documenting:
- The dependency between scripts.
- The proposed workflow. How should the scripts be used? Think of it like a scenario guide.
…ata run With -NoCollisionData the generator reports each cmdlet collision instead of throwing, and that report IS the collision inventory: data/collision-inventory is captured from it and Derive-CollisionResolutions.ps1 reads it. The output was only inspected on failure and discarded otherwise, so the one run whose purpose is to produce the inventory printed nothing and the capture had to be done by invoking the generator directly. Written to the host, not the pipeline, because Build-Module returns $result and emitted output would be merged into it.
The 13 wrapper scripts had no entry point: which one to run, what each reads and writes, and which outputs feed which other script were only discoverable by reading all of them. Documents the dependency chain and the tasks people actually do - regenerate after a generator change, package for a tester, chase a parity failure, refresh derived data, run the gates before a PR. Calls out the two dependencies that are easy to miss: the collision inventory is captured from a -NoCollisionData run rather than hand-written, and the parity derivation invokes the parity gate rather than re-implementing its oracle join. Scopes itself to the wrapper toolchain; the AutoRest v2 scripts in the same directory are driven from build.proj and are not covered.
Contributor
Author
|
Added in 2bf5417 - Dependencies. A table of all 13 wrapper scripts with what each reads and writes, plus a diagram of how the outputs chain. It calls out the two that are easy to miss:
Workflow, as scenarios. Six of them: seeing what a generator change did to the output, trying a module, producing a package a tester can install, chasing a naming-parity failure, refreshing derived data, and what to run before opening a PR (with the eight gates listed in order and what each proves). Scoped to the wrapper toolchain and says so at the top - the other 26 scripts in Every script name, parameter and path in it is checked against the branch: 13 scripts exist, 12 documented parameters all resolve against the real parameter sets, and the |
Peter Ombwa (peombwa)
previously approved these changes
Aug 27, 2026
Joywambui-maina
dismissed
Peter Ombwa (peombwa)’s stale review
August 28, 2026 18:10
The base branch was changed.
Peter Ombwa (peombwa)
approved these changes
Aug 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes proposed:
-Point Derive-ParityResolutions.ps1 and Invoke-WrapperGates.ps1 at the committed corpus. Both read //src/Cmdlets under artifacts/wrapper-modules — a layout superseded when the corpus moved under src//wrapper/ — so neither found a single module
-Make cmdlet-name collisions fatal only while the derived data is in use. Under --no-collision-data the collisions are the intended output, so throwing there made the documented capture procedure impossible: the derivation asked for a tree the generator refused to produce
-Stop passing -Configuration to Test-WrapperModule.ps1, which has no such parameter — the runtime gate errored on every run that reached the call
-Add -NoCollisionData and -GenerateOnly to Build-WrapperModule.ps1 for isolated capture, and Update-WrapperParityData.ps1 to orchestrate the refresh end to end
-Refresh the derived data itself: input ledger 13,946 → 14,001 rows, renames 1,726 → 2,126, suppressions 2,268 → 2,658, resolution ledger 10,928 → 12,588, with 27 deferred