imbajin commented on code in PR #323:
URL: https://github.com/apache/hugegraph-ai/pull/323#discussion_r3195733520
##########
hugegraph-llm/src/hugegraph_llm/operators/index_op/build_gremlin_example_index.py:
##########
@@ -38,14 +38,24 @@ def __init__(
self.vector_index = vector_index
def run(self, context: Dict[str, Any]) -> Dict[str, Any]:
- # !: We have assumed that self.example is not empty
+ if not self.examples:
+ self.vector_index.clean(self.vector_index_name)
+ context["embed_dim"] = 0
+ return context
+
queries = [example["query"] for example in self.examples]
# TODO: refactor function chain async to avoid blocking
examples_embedding =
asyncio.run(get_embeddings_parallel(self.embedding, queries))
+
+ if not examples_embedding:
+ self.vector_index.clean(self.vector_index_name)
Review Comment:
‼️ **空 embeddings 情况下 clean() 更危险，且掩盖了真实错误**
走到这个分支意味着：`self.examples` 非空，但 embedding 服务返回了空列表 —— 这通常是 **embedding
调用失败**（网络、API key、配额等），而不是正常业务场景。此时静默 `clean()` + 返回 `embed_dim=0` 会：
1. 清掉用户之前建好的正常索引
2. 让调用方以为"成功构建了 0 维索引"，掩盖真实故障
建议改为告警或抛异常：
```suggestion
if not examples_embedding:
log.error("Embedding returned empty for %d examples, skip index
build", len(self.examples))
context["embed_dim"] = 0
return context
```
或直接 `raise RuntimeError(...)` 让上层感知失败。至少**不要** `clean()`。
##########
hugegraph-llm/src/hugegraph_llm/operators/index_op/build_gremlin_example_index.py:
##########
@@ -38,14 +38,24 @@ def __init__(
self.vector_index = vector_index
def run(self, context: Dict[str, Any]) -> Dict[str, Any]:
- # !: We have assumed that self.example is not empty
+ if not self.examples:
+ self.vector_index.clean(self.vector_index_name)
Review Comment:
‼️ **不应在空 examples 时调用 `clean()`，这是破坏性副作用**
`vector_index.clean("gremlin_examples")` 在本仓库其他地方（如
`utils/graph_index_utils.py:50`）是用作**销毁整个索引文件**的语义。当前实现会导致：调用者传入 `examples=[]`
本意是"跳过构建"，但实际会**静默删除已经建好的索引**，造成数据丢失。
PR 描述只提到"修复 IndexError"，没有说明这个行为变更。建议回归到纯早返回：
```suggestion
if not self.examples:
context["embed_dim"] = 0
return context
```
如果确实需要"空 examples 时清空旧索引"的语义，请：
1. 在 PR 描述中显式说明这是设计意图
2. 确认上游调用方（operator_list.py 等）不会意外传空列表
##########
hugegraph-llm/src/hugegraph_llm/nodes/index_node/build_gremlin_example_index.py:
##########
@@ -34,7 +34,7 @@ def node_init(self):
# pylint: disable=import-outside-toplevel
from hugegraph_llm.utils.vector_index_utils import
get_vector_index_class
- if not self.wk_input.examples:
+ if self.wk_input.examples is None:
Review Comment:
‼️ **`not` → `is None` 放宽了校验，配合 operator 侧的 clean() 会放行空列表**
改动前：`if not self.wk_input.examples:` 对 `None` 和 `[]` 都返回错误，调用方拿不到 CStatus -1。
改动后：只有 `None` 报错，`[]` 会继续执行，最终调用 operator 的 run() → 触发
`self.vector_index.clean("gremlin_examples")` 把现有索引清掉。
两处改动叠加后的实际效果是：**传入 `examples=[]` 会静默销毁已有索引**，但错误信息依然写着 "examples is
required"，语义自相矛盾。
建议二选一：
- 若允许空列表走 operator 的空分支：把错误信息改为只针对 `None`，并在 PR 描述中说明设计意图
- 若不允许空列表：保留原来的 `if not ...:` 校验
##########
hugegraph-llm/src/tests/operators/index_op/test_build_gremlin_example_index.py:
##########
@@ -54,7 +54,10 @@ def test_init(self):
self.assertEqual(self.index_builder.vector_index_name,
"gremlin_examples")
@patch('asyncio.run')
- @patch('hugegraph_llm.utils.embedding_utils.get_embeddings_parallel')
+ @patch(
+
'hugegraph_llm.operators.index_op.build_gremlin_example_index.get_embeddings_parallel',
Review Comment:
🧹 **Patch 目标的修正合理**
由 `hugegraph_llm.utils.embedding_utils.get_embeddings_parallel` 改为
`hugegraph_llm.operators.index_op.build_gremlin_example_index.get_embeddings_parallel`
是正确的 —— Python patch 应该打在"被 import 到的位置"而不是"定义位置"，原来的 patch
其实没生效。`new_callable=MagicMock` 配合 `asyncio.run` 的 mock 也合理，避免协程警告。这部分无需修改。
##########
hugegraph-llm/src/tests/operators/index_op/test_build_gremlin_example_index.py:
##########
@@ -78,7 +81,10 @@ def test_run_with_examples(self,
mock_get_embeddings_parallel, mock_asyncio_run)
self.assertEqual(context["embed_dim"], 3)
@patch('asyncio.run')
- @patch('hugegraph_llm.utils.embedding_utils.get_embeddings_parallel')
+ @patch(
+
'hugegraph_llm.operators.index_op.build_gremlin_example_index.get_embeddings_parallel',
+ new_callable=MagicMock,
Review Comment:
⚠️ **测试名与实际验证不一致**
当前测试名叫 `test_run_with_empty_examples`，但 assertions 里同时验证了 `clean`
被调用、`from_name/add/save` 都不被调用 —— 等于把 "空 examples 会静默删除现有索引" 这个行为写死成了契约。如果采纳前面
operator 侧的修改建议（移除 clean 调用），这里需要同步：
```suggestion
mock_vector_store_class.clean.assert_not_called()
```
另外建议补一个 `test_run_with_non_empty_examples_but_empty_embeddings` 用例，单独覆盖
embeddings 为空（非 examples 为空）的分支。
##########
hugegraph-llm/src/hugegraph_llm/operators/index_op/build_gremlin_example_index.py:
##########
@@ -38,14 +38,24 @@ def __init__(
self.vector_index = vector_index
def run(self, context: Dict[str, Any]) -> Dict[str, Any]:
- # !: We have assumed that self.example is not empty
+ if not self.examples:
+ self.vector_index.clean(self.vector_index_name)
+ context["embed_dim"] = 0
+ return context
+
queries = [example["query"] for example in self.examples]
# TODO: refactor function chain async to avoid blocking
examples_embedding =
asyncio.run(get_embeddings_parallel(self.embedding, queries))
+
+ if not examples_embedding:
+ self.vector_index.clean(self.vector_index_name)
+ context["embed_dim"] = 0
+ return context
+
embed_dim = len(examples_embedding[0])
- if len(self.examples) > 0:
- vector_index = self.vector_index.from_name(embed_dim,
self.vector_index_name)
- vector_index.add(examples_embedding, self.examples)
- vector_index.save_index_by_name(self.vector_index_name)
+ vector_index = self.vector_index.from_name(embed_dim,
self.vector_index_name)
+ vector_index.add(examples_embedding, self.examples)
+ vector_index.save_index_by_name(self.vector_index_name)
+
context["embed_dim"] = embed_dim
- return context
+ return context
Review Comment:
⚠️ **文件末尾缺少换行符**（diff 显示 `\ No newline at end of file`）
违反 PEP 8 W292，某些 lint/pre-commit 钩子会失败。请在文件末尾补一个换行。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
