fix: redact provider api keys, bound cache writers, harden pgvector init

[SantiagoDePolonia]

Addresses a batch of code-review findings. Each reported item was validated against source before acting; three were rejected as incorrect or already handled (see below) and are intentionally not changed.

Fixed

• security(auditlog): redact api-key (Azure) and x-goog-api-key (Gemini/Vertex) request headers. azure.go sets its credential header as api-key, which was missing from RedactedHeaders, so it could be persisted in cleartext when header logging is enabled. Added both header names + a test case.
• reliability(responsecache): the semantic cache spawned an unbounded go func() per cache miss straight at the vector store — a miss storm could exhaust connections. Now uses the same bounded worker pool (cacheWriteWorkerCount=8, queued, wg-tracked, clean shutdown) that the exact cache already uses.
• reliability(responsecache): CREATE EXTENSION vector aborted startup on managed Postgres (RDS/Cloud SQL/Heroku) where the app role lacks the privilege even when a DBA already installed pgvector. Now tolerates the error only when pg_extension confirms the extension is present; otherwise still fails loudly.
• perf(gateway): batch usage logging called ResolvePricing (registry RLock) per item, up to ~50k/batch. Added a loop-local (model, provider) pricing cache (caches nil too).
• perf(server): flushStream allocated a fresh 32KB buffer per stream; now drawn from a sync.Pool.
• refactor(core): collapsed hand-written request MarshalJSON boilerplate to type alias conversions for ChatRequest, ResponsesRequest, EmbeddingRequest, BatchRequest, ToolCall, FunctionCall (−150 lines), removing the silent field-drop risk when new typed fields are added. Output is byte-identical (verified via existing round-trip tests). Message/ResponseMessage were intentionally left explicit — their duplicate json:"content" Swagger tag makes a blind alias unsafe.

Rejected (with evidence)

• "O(N·M) JSON field lookup" → map: would regress. The slices.Contains design is deliberate and already benchmarked against exactly this map baseline (json_fields_bench_test.go); UnknownJSONFields exists to avoid per-request map allocations. M is 14–27 and the gjson walk dominates.
• "Redundant map clone helpers": already delegate to maps.Clone. The premise (hand-rolled loops) is stale; they live in 4 packages and carry intent-revealing names/docs.
• Synchronous budget aggregate SQL → rollup table: valid scalability concern, but not a safe drive-by fix. SumUsageCost is already period-bounded and indexed (idx_usage_timestamp, idx_usage_user_path). A rollup must also handle hierarchical user-path prefix matching, cache-type exclusion, period boundaries, and resets across 3 storage backends — a dedicated, designed change.

Testing

• go build ./..., go vet, and full go test ./... pass.
• Pre-commit hooks pass, including make test-race, make lint, and the hot-path alloc/byte performance guard.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Added audit log redaction for additional provider credential headers.
  • Improved startup behavior when the database vector extension is already installed.

• Performance

  • Reduced repeated pricing lookups during batch processing.
  • Improved semantic cache write behavior by using a bounded worker approach.
  • Improved streaming efficiency with reusable buffers.

• Refactor

  • Simplified several request/message serialization implementations while preserving existing behavior.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR adds credential header redaction entries, refactors several JSON marshalers to use type aliases, caches batch pricing lookups, bounds semantic cache writes with a worker pool, tolerates pre-existing pgvector installs, and reuses stream copy buffers.

Changes

Audit Log Header Redaction

Layer / File(s)	Summary
Add credential headers to redaction list and tests internal/auditlog/auditlog.go, internal/auditlog/auditlog_test.go	RedactedHeaders now includes "api-key" and "x-goog-api-key"; the test table verifies these headers are redacted while Content-Type remains unchanged.

MarshalJSON Type Alias Refactor

Layer / File(s)	Summary
Convert MarshalJSON implementations to type alias pattern internal/core/batch_json.go, internal/core/chat_json.go, internal/core/embeddings_json.go, internal/core/message_json.go, internal/core/responses_json.go	BatchRequest, ChatRequest, EmbeddingRequest, ToolCall, FunctionCall, and ResponsesRequest now marshal through type alias values and marshalWithUnknownJSONFields, while still merging ExtraFields.

Batch Usage Pricing Cache

Layer / File(s)	Summary
Cache pricing lookups within batch processing internal/gateway/batch_usage.go	A local (model, provider) cache now reuses resolved pricing values, including nil, across repeated batch items.

Semantic Cache Worker Pool

Layer / File(s)	Summary
Worker pool, enqueue path, and coordinated shutdown internal/responsecache/semantic.go	Semantic cache writes now flow through a bounded job queue and worker pool; Handle enqueues writes and close() drains workers before closing dependencies.

PGVector Extension Tolerant Initialization

Layer / File(s)	Summary
Tolerant extension creation and existence check helper internal/responsecache/vecstore_pgvector.go	newPGVectorStore now checks whether the vector extension already exists before failing on CREATE EXTENSION; a helper queries pg_extension and returns false on query errors.

Stream Copy Buffer Pooling

Layer / File(s)	Summary
Pool and reuse stream copy buffers internal/server/stream_support.go	flushStream now reuses 32KB buffers from a sync.Pool instead of allocating a fresh slice per call.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

• ENTERPILOT/GoModel#200: Both PRs modify internal/responsecache/semantic.go and change semantic cache behavior.
• ENTERPILOT/GoModel#361: This PR’s audit-log redaction now explicitly covers the "api-key" header introduced for Azure auth.

Poem

A rabbit tapped the keyboard bright,
Redacting secrets out of sight.
Alias hops and pooled-up streams,
Cache the bits and calm the dreams.
Through burrowed paths, the code runs neat —
🐇 soft and swift on careful feet.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% 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 is concise and accurately captures the main security, reliability, and initialization changes in the PR.
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.

✨ 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 fix/batch-of-issues-resolved

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 62.02532% with 30 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/responsecache/semantic.go	62.50%	13 Missing and 2 partials ⚠️
internal/responsecache/vecstore_pgvector.go	0.00%	12 Missing ⚠️
internal/gateway/batch_usage.go	62.50%	1 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@internal/gateway/batch_usage.go`:
- Line 94: The cache key in the batch usage path is currently built by joining
model and provider with a NUL byte, which is less idiomatic and depends on
string contents never containing that separator. Update the map key in the code
around the cache lookup in the batch usage logic to use a dedicated struct with
model and provider fields instead of a concatenated string, and adjust the key
creation wherever that cache key is produced or consumed so the behavior stays
the same but is self-documenting.

🪄 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: 0439a938-3f24-4067-8370-5d7b0b429c72

📥 Commits

Reviewing files that changed from the base of the PR and between 1bde978 and cab4ae8.

📒 Files selected for processing (11)

• internal/auditlog/auditlog.go
• internal/auditlog/auditlog_test.go
• internal/core/batch_json.go
• internal/core/chat_json.go
• internal/core/embeddings_json.go
• internal/core/message_json.go
• internal/core/responses_json.go
• internal/gateway/batch_usage.go
• internal/responsecache/semantic.go
• internal/responsecache/vecstore_pgvector.go
• internal/server/stream_support.go

[greptile-apps]

Confidence Score: 5/5

Safe to merge. All six changes are targeted and correctly implemented with no regressions introduced.

The security fix correctly plugs two missing credential headers using the existing lowercased-set lookup. The semantic cache worker pool faithfully mirrors the already-proven simpleCacheMiddleware pattern, with the RLock-guarded enqueue/close race handled identically. The pgvector fallback now uses an independent 5-second context, eliminating the deadline-exhaustion edge case. The type-alias MarshalJSON conversions are safe for all six converted types — none carry duplicate json tags — and Message/ResponseMessage are intentionally left explicit. The stream buffer pool and batch pricing cache are both straightforward and correct.

No files require special attention.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Req as Request goroutine
    participant EW as enqueueWrite
    participant Ch as jobs channel (cap 256)
    participant W as Worker x8
    participant VS as VecStore

    Req->>EW: enqueueWrite(job)
    EW->>EW: mu.RLock, check closed
    EW->>EW: wg.Add(1)
    alt queue has space
        EW->>Ch: "jobs <- job"
        EW->>EW: mu.RUnlock
    else queue full
        EW->>EW: wg.Done, mu.RUnlock
        EW-->>Req: log warning, drop job
    end
    Req-->>Req: return immediately

    W->>Ch: range jobs
    Ch-->>W: job
    W->>VS: Insert with 5s timeout
    VS-->>W: result
    W->>W: wg.Done

    Note over EW,W: shutdown closes jobs channel, workers.Wait, wg.Wait

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Req as Request goroutine
    participant EW as enqueueWrite
    participant Ch as jobs channel (cap 256)
    participant W as Worker x8
    participant VS as VecStore

    Req->>EW: enqueueWrite(job)
    EW->>EW: mu.RLock, check closed
    EW->>EW: wg.Add(1)
    alt queue has space
        EW->>Ch: "jobs <- job"
        EW->>EW: mu.RUnlock
    else queue full
        EW->>EW: wg.Done, mu.RUnlock
        EW-->>Req: log warning, drop job
    end
    Req-->>Req: return immediately

    W->>Ch: range jobs
    Ch-->>W: job
    W->>VS: Insert with 5s timeout
    VS-->>W: result
    W->>W: wg.Done

    Note over EW,W: shutdown closes jobs channel, workers.Wait, wg.Wait

Loading

_{Reviews (2): Last reviewed commit: "refactor: address PR review feedback" | Re-trigger Greptile}

[coderabbitai]

Caution

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

⚠️ Outside diff range comments (1)

internal/responsecache/vecstore_pgvector.go (1)

94-103: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add unit test coverage for pgExtensionInstalled and the tolerant fallback path.

Codecov flagged missing coverage in this file. Since this helper directly gates whether initialization fails or continues on managed Postgres, table-driven tests covering the "installed", "not installed", and "query error" cases would materially reduce risk.

🤖 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/responsecache/vecstore_pgvector.go` around lines 94 - 103, Add unit
test coverage for pgExtensionInstalled and the fallback behavior it supports in
vecstore_pgvector.go. Create table-driven tests for pgExtensionInstalled to
cover the installed, not installed, and query-error cases by exercising the
QueryRow/Scan path on a test pool or mocked pgx behavior. Also add a test for
the initialization flow that uses this helper so the tolerant fallback path is
verified when pgExtensionInstalled returns false on query error, ensuring
managed Postgres continues without masking the original CREATE EXTENSION
failure.

🤖 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.

Outside diff comments:
In `@internal/responsecache/vecstore_pgvector.go`:
- Around line 94-103: Add unit test coverage for pgExtensionInstalled and the
fallback behavior it supports in vecstore_pgvector.go. Create table-driven tests
for pgExtensionInstalled to cover the installed, not installed, and query-error
cases by exercising the QueryRow/Scan path on a test pool or mocked pgx
behavior. Also add a test for the initialization flow that uses this helper so
the tolerant fallback path is verified when pgExtensionInstalled returns false
on query error, ensuring managed Postgres continues without masking the original
CREATE EXTENSION failure.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1471ce13-3615-4931-a9c0-13c91867bc5a

📥 Commits

Reviewing files that changed from the base of the PR and between cab4ae8 and 19f0006.

📒 Files selected for processing (2)

• internal/gateway/batch_usage.go
• internal/responsecache/vecstore_pgvector.go