fix: prevent memory drift for referential queries in fast mode

[Zhe-SH-CN]

Description

In fast search mode, TaskGoalParser._parse_fast ignored conversation history entirely. Referential queries like "再找找其他价格优惠的" were embedded as-is, causing them to match unrelated older memories instead of the current conversation topic (issue #1365).

This PR adds lightweight context-aware query expansion in fast mode:

• When conversation history is available and the query looks referential (short, or contains referential hints like 其他/这个/再/more/another), prepend the most recent user message to the query before embedding
• This is a zero-LLM-call heuristic — no latency added to fast mode
• Long user messages are truncated to 120 chars to avoid over-long embedding inputs
• Self-contained long queries (> 20 chars, no referential hints) are not modified

Related Issue: Fixes #1365

Type of change

• Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested

• Unit tests covering: no-conversation, referential query augmentation, non-referential query passthrough, English referential words, empty conversation, conversation without user messages, long message truncation, and public parse() method integration
• python3 -c "import ast; ast.parse(...)" syntax verification

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 linked the issue to this PR

[Memtensor-AI]

🤖 Open Code Review

Target: PR #2154
Task: 152e9c3f92a867df
Base: dev-v2.0.24
Head: fix/query-context-drift

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

---

1. apps/memos-local-plugin/tests/python/test_hermes_provider_pipeline.py (L95-L97)

The import importlib statement is placed inside the test method body rather than at the module level with the rest of the standard-library imports. importlib is a stdlib module that is always available, so there is no conditional reason to defer the import. Placing it here makes it invisible to static analysis tools and linters that check import ordering, and it is inconsistent with the project's own import style (e.g., import json, import sys, etc. are all at the top of this file).

Suggestion: move import importlib to the top of the file alongside the other stdlib imports.

💡 Suggested Change

Before:

        import importlib

        # MemTensorProvider is the class hermes-agent host instantiates.

After:

# At the top of the file, with the other stdlib imports:
import importlib
import json
import sys
import tempfile
import unittest

---

2. apps/memos-local-plugin/core/llm/client.ts (L265)

When there is no user message at all, appending a bare "Return valid json only." user turn creates a semantically broken prompt — the model receives a JSON instruction with no actual task. In practice, normalizeMessages() guarantees at least one user message when the input is a plain string, and throws on an empty array, so this branch is only reachable when callers pass an all-system / all-assistant message list. Silently patching that with a contextless user message hides a malformed-input bug instead of surfacing it. Consider throwing an error or a warning here to alert callers of the unexpected state:

throw new MemosError(ERROR_CODES.INVALID_ARGUMENT, "ensureJsonWordInUserMessage: no user message found in conversation");

---

3. apps/memos-local-plugin/core/llm/client.ts (L268)

The early-return here returns the original messages array reference (not a copy), while the mutation path returns a fresh copy via messages.slice(). This is inconsistent — callers receiving the original reference may mutate it and cause unexpected side-effects elsewhere. For consistency, return messages.slice() (or just [...messages]) in both the early-return and the no-last-user-message branches.

💡 Suggested Change

Before:

    if (/\bjson\b/i.test(msg.content)) return messages;

After:

    if (/\bjson\b/i.test(msg.content)) return messages.slice();

---

4. src/memos/api/client.py (L106)

conversation_id has a default of None, making it optional at the API level. Passing it to _validate_required_params forces it to be non-None (the helper raises when not param_value), effectively making it mandatory. Any caller that omits conversation_id will get a ValueError at runtime. Remove conversation_id from the validation call, or validate it separately only when a non-None value is actually required.

💡 Suggested Change

Before:

        self._validate_required_params(user_id=user_id, conversation_id=conversation_id)

After:

        self._validate_required_params(user_id=user_id)

---

5. src/memos/api/client.py (L661-L662)

Truthiness check incorrectly rejects legitimate values. If a caller passes content="" (empty string), the check raises ValueError even though the caller explicitly provided the argument. Use is None checks instead to distinguish 'not provided' from 'provided but falsy'.

💡 Suggested Change

Before:

        if not content and not title and not status:
            raise ValueError("content, title or status is required")

After:

        if content is None and title is None and status is None:
            raise ValueError("content, title or status is required")

---

6. src/memos/api/client.py (L288-L289)

Dead code: response.iter_lines(decode_unicode=True) always yields str objects, never bytes. The decode_unicode=True flag handles decoding before values are yielded. This branch can never be reached and should be removed to avoid confusion.

---

7. src/memos/api/client.py (L460)

This f-string logger call defeats lazy formatting (the string is always constructed, even when the log level is disabled) and is inconsistent with the structured logger.error("%s", ...) style used elsewhere in this file. Also, the hardcoded 3 should reference MAX_RETRY_COUNT so it stays in sync if the constant changes.

💡 Suggested Change

Before:

                logger.error(f"Failed to add knowledgebase-file form (retry {retry + 1}/3): {e}")

After:

                logger.error("Failed to add knowledgebase-file form (retry %s/%s): %s", retry + 1, MAX_RETRY_COUNT, e)

---

8. tests/api/test_lifecycle.py (L34-L38)

This test has no assertion — it only verifies that shutdown_components does not raise an exception when methods are missing, but that is not explicitly stated. An implicit "no exception" contract can be better expressed with a pytest.raises guard or at least a comment, and adding an assertion (e.g., verifying the object's state is unchanged, or that the function returns None) would make the intent explicit and ensure the test truly validates the expected behaviour rather than passing vacuously.

💡 Suggested Change

Before:

def test_shutdown_components_skips_missing_methods():
    class MinimalScheduler:
        pass

    shutdown_components({"mem_scheduler": MinimalScheduler()})

After:

def test_shutdown_components_skips_missing_methods():
    class MinimalScheduler:
        pass

    # Should not raise even when stop/rabbitmq_close are absent
    result = shutdown_components({"mem_scheduler": MinimalScheduler()})
    assert result is None  # function completes without exception

---

9. src/memos/api/product_models.py (L1151-L1158)

After this change, GetTaskStatusMessageData (the former data element type) is no longer referenced anywhere in the file — MemOSGetTaskStatusResponse.data is now typed as GetTaskStatusData and .messages returns list[GetTaskStatusData]. GetTaskStatusMessageData should be removed to avoid dead code.

If any external callers import GetTaskStatusMessageData by name (e.g. for type hints), this becomes a breaking removal, so check usages before deleting.

---

10. src/memos/api/product_models.py (L1119-L1122)

GetMemoryData uses size / current / pages for the pagination fields, whereas the sibling GetKnowledgebaseFileData added in the same PR uses page_size / page / total. These two models represent paginated lists of the same conceptual kind and are consumed side-by-side by the same client (MemOSGetMemoryResponse / MemOSGetKnowledgebaseFileResponse), yet they use different field names for identical semantics:

Concept	GetMemoryData	GetKnowledgebaseFileData
page size	size	page_size
current page	current	page
total items/pages	total (items) / pages (pages)	total (items only)

Consider aligning on a single convention (e.g. page / page_size / total) across both models to reduce integration confusion for API consumers.

---

11. src/memos/memories/textual/tree_text_memory/retrieve/task_goal_parser.py (L20-L34)

The hint "it" in _MULTI_CHAR_REFERENTIAL_HINTS is matched with a plain substring check (any(h in task_description for h in _MULTI_CHAR_REFERENTIAL_HINTS)), which will produce false positives for any English query containing "it" as a substring — e.g., "submit a report", "capital city", "unit test plan", "commit changes". These queries will all be misclassified as referential and have their effective search query silently altered.

The single-char hints correctly guard against this with startswith, but "it" needs word-boundary matching. Consider splitting the English multi-char hints into a separate set and using re.search(r'\bit\b', task_description), or move "it" to a list that is matched as a whole word:

import re
_ENGLISH_WORD_REFERENTIAL_HINTS = re.compile(
    r'\b(?:it|this|that|more|other|another)\b', re.IGNORECASE
)
# then in is_referential:
or bool(_ENGLISH_WORD_REFERENTIAL_HINTS.search(task_description))

This avoids the false-positive augmentation that could silently degrade retrieval quality for self-contained English queries.

---

12. tests/api/test_product_models.py (L25-L27)

Direct key access schema["example"] will raise an unguarded KeyError if the "example" key is absent, rather than producing a descriptive assertion failure. This test runs independently of test_add_request_exposes_model_level_example, so a failing schema will surface as KeyError with no helpful message instead of the intended assertion message.

Use .get() with an explicit fallback and assert on it, or use pytest.fail:

example = APIADDRequest.model_json_schema().get("example")
assert example is not None, "APIADDRequest schema is missing the 'example' key"

💡 Suggested Change

Before:

    example = APIADDRequest.model_json_schema()["example"]

    messages = example.get("messages")

After:

    schema = APIADDRequest.model_json_schema()
    example = schema.get("example")
    assert example is not None, "APIADDRequest schema is missing the 'example' key"

    messages = example.get("messages")

---

13. tests/api/test_product_models.py (L38-L40)

Same unguarded schema["example"] key access as in test_add_request_example_messages_is_structured_list. If the model-level example is absent, this test will raise a KeyError with no descriptive message rather than an assertion failure.

Apply the same guard:

schema = APIADDRequest.model_json_schema()
example = schema.get("example")
assert example is not None, "APIADDRequest schema is missing the 'example' key"

💡 Suggested Change

Before:

    example = APIADDRequest.model_json_schema()["example"]

    assert "user_id" in example

After:

    schema = APIADDRequest.model_json_schema()
    example = schema.get("example")
    assert example is not None, "APIADDRequest schema is missing the 'example' key"

    assert "user_id" in example

---

14. tests/api/test_client.py (L14-L24)

The _install_memos_package_stub guard silently becomes a no-op in the standard test environment. pyproject.toml sets pythonpath = "src", so pytest puts src/ on sys.path and the real memos package is importable (and typically already imported by the time any test file runs). Because of the if "memos" not in sys.modules check, the stub is skipped and _load_client_module just imports the real module — meaning the stub infrastructure provides no value. Either remove the stub entirely (since pythonpath = "src" already makes the real package available), or replace the guard with an unconditional forced-reload using importlib.reload if isolation is truly needed.

💡 Suggested Change

Before:

def _install_memos_package_stub() -> None:
    if "memos" not in sys.modules:
        memos_pkg = types.ModuleType("memos")
        memos_pkg.__path__ = [str(SRC_DIR)]
        sys.modules["memos"] = memos_pkg

    if "memos.api" not in sys.modules:
        api_pkg = types.ModuleType("memos.api")
        api_pkg.__path__ = [str(SRC_DIR / "api")]
        sys.modules["memos.api"] = api_pkg
        sys.modules["memos"].api = api_pkg

After:

# The stub is unnecessary because pyproject.toml already sets pythonpath = "src".
# Simply importing the module directly works without any sys.modules manipulation.
def _install_memos_package_stub() -> None:
    pass  # no-op: 'src/' is on sys.path via pytest's pythonpath config

---

15. tests/api/test_client.py (L55-L57)

Setting self.json_called = True before raising AssertionError makes the flag unreliable as a guard. If json() is ever called, the flag is set to True and then the AssertionError propagates — the test will abort at that point and never reach the assertion assert stream_response.json_called is False (line 685). Conversely, if json() is never called, json_called stays False and line 685 trivially passes. The flag can never be True at the assertion site, making it a dead check. Move the flag assignment after the raise, or redesign: don't raise inside json(), instead just set the flag and return a sentinel, then assert on the flag afterwards.

💡 Suggested Change

Before:

    def json(self) -> dict:
        self.json_called = True
        raise AssertionError("streaming responses must not be parsed as JSON")

After:

    def json(self) -> dict:
        self.json_called = True
        # Returning a sentinel allows the flag to be inspected after the call
        # without aborting the test via AssertionError.
        return {}  # or raise RuntimeError(...) if you want a hard stop

---

16. tests/memories/textual/test_task_goal_parser_context.py (L84-L85)

The second assertion assert "A" * 121 not in result.rephrased_query is logically vacuous and never validates truncation.

result.rephrased_query is constructed as f"{last_user_msg} {task_description}" where last_user_msg is at most 120 As. A 120-character string of As cannot contain a 121-character substring of As, so "A" * 121 not in result.rephrased_query is always True — even if the truncation logic is removed entirely.

To actually verify that truncation occurred (i.e., the full 300-char message was NOT used), assert that the augmented query does not contain the un-truncated portion:

assert "A" * 121 not in result.rephrased_query  # vacuous — always passes
# Replace with:
assert "A" * 300 not in result.rephrased_query  # confirms the full message was truncated
# Or more directly:
assert "A" * 121 not in result.rephrased_query  # keep as-is only for < 121 truncation limit

Suggested fix:

💡 Suggested Change

Before:

        assert "A" * 120 in result.rephrased_query
        assert "A" * 121 not in result.rephrased_query  # Verify truncation actually occurred

After:

        assert "A" * 120 in result.rephrased_query
        assert "A" * 300 not in result.rephrased_query  # Verify full message was not used (real truncation check)

Generated by cloud-assistant via Open Code Review.

[Memtensor-AI]

❌ Automated Test Results: FAILED

Auto-fix retry 1/2 triggered.

Failed tests:

• test_no_conversation_keeps_original_query
• test_referential_query_gets_augmented
• test_non_referential_long_query_not_augmented
• test_english_referential_query
• test_empty_conversation
• test_conversation_without_user_messages
• test_long_last_user_message_truncated
• test_parse_method_passes_conversation_in_fast_mode

Error details

Tests failed. Failed cases: test_no_conversation_keeps_original_query, test_referential_query_gets_augmented, test_non_referential_long_query_not_augmented, test_english_referential_query, test_empty_conversation

Branch: fix/query-context-drift

[Memtensor-AI]

⚠️ Automated Test Results: INCONCLUSIVE

Automated tests inconclusive (auto-generated test defect); treated as non-blocking. Manual review recommended. Details: The AI-generated test passes a tokenizer keyword argument to TaskGoalParser.__init__(), but the class does not accept such a parameter. [advisory, non-gating] AI-generated tests on branch test/auto-gen-56119b2d819611c1-20260723195034: 62/62 passed — these do NOT affect the PR verdict; review the branch manually.

Branch: fix/query-context-drift

[Memtensor-AI]

✅ Automated Test Results: PASSED

All tests passed (8/8 executed). memos_python_core/changed-repo-python: 8/8. Duration: 5s [advisory, non-gating] AI-generated tests on branch test/auto-gen-152e9c3f92a867df-20260724093617: 101/101 passed — these do NOT affect the PR verdict; review the branch manually.

Branch: fix/query-context-drift