fix review comments

Signed-off-by: roytman <roytman@il.ibm.com>

Author: roytman

docs/coordinator_architecture.md

@@ -131,8 +131,8 @@ multimodal item in parallel. The number and shape of these requests depend on th
 steps are enabled and how they are parameterized); text-only requests need no media
 download or encode at all. Across phases the coordinator sequences the round-trips
 (prefill and decode are one each), threading state from each response into the next
-request. Each Gateway call routes it by the `EPP-Phase` header to
-the matching per-phase EPP and then to a pod in that phase's pool.
+request. Each Gateway call routes it by the `EPP-Phase` header to the EPP, which runs
+the matching scheduling profile and picks a pod from that phase's pool.
 
 Steps are skipped at runtime when they do not apply (for example, `encode` is a no-op
 when the request has no multimodal entries, and `render` short-circuits when the
@@ -362,7 +362,7 @@ service in front of the Inference Gateway:
 | Pipeline versatility | Fixed E/P/D orchestration baked into the sidecar | Configurable pipeline of independent, reorderable plugin steps; new stages added without touching existing ones |
 | EPP scheduling | One cycle selects all phases (`disagg-profile-handler`) | One EPP call per phase, coordinator drives the cascade |
 | vLLM pod selection | All phase pods chosen up front in one scheduling cycle | Deferred per phase: each pod is selected only when that phase's call is made, at the point its destination becomes relevant |
-| Phase selection signal | EPP request headers `x-prefiller-host-port`, `x-encoder-hosts-ports` read by the sidecar | `EPP-Phase` header per call; per-phase EPP picks the pod |
+| Phase selection signal | EPP request headers `x-prefiller-host-port`, `x-encoder-hosts-ports` read by the sidecar | `EPP-Phase` header per call; the EPP runs the matching profile and picks the pod |
 | Tokenization | On the workers | Once, in the coordinator's render step; token IDs reused downstream (experimental path) |
 | Cross-phase state | Held by the sidecar | Held on the coordinator `RequestContext` |
 

pkg/epp/framework/plugins/scheduling/profilehandler/headerphase/README.md

@@ -13,9 +13,11 @@ Reads the configured header from the incoming request and looks up the
 `schedulingProfiles` entry with that exact name:
 
 - If a matching profile hasn't run yet, it runs that profile alone.
-- If the header is missing or names a profile that isn't configured, no profile runs; the
-  request fails with the scheduler's generic "failed to run any scheduler profile" error,
-  and the EPP logs the specific reason (missing header vs. unconfigured value).
+- If the header is missing or names a profile that isn't configured, no profile runs. The
+  request fails with the scheduler's own generic "no profile ran" error (a 429 to the
+  client, since the scheduler doesn't distinguish a malformed request from exhausted
+  capacity); the EPP logs the specific reason (missing header vs. unconfigured value) for
+  operators.
 
 This differs from the [disagg profile handler](../disagg/README.md), which decides which
 profiles to run via decider plugins rather than a header.

pkg/epp/framework/plugins/scheduling/profilehandler/headerphase/header_phase_profile_handler.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2026 The Kubernetes Authors.
+Copyright 2026 The llm-d Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -42,7 +42,7 @@ var _ fwksched.ProfileHandler = &HeaderPhaseProfileHandler{}
 // parameters configures the HeaderPhaseProfileHandler.
 type parameters struct {
 	// HeaderName is the request header whose value names the scheduling profile to run.
-	// Defaults to "EPP-Phase" when empty.
+	// Defaults to defaultHeaderName when empty.
 	HeaderName string `json:"headerName"`
 }
 
@@ -55,22 +55,24 @@ func Factory(name string, rawParameters *json.Decoder, _ fwkplugin.Handle) (fwkp
 		}
 	}
 
-	headerName := strings.TrimSpace(params.HeaderName)
-	if headerName == "" {
-		headerName = defaultHeaderName
-	}
-
-	return NewHeaderPhaseProfileHandler(headerName).WithName(name), nil
+	return NewHeaderPhaseProfileHandler(params.HeaderName).WithName(name), nil
 }
 
 // NewHeaderPhaseProfileHandler initializes a new HeaderPhaseProfileHandler and returns
-// its pointer. headerName is lowercased and trimmed: the EPP's request handler lowercases
-// every incoming header name at ingestion (pkg/epp/handlers/request.go), so the configured
-// name must be normalized the same way to match.
+// its pointer. headerName is lowercased and trimmed, falling back to defaultHeaderName
+// when that leaves it empty: the EPP's request handler lowercases every incoming header
+// name at ingestion (pkg/epp/handlers/request.go), so the configured name must be
+// normalized the same way to match, and an empty header key would never match any
+// request.
 func NewHeaderPhaseProfileHandler(headerName string) *HeaderPhaseProfileHandler {
+	headerName = strings.ToLower(strings.TrimSpace(headerName))
+	if headerName == "" {
+		headerName = strings.ToLower(defaultHeaderName)
+	}
+
 	return &HeaderPhaseProfileHandler{
 		typedName:  fwkplugin.TypedName{Type: HeaderPhaseProfileHandlerType, Name: HeaderPhaseProfileHandlerType},
-		headerName: strings.ToLower(strings.TrimSpace(headerName)),
+		headerName: headerName,
 	}
 }
 
@@ -117,9 +119,15 @@ func (h *HeaderPhaseProfileHandler) noMatchError(phase string) error {
 // Pick selects the single SchedulingProfile named by the request's phase header. It
 // returns an empty map once that profile has run, or when the header is missing or
 // names a profile that isn't configured. In the latter case the scheduler's run loop
-// (pkg/epp/scheduling.Scheduler.Schedule) stops without ever calling ProcessResults,
-// since no profile ever ran, so the specific reason is logged here rather than
-// returned from ProcessResults, where it would be unreachable in practice.
+// (pkg/epp/scheduling.Scheduler.Schedule) stops without ever calling ProcessResults, so
+// the specific reason is logged here rather than returned from ProcessResults, where it
+// would be unreachable. The client never sees that reason: it only gets the scheduler's
+// generic "failed to run any scheduler profile" error, which
+// pkg/epp/requestcontrol/director.go maps to a 429 ResourceExhausted response -
+// misleading, since a malformed or missing header is a client error, not a capacity
+// problem. Surfacing the real reason to the client needs a scheduler/ProfileHandler
+// contract change and is out of scope here; the log is a diagnostic aid for operators,
+// not an equivalent substitute for what the caller receives.
 func (h *HeaderPhaseProfileHandler) Pick(ctx context.Context, request *fwksched.InferenceRequest, profiles map[string]fwksched.SchedulerProfile,
 	profileResults map[string]*fwksched.ProfileRunResult) map[string]fwksched.SchedulerProfile {
 	// TODO(#2135): single profile per request; extend to loop over an ordered phase

pkg/epp/framework/plugins/scheduling/profilehandler/headerphase/header_phase_profile_handler_test.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2026 The Kubernetes Authors.
+Copyright 2026 The llm-d Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -52,11 +52,22 @@ func TestNewHeaderPhaseProfileHandler(t *testing.T) {
 	}
 }
 
+func TestNewHeaderPhaseProfileHandlerEmptyFallsBackToDefault(t *testing.T) {
+	// The constructor itself must uphold the "never empty" invariant, independent of
+	// Factory: an empty headerName must not produce a handler that can never match any
+	// request (request.Headers[""] is always empty).
+	handler := NewHeaderPhaseProfileHandler("")
+	if handler.headerName != ingestedHeaderKey {
+		t.Errorf("Expected headerName %q, got %q", ingestedHeaderKey, handler.headerName)
+	}
+}
+
 func TestHeaderPhaseProfileHandlerFactory(t *testing.T) {
 	tests := []struct {
 		name           string
 		rawParameters  string
 		wantHeaderName string
+		wantErr        bool
 	}{
 		{
 			name:           "no parameters, uses default header, normalized to lowercase",
@@ -78,6 +89,11 @@ func TestHeaderPhaseProfileHandlerFactory(t *testing.T) {
 			rawParameters:  `{"headerName": "   "}`,
 			wantHeaderName: ingestedHeaderKey,
 		},
+		{
+			name:          "malformed json returns an error",
+			rawParameters: `{invalid}`,
+			wantErr:       true,
+		},
 	}
 
 	for _, tt := range tests {
@@ -87,6 +103,12 @@ func TestHeaderPhaseProfileHandlerFactory(t *testing.T) {
 			decoder := fwkplugin.StrictDecoder(json.RawMessage(tt.rawParameters))
 
 			plugin, err := Factory("custom-name", decoder, nil)
+			if tt.wantErr {
+				if err == nil {
+					t.Fatalf("Factory() expected error, got nil")
+				}
+				return
+			}
 			if err != nil {
 				t.Fatalf("Factory() returned unexpected error: %v", err)
 			}
@@ -213,6 +235,18 @@ func TestHeaderPhasePick(t *testing.T) {
 			profileResults: map[string]*fwksched.ProfileRunResult{},
 			wantProfiles:   map[string]fwksched.SchedulerProfile{},
 		},
+		{
+			// Unlike the header name, the header value is matched case-sensitively
+			// against the configured profile names: only the name gets normalized to
+			// match how the EPP's request handler stores it, since that's a fixed key
+			// this plugin controls, but the value is caller-supplied and compared
+			// verbatim against schedulingProfiles names.
+			name:           "header value case does not match the configured profile name",
+			request:        &fwksched.InferenceRequest{Headers: map[string]string{ingestedHeaderKey: "Encode"}},
+			profiles:       profiles,
+			profileResults: map[string]*fwksched.ProfileRunResult{},
+			wantProfiles:   map[string]fwksched.SchedulerProfile{},
+		},
 	}
 
 	handler := NewHeaderPhaseProfileHandler(defaultHeaderName)
@@ -283,14 +317,16 @@ func TestHeaderPhaseProcessResults(t *testing.T) {
 				"encode": successResult,
 				"decode": successResult,
 			},
-			wantErr: true,
+			wantErr:         true,
+			wantErrContains: "is intended to run a single profile per request, got 2",
 		},
 		{
 			name: "nil result (profile execution failure) returns error",
 			profileResults: map[string]*fwksched.ProfileRunResult{
 				"encode": nil,
 			},
-			wantErr: true,
+			wantErr:         true,
+			wantErrContains: "failed to run scheduler profile 'encode'",
 		},
 	}