Skip to content

chore: rename internal/saturation to internal/saturationv1#1450

Open
mamy-CS wants to merge 1 commit into
llm-d:mainfrom
mamy-CS:chore/rename-saturation-v1-package
Open

chore: rename internal/saturation to internal/saturationv1#1450
mamy-CS wants to merge 1 commit into
llm-d:mainfrom
mamy-CS:chore/rename-saturation-v1-package

Conversation

@mamy-CS

@mamy-CS mamy-CS commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Relates to #1402 (Section 4 of 7: resolve the saturation package-name collision)

Proposed Changes

  • 🧹 Rename package internal/saturationinternal/saturationv1 (3 files moved, byte-identical except the package declaration)
  • 🧹 Update the 3 importers (collector/replica_metrics.go, engines/pipeline/enforcer.go, engines/saturation/engine.go): import paths + 6 saturation.Xsaturationv1.X references
  • 🧹 Point the V1 deprecation note in engine.go at its tracking issue (Track: deprecate & remove the V1 (percentage-based) saturation analyzer #1441) instead of a vague standalone line

Why

internal/saturation (V1 analyzer) and internal/engines/saturation (the Engine) were both package saturation. Because engine.go (itself package saturation) imported the V1 package unaliased, saturation.NewAnalyzer() in that file resolved to the imported package; easy to misread. After this rename, the only remaining package saturation is the Engine, so the ambiguity is gone.

V1 is deliberately not moved under internal/engines/analyzers/ alongside throughput/queueing-model/saturation-v2: unlike those, V1 does not implement the Analyzer interface (its surface is AnalyzeModelSaturation / CalculateSaturationTargets), so grouping it there would imply a peer relationship that doesn't exist. It stays the default analyzer path; eventual removal is tracked in #1441.

Pre-review Checklist

  • E2E tests for any new behavior — N/A, pure rename, no behavior change
  • Docs PR for any user-facing impact — N/A, internal-only, no user-facing surface touched
  • Proposal PR for any new enhancement or change to existing behavior — N/A, no behavior change

Release Note

NONE

Docs

None needed. V1 removal is tracked separately in #1441.

Verification for reviewers: git tracks all 3 as renames (history preserved); the moved files are byte-identical to their originals except the package line; the only remaining package saturation is internal/engines/saturation; no stale internal/saturation references anywhere. go build, go vet, gofmt -l (clean), make lint (0 issues), and the full non-e2e go test suite all pass.

Signed-off-by: Mohammed Abdi <mohammed.munir.abdi@ibm.com>
@asm582

asm582 commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

@mamy-CS saturationv1 is no longer the default; this code should be removed.

@mamy-CS

mamy-CS commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator Author

Collaborator

Yes, I was going to make it easier for the removal by explicitly renaming it. The v1 removal tracker is here, #1441) Although v1 is still going to stay selectable in v0.9 per the issue tracker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants