feat: added memory compaction to agent - #6
Conversation
|
Warning Review limit reached
Next review available in: 44 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds token-bounded conversation memory for the agent runtime. Older turns are summarized through a DBOS step, model context is rebuilt from the running summary and recent turns, and tool outputs are stored within per-turn state. ChangesConversation Memory Runtime
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant AgentWorkflow
participant MemoryHelpers
participant DBOS
participant OpenAI
participant ModelTurn
AgentWorkflow->>MemoryHelpers: estimate_tokens(context)
AgentWorkflow->>DBOS: summarize_step(old_turns, summary)
DBOS->>OpenAI: summarize transcript
OpenAI-->>DBOS: updated summary
DBOS-->>AgentWorkflow: summary
AgentWorkflow->>MemoryHelpers: build_context(task, summary, turns)
AgentWorkflow->>ModelTurn: stream rebuilt context
ModelTurn->>OpenAI: Responses API stream
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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 `@harness/memory.py`:
- Around line 56-58: Update the compaction summary prompt near the memory
compaction logic so it does not retain item IDs, draft IDs, amounts, or other
sensitive business details in summaries emitted through memory.compacted.
Replace those details with redacted or aggregate metrics, and ensure any
necessary detailed summary is protected by explicit access controls and
retention limits before EventLog persistence.
- Line 49: Update the transcript construction in the memory handling flow to
avoid prefix-truncating the flattened old-turn text with [:6000]. Preserve
complete messages while enforcing the token limit, prioritizing the newest
turns/messages so the content being folded in is retained; use chunked
summarization if needed, then pass the bounded result onward.
- Around line 40-41: Change the summary injection in the context-building flow
so generated memory is not appended as a system message. Store summary as
clearly delimited, untrusted conversational memory (using the existing
user/assistant-style representation where available), while preserving its
compacted content and keeping dangerous tool authorization enforced outside the
model.
- Around line 50-71: Update the response creation and return flow in the memory
compaction function to enforce MAX_CONTEXT_TOKENS: set an explicit output token
limit, count tokens in resp.output_text, and reject empty or oversized
summaries. When validation fails, retry compaction or safely return
prior_summary, ensuring the stored summary never exceeds the context bound.
- Around line 11-12: Update agent_workflow() to estimate the complete assembled
context via build_context(user_input, summary, turns), including SYSTEM_PROMPT,
task, and optional summary, before applying
MAX_CONTEXT_TOKENS/KEEP_CONTEXT_TOKENS compaction. Reserve the required budget
for pinned content so long tasks or summaries cannot cause the final context to
exceed MAX_CONTEXT_TOKENS.
- Around line 29-31: Update estimate_tokens to encode user/tool text with
special-token handling disabled, treating registered special-token strings as
ordinary text while preserving the existing joined-message token count behavior.
In `@harness/runtime.py`:
- Around line 124-132: Update the context-compaction loop around summarize_step
so removed turns are processed in chunks no larger than the summarizer’s
6000-character input limit. Fold each chunk into the running summary
sequentially, ensuring every turn popped from turns is summarized and no suffix
is discarded.
- Around line 124-145: The compaction logic around the context-building flow
must measure the hydrated context, not turns alone. Replace both token checks in
the outer condition and compaction loop with estimates of
build_context(user_input, summary, turns), and handle pinned content exceeding
MAX_CONTEXT_TOKENS without attempting to remove turns indefinitely; preserve the
existing summary update and memory.compacted emission behavior.
- Around line 132-145: Update the summarization and context construction around
summarize_step, build_context, and the historical turns retained near the
memory.compacted emission so summarized tool output is never inserted as a
system message. Place historical summaries in an untrusted lower-priority role
and explicitly frame all historical/tool content as data rather than
instructions, while preserving the existing compaction behavior.
🪄 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: 752917c1-bfd3-444a-abe0-5c1a5d6b0d80
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
harness/memory.pyharness/runtime.pypyproject.toml
| if summary: | ||
| context.append({"role": "system", "content": f"Summary of earlier work so far:\n{summary}"}) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | sed -n '1,200p'
echo "== memory.py summary context =="
fd -a 'memory.py' . | while read -r f; do
echo "--- $f"
wc -l "$f"
sed -n '1,180p' "$f" | cat -n
done
echo "== runtime.py agent instruction / summary search =="
fd -a 'runtime.py' . | while read -r f; do
echo "--- $f"
wc -l "$f"
rg -n "tool|instruction|summary|memory|turns|context|openai|anthropic|messages" "$f" || true
done
echo "== git diff stat/name =="
git diff --stat HEAD~1..HEAD 2>/dev/null || git diff --stat 2>/dev/null || true
git diff --name-only HEAD~1..HEAD 2>/dev/null || git diff --name-only 2>/dev/null || trueRepository: inesaranab/agent-harness
Length of output: 7198
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== runtime.py outline and relevant sections =="
wc -l harness/runtime.py
sed -n '1,220p' harness/runtime.py | cat -n
echo "== tools.py =="
wc -l harness/tools.py
sed -n '1,260p' harness/tools.py | cat -n
echo "== inspect prompt-injection model for generated summary/system authority =="
python3 - <<'PY'
from pathlib import Path
import ast, re
p=Path('harness/runtime.py')
src=p.read_text()
m=re.search(r'if estimate_tokens.*?summary\s*=\s*await\s*summarize_step\([^)]*\)(?P<body>.*?)(?=\n\n|\n # )', src, flags=re.S|re.M)
print("SUMMARIZE_SEARCH", bool(m))
if m:
print(m.group(0)[:1200])
print("HAS_SUMMARY_ASSIGNMENT", bool(re.search(r'summary\s*=\s*await\s*summarize_step', m.group(0))))
print("POST_SUMMARY_CONTEXT_CALL", bool(re.search(r'context\s*=\s*build_context', src.split(m.group(0)[-100:][0][-50:] if False else '',1)[1][:800])))
PYRepository: inesaranab/agent-harness
Length of output: 14439
Do not promote generated memory to a system message.
turns includes the model+tool-call pair plus function_call_output, so injected tool output can be summarized into summary and then reinserted as a new system message with full authority on the next turn. Keep compacted memory separate from trusted instructions: store it as delimited/untrusted data (for example a user/assistant-style memory message or clearly untrusted assistant/system block) and enforce dangerous tool permissions outside the model.
🤖 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 `@harness/memory.py` around lines 40 - 41, Change the summary injection in the
context-building flow so generated memory is not appended as a system message.
Store summary as clearly delimited, untrusted conversational memory (using the
existing user/assistant-style representation where available), while preserving
its compacted content and keeping dangerous tool authorization enforced outside
the model.
Source: MCP tools
|
|
||
| # ── STATE: compress old turns into the running summary (an LLM call) ─ | ||
| async def summarize(old_turns: list, prior_summary: str) -> str: | ||
| transcript = "\n".join(_as_text(m) for turn in old_turns for m in turn)[:6000] |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Do not prefix-truncate the transcript.
old_turns is ordered oldest-to-newest, so [:6000] drops the newest work—the exact content the prompt says to fold in—before those turns are discarded. Build a token-bounded list of complete messages or summarize in chunks instead of slicing the flattened string.
🤖 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 `@harness/memory.py` at line 49, Update the transcript construction in the
memory handling flow to avoid prefix-truncating the flattened old-turn text with
[:6000]. Preserve complete messages while enforcing the token limit,
prioritizing the newest turns/messages so the content being folded in is
retained; use chunked summarization if needed, then pass the bounded result
onward.
| resp = await client.responses.create( | ||
| model="gpt-5.6-luna", | ||
| input=[ | ||
| { | ||
| "role": "system", | ||
| "content": ( | ||
| "You compress an agent's work log into a short running summary. " | ||
| "Preserve concrete facts: item ids, categories, draft ids, amounts, " | ||
| "and what was already sent. Be terse." | ||
| ), | ||
| }, | ||
| { | ||
| "role": "user", | ||
| "content": ( | ||
| f"Prior summary:\n{prior_summary or '(none)'}\n\n" | ||
| f"Fold in this newer work:\n{transcript}\n\n" | ||
| "Return the updated summary." | ||
| ), | ||
| }, | ||
| ], | ||
| ) | ||
| return resp.output_text |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)harness/memory\.py$|^harness/|memory\.py' || true
echo "== outline harness/memory.py =="
if [ -f harness/memory.py ]; then
ast-grep outline harness/memory.py --view compact || true
echo "== relevant lines =="
nl -ba harness/memory.py | sed -n '1,130p'
fi
echo "== search MAX_CONTEXT_TOKENS and summary usage =="
rg -n "MAX_CONTEXT_TOKENS|summary|comp|compact|running summary|output_text|max_tokens|output_tokens|limit" -S . --glob '!**/.git/**' | head -200Repository: inesaranab/agent-harness
Length of output: 587
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== OpenAI Responses API output_limits docs via web source =="
python3 - <<'PY'
import urllib.request
url="https://platform.openai.com/docs/api-reference/responses/create"
try:
req=urllib.request.Request(url, headers={"User-Agent":"CodeRabbit"})
with urllib.request.urlopen(req, timeout=20) as r:
data=r.read().decode("utf-8", "replace")
for term in ["max_output_tokens", "output_tokens", "response_format", "input_tokens"]:
print(f"\n--- occurrences of {term} ---")
hits=[i for i, line in enumerate(data.splitlines(),1) if term in line]
for i in hits[:15]:
print(f"{i}: {data.splitlines()[i-1]}")
except Exception as e:
print(f"ERROR: {e}")
PYRepository: inesaranab/agent-harness
Length of output: 364
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant memory.py lines =="
awk '{printf "%6d\t%s\n", NR, $0}' harness/memory.py | sed -n '1,120p'
echo "== relevant runtime/context lines =="
awk '{printf "%6d\t%s\n", NR, $0}' harness/runtime.py | sed -n '1,180p'
awk '{printf "%6d\t%s\n", NR, $0}' harness/memory.py | rg "MAX_CONTEXT_TOKENS|summary|prior_summary|transcript|len|token|output_text" -n || trueRepository: inesaranab/agent-harness
Length of output: 11452
🌐 Web query:
OpenAI Responses API create max_output_tokens output_tokens_limit parameter
💡 Result:
The parameter max_output_tokens is the designated setting used in the OpenAI Responses API to control the maximum number of tokens generated in a single response [1][2]. Key details regarding this parameter include: Functionality: It sets a hard upper bound on the number of tokens the model can produce [1][2]. This limit encompasses both visible output tokens and any internal reasoning tokens generated by the model [1][3]. If the model reaches this limit, generation will stop, and the response will typically indicate a finish_reason of length [2]. Usage Context: * Responses API: You should use max_output_tokens for the newer Responses API [2][4]. * Chat Completions API: For the standard Chat Completions API, the equivalent parameter is max_completion_tokens [5][6]. While legacy models and some configurations may still support the older max_tokens parameter, max_completion_tokens is the modern, preferred standard for newer models, including o-series reasoning models [5][7][6]. Best Practices: * Accuracy: Because non-visible tokens (such as reasoning or structural tokens) count toward this limit, it is recommended to leave sufficient headroom when you require a specific amount of visible output [3]. * Consistency: Always verify which API endpoint (Responses vs. Chat Completions) you are utilizing, as they use different parameters to manage output limits [2][4]. There is no parameter named output_tokens_limit in the OpenAI API; max_output_tokens is the correct technical parameter name [1][8].
Citations:
- 1: https://github.com/openai/openai-python/blob/main/src/openai/types/responses/response_create_params.py
- 2: https://theneuralbase.com/openai-responses-api/learn/intermediate/max-output-tokens-limit/
- 3: https://developers.openai.com/api/docs/guides/token-counting
- 4: https://help.openai.com/en/articles/5072518-controlling-the-length-of-completions
- 5: https://developers.openai.com/api/reference/resources/chat/subresources/completions/methods/create/
- 6: Limit number of output tokens with AsyncOpenAI().chat.completions.create()? openai/openai-python#2471
- 7: https://github.com/openai/openai-python/blob/main/src/openai/resources/chat/completions/completions.py
- 8: https://modelparams.dev/parameters/max_output_tokens
Enforce a hard bound on the running summary.
“Be terse” is not a size guarantee. A verbose compaction response can make summary push build_context(...) over MAX_CONTEXT_TOKENS; runtime only compact-turns, so oversized compacted state cannot be recovered later. Set an explicit output limit, validate the returned token count, and fall back to prior_summary or retry when output is empty or oversized.
🤖 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 `@harness/memory.py` around lines 50 - 71, Update the response creation and
return flow in the memory compaction function to enforce MAX_CONTEXT_TOKENS: set
an explicit output token limit, count tokens in resp.output_text, and reject
empty or oversized summaries. When validation fails, retry compaction or safely
return prior_summary, ensuring the stored summary never exceeds the context
bound.
Source: MCP tools
| "You compress an agent's work log into a short running summary. " | ||
| "Preserve concrete facts: item ids, categories, draft ids, amounts, " | ||
| "and what was already sent. Be terse." |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Avoid persisting sensitive details in compaction events.
The summary is instructed to retain item IDs, draft IDs, amounts, and sent status. Runtime emits it in memory.compacted, and the event bus persists event payloads to EventLog, creating a durable copy of potentially sensitive business data. Emit redacted metrics or protect detailed summaries with explicit access controls and retention limits. (raw.githubusercontent.com)
🤖 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 `@harness/memory.py` around lines 56 - 58, Update the compaction summary prompt
near the memory compaction logic so it does not retain item IDs, draft IDs,
amounts, or other sensitive business details in summaries emitted through
memory.compacted. Replace those details with redacted or aggregate metrics, and
ensure any necessary detailed summary is protected by explicit access controls
and retention limits before EventLog persistence.
Source: MCP tools
| if estimate_tokens([m for turn in turns for m in turn]) > MAX_CONTEXT_TOKENS: | ||
| old: list = [] | ||
| while ( | ||
| len(turns) > 1 | ||
| and estimate_tokens([m for turn in turns for m in turn]) > KEEP_CONTEXT_TOKENS | ||
| ): | ||
| old.append(turns.pop(0)) | ||
| if old: | ||
| summary = await summarize_step(old, summary) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Do not discard turns beyond the summarizer’s 6000-character input.
This loop removes every selected turn before summarizing it, but harness/memory.py:49 truncates the combined transcript to 6000 characters. When old exceeds that size, its suffix is neither summarized nor retained, permanently losing prior tool results and actions. Chunk compaction so every removed turn is folded into the running summary.
🤖 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 `@harness/runtime.py` around lines 124 - 132, Update the context-compaction
loop around summarize_step so removed turns are processed in chunks no larger
than the summarizer’s 6000-character input limit. Fold each chunk into the
running summary sequentially, ensuring every turn popped from turns is
summarized and no suffix is discarded.
| summary = await summarize_step(old, summary) | ||
| context_tokens = estimate_tokens(build_context(user_input, summary, turns)) | ||
| await emit_step( | ||
| { | ||
| "type": "memory.compacted", | ||
| "workflowId": workflow_id, | ||
| "summarizedTurns": len(old), | ||
| "contextTokens": context_tokens, | ||
| "summary": summary, | ||
| } | ||
| ) | ||
|
|
||
| # 2 + 3. Hydrate the context and run one turn over it. | ||
| context = build_context(user_input, summary, turns) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Do not promote summarized tool output to system authority.
Lines 167-172 retain arbitrary tool output; later, Line 132 passes it to the summarizer, and build_context inserts the resulting text as a system message. An attacker controlling data returned by a permitted tool can persist instruction-like content and have it influence later turns with system priority, enabling model-mediated misuse of allowed tools. Keep historical summaries in an untrusted lower-priority message and explicitly frame historical/tool data as data, not instructions.
Also applies to: 167-175
🤖 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 `@harness/runtime.py` around lines 132 - 145, Update the summarization and
context construction around summarize_step, build_context, and the historical
turns retained near the memory.compacted emission so summarized tool output is
never inserted as a system message. Place historical summaries in an untrusted
lower-priority role and explicitly frame all historical/tool content as data
rather than instructions, while preserving the existing compaction behavior.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes