This is an automated email from the ASF dual-hosted git repository. imbajin pushed a commit to branch goal-scan in repository https://gitbox.apache.org/repos/asf/hugegraph-ai.git
commit 11e5b407b428e86b63edc57c44564771cd33ddff Author: imbajin <[email protected]> AuthorDate: Sun May 31 11:54:47 2026 +0800 fix(llm): validate request-scoped config - update graph settings only for explicit client_config fields - reject unsupported provider types before global mutation - add API contract tests for CS-011 and CS-012 - update code-scan ledgers --- .workflow/code-scan/code-scan-state.json | 4 +- .../code-scan/reports/final-code-scan-report.md | 4 +- .workflow/code-scan/reports/issues.md | 12 ++-- .workflow/code-scan/reports/test-quality-ledger.md | 2 + .../src/hugegraph_llm/api/models/rag_requests.py | 7 ++- hugegraph-llm/src/hugegraph_llm/api/rag_api.py | 22 ++++--- hugegraph-llm/src/tests/api/test_rag_api.py | 68 +++++++++++++++++++++- 7 files changed, 100 insertions(+), 19 deletions(-) diff --git a/.workflow/code-scan/code-scan-state.json b/.workflow/code-scan/code-scan-state.json index 16845f25..704cd470 100644 --- a/.workflow/code-scan/code-scan-state.json +++ b/.workflow/code-scan/code-scan-state.json @@ -59,6 +59,8 @@ "CS-002", "CS-004", "CS-008", + "CS-011", + "CS-012", "CS-013" ], "files_touched": [ @@ -125,5 +127,5 @@ "uv run ruff check <edited-python-files>", "git status --short" ], - "next_recommended_action": "Fix global config mutation and provider validation: CS-011 and CS-012." + "next_recommended_action": "Replace fake integration tests with production-flow smoke coverage." } diff --git a/.workflow/code-scan/reports/final-code-scan-report.md b/.workflow/code-scan/reports/final-code-scan-report.md index d6dc2eed..2ca80e8e 100644 --- a/.workflow/code-scan/reports/final-code-scan-report.md +++ b/.workflow/code-scan/reports/final-code-scan-report.md @@ -79,7 +79,7 @@ None. 1. Fix `CS-001` immediately: lock down `/logs` path handling and default admin credentials. Status: fixed after the scan. 2. Fix the client error-contract cluster: `CS-002`, `CS-004`, `CS-008`. Status: fixed after the scan. 3. Fix graph import correctness: `CS-013`, then add the missing round-trip tests. Status: fixed after the scan. -4. Fix global config mutation and provider validation: `CS-011`, `CS-012`. +4. Fix global config mutation and provider validation: `CS-011`, `CS-012`. Status: fixed after the scan. 5. Replace fake integration tests with production-flow smoke coverage before broader refactors. ## Fix Progress After Scan @@ -89,3 +89,5 @@ None. - `CS-004`: fixed by raising `ResponseParseError` for malformed successful responses. - `CS-008`: fixed by preserving lower-level exceptions from Gremlin execution instead of wrapping every failure as `NotFoundError`. - `CS-013`: fixed by preserving extracted property value types through the full extraction-to-commit path. +- `CS-011`: fixed by updating global graph settings only for explicitly supplied `client_config` fields. +- `CS-012`: fixed by rejecting unsupported provider types at request validation before global settings are mutated. diff --git a/.workflow/code-scan/reports/issues.md b/.workflow/code-scan/reports/issues.md index 67e56604..63dbc69f 100644 --- a/.workflow/code-scan/reports/issues.md +++ b/.workflow/code-scan/reports/issues.md @@ -81,7 +81,7 @@ Issue IDs use `CS-NNN`. Priorities are P0 highest through P5 lowest. - Module: `hugegraph-python-client` - Layer: client-api - Paths: `hugegraph-python-client/src/pyhugegraph/api/graph.py:68`, `hugegraph-python-client/src/pyhugegraph/api/graph.py:73`, `hugegraph-python-client/src/pyhugegraph/api/graph.py:173`, `hugegraph-python-client/src/pyhugegraph/utils/huge_requests.py:144` -- Status: open +- Status: fixed - Evidence: `getVertexByPage()`, `getVertexByCondition()`, and `getEdgeByPage()` concatenate labels and JSON properties directly into the URL. - Impact: Property values containing `&`, `?`, `#`, `=`, or spaces can corrupt filters and query different data. - Recommendation: Use `params=` or `urllib.parse.urlencode()` for all query parameters. @@ -144,8 +144,8 @@ Issue IDs use `CS-NNN`. Priorities are P0 highest through P5 lowest. - Status: open - Evidence: `GraphConfigRequest` has concrete defaults and `set_graph_config()` writes every field to global `huge_settings` whenever `client_config` exists. - Impact: A request intended to override one field can clear credentials, graph name, or graphspace for the whole process. -- Recommendation: Make nested fields optional and update only explicitly provided values, or keep request config out of global settings. -- Test note: Added a `FIXME:` in `test_rag_api.py`. +- Fix: request-scoped graph config updates now use `model_fields_set`, so only explicitly provided nested fields mutate `huge_settings`. +- Test note: Added `/rag` contract coverage proving partial `client_config` updates only the supplied graph fields. ### CS-012: Unsupported provider types are written into global settings before validation @@ -153,11 +153,11 @@ Issue IDs use `CS-NNN`. Priorities are P0 highest through P5 lowest. - Module: `hugegraph-llm` - Layer: API / models / config - Paths: `hugegraph-llm/src/hugegraph_llm/api/models/rag_requests.py:106`, `hugegraph-llm/src/hugegraph_llm/api/rag_api.py:163`, `hugegraph-llm/src/hugegraph_llm/api/rag_api.py:188`, `hugegraph-llm/src/hugegraph_llm/config/llm_config.py:29` -- Status: open +- Status: fixed - Evidence: Request models use bare `str` for `llm_type` and `reranker_type`; handlers write values to global settings before branch validation. - Impact: One bad request can poison process-global provider state for subsequent requests. -- Recommendation: Validate provider types with `Literal`/Enum before mutating global config. -- Test note: Added a `FIXME:` in `test_rag_api.py`. +- Fix: LLM and reranker request models now validate provider types with `Literal`, causing unsupported providers to fail with `422` before handler-side global mutation. +- Test note: Added config API coverage proving unsupported provider requests do not call apply callbacks or mutate global provider settings. ### CS-013: Property graph extraction stringifies values that importer rejects diff --git a/.workflow/code-scan/reports/test-quality-ledger.md b/.workflow/code-scan/reports/test-quality-ledger.md index a909f039..4f41704a 100644 --- a/.workflow/code-scan/reports/test-quality-ledger.md +++ b/.workflow/code-scan/reports/test-quality-ledger.md @@ -58,3 +58,5 @@ Added specific `FIXME:` comments in these files: - `CS-004`: response-validation tests now prove malformed successful JSON raises `ResponseParseError`. - `CS-008`: Gremlin tests now prove lower-level auth exceptions keep their original type through `GremlinManager.exec()`. - `CS-013`: extraction-to-commit round-trip tests now prove schema-typed numeric values survive `PropertyGraphExtract.run()` and are accepted by `Commit2Graph`. +- `CS-011`: `/rag` contract tests now prove partial `client_config` only updates explicitly supplied graph settings. +- `CS-012`: config API tests now prove unsupported LLM/reranker providers fail validation before mutating global settings. diff --git a/hugegraph-llm/src/hugegraph_llm/api/models/rag_requests.py b/hugegraph-llm/src/hugegraph_llm/api/models/rag_requests.py index 433438ee..77155978 100644 --- a/hugegraph-llm/src/hugegraph_llm/api/models/rag_requests.py +++ b/hugegraph-llm/src/hugegraph_llm/api/models/rag_requests.py @@ -23,6 +23,9 @@ from pydantic import BaseModel, field_validator from hugegraph_llm.config import prompt +LLMProviderType = Literal["openai", "ollama/local", "litellm"] +RerankerProviderType = Literal["cohere", "siliconflow"] + class GraphConfigRequest(BaseModel): url: str = Query("127.0.0.1:8080", description="hugegraph client url.") @@ -104,7 +107,7 @@ class GraphRAGRequest(BaseModel): class LLMConfigRequest(BaseModel): - llm_type: str + llm_type: LLMProviderType # The common parameters shared by OpenAI, LiteLLM, # and OLLAMA platforms. api_key: str @@ -120,7 +123,7 @@ class LLMConfigRequest(BaseModel): class RerankerConfigRequest(BaseModel): reranker_model: str - reranker_type: str + reranker_type: RerankerProviderType api_key: str cohere_base_url: Optional[str] = None diff --git a/hugegraph-llm/src/hugegraph_llm/api/rag_api.py b/hugegraph-llm/src/hugegraph_llm/api/rag_api.py index bce7de0d..9b9c7d35 100644 --- a/hugegraph-llm/src/hugegraph_llm/api/rag_api.py +++ b/hugegraph-llm/src/hugegraph_llm/api/rag_api.py @@ -90,12 +90,18 @@ def rag_http_api( } def set_graph_config(req): - if req.client_config: - huge_settings.graph_url = req.client_config.url - huge_settings.graph_name = req.client_config.graph - huge_settings.graph_user = req.client_config.user - huge_settings.graph_pwd = req.client_config.pwd - huge_settings.graph_space = req.client_config.gs + if req.client_config is None: + return + field_map = { + "url": "graph_url", + "graph": "graph_name", + "user": "graph_user", + "pwd": "graph_pwd", + "gs": "graph_space", + } + for request_field, settings_field in field_map.items(): + if request_field in req.client_config.model_fields_set: + setattr(huge_settings, settings_field, getattr(req.client_config, request_field)) @router.post("/rag/graph", status_code=status.HTTP_200_OK) def graph_rag_recall_api(req: GraphRAGRequest): @@ -163,7 +169,7 @@ def rag_http_api( def llm_config_api(req: LLMConfigRequest): llm_settings.chat_llm_type = req.llm_type - if req.llm_type == "openai": + if req.llm_type in ("openai", "litellm"): res = apply_llm_conf( req.api_key, req.api_base, @@ -179,7 +185,7 @@ def rag_http_api( def embedding_config_api(req: LLMConfigRequest): llm_settings.embedding_type = req.llm_type - if req.llm_type == "openai": + if req.llm_type in ("openai", "litellm"): res = apply_embedding_conf(req.api_key, req.api_base, req.language_model, origin_call="http") else: res = apply_embedding_conf(req.host, req.port, req.language_model, origin_call="http") diff --git a/hugegraph-llm/src/tests/api/test_rag_api.py b/hugegraph-llm/src/tests/api/test_rag_api.py index c06a52ff..24e1a219 100644 --- a/hugegraph-llm/src/tests/api/test_rag_api.py +++ b/hugegraph-llm/src/tests/api/test_rag_api.py @@ -22,13 +22,14 @@ from fastapi import APIRouter, FastAPI, status from fastapi.testclient import TestClient from hugegraph_llm.api.rag_api import rag_http_api +from hugegraph_llm.config import huge_settings, llm_settings pytestmark = pytest.mark.contract # FIXME: add negative-path tests for each /config/* endpoint that assert # provider/auth/connection failures surface distinct error details. # FIXME: add happy-path HTTP contract tests for /rag and /rag/graph response -# shaping, output-flag filtering, and request-scoped graph config behavior. +# shaping and output-flag filtering. # FIXME: distinguish graph/provider dependency failures from programmer errors # instead of asserting one generic unexpected-error response for all failures. @@ -168,6 +169,71 @@ def test_rag_api_invalid_request_body_returns_validation_shape(): assert response.json()["detail"][0]["loc"][-1] == "query" +def test_rag_client_config_updates_only_explicit_graph_fields(monkeypatch): + monkeypatch.setattr(huge_settings, "graph_url", "http://original:8080";) + monkeypatch.setattr(huge_settings, "graph_name", "original_graph") + monkeypatch.setattr(huge_settings, "graph_user", "original_user") + monkeypatch.setattr(huge_settings, "graph_pwd", "original_pwd") + monkeypatch.setattr(huge_settings, "graph_space", "original_space") + client, callbacks = _make_test_client() + + response = client.post( + "/rag", + json={ + "query": "find vertices", + "client_config": { + "url": "http://override:8080";, + }, + }, + ) + + assert response.status_code == status.HTTP_200_OK + callbacks["rag_answer_func"].assert_called_once() + assert huge_settings.graph_url == "http://override:8080"; + assert huge_settings.graph_name == "original_graph" + assert huge_settings.graph_user == "original_user" + assert huge_settings.graph_pwd == "original_pwd" + assert huge_settings.graph_space == "original_space" + + +def test_llm_config_rejects_unsupported_provider_without_mutating(monkeypatch): + monkeypatch.setattr(llm_settings, "chat_llm_type", "openai") + client, callbacks = _make_test_client() + + response = client.post( + "/config/llm", + json={ + "llm_type": "unsupported", + "api_key": "sk-test", + "api_base": "https://api.example.test";, + "language_model": "gpt-test", + "max_tokens": "1024", + }, + ) + + assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY + callbacks["apply_llm_conf"].assert_not_called() + assert llm_settings.chat_llm_type == "openai" + + +def test_rerank_config_rejects_unsupported_provider_without_mutating(monkeypatch): + monkeypatch.setattr(llm_settings, "reranker_type", "cohere") + client, callbacks = _make_test_client() + + response = client.post( + "/config/rerank", + json={ + "reranker_type": "unsupported", + "api_key": "rerank-key", + "reranker_model": "rerank-test", + }, + ) + + assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY + callbacks["apply_reranker_conf"].assert_not_called() + assert llm_settings.reranker_type == "cohere" + + def test_text2gremlin_callback_exception_returns_stable_response(): client, _ = _make_test_client(gremlin_generate_selective_func=Mock(side_effect=RuntimeError("callback failed")))
