Skip to content

fix(generator): normalize gcloud synopsis aliases and defaults - #5161

Merged
thomhurst merged 2 commits into
mainfrom
issue-5146-gcloud-alias-groups
Sep 16, 2026
Merged

thomhurst merged 2 commits into
mainfrom
issue-5146-gcloud-alias-groups

Conversation

@thomhurst

@thomhurst thomhurst commented Sep 16, 2026

Copy link
Copy Markdown
Owner

Problem and change

Gcloud SDK 585 generation on main failed in run 35047414356: 24 DNS/SQL commands were skipped because --instance=INSTANCE, -i INSTANCE produced a phantom operand, then builds triggers create manual aborted generation because a displayed default was interpreted as nested argument syntax.

Normalize comma-separated short aliases with their long option's documented value, and strip displayed default annotations from normalized synopsis options before parsing groups. Original help remains available for option metadata. Preserve real operands after aliases and quoted default text containing syntax characters.

Add 42 captured SDK 585 help fixtures covering every skipped command and all stable build-trigger create/update variants. No generated options are edited.

Validation

  • Before: 25 failures among 97 captured command regressions (24 alias failures plus the fatal manual-trigger group).
  • After: 430 parser, gcloud, AWS, and kubectl tests passed, including all 97 captured commands and seven focused alias/default cases and the manual-trigger constraint structure.
  • Scoped generator formatting passed with the pinned SDK through the 2 GB agent guard.

Related to #5146, #4863, #4869, and #4956. Keep #5146 open until fresh full generation and coverage pass.

Regenerate gcloud and AWS from latest main after the separate AWS enum defect in #5164 is fixed.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 16, 2026

Copy link
Copy Markdown

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-16T03:34:00.002867Z 66033be New commits
ℹ️ 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

coderabbitai Bot commented Sep 16, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

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: d019f3eb-1f60-44ee-af18-3ee2a257b8ec

📥 Commits

Reviewing files that changed from the base of the PR and between a2f5b12 and 66033be.

📒 Files selected for processing (2)
  • tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/Cli/GcloudResourceArgumentTests.cs
  • tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/GcloudCliScraper.cs

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


📝 Walkthrough

Walkthrough

The change updates gcloud synopsis parsing, adds regression tests, and adds 585.0.0 help fixtures for Cloud Build trigger, DNS, and Cloud SQL commands.

Changes

Gcloud option parsing

Layer / File(s) Summary
Synopsis normalization and tests
tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/GcloudCliScraper.cs, tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/Cli/GcloudResourceArgumentTests.cs
Synopsis parsing now accepts short-option values separated by whitespace or = and removes default annotations. Tests cover aliases, resource operands, optional groups, captured commands, and Cloud Build trigger constraints.
Cloud Build trigger creation fixtures
tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Fixtures/Gcloud/585.0.0/gcloud-builds-triggers-create-*.txt
Adds help fixtures for Cloud Build trigger creation across supported repository and trigger types.
Cloud Build trigger update fixtures
tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Fixtures/Gcloud/585.0.0/gcloud-builds-triggers-update-*.txt
Adds help fixtures for Cloud Build trigger update commands across supported repository and trigger types.
DNS and Cloud SQL fixtures
tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Fixtures/Gcloud/585.0.0/gcloud-dns-*.txt, tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Fixtures/Gcloud/585.0.0/gcloud-sql-*.txt
Adds help fixtures for DNS record-set commands and Cloud SQL database, operation, user, and certificate commands.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 66033

The parser changes are covered by focused regression tests and no unresolved merge-blocking issue remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main generator fix: normalization of gcloud synopsis aliases and default values.
  • 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-5146-gcloud-alias-groups

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

A rabbit parses flags in a neat little row
New fixtures record what the gcloud commands show
Triggers and DNS join SQL in the file
Tests guard each option with a careful smile
Short forms and defaults now settle just right
The scraper hops onward through help text tonight

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

@github-actions

Copy link
Copy Markdown
Contributor

Code review

Reviewed the diff and the checked-out PR head (a2f5b12) against main.

Summary of the fixGcloudCliScraper.ParseUsageSynopsis now does two things before handing the SYNOPSIS text to UsageSynopsisParser.DeferDocumentedOptionGroups:

  1. The per-argument value-hint regex optionally also consumes a trailing comma-separated short alias (, -i INSTANCE) that repeats the same value hint, collapsing --instance=INSTANCE, -i INSTANCE to --instance=VALUE so the alias's repeated value token is no longer mistaken for a positional operand.
  2. A new SynopsisDefaultAnnotationPattern strips ; default=... annotations (quoted, angle-bracketed, or bare) that trail an already-normalized --flag=VALUE, applied once per SYNOPSIS block after the per-argument loop and before group parsing — so a default="..." (or default=<Mode.VALUE: 1>, etc.) no longer gets parsed as nested argument syntax.

Both changes are scoped to the gcloud-specific override (not the shared CliScraperBase/UsageSynopsisParser), which is appropriate since these are argparse/gcloud display quirks rather than generic CLI-help conventions — consistent with how this class already overrides ParseUsageSynopsis for gcloud-only normalization.

