VGalaxies commented on code in PR #240:
URL: https://github.com/apache/hugegraph-ai/pull/240#discussion_r3400947230
##########
hugegraph-llm/src/hugegraph_llm/demo/rag_demo/text2gremlin_block.py:
##########
@@ -214,3 +214,62 @@ def graph_rag_recall(
)
context = rag.run(verbose=True, query=query, graph_search=True)
return context
+
+def gremlin_generate_selective(
+ inp: str,
+ example_num: int,
+ schema_input: str,
+ gremlin_prompt_input: str,
+ requested_outputs: Optional[List[str]] = None,
+) -> Dict[str, Any]:
+ """
+ Wraps the original gremlin_generate function and filters its output
+ based on the requested_outputs list of strings.
+ """
+
+ output_keys = [
+ "match_result",
+ "template_gremlin",
+ "raw_gremlin",
+ "template_execution_result",
+ "raw_execution_result",
+ ]
+ original_results = gremlin_generate(inp, example_num, schema_input,
gremlin_prompt_input)
Review Comment:
**High: Do not execute Gremlin before filtering requested outputs**
`hugegraph-llm/src/hugegraph_llm/demo/rag_demo/text2gremlin_block.py:237`
**Evidence**
- `gremlin_generate_selective()` calls `gremlin_generate()` unconditionally
before checking `requested_outputs`; `gremlin_generate()` executes both
generated queries through `run_gremlin_query()` at lines 98 and 102, even when
the request only asks for `template_gremlin`.
**Impact**
- `/text2gremlin` can execute LLM-generated Gremlin against HugeGraph when
the caller requested generation output only.
**Requested fix**
- Split generation from execution and call `run_gremlin_query()` only when
`template_execution_result` or `raw_execution_result` is explicitly requested.
##########
hugegraph-llm/src/hugegraph_llm/operators/index_op/semantic_id_query.py:
##########
@@ -75,18 +79,57 @@ def _exact_match_vids(self, keywords: List[str]) ->
Tuple[List[str], List[str]]:
def _fuzzy_match_vids(self, keywords: List[str]) -> List[str]:
fuzzy_match_result = []
for keyword in keywords:
- keyword_vector = self.embedding.get_text_embedding(keyword)
- results = self.vector_index.search(keyword_vector,
top_k=self.topk_per_keyword,
+ keyword_vector = self.embedding.get_texts_embeddings([keyword])
+ results = self.vector_index.search(keyword_vector[0],
top_k=self.topk_per_keyword,
dis_threshold=float(self.vector_dis_threshold))
if results:
fuzzy_match_result.extend(results[:self.topk_per_keyword])
return fuzzy_match_result
+ def _exact_match_properties(self, keywords: List[str]) -> Tuple[List[str],
List[str]]:
+ property_keys = self.schema.getPropertyKeys()
+ log.debug("property_keys: %s", property_keys)
+ matched_properties = set()
+ unmatched_keywords = set(keywords)
+ for key in property_keys:
+ for keyword in list(unmatched_keywords):
+ gremlin_query = f"g.V().has('{key.name}',
'{keyword}').limit(1)"
Review Comment:
**High: Bind or escape property Gremlin values**
`hugegraph-llm/src/hugegraph_llm/operators/index_op/semantic_id_query.py:96`
**Evidence**
- `_exact_match_properties()` interpolates `key.name` and user-derived
`keyword` directly into `g.V().has('{key.name}', '{keyword}')`; the property
fallback also formats `prop_name` and `prop_value` into Gremlin at
`graph_rag_query.py:219`.
**Impact**
- A keyword or indexed property value containing quotes or Gremlin syntax
can alter the query sent to HugeGraph.
**Requested fix**
- Use Gremlin bindings or a shared safe serialization/escaping helper for
property keys and values before execution.
##########
vermeer-python-client/src/pyvermeer/utils/vermeer_requests.py:
##########
@@ -0,0 +1,104 @@
+# !/usr/bin/env python3
+"""
+file: vermeer_requests.py
+author: [email protected]
+"""
+
+import json
+from typing import Optional
+from urllib.parse import urljoin
+
+import requests
+from requests.adapters import HTTPAdapter
+from urllib3.util.retry import Retry
+
+from pyvermeer.utils.exception import JsonDecodeError, ConnectError,
TimeOutError, UnknownError
+from pyvermeer.utils.log import log
+from pyvermeer.utils.vermeer_config import VermeerConfig
+
+
+class VermeerSession:
+ """vermeer session"""
+
+ def __init__(
+ self,
+ cfg: VermeerConfig,
+ retries: int = 3,
+ backoff_factor: int = 0.1,
+ status_forcelist=(500, 502, 504),
+ session: Optional[requests.Session] = None,
+ ):
+ """
+ Initialize the Session.
+ """
+ self._cfg = cfg
+ self._retries = retries
+ self._backoff_factor = backoff_factor
+ self._status_forcelist = status_forcelist
+ if self._cfg.token is not None:
+ self._auth = self._cfg.token
+ else:
+ raise ValueError("Vermeer Token must be provided.")
+ self._headers = {"Content-Type": "application/json", "Authorization":
self._auth}
+ self._timeout = cfg.timeout
+ self._session = session if session else requests.Session()
+ self.__configure_session()
+
+ def __configure_session(self):
+ """
+ Configure the retry strategy and connection adapter for the session.
+ """
+ retry_strategy = Retry(
+ total=self._retries,
+ read=self._retries,
+ connect=self._retries,
+ backoff_factor=self._backoff_factor,
+ status_forcelist=self._status_forcelist,
+ )
+ adapter = HTTPAdapter(max_retries=retry_strategy)
+ self._session.mount("http://";, adapter)
+ self._session.mount("https://";, adapter)
+ self._session.keep_alive = False
+ log.debug(
+ "Session configured with retries=%s and backoff_factor=%s",
+ self._retries,
+ self._backoff_factor,
+ )
+
+ def resolve(self, path: str):
+ """
+ Resolve the path to a full URL.
+ """
+ url = f"http://{self._cfg.ip}:{self._cfg.port}/";
Review Comment:
**Medium: Avoid sending Vermeer tokens only over plaintext HTTP**
`vermeer-python-client/src/pyvermeer/utils/vermeer_requests.py:72`
**Evidence**
- `VermeerSession` requires a token and sets it in the `Authorization`
header at line 42, but `resolve()` always builds `http://{ip}:{port}/` and the
client exposes only `ip`, `port`, and `token`.
**Impact**
- Bearer tokens are exposed on the network for non-local Vermeer deployments.
**Requested fix**
- Accept a full base URL or scheme option, support HTTPS, and avoid
defaulting credentialed requests to plaintext transport.
##########
hugegraph-python-client/src/pyhugegraph/client.py:
##########
@@ -50,12 +51,30 @@ def __init__(
self,
url: str,
graph: str,
- user: str,
- pwd: str,
+ user: Optional[str] = None,
+ pwd: Optional[str] = None,
+ token: Optional[str] = None,
Review Comment:
**High: Preserve positional `graphspace` compatibility**
`hugegraph-python-client/src/pyhugegraph/client.py:56`
**Evidence**
- `token` was inserted before `graphspace` in `PyHugeClient.__init__`, while
the previous fifth positional argument was `graphspace`; an existing call still
uses `PyHugeClient(url, graph, user, pwd, gs)` at
`hugegraph-llm/src/hugegraph_llm/operators/hugegraph_op/graph_rag_query.py:262`.
**Impact**
- Existing positional callers silently bind graphspace as a bearer token,
disabling graphspace routing and changing auth behavior.
**Requested fix**
- Keep `graphspace` in its old positional slot and make `token`
keyword-only, or append `token` after the existing positional parameters.
--
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]
