fix: Address Devin review feedback on versioning issues

franciscojavierarceo · claude · franciscojavierarceo · commit 8809805b688b · 2026-03-17T14:30:01.000-04:00
- Fix SQLite _table_id to prefer projection.version_tag over current_version_number so @v2 refs read from the correct table - Detect feature name collisions for multi-version queries with full_feature_names=True (e.g. fv@v1:feat vs fv@v2:feat) - Remove unused include_feature_view_version_metadata parameter from retrieve_online_documents (v1) across all providers and online stores - Remove redundant name check from _schema_or_udf_changed since callers always match by name first Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
diff --git a/sdk/python/feast/base_feature_view.py b/sdk/python/feast/base_feature_view.py
@@ -154,10 +154,10 @@ def __getitem__(self, item):
         return cp
 
     def _schema_or_udf_changed(self, other: "BaseFeatureView") -> bool:
-        """Check if schema or UDF-related fields have changed (version-worthy changes)."""
-        # Schema changes
-        if self.name != other.name:
-            return True
+        """Check if schema or UDF-related fields have changed (version-worthy changes).
+
+        Callers always match by name first, so name comparison is omitted here.
+        """
         if sorted(self.features) != sorted(other.features):
             return True
         # Skip metadata: description, tags, owner, projection
diff --git a/sdk/python/feast/feature_server.py b/sdk/python/feast/feature_server.py
@@ -392,12 +392,14 @@ async def retrieve_online_documents(
                 features=features,
                 query=request.query,
                 top_k=request.top_k,
-                include_feature_view_version_metadata=request.include_feature_view_version_metadata,
             )
             if request.api_version == 2 and request.query_string is not None:
                 read_params["query_string"] = request.query_string
 
             if request.api_version == 2:
+                read_params["include_feature_view_version_metadata"] = (
+                    request.include_feature_view_version_metadata
+                )
                 response = await run_in_threadpool(
                     lambda: store.retrieve_online_documents_v2(**read_params)  # type: ignore
                 )
