mirror of
https://github.com/alibaba/open-code-review.git
synced 2026-09-14 19:59:52 +08:00
xujiejie
b3badd376f
* feat(raw): capture raw LLM traffic via SDK middleware (opt-in) Add opt-in raw capture (OCR_RAW_LOGGING=1) built on the SDK HTTP middleware seam: every real HTTP attempt against the LLM endpoint is recorded verbatim - raw request body after extra_body merging and session-key expansion, raw response body (SSE bodies stored as text), redacted headers, model, duration and per-attempt identity from RequestMeta - to ~/.opencodereview/raw/<repo>/<session>.jsonl. The holder/writer split mirrors the RetryCollector pattern: the holder is created with the client in loadLLMRuntime, the per-session writer is bound from the cmd layer once the session exists. Default off mounts no middleware and changes nothing. * fix(raw): harden raw capture for Windows CI and partial writes Finalize the session in the bind tests so the session JSONL handle is released before TempDir cleanup, which fails on Windows while the file is open. Replace the Unix-only HOME failure injection in TestRawFileWriter_OpenFailure with a cross-platform squat on the target raw directory path. Marshal each record in memory before writing so a failed disk write cannot leave a partial JSONL line that corrupts the next record. * fix(raw): harden raw capture per review findings 1. Register the raw middleware before the retry observer. Registering it after placed it innermost in the SDK middleware chain, so the observer's next() covered raw's full-body read and synchronous disk write; under streaming that turned DurationToHeadersMS from a headers-only measurement into the whole generation time whenever OCR_RAW_LOGGING=1. Add a regression test with a deliberately slow writer that turns red under the old order. 2. An empty request body produced a zero-length RawMessage, which fails json.Marshal and made RawFileWriter silently drop the whole record. Leave Request nil in that case so the record is written with "request": null. Extend the empty-bodies test with an encodability assertion. 3. Add status_code to RawRecord so HTTP-level failures are visible without parsing the body, and document the capture-point semantics: on Bedrock the records show the pre-signing Anthropic-shaped request and the SSE-normalized streaming response, because the bedrock adaptation runs closer to the wire. Drop the "exactly what was sent" claim the Bedrock path never honoured. 4. UseTestSessions now redirects rawSubDir to "test-raw" alongside the session redirect, and TestRawSubDirIsSeparateFromSessions pins the literals instead of comparing two already-redirected variables. 5. A request-body read failure is now replayed to the SDK via errReader, mirroring the response side, instead of handing it a clean truncated body that turns a client-side fault into a server-side 400. The record carries the error and leaves Request nil, since the partial bytes are not valid JSON and would fail Encode. 6. A failed write no longer kills later captures: records go straight to the file, a failed encode or write warns once on stderr and drops only that record, and the next one retries. The truncate/offset recovery machinery was removed entirely: after a failed Truncate the anchor could drift and a later recovery could cut into records already on disk, and a partial line from a rare short write is acceptable — readers skip unparseable lines. Also correct the reopen test's stale claim that resume reuses the session ID; every run gets a fresh one. 7. Repeated request headers survive as an RFC 9110 comma-joined list instead of being truncated to the first value. 8. When the request-body read fails and next() fails too, the record keeps both errors instead of letting next's error overwrite the root cause. 9. Non-JSON request bytes fall back to a request_text field, mirroring the response side, so a malformed body cannot fail the record's encode and drop it whole. Unreachable while the SDK marshals every body; the guard completes the writer-level invariant that every record encodes. 10. The raw writer's closer detaches the holder before closing the file (holder.Set(nil)), so LLM calls that run after the closer bypass capture instead of writing to a closed file. The no-write-after-close guarantee was structurally absent and only held through defer registration order. 11. The telemetry docs (five languages) now state that with raw capture on, streaming responses are read in full before the capture hands them on to the rest of the processing, and that capture redacts request headers but records request and response bodies as-is. 12. Timestamp is now the attempt start instead of the moment next() returned. It previously sat between the two ends of the interval duration_ms measures — under streaming, headers arrive in the first second while the stream runs for minutes — so no arithmetic on (timestamp, duration_ms) could place the attempt on a timeline. The two fields now share one base: the interval is exactly [timestamp, timestamp + duration_ms]. A TTFB-shaped regression test turns red under the old capture point. * feat(raw): capture response headers in raw records Response headers were discarded entirely. They carry the server-side request ID (x-request-id, Anthropic request-id, x-amzn-requestid), which often exists nowhere in the body, so raw captures could not be reconciled against provider or gateway logs — the primary debugging workflow this switch exists for. Rate-limit headers on 429s and content-type (SSE vs JSON) come along for the same reason. - Name every part of the record explicitly, in a <direction>_<part>[_<encoding>] scheme: headers becomes request_headers, request becomes request_body, request_text becomes request_body_text, and the response side mirrors all three. A bare `request` already carried three senses in one record (the body here, while request_id is the attempt and request_no the logical request), and once headers were named the body was the only part left implicit. The schema is unreleased, so this costs nothing now and freezes correctly later. - Add response_headers with the same redaction — Set-Cookie is a credential too — and RFC 9110 comma-joining, via a captureHeaders helper shared by both sides. The key is omitted when next() failed and no response exists. - Generalize the telemetry docs' redaction note from request headers to headers, in all five languages, now that both directions are captured. - Tests cover passthrough, Set-Cookie redaction, and JSON-key omission on transport errors.
16 lines
515 B
Go
16 lines
515 B
Go
// SPDX-License-Identifier: Apache-2.0
|
|
// Copyright 2026 alibaba/open-code-review Contributors
|
|
|
|
package session
|
|
|
|
// UseTestSessions redirects session persistence to the "test-sessions"
|
|
// subdirectory and raw captures to "test-raw", so that test runs do not
|
|
// pollute the real stores.
|
|
//
|
|
// It must be called from init() in a _test.go file or from TestMain,
|
|
// before any test goroutines start. It is NOT safe for concurrent use.
|
|
func UseTestSessions() {
|
|
sessionSubDir = "test-sessions"
|
|
rawSubDir = "test-raw"
|
|
}
|