fix(chat): compact at a turn boundary, preserve user turns, and carry extracted state (#14066)

[mrveiss]

Thinking Path

Compaction replaced the oldest half of a conversation and depended entirely on one LLM call
to carry it forward. Three things followed from split_point = len(messages) // 2:

1. The cut was an index, not a boundary. It could land between an assistant message
   carrying tool_calls and its role="tool" responses. _sanitize_tool_messages exists and
   repairs exactly this — but it was applied only to the summariser's input, never to the
   half that continues on to the provider.
2. Every user message in the older half became prose. The summarisation prompt asks for
   constraints under "Key Context", which is a request, not a guarantee — and a long session
   re-summarises the same region repeatedly, each pass another chance to drop the thing the
   user said once.
3. Nothing deterministic crossed. Facts that are exact and free to compute — files
   written, commands run with exit status — were re-derived by a model from prose, or lost.

Why this approach: keep the LLM summary and add a deterministic layer beside it, rather
than replacing summarisation. The state block's value is precisely that it still works when
the summary is bad; that is a design property, not an optimisation. Preserving user turns is
made safe by bounding the set, not by choosing which ones matter.

Alternative rejected: repairing the kept half with _sanitize_tool_messages instead of
moving the boundary. That drops the orphaned tool responses — losing real content — where
snapping the cut keeps the whole batch together on one side.

What Changed

• autobot-backend/chat_history/context_overflow.py
  • _pick_boundary — snaps len(messages) // 2 back to a turn start, preferring a user turn
    within a bounded window. A role="tool" message is never a valid cut point.
  • _preserved_user_messages — the most recent N user messages, verbatim, never summarised.
  • _extract_state / _render_state_block — files written, shell commands, tools used, read
    off the tool calls. Tool-name matching reuses autobot_shared.tool_catalogue
    (FILE_WRITE_TOOLS, SHELL_EXEC_TOOLS, match_tool_name), not a local literal list.
  • _clip_tool_results — clips oversized tool results in the summarised region. Returns a
    new list; the caller's history and the persisted transcript are untouched.
  • _compose_summary — joins the three.
  • _message_text + new _text_parts — defect fixed: it read only msg["text"], so every
    API-schema message (role/content) estimated as 0 tokens and the post-compaction
    refill under-counted the retained half. That delays the next compaction rather than
    triggering one early, so it produced no error and no alert.
• autobot_shared/env_registry.py — registers the four new AUTOBOT_COMPACTION_* vars.
• changelog/unreleased/14066-compaction-boundary.md — new fragment.
• autobot-backend/chat_history/context_overflow_boundary_test.py — new, 30 tests.
• autobot-backend/chat_history/context_overflow_test.py — one assertion pair updated (see Risks).

Second defect fixed, found by a test failing at baseline: the user-turn preference could
drag the boundary to index 0. _create_summary then summarised no messages and returned
early — before the tracker reset — so the session stayed over threshold and re-attempted
compaction every turn while reporting compaction as enabled. Both searches now floor at index 1.

Verification

./venv/bin/python -m pytest autobot-backend/chat_history/ -q
# 162 passed in 14.11s

./venv/bin/python pipeline-scripts/check_env_var_registry.py   # exit 0

Mutation-tested — every guard was verified to fail when the line it guards is reverted.
This mattered: the first version of these tests called _pick_boundary directly, and all 26
passed with _create_summary reverted to len(messages) // 2. Three wiring tests were added
to close that.

Mutation	Result
boundary snap → raw len // 2	3 failed
composition → bare return summary	3 failed
tool-result clipping removed	1 failed
user-message preservation → []	2 failed
state block → ""	1 failed
boundary floor max(1, …) → max(0, …)	1 failed

Baseline restored and re-verified green after each.

The token-accounting acceptance criterion is asserted as an invariant — the composed
replacement is smaller than the region it replaces — rather than quoted as a number here.

Risks

• The compacted view is larger than before, carrying state and user turns alongside the
  summary. Bounded by AUTOBOT_COMPACTION_USER_MESSAGE_CAP (40) and
  AUTOBOT_COMPACTION_STATE_COMMAND_CAP (10); the size invariant is asserted in a test.
• Token estimates for API-schema sessions change from 0 to correct, so compaction triggers
  earlier for those sessions than it has been. This is the fix, not a regression, but it is
  the change most likely to be visible in production.
• One existing test was modified. test_a_malformed_retained_message_does_not_clear_the_counter
  pinned summary == "Summary." and await_count == 4; the return value is now composed and
  the boundary snaps from index 4 to 3. Its intent — exactly one reset, every retained message
  re-counted despite malformed entries — is unchanged and still asserted.
• summary_text remains a single string injected as a system message; no consumer contract
  changed (chat_history/overflow_integration.py).
• Rollback: revert the commit. No migration, no persisted-format change — the stored transcript
  was never touched.

Model Used

Claude Opus 5 (claude-opus-5[1m])

Issue Link

Closes #14066

Related: #14065 (the failure-path fix this builds on — its guarantee is re-asserted here so it
cannot regress), #14029 (context sizing), #13685 (tiered context stack), #13593.

