Skip to content

Deterministic offline codegen drift check on PRs #259

Description

@ptr727

Summary

Add a deterministic, offline codegen drift check that runs on every PR, so a regression in the code-emitting tooling (SaveCodeAsync, SaveJsonAsync, GetCodeGenString) is caught on the PR that introduces it, not later.

Background

The codegen emitters live in the shipped library (LanguageTags/*Data.cs, LanguageTags/LanguageSchema.cs) but are client-irrelevant tooling used only by LanguageTagsCreate. Today they are validated by:

  • run-codegen-pull-request-task.yml, which downloads fresh upstream data, regenerates, and opens a codegen-* PR only when output changed. That PR's build + test gates then validate the regenerated files.
  • Existing round-trip tests asserting Create() (generated) equals a fresh FromDataAsync() parse, plus sniff tests now covering SaveCodeAsync/GetCodeGenString.

Gap: this validation is deferred and upstream-triggered. A feature PR that edits an emitter does not run codegen, so a break stays latent until the next scheduled run (which only fires when upstream data also changes). That is the "discovered later when generated code is broken" risk.

Proposal

  1. Split download from generate in LanguageTagsCreate (today ExecuteAsync always downloads first). Add a generate-only / --skip-download path that regenerates from the committed LanguageData/ with no network.
  2. Add a PR validation job that regenerates from committed data, runs the same dotnet csharpier format, then git diff --exit-code. Reuses the exact toolchain the codegen workflow uses so formatting matches and the diff is deterministic.

Effect: every PR touching an emitter re-exercises it and fails if emitted output drifts from the committed *DataGen.cs / *.json without those files being updated in the same PR.

Design concern to resolve (raised in review)

The check must cleanly distinguish three cases without becoming something contributors have to remember to disable:

  • Emitter logic intentionally changed -> committed generated files must be regenerated in the same PR; check passes because working tree matches. This is the intended workflow and must be documented (change emitter, regenerate locally, commit both).
  • Committed source data intentionally changed -> same: regenerate and commit in the same PR.
  • Neither changed but output differs -> real regression; check fails.

Keep it deterministic by running strictly offline against committed LanguageData/ (no upstream fetch), so upstream registry drift can never trip this check. Evaluate whether the check should be skippable via a label for the rare case of a deliberate two-step rollout of new codegen logic, versus always requiring same-PR regeneration.

Acceptance criteria

  • Generate-only mode exists and needs no network.
  • PR job fails when regenerated output differs from committed files; passes when they match.
  • No dependency on upstream data availability or content.
  • Contributor workflow documented in AGENTS.md.

Notes

Related coverage work: the emitter methods are now covered by sniff tests rather than excluded, so this check is about regression-at-PR-time, not coverage.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions