bitflicker64 commented on code in PR #382:
URL: https://github.com/apache/hugegraph-ai/pull/382#discussion_r4248285295
##########
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:
Important: with both sides on their defaults,
`refresh_vid_embeddings_tool(confirm=true)` is now rejected even though MCP and
the AI service address the same graph.
MCP always sends a non-empty graphspace: `hugegraph_mcp/config.py` turns an
unset or blank value into `"DEFAULT"`, and `refresh_vid_embeddings.py:79`
forwards it as `expected_graphspace`. The AI side defaults to `graph_space:
Optional[str] = None` (`config/hugegraph_config.py:31`), and this line compares
the two with `!=`, so `"DEFAULT" != None` returns `VALIDATION_ERROR`. A blank
Graph Space box in the UI stores `""` through `apply_graph_config` and fails
the same way.
On HugeGraph 1.7.0+ the two values name the same target: with `graph_space`
unset, `HGraphConfig.__post_init__` (`pyhugegraph/utils/huge_config.py:73-77`)
sets `self.graphspace = "DEFAULT"`, so the flow that runs after this check
already works on `DEFAULT/hugegraph`.
Measured at bdf7654 (macOS, Python 3.10) with `create_api_router()` served
with login and thin writes enabled, a mocked scheduler and no `GRAPH_SPACE`,
and `refresh_vid_embeddings(confirm=True)` called from an MCP process with no
graph variables set:
```
AI graph_name='hugegraph' graph_space=None
MCP graph='hugegraph' graphspace='DEFAULT'
"type": "VALIDATION_ERROR",
"message": "Expected graph and graphspace must match the configured
HugeGraph target."
```
A `POST /vid-embeddings/refresh` with `{}`, which is what MCP sends on
`main`, was accepted by the same server, and the MCP call succeeded once the AI
side ran with `graph_space="DEFAULT"`. So a deployment where refresh works
today stops working after the upgrade until the operator edits the AI
configuration, and the error does not say which side to change. The documented
workaround also has a side effect: `GRAPH_SPACE=DEFAULT` moves the example
index folder from `hugegraph/` to `DEFAULT_hugegraph/`
(`get_index_folder_name`, used at `text2gremlin_block.py:85`).
Please normalise both sides before comparing, for example:
```python
configured_space = (huge_settings.graph_space or "").strip() or "DEFAULT"
expected_space = (req.expected_graphspace or "").strip() or "DEFAULT"
```
keeping the rule that both fields must be supplied. All nine rows of
`test_vid_embeddings_refresh_validates_target_before_flow` still pass with
that; please add `(None, {"expected_graph": "target", "expected_graphspace":
"DEFAULT"}, True)`. The request-model comment at `rag_requests.py:189-191`,
`hugegraph-mcp/README.md:244-246` and the zh-CN counterpart describe the
exact-match rule and need the same update.
##########
hugegraph-mcp/examples/stdio_check.py:
##########
@@ -0,0 +1,79 @@
+# 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.
+
+"""Launch one STDIO session, inspect it, then execute optional JSON tool
requests.
+
+Usage: python hugegraph-mcp/examples/stdio_check.py [requests.json]
+requests.json is a list of {"name": ..., "arguments": ...} tools/call payloads.
+The same session stays open for all calls. Server inherits the caller
environment.
+"""
+
+import argparse
+import asyncio
+import json
+import os
+import sys
+from pathlib import Path
+
+from mcp import ClientSession, StdioServerParameters
+from mcp.client.stdio import stdio_client
+
+
+async def run(requests_path: Path | None, interactive: bool):
+ requests = json.loads(requests_path.read_text(encoding="utf-8")) if
requests_path else []
+ if not isinstance(requests, list):
+ raise ValueError("Requests must be a JSON list")
+ params = StdioServerParameters(command=sys.executable, args=["-m",
"hugegraph_mcp.server"], env=dict(os.environ))
+ async with stdio_client(params) as (read, write), ClientSession(read,
write) as session:
+ await session.initialize()
+ listed = await session.list_tools()
+ sys.stdout.write(json.dumps({"tools": [tool.name for tool in
listed.tools]}) + "\n")
+ pending = [{"name": "inspect_graph_tool", "arguments":
{"include_counts": False}}, *requests]
+ while True:
+ if pending:
+ request = pending.pop(0)
+ elif interactive:
+ try:
+ line = await asyncio.to_thread(input, "tools/call JSON
(EOF to finish): ")
+ except EOFError:
+ break
+ if not line.strip():
+ continue
+ request = json.loads(line)
Review Comment:
Minor: one malformed input line ends the interactive session with a
traceback. `json.loads(line)` is not guarded, and 12 of the 20 `json` blocks in
`docs/integration.md` are printed across several lines, so pasting one as shown
fails on its first line (`{`).
Measured at bdf7654 with the `GraphFixture` from
`tests/distribution_smoke.py` as the server: feeding the section 2 block as
written to `stdio_check.py --interactive` exits 1 with
`json.decoder.JSONDecodeError: Expecting property name enclosed in double
quotes: line 1 column 2`. The checklist does say to paste each block as one
line, but a slip in the middle of the write steps drops the session.
Please catch `json.JSONDecodeError` (and a missing `name`) in the
interactive branch, print the error and `continue`. Printing the checklist
blocks on one line would also remove the manual step.
--
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]
