Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/contributor/validator.md`:
- Around line 160-172: Update the example under validation.performance so the
inference-ttft-p99 threshold matches the later examples/overlay guidance; change
the value for the key inference-ttft-p99 in the YAML block to use the unified
threshold (e.g., "<= 1000" or "<= 2000" as used elsewhere) so it no longer
conflicts with the rest of the doc.

In `@docs/user/validation.md`:
- Around line 169-170: Update the wording in docs/user/validation.md where the
CLI example shows "aicr validate --data <overlay>" to correctly indicate the
expected external data directory (use "aicr validate --data <dir>" or similar),
i.e., replace "<overlay>" with "<dir>" in the paragraph referencing the `aicr
validate --data` usage so it clearly reflects that `--data` expects a directory
path rather than an overlay identifier.

In `@pkg/validator/v1/job_plan_internal.go`:
- Around line 104-110: The code appends catalog-provided env vars after the
orchestrator-forwarded HF_TOKEN, allowing entry.Env to override the forwarded
token; update the loop that appends entry.Env to skip any EnvVar with Name ==
"HF_TOKEN" (or generally dedupe by checking existing env slice for that name) so
the forwarded token set when entry.Name == inferencePerfCheckName always wins;
reference the variables/idents env, entry.Env, inferencePerfCheckName, and
corev1.EnvVar when making the change.

In `@recipes/overlays/b200-gke-cos-inference-dynamo.yaml`:
- Around line 76-92: The constraints block currently contains UNTESTED
PLACEHOLDER thresholds for the B200 overlay (see constraint names
inference-model, inference-concurrency-per-gpu, inference-throughput,
inference-ttft-p99) while conformance keeps the inference-perf gate enabled;
before merging either replace these placeholder values with measured B200
results from the referenced GKE testbed run (use real throughput and p99 TTFT
numbers) or remove/disable the inference-perf gate in conformance until
validated results are available so the recipe cannot pass with
non-representative limits.

In `@validators/performance/inference_perf_constraint.go`:
- Around line 878-881: The update path for the existing HF token Secret is
missing the required resourceVersion; before calling secrets.Update(ctx.Ctx,
secret, ... ) on the IsAlreadyExists branch, set secret.ResourceVersion to the
existing secret's ResourceVersion (metadata.resourceVersion) so the update
includes the current version; locate the AlreadyExists block around
secrets.Update and assign secret.ResourceVersion =
existingSecret.ResourceVersion (or the variable holding the fetched secret)
prior to calling secrets.Update to avoid API rejections.

In `@validators/performance/model_cache.go`:
- Around line 124-125: clusterHasDefaultStorageClass only checks the GA
annotation constant (defaultStorageClassAnnotation) and misses clusters using
the legacy beta key; update the check in clusterHasDefaultStorageClass (and any
use of defaultStorageClassAnnotation) to consider both
"storageclass.kubernetes.io/is-default-class" and
"storageclass.beta.kubernetes.io/is-default-class" as indicators of a default
StorageClass, returning true if either annotation on scs.Items[i].Annotations
equals "true".

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@recipes/overlays/h100-eks-ubuntu-inference-dynamo.yaml`:
- Around line 74-86: The inference-throughput floor "value: \">= 50000\"" is too
low for the H100 gate given the measured baseline ≈108,790 tok/s; update the
constraints block (constraint with name "inference-throughput") for the H100
overlay that targets model "inference-model: Qwen/Qwen3-8B" and concurrency
"inference-concurrency-per-gpu: \"256\"" to use a more appropriate floor (e.g.,
">= 90000") so the gate catches real regressions while still allowing the
evaluator's ~10% run-to-run variance; ensure this change is only applied to this
H100-specific overlay rather than a shared accelerator minimum.

In `@validators/performance/inference_perf_test.go`:
- Around line 992-1002: The test currently only checks that
ensureHFTokenSecret(ctx, ns) returns no error but doesn't assert the secret
content changed; after calling ensureHFTokenSecret, retrieve the Secret from the
fake client (via client.CoreV1().Secrets(ns).Get(...)) and assert that the
StringData or Data for key hfTokenSecretKey equals the new env value ("hf_new")
and that the Secret name matches hfTokenSecretName (and optionally
ResourceVersion changed), so the subtest verifies the secret was actually
updated by ensureHFTokenSecret.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@validators/performance/inference_perf_constraint.go`:
- Around line 873-905: The Secret API calls (secrets.Get/Delete/Update/Create)
use ctx.Ctx directly and need a bounded context; wrap this block in a timed
context using ctxWithTimeout, errCtx := context.WithTimeout(ctx.Ctx,
defaults.DiagnosticTimeout) (or another named default), defer errCtx() (cancel)
and replace ctx.Ctx with the derived context when calling secrets.Get,
secrets.Delete, secrets.Update and secrets.Create; keep hfTokenSecretName,
hfTokenSecretKey and the existing logic for exists/secret.ResourceVersion
unchanged so only the context usage for these I/O calls is updated.

In `@validators/performance/model_cache.go`:
- Around line 175-186: The PVC/Job creation logic (buildModelCachePVC,
buildModelCachePopulateJob, pvc, jobName, modelCachePopulateJobPrefix) currently
reuses existing artifacts when apierrors.IsAlreadyExists(err), causing stale
caches for different config.model or cache settings; instead compute a short
config hash or generation key from the resolved config (including config.model,
storage class, cache flags, runID/generation), add it as an annotation or suffix
to the PVC and Job, and in the AlreadyExists path fetch the existing PVC/Job,
compare the stored annotation/hash to the current one and if they differ delete
and recreate the resource (or always create with the hash-suffixed name) so the
populate Job and PVC are regenerated when config changes.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@recipes/overlays/h100-gke-cos-inference-dynamo.yaml`:
- Around line 74-88: The throughput floor in the constraints block
("inference-throughput" value) is too low (">= 50000") relative to the stated
H100 baseline (~108,164 tok/s) and the inline rationale; update the
inference-throughput constraint to a tighter H100-specific floor (recommend
setting value to ">= 90000") so the gate captures meaningful regressions and
aligns with the comment's described margin beyond the evaluator's 10% tolerance.

In `@recipes/validators/catalog.yaml`:
- Around line 70-79: The comment around the timeout: 65m value is ambiguous
about whether "cache-populate (10m)" is included in the 50m baseline or counted
separately; update the comment near the timeout field to explicitly state that
the 50m worst-case sum includes the nominal WORKLOAD_READY_TIMEOUT (10m) for
cache-populate but the extra 15m buffer (65m − 50m) is intended to accommodate
cache-populate overruns and deferred cleanup, and reference the relevant knobs
(CheckExecutionTimeout, WORKLOAD_READY_TIMEOUT, cache-populate) so readers
understand that large-model cache-populate can exceed its nominal 10m and the
extra time covers that overrun plus cleanup.

In `@validators/performance/inference_perf_constraint.go`:
- Around line 1389-1393: Update the stale comment for resolveInferenceModel so
it no longer calls the fallback the “default smoke-test model”; change the
wording to state that unsetting or empty AICR_INFERENCE_PERF_MODEL falls back to
the real-workload default (Qwen/Qwen3-8B). Mention AICR_INFERENCE_PERF_MODEL and
resolveInferenceModel in the comment to make it clear which override and
function are affected.

In `@validators/performance/model_cache.go`:
- Around line 241-258: The populate Job's container (Name: "populate", Image:
cacheWorkerImage) lacks resource requests/limits; add a Resources field on that
v1.Container with a v1.ResourceRequirements struct setting sensible Requests and
Limits (e.g. cpu and memory) so the pod can be scheduled and prevented from
OOMing during snapshot_download; update the Container literal inside the model
cache job creation in validators/performance/model_cache.go to include
Resources: v1.ResourceRequirements{Requests: v1.ResourceList{"cpu":
resource.MustParse("250m"), "memory": resource.MustParse("512Mi")}, Limits:
v1.ResourceList{"cpu": resource.MustParse("1"), "memory":
resource.MustParse("4Gi")}} (adjust values as needed or make them configurable).

---

Duplicate comments:
In `@recipes/overlays/h100-eks-ubuntu-inference-dynamo.yaml`:
- Around line 74-88: The throughput floor for the constraint named
inference-throughput is far too low for the H100 baseline and contradicts the
inline rationale; update the constraint value from ">= 50000" to ">= 90000" (or
make the gate accelerator-specific) so the gate reflects the H100 measured
throughput (~108,790 tok/s) and enforces a meaningful regression threshold
consistent with the evaluator's ~10% tolerance; ensure the change is applied to
the constraints block where inference-throughput is defined.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@validators/performance/model_cache.go`:
- Around line 175-183: In ensureModelCache, wrap the PVC and Job Create calls
with per-operation contexts instead of using ctx.Ctx directly: for the PVC
created via buildModelCachePVC and
ctx.Clientset.CoreV1().PersistentVolumeClaims(...).Create use a
context.WithTimeout(ctx.Ctx, defaults.DiagnosticTimeout) (remember to defer
cancel()) and pass that new ctx to Create; similarly, for the Job created via
buildModelCachePopulateJob and ctx.Clientset.BatchV1().Jobs(...).Create use
context.WithTimeout(ctx.Ctx, defaults.K8sJobCreationTimeout) (defer cancel())
and pass that timed context to Create; keep existing error handling (including
apierrors.IsAlreadyExists) unchanged.

---

Duplicate comments:
In `@recipes/overlays/h100-eks-ubuntu-inference-dynamo.yaml`:
- Around line 74-88: The inference throughput floor in the constraints block is
too low: update the constraint with name inference-throughput (currently value
">= 50000") to a tighter floor ">= 90000" so the H100 gate aligns with the
measured ~108,790 tok/s baseline and catches meaningful regressions as described
in the header comment.

In `@recipes/overlays/h100-gke-cos-inference-dynamo.yaml`:
- Around line 74-88: The inference throughput floor is too loose for H100;
update the constraints block for the H100 overlay by changing the
inference-throughput constraint's value from ">= 50000" to ">= 90000" so the
gate detects meaningful H100 regressions; locate the constraints list (entries
with name: inference-model, name: inference-concurrency-per-gpu, name:
inference-throughput) and only adjust the value for inference-throughput to ">=
90000".

In `@recipes/validators/catalog.yaml`:
- Around line 70-79: The comment above the timeout: 65m value has inconsistent
arithmetic — it lists phase durations that sum to 55m (10+5+10+5+5+15) yet
claims a "50m worst-case phase sum" and references model-cache populate vs
workload-ready ambiguously; update the comment in
recipes/validators/catalog.yaml to either (A) clarify that "model-cache
populate" and "workload-ready" are the same phase (remove the duplicate
duration) if that is true, or (B) correct the summed total to 55m and adjust the
explanatory text (including references to CheckExecutionTimeout and
WORKLOAD_READY_TIMEOUT) so the timeout rationale consistently explains why
timeout: 65m was chosen.

In `@validators/performance/inference_perf_constraint.go`:
- Around line 1389-1393: Update the stale comment in resolveInferenceModel:
change the phrase that says it "falls back to the default smoke-test model" to
explicitly name the compiled default (e.g., "compiled default Qwen/Qwen3-8B")
and/or clarify that an unset/empty AICR_INFERENCE_PERF_MODEL will use the
compiled default model ID (Qwen/Qwen3-8B) so the precedence docs reflect the
current default behavior.
- Around line 866-905: The secret CRUD calls in ensureHFTokenSecret use ctx.Ctx
directly and must be bounded with a timeout; wrap calls to secrets.Get, Delete,
Update, and Create with a derived context created via
context.WithTimeout(ctx.Ctx, defaults.HTTPClientTimeout) (or the project's
chosen timeout constant), assign that to a local timedCtx and defer cancel()
before each K8s call (or reuse one timedCtx per function scope) and replace
ctx.Ctx with timedCtx when invoking secrets.Get/Delete/Update/Create to ensure
the operations respect the configured deadline.