6 commits

Author	SHA1	Message	Date
Gongyl01	082c776ab0	Feat/llm retry report:SDK retry-attempt observability for review (#785) (#790) ... * feat(llm): add retry report data layer Add the internal data layer for an explicit LLM request retry report: request identity, attempt classification, and a per-run collector that freezes into an immutable report. No behavior change — nothing is mounted on any client and no output is produced, so this is inert until the observer is wired up. - RequestMeta identifies one logical request (provider, model, file path, task type, request no) and travels through the request context, so the single-method LLMClient interface and every call site stay unchanged. - logical_request_id is SHA-256 over a canonical NUL-terminated encoding of run_id plus the meta. It is computed in Freeze, so the collector can be constructed before the session exists. - classifyAttempt derives error_class and failure_phase from the HTTP status and the Go error type only, never from error message text. A non-2xx status outranks the error, since it is the stronger fact. - RetryCollector is created per run with no package-level state, is safe for concurrent use, and drops attempts that carry no identity, which is how scan and llm test requests stay out of the report. - The request outcome is decided once, in Finalize, from the attempt sequence plus the returned error and the parent context state, rather than inferred from the last attempt: cancelling during backoff produces no new attempt, so the sequence still ends in an error while the outcome is cancelled. - Freeze recomputes every aggregate from the listed requests and returns a construction error instead of publishing self-contradictory numbers. Ordering bugs (double Finalize, mutation after Finalize) are recorded as violations and surface there. The report has no free-text field, so there is nothing to redact: no bodies, prompts, URLs or raw SDK error strings. A test pins the exact set of plain string fields so adding one has to be argued for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: 艺临 <gongyiling.gyl@alibaba-inc.com> * feat(llm): observe retry attempts via SDK middleware Mount a shared observer on all three LLM clients (Anthropic, OpenAI Chat Completions, OpenAI Responses) through option.WithMiddleware, so every real HTTP attempt the SDK retry loop makes is recorded against the logical request that issued it. The observer reads response headers only -- status code, request-id / x-request-id, Retry-After (all three forms, at the SDK's own precedence), x-should-retry -- and never touches the body, which the SDK owns and closes before retrying. Attempts without a RequestMeta on the context are dropped whole, which is how scan and `ocr llm test` stay out of the report. RecordAttempt now takes the attempt's start and end timestamps instead of pre-computed durations. observed_backoff_ms spans two attempts, so only the collector can derive it; deriving both durations there also means the observer cannot desynchronize numbering from the real call order. No clock abstraction is needed and the values stay deterministic in tests. The collector is reached through an unexported ClientConfig field rather than new constructor parameters, keeping the three exported constructors unchanged. It is created per run in loadLLMRuntime, not package-level, so two runs in one process cannot share data. Nothing consumes it yet -- P5 calls Freeze at the run boundary. The roadmap's X-Stainless-Retry-Count cross-check is deliberately not implemented: the SDK stops maintaining that header once ExtraHeaders overrides it, so the mismatch branch is only reachable from a legitimate configuration, and the desync it guards against is already caught at build time by the exhaustion and recovery tests asserting exact attempt counts. WithMaxRetries(5) and WithRequestTimeout are untouched; the SDK's retry decisions are observed, never overridden. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(llm): correct attempts and finalize requests at the client boundary The middleware can only observe real HTTP attempts, so an HTTP 200 that carried a truncated body, undecodable JSON, a mid-stream failure, or a dead Responses object was recorded as a success. Each client now corrects its last attempt before returning and finalizes the logical request exactly once. - add retry_boundary.go: classifyBoundaryError (unrecognized errors are left alone rather than bucketed as unknown, since the only way left to tell them apart would be message text), classifyStreamError, reviseAttempt, finalizeRequest, streamIntegrityError and the panic sentinel - defer the boundary on all three CompletionsWithCtx, which now use named results; correction runs before Finalize, as the reverse order would be a "revised after Finalize" violation and drop the whole run's report - correct both EOF branches ahead of their ctx early return, so a parent cancel between the two SDK calls cannot leave a truncated attempt as success - split completionsStreaming into a wrapper with a single exit, so the four inner returns need no correction call of their own - replace the three bare fmt.Errorf stream integrity errors with a dedicated type, messages unchanged - parentCancelled reads only context.Canceled: the per-attempt deadline from WithRequestTimeout must surface as failed, not as a user abort - drop finalizeForTest from the observer tests; every case now reaches Freeze through a client, so a missing defer fails that case instead of passing Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(llm): stamp request identity on review LLM requests review 的五类逻辑请求在调用 SDK 前建立 RequestMeta，使 observer 能按请求身份收集 attempt；scan 的六类请求保持无 meta、不进报告。 - Deps 增加 NewRequestMeta 工厂字段：review 在 agent.New 注入闭包，scan 保持 nil；不用空 provider 当开关，空串是 unnamed endpoint 的合法值 - main_task / memory compression / re-location / plan / review filter 五个落点遵循固定顺序：AppendTaskRecord -> requestCtx -> 请求 - compression 的记录创建移到请求之前，使 request_no 在请求发起时即存在；orphan llm_request 对 resume 无害（applyResumeLine 无该分支），补回归断言 - ReLocateComment 拆出纯 prompt 构造 BuildReLocationMessages，internal/diff 不接触 session / meta；Duration 口径保持含 prompt 构造时间不变 - 导出 RequestMetaFromContext，供 llmloop / agent / scan 三包的测试跨包验收请求身份 * feat(cmd): publish the frozen retry report at the run boundary 在 review 运行边界冻结重试报告并经两个出口发布；scan 与 llm test 输出不变，session JSONL 与 run manifest 契约不动。 - Runner 增加后台 WaitGroup 与 WaitBackground()：agent.Run 在 dispatchSubtasks 之后、finalizeManifest 之前收口 async compression，消除 Freeze 见到未 Finalize 请求而吞掉整份报告的竞态；不加第二个超时，等待依赖 SDK 遵守取消契约 - review_cmd.go 在 ag.Run 返回后调用 Freeze，run_id 取 session 内存 UUID 而非持久化门控的 SessionID()；构造错误并入 emitErr 而非 runErr，不包装成 review failed、不触发失败 usage、不打 --resume 提示 - 报告以末位参数传给 emitRunResult / outputJSONWithWarnings，不扩展 ResultProvider；双出口去重：emitRunResult 已执行时 emitFailureUsage 不重复携带 - 终端摘要走 stdout，位于评审结果与项目摘要之间，全量渲染不截断，file_path / task_type 经 sanitizeTerminal 防控制字符注入 - JSON 在 jsonOutput 末位追加 retry_report（omitempty），直接复用 llm.RetryReport 的字段与 tag；首次成功运行输出逐字节不变 - 端到端：假 Anthropic server + 真 git 仓库驱动 runReview，覆盖干净运行、recovered+failed、全失败去重、Freeze 构造错误、session 持久化失败五个场景；manual_e2e tag 保留写码前的手工验证夹具 * test(cmd): consolidate retry report tests by responsibility The retry-report tests for #368 P5 split coverage of emitRunResult and emitFailureUsage into their own file, leaving the review-run emit functions tested in two places. Move those emit-boundary cases into emit_run_result_test.go beside the pre-existing emitRunResult tests, and rename the remaining file to retry_report_render_test.go so it holds only the report-rendering cases (outputRetryReportText, the JSON key-set allowlist, retryAttemptChain). The shared retryReportFixture stays with the rendering tests; both files are package main so it is still reachable. No test logic changes; only relocation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: separate cancelled retry requests --------- Signed-off-by: 艺临 <gongyiling.gyl@alibaba-inc.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-08-13 14:31:18 +08:00
kite	533b526b4c	chore: add SPDX license headers and automated verification (#740) ... * chore: add SPDX license headers to all source files Add Apache-2.0 SPDX license identifiers and copyright notices to all tracked .go, .sh, .js, .mjs, .ts, and .tsx source files. Introduce scripts/verify-license.sh and scripts/add-license.sh for automated verification and bulk addition of license headers. Integrate the check into CI (ci.yml) and the Makefile (license-check target as a prerequisite of the existing check target). This satisfies the OpenSSF Best Practices Badge requirements for copyright_per_file and license_per_file. * fix: restore execute permissions on scripts * docs: add license header instructions to CONTRIBUTING guides * docs: add license header instructions to pages contributing guides * fix(pages): strip unclosed HTML comment markers to satisfy CodeQL * fix: apply code review suggestions for license scripts - Fix portability: detect macOS vs Linux stat for permission copy - Fix has_header: check both SPDX and copyright (match verify logic) - Fix is_ignored: match on path boundaries to avoid false positives - Fix year extraction: use consistent pipeline across both scripts - Fix Bash 3.2 compat: quote array length expansion for set -u * fix(pages): use loop-until-clean for HTML comment stripping (CodeQL) * fix(pages): use split/join instead of replace to avoid CodeQL false positive CodeQL's js/incomplete-multi-character-sanitization rule flags any .replace() that removes multi-character sequences like '<!--...-->', regardless of context. The data here comes from readFileSync on the project's own index.html (no untrusted input), making this a false positive. Using split(regex).join('') achieves the same result without triggering the taint-tracking rule.	2026-08-05 21:26:27 +08:00
kite	0035d124bc	fix: align Go module path with actual GitHub repository (#526) ... The module path was github.com/open-code-review/open-code-review but the repo lives at github.com/alibaba/open-code-review. This mismatch prevents pkg.go.dev indexing and breaks Go Report Card resolution.	2026-07-27 19:52:35 +08:00
Kunal Jaiswal	0391f7805d	fix: remove hardcoded 180s timeout from REVIEW_FILTER_TASK (#474) ... * fix: remove hardcoded 180s timeout from REVIEW_FILTER_TASK The filter task had a hidden inner timeout (ft.Timeout) that always fired before the global OCR_LLM_TIMEOUT (300s) could apply. Removing it lets the task flow through the same two timeout knobs (OCR_LLM_TIMEOUT / --timeout) as every other task type. Also cleaned up unused timeout fields from task_template.json that were never read in production code. * fix: remove dead timeout block from RE_LOCATION_TASK per review feedback	2026-07-24 23:15:03 +08:00
xujiejie	46dde274d8	Feat/telemetry http exporter (#314) ... * feat(telemetry): add OTLP HTTP exporter and print TraceID - Add HTTP/protobuf exporter support alongside existing gRPC exporter - Route based on OTEL_EXPORTER_OTLP_PROTOCOL config (http/protobuf vs grpc) - Print TraceID to stderr when telemetry is enabled for easier correlation - Add corresponding unit tests * feat(telemetry): add span coverage for LLM calls, tool execution, plan/filter phases - Add StartLLMSpan / RecordLLMResult helpers (span.go), symmetric with existing StartToolSpan / RecordToolResult - Wrap LLM completion calls in llmloop.RunPerFile with llm.request spans - Wrap all three tool execution paths in executeToolCall with tool.execute.* spans (dynamic tools, code_comment sync/async, other tools) - Add plan.execute span around executePlanPhase - Add main.loop span around RunPerFile call in executeSubtask - Add review_filter.execute span around executeReviewFilter, with comments.before / comments.filtered attributes - Record llm.error attribute on LLM failures for diagnosability - Record review.repo / review.from / review.to / review.model on the top-level review.run span - Metrics (RecordLLMRequest / RecordToolCall) are preserved alongside the new spans — they serve different purposes (aggregate dashboards vs per-run diagnosis) Verified end-to-end against Sunfire (OTLP HTTP gateway): full span tree observed for review.run -> subtask.execute -> plan.execute/main.loop/ review_filter.execute -> llm.request/tool.execute.* * fix(telemetry): address CR findings — span error handling, async span lifecycle, protocol robustness - Add span.RecordError(err) to RecordLLMResult and RecordToolResult for consistency with EndSpan - Use OTel standard pattern (span.SetStatus + span.RecordError) in error paths of review.run, plan.execute, main.loop, review_filter.execute - Move async code_comment span end into pool.Submit callback so span duration reflects actual execution time - Unify time.Since(startTime) in code_comment error path to a single dur - Remove http/json from supported OTLP protocols (not actually implemented) - Add stderr warning when unknown OTLP protocol falls back to gRPC * feat(telemetry): include trace_id in JSON output, restrict stderr to text format - Add trace_id as top-level field in jsonOutput struct (omitempty) - JSON format: trace_id in structured response for programmatic extraction - Text format: TraceID printed to stderr for human debugging - Telemetry disabled: trace_id field omitted entirely * fix: address PR review findings - loop.go: wrap async span lifecycle in defer to prevent leak on panic - exporter.go: update parseOTLPEndpoint comment to reflect gRPC+HTTP usage - scan_cmd.go: align traceID extraction and OTel error handling with review_cmd - output.go/shared.go: propagate traceID to outputJSONNoFiles for consistency - agent.go: move comments.filtered attribute before early return so 0 is distinguishable from not-executed * feat(telemetry): address PR review — http/json routing, LLM span coverage, trace_id tests - Route http/json to HTTP exporter (Go OTel SDK HTTP transport only supports protobuf serialization; users need HTTP transport, not JSON encoding) - Add llm.request spans to executePlanPhase, executeReviewFilter, and ReLocateComment with Usage nil-safety consistent with loop.go - Add trace_id assertions to output helper tests and emitRunResult end-to-end tests using real TracerProvider * docs: add OTLP protocol selection and endpoint format to telemetry section Sync across all 5 README language versions (en, zh-CN, ja-JP, ko-KR, ru-RU). * fix: unify time.Since in async code_comment defer to single dur variable	2026-07-08 13:12:21 +08:00
kite	f363702a15	feat(agent): integrate session recording and token tracking for re-location task ... Re-location LLM calls were invisible in session history and their token consumption was unaccounted for. Refactor the code_comment handling path to record re-location requests/responses into session JSONL, accumulate token usage into global counters, and display them in the viewer. Also fixes: async context cancellation risk (WithoutCancel), duplicate telemetry recording, missing timeout enforcement, missing {existing_code} placeholder in the re-location prompt, and renames Parse to ParseComments for clarity.	2026-05-30 23:25:12 +08:00