Repository navigation
Auto-suppress TypeSpec validation failures by harvesting TypeSpec Validation - All runs - #45589
Closed
iscai-msft wants to merge 10 commits into
Closed
iscai-msft wants to merge 10 commits into
iscai-msft wants to merge 10 commits into
Conversation
Adds a sharded scan workflow that runs the same tsv compile as TypeSpec Validation - All, plus an agentic workflow that consumes all shards for a branch and opens a single PR applying simple suppressions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e7b3fe66-9c12-4393-872a-4ae3ae669c27
Next Steps to Merge⌛ Please wait. Next steps to merge this PR are being evaluated by automation. ⌛Comment generated by summarize-checks workflow run. |
Replaces the redundant full recompile with a harvest step that parses the logs of an existing TSV-All run. TSV-All already runs 4x/day on both main and typespec-next, so this removes 6 duplicate compile jobs per cycle and makes the agent react to exactly what CI observed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e7b3fe66-9c12-4393-872a-4ae3ae669c27
Style and convention diagnostics report an exact file:line, so they are better suppressed inline against the single declaration that deviates than by suppressing an entire folder in suppressions.yaml. This follows the pattern used in PR #45367. The parser now extracts per-diagnostic locations and only marks a folder inline-eligible when every diagnostic in it is inline-suppressible, so a folder mixing a suppressible warning with a real compiler error still goes to a human. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e7b3fe66-9c12-4393-872a-4ae3ae669c27
Each #suppress directive adds a line, shifting every later line in the file down by one, so applying multiple suppressions to a file top-down misplaces all but the first. Diagnostics are now emitted sorted by descending line number within each file, and the prompt requires bottom-up application. Also normalizes the absolute runner paths in the logs to repo-relative paths and deduplicates diagnostics reported more than once, so the suppression count is accurate and no directive is inserted twice. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e7b3fe66-9c12-4393-872a-4ae3ae669c27
Any warning from a TypeSpec linter package (@azure-tools/ or @typespec/) is now inline-suppressible, replacing the hardcoded list of four rules. A warning is advisory by definition, so #suppress simply records that the deviation is intentional. Severity is the real gate: errors are never inline-suppressible regardless of the emitting package, since an error means the spec is broken or invalid. The agent is still told to leave any warning that looks like a genuine API design problem for a human. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e7b3fe66-9c12-4393-872a-4ae3ae669c27
Validated the harvester against TSV-All run 31362706542, which surfaced three bugs that synthetic test logs could not: - The TypeSpec compiler colorizes diagnostics, so file:line:col arrives with ANSI escapes interleaved between every field and the pattern never matched real output. Extracted diagnostics went from 0 to 147. - GitHub serves job logs as UTF-8 with a BOM, which corrupted the first line. - TSV compiles with --warn-as-error, so every linter warning prints as "error" and severity carries no information. Eligibility now keys on the presence of a namespaced rule id, which is the real signal: #suppress takes a rule id as its argument, so a diagnostic without one cannot be suppressed inline even in principle. The regex now captures rule-less compile errors as rule: null so they still count against folder eligibility. Against the real run: 589 folders, 104 already suppressed, 29 failures, all 29 eligible, 147 diagnostics across 69 files. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e7b3fe66-9c12-4393-872a-4ae3ae669c27
A folder can carry a `Suppressed:` line for one rule and still fail compilation on a different, unsuppressed rule. The parser checked for `Suppressed:` first and skipped the folder, silently discarding the real failure. Validated against run 31079418235 (typespec-next): raw logs contain 71 failure markers; the parser previously reported 63. Reordering the checks recovers all 8, each with genuine inline-suppressible diagnostics (compute, automation, policyinsights, reservations, monitor, applicationinsights). Run 31137822383 is unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e7b3fe66-9c12-4393-872a-4ae3ae669c27
Validated end-to-end against TSV-All run 31079418235 by checking out typespec-next at 7316bb7 (the ref the run actually validated, with its package-lock regenerated to `next` versions as the job does), reproducing the failures, applying suppressions, and re-running `npx tsv`. Three findings, all now reflected in the prompt: - Suppressing can leave a folder failing the Format rule. Reservations and ComputeDisk reached 0 compile errors but still exited 1 until `tsp format` ran. Step 3 now formats each touched folder, and Step 4 tolerates a formatter reflow of an inserted directive. - Walking upward to find the insertion point can overshoot onto the previous declaration. In AppService a directive landed above an `@@OverRide(...)` line, leaving the original diagnostic firing. The placement rule now names the terminators to stop at and requires confirming only decorators, doc comments, and other `#suppress` lines sit between directive and target. - Fixing the harvested set can reveal diagnostics the compiler had masked (AppService models.tsp:8627, absent from the report). Step 4 now treats a new diagnostic as expected rather than as a placement error. Results: automation, Reservations, ComputeDisk and AppService (113 suppressions across 3 files) all reach `npx tsv` exit 0. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e7b3fe66-9c12-4393-872a-4ae3ae669c27
Step 3 now works one folder at a time: apply the report's suppressions, run `tsp format`, run `npx tsv`, and react to what comes back. A repeated diagnostic is a placement mistake to fix; a previously unseen one is a masked diagnostic to suppress if eligible. Capped at three passes per folder. Re-validation is cheap (6s for a small folder, 19s for AppService), so the cap is not about compute — it is about scope. Unbounded iteration turns "suppress the reported set" into "make this folder pass at any cost", which is exactly the pressure that produces over-broad suppressions, and it builds with every pass. Three passes clears the ordinary masked-diagnostic case without getting there. Two invariants hold across passes: only ever suppress a diagnostic the compiler just reported, and apply the Step 2 eligibility rule unchanged to revealed ones. Folders that hit the cap or end on a non-suppressible diagnostic are recorded and must be listed in the PR body under "Needs a second pass", so a reviewer sees a partially suppressed folder without reading the diff rather than discovering it when CI fails. Step 4 collapses to a final reconciliation now that iteration lives in Step 3. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e7b3fe66-9c12-4393-872a-4ae3ae669c27
iscai-msft
marked this pull request as ready for review
August 21, 2026 17:48
iscai-msft
requested review from
Ben Broderick Phillips (benbp) and
Mike Harder (mikeharder)
as code owners
August 21, 2026 17:48
Contributor
|
Hi @iscai-msft. Your PR has had no activity for 14 days and is marked as stale. If no further updates are made within 14 days, this PR will automatically be closed. To keep this PR active, please leave a comment as the PR author or remove the |
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.
Summary
Adds a scheduled and manually triggerable agentic workflow that applies simple, well-understood TypeSpec suppressions automatically.
Rather than recompiling every spec, it harvests the logs of an existing "TypeSpec Validation - All" (TSV-All) run. TSV-All already runs 4×/day on both
mainandtypespec-next, so there is no reason to pay for a second full compile — and harvesting guarantees the agent reacts to exactly what CI observed.TSV-All itself is not modified. This pipeline only reads its logs.
typespec-suppression-harvest.yamlsrc/harvest-tsv-failures.js##[group]Validating <folder>blocks out of the logs and classifies each failure.typespec-auto-suppression.md(+ generated.lock.yml)Why harvest instead of recompile
TSV-All shards specs across 3 ubuntu + 3 windows jobs per ref. An independent scan would have duplicated all of that. Harvesting instead:
tsp compileThe parser was validated against real TSV-All logs (run
32135569949): 196 spec folders parsed, 62 already-suppressed correctly skipped. Synthetic failures injected into that log were classified correctly into eligible / infrastructure / unclassified.One PR per branch
The harvest job runs per ref (
main,typespec-next), thendispatchtriggers the agent once per ref — so all three shards are united into a single PR per branch, not one per shard. Ubuntu only, as requested; a Windows-only failure should be fixed, not auto-suppressed.Schedule
Runs weekly, Mondays 03:00 UTC, shortly after the TSV-All 00:00 scheduled run:
Weekly rather than daily on purpose: suppression churn is slow-moving, and daily runs would open PRs faster than they can be reviewed. For daily, change the cron to
"0 3 * * *".How to invoke it manually
Harvest the latest completed scheduled TSV-All run, both branches:
Harvest a specific TSV-All run — the main reason you'd trigger this by hand, e.g. right after a TSV-All run went red:
Limit to one branch, or skip opening PRs entirely and just produce the failure report:
From the UI: Actions → TypeSpec Auto-Suppression - Harvest → Run workflow.
Re-run only the agent against an existing harvest:
Inputs:
run-idrefsboth(default),default,nextdispatch-agenttrue(default),falsetarget-refmain/typespec-nextharvest-run-idref-keydefault/nextNote: only the scheduled TSV-All trigger covers
typespec-next, so harvestingnextfrom a push-triggered run will correctly report that no matching jobs exist.Two suppression mechanisms
The agent picks the right mechanism per failure rather than forcing everything into
suppressions.yaml.Inline
#suppressin the.tspsource — preferred whenever the compiler reported an exact location, following the pattern in #45367:This is scoped to the single declaration that deviates, instead of blanket-suppressing a whole folder.
A diagnostic qualifies when its rule id comes from a TypeSpec linter package (
@azure-tools/...or@typespec/...). There is deliberately no hardcoded rule allowlist — one would go stale every time a linter adds a rule.The rule id is the gate, not severity.
#suppresstakes a rule id as its argument, so a diagnostic without one cannot be suppressed inline even in principle. Severity turns out to be useless here: TSV compiles with--warn-as-error, so every linter warning prints aserror— across every run harvested so far there were zerowarning-severity diagnostics. A severity-based gate would have suppressed nothing, ever. Rule-less compile errors are still parsed (asrule: null) so that they disqualify their folder. The agent is additionally told to leave any diagnostic that looks like a real API design problem (ARM resource shape, provisioning state, envelope properties, versioning) for a human.Entry in
specification/suppressions.yaml— used only for failures that belong to a folder as a whole and have no single source location:FolderStructure/MustUseV2,Compile/ExtraSwagger,SdkTspConfigValidation.The harvest step parses each
<file>:<line>:<col> - error|warning <rule>diagnostic out of the logs, so the agent gets exact insertion points rather than having to guess.Importantly, a folder is only inline-eligible when every diagnostic in it is inline-suppressible. A folder mixing a suppressible warning with a real compiler error goes to a human, since suppressing the warning would leave it failing anyway. This is covered by a test case.
Safety constraints
specification/suppressions.yamland.tspfiles underspecification/may be edited, and only to add a suppression.tspconfig.yaml,package.json,eng/, and workflows are off-limits.#suppressdirective, asuppressions.yamlentry, or the result oftsp formaton a folder that was edited. The agent must read its own diff line by line and revert anything that changes a model, operation, property, decorator, or doc comment.ETIMEDOUT,npm ERR!, runner shutdown, and similar) is detected and reported as "needs a rerun", never suppressed.package.jsonand lockfile edits.tsp format, re-runnpx tsv, and react to the result. TSV has its own format check, so an inserted directive that does not match the formatter's layout would otherwise trade a suppression failure for a formatting one.get-suppressionsand re-runnpx tsvon every folder it touched before opening a PR, and emits a no-op rather than an empty or broken PR.Validation gate
Generated PRs must still pass TypeSpec Validation - All — all three shards on both Ubuntu and Windows. The prompt explicitly reminds the agent that one green shard proves nothing about the others.
Validation against real runs
The parser was developed against hand-written logs, which turned out to prove very little — every real run since has exposed a distinct bug class. Each was found by cross-checking parser output against the raw logs and fixed:
typespec-next--warn-as-errorservice.yamltypespec-nexttypespec-nextSuppressed:lineThe highest-impact bug was ANSI colour codes: the compiler colourises diagnostics, so
file:line:colis never contiguous plain text and the regex matched zero real diagnostics while passing every synthetic test. The most recent one dropped 8 real failures because a folder can carry a suppression for one rule and still fail on another, and the parser checkedSuppressed:before checking for failure.End-to-end proof
Parsing correctly is only half the claim — the suppressions also have to actually fix the folders. For run 31079418235 this was verified by checking out
typespec-nextat7316bb7(the commit that run validated) with itspackage-lock.jsonregenerated tonextversions exactly as the job does, since these rules only fire against prerelease compiler versions:npx tsvafterautomation/Automation.Management.../Reservations/Reservations.../Compute/ComputeDisk.../Microsoft.Web/AppServiceEvery folder reproduced its reported failure at the exact reported line, and reached
npx tsvexit 0 after suppressing. Three behaviours surfaced that the prompt now handles explicitly:tsp formatstep.@@override(...)belonging to the previous declaration, leaving the original diagnostic firing. The placement rule now names the terminators to stop at.models.tsp:8627, absent from the report because the compiler stopped before reaching it. Suppressing it too brought the folder to exit 0. This is why the agent iterates rather than making a single pass: it distinguishes a repeated diagnostic (a placement mistake to fix) from a new one (a masked diagnostic to suppress if eligible).The 5 rejected folders in that run are all
specification/cognitiveservices/*failingtsp format— a reformat commit, which no#suppresscan fix, so refusing them is correct.Note that harvest reads only the ubuntu shards. A Windows-only failure should be fixed rather than hidden, so it is deliberately never auto-suppressed.
Notes for reviewers
.lock.ymlis generated bygh aw compile; edit the.mdand recompile.Protected Filescheck fails because this adds files under.github/*— the expected known failure for workflow changes.typespec-nextdispatch will fail with "workflow not found" until this also lands ontypespec-next, sincegh workflow run --refrequires the lock file to exist on that branch. Noted in a comment in the harvest workflow.