fix(generator): preserve Vault switch identity and sync source API

[thomhurst]

Vault help uses single-dash switches, but the scraper changed them to double-dash names. Shared synopsis resolution could then discard a positional operand after a presence-only flag and omit the command during traversal. Preserve the documented switch spelling, retain those operands across alternative synopses, and keep value-option placeholders excluded.

Synchronize the generator public API baseline with the source that landed in #5116, including the removed static GetPositionalArguments overloads and the new instance method. The repository sync script recovered the stale entries; an explicit entry also records the linked CommandLinePhase.LateOperand member omitted by its package-path filter.

This follow-up was prepared during review of #5116, which merged before the fixes were pushed. It contains scraper source, tests, and the generator API baseline. Parent #4692 remains open for latest-main generation and integration verification.

Validation: all 1,088 scraper tests pass, including three regressions that failed before the fix. Release generator solution build with PublicApiAnalyzerProject=ModularPipelines.OptionsGenerator passes with zero warnings/errors. Scoped formatting and git diff --check pass. The commit was replayed onto main after #5116 merged; its resulting tree is identical to the validated tree. The existing 600-second/2-GB limits remain unchanged; previously memory-limited broad compiled-generator validation remains deferred to CI.

Refs #5115, #4692

Summary by CodeRabbit

• Bug Fixes
  • Improved Vault CLI option parsing to preserve original flag names and prevent duplicate options.
  • Corrected positional argument handling when flags appear before operands.
  • Continued excluding option-value placeholders from positional arguments.
• Tests
  • Added coverage for standard and alternative Vault command syntaxes, including help output handling.

