Refresh Azure CLI options from current output - #4546
Conversation
thomhurst
added
dependencies
|
@claude Please review this generated Azure CLI synchronization. |
|
Too many files changed for review (3000 files, 100 file limit). |
|
Caution CodeRabbit couldn't post its review summary. Error details |
GitHub Actions
Inactive
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: Refresh Azure CLI options from current output (#4546)
Scope note: GitHub's diff view truncates at 300 files; this PR touches ~6,260 files. I reconstructed the full diff locally via git diff be8fc008^ be8fc008 (single commit, chore(azure): refresh generated options). It consists of ~6,260 auto-generated *.Generated.cs files rebuilt from current az CLI help output, one generated coverage JSON, and hand-written updates to test/ModularPipelines.Azure.UnitTests/AzureCommandTests.cs for the new required positional constructors and renamed ACR properties (ResourceGroupValue→ResourceGroup, TargetValue→Target, plus removal of the old [Obsolete] bool shims).
Verified clean:
- No duplicate property/constructor-parameter names introduced (no compile breakage from the new required-positional-parameter constructors).
- No other source in the repo still references the old ACR property names or constructs the newly-required-constructor types via object initializers that would now fail to compile.
AzureCommandTests.csis internally consistent with the regenerated options it exercises.- The regenerated coverage JSON is well-formed.
- This aligns with CLAUDE.md's guidance that "current tool output is the sole source of truth for generated options" — no hand-preserved shims were kept, which is correct per project convention.
Finding (BLOCKING): the refresh is inconsistent within the very same generated records — some value-taking options were correctly promoted from CliFlag bool? to CliOption, while sibling options with identically-worded doc comments were left as broken boolean flags.
-
src/ModularPipelines.Azure/Options/AzAcrBuildOptions.Generated.cs(lines ~34, ~91) —BuildArgandSecretBuildArgremain[CliFlag("--build-arg")] public bool? BuildArgeven though their doc comments explicitly describe aname[=value]argument ("Build argument in '--build-arg name[=value]' format..."). In this exact same file/commit,AuthMode,Image, andSourceAcrAuthIdwere correctly reclassified fromCliFlag/bool?toCliOption/string?for the same reason. SettingBuildArg = trueemits the bare--build-argswitch with no value — a silently brokenaz acr buildinvocation. -
src/ModularPipelines.Azure/Options/AzSynapseSparkJobSubmitOptions.Generated.cs(lines ~55-65) —MainClassName,Configuration, andReferenceFilesremainCliFlag/bool?despite doc comments describing value-carrying arguments (a class identifier, a job configuration, a file list). This same commit correctly fixedArchives,Language,PythonFiles, andTagsin the identical record fromCliFlag/bool?toCliOption/string?orIEnumerable<string>?.
Why this matters / suggested approach: Per CLAUDE.md, the fix belongs in the scraper/generator (tools/ModularPipelines.OptionsGenerator/), not in hand-edits to the generated files. The generator's flag-vs-option classification heuristic appears to key off some signal (likely presence of an explicit value placeholder or specific phrasing) that misses arguments described via name[=value]-style syntax or multi-word value descriptions — worth checking whether the classifier is keying off the CLI help's argument-metavar pattern and failing for these non-standard forms. Since this PR's diff shows the generator getting some properties in a record right and others in the same record wrong, this looks like a narrow gap in that heuristic rather than a systemic breakage, but it should be fixed in the generator so it doesn't need to be manually re-caught on every future refresh. A heuristic scan for the same doc-text-vs-attribute mismatch pattern across the diff surfaced this as widespread (order of 100+ occurrences), so an isolated manual patch of just these two files would not be sufficient — the generator itself needs the fix, then a re-run of the refresh.
|
Addressed the exact-head Claude blocker at 01f8860. Extended the Azure scraper—not generated files—to recognize description-only value families, including build/secret arguments, paths, platforms, timeouts, configurations, class identifiers, commands, and file collections; added numeric/list cardinality detection and exact ACR/Synapse regression tests. Validation: Azure scraper tests 26/26; full generator tests 1,265/1,265. A full local Azure regeneration reached command 413 but hit the repository-mandated 2-GB process-tree guard, so I did not raise/retry it. Dispatched the 60-minute CI generator on this exact branch to produce the complete regenerated artifact set; auto-merge is disabled. |
|
Caution CodeRabbit couldn't post its review summary. Error details |
GitHub Actions
Failure
There was a problem hiding this comment.
Review: Refresh Azure CLI options from current output (#4546)
Re-review scope: since the last Claude review (head be8fc008), this branch adds exactly one commit, 01f886096b ("fix(azure): detect description-only values"), touching only AzCliScraper.cs and its test file — no *.Generated.cs files changed. I diffed be8fc008..01f886096b directly to confirm this.
The prior BLOCKING finding is not actually resolved yet.
The PR comment states the scraper was extended to "recognize description-only value families" and that CI was dispatched to "produce the complete regenerated artifact set." That scraper fix is real, narrowly scoped, and well covered by two new tests (Nonstandard_Description_Only_Values_Are_Not_Flags, Synapse_Description_Only_Values_Are_Not_Flags) — this is the right place to fix it per CLAUDE.md ("update the scraper or generator", not hand-edit generated files). However, the regenerated output implied by those tests has not actually landed on this branch. At the current head (01f886096b8df9b4ae49d6283a91f23c1d5f8fe0), the exact same generated files from the original finding are unchanged and still broken:
AzAcrBuildOptions.Generated.cs:BuildArg,SecretBuildArg,File,Platform, andTimeoutare all still[CliFlag] public bool?despite value-carrying descriptions (the new scraper test literally asserts these should now beIEnumerable<string>?/string?/int?):
AzSynapseSparkJobSubmitOptions.Generated.cs:Configuration,MainClassName, andReferenceFilesare likewise still[CliFlag] public bool?:
Setting BuildArg = true (or Configuration = true, etc.) still emits a bare switch with no value — the same silently-broken CLI invocation as before. Also worth noting: File, which shares the same bug pattern, wasn't called out in the original finding but is covered by the new scraper test, and it's still broken in the committed output too — reinforcing that this is a mechanical "generator changed, artifacts didn't" gap rather than something that needs more heuristic work.
Why this matters / suggested approach: the fix isn't done until the regenerated *.Generated.cs files are actually committed to this branch — a scraper change with no corresponding regeneration leaves the shipped bug in place regardless of how correct the scraper logic now is. Since local regeneration hits the repo's 2GB agent memory guard, this has to come from the dispatched CI run's artifact being committed back onto the branch before this can be considered resolved; please push that regenerated output (or confirm the CI run failed/is still pending) rather than merging on the strength of the scraper-only fix.
As a structural improvement worth considering separately from this PR: a lightweight CI check that fails when tools/ModularPipelines.OptionsGenerator/** changes without a corresponding diff in the generated packages (or that just runs the generator and diffs its output against what's committed) would catch this "scraper fixed, artifacts stale" gap automatically, instead of relying on a reviewer to manually diff two heads of a 6,000+ file PR to notice regeneration didn't happen.
|
Ubuntu failure is outside this PR's Azure-generated diff: |
|
Caution CodeRabbit couldn't post its review summary. Error details |
There was a problem hiding this comment.
Review
This PR adds new scraper detection logic (AzCliScraper.cs) for description-only Azure CLI options that don't declare an explicit value placeholder, so they're correctly typed instead of misclassified as boolean flags. The scraper-logic tests (AzCliScraperTests.cs) are well-targeted and pass.
Blocking issue: the fix isn't actually applied to the shipped generated code.
The PR's commits land in this order:
be8fc008—chore(azure): refresh generated options(bulk regeneration, 6263 files)01f88609—fix(azure): detect description-only values(the actual scraper regex fix, touches onlyAzCliScraper.cs+ its tests)f99a80db—ci(options): retain Azure generation patch(CI workflow only)
The regeneration happened before the scraper fix, and nothing in commit 2 or 3 re-runs generation. So the checked-in .Generated.cs files still contain the exact bug this PR claims to fix.
Concrete, verifiable contradiction: the new test Nonstandard_Description_Only_Values_Are_Not_Flags asserts that for az acr build, --build-arg, --secret-build-arg should be IEnumerable<string>?, --platform/--file should be string?, and --timeout should be int?. But src/ModularPipelines.Azure/Options/AzAcrBuildOptions.Generated.cs (shipped in this same PR, via commit 1) still has all four of BuildArg, SecretBuildArg, Platform, Timeout as [CliFlag] bool? presence-only flags:
[CliFlag("--build-arg")]
public bool? BuildArg { get; set; }
...
[CliFlag("--platform")]
public bool? Platform { get; set; }
...
[CliFlag("--secret-build-arg")]
public bool? SecretBuildArg { get; set; }
...
[CliFlag("--timeout")]
public bool? Timeout { get; set; }Notably, other properties in that same file (AuthMode, Image, SourceAcrAuthId, ResourceGroup, Target) were correctly fixed by commit 1 (from an earlier scraper improvement) — so this isn't a case where the file was untouched; it's specifically the new description-only detection pattern from commit 2 that never got applied. Same mismatch pattern applies to AzSynapseSparkJobSubmitOptions.Generated.cs vs. the new Synapse_Description_Only_Values_Are_Not_Flags test (Configuration, MainClassName, ReferenceFiles).
Impact: A consumer calling new AzAcrBuildOptions(...) { Platform = true } cannot actually pass a platform value (e.g. "linux/amd64") to az acr build --platform — the property is still boolean presence-only. The PR's own tests prove the shipped API is wrong; CI is green only because the scraper-level unit tests check the scraper's output in isolation, not that the checked-in generated corpus matches current scraper output.
Suggested fix: re-run the Azure options generation after the scraper fix (i.e., squash/reorder so generation happens after 01f88609, or add a follow-up regeneration commit) so the .Generated.cs files reflect the new detection logic. Given AzDescriptionOnlyValuePattern/AzNumericValueDescriptionPattern/AzCollectionValueDescriptionPattern look like broadly-applicable patterns, it's worth doing a full diff of the regenerated Azure corpus against what's currently checked in — there are likely more than the two files above affected (a substantial number of other [CliFlag] properties elsewhere in the Azure package could match these same patterns).
As a process improvement: consider adding a CI check that fails if the checked-in generated corpus differs from a fresh run of the current generator/scraper — this class of "logic fixed, output not regenerated" bug would be easy to detect automatically before merge rather than requiring a manual cross-reference like this review did.
GitHub Actions
Inactive
|
Azure generation completed 6,422 files at run 33752281684, then PublicAPI snapshot collection failed before the recovery artifact step. Moved temporary patch staging/upload ahead of that sync and merged current main/provenance changes at exact head 5ef57b6. Replacement authoritative run: https://github.com/thomhurst/ModularPipelines/actions/runs/33766998203 |
|
Caution CodeRabbit couldn't post its review summary. Error details |
There was a problem hiding this comment.
Review: Refresh Azure CLI options from current output (#4546)
Re-review scope: this is the fourth Claude review round on this PR. Three prior reviews (heads be8fc008, 01f886096b, f99a80dba) all landed the same BLOCKING verdict for the same root cause. I re-checked whether it's resolved at the current head, 5ef57b68, and it is not — plus a targeted spot-check across ~20 other generated files surfaced two more instances of the same failure class, including one outright regression.
The previously-flagged blocker is still unresolved
Across three rounds, this PR has claimed the scraper fix (AzCliScraper.cs, commit 01f886096) for "description-only value" options was regenerated into the shipped corpus. It still hasn't been. At the current head:
AzAcrBuildOptions.Generated.cs—BuildArg,File,Platform,SecretBuildArg, andTimeoutare all still[CliFlag] public bool?, even though their own doc comments describe value-carrying arguments ("Build argument in '--build-arg name[=value]' format...","The timeout in seconds.") and the PR's own new scraper test (Nonstandard_Description_Only_Values_Are_Not_Flags) asserts these should beIEnumerable<string>?/string?/int?.AzSynapseSparkJobSubmitOptions.Generated.cs—Configuration,MainClassName,ReferenceFilesare likewise still[CliFlag] bool?, contradicted by the PR's ownSynapse_Description_Only_Values_Are_Not_Flagstest.
Setting e.g. BuildArg = true or Configuration = true still emits a bare switch with no value — the same silently-broken CLI invocation flagged three reviews ago. The comment thread describes several CI regeneration attempts (a run reaching 6,422 files, then failing "before the recovery artifact step") that were apparently never actually applied back onto this branch — the merge to the current head brought in main, not the regenerated Azure output.
Two more instances of the same failure class, one a regression
Spot-checking beyond the two files above (Keyvault, Network, Cosmosdb, Functionapp, Monitor, Storage, Eventhubs):
AzEventhubsNamespaceAuthorizationRuleShowOptions.Generated.cs(newly added) —AuthorizationRuleName("The authorization rule name.") andNamespaceName("The Namespace name.") are generated as[CliFlag] bool?instead of[CliOption] string?, and neither was promoted to the constructor. As written, this record cannot specify which namespace or rule to target at all — worse than "drops a value," this command is unusable. The siblingAzEventhubsEventhubConsumerGroupDeleteOptions.Generated.cshas the identical pattern forConsumerGroupName/EventhubName/NamespaceName, andAzMonitorAccountCreateOptions.Generated.cs'sLocationshows the same misclassification — this looks systemic for at least theeventhubsextension command family, not isolated to the two files already reported.AzAcrTaskCreateOptions.Generated.cs— this is an actual regression, not just an unresolved gap: onmain,ImageisImageValues : IEnumerable<string>?(correctly multi-valued, with a deprecated bool shim for back-compat). On this branch it was narrowed toImage : string?— single-valued — while the doc comment is unchanged and still reads "Multiple tags are supported by passing -t multiple times." This refresh makes an already-correct property worse.
Why this matters / suggested approach
Per CLAUDE.md, fixes belong in the scraper/generator, and "current tool output is the sole source of truth" — so a breaking regen is expected and fine on its own. The problem here isn't the scraper logic (which is narrowly scoped and well-tested in isolation); it's that the shipped .Generated.cs corpus in this PR doesn't reflect that scraper's own output, in both directions (some fixes never applied, and at least one prior-correct property got worse). Given three review rounds have now independently hand-verified the same "generator changed, artifacts stale" gap on different file pairs, this isn't something a reviewer should need to keep manually spot-checking — as suggested in the prior review, a CI gate that runs the generator and fails if its output differs from what's committed (or a required "generator diff is empty" check gating merge) would catch this mechanically before every future refresh, rather than relying on someone diffing two heads of a 6,000+ file PR by hand each time.
Minor, non-blocking
.github/workflows/generate-cli-options.yml still carries the "Stage generated Azure output for recovery" / "Preserve generated Azure patch" / "Upload generated Azure patch" steps added purely to recover from this PR's own troubled regeneration attempts, hardcoded to matrix.tool == 'az' only (no other of the ~40 tools in this matrix has an equivalent). The PR's own comment thread says "the temporary workflow instrumentation will be reverted after applying the exact generated output" — worth reverting now that generation is expected to land, or generalizing into the matrix if it's meant to be permanent, rather than leaving a one-off special case for az.
Looked clean
Required-parameter promotion into record constructors (e.g. AzAcrCreateOptions), removal of obsolete bool shims, the AzureCommandTests.cs/AzureCommandTests updates for renamed/positional ACR properties, AzExtensions.Generated.cs's service registrations (renames like AzComputefleet→AzComputeFleet are consistent, additions match new commands), and Az.CommandCoverage.json (valid, commandCount/commands.length both 5131, matches the PR's claimed figures).
GitHub Actions
Failure
|
#4609 (empty-baseline handling in the shared sync script) has merged, so the Azure regeneration that previously died in |
|
Cancelled the just-dispatched |
5ef57b6 to
e399168
Compare
|
Caution CodeRabbit couldn't post its review summary. Error details |
|
Pushed e399168 after rebasing onto current main. This addresses the remaining scraper gaps behind the latest review: resource-name/ID descriptions, locations, and repeated ACR image tags. Live Azure help confirms the resource-ID alternatives are optional value options, so they remain optional rather than becoming mandatory constructor inputs. Existing description-only tests cover the BuildArg/SecretBuildArg and Synapse findings. All 1,360 generator tests pass; Release generator build and scoped formatting pass. The temporary Azure recovery workflow changes are removed. Full regeneration is running at https://github.com/thomhurst/ModularPipelines/actions/runs/34765636116 from this exact branch, with auto-merge and shrinkage approval disabled. The generated-output review blocker remains open until that output lands and is validated. Full local Azure validation previously hit the repository's 2 GB guard and was not retried. |
Review summaryScraper/generator changes (the actual hand-written logic in this PR) look correct and well-tested.
Blocking issue: the committed generated corpus in this PR does not yet reflect the scraper fixes it contains, and the PR is self-described as not ready to merge. I verified this concretely rather than just taking the PR description at its word: the new This matches what the author has already stated across the PR thread: full regeneration (run Suggestion: rather than relying on comment-thread discipline to keep the generated output and scraper in sync across a multi-day PR, consider making the No other architectural concerns in the reviewable (non-generated) code. |
|
Pushed 28799c7 with recovered authoritative Azure output and the next two fixes. Run 34805875507 passed API synchronization (591.3s snapshot, 122.9s removals), reconstructed 150,041 APIs, and passed exact staging. Its final build failed only at AzureCommandTests.cs:60 because Executors is now int. The handwritten test now supplies 1 and retains its command-rendering expectation. Recovered all 6,422 generated files byte-for-byte from that run, including 62 exact filename corrections. Reconstructed the baselines with Merge-PublicApiBaselineSnapshot.ps1 from the run's original baselines, current API snapshot, and confirmed removals. The originals match this branch after line-ending normalization. Verified 4,987 staged generated/API paths against the CI manifest and baseline allowlist. API impact: 88,017 additions, 16,183 removals/changes, and 9,067 same-name signature changes. Generated files and baselines were not hand-edited. Manual review also found that actual help says "Multiples are supported by passing '--build-arg name[=value]' multiple times", while the earlier fixture omitted quotes and the value example. Shared repeat detection now recognizes matching single/double/backtick quotes; all three added cases failed before the fix. The old unquoted case remains covered, and repeated options remain ungrouped. BuildArg and SecretBuildArg are still scalar in the recovered d9b6 output, so fresh generation remains a merge prerequisite. All 1,451 generator tests pass after the fix and formatting cleanup; Release generator build has zero warnings/errors. Scoped severity-info generator verification, scoped Azure test formatting, all 61 API baseline checks, artifact hash/name checks, and diff checks pass. Local Azure compilation/test remains deferred after the earlier 2-GB limit, without a retry or increased limit. Fresh authoritative Azure generation from this exact head: https://github.com/thomhurst/ModularPipelines/actions/runs/34810397663. Auto-merge and coverage-shrinkage approval are disabled. This run must update the repeatable properties, provenance, and generated documentation before this PR can merge. |
Review summaryScope note: this PR regenerates ~6,400 Azure CLI option files. Per this repo's Blocking — unchanged from the prior review (originally flagged at I re-verified this at the current head ( [CliOption("--build-arg")]
public string? BuildArg { get; set; }
...
[SecretValue]
[CliOption("--secret-build-arg")]
public string? SecretBuildArg { get; set; }Their doc comments (lines 34 and 88) contain the exact real Azure CLI text: This is the same defect the prior review blocked on, just re-verified against the current head rather than assumed carried-over. Architectural point (repeated from the prior review, since it's still what's letting this recur): No other concerns in the reviewable (non-generated) code — the scraper heuristics, the two-pass |
Co-authored-by: thomhurst <9139608+thomhurst@users.noreply.github.com>
Review summaryThe previously-blocking issue is resolved. Every prior review round on this PR (7 rounds, heads I re-verified this directly at the current head (
The two commits since the last review ( Hand-written logic (unchanged from prior rounds, still solid): the scraper heuristics in Non-blocking, for awareness (low confidence, no confirmed real-world trigger across the 6,422 regenerated commands):
Architectural point worth reconsidering (repeated from prior rounds, since it is what let the corpus/scraper mismatch recur across five-plus push cycles): No other concerns in the reviewable, non-generated code. This PR is in a mergeable state as far as the diff content goes. |
27b7166 to
6c637dd
Compare
|
Pushed 6c637dd after manual review of the regenerated Azure corpus found two additional defects.
Validation: all 1,459 generator tests pass; generator Release solution build has zero warnings/errors; scoped severity-info verification passes. Fixture normalization only removes trailing blank lines. No generated option or API baseline was hand-edited. The existing local Azure 2-GB compilation limit remains deferred to CI. Authoritative regeneration for this exact source: https://github.com/thomhurst/ModularPipelines/actions/runs/34817378641. Generator fingerprint: 99abaa76e0a93723e9a40cff3069b062b5876c8fc306e94e283120506110a1bc. Auto-merge and coverage-shrinkage approval are disabled. The parent must remain unmerged until the new value-bearing options and corrected documentation are incorporated and reviewed. |
Review summaryBlocking — the committed generated corpus still doesn't reflect this PR's own final scraper fixes. This is the same defect class every prior round on this PR blocked on (7+ rounds through That commit adds
The second fix in the same commit (
Both the PR description and the commit message acknowledge this directly — "This PR remains unmerged until the new output is incorporated and reviewed" — so this isn't a surprise, but it does mean the PR is not yet in a mergeable state: the 6,422-file Azure corpus and the docs need one more authoritative regeneration pass before these two fixes actually take effect anywhere outside the generator's own test suite. Architectural point (repeated from every prior round, since it's what lets this recur every time): Hand-written logic (unchanged from prior rounds, still solid): the scraper heuristics in |
|
Pushed aecd27d with both #4955 defects fixed in source. Pwd identifier segments now receive secret metadata while paths, names, IDs, and unrelated segments remain visible. Azure list inference uses the input definition instead of later references to reserved/allowed-value lists, keeping VM/VMSS AdminUsername scalar. All 1,479 generator tests pass; eleven new cases fail before the fixes. Full captured Azure CLI 2.84.0 VM, VMSS, and SQL VM help reproduces the affected descriptions; only trailing blank lines were trimmed. Seven new Azure integration regressions cover the generated password annotations and actual scalar username rendering. Their execution remains deferred until generated output is incorporated, alongside the previously recorded Azure compilation 2-GB limit. Release generator solution build has zero warnings/errors. Scoped information-level generator and Azure test formatting verification pass. Full authoritative generation from this exact commit: https://github.com/thomhurst/ModularPipelines/actions/runs/34823681275, with auto-merge and coverage-shrinkage approval disabled. Fingerprint: 44ca29f5f7a70bca3524d42172fd82be9545fbcf615b95d441cd8609a513bad8. The documentation-order concern on #4955 is already fixed in this parent: MarkdownDocumentationGenerator uses tool.GlobalOptionsBeforeSubcommands and both before/after cases are tested. Keeping the generated child targeted at this source branch preserves that fix when #4546 merges into main; retargeting the child alone would omit its generator prerequisite. Current #4955 remains blocked pending final-source regeneration. |
GitHub Actions
Failure
|
Review This PR regenerates Azure CLI options from a fresh, larger CLI corpus (about 6,422 commands) and includes real fixes to the OptionsGenerator scraping/documentation logic. Per repo guidance, generated Blocking 1.
This contradicts the feature described in this PR ("Generated global-option documentation follows each tool declared argument order") and the generator own logic/tests. It suggests 2. The prior check was a plain substring test, Any option whose help text places "list of" mid-sentence -- for example a second sentence such as "Resource tags. A list of key=value pairs." -- will no longer be recognized as a list, silently narrowing it from Non-blocking, worth a look 3.
Not reviewed in depth The |
|
Addressed comment 5661442109 in 9eaa3b9.
Release generator build has zero warnings/errors; scoped information-level formatting and diff checks pass. Existing 2-GB validation limits remain unchanged. Current Linux Azure integration failures are the seven expected old-output contract failures; final-source regeneration remains required. Fingerprint: a6c1ce44cc70d83e8347ded58b42d94a37c0ba27ca672676467daf26a2fc6bb6. |
Review: Refresh Azure CLI options from current output (#4546)Re-review scope: this PR has been through 9+ prior rounds, all blocked on the same defect class — the committed ~6,400-file generated Azure corpus not reflecting this PR's own scraper fixes. The latest push (head Still blocking — the generated corpus still doesn't reflect this PR's own fixes (recurring for the 10th round)1. 2. The "list of after an introductory sentence" fix is not reflected in 3. The VM/VMSS admin-username and SQL VM password fixes (the headline fixes of the latest commit, Taken together, none of the three concrete scenarios called out across the last two review rounds (list-after-introduction, VM/VMSS username, SQL VM passwords) actually took effect in the committed output at this head, despite the PR description and commit messages asserting the "final-source" regeneration incorporated them. This is the same root cause as every prior round: whatever regeneration pipeline run produced the currently-committed Hand-written logic (unchanged from prior rounds — still solid)
Architectural point (repeated from every prior round)
Not reviewed in depth: the remaining ~6,400 generated |
GitHub Actions
Failure
|
Pushed Eight regressions failed before the change. All 70 Azure scraper tests and the full 1,494-test generator suite pass. Release generator build has zero warnings/errors; scoped severity-info generator and Azure test formatting/verification passes. Five new Azure rendering contracts cover all affected properties; their execution remains deferred to CI under the prior Azure compilation 2 GB limit. Fresh full Azure regeneration is https://github.com/thomhurst/ModularPipelines/actions/runs/34832142558 from this exact source, with auto-merge and coverage-shrinkage approval disabled. #4955 remains blocked until replacement generated options/API/provenance incorporate these corrections. Existing parent output findings also remain held until generation and integration validation complete. |
GitHub Actions
Failure
Review: Refresh Azure CLI options from current output (#4546)Re-review scope: this PR has now been through more than a dozen review rounds, essentially all blocked on the same defect class — the committed ~6,400-file generated Azure corpus not reflecting this PR's own scraper fixes. Since the last review (head Still blocking — the generated corpus still doesn't reflect this PR's own fixes1. The three previously-flagged stale files are unchanged.
2. New evidence from this round's own commit, in a single file. Given the two Hand-written logic (unchanged from the last-confirmed-good round)
Architectural point (repeated from every prior round)
Not reviewed in depth: the remaining ~6,400 generated |
Co-authored-by: thomhurst <9139608+thomhurst@users.noreply.github.com>
Review (head
|
|
Regarding review 5663839708: the reported enum-import removal is not part of this PR's changes. The PR's three-dot diff changes global-option ordering and equivalent formatting in MarkdownDocumentationGenerator; it does not remove the enum import or the enum test scenario. Those additions landed separately on main in 45b3ba1 (#4713), after this branch diverged. I computed the conflict-free merge of current main 4f9c42c and this PR's exact head 14b65bb. The resulting tree 22f3652c406c49bf8525ddd1e85021928c7026cd retains both:
The proposed fix is therefore already present in the merged result; no additional source change is needed. The latest review confirms the previous Azure-output blockers are resolved. All 17 current checks are terminal and successful/neutral/expected-skipped, including Linux pipeline, Windows/macOS builds, and documentation deployment. There are no unresolved review threads. |
1 participant
Refreshes Azure options from Azure CLI 2.90.0 help, covering 5,131 commands. The scraper preserves description-only values, credentials, datetime values, optional names and IDs, repeated image tags, boolean actions, and delimited values. Password identifier segments receive secret metadata while paths, names, and IDs remain visible. VM/VMSS usernames stay scalar; actual NetApp, SignalR, and authentication-audience list definitions produce grouped values, including the CLI's documented
Space-separeted listspelling.Generated global-option documentation follows each tool's argument order. Public API synchronization discovers additions before checking removals, restores baselines on failure, and handles case-only filename changes. Compiler diagnostics track PID plus process start time and reset sampling delay for replacement processes.
Final-source generation run 34832142558 produced the code, documentation, API baselines, command coverage, and provenance incorporated through #4955. Its source fingerprint is fa293cbb7c3611630fc4377b1c0a68bcb5d200e2a13b2c626592ac72cad5ac7d, matching the final Azure generator source. No generated implementation was hand-edited.
Validation:
Refs #4331