From 278bf94d19fef131d34914a8517b36fa1b6ca117 Mon Sep 17 00:00:00 2001 From: zhouyanjue Date: Wed, 13 May 2026 12:52:54 +0800 Subject: [PATCH 1/7] fix(llm): improve rag demo error handling Change-Id: I1658c1a7aefe353f48be4d48c7d4425aded13a3d --- .../src/hugegraph_llm/demo/rag_demo/app.py | 2 +- .../hugegraph_llm/models/embeddings/ollama.py | 13 ++++---- .../src/hugegraph_llm/models/llms/litellm.py | 4 +-- .../src/hugegraph_llm/models/llms/openai.py | 18 +++++++---- .../src/hugegraph_llm/nodes/base_node.py | 8 ++++- .../nodes/llm_node/schema_build.py | 3 +- .../hugegraph_op/fetch_graph_data.py | 32 +++++++++---------- .../operators/hugegraph_op/schema_manager.py | 8 +++-- .../src/pyhugegraph/utils/util.py | 10 ++++-- 9 files changed, 59 insertions(+), 39 deletions(-) diff --git a/hugegraph-llm/src/hugegraph_llm/demo/rag_demo/app.py b/hugegraph-llm/src/hugegraph_llm/demo/rag_demo/app.py index 34cd07110..c21959c6d 100644 --- a/hugegraph-llm/src/hugegraph_llm/demo/rag_demo/app.py +++ b/hugegraph-llm/src/hugegraph_llm/demo/rag_demo/app.py @@ -202,5 +202,5 @@ def create_app(): host=args.host, port=args.port, factory=True, - reload=True, + reload=False, ) diff --git a/hugegraph-llm/src/hugegraph_llm/models/embeddings/ollama.py b/hugegraph-llm/src/hugegraph_llm/models/embeddings/ollama.py index 195ea6f6f..40da805d1 100644 --- a/hugegraph-llm/src/hugegraph_llm/models/embeddings/ollama.py +++ b/hugegraph-llm/src/hugegraph_llm/models/embeddings/ollama.py @@ -81,14 +81,13 @@ def get_texts_embeddings(self, texts: List[str], batch_size: int = 32) -> List[L async def async_get_text_embedding(self, text: str) -> List[float]: """Get embedding for a single text asynchronously.""" - response = await self.async_client.embeddings(model=self.model, prompt=text) - return list(response["embedding"]) + response = await self.async_client.embed(model=self.model, input=[text]) + return list(response["embeddings"][0]) async def async_get_texts_embeddings(self, texts: List[str], batch_size: int = 32) -> List[List[float]]: - # Ollama python client may not provide batch async embeddings; fallback per item - # batch_size parameter included for consistency with base class signature results: List[List[float]] = [] - for t in texts: - response = await self.async_client.embeddings(model=self.model, prompt=t) - results.append(list(response["embedding"])) + for i in range(0, len(texts), batch_size): + batch = texts[i : i + batch_size] + response = await self.async_client.embed(model=self.model, input=batch) + results.extend([list(v) for v in response["embeddings"]]) return results diff --git a/hugegraph-llm/src/hugegraph_llm/models/llms/litellm.py b/hugegraph-llm/src/hugegraph_llm/models/llms/litellm.py index dcf479c2f..74ed12e01 100644 --- a/hugegraph-llm/src/hugegraph_llm/models/llms/litellm.py +++ b/hugegraph-llm/src/hugegraph_llm/models/llms/litellm.py @@ -75,7 +75,7 @@ def generate( return response.choices[0].message.content except (RateLimitError, BudgetExceededError, APIError) as e: log.error("Error in LiteLLM call: %s", e) - return f"Error: {str(e)}" + raise @retry( stop=stop_after_attempt(2), @@ -104,7 +104,7 @@ async def agenerate( return response.choices[0].message.content except (RateLimitError, BudgetExceededError, APIError) as e: log.error("Error in async LiteLLM call: %s", e) - return f"Error: {str(e)}" + raise def generate_streaming( self, diff --git a/hugegraph-llm/src/hugegraph_llm/models/llms/openai.py b/hugegraph-llm/src/hugegraph_llm/models/llms/openai.py index f7a6d3f9c..089c356cf 100644 --- a/hugegraph-llm/src/hugegraph_llm/models/llms/openai.py +++ b/hugegraph-llm/src/hugegraph_llm/models/llms/openai.py @@ -70,16 +70,19 @@ def generate( max_tokens=self.max_tokens, messages=messages, ) - log.info("Token usage: %s", completions.usage.model_dump_json()) + if not hasattr(completions, 'choices'): + raise RuntimeError(f"Unexpected LLM response: {type(completions).__name__}: {str(completions)[:200]}") + if completions.usage: + log.info("Token usage: %s", completions.usage.model_dump_json()) return completions.choices[0].message.content # catch context length / do not retry except openai.BadRequestError as e: log.critical("Fatal: %s", e) - return str(f"Error: {e}") + raise # catch authorization errors / do not retry except openai.AuthenticationError: log.critical("The provided OpenAI API key is invalid") - return "Error: The provided OpenAI API key is invalid" + raise except Exception as e: log.error("Retrying LLM call %s", e) raise e @@ -105,16 +108,19 @@ async def agenerate( max_tokens=self.max_tokens, messages=messages, ) - log.info("Token usage: %s", completions.usage.model_dump_json()) + if not hasattr(completions, 'choices'): + raise RuntimeError(f"Unexpected LLM response: {type(completions).__name__}: {str(completions)[:200]}") + if completions.usage: + log.info("Token usage: %s", completions.usage.model_dump_json()) return completions.choices[0].message.content # catch context length / do not retry except openai.BadRequestError as e: log.critical("Fatal: %s", e) - return str(f"Error: {e}") + raise # catch authorization errors / do not retry except openai.AuthenticationError: log.critical("The provided OpenAI API key is invalid") - return "Error: The provided OpenAI API key is invalid" + raise except Exception as e: log.error("Retrying LLM call %s", e) raise e diff --git a/hugegraph-llm/src/hugegraph_llm/nodes/base_node.py b/hugegraph-llm/src/hugegraph_llm/nodes/base_node.py index 7ed8af8a7..e65e607d9 100644 --- a/hugegraph-llm/src/hugegraph_llm/nodes/base_node.py +++ b/hugegraph-llm/src/hugegraph_llm/nodes/base_node.py @@ -75,7 +75,13 @@ def run(self): node_info = f"Node type: {type(self).__name__}, Node object: {self}" err_msg = f"Node failed: {exc}\n{node_info}\n{traceback.format_exc()}" return CStatus(-1, err_msg) - # For unexpected exceptions, re-raise to let them propagate or be caught elsewhere + except Exception as exc: # pylint: disable=broad-exception-caught + import traceback + + node_info = f"Node type: {type(self).__name__}, Node object: {self}" + err_msg = f"Node unexpected error: {exc}\n{node_info}\n{traceback.format_exc()}" + log.error(err_msg) + return CStatus(-1, err_msg) self.context.lock() try: diff --git a/hugegraph-llm/src/hugegraph_llm/nodes/llm_node/schema_build.py b/hugegraph-llm/src/hugegraph_llm/nodes/llm_node/schema_build.py index 01b2ca64d..e40d69c90 100644 --- a/hugegraph-llm/src/hugegraph_llm/nodes/llm_node/schema_build.py +++ b/hugegraph-llm/src/hugegraph_llm/nodes/llm_node/schema_build.py @@ -81,8 +81,7 @@ def node_init(self): def operator_schedule(self, data_json): try: schema_result = self.schema_builder.run(data_json) - return {"schema": schema_result} except (ValueError, RuntimeError) as e: log.error("Failed to generate schema: %s", e) - return {"schema": f"Schema generation failed: {e}"} + raise ValueError(f"Failed to generate schema: {e}") from e diff --git a/hugegraph-llm/src/hugegraph_llm/operators/hugegraph_op/fetch_graph_data.py b/hugegraph-llm/src/hugegraph_llm/operators/hugegraph_op/fetch_graph_data.py index c3f427e93..0012e39bb 100644 --- a/hugegraph-llm/src/hugegraph_llm/operators/hugegraph_op/fetch_graph_data.py +++ b/hugegraph-llm/src/hugegraph_llm/operators/hugegraph_op/fetch_graph_data.py @@ -32,20 +32,20 @@ def run(self, graph_summary: Optional[Dict[str, Any]]) -> Dict[str, Any]: # TODO: v_limit will influence the vid embedding logic in build_semantic_index.py v_limit = 10000 e_limit = 200 - keys = ["vertex_num", "edge_num", "vertices", "edges", "note"] - - groovy_code = f""" - def res = [:]; - res.{keys[0]} = g.V().count().next(); - res.{keys[1]} = g.E().count().next(); - res.{keys[2]} = g.V().id().limit({v_limit}).toList(); - res.{keys[3]} = g.E().id().limit({e_limit}).toList(); - res.{keys[4]} = "Only ≤{v_limit} VIDs and ≤ {e_limit} EIDs for brief overview ."; - return res; - """ - - result = self.graph.gremlin().exec(groovy_code)["data"] - - if isinstance(result, list) and len(result) > 0: - graph_summary.update({key: result[i].get(key) for i, key in enumerate(keys)}) + + graph_api = self.graph.graph() + + vertices = graph_api.getVertexByCondition(limit=v_limit) or [] + edges = graph_api.getEdgeByPage(limit=e_limit)[0] or [] + + vertex_ids = [str(v.id) for v in vertices] + edge_ids = [str(e.id) for e in edges] + + graph_summary.update({ + "vertex_num": len(vertex_ids), + "edge_num": len(edge_ids), + "vertices": vertex_ids, + "edges": edge_ids, + "note": f"Only <={v_limit} VIDs and <={e_limit} EIDs for brief overview .", + }) return graph_summary diff --git a/hugegraph-llm/src/hugegraph_llm/operators/hugegraph_op/schema_manager.py b/hugegraph-llm/src/hugegraph_llm/operators/hugegraph_op/schema_manager.py index c265646fa..1969149b4 100644 --- a/hugegraph-llm/src/hugegraph_llm/operators/hugegraph_op/schema_manager.py +++ b/hugegraph-llm/src/hugegraph_llm/operators/hugegraph_op/schema_manager.py @@ -17,6 +17,7 @@ from typing import Any, Dict, Optional from pyhugegraph.client import PyHugeClient +from requests.exceptions import RequestException from hugegraph_llm.config import huge_settings @@ -57,9 +58,12 @@ def simple_schema(self, schema: Dict[str, Any]) -> Dict[str, Any]: def run(self, context: Optional[Dict[str, Any]]) -> Dict[str, Any]: if context is None: context = {} - schema = self.schema.getSchema() + try: + schema = self.schema.getSchema() + except RequestException as e: + raise ValueError(f"Failed to connect HugeGraph to get schema '{self.graph_name}': {e}") from e if not schema["vertexlabels"] and not schema["edgelabels"]: - raise Exception(f"Can not get {self.graph_name}'s schema from HugeGraph!") + raise ValueError(f"Can not get {self.graph_name}'s schema from HugeGraph!") context.update({"schema": schema}) # TODO: enhance the logic here diff --git a/hugegraph-python-client/src/pyhugegraph/utils/util.py b/hugegraph-python-client/src/pyhugegraph/utils/util.py index d8c833b49..2f204c788 100644 --- a/hugegraph-python-client/src/pyhugegraph/utils/util.py +++ b/hugegraph-python-client/src/pyhugegraph/utils/util.py @@ -101,9 +101,15 @@ def __call__(self, response: requests.Response, method: str, path: str): log.info("Resource %s not found (404)", path) else: try: - details = response.json().get("exception", "key 'exception' not found") + body = response.json() + details = ( + body.get("exception") + or body.get("status", {}).get("message") + or response.text + or "unknown error" + ) except (ValueError, KeyError): - details = "key 'exception' not found" + details = response.text or "unknown error" req_body = response.request.body if response.request.body else "Empty body" req_body = req_body.encode("utf-8").decode("unicode_escape") From 334711c32d47ac70921d2df2f1a4509050251d98 Mon Sep 17 00:00:00 2001 From: zhouyanjue Date: Tue, 19 May 2026 13:46:25 +0800 Subject: [PATCH 2/7] fix(llm): restore ci error handling contracts Change-Id: I275a618f6b6a43448c40d91fc6e4667084d26f4f --- .../src/hugegraph_llm/models/llms/openai.py | 12 ++--- .../hugegraph_op/fetch_graph_data.py | 44 ++++++++++++------- 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/hugegraph-llm/src/hugegraph_llm/models/llms/openai.py b/hugegraph-llm/src/hugegraph_llm/models/llms/openai.py index 089c356cf..86ad340af 100644 --- a/hugegraph-llm/src/hugegraph_llm/models/llms/openai.py +++ b/hugegraph-llm/src/hugegraph_llm/models/llms/openai.py @@ -70,7 +70,7 @@ def generate( max_tokens=self.max_tokens, messages=messages, ) - if not hasattr(completions, 'choices'): + if not hasattr(completions, "choices"): raise RuntimeError(f"Unexpected LLM response: {type(completions).__name__}: {str(completions)[:200]}") if completions.usage: log.info("Token usage: %s", completions.usage.model_dump_json()) @@ -78,11 +78,11 @@ def generate( # catch context length / do not retry except openai.BadRequestError as e: log.critical("Fatal: %s", e) - raise + return str(f"Error: {e}") # catch authorization errors / do not retry except openai.AuthenticationError: log.critical("The provided OpenAI API key is invalid") - raise + return "Error: The provided OpenAI API key is invalid" except Exception as e: log.error("Retrying LLM call %s", e) raise e @@ -108,7 +108,7 @@ async def agenerate( max_tokens=self.max_tokens, messages=messages, ) - if not hasattr(completions, 'choices'): + if not hasattr(completions, "choices"): raise RuntimeError(f"Unexpected LLM response: {type(completions).__name__}: {str(completions)[:200]}") if completions.usage: log.info("Token usage: %s", completions.usage.model_dump_json()) @@ -116,11 +116,11 @@ async def agenerate( # catch context length / do not retry except openai.BadRequestError as e: log.critical("Fatal: %s", e) - raise + return str(f"Error: {e}") # catch authorization errors / do not retry except openai.AuthenticationError: log.critical("The provided OpenAI API key is invalid") - raise + return "Error: The provided OpenAI API key is invalid" except Exception as e: log.error("Retrying LLM call %s", e) raise e diff --git a/hugegraph-llm/src/hugegraph_llm/operators/hugegraph_op/fetch_graph_data.py b/hugegraph-llm/src/hugegraph_llm/operators/hugegraph_op/fetch_graph_data.py index 0012e39bb..e75f5bb6c 100644 --- a/hugegraph-llm/src/hugegraph_llm/operators/hugegraph_op/fetch_graph_data.py +++ b/hugegraph-llm/src/hugegraph_llm/operators/hugegraph_op/fetch_graph_data.py @@ -20,6 +20,8 @@ from pyhugegraph.client import PyHugeClient +from hugegraph_llm.utils.log import log + class FetchGraphData: def __init__(self, graph: PyHugeClient): @@ -32,20 +34,30 @@ def run(self, graph_summary: Optional[Dict[str, Any]]) -> Dict[str, Any]: # TODO: v_limit will influence the vid embedding logic in build_semantic_index.py v_limit = 10000 e_limit = 200 - - graph_api = self.graph.graph() - - vertices = graph_api.getVertexByCondition(limit=v_limit) or [] - edges = graph_api.getEdgeByPage(limit=e_limit)[0] or [] - - vertex_ids = [str(v.id) for v in vertices] - edge_ids = [str(e.id) for e in edges] - - graph_summary.update({ - "vertex_num": len(vertex_ids), - "edge_num": len(edge_ids), - "vertices": vertex_ids, - "edges": edge_ids, - "note": f"Only <={v_limit} VIDs and <={e_limit} EIDs for brief overview .", - }) + keys = ["vertex_num", "edge_num", "vertices", "edges", "note"] + + groovy_code = f""" + def res = [:]; + res.{keys[0]} = g.V().count().next(); + res.{keys[1]} = g.E().count().next(); + res.{keys[2]} = g.V().id().limit({v_limit}).toList(); + res.{keys[3]} = g.E().id().limit({e_limit}).toList(); + res.{keys[4]} = "Only ≤{v_limit} VIDs and ≤ {e_limit} EIDs for brief overview ."; + return res; + """ + + try: + response = self.graph.gremlin().exec(groovy_code) + except Exception as e: + log.warning("Failed to fetch graph summary: %s", e) + return graph_summary + + result = response.get("data") if isinstance(response, dict) else None + if isinstance(result, list) and len(result) > 0: + graph_summary.update( + { + key: result[i].get(key) if i < len(result) and isinstance(result[i], dict) else None + for i, key in enumerate(keys) + } + ) return graph_summary From f558261507c52e95edcb3c87eacd35d109cd4a1d Mon Sep 17 00:00:00 2001 From: zhouyanjue Date: Tue, 19 May 2026 21:43:11 +0800 Subject: [PATCH 3/7] fix(llm): handle graph summary response shapes Change-Id: I65b37b72baea21e2cf2cf2c471ff14c56869520e --- .../hugegraph_op/fetch_graph_data.py | 24 ++++---- .../hugegraph_op/test_fetch_graph_data.py | 56 +++++++++++++++++++ .../src/pyhugegraph/utils/util.py | 5 +- .../src/tests/api/test_response_validation.py | 38 +++++++++++++ 4 files changed, 108 insertions(+), 15 deletions(-) create mode 100644 hugegraph-python-client/src/tests/api/test_response_validation.py diff --git a/hugegraph-llm/src/hugegraph_llm/operators/hugegraph_op/fetch_graph_data.py b/hugegraph-llm/src/hugegraph_llm/operators/hugegraph_op/fetch_graph_data.py index e75f5bb6c..729890f83 100644 --- a/hugegraph-llm/src/hugegraph_llm/operators/hugegraph_op/fetch_graph_data.py +++ b/hugegraph-llm/src/hugegraph_llm/operators/hugegraph_op/fetch_graph_data.py @@ -20,8 +20,6 @@ from pyhugegraph.client import PyHugeClient -from hugegraph_llm.utils.log import log - class FetchGraphData: def __init__(self, graph: PyHugeClient): @@ -46,18 +44,16 @@ def res = [:]; return res; """ - try: - response = self.graph.gremlin().exec(groovy_code) - except Exception as e: - log.warning("Failed to fetch graph summary: %s", e) - return graph_summary - + response = self.graph.gremlin().exec(groovy_code) result = response.get("data") if isinstance(response, dict) else None if isinstance(result, list) and len(result) > 0: - graph_summary.update( - { - key: result[i].get(key) if i < len(result) and isinstance(result[i], dict) else None - for i, key in enumerate(keys) - } - ) + if len(result) == 1 and isinstance(result[0], dict) and any(key in result[0] for key in keys): + graph_summary.update({key: result[0].get(key) for key in keys}) + else: + graph_summary.update( + { + key: result[i].get(key) if i < len(result) and isinstance(result[i], dict) else None + for i, key in enumerate(keys) + } + ) return graph_summary diff --git a/hugegraph-llm/src/tests/operators/hugegraph_op/test_fetch_graph_data.py b/hugegraph-llm/src/tests/operators/hugegraph_op/test_fetch_graph_data.py index 64c093eda..1f26beaf4 100644 --- a/hugegraph-llm/src/tests/operators/hugegraph_op/test_fetch_graph_data.py +++ b/hugegraph-llm/src/tests/operators/hugegraph_op/test_fetch_graph_data.py @@ -97,6 +97,62 @@ def test_run_with_existing_graph_summary(self): self.assertEqual(result["edges"], ["e1", "e2"]) self.assertIn("note", result) + def test_run_with_single_summary_dict_result(self): + """Test run method with Gremlin map result wrapped as one data row.""" + # Setup mock + self.mock_gremlin.exec.return_value = { + "data": [ + { + "vertex_num": 100, + "edge_num": 200, + "vertices": ["v1", "v2", "v3"], + "edges": ["e1", "e2"], + "note": "Only ≤10000 VIDs and ≤ 200 EIDs for brief overview .", + } + ] + } + + # Call the method + result = self.fetcher.run({}) + + # Verify the result + self.assertEqual(result["vertex_num"], 100) + self.assertEqual(result["edge_num"], 200) + self.assertEqual(result["vertices"], ["v1", "v2", "v3"]) + self.assertEqual(result["edges"], ["e1", "e2"]) + self.assertEqual(result["note"], "Only ≤10000 VIDs and ≤ 200 EIDs for brief overview .") + + def test_run_with_partial_single_summary_dict_result(self): + """Test run method handles a single Gremlin map with missing summary fields.""" + # Setup mock + self.mock_gremlin.exec.return_value = { + "data": [ + { + "vertex_num": 100, + "vertices": ["v1", "v2", "v3"], + } + ] + } + + # Call the method + result = self.fetcher.run({}) + + # Verify the result + self.assertEqual(result["vertex_num"], 100) + self.assertIsNone(result["edge_num"]) + self.assertEqual(result["vertices"], ["v1", "v2", "v3"]) + self.assertIsNone(result["edges"]) + self.assertIsNone(result["note"]) + + def test_run_reraises_gremlin_exec_exception(self): + """Test run method does not hide Gremlin execution failures.""" + # Setup mock + self.mock_gremlin.exec.side_effect = RuntimeError("Gremlin endpoint unavailable") + + # Call the method and verify the original failure is visible + with self.assertRaisesRegex(RuntimeError, "Gremlin endpoint unavailable"): + self.fetcher.run({}) + def test_run_with_empty_result(self): """Test run method with empty result from gremlin.""" # Setup mock diff --git a/hugegraph-python-client/src/pyhugegraph/utils/util.py b/hugegraph-python-client/src/pyhugegraph/utils/util.py index 2f204c788..65017f52c 100644 --- a/hugegraph-python-client/src/pyhugegraph/utils/util.py +++ b/hugegraph-python-client/src/pyhugegraph/utils/util.py @@ -102,9 +102,12 @@ def __call__(self, response: requests.Response, method: str, path: str): else: try: body = response.json() + status = body.get("status") + status_message = status.get("message") if isinstance(status, dict) else None details = ( body.get("exception") - or body.get("status", {}).get("message") + or status_message + or body.get("message") or response.text or "unknown error" ) diff --git a/hugegraph-python-client/src/tests/api/test_response_validation.py b/hugegraph-python-client/src/tests/api/test_response_validation.py new file mode 100644 index 000000000..0504b7018 --- /dev/null +++ b/hugegraph-python-client/src/tests/api/test_response_validation.py @@ -0,0 +1,38 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from unittest.mock import Mock + +import pytest +import requests +from pyhugegraph.utils.util import ResponseValidation + + +def test_response_validation_raises_http_error_with_numeric_status_body(): + response = Mock(spec=requests.Response) + response.status_code = 400 + response.text = '{"status":400,"message":"bad gremlin"}' + response.content = response.text.encode("utf-8") + response.json.return_value = {"status": 400, "message": "bad gremlin"} + response.request = Mock() + response.request.body = "g.V2()" + response.raise_for_status.side_effect = requests.exceptions.HTTPError("400 Client Error") + + validator = ResponseValidation() + + with pytest.raises(Exception, match="bad gremlin"): + validator(response, "POST", "/gremlin") From ddcc96f8c073b4f3bdc08746d54a6b161fb512b1 Mon Sep 17 00:00:00 2001 From: Makoto <2762006003@qq.com> Date: Wed, 20 May 2026 13:02:31 +0800 Subject: [PATCH 4/7] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../src/pyhugegraph/utils/util.py | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/hugegraph-python-client/src/pyhugegraph/utils/util.py b/hugegraph-python-client/src/pyhugegraph/utils/util.py index 65017f52c..83c8ab18f 100644 --- a/hugegraph-python-client/src/pyhugegraph/utils/util.py +++ b/hugegraph-python-client/src/pyhugegraph/utils/util.py @@ -102,16 +102,19 @@ def __call__(self, response: requests.Response, method: str, path: str): else: try: body = response.json() - status = body.get("status") - status_message = status.get("message") if isinstance(status, dict) else None - details = ( - body.get("exception") - or status_message - or body.get("message") - or response.text - or "unknown error" - ) - except (ValueError, KeyError): + if isinstance(body, dict): + status = body.get("status") + status_message = status.get("message") if isinstance(status, dict) else None + details = ( + body.get("exception") + or status_message + or body.get("message") + or response.text + or "unknown error" + ) + else: + details = response.text or "unknown error" + except (ValueError, KeyError, TypeError, AttributeError): details = response.text or "unknown error" req_body = response.request.body if response.request.body else "Empty body" From 22ec7e418f5cfd8e151cd7aa7ea19fac24a02f98 Mon Sep 17 00:00:00 2001 From: zhouyanjue Date: Wed, 20 May 2026 13:38:08 +0800 Subject: [PATCH 5/7] fix(llm): address review error contracts Change-Id: I939ca3a2df3ecc55471f9b98155b5ab03fa43d03 --- .../src/hugegraph_llm/demo/rag_demo/app.py | 3 +- .../hugegraph_llm/models/embeddings/ollama.py | 26 +++++- .../src/hugegraph_llm/models/llms/litellm.py | 10 ++- .../src/hugegraph_llm/models/llms/openai.py | 8 +- .../src/hugegraph_llm/nodes/base_node.py | 18 ++-- .../hugegraph_op/fetch_graph_data.py | 2 +- .../operators/hugegraph_op/schema_manager.py | 4 +- .../embeddings/test_ollama_embedding.py | 46 ++++++++++ .../tests/models/llms/test_litellm_client.py | 90 +++++++++++++++++++ .../tests/models/llms/test_openai_client.py | 37 ++++++++ .../hugegraph_op/test_fetch_graph_data.py | 30 +++++++ .../src/pyhugegraph/utils/util.py | 4 +- .../src/tests/api/test_response_validation.py | 44 ++++++--- 13 files changed, 283 insertions(+), 39 deletions(-) create mode 100644 hugegraph-llm/src/tests/models/llms/test_litellm_client.py diff --git a/hugegraph-llm/src/hugegraph_llm/demo/rag_demo/app.py b/hugegraph-llm/src/hugegraph_llm/demo/rag_demo/app.py index c21959c6d..c687df6d6 100644 --- a/hugegraph-llm/src/hugegraph_llm/demo/rag_demo/app.py +++ b/hugegraph-llm/src/hugegraph_llm/demo/rag_demo/app.py @@ -16,6 +16,7 @@ # under the License. import argparse +import os import gradio as gr import uvicorn @@ -202,5 +203,5 @@ def create_app(): host=args.host, port=args.port, factory=True, - reload=False, + reload=os.getenv("HG_DEV_RELOAD") == "1", ) diff --git a/hugegraph-llm/src/hugegraph_llm/models/embeddings/ollama.py b/hugegraph-llm/src/hugegraph_llm/models/embeddings/ollama.py index 40da805d1..088c084c1 100644 --- a/hugegraph-llm/src/hugegraph_llm/models/embeddings/ollama.py +++ b/hugegraph-llm/src/hugegraph_llm/models/embeddings/ollama.py @@ -79,15 +79,37 @@ def get_texts_embeddings(self, texts: List[str], batch_size: int = 32) -> List[L all_embeddings.extend([list(inner_sequence) for inner_sequence in response]) return all_embeddings + def _get_embeddings_from_response(self, response) -> List[List[float]]: + if "embeddings" not in response: + raise ValueError("Ollama embedding response missing 'embeddings'.") + embeddings = response["embeddings"] + if not embeddings: + raise ValueError("Ollama embedding response returned no embeddings.") + return [list(inner_sequence) for inner_sequence in embeddings] + async def async_get_text_embedding(self, text: str) -> List[float]: """Get embedding for a single text asynchronously.""" + if not hasattr(self.async_client, "embed"): + error_message = ( + "The required 'embed' method was not found on the Ollama async client. " + "Please ensure your ollama library is up-to-date and supports batch embedding. " + ) + raise AttributeError(error_message) + response = await self.async_client.embed(model=self.model, input=[text]) - return list(response["embeddings"][0]) + return self._get_embeddings_from_response(response)[0] async def async_get_texts_embeddings(self, texts: List[str], batch_size: int = 32) -> List[List[float]]: + if not hasattr(self.async_client, "embed"): + error_message = ( + "The required 'embed' method was not found on the Ollama async client. " + "Please ensure your ollama library is up-to-date and supports batch embedding. " + ) + raise AttributeError(error_message) + results: List[List[float]] = [] for i in range(0, len(texts), batch_size): batch = texts[i : i + batch_size] response = await self.async_client.embed(model=self.model, input=batch) - results.extend([list(v) for v in response["embeddings"]]) + results.extend(self._get_embeddings_from_response(response)) return results diff --git a/hugegraph-llm/src/hugegraph_llm/models/llms/litellm.py b/hugegraph-llm/src/hugegraph_llm/models/llms/litellm.py index 74ed12e01..60ad481ff 100644 --- a/hugegraph-llm/src/hugegraph_llm/models/llms/litellm.py +++ b/hugegraph-llm/src/hugegraph_llm/models/llms/litellm.py @@ -51,7 +51,8 @@ def __init__( @retry( stop=stop_after_attempt(2), wait=wait_exponential(multiplier=1, min=2, max=5), - retry=retry_if_exception_type((RateLimitError, BudgetExceededError, APIError)), + retry=retry_if_exception_type((RateLimitError, APIError)), + reraise=True, ) def generate( self, @@ -80,7 +81,8 @@ def generate( @retry( stop=stop_after_attempt(2), wait=wait_exponential(multiplier=1, min=2, max=5), - retry=retry_if_exception_type((RateLimitError, BudgetExceededError, APIError)), + retry=retry_if_exception_type((RateLimitError, APIError)), + reraise=True, ) async def agenerate( self, @@ -138,7 +140,7 @@ def generate_streaming( return result except (RateLimitError, BudgetExceededError, APIError) as e: log.error("Error in streaming LiteLLM call: %s", e) - return f"Error: {str(e)}" + raise async def agenerate_streaming( self, @@ -170,7 +172,7 @@ async def agenerate_streaming( yield chunk.choices[0].delta.content except (RateLimitError, BudgetExceededError, APIError) as e: log.error("Error in async streaming LiteLLM call: %s", e) - yield f"Error: {str(e)}" + raise def num_tokens_from_string(self, string: str) -> int: """Get token count from string.""" diff --git a/hugegraph-llm/src/hugegraph_llm/models/llms/openai.py b/hugegraph-llm/src/hugegraph_llm/models/llms/openai.py index 86ad340af..3370d47d0 100644 --- a/hugegraph-llm/src/hugegraph_llm/models/llms/openai.py +++ b/hugegraph-llm/src/hugegraph_llm/models/llms/openai.py @@ -70,8 +70,8 @@ def generate( max_tokens=self.max_tokens, messages=messages, ) - if not hasattr(completions, "choices"): - raise RuntimeError(f"Unexpected LLM response: {type(completions).__name__}: {str(completions)[:200]}") + if not completions.choices: + raise RuntimeError(f"Empty choices in LLM response: {str(completions)[:200]}") if completions.usage: log.info("Token usage: %s", completions.usage.model_dump_json()) return completions.choices[0].message.content @@ -108,8 +108,8 @@ async def agenerate( max_tokens=self.max_tokens, messages=messages, ) - if not hasattr(completions, "choices"): - raise RuntimeError(f"Unexpected LLM response: {type(completions).__name__}: {str(completions)[:200]}") + if not completions.choices: + raise RuntimeError(f"Empty choices in LLM response: {str(completions)[:200]}") if completions.usage: log.info("Token usage: %s", completions.usage.model_dump_json()) return completions.choices[0].message.content diff --git a/hugegraph-llm/src/hugegraph_llm/nodes/base_node.py b/hugegraph-llm/src/hugegraph_llm/nodes/base_node.py index e65e607d9..4da43e902 100644 --- a/hugegraph-llm/src/hugegraph_llm/nodes/base_node.py +++ b/hugegraph-llm/src/hugegraph_llm/nodes/base_node.py @@ -13,6 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import traceback from typing import Dict, Optional from pycgraph import CStatus, GNode @@ -22,6 +23,11 @@ from hugegraph_llm.utils.log import log +def _format_node_err(node, exc: Exception, prefix: str = "Node failed") -> str: + node_info = f"Node type: {type(node).__name__}, Node object: {node}" + return f"{prefix}: {exc}\n{node_info}\n{traceback.format_exc()}" + + class BaseNode(GNode): """ Base class for workflow nodes, providing context management and operation scheduling. @@ -70,18 +76,10 @@ def run(self): try: res = self.operator_schedule(data_json) except (ValueError, TypeError, KeyError, NotImplementedError) as exc: - import traceback - - node_info = f"Node type: {type(self).__name__}, Node object: {self}" - err_msg = f"Node failed: {exc}\n{node_info}\n{traceback.format_exc()}" - return CStatus(-1, err_msg) - except Exception as exc: # pylint: disable=broad-exception-caught - import traceback - - node_info = f"Node type: {type(self).__name__}, Node object: {self}" - err_msg = f"Node unexpected error: {exc}\n{node_info}\n{traceback.format_exc()}" + err_msg = _format_node_err(self, exc) log.error(err_msg) return CStatus(-1, err_msg) + # For unexpected exceptions, re-raise to let them propagate or be caught elsewhere self.context.lock() try: diff --git a/hugegraph-llm/src/hugegraph_llm/operators/hugegraph_op/fetch_graph_data.py b/hugegraph-llm/src/hugegraph_llm/operators/hugegraph_op/fetch_graph_data.py index 729890f83..8916356ae 100644 --- a/hugegraph-llm/src/hugegraph_llm/operators/hugegraph_op/fetch_graph_data.py +++ b/hugegraph-llm/src/hugegraph_llm/operators/hugegraph_op/fetch_graph_data.py @@ -47,7 +47,7 @@ def res = [:]; response = self.graph.gremlin().exec(groovy_code) result = response.get("data") if isinstance(response, dict) else None if isinstance(result, list) and len(result) > 0: - if len(result) == 1 and isinstance(result[0], dict) and any(key in result[0] for key in keys): + if len(result) == 1 and isinstance(result[0], dict): graph_summary.update({key: result[0].get(key) for key in keys}) else: graph_summary.update( diff --git a/hugegraph-llm/src/hugegraph_llm/operators/hugegraph_op/schema_manager.py b/hugegraph-llm/src/hugegraph_llm/operators/hugegraph_op/schema_manager.py index 1969149b4..c51cb9587 100644 --- a/hugegraph-llm/src/hugegraph_llm/operators/hugegraph_op/schema_manager.py +++ b/hugegraph-llm/src/hugegraph_llm/operators/hugegraph_op/schema_manager.py @@ -61,9 +61,9 @@ def run(self, context: Optional[Dict[str, Any]]) -> Dict[str, Any]: try: schema = self.schema.getSchema() except RequestException as e: - raise ValueError(f"Failed to connect HugeGraph to get schema '{self.graph_name}': {e}") from e + raise ValueError(f"Failed to connect to HugeGraph to get schema '{self.graph_name}': {e}") from e if not schema["vertexlabels"] and not schema["edgelabels"]: - raise ValueError(f"Can not get {self.graph_name}'s schema from HugeGraph!") + raise ValueError(f"Cannot get {self.graph_name}'s schema from HugeGraph!") context.update({"schema": schema}) # TODO: enhance the logic here diff --git a/hugegraph-llm/src/tests/models/embeddings/test_ollama_embedding.py b/hugegraph-llm/src/tests/models/embeddings/test_ollama_embedding.py index c919a2d65..aa9b8b712 100644 --- a/hugegraph-llm/src/tests/models/embeddings/test_ollama_embedding.py +++ b/hugegraph-llm/src/tests/models/embeddings/test_ollama_embedding.py @@ -18,6 +18,7 @@ import os import unittest +from unittest.mock import AsyncMock from hugegraph_llm.models.embeddings.base import SimilarityMode from hugegraph_llm.models.embeddings.ollama import OllamaEmbedding @@ -40,3 +41,48 @@ def test_get_cosine_similarity(self): embedding2 = ollama_embedding.get_text_embedding("bye world") similarity = OllamaEmbedding.similarity(embedding1, embedding2, SimilarityMode.DEFAULT) print(similarity) + + def test_async_get_texts_embeddings_preserves_batch_order(self): + ollama_embedding = OllamaEmbedding(model="test-model") + ollama_embedding.async_client = AsyncMock() + ollama_embedding.async_client.embed.side_effect = [ + {"embeddings": [[1.0], [2.0]]}, + {"embeddings": [[3.0]]}, + ] + + async def run_async_test(): + result = await ollama_embedding.async_get_texts_embeddings(["a", "b", "c"], batch_size=2) + self.assertEqual(result, [[1.0], [2.0], [3.0]]) + self.assertEqual(ollama_embedding.async_client.embed.call_count, 2) + ollama_embedding.async_client.embed.assert_any_call(model="test-model", input=["a", "b"]) + ollama_embedding.async_client.embed.assert_any_call(model="test-model", input=["c"]) + + import asyncio + + asyncio.run(run_async_test()) + + def test_async_get_text_embedding_requires_embeddings_key(self): + ollama_embedding = OllamaEmbedding(model="test-model") + ollama_embedding.async_client = AsyncMock() + ollama_embedding.async_client.embed.return_value = {} + + async def run_async_test(): + with self.assertRaisesRegex(ValueError, "missing 'embeddings'"): + await ollama_embedding.async_get_text_embedding("a") + + import asyncio + + asyncio.run(run_async_test()) + + def test_async_get_text_embedding_requires_non_empty_embeddings(self): + ollama_embedding = OllamaEmbedding(model="test-model") + ollama_embedding.async_client = AsyncMock() + ollama_embedding.async_client.embed.return_value = {"embeddings": []} + + async def run_async_test(): + with self.assertRaisesRegex(ValueError, "returned no embeddings"): + await ollama_embedding.async_get_text_embedding("a") + + import asyncio + + asyncio.run(run_async_test()) diff --git a/hugegraph-llm/src/tests/models/llms/test_litellm_client.py b/hugegraph-llm/src/tests/models/llms/test_litellm_client.py new file mode 100644 index 000000000..01fdeaeb6 --- /dev/null +++ b/hugegraph-llm/src/tests/models/llms/test_litellm_client.py @@ -0,0 +1,90 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import asyncio +import unittest +from unittest.mock import AsyncMock, patch + +from litellm.exceptions import APIError, BudgetExceededError +from tenacity.retry import retry_if_exception_type + +from hugegraph_llm.models.llms.litellm import LiteLLMClient + + +class TestLiteLLMClient(unittest.TestCase): + def test_budget_exceeded_error_is_not_retried(self): + client = LiteLLMClient(model_name="openai/gpt-4.1-mini") + error = BudgetExceededError(current_cost=2.0, max_budget=1.0) + + with patch("hugegraph_llm.models.llms.litellm.completion", side_effect=error) as mock_completion: + with self.assertRaises(BudgetExceededError): + client.generate(prompt="hello") + + mock_completion.assert_called_once() + + def test_generate_retries_api_error_and_reraises_original_exception(self): + client = LiteLLMClient(model_name="openai/gpt-4.1-mini") + error = APIError(status_code=500, message="upstream failed", llm_provider="openai", model="gpt-4.1-mini") + + with patch("hugegraph_llm.models.llms.litellm.completion", side_effect=error) as mock_completion: + with self.assertRaises(APIError): + client.generate(prompt="hello") + + self.assertEqual(mock_completion.call_count, 2) + + def test_generate_retry_policy_excludes_budget_exceeded_error(self): + retry_policy = LiteLLMClient().generate.retry.retry + + self.assertIsInstance(retry_policy, retry_if_exception_type) + self.assertNotIn(BudgetExceededError, retry_policy.exception_types) + + def test_generate_streaming_reraises_api_error(self): + client = LiteLLMClient(model_name="openai/gpt-4.1-mini") + error = APIError(status_code=500, message="upstream failed", llm_provider="openai", model="gpt-4.1-mini") + + with patch("hugegraph_llm.models.llms.litellm.completion", side_effect=error): + with self.assertRaises(APIError): + client.generate_streaming(prompt="hello") + + def test_agenerate_retries_api_error_and_reraises_original_exception(self): + client = LiteLLMClient(model_name="openai/gpt-4.1-mini") + error = APIError(status_code=500, message="upstream failed", llm_provider="openai", model="gpt-4.1-mini") + + async def run_async_test(): + with patch("hugegraph_llm.models.llms.litellm.acompletion", new=AsyncMock(side_effect=error)) as mock_call: + with self.assertRaises(APIError): + await client.agenerate(prompt="hello") + + self.assertEqual(mock_call.call_count, 2) + + asyncio.run(run_async_test()) + + def test_agenerate_streaming_reraises_api_error(self): + client = LiteLLMClient(model_name="openai/gpt-4.1-mini") + error = APIError(status_code=500, message="upstream failed", llm_provider="openai", model="gpt-4.1-mini") + + async def run_async_test(): + with patch("hugegraph_llm.models.llms.litellm.acompletion", new=AsyncMock(side_effect=error)): + with self.assertRaises(APIError): + async for _ in client.agenerate_streaming(prompt="hello"): + pass + + asyncio.run(run_async_test()) + + +if __name__ == "__main__": + unittest.main() diff --git a/hugegraph-llm/src/tests/models/llms/test_openai_client.py b/hugegraph-llm/src/tests/models/llms/test_openai_client.py index b9f8a113d..20e5aaacc 100644 --- a/hugegraph-llm/src/tests/models/llms/test_openai_client.py +++ b/hugegraph-llm/src/tests/models/llms/test_openai_client.py @@ -92,6 +92,23 @@ def test_generate_with_messages(self, mock_openai_class): messages=messages, ) + @patch("hugegraph_llm.models.llms.openai.OpenAI") + def test_generate_raises_runtime_error_with_empty_choices(self, mock_openai_class): + """Test generate method with an empty choices response.""" + # Setup mock client + mock_client = MagicMock() + empty_response = MagicMock() + empty_response.choices = [] + empty_response.usage = None + mock_client.chat.completions.create.return_value = empty_response + mock_openai_class.return_value = mock_client + + # Test the method + openai_client = OpenAIClient(model_name="gpt-3.5-turbo") + + with self.assertRaisesRegex(RuntimeError, "Empty choices in LLM response"): + openai_client.generate(prompt="What is the capital of France?") + @patch("hugegraph_llm.models.llms.openai.AsyncOpenAI") def test_agenerate(self, mock_async_openai_class): """Test agenerate method with mocked async OpenAI client.""" @@ -118,6 +135,26 @@ async def run_async_test(): asyncio.run(run_async_test()) + @patch("hugegraph_llm.models.llms.openai.AsyncOpenAI") + def test_agenerate_raises_runtime_error_with_empty_choices(self, mock_async_openai_class): + """Test agenerate method with an empty choices response.""" + # Setup mock async client + mock_async_client = MagicMock() + empty_response = MagicMock() + empty_response.choices = [] + empty_response.usage = None + mock_async_client.chat.completions.create = AsyncMock(return_value=empty_response) + mock_async_openai_class.return_value = mock_async_client + + # Test the method + openai_client = OpenAIClient(model_name="gpt-3.5-turbo") + + async def run_async_test(): + with self.assertRaisesRegex(RuntimeError, "Empty choices in LLM response"): + await openai_client.agenerate(prompt="What is the capital of France?") + + asyncio.run(run_async_test()) + @patch("hugegraph_llm.models.llms.openai.OpenAI") def test_stream_generate(self, mock_openai_class): """Test generate_streaming method with mocked OpenAI client.""" diff --git a/hugegraph-llm/src/tests/operators/hugegraph_op/test_fetch_graph_data.py b/hugegraph-llm/src/tests/operators/hugegraph_op/test_fetch_graph_data.py index 1f26beaf4..8527502d0 100644 --- a/hugegraph-llm/src/tests/operators/hugegraph_op/test_fetch_graph_data.py +++ b/hugegraph-llm/src/tests/operators/hugegraph_op/test_fetch_graph_data.py @@ -73,6 +73,21 @@ def test_run_with_none_graph_summary(self): self.assertIn("g.V().id().limit(10000).toList()", groovy_code) self.assertIn("g.E().id().limit(200).toList()", groovy_code) + def test_run_with_legacy_ordered_single_field_dicts_result(self): + """Test run method with legacy ordered single-field dict rows.""" + # Setup mock + self.mock_gremlin.exec.return_value = self.sample_result + + # Call the method + result = self.fetcher.run({}) + + # Verify the result + self.assertEqual(result["vertex_num"], 100) + self.assertEqual(result["edge_num"], 200) + self.assertEqual(result["vertices"], ["v1", "v2", "v3"]) + self.assertEqual(result["edges"], ["e1", "e2"]) + self.assertEqual(result["note"], "Only ≤10000 VIDs and ≤ 200 EIDs for brief overview .") + def test_run_with_existing_graph_summary(self): """Test run method with existing graph_summary.""" # Setup mock @@ -144,6 +159,21 @@ def test_run_with_partial_single_summary_dict_result(self): self.assertIsNone(result["edges"]) self.assertIsNone(result["note"]) + def test_run_with_empty_single_summary_dict_result(self): + """Test run method treats one empty dict as a summary row.""" + # Setup mock + self.mock_gremlin.exec.return_value = {"data": [{}]} + + # Call the method + result = self.fetcher.run({}) + + # Verify the result + self.assertIsNone(result["vertex_num"]) + self.assertIsNone(result["edge_num"]) + self.assertIsNone(result["vertices"]) + self.assertIsNone(result["edges"]) + self.assertIsNone(result["note"]) + def test_run_reraises_gremlin_exec_exception(self): """Test run method does not hide Gremlin execution failures.""" # Setup mock diff --git a/hugegraph-python-client/src/pyhugegraph/utils/util.py b/hugegraph-python-client/src/pyhugegraph/utils/util.py index 83c8ab18f..b4a6f6448 100644 --- a/hugegraph-python-client/src/pyhugegraph/utils/util.py +++ b/hugegraph-python-client/src/pyhugegraph/utils/util.py @@ -107,14 +107,14 @@ def __call__(self, response: requests.Response, method: str, path: str): status_message = status.get("message") if isinstance(status, dict) else None details = ( body.get("exception") - or status_message or body.get("message") + or status_message or response.text or "unknown error" ) else: details = response.text or "unknown error" - except (ValueError, KeyError, TypeError, AttributeError): + except (ValueError, KeyError, AttributeError, TypeError): details = response.text or "unknown error" req_body = response.request.body if response.request.body else "Empty body" diff --git a/hugegraph-python-client/src/tests/api/test_response_validation.py b/hugegraph-python-client/src/tests/api/test_response_validation.py index 0504b7018..759d97184 100644 --- a/hugegraph-python-client/src/tests/api/test_response_validation.py +++ b/hugegraph-python-client/src/tests/api/test_response_validation.py @@ -15,24 +15,42 @@ # specific language governing permissions and limitations # under the License. +import unittest from unittest.mock import Mock -import pytest import requests from pyhugegraph.utils.util import ResponseValidation -def test_response_validation_raises_http_error_with_numeric_status_body(): - response = Mock(spec=requests.Response) - response.status_code = 400 - response.text = '{"status":400,"message":"bad gremlin"}' - response.content = response.text.encode("utf-8") - response.json.return_value = {"status": 400, "message": "bad gremlin"} - response.request = Mock() - response.request.body = "g.V2()" - response.raise_for_status.side_effect = requests.exceptions.HTTPError("400 Client Error") +class TestResponseValidation(unittest.TestCase): + def _mock_error_response(self, body, text): + response = Mock(spec=requests.Response) + response.status_code = 400 + response.text = text + response.content = response.text.encode("utf-8") + response.json.return_value = body + response.request = Mock() + response.request.body = "g.V2()" + response.raise_for_status.side_effect = requests.exceptions.HTTPError("400 Client Error") + return response - validator = ResponseValidation() + def test_numeric_status_body_raises_server_exception_with_message(self): + response = self._mock_error_response( + {"status": 400, "message": "bad gremlin"}, + '{"status":400,"message":"bad gremlin"}', + ) + validator = ResponseValidation() - with pytest.raises(Exception, match="bad gremlin"): - validator(response, "POST", "/gremlin") + with self.assertRaisesRegex(Exception, "Server Exception: bad gremlin"): + validator(response, "POST", "/gremlin") + + def test_non_dict_json_body_raises_server_exception_with_response_text(self): + response = self._mock_error_response(["bad gremlin"], "bad gremlin") + validator = ResponseValidation() + + with self.assertRaisesRegex(Exception, "Server Exception: bad gremlin"): + validator(response, "POST", "/gremlin") + + +if __name__ == "__main__": + unittest.main() From 76248e6385af9d3ba85f9fe9b3f287e3e81e33bc Mon Sep 17 00:00:00 2001 From: zhouyanjue Date: Wed, 20 May 2026 14:46:08 +0800 Subject: [PATCH 6/7] test(llm): update schema manager error expectation Change-Id: Ib8bb730fb9c28e4b3e0a05e72d531544d0f623a9 --- .../src/tests/operators/hugegraph_op/test_schema_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hugegraph-llm/src/tests/operators/hugegraph_op/test_schema_manager.py b/hugegraph-llm/src/tests/operators/hugegraph_op/test_schema_manager.py index a20857aec..b77ae01f7 100644 --- a/hugegraph-llm/src/tests/operators/hugegraph_op/test_schema_manager.py +++ b/hugegraph-llm/src/tests/operators/hugegraph_op/test_schema_manager.py @@ -158,7 +158,7 @@ def test_run_with_empty_schema(self): self.schema_manager.run({}) # Verify the exception message - self.assertIn(f"Can not get {self.graph_name}'s schema from HugeGraph!", str(cm.exception)) + self.assertIn(f"Cannot get {self.graph_name}'s schema from HugeGraph!", str(cm.exception)) def test_run_with_existing_context(self): """Test run method with an existing context.""" From 35f460404ada658a57c91094b33801f2e52e5326 Mon Sep 17 00:00:00 2001 From: zhouyanjue Date: Wed, 20 May 2026 14:58:12 +0800 Subject: [PATCH 7/7] fix(llm): align ollama embedding response validation Change-Id: I815753ca0350e6b5f734937973eca956d9ef422a --- .../hugegraph_llm/models/embeddings/ollama.py | 4 ++-- .../models/embeddings/test_ollama_embedding.py | 18 +++++++++++++++++- .../tests/models/llms/test_litellm_client.py | 7 ------- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/hugegraph-llm/src/hugegraph_llm/models/embeddings/ollama.py b/hugegraph-llm/src/hugegraph_llm/models/embeddings/ollama.py index 088c084c1..bb0abccfe 100644 --- a/hugegraph-llm/src/hugegraph_llm/models/embeddings/ollama.py +++ b/hugegraph-llm/src/hugegraph_llm/models/embeddings/ollama.py @@ -75,8 +75,8 @@ def get_texts_embeddings(self, texts: List[str], batch_size: int = 32) -> List[L all_embeddings = [] for i in range(0, len(texts), batch_size): batch = texts[i : i + batch_size] - response = self.client.embed(model=self.model, input=batch)["embeddings"] - all_embeddings.extend([list(inner_sequence) for inner_sequence in response]) + response = self.client.embed(model=self.model, input=batch) + all_embeddings.extend(self._get_embeddings_from_response(response)) return all_embeddings def _get_embeddings_from_response(self, response) -> List[List[float]]: diff --git a/hugegraph-llm/src/tests/models/embeddings/test_ollama_embedding.py b/hugegraph-llm/src/tests/models/embeddings/test_ollama_embedding.py index aa9b8b712..e7a2702a4 100644 --- a/hugegraph-llm/src/tests/models/embeddings/test_ollama_embedding.py +++ b/hugegraph-llm/src/tests/models/embeddings/test_ollama_embedding.py @@ -18,7 +18,7 @@ import os import unittest -from unittest.mock import AsyncMock +from unittest.mock import AsyncMock, MagicMock from hugegraph_llm.models.embeddings.base import SimilarityMode from hugegraph_llm.models.embeddings.ollama import OllamaEmbedding @@ -74,6 +74,22 @@ async def run_async_test(): asyncio.run(run_async_test()) + def test_get_texts_embeddings_requires_embeddings_key(self): + ollama_embedding = OllamaEmbedding(model="test-model") + ollama_embedding.client = MagicMock() + ollama_embedding.client.embed.return_value = {} + + with self.assertRaisesRegex(ValueError, "missing 'embeddings'"): + ollama_embedding.get_texts_embeddings(["a"]) + + def test_get_texts_embeddings_requires_non_empty_embeddings(self): + ollama_embedding = OllamaEmbedding(model="test-model") + ollama_embedding.client = MagicMock() + ollama_embedding.client.embed.return_value = {"embeddings": []} + + with self.assertRaisesRegex(ValueError, "returned no embeddings"): + ollama_embedding.get_texts_embeddings(["a"]) + def test_async_get_text_embedding_requires_non_empty_embeddings(self): ollama_embedding = OllamaEmbedding(model="test-model") ollama_embedding.async_client = AsyncMock() diff --git a/hugegraph-llm/src/tests/models/llms/test_litellm_client.py b/hugegraph-llm/src/tests/models/llms/test_litellm_client.py index 01fdeaeb6..3b27d780b 100644 --- a/hugegraph-llm/src/tests/models/llms/test_litellm_client.py +++ b/hugegraph-llm/src/tests/models/llms/test_litellm_client.py @@ -20,7 +20,6 @@ from unittest.mock import AsyncMock, patch from litellm.exceptions import APIError, BudgetExceededError -from tenacity.retry import retry_if_exception_type from hugegraph_llm.models.llms.litellm import LiteLLMClient @@ -46,12 +45,6 @@ def test_generate_retries_api_error_and_reraises_original_exception(self): self.assertEqual(mock_completion.call_count, 2) - def test_generate_retry_policy_excludes_budget_exceeded_error(self): - retry_policy = LiteLLMClient().generate.retry.retry - - self.assertIsInstance(retry_policy, retry_if_exception_type) - self.assertNotIn(BudgetExceededError, retry_policy.exception_types) - def test_generate_streaming_reraises_api_error(self): client = LiteLLMClient(model_name="openai/gpt-4.1-mini") error = APIError(status_code=500, message="upstream failed", llm_provider="openai", model="gpt-4.1-mini")