Repository navigation
Integration: compaction fixes round 2 (development -> main) - #134
Conversation
Compaction treated a truncated stub as the whole message. When earlier lines had already filled the 64 KiB summary source, a later message that cannot fit was cut to the leftover sliver and then dropped. Stub that line only on an empty source so this pass summarizes the prefix and the next pass keeps about 64 KiB of the oversized line. Consolidated from cursor/critical-bug-identification-d77c onto the post-promote development base: the only-empty rule replaces the half-source stub threshold from 0d4e380 (same if/branch), and the later-line regression now drives the oversized turn through several turns with a 50 KiB predecessor to assert the defer-then-full-stub sequence instead of a one-shot sliver stub. Supersedes #132.
… line The only-empty stub rule lets a pass defer a never-fit line to the next one, which works for a user line: the pass folds its summary into the kept line, the line lands at the head of the session and the next pass stubs it against a full 64 KiB source. An assistant line cannot take that path. The kept tail starts on it, a user summary gets prepended in front of it, and the next pass sees the same pair again - the wrapper always counts toward used, so no pass ever reaches the only-empty stub. The count then only grows until SESSION_JSON_MAX stops the saves and the stored session stays above max_context_messages forever. The new regression test reproduces the stall on the exact post-partial-pass state: without the fix the count stayed at 6..22 across eight turns. Fix shape: a partial pass that consumed exactly one line and breaks on a never-fit non-user line now folds its summary into that line instead of prepending a wrapper. The line keeps its content and role, the count shrinks, and the next pass starts on the line with an empty source and stubs it whole. Chosen over forcing a truncated stub at the stuck pass: that restores progress but re-creates the sliver this branch exists to prevent whenever the one consumed line was large; the fold always reaches the full stub. The wrapper variant (not counting the summary toward used) was rejected: the wrapper cannot be told from real content structurally and dropping it from the source would lose the accumulated summary. The full-buffer next-pass test now expects the count to shrink on the first partial pass (5 + 2 instead of 6 + 2): the fold consumes the head line together with the oversized line, where the old half-source threshold left the count unchanged for one pass. The full-buffer stub assertion itself is unchanged and still holds.
A line that fits a fresh source but not the wrapped remainder behind the previous head used to miss the fold (it is not "never-fit"), take the old prepend path and freeze the session at the same count: every pass rebuilt [summary, bigA] and the count only grew until SESSION_JSON_MAX. The fold now fires for any non-user line that stops the pass at index 1, never-fit or not; regression pins the 65500-byte band (stalled red at count 14 after 4 turns, converges by turn 3 green).
The assistant regression now requires convergence within three turns instead of accepting any of eight, and a tool-role oversized line behind two consumed lines pins the normal prepend path: the first pass must shrink the count and the session must still converge like the assistant cases.
…avior The stub-only-empty, defer-whole and fold behaviors are one mechanism; the release note now reads as the resulting guarantee instead of three incremental fixes.
…-stub fix(agent): stub oversized history only on an empty summary
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
This PR promotes #133 (development to main): session compaction now stubs an oversized line only against an empty summary source, defers it otherwise, and folds the summary into the non-user line that stops a pass at index 1 so the message count keeps shrinking. It is 3 files (src/core/agent.c, tests/test_agent.c, CHANGELOG.md). test_agent passes here (I stubbed libcurl because the dev headers are not installed). The 65500-byte livelock I reported on #133 is fixed by 7cf7beb.
I also probed 316 combinations (line sizes 65400 to 200000, roles assistant/tool/user, indices 0 to 3, two consecutive oversized lines, 10 turns each). Compaction converged in 4 turns or fewer every time. The only non-converging cases were sessions already over SESSION_JSON_MAX (128 KB), which the existing too-large guard freezes. That is outside this change.
Must Fix
None.
Should Fix
-
The window sent to the provider can start with an assistant or tool message, and the summary can be dropped on that turn. Compaction can now return
max_ctx + 1entries (summary or folded line, plus the oversized line, plus the tail).parse_session_into_messages(src/core/agent.c:452-466) keeps only the lastmax_ctxentries, so the summary user turn is sliced off. Repro ondevelopment:max_context_messages = 4, seed[user m0, assistant m1, assistant <65520 x 'A'>, assistant m3, user m4, assistant m5], oneagent_run. The provider receives[system, assistant, assistant, user, assistant, user]. With a tool line at the same spot it receivestoolfirst. In that probe the stored count went 7, 8, 9, then 6 on turn 4. The fold path can do the same: 10 messages, max 5, an assistant line of 65500 at index 1 sentA U A U A Uon turn 1. Anthropic needs a leading user turn (append_anthropic_messagesdoes not guard it; I raised this on #133). Because a failed provider call leaves the stored session unchanged, the next turn replays the same compaction and likely the same window. I could not test that against the real API. Suggested fix: inagent_build_messages, drop leading non-user entries after slicing, or make compaction produce at mostmax_ctxentries. -
fold_into_oversizedis a misleading name (src/core/agent.c:250,284,fold_summary_into_messagedoc at168-170). It is set for any non-user line that stops the pass at index 1, including one that fits a fresh source and is not oversized (the 65500 case). Something likefold_into_stop_linematches the behavior and is easier to grep. -
compact_session_via_llmis now about 130 lines with three nested flags (used,next_unsummarized,fold_into_oversized). The summary-source loop (251-289) can be pulled into a helper that returns used bytes, the stop index and the fold flag. That would also make the stub/defer/fold rules unit-testable without five 90 to 130 line end-to-end tests. -
Test gaps (
tests/test_agent.c):- Nothing asserts the roles sent to the provider after an assistant fold, so finding 1 is untested.
test_compaction_merges_summary_when_tail_starts_with_useronly guards consecutive users. test_compaction_progresses_when_tool_message_is_oversized_mid_history(about1137) uses index 2, so it never exercises the fold. There is no oversized tool or assistant line at index 1 directly behind a plain user line.- No test has two consecutive oversized lines, or a multibyte UTF-8 cut on the deferred (second-line) path.
test_compaction_folds_assistant_line_that_fits_a_fresh_source(about1236) andtest_compaction_progresses_when_assistant_message_is_oversized(about1349) are the same skeleton with a differentOVERSIZEand turn bound. A shared scenario helper would remove 200+ duplicated lines.- The tool and assistant stall tests mostly assert "marker gone and count <= 6", which does not distinguish a summarized line from a stubbed one.
- Nothing asserts the roles sent to the provider after an assistant fold, so finding 1 is untested.
Nice to Have
- The new block comments in
compact_session_via_llm(263-283) and at320-324are long and partly restate the code. Keep the WHY (a sliver stub counts as the whole message, so the unread tail is deleted; the wrapper always eatsused, so the only-empty stub is never reached) and cut the rest. The hyphen-as-dash inpair again - the wrappershould be rephrased. - Test sizes (
50*1024,34*1024,65500,66*1024,60*1024) and counts (4+2,5+2) are all tied toSUMMARY_SOURCE_MAX. Derive them from it, or add one comment explaining the+2(the new user and assistant turn). - The new tests reset
compaction_call_countbut notcompaction_last_sourceorcompaction_source_truncated. Harmless today, brittle if a test starts reading them. - The
CHANGELOG.mdbullet at line 25 is one sentence of about 70 words with three behaviors. Splitting it into stub, defer and fold would read better.
Positive Highlights
- Deferring the stub until the next pass (full 64 KiB source) instead of cutting an oversized line to the leftover sliver is the right call. The turn-0 assertions that the later marker survives and the earlier one is summarized pin that behavior well.
- The
7cf7bebfix widened the fold condition from "never fits" to "any non-user line at index 1", which closes the band between the wrapped remainder and a fresh source.test_compaction_gives_oversized_line_full_buffer_on_next_passnow asserts the exact count (7) after pass 1, which directly checks the fold. - The UTF-8 back-off in
summary_source_append_truncatedkeeps the stub valid for providers, and the integration PR carries only the already-reviewed #133 content, so the scope is clean.
Sent by Cursor Automation: Code review
Summary
Promotes the last consolidated fix round: PR #133 (compaction only-empty stub policy + assistant/tool convergence folds + review follow-ups), consolidating and superseding Bugbot PR #132.
Testing
CI=true make test— all 46 test binaries passed;make static— passed;make test_agent— 26 tests, 0 failed.