diff --git a/src/hflow/build_ai_vlm_checks.py b/src/hflow/build_ai_vlm_checks.py index a51c1031..3748d943 100644 --- a/src/hflow/build_ai_vlm_checks.py +++ b/src/hflow/build_ai_vlm_checks.py @@ -659,8 +659,13 @@ def _check_version(configuration: _RegisteredBuildAICheckConfiguration) -> StepV } match configuration.execution: case OpenAICompatibleExecution() as execution: - # Keep the existing contract shape so migrating to the explicit - # execution value does not invalidate otherwise identical results. + # The contract shape was kept stable during the migration to the + # explicit execution value so that migration alone did not + # invalidate otherwise identical results. That constraint applied + # to that migration only: the contract must include every knob + # that changes which items produce results, so max_retries is + # version-worthy even though it only affects request liveness + # (#404). Adding a field re-mints the version by design. version_contract.update( { "endpoint": execution.endpoint, @@ -668,15 +673,19 @@ def _check_version(configuration: _RegisteredBuildAICheckConfiguration) -> StepV "response_format": execution.response_format.value, "temperature": execution.temperature, "max_tokens": execution.max_tokens, + "max_retries": execution.max_retries, } ) case HFlowHostedExecution() as execution: + # Same rule, applied symmetrically: the timeout decides whether a + # slow-but-valid response is included in the corpus at all. version_contract.update( { "execution": "hflow-hosted", "hosted_check_endpoint": _hosted_check_endpoint( execution, configuration.task_definition.task ), + "request_timeout_seconds": execution.request_timeout_seconds, } ) case unexpected_execution: diff --git a/tests/test_build_ai_vlm_checks.py b/tests/test_build_ai_vlm_checks.py index 2609f7ea..4f35227f 100644 --- a/tests/test_build_ai_vlm_checks.py +++ b/tests/test_build_ai_vlm_checks.py @@ -274,3 +274,155 @@ def test_hosted_response_refuses_a_prediction_outside_the_check_contract() -> No "raw_response": "3", }, ) + + +# --- version contract covers every knob that changes result completeness (#404) + + +def _versions_for(executions: list) -> list: + versions = [] + for index, execution in enumerate(executions): + application = hflow.App( + f"app-{index}-{abs(hash(execution))}", + data_root=Path(f"/tmp/unused-{index}-{abs(hash(execution))}"), + default_checks=(), + ) + hflow.build_ai_vlm_checks.register_hand_visibility(application, execution=execution) + versions.append(application.checks[0].version) + return versions + + +def test_check_version_stable_when_every_covered_field_is_identical(tmp_path: Path) -> None: + """DoD 3: a configuration identical in every covered field keeps its + current version, so unchanged methodology never silently invalidates.""" + first_application = hflow.App("first", data_root=tmp_path / "first", default_checks=()) + second_application = hflow.App("second", data_root=tmp_path / "second", default_checks=()) + execution = hflow.build_ai_vlm_checks.OpenAICompatibleExecution( + endpoint="http://localhost:8000/v1", + model="model-a", + temperature=0.5, + max_tokens=512, + max_retries=3, + ) + hflow.build_ai_vlm_checks.register_hand_visibility(first_application, execution=execution) + hflow.build_ai_vlm_checks.register_hand_visibility(second_application, execution=execution) + + assert first_application.checks[0].version == second_application.checks[0].version + + +# Golden versions for two fixed configurations, one per execution branch. +# +# The equality test above only proves _check_version is a function: it cannot +# fail unless the same input starts producing two answers. The property DoD 3 +# actually claims is that unchanged methodology keeps its identity across +# changes to this module, and only a recorded value can hold that. Adding a +# field to the contract, renaming a key, or reordering nothing at all silently +# re-mints every stored check version; here it fails instead. +# +# Editing these strings is the signal, not the chore. Change them only +# together with a deliberate contract change, and say in the PR why every +# existing Build AI result is being invalidated. +_GOLDEN_OPENAI_CHECK_VERSION = "build-ai-single-frame-v1-2aed30388241d554" +_GOLDEN_HOSTED_CHECK_VERSION = "build-ai-single-frame-v1-400d7f82abd83534" + + +def test_check_version_is_pinned_for_a_fixed_openai_configuration(tmp_path: Path) -> None: + application = hflow.App("golden-openai", data_root=tmp_path, default_checks=()) + hflow.build_ai_vlm_checks.register_hand_visibility( + application, + execution=hflow.build_ai_vlm_checks.OpenAICompatibleExecution( + endpoint="http://localhost:8000/v1", + model="model-a", + temperature=0.5, + max_tokens=512, + max_retries=3, + ), + ) + + assert str(application.checks[0].version) == _GOLDEN_OPENAI_CHECK_VERSION + + +def test_check_version_is_pinned_for_a_fixed_hosted_configuration(tmp_path: Path) -> None: + application = hflow.App("golden-hosted", data_root=tmp_path, default_checks=()) + hflow.build_ai_vlm_checks.register_hand_visibility( + application, + execution=hflow.build_ai_vlm_checks.HFlowHostedExecution( + check_version=1, + request_timeout_seconds=30.0, + ), + ) + + assert str(application.checks[0].version) == _GOLDEN_HOSTED_CHECK_VERSION + + +def test_check_version_changes_with_max_retries(tmp_path: Path) -> None: + """max_retries decides whether a transient error becomes a prediction or + a failed run: retries change which items produce answers at all, so two + executions differing only in retries must not share a version.""" + first_application = hflow.App("first", data_root=tmp_path / "first", default_checks=()) + second_application = hflow.App("second", data_root=tmp_path / "second", default_checks=()) + hflow.build_ai_vlm_checks.register_hand_visibility( + first_application, + execution=hflow.build_ai_vlm_checks.OpenAICompatibleExecution( + endpoint="http://localhost:8000/v1", model="model-a", max_retries=0 + ), + ) + hflow.build_ai_vlm_checks.register_hand_visibility( + second_application, + execution=hflow.build_ai_vlm_checks.OpenAICompatibleExecution( + endpoint="http://localhost:8000/v1", model="model-a", max_retries=5 + ), + ) + + assert first_application.checks[0].version != second_application.checks[0].version + + +def test_check_version_changes_with_request_timeout_seconds(tmp_path: Path) -> None: + """The hosted branch applies the same rule: a timeout decides whether a + slow-but-valid response is included, so the field belongs in identity.""" + first_application = hflow.App("first", data_root=tmp_path / "first", default_checks=()) + second_application = hflow.App("second", data_root=tmp_path / "second", default_checks=()) + hflow.build_ai_vlm_checks.register_hand_visibility( + first_application, + execution=hflow.build_ai_vlm_checks.HFlowHostedExecution( + check_version=1, request_timeout_seconds=1.0 + ), + ) + hflow.build_ai_vlm_checks.register_hand_visibility( + second_application, + execution=hflow.build_ai_vlm_checks.HFlowHostedExecution( + check_version=1, request_timeout_seconds=60.0 + ), + ) + + assert first_application.checks[0].version != second_application.checks[0].version + + +def test_check_version_applies_the_rule_symmetrically_across_branches( + tmp_path: Path, +) -> None: + """DoD 4: both branches treat the rule the same way. Each branch must + change its version when its own completeness knob changes, by the same + mechanism (the contract), not by an asymmetric special case.""" + openai_versions = _versions_for( + [ + hflow.build_ai_vlm_checks.OpenAICompatibleExecution( + endpoint="http://localhost:8000/v1", model="model-a", max_retries=0 + ), + hflow.build_ai_vlm_checks.OpenAICompatibleExecution( + endpoint="http://localhost:8000/v1", model="model-a", max_retries=5 + ), + ] + ) + hosted_versions = _versions_for( + [ + hflow.build_ai_vlm_checks.HFlowHostedExecution( + check_version=1, request_timeout_seconds=1.0 + ), + hflow.build_ai_vlm_checks.HFlowHostedExecution( + check_version=1, request_timeout_seconds=60.0 + ), + ] + ) + assert openai_versions[0] != openai_versions[1] + assert hosted_versions[0] != hosted_versions[1]