fix(generator): respect AWS enum description boundaries - #5165
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughThe AWS CLI scraper now uses preserved description indentation to distinguish option-level enum headings from nested attribute documentation. It also validates bullet markers more strictly. New tests cover SNS, ACM, and enum boundary cases. ChangesAWS CLI enum boundary handling
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The enum-boundary changes preserve the intended string fallback and are covered by focused regression tests, with no merge-blocking risk identified. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Linked Issues checkExplanation Issue Resolution Run the options generator with the corrected AWS scraper against the AWS CLI help snapshot required by
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. A rabbit checks each enum line, Comment |
ReviewTraced the new
One non-blocking thought for future maintainability: I wasn't able to run the build/tests in this sandboxed review environment, but the PR description's reported validation (322/322 passing, zero-warning release build) combined with the manual trace above gives me confidence in the fix. Nice fix — good regression coverage for a genuinely subtle text-parsing boundary problem. |
|
Thanks for the review in comment 5692160135. The indentation comparison already rejects any heading deeper than the option description, including more than two indentation tiers. An indentation stack would only be needed if future help formats require interpreting those nested values; this change intentionally keeps the existing string fallback for them. No additional parser state is needed for the current option-level enum contract. |
|
Disposition of the two pre-merge warnings in comment 5692102410: #5163 is the closed generated PR, not the source issue. This PR implements source issue #5164. Repository guidance explicitly requires merging the source fix separately before regenerating a fresh AWS PR, so generated output belongs in the replacement PR after this merges. The new methods are named regression tests and a private helper; they follow the surrounding test/helper documentation convention, and the helper includes the non-obvious indentation rationale. @coderabbitai review the final head 1b82365, which additionally rejects prose inside bullet entries. |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== final commit and changed files ==='
git show --no-ext-diff --format=fuller --stat 1b82365ef330b98f20d9e51832f817e42b71ee57
git diff --no-ext-diff --unified=80 1b82365ef330b98f20d9e51832f817e42b71ee57^ 1b82365ef330b98f20d9e51832f817e42b71ee57 -- \
tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/AwsCliScraper.cs \
tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/AwsCliScraperTests.EnumBoundaries.cs
echo '=== scraper outline ==='
ast-grep outline tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/AwsCliScraper.cs --items all
echo '=== tests outline ==='
ast-grep outline tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/AwsCliScraperTests.EnumBoundaries.cs --items all
echo '=== enum parser call sites and definitions ==='
rg -n -C 6 -P '\b(TryDetectEnum|ParseEnumChoices|IsOptionLevelEnumHeading)\s*\(' \
tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator \
tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests
echo '=== adjacent scraper tests ==='
fd -a 'AwsCliScraperTests.*\.cs' tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers -x sh -c 'echo "--- $1"; rg -n -C 2 -P "\b(enum|Enum|Possible values|Valid values)\b" "$1"' _ {}Length of output: 76181 🏁 Script executed: #!/bin/bash
set -uo pipefail
base=$(git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD origin/master 2>/dev/null || true)
echo "=== base: ${base:-unavailable} ==="
if [ -n "$base" ]; then
git diff --no-ext-diff --stat "$base"...1b82365ef330b98f20d9e51832f817e42b71ee57
git diff --no-ext-diff --unified=30 "$base"...1b82365ef330b98f20d9e51832f817e42b71ee57 -- \
tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/AwsCliScraper.cs \
tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/AwsCliScraperTests.EnumBoundaries.cs
fi
echo '=== ParseOptions and description extraction ==='
sed -n '390,480p' tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/AwsCliScraper.cs
echo '=== enum parsing and regex declarations ==='
sed -n '655,760p' tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/AwsCliScraper.cs
sed -n '800,890p' tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator/Scrapers/Cli/AwsCliScraper.cs
echo '=== full enum-boundary tests ==='
cat -n tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/AwsCliScraperTests.EnumBoundaries.cs
echo '=== tests for bullet parse behavior ==='
sed -n '96,120p' tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/AwsCliScraperTests.cs
sed -n '156,176p' tools/ModularPipelines.OptionsGenerator/src/ModularPipelines.OptionsGenerator.Tests/Scrapers/AwsCliScraperTests.csLength of output: 44459
I found no blocking issues. The final parser logic rejects prose within The source-fix-only scope is also correct. The generated AWS option changes belong in the replacement regeneration PR after this PR merges.
|
Problem and fix
AWS enum extraction accepted nested attribute prose and treated the single bullet in
Possible values: o PRE_APPROVEDas a second allowed value. In the generated AWS PR #5163 and runtime PR #4367, this made valid SNS attribute names impossible to pass and exposed invalidOenum members.Keep the original description indentation until enum detection. Accept enum headings at the option description level and at the start of a sentence, excluding nested attribute text. Recognize both
oand*bullet lists, including one-item lists, and reject prose within their entries before applying the existing shared enum member limits. A single allowed value therefore retains the existing string fallback.Regression coverage includes the SNS documentation excerpt, ACM authorization/contact values, the VPN
ipsec.1value, nested headings, inline and separate option-level headings, and generated constructor/enum output. Generated options are not edited in this PR.Validation
info; existing diagnostics without code fixes remain.Regeneration
Fixes the generator portion of #5164. After this source PR merges, regenerate AWS and gcloud together from the latest
main, review the replacement for closed #5163, and update #4367 with the authoritative output. Keep #5164 open until regeneration is verified.Summary by CodeRabbit