Skip to content

feat(trace): cover compact and response-cache hits - #301

Draft
OnlineChef (ChefGroep) wants to merge 2 commits into
feat/trace-readerfrom
feat/trace-compact-cache
Draft

OnlineChef (ChefGroep) wants to merge 2 commits into
feat/trace-readerfrom
feat/trace-compact-cache

Conversation

@ChefGroep

@ChefGroep OnlineChef (ChefGroep) commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Extends the opt-in trace store from #298 to the two remaining HTTP/cache paths that materially affect cache debugging:

  • trace /v1/responses/compact
  • trace proxy response-cache HITs without duplicating cached response bodies

This PR is intentionally stacked on feat/trace-store / #298. It does not add WebSocket/live/realtime tracing, a remote reader, Redis, or a new persistence format.

Cache-hit trace model

A response-cache entry now keeps the request id that originally populated it. On a later HIT the new request trace records:

  • cacheHit: true
  • cacheSourceTraceId when known
  • response byte count
  • response hash

The cached response body is not copied again into trace.sqlite. In redacted / full mode only the hit request's own inbound body may be stored under its new trace id.

The persisted cache source id is allowlisted before it can enter usage/request-log metadata.

Compact coverage

/v1/responses/compact now uses the same trace lifecycle as the other HTTP data-plane routes:

beginTrace -> runWithTrace -> response observation -> addFinalRequestLog

The compact implementation already has a 32 MiB hard response-buffer bound; the trace store still applies its own lower configurable per-body cap.

Verification

Exact clean-tree head 84f105ba56240f31ca57f8e348bcc7290c6cc7bd:

  • focused trace/cache/request-log suite: 105 pass, 0 fail
  • real server response-cache e2e: 4 pass, 0 fail
  • full repository suite: 7030 pass, 11 explicit skips, 0 fail across 523 files
  • 34,915 assertions
  • bun x tsc --noEmit ✅
  • bun run privacy:scan ✅
  • git diff --check ✅
  • clean-tree client-artifact build/guard tests ✅

The earlier dirty-tree full-suite attempt hit only the repository's intentional client-artifact clean-tree guard before this lane was committed. The clean exact-head rerun passed that guard and the complete suite.

Remaining boundary

WebSocket/live/realtime traffic is intentionally not included here. That lifecycle needs per-session/frame bounds and separate finalization semantics rather than being bolted onto the HTTP trace path.

Dependency

Stacked on #298. Merge/rebase only after the trace-store parent lands.

Summary by CodeRabbit

  • New Features
    • Request tracing now covers compact Responses requests and proxy response-cache hits across Responses, Claude Messages, and Chat Completions.
    • Cache-hit trace metadata includes a response hash, byte count, and—when available—a link to the request that populated the cache, without duplicating the cached response body.
  • Documentation
    • Updated request-trace coverage details, including traffic types that remain unsupported.

PR-stack (mergen van onder naar boven; elke PR bouwt op de vorige):

  1. feat: make prompt cache behavior measurable #296 feat: make prompt cache behavior measurable
  2. feat: expand providers and make native model discovery account-aware #297 feat: expand providers and make native model discovery account-aware
  3. feat(gui): start auth-inspired OpenCodex redesign #299 feat(gui): start auth-inspired OpenCodex redesign
  4. feat(trace): opt-in trace store linked to usage.jsonl #298 feat(trace): opt-in trace store linked to usage.jsonl
  5. feat(trace): add safe local trace reader #300 feat(trace): add safe local trace reader
  6. 👉 feat(trace): cover compact and response-cache hits #301 feat(trace): cover compact and response-cache hits
  7. feat(trace): cover Responses WebSocket turns #302 feat(trace): cover Responses WebSocket turns
  8. feat(trace): cover live call-create HTTP #303 feat(trace): cover live call-create HTTP