diff --git a/sdk/python/feast/feature_store.py b/sdk/python/feast/feature_store.py
@@ -2586,7 +2586,6 @@ def retrieve_online_documents(
         top_k: int,
         features: List[str],
         distance_metric: Optional[str] = "L2",
-        include_feature_view_version_metadata: bool = False,
     ) -> OnlineResponse:
         """
         Retrieves the top k closest document features. Note, embeddings are a subset of features.
@@ -2642,7 +2641,6 @@ def retrieve_online_documents(
             query,
             top_k,
             distance_metric,
-            include_feature_view_version_metadata,
         )
 
         # TODO currently not return the vector value since it is same as feature value, if embedding is supported,
@@ -2863,7 +2861,6 @@ def _retrieve_from_online_store(
         query: List[float],
         top_k: int,
         distance_metric: Optional[str],
-        include_feature_view_version_metadata: bool = False,
     ) -> List[
         Tuple[
             Timestamp, Optional[EntityKey], "FieldStatus.ValueType", Value, Value, Value
@@ -2879,7 +2876,6 @@ def _retrieve_from_online_store(
             query=query,
             top_k=top_k,
             distance_metric=distance_metric,
-            include_feature_view_version_metadata=include_feature_view_version_metadata,
         )
 
         read_row_protos = []
diff --git a/sdk/python/feast/infra/online_stores/elasticsearch_online_store/elasticsearch.py b/sdk/python/feast/infra/online_stores/elasticsearch_online_store/elasticsearch.py
@@ -280,7 +280,6 @@ def retrieve_online_documents(
         embedding: List[float],
         top_k: int,
         distance_metric: Optional[str] = None,
-        include_feature_view_version_metadata: bool = False,
     ) -> List[
         Tuple[
             Optional[datetime],
diff --git a/sdk/python/feast/infra/online_stores/faiss_online_store.py b/sdk/python/feast/infra/online_stores/faiss_online_store.py
@@ -180,7 +180,6 @@ def retrieve_online_documents(
         embedding: List[float],
         top_k: int,
         distance_metric: Optional[str] = None,
-        include_feature_view_version_metadata: bool = False,
     ) -> List[
         Tuple[
             Optional[datetime],
diff --git a/sdk/python/feast/infra/online_stores/online_store.py b/sdk/python/feast/infra/online_stores/online_store.py
@@ -420,7 +420,6 @@ def retrieve_online_documents(
         embedding: List[float],
         top_k: int,
         distance_metric: Optional[str] = None,
-        include_feature_view_version_metadata: bool = False,
     ) -> List[
         Tuple[
             Optional[datetime],
diff --git a/sdk/python/feast/infra/online_stores/postgres_online_store/postgres.py b/sdk/python/feast/infra/online_stores/postgres_online_store/postgres.py
@@ -381,7 +381,6 @@ def retrieve_online_documents(
         embedding: List[float],
         top_k: int,
         distance_metric: Optional[str] = "L2",
-        include_feature_view_version_metadata: bool = False,
     ) -> List[
         Tuple[
             Optional[datetime],
diff --git a/sdk/python/feast/infra/online_stores/qdrant_online_store/qdrant.py b/sdk/python/feast/infra/online_stores/qdrant_online_store/qdrant.py
@@ -264,7 +264,6 @@ def retrieve_online_documents(
         embedding: List[float],
         top_k: int,
         distance_metric: Optional[str] = "cosine",
-        include_feature_view_version_metadata: bool = False,
     ) -> List[
         Tuple[
             Optional[datetime],
diff --git a/sdk/python/feast/infra/online_stores/remote.py b/sdk/python/feast/infra/online_stores/remote.py
@@ -228,7 +228,6 @@ def retrieve_online_documents(
         embedding: Optional[List[float]],
         top_k: int,
         distance_metric: Optional[str] = "L2",
-        include_feature_view_version_metadata: bool = False,
     ) -> List[
         Tuple[
             Optional[datetime],
diff --git a/sdk/python/feast/infra/online_stores/sqlite.py b/sdk/python/feast/infra/online_stores/sqlite.py
@@ -341,7 +341,6 @@ def retrieve_online_documents(
         embedding: List[float],
         top_k: int,
         distance_metric: Optional[str] = None,
-        include_feature_view_version_metadata: bool = False,
     ) -> List[
         Tuple[
             Optional[datetime],
@@ -703,7 +702,11 @@ def _initialize_conn(
 
 def _table_id(project: str, table: FeatureView) -> str:
     name = table.name
-    version = getattr(table, "current_version_number", None)
+    # Prefer version_tag from the projection (set by version-qualified refs like @v2)
+    # over current_version_number (the FV's active version in metadata).
+    version = getattr(table.projection, "version_tag", None)
+    if version is None:
+        version = getattr(table, "current_version_number", None)
     if version is not None and version > 0:
         name = f"{table.name}_v{version}"
     return f"{project}_{name}"
diff --git a/sdk/python/feast/infra/passthrough_provider.py b/sdk/python/feast/infra/passthrough_provider.py
@@ -304,7 +304,6 @@ def retrieve_online_documents(
         query: List[float],
         top_k: int,
         distance_metric: Optional[str] = None,
-        include_feature_view_version_metadata: bool = False,
     ) -> List:
         result = []
         if self.online_store:
@@ -315,7 +314,6 @@ def retrieve_online_documents(
                 query,
                 top_k,
                 distance_metric,
-                include_feature_view_version_metadata,
             )
         return result
 
diff --git a/sdk/python/feast/infra/provider.py b/sdk/python/feast/infra/provider.py
@@ -435,7 +435,6 @@ def retrieve_online_documents(
         query: List[float],
         top_k: int,
         distance_metric: Optional[str] = None,
-        include_feature_view_version_metadata: bool = False,
     ) -> List[
         Tuple[
             Optional[datetime],
diff --git a/sdk/python/feast/utils.py b/sdk/python/feast/utils.py
@@ -543,9 +543,21 @@ def _validate_feature_refs(feature_refs: List[str], full_feature_names: bool = F
     collided_feature_refs = []
 
     if full_feature_names:
-        collided_feature_refs = [
-            ref for ref, occurrences in Counter(feature_refs).items() if occurrences > 1
+        # Use clean names (without @vN) to detect collisions, since the response
+        # strips version tags. E.g. 'fv@v1:feat' and 'fv@v2:feat' both produce 'fv__feat'.
+        clean_refs = [
+            f"{_parse_feature_ref(ref)[0]}__{_parse_feature_ref(ref)[2]}"
+            for ref in feature_refs
         ]
+        collided_clean = [
+            name for name, count in Counter(clean_refs).items() if count > 1
+        ]
+        if collided_clean:
+            collided_feature_refs = [
+                ref
+                for ref, clean in zip(feature_refs, clean_refs)
+                if clean in collided_clean
+            ]
     else:
         feature_names = [_parse_feature_ref(ref)[2] for ref in feature_refs]
         collided_feature_names = [
