fix(generator): preserve gcloud group dispatch coverage

[thomhurst]

Problem and fix

Gcloud generation from main failed coverage after all leaf-command parser fixes had landed: SDK 585.0.0 produced 7,594 commands versus the 7,927-command baseline, with 21 additions and 354 removed dispatch paths. The shared traversal did not recognize GROUP selectors, and its child-discovery condition also missed two command groups that currently expose no children.

Let the shared synopsis normalizer accept an adapter-specific set of dispatch names. Gcloud removes undocumented GROUP and COMMAND selectors before argument parsing, while preserving explicitly documented positional arguments with those names. Other adapters retain their existing placeholder policy. Coverage guards and operand validation stay unchanged.

The regression corpus contains all 355 requested help pages from the failed coverage diagnostics, including the root page and two empty command groups captured locally using the same SDK. Tests verify each non-root dispatch page, traversal of the captured tree, and preservation of real positional operands.

Validation

• Before the source fix: all 354 dispatch-page regressions and the captured-tree regression failed; three operand-preservation tests passed.
• After the fix: all 852 dispatch, gcloud resource/list/group, shared synopsis/traversal, and AWS scraper tests pass.
• Scoped generator formatting passed; the Release solution build passed with zero warnings and zero errors.

Generation and merge order

Fixes the source portion of #5166. Related generation issue: #5146; runtime PRs: #4863, #4869, #4956.

Failure evidence: https://github.com/thomhurst/ModularPipelines/actions/runs/35058181945/job/104672916300. Captured diagnostics artifact: 10432345934.

AWS generation in that run is still active. Hold this source PR's merge until its healthy AWS replacement is reviewed, green, and merged, so the generator fingerprint stays valid. Then regenerate gcloud from the latest main and verify coverage before updating the runtime PRs and closing #5166. No generated files are patched here.

Summary by CodeRabbit

• Bug Fixes

  • Improved parsing of gcloud command groups and dispatch paths.
  • Preserved documented group and command placeholders when command groups have no available child commands.
  • Correctly recognizes positional GROUP and COMMAND arguments and their required status.
  • Maintained nested command groups, including groups with empty child lists.

• Tests

  • Added coverage using captured gcloud SDK help output to verify dispatch parsing and placeholder behavior.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-16T06:14:17.458093Z	de5c0ff	Manual request

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 6e18302e-7f9b-4f29-8eae-6b537a180357

📥 Commits

Reviewing files that changed from the base of the PR and between 6934e1f and de5c0ff.

📒 Files selected for processing (1)

• tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/GcloudCliScraper.cs

🚧 Files skipped from review as they are similar to previous changes (1)

• tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/GcloudCliScraper.cs

Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review.

---

📝 Walkthrough

Walkthrough

The parser now supports caller-defined placeholder names. GcloudCliScraper preserves declared GROUP and COMMAND operands while filtering dispatch selectors. Fixture-based tests cover dispatch paths, nested groups, synthesized child listings, and required positional arguments.

Changes

Gcloud dispatch placeholder handling

Layer / File(s)	Summary
Configurable placeholder filtering tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/UsageSynopsisParser.cs	RemoveCommandGroupPlaceholders accepts an optional placeholder-name set and passes it through recursive requiredness processing.
Gcloud scraping and fixture validation tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/GcloudCliScraper.cs, tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/Cli/GcloudDispatchTests.cs	GcloudCliScraper preserves declared Group and Command operands while filtering dispatch selectors. Tests cover captured dispatch paths, nested groups, synthesized child listings, and required positional arguments.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant GcloudDispatchTests
  participant DispatchExecutor
  participant GcloudCliScraper
  participant UsageSynopsisParser
  GcloudDispatchTests->>GcloudCliScraper: Scrape captured gcloud help
  GcloudCliScraper->>DispatchExecutor: Request command help
  DispatchExecutor-->>GcloudCliScraper: Return captured or synthesized help
  GcloudCliScraper->>UsageSynopsisParser: Filter configured dispatch placeholders
  UsageSynopsisParser-->>GcloudCliScraper: Return filtered synopsis arguments
  GcloudCliScraper-->>GcloudDispatchTests: Return command definitions

Loading

Merge Risk: ⚪ Minimal · up to de5c0

The gcloud scraper preserves documented positional operands while removing dispatch selectors, including captured groups without child listings. No actionable merge risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 7.69% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: preserving gcloud group dispatch coverage in the generator.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch issue-5166-gcloud-group-dispatch

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit reads each line,
The patch grows clear beneath the moon,
Small changes hop in place,
Tests guard the garden path,
Reviews bloom before the dawn.}

_{Comment @coderabbitai help to get the list of available commands.}

[thomhurst]

The docstring-coverage warning in comment 5692661463 covers named regression tests and internal/private parser helpers. The tests follow the surrounding TUnit documentation convention; comments record the captured-help provenance, synthetic ancestor behavior, and why gcloud handles empty dispatch groups before child discovery. No public library API was added, and the source build has no documentation warnings.

[github-actions]

Review

This closes the gcloud group-dispatch gap cleanly: PreserveCommandGroupPlaceholders now lets GcloudCliScraper opt out of the base class's blanket placeholder stripping, and the scraper does its own, more precise stripping that only removes a bare `GROUP`/`COMMAND` synopsis token when it isn't backed by a documented positional argument. The new GcloudDispatchTests (captured-fixture regression coverage, nested/empty-group preservation, and the Documented_Positional_Arguments_Are_Not_Dispatch_Selectors case) directly exercise the scenario this is meant to fix and the one it's meant to avoid regressing.

I initially flagged the new protected override bool PreserveCommandGroupPlaceholders as missing a PublicAPI.Unshipped.txt entry (the sibling VaultCliScraper override is tracked there). After checking Directory.Build.props, that's a false alarm: the Microsoft.CodeAnalysis.PublicApiAnalyzers package reference and the RS0016/RS0017 WarningsAsErrors gate are both scoped to '$(MSBuildProjectName)' == 'ModularPipelines' only, so ModularPipelines.OptionsGenerator never loads that analyzer, and Assert-PublicApiBaselines.ps1 only checks the baseline files for internal consistency (duplicates/orphaned markers), not completeness against the compiled API. This matches the PR author's own comment that no tracked public API was added — confirmed rather than just taken on faith.

Two minor, non-blocking observations if you want to tighten this further:

• ParseUsageSynopsis now strips dispatch placeholders unconditionally based on whether they're documented, whereas the base class's normal path (ParseAndWriteCommandAsync) only ever stripped when the command actually had traversable subcommands. Because PreserveCommandGroupPlaceholders = true disables that base-level gate for gcloud entirely, a leaf command that happens to emit an undocumented bare GROUP/COMMAND token in its SYNOPSIS (a doc/parsing inconsistency the section-parser doesn't catch) would now get it stripped, where previously a leaf command was never touched regardless of documentation. Given gcloud's argparse output consistently documents real positional operands, this is a low-probability edge case, but it's a real narrowing of the existing safety net worth keeping in mind if a future SDK version misbehaves.
• groups.SelectMany(group => group.FlattenArguments()) in GcloudCliScraper.ParseUsageSynopsis (lines 164 and 190) walks the same argument tree twice for two disjoint filters (non-positional vs. positional). Materializing the flattened list once and filtering it twice would avoid the redundant recursive walk across the full gcloud command tree scrape — a minor efficiency win, not a correctness issue.

Neither point blocks this PR; the added tests give good confidence in the primary fix.

[thomhurst]

Addressed the repeated argument-tree traversal from #5167 (comment) in de5c0ff, confirmed on the remote PR head. The scraper now materializes declared arguments once. All 852 focused parser, traversal, AWS, and gcloud tests still pass; scoped formatting passes.

On the undocumented operand observation: SDK 585.0.0 contains two real dispatch pages with no listed children, so child discovery cannot gate removal for gcloud. Explicitly documented positional GROUP/COMMAND operands take precedence and remain required, covered by regression tests. Other unrepresented operands still hit the existing guard. This follows the captured SDK grammar; a future inconsistent help page would need its own captured regression.

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: de5c0ff183

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[github-actions]

Review

Re-reviewed at head de5c0ff. The fix itself is sound: GcloudCliScraper opts out of the base class's blanket PreserveCommandGroupPlaceholders gate and does its own, narrower stripping — it only removes a bare Group/Command synopsis placeholder when that name isn't backed by a documented POSITIONAL ARGUMENTS entry. UsageSynopsisParser.RemoveCommandGroupPlaceholders threading an adapter-supplied placeholder set (instead of the hardcoded default) is a clean, generic extension point — the other five callers (ArgoCd, Snyk, WinGet, Gh, Vault) are unaffected since they keep using the single-arg overload. GcloudDispatchTests covers the dispatch-coverage regression, nested/empty-group preservation, and the documented-positional-operand exception directly.

Previous comment follow-up: the repeated FlattenArguments() walk flagged in the prior round (#5167 (comment)) is fixed here — declaredArguments is now materialized once (GcloudCliScraper.cs:164) and reused for both the non-positional argument filter and the dispatch-placeholder ExceptWith (GcloudCliScraper.cs:190-192), confirmed against the current head. The undocumented-operand edge case raised in that same comment was answered on the PR with a concrete justification (SDK 585.0.0 ships real dispatch pages with no listed children, so gating on child discovery isn't viable for gcloud) and is unchanged/accepted; I don't have a better alternative to offer there either — any tightening would need a captured regression fixture for a case that doesn't exist in the current SDK output, which isn't worth speculative complexity.

No new architectural concerns or correctness issues found. This looks ready to merge from a design/quality standpoint.

[thomhurst]

Head de5c0ff now passes all 17 checks. Codex, CodeRabbit, and Claude re-reviews are clear; there are no unresolved review threads. The repository merge gate passes.

Merge remains queued behind the active AWS generation in run 35058181945. Merging this source change earlier would invalidate that run's generator fingerprint. After healthy AWS output merges, this PR can merge and gcloud-only regeneration can run from the resulting main.