Base van deze PR = de branch van de PR erboven in de lijst (#296 = main).

@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: GroepOnline/opencodex/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 156273bb-7f60-45fb-ab12-fea8a75d9d81
📥 Commits

Reviewing files that changed from the base of the PR and between faeaa19 and 84f105b.

📒 Files selected for processing (10)
  • docs-site/src/content/docs/reference/cli.md
  • src/cache/kv-cache.ts
  • src/cache/response-cache-middleware.ts
  • src/server/index.ts
  • src/trace/capture.ts
  • src/trace/types.ts
  • tests/kv-cache.test.ts
  • tests/response-cache-e2e.test.ts
  • tests/response-cache-middleware.test.ts
  • tests/trace.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Cache entries now preserve the ID of the request that populated them. Cache-hit traces record the cache-hit flag, response hash and byte count, and source trace ID when available, without storing the cached response body. Compact Responses requests now run within a trace context.

Changes

Cache-hit tracing

Layer / File(s) Summary
Carry source trace IDs through the cache
src/cache/kv-cache.ts, src/cache/response-cache-middleware.ts, src/server/index.ts, tests/kv-cache.test.ts, tests/response-cache-middleware.test.ts
Cache entries and middleware carry an optional source trace ID to cache hits. Server cache-store callbacks pass the request ID. Tests check persistence and cache-hit results.
Record cache-hit and compact Responses traces
src/trace/types.ts, src/trace/capture.ts, src/server/index.ts, tests/trace.test.ts, tests/response-cache-e2e.test.ts, docs-site/src/content/docs/reference/cli.md
Cache-hit traces record cache metadata and validate source trace IDs. Compact Responses requests run within a trace context. Tests check trace metadata and the documentation describes trace coverage.

Priority: ⬇️ Low

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

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant ResponseCacheMiddleware
  participant ResponseCache
  participant Server
  participant TraceCapture
  ResponseCacheMiddleware->>ResponseCache: get cached entry
  ResponseCache-->>ResponseCacheMiddleware: return body and sourceTraceId
  ResponseCacheMiddleware-->>Server: return CacheHit
  Server->>TraceCapture: begin trace for cached request
  Server->>TraceCapture: noteTraceCacheHit with body and sourceTraceId
Loading

Suggested reviewers: misterwanted

Merge Risk: ⚪ Minimal · up to 84f10

No actionable issue remains from this review; the change is mergeable after normal checks and the stated trace-store parent dependency is satisfied.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 84f10

This expands optional capture of potentially sensitive traffic while retaining existing authentication and caller-isolation checks. No introduced security defect was established, but deployed access and retention assumptions are not fully verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated exposure is additional retention of admitted HTTP traffic under the proxy process's local configuration directory when body tracing is enabled. The inspected change does not make source provenance an authorization capability or a cross-caller cache selector. Deployment-specific reader access and tenant mapping remain unknown.

Security Findings and Attack Paths

  • observed — Request bodies and upstream or cached response text flow into trace capture under the configured mode. The request-telemetry call uses an explicit field list that excludes trace bodies, trace metadata and cacheSourceTraceId; this inspected outbound sink does not inherit the newly added data.

Trust Boundaries and Controls

  • observed — Existing API authentication, origin checks and admission gates precede the changed handling. Cache lookup includes hashed authorization/session material and fails closed without scope on auth-required bindings. Source IDs must match the same 1–128-character allowlist at capture and metadata normalization; validation is syntactic, not proof that a source trace exists.

Resilience and Maintainability Implications

  • observed — Inbound trace reading stops above 8 MiB, stored bodies have a separately clamped cap, and normal cache writes reject oversized UTF-8 bodies with a 2 MiB default. Native compact buffering already enforces 32 MiB and returns explicit cancellation or read-failure responses. Hashing covers full bodies rather than only stored prefixes.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 81.82% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 9 files. (1 skipped: 1 …
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main changes: tracing /v1/responses/compact requests and response-cache hits. It is concise and specific.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

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

Copy link
Copy Markdown
Contributor Author

CodeRabbit (@coderabbitai) review

@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant