[shaohuzhang1]

refactor: update Xunfei model interfaces to align with OpenAI compatibility and remove deprecated components

[shaohuzhang1]

Review of embedding.py diff

1. Functional bugs / regressions

a) optional_params are stored but never used — self.optional_params is set in __init__ and then ignored in embed_documents. Any user-configured OpenAI-compatible options (dimensions, user, encoding_format, timeout, etc.) silently have no effect. Merge them into the request:

res = self.client.create(input=texts, model=self.model_name, **self.optional_params)

b) chunk_size is accepted but ignored — Neither the caller-supplied chunk_size nor any default batching is applied. The OpenAI embeddings API caps inputs per request (2,048 items) and total tokens per request; large MaxKB document imports will fail. Batch it.

c) Response ordering is not guaranteed — The OpenAI schema includes index per item, and some compatible providers return data out of order. [e.embedding for e in res.data] can silently misalign embeddings with texts. Sort by e.index, and validate len(res.data) == len(texts).

d) No handling of empty input — embed_documents([]) goes straight to the API and typically errors. Return [] early.

e) Credentials are not validated — api_base may be None/"" and api_key may be missing; openai.OpenAI(...) will either fall back to the public OpenAI endpoint (unintended) or fail with an opaque error. Validate in new_instance.

f) Return type changed — The old implementation returned np.ndarray (float32, little-endian) for embed_query/_parser_message; the new one returns List[float]. Verify every downstream consumer (vector store, schema dimension check, cast calls) accepts plain lists — otherwise you get silent type errors at runtime.

g) Output dimensionality — The old code truncated to 2560 dimensions. There is no equivalent now: if dimensions isn't passed (or model_id differs), you can get a different vector length than the DB column expects. Pass dimensions via optional_params or validate length.

h) Class rename — XFEmbedding → XunFeiOpenAIEmbeddingModel. Any imports/registries referencing the old name will break. Confirm new_instance is wired to the correct provider/model type, and note that model_credential.get("model_id") or model_name changes the model-selection precedence versus the old code.

2. Robustness / reliability

i) No error context — Raw openai exceptions propagate. The old code retried 3× with backoff; the OpenAI SDK retries automatically (max_retries, default 2) but does not handle rate-limit-specific logic for this provider. Set explicit timeout and max_retries and wrap/log errors.

j) QPS throttling removed — The old worker enforced time.sleep(0.5) (QPS=2). That serialized requests globally (a bottleneck, but also a guard). Without it, concurrent embed_documents calls can blow through the provider's rate limits. Consider an asyncio.Semaphore/threading.Semaphore or batching instead of per-call throttling.

k) Client lifetime — openai.OpenAI(...) owns an httpx client. Instantiating one per new_instance call (if called per request) creates and leaks connection pools. Keep one client per model instance and ensure close()/context-manager use, or cache instances.

l) encoding_format="float" — Fine as an explicit default, but some OpenAI-compatible gateways reject it or return base64 despite the request. Make it a default in optional_params rather than hard-coded, and defensively handle base64 responses if you support them.

3. Style / typing

m) Inconsistent typing — from typing import Dict, List combined with int | None (PEP 604, Python ≥3.10). Pick one: either list[str] / int | None (and drop typing.List), or Optional[int]. Make sure the declared Python version matches.

n) embed_query lacks a return annotation, and self.client = ...embeddings stores a resource rather than the client; naming it self._embeddings better reflects that.

4. Suggested implementation

from typing import Dict, List, Optional
import openai

class XunFeiOpenAIEmbeddingModel(MaxKBBaseEmbeddingModel):
    def supports_image_embedding(self) -> bool:
        return False

    def is_cache_model(self) -> bool:
        return False

    def __init__(self, api_key, base_url, model_name: str,
                 optional_params: dict, chunk_size: int = 16,
                 timeout: float = 60.0, max_retries: int = 3):
        self._embeddings = openai.OpenAI(
            api_key=api_key, base_url=base_url or None,
            timeout=timeout, max_retries=max_retries,
        ).embeddings
        self.model_name = model_name
        self.optional_params = dict(optional_params or {})
        self.optional_params.setdefault("encoding_format", "float")
        self.chunk_size = chunk_size

    @staticmethod
    def new_instance(model_type, model_name, model_credential: Dict[str, object], **model_kwargs):
        api_key = model_credential.get("api_key")
        base_url = model_credential.get("api_base")
        if not api_key or not base_url:
            raise ValueError("XunFei OpenAI-compatible embedding requires 'api_key' and 'api_base'")
        return XunFeiOpenAIEmbeddingModel(
            api_key=api_key,
            base_url=base_url,
            model_name=model_credential.get("model_id") or model_name,
            optional_params=MaxKBBaseEmbeddingModel.filter_optional_params(model_kwargs),
        )

    def embed_query(self, text: str) -> List[float]:
        return self.embed_documents([text])[0]

    def embed_documents(self, texts: List[str], chunk_size: Optional[int] = None) -> List[List[float]]:
        if not texts:
            return []
        size = chunk_size or self.chunk_size
        vectors: List[List[float]] = []
        for i in range(0, len(texts), size):
            batch = texts[i:i + size]
            res = self._embeddings.create(
                input=batch, model=self.model_name, **self.optional_params
            )
            data = sorted(res.data, key=lambda d: d.index)
            if len(data) != len(batch):
                raise ValueError(
                    f"Embedding count mismatch: sent {len(batch)}, got {len(data)}"
                )
            vectors.extend(d.embedding for d in data)
        return vectors

5. Checklist before merging

• Confirm maxkb base class contract for return type (list vs ndarray) and chunk_size.
• Verify vector dimension matches the DB column (dimensions param if applicable).
• Update provider registry/imports for the renamed class.
• Add a unit test for batching, ordering, and empty input.