From 9e6989d582a79d14bec39740c738e5152395e378 Mon Sep 17 00:00:00 2001 From: Rashid Kaleem <230885705+arekay-nv@users.noreply.github.com> Date: Thu, 9 Jul 2026 20:56:12 -0500 Subject: [PATCH 1/6] Salt failure is a hard error Signed-off-by: Rashid Kaleem <230885705+arekay-nv@users.noreply.github.com> --- .../commands/benchmark/execute.py | 6 + .../dataset_manager/dataset.py | 94 +++++++++------ tests/unit/commands/test_benchmark.py | 45 ++++++- .../dataset_manager/test_salted_dataset.py | 112 +++++++++--------- 4 files changed, 166 insertions(+), 91 deletions(-) diff --git a/src/inference_endpoint/commands/benchmark/execute.py b/src/inference_endpoint/commands/benchmark/execute.py index 6c315cad8..14c9d44b5 100644 --- a/src/inference_endpoint/commands/benchmark/execute.py +++ b/src/inference_endpoint/commands/benchmark/execute.py @@ -367,6 +367,12 @@ def _load_datasets( except Exception as e: raise SetupError(f"Failed to load dataset: {e}") from e + # Fail fast on a warmup dataset that salt cannot bust — at load time, + # before any worker/aggregator subprocess is spawned. + warmup = config.settings.warmup + if warmup.enabled and warmup.salt: + dataloader.validate_saltable() + if perf_cfg.accuracy_config is not None: accuracy_config = perf_cfg.accuracy_config if accuracy_config.num_repeats != 1: diff --git a/src/inference_endpoint/dataset_manager/dataset.py b/src/inference_endpoint/dataset_manager/dataset.py index bd259d7a1..6d4e160d6 100644 --- a/src/inference_endpoint/dataset_manager/dataset.py +++ b/src/inference_endpoint/dataset_manager/dataset.py @@ -30,6 +30,7 @@ from datasets import load_dataset, load_from_disk from ..config.schema import APIType, ModelParams +from ..exceptions import DatasetValidationError from .transforms import ( ColumnFilter, Transform, @@ -257,6 +258,23 @@ def load_from_huggingface( return ds[split].to_pandas() +def _salt_violation(sample: Any) -> str | None: + """Return a human-readable reason a sample cannot be salted, or None if it can. + + Salt requires a dict sample with a str 'prompt' and no 'input_tokens' (which + adapters send verbatim, so a salted 'prompt' would not reach the server). + """ + if not isinstance(sample, dict): + return f"is a {type(sample).__name__}, not a dict" + if "input_tokens" in sample: + return "has 'input_tokens' (salt cannot bust a pre-tokenized cache)" + if "prompt" not in sample: + return "has no 'prompt' field" + if not isinstance(sample["prompt"], str): + return f"has a 'prompt' of type {type(sample['prompt']).__name__}, not str" + return None + + class Dataset: """Class for loading and managing benchmark datasets. @@ -437,55 +455,57 @@ def load_sample(self, index: int) -> Any: data = self._apply_salt(data) return data + def validate_saltable(self) -> None: + """Raise if any loaded sample cannot be salted. + + salt requires a dict sample with a text ('str') 'prompt' and no + 'input_tokens' (adapters send those verbatim, so a salted 'prompt' would + never reach the server). A sample salt cannot bust would silently defeat + cache-busting, so it is rejected rather than skipped. Called before any + load is issued — at benchmark setup and again from with_salt(). + + Raises: + DatasetValidationError: naming the first offending sample. + """ + if self.data is None: + return + for i, sample in enumerate(self.data): + reason = _salt_violation(sample) + if reason is not None: + raise DatasetValidationError( + f"salt=True requires every sample to be a dict with a text " + f"'prompt' and no 'input_tokens', but sample {i} {reason}. " + f"Disable salt (--warmup-salt / warmup.salt: false) or use a " + f"text-prompt dataset." + ) + def with_salt(self, rng: random.Random) -> "Dataset": """Return a shallow copy of this dataset that salts each load_sample() call. The returned dataset shares the same loaded data — no re-loading needed. Each load_sample() call on the returned dataset prepends a unique hex salt - derived from rng to the prompt field, preventing KV-cache reuse. + derived from rng to the 'prompt' field, preventing KV-cache reuse. + + Validates every sample first (see validate_saltable), so a dataset salt + cannot bust fails here rather than silently issuing unsalted prompts. + + Raises: + DatasetValidationError: if any sample cannot be salted. """ + self.validate_saltable() clone = copy.copy(self) clone._salt_rng = rng return clone - def _apply_salt(self, data: Any) -> Any: - """Prepend a unique salt to the prompt field of a sample dict.""" + def _apply_salt(self, data: dict[str, Any]) -> dict[str, Any]: + """Prepend a unique salt to the 'prompt' field. + + with_salt() has validated every sample, so ``data`` is guaranteed to be a + dict with a str 'prompt' and no 'input_tokens'. + """ assert self._salt_rng is not None - if not isinstance(data, dict): - return data - if "input_tokens" in data and "prompt" not in data: - self.logger.warning( - "salt=True: sample has 'input_tokens' but no 'prompt' — " - "salt cannot be applied to pre-tokenized input; KV-cache reuse may not be prevented" - ) - return data - if "input_tokens" in data and "prompt" in data: - self.logger.warning( - "salt=True: sample has both 'input_tokens' and 'prompt' — " - "salt applied to 'prompt' only; adapters that use 'input_tokens' " - "directly will still reuse the KV cache" - ) - if "prompt" not in data: - return data - prompt = data["prompt"] salt = self._salt_rng.randbytes(8).hex() - if isinstance(prompt, str): - return {**data, "prompt": f"[{salt}] {prompt}"} - if isinstance(prompt, list) and prompt: - # Find the first text part at any index (image-first prompts place text at index 1+) - for i, part in enumerate(prompt): - if isinstance(part, dict) and part.get("type") == "text": - salted_parts = [ - *prompt[:i], - {**part, "text": f"[{salt}] {part['text']}"}, - *prompt[i + 1 :], - ] - return {**data, "prompt": salted_parts} - self.logger.warning( - "salt=True: multimodal prompt has no text part — " - "salt cannot be applied; KV-cache reuse may not be prevented" - ) - return data # unsupported prompt type — skip salting + return {**data, "prompt": f"[{salt}] {data['prompt']}"} def num_samples(self) -> int: assert self.data is not None, "Dataset not loaded. Call load() first." diff --git a/tests/unit/commands/test_benchmark.py b/tests/unit/commands/test_benchmark.py index 491dc72b6..d58923711 100644 --- a/tests/unit/commands/test_benchmark.py +++ b/tests/unit/commands/test_benchmark.py @@ -77,7 +77,10 @@ from inference_endpoint.dataset_manager.dataset import Dataset from inference_endpoint.endpoint_client.config import HTTPClientConfig from inference_endpoint.evaluation.scoring import Scorer -from inference_endpoint.exceptions import InputValidationError, SetupError +from inference_endpoint.exceptions import ( + InputValidationError, + SetupError, +) from inference_endpoint.load_generator.sample_order import create_sample_order from inference_endpoint.load_generator.session import PhaseType from inference_endpoint.metrics.metric import Throughput @@ -221,6 +224,46 @@ def test_dataset_string_coercion( assert ds.accuracy_config.eval_method == acc_eval_method +@pytest.mark.unit +class TestLoadDatasetsSaltValidation: + """_load_datasets validates salt-compatibility at dataset-load time — before + any worker/aggregator subprocess is spawned — when warmup salt is enabled. + """ + + def _config(self, tmp_path: Path, warmup: WarmupConfig) -> OfflineConfig: + ds = tmp_path / "perf.jsonl" + ds.write_text('{"prompt": "hello world"}\n{"prompt": "second prompt"}\n') + return OfflineConfig( + endpoint_config={"endpoints": ["http://test:8000"]}, + model_params={"name": "test-model"}, + datasets=[{"path": str(ds)}], + settings=OfflineSettings( + client=HTTPClientConfig( + num_workers=1, warmup_connections=0, max_connections=10 + ), + warmup=warmup, + ), + ) + + @patch.object(Dataset, "validate_saltable") + def test_validates_when_warmup_salt_enabled(self, mock_validate, tmp_path): + config = self._config(tmp_path, WarmupConfig(enabled=True, salt=True)) + _load_datasets(config, tmp_path, TestMode.PERF) + mock_validate.assert_called_once() + + @patch.object(Dataset, "validate_saltable") + def test_skips_validation_when_warmup_disabled(self, mock_validate, tmp_path): + config = self._config(tmp_path, WarmupConfig(enabled=False, salt=True)) + _load_datasets(config, tmp_path, TestMode.PERF) + mock_validate.assert_not_called() + + @patch.object(Dataset, "validate_saltable") + def test_skips_validation_when_salt_off(self, mock_validate, tmp_path): + config = self._config(tmp_path, WarmupConfig(enabled=True, salt=False)) + _load_datasets(config, tmp_path, TestMode.PERF) + mock_validate.assert_not_called() + + class TestCommandHandlers: """Test offline/online/from_config handlers (mock run_benchmark).""" diff --git a/tests/unit/dataset_manager/test_salted_dataset.py b/tests/unit/dataset_manager/test_salted_dataset.py index acff8900f..329b17fa2 100644 --- a/tests/unit/dataset_manager/test_salted_dataset.py +++ b/tests/unit/dataset_manager/test_salted_dataset.py @@ -17,11 +17,11 @@ import random import re -from unittest.mock import MagicMock import pandas as pd import pytest from inference_endpoint.dataset_manager.dataset import Dataset +from inference_endpoint.exceptions import DatasetValidationError def _make_loaded_dataset(rows: list[dict]) -> Dataset: @@ -31,7 +31,6 @@ def _make_loaded_dataset(rows: list[dict]) -> Dataset: ds.transforms = None ds.repeats = 1 ds.data = list(rows) - ds.logger = MagicMock() ds._salt_rng = None return ds @@ -143,77 +142,84 @@ def test_seeded_rng_is_reproducible(self): @pytest.mark.unit -class TestSaltPassthrough: - """Samples without a 'prompt' key, or non-dict samples, are passed through unchanged.""" +class TestSaltValidation: + """with_salt() hard-errors up front unless every sample has a text 'prompt'. - def test_dict_without_prompt_key_is_unchanged(self): + salt=True guarantees a KV-cache-busting prefix; a sample it cannot salt is a + configuration error, not something to skip silently. Validation runs in + with_salt() (before any load is issued), so the error names the offending + sample and no partial warmup runs against an unsalted dataset. + """ + + def test_dict_without_prompt_key_raises(self): inner = _make_loaded_dataset([{"question": "what is 2+2?", "answer": "4"}]) - sd = inner.with_salt(random.Random()) - assert sd.load_sample(0) == {"question": "what is 2+2?", "answer": "4"} + with pytest.raises(DatasetValidationError, match="prompt"): + inner.with_salt(random.Random()) - def test_empty_dict_is_unchanged(self): + def test_empty_dict_raises(self): inner = _make_loaded_dataset([{}]) - sd = inner.with_salt(random.Random()) - assert sd.load_sample(0) == {} + with pytest.raises(DatasetValidationError, match="prompt"): + inner.with_salt(random.Random()) - def test_non_dict_sample_is_returned_as_is(self): + def test_non_dict_sample_raises(self): inner = _make_loaded_dataset([{"prompt": "x"}]) inner.data = ["raw string sample"] - sd = inner.with_salt(random.Random()) - assert sd.load_sample(0) == "raw string sample" + with pytest.raises(DatasetValidationError, match="dict"): + inner.with_salt(random.Random()) - def test_multimodal_list_prompt_first_text_part_is_salted(self): + def test_multimodal_list_prompt_raises(self): content_parts = [ {"type": "text", "text": "describe this image"}, {"type": "image_url", "image_url": {"url": "data:image/png;base64,abc"}}, ] inner = _make_loaded_dataset([{"prompt": content_parts}]) - sd = inner.with_salt(random.Random()) - parts = sd.load_sample(0)["prompt"] - assert isinstance(parts, list) - assert len(parts) == 2 - assert re.match(r"^\[([0-9a-f]{16})\] describe this image$", parts[0]["text"]) - assert parts[1] == content_parts[1] - - def test_multimodal_image_first_text_at_index_1_is_salted(self): - content_parts = [ - {"type": "image_url", "image_url": {"url": "data:image/png;base64,abc"}}, - {"type": "text", "text": "what do you see?"}, - ] - inner = _make_loaded_dataset([{"prompt": content_parts}]) - sd = inner.with_salt(random.Random()) - parts = sd.load_sample(0)["prompt"] - assert parts[0] == content_parts[0] - assert re.match(r"^\[([0-9a-f]{16})\] what do you see\?$", parts[1]["text"]) + with pytest.raises(DatasetValidationError, match="str"): + inner.with_salt(random.Random()) - def test_multimodal_list_prompt_original_not_mutated(self): - content_parts = [{"type": "text", "text": "original text"}] - inner = _make_loaded_dataset([{"prompt": content_parts}]) - sd = inner.with_salt(random.Random()) - sd.load_sample(0) - assert inner.data[0]["prompt"][0]["text"] == "original text" - - def test_unknown_prompt_type_is_not_salted(self): + def test_non_str_prompt_raises(self): inner = _make_loaded_dataset([{"prompt": 42}]) - sd = inner.with_salt(random.Random()) - assert sd.load_sample(0) == {"prompt": 42} + with pytest.raises(DatasetValidationError, match="str"): + inner.with_salt(random.Random()) - def test_input_tokens_only_warns_and_passes_through(self): + def test_input_tokens_only_raises(self): inner = _make_loaded_dataset([{"input_tokens": [1, 2, 3]}]) - sd = inner.with_salt(random.Random()) - result = sd.load_sample(0) - assert result == {"input_tokens": [1, 2, 3]} - sd.logger.warning.assert_called_once() - assert "input_tokens" in sd.logger.warning.call_args[0][0] + with pytest.raises(DatasetValidationError, match="input_tokens"): + inner.with_salt(random.Random()) - def test_input_tokens_and_prompt_warns_and_salts_prompt(self): + def test_input_tokens_and_prompt_raises(self): inner = _make_loaded_dataset([{"input_tokens": [1, 2, 3], "prompt": "hello"}]) + with pytest.raises(DatasetValidationError, match="input_tokens"): + inner.with_salt(random.Random()) + + def test_error_names_offending_sample_index(self): + inner = _make_loaded_dataset([{"prompt": "ok"}, {"prompt": 42}]) + with pytest.raises(DatasetValidationError, match=r"\b1\b"): + inner.with_salt(random.Random()) + + def test_valid_str_prompt_dataset_does_not_raise(self): + inner = _make_loaded_dataset([{"prompt": "a"}, {"prompt": "b"}]) sd = inner.with_salt(random.Random()) - result = sd.load_sample(0) - assert result["input_tokens"] == [1, 2, 3] - assert result["prompt"].startswith("[") - sd.logger.warning.assert_called_once() - assert "input_tokens" in sd.logger.warning.call_args[0][0] + assert sd.load_sample(0)["prompt"].startswith("[") + + def test_data_none_does_not_raise(self): + inner = _make_loaded_dataset([{"prompt": "x"}]) + inner.data = None + # No samples to salt (e.g. EmptyDataset) — nothing to validate. + assert inner.with_salt(random.Random())._salt_rng is not None + + def test_data_empty_list_does_not_raise(self): + inner = _make_loaded_dataset([]) + # Zero samples — no violation, so no error. + assert inner.with_salt(random.Random())._salt_rng is not None + + def test_validate_saltable_noop_on_valid(self): + inner = _make_loaded_dataset([{"prompt": "a"}, {"prompt": "b"}]) + assert inner.validate_saltable() is None + + def test_validate_saltable_raises_on_bad_sample(self): + inner = _make_loaded_dataset([{"prompt": "ok"}, {"input_tokens": [1, 2]}]) + with pytest.raises(DatasetValidationError, match="input_tokens"): + inner.validate_saltable() @pytest.mark.unit From 6f6c16590a86be31bb6f0d6be4c5391f19cecd23 Mon Sep 17 00:00:00 2001 From: Rashid Kaleem <230885705+arekay-nv@users.noreply.github.com> Date: Fri, 24 Jul 2026 19:49:33 -0500 Subject: [PATCH 2/6] fix: address review feedback on salt-failure hard error - Default warmup.salt to False so pre-tokenized (input_tokens) workloads no longer hard-fail on a plain --warmup; the hard error now fires only when salt is explicitly enabled. - Replace raw-string salt-violation reasons with a typed DatasetValidationError.Reason enum plus optional detail; _salt_violation becomes _can_salt returning the enum. UNSPECIFIED covers not-yet-mapped --dataset parse errors. - validate_saltable: assert on unloaded data (no silent skip); clarify the error index is into the loaded post-transform order; document the intentional whole-dataset (fail-on-any-invalid) strictness and the deliberate pre-spawn check. - Tests: unpatched integration coverage (offline/online raise, accuracy-only skip), agentic messages sample, typed-reason assertions; drop brittle index regex. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../commands/benchmark/cli.py | 9 +- .../commands/benchmark/execute.py | 4 +- src/inference_endpoint/config/schema.py | 13 ++- .../templates/concurrency_template_full.yaml | 2 +- .../templates/offline_template_full.yaml | 2 +- .../templates/online_template_full.yaml | 2 +- .../dataset_manager/dataset.py | 44 ++++++---- src/inference_endpoint/exceptions.py | 29 ++++++- tests/unit/commands/test_benchmark.py | 87 +++++++++++++++++-- .../dataset_manager/test_salted_dataset.py | 40 ++++++++- 10 files changed, 194 insertions(+), 38 deletions(-) diff --git a/src/inference_endpoint/commands/benchmark/cli.py b/src/inference_endpoint/commands/benchmark/cli.py index 0edb4da79..e17400412 100644 --- a/src/inference_endpoint/commands/benchmark/cli.py +++ b/src/inference_endpoint/commands/benchmark/cli.py @@ -67,9 +67,14 @@ def _run( f"{'.'.join(str(p) for p in err['loc'])}: {err['msg']}" for err in e.errors() ) - raise DatasetValidationError(f"Invalid --dataset: {msgs}") from e + # --dataset parse failures aren't yet mapped to a specific Reason. + raise DatasetValidationError( + DatasetValidationError.Reason.UNSPECIFIED, f"Invalid --dataset: {msgs}" + ) from e except ValueError as e: - raise DatasetValidationError(f"Invalid --dataset: {e}") from e + raise DatasetValidationError( + DatasetValidationError.Reason.UNSPECIFIED, f"Invalid --dataset: {e}" + ) from e if config.audit is None: run_benchmark(config, mode) return diff --git a/src/inference_endpoint/commands/benchmark/execute.py b/src/inference_endpoint/commands/benchmark/execute.py index e4b6805ea..3e90a1dcf 100644 --- a/src/inference_endpoint/commands/benchmark/execute.py +++ b/src/inference_endpoint/commands/benchmark/execute.py @@ -436,7 +436,9 @@ def _load_datasets( raise SetupError(f"Failed to load dataset: {e}") from e # Fail fast on a warmup dataset that salt cannot bust — at load time, - # before any worker/aggregator subprocess is spawned. + # before any worker/aggregator subprocess is spawned. with_salt() runs + # the same check later; this earlier call is deliberate (not redundant), + # so an invalid dataset aborts before the subprocess fan-out. warmup = config.settings.warmup if warmup.enabled and warmup.salt: dataloader.validate_saltable() diff --git a/src/inference_endpoint/config/schema.py b/src/inference_endpoint/config/schema.py index b4d6f6c5e..3f6c0238a 100644 --- a/src/inference_endpoint/config/schema.py +++ b/src/inference_endpoint/config/schema.py @@ -759,10 +759,19 @@ class WarmupConfig(BaseModel): bool, cyclopts.Parameter( alias="--warmup-salt", - help="Prepend a unique random hex salt to each warmup prompt", + help=( + "Prepend a unique random hex salt to each warmup prompt. Requires " + "text-'prompt' samples; enabling it on a pre-tokenized " + "('input_tokens') dataset is a hard error." + ), ), ] = Field( - True, description="Prepend a unique random hex salt to each warmup prompt" + False, + description=( + "Prepend a unique random hex salt to each warmup prompt. Requires " + "text-'prompt' samples; enabling it on a pre-tokenized " + "('input_tokens') dataset is a hard error." + ), ) drain: Annotated[ bool, diff --git a/src/inference_endpoint/config/templates/concurrency_template_full.yaml b/src/inference_endpoint/config/templates/concurrency_template_full.yaml index 9c306a841..bfa797fe8 100644 --- a/src/inference_endpoint/config/templates/concurrency_template_full.yaml +++ b/src/inference_endpoint/config/templates/concurrency_template_full.yaml @@ -93,7 +93,7 @@ settings: warmup: enabled: false # Enable warmup phase before performance run n_requests: null # Warmup request count (None = full dataset once) - salt: true # Prepend a unique random hex salt to each warmup prompt + salt: false # Prepend a unique random hex salt to each warmup prompt. Requires text-'prompt' samples; enabling it on a pre-tokenized ('input_tokens') dataset is a hard error. drain: false # Drain in-flight warmup requests before starting the performance phase warmup_random_seed: 42 # RNG seed for warmup scheduling and sample ordering profiling: diff --git a/src/inference_endpoint/config/templates/offline_template_full.yaml b/src/inference_endpoint/config/templates/offline_template_full.yaml index 29d52a270..faa7649eb 100644 --- a/src/inference_endpoint/config/templates/offline_template_full.yaml +++ b/src/inference_endpoint/config/templates/offline_template_full.yaml @@ -93,7 +93,7 @@ settings: warmup: enabled: false # Enable warmup phase before performance run n_requests: null # Warmup request count (None = full dataset once) - salt: true # Prepend a unique random hex salt to each warmup prompt + salt: false # Prepend a unique random hex salt to each warmup prompt. Requires text-'prompt' samples; enabling it on a pre-tokenized ('input_tokens') dataset is a hard error. drain: false # Drain in-flight warmup requests before starting the performance phase warmup_random_seed: 42 # RNG seed for warmup scheduling and sample ordering profiling: diff --git a/src/inference_endpoint/config/templates/online_template_full.yaml b/src/inference_endpoint/config/templates/online_template_full.yaml index 89fac1903..e07206b46 100644 --- a/src/inference_endpoint/config/templates/online_template_full.yaml +++ b/src/inference_endpoint/config/templates/online_template_full.yaml @@ -94,7 +94,7 @@ settings: warmup: enabled: false # Enable warmup phase before performance run n_requests: null # Warmup request count (None = full dataset once) - salt: true # Prepend a unique random hex salt to each warmup prompt + salt: false # Prepend a unique random hex salt to each warmup prompt. Requires text-'prompt' samples; enabling it on a pre-tokenized ('input_tokens') dataset is a hard error. drain: false # Drain in-flight warmup requests before starting the performance phase warmup_random_seed: 42 # RNG seed for warmup scheduling and sample ordering profiling: diff --git a/src/inference_endpoint/dataset_manager/dataset.py b/src/inference_endpoint/dataset_manager/dataset.py index cc21046fc..5b6345cb6 100644 --- a/src/inference_endpoint/dataset_manager/dataset.py +++ b/src/inference_endpoint/dataset_manager/dataset.py @@ -258,20 +258,21 @@ def load_from_huggingface( return ds[split].to_pandas() -def _salt_violation(sample: Any) -> str | None: - """Return a human-readable reason a sample cannot be salted, or None if it can. +def _can_salt(sample: Any) -> DatasetValidationError.Reason | None: + """Return the Reason a sample cannot be salted, or None if it can. Salt requires a dict sample with a str 'prompt' and no 'input_tokens' (which adapters send verbatim, so a salted 'prompt' would not reach the server). """ + Reason = DatasetValidationError.Reason if not isinstance(sample, dict): - return f"is a {type(sample).__name__}, not a dict" + return Reason.TYPE_MISMATCH if "input_tokens" in sample: - return "has 'input_tokens' (salt cannot bust a pre-tokenized cache)" + return Reason.INPUT_TOKENS_SHADOWING if "prompt" not in sample: - return "has no 'prompt' field" + return Reason.PROMPT_MISSING if not isinstance(sample["prompt"], str): - return f"has a 'prompt' of type {type(sample['prompt']).__name__}, not str" + return Reason.PROMPT_TYPE_MISMATCH return None @@ -463,23 +464,28 @@ def validate_saltable(self) -> None: salt requires a dict sample with a text ('str') 'prompt' and no 'input_tokens' (adapters send those verbatim, so a salted 'prompt' would - never reach the server). A sample salt cannot bust would silently defeat - cache-busting, so it is rejected rather than skipped. Called before any - load is issued — at benchmark setup and again from with_salt(). + never reach the server). A non-saltable sample is an error, not a silent + skip: skipping would leave the KV cache un-busted. Every sample is + checked — a single invalid item fails the run, because the seeded warmup + subset can draw any index and salt correctness is all-or-nothing. Called + before any load is issued — at benchmark setup and again from with_salt(). Raises: - DatasetValidationError: naming the first offending sample. + DatasetValidationError: naming the first offending sample. The index + is into the loaded, post-transform sample order, not the source + file line. """ - if self.data is None: - return + assert self.data is not None, "Dataset not loaded. Call load() first." for i, sample in enumerate(self.data): - reason = _salt_violation(sample) + reason = _can_salt(sample) if reason is not None: raise DatasetValidationError( - f"salt=True requires every sample to be a dict with a text " - f"'prompt' and no 'input_tokens', but sample {i} {reason}. " - f"Disable salt (--warmup-salt / warmup.salt: false) or use a " - f"text-prompt dataset." + reason, + detail=( + f"sample {i} (index into the loaded, post-transform " + f"order); disable salt (--warmup-salt / warmup.salt: " + f"false) or use a text-prompt dataset" + ), ) def with_salt(self, rng: random.Random) -> "Dataset": @@ -489,8 +495,8 @@ def with_salt(self, rng: random.Random) -> "Dataset": Each load_sample() call on the returned dataset prepends a unique hex salt derived from rng to the 'prompt' field, preventing KV-cache reuse. - Validates every sample first (see validate_saltable), so a dataset salt - cannot bust fails here rather than silently issuing unsalted prompts. + Validates every sample first (see validate_saltable): a non-saltable + dataset raises here, before any load is issued. Raises: DatasetValidationError: if any sample cannot be salted. diff --git a/src/inference_endpoint/exceptions.py b/src/inference_endpoint/exceptions.py index 86f25a2f9..6853b9e49 100644 --- a/src/inference_endpoint/exceptions.py +++ b/src/inference_endpoint/exceptions.py @@ -15,6 +15,8 @@ """Custom exceptions for CLI error handling.""" +from enum import Enum + class CLIError(Exception): """Base exception for CLI errors. @@ -37,9 +39,32 @@ class InputValidationError(CLIError): class DatasetValidationError(InputValidationError): - """Invalid --dataset string or dataset configuration.""" + """Invalid --dataset string or dataset configuration. - pass + The failure category is a ``Reason``; ``detail`` carries the specifics + (offending sample index, remediation hint, parser error text). + """ + + class Reason(Enum): + """Why a dataset failed validation.""" + + TYPE_MISMATCH = "sample is not a dict" + INPUT_TOKENS_SHADOWING = ( + "sample has 'input_tokens'; salt cannot bust a pre-tokenized cache" + ) + PROMPT_MISSING = "sample has no 'prompt' field" + PROMPT_TYPE_MISMATCH = "sample 'prompt' is not a str" + UNSPECIFIED = "dataset validation failed" + + def __init__( + self, + reason: "DatasetValidationError.Reason", + detail: str | None = None, + ) -> None: + self.reason = reason + self.detail = detail + message = reason.value if detail is None else f"{reason.value}: {detail}" + super().__init__(message) class SetupError(CLIError): diff --git a/tests/unit/commands/test_benchmark.py b/tests/unit/commands/test_benchmark.py index 76365e441..3ca527ac0 100644 --- a/tests/unit/commands/test_benchmark.py +++ b/tests/unit/commands/test_benchmark.py @@ -85,7 +85,11 @@ Scorer, SWEBenchScorer, ) -from inference_endpoint.exceptions import InputValidationError, SetupError +from inference_endpoint.exceptions import ( + DatasetValidationError, + InputValidationError, + SetupError, +) from inference_endpoint.load_generator.sample_order import create_sample_order from inference_endpoint.load_generator.session import ( PhaseResult, @@ -435,6 +439,78 @@ def test_skips_validation_when_salt_off(self, mock_validate, tmp_path): _load_datasets(config, tmp_path, TestMode.PERF) mock_validate.assert_not_called() + def test_unsaltable_perf_dataset_raises_before_spawn(self, tmp_path): + """Real validation (unpatched) rejects a non-saltable perf dataset at + load time — an int 'prompt' cannot be salted.""" + ds = tmp_path / "perf.jsonl" + ds.write_text('{"prompt": 1}\n{"prompt": 2}\n') + config = OfflineConfig( + endpoint_config={"endpoints": ["http://test:8000"]}, + model_params={"name": "test-model"}, + datasets=[{"path": str(ds)}], + settings=OfflineSettings( + client=HTTPClientConfig( + num_workers=1, warmup_connections=0, max_connections=10 + ), + warmup=WarmupConfig(enabled=True, salt=True), + ), + ) + with pytest.raises(DatasetValidationError, match=r"sample 0\b"): + _load_datasets(config, tmp_path, TestMode.PERF) + + def test_online_unsaltable_perf_dataset_raises(self, tmp_path): + """The salt check is load-pattern agnostic — online runs validate too.""" + ds = tmp_path / "perf.jsonl" + ds.write_text('{"prompt": 1}\n') + config = OnlineConfig( + endpoint_config={"endpoints": ["http://test:8000"]}, + model_params={"name": "test-model"}, + datasets=[{"path": str(ds)}], + settings=OnlineSettings( + load_pattern=LoadPattern(type=LoadPatternType.POISSON, target_qps=10), + client=HTTPClientConfig( + num_workers=1, warmup_connections=0, max_connections=10 + ), + warmup=WarmupConfig(enabled=True, salt=True), + ), + ) + with pytest.raises(DatasetValidationError, match=r"sample 0\b"): + _load_datasets(config, tmp_path, TestMode.PERF) + + def test_accuracy_only_skips_salt_validation(self, tmp_path): + """TestMode.ACC never loads the perf dataset, so an unsaltable perf + dataset with warmup salt on must NOT be validated (dataloader is None). + Guards against a refactor validating a None dataloader.""" + perf = tmp_path / "perf.jsonl" + perf.write_text('{"prompt": 1}\n') # unsaltable — would raise if validated + fake_acc_df = pd.DataFrame( + [{"instance_id": "repo__repo-0", "prompt": "Fix bug 0"}] + ) + config = OfflineConfig( + endpoint_config={"endpoints": ["http://test:8000"]}, + model_params={"name": "test-model"}, + datasets=[ + {"type": "performance", "path": str(perf)}, + { + "name": "swe_bench", + "type": "accuracy", + "accuracy_config": {"eval_method": "swe_bench_scorer"}, + }, + ], + settings=OfflineSettings( + client=HTTPClientConfig( + num_workers=1, warmup_connections=0, max_connections=10 + ), + warmup=WarmupConfig(enabled=True, salt=True), + ), + ) + with ( + patch.object(SWEBenchScorer, "preflight"), + patch.object(SWEBench, "generate", return_value=fake_acc_df), + ): + perf_loader, _, _ = _load_datasets(config, tmp_path, TestMode.ACC) + assert perf_loader is None + class TestCommandHandlers: """Test offline/online/from_config handlers (mock run_benchmark).""" @@ -724,7 +800,7 @@ def test_preflight_error_propagates(self, tmp_path): @pytest.mark.unit @pytest.mark.parametrize( - ("datasets, expected_scorer, expected_type, " "expected_accuracy_datasets"), + ("datasets, expected_scorer, expected_type, expected_accuracy_datasets"), [ ( [ @@ -994,7 +1070,7 @@ def test_defaults(self): cfg = WarmupConfig() assert cfg.enabled is False assert cfg.n_requests is None - assert cfg.salt is True + assert cfg.salt is False assert cfg.drain is False @pytest.mark.unit @@ -1532,7 +1608,7 @@ def test_warmup_n_requests_none_when_unset(self, base_rt_settings, simple_datase assert phases[0].runtime_settings.n_samples_to_issue is None @pytest.mark.unit - def test_warmup_defaults_uses_salt(self, base_rt_settings, simple_dataset): + def test_warmup_defaults_no_salt(self, base_rt_settings, simple_dataset): config = OfflineConfig( **_OFFLINE_KWARGS, settings=OfflineSettings(warmup=WarmupConfig(enabled=True)), @@ -1540,7 +1616,8 @@ def test_warmup_defaults_uses_salt(self, base_rt_settings, simple_dataset): ctx = self._make_ctx(config, base_rt_settings, simple_dataset) phases = _build_phases(ctx) - assert phases[0].dataset._salt_rng is not None + assert phases[0].dataset._salt_rng is None + assert phases[0].dataset is simple_dataset @pytest.mark.unit def test_warmup_without_salt_uses_raw_dataloader( diff --git a/tests/unit/dataset_manager/test_salted_dataset.py b/tests/unit/dataset_manager/test_salted_dataset.py index 329b17fa2..cf72ab5c0 100644 --- a/tests/unit/dataset_manager/test_salted_dataset.py +++ b/tests/unit/dataset_manager/test_salted_dataset.py @@ -186,6 +186,16 @@ def test_input_tokens_only_raises(self): with pytest.raises(DatasetValidationError, match="input_tokens"): inner.with_salt(random.Random()) + def test_agentic_messages_sample_raises_prompt_missing(self): + # Agentic datasets store dict samples keyed by 'messages', not 'prompt' — + # salt has no text field to prepend, so it's a clear PROMPT_MISSING error. + inner = _make_loaded_dataset( + [{"messages": [{"role": "user", "content": "hi"}]}] + ) + with pytest.raises(DatasetValidationError) as exc_info: + inner.with_salt(random.Random()) + assert exc_info.value.reason is DatasetValidationError.Reason.PROMPT_MISSING + def test_input_tokens_and_prompt_raises(self): inner = _make_loaded_dataset([{"input_tokens": [1, 2, 3], "prompt": "hello"}]) with pytest.raises(DatasetValidationError, match="input_tokens"): @@ -193,7 +203,7 @@ def test_input_tokens_and_prompt_raises(self): def test_error_names_offending_sample_index(self): inner = _make_loaded_dataset([{"prompt": "ok"}, {"prompt": 42}]) - with pytest.raises(DatasetValidationError, match=r"\b1\b"): + with pytest.raises(DatasetValidationError, match=r"sample 1\b"): inner.with_salt(random.Random()) def test_valid_str_prompt_dataset_does_not_raise(self): @@ -201,11 +211,13 @@ def test_valid_str_prompt_dataset_does_not_raise(self): sd = inner.with_salt(random.Random()) assert sd.load_sample(0)["prompt"].startswith("[") - def test_data_none_does_not_raise(self): + def test_data_none_raises_not_loaded(self): inner = _make_loaded_dataset([{"prompt": "x"}]) inner.data = None - # No samples to salt (e.g. EmptyDataset) — nothing to validate. - assert inner.with_salt(random.Random())._salt_rng is not None + # None means "not loaded" — load() always sets a list. Validating an + # unloaded dataset is a programming error, not a silent no-op. + with pytest.raises(AssertionError, match="not loaded"): + inner.with_salt(random.Random()) def test_data_empty_list_does_not_raise(self): inner = _make_loaded_dataset([]) @@ -221,6 +233,26 @@ def test_validate_saltable_raises_on_bad_sample(self): with pytest.raises(DatasetValidationError, match="input_tokens"): inner.validate_saltable() + @pytest.mark.parametrize( + "sample, expected_reason", + [ + ("raw string", DatasetValidationError.Reason.TYPE_MISMATCH), + ( + {"input_tokens": [1, 2]}, + DatasetValidationError.Reason.INPUT_TOKENS_SHADOWING, + ), + ({"question": "?"}, DatasetValidationError.Reason.PROMPT_MISSING), + ({"prompt": 42}, DatasetValidationError.Reason.PROMPT_TYPE_MISMATCH), + ], + ) + def test_error_exposes_typed_reason(self, sample, expected_reason): + inner = _make_loaded_dataset([{"prompt": "ok"}]) + inner.data = [sample] + with pytest.raises(DatasetValidationError) as exc_info: + inner.validate_saltable() + assert exc_info.value.reason is expected_reason + assert exc_info.value.detail is not None + @pytest.mark.unit class TestSaltWithRealDataset: From 659debf26f16a7a71ce7ac4652656bbf66c6f731 Mon Sep 17 00:00:00 2001 From: arekay-nv <230885705+arekay-nv@users.noreply.github.com> Date: Fri, 14 Aug 2026 21:26:28 -0500 Subject: [PATCH 3/6] Address messages list in data Signed-off-by: arekay-nv <230885705+arekay-nv@users.noreply.github.com> --- .../dataset_manager/dataset.py | 11 +++++--- src/inference_endpoint/exceptions.py | 4 +++ .../dataset_manager/test_salted_dataset.py | 25 ++++++++++++++++--- tests/unit/openai/test_msgspec_adapter.py | 24 ++++++++++++++++++ 4 files changed, 57 insertions(+), 7 deletions(-) diff --git a/src/inference_endpoint/dataset_manager/dataset.py b/src/inference_endpoint/dataset_manager/dataset.py index 5b6345cb6..701135801 100644 --- a/src/inference_endpoint/dataset_manager/dataset.py +++ b/src/inference_endpoint/dataset_manager/dataset.py @@ -261,14 +261,19 @@ def load_from_huggingface( def _can_salt(sample: Any) -> DatasetValidationError.Reason | None: """Return the Reason a sample cannot be salted, or None if it can. - Salt requires a dict sample with a str 'prompt' and no 'input_tokens' (which - adapters send verbatim, so a salted 'prompt' would not reach the server). + Salt requires a dict sample with a str 'prompt' and neither 'input_tokens' + nor 'messages'. Both are sent to the server ahead of 'prompt' — adapters + forward 'input_tokens' verbatim, and the OpenAI chat adapter prefers + 'messages' over 'prompt' (openai_msgspec_adapter.py) — so a sample carrying + either would ship an unsalted payload even after 'prompt' is salted. """ Reason = DatasetValidationError.Reason if not isinstance(sample, dict): return Reason.TYPE_MISMATCH if "input_tokens" in sample: return Reason.INPUT_TOKENS_SHADOWING + if "messages" in sample: + return Reason.MESSAGES_SHADOWING if "prompt" not in sample: return Reason.PROMPT_MISSING if not isinstance(sample["prompt"], str): @@ -510,7 +515,7 @@ def _apply_salt(self, data: dict[str, Any]) -> dict[str, Any]: """Prepend a unique salt to the 'prompt' field. with_salt() has validated every sample, so ``data`` is guaranteed to be a - dict with a str 'prompt' and no 'input_tokens'. + dict with a str 'prompt' and neither 'input_tokens' nor 'messages'. """ assert self._salt_rng is not None salt = self._salt_rng.randbytes(8).hex() diff --git a/src/inference_endpoint/exceptions.py b/src/inference_endpoint/exceptions.py index 6853b9e49..093df3730 100644 --- a/src/inference_endpoint/exceptions.py +++ b/src/inference_endpoint/exceptions.py @@ -52,6 +52,10 @@ class Reason(Enum): INPUT_TOKENS_SHADOWING = ( "sample has 'input_tokens'; salt cannot bust a pre-tokenized cache" ) + MESSAGES_SHADOWING = ( + "sample has 'messages'; adapters send that verbatim and prefer it " + "over 'prompt', so a salted 'prompt' would never reach the server" + ) PROMPT_MISSING = "sample has no 'prompt' field" PROMPT_TYPE_MISMATCH = "sample 'prompt' is not a str" UNSPECIFIED = "dataset validation failed" diff --git a/tests/unit/dataset_manager/test_salted_dataset.py b/tests/unit/dataset_manager/test_salted_dataset.py index cf72ab5c0..007c5007e 100644 --- a/tests/unit/dataset_manager/test_salted_dataset.py +++ b/tests/unit/dataset_manager/test_salted_dataset.py @@ -186,15 +186,28 @@ def test_input_tokens_only_raises(self): with pytest.raises(DatasetValidationError, match="input_tokens"): inner.with_salt(random.Random()) - def test_agentic_messages_sample_raises_prompt_missing(self): - # Agentic datasets store dict samples keyed by 'messages', not 'prompt' — - # salt has no text field to prepend, so it's a clear PROMPT_MISSING error. + def test_agentic_messages_sample_raises_messages_shadowing(self): + # Agentic datasets store dict samples keyed by 'messages'. The chat + # adapter sends 'messages' verbatim, so salting 'prompt' cannot reach the + # server — reject with the shadowing reason. inner = _make_loaded_dataset( [{"messages": [{"role": "user", "content": "hi"}]}] ) with pytest.raises(DatasetValidationError) as exc_info: inner.with_salt(random.Random()) - assert exc_info.value.reason is DatasetValidationError.Reason.PROMPT_MISSING + assert exc_info.value.reason is DatasetValidationError.Reason.MESSAGES_SHADOWING + + def test_messages_and_prompt_raises_messages_shadowing(self): + # A sample carrying both 'messages' and a valid str 'prompt' would pass a + # prompt-only check, but to_endpoint_request() prefers 'messages' — so + # _apply_salt() would salt an ignored field and silently fail to bust the + # cache. This must be a hard error, not a valid sample. + inner = _make_loaded_dataset( + [{"messages": [{"role": "user", "content": "hi"}], "prompt": "hello"}] + ) + with pytest.raises(DatasetValidationError) as exc_info: + inner.with_salt(random.Random()) + assert exc_info.value.reason is DatasetValidationError.Reason.MESSAGES_SHADOWING def test_input_tokens_and_prompt_raises(self): inner = _make_loaded_dataset([{"input_tokens": [1, 2, 3], "prompt": "hello"}]) @@ -241,6 +254,10 @@ def test_validate_saltable_raises_on_bad_sample(self): {"input_tokens": [1, 2]}, DatasetValidationError.Reason.INPUT_TOKENS_SHADOWING, ), + ( + {"messages": [{"role": "user", "content": "hi"}], "prompt": "hi"}, + DatasetValidationError.Reason.MESSAGES_SHADOWING, + ), ({"question": "?"}, DatasetValidationError.Reason.PROMPT_MISSING), ({"prompt": 42}, DatasetValidationError.Reason.PROMPT_TYPE_MISMATCH), ], diff --git a/tests/unit/openai/test_msgspec_adapter.py b/tests/unit/openai/test_msgspec_adapter.py index 1fac94d26..6091bf814 100644 --- a/tests/unit/openai/test_msgspec_adapter.py +++ b/tests/unit/openai/test_msgspec_adapter.py @@ -359,3 +359,27 @@ def test_dataset_transforms_preserve_chat_template_kwargs_dict(): request = OpenAIMsgspecAdapter.to_endpoint_request(Query(id="q5", data=row)) payload = json.loads(msgspec.json.encode(request)) assert payload["chat_template_kwargs"] == chat_template_kwargs + + +@pytest.mark.unit +def test_messages_shadow_prompt_in_request(): + """'messages' takes precedence over 'prompt' in the emitted request. + + A sample carrying both fields sends only 'messages'; the 'prompt' is dropped + entirely. This is why Dataset._can_salt() rejects such samples — salting the + ignored 'prompt' would not change the payload and would silently fail to bust + the KV cache. + """ + query = Query( + id="q6", + data={ + "model": "m", + "messages": [{"role": "user", "content": "authoritative"}], + "prompt": "[deadbeefdeadbeef] shadowed", + }, + ) + request = OpenAIMsgspecAdapter.to_endpoint_request(query) + payload = json.loads(msgspec.json.encode(request)) + + assert [m["content"] for m in payload["messages"]] == ["authoritative"] + assert "shadowed" not in json.dumps(payload) From 6d28c00e832c2db08bcfe19b5e2e0be6527cddfa Mon Sep 17 00:00:00 2001 From: arekay-nv <230885705+arekay-nv@users.noreply.github.com> Date: Fri, 14 Aug 2026 21:34:21 -0500 Subject: [PATCH 4/6] feat(warmup): warn when warmup runs without salt Warmup without --warmup-salt issues prompts verbatim, priming the server KV/prefix cache so the measured phase can be served warm and understate latency. Surface it as a warning at dataset-load time. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../commands/benchmark/execute.py | 13 ++++++++++-- tests/unit/commands/test_benchmark.py | 20 +++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/inference_endpoint/commands/benchmark/execute.py b/src/inference_endpoint/commands/benchmark/execute.py index 592e01d90..6cd23fe43 100644 --- a/src/inference_endpoint/commands/benchmark/execute.py +++ b/src/inference_endpoint/commands/benchmark/execute.py @@ -406,8 +406,17 @@ def _load_datasets( # the same check later; this earlier call is deliberate (not redundant), # so an invalid dataset aborts before the subprocess fan-out. warmup = config.settings.warmup - if warmup.enabled and warmup.salt: - dataloader.validate_saltable() + if warmup.enabled: + if warmup.salt: + dataloader.validate_saltable() + else: + logger.warning( + "Warmup is enabled without salt (--warmup-salt): warmup " + "prompts are issued verbatim, so the server may serve the " + "measured phase from a warm KV/prefix cache and understate " + "latency. Enable --warmup-salt on text-'prompt' datasets to " + "bust the cache." + ) if perf_cfg.accuracy_config is not None: accuracy_config = perf_cfg.accuracy_config diff --git a/tests/unit/commands/test_benchmark.py b/tests/unit/commands/test_benchmark.py index 269ed7157..3e0d3819b 100644 --- a/tests/unit/commands/test_benchmark.py +++ b/tests/unit/commands/test_benchmark.py @@ -443,6 +443,26 @@ def test_skips_validation_when_salt_off(self, mock_validate, tmp_path): _load_datasets(config, tmp_path, TestMode.PERF) mock_validate.assert_not_called() + def test_warns_when_warmup_enabled_without_salt(self, tmp_path, caplog): + """Warmup without salt primes the server cache with verbatim prompts, + risking an understated measured phase — surface it as a warning.""" + config = self._config(tmp_path, WarmupConfig(enabled=True, salt=False)) + with caplog.at_level(logging.WARNING): + _load_datasets(config, tmp_path, TestMode.PERF) + assert any( + "warmup is enabled without salt" in r.message.lower() + for r in caplog.records + ) + + def test_no_warning_when_warmup_salt_enabled(self, tmp_path, caplog): + config = self._config(tmp_path, WarmupConfig(enabled=True, salt=True)) + with caplog.at_level(logging.WARNING): + _load_datasets(config, tmp_path, TestMode.PERF) + assert not any( + "warmup is enabled without salt" in r.message.lower() + for r in caplog.records + ) + def test_unsaltable_perf_dataset_raises_before_spawn(self, tmp_path): """Real validation (unpatched) rejects a non-saltable perf dataset at load time — an int 'prompt' cannot be salted.""" From cbaa2c3441dbfefbf16d0cdba23f6ad2da912ec8 Mon Sep 17 00:00:00 2001 From: Rashid Kaleem <230885705+arekay-nv@users.noreply.github.com> Date: Mon, 17 Aug 2026 20:55:43 -0500 Subject: [PATCH 5/6] fix(warmup): harden salt-validation error handling per review Address review-council + reviewer feedback on the salt-failure-hard-error change: - Rename _can_salt -> _check_unsaltable (truthy Reason == unsaltable) and fix the validate_saltable walrus so it raises when a sample IS unsaltable - Add DatasetParseError for --dataset string parse failures; remove the vestigial Reason.UNSPECIFIED - Rename the list-prompt reason to PROMPT_LIST_UNSUPPORTED (spec-accurate: an OpenAI list prompt is a batch / token-ID array, not necessarily multimodal) - Drop the duplicate cyclopts help= on WarmupConfig.salt; Field(description=) is the single source for --help and the YAML templates - Make DatasetValidationError copy/pickle-safe via a 3-tuple __reduce__ (restores __dict__/__notes__ state, matching CPython's default reducer) - Propagate typed CLI errors from audit phases (InputValidationError -> exit 2) instead of recasting them to ExecutionError (exit 4) - Raise a clear ValueError on conflicting scalar/nested --dataset keys instead of an uncaught TypeError that escaped to an exit-1 traceback - Document the newly reachable exit 2 in audit.py and compliance_audit_plan.md - Add regression tests for all of the above Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/compliance_audit_plan.md | 18 ++-- src/inference_endpoint/commands/audit.py | 21 ++-- .../commands/benchmark/cli.py | 11 +-- src/inference_endpoint/config/schema.py | 9 +- src/inference_endpoint/config/utils.py | 8 ++ .../dataset_manager/dataset.py | 21 ++-- src/inference_endpoint/exceptions.py | 32 +++++- tests/unit/commands/test_benchmark.py | 99 +++++++++++++++++++ .../dataset_manager/test_salted_dataset.py | 16 ++- tests/unit/openai/test_msgspec_adapter.py | 2 +- tests/unit/test_exceptions.py | 56 +++++++++++ 11 files changed, 249 insertions(+), 44 deletions(-) diff --git a/docs/compliance_audit_plan.md b/docs/compliance_audit_plan.md index 4f0625b44..dd34df7c9 100644 --- a/docs/compliance_audit_plan.md +++ b/docs/compliance_audit_plan.md @@ -140,7 +140,7 @@ benchmark from-config ### Program flow (output-caching audit / MLPerf TEST04, two phases) Every decision gate is shown, with the exit code it produces. Exit codes: -`0` PASS · `1` FAIL · `3` SetupError · `4` ExecutionError · `130` interrupted +`0` PASS · `1` FAIL · `2` InputValidationError · `3` SetupError · `4` ExecutionError · `130` interrupted (during the main run: the audit never starts; during the audit: the perf report is already written). @@ -397,13 +397,15 @@ The generic loop never names a specific test: or re-scores them) and has `audit=None` to prevent re-entry into `run_audit`. A phase can override this per-`AuditRunSpec` via `test_mode` (`ACC`/`BOTH` keeps its accuracy datasets); the orchestrator reads `spec.test_mode` rather than hardcoding perf-only. This override is - supported but currently unused — every registered audit (TEST04) runs perf-only. If any phase raises - (`SetupError` / `ExecutionError`), `run_audit` aborts **without verifying** — a crashed - phase must never produce a result. A phase that returns but whose `Report.complete` is - `False` (metrics drain timed out, or the run was interrupted → partial stats) is likewise - rejected with `ExecutionError` — a result is never certified on partial data. Errors - propagate to the standard CLI handler (`main.py`), which maps `SetupError` → exit `3` and - `ExecutionError` → exit `4`. + supported but currently unused — every registered audit (TEST04) runs perf-only. If any phase + raises a typed CLI error (`InputValidationError` / `SetupError` / `ExecutionError`), `run_audit` + aborts **without verifying** — a crashed phase must never produce a result. In particular a + phase's `setup_benchmark` can raise `InputValidationError` (e.g. `DatasetValidationError` for an + unsaltable warmup dataset); it propagates verbatim rather than being recast as `ExecutionError`. + A phase that returns but whose `Report.complete` is `False` (metrics drain timed out, or the run + was interrupted → partial stats) is likewise rejected with `ExecutionError` — a result is never + certified on partial data. Errors propagate to the standard CLI handler (`main.py`), which maps + `InputValidationError` → exit `2`, `SetupError` → exit `3`, and `ExecutionError` → exit `4`. 5. `result = test.verify(runs, cfg)` 6. Atomically write the result (`tmp → fsync → rename → fsync(parent)`). 7. Return the typed `AuditResult`. Because `run_benchmark` currently returns `None` and diff --git a/src/inference_endpoint/commands/audit.py b/src/inference_endpoint/commands/audit.py index 1b8eddca1..b29000ae4 100644 --- a/src/inference_endpoint/commands/audit.py +++ b/src/inference_endpoint/commands/audit.py @@ -20,10 +20,12 @@ run_audit returns an AuditResult; cli.py maps PASS/FAIL and main.run() maps exceptions to process exit codes: - 0 PASS — result.passed is True - 1 FAIL — result.passed is False (cli.py raises CLIError) - 3 SetupError — config invalid for the audit (bad sample count/index) - 4 ExecutionError — a phase run failed or produced partial data + 0 PASS — result.passed is True + 1 FAIL — result.passed is False (cli.py raises CLIError) + 2 InputValidationError — a phase rejected user input (e.g. an unsaltable + warmup dataset), propagated verbatim from setup + 3 SetupError — config invalid for the audit (bad sample count/index) + 4 ExecutionError — a phase run failed or produced partial data """ from __future__ import annotations @@ -35,7 +37,7 @@ from ..compliance import AuditRunArtifacts, get_audit_test from ..compliance.result import AuditResult, write_result from ..config.schema import BenchmarkConfig, DatasetType -from ..exceptions import ExecutionError, SetupError +from ..exceptions import CLIError, ExecutionError, SetupError from .benchmark.execute import ( BenchmarkResult, TestMode, @@ -65,6 +67,9 @@ def run_audit(config: BenchmarkConfig, base_report_dir: Path) -> AuditResult: AuditResult — always returned; caller maps passed/failed to exit code. Raises: + InputValidationError: A phase rejected user input (e.g. setup_benchmark + raising DatasetValidationError for an unsaltable warmup dataset); + propagated verbatim rather than recast as a phase ExecutionError. SetupError: Config invalid for audit (missing audit block, bad sample count/index for the dataset). ExecutionError: A phase benchmark run failed. @@ -116,7 +121,11 @@ def run_audit(config: BenchmarkConfig, base_report_dir: Path) -> AuditResult: ) bench = run_benchmark_async(ctx) finalize_benchmark(ctx, bench) - except (SetupError, ExecutionError): + except CLIError: + # Typed CLI errors already carry the right exit code — SetupError (3), + # ExecutionError (4), and the InputValidationError (2) that + # setup_benchmark raises for an unsaltable warmup dataset. Propagate + # them; only genuinely unexpected exceptions become a phase failure. raise except Exception as exc: raise ExecutionError(f"Audit phase '{spec.label}' failed: {exc}") from exc diff --git a/src/inference_endpoint/commands/benchmark/cli.py b/src/inference_endpoint/commands/benchmark/cli.py index e17400412..319bb03c3 100644 --- a/src/inference_endpoint/commands/benchmark/cli.py +++ b/src/inference_endpoint/commands/benchmark/cli.py @@ -38,7 +38,7 @@ ) from inference_endpoint.exceptions import ( CLIError, - DatasetValidationError, + DatasetParseError, InputValidationError, ) @@ -67,14 +67,9 @@ def _run( f"{'.'.join(str(p) for p in err['loc'])}: {err['msg']}" for err in e.errors() ) - # --dataset parse failures aren't yet mapped to a specific Reason. - raise DatasetValidationError( - DatasetValidationError.Reason.UNSPECIFIED, f"Invalid --dataset: {msgs}" - ) from e + raise DatasetParseError(f"Invalid --dataset: {msgs}") from e except ValueError as e: - raise DatasetValidationError( - DatasetValidationError.Reason.UNSPECIFIED, f"Invalid --dataset: {e}" - ) from e + raise DatasetParseError(f"Invalid --dataset: {e}") from e if config.audit is None: run_benchmark(config, mode) return diff --git a/src/inference_endpoint/config/schema.py b/src/inference_endpoint/config/schema.py index 3f6c0238a..d7d8be480 100644 --- a/src/inference_endpoint/config/schema.py +++ b/src/inference_endpoint/config/schema.py @@ -757,14 +757,7 @@ class WarmupConfig(BaseModel): ] = Field(None, gt=0, description="Warmup request count (None = full dataset once)") salt: Annotated[ bool, - cyclopts.Parameter( - alias="--warmup-salt", - help=( - "Prepend a unique random hex salt to each warmup prompt. Requires " - "text-'prompt' samples; enabling it on a pre-tokenized " - "('input_tokens') dataset is a hard error." - ), - ), + cyclopts.Parameter(alias="--warmup-salt"), ] = Field( False, description=( diff --git a/src/inference_endpoint/config/utils.py b/src/inference_endpoint/config/utils.py index fd62d2bb5..9faf4a1a0 100644 --- a/src/inference_endpoint/config/utils.py +++ b/src/inference_endpoint/config/utils.py @@ -119,6 +119,14 @@ def parse_dataset_string(s: str) -> dict[str, object]: for seg in segments[:-1]: if seg not in target: target[seg] = {} # type: ignore[index] + elif not isinstance(target[seg], dict): + # e.g. 'parser=x,parser.prompt=y' — 'parser' is set as both a + # scalar and a nested key. Without this, target[seg] would be a + # str and the next assignment raises an uncaught TypeError. + raise ValueError( + f"Conflicting option '{key}': '{seg}' is set both as a value " + f"and as a nested key. Use one form for '{seg}'." + ) target = target[seg] # type: ignore[assignment] target[segments[-1]] = value # type: ignore[index] diff --git a/src/inference_endpoint/dataset_manager/dataset.py b/src/inference_endpoint/dataset_manager/dataset.py index 701135801..4cff1fc3b 100644 --- a/src/inference_endpoint/dataset_manager/dataset.py +++ b/src/inference_endpoint/dataset_manager/dataset.py @@ -258,7 +258,7 @@ def load_from_huggingface( return ds[split].to_pandas() -def _can_salt(sample: Any) -> DatasetValidationError.Reason | None: +def _check_unsaltable(sample: Any) -> DatasetValidationError.Reason | None: """Return the Reason a sample cannot be salted, or None if it can. Salt requires a dict sample with a str 'prompt' and neither 'input_tokens' @@ -266,6 +266,9 @@ def _can_salt(sample: Any) -> DatasetValidationError.Reason | None: forward 'input_tokens' verbatim, and the OpenAI chat adapter prefers 'messages' over 'prompt' (openai_msgspec_adapter.py) — so a sample carrying either would ship an unsalted payload even after 'prompt' is salted. + A list-form 'prompt' (an OpenAI batch / token-ID array, or this project's + multimodal content-part convention) is an explicitly unsupported salt path + and is rejected with its own reason. """ Reason = DatasetValidationError.Reason if not isinstance(sample, dict): @@ -276,6 +279,8 @@ def _can_salt(sample: Any) -> DatasetValidationError.Reason | None: return Reason.MESSAGES_SHADOWING if "prompt" not in sample: return Reason.PROMPT_MISSING + if isinstance(sample["prompt"], list): + return Reason.PROMPT_LIST_UNSUPPORTED if not isinstance(sample["prompt"], str): return Reason.PROMPT_TYPE_MISMATCH return None @@ -467,9 +472,12 @@ def load_sample(self, index: int) -> Any: def validate_saltable(self) -> None: """Raise if any loaded sample cannot be salted. - salt requires a dict sample with a text ('str') 'prompt' and no - 'input_tokens' (adapters send those verbatim, so a salted 'prompt' would - never reach the server). A non-saltable sample is an error, not a silent + salt requires a dict sample with a text ('str') 'prompt' and neither + 'input_tokens' nor 'messages' (adapters send those verbatim / prefer + 'messages' over 'prompt', so a salted 'prompt' would never reach the + server); a list-form 'prompt' (batch / token-IDs / multimodal content + parts) is an unsupported salt path and is rejected too. A non-saltable + sample is an error, not a silent skip: skipping would leave the KV cache un-busted. Every sample is checked — a single invalid item fails the run, because the seeded warmup subset can draw any index and salt correctness is all-or-nothing. Called @@ -482,13 +490,12 @@ def validate_saltable(self) -> None: """ assert self.data is not None, "Dataset not loaded. Call load() first." for i, sample in enumerate(self.data): - reason = _can_salt(sample) - if reason is not None: + if reason := _check_unsaltable(sample): raise DatasetValidationError( reason, detail=( f"sample {i} (index into the loaded, post-transform " - f"order); disable salt (--warmup-salt / warmup.salt: " + f"order); disable salt (--no-warmup-salt / warmup.salt: " f"false) or use a text-prompt dataset" ), ) diff --git a/src/inference_endpoint/exceptions.py b/src/inference_endpoint/exceptions.py index 093df3730..555de5110 100644 --- a/src/inference_endpoint/exceptions.py +++ b/src/inference_endpoint/exceptions.py @@ -39,14 +39,14 @@ class InputValidationError(CLIError): class DatasetValidationError(InputValidationError): - """Invalid --dataset string or dataset configuration. + """A loaded dataset sample fails salt validation. The failure category is a ``Reason``; ``detail`` carries the specifics - (offending sample index, remediation hint, parser error text). + (offending sample index, remediation hint). """ class Reason(Enum): - """Why a dataset failed validation.""" + """Why a sample cannot be salted.""" TYPE_MISMATCH = "sample is not a dict" INPUT_TOKENS_SHADOWING = ( @@ -58,7 +58,11 @@ class Reason(Enum): ) PROMPT_MISSING = "sample has no 'prompt' field" PROMPT_TYPE_MISMATCH = "sample 'prompt' is not a str" - UNSPECIFIED = "dataset validation failed" + PROMPT_LIST_UNSUPPORTED = ( + "sample 'prompt' is a list (OpenAI batch / token-IDs, or this " + "project's multimodal content parts); salt supports only a single " + "text 'prompt'" + ) def __init__( self, @@ -70,6 +74,26 @@ def __init__( message = reason.value if detail is None else f"{reason.value}: {detail}" super().__init__(message) + def __reduce__( + self, + ) -> tuple[type, tuple["DatasetValidationError.Reason", str | None], dict]: + # args holds the formatted message, but __init__ takes (reason, detail); + # reconstruct from the structured fields so copy/pickle round-trips. The + # third element mirrors CPython's default exception reducer: it restores + # __dict__ (and thus __notes__ / any caller-added attribute) as state. + return (type(self), (self.reason, self.detail), self.__dict__) + + +class DatasetParseError(InputValidationError): + """A --dataset CLI string could not be parsed into a dataset config. + + Distinct from DatasetValidationError: the failure is in the user's input + string (bad format / key=value), before any dataset is loaded — so there is + no sample index or Reason, just the underlying parse message. + """ + + pass + class SetupError(CLIError): """Error during initialization/setup. diff --git a/tests/unit/commands/test_benchmark.py b/tests/unit/commands/test_benchmark.py index 3e0d3819b..7abc636ae 100644 --- a/tests/unit/commands/test_benchmark.py +++ b/tests/unit/commands/test_benchmark.py @@ -403,6 +403,78 @@ def test_dataset_string_coercion( assert ds.accuracy_config is not None assert ds.accuracy_config.eval_method == acc_eval_method + @pytest.mark.unit + def test_malformed_dataset_string_raises_parse_error(self): + """A malformed --dataset string surfaces as DatasetParseError, not a + DatasetValidationError with a vestigial Reason. 'badoption' (no '=') + raises inside the field validator, which pydantic wraps — exercising the + `except ValidationError` arm of _run.""" + from inference_endpoint.commands.benchmark import cli + from inference_endpoint.exceptions import DatasetParseError + + config = OfflineConfig(**_OFFLINE_KWARGS | {"datasets": []}) + with pytest.raises(DatasetParseError, match="Invalid --dataset"): + cli._run(config, ["data.csv,badoption"], TestMode.PERF) + + @pytest.mark.unit + def test_dataset_string_raw_valueerror_raises_parse_error(self): + """A bare ValueError from with_updates (not wrapped by pydantic) also + surfaces as DatasetParseError — covering _run's second `except` arm.""" + from inference_endpoint.commands.benchmark import cli + from inference_endpoint.exceptions import DatasetParseError + + config = MagicMock() + config.datasets = [] + config.with_updates.side_effect = ValueError("raw parse failure") + with pytest.raises(DatasetParseError, match="Invalid --dataset"): + cli._run(config, ["x.jsonl"], TestMode.PERF) + + @pytest.mark.unit + def test_scalar_then_dotted_key_collision_raises_parse_error(self): + """A scalar option shadowed by a dotted one (parser=x,parser.prompt=y) + must surface as DatasetParseError, not an uncaught TypeError → exit-1 + traceback. parse_dataset_string raises ValueError, which _run wraps.""" + from inference_endpoint.commands.benchmark import cli + from inference_endpoint.exceptions import DatasetParseError + + config = OfflineConfig(**_OFFLINE_KWARGS | {"datasets": []}) + with pytest.raises(DatasetParseError, match="Invalid --dataset"): + cli._run(config, ["d.jsonl,parser=x,parser.prompt=y"], TestMode.PERF) + + +@pytest.mark.unit +class TestRunAuditErrorPropagation: + """run_audit must not mask a phase's typed CLI error as a generic + ExecutionError. An unsaltable warmup makes setup_benchmark raise + DatasetValidationError (InputValidationError → exit 2); in the audit.only + path that must propagate, not become ExecutionError (exit 4).""" + + def test_phase_input_validation_error_propagates(self, monkeypatch, tmp_path): + from inference_endpoint.commands import audit as audit_mod + from inference_endpoint.config.schema import DatasetType + from inference_endpoint.exceptions import DatasetValidationError + + spec = MagicMock(label="reference", test_mode=TestMode.PERF) + fake_test = MagicMock() + fake_test.plan_runs.return_value = [spec] + monkeypatch.setattr(audit_mod, "get_audit_test", lambda _test: fake_test) + + def _raise_validation(*args, **kwargs): + raise DatasetValidationError( + DatasetValidationError.Reason.INPUT_TOKENS_SHADOWING, "sample 0" + ) + + monkeypatch.setattr(audit_mod, "setup_benchmark", _raise_validation) + + config = MagicMock() + config.audit = MagicMock() + config.datasets = [MagicMock(type=DatasetType.PERFORMANCE)] + config.with_updates.return_value = MagicMock() + + # Not recast to ExecutionError — the typed validation error propagates. + with pytest.raises(DatasetValidationError): + audit_mod.run_audit(config, tmp_path) + @pytest.mark.unit class TestLoadDatasetsSaltValidation: @@ -625,6 +697,33 @@ def test_use_legacy_loadgen_qps_metrics_default_and_disable(self): lp = bound.arguments["config"].settings.load_pattern assert lp.use_legacy_loadgen_qps_metrics is False + @pytest.mark.unit + def test_warmup_salt_flag_default_and_negative(self): + """warmup.salt defaults off; --warmup-salt enables it; --no-warmup-salt + (the flag the salt-validation remediation message points to) disables + it.""" + base = [ + "offline", + "--endpoints", + "http://h:80", + "--model", + "m", + "--dataset", + "d.jsonl", + ] + _, bound, _ = benchmark_app.parse_args(base, exit_on_error=False) + assert bound.arguments["config"].settings.warmup.salt is False + + _, bound, _ = benchmark_app.parse_args( + [*base, "--warmup-salt"], exit_on_error=False + ) + assert bound.arguments["config"].settings.warmup.salt is True + + _, bound, _ = benchmark_app.parse_args( + [*base, "--no-warmup-salt"], exit_on_error=False + ) + assert bound.arguments["config"].settings.warmup.salt is False + @pytest.mark.unit def test_loadgen_flag_serialized_only_for_poisson(self): """``use_legacy_loadgen_qps_metrics`` is dropped from the serialized diff --git a/tests/unit/dataset_manager/test_salted_dataset.py b/tests/unit/dataset_manager/test_salted_dataset.py index 007c5007e..324e89b73 100644 --- a/tests/unit/dataset_manager/test_salted_dataset.py +++ b/tests/unit/dataset_manager/test_salted_dataset.py @@ -167,14 +167,22 @@ def test_non_dict_sample_raises(self): with pytest.raises(DatasetValidationError, match="dict"): inner.with_salt(random.Random()) - def test_multimodal_list_prompt_raises(self): + def test_list_prompt_raises(self): + # A list-form 'prompt' (here, multimodal content parts; also OpenAI + # batch / token-ID arrays per the spec) is an explicitly unsupported + # salt path — reject with a dedicated, actionable reason, not the + # generic non-str message. content_parts = [ {"type": "text", "text": "describe this image"}, {"type": "image_url", "image_url": {"url": "data:image/png;base64,abc"}}, ] inner = _make_loaded_dataset([{"prompt": content_parts}]) - with pytest.raises(DatasetValidationError, match="str"): + with pytest.raises(DatasetValidationError) as exc_info: inner.with_salt(random.Random()) + assert ( + exc_info.value.reason + is DatasetValidationError.Reason.PROMPT_LIST_UNSUPPORTED + ) def test_non_str_prompt_raises(self): inner = _make_loaded_dataset([{"prompt": 42}]) @@ -260,6 +268,10 @@ def test_validate_saltable_raises_on_bad_sample(self): ), ({"question": "?"}, DatasetValidationError.Reason.PROMPT_MISSING), ({"prompt": 42}, DatasetValidationError.Reason.PROMPT_TYPE_MISMATCH), + ( + {"prompt": [{"type": "text", "text": "hi"}]}, + DatasetValidationError.Reason.PROMPT_LIST_UNSUPPORTED, + ), ], ) def test_error_exposes_typed_reason(self, sample, expected_reason): diff --git a/tests/unit/openai/test_msgspec_adapter.py b/tests/unit/openai/test_msgspec_adapter.py index 6091bf814..72e8127f2 100644 --- a/tests/unit/openai/test_msgspec_adapter.py +++ b/tests/unit/openai/test_msgspec_adapter.py @@ -366,7 +366,7 @@ def test_messages_shadow_prompt_in_request(): """'messages' takes precedence over 'prompt' in the emitted request. A sample carrying both fields sends only 'messages'; the 'prompt' is dropped - entirely. This is why Dataset._can_salt() rejects such samples — salting the + entirely. This is why _check_unsaltable() rejects such samples — salting the ignored 'prompt' would not change the payload and would silently fail to bust the KV cache. """ diff --git a/tests/unit/test_exceptions.py b/tests/unit/test_exceptions.py index 22834e64d..d232bda57 100644 --- a/tests/unit/test_exceptions.py +++ b/tests/unit/test_exceptions.py @@ -24,8 +24,13 @@ - Clear error categorization (validation vs setup vs execution) """ +import copy +import pickle + +import pytest from inference_endpoint.exceptions import ( CLIError, + DatasetValidationError, ExecutionError, InputValidationError, SetupError, @@ -78,3 +83,54 @@ def test_exception_chaining(self): assert chained.__cause__ is original assert isinstance(chained, InputValidationError) + + +class TestDatasetValidationErrorRoundTrip: + """DatasetValidationError carries structured fields (reason + detail), so it + overrides __reduce__ to stay copy/pickle-safe: args holds only the formatted + message, which __init__ cannot consume as (reason, detail).""" + + @pytest.mark.unit + @pytest.mark.parametrize( + "exc", + [ + DatasetValidationError( + DatasetValidationError.Reason.INPUT_TOKENS_SHADOWING + ), + DatasetValidationError( + DatasetValidationError.Reason.PROMPT_LIST_UNSUPPORTED, "sample 3" + ), + ], + ids=["reason-only", "reason-and-detail"], + ) + @pytest.mark.parametrize( + # pickle round-trips a self-constructed exception (trusted, not external + # input) — this asserts __reduce__ reconstructs it from reason + detail. + "clone_fn", + [copy.copy, copy.deepcopy, lambda e: pickle.loads(pickle.dumps(e))], + ids=["copy", "deepcopy", "pickle"], + ) + def test_round_trip_preserves_reason_detail_message(self, exc, clone_fn): + clone = clone_fn(exc) + assert clone.reason is exc.reason + assert clone.detail == exc.detail + assert str(clone) == str(exc) + + @pytest.mark.unit + @pytest.mark.parametrize( + "clone_fn", + [copy.copy, copy.deepcopy, lambda e: pickle.loads(pickle.dumps(e))], + ids=["copy", "deepcopy", "pickle"], + ) + def test_round_trip_preserves_notes_and_extra_state(self, clone_fn): + # The 3-tuple __reduce__ restores __dict__ as state, so __notes__ (PEP + # 678) and any caller-added attribute survive — matching CPython's + # default exception reducer. + exc = DatasetValidationError( + DatasetValidationError.Reason.MESSAGES_SHADOWING, "sample 7" + ) + exc.add_note("diagnostic note") + exc.extra_attr = "kept" + clone = clone_fn(exc) + assert getattr(clone, "__notes__", None) == ["diagnostic note"] + assert clone.extra_attr == "kept" From a6e284f388e41ff053e2852b09798e6b6658b88a Mon Sep 17 00:00:00 2001 From: Rashid Kaleem <230885705+arekay-nv@users.noreply.github.com> Date: Mon, 17 Aug 2026 21:31:27 -0500 Subject: [PATCH 6/6] fix(test): unpack 2-tuple from _load_datasets in accuracy-only test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit main #437 refactored _load_datasets to return (dataloader, eval_configs). test_accuracy_only_skips_salt_validation is branch-only, so the merge kept its stale 3-tuple unpack — the sole site the merge missed. Adopt the 2-tuple form; perf_loader is still the first element, so the assertion is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/unit/commands/test_benchmark.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/commands/test_benchmark.py b/tests/unit/commands/test_benchmark.py index c70791ba5..26f2285a2 100644 --- a/tests/unit/commands/test_benchmark.py +++ b/tests/unit/commands/test_benchmark.py @@ -608,7 +608,7 @@ def test_accuracy_only_skips_salt_validation(self, tmp_path): patch.object(SWEBenchScorer, "preflight"), patch.object(SWEBench, "generate", return_value=fake_acc_df), ): - perf_loader, _, _ = _load_datasets(config, tmp_path, TestMode.ACC) + perf_loader, _ = _load_datasets(config, tmp_path, TestMode.ACC) assert perf_loader is None