fix: remove dead code in PolarDB and redact chat request logs

[fuxicodex]

Summary

Cleaning pass on two files found during a full project audit:

• src/memos/graph_dbs/polardb.py (–597 lines): removes six unreferenced legacy methods (edge_exists_old, get_edges_old, get_neighbors_by_tag_old, get_grouped_counts1, get_all_memory_items_old, get_neighbors_by_tag_ccl) plus unreachable code in drop_database. All of them referenced self.connection, which is never assigned in the class (only self.connection_pool is), so they would raise AttributeError if ever called. __del__ is fixed to close the real connection pool via closeall() instead of checking the phantom self.connection.
• src/memos/api/handlers/chat_handler.py: request logging no longer prints the full Pydantic model, which could leak memory content (query/history/system_prompt) and the business_key auth credential. A whitelisted summary now logs safe fields and masks business_key.
• .gitignore: ignore FuXi CLI local state (.fuxi/).

Why

• Dead code accumulates risk: the _old/_ccl variants silently crash on self.connection.
• Connection pool was never closed on destruction (pool exhaustion risk under long-lived daemon).
• Chat request logs could expose user memory content and business credentials.

Test plan

• python -m py_compile passes for both modified .py files
• AST check: all live methods intact, zero self.connection (non-pool) references remain
• Full-repo grep: deleted methods have no remaining callers
• Functional test of the log helper: masks business_key, excludes query/prompt, keeps whitelist fields (3 cases pass)

Checklist

• Scoped: one logical change set (dead code + log redaction + gitignore)
• Public API behavior unchanged

🤖 Generated with FuXi

[Memtensor-AI]

🤖 Open Code Review

Target: PR #2377
Task: 6a76389d7ce0b3cd
Base: main
Head: fix/cleanup-dead-code-log-redaction

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

⚠️ 1 warning(s) occurred during review.

---

1. src/memos/graph_dbs/neo4j.py (L868-L873)

The WHERE filter is applied after shortestPath resolves, not during traversal. Neo4j computes the shortest path across all nodes first, then discards it if it fails the user_name predicate — which means a shorter cross-tenant path causes the real same-user path to be silently dropped and [] returned. The fix is to push the user filter into the node patterns themselves:

if not self.config.use_multi_db:
    if not user_name:
        raise ValueError("user_name is required in non-multi-db mode")
    node_filter = ", user_name: $user_name"
    params["user_name"] = user_name
else:
    node_filter = ""

query = f"""
    MATCH p = shortestPath(
        (n:Memory {{id: $source_id{node_filter}}})-[*1..{hops}]-
        (m:Memory {{id: $target_id{node_filter}}})
    )
    WHERE all(x IN nodes(p) WHERE x.user_name = $user_name)
    RETURN [x IN nodes(p) | x.id] AS path_ids
    LIMIT 1
"""

Filtering on both terminal nodes and the interior nodes ensures the path stays within the tenant during traversal.

---

2. src/memos/graph_dbs/neo4j.py (L957)

get_context_chain is documented to return an "ordered list of node IDs in the chain" — implying multi-hop chain traversal following sequential FOLLOWS edges. Delegating to get_neighbors returns only the immediate one-hop neighbours of id, with no ordering guarantee (no ORDER BY in the underlying query). Any caller expecting a traversal of the full chain (e.g. node A → B → C → D) receives only A's direct children.

A proper chain traversal needs a path query, for example:

MATCH p = (a:Memory {id: $id})-[:FOLLOWS*1..]->(b:Memory)
RETURN [x IN nodes(p) | x.id] AS chain
ORDER BY length(p) ASC
LIMIT 1

or an iterative call following the single next node until the chain terminates. This is a silent correctness regression from NotImplementedError.

---

3. src/memos/graph_dbs/neo4j.py (L867-L869)

