fix(gcp): remove GroupCommitments, which merged machine families into one untyped commitment

[cristim]

Summary

Takes option 1 of #61: delete GroupCommitments and the CommitmentRequest / ResourceCommitment types it returns. They lived in providers/gcp/services/computeengine/client.go.

The function keyed requests on {account, region, term} only, and CommitmentRequest had no type field. An n2-standard-16 recommendation and a c3-highcpu-22 one were therefore summed into one request that named no commitment type. Nothing calls it, so nothing mis-purchases today. But an exported entry point to that result is a latent wrong purchase, and there is nothing to keep.

• client.go: -75. client_test.go: -100, the three TestGroupCommitments_* tests, which only exercised the deleted function.
• No other edits. The shared helpers termPlan and memoryMBFromDetails stay. Both still have callers and their own direct tests.

Closes #61

Consumers checked

Grepped cloud-commitments-{cli,mcp,platform} and their local clones for the symbols:

• No Go code imports them.
• The only mentions are platform docs. One is a resolved known-issues note. The other is audit finding A08-022 (docs/audits/codebase-audit-2026-09-02.md), which recommends this deletion.
• Older pinned providers/gcp versions in the module cache still contain the symbols. Nothing imports them, so no consumer breaks on a pin bump.

Verification (macOS, Go 1.26.9)

• make build and make vet pass.
• (cd providers/gcp && GOWORK=off go test -race -short ./...) passes.
• golangci-lint v2.10.1 (the CI pin) reports 0 issues.
• GOWORK=off go build ./... passes in providers/gcp and ci_cd_sanity_tests.
• An independent non-author review found no actionable findings on tree 7e69fecd. It covered remaining references across all five modules, external imports including the module-cache copies, and test coverage left for the shared helpers.

Overlap: open PR #307 (superseded by #401, pending close) also edits computeengine/client.go. Its hunks are around lines 465 and 1171-1187. This deletion is at lines 603-677, so they don't overlap.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Changes
  • GCP Compute recommendations are no longer grouped into commitment requests by account, region, and term.
  • The previous behavior of combining vCPU and memory amounts and skipping recommendations from other providers or services, with invalid terms, or without memory details has been removed. Workflows that relied on these grouped requests will no longer receive them.

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 2 billable files and costs up to $0.50.}

• 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 16 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 65 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-go/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: 861d4374-15fc-4457-9375-9d9cb15b270a

📥 Commits

Reviewing files that changed from the base of the PR and between 04faaee and 9372ae6.

📒 Files selected for processing: 2

• providers/gcp/services/computeengine/client.go
• providers/gcp/services/computeengine/client_test.go

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-go/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: 3f207692-e07b-4e95-b7a2-4e54a4d33ae2

📥 Commits

Reviewing files that changed from the base of the PR and between b69a17e and 04faaee.

📒 Files selected for processing: 2

• providers/gcp/services/computeengine/client.go
• providers/gcp/services/computeengine/client_test.go

💤 Files with no reviewable changes: 2

• providers/gcp/services/computeengine/client_test.go
• providers/gcp/services/computeengine/client.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

The GCP Compute client no longer exports ResourceCommitment, CommitmentRequest, or GroupCommitments. Tests for GroupCommitments were also removed.

Changes: GCP commitment aggregation API

Layer / File(s)	Summary
Remove commitment aggregation API providers/gcp/services/computeengine/client.go, providers/gcp/services/computeengine/client_test.go	The exported commitment types and GroupCommitments function were removed. Tests for grouping recommendations and skipping recommendations without memory details were removed.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 04faa

The removal addresses the reported risk of combining incompatible commitment recommendations. No actionable issue remains in the supplied review evidence; normal checks should still complete before merge.

Pre-merge checks: 5

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: removing GroupCommitments because it merged machine families into an untyped commitment. This matches the changeset and PR objective.
Linked Issues check		Direct issue #61 permits deletion of the uncalled function and its returned types. The whole-PR diff removes GroupCommitments, CommitmentRequest, and ResourceCommitment from client.go. It also…
Out of Scope Changes check		The diff changes only providers/gcp/services/computeengine/client.go and its test file. The implementation deletion directly addresses issue #61. The test deletions remove tests for the deleted API.…
Docstring Coverage		No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…

