Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@config/config.go`:
- Around line 381-387: The default config currently pre-populates
Cache.Model.Local in buildDefaultConfig(), which causes unmarshaled YAML that
only sets cache.model.redis to incorrectly have both backends set and fail
ValidateCacheConfig; remove the Local: &LocalCacheConfig{CacheDir: ".cache"}
assignment from the ModelCacheConfig defaults so buildDefaultConfig() leaves
Local nil by default, and instead apply the ".cache" fallback only during
runtime initialization when the Local backend is actually selected (e.g., in the
cache initialization path that consumes ModelCacheConfig), setting
Local.CacheDir if Local is non-nil and CacheDir is empty; ensure
ValidateCacheConfig no longer sees a preselected Local and update any tests that
expected Local to be nil.
- Around line 537-542: The applyEnvOverridesValue branch currently skips nil
pointer struct fields (check using field.Type.Kind() == reflect.Ptr &&
!fieldVal.IsNil()), preventing env-only population; update
applyEnvOverridesValue to detect when a nil pointer-to-struct has any descendant
fields with an env tag, allocate a new zero-value struct (set fieldVal to a new
pointer) before recursing into applyEnvOverridesValue(fieldVal.Elem()), and only
leave it nil if no env-tagged descendants exist; add a regression test that
exercises env-only Redis config (e.g., populate REDIS_URL/REDIS_TTL) to ensure
pointer allocation materializes Cache.Response.Simple.Redis when YAML is absent.
- Line 296: Add a Go doc comment above the exported function ValidateCacheConfig
that briefly describes what the function does, its input (the *CacheConfig
parameter), and the returned error behavior; ensure the comment follows Go doc
style (starts with "ValidateCacheConfig ...") and is updated to reflect any
specific validation rules or side effects implemented by ValidateCacheConfig.

In `@internal/app/app.go`:
- Around line 198-205: The failure path for
responsecache.NewResponseCacheMiddleware currently closes app.usage, app.audit,
and app.providers but forgets to close app.batch, leaving batch storage/workers
running; update the error cleanup to also call app.batch.Close() (include it in
errors.Join alongside app.usage.Close(), app.audit.Close(),
app.providers.Close()) so that when response cache initialization fails the
batch resources are properly closed before returning the error from the
initialization block in function where rcm is created.

In `@internal/cache/modelcache/redis.go`:
- Around line 24-55: The wrapper currently always closes the underlying
cache.Store even when callers inject a shared store; update the redisModelCache
struct to track ownership (e.g., an owned bool flag), set owned=true in
NewRedisModelCache and owned=false in NewRedisModelCacheWithStore, and modify
the redisModelCache.Close method to only call store.Close when owned is true;
reference redisModelCache, NewRedisModelCache, NewRedisModelCacheWithStore, and
Close to locate the change.

In `@internal/cache/redis.go`:
- Around line 31-40: In NewRedisStore, the redis client created by
redis.NewClient(opts) must be closed if startup Ping fails to avoid leaking the
client's connection pool; update the error path after client.Ping(ctx).Err() to
call client.Close() (or defer a close only after successful init) before
returning the fmt.Errorf("failed to connect to redis: %w", err) error so the
client is properly cleaned up; reference symbols: NewRedisStore,
redis.NewClient, client.Ping, client.Close.

In `@internal/providers/init.go`:
- Around line 160-165: The startup log currently prints the Redis connection
string directly (slog.Info call using m.Redis.URL), which can leak credentials;
update the code that prepares the log (the block using variables key :=
m.Redis.Key and calling slog.Info("using redis cache", "url", m.Redis.URL,
"key", key)) to redact sensitive parts of m.Redis.URL before logging—parse the
URL (or strip userinfo) and replace credentials with a placeholder (e.g.,
"REDACTED") or log only the host/port, then pass the redacted value to slog.Info
so secrets are not emitted.

In `@internal/responsecache/simple.go`:
- Around line 75-80: The current code launches a detached goroutine inside the
cache path (the anonymous func that calls m.store.Set with storeCtx) which drops
the error and is not tracked by shutdown; replace the fire-and-forget write with
a tracked background worker or inline blocking write so Close() can wait for
pending writes: add a small bounded worker queue (e.g., a buffered channel of
jobs) and a sync.WaitGroup that increments when enqueuing a job for m.store.Set
and decrements after the Set returns (include context timeout handling and
propagate/log errors instead of ignoring them), or perform the Set synchronously
in the request goroutine; ensure the Cache type has Close() that waits on the
WaitGroup and drains the worker channel before closing m.store so no write races
store.Close().
- Around line 102-106: The isStreamingRequest function currently byte-matches
for `"stream":true` which misses variants like `"stream" : true`, multiline JSON
and can false-positive on prompt text; change isStreamingRequest to parse the
request body as JSON into a small struct (e.g., type payload struct { Stream
*bool `json:"stream"` }) and return true only if Stream != nil && *Stream ==
true (still return false for path "/v1/embeddings"); on JSON unmarshal errors or
non-boolean/missing stream fields treat as non-streaming to avoid false
positives that break responseCapture and SSE handling.

In `@internal/server/http.go`:
- Around line 236-241: The shutdown sequence in Server.Shutdown is closing
s.responseCacheMiddleware before stopping s.echo, risking handlers hitting a
closed store and swallowing Close errors; change the order to call
s.echo.Shutdown(ctx) first, capture its error, then call
s.responseCacheMiddleware.Close(), capture that error, and return a combined
error (use errors.Join) so both errors are propagated; also add the errors
import.

---

Outside diff comments:
In `@internal/providers/registry.go`:
- Around line 242-275: The code currently ranges over modelCache.Providers which
yields non-deterministic iteration order and breaks the "first provider wins"
rule; change the loop to iterate providers in the registry's
configured/registration order (not the cache map order) when populating
newModels/newModelsByProvider: build a deterministic ordered list of provider
names from the registry's registration info (use the existing registration-order
slice if one exists, or record provider names in registration order when
populating r.providerNames), then loop over that ordered slice instead of
ranging modelCache.Providers so that duplicate model IDs consistently honor the
first-registered provider; update the loop that references modelCache.Providers,
providerName, cachedProv, newModels, and newModelsByProvider accordingly.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@config/config.go`:
- Around line 281-288: The response cache currently ignores
RedisResponseConfig.Key (REDIS_KEY_RESPONSES) and always uses the hard-coded
responseCachePrefix; update the response-cache middleware where the Redis store
is created to use the configured key (e.g., cfg.RedisResponse.Key) as the
namespace/prefix (falling back to responseCachePrefix when empty) instead of the
constant, so operators' REDIS_KEY_RESPONSES takes effect; alternatively remove
the Key field from RedisResponseConfig and docs if you prefer not to support
per-response namespaces.

In `@docs/advanced/configuration.mdx`:
- Around line 53-56: Add a migration note explaining that the env vars were
renamed: map old names REDIS_KEY -> REDIS_KEY_MODELS and REDIS_TTL ->
REDIS_TTL_MODELS so users upgrading should set the new vars (or mention
backward-compatibility steps), and add a short clarifying sentence that
REDIS_KEY_RESPONSES intentionally includes a trailing colon
(`gomodel:response:`) because it’s used as a key prefix for response entries
whereas REDIS_KEY_MODELS (`gomodel:models`) is a discrete key; place this note
immediately after the Cache table and reference the exact env var names
(REDIS_KEY, REDIS_TTL, REDIS_KEY_MODELS, REDIS_TTL_MODELS, REDIS_KEY_RESPONSES)
so readers can map old→new.

In `@helm/templates/deployment.yaml`:
- Around line 69-78: The deployment Helm template is missing environment
variables for REDIS_TTL_MODELS and REDIS_TTL_RESPONSES so pods never receive TTL
settings; add two env entries (names REDIS_TTL_MODELS and REDIS_TTL_RESPONSES)
alongside REDIS_KEY_MODELS/RESPONSES using valueFrom configMapKeyRef with the
same {{ include "gomodel.fullname" . }} name and corresponding keys
REDIS_TTL_MODELS and REDIS_TTL_RESPONSES, and also update the ConfigMap template
to expose those two keys so values from chart values can flow through into the
pod.

