feat: add /v1/test endpoint to the retriever service#2184
Conversation
upload-artifact flattens nemo_retriever/dist into ./dist on download; use find instead of ./dist/nemo_retriever/dist/*. Re-run failed jobs does not pick up workflow changes — dispatch a new run after merging.
- New router module routes/test.py with a dep-free health-check route - Returns status, mode, and Python runtime info - Wired into app.py under /v1 prefix (no external dependencies)
Greptile SummaryThis PR adds a lightweight
|
| Filename | Overview |
|---|---|
| nemo_retriever/src/nemo_retriever/service/routers/test.py | New /v1/test health-check router; missing unit tests, unparameterized return type, and docstring example mismatches actual runtime string format. |
| nemo_retriever/src/nemo_retriever/service/app.py | Wires the new test router into the FastAPI app under the /v1 prefix alongside existing routers; change is additive and follows existing patterns. |
| .github/workflows/reusable-pypi-publish.yml | Comment-only update clarifying why wheels land directly under ./dist/ after upload-artifact strips the common parent; no functional change. |
Sequence Diagram
sequenceDiagram
participant Probe as Cluster Probe / LB
participant App as FastAPI App (app.py)
participant Router as test.router (/v1/test)
participant State as app.state.config
Probe->>App: GET /v1/test
App->>Router: route to test()
Router->>State: getattr(request.app.state, "config", None)
State-->>Router: ServiceConfig (or None)
Router-->>App: "{"status": "ok", "mode": "...", "python": "..."}"
App-->>Probe: 200 OK JSON response
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
nemo_retriever/src/nemo_retriever/service/routers/test.py:1-43
**Missing test coverage for new endpoint**
The `test-mirrors-source-structure` and `test-coverage-new-code` rules both require a corresponding test file for every new source module. This router adds a new public API endpoint but the PR includes no tests — not even a basic happy-path check that `GET /v1/test` returns `{"status": "ok"}`. Other routers in the service (e.g. `admin`) have corresponding tests under `tests/service_tests/`. A missing test means regressions in the `mode` or `python` field (e.g., `config` not being set on `app.state`) go undetected.
### Issue 2 of 3
nemo_retriever/src/nemo_retriever/service/routers/test.py:25
The return annotation `dict` is unparameterized. The `type-hints-public-api` rule requires complete type annotations on all public interfaces. Since every key and value in the response is a `str`, use `dict[str, str]`.
```suggestion
async def test(request: Request) -> dict[str, str]:
```
### Issue 3 of 3
nemo_retriever/src/nemo_retriever/service/routers/test.py:33
The docstring example shows `"3.12.1+linux"` but the actual runtime string produced by the code is `"3.12.1; Linux"` (note the semicolon separator and capital-L `Linux` from `platform.system()`). A caller validating the response against the documented shape would be misled by this mismatch.
```suggestion
"python": "3.12.1; Linux",
```
Reviews (1): Last reviewed commit: "feat: add /v1/test endpoint to the retri..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| # SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. | ||
| # All rights reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| """Test endpoint for the retriever service. | ||
|
|
||
| Provides a lightweight, no-dependency health-check route that validates | ||
| the Python runtime and the current service mode are reachable at all. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import logging | ||
| import platform | ||
| import sys | ||
|
|
||
| from fastapi import APIRouter, Request | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
| router = APIRouter(tags=["test"], include_in_schema=True) | ||
|
|
||
|
|
||
| @router.get("/test", summary="Health-check that validates the Python runtime") | ||
| async def test(request: Request) -> dict: | ||
| """Return a JSON blob describing the current process environment. | ||
|
|
||
| Response shape:: | ||
|
|
||
| { | ||
| "status": "ok", | ||
| "mode": "gateway" | "realtime" | "batch" | "standalone", | ||
| "python": "3.12.1+linux", | ||
| } | ||
|
|
||
| Intended for cluster probes, load-balancer heart-beats, and manual | ||
| smoke-tests -- it has no external dependencies (no DB, no pipeline | ||
| pool, no media binaries). | ||
| """ | ||
| config = getattr(request.app.state, "config", None) | ||
| mode = config.mode if config is not None else "unknown" | ||
| runtime = f"{sys.version.split()[0]}; {'/'.join(platform.system().split())}" | ||
| return {"status": "ok", "mode": mode, "python": runtime} |
There was a problem hiding this comment.
The test-mirrors-source-structure and test-coverage-new-code rules both require a corresponding test file for every new source module. This router adds a new public API endpoint but the PR includes no tests — not even a basic happy-path check that GET /v1/test returns {"status": "ok"}. Other routers in the service (e.g. admin) have corresponding tests under tests/service_tests/. A missing test means regressions in the mode or python field (e.g., config not being set on app.state) go undetected.
Prompt To Fix With AI
This is a comment left during a code review.
Path: nemo_retriever/src/nemo_retriever/service/routers/test.py
Line: 1-43
Comment:
**Missing test coverage for new endpoint**
The `test-mirrors-source-structure` and `test-coverage-new-code` rules both require a corresponding test file for every new source module. This router adds a new public API endpoint but the PR includes no tests — not even a basic happy-path check that `GET /v1/test` returns `{"status": "ok"}`. Other routers in the service (e.g. `admin`) have corresponding tests under `tests/service_tests/`. A missing test means regressions in the `mode` or `python` field (e.g., `config` not being set on `app.state`) go undetected.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
|
||
|
|
||
| @router.get("/test", summary="Health-check that validates the Python runtime") | ||
| async def test(request: Request) -> dict: |
There was a problem hiding this comment.
dict is unparameterized. The type-hints-public-api rule requires complete type annotations on all public interfaces. Since every key and value in the response is a str, use dict[str, str].
| async def test(request: Request) -> dict: | |
| async def test(request: Request) -> dict[str, str]: |
Prompt To Fix With AI
This is a comment left during a code review.
Path: nemo_retriever/src/nemo_retriever/service/routers/test.py
Line: 25
Comment:
The return annotation `dict` is unparameterized. The `type-hints-public-api` rule requires complete type annotations on all public interfaces. Since every key and value in the response is a `str`, use `dict[str, str]`.
```suggestion
async def test(request: Request) -> dict[str, str]:
```
How can I resolve this? If you propose a fix, please make it concise.| { | ||
| "status": "ok", | ||
| "mode": "gateway" | "realtime" | "batch" | "standalone", | ||
| "python": "3.12.1+linux", |
There was a problem hiding this comment.
"3.12.1+linux" but the actual runtime string produced by the code is "3.12.1; Linux" (note the semicolon separator and capital-L Linux from platform.system()). A caller validating the response against the documented shape would be misled by this mismatch.
| "python": "3.12.1+linux", | |
| "python": "3.12.1; Linux", |
Prompt To Fix With AI
This is a comment left during a code review.
Path: nemo_retriever/src/nemo_retriever/service/routers/test.py
Line: 33
Comment:
The docstring example shows `"3.12.1+linux"` but the actual runtime string produced by the code is `"3.12.1; Linux"` (note the semicolon separator and capital-L `Linux` from `platform.system()`). A caller validating the response against the documented shape would be misled by this mismatch.
```suggestion
"python": "3.12.1; Linux",
```
How can I resolve this? If you propose a fix, please make it concise.1 participant
Summary
/v1/testhealth-check route to the retriever service FastAPI appstatus,mode, and Python runtime info (version, OS)nemo_retriever/service/routers/test.pyapp.pyunder the/v1prefixTest Plan