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]