Skip to content

feat(typespec-python): test-proxy-style regen HTML diff PR previews - #4752

Closed
Libba Lawrence (l0lawrence) wants to merge 2 commits into
Azure:mainfrom
l0lawrence:l0lawrence/typespec-python-regen-diff
Closed

Libba Lawrence (l0lawrence) wants to merge 2 commits into
Azure:mainfrom
l0lawrence:l0lawrence/typespec-python-regen-diff

Conversation

@l0lawrence

Copy link
Copy Markdown
Member

Summary

Ports a test-proxy-style baseline + HTML-diff PR-preview system to typespec-python, modeled on the Azure test-proxy assets convention. The goal is to replace the current "regenerate and read a GitHub compare link" flow with a per-PR rendered HTML diff plus a failing commit status when generated output drifts from the baseline.

On each PR that touches packages/typespec-python/**, CI:

  1. Builds the emitter and regenerates the test output.
  2. Restores the baseline from an external assets store (assets.json -> Azure/azure-sdk-assets, test-proxy convention) via an anonymous clone.
  3. Renders an HTML diff (baseline vs. fresh output), uploads it as an artifact.
  4. A companion workflow_run job publishes the HTML to GitHub Pages (pr/<num>/), posts a sticky PR comment linking the preview, and sets a python-regen-diff/baseline commit status (failure when output differs).

Uses only the built-in GITHUB_TOKEN via the workflow_run split (fork-safe, no PAT / extra secret).

What's included

  • packages/typespec-python/eng/scripts/ci/assets.ts — anonymous restore of the baseline tag from the assets repo.
  • packages/typespec-python/eng/scripts/ci/render-diff.ts — HTML diff renderer (also supports --open browser preview and --vscode native diff for local dev).
  • packages/typespec-python/eng/scripts/ci/push-assets.ts — maintainer-run baseline publish + tag bump.
  • packages/typespec-python/assets.json — baseline pointer (Azure/azure-sdk-assets, prefix typespec/python). Tag is empty until bootstrapped.
  • package.json — regenerate:render-diff / :diff / :vscode-diff / :review / :push-assets scripts + diff2html (catalog).
  • pnpm-workspace.yaml — diff2html catalog entry.
  • .github/workflows/python-regen-diff*.yml — build+artifact / publish (pages+comment+status) / cleanup-on-close.

Local developer experience

  • pnpm run regenerate:diff — regenerate (implicit) then render + open the HTML diff in a browser, no token needed (anonymous baseline clone).
  • pnpm run regenerate:vscode-diff — open each changed file as a native VS Code diff tab.
  • pnpm run regenerate:review — regenerate then render+open.

⚠️ Pre-merge checklist (draft)

  • Regenerate pnpm-lock.yaml — run pnpm install to lock the new diff2html catalog dep (this draft intentionally omits the lockfile change).
  • Bootstrap the baseline — a maintainer runs pnpm run regenerate:push-assets (needs write to Azure/azure-sdk-assets) to seed the first tag, then commits the assets.json Tag bump. Until then, the diff shows all output as "added".
  • Enable GitHub Pages on the base repo (gh-pages branch) for the preview to serve.
  • The Pages preview + PR comment only activate once this lands on the base default branch (workflow_run runs from base HEAD) — expected.
  • Optionally make python-regen-diff/baseline a required status via branch protection.
  • Add a chronus changeset if the repo's consistency check requires one.

Notes

  • The 3 scripts are self-contained (only import each other + node builtins + picocolors + diff2html); they are not yet wired into eng/scripts/sync.ts INCLUDES, so they remain standalone local files for now.
  • prepareBaselineOfGeneratedCode is intentionally left unchanged — the diff baseline is restored independently by render-diff.ts; the synced regenerate-common.ts is untouched to keep pnpm sync:check green.

Port the assets-baseline + HTML-diff PR-preview system to typespec-python.
On each PR touching packages/typespec-python, regenerate the emitter test
output, diff it against an external assets baseline (assets.json ->
Azure/azure-sdk-assets), render an HTML diff, publish it to GitHub Pages,
and post a sticky PR comment. Uses only the built-in GITHUB_TOKEN via the
workflow_run pattern (fork-safe, no PAT).

- eng/scripts/ci/assets.ts: anonymous restore of the baseline tag
- eng/scripts/ci/render-diff.ts: HTML/VS Code/browser diff renderer
- eng/scripts/ci/push-assets.ts: maintainer-run baseline publish + tag bump
- assets.json: baseline pointer (Tag empty until bootstrapped)
- package.json: regenerate:* scripts + diff2html (catalog)
- pnpm-workspace.yaml: diff2html catalog entry
- workflows: python-regen-diff (build+artifact), -publish (pages+comment+
  commit status), -cleanup (remove per-PR preview on close)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@microsoft-github-policy-service microsoft-github-policy-service Bot added eng emitter:python Issues for @azure-tools/typespec-python emitter labels Jun 29, 2026
Comment thread eng/common/scripts/regen-diff/src/render-diff.ts Fixed
Comment thread eng/common/scripts/regen-diff/src/render-diff.ts Fixed
@azure-sdk-automation

Copy link
Copy Markdown
Contributor

❌ There is undocummented changes. Run chronus add to add a changeset or click here.

The following packages have changes but are not documented.

  • ❌@azure-tools/typespec-python
Show changes

…reviews

Replace the package-local regen-diff scripts with the shared, config-driven
tool under eng/common/scripts/regen-diff (render + push-assets CLI). Add
regen-diff.config.json, point the regenerate:* scripts at the shared CLI, and
delete the package's render-diff/push-assets/assets scripts. Generalize the
publish/cleanup workflows (slug-namespaced, language-agnostic) and drop the
now-unused diff2html dependency and catalog entry. assets.json targets the
configured assets repo; nothing is pushed to any Azure assets repo.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
// `code` is a shell script / .cmd on most platforms, so it must be launched
// through a shell (Node refuses to execFile a .cmd directly). Compose a single
// quoted command string so the shell parses paths with spaces correctly.
const q = (p: string) => `"${p.replace(/"/g, '\\"')}"`;
function openInBrowser(target: string): void {
try {
if (process.platform === "win32") {
execFileSync("cmd", ["/c", "start", "", target], { stdio: "ignore" });
@github-actions

Copy link
Copy Markdown
Contributor

⚡ Benchmark Results

⚠️ 4 metric(s) regressed above the +5% threshold:

Metric Baseline Current Change
resolver 🟢 19.6ms 🟢 21.0ms +7.3% 🔴
validation 🟢 45.1ms 🟢 49.1ms +8.7% 🔴
 ↳ validation/@typespec/versioning 🔴 28.9ms 🔴 33.5ms +15.6% 🔴
 ↳ emit/@typespec/openapi3/write 🟢 18.5ms 🟢 21.5ms +16.0% 🔴
Full details – comparing 272b031 vs baseline a351bf4
Metric Baseline Current Change
total 🔴 675.3ms 🔴 707.1ms +4.7%
loader 🟢 188.3ms 🟢 188.5ms +0.1%
resolver 🟢 19.6ms 🟢 21.0ms +7.3% 🔴
checker 🟡 211.1ms 🟢 196.3ms -7.0% 🟢
validation 🟢 45.1ms 🟢 49.1ms +8.7% 🔴
 ↳ validation/@azure-tools/typespec-azure-core 🟢 7.1ms 🟢 6.6ms -6.6%
 ↳ validation/@typespec/http 🟢 6.0ms 🟢 5.7ms -4.3%
 ↳ validation/@typespec/rest 🟢 0.6ms 🟢 0.7ms +27.6%
 ↳ validation/@typespec/versioning 🔴 28.9ms 🔴 33.5ms +15.6% 🔴
 ↳ validation/compiler 🟢 1.7ms 🟢 1.8ms +7.4%
linter 🟢 159.0ms 🟢 143.8ms -9.5% 🟢
 ↳ linter/@azure-tools/typespec-azure-core/auth-required 🟢 0.0ms 🟢 0.0ms +20.6%
 ↳ linter/@azure-tools/typespec-azure-core/bad-record-type 🟢 0.2ms 🟢 0.2ms +4.9%
 ↳ linter/@azure-tools/typespec-azure-core/byos 🟢 6.7ms 🟢 6.8ms +2.5%
 ↳ linter/@azure-tools/typespec-azure-core/casing-style 🟢 0.7ms 🟢 0.6ms -3.9%
 ↳ linter/@azure-tools/typespec-azure-core/composition-over-inheritance 🟢 0.1ms 🟢 0.1ms +4.0%
 ↳ linter/@azure-tools/typespec-azure-core/documentation-required 🟢 0.8ms 🟢 1.1ms +29.5%
 ↳ linter/@azure-tools/typespec-azure-core/friendly-name 🟢 0.6ms 🟢 0.6ms -1.6%
 ↳ linter/@azure-tools/typespec-azure-core/key-visibility-required 🟢 0.2ms 🟢 0.2ms -9.7%
 ↳ linter/@azure-tools/typespec-azure-core/known-encoding 🟢 0.3ms 🟢 0.3ms +3.5%
 ↳ linter/@azure-tools/typespec-azure-core/long-running-polling-operation-required 🟢 0.3ms 🟢 0.3ms -5.0%
 ↳ linter/@azure-tools/typespec-azure-core/no-case-mismatch 🟢 0.2ms 🟢 0.3ms +0.8%
 ↳ linter/@azure-tools/typespec-azure-core/no-closed-literal-union 🟢 0.3ms 🟢 0.3ms +20.3%
 ↳ linter/@azure-tools/typespec-azure-core/no-enum 🟢 0.0ms 🟢 0.0ms +4.0%
 ↳ linter/@azure-tools/typespec-azure-core/no-error-status-codes 🟢 0.1ms 🟢 0.1ms +5.1%
 ↳ linter/@azure-tools/typespec-azure-core/no-explicit-routes-resource-ops 🟢 0.1ms 🟢 0.1ms +13.4%
 ↳ linter/@azure-tools/typespec-azure-core/no-format 🟢 0.6ms 🟢 0.6ms +2.5%
 ↳ linter/@azure-tools/typespec-azure-core/no-generic-numeric 🟢 0.4ms 🟢 0.5ms +10.6%
 ↳ linter/@azure-tools/typespec-azure-core/no-header-explode 🔴 22.7ms 🔴 20.4ms -10.3% 🟢
 ↳ linter/@azure-tools/typespec-azure-core/no-legacy-usage 🟢 1.3ms 🟢 1.2ms -8.8%
 ↳ linter/@azure-tools/typespec-azure-core/no-multiple-discriminator 🟢 0.1ms 🟢 0.1ms +3.4%
 ↳ linter/@azure-tools/typespec-azure-core/no-nullable 🟢 0.3ms 🟢 0.3ms -0.1%
 ↳ linter/@azure-tools/typespec-azure-core/no-offsetdatetime 🟢 1.2ms 🟢 1.2ms -1.8%
 ↳ linter/@azure-tools/typespec-azure-core/no-openapi 🟢 2.1ms 🟢 2.1ms +0.5%
 ↳ linter/@azure-tools/typespec-azure-core/no-private-usage 🟢 2.0ms 🟢 2.2ms +14.5%
 ↳ linter/@azure-tools/typespec-azure-core/no-query-explode 🔴 25.2ms 🔴 21.8ms -13.5% 🟢
 ↳ linter/@azure-tools/typespec-azure-core/no-response-body 🔴 26.3ms 🔴 26.6ms +0.9%
 ↳ linter/@azure-tools/typespec-azure-core/no-rest-library-interfaces 🟢 0.0ms 🟢 0.0ms +12.2%
 ↳ linter/@azure-tools/typespec-azure-core/no-route-parameter-name-mismatch 🟢 5.7ms 🟢 5.7ms +0.2%
 ↳ linter/@azure-tools/typespec-azure-core/no-rpc-path-params 🟢 0.2ms 🟢 0.2ms -6.5%
 ↳ linter/@azure-tools/typespec-azure-core/no-string-discriminator 🟢 0.0ms 🟢 0.0ms +0.4%
 ↳ linter/@azure-tools/typespec-azure-core/no-unknown 🟢 0.2ms 🟢 0.2ms -6.7%
 ↳ linter/@azure-tools/typespec-azure-core/no-unnamed-union 🟢 0.4ms 🟢 0.4ms +8.5%
 ↳ linter/@azure-tools/typespec-azure-core/operation-missing-api-version 🟢 0.2ms 🟢 0.2ms +20.3%
 ↳ linter/@azure-tools/typespec-azure-core/request-body-problem 🟢 0.3ms 🟢 0.3ms +12.0%
 ↳ linter/@azure-tools/typespec-azure-core/require-versioned 🟢 0.0ms 🟢 0.0ms +41.5%
 ↳ linter/@azure-tools/typespec-azure-core/response-schema-problem 🔴 24.9ms 🔴 24.2ms -2.8%
 ↳ linter/@azure-tools/typespec-azure-core/rpc-operation-request-body 🟢 0.4ms 🟢 0.3ms -10.4%
 ↳ linter/@azure-tools/typespec-azure-core/spread-discriminated-model 🟢 0.3ms 🟢 0.3ms -0.2%
 ↳ linter/@azure-tools/typespec-azure-core/use-standard-names 🟢 6.6ms 🟢 5.9ms -10.4%
 ↳ linter/@azure-tools/typespec-azure-core/use-standard-operations 🟢 0.1ms 🟢 0.1ms +0.2%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-agent-base-type-child-resources 🟢 5.1ms 🟢 4.3ms -16.5%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-agent-base-type-lifecycle-operations 🟢 0.0ms 🟢 0.0ms +9.7%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-common-types-version 🟢 4.6ms 🟢 4.1ms -11.6%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-custom-resource-no-key 🟢 0.1ms 🟢 0.1ms -1.3%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-custom-resource-usage-discourage 🟢 0.1ms 🟢 0.1ms +8.4%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-delete-operation-response-codes 🟢 1.9ms 🟢 1.2ms -36.8%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-no-path-casing-conflicts 🟢 5.6ms 🟢 4.5ms -20.3% 🟢
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-no-record 🟢 0.4ms 🟢 0.4ms +7.3%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-post-operation-response-codes 🟢 0.5ms 🟢 0.5ms -13.2%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-put-operation-response-codes 🟢 0.0ms 🟢 0.0ms +22.9%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-action-no-segment 🟢 0.2ms 🟢 0.2ms -10.8%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-duplicate-property 🟢 0.2ms 🟢 0.1ms -9.9%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-interface-requires-decorator 🟢 0.0ms 🟢 0.0ms +22.4%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-action-verb 🟢 0.1ms 🟢 0.1ms +8.1%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-envelope-property 🟢 0.1ms 🟢 0.1ms -4.4%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-version-format 🟢 0.0ms 🟢 0.0ms +4.7%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-key-invalid-chars 🟢 0.3ms 🟢 0.2ms -11.2%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-name-pattern 🟢 0.0ms 🟢 0.0ms +27.4%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-operation 🟢 0.2ms 🟢 0.2ms +0.0%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-operation-response 🟢 5.1ms 🟢 4.6ms -10.3%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-patch 🟢 0.3ms 🟢 0.3ms -5.8%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-path-segment-invalid-chars 🟢 0.2ms 🟢 0.2ms -5.7%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-provisioning-state 🟢 0.1ms 🟢 0.1ms +0.5%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/beyond-nesting-levels 🟢 0.1ms 🟢 0.1ms -0.7%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/empty-updateable-properties 🟢 0.2ms 🟢 0.2ms -4.1%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/improper-subscription-list-operation 🟢 0.0ms 🟢 0.0ms +12.7%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/lro-location-header 🟡 15.6ms 🟡 13.1ms -16.2% 🟢
 ↳ linter/@azure-tools/typespec-azure-resource-manager/missing-operations-endpoint 🟢 0.0ms 🟢 0.0ms +8.1%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/missing-x-ms-identifiers 🟢 0.3ms 🟢 0.3ms +6.9%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/no-empty-model 🟢 0.2ms 🟢 0.1ms -5.1%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/no-override-props 🟢 0.1ms 🟢 0.1ms +2.6%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/no-resource-delete-operation 🟢 0.2ms 🟢 0.2ms -9.8%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/no-response-body 🔴 23.1ms 🟡 19.5ms -15.7% 🟢
 ↳ linter/@azure-tools/typespec-azure-resource-manager/patch-envelope 🟢 0.2ms 🟢 0.2ms -4.3%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/resource-name 🟢 0.2ms 🟢 0.1ms -10.1%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/secret-prop 🟢 2.6ms 🟢 2.8ms +7.9%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/unsupported-type 🟢 0.4ms 🟢 0.4ms +5.7%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/version-progression 🟢 0.0ms 🟢 0.0ms +20.8%
 ↳ linter/@azure-tools/typespec-client-generator-core/property-name-conflict 🟢 1.2ms 🟢 1.1ms -3.7%
 ↳ linter/@azure-tools/typespec-client-generator-core/require-client-suffix 🟢 0.2ms 🟢 0.2ms +28.6%
emit 🔴 5.84s 🔴 5.53s -5.3% 🟢
 ↳ emit/@azure-tools/typespec-autorest 🟢 174.7ms 🟢 168.8ms -3.4%
 ↳ emit/@azure-tools/typespec-python 🔴 4.50s 🔴 4.28s -4.9%
 ↳ emit/@typespec/http-client-js 🔴 992.8ms 🔴 858.2ms -13.6% 🟢
 ↳ emit/@typespec/openapi3 🟢 157.4ms 🟢 153.5ms -2.5%
 ↳ emit/@typespec/openapi3/compute 🟢 138.1ms 🟢 130.9ms -5.2% 🟢
 ↳ emit/@typespec/openapi3/write 🟢 18.5ms 🟢 21.5ms +16.0% 🔴

Averaged across 3 specs (azure-arm-resource-manager, azure-core-dataplane, azure-full).
Threshold: changes > ±5% are highlighted.
🟢 Fast · 🟡 Moderate (stages >200ms, rules >10ms) · 🔴 Slow (stages >400ms, rules >20ms)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

emitter:python Issues for @azure-tools/typespec-python emitter eng

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants