Skip to content

Fix #2272: PostgresGraphDB missing reorganizer/handler methods when MOS_ENABLE_REORGANIZE=t - #2276

Open
Memtensor-AI wants to merge 2 commits into
MemTensor:dev-v2.0.30from
Memtensor-AI:bugfix/autodev-2272-20260822153326438
Open

Fix #2272: PostgresGraphDB missing reorganizer/handler methods when MOS_ENABLE_REORGANIZE=t#2276
Memtensor-AI wants to merge 2 commits into
MemTensor:dev-v2.0.30from
Memtensor-AI:bugfix/autodev-2272-20260822153326438

Conversation

@Memtensor-AI

Copy link
Copy Markdown
Collaborator

Description

Fixed issue #2272: PostgresGraphDB was missing the graph-store interface used by the tree text memory reorganizer and handler, crashing the background structure optimization thread (AttributeError: 'PostgresGraphDB' object has no attribute 'node_not_exist') when MOS_ENABLE_REORGANIZE=true with graph_db.backend=postgres, and blocking MERGED_TO edge creation during redundant-memory merge.

Changes (all in src/memos/graph_dbs/):

  • Added node_not_exist(scope, user_name) and get_memory_count(memory_type, user_name) to PostgresGraphDB.
  • Extended edge_exists with type="ANY" (default), direction ("OUT"/"OUTGOING", "IN"/"INCOMING", "ANY") and user_name; the legacy 3-argument call still works and defaults to bidirectional matching.
  • Added get_edges(id, type, direction, user_name) to PostgresGraphDB returning [{"from","to","type"}] like Neo4j, plus a default contract on BaseGraphDB; supports the scheduler's direction="OUT" alias.
  • get_structure_optimization_candidates now accepts **kwargs and honors the caller's user_name (previously hardcoded to config.user_name, which leaked across users).
  • Added search_by_fulltext built on PostgreSQL full-text search (to_tsvector/to_tsquery/ts_rank) with safe word quoting, tsquery_config defaulting to 'simple', and return shape [{"id","score"}] matching the polardb backend; the recall/searcher keyword path no longer raises AttributeError.

Tests: new tests/graph_dbs/test_postgres_graph_db.py with 22 unit tests stubbing psycopg2 (no live DB needed) - all pass. Regression: tests/graph_dbs (53 passed, 3 skipped), tests/memories/textual (60 passed), full suite 743 passed; the only failures are pre-existing and environment-related (missing optional markitdown/qdrant packages, and an unrelated kv-cache bug reproduced on a clean checkout of dev-v2.0.30). ruff check and ruff format are clean.

Committed as 14720c2 on bugfix/autodev-2272-20260822153326438 (pushed to origin) and the opsp artifacts (.ai-tasks task file + openspec proposal/spec/design/tasks/test-cases/verification-report) were synced to the memos-autodev-specs repo.

Related Issue (Required): Fixes #2272

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (does not change functionality, e.g. code style improvements, linting)
  • Documentation update

How Has This Been Tested?

Automated tests are pending.

  • Unit Test
  • Test Script Or Test Steps (please provide)
  • Pipeline Automated API Test (please provide)

Checklist

  • I have performed a self-review of my own code
  • I have commented my code in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • I have created related documentation issue/PR in MemOS-Docs (if applicable)
  • I have linked the issue to this PR (if applicable)
  • I have mentioned the person who will review this PR

@wustzdy please review this PR.

Reviewer Checklist

PostgresGraphDB lacked the graph-store interface used by the tree text
memory reorganizer and handler (node_not_exist, get_memory_count,
get_edges, edge_exists direction/user_name semantics, per-user structure
optimization candidates, search_by_fulltext), crashing the background
structure optimization thread with AttributeError when
MOS_ENABLE_REORGANIZE=true and graph_db.backend=postgres, and blocking
MERGED_TO edge creation during redundant-memory merge.

Add the missing methods with Neo4j-compatible semantics, extend
edge_exists with direction/type-ANY handling, honor user_name in
get_structure_optimization_candidates, implement search_by_fulltext on
PostgreSQL built-in full-text search, and add a default get_edges
contract on BaseGraphDB. Covered by 22 new unit tests stubbing psycopg2.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Memtensor-AI Memtensor-AI added ai:generated Generated or modified by AI | 由 AI 生成或修改 area:database graph_db + vector_db | 图数据库与向量数据库 status:in-progress Someone or AI is working on it | 人工或 AI 正在处理 labels Aug 22, 2026
@Memtensor-AI
Memtensor-AI requested a review from wustzdy August 22, 2026 17:12
@Memtensor-AI

Copy link
Copy Markdown
Collaborator Author

🤖 Open Code Review

Target: PR #2276
Task: 06bd58c4de865475
Base: dev-v2.0.30
Head: bugfix/autodev-2272-20260822153326438
Head SHA: 14720c2036578c08b522ec7a166b69cc4c4052b5

🔍 OpenCodeReview found 5 issue(s) in this PR.


1. src/memos/graph_dbs/base.py (L83-L85)

The method is declared as a plain def with raise NotImplementedError, rather than being decorated with @abstractmethod. Every other required method in BaseGraphDB uses @abstractmethod to prevent incomplete concrete subclasses from being instantiated. Without @abstractmethod, a subclass that neglects to implement get_edges will be happily instantiated and only fail at call time — defeating the safety guarantee of the ABC pattern.

Suggested fix: add @abstractmethod (as done for all peers such as add_edge, delete_edge, edge_exists, etc.).

💡 Suggested Change

Before:

    def get_edges(
        self, id: str, type: str = "ANY", direction: str = "ANY", user_name: str | None = None
    ) -> list[dict[str, str]]:

After:

    @abstractmethod
    def get_edges(
        self, id: str, type: str = "ANY", direction: str = "ANY", user_name: str | None = None
    ) -> list[dict[str, str]]:

2. tests/graph_dbs/test_postgres_graph_db.py (L309-L312)

The tsquery_param is retrieved at index [-2] of the execute params tuple, which is the second tsquery_string argument. The actual tuple is (user_name, tsquery_string, tsquery_string, top_k), so [-2] resolves to tsquery_string (index 2). This is correct for this specific call, but the index is fragile: if the caller adds scope or status arguments, extra entries are prepended to params, shifting tsquery_string closer to the end and keeping [-3]/[-2]/[-1] stable — but the assumption depends on knowing the exact implementation detail. More importantly, call_args[0][1][-2] picks the second occurrence of tsquery_string rather than the first. Both are equal here, but the assertion comment says 'the tsquery param', implying a specific position. Consider using a named variable or [-3] for the first occurrence, and add an explicit comment explaining which positional index maps to which SQL %s placeholder, so future changes to the param list don't silently test the wrong element.


3. tests/graph_dbs/test_postgres_graph_db.py (L261-L263)

This assertion checks that "alice" (the config default user) is not present anywhere in the params list using Python's in operator over a list, which tests for exact list-element membership, not substring containment. So "alice" not in params will be True as long as the string "alice" is not an exact element — it correctly catches the case where the config default leaks in as a direct parameter. However, the intent reads more clearly as a substring guard. The assertion is actually correct for its stated purpose, but the correctness depends on parameters being plain strings (not embedded in JSON or other structures). If the implementation ever passes user_name as part of a JSON blob, this check would silently miss it. Consider asserting more specifically, e.g. assert params[0] == "bob" and assert "alice" not in params, to pin both the positive and negative expectations.


4. src/memos/graph_dbs/postgres.py (L743-L749)

When direction is "OUTGOING" or "INCOMING" and type="ANY" (the default), the literal string "ANY" is passed as a bound parameter to edge_type = %s. No row will have an edge_type equal to the string "ANY", so the method always returns False instead of matching all relationship types. The "ANY" wildcard semantics are only implemented for the direction="ANY" branch.

Fix: skip the edge_type condition when type == "ANY", consistent with how the direction="ANY" branch handles it.

💡 Suggested Change

Before:

        if direction in ("OUTGOING", "OUT"):
            conditions = ["source_id = %s", "target_id = %s", "edge_type = %s"]
            params = [source_id, target_id, type]
        elif direction in ("INCOMING", "IN"):
            # Incoming edge: source_id is the edge's target, and vice versa.
            conditions = ["source_id = %s", "target_id = %s", "edge_type = %s"]
            params = [target_id, source_id, type]

After:

        if direction in ("OUTGOING", "OUT"):
            conditions = ["source_id = %s", "target_id = %s"]
            params = [source_id, target_id]
            if type != "ANY":
                conditions.append("edge_type = %s")
                params.append(type)
        elif direction in ("INCOMING", "IN"):
            # Incoming edge: source_id is the edge's target, and vice versa.
            conditions = ["source_id = %s", "target_id = %s"]
            params = [target_id, source_id]
            if type != "ANY":
                conditions.append("edge_type = %s")
                params.append(type)

5. src/memos/graph_dbs/postgres.py (L818-L823)

The user_name filter in get_edges() uses OR between the subqueries: it returns an edge if either the source or the target belongs to the user. Because the edges table has no user_name column, this can return edges whose other endpoint belongs to a completely different tenant, leaking cross-tenant relationship data.

For strict tenant isolation both endpoints should belong to the same user (AND instead of OR), or the query should only return edges where the requested node (id) belongs to the user — which is the more natural and correct filter given the call context.

💡 Suggested Change

Before:

        if user_name:
            where_clause += (
                f" AND (source_id IN (SELECT id FROM {self.schema}.memories WHERE user_name = %s)"
                f" OR target_id IN (SELECT id FROM {self.schema}.memories WHERE user_name = %s))"
            )
            params.extend([user_name, user_name])

After:

        if user_name:
            where_clause += (
                f" AND (source_id IN (SELECT id FROM {self.schema}.memories WHERE user_name = %s)"
                f" AND target_id IN (SELECT id FROM {self.schema}.memories WHERE user_name = %s))"
            )
            params.extend([user_name, user_name])

Generated by cloud-assistant via Open Code Review.

@Memtensor-AI

Copy link
Copy Markdown
Collaborator Author

🔧 Open Code Review requested Agent fix

Open Code Review found 5 issue(s). I have resumed the development Agent to fix them.

  • Task: 06bd58c4de865475
  • Fix attempt: 1/2
  • Finding delta: 0 repeated / 5 new / 0 likely resolved

The Agent will push a new commit to this PR branch. OCR will recheck after the commit is pushed.

- edge_exists: skip edge_type condition when type="ANY" for OUT/IN
  directions, matching the direction="ANY" branch and Neo4j semantics
- get_edges: require both endpoints in the same user scope (AND instead
  of OR) for strict tenant isolation, matching Neo4j reference
- base.py: mark get_edges @AbstractMethod to guarantee subclasses
  implement the edge query contract
- tests: pin params[1] == user_name in optimization candidates test and
  select the first tsquery_string slot via [-3] with layout comments

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Memtensor-AI

Copy link
Copy Markdown
Collaborator Author

⚠️ Automated Test Results: ENV ISSUE

The test environment encountered an issue that requires manual attention.

Details: Executor error: Command failed: git clone --depth 1 --branch bugfix/autodev-2272-20260822153326438 git@github.com:Memtensor-AI/MemOS.git /data/test-workspaces/06bd58c4de865475/repo
Cloning into '/data/test-workspaces/06bd58c4de865475/repo'...
kex_exchange_identification: Connection closed by remote host
Connection closed by UNKNOWN port 65535
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.
Branch: bugfix/autodev-2272-20260822153326438

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai:generated Generated or modified by AI | 由 AI 生成或修改 area:database graph_db + vector_db | 图数据库与向量数据库 status:in-progress Someone or AI is working on it | 人工或 AI 正在处理

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants