chore(db): drop deprecated (aws, sagemaker) service_configs row (closes #133)

[cristim]

Summary

Closes #133.

Migration 000040 (PR #123) split (aws, savings-plans) into four
per-plan-type rows and copied PR #71's (aws, sagemaker) term/payment
forward into (aws, savings-plans-sagemaker), but intentionally KEPT
the deprecated (aws, sagemaker) row for one stable release as a
backward-compat safety net (see the Deprecation follow-up comment at
the bottom of 000040_split_savingsplans.up.sql). This PR ships the
follow-up migration that removes that deprecated row now that the
per-plan-type cards are canonical.

Gap

(aws, sagemaker) still exists in service_configs after PR #123
landed. Per-plan-type cards now drive SageMaker SP defaults via the
(aws, savings-plans-sagemaker) row, so the legacy row is dead state.

Fix

New migration 000045_drop_aws_sagemaker_service_config.{up,down}.sql:

• Up: idempotent DELETE FROM service_configs WHERE provider = 'aws' AND service = 'sagemaker'. WHERE clause makes it a safe no-op on
  installations that never had PR ux(settings): add SageMaker and Lambda purchasing-defaults cards #71's row, or that already ran this
  migration.
• Down: emergency rollback that recreates the row by
  INSERT...SELECT from (aws, savings-plans-sagemaker), lossy by
  design (only post-040 term/payment survive — any divergent edits to
  (aws, sagemaker) between 000040 and 000045 are not recoverable).
  Idempotent in two directions:
  • Empty source row → no-op INSERT (graceful when 000040 was rolled
    back first).
  • Existing target row → ON CONFLICT (provider, service) DO NOTHING
    keeps it untouched.

Also updates split_savingsplans_test.go scenario 2 (integration-tagged
test) to reflect the new post-migration state: (aws, sagemaker) is now
absent after running all migrations. The "sagemaker slot inherits 1,
partial-upfront" assertion still holds because 000040 runs before
000045 and copies the values forward into the persisted split row
before the source row is deleted.

Blast-radius check (production code paths)

Grepped internal/, pkg/, cmd/ (excluding _test.go) for
references to the bare 'sagemaker' service slug — zero hits. All
production references are to:

• pkg/common/types.go: ServiceSavingsPlansSageMaker = "savings-plans-sagemaker"
• cmd/main.go, internal/purchase/execution.go: maps from
  "savings-plans-sagemaker" and "savingsplans-sagemaker" to the
  typed constant.
• internal/config/defaults.go, internal/config/README.md:
  aws.savings_plans.sagemaker_enabled config key (independent of the
  service_configs row).

The only callers of the bare (aws, sagemaker) service_configs row
were the SQL paths inside migration 000040 (now historic), which read
it during the split and are not re-run.

Test plan

• go build ./... — green.
• go vet -tags=integration ./internal/database/postgres/migrations/... — green; integration-tagged test compiles with the updated scenario 2 expectations.
• Production-code grep for bare 'sagemaker' — empty.
• (CI) integration test suite runs scenario 1 / 2 / 3 of TestMigration_SplitSavingsPlans plus the JSONB rewrite test against a real Postgres container; scenario 2 now requires 4 rows (down from 5) and hasSagemaker == false.
• (Operator) on rollback past 000045, the down migration recreates (aws, sagemaker) from the current (aws, savings-plans-sagemaker) row.

Summary by CodeRabbit

• Chores
  • Removed deprecated AWS SageMaker service configuration from the database.

[cristim]

@coderabbitai review

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0db80bb6-bda0-4b5f-9a66-3170c813b1c2

📥 Commits

Reviewing files that changed from the base of the PR and between c84fd02 and 901c114.

📒 Files selected for processing (3)

• internal/database/postgres/migrations/000045_drop_aws_sagemaker_service_config.down.sql
• internal/database/postgres/migrations/000045_drop_aws_sagemaker_service_config.up.sql
• internal/database/postgres/migrations/split_savingsplans_test.go

---

📝 Walkthrough

Walkthrough

Migration 000045 removes the deprecated (aws, sagemaker) service config row that was retained for backward compatibility after migration 000040 split legacy entries. The up migration deletes the row idempotently; the down migration recreates it from the savings-plans-sagemaker row for rollback. Tests are updated to expect four rows instead of five and verify the sagemaker row is deleted.

Changes

Database Cleanup Migration

Layer / File(s)	Summary
Up Migration internal/database/postgres/migrations/000045_drop_aws_sagemaker_service_config.up.sql	Deletes the deprecated (aws, sagemaker) row from service_configs with idempotency guarantees and inline documentation of migration context.
Down Migration internal/database/postgres/migrations/000045_drop_aws_sagemaker_service_config.down.sql	Recreates (aws, sagemaker) from the existing (aws, savings-plans-sagemaker) row using ON CONFLICT DO NOTHING to prevent overwriting on repeated runs; intentionally lossy (preserves only term and payment).
Test Updates internal/database/postgres/migrations/split_savingsplans_test.go	Test expectations updated: scenario 2 now expects 4 rows (down from 5) and asserts that the sagemaker row is deleted; comments clarified to reflect 000040's inheritance preceding 000045's deletion.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

triaged, priority/p3, severity/low, urgency/this-sprint, impact/few, effort/xs, type/chore

Poem

🐰 A row once kept for gentle grace,
Now finds its time and takes its place,
Savings-plans steps up to stand,
As sagemaker bids farewell—well-planned!
Idempotent, reversible, and clean, ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: adding a migration to drop the deprecated (aws, sagemaker) service_configs row, which is the primary objective of this PR.
Linked Issues check	✅ Passed	The PR fulfills all coding requirements from issue #133: adds an idempotent up migration deleting the row, provides a down migration using INSERT...SELECT with ON CONFLICT DO NOTHING, and updates tests accordingly.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to issue #133: two migration SQL files (up/down) and one test file update reflecting the removal of the deprecated row.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch chore/issue-133-drop-sagemaker

---

_{Review rate limit: 4/5 reviews remaining, refill in 12 minutes.}

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

[coderabbitai]

✅ Actions performed

Review triggered.

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]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

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]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

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.