Skip to content

Conversation

ErinnerMO
Copy link

Description

  • Fix: Categorization now respects the configured LLM provider/model (DeepSeek or OpenAI). Previously it was hardcoded to an OpenAI model, so even when both the LLM and embeddings used non‑OpenAI providers, categorization still forced OpenAI. It now follows the configured provider/model for these two backends.

  • Fix: Normalize MCP server tool-handler return type to str per the MCP interface specification (some handlers previously returned a Python dict).

  • Enhancement: Preserve Unicode in JSON responses (no ASCII escaping, e.g., ensure_ascii=False), improving non‑ASCII handling.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactor (does not change functionality, e.g. code style improvements, linting)

How Has This Been Tested?

  • Test Script (manual verification using live provider APIs; no new unit tests were added)

Manual test setup:

  • Environment variables:

    • OPENAI_BASE_URL = Aliyun DashScope’s OpenAI‑compatible endpoint
    • OPENAI_API_KEY = Aliyun DashScope’s OpenAI‑compatible endpoint API key
    • DEEPSEEK_BASE_URL = official DeepSeek API base URL
    • DEEPSEEK_API_KEY = official DeepSeek API key
  • Models:

    • LLM: DeepSeek (model: deepseek-chat)
    • Embeddings: Qwen3 (model: text-embedding-v4, via DashScope)
  • Note: At this time, categorization supports DeepSeek and OpenAI only; this run validates the DeepSeek path with Qwen3 embeddings.

Steps and expected results:

  1. Categorization respects configured provider/model (DeepSeek)

    • Configure LLM provider = DeepSeek (deepseek-chat); embeddings = Qwen3 text-embedding-v4 via DashScope (through OPENAI_BASE_URL).
    • Trigger a memory entry categorization.
    • Expectation: categorization calls DeepSeek (no OpenAI fallback). Verify via request/trace logs that calls hit DEEPSEEK_BASE_URL; no requests are sent to api.openai.com.
  2. Unicode JSON responses

    • Submit text containing non‑ASCII characters (e.g., “Unicode character test — mañana 🚀”).
    • Expectation: Unicode remains unescaped (no \uXXXX sequences); response encoding is UTF‑8.
  3. MCP tool call return type is normalized to string

    • Invoke a tool handler that previously could return a Python dict.
    • Expectation: server now returns a string payload per the MCP spec; json.loads(response) succeeds and isinstance(response, str) is true.
  4. Lint sanity (F811 fix)

    • Run pre‑commit hooks (e.g., pre-commit run -a).
    • Expectation: no F811 (“redefinition of unused name”) warning; checks pass.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My changes generate no new warnings
  • I have checked my code and corrected any misspellings

Maintainer Checklist

  • N/A — no associated GitHub issue for this PR
  • Made sure Checks passed

@CLAassistant
Copy link

CLAassistant commented Sep 8, 2025

CLA assistant check
All committers have signed the CLA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants