Commit d433a5e
* feat(vectorstore): task #61 P1-V vector adapter family — capability + filter Or guard + retrieve defense-in-depth
Closes task #83 per PM @不穷 dispatch (msg=29c9e753). Folds 4 P1-V
items from task #61 spec v1 § 2.3 into a single PR:
P1-V1 — collection init failure contract documentation
------------------------------------------------------
``ensure_collection`` Protocol docstring now spells out the cross-
adapter contract (idempotent / race-safe / fail-loud / cache-not-
poisoned-on-failure). Both adapters already implement these
behaviours; the documentation closes the spec drift gap so future
implementers have a checklist.
P1-V2 — batch upsert atomicity capability declaration
-----------------------------------------------------
New :class:`VectorBackendCapabilities` frozen dataclass on the base
module declares static per-backend behaviour flags. Each
``VectorStoreConnector`` subclass exposes an instance via the
``BACKEND_CAPABILITIES`` class-level attribute:
* ``PgvectorVectorStoreConnector.BACKEND_CAPABILITIES.supports_atomic_batch_upsert = True``
(PGVector wraps bulk INSERT ON CONFLICT in ``engine.begin()`` —
mid-batch failure rolls back the whole batch).
* ``QdrantVectorStoreConnector.BACKEND_CAPABILITIES.supports_atomic_batch_upsert = False``
(Qdrant ``client.upsert(points, wait=True)`` is best-effort
per-point — partial writes possible on mid-batch failure).
``upsert`` Protocol docstring now points at the capability flag so
callers know to chunk + verify on backends that declare ``False``.
P1-V3 — filter Or empty-parts guard
-----------------------------------
``Or.__post_init__`` already rejects empty ``parts`` at DSL
construction. Both adapter translators now also guard at the
translator boundary so a future refactor that bypasses the
constructor (e.g. ``object.__setattr__(or_node, "parts", ())`` on
the frozen dataclass, or a ``dataclasses.replace`` with empty
parts) can't silently degrade to a vacuous "match everything"
disjunction:
* ``aperag/vectorstore/pgvector_connector.py:_SqlFilter._walk`` —
raises ``UnsupportedFilterError`` on empty post-walk parts.
* ``aperag/vectorstore/qdrant_connector.py:_translate_filter`` —
raises ``UnsupportedFilterError`` on empty post-prune subs (so
``rest.Filter(should=[])`` — which Qdrant treats as match-all —
is unreachable).
P1-V4 — Qdrant legacy mode defense-in-depth
-------------------------------------------
``QdrantVectorStoreConnector.retrieve`` now applies the same
``TENANT_PAYLOAD_KEY`` filter in **both** multitenant and legacy
modes, but with a backwards-compatible "no payload key → pass
through" branch so legacy-only rows that don't carry the payload
key keep working:
* In multitenant mode: filter is the primary tenant-isolation
layer (unchanged behaviour).
* In legacy mode: collection-name isolation is the primary layer;
the new payload-level filter is belt-and-braces against tooling
drift / migration mistakes that could plant a stray foreign-tenant
row in a legacy collection.
The new ``BACKEND_CAPABILITIES.supports_legacy_mode`` flag declares
which adapter supports the legacy layout (PGVector ``False``,
Qdrant ``True``) so callers can tell the difference machine-
readably.
Tests
-----
* ``tests/unit_test/vectorstore/test_backend_capabilities.py``
(new) — pins shape + per-flag values for each adapter. Coordinates
with cuiwenbo task #87 P1-D3 collection metadata Pydantic
projection so the static capability matrix stays consistent
across PRs.
* ``tests/unit_test/vectorstore/test_pgvector_translator.py`` and
``test_qdrant_filter_translation.py`` — pin the new Or empty-parts
guard with frozen-dataclass-bypass coverage.
* ``tests/unit_test/vectorstore/test_qdrant_multitenancy_integration.py``
— new ``test_retrieve_legacy_mode_filters_stray_foreign_payload``
exercises the P1-V4 belt-and-braces filter on a real ``:memory:``
Qdrant client: legacy-mode rows without payload key pass through
(backward compat), own-tenant payload passes, foreign-tenant
payload is dropped.
Local: ``uv run pytest tests/unit_test/vectorstore/`` →
**156 passed, 10 skipped, 1 warning**.
Spec / scope alignment
----------------------
* task #61 spec v1 § 2.3 P1-V1 → ensure_collection contract doc ✅
* task #61 spec v1 § 2.3 P1-V2 → BACKEND_CAPABILITIES.supports_atomic_batch_upsert ✅
* task #61 spec v1 § 2.3 P1-V3 → Or empty-parts guard ✅
* task #61 spec v1 § 2.3 P1-V4 → retrieve defense-in-depth + supports_legacy_mode ✅
* Lesson #14 multi-iteration cleanup — legacy mode flagged via
``supports_legacy_mode`` so a future PR can drop the mode
entirely once telemetry confirms zero production usage ✅
* Lesson #17 backend 收敛 contract — capability declaration is the
backend-side contract that lets callers (FE / API / MCP) read a
single source of truth instead of forking on backend type ✅
Follow-ups (NOT in this PR)
---------------------------
* task #84 P1-G1+G2 graph store boundary tests — ziang
* task #85 P1-D1 e2e shape matrix — huangzhangshu
* task #86 P1-D2 Helm Nebula first-class — Planetegg
* task #87 P1-D3 collection metadata vector_backend projection —
cuiwenbo + dongdong (consumes ``BACKEND_CAPABILITIES`` values)
* task #88 P2-S1+S2 batch alias resolution — Bryce after this PR
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(vectorstore): mode-specific tenant filter on Qdrant retrieve (Weston PR #1948 BLOCKER)
Weston PR #1948 architecture CR (msg=910cad66 BLOCKER) caught a real
correctness regression in the initial P1-V4 commit: the uniform
"no payload key → pass through" branch leaked stray ``{}`` payload
rows in the **shared multitenant collection** to every tenant on a
``retrieve(ids=...)`` call.
Local Qdrant ``:memory:`` repro (per Weston): a multitenant
connector ``tenant_a`` writes a point with ``payload={}`` directly
to the shared collection, then ``tenant_a.retrieve([id])`` returns
the row. Because ``upsert()`` always stamps the payload key, the
only way a missing-key row reaches the shared collection is tooling
drift / migration drift — exactly the case P1-V4 defense-in-depth
is supposed to catch.
Fix
---
Mode-specific semantics:
* **Multitenant mode** (shared physical collection): STRICT —
every row MUST carry ``TENANT_PAYLOAD_KEY`` matching the
connector's tenant id. No "no payload key → pass through"
branch, because the shared collection means a missing key would
expose the row to every tenant.
* **Legacy mode** (per-tenant physical collection, unchanged from
initial commit): PERMISSIVE — a row that doesn't carry the
payload key still passes through (typical pre-multitenant data
shape), but a stray foreign-tenant payload gets dropped (catches
tooling drift / migration mistakes).
Tests
-----
``test_retrieve_multitenant_mode_strict_requires_payload_key`` (new)
— Weston's exact repro: seed shared collection with ``{}`` payload
+ own-tenant payload + foreign-tenant payload, assert only the
own-tenant row passes through. The legacy-mode permissive
counterpart (``test_retrieve_legacy_mode_filters_stray_foreign_payload``)
stays unchanged so a future refactor that unifies them silently
re-opens the leak fails fast.
Local: ``uv run pytest tests/unit_test/vectorstore/`` →
**157 passed, 10 skipped** (one new case).
Sediment trigger
----------------
This is Lesson #12 v9 fifth-application demo same family — Weston
first-principles repro catches the unified branch as silent leak
that I missed when applying the legacy-compat optimization
uniformly. The narrower ``mode-specific`` framing matches the spec
language ("legacy compat for legacy mode only") more precisely.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 4aaae55 commit d433a5e
7 files changed
Lines changed: 430 additions & 11 deletions
File tree
- aperag/vectorstore
- tests/unit_test/vectorstore
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
46 | 46 | | |
47 | 47 | | |
48 | 48 | | |
49 | | - | |
| 49 | + | |
| 50 | + | |
50 | 51 | | |
51 | 52 | | |
52 | 53 | | |
| |||
185 | 186 | | |
186 | 187 | | |
187 | 188 | | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
188 | 231 | | |
189 | 232 | | |
190 | 233 | | |
| |||
194 | 237 | | |
195 | 238 | | |
196 | 239 | | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
197 | 245 | | |
198 | 246 | | |
199 | 247 | | |
| |||
214 | 262 | | |
215 | 263 | | |
216 | 264 | | |
217 | | - | |
218 | | - | |
219 | | - | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
220 | 281 | | |
221 | 282 | | |
222 | 283 | | |
| |||
242 | 303 | | |
243 | 304 | | |
244 | 305 | | |
245 | | - | |
246 | | - | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
247 | 322 | | |
248 | 323 | | |
249 | 324 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
73 | 73 | | |
74 | 74 | | |
75 | 75 | | |
| 76 | + | |
76 | 77 | | |
77 | 78 | | |
78 | 79 | | |
| |||
254 | 255 | | |
255 | 256 | | |
256 | 257 | | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
257 | 273 | | |
258 | 274 | | |
259 | 275 | | |
| |||
325 | 341 | | |
326 | 342 | | |
327 | 343 | | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
| 350 | + | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
328 | 355 | | |
329 | 356 | | |
330 | 357 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
51 | 51 | | |
52 | 52 | | |
53 | 53 | | |
| 54 | + | |
54 | 55 | | |
55 | 56 | | |
56 | 57 | | |
| |||
271 | 272 | | |
272 | 273 | | |
273 | 274 | | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
274 | 290 | | |
275 | 291 | | |
276 | 292 | | |
| |||
416 | 432 | | |
417 | 433 | | |
418 | 434 | | |
| 435 | + | |
| 436 | + | |
| 437 | + | |
| 438 | + | |
| 439 | + | |
| 440 | + | |
| 441 | + | |
| 442 | + | |
| 443 | + | |
| 444 | + | |
| 445 | + | |
| 446 | + | |
419 | 447 | | |
420 | 448 | | |
421 | 449 | | |
| |||
713 | 741 | | |
714 | 742 | | |
715 | 743 | | |
716 | | - | |
717 | | - | |
718 | | - | |
719 | | - | |
720 | | - | |
| 744 | + | |
| 745 | + | |
| 746 | + | |
| 747 | + | |
| 748 | + | |
| 749 | + | |
| 750 | + | |
| 751 | + | |
| 752 | + | |
| 753 | + | |
| 754 | + | |
| 755 | + | |
| 756 | + | |
| 757 | + | |
| 758 | + | |
| 759 | + | |
| 760 | + | |
| 761 | + | |
| 762 | + | |
| 763 | + | |
| 764 | + | |
| 765 | + | |
| 766 | + | |
| 767 | + | |
| 768 | + | |
| 769 | + | |
| 770 | + | |
| 771 | + | |
| 772 | + | |
| 773 | + | |
| 774 | + | |
| 775 | + | |
| 776 | + | |
| 777 | + | |
| 778 | + | |
| 779 | + | |
| 780 | + | |
721 | 781 | | |
722 | 782 | | |
723 | 783 | | |
| |||
Lines changed: 102 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
0 commit comments