imbajin commented on code in PR #382:
URL: https://github.com/apache/hugegraph-ai/pull/382#discussion_r4250381381
##########
hugegraph-llm/src/hugegraph_llm/api/thin_api.py:
##########
@@ -151,10 +151,31 @@ def graph_import_api(req: GraphImportRequest):
@thin_router.post("/vid-embeddings/refresh", status_code=status.HTTP_200_OK,
response_model=ThinAPIResponse)
-def vid_embeddings_refresh_api(_req: VidEmbeddingsRefreshRequest):
+def vid_embeddings_refresh_api(req: VidEmbeddingsRefreshRequest):
if blocked := _thin_write_disabled():
return blocked
- return _wrap_flow_call(FlowName.UPDATE_VID_EMBEDDINGS)
+ # Keep target validation and flow execution atomic with respect to runtime
+ # graph configuration updates. The flow reads the same shared settings.
+ with runtime_config_lock:
+ expected_fields = {"expected_graph", "expected_graphspace"}
+ supplied_fields = req.model_fields_set & expected_fields
+ if supplied_fields and (
+ supplied_fields != expected_fields
+ or req.expected_graph != huge_settings.graph_name
+ or req.expected_graphspace != huge_settings.graph_space
+ ):
+ return _envelope_err(
+ "VALIDATION_ERROR",
+ "Expected graph and graphspace must match the configured
HugeGraph target.",
+ suggestion="Provide both expected_graph and
expected_graphspace matching the AI service target.",
+ )
+ return _wrap_flow_call(FlowName.UPDATE_VID_EMBEDDINGS)
Review Comment:
Correction: the cached-pipeline conclusion above is incorrect.
[`BaseNode.run()`](https://github.com/apache/hugegraph-ai/blob/bdf76543dd59cc19da4e3eb1337e2e1d1bdc8a7f/hugegraph-llm/src/hugegraph_llm/nodes/base_node.py#L60-L65)
calls `node_init()` on every execution.
[`FetchGraphDataNode`](https://github.com/apache/hugegraph-ai/blob/bdf76543dd59cc19da4e3eb1337e2e1d1bdc8a7f/hugegraph-llm/src/hugegraph_llm/nodes/hugegraph_node/fetch_graph_data.py#L29-L32)
rebuilds its client/operator there, and
[`BuildSemanticIndexNode`](https://github.com/apache/hugegraph-ai/blob/bdf76543dd59cc19da4e3eb1337e2e1d1bdc8a7f/hugegraph-llm/src/hugegraph_llm/nodes/index_node/build_semantic_index.py#L28-L36)
rebuilds its index operator. The pinned pycgraph runtime dispatches the Python
run override on each pipeline run.
Pipeline reuse therefore does not itself retain graph A's client/index after
switching to B. Please withdraw this particular critical blocker; pipeline
invalidation is not justified solely by this argument. This does not dismiss
the separate default-GraphSpace compatibility issue or physical-instance
identity limitation.
##########
hugegraph-mcp/hugegraph_mcp/tools/refresh_vid_embeddings.py:
##########
@@ -41,7 +42,42 @@ def refresh_vid_embeddings(confirm: bool = False) ->
dict[str, Any]:
suggestion="Pass confirm=True to refresh VID embeddings.",
)
- ai_result = post("/vid-embeddings/refresh", json={})
+ cfg = MCPConfig.from_env()
+ capability = get("/vid-embeddings/refresh-capabilities", cfg=cfg)
+ if not capability.get("ok"):
+ error = capability.get("error") or {}
+ # Only an explicitly absent/unsupported endpoint means the protocol is
+ # unavailable. Keep transient and authentication errors retryable as
+ # reported by the client, without ever dispatching the refresh.
+ status_code = (error.get("details") or {}).get("status_code")
+ if error.get("type") in {
+ "AUTHENTICATION_FAILED",
+ "AUTHORIZATION_FAILED",
+ "FEATURE_DISABLED",
+ } or status_code not in {404, 405}:
+ return capability
+ return envelope_err(
+ ErrorType.FEATURE_DISABLED,
+ "AI refresh target-validation endpoint is unavailable; refresh was
not sent.",
+ suggestion="Check the AI service URL and routing, or upgrade
HugeGraph-AI to support target validation.",
+ details={"status_code": status_code},
+ )
+ support = capability.get("data")
+ if (
+ not isinstance(support, dict)
+ or type(support.get("protocol_version")) is not int
+ or support["protocol_version"] != 1
+ ):
+ return envelope_err(
+ ErrorType.FEATURE_DISABLED,
+ "AI refresh target-validation protocol is unsupported; refresh was
not sent.",
+ suggestion="Upgrade HugeGraph-AI to a compatible target-validation
version.",
+ )
+ ai_result = post(
+ "/vid-embeddings/refresh",
+ cfg=cfg,
+ json={"expected_graph": cfg.graph, "expected_graphspace":
cfg.graphspace},
Review Comment:
The instance-identity limitation is real, but the impact should be stated
precisely. This flow reads IDs/schema from HugeGraph and updates the configured
VID vector index; it does not write graph records. If the URLs identify
different instances with the same graph/graphspace names, it can read the
unintended instance and rebuild the vector index from that data.
The independent URLs already existed on the base branch, and this PR
documents the remaining operator responsibility. Keep this as an explicit
safety boundary and follow-up design requirement, rather than describing it as
a newly introduced graph-database write regression.
##########
hugegraph-llm/src/hugegraph_llm/api/thin_api.py:
##########
@@ -151,10 +151,31 @@ def graph_import_api(req: GraphImportRequest):
@thin_router.post("/vid-embeddings/refresh", status_code=status.HTTP_200_OK,
response_model=ThinAPIResponse)
-def vid_embeddings_refresh_api(_req: VidEmbeddingsRefreshRequest):
+def vid_embeddings_refresh_api(req: VidEmbeddingsRefreshRequest):
if blocked := _thin_write_disabled():
return blocked
- return _wrap_flow_call(FlowName.UPDATE_VID_EMBEDDINGS)
+ # Keep target validation and flow execution atomic with respect to runtime
+ # graph configuration updates. The flow reads the same shared settings.
+ with runtime_config_lock:
+ expected_fields = {"expected_graph", "expected_graphspace"}
+ supplied_fields = req.model_fields_set & expected_fields
+ if supplied_fields and (
+ supplied_fields != expected_fields
+ or req.expected_graph != huge_settings.graph_name
+ or req.expected_graphspace != huge_settings.graph_space
Review Comment:
Confirmed by independent source tracing: MCP normalizes missing graphspace
to `DEFAULT`, while AI settings can remain `None` or blank and the new
comparison rejects the equivalent target. Normalize the effective identity
without changing the existing index-path convention, and cover
unset/blank/explicit DEFAULT as well as genuinely different non-default spaces.
--
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]