Full details: Linked Issues check

Explanation

Direct issue #61 permits deletion of the uncalled function and its returned types. The whole-PR diff removes GroupCommitments, CommitmentRequest, and ResourceCommitment from client.go. It also removes only the three tests that exercised the deleted API. This satisfies issue #61 option 1 and its acceptance criteria. No retained request can be produced by these declarations.

Full details: Out of Scope Changes check

Explanation

The diff changes only providers/gcp/services/computeengine/client.go and its test file. The implementation deletion directly addresses issue #61. The test deletions remove tests for the deleted API. No unrelated production code, helper, or other module changes are present.

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files.

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

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

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

[cristim]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cristim]

Independent review verdict: CLEAN at 624740c

Reviewer: fresh Claude agent (Sonnet, high effort), not the author. Coordinator: claude-cc-reviewer.

No remaining callers (git grep GroupCommitments):

Location	Hits
this head, all modules and build tags	0
go origin/main	only the removed definition (client.go:624) and its tests (client_test.go:1469-1510)
cli, mcp origin/main	0
platform origin/main	0 Go hits; docs only (docs/audits/codebase-audit-2026-09-02.md A08-022 recommends this exact deletion, and known_issues/11_gcp_provider.md:142,148)

• No pkg/ interface declares it and no mock implements it. CommitmentRequest/ResourceCommitment have no remaining references outside computepb.
• termPlan and memoryMBFromDetails stay live (client.go:808, 840, 1036; commitment_type.go:207).

Removal is the right fix (#61 option 1): the function keyed groups on account+region+term only, and CommitmentRequest had no Type, so it merged machine families into one untyped commitment. It also built names from time.Now().UnixNano(). The supported purchase path, buildInsertRequest (client.go:803), derives the type via commitmentTypeForMachineType. No needed functionality is lost.

Runs (providers/gcp, GOWORK=off): go build ./... and go vet ./... pass, and go test -count=1 ./services/computeengine/... passes 458.

Non-blocking: the platform's known_issues/11_gcp_provider.md and the audit doc still mention the function, and could be updated after merge.

Gate:

• Labels are mirrored from fix(gcp): GroupCommitments merges different machine families into one untyped commitment #61.
• Merge waits on CI being green at this SHA. (claude-cc-reviewer)

[cristim]

CI is green on head 624740c8: pre-commit 38925629917 and Build & Test 38925629831.

[cristim]

Rebased onto main (server-side gh pr update-branch --rebase; the branch was BEHIND). git range-diff: 1: 624740c = 1: 2c84a42, so the commit is identical. The CLEAN verdict carries over to 2c84a42. Merge waits on CI at this head. (claude-cc-reviewer)

[cristim]

Rebased onto main again (BEHIND after another go merge). git range-diff: 1: 2c84a42 = 1: 3001f28, so the commit is identical. The CLEAN verdict carries over to 3001f28. (claude-cc-reviewer)

[cristim]

Rebased onto main again (BEHIND after #447). The range-diff is identical, so the CLEAN verdict carries over to 04faaee. (claude-cc-reviewer)

[cristim]

Rebased again (BEHIND). The range-diff is identical, so the verdict carries over to f1b1879. (claude-cc-reviewer)

[cristim]

Pure rebase onto main after go#449 merged. git range-diff of reviewed f1b1879 against new head 51829d9 shows the commit as identical (=), so the CLEAN verdict carries to 51829d9. Merge after CI is green. (claude-cc-reviewer)

[cristim]

Pure rebase onto main after go#448 merged. git range-diff of 51829d9 against new head 10aac4f shows the commit as identical (=), so the CLEAN verdict carries to 10aac4f. (claude-cc-reviewer)

[cristim]

Pure rebase onto main after go#456 merged. git range-diff of 10aac4f against new head 9372ae6 shows the commit as identical (=), so the CLEAN verdict carries. (claude-cc-reviewer)