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

Inline comments:
In `@internal/admin/handler_test.go`:
- Around line 1224-1262: Add a test e.g.
TestCacheOverview_ReturnsErrorWhenReaderFails that sets mockUsageReader.cacheErr
= errors.New("boom"), creates the handler via NewHandler(...,
WithDashboardRuntimeConfig(...)), calls h.CacheOverview with
newHandlerContext("/admin/api/v1/cache/overview?days=30"), and asserts the
recorder returns a non-200 code (expect http.StatusInternalServerError) and the
response body contains the API error payload (unmarshal or assert it contains an
"error" field/string) to ensure reader errors map to the correct HTTP status and
error shape; reference mockUsageReader.cacheErr, NewHandler, h.CacheOverview,
and newHandlerContext.

In `@internal/admin/handler.go`:
- Line 166: The new query parameter params.CacheMode and the new route
/admin/api/v1/cache/overview are not documented for Swagger; update the handler
function comment (the Swagger docblock for the cache overview handler) to add
`@Param` for "cache_mode" (query, string, optional, description), `@Success` 200
with the appropriate response model for the overview, `@Failure` 503 for the
service-unavailable response, and a matching `@Router` entry for
/admin/api/v1/cache/overview (GET); also add the same annotations to the other
cache-related handler comment block referenced around lines 426-455 so generated
docs/SDKs include the filter and the 200/503 contract.

In `@internal/app/app.go`:
- Line 341: The code constructs the response cache middleware via
responsecache.NewResponseCacheMiddleware using appCfg.Cache.Response and always
sets CacheEnabled from the response-cache config, which causes the UI to
advertise cache analytics even when usage logging is disabled; change the logic
that derives CacheEnabled (or introduce a separate CacheAnalyticsEnabled) so it
is true only when both the response cache is configured and usageResult.Enabled
(or usageResult.Logger is non-noop) is true, and update any UI/exposed flags
that currently reference CacheEnabled to use the new combined predicate; locate
the computation that sets CacheEnabled and the call to
responsecache.NewResponseCacheMiddleware (and the analogous logic around the
block referenced later) to apply the guard consistently.

In `@internal/responsecache/usage_hit.go`:
- Around line 48-58: The cached-response path is incorrectly passing resolved
pricing into a normal UsageEntry (via pricingResolver.ResolvePricing and
usage.ExtractFromCachedResponseBody) which causes cache-hit rows to populate
input_cost/output_cost/total_cost; change the cache-hit flow to avoid writing
those provider-cost fields by not passing pricing into
ExtractFromCachedResponseBody (or passing nil/zero pricing) and instead populate
a separate savings field or set a cache-flag on the UsageEntry to record avoided
spend; ensure entry.UserPath is still set and logger.Write writes the entry
without any cost-bearing fields so cached hits are excluded from existing cost
aggregates (or persisted to a separate savings table/stream).

In `@internal/usage/extractor_test.go`:
- Around line 484-509: Update TestExtractFromCachedResponseBody to also assert
cache-hit metadata: verify entry.RequestID equals "req-cache", entry.Provider
equals "openai", entry.Endpoint equals "/v1/chat/completions", and entry.Model
matches the explicit model argument passed to ExtractFromCachedResponseBody (not
the model inside the marshaled body). To ensure the test exercises the
explicit-model code path, make the resp.Model in the marshaled body different
(e.g., "gpt-4o-body") while passing "gpt-4o" as the model argument, then add
assertions for RequestID, Provider, Endpoint, and Model along with the existing
token and CacheType checks in TestExtractFromCachedResponseBody.

In `@internal/usage/reader_mongodb_test.go`:
- Line 10: Rename the test function
TestMongoUsageLogMatchFiltersAndsSearchWithCacheMode to fix the typo (change
"Ands" to "And"), e.g. TestMongoUsageLogMatchFiltersAndSearchWithCacheMode;
update the function declaration and any references (calls, test lists, or grep
patterns) to the new name so the test still runs under `go test` and any tooling
picks up the corrected identifier.

In `@internal/usage/reader_mongodb.go`:
- Around line 378-458: The code runs two separate Aggregates (summaryPipeline
and dailyPipeline) against r.collection which doubles work and risks snapshot
drift; replace the two calls with a single Aggregate using a $facet stage that
contains the current summaryPipeline and dailyPipeline as sub-pipelines, then
decode the single result into the existing CacheOverview structure (populate
CacheOverview.Summary and CacheOverview.Daily from the facet outputs) while
preserving logic that uses params.Interval, mongoDateFormat(interval),
usageTimeZone(params), CacheTypeExact/CacheTypeSemantic, and handling of
has_saved_cost/total_saved_cost; ensure you remove the second
r.collection.Aggregate call and adapt cursor.Decode handling to extract both
facet results from one cursor.
- Around line 535-543: The current code treats params.Search as a raw Mongo
regex which allows special chars to change semantics and can be expensive;
update the regex construction in the block that defines regex (the variable
using params.Search) to escape the user input first (e.g., use Go's
regexp.QuoteMeta on params.Search) and then pass that escaped string to the
$regex value with the existing $options "i" so the search performs a
case-insensitive literal substring match; keep the rest of the searchFilter and
mongoAndFilters usage unchanged.

In `@internal/usage/reader_postgresql.go`:
- Around line 249-256: The SQL embeds package constants CacheTypeExact and
CacheTypeSemantic via string concatenation in summaryQuery (and the similar
query around the 276-284 block); replace those concatenations with parameterized
placeholders (e.g., $1, $2) and pass the constants in the args slice to the DB
call (QueryRow/Query/QueryContext) so the query string is stable and the cache
types are supplied as parameters; do the same for the other query block to keep
usage consistent and maintainable.

In `@internal/usage/store_mongodb.go`:
- Around line 85-87: The single-field index on cache_type (Keys: bson.D{{Key:
"cache_type", Value: 1}}) should be replaced or augmented with a compound index
on (cache_type, timestamp) so queries that filter by cache_type and timestamp
range can use the index directly; update the MongoDB index creation logic that
builds the bson.D keys to include bson.D{{Key:"cache_type", Value:1},
{Key:"timestamp", Value:1}} (or add it alongside the existing index), and
consider mirroring this optimization or documenting the intentional choice
across the SQLite/Postgres index setup if you prefer consistency.

In `@internal/usage/store_sqlite.go`:
- Line 19: The inline comment for maxEntriesPerBatch is stale and incorrect;
update the comment next to the declaration of maxEntriesPerBatch (which is
computed as maxSQLiteParams / columnsPerUsageEntry) to reflect the correct value
(999 / 17 = 58 entries) or remove the explicit numeric count and leave a
descriptive note, ensuring it references the constants maxSQLiteParams and
columnsPerUsageEntry so future changes remain accurate.

In `@internal/usage/usage.go`:
- Around line 40-44: The CacheType field is currently free-form and can create
arbitrary storage buckets; change it from a plain string to a closed/validated
type (e.g., an enum type or custom type with constants) and enforce validation
before persisting (validate in the code path that calls WriteBatch);
specifically update the struct field CacheType to use the new typed enum and add
a validation step where WriteBatch is invoked (or implement a Validate() method
on the struct) to reject or normalize unknown values so only the allowed set is
written.

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

Inline comments:
In `@internal/admin/handler_test.go`:
- Around line 1278-1284: The test currently only checks that body["error"]
exists; strengthen it by asserting the error payload follows the
OpenAI-compatible shape: after unmarshalling rec.Body.Bytes() into body, cast
body["error"] to a map[string]any and assert it contains keys "type", "message",
"param", and "code" (with "param" and "code" allowed to be nil/null). Update the
assertions around json.Unmarshal/rec.Body.Bytes() and the existing presence
check to verify the types/values of error["type"] and error["message"] are
strings and that error["param"] and error["code"] are present (nil acceptable),
using the same variables (body, rec.Body.Bytes()) so the test fails clearly on
missing fields.

In `@internal/app/app.go`:
- Around line 118-123: The nil-logger error path leaks resources from
usage.New(...) because when usageResult != nil but usageResult.Logger == nil we
only close app.audit and app.providers; update the branch handling "if
usageResult == nil || usageResult.Logger == nil" to also close usageResult
(e.g., call usageResult.Close() or include it in the errors.Join) when
usageResult is non-nil so storage handles/background goroutines are cleaned up
and app.usage is not left unclosed.

In `@internal/usage/extractor_test.go`:
- Around line 484-521: Add a subtest to TestExtractFromCachedResponseBody that
exercises a fallback path of ExtractFromCachedResponseBody by passing either
invalid JSON (malformed body) or an unsupported endpoint value; assert that the
function returns a non-nil synthetic entry with zero tokens (InputTokens,
OutputTokens, TotalTokens == 0) and preserves RequestID/Provider/Endpoint/Model
where applicable, so the fallback behavior is covered and locked in test
expectations.

In `@internal/usage/extractor.go`:
- Around line 250-266: The switch on endpoint in the dispatcher relies on exact
literals and misses semantically equivalent values (e.g.,
"/v1/chat/completions/"); update the code that handles the endpoint variable
(before the switch that calls ExtractFromChatResponse,
ExtractFromResponsesResponse, ExtractFromEmbeddingResponse) to normalize
endpoints by trimming trailing slashes and/or cleaning the path (e.g., use
strings.TrimSuffix or path.Clean) so "/v1/chat/completions/" and
"/v1/chat/completions" map the same way; then use the normalized endpoint in the
switch so cached-body dispatch correctly selects the parsing branch.

In `@PossibleRefactoring.md`:
- Around line 57-60: Before deleting the compatibility wrapper and removing
internal/responsecache/middleware_test.go, add replacement tests that explicitly
cover the HandleRequest() cache-hit and cache-miss behaviors so coverage is
preserved; update or add unit tests exercising ResponseCache.HandleRequest (or
the exported handler function used by the wrapper) to simulate requests that
should return a cached response and requests that should populate the cache,
asserting stored values, response headers/status and that the appropriate cache
backend methods are invoked, then only remove the middleware_test.go file after
these new tests are passing.
- Around line 10-16: For each item claiming "no call sites" (notably the
constant defined in internal/responsecache/semantic.go and the items labeled 1,
4, and 53–56), add a short "How verified" line that shows the exact command you
ran to confirm there are no references (for example provide the exact ripgrep or
ast-grep invocation you used, such as the rg/ast-grep pattern and repo
scope/options), so reviewers can reproduce the search before deleting code;
include the symbol name you searched for and the full command string used for
verification.

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

Inline comments:
In `@internal/admin/handler_test.go`:
- Around line 1264-1298: The test TestCacheOverview_ReturnsErrorWhenReaderFails
currently only checks non-empty error fields, which allows raw reader errors to
leak; update the assertions in that test (which uses mockUsageReader, NewHandler
and calls h.CacheOverview) to assert the error payload pins to the generic
contract: assert error.type == "internal_error", error.message equals the
generic message used by your handler (e.g., "An internal server error occurred")
and assert the original reader error string ("boom") is NOT present in
error.message; keep the existing presence checks for "param" and "code" but
replace the non-empty-field checks for "type"/"message" with exact equality and
a negative contains check for "boom".

In `@internal/usage/extractor_test.go`:
- Around line 484-572: Add a subtest in TestExtractFromCachedResponseBody that
verifies ExtractFromCachedResponseBody defaults an unrecognized cache type to
CacheTypeExact: create a minimal core.ChatResponse, marshal it, call
ExtractFromCachedResponseBody with a cacheType string like "unknown", and assert
entry is non-nil and entry.CacheType == CacheTypeExact (also check other basic
fields if desired); reference the existing test harness and subtest pattern used
in TestExtractFromCachedResponseBody to mirror style.

In `@internal/usage/extractor.go`:
- Around line 270-296: The synthetic/fallback branch that constructs a new
UsageEntry sets RequestID, Model, Provider, and Endpoint directly using raw
values, while the normal update path trims whitespace; update the fallback
branch (where a new UsageEntry is created) to apply the same normalization logic
(strings.TrimSpace) to requestID, model, provider and endpoint before assigning
to UsageEntry fields (ID, RequestID, Timestamp, Model, Provider, Endpoint,
CacheType, ProviderID) so both paths produce consistent, trimmed metadata for
the UsageEntry struct.
- Around line 294-296: Redundant TrimSpace: remove the extra strings.TrimSpace
call and rely on the earlier normalizeCachedResponseEndpoint processing; change
the block that currently does "if normalized := strings.TrimSpace(endpoint);
normalized != "" { entry.Endpoint = normalized }" to simply check endpoint for
emptiness and assign (e.g., if endpoint != "" { entry.Endpoint = endpoint }) so
it uses the already-normalized value — reference
normalizeCachedResponseEndpoint, the local variable endpoint, and the assignment
to entry.Endpoint in extractor.go.

In `@PossibleRefactoring.md`:
- Around line 12-17: The verification claim "No call sites found in the repo"
overstates the scope because the provided command only searched internal/;
either update the search command to cover the whole repository (e.g., run
ripgrep from repo root: rg -n "CacheTypeBoth") or change the claim text to say
"No call sites found in internal/" (and do the same for the other occurrences on
lines 61-65); ensure the symbol name CacheTypeBoth and the exact rg command are
updated so the scope in the sentence matches the actual verification performed.

---

Outside diff comments:
In `@internal/admin/handler_test.go`:
- Around line 23-35: The mockUsageReader currently drops the
usage.UsageQueryParams passed to GetCacheOverview; update the mock type
(mockUsageReader) to add a field like lastCacheOverview usage.UsageQueryParams
and implement GetCacheOverview to store the received params into that field
before returning cacheOverview/cacheErr, so tests can assert that
Handler.CacheOverview forces usage.CacheModeCached and preserves filters (e.g.,
UserPath); update relevant tests to assert reader.lastCacheOverview.CacheMode ==
usage.CacheModeCached and that representative filters are preserved.