mirror of
https://github.com/alibaba/open-code-review.git
synced 2026-09-14 19:59:52 +08:00
9cd9297305
* refactor(llmloop): rename RunPerFile to RunMainTask The loop stopped being per-file when review switched from one subtask per file to one subtask per group: internal/agent passes a groupKey that may name several files, while internal/scan still passes a single path. The name asserted a granularity only one of the two callers has. RunMainTask names what the function actually drives — one MAIN_TASK conversation — matching the vocabulary already in place around it (MainLoopStop, the main.loop span, session.MainTask). RunSubtask would have been a second misfit, since review calls this once per review round and one subtask spans several rounds. The newPath parameter is renamed taskKey along the whole private chain (runGraceRound, executeToolCall, addNextMessage, runCompression, triggerAsyncCompression, requestCtx). Deps.NewRequestMeta keeps filePath: the session layer still spells that slot "file path" in its persisted records, and requestCtx now documents the seam instead of hiding it. ToolFailureDetail.FilePath is where a group key becomes externally visible — it reaches --format json as file_path — so the field now says so rather than letting a consumer assume it parses as one path. Two comments claimed executeToolCall overrides a comment's path with the loop's path argument. It has always been a fallback that applies only when the model named no path, and the difference decides whether a grouped review files its comments under the right files, so both now describe the fallback. Pure rename plus comment edits; no behavior change. * refactor(cli): retire per-file naming left over from grouped review perFileTimeout was the last holdout of the old granularity in the CLI options: its flag is --timeout, its help already said "concurrent task timeout", and it feeds Args.ConcurrentTaskTimeout. Only the field name still claimed a per-file deadline it never set. Two review help strings also outlived the change to grouped subtasks, and disagreed with cli-reference.md, which already documents both as per-group: --concurrency dispatches file groups, and --max-tokens caps a group's prompt. The scan command keeps its per-file wording, where one subtask is still one file. Same for six agent comments that describe per-group work as per-file — the budget look-ahead, the dispatch step and its function doc, the only deadline classifyItemError can see, and the goroutines allDiffs is read from — plus Run's pipeline summary, which omitted the grouping step altogether. Granularity that really is per-file stays as it is, and the two are easy to confuse: filterLargeDiffs measures each file's diff against the token ceiling on its own, review items and their status badges are per file, and a group's prompt embeds one XML element per file. The comment in review_cmd.go now names filterLargeDiffs so the distinction is checkable rather than a matter of trusting the adjective. The estimator keeps both its name avgMainRoundsPerFile and its arithmetic: it still bills every file for its own MAIN_TASK rounds, so a group of files is over-estimated. That is a costing bug rather than a naming one and needs its own change; only estimateDiffFileTokens' doc comment is corrected here, since the look-ahead consuming it sums per group.
391 lines
12 KiB
Go
391 lines
12 KiB
Go
// SPDX-License-Identifier: Apache-2.0
|
|
// Copyright 2026 alibaba/open-code-review Contributors
|
|
|
|
package llmloop
|
|
|
|
import (
|
|
"context"
|
|
"fmt"
|
|
"strings"
|
|
"sync"
|
|
"sync/atomic"
|
|
"time"
|
|
|
|
"github.com/alibaba/open-code-review/internal/llm"
|
|
"github.com/alibaba/open-code-review/internal/session"
|
|
"github.com/alibaba/open-code-review/internal/stdout"
|
|
)
|
|
|
|
// Compression thresholds, as fractions of MaxTokens.
|
|
const (
|
|
tokenSoftThreshold = 0.60 // async background compression
|
|
tokenWarningThreshold = 0.80 // immediate sync compression
|
|
)
|
|
|
|
// PromptTokenLimit returns tokenWarningThreshold (80%) of maxTokens. It is
|
|
// shared by the agent and scan pre-flight gates, their large-input filters, and
|
|
// computeActiveZoneSize so the threshold has a single definition. Non-positive
|
|
// input is not special-cased — each caller decides what that means.
|
|
func PromptTokenLimit(maxTokens int) int {
|
|
return int(float64(maxTokens) * tokenWarningThreshold)
|
|
}
|
|
|
|
// round groups consecutive messages starting with an assistant message
|
|
// followed by zero or more tool result messages.
|
|
type round struct {
|
|
assistantIdx int
|
|
toolIdxs []int
|
|
}
|
|
|
|
// partitionResult describes how messages should be split for compression.
|
|
type partitionResult struct {
|
|
frozenEnd int
|
|
compressEnd int
|
|
rounds []round
|
|
activeCount int
|
|
}
|
|
|
|
// compressionJob tracks an in-flight background compression operation.
|
|
type compressionJob struct {
|
|
done chan struct{}
|
|
rebuilt []llm.Message
|
|
cancel context.CancelFunc
|
|
snapshotLen int // message count when the snapshot was taken
|
|
}
|
|
|
|
// compressionState is the async-compression bookkeeping for a single
|
|
// conversation (one RunMainTask call). The Runner is shared by concurrent
|
|
// per-subtask goroutines, so this state must not live on the Runner: a shared
|
|
// slot lets one subtask apply, cancel, or replace another subtask's
|
|
// compression job (#384).
|
|
type compressionState struct {
|
|
mu sync.Mutex
|
|
pendingJob *compressionJob
|
|
}
|
|
|
|
// messageTokens counts visible text plus the Native replay payload that
|
|
// ExtractText() does not see.
|
|
func messageTokens(m llm.Message) int {
|
|
return llm.CountTokens(m.ExtractText()) + m.Native.EstimatedTokens()
|
|
}
|
|
|
|
// CountMessagesTokens returns the rough token count of msgs by summing the
|
|
// per-message token count. Exported because both review and scan top
|
|
// layers may want it for pre-flight checks.
|
|
func CountMessagesTokens(msgs []llm.Message) int {
|
|
var total int
|
|
for _, m := range msgs {
|
|
total += messageTokens(m)
|
|
}
|
|
return total
|
|
}
|
|
|
|
// groupIntoRounds parses messages[start:] into logical
|
|
// (assistant + tool_results) pairs.
|
|
func groupIntoRounds(messages []llm.Message, start int) []round {
|
|
var rounds []round
|
|
i := start
|
|
for i < len(messages) {
|
|
if messages[i].Role == "assistant" {
|
|
r := round{assistantIdx: i}
|
|
i++
|
|
for i < len(messages) && messages[i].Role == "tool" {
|
|
r.toolIdxs = append(r.toolIdxs, i)
|
|
i++
|
|
}
|
|
rounds = append(rounds, r)
|
|
} else {
|
|
i++
|
|
}
|
|
}
|
|
return rounds
|
|
}
|
|
|
|
// computeActiveZoneSize returns how many trailing rounds fit within the
|
|
// remaining token budget after accounting for the frozen zone and the
|
|
// compressed summary.
|
|
func computeActiveZoneSize(rounds []round, messages []llm.Message, maxTokens int, reservedTokens int) int {
|
|
budget := PromptTokenLimit(maxTokens) - reservedTokens
|
|
if budget <= 0 {
|
|
return 0
|
|
}
|
|
|
|
count := 0
|
|
tokensUsed := 0
|
|
for i := len(rounds) - 1; i >= 0; i-- {
|
|
roundTokens := messageTokens(messages[rounds[i].assistantIdx])
|
|
for _, ti := range rounds[i].toolIdxs {
|
|
roundTokens += messageTokens(messages[ti])
|
|
}
|
|
if tokensUsed+roundTokens > budget {
|
|
break
|
|
}
|
|
tokensUsed += roundTokens
|
|
count++
|
|
}
|
|
return count
|
|
}
|
|
|
|
// partitionMessages divides messages into frozen, compress, and active zones.
|
|
// Frozen zone is always messages[0:2]. Active zone preserves the K most
|
|
// recent complete rounds based on available token budget.
|
|
func partitionMessages(messages []llm.Message, maxTokens int, prevSummaryTokenEstimate int) partitionResult {
|
|
result := partitionResult{frozenEnd: 2}
|
|
if len(messages) <= 2 {
|
|
result.compressEnd = len(messages)
|
|
return result
|
|
}
|
|
|
|
result.rounds = groupIntoRounds(messages, 2)
|
|
if len(result.rounds) == 0 {
|
|
result.compressEnd = len(messages)
|
|
return result
|
|
}
|
|
|
|
result.activeCount = computeActiveZoneSize(result.rounds, messages, maxTokens, prevSummaryTokenEstimate)
|
|
if result.activeCount >= len(result.rounds) {
|
|
// Everything fits — no compression needed.
|
|
result.compressEnd = len(messages)
|
|
result.activeCount = 0
|
|
return result
|
|
}
|
|
|
|
// compressEnd = index after the last round NOT in active zone.
|
|
activeStartIdx := len(result.rounds) - result.activeCount
|
|
lastCompressRound := result.rounds[activeStartIdx-1]
|
|
if len(lastCompressRound.toolIdxs) > 0 {
|
|
result.compressEnd = lastCompressRound.toolIdxs[len(lastCompressRound.toolIdxs)-1] + 1
|
|
} else {
|
|
result.compressEnd = lastCompressRound.assistantIdx + 1
|
|
}
|
|
|
|
return result
|
|
}
|
|
|
|
// StripMarkdownFences removes ```json and ``` wrappers some models add
|
|
// around structured outputs. Exposed so callers (e.g. agent's review-filter
|
|
// post-step) that parse LLM JSON output can reuse the same heuristic.
|
|
func StripMarkdownFences(s string) string { return stripMarkdownFences(s) }
|
|
|
|
// stripMarkdownFences is the package-private workhorse used by the
|
|
// internal compression code paths.
|
|
func stripMarkdownFences(s string) string {
|
|
s = strings.TrimSpace(s)
|
|
if strings.HasPrefix(s, "```") {
|
|
if nl := strings.IndexByte(s, '\n'); nl >= 0 {
|
|
s = s[nl+1:]
|
|
} else {
|
|
s = strings.TrimPrefix(s, "```json")
|
|
s = strings.TrimPrefix(s, "```")
|
|
}
|
|
}
|
|
s = strings.TrimSpace(s)
|
|
if strings.HasSuffix(s, "```") {
|
|
s = strings.TrimSuffix(s, "```")
|
|
s = strings.TrimSpace(s)
|
|
}
|
|
return s
|
|
}
|
|
|
|
// buildMessageXML serializes msgs into the <message><content> form expected
|
|
// by the MEMORY_COMPRESSION_TASK prompt template.
|
|
func buildMessageXML(msgs []llm.Message) string {
|
|
var sb strings.Builder
|
|
for i, m := range msgs {
|
|
sb.WriteString(fmt.Sprintf("<message id=\"%d\" role=\"%s\">\n", i, m.Role))
|
|
sb.WriteString(" <content>\n")
|
|
sb.WriteString(fmt.Sprintf(" %s\n", m.ExtractText()))
|
|
sb.WriteString(" </content>\n")
|
|
if m.ReasoningContent != "" {
|
|
sb.WriteString(" <reasoning>\n")
|
|
sb.WriteString(fmt.Sprintf(" %s\n", m.ReasoningContent))
|
|
sb.WriteString(" </reasoning>\n")
|
|
}
|
|
sb.WriteString("</message>")
|
|
if i < len(msgs)-1 {
|
|
sb.WriteString("\n")
|
|
}
|
|
}
|
|
return sb.String()
|
|
}
|
|
|
|
// copyMessages creates a shallow copy of a message slice.
|
|
func copyMessages(msgs []llm.Message) []llm.Message {
|
|
out := make([]llm.Message, len(msgs))
|
|
copy(out, msgs)
|
|
return out
|
|
}
|
|
|
|
// runCompression performs three-zone memory compression on the given
|
|
// messages, summarizing the compress zone while preserving the active zone
|
|
// intact. Returns rebuilt as [frozen] + [compressed_summary appended to
|
|
// the user prompt] + [active].
|
|
func (r *Runner) runCompression(ctx context.Context, msgs []llm.Message, taskKey string) ([]llm.Message, error) {
|
|
if len(r.deps.Template.MemoryCompressionTask.Messages) == 0 || len(msgs) <= 2 {
|
|
return msgs[:min(len(msgs), 2)], nil
|
|
}
|
|
|
|
part := partitionMessages(msgs, r.deps.Template.MaxTokens, 0)
|
|
if part.compressEnd <= part.frozenEnd {
|
|
return msgs, nil
|
|
}
|
|
|
|
contextXML := buildMessageXML(msgs[part.frozenEnd:part.compressEnd])
|
|
|
|
compressionMsgs := make([]llm.Message, 0, len(r.deps.Template.MemoryCompressionTask.Messages))
|
|
for _, m := range r.deps.Template.MemoryCompressionTask.Messages {
|
|
content := strings.ReplaceAll(m.Content, "{{context}}", contextXML)
|
|
compressionMsgs = append(compressionMsgs, llm.NewTextMessage(m.Role, content))
|
|
}
|
|
|
|
// The task record is created before the request, not after it, because the
|
|
// retry report keys request identity on RequestNo and that number only
|
|
// exists once the record does. The visible consequence is that the
|
|
// llm_request line reaches the session JSONL before the response: a run
|
|
// killed mid-request now leaves an llm_request with no response, which
|
|
// resume ignores (applyResumeLine has no case for it).
|
|
fs := r.deps.Session.GetOrCreateFileSession(taskKey)
|
|
rec := fs.AppendTaskRecord(session.MemoryCompressionTask, compressionMsgs)
|
|
|
|
ctx = llm.ContextWithSessionKey(ctx,
|
|
llm.SessionTaskKey(r.deps.Session.SessionID, string(session.MemoryCompressionTask), taskKey))
|
|
|
|
startTime := time.Now()
|
|
reqCtx := r.requestCtx(ctx, taskKey, session.MemoryCompressionTask, rec.RequestNo)
|
|
resp, err := r.deps.LLMClient.CompletionsWithCtx(reqCtx, llm.ChatRequest{
|
|
Model: r.deps.Model,
|
|
Messages: compressionMsgs,
|
|
MaxTokens: r.deps.Template.CompletionTokenLimit(),
|
|
})
|
|
duration := time.Since(startTime)
|
|
|
|
if err != nil {
|
|
rec.SetError(err, duration)
|
|
// Return msgs unchanged: truncating to frozenEnd would discard all
|
|
// conversation context, which is worse than staying over the token
|
|
// limit temporarily.
|
|
return msgs, fmt.Errorf("memory compression: %w", err)
|
|
}
|
|
rec.SetResponse(resp, duration)
|
|
if resp.Usage != nil {
|
|
atomic.AddInt64(&r.totalInputTokens, resp.Usage.PromptTokens)
|
|
atomic.AddInt64(&r.totalOutputTokens, resp.Usage.CompletionTokens)
|
|
atomic.AddInt64(&r.totalCacheReadTokens, resp.Usage.CacheReadTokens)
|
|
atomic.AddInt64(&r.totalCacheWriteTokens, resp.Usage.CacheWriteTokens)
|
|
}
|
|
|
|
rawSummary := stripMarkdownFences(resp.Content())
|
|
if rawSummary == "" {
|
|
// Empty summary: keep the original conversation rather than dropping
|
|
// everything below the frozen zone.
|
|
return msgs, nil
|
|
}
|
|
|
|
rebuilt := make([]llm.Message, 2)
|
|
copy(rebuilt, msgs[:2])
|
|
|
|
userMsg := rebuilt[1]
|
|
currentText := userMsg.ExtractText()
|
|
rebuilt[1] = llm.NewTextMessage(userMsg.Role, currentText+"\n\n<previous_review_summary>\n"+rawSummary+"\n</previous_review_summary>")
|
|
|
|
for i := part.compressEnd; i < len(msgs); i++ {
|
|
rebuilt = append(rebuilt, msgs[i])
|
|
}
|
|
|
|
return rebuilt, nil
|
|
}
|
|
|
|
// 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, taskKey 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)}
|
|
st.pendingJob = job
|
|
st.mu.Unlock()
|
|
|
|
// Registered before the goroutine starts so WaitBackground can never miss
|
|
// a job that was launched but has not run yet.
|
|
r.bg.Add(1)
|
|
go func() {
|
|
defer r.bg.Done()
|
|
defer cancel()
|
|
rebuilt, err := r.runCompression(asyncCtx, msgSnapshot, taskKey)
|
|
|
|
st.mu.Lock()
|
|
defer st.mu.Unlock()
|
|
|
|
if st.pendingJob != job {
|
|
return // cancelled or superseded
|
|
}
|
|
if err != 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
|
|
}
|
|
job.rebuilt = rebuilt
|
|
close(job.done)
|
|
}()
|
|
}
|
|
|
|
// tryApplyPendingCompression checks whether a background compression has
|
|
// completed and swaps the rebuilt messages into place. Returns true if
|
|
// applied.
|
|
func (r *Runner) tryApplyPendingCompression(st *compressionState, messages *[]llm.Message) bool {
|
|
st.mu.Lock()
|
|
job := st.pendingJob
|
|
st.mu.Unlock()
|
|
|
|
if job == nil {
|
|
return false
|
|
}
|
|
|
|
select {
|
|
case <-job.done:
|
|
applied := false
|
|
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].
|
|
if job.snapshotLen < len(*messages) {
|
|
rebuilt = append(rebuilt, (*messages)[job.snapshotLen:]...)
|
|
}
|
|
*messages = rebuilt
|
|
applied = true
|
|
}
|
|
if st.pendingJob == job {
|
|
st.pendingJob = nil
|
|
}
|
|
st.mu.Unlock()
|
|
return applied
|
|
default:
|
|
return false
|
|
}
|
|
}
|
|
|
|
// 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 st.pendingJob != nil {
|
|
st.pendingJob.cancel()
|
|
st.pendingJob = nil
|
|
}
|
|
}
|