Skip to content

feat(server): add /health/ready readiness endpoint#405

Merged
SantiagoDePolonia merged 2 commits into
mainfrom
feat/health-ready-endpoint
Jun 16, 2026
Merged

feat(server): add /health/ready readiness endpoint#405
SantiagoDePolonia merged 2 commits into
mainfrom
feat/health-ready-endpoint

Conversation

@SantiagoDePolonia

@SantiagoDePolonia SantiagoDePolonia commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a readiness probe (GET /health/ready) distinct from the existing /health liveness check, matching the gateway's actual failure model.

The key design choice: readiness probes only dependencies the gateway owns; upstream providers are deliberately excluded. Gating readiness on provider reachability would let a transient OpenAI/Anthropic outage pull every healthy replica out of rotation simultaneously — turning a recoverable degradation into an outage.

Probe Healthy when HTTP
/health (liveness, unchanged) process serves HTTP 200
/health/ready (readiness, new) storage reachable 200 ready
storage ok, Redis cache down 200 degraded (still serving)
storage unreachable 503 not_ready

Response shape:

{ "status": "ready", "components": { "storage": "ok", "cache": "ok" } }

What changed

  • Endpoint (internal/server/readiness.go): public route (added to authSkipPaths), bounded 2s per-dependency ping that also honors request-context cancellation. Storage failure → not_ready (503); Redis exact-cache failure → degraded (200, non-blocking).
  • Decoupling: new server.ReadinessProbe interface; storage.HealthChecker and cache.Pinger expose Ping(ctx) on the SQLite/PostgreSQL/MongoDB backends and RedisStore. Wired in app.go — storage always, cache only when a Redis exact cache is configured (ResponseCacheMiddleware.UsesRedis()).
  • CLI: new --ready / --ready-timeout (default 4s, larger than the server's 2s per-probe timeout so a slow backend returns a clean not_ready instead of the client cutting off). Exits 0 on ready/degraded, non-zero on not_ready. --health and the Docker HEALTHCHECK are unchanged (liveness is the right restart signal).
  • Docs: README endpoint table; docs/advanced/cli.mdx readiness section with K8s readinessProbe vs Docker HEALTHCHECK guidance; regenerated swagger/OpenAPI (additions only).

User-visible impact

  • New unauthenticated GET /health/ready for orchestrator readiness gating (e.g. K8s readinessProbe).
  • New gomodel --ready CLI probe.
  • No change to /health, --health, or the Docker HEALTHCHECK.

Provider-specific behavior

None. Provider reachability is intentionally not part of any orchestrator-facing probe.

Tests

  • internal/server/readiness_test.go — table-driven handler (ready/degraded/not_ready precedence) + auth-skip.
  • cmd/gomodel/ready_test.go — CLI probe URL + status handling.
  • internal/responsecache/readiness_test.goUsesRedis/Ping for pinger vs in-memory stores.
  • internal/storage/sqlite_test.goTestSQLitePing.

All pre-commit hooks pass (race tests, lint, mint validate).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added /health/ready readiness endpoint that checks storage and cache, returning 200 for ready/degraded and 503 for not_ready
    • Added /v1/realtime endpoint for OpenAI-compatible realtime (websocket) sessions
    • Added CLI flags --ready and --ready-timeout to perform readiness checks at startup
  • Documentation
    • Updated README, CLI docs, and OpenAPI/Swagger docs to reflect /health/ready and /v1/realtime
  • Tests
    • Added unit tests covering readiness probe URL building, readiness HTTP behavior, and dependency health outcomes

Adds a readiness probe distinct from the existing /health liveness check,
following the gateway's actual failure model.

- GET /health/ready pings gateway-owned dependencies with a bounded 2s
  per-probe timeout. Storage is required (down -> not_ready, HTTP 503);
  the Redis exact cache is a performance optimization (down -> degraded,
  HTTP 200, still serving). Upstream providers are deliberately excluded
  so a provider outage cannot pull a healthy gateway out of rotation.
- New ReadinessProbe interface keeps the server decoupled from concrete
  storage/cache types; storage.HealthChecker and cache.Pinger expose
  Ping(ctx) on the backends, wired through app.go.
- CLI: add --ready / --ready-timeout (default 4s, larger than the
  server's per-probe timeout) returning 0 on ready/degraded, non-zero on
  not_ready. Liveness (--health) and the Docker HEALTHCHECK are unchanged.
- Docs: README endpoint table, docs/advanced/cli.mdx readiness section
  with K8s readinessProbe vs Docker HEALTHCHECK guidance, regenerated
  swagger/OpenAPI.
- Tests: server handler (incl. auth-skip), CLI probe, responsecache
  UsesRedis/Ping, and storage Ping.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a GET /health/ready readiness endpoint that probes storage (HealthChecker) and Redis cache (Pinger) backends, returning ready, degraded, or not_ready JSON status. Storage failure yields HTTP 503; cache failure yields HTTP 200 degraded. A --ready CLI flag enables readiness probe mode. OpenAPI specs, README, and CLI docs are updated accordingly.

Changes

Readiness Probe Feature

Layer / File(s) Summary
Storage HealthChecker interface and backend Ping methods
internal/storage/storage.go, internal/storage/mongodb.go, internal/storage/postgresql.go, internal/storage/sqlite.go, internal/storage/sqlite_test.go
Introduces the HealthChecker interface with Ping(ctx) error and implements it on all three storage backends (SQLite, PostgreSQL, MongoDB), with a SQLite test verifying pre- and post-close behavior.
Cache Pinger interface, RedisStore.Ping, and ResponseCacheMiddleware readiness
internal/cache/store.go, internal/cache/redis.go, internal/responsecache/responsecache.go, internal/responsecache/readiness_test.go
Introduces cache.Pinger interface, implements Ping on RedisStore, adds UsesRedis() and Ping() to ResponseCacheMiddleware, and tests nil/non-pinger/reachable/failing-pinger scenarios.
Server ReadinessProbe interface, Handler.Ready, and route registration
internal/server/http.go, internal/server/handlers.go, internal/server/readiness.go, internal/server/readiness_test.go
Defines ReadinessProbe interface and Config probe fields, adds storageProbe/cacheProbe to Handler, implements Handler.Ready with per-probe timeouts and storage-503/cache-degraded-200 logic, registers GET /health/ready unauthenticated, and covers behavior with table-driven and auth-skip tests.
App startup probe wiring
internal/app/app.go
Conditionally wires serverCfg.StorageProbe via HealthChecker type assertion and serverCfg.CacheProbe via rcm.UsesRedis() at startup.
CLI --ready flag, runReadyProbe, checkReadyEndpoint, and main wiring
cmd/gomodel/flags.go, cmd/gomodel/health.go, cmd/gomodel/main.go, cmd/gomodel/ready_test.go
Adds defaultReadyTimeout, Ready/ReadyTimeout to cliOptions, --ready/--ready-timeout flags, refactors probeURL to accept a path argument, introduces runReadyProbe and checkReadyEndpoint (treating ready/degraded as success), wires opts.Ready in main(), and tests URL construction and HTTP response parsing.
Documentation and OpenAPI spec updates
README.md, docs/advanced/cli.mdx, docs/openapi.json, cmd/gomodel/docs/docs.go
Updates README, CLI docs, OpenAPI JSON, and generated Swagger spec to document /health/ready semantics, --ready/--ready-timeout flags, Kubernetes/Docker guidance, and adds the GET /v1/realtime API path.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant Server as GET /health/ready
  participant StorageProbe as storage.HealthChecker
  participant CacheProbe as ResponseCacheMiddleware
  Client->>Server: GET /health/ready (no auth required)
  Server->>StorageProbe: pingWithTimeout(ctx, readinessProbeTimeout)
  alt storage ping fails
    StorageProbe-->>Server: error
    Server-->>Client: 503 {"status":"not_ready","components":{"storage":"down"}}
  else storage ping ok
    StorageProbe-->>Server: nil
    Server->>CacheProbe: pingWithTimeout(ctx, readinessProbeTimeout)
    alt cache ping fails
      CacheProbe-->>Server: error
      Server-->>Client: 200 {"status":"degraded","components":{"storage":"ok","cache":"down"}}
    else cache ping ok
      CacheProbe-->>Server: nil
      Server-->>Client: 200 {"status":"ready","components":{"storage":"ok","cache":"ok"}}
    end
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • ENTERPILOT/GoModel#404: Modifies the same cmd/gomodel/flags.go, cmd/gomodel/health.go, and cmd/gomodel/main.go health-probe plumbing that this PR extends with readiness probe support.
  • ENTERPILOT/GoModel#182: Refactors the storage interface layer in internal/storage/storage.go where this PR adds the HealthChecker interface and Ping methods on storage backends.
  • ENTERPILOT/GoModel#196: Modifies the response-cache middleware in internal/responsecache/responsecache.go where this PR adds Redis readiness helpers (UsesRedis/Ping).

Suggested labels

release:feature

Poem

🐇 Hop, hop! A new path is born today,
/health/ready checks if storage's okay,
Redis pings back — "degraded" or "fine,"
No more blindly routing down the line.
With --ready the rabbit confirms: "All clear!"
Kubernetes rejoices, the endpoint is here! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(server): add /health/ready readiness endpoint' accurately summarizes the main change: introducing a new readiness probe endpoint distinct from the existing liveness check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/health-ready-endpoint

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps

greptile-apps Bot commented Jun 16, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds a readiness probe separate from the existing liveness check. The main changes are:

  • New unauthenticated GET /health/ready endpoint.
  • Storage ping controls ready versus not_ready responses.
  • Redis exact-cache ping can report degraded while keeping HTTP 200.
  • New gomodel --ready CLI probe with its own timeout.
  • Documentation and OpenAPI updates for the new readiness behavior.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
internal/server/readiness.go Adds the readiness handler with bounded storage and cache checks.
internal/app/app.go Wires shared storage and Redis exact-cache probes into server configuration.
cmd/gomodel/health.go Adds the CLI readiness probe and shared probe URL construction.
internal/responsecache/responsecache.go Exposes Redis-backed exact-cache readiness through UsesRedis and Ping.

Reviews (1): Last reviewed commit: "feat(server): add /health/ready readines..." | Re-trigger Greptile

@mintlify

mintlify Bot commented Jun 16, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
gomodel 🟢 Ready View Preview Jun 16, 2026, 11:01 AM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 63.63636% with 36 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
cmd/gomodel/health.go 55.17% 11 Missing and 2 partials ⚠️
cmd/gomodel/main.go 0.00% 5 Missing ⚠️
internal/app/app.go 0.00% 4 Missing ⚠️
internal/cache/redis.go 0.00% 4 Missing ⚠️
internal/storage/mongodb.go 0.00% 4 Missing ⚠️
internal/storage/postgresql.go 0.00% 4 Missing ⚠️
internal/storage/sqlite.go 50.00% 1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/advanced/cli.mdx`:
- Around line 65-97: The phrases "Liveness (`--health`) is the right signal" and
"Readiness (`/health/ready`) is the right signal" repeat the same structure and
wording in consecutive sentences. To improve readability, vary the phrasing by
either replacing "right" with an alternative adjective such as "appropriate" in
one of the sentences, or restructure both sentences using different grammatical
patterns (for example, converting one to an imperative or different verb
structure). Choose whichever approach better matches the documentation's tone
and style.

In `@docs/openapi.json`:
- Around line 2119-2138: The `/health/ready` endpoint response schemas in both
the 200 and 503 responses currently use generic objects with
additionalProperties set to true, which does not match the documented contract
that should include specific status and components fields. Replace the
permissive schema definitions (the object type with additionalProperties: true)
in both response definitions with a properly defined schema that explicitly
declares the status and components properties as required fields, removing
additionalProperties: true to enforce the contract structure and generate
accurate client types.

In `@internal/server/readiness.go`:
- Around line 49-50: The Swagger annotations for the Ready function in
internal/server/readiness.go use generic map[string]interface{} types in both
the `@Success` and `@Failure` annotations, but the function actually returns a
concrete readinessResponse type. Replace map[string]interface{} with
readinessResponse in both the `@Success` and `@Failure` Swagger annotations to align
the OpenAPI schema with the actual return type and maintain type consistency in
the API documentation.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: eaafb5dc-3a5d-4c1d-ae99-925d7b43fad8

📥 Commits

Reviewing files that changed from the base of the PR and between 9115c3b and 63ab1a5.

📒 Files selected for processing (22)
  • README.md
  • cmd/gomodel/docs/docs.go
  • cmd/gomodel/flags.go
  • cmd/gomodel/health.go
  • cmd/gomodel/main.go
  • cmd/gomodel/ready_test.go
  • docs/advanced/cli.mdx
  • docs/openapi.json
  • internal/app/app.go
  • internal/cache/redis.go
  • internal/cache/store.go
  • internal/responsecache/readiness_test.go
  • internal/responsecache/responsecache.go
  • internal/server/handlers.go
  • internal/server/http.go
  • internal/server/readiness.go
  • internal/server/readiness_test.go
  • internal/storage/mongodb.go
  • internal/storage/postgresql.go
  • internal/storage/sqlite.go
  • internal/storage/sqlite_test.go
  • internal/storage/storage.go

Comment thread docs/advanced/cli.mdx
Comment on lines +65 to +97
## Readiness probe

`--ready` probes `/health/ready`, which reports whether the instance should
receive traffic. Unlike liveness, readiness checks the dependencies the gateway
owns:

- **Storage** is required. If the backend (SQLite/PostgreSQL/MongoDB) is
unreachable, readiness reports `not_ready` (HTTP `503`).
- **Redis exact cache** (when configured) is a performance optimization. If it
is unreachable, readiness reports `degraded` but stays HTTP `200` — the
gateway still serves requests.

Upstream **provider** reachability is deliberately excluded: a provider outage
must not pull a healthy gateway out of rotation.

```bash
gomodel --ready
```

- Exits `0` when the endpoint returns HTTP `200` (`ready` or `degraded`).
- Exits non-zero on `not_ready` (HTTP `503`), connection refused, or an
unexpected status value.

```json
{ "status": "ready", "components": { "storage": "ok", "cache": "ok" } }
```

Liveness (`--health`) is the right signal for a Docker `HEALTHCHECK` (restart on
crash). Readiness (`/health/ready`) is the right signal for a Kubernetes
`readinessProbe` (gate traffic) — point it at the HTTP endpoint directly or run
`gomodel --ready` as an exec probe. Keep `--ready-timeout` larger than the
server's internal per-dependency probe timeout (`2s`) so a slow backend returns
a clean `not_ready` instead of the client timing out first.

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.

🧹 Nitpick | 🔵 Trivial | 💤 Low value

Consider minor wording variation for stylistic polish (optional).

Lines 92-93 repeat "right signal" in consecutive sentences. While the parallel structure effectively emphasizes the liveness/readiness distinction, you could vary the phrasing for smoother reading:

"Liveness (--health) is the appropriate signal for a Docker HEALTHCHECK (restart on crash). Readiness (/health/ready) is the right signal for a Kubernetes readinessProbe..."

or

"Use liveness (--health) for Docker HEALTHCHECK (restart on crash) and readiness (/health/ready) for Kubernetes readinessProbe..."

🧰 Tools
🪛 LanguageTool

[style] ~93-~93: You have already used ‘right’ in nearby sentences. Consider using an alternative word to let your writing stand out and sound more polished.
Context: ...sh). Readiness (/health/ready) is the right signal for a Kubernetes `readinessProbe...

(REP_RIGHT)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/advanced/cli.mdx` around lines 65 - 97, The phrases "Liveness
(`--health`) is the right signal" and "Readiness (`/health/ready`) is the right
signal" repeat the same structure and wording in consecutive sentences. To
improve readability, vary the phrasing by either replacing "right" with an
alternative adjective such as "appropriate" in one of the sentences, or
restructure both sentences using different grammatical patterns (for example,
converting one to an imperative or different verb structure). Choose whichever
approach better matches the documentation's tone and style.

Source: Linters/SAST tools

Comment thread docs/openapi.json
Comment on lines +2119 to +2138
"responses": {
"200": {
"description": "ready or degraded",
"content": {
"application/json": {
"schema": {
"type": "object",
"additionalProperties": true
}
}
}
},
"503": {
"description": "not ready",
"content": {
"application/json": {
"schema": {
"type": "object",
"additionalProperties": true
}

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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Tighten /health/ready response schema to match the declared contract.

Both 200 and 503 currently allow arbitrary objects (additionalProperties: true), but this endpoint is documented to return a stable readiness shape (status + components). Leaving it fully open weakens generated client types and can mask contract drift.

Suggested OpenAPI schema refinement
         "responses": {
           "200": {
             "description": "ready or degraded",
             "content": {
               "application/json": {
                 "schema": {
                   "type": "object",
-                  "additionalProperties": true
+                  "required": ["status", "components"],
+                  "properties": {
+                    "status": {
+                      "type": "string",
+                      "enum": ["ready", "degraded"]
+                    },
+                    "components": {
+                      "type": "object",
+                      "properties": {
+                        "storage": { "type": "string" },
+                        "cache": { "type": "string" }
+                      },
+                      "additionalProperties": false
+                    }
+                  },
+                  "additionalProperties": false
                 }
               }
             }
           },
           "503": {
             "description": "not ready",
             "content": {
               "application/json": {
                 "schema": {
                   "type": "object",
-                  "additionalProperties": true
+                  "required": ["status", "components"],
+                  "properties": {
+                    "status": {
+                      "type": "string",
+                      "enum": ["not_ready"]
+                    },
+                    "components": {
+                      "type": "object",
+                      "properties": {
+                        "storage": { "type": "string" },
+                        "cache": { "type": "string" }
+                      },
+                      "additionalProperties": false
+                    }
+                  },
+                  "additionalProperties": false
                 }
               }
             }
           }
         },
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/openapi.json` around lines 2119 - 2138, The `/health/ready` endpoint
response schemas in both the 200 and 503 responses currently use generic objects
with additionalProperties set to true, which does not match the documented
contract that should include specific status and components fields. Replace the
permissive schema definitions (the object type with additionalProperties: true)
in both response definitions with a properly defined schema that explicitly
declares the status and components properties as required fields, removing
additionalProperties: true to enforce the contract structure and generate
accurate client types.

Comment on lines +49 to +50
// @Success 200 {object} map[string]interface{} "ready or degraded"
// @Failure 503 {object} map[string]interface{} "not ready"

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.

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Use the concrete readiness response type in Swagger annotations.

Ready returns a stable typed payload (readinessResponse), but the docs currently advertise a generic map[string]interface{}. Using the concrete type keeps the OpenAPI schema aligned and prevents drift.

Suggested diff
-// `@Success`      200  {object}  map[string]interface{}  "ready or degraded"
-// `@Failure`      503  {object}  map[string]interface{}  "not ready"
+// `@Success`      200  {object}  readinessResponse  "ready or degraded"
+// `@Failure`      503  {object}  readinessResponse  "not ready"

Based on learnings: “Do not use interface{} or map[string]interface{} for API request/response payload types; prefer strongly-typed structs.”

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// @Success 200 {object} map[string]interface{} "ready or degraded"
// @Failure 503 {object} map[string]interface{} "not ready"
// `@Success` 200 {object} readinessResponse "ready or degraded"
// `@Failure` 503 {object} readinessResponse "not ready"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/server/readiness.go` around lines 49 - 50, The Swagger annotations
for the Ready function in internal/server/readiness.go use generic
map[string]interface{} types in both the `@Success` and `@Failure` annotations, but
the function actually returns a concrete readinessResponse type. Replace
map[string]interface{} with readinessResponse in both the `@Success` and `@Failure`
Swagger annotations to align the OpenAPI schema with the actual return type and
maintain type consistency in the API documentation.

Source: Learnings

PR #395 added OpenCode Go to the generated docs.go/openapi.json by hand
without updating the @description annotation in main.go, so the generated
spec drifted from its source. Regenerating swagger for /health/ready
re-synced both files and dropped the manual edit.

Add OpenCode Go to main.go's @description (the source of truth) and
regenerate, so the provider stays listed and survives future swagger
regenerations.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
cmd/gomodel/docs/docs.go (1)

1594-1616: ⚠️ Potential issue | 🟡 Minor

Use the actual response struct type in the swagger annotation, not a generic map.

The /health/ready handler returns readinessResponse with fields Status (string) and Components (map[string]string, optional), but the swagger annotation at lines 49–50 of internal/server/readiness.go declares the response as map[string]interface{}, which generates an overly permissive OpenAPI schema. Update the annotations to readinessResponse so generated clients can see the actual response fields:

Annotation change in internal/server/readiness.go
- // `@Success`      200  {object}  map[string]interface{}  "ready or degraded"
- // `@Failure`      503  {object}  map[string]interface{}  "not ready"
+ // `@Success`      200  {object}  readinessResponse  "ready or degraded"
+ // `@Failure`      503  {object}  readinessResponse  "not ready"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/gomodel/docs/docs.go` around lines 1594 - 1616, The swagger annotations
for the readiness handler in internal/server/readiness.go at lines 49-50 use a
generic map[string]interface{} type instead of the actual readinessResponse
struct type. Replace the `@Success` and `@Failure` annotations to reference the
readinessResponse struct type instead of map[string]interface{} so that the
generated OpenAPI schema accurately reflects the actual response fields (Status
string and optional Components map). The changes in the docs.go file will be
automatically regenerated once the source annotations are corrected.

Source: Learnings

♻️ Duplicate comments (1)
docs/openapi.json (1)

2119-2138: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Tighten /health/ready response schemas to the actual readiness contract.

At Line 2125 and Line 2136, both 200 and 503 are defined as fully open objects. That does not match the runtime contract (status + components) and weakens client typing/compat checks.

Proposed OpenAPI schema refinement
         "responses": {
           "200": {
             "description": "ready or degraded",
             "content": {
               "application/json": {
                 "schema": {
                   "type": "object",
-                  "additionalProperties": true
+                  "required": ["status", "components"],
+                  "properties": {
+                    "status": {
+                      "type": "string",
+                      "enum": ["ready", "degraded"]
+                    },
+                    "components": {
+                      "type": "object",
+                      "properties": {
+                        "storage": { "type": "string" },
+                        "cache": { "type": "string" }
+                      },
+                      "additionalProperties": false
+                    }
+                  },
+                  "additionalProperties": false
                 }
               }
             }
           },
           "503": {
             "description": "not ready",
             "content": {
               "application/json": {
                 "schema": {
                   "type": "object",
-                  "additionalProperties": true
+                  "required": ["status", "components"],
+                  "properties": {
+                    "status": {
+                      "type": "string",
+                      "enum": ["not_ready"]
+                    },
+                    "components": {
+                      "type": "object",
+                      "properties": {
+                        "storage": { "type": "string" },
+                        "cache": { "type": "string" }
+                      },
+                      "additionalProperties": false
+                    }
+                  },
+                  "additionalProperties": false
                 }
               }
             }
           }
         },

Based on current PR objectives and the supplied runtime readiness handler contract in internal/server/readiness.go, /health/ready is intended to return a stable structured payload.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/openapi.json` around lines 2119 - 2138, The OpenAPI schema for the
/health/ready endpoint defines both the 200 and 503 responses with overly
permissive object schemas that use additionalProperties: true, which does not
match the actual runtime contract. Update both response schemas (200 and 503) to
remove additionalProperties: true and instead define a strict schema with the
required properties that match the actual readiness handler contract from
internal/server/readiness.go, which returns status and components fields,
ensuring client typing and compatibility checks are properly enforced.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cmd/gomodel/main.go`:
- Line 80: The Swagger description at line 80 in cmd/gomodel/main.go lists
specific LLM providers but omits providers that are actually registered in the
same file, such as Azure, Bedrock, Vertex, and vLLM. This causes a mismatch
between the documented and actual supported providers. Either update the
description to be non-exhaustive by removing the explicit provider list and
keeping it generic, or fully synchronize the description to include all
registered providers (Azure, Bedrock, Vertex, vLLM along with the currently
listed ones) to ensure the generated API documentation remains accurate and
maintainable.

---

Outside diff comments:
In `@cmd/gomodel/docs/docs.go`:
- Around line 1594-1616: The swagger annotations for the readiness handler in
internal/server/readiness.go at lines 49-50 use a generic map[string]interface{}
type instead of the actual readinessResponse struct type. Replace the `@Success`
and `@Failure` annotations to reference the readinessResponse struct type instead
of map[string]interface{} so that the generated OpenAPI schema accurately
reflects the actual response fields (Status string and optional Components map).
The changes in the docs.go file will be automatically regenerated once the
source annotations are corrected.

---

Duplicate comments:
In `@docs/openapi.json`:
- Around line 2119-2138: The OpenAPI schema for the /health/ready endpoint
defines both the 200 and 503 responses with overly permissive object schemas
that use additionalProperties: true, which does not match the actual runtime
contract. Update both response schemas (200 and 503) to remove
additionalProperties: true and instead define a strict schema with the required
properties that match the actual readiness handler contract from
internal/server/readiness.go, which returns status and components fields,
ensuring client typing and compatibility checks are properly enforced.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: f8c3abcd-3c95-4ccc-84e8-e2fd2612a6f6

📥 Commits

Reviewing files that changed from the base of the PR and between 63ab1a5 and 3eec839.

📒 Files selected for processing (3)
  • cmd/gomodel/docs/docs.go
  • cmd/gomodel/main.go
  • docs/openapi.json

Comment thread cmd/gomodel/main.go
// @title GoModel API
// @version 1.0
// @description AI gateway routing requests to multiple LLM providers (OpenAI, Anthropic, Gemini, Groq, OpenRouter, DeepSeek, Z.ai, xAI, MiniMax, Xiaomi MiMo, Oracle, Ollama, Bailian). Drop-in OpenAI-compatible API.
// @description AI gateway routing requests to multiple LLM providers (OpenAI, Anthropic, Gemini, Groq, OpenRouter, DeepSeek, Z.ai, xAI, MiniMax, Xiaomi MiMo, OpenCode Go, Oracle, Ollama, Bailian). Drop-in OpenAI-compatible API.

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.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Keep the Swagger provider description from drifting.

Line 80 lists providers but omits providers registered in this same file (e.g., Azure, Bedrock, Vertex, vLLM), so the generated API description can be misleading. Prefer a non-exhaustive description or keep the list fully synchronized.

Proposed fix
-// `@description`    AI gateway routing requests to multiple LLM providers (OpenAI, Anthropic, Gemini, Groq, OpenRouter, DeepSeek, Z.ai, xAI, MiniMax, Xiaomi MiMo, OpenCode Go, Oracle, Ollama, Bailian). Drop-in OpenAI-compatible API.
+// `@description`    AI gateway routing requests to multiple LLM providers. Drop-in OpenAI-compatible API.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// @description AI gateway routing requests to multiple LLM providers (OpenAI, Anthropic, Gemini, Groq, OpenRouter, DeepSeek, Z.ai, xAI, MiniMax, Xiaomi MiMo, OpenCode Go, Oracle, Ollama, Bailian). Drop-in OpenAI-compatible API.
// `@description` AI gateway routing requests to multiple LLM providers. Drop-in OpenAI-compatible API.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/gomodel/main.go` at line 80, The Swagger description at line 80 in
cmd/gomodel/main.go lists specific LLM providers but omits providers that are
actually registered in the same file, such as Azure, Bedrock, Vertex, and vLLM.
This causes a mismatch between the documented and actual supported providers.
Either update the description to be non-exhaustive by removing the explicit
provider list and keeping it generic, or fully synchronize the description to
include all registered providers (Azure, Bedrock, Vertex, vLLM along with the
currently listed ones) to ensure the generated API documentation remains
accurate and maintainable.

@SantiagoDePolonia SantiagoDePolonia merged commit 85c772b into main Jun 16, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants