diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ac1420e7..74328b5e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,6 +9,10 @@ on: permissions: contents: read +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + jobs: test: runs-on: self-hosted diff --git a/internal/llmloop/compression.go b/internal/llmloop/compression.go index 89f9f8ca..68457232 100644 --- a/internal/llmloop/compression.go +++ b/internal/llmloop/compression.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "strings" + "sync" "sync/atomic" "time" @@ -41,6 +42,16 @@ type compressionJob struct { snapshotLen int // message count when the snapshot was taken } +// compressionState is the async-compression bookkeeping for a single +// conversation (one RunPerFile call). The Runner is shared by concurrent +// per-file goroutines, so this state must not live on the Runner: a shared +// slot lets one file apply, cancel, or replace another file's compression +// job (#384). +type compressionState struct { + mu sync.Mutex + pendingJob *compressionJob +} + // CountMessagesTokens returns the rough token count of msgs by summing the // per-message text token count. Exported because both review and scan top // layers may want it for pre-flight checks. @@ -217,7 +228,6 @@ func (r *Runner) runCompression(ctx context.Context, msgs []llm.Message, filePat rec := fs.AppendTaskRecord(session.MemoryCompressionTask, compressionMsgs) if err != nil { rec.SetError(err, duration) - fmt.Fprintf(stdout.Writer(), "[ocr] Memory compression failed: %v\n", err) // Return msgs unchanged: truncating to frozenEnd would discard all // conversation context, which is worse than staying over the token // limit temporarily. @@ -252,31 +262,40 @@ func (r *Runner) runCompression(ctx context.Context, msgs []llm.Message, filePat return rebuilt, nil } -// triggerAsyncCompression kicks off a background compression job. -func (r *Runner) triggerAsyncCompression(ctx context.Context, messages []llm.Message, filePath string) { +// triggerAsyncCompression kicks off a background compression job for the +// conversation owning st. A no-op when a job is already pending — the +// check-and-set happens under st.mu so concurrent callers cannot replace +// (and thereby leak) an in-flight job. +func (r *Runner) triggerAsyncCompression(ctx context.Context, st *compressionState, messages []llm.Message, filePath string) { + st.mu.Lock() + if st.pendingJob != nil { + st.mu.Unlock() + return + } msgSnapshot := copyMessages(messages) - asyncCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 5*time.Minute) - job := &compressionJob{done: make(chan struct{}), cancel: cancel, snapshotLen: len(messages)} - r.compressionMu.Lock() - r.pendingJob = job - r.compressionMu.Unlock() + st.pendingJob = job + st.mu.Unlock() go func() { defer cancel() rebuilt, err := r.runCompression(asyncCtx, msgSnapshot, filePath) - r.compressionMu.Lock() - defer r.compressionMu.Unlock() + st.mu.Lock() + defer st.mu.Unlock() - if r.pendingJob != job { + if st.pendingJob != job { return // cancelled or superseded } if err != nil { - // Compression failed — abandon the job rather than applying a - // truncated/unmodified snapshot over live messages. - r.pendingJob = nil + // Still the owner, so this is a genuine failure rather than a + // deliberate cancel (cancelPendingCompression cancels and clears + // pendingJob under the lock, so cancelled jobs fail the ownership + // check above and die silently). Abandon the job rather than + // applying a truncated/unmodified snapshot over live messages. + fmt.Fprintf(stdout.Writer(), "[ocr] Memory compression failed: %v\n", err) + st.pendingJob = nil close(job.done) return } @@ -288,10 +307,10 @@ func (r *Runner) triggerAsyncCompression(ctx context.Context, messages []llm.Mes // tryApplyPendingCompression checks whether a background compression has // completed and swaps the rebuilt messages into place. Returns true if // applied. -func (r *Runner) tryApplyPendingCompression(messages *[]llm.Message) bool { - r.compressionMu.Lock() - job := r.pendingJob - r.compressionMu.Unlock() +func (r *Runner) tryApplyPendingCompression(st *compressionState, messages *[]llm.Message) bool { + st.mu.Lock() + job := st.pendingJob + st.mu.Unlock() if job == nil { return false @@ -300,8 +319,8 @@ func (r *Runner) tryApplyPendingCompression(messages *[]llm.Message) bool { select { case <-job.done: applied := false - r.compressionMu.Lock() - if r.pendingJob == job && job.rebuilt != nil { + st.mu.Lock() + if st.pendingJob == job && job.rebuilt != nil { rebuilt := job.rebuilt // Preserve any messages appended after the snapshot was taken — // the background job only compressed messages[:snapshotLen]. @@ -311,23 +330,24 @@ func (r *Runner) tryApplyPendingCompression(messages *[]llm.Message) bool { *messages = rebuilt applied = true } - if r.pendingJob == job { - r.pendingJob = nil + if st.pendingJob == job { + st.pendingJob = nil } - r.compressionMu.Unlock() + st.mu.Unlock() return applied default: return false } } -// cancelPendingCompression aborts any in-flight background compression. -func (r *Runner) cancelPendingCompression() { - r.compressionMu.Lock() - defer r.compressionMu.Unlock() +// cancelPendingCompression aborts the conversation's in-flight background +// compression, if any. +func (r *Runner) cancelPendingCompression(st *compressionState) { + st.mu.Lock() + defer st.mu.Unlock() - if r.pendingJob != nil { - r.pendingJob.cancel() - r.pendingJob = nil + if st.pendingJob != nil { + st.pendingJob.cancel() + st.pendingJob = nil } } diff --git a/internal/llmloop/loop.go b/internal/llmloop/loop.go index 0b435203..1aa05403 100644 --- a/internal/llmloop/loop.go +++ b/internal/llmloop/loop.go @@ -39,8 +39,9 @@ type Deps struct { } // Runner is a per-session (across files) executor of the LLM tool-use -// loop. Token counters, warnings, and the optional background compression -// job are aggregated across every RunPerFile call. +// loop. Token counters and warnings are aggregated across every RunPerFile +// call; background memory compression is scoped to each RunPerFile +// conversation (see compressionState). type Runner struct { deps Deps totalInputTokens int64 // atomically updated @@ -51,8 +52,6 @@ type Runner struct { warnings []AgentWarning toolCallsMu sync.Mutex toolCalls map[string]int64 - compressionMu sync.Mutex - pendingJob *compressionJob } // NewRunner returns a Runner bound to the given dependencies. @@ -153,6 +152,11 @@ func (r *Runner) RunPerFile(ctx context.Context, messages []llm.Message, newPath consecutiveEmptyRounds := 0 sessionID := uuid.NewString() + // Async compression is owned by this conversation alone; the deferred + // cancel aborts any job still in flight when the conversation ends. + st := &compressionState{} + defer r.cancelPendingCompression(st) + for toolReqCount > 0 { select { case <-ctx.Done(): @@ -250,7 +254,7 @@ func (r *Runner) RunPerFile(ctx context.Context, messages []llm.Message, newPath consecutiveEmptyRounds = 0 } - succeed := r.addNextMessage(ctx, content, calls, results, &messages, newPath) + succeed := r.addNextMessage(ctx, content, calls, results, &messages, newPath, st) if !succeed { fmt.Fprintf(stdout.Writer(), "[ocr] Context compression exceeded threshold for %s, stopping.\n", newPath) break @@ -430,23 +434,23 @@ func (r *Runner) executeToolCall(ctx context.Context, newPath string, call llm.T // warning (80%) MaxTokens thresholds. Returns false when even after // synchronous compression the conversation is still over the warning // threshold — caller should stop the loop in that case. -func (r *Runner) addNextMessage(ctx context.Context, assistantContent string, toolCalls []llm.ToolCall, results []tool.ToolCallResult, messages *[]llm.Message, filePath string) bool { +func (r *Runner) addNextMessage(ctx context.Context, assistantContent string, toolCalls []llm.ToolCall, results []tool.ToolCallResult, messages *[]llm.Message, filePath string, st *compressionState) bool { maxAllowed := r.deps.Template.MaxTokens softLimit := int(float64(maxAllowed) * tokenSoftThreshold) warnLimit := int(float64(maxAllowed) * tokenWarningThreshold) - r.tryApplyPendingCompression(messages) - - tokenCount := CountMessagesTokens(*messages) - - if tokenCount > warnLimit { - r.cancelPendingCompression() - *messages, _ = r.runCompression(ctx, *messages, filePath) - tokenCount = CountMessagesTokens(*messages) - } - - if tokenCount > softLimit && r.pendingJob == nil { - r.triggerAsyncCompression(ctx, *messages, filePath) + r.tryApplyPendingCompression(st, messages) + + // A conversation can already be over the warning threshold before this + // round's messages are appended (e.g. an oversized initial prompt). + if CountMessagesTokens(*messages) > warnLimit { + r.cancelPendingCompression(st) + var err error + if *messages, err = r.runCompression(ctx, *messages, filePath); err != nil { + // Compression failed; continue with over-limit messages — the + // post-append check below will retry. + fmt.Fprintf(stdout.Writer(), "[ocr] Memory compression failed: %v\n", err) + } } if len(toolCalls) > 0 { @@ -461,11 +465,22 @@ func (r *Runner) addNextMessage(ctx context.Context, assistantContent string, to finalCount := CountMessagesTokens(*messages) if finalCount > warnLimit { - r.cancelPendingCompression() - *messages, _ = r.runCompression(ctx, *messages, filePath) + r.cancelPendingCompression(st) + var err error + if *messages, err = r.runCompression(ctx, *messages, filePath); err != nil { + fmt.Fprintf(stdout.Writer(), "[ocr] Memory compression failed: %v\n", err) + } + finalCount = CountMessagesTokens(*messages) + } + + // Trigger async compression only after all appends for this update, so + // a job is never started and then immediately cancelled by the same + // call (#384), and never started when we are about to return false. + if finalCount > softLimit && finalCount < warnLimit { + r.triggerAsyncCompression(ctx, st, *messages, filePath) } - return CountMessagesTokens(*messages) < warnLimit + return finalCount < warnLimit } // parseToolArgs unmarshals a tool call's raw JSON arguments, always diff --git a/internal/llmloop/runner_test.go b/internal/llmloop/runner_test.go index 6866cfdf..f578271c 100644 --- a/internal/llmloop/runner_test.go +++ b/internal/llmloop/runner_test.go @@ -2,7 +2,10 @@ package llmloop import ( "context" + "fmt" "strings" + "sync" + "sync/atomic" "testing" "github.com/open-code-review/open-code-review/internal/config/template" @@ -21,6 +24,60 @@ func (f *fakeLLMClient) CompletionsWithCtx(_ context.Context, _ llm.ChatRequest) return f.response, f.err } +// gatedLLMClient signals when a request starts, then blocks until release +// is closed. Requests whose context is canceled while blocked are counted, +// letting tests assert that no in-flight compression was aborted. +type gatedLLMClient struct { + started chan struct{} + release chan struct{} + canceled atomic.Int64 + response *llm.ChatResponse +} + +func (g *gatedLLMClient) CompletionsWithCtx(ctx context.Context, _ llm.ChatRequest) (*llm.ChatResponse, error) { + g.started <- struct{}{} + select { + case <-g.release: + return g.response, nil + case <-ctx.Done(): + g.canceled.Add(1) + return nil, ctx.Err() + } +} + +// concurrentFakeClient is goroutine-safe and distinguishes compression +// requests (no tools attached) from main-loop requests (tools attached), +// mirroring how runCompression and RunPerFile build their ChatRequests. +type concurrentFakeClient struct { + compressionCalls atomic.Int64 +} + +func (c *concurrentFakeClient) CompletionsWithCtx(_ context.Context, req llm.ChatRequest) (*llm.ChatResponse, error) { + if len(req.Tools) == 0 { + c.compressionCalls.Add(1) + summary := "compressed summary" + return &llm.ChatResponse{ + Choices: []llm.Choice{{Message: llm.ResponseMessage{Content: &summary}}}, + }, nil + } + content := strings.Repeat("word ", 650) + return &llm.ChatResponse{ + Choices: []llm.Choice{{ + Message: llm.ResponseMessage{ + Content: &content, + ToolCalls: []llm.ToolCall{{ + ID: "call_1", + Type: "function", + Function: llm.FunctionCall{ + Name: "file_read", + Arguments: `{"path":"f.go"}`, + }, + }}, + }, + }}, + }, nil +} + func newTestRunner(client llm.LLMClient, tpl template.Template) *Runner { sess := session.New(t_tempDir, "main", "test-model", session.SessionOptions{ReviewMode: "diff"}) collector := tool.NewCommentCollector() @@ -122,7 +179,7 @@ func TestCollectPendingComments_NilPool(t *testing.T) { func TestCancelPendingCompression_NilJob(t *testing.T) { t_tempDir = t.TempDir() r := newTestRunner(&fakeLLMClient{}, template.Template{}) - r.cancelPendingCompression() + r.cancelPendingCompression(&compressionState{}) } func TestCancelPendingCompression_WithJob(t *testing.T) { @@ -134,13 +191,13 @@ func TestCancelPendingCompression_WithJob(t *testing.T) { done: make(chan struct{}), cancel: func() { cancelled = true }, } - r.pendingJob = job - r.cancelPendingCompression() + st := &compressionState{pendingJob: job} + r.cancelPendingCompression(st) if !cancelled { t.Error("cancel was not called") } - if r.pendingJob != nil { + if st.pendingJob != nil { t.Error("pendingJob should be nil after cancel") } } @@ -149,7 +206,7 @@ func TestTryApplyPendingCompression_NilJob(t *testing.T) { t_tempDir = t.TempDir() r := newTestRunner(&fakeLLMClient{}, template.Template{}) msgs := []llm.Message{msg("user", "hi")} - if r.tryApplyPendingCompression(&msgs) { + if r.tryApplyPendingCompression(&compressionState{}, &msgs) { t.Error("expected false for nil job") } } @@ -162,10 +219,10 @@ func TestTryApplyPendingCompression_NotDone(t *testing.T) { done: make(chan struct{}), cancel: func() {}, } - r.pendingJob = job + st := &compressionState{pendingJob: job} msgs := []llm.Message{msg("user", "hi")} - if r.tryApplyPendingCompression(&msgs) { + if r.tryApplyPendingCompression(st, &msgs) { t.Error("expected false for non-completed job") } } @@ -182,7 +239,7 @@ func TestTryApplyPendingCompression_Applied(t *testing.T) { snapshotLen: 3, } close(job.done) - r.pendingJob = job + st := &compressionState{pendingJob: job} msgs := []llm.Message{ msg("system", "sys"), @@ -190,7 +247,7 @@ func TestTryApplyPendingCompression_Applied(t *testing.T) { msg("assistant", "resp"), msg("tool", "appended after snapshot"), } - applied := r.tryApplyPendingCompression(&msgs) + applied := r.tryApplyPendingCompression(st, &msgs) if !applied { t.Fatal("expected applied=true") } @@ -203,7 +260,7 @@ func TestTryApplyPendingCompression_Applied(t *testing.T) { if msgs[2].ExtractText() != "appended after snapshot" { t.Errorf("msgs[2] = %q, want appended after snapshot", msgs[2].ExtractText()) } - if r.pendingJob != nil { + if st.pendingJob != nil { t.Error("pendingJob should be nil after apply") } } @@ -219,14 +276,14 @@ func TestTryApplyPendingCompression_NilRebuilt(t *testing.T) { snapshotLen: 3, } close(job.done) - r.pendingJob = job + st := &compressionState{pendingJob: job} msgs := []llm.Message{msg("user", "hi")} - applied := r.tryApplyPendingCompression(&msgs) + applied := r.tryApplyPendingCompression(st, &msgs) if applied { t.Error("expected false when rebuilt is nil (compression failed)") } - if r.pendingJob != nil { + if st.pendingJob != nil { t.Error("pendingJob should be nil even on non-apply") } } @@ -435,11 +492,12 @@ func TestTriggerAsyncCompression(t *testing.T) { msgs = append(msgs, msg("tool", strings.Repeat("data ", 50))) } - r.triggerAsyncCompression(context.Background(), msgs, "test.go") + st := &compressionState{} + r.triggerAsyncCompression(context.Background(), st, msgs, "test.go") - r.compressionMu.Lock() - job := r.pendingJob - r.compressionMu.Unlock() + st.mu.Lock() + job := st.pendingJob + st.mu.Unlock() if job == nil { t.Fatal("expected pendingJob to be set") @@ -451,6 +509,213 @@ func TestTriggerAsyncCompression(t *testing.T) { } } +func TestCompression_CrossFileIsolation(t *testing.T) { + t_tempDir = t.TempDir() + summary := "summary A" + gated := &gatedLLMClient{ + started: make(chan struct{}, 1), + release: make(chan struct{}), + response: &llm.ChatResponse{ + Choices: []llm.Choice{{Message: llm.ResponseMessage{Content: &summary}}}, + }, + } + tpl := template.Template{ + MemoryCompressionTask: template.LlmConversation{ + Messages: []template.ChatMessage{{Role: "user", Content: "{{context}}"}}, + }, + MaxTokens: 50, + } + r := newTestRunner(gated, tpl) + + msgsA := []llm.Message{msg("system", "sys"), msg("user", "file A prompt")} + for i := 0; i < 10; i++ { + msgsA = append(msgsA, msg("assistant", strings.Repeat("word ", 100))) + msgsA = append(msgsA, msg("tool", strings.Repeat("data ", 50))) + } + + // File A starts an async compression; its LLM request is now in flight. + stA := &compressionState{} + r.triggerAsyncCompression(context.Background(), stA, msgsA, "a.go") + <-gated.started + + // File B, sharing the Runner, cancels and tries to consume a pending + // job. Neither operation may touch file A's in-flight compression. + stB := &compressionState{} + msgsB := []llm.Message{msg("system", "sys"), msg("user", "file B prompt")} + wantB := msgsB[1].ExtractText() + + r.cancelPendingCompression(stB) + if r.tryApplyPendingCompression(stB, &msgsB) { + t.Error("file B must not consume a compression job it never started") + } + if len(msgsB) != 2 || msgsB[1].ExtractText() != wantB { + t.Error("file B's messages must be unchanged") + } + + stA.mu.Lock() + pending := stA.pendingJob + stA.mu.Unlock() + if pending == nil { + t.Fatal("file A's job must still be pending after file B's cancel") + } + if got := gated.canceled.Load(); got != 0 { + t.Errorf("in-flight compression requests canceled = %d, want 0", got) + } + + // A second trigger while a job is pending must be a no-op, not a + // replacement — the in-flight job stays the owner of the slot. + r.triggerAsyncCompression(context.Background(), stA, msgsA, "a.go") + stA.mu.Lock() + replaced := stA.pendingJob != pending + stA.mu.Unlock() + if replaced { + t.Error("second trigger must not replace the in-flight job") + } + + // Let A's compression finish. + close(gated.release) + <-pending.done + + t.Run("SummaryAppliesOnlyToOwningConversation", func(t *testing.T) { + if r.tryApplyPendingCompression(stB, &msgsB) { + t.Error("file B must not receive file A's summary") + } + if len(msgsB) != 2 || msgsB[1].ExtractText() != wantB { + t.Error("file B's messages must be byte-identical") + } + + // A message appended after the snapshot must survive the apply. + msgsA = append(msgsA, msg("tool", "post-snapshot")) + if !r.tryApplyPendingCompression(stA, &msgsA) { + t.Fatal("file A should apply its own completed compression") + } + if !strings.Contains(msgsA[1].ExtractText(), "") { + t.Errorf("file A's rebuilt prompt should embed the summary, got: %s", msgsA[1].ExtractText()) + } + if last := msgsA[len(msgsA)-1].ExtractText(); last != "post-snapshot" { + t.Errorf("post-snapshot suffix lost, last message = %q", last) + } + }) +} + +func TestAddNextMessage_NoStartThenCancelSameCall(t *testing.T) { + t_tempDir = t.TempDir() + summary := "compressed summary" + // release is pre-closed: requests complete instantly, and canceled + // only increments if a compression context was aborted mid-flight. + gated := &gatedLLMClient{ + started: make(chan struct{}, 8), + release: make(chan struct{}), + response: &llm.ChatResponse{ + Choices: []llm.Choice{{Message: llm.ResponseMessage{Content: &summary}}}, + }, + } + close(gated.release) + tpl := template.Template{ + MemoryCompressionTask: template.LlmConversation{ + Messages: []template.ChatMessage{{Role: "user", Content: "{{context}}"}}, + }, + MaxTokens: 1000, // soft 600, warn 800 + } + r := newTestRunner(gated, tpl) + + // Pre-append the conversation sits between the soft and warning + // thresholds; the appended round pushes it over the warning threshold — + // the same-file case from #384 where a job was started and then + // cancelled within one update. + msgs := []llm.Message{ + msg("system", "sys"), + msg("user", "prompt"), + msg("assistant", strings.Repeat("word ", 400)), + msg("tool", strings.Repeat("data ", 290)), + } + calls := []llm.ToolCall{{ + ID: "c1", + Type: "function", + Function: llm.FunctionCall{Name: "file_read", Arguments: `{"path":"f.go"}`}, + }} + results := []tool.ToolCallResult{{ + ToolCallID: "c1", + Name: "file_read", + Result: strings.Repeat("data ", 100), + }} + + st := &compressionState{} + ok := r.addNextMessage(context.Background(), strings.Repeat("word ", 200), calls, results, &msgs, "f.go", st) + + if !ok { + t.Error("expected true: sync compression should bring the count under the warning threshold") + } + if got := gated.canceled.Load(); got != 0 { + t.Errorf("compression requests canceled mid-flight = %d, want 0", got) + } + st.mu.Lock() + pending := st.pendingJob + st.mu.Unlock() + if pending != nil { + t.Error("no async job should be pending after a sync compression in the same call") + } + if len(gated.started) != 1 { + t.Errorf("compression LLM calls = %d, want exactly 1 (the sync one)", len(gated.started)) + } + if !strings.Contains(msgs[1].ExtractText(), "") { + t.Errorf("sync compression should have rebuilt the prompt, got: %s", msgs[1].ExtractText()) + } +} + +func TestRunPerFile_ConcurrentFilesCompression_Race(t *testing.T) { + t_tempDir = t.TempDir() + tpl := template.Template{ + MemoryCompressionTask: template.LlmConversation{ + Messages: []template.ChatMessage{{Role: "user", Content: "{{context}}"}}, + }, + MaxTokens: 1000, + MaxToolRequestTimes: 5, + } + reg := tool.NewRegistry() + reg.Register(&fakeFileReadProvider{result: "package main\n"}) + client := &concurrentFakeClient{} + r := NewRunner(Deps{ + LLMClient: client, + Model: "test-model", + Template: tpl, + Tools: reg, + CommentCollector: tool.NewCommentCollector(), + // MainToolDefs must be non-empty: the fake client classifies a + // request with no tools as a compression request, mirroring how + // RunPerFile and runCompression build their ChatRequests. + MainToolDefs: []llm.ToolDef{{Type: "function", Function: llm.FunctionDef{Name: "file_read"}}}, + Session: session.New(t_tempDir, "main", "test-model", session.SessionOptions{ReviewMode: "diff"}), + }) + + // Four files reviewed concurrently over one shared Runner, like the + // agent/scan fan-out. Each conversation crosses the soft and warning + // thresholds repeatedly; the real assertion is -race cleanliness. + var wg sync.WaitGroup + errs := make([]error, 4) + for i := 0; i < 4; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + msgs := []llm.Message{msg("system", "sys"), msg("user", "review this file")} + _, err := r.RunPerFile(context.Background(), msgs, fmt.Sprintf("f%d.go", i)) + errs[i] = err + }(i) + } + wg.Wait() + for i, err := range errs { + if err != nil { + t.Errorf("file %d: RunPerFile: %v", i, err) + } + } + // Guard against the test going vacuous: if no compression request was + // ever issued, the conversations never crossed the thresholds and the + // -race assertion proved nothing. + if client.compressionCalls.Load() == 0 { + t.Error("no compression requests were made; the compression path was not exercised") + } +} + func TestStripMarkdownFences_AdditionalCases(t *testing.T) { tests := []struct { name string diff --git a/internal/telemetry/metrics.go b/internal/telemetry/metrics.go index fbbfafa1..e7c4ecb6 100644 --- a/internal/telemetry/metrics.go +++ b/internal/telemetry/metrics.go @@ -67,6 +67,8 @@ func ensureMetrics() { checkMetricErr(err) } +// checkMetricErr intentionally ignores metric registration errors: +// telemetry is best-effort and must not interrupt the main flow. func checkMetricErr(err error) {} func RecordReviewDuration(ctx context.Context, dur time.Duration) {