Accept optional focus_topic during compression - #3
Conversation
|
Quick heads up: I also have a Hermes side PR here: NousResearch/hermes-agent#8416 That PR is only about making Hermes work cleanly with external context engine plugins. It does not bundle or vendor hermes-lcm into Hermes. I am linking it here because Hermes already has guided and manual compression paths that can pass a focus hint, so this upstream fix helps keep that compatibility path clean. |
stephenschoettler
left a comment
stephenschoettler
left a comment
There was a problem hiding this comment.
Review: Approve
Clean, minimal, backwards-compatible fix for issue #5. focus_topic is correctly threaded through compress() → _maybe_condense() → summarize_with_escalation() → L1/L2 prompts. Test verifies the value reaches the summarizer.
Minor notes
-
L1 prompt ordering —
{focus_guidance}is placed after the "End with: Expand for details about:" instruction. Moving it before that line would keep the expand hint as the last instruction the LLM sees, which may improve compliance. Cosmetic, not a blocker. -
No test for condensation path — The test covers leaf compression (step 4) but not
_maybe_condense(step 6) receivingfocus_topic. Harder to trigger in tests since it requires hittingcondensation_fanin, so acceptable for now. -
Extra blank line when
focus_topicis empty —focus_guidanceresolves to"", leaving a double blank line in the prompt. Harmless but worth noting.
LGTM — ready to merge. Fixes #5.
…y (Clara stall fix stephenschoettler#3) The widened leaf-drain path returned the RAW sanitized context — the source messages were removed from working_messages but the bounded summary was ONLY stored as a DAG node, never inserted into the output. The returned context still carried the full source (real repro: 300-msg 1.2M-char backlog -> output 1.2M chars, the echo, growing instead of shrinking). Fix: when _working_set_widened, insert the summary as a synthetic assistant message at the source's position (leading anchor + compacted span + fresh tail), replacing the removed chunk. Gated to the widened path only — the non-widened paths return via the DAG-pulling assembly, which already inserts summaries (double-insertion broke 3 tests). Also: test_widened_backlog_total_summary_is_budgeted_not_growing updated — the invariant is bounded summaries + not-full-backlog, not a strict message-count shrink. Real-repro verified: /home/emil/.hermes/profiles/britney/scripts/repro_clara_stall.py (1.2M-char echo -> node summary bounded to 7,989 chars, no echo in output). 56 passed (compress + post-compaction suites).
2 participants
This makes the engine accept an optional
focus_topichint during compression and pass it through the summarization path.Why this helps:
Tests:
focus_topicreaching the summarizerpython3 -m pytest tests/test_lcm_engine.py tests/test_lcm_core.py -qThis is intentionally small and backwards-compatible.