---

Outside diff comments:
In `@docs/advanced/configuration.mdx`:
- Around line 46-56: Add a new row to the Cache configuration table documenting
CACHE_REFRESH_INTERVAL: the YAML key is cache.refresh_interval, it controls how
often the model registry auto-refreshes (seconds) and defaults to 3600 (1h);
update the table near the existing CACHE_TYPE/GOMODEL_CACHE_DIR/REDIS_* entries
to include the row with Variable `CACHE_REFRESH_INTERVAL`, Description `How
often to refresh the model registry (seconds)`, and Default ``3600` (1h)` to
keep the docs complete and consistent.
- Around line 179-183: Update the YAML example to show per-category cache
configuration rather than a single top-level cache block: replace the current
cache keys with separate sections for cache.model and cache.response (e.g.,
cache.model.redis.url = "redis://..." and cache.response.simple.redis.url =
"redis://..." or similar), preserving the indicated types (redis/simple) so
readers can see both model registry and response-cache settings; edit the
example around the existing cache block to use cache.model and cache.response
keys instead of cache.type and cache.redis.

---

Duplicate comments:
In `@config/config.go`:
- Line 305: Add a Go doc comment for the exported function ValidateCacheConfig
that begins with the function name and succinctly describes its purpose,
expected input (the CacheConfig pointer), and what kind of error it returns on
invalid configuration; update the comment to mention any important validation
rules or side effects performed by ValidateCacheConfig so callers know when and
why it might return an error.
- Around line 390-396: The default config currently seeds Cache.Model.Local
inside buildDefaultConfig() which forces both backends to appear and breaks
validation when users only configure cache.model.redis; remove the hard-coded
Local assignment (and any explicit nil/placeholder for Redis) from the
ModelCacheConfig defaults in buildDefaultConfig(), leaving both backend fields
absent/unset by default, and instead implement backend selection/normalization
after YAML/env loading (e.g., a dedicated normalizeCacheModel or finalizeConfig
step) that decides and fills Local or Redis based on user input; target the
ModelCacheConfig struct and buildDefaultConfig() function to make this change.
- Around line 546-551: The pointer-branch in applyEnvOverridesValue currently
skips nil pointer fields so env-only settings (e.g., REDIS_URL /
REDIS_TTL_RESPONSES) can’t create nested structs; change the logic so when
field.Type.Kind() == reflect.Ptr and fieldVal.IsNil() you detect whether any
descendant env var for that pointer-to-struct exists, and if so allocate a new
value with reflect.New(field.Type.Elem()), set it into the parent
(fieldVal.Set(newVal)) and then call applyEnvOverridesValue(newVal.Elem()) to
populate it; also add a regression test that sets REDIS_URL and
REDIS_TTL_RESPONSES (or equivalent env names used by Cache.Model.Redis and
Cache.Response.Simple.Redis) and asserts the config now materializes those
nested Redis structs instead of leaving them nil.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/responsecache/simple.go`:
- Around line 46-49: The handler currently calls io.ReadAll(c.Request().Body)
and on error falls back to return next(c), which lets downstream handlers see a
partially-consumed request body; instead, when io.ReadAll returns an error you
should propagate that error (return the read error) rather than calling next(c),
so replace the fallback to next(c) with returning the read error from the
middleware surrounding the io.ReadAll call (ensure the error from io.ReadAll is
returned to the caller so the request is not forwarded with a truncated body).

---

Duplicate comments:
In `@internal/responsecache/simple.go`:
- Around line 102-106: isStreamingRequest currently byte-matches the body and
can miss variants or false-positive in prompt text; change isStreamingRequest to
parse the JSON body (e.g., json.Unmarshal into a map[string]interface{} or a
small struct with Stream *bool) and return true only when the parsed "stream"
field is true, keep the existing path == "/v1/embeddings" early-return, and on
JSON unmarshal errors conservatively return false so responseCapture handling
(which doesn't preserve streaming/SSE) is not used for real streamed requests.
- Around line 75-80: The code currently detaches cache writes in an anonymous
goroutine (checking capture.Response().Status and capture.body.Len()) which
drops Set errors and cannot be drained on shutdown; change this to track
background writes: either perform m.store.Set inline (replace the goroutine with
a direct call using a context.WithTimeout) or add a bounded worker/WaitGroup
that Close() waits on. If using WaitGroup, call m.wg.Add(1) before starting the
goroutine, defer m.wg.Done() inside it, capture and log/return the error from
m.store.Set, and ensure Close() waits (with a timeout) for m.wg to finish;
alternatively implement a buffered channel and a fixed number of worker
goroutines that read write requests (key, capture.body.Bytes(), m.ttl) and call
m.store.Set, and have Close() close the channel and wait for workers to exit.
Ensure you keep the same checks for capture.Response().Status == http.StatusOK
and capture.body.Len() > 0 and reuse the same context timeout pattern for Set
calls.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@config/config.go`:
- Around line 281-288: The struct comment for RedisResponseConfig should
explicitly state that the URL field shares the same env var (REDIS_URL) as
RedisModelConfig, so a single REDIS_URL configures the Redis instance for both
response and model caches while Key and TTL can be set independently via
REDIS_KEY_RESPONSES and REDIS_TTL_RESPONSES; update the doc comment above
RedisResponseConfig to mention RedisModelConfig, call out the shared REDIS_URL
behavior, and note that Key and TTL are separate env vars allowing independent
cache keys/expirations.

In `@helm/values.yaml`:
- Around line 93-100: Add the four new Redis cache fields to the Helm values
schema by extending the cache.redis object in values.schema.json to include
keyModels (string), ttlModels (integer), keyResponses (string), and ttlResponses
(integer); ensure each property has an appropriate type (string for
keyModels/keyResponses, integer for ttlModels/ttlResponses) and include brief
descriptions/defaults consistent with existing cache.redis properties (e.g.,
match naming and units for TTLs) so users see and validate these inputs in the
chart schema UI.

In `@internal/cache/modelcache/redis_test.go`:
- Around line 67-88: The test currently only verifies Set/Get succeed but
doesn't confirm that NewRedisModelCacheWithStore applied defaults; update
TestRedisModelCache_DefaultKeyAndTTL to either type-assert the returned cache to
*redisModelCache and assert its key and ttl fields equal DefaultRedisKey and
cache.DefaultRedisTTL, or inspect the backing store (store from
cache.NewMapStore) to verify the entry was written under DefaultRedisKey and
expires with cache.DefaultRedisTTL; reference NewRedisModelCacheWithStore,
redisModelCache, DefaultRedisKey, cache.DefaultRedisTTL, Set/Get and the store
object to locate and implement the assertions.
- Around line 90-119: Replace the MapStore usage in both tests with a small spy
implementation of cache.Store that records whether Close() was called; in
TestRedisModelCacheWithStore_CloseDoesNotCloseSharedStore create the spy, pass
it to NewRedisModelCacheWithStore, call c.Close(), and assert the spy’s Close
was not called (owned=false), and in TestRedisModelCache_CloseClosesOwnedStore
create the spy, construct redisModelCache with owned=true, call c.Close() and
assert the spy’s Close was called exactly once and that a second c.Close() is
safe (idempotent); reference symbols:
TestRedisModelCacheWithStore_CloseDoesNotCloseSharedStore,
TestRedisModelCache_CloseClosesOwnedStore, NewRedisModelCacheWithStore,
redisModelCache, cache.Store, Close.

In `@internal/cache/modelcache/redis.go`:
- Around line 13-15: Add a Go doc comment for the exported constant
DefaultRedisKey: place a single-line comment immediately above the const
declaration that begins with "DefaultRedisKey" and succinctly describes its
purpose (the default Redis key/namespace used for storing model entries) and its
default value ("gomodel:models"); ensure the comment follows Go doc style
(starts with the symbol name) so the package surface is properly documented.

In `@internal/responsecache/simple.go`:
- Around line 138-153: The responseCapture currently wraps http.ResponseWriter
but does not implement http.Flusher, so any downstream Flush() calls (e.g., SSE)
will be ignored; add a Flush method on responseCapture that type-asserts
r.ResponseWriter to http.Flusher and, if present, calls its Flush() (no-op
otherwise) so flushes are forwarded; update the responseCapture type to include
this method alongside Write to preserve existing behavior.

