alibaba / alibaba/open-code-review

Three-zone compression summarizes or drops live rounds it is meant to preserve

Open
#838 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
24.4k
Forks
1.8k
Avg merge
2d 6h
Merged PRs (30d)
105

Description

### Summary

Three-zone compression (`internal/llmloop/compression.go`) discards live context it is designed to preserve, in three related ways. Split out of #823 per review feedback there — that PR now only fixes the no-tool retry bound; this issue tracks the compression-zone behavior on its own.

Verified on `main` @ 071debe.

### 1. Background compression summarizes the entire live tail

In `partitionMessages`, when every round still fits the token budget, the "everything fits — no compression needed" branch sets `compressEnd = len(messages)` and `activeCount = 0` — which marks the *whole tail* as the compress zone. `runCompression` then summarizes every round, however recent, into the prompt.

Because the async trigger fires at 60% of `MaxTokens` while the budget is 80%, "everything fits" is the *normal* case for background compression: in practice the active zone never survives a background pass.

### 2. The frozen zone's tokens are never reserved from the budget

`computeActiveZoneSize` receives only `prevSummaryTokenEstimate` as reserved tokens. The system prompt + user prompt (which grow as summaries get folded in) never reduce the room available to active rounds, so the active zone can overfill past the warning threshold.

The two defects compound: an unreserved frozen zone makes rounds *appear* to fit, which takes the everything-fits branch from defect 1 and summarizes all of them (the reproduction below shows `activeCount = 0` rather than an oversized active zone, for exactly this reason).

### 3. A missing compression template truncates the conversation to 2 messages

`runCompression` returns `msgs[:min(len(msgs), 2)]` when `MemoryCompressionTask` has no messages — for configs without a compression template, every round of live context is dropped on the spot.

### Reproduction

Drop this file into `internal/llmloop` as `compression_zone_repro_test.go` (each test asserts the intended behavior, so each failure prints the defect):

```go
package llmloop

import (
"context"
"strings"
"testing"

"github.com/alibaba/open-code-review/internal/llm"
"github.com/alibaba/open-code-review/internal/config/template"
)

// Repro 1: when every round fits the budget (the normal case for the async
// trigger, which fires at 60% while the budget is 80%), partitionMessages
// marks the WHOLE tail as the compress zone instead of leaving it empty.
func TestRepro_EverythingFitsSummarizesLiveTail(t *testing.T) {
messages := []llm.Message{
msg("system", "sys"),
msg("user", "prompt"),
msg("assistant", "resp1"),
msg("tool", "result1"),
msg("assistant", "resp2"),
msg("tool", "result2"),
}
result := partitionMessages(messages, 1_000_000, 0)
if result.compressEnd != result.frozenEnd {
t.Errorf("compressEnd = %d, want %d (empty compress zone): every live round is marked for summarization", result.compressEnd, result.frozenEnd)
}
}

// Repro 2: the frozen zone's own tokens are not counted against the prompt
// budget, so the active zone can overfill past the warning threshold.
func TestRepro_FrozenZoneNotReservedFromBudget(t *testing.T) {
roundText := strings.Repeat("round content ", 100)
messages := []llm.Message{
msg("system", "sys"),
msg("user", strings.Repeat("frozen prompt content ", 200)),
msg("assistant", roundText),
msg("tool", roundText),
msg("assistant", roundText),
msg("tool", roundText),
}

frozenTokens := CountMessagesTokens(messages[:2])
oneRound := CountMessagesTokens(messages[2:4])
budget := frozenTokens + oneRound + oneRound/2
maxTokens := budget * 5 / 4 // PromptTokenLimit is 80% of MaxTokens

actualBudget := PromptTokenLimit(maxTokens)
if actualBudget-frozenTokens < oneRound || actualBudget-frozenTokens >= 2*oneRound || actualBudget < 2*oneRound {
t.Fatalf("test setup out of range: budget=%d frozen=%d round=%d", actualBudget, frozenTokens, oneRound)
}

result := partitionMessages(messages, maxTokens, 0)
if result.activeCount != 1 {
t.Errorf("activeCount = %d, want 1: frozen zone must reserve its own tokens from the budget", result.activeCount)
}
}

// Repro 3: a config without a compression template truncates the whole
// conversation to the frozen zone instead of leaving it alone.
func TestRepro_MissingTemplateTruncatesConversation(t *testing.T) {
t_tempDir = t.TempDir()
r := newTestRunner(&fakeLLMClient{}, template.Template{MaxTokens: 1000})

msgs := []llm.Message{
msg("system", "sys"),
msg("user", "prompt"),
msg("assistant", "resp"),
}
got, err := r.runCompression(context.Background(), msgs, "test.go")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if len(got) != len(msgs) {
t.Errorf("runCompression returned %d messages, want %d: live context dropped when no template is configured", len(got), len(msgs))
}
}
```

Actual output on `main` @ 071debe:

```
--- FAIL: TestRepro_EverythingFitsSummarizesLiveTail (0.06s)
compression_zone_repro_test.go:29: compressEnd = 6, want 2 (empty compress zone): every live round is marked for summarization
--- FAIL: TestRepro_FrozenZoneNotReservedFromBudget (0.00s)
compression_zone_repro_test.go:58: activeCount = 0, want 1: frozen zone must reserve its own tokens from the budget
--- FAIL: TestRepro_MissingTemplateTruncatesConversation (0.00s)
compression_zone_repro_test.go:78: runCompression returned 2 messages, want 3: live context dropped when no template is configured
```

### Candidate direction

One candidate fix (previously part of #823, kept for reference in [its pre-split revision](https://github.com/alibaba/open-code-review/pull/823/commits)): represent the empty compress zone as `compressEnd == frozenEnd`, reserve `CountMessagesTokens(messages[:frozenEnd]) + prevSummaryTokenEstimate` when sizing the active zone, and make `runCompression` return the conversation unchanged when no template is configured. The maintainers suggested there may be a better overall design for background compression — happy to implement whichever direction is preferred, or to leave this to the team.

Contributor guide

Open the contributing guide

Research direction

Start in internal/llmloop/compression.go by reading partitionMessages and runCompression, then add or run the compression_zone_repro_test.go cases from the issue. Compare the behavior with the three stated preservation requirements; done means all three regression cases pass without dropping live context.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
ai
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
50/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.