From 5af7e26f2af9cb0d4e923b07eb288feb2f8567ae Mon Sep 17 00:00:00 2001 From: Gabor Szabo Date: Mon, 1 Jun 2026 02:10:22 +0200 Subject: [PATCH] fix(api): honor feature_frame_version >= 3 in ops + registry (#338) --- app/features/ops/service.py | 5 ++- app/features/ops/tests/test_service.py | 43 +++++++++++++++++---- app/features/registry/service.py | 5 ++- app/features/registry/tests/test_service.py | 15 +++++-- 4 files changed, 55 insertions(+), 13 deletions(-) diff --git a/app/features/ops/service.py b/app/features/ops/service.py index 43c59318..4ec3af78 100644 --- a/app/features/ops/service.py +++ b/app/features/ops/service.py @@ -154,7 +154,10 @@ def _run_feature_frame_version(run: ModelRun) -> int: """ info = run.runtime_info or {} value = info.get("feature_frame_version") - if isinstance(value, int) and value in (1, 2): + # Honor any positive int V (feature_frame_version is an opaque incrementing + # integer per docs/_base/DOMAIN_MODEL.md). bool is excluded because it + # subclasses int. Missing / invalid value -> V=1 back-compat (#338). + if isinstance(value, int) and not isinstance(value, bool) and value >= 1: return value return 1 diff --git a/app/features/ops/tests/test_service.py b/app/features/ops/tests/test_service.py index fcfb61a7..8a4c107c 100644 --- a/app/features/ops/tests/test_service.py +++ b/app/features/ops/tests/test_service.py @@ -188,14 +188,23 @@ def test_run_feature_frame_version_reads_runtime_info() -> None: assert _run_feature_frame_version(_make_run(run_id="b")) == 1 -def test_run_feature_frame_version_rejects_unsupported_value() -> None: - """Unknown int (e.g. 3) or non-int values fall back to V=1 (defensive).""" - legacy_explicit_v3 = _make_run(run_id="bad-int") - legacy_explicit_v3.runtime_info = {"feature_frame_version": 3} - legacy_str = _make_run(run_id="bad-str") - legacy_str.runtime_info = {"feature_frame_version": "2"} - assert _run_feature_frame_version(legacy_explicit_v3) == 1 - assert _run_feature_frame_version(legacy_str) == 1 +def test_run_feature_frame_version_honors_any_positive_int() -> None: + """Any positive int V is honored (e.g. 3); non-int / non-positive / bool -> V=1. + + Regression for #338: feature_frame_version is an opaque incrementing integer + (docs/_base/DOMAIN_MODEL.md), so V>=3 must NOT be clamped to 1 — the showcase + stale_alias_trigger step registers a V=3 run to fire the + feature_frame_version_mismatch verdict. + """ + v3 = _make_run(run_id="v3") + v3.runtime_info = {"feature_frame_version": 3} + assert _run_feature_frame_version(v3) == 3 + + # Non-int / non-positive / bool all fall back to V=1. + for bad in ("2", 0, -1, True): + run = _make_run(run_id=f"bad-{bad!r}") + run.runtime_info = {"feature_frame_version": bad} + assert _run_feature_frame_version(run) == 1 def test_alias_staleness_legacy_run_treated_as_v1_no_spurious_mismatch() -> None: @@ -241,6 +250,24 @@ def test_alias_staleness_v_mismatch_wins_over_newer_run() -> None: assert comparable_v == 2 +def test_alias_staleness_v1_alias_v3_latest_reports_mismatch() -> None: + """A V1 alias with a newer V3 comparable reports MISMATCH, not NEWER (#338). + + Mirrors the showcase stale_alias_trigger scenario: the demo-production alias + points at a V1 run while the grain's newest run is V=3. Before #338 the V=3 + latest was clamped to V=1, so this fell through to NEWER_SUCCESS_RUN. + """ + older = datetime(2026, 1, 1, tzinfo=UTC) + newer = datetime(2026, 5, 1, tzinfo=UTC) + run = _make_run(run_id="v1-alias", created_at=older, feature_frame_version=1) + latest = _make_run(run_id="v3-latest", created_at=newer, feature_frame_version=3) + is_stale, reason, alias_v, comparable_v = _alias_staleness(run, {(1, 1): latest}) + assert is_stale is True + assert reason == StaleReason.FEATURE_FRAME_VERSION_MISMATCH.value + assert alias_v == 1 + assert comparable_v == 3 + + def test_alias_staleness_same_v_newer_run_uses_newer_reason() -> None: """V matches but the comparable is newer → NEWER_SUCCESS_RUN reason.""" older = datetime(2026, 1, 1, tzinfo=UTC) diff --git a/app/features/registry/service.py b/app/features/registry/service.py index 503c45d2..37e8a3fe 100644 --- a/app/features/registry/service.py +++ b/app/features/registry/service.py @@ -649,7 +649,10 @@ def _extract_feature_frame_version( if not runtime_info_extras: return 1 value = runtime_info_extras.get("feature_frame_version") - if isinstance(value, int) and value in (1, 2): + # Honor any positive int V (feature_frame_version is an opaque + # incrementing integer per docs/_base/DOMAIN_MODEL.md). bool is excluded + # because it subclasses int. Missing / invalid value -> V=1 (#338). + if isinstance(value, int) and not isinstance(value, bool) and value >= 1: return value return 1 diff --git a/app/features/registry/tests/test_service.py b/app/features/registry/tests/test_service.py index abd2a2ce..1014d7bc 100644 --- a/app/features/registry/tests/test_service.py +++ b/app/features/registry/tests/test_service.py @@ -165,10 +165,19 @@ def test_extract_feature_frame_version_explicit_v2(self) -> None: """Explicit feature_frame_version=2 round-trips.""" assert RegistryService._extract_feature_frame_version({"feature_frame_version": 2}) == 2 - def test_extract_feature_frame_version_rejects_unsupported_value(self) -> None: - """Unknown int (e.g. 3) and non-int (e.g. '2') fall back to V1.""" - assert RegistryService._extract_feature_frame_version({"feature_frame_version": 3}) == 1 + def test_extract_feature_frame_version_honors_any_positive_int(self) -> None: + """Any positive int V is honored (e.g. 3); non-int / non-positive / bool -> V1. + + Regression for #338: feature_frame_version is an opaque incrementing + integer, so V>=3 must not be clamped to 1 (the showcase + stale_alias_trigger step registers a V=3 run). + """ + assert RegistryService._extract_feature_frame_version({"feature_frame_version": 3}) == 3 + assert RegistryService._extract_feature_frame_version({"feature_frame_version": 7}) == 7 + # Non-int / non-positive / bool all fall back to V1. assert RegistryService._extract_feature_frame_version({"feature_frame_version": "2"}) == 1 + assert RegistryService._extract_feature_frame_version({"feature_frame_version": 0}) == 1 + assert RegistryService._extract_feature_frame_version({"feature_frame_version": True}) == 1 class TestRegistryServiceConfigDiff: