imbajin commented on code in PR #357:
URL: https://github.com/apache/hugegraph-ai/pull/357#discussion_r3340652771
##########
.github/workflows/hugegraph-python-client.yml:
##########
@@ -7,8 +7,62 @@ on:
- 'release-*'
pull_request:
+permissions:
+ contents: read
+
+env:
+ UV_VERSION: "0.9.22"
+
jobs:
- build:
+ client-unit-contract:
+ runs-on: ubuntu-latest
+ strategy:
+ fail-fast: false
+ matrix:
+ python-version: ["3.10", "3.11", "3.12"]
Review Comment:
‼️ **The 3.12 matrix does not actually validate Python 3.12**
Evidence: from this head, `uv sync --extra python-client --extra dev
--python 3.12 --locked --dry-run` fails because the workspace still includes
`hugegraph-llm` with `requires-python = ">=3.10,<3.12"`. Running the workflow
command without `--python 3.12` resolves the project environment back to Python
3.10 locally, so a green `3.12` matrix leg does not prove the client package
works on 3.12. Please either split the client job out of the root workspace
constraint or drop 3.12 until the whole workspace can install under it.
##########
hugegraph-llm/src/hugegraph_llm/config/models/base_prompt_config.py:
##########
@@ -16,17 +16,26 @@
# under the License.
import os
-import sys
+from importlib import resources
from pathlib import Path
import yaml
-from hugegraph_llm.utils.anchor import get_project_root
from hugegraph_llm.utils.log import log
dir_name = os.path.dirname
F_NAME = "config_prompt.yaml"
-yaml_file_path = os.path.join(os.getcwd(), "src/hugegraph_llm/resources/demo",
F_NAME)
+
+
+def resolve_prompt_yaml_path() -> str:
Review Comment:
⚠️ **Prompt config still writes back into package resources**
Evidence: this resolver now points at
`resources.files("hugegraph_llm.resources.demo").joinpath(F_NAME)`, but
`save_to_yaml()` still opens that resolved path for writing and `create_app()`
calls `prompt.update_yaml_file()` during startup. That is fragile for installed
wheels or read-only `site-packages`, where package resources should be treated
as immutable. Please copy mutable prompt config to an app/user config path
before writing, or skip write-back when the package resource is not writable.
##########
.github/workflows/hugegraph-llm.yml:
##########
@@ -68,15 +72,148 @@ jobs:
uv run python -c "import nltk; nltk.download('stopwords');
nltk.download('punkt')"
- name: Run unit tests
- working-directory: hugegraph-llm
env:
SKIP_EXTERNAL_SERVICES: true
run: |
- uv run pytest src/tests/config/ src/tests/document/
src/tests/middleware/ src/tests/operators/ src/tests/models/ src/tests/indices/
src/tests/test_utils.py -v --tb=short
+ uv run pytest hugegraph-llm/src/tests -m "unit or contract"
--cov=hugegraph_llm --cov-fail-under=34 --cov-report=term
--cov-report=xml:llm-unit-contract.xml -v --tb=short --durations=20
Review Comment:
‼️ **Marker split drops existing LLM tests from CI**
Evidence: the old job explicitly ran broad paths including
`src/tests/indices/` and `src/tests/test_utils.py`, while this new job only
runs `-m "unit or contract"`. On this head, `uv run --extra llm --extra dev
pytest hugegraph-llm/src/tests -m "unit or contract" --collect-only -q`
collects `173/309` tests and deselects `136`, including unmarked
non-integration tests such as `test_utils.py`, index tests, and common operator
tests. Please either mark the remaining deterministic tests or add a
no-marker/default lane so the CI rewrite does not reduce coverage.
--
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]
