Skip to content

chore(cli): delete the unused legacy processService purchase path - #2254

Merged
cristim merged 1 commit into
mainfrom
chore/delete-legacy-processservice
Oct 17, 2026
Merged

cristim merged 1 commit into
mainfrom
chore/delete-legacy-processservice

Conversation

@cristim

@cristim cristim commented Oct 17, 2026

Copy link
Copy Markdown
Member

Closes #2110
Closes #2053

Summary

This PR deletes the legacy per-region purchase path. Only tests called it. It also carried its own purchase logic, so if anything called it again, it would buy without the up-front confirmPurchaseRun confirmation (#2110).

Removed:

  • processService (cmd/multi_service.go).
  • processRegionRecommendations and the regionRecommendations type (cmd/multi_service_helpers.go). This includes its per-region --max-instances refusal, which only made sense on that path.
  • The test-only applyCommonCoverage wrapper and the unread Config.Providers field (chore(cli): unread Config.Providers field and the test-only legacy processService path #2053). The field had no flag, no setter and no reader.
  • The tests that only exercised this path, and the imports that became unused.

Kept: processPurchaseLoop, which the live pipeline uses (cmd/multi_service.go:873).

Two pieces of coverage moved instead of disappearing:

  • TestProcessService_WithOverrideCount was the only test of the override branch in applyCoverageAndOverrides. It is replaced by TestApplyCoverageAndOverrides_OverrideCount, so the function is back to 100%.
  • TestApplyCommonCoverage now calls ApplyCoverage directly, renamed TestApplyCoverage_FloorsPerRecommendationCounts.

Evidence (local, macOS, GOTOOLCHAIN=go1.26.9)

  • No callers: git grep finds no reference anywhere in the repo to any deleted identifier: processService, processRegionRecommendations, regionRecommendations, applyCommonCoverage, .Providers / Providers:. cmd/ has no build tags.
  • Coverage: cmd coverage went from 82.94% to 82.96%. The shared code the deleted tests touched is still at 100%: processPurchaseLoop, checkDuplicates, adjustRecsForDuplicates and populateRecommendationAccountNames. The --max-instances live path keeps its own test files.
  • Checks:
    • make build and go vet ./... pass.
    • go test -race ./cmd/ passes 1581 tests.
    • golangci-lint v2.10.1 reports 0 issues.
    • scripts/test-git-secrets-allowlist.sh passes 44/44.
  • Review: an independent non-author reviewer (sonnet, high) did two passes. The only findings were a stale comment and a test count in the commit message, both fixed.

🤖 Generated with Claude Code

processService and processRegionRecommendations (plus the
regionRecommendations type) had no production caller; only tests drove
them. Since #2105 moved confirmation to confirmPurchaseRun they would
purchase without confirmation if anything called them again. Delete them
and the tests that only exercised them.

Also drop the test-only applyCommonCoverage wrapper (its assertions now
call ApplyCoverage directly) and the never-read Config.Providers field.
The override-count assertion moves to a test of applyCoverageAndOverrides,
which the live pipeline shares. processPurchaseLoop stays: the live
pipeline and --input-csv use it.

Closes #2110
Closes #2053

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cristim cristim added priority/p2 Backlog-worthy urgency/this-quarter Within the quarter impact/internal Team-internal only effort/xs Trivial / one-liner effort/s Hours type/security Security finding type/chore Maintenance / non-user-visible severity/low Minor harm triaged Item has been triaged labels Oct 17, 2026
@coderabbitai

coderabbitai Bot commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

  • Run on-demand review

This review includes 6 billable files and costs up to $1.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 50 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 73 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-cli/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: 3fe8d138-1893-46f8-9c94-74b9af1c811e
📥 Commits

Reviewing files that changed from the base of the PR and between 7e885a2 and 58c16b3.

📒 Files selected for processing: 6
  • cmd/main.go
  • cmd/multi_service.go
  • cmd/multi_service_coverage_test.go
  • cmd/multi_service_helpers.go
  • cmd/multi_service_helpers_test.go
  • cmd/multi_service_test.go
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@cristim cristim added severity/medium Moderate harm and removed severity/low Minor harm labels Oct 17, 2026
@cristim

cristim commented Oct 17, 2026

Copy link
Copy Markdown
Member Author

Independent review verdict: CLEAN at 58c16b3

The reviewer was a fresh Claude agent (Sonnet 5.5, high effort), not the author, with a cold brief. The PR body and issues were treated as claims. Coordinator: claude-cc-reviewer.

Blocking findings: none.

The deleted code was dead:

  • Deleted: processService, processRegionRecommendations, regionRecommendations, applyCommonCoverage and Config.Providers.
  • A git grep of the head tree finds no remaining references in code, docs, CHANGELOG or comments.
  • go build and go vet pass. Build tags appear only in test files, and there is no reflection or string dispatch.
  • The helpers it called (determineRegionsForService, applyRegionFilters, createServiceClient, checkDuplicates) are still used, and lint reports none unused.

No safety check is lost from the live path:

Other edits:

  • Only two lines are not deletions: one comment and one direct ApplyCoverage test. Flags, defaults, help text and exit codes are unchanged.
  • go test -cover ./cmd/ gives 82.9% on main and 83.0% at head. Per function, only the deleted functions disappear, and coverage of the remaining helpers is unchanged.

Local verification (tests only, no cloud calls):

  • make build succeeds and go vet ./... is clean.
  • go test -race ./... -count=1 passes.
  • golangci-lint v2.10.1 reports 0 issues.
  • There is no runtime behaviour to exercise, because only dead code was removed.

Gate:

@cristim
cristim merged commit ee2b597 into main Oct 17, 2026
12 checks passed
@cristim
cristim deleted the chore/delete-legacy-processservice branch October 17, 2026 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/s Hours effort/xs Trivial / one-liner impact/internal Team-internal only priority/p2 Backlog-worthy severity/medium Moderate harm triaged Item has been triaged type/chore Maintenance / non-user-visible type/security Security finding urgency/this-quarter Within the quarter

Projects

None yet

1 participant