When max_depth is silently clamped (e.g. a caller passes 25, which becomes 10, or passes 0/-1, which becomes 1), the method returns [] without any indication that the cap was applied. A caller asking for depth 25 that genuinely needs it gets a wrong "no path found" result with no way to distinguish it from a real absence of a path. Consider either raising a ValueError for out-of-range values, or at minimum logging a warning when the input is clamped:

if max_depth < 1 or max_depth > 10:
    logger.warning(
        "get_path: max_depth=%d is outside [1, 10]; clamping to %d",
        max_depth, max(1, min(max_depth, 10))
    )
hops = max(1, min(max_depth, 10))

---

4. src/memos/api/handlers/chat_handler.py (L80-L83)

The except AttributeError guard is intended to catch the case where chat_req has no model_dump() method (i.e., it is not a Pydantic model). However, it also silently swallows any AttributeError raised inside model_dump() itself — for example, from a Pydantic computed field or validator that does an internal attribute lookup. When that happens, the fallback __dict__ may expose Pydantic private internals (e.g., __fields_set__, __private_attributes__) and the underlying programming error is completely hidden.

Prefer an explicit hasattr check to limit the guard strictly to the 'method not present' case:

if hasattr(chat_req, "model_dump"):
    data = chat_req.model_dump()
else:
    data = getattr(chat_req, "__dict__", {})

---

5. src/memos/api/handlers/chat_handler.py (L78-L87)

The code after the try/except block — the dict comprehension, the business_key masking, and the f-string formatting — has no exception guard. If any step fails (e.g., a whitelisted field value has a broken __repr__, or data.items() raises), the exception propagates through _log_chat_req and into the calling chat handler, turning an innocuous log call into an unhandled runtime error that aborts the request.

Logging helpers should be unconditionally non-throwing. Wrap the entire body in a top-level try/except Exception:

def _safe_chat_req_log(chat_req: Any, prefix: str) -> str:
    try:
        if hasattr(chat_req, "model_dump"):
            data = chat_req.model_dump()
        else:
            data = getattr(chat_req, "__dict__", {})
        safe = {k: v for k, v in data.items() if k in _CHAT_REQ_LOG_WHITELIST}
        if "business_key" in data:
            safe["business_key"] = "***" if data.get("business_key") else None
        return f"{prefix} Chat Req: {safe}"
    except Exception as exc:  # noqa: BLE001
        return f"{prefix} Chat Req: <serialization error: {exc}>"

---

6. src/memos/api/handlers/chat_handler.py (L94)

Passing a pre-built string to logger.info() bypasses Python's lazy %-style formatting protocol. While the isEnabledFor guard above already prevents the expensive _safe_chat_req_log call when INFO is disabled — so there is no performance regression — the canonical idiom is:

logger.info("%s", _safe_chat_req_log(chat_req, prefix))

This allows third-party log handlers and structured-logging libraries to inspect the raw arguments before formatting, and is consistent with the %s-style fix applied to config.py in this same PR (logger.info("nacos config: %s", data_props)).

---

7. src/memos/graph_dbs/polardb.py (L1142-L1145)

When user_name is absent (multi-db mode), user_clause is an empty string and the MATCH pattern has no WHERE clause at all — the path query will traverse every node in the graph across all tenants. In multi-db mode each database is per-tenant, so this is safe, but the comment in __init__ says user_name can be None even in multi-db mode. If both can co-exist in the same graph, a missing user_name silently returns cross-tenant paths with no error or warning. Compare get_neighbors, which uses the same conditional with the same gap.

Suggestion: either assert that user_name must be set in shared-db mode before executing, or emit a warning log when user_name is None in a context where tenant isolation is expected.

---

8. src/memos/graph_dbs/polardb.py (L1166-L1176)

When raw is not a list (e.g., AGE returns an unexpected scalar or a string-encoded agtype), the method silently returns []. This is indistinguishable from a genuine "no path found" result, masking driver compatibility issues or malformed query output.

Suggestion: add a warning log before the fallback return so regressions are visible:

else:
    logger.warning("get_path: unexpected result type %s for row[0]: %r", type(raw).__name__, raw)
    return []

---

9. src/memos/graph_dbs/polardb.py (L1385)

The base class docstring (confirmed in base.py:158-166) specifies that get_context_chain returns an ordered chain of node IDs. The new implementation delegates to get_neighbors, which uses RETURN DISTINCT b.id AS neighbor_id — a set-based, unordered operation. For a FOLLOWS chain where traversal order carries semantic meaning (e.g., context sequence), returning an unordered set of direct neighbours is semantically incorrect relative to the contract.

A proper ordered chain would require a path-based or iterative traversal (e.g., MATCH (a)-[:FOLLOWS*]->(b) with ORDER BY on path length, or a recursive walk), not a single-hop DISTINCT neighbor lookup.

---

10. src/memos/graph_dbs/polardb.py (L1045-L1048)

The ad-hoc quote-stripping logic (raw[1:-1]) is duplicated between get_neighbors and get_path (inner loop). If the AGE driver behaviour changes or the logic needs a fix, both sites must be updated in sync. Consider extracting a shared helper:

def _unwrap_agtype_string(val: Any) -> str:
    raw = val.value if hasattr(val, "value") else val
    if isinstance(raw, str) and raw.startswith('"') and raw.endswith('"'):
        raw = raw[1:-1]
    return str(raw)

---

🧹 Filtered 1 low-confidence OCR finding(s) before posting/fix-loop (existing_code_mismatch: 1).

Generated by cloud-assistant via Open Code Review.

[Memtensor-AI]

✅ Automated Test Results: PASSED

All tests passed (3/3 executed). memos_python_core/changed-python-source: 3/3. Duration: 8s

Branch: fix/cleanup-dead-code-log-redaction

[Memtensor-AI]

❌ Automated Test Results: FAILED

Auto-fix retry 1/2 triggered.

Error details

Tests failed. Failed cases:

Branch: fix/cleanup-dead-code-log-redaction

[Memtensor-AI]

⚠️ Automated Test Results: ENV ISSUE

The test environment encountered an issue that requires manual attention.

Details: All 387 failures occur in the shared authed_viewer fixture at conftest.py:372 before any test logic runs. The viewer login consistently returns 401 'login required', indicating the viewer user account is not present or the authentication configuration no longer accepts the fixture's credentials.
Branch: fix/cleanup-dead-code-log-redaction

[Memtensor-AI]

⚠️ Automated Test Results: ENV ISSUE

The test environment encountered an issue that requires manual attention.

Details: Viewer login is returning 401 unauthenticated during test fixture setup in conftest.py, preventing the tests from executing their actual assertions. This is a shared authentication fixture failure affecting all 10 failing tests in the same file.
Branch: fix/cleanup-dead-code-log-redaction

[Memtensor-AI]

❌ Automated Test Results: FAILED

Auto-fix retry 1/2 triggered.

Failed tests:

• test_llm_slot_has_only_whitelisted_keys
• test_embedder_slot_has_only_whitelisted_keys
• test_llm_and_embedder_slots_have_no_sensitive_keys
• test_provider_and_model_are_string_type
• test_provider_and_model_do_not_reflect_script_or_sql
• test_runtime_fields_are_not_overridden_by_disk_config
• test_concurrent_llm_model_provider_stable
• test_concurrent_embedder_model_provider_stable
• test_concurrent_slots_shape_no_torn_read
• test_concurrent_no_field_downgrade_to_null

Error details

Tests failed. Failed cases: test_llm_slot_has_only_whitelisted_keys, test_embedder_slot_has_only_whitelisted_keys, test_llm_and_embedder_slots_have_no_sensitive_keys, test_provider_and_model_are_string_type, test_provider_and_model_do_not_reflect_script_or_sql [advisory, non-gating] AI-generated tests on branch test/auto-gen-6a76389d7ce0b3cd-20260916181613: 89/97 passed, 8 failed — these do NOT affect the PR verdict; review the branch manually.

Branch: fix/cleanup-dead-code-log-redaction