Skip to content

Commit f99f4cc

Browse files
zhanghuiclaude
andcommitted
fix(milvus): verify the physical collection schema, not just field names
verify_collection() compared field *names* only, so a collection whose columns happened to share our names but carried the wrong datatype, primary key, nullability or vector dimension passed startup and then failed opaquely on the first write or search. That is the shape a stale model or a reused collection_prefix produces. Creation and verification now share one _PhysicalField descriptor, so a reported mismatch always means the server disagrees with us — never that the builder and the checker have drifted apart. The created schema is byte-identical to before for all seven tables. What is strict and what is not. describe_collection omits `nullable` and `is_primary` when false and omits `element_type` for non-array columns, so absence is unambiguous and those are compared exactly, along with datatype and vector dimension — the drift that fails silently and late. max_length and max_capacity are advisory: the server may normalize them, and a wrong length already surfaces as a loud write rejection, so blocking startup on them would trade a small gap for an outage risk. They are logged, never swallowed. Index and metric verification is deliberately not included. It needs a second descriptor shape (describe_index) that cannot be confirmed without a live server, and shipping an unverifiable check here would risk exactly the false positive this change is careful to avoid. Tests. The offline tests build their fake describe_collection reply from the same descriptor the verifier reads, so they can only prove creation and verification agree with each other; a hand-written snapshot of episode's physical layout is the independent statement of intent, and foresight.evidence_tokens — the only column that is both nullable and a BM25 input — pins the interaction neither episode nor user_profile can reach. Whether the descriptor matches what Milvus actually reports is asserted in the remote suite, which re-verifies every collection after clearing the process readiness cache. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 8029421 commit f99f4cc

3 files changed

Lines changed: 516 additions & 87 deletions

File tree

‎src/everos/infra/persistence/milvus/repository.py‎

Lines changed: 222 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import asyncio
66
import datetime as dt
77
from collections.abc import Sequence
8+
from dataclasses import dataclass
89
from typing import Any, ClassVar
910

1011
from pydantic import BaseModel
@@ -45,6 +46,100 @@ class MilvusValueLimitError(ValueError):
4546
"""A row exceeds a documented Milvus VARCHAR, array, or vector limit."""
4647

4748

49+
@dataclass(frozen=True)
50+
class _PhysicalField:
51+
"""One Milvus column exactly as EverOS declares it.
52+
53+
The same descriptor drives collection creation and startup verification,
54+
so a reported mismatch always means the server disagrees with us — never
55+
that the builder and the checker have drifted apart from each other.
56+
"""
57+
58+
name: str
59+
datatype: DataType
60+
is_primary: bool = False
61+
nullable: bool = False
62+
dim: int | None = None
63+
element_type: DataType | None = None
64+
max_length: int | None = None
65+
max_capacity: int | None = None
66+
enable_analyzer: bool = False
67+
68+
def create_kwargs(self) -> dict[str, Any]:
69+
kwargs: dict[str, Any] = {"field_name": self.name, "datatype": self.datatype}
70+
if self.is_primary:
71+
kwargs["is_primary"] = True
72+
if self.element_type is not None:
73+
kwargs["element_type"] = self.element_type
74+
if self.max_length is not None:
75+
kwargs["max_length"] = self.max_length
76+
if self.max_capacity is not None:
77+
kwargs["max_capacity"] = self.max_capacity
78+
if self.dim is not None:
79+
kwargs["dim"] = self.dim
80+
if self.enable_analyzer:
81+
kwargs["enable_analyzer"] = True
82+
if self.nullable:
83+
kwargs["nullable"] = True
84+
return kwargs
85+
86+
def mismatches(self, actual: dict[str, Any]) -> list[str]:
87+
"""Report how a server-reported field differs from this declaration.
88+
89+
``describe_collection`` omits ``nullable`` and ``is_primary`` when they
90+
are false and omits ``element_type`` for non-array columns, so absence
91+
is unambiguous and can be compared strictly. ``max_length`` and
92+
``max_capacity`` are reported only for the types that carry them and
93+
the server may normalize them, so they are advisory: a wrong length
94+
surfaces as a loud write rejection anyway, whereas a wrong datatype or
95+
dimension is the kind that fails opaquely much later.
96+
"""
97+
params = actual.get("params") or {}
98+
out: list[str] = []
99+
if actual.get("type") != self.datatype:
100+
out.append(
101+
f"{self.name}: datatype {_name_of(actual.get('type'))} "
102+
f"!= expected {_name_of(self.datatype)}"
103+
)
104+
if bool(actual.get("is_primary", False)) != self.is_primary:
105+
out.append(
106+
f"{self.name}: is_primary {actual.get('is_primary', False)} "
107+
f"!= expected {self.is_primary}"
108+
)
109+
if bool(actual.get("nullable", False)) != self.nullable:
110+
out.append(
111+
f"{self.name}: nullable {actual.get('nullable', False)} "
112+
f"!= expected {self.nullable}"
113+
)
114+
if self.dim is not None and params.get("dim") != self.dim:
115+
out.append(f"{self.name}: dim {params.get('dim')} != expected {self.dim}")
116+
if self.element_type is not None and actual.get("element_type") != (
117+
self.element_type
118+
):
119+
out.append(
120+
f"{self.name}: element_type {_name_of(actual.get('element_type'))} "
121+
f"!= expected {_name_of(self.element_type)}"
122+
)
123+
return out
124+
125+
def soft_mismatches(self, actual: dict[str, Any]) -> list[str]:
126+
"""Advisory-only limit drift (see :meth:`mismatches`)."""
127+
params = actual.get("params") or {}
128+
out: list[str] = []
129+
for key, want in (
130+
("max_length", self.max_length),
131+
("max_capacity", self.max_capacity),
132+
):
133+
got = params.get(key)
134+
if want is not None and got is not None and int(got) != want:
135+
out.append(f"{self.name}: {key} {got} != declared {want}")
136+
return out
137+
138+
139+
def _name_of(datatype: Any) -> str:
140+
return getattr(datatype, "name", repr(datatype))
141+
142+
48143
class MilvusRepoBase[T: BaseModel]:
49144
"""Generic Milvus repository backed by one neutral index schema."""
50145

@@ -140,17 +235,48 @@ async def _create_collection(self, client: MilvusClient) -> None:
140235
)
141236

142237
async def verify_collection(self) -> None:
238+
"""Reject a collection whose physical schema disagrees with ours.
239+
240+
A name-only check waves through a collection whose columns happen to
241+
share our names but carry the wrong datatype, primary key, nullability
242+
or vector dimension — a stale model, or a ``collection_prefix``
243+
collision with someone else's data. That collection starts fine and
244+
then fails opaquely on the first write or search. Comparing the
245+
physical shape turns it back into a startup error with a recovery
246+
path.
247+
"""
143248
client = await get_client()
144249
description = await _run(client.describe_collection, self.collection_name)
145-
actual = {field["name"] for field in description.get("fields", [])}
146-
expected = set(self._stored_field_names())
147-
missing = expected - actual
148-
stale = actual - expected
149-
if missing or stale:
250+
reported = {
251+
field["name"]: field
252+
for field in description.get("fields", [])
253+
if "name" in field
254+
}
255+
expected = {physical.name: physical for physical in self._physical_fields()}
256+
257+
missing = sorted(set(expected) - set(reported))
258+
stale = sorted(set(reported) - set(expected))
259+
drift: list[str] = []
260+
advisory: list[str] = []
261+
for name, physical in expected.items():
262+
actual = reported.get(name)
263+
if actual is None:
264+
continue
265+
drift.extend(physical.mismatches(actual))
266+
advisory.extend(physical.soft_mismatches(actual))
267+
268+
if advisory:
269+
logger.warning(
270+
"milvus_collection_limit_drift",
271+
collection=self.collection_name,
272+
details=advisory,
273+
)
274+
if missing or stale or drift:
150275
raise MilvusSchemaMismatchError(
151276
f"Milvus collection {self.collection_name!r} schema drift: "
152-
f"missing={sorted(missing)}, stale={sorted(stale)}. The index is "
153-
"rebuildable from markdown; run `everos cascade rebuild`."
277+
f"missing={missing}, stale={stale}, incompatible={drift}. "
278+
"The index is rebuildable from markdown; run "
279+
"`everos cascade rebuild`."
154280
)
155281

156282
async def add(self, records: Sequence[T]) -> None:
@@ -481,20 +607,10 @@ async def find_by_parent(
481607

482608
def _build_collection_schema(self) -> Any:
483609
schema = MilvusClient.create_schema(auto_id=False, enable_dynamic_field=False)
484-
for field in self.index_schema.fields:
485-
self._add_schema_field(schema, field)
486-
if not self.index_schema.vector_fields:
487-
schema.add_field(
488-
field_name=_DUMMY_VECTOR_FIELD,
489-
datatype=DataType.FLOAT_VECTOR,
490-
dim=_DUMMY_VECTOR_DIMENSION,
491-
)
610+
for physical in self._physical_fields():
611+
schema.add_field(**physical.create_kwargs())
492612
for field in self.index_schema.bm25_fields:
493613
sparse_name = _sparse_field(field)
494-
schema.add_field(
495-
field_name=sparse_name,
496-
datatype=DataType.SPARSE_FLOAT_VECTOR,
497-
)
498614
schema.add_function(
499615
Function(
500616
name=f"{field}_bm25",
@@ -505,73 +621,97 @@ def _build_collection_schema(self) -> Any:
505621
)
506622
return schema
507623

508-
def _add_schema_field(self, schema: Any, field: IndexField) -> None:
509-
if field.kind is IndexFieldKind.STRING:
510-
kwargs: dict[str, Any] = {
511-
"field_name": field.name,
512-
"datatype": DataType.VARCHAR,
513-
"max_length": field.max_length,
514-
}
515-
if field.primary:
516-
kwargs["is_primary"] = True
517-
elif field.nullable and field.name not in self.index_schema.bm25_fields:
518-
kwargs["nullable"] = True
519-
if field.name in self.index_schema.bm25_fields:
520-
kwargs["enable_analyzer"] = True
521-
schema.add_field(**kwargs)
522-
elif field.kind is IndexFieldKind.STRING_ARRAY:
523-
schema.add_field(
524-
field_name=field.name,
525-
datatype=DataType.ARRAY,
526-
element_type=DataType.VARCHAR,
527-
max_capacity=field.max_capacity,
528-
max_length=field.max_length,
529-
nullable=field.nullable,
530-
)
531-
elif field.kind is IndexFieldKind.FLOAT:
532-
schema.add_field(
533-
field_name=field.name,
534-
datatype=DataType.DOUBLE,
535-
nullable=field.nullable,
536-
)
537-
elif field.kind is IndexFieldKind.INTEGER:
538-
schema.add_field(
539-
field_name=field.name,
540-
datatype=DataType.INT64,
541-
nullable=field.nullable,
542-
)
543-
elif field.kind is IndexFieldKind.DATETIME:
544-
schema.add_field(
545-
field_name=_datetime_storage_field(field.name),
546-
datatype=DataType.INT64,
547-
nullable=field.nullable,
548-
)
549-
elif field.kind is IndexFieldKind.DENSE_VECTOR:
550-
schema.add_field(
551-
field_name=field.name,
552-
datatype=DataType.FLOAT_VECTOR,
553-
dim=field.dimension,
624+
def _physical_fields(self) -> tuple[_PhysicalField, ...]:
625+
"""Every Milvus column this table declares, in creation order."""
626+
out: list[_PhysicalField] = []
627+
for field in self.index_schema.fields:
628+
out.extend(self._physical_for(field))
629+
if not self.index_schema.vector_fields:
630+
out.append(
631+
_PhysicalField(
632+
name=_DUMMY_VECTOR_FIELD,
633+
datatype=DataType.FLOAT_VECTOR,
634+
dim=_DUMMY_VECTOR_DIMENSION,
635+
)
554636
)
555-
schema.add_field(
556-
field_name=_present_field(field.name),
557-
datatype=DataType.BOOL,
637+
out.extend(
638+
_PhysicalField(
639+
name=_sparse_field(field),
640+
datatype=DataType.SPARSE_FLOAT_VECTOR,
558641
)
559-
else: # pragma: no cover - enum exhaustiveness guard
560-
raise TypeError(f"unsupported index field kind: {field.kind}")
642+
for field in self.index_schema.bm25_fields
643+
)
644+
return tuple(out)
645+
646+
def _physical_for(self, field: IndexField) -> list[_PhysicalField]:
647+
is_bm25 = field.name in self.index_schema.bm25_fields
648+
if field.kind is IndexFieldKind.STRING:
649+
return [
650+
_PhysicalField(
651+
name=field.name,
652+
datatype=DataType.VARCHAR,
653+
is_primary=field.primary,
654+
# A BM25 input is written as "" rather than null so the
655+
# analyzer always has something to tokenize.
656+
nullable=field.nullable and not field.primary and not is_bm25,
657+
max_length=field.max_length,
658+
enable_analyzer=is_bm25,
659+
)
660+
]
661+
if field.kind is IndexFieldKind.STRING_ARRAY:
662+
return [
663+
_PhysicalField(
664+
name=field.name,
665+
datatype=DataType.ARRAY,
666+
nullable=field.nullable,
667+
element_type=DataType.VARCHAR,
668+
max_length=field.max_length,
669+
max_capacity=field.max_capacity,
670+
)
671+
]
672+
if field.kind is IndexFieldKind.FLOAT:
673+
return [
674+
_PhysicalField(
675+
name=field.name,
676+
datatype=DataType.DOUBLE,
677+
nullable=field.nullable,
678+
)
679+
]
680+
if field.kind is IndexFieldKind.INTEGER:
681+
return [
682+
_PhysicalField(
683+
name=field.name,
684+
datatype=DataType.INT64,
685+
nullable=field.nullable,
686+
)
687+
]
688+
if field.kind is IndexFieldKind.DATETIME:
689+
return [
690+
_PhysicalField(
691+
name=_datetime_storage_field(field.name),
692+
datatype=DataType.INT64,
693+
nullable=field.nullable,
694+
)
695+
]
696+
if field.kind is IndexFieldKind.DENSE_VECTOR:
697+
return [
698+
_PhysicalField(
699+
name=field.name,
700+
datatype=DataType.FLOAT_VECTOR,
701+
dim=field.dimension,
702+
),
703+
# Milvus cannot store a null dense vector, so a logical null is
704+
# a zero vector plus this presence marker.
705+
_PhysicalField(
706+
name=_present_field(field.name),
707+
datatype=DataType.BOOL,
708+
),
709+
]
710+
# pragma: no cover - enum exhaustiveness guard
711+
raise TypeError(f"unsupported index field kind: {field.kind}")
561712

562713
def _stored_field_names(self) -> list[str]:
563-
names: list[str] = []
564-
for field in self.index_schema.fields:
565-
if field.kind is IndexFieldKind.DATETIME:
566-
names.append(_datetime_storage_field(field.name))
567-
else:
568-
names.append(field.name)
569-
if field.kind is IndexFieldKind.DENSE_VECTOR:
570-
names.append(_present_field(field.name))
571-
if not self.index_schema.vector_fields:
572-
names.append(_DUMMY_VECTOR_FIELD)
573-
names.extend(_sparse_field(field) for field in self.index_schema.bm25_fields)
574-
return names
714+
return [physical.name for physical in self._physical_fields()]
575715

576716
def _output_fields(self, *, include_vectors: bool) -> list[str]:
577717
fields: list[str] = []

‎tests/integration/test_milvus_remote.py‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,3 +310,50 @@ async def test_datetime_round_trip_survives_pre_2001_instants() -> None:
310310
await episode_repo.delete_by_md_path(
311311
"test_app/test_project/users/u1/episodes/day.md"
312312
)
313+
314+
315+
async def test_verify_accepts_a_collection_this_adapter_just_created() -> None:
316+
"""The decisive check for physical schema verification.
317+
318+
Every offline test builds its fake ``describe_collection`` reply from the
319+
same descriptor the verifier compares against, so they can only prove that
320+
creation and verification agree with each other. Whether that descriptor
321+
matches what Milvus actually stores and reports back — VARCHAR lengths it
322+
may normalize, flags it may omit, the analyzer it attaches for BM25 — can
323+
only be established against a real server.
324+
325+
The fixture creates the collections with a fresh prefix, so verification
326+
never runs during setup; dropping the process-local readiness cache forces
327+
the existing-collection path.
328+
"""
329+
from everos.infra.persistence.index import ALL_REPOS, episode_repo
330+
from everos.infra.persistence.milvus.repository import MilvusRepoBase
331+
332+
await episode_repo.upsert(
333+
[
334+
_episode(
335+
row_id="u1_verify",
336+
entry_id="verify",
337+
session_id="verify",
338+
text="a row so the collection is not empty",
339+
vector_axis=1,
340+
subject_axis=2,
341+
timestamp=dt.datetime(2026, 5, 1, tzinfo=dt.UTC),
342+
)
343+
]
344+
)
345+
346+
for repo in ALL_REPOS:
347+
milvus_repo = repo._repo() # type: ignore[attr-defined]
348+
await milvus_repo.ensure_collection()
349+
350+
MilvusRepoBase._reset_collection_cache()
351+
352+
for repo in ALL_REPOS:
353+
# Raises MilvusSchemaMismatchError if our declaration and the server's
354+
# description disagree on any field.
355+
await repo._repo().verify_collection() # type: ignore[attr-defined]
356+
357+
await episode_repo.delete_by_md_path(
358+
"test_app/test_project/users/u1/episodes/day.md"
359+
)

0 commit comments

Comments
 (0)