Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/reusable-pypi-publish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ jobs:
DIST_FILES=(./dist/nemo_retriever/dist/*.whl ./dist/nemo_retriever/dist/*.tar.gz)
echo "After legacy glob: count=${#DIST_FILES[@]} files=${DIST_FILES[*]:-<none>}"

# Flat layout (older upload-artifact glob uploads): ./dist/*
# Flat layout (upload-artifact strips to the common parent, so wheels
# often land directly under ./dist/): ./dist/*
if [ "${#DIST_FILES[@]}" -eq 0 ]; then
DIST_FILES=(./dist/*.whl ./dist/*.tar.gz)
echo "After flat glob: count=${#DIST_FILES[@]} files=${DIST_FILES[*]:-<none>}"
Expand Down
7 changes: 4 additions & 3 deletions nemo_retriever/src/nemo_retriever/service/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -271,14 +271,15 @@ def create_app(config: ServiceConfig) -> FastAPI:
else:
logger.info("Bearer-token authentication DISABLED (no api_token configured)")

from nemo_retriever.service.routers import admin, ingest, metrics
from nemo_retriever.service.routers import admin, ingest, metrics, test
from nemo_retriever.service.services.prometheus import instrument_app

app.include_router(ingest.router, prefix="/v1")
app.include_router(metrics.router, prefix="/v1")
# Admin/internal endpoints — pool_stats etc. Registered on every
# role; the handler self-reports an empty pool dict on gateway pods.
# Admin/internal endpoints — pool_stats etc.
app.include_router(admin.router, prefix="/v1")
# General-purpose smoke-test route — no external deps.
app.include_router(test.router, prefix="/v1")
instrument_app(app, role=config.mode)

if config.mode == "gateway":
Expand Down
43 changes: 43 additions & 0 deletions nemo_retriever/src/nemo_retriever/service/routers/test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# 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:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Suggested change
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.

"""Return a JSON blob describing the current process environment.

Response shape::

{
"status": "ok",
"mode": "gateway" | "realtime" | "batch" | "standalone",
"python": "3.12.1+linux",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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.

Suggested change
"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.

}

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}
Comment on lines +1 to +43
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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.

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!

Loading