Changelog fragment

• Added changelog/unreleased/14066-compaction-boundary.md

Checklist

• Code follows AutoBot patterns from CLAUDE.md
• Tests added or updated (or N/A with reason)
• Documentation updated if behavior changed
• Pre-commit hooks pass (git commit runs them automatically)
• PR targets Dev_new_gui (not main)
• No secrets or credentials in the diff

---

Review finding fixed after CI first went green (747e9c730)

Self-review before merge caught a real bug in this PR. _clip_tool_results chose its write
key with isinstance(msg.get("content"), str), but _format_messages reads
msg.get("content") or msg.get("text", ""). For a tool result whose content is a
multimodal list, the clipped body was written to text while the untouched list stayed in
content — so the full body still reached the summarizer while the clip looked applied.

Every clipping test passed, because they all used string content. That is the
empty-result-reads-as-clean shape this PR is otherwise about.

Fixed by _body_key, which mirrors _message_text's own precedence so a rewrite lands in
the key that is actually read. Four regression tests added, asserting through
_format_messages (what the summarizer sees) rather than through the returned dict.
Mutation-verified: reverting to the old key selection fails
test_a_list_content_tool_result_is_actually_clipped.

186 passed    # chat_history/ + check_env_var_registry_helpers_test.py, after merging base

[github-actions]

✅ SSOT Configuration Compliance: Passing

🎉 No hardcoded values detected that have SSOT config equivalents!

[mrveiss]

Validate PR template sections is red on this PR, and it is the only failing check — everything
else is green.

The gate requires four headings by exact name (.github/workflows/pr-template-check.yml, which
awks for ^## <name> and fails on an empty section):

• ## Thinking Path
• ## What Changed
• ## Verification
• ## Model Used

This body has ## Summary, ## Changes, ## Testing, ## Risk. Three map across directly
(Changes → What Changed, Testing → Verification, plus a Model Used line); Thinking Path has no counterpart here, and I am not going to invent one for someone else's work.

Not editing the body since this is another session's PR and a concurrent edit would collide —
flagging it so whoever picks it up can clear it in one pass. Noted from a sweep, not a review;
this PR has no code-reviewer pass yet either.

[mrveiss]

python-suite shard 12/12 is red on the same new check, with three variables this PR introduces:

neither registered nor baselined:
  ['AUTOBOT_COMPACTION_BOUNDARY_WINDOW', 'AUTOBOT_COMPACTION_STATE_COMMAND_CAP',
   'AUTOBOT_COMPACTION_TOOL_RESULT_CLIP_C…']

#14266 taught check_env_var_registry.py to see the env_utils helper form, not just os.getenv, and #14269 drained the backlog to empty — so new variables need registering in autobot_shared/env_registry.py, then python3 pipeline-scripts/generate_env_docs.py to regenerate the docs table.

Not a stale-branch artefact: these are yours, and the defaults matter for a compaction path where a wrong bound silently changes what the model sees.

Validate PR template sections is separate — the body needs ## Thinking Path, ## What Changed, ## Verification and ## Model Used as literal headings. Noted from a sweep, not a full review.

[mrveiss]

Code review — clean to merge

No user-turn loss, no reordering, no active crash path. I traced the boundary cases specifically
asked about and they hold: the only route to boundary 0 is the fully-degenerate history (every
index 1..target is role="tool"), and that fails safe — messages_to_summarize becomes [],
_create_summary returns "" at its early guard, and nothing is touched. Compaction is skipped
that turn rather than corrupting anything. The retained half is an untouched suffix slice, so order
and roles are never rearranged. _clip_tool_results builds a new list rather than mutating, and the
extraction helpers see the unclipped messages, so the persisted transcript is never truncated.

Two findings worth recording. Neither is triggerable today, which is exactly why they are easy to
lose.

1. context_overflow.py:795 vs :802 — composition runs after the tracker reset.

The same function deliberately computes retained_tokens before reset_session, with a comment
citing #14065: "the tracker reset below must not run when the history it claims to have compressed
is still uncompressed." _compose_summary is new code that did not get that treatment — it runs
after.

Latent only because every helper is isinstance-guarded and falls back safely on bad shapes. But if
a future edit to _extract_state, _render_state_block or _preserved_user_messages introduces a
raise, the tracker is already reset — the session reads under threshold — while _create_summary
never returns and no summary is injected. That is #14065's exact "counter reset without a delivered
summary" shape, one call deeper, and the caller's except SummarizationFailed will not catch it.
Moving the composition above the reset restores the invariant the function already states.

2. context_overflow_boundary_test.py:555 — the idempotency test uses a shape production never
writes.

test_two_successive_compactions_do_not_erode_the_instruction feeds round 1's output back as
{"role": "user", "content": first}. overflow_integration.py:160-171 persists it as
"sender": "system", messageType: "context_summary". _role_of reads role or sender, so the
real artifact's role is "system" — and _preserved_user_messages only rescues role == "user".

So the test passes for the wrong reason: faking the role as user makes the whole blob re-protected
by a path that will not fire in production. Once a round-1 summary lands inside a later round's
window, its verbatim block is just prose to round 2's LLM call, with no deterministic
re-extraction — _extract_state reads live tool_calls, not a prior summary's markdown. That is
the "prose, not guarantee" defect this PR's own Thinking Path names as motivation, deferred to
round 2 rather than removed.

Re-running that test with sender: "system" would say whether the multi-round claim holds. If it
does not, scoping the guarantee to one round in the docstring is a legitimate answer — the current
wording just claims more than the test shows.

3. Minor, pre-merge: the 4 new AUTOBOT_COMPACTION_* vars are registered but
pipeline-scripts/generate_env_docs.py has not been re-run — grep AUTOBOT_COMPACTION docs/developer/CLAUDE_RULES.md returns nothing. Not CI-enforced, so nothing is red.

4. Informational: verbatim preservation is capped at AUTOBOT_COMPACTION_USER_MESSAGE_CAP
(default 40), so "preserve user turns" is not unconditional for a discarded prefix with more than
40 of them. Documented and intentional — noting it only so the guarantee is not over-read.

Test quality is above average otherwise: test_create_summary_cuts_at_the_snapped_boundary_not_the_raw_index
and test_a_leading_user_turn_does_not_drag_the_boundary_to_zero drive the real _create_summary
path rather than the helpers, and the mutation table in the body is genuine evidence — six
mutations, each shown to fail a named test. Finding 2 is the one gap in it.

Filing 1 and 2 as a follow-up rather than holding the merge.

[mrveiss]

Re-checked at d5dc06e9c — that head is a bot merge of Dev_new_gui, so nothing substantive changed since my review. CI is fully green and the verdict stands: clean to merge.

One thing still outstanding, and it is a single command:

grep -c AUTOBOT_COMPACTION docs/developer/CLAUDE_RULES.md   ->  0

The four AUTOBOT_COMPACTION_* vars are registered in env_registry.py but pipeline-scripts/generate_env_docs.py has not been re-run, so the docs table does not list them. No CI check enforces it, so nothing is red — which is why it is easy to merge past.

I have not run it myself: regenerating means executing a script from the checkout, and this session is under a standing instruction never to do that (the checkout is not the deployment and would produce different results). So it needs whoever owns this branch.

The two findings from my review are filed as #14322 and are deliberately not blocking:

1. context_overflow.py still composes the summary after reset_session (line 795 vs 802), reversing the invariant the same function states two statements earlier for bug(chat): summarization failure returns a success-shaped placeholder — half the conversation is destroyed and reported as compacted #14065. Latent — every helper is isinstance-guarded — but the next edit to one of them reintroduces bug(chat): summarization failure returns a success-shaped placeholder — half the conversation is destroyed and reported as compacted #14065 one call deeper, where except SummarizationFailed will not catch it.
2. test_two_successive_compactions_do_not_erode_the_instruction feeds round 1's output back as role: user, while production persists sender: "system". _preserved_user_messages rescues only role == "user", so the test passes for the wrong reason and the multi-round guarantee is untested.

[mrveiss]

Blocked on #14111, not on this diff — root cause and evidence

python-suite shard 2/12 fails on code_intelligence/co_change_test.py::test_dividing_by_the_smaller_count_would_have_reported_it:

GitCommandError: git ('log','--no-merges','--pretty=format:%x00%H','--name-only') exited 128
error: Could not read a9bac6ea794a87a14c93b7dd1215a5318ad9167f

This is the third recorded instance of #14111 (same file, same shard as the #13991 instance). Evidence and the ruled-out hypotheses are on that issue; the summary for this PR:

Not caused by this diff

Check	Result
co_change_test.py at this branch's HEAD	40 passed
code_intelligence/ at this branch's HEAD, -n 4	778 passed, 1 skipped
The exact CI shard command, this branch, -n auto (22 workers)	2093 passed, 3 skipped
Base at b9dcc580	AutoBot CI/CD Pipeline success

The diff touches chat_history/, env_registry.py and a changelog fragment. Nothing in code_intelligence/.

Not a flake either — it is deterministic

rerun-failed-jobs reproduced it identically. So re-running does not clear it, and the earlier "probably flaky" read was wrong.

The variable is worker count, not shard membership

The runner reports 2 workers [1991 items]; my machine's -n auto gives 22. Under --dist loadscope the worker count decides which modules share a process, so a 22-worker local run cannot reproduce a 2-worker pollution window. A matched -n 2 run is in progress; result will be posted to #14111 either way.

Supporting signal from the same CI job — five _SysModulesLeakWarnings, one of them in the failing test's own package:

sys.modules['code_intelligence.shared.scoring'] was replaced by
autobot-backend/code_intelligence/conftest.py and is STILL INSTALLED while pytest
handles the import of autobot-backend/command_manual_manager_category_default_test.py

Ruled out, so nobody re-checks it

GIT_* environment leakage across workers (the #13983 failure mode, and the error text is the same family). Both directions are clean: code_evolution_miner.py:116 strips every GIT_-prefixed variable before invoking git, and grep -rn 'environ\[.GIT_|putenv(.GIT_|environ.setdefault(.GIT_' --include=*.py (excluding venv/) hits only co_change_test.py, where every mutation is wrapped in try/finally.

Why this is labelled blocked rather than merged

Red CI does not merge, regardless of whose defect it is. The fix belongs in code_intelligence/ (or in the split configuration) and is out of scope for a compaction PR, so it goes as its own change against #14111 and this branch rebases on it.