In `@internal/server/http.go`:
- Around line 49-50: Add a Go doc comment for the exported
ResponseCacheMiddleware field on the server.Config struct: describe that it
allows injecting a *responsecache.ResponseCacheMiddleware instance to
enable/override response caching behavior for HTTP responses (and note its
effect or expected lifecycle if relevant). Ensure the comment is a single-line
prefix comment placed immediately above the ResponseCacheMiddleware field in the
server.Config type to match surrounding field docs.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@config/config.go`:
- Around line 306-325: ValidateCacheConfig currently dereferences c (m :=
&c.Model) and panics when called with nil; add a guard at the top of
ValidateCacheConfig to return a descriptive error if c == nil (e.g.,
fmt.Errorf("cache: config is nil")) before any dereference, then proceed with
the existing checks on c.Model, c.Model.Local and c.Model.Redis (and the
m.Redis.URL check) so the function safely handles nil input.

In `@helm/values.schema.json`:
- Around line 191-200: The redis properties object in the JSON schema currently
allows unknown properties so removed fields like cache.redis.key/cache.redis.ttl
will still validate but be ignored; update the "redis" schema block (the object
named "redis" with properties
"url","keyModels","ttlModels","keyResponses","ttlResponses") to either add
"additionalProperties": false to make it strict, or add explicit
backward-compatibility aliases "key" and "ttl" (with descriptions marking them
deprecated and mapping guidance) so old values files are validated and the
templates can read them; implement one of these fixes in the
helm/values.schema.json redis object.

In `@internal/cache/modelcache/redis.go`:
- Around line 18-23: The exported type RedisModelCacheConfig and its fields lack
proper Go doc comments and the existing top comment is misleading; update the
comment on RedisModelCacheConfig to state that it is the configuration used by
NewRedisModelCache (which creates the Redis store) and document each exported
field (URL, Key, TTL) with their meaning, defaults (if any), and the TTL units
(e.g., time.Duration in seconds), so go doc consumers understand expected values
and behavior; ensure the package-level comment remains accurate and that
NewRedisModelCache is referenced in the type comment.
- Around line 77-82: The Set method on redisModelCache should reject nil
ModelCache payloads to avoid writing JSON "null" to Redis; modify
redisModelCache.Set to check if mc == nil and return a descriptive error (e.g.,
"nil ModelCache") before calling json.Marshal and c.store.Set (referencing
redisModelCache.Set, ModelCache, c.store.Set, c.key, c.ttl), and add a
regression test in a *_test.go that calls redisModelCache.Set(ctx, nil) and
asserts an error is returned and nothing is written to the store.

---

Outside diff comments:
In `@config/config.go`:
- Around line 466-483: After applyEnvOverrides(cfg) returns, ensure the
env-selected mutually-exclusive model cache backend wins by detecting which
backend-specific environment variables were used and clearing the other backend
on cfg.Cache.Model; specifically, check for Redis env vars (e.g., REDIS_URL,
REDIS_KEY_MODELS, REDIS_TTL_MODELS) and if any present set cfg.Cache.Model.Local
= nil, or check for the local env var (e.g., GOMODEL_CACHE_DIR) and if present
set cfg.Cache.Model.Redis = nil, so that the subsequent defaulting/validation
(the block that sets Local when both nil and the call to ValidateCacheConfig)
honors environment overrides rather than the YAML value.

---

Duplicate comments:
In `@internal/responsecache/simple.go`:
- Around line 79-89: The current code spawns an unbounded goroutine per
cache-miss (m.wg, m.store.Set) which can backlog if the store is slow; fix by
bounding concurrent cache writers: add a semaphore field (e.g., m.writeSem chan
struct{} with a fixed capacity), acquire from m.writeSem before launching the
writer goroutine (or acquire synchronously and fall back to doing the Set inline
if the sem is full), then release the sem after Set and still use m.wg.Done;
update the code referencing capture.body, key and m.ttl to perform the Set under
the semaphore to prevent unbounded goroutine growth.