Port the remaining nine CLI scrapers onto the shared wrapped-description helper

[thomhurst]

Summary

Second child of #4646. Ports the remaining nine scrapers onto CliScraperBase.AccumulateWrappedDescription (from #4645): Az, DotNet, Go, Maven, pip, pnpm, Terraform, WinGet, Yarn. Their private accumulators (dash-led stops, absolute indent thresholds of 8–30 spaces, and per-tool section-header heuristics) and Go's private GetIndentationWidth are deleted; each scraper now supplies only its own option-row predicate. Every ported scraper has a fixture that pins a wrapped --flag-led line as prose (Go's existing Preserves_Hyphen_Led_Description_Continuations already covers it).

Base: a row whose prose only begins on the next line (go, picocli, argparse) had no description column, so the column-aware rule could not protect its later wrapped lines. AccumulateWrappedDescription now infers the column from the first wrapped line. The same four-line hunk is in #4657 (deliberately identical so either PR can land first; git merges it as a no-op), and it is pinned here by two direct ContinuationLineTests cases.

Behavioural notes

• The dropped section-header stops were redundant: every option loop is already section-scoped before accumulation, and no real sub-header sits deeper than its option rows in these nine formats.
• Az: knack prints an argument's long summary and preview notice four columns under the row (Argument '--x' is in preview…); the old < 20 threshold dropped those lines, the depth rule now joins them into the description. Pinned in AzCliScraperTests.
• Per the parent issue's scope note this changes generated output for these tools wherever wrapped descriptions previously mis-parsed; regenerate the nine packages via the workflow after merge. The base lookahead in HelpDeclaresRepeatableOption is tracked in Use the shared continuation rule in CliScraperBase.HelpDeclaresRepeatableOption #4655.

Test plan

• New fixtures for Az, DotNet, Maven, pip (new PipCliScraperTests), pnpm (new PnpmCliScraperTests), Terraform, WinGet, Yarn; two direct base tests for the inferred column.
• ModularPipelines.OptionsGenerator.Tests: 1328 passed, 0 failed (guarded local run).
• dotnet format whitespace --verify-no-changes clean for the touched files (the only remaining hit, IHelpTextCache.cs, pre-exists on main).

Closes #4654

https://claude.ai/code/session_01PkLNTUfwGXjqXZrYrHaDGC

Summary by CodeRabbit

• Bug Fixes

  • Improved command-line help parsing across supported tools.
  • Wrapped descriptions that resemble option rows are now preserved as prose instead of being incorrectly parsed as separate options.
  • Multi-line descriptions are more reliably combined, including continuation text with varying indentation.

• Tests

  • Added coverage for wrapped descriptions and continuation-line parsing across multiple CLI formats.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: a929b90a-b805-4c51-976e-c8ad2fa28272

📥 Commits

Reviewing files that changed from the base of the PR and between f65c882 and 932832d.

📒 Files selected for processing (19)

• tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/AzCliScraperTests.cs
• tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/Cli/ContinuationLineTests.cs
• tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/Cli/TerraformCliScraperTests.cs
• tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/DotNetCliScraperTests.cs
• tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/MavenCliScraperTests.cs
• tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/PipCliScraperTests.cs
• tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/PnpmCliScraperTests.cs
• tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/WinGetCliScraperTests.cs
• tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/YarnCliScraperTests.cs
• tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/AzCliScraper.cs
• tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/CliScraperBase.cs
• tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/DotNetCliScraper.cs
• tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/GoCliScraper.cs
• tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/MavenCliScraper.cs
• tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/PipCliScraper.cs
• tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/PnpmCliScraper.cs
• tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/TerraformCliScraper.cs
• tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/WinGetCliScraper.cs
• tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/YarnCliScraper.cs

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

---

📝 Walkthrough

Walkthrough

The CLI scrapers now share wrapped-description parsing. The parser tracks the description column and uses scraper-specific option-row predicates. Tests cover wrapped lines that resemble options across nine CLI formats.

Changes

Wrapped description parsing

