Skip to content

test: check if code is API breaking#2141

Open
fl0rianr wants to merge 2 commits into
mainfrom
fl0rianr/ci_validate_api
Open

test: check if code is API breaking#2141
fl0rianr wants to merge 2 commits into
mainfrom
fl0rianr/ci_validate_api

Conversation

@fl0rianr

@fl0rianr fl0rianr commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • add a static public API route contract manifest for Lemonade's HTTP and websocket routes
  • add a dependency-free CI check that compares the manifest with the C++ route registrations
  • cover the current public route surface, including cloud auth, MCP, and version-prefixed websocket endpoints
  • run the check on PRs, pushes to main, and merge queue entries

Fixes #2137

Why

Issue #2137 asks for CI protection against breaking API changes. The public API is spread across Lemonade's OpenAI-compatible routes, Lemonade-specific routes, Ollama-compatible routes, Anthropic-compatible /v1/messages, and websocket endpoints.

This PR adds a lightweight guardrail that fails when a protected public route is accidentally removed, renamed, or has its HTTP method changed. It intentionally avoids building the server or downloading models, so it stays fast and reliable in CI.

Scope

This protects the public route-level contract: HTTP method + path, including dynamic path segments and websocket paths. It does not validate request or response JSON schemas; those should be added later once the API has a generated OpenAPI/schema source of truth.

Testing

python test/test_api_contract.py

Expected output:

Public API contract check passed (160 expected routes).

I also checked that removing a protected route makes the test fail with a focused missing-route message.
e.g. Since main moved forward the old version before this PR update already complained before being updated.
image

@fl0rianr fl0rianr requested a review from jeremyfowers June 8, 2026 18:01
@github-actions github-actions Bot added enhancement New feature or request area::api HTTP REST API surface and route handlers labels Jun 8, 2026
@superm1

superm1 commented Jun 11, 2026

Copy link
Copy Markdown
Member

Probably from other code that merged but I notice:

The regex r"path\s*==\s*"([^\"]+)"" in extract_websocket_routes() does not match the actual code in websocket_server.cpp. The comparison uses stripped == "/realtime" and stripped == "/logs/stream", not
path == "...".

@superm1 superm1 force-pushed the fl0rianr/ci_validate_api branch from 0ee23d1 to add6f7c Compare June 11, 2026 23:56
fl0rianr added 2 commits June 22, 2026 15:28
Update the static API contract check to extract WebSocket routes from
WebSocketServer::classify_path(), including version-prefixed websocket paths.

Extend the manifest to cover the current public route surface: cloud auth,
MCP, and version-prefixed websocket endpoints.
@fl0rianr fl0rianr force-pushed the fl0rianr/ci_validate_api branch from a0d1a09 to be80d6f Compare June 22, 2026 13:28
@fl0rianr fl0rianr requested a review from superm1 June 22, 2026 13:44
@fl0rianr

Copy link
Copy Markdown
Collaborator Author

@superm1 Thanks, good catch. The checker was only matching path == ..., but the current websocket implementation normalizes into stripped inside WebSocketServer::classify_path().

I updated the extractor to derive the bare and version-prefixed websocket routes from classify_path(), and updated the manifest to cover the current public API surface including cloud auth and MCP.

@fl0rianr fl0rianr added this to the Lemonade v10.9 milestone Jun 22, 2026

@superm1 superm1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This feels fragile to parse C/C++. What's wrong with compiling and checking the interfaces work?

@fl0rianr

Copy link
Copy Markdown
Collaborator Author

This feels fragile to parse C/C++. What's wrong with compiling and checking the interfaces work?

That's a fair concern. I agree that a full compile-and-run interface test would be stronger.

The intent of this PR is narrower: a cheap route-surface guardrail that runs quickly in CI and catches accidental removal/rename/method changes of public routes without needing to build/start the server, configure backends, or load models. Compile-only would not catch route contract drift, and a true runtime API test would be a larger integration-test follow-up.

I tightened the static check so it only recognizes the route registration idioms currently used by the server, but I can also make the limitation explicit in the PR description/docstring: this is a first-line static route contract check, not a replacement for runtime API compatibility tests.

A large full CI test makes sense but I'm also slightly hesitant bloating CI more. This makes sense for sure but is a separate PR IMO and this one has it's pros as well since it so fast it can be a docs and style check.

How would you like to proceed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area::api HTTP REST API surface and route handlers enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI to protect against breaking API changes

2 participants