refactor(ai-gateway): fold request logging into rewriteModelResponse

[chrarnoldus]

What

Combines request logging and response rewriting so the upstream event stream is only processed once, and removes handleRequestLogging as a separate step.

Previously, when both request logging and response rewriting were active, the stream was consumed twice: once by handleRequestLogging (reading a cloned response via .text()) and once by rewriteModelResponse (parsing and rewriting SSE events).

Now:

• handleRequestLogging.ts is deleted. Its logic (opt-in check, api_request_log insert from after()) lives in rewriteModelResponse.ts, which creates a RequestLogCapture from the passed RequestLoggingParams and records the raw upstream body while rewriting: the raw text for JSON responses, the raw decoded chunks for SSE streams.
• When request logging is enabled, the rewrite pipeline runs for everyone with logging enabled (even when the rewrite would otherwise be skipped), so the body is captured in the same single pass. The organizationId !== KILO_ORGANIZATION_ID check in the skip condition is removed as redundant: the Kilo organization always has logging enabled and therefore always gets a capture.
• The capture is settled on every path: stream completion, stream read errors, client disconnect (cancel()), JSON read errors, and before readResponseText rethrows unrecognized errors — so the after() callback awaiting it can never hang and the request is always logged (with response_body_read_error when the body is unavailable, preserving previous semantics).
• Error responses replaced early by makeErrorReadable are logged via logUnrewrittenResponse, which reads the body once on that path.

processUsage is unchanged.

Tests

Added coverage in rewriteModelResponse.test.ts for body capture on JSON and SSE paths, empty bodies, stream read errors, JSON read errors, stream cancellation, and forced processing when request logging is enabled. route.test.ts now asserts the Vercel request ID is passed through to rewriteModelResponse's logging params.

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental commit resolves the previously flagged capture-hang risk by settling capture.setReadError in readResponseText before rethrowing unexpected errors and in the unrecognized-kind fallthrough; no new issues found in the changed lines.

Files Reviewed (1 file)

• apps/web/src/lib/rewriteModelResponse.ts - previous WARNING resolved, no new issues

Previous Review Summaries (4 snapshots, latest commit cf5cd24)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit cf5cd24)

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

The incremental change removes the try/catch that used to settle the request-log capture on unexpected rewriteModelResponse_* rejections, which can leave the after() log-insert task waiting on an unresolved promise indefinitely and silently drop that request's log row.

Overview

Severity	Count
CRITICAL	0
WARNING	1
SUGGESTION	0

Issue Details (click to expand)

WARNING

File	Line	Issue
apps/web/src/lib/rewriteModelResponse.ts	699	Removed try/catch no longer settles the log capture when rewriteModelResponse_* rejects with an unexpected error, leaving the after() insert task hanging and the log row unwritten

Files Reviewed (3 files)

• apps/web/src/app/api/openrouter/[...path]/route.ts - comment-only changes
• apps/web/src/lib/rewriteModelResponse.test.ts - type rename/cleanup, no issues
• apps/web/src/lib/rewriteModelResponse.ts - 1 issue

Fix these issues in Kilo Cloud

Previous review (commit fd68f42)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental change moves request-log capture creation from the deleted handleRequestLogging.ts into rewriteModelResponse.ts (adding logUnrewrittenResponse for the early-error path) and updates callers/tests accordingly; the refactor preserves prior logging-gate and single-pass-read semantics with no new issues found.

Files Reviewed (4 files)

• apps/web/src/app/api/openrouter/[...path]/route.ts
• apps/web/src/app/api/openrouter/[...path]/route.test.ts
• apps/web/src/lib/rewriteModelResponse.ts
• apps/web/src/lib/rewriteModelResponse.test.ts

Previous review (commit dfd96c5)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental change since the last review is a pure formatting update (Prettier line-wrapping of test.each calls) in rewriteModelResponse.test.ts, with no logic or assertion changes; combined with the previously reviewed request-logging/response-rewrite refactor, no issues were found.

Files Reviewed (4 files)

• apps/web/src/app/api/openrouter/[...path]/route.ts
• apps/web/src/lib/ai-gateway/handleRequestLogging.ts
• apps/web/src/lib/rewriteModelResponse.ts
• apps/web/src/lib/rewriteModelResponse.test.ts (formatting-only change since last review)

Previous review (commit ca00505)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Reviewed the single-pass request-logging/response-rewrite refactor across all four changed files; the RequestLogCapture handle is consistently set (body or read error) on every code path (JSON success/error, SSE normal completion, SSE stream error, SSE cancellation, empty body, and the early error-response path), with no signs of unresolved promises, double body reads, or logic errors introduced by this change.

Files Reviewed (4 files)

• apps/web/src/app/api/openrouter/[...path]/route.ts
• apps/web/src/lib/ai-gateway/handleRequestLogging.ts
• apps/web/src/lib/rewriteModelResponse.ts
• apps/web/src/lib/rewriteModelResponse.test.ts

---

_{Reviewed by claude-sonnet-5 · Input: 22 · Output: 3.9K · Cached: 447.6K}

_{Review guidance: REVIEW.md from base branch main}