Layer / File(s)	Summary
Continuation-column rules tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/CliScraperBase.cs, tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/Cli/ContinuationLineTests.cs	The shared accumulator accepts deeper lines before the description column is known and records the first wrapped line column.
Scraper accumulator migration tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/*	Az, .NET, Go, Maven, pip, pnpm, Terraform, WinGet, and Yarn use AccumulateWrappedDescription with their option-row predicates. Local multiline accumulators were removed.
Wrapped-description fixture coverage tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/*	Tests verify that option-like wrapped lines remain prose and that parsed option names and descriptions remain correct across the supported scrapers.

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

Merge Risk: ⚪ Minimal · up to 93283

The wrapped-description migration preserves option-like prose and next-option boundaries across the updated CLI scrapers. No merge-blocking risk remains.

Poem

A rabbit checks each wrapped line,
And keeps the prose in proper design.
Flags may hop across the page,
The parser guards the option stage.
Tests now watch each burrowing phrase.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 24.07% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 54 functions across 19 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 identifies the primary change: migrating the remaining nine CLI scrapers to the shared wrapped-description helper.
Linked Issues check	✅ Passed	The PR ports all nine scrapers in issue #4654 to AccumulateWrappedDescription, removes scraper-specific accumulation logic, adds fixture coverage for wrapped flag-like prose, and reports that all 1329…
Out of Scope Changes check	✅ Passed	The changes remain within issue #4654. The shared helper update, scraper migrations, continuation-line tests, and scraper fixture tests directly support the stated requirements.

• 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-4654-port-scrapers-group-b

---

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

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

[github-actions]

Review

Solid refactor: deleting nine near-duplicate AccumulateMultiLineDescription implementations in favor of the shared, column-aware CliScraperBase.AccumulateWrappedDescription is exactly the right direction — it matches the CLAUDE.md guidance to fix generic generator logic once rather than per-tool, and each new fixture pins the specific behavioural change it introduces (e.g. the Az knack preview-notice join, the terraform/dotnet/maven/pip/pnpm/winget/yarn "wrapped line that looks like a flag" cases). The new descriptionColumn inference in CliScraperBase.cs (falling back to the first wrapped line's column when a row has no inline description) is a clean way to unify the "prose starts on the next line" formats (go/picocli/argparse) with the "prose is inline" formats without another per-tool branch.

Finding

CliScraperBase.IsContinuationLine can drop the very first wrapped line it's meant to infer a column from, when that line also looks like an option row.

tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/CliScraperBase.cs (around IsContinuationLine, ~line 1260, and the descriptionColumn ??= GetIndentation(candidate); fallback added at ~line 1323):

var indentation = GetIndentation(line);
var wrappedAtDescriptionColumn = indentation >= descriptionColumn;
return (!looksLikeOptionRow || wrappedAtDescriptionColumn)
       && indentation > declarationIndentation;

When a row has no inline description, descriptionColumn starts as null. In C#, a lifted >= against a null right-hand operand is always false, so wrappedAtDescriptionColumn is false for the very first candidate line. If that first candidate also happens to satisfy the scraper's looksLikeOptionRow predicate, (!looksLikeOptionRow || wrappedAtDescriptionColumn) evaluates to false and the line is rejected as a continuation — before the column-inference fallback (descriptionColumn ??= GetIndentation(candidate)) ever gets a chance to run. The one case this new fallback was added to handle (no-inline-description rows) is exactly the case most exposed to this gap.

This is largely theoretical for the other eight scrapers here, since their IsOptionRow predicates require the full --flag ... <2+ spaces> ...desc shape, which plain prose is unlikely to satisfy by accident. But Go's IsOptionRow (GoOptionLinePattern, ^[ \t]+(?<flag>-[A-Za-z][\w-]*)(?<remainder>[^\r\n]*)$) matches any indented line starting with a dash + letter, remainder unconstrained — and Go is also the scraper that always passes inlineDescription: null (GoCliScraper.cs ~line 774), i.e. it always relies on this fallback. If a Go flag's first wrapped description line begins with a hyphenated token (e.g. a sentence that opens with a flag name), that line will be misread as the start of a new option instead of joined into the current one — which, for this generator, doesn't just truncate a description, it can mint a bogus generated option in the public API surface.

Suggested fix: treat "column not yet known" as "accept," not "reject," since the whole point of the fallback is that there's nothing to compare against yet:

var wrappedAtDescriptionColumn = descriptionColumn is null || indentation >= descriptionColumn;

This only changes behavior for the untested null-column + option-row-looking-first-line combination — both new ContinuationLineTests cases and the Az test have their first candidate line not matching looksLikeOptionRow, so they're unaffected, and rows with an inline description already have descriptionColumn set before the loop starts.

Given the narrowness of the trigger (Go, no inline description, first wrapped line only), consider adding one direct ContinuationLineTests case with a looksLikeOptionRow: true first candidate and no inline description, alongside the fix, to lock the corrected behavior in the same place the other two column-inference cases live.

[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-06T05:16:37.177776Z	932832d	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.

[thomhurst]

Addressed the finding in 932832d: IsContinuationLine now treats an unknown description column as accept (descriptionColumn is null || indentation >= descriptionColumn), so the first wrapped line can establish the column even when it matches the scraper's option pattern. The null-column ContinuationLineTests case is flipped to the corrected behaviour (and still rejects a line no deeper than the declaration), and a new direct AccumulateWrappedDescription case pins a first wrapped line starting with --namespace being joined while a shallower real option row still ends the description. Full ModularPipelines.OptionsGenerator.Tests run: 1329/1329.

@claude please re-review exact head 932832d.

[github-actions]

Review

Re-reviewed at head 932832d. The blocking finding from the previous round (IsContinuationLine rejecting the very first wrapped line when it looked like an option row, because a null descriptionColumn always failed indentation >= descriptionColumn) has been correctly fixed:

var wrappedAtDescriptionColumn = descriptionColumn is null || indentation >= descriptionColumn;

This is exactly the right fix — "column not yet known" now means "accept," which is what lets the first wrapped line actually establish the column instead of being rejected before the descriptionColumn ??= GetIndentation(candidate) inference ever runs. It's backed by a new direct test (First_Wrapped_Line_That_Looks_Like_An_Option_Row_Still_Establishes_The_Column) that reproduces the exact failure shape (a first candidate line starting with --namespace, no inline description), alongside a companion test (Option_Rows_Shallower_Than_The_Inferred_Column_Still_Start_The_Next_Option) proving the fix didn't overcorrect into swallowing real next-option rows. Good regression coverage at the right layer — the shared base class — rather than duplicated per scraper.

The rest of the diff is a clean, mechanical continuation of the #4645 pattern: each of the nine scrapers (Az, DotNet, Go, Maven, pip, pnpm, Terraform, WinGet, Yarn) drops its own hand-rolled AccumulateMultiLineDescription/stop-heuristics in favor of CliScraperBase.AccumulateWrappedDescription plus a one-line IsOptionRow predicate built from each scraper's existing option regex. This is the right direction per the project's own guidance to fix generic generator logic once rather than per-tool, and it's a meaningful simplification — each of the nine deleted methods carried its own ad hoc absolute-indent threshold and section-header guess that's now provably unnecessary.

One non-blocking suggestion

Go is the scraper most exposed to the bug that was just fixed, since GoOptionLinePattern (^[ \t]+(?<flag>-[A-Za-z][\w-]*)(?<remainder>[^\r\n]*)$) matches any indented -word line with an unconstrained remainder, and Go always calls AccumulateWrappedDescription with inlineDescription: null. The existing Preserves_Hyphen_Led_Description_Continuations Go test covers a hyphen-led second wrapped line, but its first wrapped line doesn't match IsOptionRow, so it doesn't actually exercise the "first wrapped line looks like an option" path through the real GoCliScraper pipeline — only the base-class test does, with a synthetic predicate. Since this was the specific scraper called out as most at risk, it'd be worth adding one GoCliScraperTests fixture where a flag's first wrapped description line itself begins with a hyphenated token (e.g. -x is implied when -y is set.), to pin the fix through the actual regex/pipeline Go uses rather than only the isolated primitive. Not blocking — the underlying algorithm is already correctly proven at the base-class level — just closes the loop on the exact risk that was flagged.

Nice work addressing the review; the fix and tests both landed in the right place.

[github-actions]

Review

Re-reviewed at head 932832d (matches the head this PR's own review thread already reached CLEAR on). This is a clean, mechanical continuation of the #4645 pattern: each of the nine scrapers (Az, DotNet, Go, Maven, pip, pnpm, Terraform, WinGet, Yarn) drops its own hand-rolled AccumulateMultiLineDescription/stop-heuristics in favor of the shared, column-aware CliScraperBase.AccumulateWrappedDescription plus a one-line IsOptionRow predicate built from each scraper's existing option regex. That's exactly the right direction per this repo's own guidance (fix generic generator logic once rather than per-tool), and it's a real simplification — each deleted method carried its own ad hoc absolute-indent threshold and section-header guess that's now provably unnecessary, backed by a new fixture per scraper pinning the specific wrapped-line-that-looks-like-a-flag case it used to mis-parse.

I independently traced the same risk area the prior review round flagged as blocking: IsContinuationLine's descriptionColumn is null || indentation >= descriptionColumn check. With the fix in place, a null column (row with no inline description) now means "accept," which is what lets the first wrapped line actually establish the column via descriptionColumn ??= GetIndentation(candidate), instead of being rejected before that inference ever runs. Traced through the AzCliScraper preview-notice fixture and the new ContinuationLineTests cases by hand — both the "first wrapped line looks like an option row" and "shallower real option row still ends the description" paths check out.

The one gap noted in the prior round — Go being the scraper most exposed to this, since GoOptionLinePattern matches any indented -word line with an unconstrained remainder, and Go always calls AccumulateWrappedDescription with inlineDescription: null — is still only covered at the base-class level, not through a GoCliScraperTests fixture that exercises the real regex/pipeline with a first-wrapped-line-starting-with-a-hyphen case. That was already called out as non-blocking last round (the algorithm is correctly proven at the shared-primitive level), and nothing here changes that assessment — just flagging it's still open if a future pass wants to close the loop.

No other issues found. The other eight scrapers all pass their real inline-description Group into AccumulateWrappedDescription, so they don't share Go's null-inline-description exposure at all.