[tombii]

Summary

Follow-up fix for PR #752 which was merged before this critical fix was included.

Changes

• Remove in-memory metrics tracking (, , )
• Cache metrics now computed on-the-fly from table (single source of truth)
• Eliminates race condition where concurrent requests could overwrite each other's metrics
• Removes duplicate metric tracking logic in streaming/non-streaming paths

Addresses Code Review Issues

From the original PR review bots:

1. CRITICAL - Race condition in metrics updates: Fixed by removing read-modify-write pattern entirely
2. WARNING - Duplicate metric tracking logic: Fixed by removing redundant tracking calls

Testing

All cache-related unit tests pass. Two pre-existing test failures unrelated to this change:

• • CodexExecutor test (pre-existing)
• • Auto-update test (pre-existing)

[gemini-code-assist]

Code Review

This pull request removes cache metrics tracking and persistence from the handleChatCore function and adds usage saving for streaming responses. The review feedback suggests cleaning up resulting dead code in utility files and refactoring duplicated usage-saving logic into a helper function to improve maintainability.

[gemini-code-assist]

Now that trackCacheMetrics and updateCacheMetrics are no longer used here, the corresponding functions trackCacheMetrics and updateCacheTokenMetrics in open-sse/utils/cacheControlPolicy.ts appear to be dead code. Please consider removing them to improve code maintainability.

[gemini-code-assist]

This saveRequestUsage call is very similar to the one for non-streaming responses around line 1450. To improve maintainability and reduce code duplication, consider refactoring the common logic into a helper function.

For example, a helper could create the base object with common properties like provider, model, timestamp, etc., which can then be spread and extended in both call sites.

function getBaseUsageData() {
    return {
        provider: provider || "unknown",
        model: model || "unknown",
        errorCode: null,
        timestamp: new Date().toISOString(),
        connectionId: connectionId || undefined,
        apiKeyId: apiKeyInfo?.id || undefined,
        apiKeyName: apiKeyInfo?.name || undefined,
    };
}

[kilo-code-bot]

Code Review Summary

Status: 1 CRITICAL Issue Found | Recommendation: Revert or hotfix immediately

⚠️ This PR appears to be already merged. The issue below is now live on main.

CRITICAL Issues

File	Line	Issue
open-sse/handlers/chatCore.ts	1558	Runtime ReferenceError: updateCacheMetrics undefined, currentMetrics not defined

Issue Details

open-sse/handlers/chatCore.ts:1558

The PR removes the trackCacheMetrics import and updateCacheMetrics import, plus removes the currentMetrics variable declaration. However, the usage of these removed symbols at line 1558 was not removed:

// Line 1558 - still references removed symbols
updateCacheMetrics(currentMetrics).catch((err) => {

• updateCacheMetrics is no longer imported (import on line 47 only has getCacheMetrics)
• currentMetrics variable no longer exists (was removed along with trackCacheMetrics block around line 701)

Impact: All non-streaming chat completions that reach line 1558 will throw a ReferenceError: updateCacheMetrics is not defined at runtime.

Fix required: Either:

1. Remove the updateCacheMetrics(currentMetrics) call entirely, OR
2. Add back the import and ensure currentMetrics is defined before this line

Since updateCacheMetrics in src/lib/db/settings.ts is now a no-op (line 650: // No-op: metrics are now computed from usage_history on-the-fly), the simplest fix is to remove the call entirely.

---

Files Reviewed (1 file)

• open-sse/handlers/chatCore.ts - 1 critical issue

Additional Notes

The streaming path also had a similar updateCacheMetrics call that was removed (previously around line 1677), but that removal appears complete. The non-streaming path at line 1558 was missed.

[diegosouzapw]

Thank you for the contribution! This fix has been merged into release v3.2.8.