docs(cli): add structured CLI reference for undocumented flags (filters, purchase-safety, cloud-setup)#1090
Open
cristim wants to merge 8 commits into
Open
docs(cli): add structured CLI reference for undocumented flags (filters, purchase-safety, cloud-setup)#1090cristim wants to merge 8 commits into
cristim wants to merge 8 commits into
Conversation
cristim
added
triaged
Contributor
|
Warning Review limit reached
More reviews will be available in 25 minutes and 56 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR adds comprehensive CLI reference documentation for previously undocumented ChangesCLI Documentation Structure
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Member
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
cristim
force-pushed
the
docs/cli-reference
branch
from
ed9e0e5 to
a2e6db8
Compare
Member
Author
|
@coderabbitai review Generated by Claude Code |
Contributor
|
🧠 Learnings used✅ Action performedReview finished.
|
Member
Author
|
@coderabbitai full review |
Contributor
✅ Action performedFull review finished. |
Member
Author
|
Pushed a follow-up commit adding two real root flags that were missing from the "Complete flag reference" table: @coderabbitai review |
Contributor
|
✅ Action performedReview finished.
|
Member
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
.gitignore (1)
134-137: Confirm.gitignorenegation fordocs/cliis correct (lines 134-137).With
docs/*followed by!docs/cli, Git does not ignoredocs/cli/...paths (e.g.,git check-ignore -v docs/cli/README.mdreturns no matching ignore rule), anddocs/cli/*files are present ingit ls-files. Optional: use!docs/cli/(and/or!docs/cli/**) to make the intent more explicit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.gitignore around lines 134 - 137, The current .gitignore uses the patterns "docs/*" and "!docs/cli" which can be ambiguous for excluding the directory contents; update the negation to explicitly include the directory and its contents by changing the negation pattern to include a trailing slash and wildcards (e.g., use "!docs/cli/" and/or "!docs/cli/**") so that files under docs/cli are explicitly not ignored while keeping other docs/ subdirectories ignored.docs/cli/cloud-setup.md (1)
21-21: Fix the IAM-action claim in cloud-setup prerequisites
configure-azure/configure-gcpusecmd/secrets_store.go’sAWSSecretsStore.UpdateSecret, which callssecretsmanager.UpdateSecretwithSecretStringto write the credential JSON; so the documentedsecretsmanager:UpdateSecretpermission is the correct one for these bootstrap commands (notPutSecretValue).
secretsmanager:PutSecretValueis used elsewhere by the server when syncing the admin password, so runtime permissions (separate from these CLI prerequisites) may need it, but thisPrerequisitesbullet for the bootstrap CLI commands does not.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/cli/cloud-setup.md` at line 21, Update the docs to correct the IAM action: change the prerequisite to require secretsmanager:UpdateSecret (not PutSecretValue) because the bootstrap CLI commands (configure-azure/configure-gcp) call cmd/secrets_store.go's AWSSecretsStore.UpdateSecret which uses secretsmanager.UpdateSecret with SecretString; if needed, add a note that secretsmanager:PutSecretValue is used elsewhere by the server for admin password sync and may be required at runtime but is not part of the CLI bootstrap prerequisites.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/cli/filtering.md`:
- Line 74: Update the doc text to remove the misleading “partial prefixes”
wording and explicitly state that --include-instance-types and
--exclude-instance-types require exact full instance-type strings (e.g.,
db.t3.micro) because filtering uses exact membership checks (slices.Contains in
shouldIncludeInstanceType); mention that prefix matching is not supported and
users must enumerate each full instance type they want included or excluded.
- Line 40: Update the docs/cli/filtering.md claim: instead of saying omitting
--regions causes CUDly to auto-discover regions from Cost Explorer
recommendations, state that the code first enumerates all AWS regions via EC2
DescribeRegions (opted-in regions) and only falls back to Cost Explorer-derived
regions when region listing fails; also clarify that --include-regions and
--exclude-regions are applied as a post-recommendation filter (they filter
recommendations by rec.Region via shouldIncludeRegion) and do not change which
regions are queried, and note that if both --regions and --include-regions are
provided the queried regions come from --regions and results are then filtered
by the include/exclude lists.
In `@docs/cli/purchase-safety.md`:
- Around line 56-63: Update the documentation's audit record status list to
include the missing "skipped_covered" status: in the description of the audit
record's status field (the "status" audit field), add `"skipped_covered"` with a
brief note that it represents an idempotency hit where the recommendation was
already purchased within the window; mirror the exact status token
`"skipped_covered"` used in the code and keep wording consistent with the other
statuses ("success", "error", "skipped").
- Around line 20-25: Update the purchase-safety doc to reflect actual behavior:
change table row 2 to "Real purchases" because applyFlagsDryRun
(pkg/config/load.go) sets cfg.DryRun=false when --purchase is provided
(purchaseChanged), and cmd/multi_service.go computes isDryRun :=
!cfg.ActualPurchase || cfg.DryRun so with --purchase (ActualPurchase=true) and
cfg.DryRun=false you get real purchases; also add a note that runToolFromCSV
ignores cfg.DryRun and derives isDryRun := !cfg.ActualPurchase (so behavior
differs between code paths) and adjust any single-formula statements to show
these two separate rules.
In `@docs/cli/README.md`:
- Around line 3-9: The README incorrectly states the root CLI name; update the
docs to reflect the actual root cobra command Use: "ri-helper" as defined in
cmd/main.go: replace references to `cudly` with `ri-helper` (or add a short note
explaining that some builds ship the binary as `cudly` if you prefer to preserve
that alias), while keeping `configure-azure`, `configure-gcp` and the note that
`rekey` and `server` are separate binaries with their own main.go entrypoints
intact so the documented commands match the code.
- Line 29: Update the `--regions` documentation to correctly describe
auto-discovery: when `--regions` is empty non-Savings-Plan services enumerate
all opted-in AWS regions using EC2 DescribeRegions (not Cost Explorer), Cost
Explorer recommendations are only used as a fallback if the EC2 DescribeRegions
call fails, and Savings Plans with an empty `--regions` always target us-east-1;
keep mention that `--include-regions` / `--exclude-regions` are applied on top
of the resulting set.
- Around line 52-54: Update the docs to explicitly state that CSV mode (when
using the --input-csv option) computes isDryRun using isDryRun :=
!cfg.ActualPurchase (i.e., it ignores cfg.DryRun), so passing --purchase alone
can trigger real purchases from a CSV; adjust the README entries for --dry-run
and --purchase and the purchase-safety.md wording to call out this CSV exception
and advise requiring --dry-run=false when using --input-csv if you want to
prevent actual purchases; reference the isDryRun variable and
cfg.ActualPurchase/cfg.DryRun logic in the note so readers can find the code
behavior easily.
---
Nitpick comments:
In @.gitignore:
- Around line 134-137: The current .gitignore uses the patterns "docs/*" and
"!docs/cli" which can be ambiguous for excluding the directory contents; update
the negation to explicitly include the directory and its contents by changing
the negation pattern to include a trailing slash and wildcards (e.g., use
"!docs/cli/" and/or "!docs/cli/**") so that files under docs/cli are explicitly
not ignored while keeping other docs/ subdirectories ignored.
In `@docs/cli/cloud-setup.md`:
- Line 21: Update the docs to correct the IAM action: change the prerequisite to
require secretsmanager:UpdateSecret (not PutSecretValue) because the bootstrap
CLI commands (configure-azure/configure-gcp) call cmd/secrets_store.go's
AWSSecretsStore.UpdateSecret which uses secretsmanager.UpdateSecret with
SecretString; if needed, add a note that secretsmanager:PutSecretValue is used
elsewhere by the server for admin password sync and may be required at runtime
but is not part of the CLI bootstrap prerequisites.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4a6ca616-9ca2-4ce0-8cc8-ee40e05c3bef
📒 Files selected for processing (6)
.gitignoreREADME.mddocs/cli/README.mddocs/cli/cloud-setup.mddocs/cli/filtering.mddocs/cli/purchase-safety.md
Member
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
cristim
added a commit
that referenced
this pull request
Jun 11, 2026
Address CodeRabbit review findings on PR #1090: - README.md + filtering.md: when --regions is empty the code enumerates all opted-in regions via EC2 DescribeRegions (Savings Plans always query once via us-east-1); Cost Explorer-based discovery is only a fallback when region listing fails. --include/--exclude-regions filter fetched recommendations and do not constrain queried regions. - README.md + purchase-safety.md: document that --input-csv mode computes isDryRun = !ActualPurchase and ignores --dry-run, so --purchase alone performs real purchases there; split the decision formula per code path and add a CSV-mode warning example. - filtering.md: instance-type include/exclude lists match by exact string equality (slices.Contains); remove the incorrect "partial prefixes" claim. - purchase-safety.md: add the missing skipped_covered audit status (idempotency hit). - README.md: clarify the binary is built as cudly while the cobra root command is named ri-helper; fix the auto-generated CSV filename format (ri-helper-<mode>-<timestamp>.csv).
cristim
force-pushed
the
docs/cli-reference
branch
from
971a91b to
b912ab6
Compare
Member
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Adds four tracked markdown pages covering the full CLI surface: - docs/cli/README.md: index with a complete flag reference table for all ~35 root-command flags, grouped by concern - docs/cli/filtering.md: account/region/engine/instance-type/SP-type include-exclude filters plus threshold filters (min-count, min-savings-pct, max-break-even-months, min-pool-size, max-instances), with examples; explicitly calls out the --min-savings-pct percentage vs GUI min_savings dollar naming distinction (P3 trap from gap analysis) - docs/cli/purchase-safety.md: --dry-run/--purchase interaction, --audit-log, --idempotency-window, --rebuy-window-days, DISABLE_PURCHASE_DELAY (internal only), and a pre-production safety checklist - docs/cli/cloud-setup.md: configure-azure and configure-gcp subcommands, flags, guided vs non-interactive modes, and when to use vs SaaS onboarding Updates .gitignore to expose docs/cli/ as a tracked directory (docs/* glob replaces the previous docs/ directory ignore, preserving the ignore for docs/code-review/ and other scratch dirs while allowing docs/cli/ through). Links the new docs/cli/ index from README.md. Closes #1088
Add language tags to all 20 bare fenced code blocks in docs/cli/*.md (MD040) and replace the illustrative RSA private key marker in cloud-setup.md with a placeholder to pass detect-private-key.
The "Complete flag reference" claimed all ~35 root flags but omitted two real cmd/main.go flags: --regions (-r), the primary region scoping flag (referenced in filtering.md but never tabulated), and --coverage-lookback-days (default 30), which directly tunes the --target-coverage sizing path the docs cover in depth. Add both to the flag reference so the table matches the binary.
… deployment overview - Remove false claim of a "SaaS offering" and "Settings UI": CUDly is self-hosted only; there is no managed/hosted tier. - Replace all "CloudFormation stack" references with "Terraform deployment": the project deploys via Terraform (terraform/environments/ aws|gcp|azure) driven by GitHub Actions workflows, not CloudFormation. - Rewrite --stack-name flag description in both configure-azure and configure-gcp tables to describe what it actually does (naming prefix for Secrets Manager secret lookup). - Add "Deployment overview" section documenting the three Terraform environments (AWS Lambda, GCP Cloud Run, Azure Container Apps) and the ci-cd-permissions bootstrap/runtime IAM split. - Replace fabricated "When to use vs. SaaS onboarding" table with a minimal "Usage notes" table covering first-time setup and rotation.
cloud-setup.md "Deployment overview" had a near-verbatim copy of the runtime/Terraform table that now lives in README.md under "Deployment (self-hosted via Terraform)". Replace the duplicated table with a short forward-reference link to the README section; keep the CI/CD bootstrap note that is not in the README. No other material duplication was found: the CLI docs carry genuinely deeper detail (flag-by-flag reference with defaults and edge cases, configure-azure/configure-gcp subcommand flags, filtering semantics, purchase-safety mechanics) that the README does not cover.
Address CodeRabbit review findings on PR #1090: - README.md + filtering.md: when --regions is empty the code enumerates all opted-in regions via EC2 DescribeRegions (Savings Plans always query once via us-east-1); Cost Explorer-based discovery is only a fallback when region listing fails. --include/--exclude-regions filter fetched recommendations and do not constrain queried regions. - README.md + purchase-safety.md: document that --input-csv mode computes isDryRun = !ActualPurchase and ignores --dry-run, so --purchase alone performs real purchases there; split the decision formula per code path and add a CSV-mode warning example. - filtering.md: instance-type include/exclude lists match by exact string equality (slices.Contains); remove the incorrect "partial prefixes" claim. - purchase-safety.md: add the missing skipped_covered audit status (idempotency hit). - README.md: clarify the binary is built as cudly while the cobra root command is named ri-helper; fix the auto-generated CSV filename format (ri-helper-<mode>-<timestamp>.csv).
The --idempotency-window flag is accepted and validated by the CLI but its value is stored and never consumed by the purchase pipeline: cfg.IdempotencyWindow is set but no code in cmd/ reads it to deduct previously-purchased commitments from new recommendations. That deduction path runs in the server-side scheduler, not the CLI loop. Update purchase-safety.md and the flag reference table in README.md to describe what the flag actually does (accepted/validated, no-op in CLI) rather than claiming it subtracts prior purchases. Also correct the audit-status list to note that skipped_covered is a schema-level value used by the server path and is not emitted by CLI runs.
cristim
force-pushed
the
docs/cli-reference
branch
from
b912ab6 to
051bacf
Compare
Member
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
- filtering.md: account-name resolution uses organizations:DescribeAccount (per-ID + cached), not organizations:ListAccounts. IAM-policy authors granting only ListAccounts on the prior wording would have hit a permission gap. - filtering.md: scope the include/exclude conflict-check note to the three pairs actually validated at startup (regions, instance types, engines) and document that --include-accounts/--exclude-accounts and --include-sp-types/--exclude-sp-types fall through without that check, with exclude winning on overlap. - README.md + purchase-safety.md: drop the "validated as a Go duration string" claim for --idempotency-window; the CLI does not parse the value at startup, only the server-side scheduler does.
Member
Author
Adversarial review - PR #1090Reviewed against the stated risk surfaces (flag-doc accuracy, purchase-safety claims, cloud-setup samples, runnable examples, machine-specific paths, em-dashes, drift since open, base-branch). Net: three doc-accuracy fixes pushed in What I checked
Fixes pushed in
|
Member
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Contributor
|
(•ㅅ•)ゞ ✏️ Learnings added
✅ Action performedReview finished.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
docs/cli/hierarchy: index + three topic pages covering every CLI flag, the purchase-safety guardrails, and theconfigure-azure/configure-gcpsubcommands -- all previously absent from user-facing docs--min-savings-pctpercentage vs GUImin_savingsdollar naming distinction to prevent silent misconfiguration (gap analysis P3).gitignoresodocs/cli/is tracked whiledocs/code-review/and other scratch dirs remain ignoredREADME.mdFile tree created
Flags verified against source
All flags verified against
cmd/main.go(init() block),cmd/configure_azure.go,cmd/configure_gcp.go,cmd/validators.go, andcmd/multi_service.go.Test plan
docs/cli/files appear in the PR diff (not filtered by .gitignore)docs/code-review/files do NOT appear in the diffcmd/main.goinit() blockCloses #1088
Summary by CodeRabbit