bitflicker64 opened a new pull request, #404:
URL: https://github.com/apache/hugegraph-ai/pull/404

   ## Purpose
   
   A scan of `hugegraph-llm` found three small bugs and a slow membership 
check. This PR fixes them, each with a test that fails on `main`, and adds 
tests for two existing vector-store behaviours. It does not change prompts, 
retrieval or ranking, request or response models, config keys or index formats.
   
   ## Main changes
   
   - `operators/common_op/check_schema.py`: `_validate_edge_label` never 
checked that `properties` is a list. A user schema with `"properties": "since"` 
passed, and `_add_missing_properties` iterated the string, so the schema gained 
the property keys `s`, `i`, `n`, `c` and `e`. It now raises the module's usual 
`ValueError`, as vertex labels already do and as the graph-extract request 
model already requires (`api/models/graph_extract_requests.py:91`). Edge labels 
without `properties` and with a list are unchanged. A string, a dict or `None` 
passed as edge `properties` through the RAG, Text2Gremlin or Gradio schema 
input is now rejected; before, a dict was read as its keys, a string was split 
into characters and `None` raised `TypeError`.
   - `middleware/middleware.py`: for requests slower than one second the value 
was divided by 1000 for the `X-Process-Time` header, but the log line still 
printed it with a `ms` unit, so a 2.5 s request logged `2.50 ms`. The log now 
gets the millisecond value and the header is unchanged. The access log line 
also read `request.client.host`, and Starlette's `Request.client` is `None` 
when the ASGI scope has no client. It now logs `None` there instead of raising 
after the endpoint has run.
   - `indices/vector_index/milvus_vector_store.py`: on an embedding dimension 
mismatch, `__init__` dropped the existing collection and then opened 
`Collection(self.name)` without a schema, which fails because the collection no 
longer exists. It now calls `_create_collection()` after the drop, the same as 
`QdrantVectorIndex`. A matching dimension still reuses the collection.
   - `indices/vector_index/faiss_vector_store.py`: `remove` checked each 
position against a list of removed positions, which is quadratic. It now uses a 
set. The result is the same.
   - Tests: `test_check_schema.py` covers the rejection and that list 
properties on an edge label are added as property keys with the default data 
type and cardinality. `test_middleware.py` covers the slow-request log and a 
request without a client. `test_optional_vector_clients.py` drives 
`MilvusVectorIndex.__init__` against a fake `pymilvus` for a changed and an 
unchanged dimension. `test_faiss_vector_index.py` covers loading a saved index 
with a different dimension and the property order after `remove`.
   
   ## Verification
   
   Run on Linux with Python 3.14 through uv 0.13.5, at commit `0865073` on top 
of `apache/main` `a90b1c4`, after `uv sync --all-extras` so the `vectordb` 
extra is installed as in CI:
   
   ```bash
   uv run --extra dev ruff format --check .      # 581 files already formatted
   uv run --extra dev ruff check .               # All checks passed!
   uv run --extra dev ty check hugegraph-llm/src/hugegraph_llm   # one 
diagnostic fewer than main, none new
   SKIP_EXTERNAL_SERVICES=true uv run pytest hugegraph-llm/src/tests \
     -m "not integration and not hugegraph and not smoke and not external"
   ```
   
   | Run | Result |
   |---|---|
   | This PR | 1264 passed, 3 skipped, 18 deselected, 14 failed |
   | This PR's tests with the production changes reverted | 1260 passed, 3 
skipped, 18 deselected, 18 failed |
   
   The 14 failures that appear in both runs are `test_config.py` (1) and 
`test_nltk_helper.py` (13). They need NLTK corpora that the CI job downloads 
and this machine does not have, and they fail the same way on `main`. The 4 
extra failures with the fixes reverted are the new regression tests: the two 
middleware tests, `test_schema_check_rejects_non_list_edge_properties` and the 
changed-dimension case of 
`test_milvus_index_recreates_collection_only_when_dimension_changes`.
   
   The `(integration and hugegraph) or smoke` job was not run locally: the 
shared test host was busy, and none of the changed code talks to HugeGraph 
Server.
   
   ## Does this PR potentially affect the following parts?
   
   - [ ] Dependencies
   - [ ] Modify configurations
   - [ ] The public API
   - [ ] Other affects
   - [x] Nope
   


-- 
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]

Reply via email to