Address vectorsdb dimension review comments#211
Conversation
syncVectorDimension now only writes when the collection's dimension is the placeholder, so a second vector column can't overwrite an archived value, and Collection::jsonSerialize omits dimension when unset so non-vector collection lines keep their previous shape. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Greptile SummaryThis PR updates how VectorsDB collection dimensions are serialized and synced. The main changes are:
Confidence Score: 4/5The VectorsDB dimension sync path can leave stale collection metadata after import.
src/Migration/Destinations/Appwrite.php Important Files Changed
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
src/Migration/Destinations/Appwrite.php:1338-1340
**Stale Vector Dimension Persists**
When an imported VectorsDB collection already has a non-zero archived `dimension` that differs from the vector column size, this guard now skips the sync that used to correct the mismatch. A collection archived with `dimension: 256` and a vector column size of `1536` can leave the destination collection metadata at `256` after the vector column is created, so later vector writes or queries can validate against stale dimensions.
```suggestion
if ((int) $table->getAttribute('dimension', 0) === $resource->getSize()) {
return;
}
```
Reviews (1): Last reviewed commit: "Merge branch 'main' into feat/vectorsdb-..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if ((int) $table->getAttribute('dimension', 0) !== 0) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
When an imported VectorsDB collection already has a non-zero archived dimension that differs from the vector column size, this guard now skips the sync that used to correct the mismatch. A collection archived with dimension: 256 and a vector column size of 1536 can leave the destination collection metadata at 256 after the vector column is created, so later vector writes or queries can validate against stale dimensions.
| if ((int) $table->getAttribute('dimension', 0) !== 0) { | |
| return; | |
| } | |
| if ((int) $table->getAttribute('dimension', 0) === $resource->getSize()) { | |
| return; | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/Migration/Destinations/Appwrite.php
Line: 1338-1340
Comment:
**Stale Vector Dimension Persists**
When an imported VectorsDB collection already has a non-zero archived `dimension` that differs from the vector column size, this guard now skips the sync that used to correct the mismatch. A collection archived with `dimension: 256` and a vector column size of `1536` can leave the destination collection metadata at `256` after the vector column is created, so later vector writes or queries can validate against stale dimensions.
```suggestion
if ((int) $table->getAttribute('dimension', 0) === $resource->getSize()) {
return;
}
```
How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
Valid — but the suggested revert reintroduces the multi-vector overwrite from the #210 review. Fixed in the follow-up by keying the sync to the schema-defining embeddings column: it alone drives dimension (correcting mismatches and the placeholder), other vector columns never touch it.
Follow-up to #210 — the two Greptile findings were fixed on the branch but #210 merged moments before the push, so they missed the merge.
syncVectorDimensionnow only writes when the collection'sdimensionis still the placeholder0, so it fills the value for pre-dimension archives but never overwrites an archived dimension; additional vector column imports are no-ops.Collection::jsonSerializeomitsdimensionwhen unset, so documentsdb and other non-vector collection archive lines keep their previous shape.🤖 Generated with Claude Code