Verification

  • Traced the regex changes by hand against the real captured fixtures: gcloud-sql-databases-list.txt reproduces the --instance=INSTANCE, -i INSTANCE alias case, and gcloud-builds-triggers-create-manual.txt reproduces the --dockerfile-dir=DOCKERFILE_DIR; default="/" case that previously aborted generation.
  • The two new unit tests (Required_Option_Alias_Preserves_The_Following_Resource_Operand, Default_Annotations_Do_Not_Change_Optional_Group_Operands) isolate exactly these two scenarios with minimal literal fixtures, including the tricky default-value shapes from the regex's alternation (quoted with embedded ]/)/: characters, angle-bracket enum repr, bare word) — good targeted coverage of the exact edge cases the new regex's alternation is designed for.
  • The 42 new fixtures added under Fixtures/Gcloud/585.0.0/ match the PR description's count (18 build-trigger create/update variants + 6 DNS + 18 SQL commands), and all 42 are wired into Captured_585_Commands_Retain_Coverage.
  • No generated option files are touched, consistent with fixing the scraper rather than hand-editing generated output.

Non-blocking observations (not requesting changes):

  • The new alias-stripping group only matches a space-separated short alias (-i INSTANCE); an =-separated alias in a synopsis (-i=INSTANCE) wouldn't be stripped. The sibling GcloudFlagPattern used for the FLAGS section already tolerates both forms. Not currently exercised by any captured fixture, so it's a latent gap rather than an observed bug — worth a quick check if a future SDK capture ever shows that form in a SYNOPSIS line.
  • Captured_585_Commands_Retain_Coverage only asserts the fixture parses without throwing and preserves FullCommand; the manual trigger fixture (the one that previously hit the fatal group-nesting bug) doesn't get a deeper structural assertion the way gcloud-dataplex-metadata-jobs-create.txt does in Captured_Metadata_Job_Retains_Nested_Export_Import_Constraints. Given the regression here was "aborts generation," not-throwing is a reasonable regression guard, but a follow-up assertion on manual's required-alternative-group shape would pin down its structure too.

No correctness issues or CLAUDE.md violations found.

@thomhurst

Copy link
Copy Markdown
Owner Author

Addressed both non-blocking observations in review comment 5691564841 with 66033be, confirmed in the remote PR head: short aliases now accept both space and equals separators, and the captured manual-trigger regression asserts the required trigger/config choice, nested build-config exclusivity, Dockerfile requirement, and absence of positional operands. All 430 related parser tests pass; scoped formatting passes. The CodeRabbit docstring-coverage warning counts named regression tests and private helpers; these follow existing repository conventions rather than adding repetitive XML comments to test methods.

@github-actions

Copy link
Copy Markdown
Contributor

Code review

Reviewed the gcloud synopsis normalization fix in GcloudCliScraper.cs plus the accompanying tests and 42 captured SDK 585 fixtures.

Summary of the fix

  • Extends the per-option synopsis normalization regex to also consume a trailing comma-separated short alias with its own repeated value (e.g. --instance=INSTANCE, -i INSTANCE), collapsing the whole thing to --instance=VALUE. This directly matches the described DNS/SQL "phantom operand" failure from short-flag aliases.
  • Adds SynopsisDefaultAnnotationPattern to strip trailing ; default=... annotations (quoted strings, <Mode.VALUE: 1>-style enum reprs, and bare tokens) after an option has already been normalized to =VALUE, so a displayed default no longer gets misread as nested synopsis/group syntax. I checked this against the real gcloud-builds-triggers-create-manual.txt fixture (--dockerfile-dir=DOCKERFILE_DIR; default="/") and the fix cleanly resolves that exact case.

Verification

  • Both regex changes are covered by new, well-targeted unit tests (Required_Option_Alias_Preserves_The_Following_Resource_Operand, Default_Annotations_Do_Not_Change_Optional_Group_Operands) with multiple parameterized edge cases (space/= alias separators; quoted, angle-bracket, and bare default values).
  • Captured_585_Commands_Retain_Coverage and the new Captured_Manual_Trigger_Retains_Configuration_And_Dockerfile_Constraints test exercise the real fixtures end-to-end, including the nested mutually-exclusive/required group structure for the manual trigger command.
  • The change is scoped correctly per this repo's constraints: only the scraper and its tests/fixtures are touched, no generated options under src/ModularPipelines.Google are hand-edited, and no PublicAPI.*.txt baselines were added.

No correctness issues or design concerns found in the diff. This is a well-isolated generator fix with solid regression coverage.

@thomhurst
thomhurst deployed to Pull Requests September 16, 2026 03:38 — with GitHub Actions Active
@thomhurst
thomhurst merged commit 3cdd8dd into main Sep 16, 2026
17 checks passed
@thomhurst
thomhurst deleted the issue-5146-gcloud-alias-groups branch September 16, 2026 04:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant