Fix #2270: fix: Postgres reorganizer fails when pgvector embedding is returned as string - #2275
Conversation
…es backend PostgresGraphDB._parse_row copied the pgvector `embedding` column straight into `metadata["embedding"]`. Without a registered psycopg2 type adapter, pgvector returns its Postgres text form (`"[0.1, 0.2, ...]"`) as a Python str, so downstream code such as the tree-text memory reorganizer's `_convert_id_to_node` -> `GraphDBNode` construction hit `pydantic_core.ValidationError` (embedding expects `list[float]`) and reorganize never ran. Introduce a `_normalize_embedding` helper that accepts `None`, `list`, `tuple`, and both bracket- and paren-style text forms, returning `list[float] | None`. Route `_parse_row` through it so every read path (`get_node`, `get_nodes`, `get_all_memory_items`, `get_by_metadata`, `get_all_by_scope`, `export_graph`) yields a numeric list. Unrecoverable input logs a warning and returns None so reads degrade gracefully rather than crash. No new runtime dependencies. Add 13 regression tests under `tests/graph_dbs/test_postgres_embedding_normalize.py` covering the helper (None / list / str / paren / empty / bad-string / unexpected type) and an end-to-end reproduction that feeds `_parse_row` output into `GraphDBNode(**raw)` without raising. Fixes MemTensor#2270
Memtensor-AI
added
ai:generated
🤖 Open Code ReviewTarget: PR #2275 ✅ OpenCodeReview: Review complete: 0 finding(s) across 2 selected item(s). Generated by cloud-assistant via Open Code Review. |
|
🔧 Open Code Review requested Agent fixOpen Code Review found 3 issue(s). I have resumed the development Agent to fix them.
The Agent will push a new commit to this PR branch. OCR will recheck after the commit is pushed. |
Addresses three OCR findings on PR MemTensor#2275: - Enforce matching bracket/paren pairs in the pgvector text-form fallback so "[0.1, 0.2)" and "(0.1, 0.2]" no longer silently parse into a numeric list; they now return None and warn. - Add real caplog record assertions to the invalid-string and unexpected-type tests so a silent no-log regression can no longer pass. Scope caplog to the memos.graph_dbs.postgres logger. - Add a parametrized regression test for the mismatched-delimiter behavior. - Correct docstring spelling from "Normalise" to "Normalize" to match the American spelling used in the function name. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 participants
Description
Fixes #2270 by normalising the pgvector
embeddingcolumn insidePostgresGraphDB._parse_row. A new module-level helper_normalize_embedding(value) -> list[float] | NoneacceptsNone,list,tuple, and both bracket- and paren-style Postgres text forms (e.g."[0.1, 0.2, ...]"). psycopg2 returns the pgvector column as a str unless a type adapter is registered, and downstream code such as the tree-text reorganizer's_convert_id_to_nodeconstructsGraphDBNode(**raw), whosemetadata.embeddingis typedlist[float] | None; the previous direct assignment triggeredpydantic_core.ValidationErrorand reorganize never ran.Because
_parse_rowis the single choke-point for every read path (get_node,get_nodes,get_all_memory_items,get_by_metadata,get_all_by_scope,export_graph), routing embeddings through the helper fixes the bug for every caller with minimal blast radius. Unrecoverable input logs a warning and returns None so reads degrade gracefully rather than crash. No new runtime or optional dependencies; only stdlibjson(already imported) plus a bracket/paren fallback.Testing: added
tests/graph_dbs/test_postgres_embedding_normalize.pywith 13 unit + integration tests covering every helper branch and an end-to-end reproduction that feeds_parse_rowoutput intoGraphDBNode(**raw). Verification:ruff checkandruff formatclean;pytest tests/graph_dbs/→ 44 passed, 3 pre-existing skips; two pre-existing failures intests/memories/activation/test_kv.pyreproduce on the untouched base branch (upstreamtransformers.DynamicCacheAPI change) and are unrelated. Branch pushed; specs archived to memos-autodev-specsmain.Related Issue (Required): Fixes #2270
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Not run; documentation-only change.
Checklist
@wustzdy please review this PR.
Reviewer Checklist