[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: 68e12d48-ef9c-45e7-b6ee-7e24ae1f0319

📥 Commits

Reviewing files that changed from the base of the PR and between 8ea298f and 7840fc9.

📒 Files selected for processing (3)

• tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/VaultCliScraperTests.cs
• tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/PublicAPI.Unshipped.txt
• tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/VaultCliScraper.cs

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

---

📝 Walkthrough

Walkthrough

The change updates CLI parsing API declarations, preserves raw Vault option switches, and adds tests for positional operands after flags. Tests also confirm that option-value placeholders remain excluded from positional arguments.

Changes

Vault operand parsing

Layer / File(s)	Summary
CLI parsing contracts tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/PublicAPI.Unshipped.txt	The public API declarations add operand phases, required-alternative records, parser helpers, result state, and scraper overrides. They also record removed and changed APIs.
Vault raw option handling tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/VaultCliScraper.cs	Vault option deduplication and property-name normalization use raw flag names. CliOptionDefinition.SwitchName now stores the raw switch.
Vault traversal validation tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/VaultCliScraperTests.cs	Tests cover positional operands after flags, alternative usage syntaxes, requiredness, switch mapping, and exclusion of option-value placeholders. The test executor supports optional leaf help text.

Priority: ⬇️ Low

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

Change: Bug fix

Possibly related PRs

• thomhurst/ModularPipelines#5116: Changes shared usage resolution, including GetPositionalArguments, UsageSynopsisParser, and scraper APIs used by this PR.

Merge Risk: ⚪ Minimal · up to 7840f

The Vault parsing and API-baseline updates have reported test and build coverage with no established user-facing failure, so the change is ready to merge pending routine CI.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. (1 skipped: 1…	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 identifies the main changes: preserving Vault switch identity and synchronizing the generator API. It is concise and directly related to the pull request.
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.

Full details: Docstring Coverage

Explanation

Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. (1 skipped: 1 unsupported.)

• 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-5115-vault-api-followup

---

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 hops past flags in line
Raw switches keep their proper sign
PATH stays ready after the dash
ADDRESS placeholders leave in a flash
Tests guard the trail through Vault’s design}

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

[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-14T17:36:44.797748Z	7840fc9	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.

[greptile-apps]

Greptile Summary

This PR corrects Vault CLI scraping and synchronizes the generator’s public API baseline.

• Preserves Vault’s documented single-dash option spelling instead of rewriting switches with two dashes.
• Adds regression coverage for operands following presence-only flags, alternative synopses, and exclusion of value-option placeholders.
• Updates the unshipped API baseline to reflect the current generator source, including the instance positional-argument method and LateOperand.

Confidence Score: 5/5

The PR appears safe to merge, with no outstanding correctness, security, or repository-rule issues identified.

The Vault scraper now retains documented switch spellings without breaking downstream generation, its operand behavior is covered by focused regressions, and the public API entries align with the current source signatures.

Important Files Changed

Filename	Overview
tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/VaultCliScraper.cs	Preserves raw Vault switch spelling while continuing to derive stable option property names.
tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/VaultCliScraperTests.cs	Adds focused regressions for switch identity and positional-operand handling across usage synopsis variants.
tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/PublicAPI.Unshipped.txt	Synchronizes the public API baseline with the generator’s current source-level API.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    H[Vault help text] --> P[Parse documented switches and usage]
    P --> S[Preserve single-dash switch identity]
    P --> O[Resolve positional operands]
    S --> C[CLI command definition]
    O --> C
    C --> G[Generated Vault options]

Loading

_{Reviews (2): Last reviewed commit: "fix(generator): preserve Vault switches ..." | Re-trigger Greptile}

[github-actions]

Review

Reviewed the diff (gh pr diff 5119) and cross-checked it against the current main tree (which already includes #5116).

Vault switch-spelling fix (VaultCliScraper.cs) — correct and narrowly scoped. Previously ParseOptions force-rewrote every scraped flag into a ---prefixed "long form" (var longForm = flagName.StartsWith("--") ? flagName : $"--{flagName.TrimStart('-')}";) before using it as both the dedup key and SwitchName. Vault's CLI only recognizes single-dash switches (e.g. -tls-skip-verify), so the rewritten SwitchName didn't match what the real binary accepts. The fix now keeps the flag exactly as scraped for both the seenOptions dedup key and SwitchName. I checked NormalizePropertyName (CliScraperBase.cs:1202) — it does TrimStart('-') before splitting, so passing the raw single-dash flagName instead of the double-dash longForm produces an identical property name, confirming no behavior change there. The regex (VaultOptionPattern) already captures whatever dash-prefix is present in the help text, so this also correctly preserves any option that legitimately uses -- in Vault's docs.

New tests — both are meaningful integration-level regressions, not tautological unit checks:

• Shared_Traversal_Preserves_Operands_After_Flags drives the full ScrapeAsync() path (not an isolated helper) and verifies the leaf command's positional Path survives a presence-only flag (-tls-skip-verify) preceding it in the synopsis, that requiredness correctly varies with an alternative synopsis, and that the option's SwitchName keeps its single-dash spelling — tying the traversal fix from fix(generator): resolve shared usage shapes before regeneration #5116 and this PR's switch fix together in one scenario.
• Value_Option_Placeholders_Remain_Excluded_From_Positionals confirms a value-option's placeholder (<ADDRESS> for -address <ADDRESS>) isn't misidentified as its own positional argument alongside the real <PATH> operand.

PublicAPI.Unshipped.txt sync — spot-checked several of the added/removed entries against current source rather than taking the diff at face value:

• CommandLinePhase.LateOperand/removal of EndOfOptions matches src/ModularPipelines/Attributes/CommandLinePhase.cs (linked into the generator project) exactly.
• The non-static CliScraperBase.GetPositionalArguments(usage, options) addition and removal of the two old static overloads matches the actual (already non-static) method at CliScraperBase.cs:1035.
• CliRequiredAlternativeGroup/CliRequiredAlternativeMember records exist in Models/CliRequiredAlternativeGroup.cs on main already (landed via fix(generator): resolve shared usage shapes before regeneration #5116).

All match real, already-existing declarations rather than being hand-authored — consistent with this repo's rule that the baseline should reflect current generator output, not a manual edit.

No prior bot reviews (Codex, Greptile) raised findings on this PR, and CodeRabbit hit its review-rate limit without producing one; nothing outstanding to reconcile.

No actionable issues found — this is a clean, well-tested follow-up.

[thomhurst]

Rebased onto main 8ea298f after #5118 merged. Resolved the public API header conflict while retaining the shared enum API additions. Remote head 7840fc9 passes the Release generator solution build with PublicApiAnalyzerProject=ModularPipelines.OptionsGenerator explicitly enabled (zero warnings/errors), all five Vault regressions, and git diff --check. Source formatting remains unchanged. @codex review

[chatgpt-codex-connector]

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

Reviewed commit: 7840fc912f

ℹ️ 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 the current head (7840fc91), which is a rebase of the previously-reviewed commit onto main after #5118 merged, resolving a PublicAPI.Unshipped.txt conflict.

Core fix (VaultCliScraper.cs) — unchanged from the prior review and still correct. ParseOptions previously force-rewrote every scraped flag into a ---prefixed form (var longForm = flagName.StartsWith("--") ? ... : $"--{flagName.TrimStart('-')}";) before using it for both the seenOptions dedup key and SwitchName. Vault only recognizes single-dash switches (e.g. -tls-skip-verify), so the generated SwitchName didn't match the real binary. The fix keeps the flag exactly as scraped for both purposes. NormalizePropertyName (CliScraperBase.cs) already TrimStart('-')s before deriving the property name, so passing the raw flag instead of the ---prefixed form doesn't change generated property names — no behavioral regression there.

Tests — both new tests exercise the full ScrapeAsync() traversal (not isolated helpers): one confirms a positional operand survives a presence-only flag preceding it across two synopsis variants and that SwitchName keeps its single-dash spelling; the other confirms a value-option's placeholder (<ADDRESS>) isn't misidentified as its own positional alongside the real <PATH> operand. Both are meaningful regressions tied to the traversal fix from #5116 plus this PR's switch fix.

PublicAPI.Unshipped.txt rebase resolution — verified this is where the current head differs from what I reviewed at commit 64997aaf. Confirmed no duplicate entries in the merged file (sort | uniq -d is empty) and that both API members reintroduced from #5118's merge (CliEnumDefinition.OrderValues, GeneratorUtils.UsesGeneratedEnums) exist in current source (Models/CliEnumDefinition.cs, Generators/GeneratorUtils.cs) alongside this PR's own additions (CommandLinePhase.LateOperand, the non-static GetPositionalArguments overload, CliRequiredAlternativeGroup/Member, etc.). The rebase correctly merged both PRs' baseline changes rather than dropping either side — consistent with the repo rule that this file must reflect current generator output, not a hand-edit.

No actionable issues found. Clean, well-tested follow-up; the rebase was resolved correctly.

[thomhurst]

Merged as 34b8b18 after current-head reviews cleared and all 17 checks passed or completed with allowed skipped/neutral conclusions. Paginated branch-run inspection confirms no queued, waiting, or running CI remains, including superseded commits.