bitflicker64 opened a new pull request, #403:
URL: https://github.com/apache/hugegraph-ai/pull/403
## Purpose
A scan of `hugegraph-mcp` found seven small bugs. Most are inputs that
should produce a validation error but crash with a raw `TypeError`,
`AttributeError` or `OverflowError` instead. Each fix is a few lines and comes
with a test that fails on `main`. Tool signatures, input and output schemas,
envelope shapes and error types are unchanged, and so is the result for every
valid input.
## Main changes
| File | Bug | Fix |
|---|---|---|
| `gremlin_tools.py` | `requests.Response.__bool__` returns `ok`, which is
`False` for 4xx/5xx, so `e.response and ...` turned the status of every real
`HTTPError` into `"unknown"`. The existing tests used truthy `Mock` responses
and never hit this. | Compare the response with `None`. Real responses now get
the mapping the existing tests already expect (401, 403, 404, 500, 503).
pyhugegraph usually converts `HTTPError` before it reaches this handler, so few
users will see the difference. |
| `plan_store.py` | `with sqlite3.connect(...) as connection:` commits or
rolls back but never closes, so every store call left a connection to the
garbage collector (`ResourceWarning: unclosed database` on Python 3.13+). | Use
`with closing(sqlite3.connect(...)) as connection, connection:` at all ten
sites, the pattern `confirmation_store.py` already uses. The transaction exits
first, then the connection closes, so commit, rollback and lock timing are
unchanged. |
| `tools/mutate_graph_properties.py` | An `append` to a LIST or SET property
with `None` passed validation, then the preview ran `*value` and raised
`TypeError`. A non-array value did the same when the property key has no
`data_type`. | Reject a non-list append value for a collection property with
the usual `VALIDATION_ERROR`. Valid list appends and SINGLE properties are
unchanged. |
| `tools/manage_schema.py` | `_collect_planned_creates` runs before
identifier validation and put `name` into a set, so a list or dict name raised
`TypeError: unhashable type`. | Skip non-string names there. Identifier
validation already reports them as `name must be a non-empty string`. |
| `tools/ingest_graph_data.py` | The duplicate-edge check read only
`source_label`, `target_label`, `source` and `target`. Edges written with the
supported `outV`, `inV`, `outVLabel` and `inVLabel` keys all produced the same
key, so any two edges with one label raised "potential duplicate edges
detected". | Build the key with the existing `_edge_endpoint()` helper. The key
is unchanged for valid `source`/`target` edges, and the result is only a
warning. |
| `tools/manage_graph_data.py` | In delete mode the isolated-vertex check
called `operation.get(...)` before validation, so `{"operations": ["oops"]}`
raised `AttributeError`. | Skip non-object operations in that check.
`_validate_mode_operations` then reports `operation must be an object`. |
| `plan_hash.py` | `_coerce_expires_at` caught `TypeError` and `ValueError`
but not `OverflowError`, so `expires_at=float("inf")` crashed
`verify_plan_hash`. NaN and strings were already treated as expired. | Catch
`OverflowError` too, so infinity is treated as expired. |
## Verification
Run on Linux with Python 3.14, at commit `5c6ef2a` on top of `apache/main`
`a90b1c4`, with the environment built the same way as in
`.github/workflows/hugegraph-mcp.yml`:
```bash
uv venv .venv-mcp
uv --no-config pip install --python .venv-mcp/bin/python \
-e ./hugegraph-python-client -e ./hugegraph-mcp "pytest>=9.2,<10"
SKIP_EXTERNAL_SERVICES=true .venv-mcp/bin/python -m pytest hugegraph-mcp \
-m "not live and not integration and not llm"
uv run --extra dev ruff format --check . # 582 files already formatted
uv run --extra dev ruff check . # All checks passed!
ty check --python .venv-mcp/bin/python hugegraph-mcp/hugegraph_mcp # 33
diagnostics, the same 33 as on main
```
| Run | Result |
|---|---|
| `main` | 885 passed, 17 deselected |
| This PR | 921 collected: 904 passed, 17 deselected |
| This PR's tests with the seven production files reverted | 16 failed, 888
passed |
Each new regression test fails only when its own fix is reverted. The 17
deselected tests are the `real_hugegraph` tests in
`tests/integration/test_real_write_path.py`.
## 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]
