feat: add native OpenAI Responses compaction - #3
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughAdds provider-native OpenAI Responses compaction with opaque state preservation, replacement-window context installation, fallback to local summarization, cross-provider filtering, and tests and documentation. ChangesOpenAI Compaction State
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant FullCompaction
participant AgentLLMRequesterService
participant OpenAIResponsesChatProvider
participant ContextMemory
FullCompaction->>AgentLLMRequesterService: request provider compaction
AgentLLMRequesterService->>OpenAIResponsesChatProvider: call compact with history
OpenAIResponsesChatProvider-->>AgentLLMRequesterService: replacement messages and usage
AgentLLMRequesterService-->>FullCompaction: ProviderCompactionResult
FullCompaction->>ContextMemory: apply replacement window
ContextMemory-->>FullCompaction: compaction complete
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/agent-core/src/agent/index.ts (1)
323-346: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNative compact requests bypass request logging/recording.
compactProviderdoesn't callllmRequestLogger.logRequest/llmRequestRecorder.recordthe waygenerate'srunclosure does, so native compaction requests won't appear in the same request log/record trail as ordinary generate calls (including the local-summarizer fallback, which does log via the normalgeneratepath). Telemetry usage is still captured separately, so this is an observability nice-to-have rather than a functional gap.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/agent-core/src/agent/index.ts` around lines 323 - 346, Update compactProvider to wrap native provider.compact calls with the same llmRequestLogger.logRequest and llmRequestRecorder.record flow used by generate’s run closure, including both authenticated and unauthenticated paths. Preserve the existing auth resolution and compaction behavior while ensuring every native compaction request is logged and recorded.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/agent-core/src/agent/compaction/full.ts`:
- Around line 454-479: The remote compaction path in the visible branch of full
compaction must preserve real user messages appended during the provider call.
In the `remote` handling around `applyCompaction`, append the live history tail
after `originalHistory.length` to `remote.messages` when constructing
`replacementMessages`; apply the same change to the corresponding v2
remote-compaction branch, while retaining the existing validation and
cancellation behavior.
In `@packages/agent-core/src/agent/context/index.ts`:
- Around line 315-347: The replacementMessages branch in the compaction flow
must add a compaction boundary marker compatible with undo() before or alongside
the retained provider messages. Ensure native/remote compaction creates a
compaction_summary-origin checkpoint so undo() stops at the boundary and does
not remove both the proxy checkpoint and retained user prompt; preserve the
existing result bookkeeping and history replacement behavior.
---
Nitpick comments:
In `@packages/agent-core/src/agent/index.ts`:
- Around line 323-346: Update compactProvider to wrap native provider.compact
calls with the same llmRequestLogger.logRequest and llmRequestRecorder.record
flow used by generate’s run closure, including both authenticated and
unauthenticated paths. Preserve the existing auth resolution and compaction
behavior while ensuring every native compaction request is logged and recorded.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5e824882-4791-4e16-84f1-14698093fa6b
📒 Files selected for processing (48)
.changeset/preserve-openai-compaction-state.mddocs/en/guides/sessions.mddocs/zh/guides/sessions.mdpackages/agent-core-v2/docs/wire-manifest.d.tspackages/agent-core-v2/src/agent/contextMemory/compactionHandoff.tspackages/agent-core-v2/src/agent/contextMemory/contextMemory.tspackages/agent-core-v2/src/agent/contextMemory/contextMemoryService.tspackages/agent-core-v2/src/agent/contextMemory/contextOps.tspackages/agent-core-v2/src/agent/contextMemory/messageProjection.tspackages/agent-core-v2/src/agent/fullCompaction/fullCompactionService.tspackages/agent-core-v2/src/agent/llmRequester/llmRequester.tspackages/agent-core-v2/src/agent/llmRequester/llmRequesterService.tspackages/agent-core-v2/src/agent/loop/loopService.tspackages/agent-core-v2/src/agent/mcp/output.tspackages/agent-core-v2/src/kosong/contract/message.tspackages/agent-core-v2/src/kosong/contract/provider.tspackages/agent-core-v2/src/kosong/contract/tokens.tspackages/agent-core-v2/src/kosong/model/modelRequester.tspackages/agent-core-v2/src/kosong/model/modelRequesterImpl.tspackages/agent-core-v2/src/kosong/provider/bases/google-genai/google-genai.tspackages/agent-core-v2/src/kosong/provider/bases/openai/openai-common.tspackages/agent-core-v2/src/kosong/provider/bases/openai/openai-legacy.tspackages/agent-core-v2/src/kosong/provider/bases/openai/openai-responses.tspackages/agent-core-v2/test/agent/contextMemory/context.test.tspackages/agent-core-v2/test/agent/fullCompaction/fullCompaction.test.tspackages/agent-core-v2/test/agent/llmRequester/llmRequesterService.test.tspackages/agent-core-v2/test/kosong/provider/composition.test.tspackages/agent-core/src/agent/compaction/full.tspackages/agent-core/src/agent/compaction/types.tspackages/agent-core/src/agent/context/index.tspackages/agent-core/src/agent/index.tspackages/agent-core/src/mcp/output.tspackages/agent-core/src/services/message/message.tspackages/agent-core/src/services/message/transcript.tspackages/agent-core/src/utils/tokens.tspackages/agent-core/test/agent/context.test.tspackages/agent-core/test/services/message-transcript.test.tspackages/kosong/src/message.tspackages/kosong/src/provider.tspackages/kosong/src/providers/google-genai.tspackages/kosong/src/providers/kimi.tspackages/kosong/src/providers/openai-common.tspackages/kosong/src/providers/openai-legacy.tspackages/kosong/src/providers/openai-responses.tspackages/kosong/test/kimi.test.tspackages/kosong/test/openai-legacy.test.tspackages/kosong/test/openai-responses.test.tspackages/kosong/test/type-safety.test.ts
| if (remote !== undefined) { | ||
| const newHistory = this.agent.context.history; | ||
| for (let i = 0; i < originalHistory.length; i++) { | ||
| if (newHistory[i] !== originalHistory[i]) { | ||
| this.cancel(); | ||
| return undefined; | ||
| } | ||
| } | ||
| if ( | ||
| newHistory | ||
| .slice(originalHistory.length) | ||
| .some((message) => !isRealUserInput(message)) | ||
| ) { | ||
| this.cancel(); | ||
| return undefined; | ||
| } | ||
| if (remote.usage !== undefined) { | ||
| this.agent.usage.record(model, remote.usage); | ||
| } | ||
| const result = this.agent.context.applyCompaction({ | ||
| summary: REMOTE_COMPACTION_SUMMARY, | ||
| contextSummary: REMOTE_COMPACTION_SUMMARY, | ||
| replacementMessages: remote.messages, | ||
| compactedCount: originalHistory.length, | ||
| tokensBefore, | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -B5 -A20 "replacementMessages: remote.messages" packages/agent-core-v2/src/agent/fullCompaction/fullCompactionService.tsRepository: YaseenHQ/kimi
Length of output: 1380
🏁 Script executed:
#!/bin/bash
set -eu
echo "## locate files"
fd -a 'full.ts|fullCompactionService.ts|context/index.ts|interface.ts' packages | sed 's#^\./##'
echo
echo "## inspect full.ts relevant sections"
sed -n '400,540p' packages/agent-core/src/agent/compaction/full.ts
echo
echo "## inspect fullCompactionService.ts remote section"
sed -n '520,590p' packages/agent-core-v2/src/agent/fullCompaction/fullCompactionService.py 2>/dev/null || sed -n '520,590p' packages/agent-core-v2/src/agent/fullCompaction/fullCompactionService.ts
echo
echo "## inspect applyCompaction implementation candidates"
fd -a 'context/index.ts|context.ts|interface.ts' packages/agent-core packages/agent-core-v2 | while read -r f; do
echo "--- $f"
rg -n -B5 -A40 "applyCompaction|replacementMessages|_history" "$f"
done
echo
echo "## behavioral probe of replacementMessages semantics from source"
python3 - <<'PY'
import re, pathlib
for root in ["packages/agent-core","packages/agent-core-v2"]:
files=list(pathlib.Path(root).rglob("*"))
for f in files:
if f.is_file() and "context" in str(f).split("/")[-2:][-1].lower():
p=root+"/"+str(f).replace(str(root+"/"),"")
text=f.read_text(errors="ignore")
if "applyCompaction" in text and "replacementMessages" in text:
print(f"--- {p}")
for line_no, line in enumerate(text.splitlines(),1):
if "applyCompaction" in line or "replacementMessages" in line or "_history" in line:
print(f"{line_no}: {line}")
PYRepository: YaseenHQ/kimi
Length of output: 9668
🏁 Script executed:
#!/bin/bash
set -eu
echo "## v2 applyCompaction definitions/usages"
rg -n -B10 -A40 "applyCompaction|replacementMessages" packages/agent-core-v2/src packages/agent-core/src | head -n 220
echo
echo "## agent-core context compact implementation"
path="$(fd 'context.ts|Context.ts|context/index.ts' packages/agent-core/src | head -n 5 | tr '\n' ' ')"
for f in $path; do
echo "--- $f"
wc -l "$f"
rg -n -B10 -A50 "applyCompaction|replacementMessages|_history|this\.history" "$f"
done
echo
echo "## local compaction tail handling around local path"
line="$(rg -n "collectCompactableUserMessages|replacementMessages:" packages/agent-core/src/agent/compaction/full.ts | tail -n 20 | awk -F: '{print $1}')"
for l in $line; do
n=$((l-35)); [ "$n" -lt 1 ] && n=1
e=$((l+45))
echo "--- lines $n-$e"
sed -n "${n},${e}p" packages/agent-core/src/agent/compaction/full.ts
doneRepository: YaseenHQ/kimi
Length of output: 24015
🏁 Script executed:
#!/bin/bash
set -eu
echo "## agent-core context applyCompaction"
rg -n -B20 -A80 "applyCompaction|replacementMessages|_history|context" packages/agent-core/src/profile/context.ts
echo
echo "## agent-core-v2 applyCompaction definitions"
rg -n -B25 -A65 "applyCompaction|replacementMessages|set\(|this\([^)]*history" packages/agent-core-v2/src/agent | head -n 300
echo
echo "## read-only source-based consistency check"
python3 - <<'PY'
from pathlib import Path
import re
for root in ["packages/agent-core/src/agent/compaction/full.ts",
"packages/agent-core-v2/src/agent/fullCompaction/fullCompactionService.ts"]:
text = Path(root).read_text()
print(f"\n--- {root}")
m = re.search(r"replacementMessages:\s*(?:remote\.messages|[^\n]+)\s*,", text)
print("replacement assignment contains remote.messages:", "remote.messages" in (m.group(0) or ""))
m2 = re.search(r"newHistory\s*=\s*.*\.history|this\.context\.get\(\)", text)
print("uses live history snapshot:", bool(m2))
PYRepository: YaseenHQ/kimi
Length of output: 34404
Remote compaction silently drops user messages appended mid-flight.
Both v1 and v2 remote-compaction branches pass only remote.messages to applyCompaction after confirming the live tail changed. Since replacementMessages replaces the compacted window, real user input added while the provider call is in flight is neither summarized nor preserved. Include the grown tail in the replacement array before applying compaction.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/agent-core/src/agent/compaction/full.ts` around lines 454 - 479, The
remote compaction path in the visible branch of full compaction must preserve
real user messages appended during the provider call. In the `remote` handling
around `applyCompaction`, append the live history tail after
`originalHistory.length` to `remote.messages` when constructing
`replacementMessages`; apply the same change to the corresponding v2
remote-compaction branch, while retaining the existing validation and
cancellation behavior.
The compact() request omitted include: ['reasoning.encrypted_content'], so compaction preserved message continuity but dropped the model's accumulated reasoning chain. Codex sets this unconditionally on every Responses request (client.rs:888); match that behavior here.
# Conflicts: # packages/agent-core-v2/src/kosong/provider/bases/openai/openai-legacy.ts # packages/kosong/src/providers/kimi.ts
Related Issue
None. This PR adds native OpenAI Responses compaction while retaining Kimi existing local compaction as the portable fallback.
Problem
Kimi previously owned compaction entirely on the client. Responses-compatible providers can instead return an opaque
compactionitem from/responses/compact, which must be persisted and replayed on later requests. Dropping it, rendering it as normal content, or forwarding it to another model or endpoint is incorrect.What changed
/responses/compactfor Responses providers during normal full compaction.The branch is synced with current
mainafter the upstream 0.29.1 integration. Conflict resolution combines both required behaviors: endpoint-specific reasoning-key selection and opaque-message filtering.Validation
mainfull suites: Kimi CLI 2,411 passed; agent-core-v2 4,084 passed; kap-server 797 passed; pi-tui 745 passedgit diff --checkChecklist
gen-changesetsskill; the feature changeset remains in the branch.Summary by CodeRabbit