mirror of
https://github.com/willmiao/ComfyUI-Lora-Manager.git
synced 2026-09-20 18:51:26 -03:00
fix(llm): stop DeepSeek enrichment failing on json_schema rejection
Enriching a model with `llm_provider=deepseek` failed outright with
HTTP 400 "This response_format type is unavailable now". Probing the
endpoint shows why:
response_format absent -> 200
{"type": "json_object"} -> 200
{"type": "json_schema",...} -> 400
`chat_completion_json` preferred `json_schema` for a real reason -- LM
Studio and other local OpenAI-compatible servers reject `json_object`
but accept `json_schema` -- and guarded the fallback with a substring
test for `'response_format.type'` (the wording of those servers'
rejection). DeepSeek's message is "This response_format type is
unavailable now", which does not contain that substring, so the guard
re-raised and the retry never ran.
Make the format a per-provider chain instead of a single guess:
- `_JSON_OBJECT_ONLY_PROVIDERS` lists providers known to reject
json_schema (currently just deepseek). They ask for `json_object`
first, so the common case costs one request and no wasted retry.
- Everyone else keeps `json_schema` first, then downgrades through
`json_object` and finally prompt-only mode.
- A downgrade now happens on any error mentioning `response_format`,
which covers wording variants without swallowing unrelated failures:
auth errors, unknown models, and rate limits still surface unchanged
because their messages never name the parameter.
`json_object` is sufficient here: the skill prompt already specifies the
exact JSON shape, and `_try_salvage_json` repairs imperfect output.
Verified against the real configured endpoint with the real
`enrich_hf_metadata` prompt, prompt renderer, and ModelScope model card
for jj3550945163/Krea-2-LORA: a 9,815-character prompt returns
parseable JSON (base_model "Flux.1 Krea", description, tags, notes).
Three regression tests cover the DeepSeek ordering, the
json_schema -> json_object downgrade, and the no-retry-on-unrelated-400
path. Full backend suite: 2856 passed.
This commit is contained in:
+58
-34
@@ -267,6 +267,16 @@ _PROVIDER_DEFAULTS: Dict[str, str] = {
|
|||||||
# Request timeout for LLM calls (seconds)
|
# Request timeout for LLM calls (seconds)
|
||||||
_LLM_TIMEOUT = aiohttp.ClientTimeout(total=120)
|
_LLM_TIMEOUT = aiohttp.ClientTimeout(total=120)
|
||||||
|
|
||||||
|
# Providers that do NOT implement ``response_format: {"type": "json_schema"}``
|
||||||
|
# and reject it with HTTP 400. For these the weaker, widely supported
|
||||||
|
# ``json_object`` mode is used instead (the prompt already specifies the
|
||||||
|
# expected JSON shape, and ``_try_salvage_json`` repairs imperfect output).
|
||||||
|
# DeepSeek answers a json_schema request with
|
||||||
|
# ``{"error":{"message":"This response_format type is unavailable now"}}``.
|
||||||
|
# LM Studio and some other local OpenAI-compatible servers reject
|
||||||
|
# ``json_object`` but accept ``json_schema``, so they are not listed here.
|
||||||
|
_JSON_OBJECT_ONLY_PROVIDERS = frozenset({"deepseek"})
|
||||||
|
|
||||||
|
|
||||||
class LLMService:
|
class LLMService:
|
||||||
"""Centralized LLM API client.
|
"""Centralized LLM API client.
|
||||||
@@ -614,47 +624,61 @@ class LLMService:
|
|||||||
if effective_max is None:
|
if effective_max is None:
|
||||||
effective_max = 4096
|
effective_max = 4096
|
||||||
|
|
||||||
# Use json_schema (not json_object) for broader provider compatibility:
|
# Structured-output format. ``json_schema`` is preferred because LM
|
||||||
# LM Studio and some other OpenAI-compatible servers reject
|
# Studio and other local OpenAI-compatible servers reject
|
||||||
# json_object but accept json_schema. {"type": "object"} is
|
# ``json_object`` but accept ``json_schema``; ``{"type": "object"}``
|
||||||
# functionally equivalent — it accepts any JSON object without
|
# accepts any JSON object without constraining specific fields, so the
|
||||||
# constraining specific fields.
|
# two modes are functionally equivalent here. Providers known to
|
||||||
response_format = {
|
# reject json_schema (see _JSON_OBJECT_ONLY_PROVIDERS) get
|
||||||
|
# ``json_object`` instead.
|
||||||
|
schema_format: Dict[str, Any] = {
|
||||||
"type": "json_schema",
|
"type": "json_schema",
|
||||||
"json_schema": {
|
"json_schema": {
|
||||||
"name": "metadata",
|
"name": "metadata",
|
||||||
"schema": {"type": "object"},
|
"schema": {"type": "object"},
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
|
json_object_format: Dict[str, Any] = {"type": "json_object"}
|
||||||
|
|
||||||
try:
|
if self._get_config()["provider"] in _JSON_OBJECT_ONLY_PROVIDERS:
|
||||||
result = await self.chat_completion(
|
format_chain: List[Optional[Dict[str, Any]]] = [
|
||||||
messages=messages,
|
json_object_format,
|
||||||
model=model,
|
None,
|
||||||
temperature=temperature,
|
]
|
||||||
response_format=response_format,
|
else:
|
||||||
max_tokens=effective_max,
|
format_chain = [schema_format, json_object_format, None]
|
||||||
)
|
|
||||||
except LLMResponseError as e:
|
result: Optional[Dict[str, Any]] = None
|
||||||
# Only fall back when the provider rejects the response_format
|
for index, fmt in enumerate(format_chain):
|
||||||
# type value (e.g. "'response_format.type' must be..."). Avoid
|
try:
|
||||||
# catching unrelated 400 errors whose body happens to mention
|
result = await self.chat_completion(
|
||||||
# "response_format" (e.g. "model does not support
|
messages=messages,
|
||||||
# response_format restrictions on this endpoint").
|
model=model,
|
||||||
if "'response_format.type'" not in str(e).lower():
|
temperature=temperature,
|
||||||
raise
|
response_format=fmt,
|
||||||
logger.info(
|
max_tokens=effective_max,
|
||||||
"Provider rejected response_format, retrying without it. "
|
)
|
||||||
"Falling back to prompt-only JSON mode. Error: %s",
|
break
|
||||||
e,
|
except LLMResponseError as e:
|
||||||
)
|
message = str(e).lower()
|
||||||
result = await self.chat_completion(
|
if index + 1 >= len(format_chain):
|
||||||
messages=messages,
|
raise
|
||||||
model=model,
|
# Only downgrade when the failure is about ``response_format``.
|
||||||
temperature=temperature,
|
# Everything else (auth, unknown model, rate limits) must
|
||||||
response_format=None,
|
# surface unchanged. Matching on the bare parameter name also
|
||||||
max_tokens=effective_max,
|
# covers variants such as DeepSeek's "This response_format
|
||||||
)
|
# type is unavailable now" without swallowing unrelated 400s.
|
||||||
|
if "response_format" not in message:
|
||||||
|
raise
|
||||||
|
logger.info(
|
||||||
|
"Provider rejected response_format=%s, retrying with %s. "
|
||||||
|
"Error: %s",
|
||||||
|
(fmt or {}).get("type", "none"),
|
||||||
|
(format_chain[index + 1] or {}).get("type", "none"),
|
||||||
|
e,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert result is not None # non-empty chain always sets or raises
|
||||||
|
|
||||||
content = result.get("content", "") or ""
|
content = result.get("content", "") or ""
|
||||||
if not content:
|
if not content:
|
||||||
|
|||||||
@@ -76,6 +76,25 @@ class MockSession:
|
|||||||
pass
|
pass
|
||||||
|
|
||||||
|
|
||||||
|
class RecordingSession:
|
||||||
|
"""Mock session that records each request payload and replays responses."""
|
||||||
|
|
||||||
|
def __init__(self, responses):
|
||||||
|
self._responses = list(responses)
|
||||||
|
self.payloads = []
|
||||||
|
|
||||||
|
def post(self, url, json=None, headers=None):
|
||||||
|
self.payloads.append(json)
|
||||||
|
index = min(len(self.payloads) - 1, len(self._responses) - 1)
|
||||||
|
return self._responses[index]
|
||||||
|
|
||||||
|
async def __aenter__(self):
|
||||||
|
return self
|
||||||
|
|
||||||
|
async def __aexit__(self, *args):
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture
|
@pytest.fixture
|
||||||
def llm_service():
|
def llm_service():
|
||||||
"""Create an LLMService with mock settings."""
|
"""Create an LLMService with mock settings."""
|
||||||
@@ -298,6 +317,107 @@ class TestLLMServiceChatCompletionJson:
|
|||||||
assert result == {"key": "value"}
|
assert result == {"key": "value"}
|
||||||
assert call_index == 2
|
assert call_index == 2
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_chat_completion_json_prefers_json_object_for_deepseek(self):
|
||||||
|
"""DeepSeek rejects json_schema, so json_object is used first.
|
||||||
|
|
||||||
|
Regression: DeepSeek answers json_schema with
|
||||||
|
"This response_format type is unavailable now", which the old
|
||||||
|
substring check did not recognise, so enrichment failed outright.
|
||||||
|
"""
|
||||||
|
settings = MockSettings(
|
||||||
|
llm_enabled=True,
|
||||||
|
llm_provider="deepseek",
|
||||||
|
llm_api_key="sk-test-key",
|
||||||
|
llm_api_base="https://api.deepseek.com/v1",
|
||||||
|
llm_model="deepseek-v4-flash",
|
||||||
|
)
|
||||||
|
service = LLMService(settings)
|
||||||
|
|
||||||
|
session = RecordingSession(
|
||||||
|
[
|
||||||
|
MockResponse(
|
||||||
|
200,
|
||||||
|
json_data={
|
||||||
|
"choices": [{"message": {"content": '{"key": "value"}'}}],
|
||||||
|
"usage": {},
|
||||||
|
},
|
||||||
|
)
|
||||||
|
]
|
||||||
|
)
|
||||||
|
|
||||||
|
with mock.patch("aiohttp.ClientSession", return_value=session):
|
||||||
|
result = await service.chat_completion_json(
|
||||||
|
system_prompt="You are helpful.",
|
||||||
|
user_prompt="Return JSON.",
|
||||||
|
)
|
||||||
|
|
||||||
|
assert result == {"key": "value"}
|
||||||
|
assert len(session.payloads) == 1
|
||||||
|
assert session.payloads[0]["response_format"] == {"type": "json_object"}
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_chat_completion_json_downgrades_from_json_schema(
|
||||||
|
self, llm_service,
|
||||||
|
):
|
||||||
|
"""json_schema → json_object when the provider rejects json_schema."""
|
||||||
|
session = RecordingSession(
|
||||||
|
[
|
||||||
|
MockResponse(
|
||||||
|
400,
|
||||||
|
text_data=(
|
||||||
|
'{"error":{"message":"This response_format type is '
|
||||||
|
'unavailable now","type":"invalid_request_error"}}'
|
||||||
|
),
|
||||||
|
),
|
||||||
|
MockResponse(
|
||||||
|
200,
|
||||||
|
json_data={
|
||||||
|
"choices": [{"message": {"content": '{"key": "value"}'}}],
|
||||||
|
"usage": {},
|
||||||
|
},
|
||||||
|
),
|
||||||
|
]
|
||||||
|
)
|
||||||
|
|
||||||
|
with mock.patch("aiohttp.ClientSession", return_value=session):
|
||||||
|
result = await llm_service.chat_completion_json(
|
||||||
|
system_prompt="You are helpful.",
|
||||||
|
user_prompt="Return JSON.",
|
||||||
|
)
|
||||||
|
|
||||||
|
assert result == {"key": "value"}
|
||||||
|
assert [p.get("response_format") for p in session.payloads] == [
|
||||||
|
{
|
||||||
|
"type": "json_schema",
|
||||||
|
"json_schema": {"name": "metadata", "schema": {"type": "object"}},
|
||||||
|
},
|
||||||
|
{"type": "json_object"},
|
||||||
|
]
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_chat_completion_json_does_not_retry_unrelated_errors(
|
||||||
|
self, llm_service,
|
||||||
|
):
|
||||||
|
"""Unrelated 400s are surfaced unchanged, without format downgrades."""
|
||||||
|
session = RecordingSession(
|
||||||
|
[
|
||||||
|
MockResponse(
|
||||||
|
400,
|
||||||
|
text_data='{"error":{"message":"Model not found"}}',
|
||||||
|
)
|
||||||
|
]
|
||||||
|
)
|
||||||
|
|
||||||
|
with mock.patch("aiohttp.ClientSession", return_value=session):
|
||||||
|
with pytest.raises(LLMResponseError, match="HTTP 400"):
|
||||||
|
await llm_service.chat_completion_json(
|
||||||
|
system_prompt="You are helpful.",
|
||||||
|
user_prompt="Return JSON.",
|
||||||
|
)
|
||||||
|
|
||||||
|
assert len(session.payloads) == 1
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_chat_completion_json_raises_on_non_json(self, llm_service):
|
async def test_chat_completion_json_raises_on_non_json(self, llm_service):
|
||||||
# Non-JSON content raises LLMResponseError (salvage also fails)
|
# Non-JSON content raises LLMResponseError (salvage also fails)
|
||||||
|
|||||||
Reference in New Issue
Block a user