Skip to content

feat: add health and readiness check endpoints to CLI serve API #1100

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
markstur wants to merge 2 commits into
base: main
Choose a base branch
from
+26 −4
Open

feat: add health and readiness check endpoints to CLI serve API #1100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fa4df43
feat: add health and readiness check endpoints to CLI serve API
markstur May 19, 2026
e775d8b
feat: cli serve health endpoint
markstur May 20, 2026
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
14 changes: 13 additions & 1 deletion cli/serve/app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import sys
import time
import uuid
from typing import Any, Literal, cast
from typing import Any, cast

try:
import typer
Expand Down Expand Up @@ -52,6 +52,18 @@
)


@app.get("/health", status_code=200)
Copy link
Copy Markdown
Contributor

@planetf1 planetf1 May 21, 2026

Choose a reason for hiding this comment

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

FYI: status_code=200 is FastAPI's default for GET so it's a no-op — the code works correctly either way. Just flagging in case you want to trim it:

Suggested change
@app.get("/health", status_code=200)
@app.get("/health")

async def health_check():
Copy link
Copy Markdown
Contributor

@planetf1 planetf1 May 21, 2026

Choose a reason for hiding this comment

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

Missing return type — validation_exception_handler below has one, so this reads a bit inconsistent. Easy fix:

Suggested change
async def health_check():
async def health_check() -> dict[str, str]:

"""Basic liveness check endpoint.

Returns a 200 OK status to signal that the Python process is alive and responding.

Returns:
dict: A dictionary with status "pass".
"""
return {"status": "pass"}


@app.exception_handler(RequestValidationError)
async def validation_exception_handler(
request: Request, exc: RequestValidationError
Expand Down
16 changes: 13 additions & 3 deletions test/cli/test_serve.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from fastapi.testclient import TestClient
from pydantic import BaseModel, ValidationError

from cli.serve.app import make_chat_endpoint, validation_exception_handler
from cli.serve.app import app, make_chat_endpoint, validation_exception_handler
from cli.serve.models import (
ChatCompletion,
ChatCompletionRequest,
Expand Down Expand Up @@ -45,6 +45,18 @@ def sample_request():
)


class TestHealthCheckEndpoint:
"""Tests for the health check endpoint."""

def test_health_check(self):
"""Test that /health endpoint returns 200 with correct JSON response."""
client = TestClient(app)
Copy link
Copy Markdown
Contributor

@planetf1 planetf1 May 21, 2026

Choose a reason for hiding this comment

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

Nit: the other test classes here use fixtures or class-level setup rather than constructing the client inline, but this is totally fine for a single test. Just worth a comment so the next person adding tests knows this is intentional:

Suggested change
client = TestClient(app)
# /health is registered at module-load time — TestClient(app) is correct here
client = TestClient(app)

response = client.get("/health")

assert response.status_code == 200
assert response.json() == {"status": "pass"}
Copy link
Copy Markdown
Contributor

@planetf1 planetf1 May 21, 2026

Choose a reason for hiding this comment

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

Nit: one edge case worth pinning while you're here — what does POST /health return? FastAPI will give 405 by default, which is the right behaviour (a misconfigured probe fails loudly rather than silently passing). A quick check:

    def test_health_check_rejects_post(self):
        client = TestClient(app)
        assert client.post("/health").status_code == 405



class TestChatEndpoint:
"""Tests for the chat completion endpoint."""

Expand Down Expand Up @@ -722,8 +734,6 @@ async def test_text_format_no_schema(self, mock_module):
@pytest.mark.asyncio
async def test_json_schema_missing_schema_field(self, mock_module):
"""Test that json_schema without schema field raises ValidationError."""
from pydantic import ValidationError

# Should raise ValidationError when creating ResponseFormat
with pytest.raises(ValidationError) as exc_info:
ResponseFormat(
Expand Down
Loading