fix(compression): preserve human intent and durable handoffs - #66637
fix(compression): preserve human intent and durable handoffs#66637enzo-adami wants to merge 5 commits into
Conversation
Related to #58162's compression-continuity work, but this PR addresses human-anchor selection, empty rotations, stale-parent forks, and durable child persistence. |
tonydwb
left a comment
There was a problem hiding this comment.
{
"event": "APPROVE",
Code Review Summary
Verdict: Approved
Context compressor changes: adds _HISTORICAL_TASK_SECTION_RE to strip historical task sections during compaction, and revises summary template prompts to reduce generic placeholder outputs. Uses more explicit placeholder markers (<specific user task>, <exact latest user request>) to guide the summarizer LLM toward precise output. Also introduces _ACTIVE_TASK_MAX_CHARS = 1400 constant.
Looks Good
_HISTORICAL_TASK_SECTION_REuses(?ms)multiline/dotall flags correctly- Template revisions aim at concrete, non-generic summary output
- Preserves human intent by stripping stale historical task sections
Reviewed by Hermes Agent",
"comments": []
}
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Comment
+345/-44 compression fix for human intent preservation. See also PR 66637 (likely same author batch). No concerns.
Reviewed by Hermes Agent
|
Thanks — the human-anchor grounding and the empty-transcript guard here are solid, and the root-cause narrative checks out (we verified the any-user-role guard and the flush-cursor reset on current main, and the summarizer-copies-prompt-example failure mode is real). Two asks before this can move:
One small note for the rebase: |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused compression hardening. The human-anchor premise is real on current main: agent/conversation_compression.py:440-448 treats every role="user" message as a valid anchor, while agent/conversation_loop.py:5211-5536 produces synthetic user-role scaffolding.
Problems
agent/context_compressor.py:2615does not exclude those synthetic turns when constructing its new deterministic “real user” snapshot. Reuse the real-user predicate or apply the same exclusions.agent/conversation_compression.py:479can prepend a user anchor before an existing synthetic user-only compressed output, creating adjacent user roles.agent/conversation_compression.py:1068is a separatereplace_messages()transaction. Parent end, child creation, and child handoff remain separate writes (agent/conversation_compression.py:966-1000;hermes_state.py:1257-1285), so this is not an atomic rotation handoff.
Suggested changes
- Cover synthetic-latest-turn and synthetic-user-only alternation cases.
- Move the rotation plus initial child transcript into one SessionDB transaction, with an injected failure test between child creation and persistence.
- Per the member review, separate the stale-parent guard overlapping open #64511.
Automated hermes-sweeper review.
| context. | ||
| """ | ||
| for msg in reversed(messages): | ||
| if msg.get("role") != "user": |
There was a problem hiding this comment.
This calls every role="user" turn a real user task. Current runtime messages can carry _empty_recovery_synthetic, _verification_stop_synthetic, or _pre_verify_synthetic; unlike _is_real_user_message, this path does not reject them. Please share the real-human predicate or apply the same exclusions before using this as the deterministic task snapshot.
| ): | ||
| messages.append(anchor) | ||
| else: | ||
| messages.insert(0, anchor) |
There was a problem hiding this comment.
If a compressor returns only synthetic user scaffolding, there is no assistant boundary and the last message is already user, so this produces two consecutive user messages. Preserve role alternation in this fallback path and add a synthetic-user-only regression.
| # A headless turn can be killed before its finalizer. Persist | ||
| # the rotated child's compacted handoff at the boundary so | ||
| # the new session is immediately resumable. | ||
| agent._session_db.replace_messages(agent.session_id, compressed) |
There was a problem hiding this comment.
replace_messages() is atomic only for this child transcript rewrite. The parent end_session() and child create_session() have already committed in separate transactions, so an interruption before this line still creates an empty rotated child. Use one SessionDB transaction for the full rotation/handoff or narrow the atomicity claim and cover the interruption window.
…ing scaffolding-proof Follow-up hardening on top of the salvaged #66637 commits: - _insert_real_user_anchor could place the restored human turn directly next to user-role scaffolding (index-0 insert before a leading synthetic user turn, or a scaffolding-only transcript), breaking the strict alternation contract (#55677). Restoration now merges into trailing scaffolding (anchor text leads, synthetic flags cleared) and appends after a user-role compaction summary instead of inserting adjacent. - _is_real_user_message now also rejects user-role compaction summaries (the compressor pins the summary to role=user when the tail opens with an assistant turn), so a summary can no longer satisfy the human-anchor check and skip restoration. - _latest_user_task_snapshot reuses the same real-user predicate, so the deterministic task snapshot can no longer anchor on todo snapshots, truncation notices, or background-process reports. - The Historical Task Snapshot rewrite keeps the section terminated with a blank line; the previous replacement consumed the boundary newlines, gluing the next '## ' heading mid-line and deleting all later sections on the next iterative compaction. - Drop _length_continuation_synthetic (no producer anywhere). - AUTHOR_MAP entry for enzo-adami.
|
Salvaged and merged in #67275 with your four commits cherry-picked to preserve authorship — thanks @enzo-adami! The follow-up commit on top hardened the anchor insertion for strict role alternation (#55677 edge), made the deterministic task snapshot reuse the real-user predicate (so it can't anchor on todo/truncation scaffolding), and fixed a newline-boundary bug in the snapshot-section rewrite that could delete later summary sections on iterative compaction. The stale-parent post-lock guard was excluded as discussed — that concern is tracked with #64511. |
…ing scaffolding-proof Follow-up hardening on top of the salvaged NousResearch#66637 commits: - _insert_real_user_anchor could place the restored human turn directly next to user-role scaffolding (index-0 insert before a leading synthetic user turn, or a scaffolding-only transcript), breaking the strict alternation contract (NousResearch#55677). Restoration now merges into trailing scaffolding (anchor text leads, synthetic flags cleared) and appends after a user-role compaction summary instead of inserting adjacent. - _is_real_user_message now also rejects user-role compaction summaries (the compressor pins the summary to role=user when the tail opens with an assistant turn), so a summary can no longer satisfy the human-anchor check and skip restoration. - _latest_user_task_snapshot reuses the same real-user predicate, so the deterministic task snapshot can no longer anchor on todo snapshots, truncation notices, or background-process reports. - The Historical Task Snapshot rewrite keeps the section terminated with a blank line; the previous replacement consumed the boundary newlines, gluing the next '## ' heading mid-line and deleting all later sections on the next iterative compaction. - Drop _length_continuation_synthetic (no producer anywhere). - AUTHOR_MAP entry for enzo-adami.
What changed
Root cause
Compression could treat synthetic user messages or summarizer prose as the active task. The newly compacted child also remained deferred until finalization, so a headless interruption or duplicate rotation could lose the resume point or fork stale lineage.
Impact
Strict chat templates retain the actual user anchor, interrupted runs can resume from the durable compacted child, and concurrent compression cannot rotate the same parent twice.
Validation
Ruff and
git diff --checkpass on the branch diff.