fix(recommendations): preserve incomplete AWS sweeps

[cristim]

Incomplete AWS recommendation sweeps can return valid rows alongside an error. Preserve those rows while excluding the incomplete account from stale-row eviction. When retrying with configured parameters, query each explicit service and retain incomplete status across the fallback results.

Refs LeanerCloud/cloud-commitments-go#54. The parent remains open until the CLI, MCP, and Platform rollout is complete.

Local verification exercised the published AWS adapter, synthetic SDK HTTP responses, actual collection and PostgreSQL persistence across 11 scenarios. Baseline and mutation probes demonstrate the survivor, fallback-parameter, and eviction assertions. Full backend race tests, build, vet, pinned lint, and normal installed hooks passed. Independent gpt-6-astra source and staged reviews found no actionable findings. No live cloud calls were made.

Independent final review approved exact committed SHA 9a40c12dc7881439f011bd6bdacdf6f1c565bf36. Fresh race-enabled connected PostgreSQL verification passed all 11 cases (25.740s). Published AWS/pkg module selection at that SHA passed with no replacements. The unrelated OpenTelemetry advisory is tracked by the active repair plan; required CI must pass before merge.

Summary by CodeRabbit

• Bug Fixes
  • AWS recommendation sweeps now retain collected results when some recommendation details or service scopes fail, without treating the incomplete sweep as authorization to remove existing offers.
  • When the initial AWS recommendation fetch returns no results, fallback requests are made separately for each supported service. Results from successful requests are retained, while errors from failed requests continue to be reported.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Warning

Review limit reached

• Run on-demand review

_{This review includes 5 billable files and costs up to $1.25.}

• Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 51 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 83 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: 0e18139e-8f1d-4325-9ee2-a36830bef2a6

📥 Commits

Reviewing files that changed from the base of the PR and between 9a40c12 and ed80d76.

⛔ Files ignored due to path filters (1)

• go.sum is excluded by !**/*.sum

📒 Files selected for processing (5)

• go.mod
• internal/scheduler/partial_sweep_eviction_test.go
• internal/scheduler/recommendation_completeness_integration_test.go
• internal/scheduler/scheduler.go
• internal/scheduler/scheduler_test.go

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: 2fd0a4ff-01cf-4bae-94ad-eb202e79e9d5

📥 Commits

Reviewing files that changed from the base of the PR and between 6d9a70f and 9a40c12.

⛔ Files ignored due to path filters (1)

• go.sum is excluded by !**/*.sum

📒 Files selected for processing (5)

• go.mod
• internal/scheduler/partial_sweep_eviction_test.go
• internal/scheduler/recommendation_completeness_integration_test.go
• internal/scheduler/scheduler.go
• internal/scheduler/scheduler_test.go

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

---

📝 Walkthrough

Walkthrough

The scheduler now treats AWS incomplete-recommendation errors as nonfatal while marking the sweep incomplete. When the initial fetch returns no recommendations, it requests fallback recommendations separately for each supported service. Tests cover error handling, fallback requests, and collection persistence.

Changes

AWS recommendation collection

Layer / File(s)	Summary
Handle incomplete sweeps go.mod, internal/scheduler/scheduler.go, internal/scheduler/partial_sweep_eviction_test.go, internal/scheduler/recommendation_completeness_integration_test.go	The scheduler recognizes AWS IncompleteRecommendationsError values, logs failed-detail and failed-scope counts, and returns an incomplete status without a fatal error. Tests cover direct and wrapped errors, completeness, and persistence effects. The AWS modules use updated pseudo-versions.
Request fallback per service internal/scheduler/scheduler.go, internal/scheduler/scheduler_test.go	When the initial fetch returns no recommendations, the scheduler requests fallback recommendations separately for each supported service. It appends results and combines completeness across requests. Tests check service and request parameters.

Priority: ⬇️ Low

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 9a40c

Incomplete AWS sweeps retain valid recommendations without evicting stale rows for the affected account. No identified issue blocks merge after the required checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 4 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 summarizes the main change: preserving incomplete AWS recommendation sweeps.
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 42.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 4 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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

[cristim]

Platform #472 independent exact-head review

Reviewed head: ed80d762cd269b45482f57296306611d966beb8b.
Reviewed base: fa8c39e77ee917723ef6f140441ac4559e22e8fb.
Reviewer: gpt-6-astra, independent of author; same review stream resumed with full source re-read. User authorized this reviewer and synthetic connected local verification instead of unavailable exact Opus/live-cloud gates.
Date: 2026-10-05.

Verdict: no actionable findings. Exact-head source review and fresh local verification pass. This supersedes the prior 9a40 source/runtime approval only for the named head; publication or merge still requires the owner's current remote-head/CI/gate checks. Reviewer performed no public writes or merge.

Source review

Re-read the complete six-file base-to-head diff, 360 additions and 28 deletions, applicable committed CLAUDE.md, test assertions, and actual published SDK contracts. Covered completeness, correctness, security, bugs, duplication, scope, and over-engineering, with particular attention to surviving monetary recommendations and account/provider-scoped eviction.

• internal/scheduler/scheduler.go:988 retains typed AWS incomplete results, returning incomplete status; nil, ordinary error and cancellation behavior follows the existing path. Published SDK recommendations/collection_error.go, recommendations/client.go:294 and :427 separate cancellation from incomplete diagnostics. service_client.go:76 preserves filtered survivors plus their diagnostic.
• scheduler.go:1027 enumerates the same six services as actual SDK fanout: EC2, RDS, ElastiCache, OpenSearch, Redshift, SavingsPlansAll. Fallback uses configured lookback/term/payment and accumulates completeness with AND, preventing a later complete response from authorizing eviction after an earlier incomplete response.
• Existing ambient and registered-account collection paths retain survivors while withholding their account's eviction eligibility; the existing PostgresStore upsert uses provider/account-key scoped eviction. Integration assertions check surviving rows, their account IDs, old-row retention, clean sibling eviction and unswept-provider retention.
• The integration HTTP fixture exercises the published SDK boundary and production scheduler/store; its fallback assertions check all five RI names and four Savings Plans types plus configured parameters. Unit cases cover wrapped diagnostics, ordinary errors, cancellation/deadline, and three incomplete-fallback combinations.
• No production purchase, auth, migration, tenant boundary, or Azure/GCP provider pin change appears in the PR diff. No new general-purpose production abstraction is introduced.

Head is a merge of old reviewed 9a40 and base fa8c39. Explicit git comparison confirms zero changes from 9a40 to ed80 across internal/scheduler, internal/config, internal/database and CLAUDE.md. Merged base adds OTel 1.45/logr upgrades and unrelated frontend/Docker/Terraform changes. Those make an old binary insufficient; all runtime evidence below was freshly compiled/run against ed80. git diff --check passed. Initial read-only object review used another session's checkout at 8ce601e but read ed80 explicitly through git objects; runtime exclusively used the owned ed80 clone.

Fresh execution evidence

Runtime tree: /Users/cristi/.claude/worktrees/platform472-final-20261005, clean detached ed80 before/after. Runner run.py holds the canonical cloud-commitments-platform-test-suite OS lock. script-review.md records three clean pre-execution reviews and the reviewed retry changes. Per-run *-manifest.json records exact argv, environment, SHA and SHA256 hashes of runner, overlay, native helper, go.mod and go.sum. Per-run *-result.json records process exit, SHA after execution and clean status.

• modules.log, exit 0: published pkg b3b4cb5e3d80, AWS 006ef5c8d0a2, OTel 1.45.0. No Replace fields. GOWORK=off, GOPROXY=off, GOSUMDB=off, fixed Go1.26.6, readonly modules.
• unit-loopback.log, exit 0: go test -mod=readonly -p=1 -race -count=1 -short ./internal/scheduler. The initial unit.log was denied a local httptest bind by the sandbox; the separately recorded approved loopback retry passed.
• integration-address.log, exit 0: go test -mod=readonly -p=1 -race -count=1 -tags=integration -overlay=<overlay.json> -run=^TestAWSRecommendationCompletenessPersistence$ -v ./internal/scheduler. All 11 subcases PASS; package runtime 8.769s. I observed terminal process exit 0 in session 57991, then re-read result files and clean status.

Native overlay replaces ONLY internal/database/postgres/testhelpers/postgres.go; scheduler, SDK, assertions, migrations and persistence code remain unmodified. Before creating any database it verifies server data_directory, host 127.0.0.1, port 57230, and PostgreSQL 17.11. Each case creates a fresh quoted platform472_<UUID> database; all 11 are retained and their names are in the log. Cleanup closes only connections; truncate and reset methods refuse operation. No existing database/table was truncated or dropped. Initial integration.log failed the endpoint guard before CREATE DATABASE because inet text included /32; the revised host() extraction retains exact address validation. That setup failure is not application-failure evidence.

Prior parent and one-line AND/fatal mutant evidence remains supplementary: the scheduler/test files are byte-identical across the merge, but those probes were not rerun on ed80 and are not represented as fresh final-head executions.

Limits

This is synthetic AWS HTTP data through the real SDK and production scheduler into real local PostgreSQL 17.11, with a native helper replacing the committed Docker PostgreSQL16 helper. It is not a live-cloud or Docker-parity claim. Empty HOME, synthetic credentials, disabled metadata, rejection proxy, and offline modules avoid real cloud credentials/APIs. No full application build, whole-repository test run, browser, outer scheduled process, real purchase, or deployment was performed in this final-head review. Diagnostic log text is visible but not asserted. Root must independently confirm current remote SHA, current CI and any other merge gates.