feat: add support for alter schema drop vector index - #1991
Conversation
![orca-security-eu[bot]](https://avatars.githubusercontent.com/in/276244?s=60&v=4){kind=small}
There was a problem hiding this comment.
Orca Security Scan Summary
| Status | Check | Issues by priority | |
|---|---|---|---|
 Passed |
Infrastructure as Code |  0  0  0  0 |
View in Orca |
| Passed |
SAST | 0 0 0 0 |
View in Orca |
| Passed |
Secrets | 0 0 0 0 |
View in Orca |
| Passed |
Vulnerabilities | 0 0 0 0 |
View in Orca |
There was a problem hiding this comment.
Pull request overview
Adds client support for dropping a named vector’s index via the schema REST endpoint, mirroring the existing “delete property index” capability in the collections config API.
Changes:
- Added
delete_vector_index(vector_name)to the collection config executor, issuingDELETE /v1/schema/{className}/vectors/{vectorIndexName}/index. - Added sync and async type stubs for
delete_vector_index(...).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| weaviate/collections/config/executor.py | Implements the new delete-vector-index operation via the config executor. |
| weaviate/collections/config/sync.pyi | Exposes the new method in the sync config type stub. |
| weaviate/collections/config/async_.pyi | Exposes the new method in the async config type stub. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
d728dd9 to
7e5cd7f
Compare
There was a problem hiding this comment.
Orca Security Scan Summary
| Status | Check | Issues by priority | |
|---|---|---|---|
| Passed |
Secrets | 0 0 0 0 |
View in Orca |
3657487 to
736a7cd
Compare
| NONE: The index of this vector has been dropped, see ``collection.config.delete_vector_index``. | ||
| The vector data is still stored, but it cannot be searched. This value is reported by the | ||
| server only, it cannot be used to configure a vector. | ||
| """ |
There was a problem hiding this comment.
Leaving as-is for now: no public Configure.VectorIndex.* factory emits NONE (only the internal create models reference the field), and the server rejects "none" on both create and update, so the exposure is constructing private classes directly. Can add a validator in a follow-up if users actually hit it.
7e5cd7f to
4e2a580
Compare
736a7cd to
be6c8dc
Compare
4e2a580 to
2dbfaed
Compare
|
thought: Do you think this PR could do with fewer tests? The new endpoint is very simple and it seems to me like one "happy path" test will catch most things. Other tests verify Weaviate's behavior more than client's logic, and some may add up to 30s of pipeline time. |
There was a problem hiding this comment.
Orca Security Scan Summary
| Status | Check | Issues by priority | |
|---|---|---|---|
| Passed |
Infrastructure as Code | 0 0 0 0 |
View in Orca |
| Passed |
SAST | 0 0 0 0 |
View in Orca |
| Passed |
Secrets | 0 0 0 0 |
View in Orca |
| Passed |
Vulnerabilities | 0 0 0 0 |
View in Orca |
|
@bevzzz fair point — slimmed in b92dbb7, keeping the coverage but moving it down the pyramid: the integration test is now a single happy-path journey (create → drop → poll → sibling index intact + searchable). The unknown-name case moved to the mock suite (422 → |
|
@copilot review |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1991 +/- ##
==========================================
+ Coverage 86.64% 88.75% +2.11%
==========================================
Files 300 304 +4
Lines 23172 23955 +783
==========================================
+ Hits 20077 21262 +1185
+ Misses 3095 2693 -402 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
9d7b916 to
ccad4cc
Compare
ccad4cc to
16ea348
Compare
dirkkul
left a comment
dirkkul
left a comment
There was a problem hiding this comment.
a few smaller things, none of them blocking:
-
the
delete_vector_indexdocstring goes stale as soon as the endpoint is promoted or the status code changes: it namesENABLE_EXPERIMENTAL_ALTER_SCHEMA_DROP_VECTOR_INDEX_ENDPOINT=trueand says the server answers 500 without it. someone reading it after that sets an env var the server no longer looks at. could we say the endpoint is experimental and may be disabled server-side, without naming the flag or the status code? -
in
test/collection/test_config_methods.py,test_collection_config_simple_from_json_with_dropped_vector_indexandtest_collection_config_simple_from_json_all_vectors_droppedparse the same schemas as the two non-simple tests above them -__get_vector_config(schema, simple)never readssimple, and__get_vectorizertakes no such parameter. the simple path does build a different dataclass, so could we keep one of the two as thelist_all()guard and drop the other? -
test_delete_vector_index_endpoint_disabledoverlaps the 422 case already insidetest_delete_vector_index- both assert that a non-OK status comes back asUnexpectedStatusCodeErrorcarrying the status code. the one thing it adds is that the server's error message reaches the exception, so could that move intotest_delete_vector_indexas another status/message case instead of standing up its own mock server?
reviewed with claude code, I went through every comment below myself
There was a problem hiding this comment.
🟡 Changes recommended
Critical and moderate dropped-index update and recreation issues remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (3)
Previously missed (1) — in code that hasn't changed since the last review.
weaviate/collections/config/executor.py:679
- The drop cleanup does not retain these vector values permanently: after the marker is applied, Weaviate asynchronously strips the target vector from stored objects before finalizing the schema. Saying that “the vectors themselves are kept” gives callers an incorrect data-retention guarantee; describe them as temporarily retained while cleanup is pending and then removed.
weaviate/collections/classes/config.py:1697
- When a drop is still in progress, Weaviate returns the dropped entry as
vectorIndexType: "none"; this merge preserves that entry while applying a change to another vector. The server's update path skipsUpdateVectorIndexConfigsfor all vectors whenever any dropped entry is present, soconfig.update(vector_config=...)can return success while silently not applying the active vector's change. Please handle this intermediate state (or reject/wait for the update) and cover the actual REST update, not only the local merge.
existing = self.__existing_vector_index_config(schema, vc.name)
self.__check_quantizers(vc.vectorIndexConfig.quantizer, existing)
schema["vectorConfig"][vc.name]["vectorIndexConfig"] = (
vc.vectorIndexConfig.merge_with_existing(existing)
)
weaviate/collections/config/executor.py:684
- The server can keep a dropped named-vector entry in
vector_configwithVectorIndexConfigNoneeven after the asynchronous cleanup; the parser and integration helper already support both that shape and an omitted entry. Avoid promising that the entry is always removed once the on-disk index is gone, because callers may still observe thenonemarker.
The drop is applied asynchronously. A successful call means that Weaviate accepted the
request, not that the index is already gone. `collection.config.get()` first reports the
vector with a `vector_index_config` of `VectorIndexConfigNone` and drops it from
`vector_config` altogether once the index has been removed from disk.
- Files reviewed: 15/15 changed files
- Comments generated: 1
- Review effort level: Lite
dirkkul
left a comment
dirkkul
left a comment
There was a problem hiding this comment.
reviewed with claude code, I went through every comment below myself
Follow-up to the `delete_vector_index` support, addressing review findings. After a successful drop, Weaviate keeps the vector in the schema as `vectorIndexType: "none"` with no `vectorIndexConfig`. The client asserted that every named vector has an index config, so `collection.config.get()` and `client.collections.list_all()` raised `AssertionError` for every collection in the cluster once any vector index had been dropped. `_NamedVectorConfig. vector_index_config` is now optional, `VectorIndexType` gained a server-reported `NONE` member and `to_dict()` round-trips it. `collection.config.update()` on a dropped vector raised a bare `KeyError: 'vectorIndexConfig'` from the schema merge. Both the current and the deprecated merge paths now go through one helper that raises a `WeaviateInvalidInputError` explaining that a dropped index cannot be re-created. The docstring claimed the index could be regenerated and that a missing vector raises `WeaviateInvalidInputError`. Neither is true: Weaviate rejects re-creating a dropped index, and an unknown vector name comes back as a 422. It now also documents that the endpoint is experimental and needs `ENABLE_EXPERIMENTAL_ALTER_SCHEMA_DROP_VECTOR_INDEX_ENDPOINT=true`, that only named vectors can be dropped, and that the drop is applied asynchronously. The error message no longer blames a missing vector for what is usually a disabled endpoint. Tests: unit coverage for parsing, exporting and updating a dropped vector, mock coverage for the request path and the disabled-endpoint response, and integration coverage gated at 1.39.0. The CI compose file enables the experimental endpoint; that flag can be dropped once 1.39.0 is GA. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Replace the assert on dropped-vector schema shape with an explicit SchemaValidationError so a named vector missing vectorIndexConfig fails fast even under python -O; pinned by a new parser unit test - Slim the integration test to the happy path: the unknown-name error contract moved to the mock suite (422 -> UnexpectedStatusCodeError) and the redundant list_all/invalid-input assertions are covered by the existing unit and mock tests Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A single-vector (non-named) collection whose vector index is dropped with `collection.config.delete_vector_index` comes back from the server with no top-level `vectorizer` (and no `vectorConfig`, `vectorIndexType` or `vectorIndexConfig`). `__get_vectorizer` accessed `schema["vectorizer"]` unguarded and raised `KeyError: 'vectorizer'`, so both `config.get()` and `collections.list_all()` crashed on such a collection. Return `None` when the key is absent, matching how a dropped named vector yields `vector_index_config is None`. Add parser tests for both entry points. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Verified against the Weaviate server source that these comments were wrong: - The endpoint is still experimental and off by default in current main; it is not enabled by default at 1.39.0 GA. The server rejects the drop unless ENABLE_EXPERIMENTAL_ALTER_SCHEMA_DROP_VECTOR_INDEX_ENDPOINT=true (usecases/schema/property.go:255). Fix the misleading ci/docker-compose.yml comment. - A legacy single-vector collection cannot reach the "no vectorConfig, no vectorizer" shape: the server rejects dropping its index because len(class.VectorConfig) == 0 (property.go:288), and setClassDefaults always forces a non-empty top-level vectorizer for legacy classes (class.go:750). That shape can only come from a named-vector collection whose vectors were all dropped. Reword the __get_vectorizer guard comment and rename the parser tests accordingly. Behavior and assertions are unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- config.py: __existing_vector_index_config raised a raw KeyError when the collection had no vectors left (server omits vectorConfig once every named vector is dropped). Guard the key so the intended WeaviateInvalidInputError is raised instead. Add a regression test. - config_methods.py: __get_vector_config reported "no vectorIndexConfig" for a vectorIndexType the client does not know, even though the config was present (an older client against a newer server). Branch on "vectorIndexConfig" in the named vector and give the unknown-type case its own message. Add a regression test. - executor.py: delete_vector_index no longer names the env flag or the 500 status in its docstring (both go stale when the endpoint is promoted); it now returns None instead of a bool that could only ever be True. Regenerate stubs. - tests: fold the disabled-endpoint case into test_delete_vector_index (its only unique check is that the server message reaches the exception) and drop the redundant simple-parser test; the all-vectors-dropped simple test remains the list_all() guard. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The round-4 commit changed delete_vector_index to return None but only updated the mock test and stubs; the integration assertion still expected True and would fail on every >=1.39 CI job. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Represent a dropped vector index with _VectorIndexConfigNone instead of None: the union in _NamedVectorConfig stays concrete, to_dict() derives the type via vector_index_type() like every other index, and the index type of a dropped vector is visible without serializing. The dict output still omits vectorIndexConfig, matching what the server sends. Exported as VectorIndexConfigNone in weaviate.outputs.config. - Strip dropped-vector entries in collections __create (shared by create, create_from_dict and create_from_config): the server rejects vectorIndexType "none" on create, so importing an exported config that ever had a vector index dropped answered 422. The whole entry is skipped, not just the type, because keeping it would re-create an index that was deliberately dropped. A Col001 warning names the skipped vectors; an emptied vectorConfig block is omitted entirely. Mock test verifies the request body both ways, confirmed failing without the strip. - Document in create_from_dict/create_from_config that the round trip is lossy: there is no API to re-create a vector without an index, so the new collection does not contain the dropped vectors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Reference delete_vector_index with parentheses in every docstring, comment, and warning string so it reads as a method, matching the existing () usage in the update-path error message. - Fix the Col001 warning wording: "their index was dropped" (the subject is plural) and "will be created" (the warning fires before the POST, so the create can still fail).
Verified against a 1.39.0 server by the reviewer, and against the server source: - The drop removes the vector's data from every object once the cleanup runs; the docstrings claimed the vectors were kept. Corrected in delete_vector_index, _VectorIndexConfigNone and VectorIndexType.NONE. - "cannot be re-created" only holds while the drop is in progress; once it completes, add_vector() accepts a new vector with the same name. Corrected in the docstring, the config.update() error and the Col001 warning. - Document the repeated-call semantics: a second delete_vector_index() while the drop is in progress succeeds and re-triggers the cleanup; after it completes the same call gets a 422 because the vector no longer exists. - Reject a create whose vectorConfig would end up empty after stripping dropped vectors: the server treats a create without named vectors as a legacy collection (setClassDefaults applies the default vectorizer and vector index whenever len(VectorConfig) == 0, and an empty map behaves like an absent one), so the previous behavior silently created a default legacy HNSW collection instead of one without vectors. Raise WeaviateInvalidInputError instead.
2e8450f to
acd2390
Compare
There was a problem hiding this comment.
🟡 Changes recommended
All vectors being dropped can round-trip as a legacy schema, causing the server to apply a default vector index.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 15/15 changed files
- Comments generated: 1
- Review effort level: Lite
Once a drop's cleanup finishes, an exported config carries no vectorConfig key and no "none" markers, so the round-7 marker check in __create could not catch an all-dropped export and create_from_config posted a legacy-style schema (the server would apply its default vectorizer and vector index). Reject that shape in _create_from_config: a CollectionConfig with neither named vectors nor a legacy vectorizer can only be such an export, because the server always fills a vectorizer on legacy collections. A plain dict without vector fields keeps working through create_from_dict, since minimal hand-written dicts legitimately rely on the server defaults.
There was a problem hiding this comment.
🟡 Changes recommended
Collection recreation mishandles cleaned-up all-dropped schemas and valid vectorless configurations.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (2)
weaviate/collections/collections/executor.py:485
- This guard prevents
create_from_configfrom recreating a valid vectorless collection after its last named vector has been finalized: the exported config hasvector_config=Noneandvectorizer=None, and Weaviate intentionally preserves that state without applying legacy defaults when no legacy vector fields are supplied.create_from_dict(config.to_dict())already sends the same vectorless shape, so rejecting only the config-object path breaks export/recreate and contradicts the new docstrings. Remove this guard (and its corresponding rejection documentation).
if config.vector_config is None and config.vectorizer is None:
raise WeaviateInvalidInputError(
f"Collection config {config.name!r} has no vector config left; its vectors were "
"dropped with collection.config.delete_vector_index() and removed by the drop's "
"cleanup. Creating it would make the server apply its default legacy vector index "
"instead. Give the config at least one vector before creating the collection."
weaviate/collections/collections/executor.py:173
- This branch assumes that removing the last
vectorConfigentry would make Weaviate apply a legacy default, but Weaviate also supports genuinely vectorless collections: a create body with no named vectors or legacy vector fields is persisted without an index. As a result, a config containing only dropped vectors cannot be recreated even though the safe result is a vectorless collection; do not reject this solely becauseremainingis empty (or otherwise distinguish dropped provenance without relying on the schema shape).
raise WeaviateInvalidInputError(
f"Every vector config in this collection ({dropped}) has vectorIndexType 'none' "
"(its index was dropped with collection.config.delete_vector_index()). Creating "
"the collection without them would make the server apply its default legacy "
"vector index instead. Give at least one vector a real index config before "
- Files reviewed: 15/15 changed files
- Comments generated: 1
- Review effort level: Lite
Narrow the create_from_dict docstring: the all-dropped rejection needs the "none" markers to still be present in the dictionary. A dictionary exported after the drops finished carries no vector fields, cannot be told apart from a minimal legacy config, and is sent unchanged; create_from_config() is the path that rejects that shape.
There was a problem hiding this comment.
🟢 Approval recommended
The endpoint, schema lifecycle handling, validation, documentation, and automated coverage are consistent and complete.
Review details
- Files reviewed: 15/15 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Summary
delete_vector_index(vector_name)method to collection config, allowing users to drop a named vector's index viaDELETE /v1/schema/{className}/vectors/{vectorIndexName}/indexdelete_property_indexmethodCloses #1990