Skip to content

fix(agent): make the compression retry cap config-driven (compression.max_attempts) - #64010

Closed
Kenmege wants to merge 2 commits into
NousResearch:mainfrom
Kenmege:fix/config-driven-compression-attempts
Closed

fix(agent): make the compression retry cap config-driven (compression.max_attempts)#64010
Kenmege wants to merge 2 commits into
NousResearch:mainfrom
Kenmege:fix/config-driven-compression-attempts

Conversation

@Kenmege

@Kenmege Kenmege commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Makes the conversation loop's compression retry cap configurable: compression.max_attempts (default 3 — identical to today's hardcoded value, so an unset key is behavior-neutral).

Why: the cap is currently max_compression_attempts = 3, hardcoded in the loop. Sessions that legitimately need more rounds are stranded with no recourse: on a restart history reload, incompressible tool schemas can keep the per-request estimate above the compressor threshold even though the message floor compresses correctly — three rounds cannot clear it and the turn dies with Context length exceeded: max compression attempts (3) reached. This is the same failure class as #62605, where the rough token estimate leaves 3 retries short of converging. Observed live on a 272K-context lane with a large tool surface: raising the cap to 6 resolved the dead-end with no other change.

How:

  • Parsed and validated in agent_init alongside the other compression.* keys: >= 1 enforced, hard-capped at 10, non-integer values fall back to 3; attached as agent.max_compression_attempts.
  • The loop reads getattr(agent, "max_compression_attempts", 3), so any object without the attribute keeps today's behavior.
  • Documented in the DEFAULT_CONFIG compression block.

All existing exhaustion/retry semantics (413 payload path, context-length path, /compress/new advice) are unchanged — only the bound becomes configurable.

Related Issue

Related: #62605 (auto-compression fails after 3 retries when the estimate underruns) — this knob is the direct operator-side mitigation for that class; it does not claim to fix the estimator itself.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • agent/agent_init.py — parse + validate compression.max_attempts (default 3, floor 1, cap 10, garbage-tolerant), attach to the agent.
  • agent/conversation_loop.py — replace the hardcoded 3 with the getattr pickup (default preserved).
  • hermes_cli/config.pyDEFAULT_CONFIG documentation for the new key.
  • cli-config.yaml.example — the new key documented in the compression: block.
  • tests/agent/test_compression_max_attempts_config.py — six tests pinning the parse/validate/attach seam (default, custom, ceiling, floor, garbage, loop-side getattr degradation), following the existing test_codex_gpt55_autoraise_notice.py pattern of building a real AIAgent under a monkeypatched config.

How to Test

  1. pytest tests/agent/test_compression_max_attempts_config.py -q — 6 new tests pass.
  2. Behavior-neutral proof: with the key unset, agent.max_compression_attempts == 3 and the loop bound is unchanged.
  3. Operator repro: in a session that dies with "max compression attempts (3) reached", set compression.max_attempts: 6 in config.yaml and restart — the additional rounds allow the preflight to converge (verified live on a tool-schema-heavy 272K lane).

Checklist

Code

Documentation & Housekeeping

  • I've updated relevant documentation (DEFAULT_CONFIG comment block documents the key, when to raise it, and its bounds) — README/docs/ N/A
  • I've updated cli-config.yaml.example (new max_attempts entry in the compression: block, matching the surrounding key documentation style)
  • CONTRIBUTING.md / AGENTS.md — N/A (no architecture/workflow change)
  • Cross-platform impact considered: pure-Python config parsing, no platform-specific behavior
  • Tool descriptions/schemas — N/A (no tool behavior changed)

Screenshots / Logs

Live dead-end this resolves (gateway restart, 272K lane, large tool surface):

❌ Max compression attempts (3) reached before API request.
Context length exceeded: max compression attempts (3) reached.

With compression.max_attempts: 6, the same reload converges and the turn completes; sessions without the key see byte-identical behavior to today.

🤖 Generated with Claude Code

@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint area/config Config system, migrations, profiles P2 Medium — degraded but workaround exists labels Jul 13, 2026
….max_attempts)

The conversation loop hardcodes max_compression_attempts = 3. Sessions
that legitimately need more rounds are stranded: on a restart history
reload, incompressible tool schemas can keep the per-request estimate
above the compressor threshold even though the message floor compresses
correctly, so three rounds cannot clear it and the turn dies with
"Context length exceeded: max compression attempts (3) reached" — the
same failure class as NousResearch#62605, where the rough estimate similarly leaves
3 retries short.

Make the cap a config key, compression.max_attempts:

- default 3 = identical to today, so an unset key is behavior-neutral;
- parsed and validated in agent_init alongside the other compression.*
  keys (>= 1, hard-capped at 10, non-integer values fall back to 3),
  attached as agent.max_compression_attempts;
- the loop reads it via getattr(agent, "max_compression_attempts", 3),
  so objects without the attribute keep the prior behavior;
- documented in the DEFAULT_CONFIG compression block.

Tests pin the parse/validate/attach seam: default preserved, custom
value honored, floor and ceiling enforced, garbage tolerated, and the
loop-side getattr degradation.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Kenmege
Kenmege force-pushed the fix/config-driven-compression-attempts branch from 68eff97 to c040037 Compare July 13, 2026 22:34
@Kenmege

Kenmege commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

CI triage on the slice 8/8 failure: all 7 failed tests are in tests/tools/test_managed_browserbase_and_modal.py, and every one dies at import-time inside urllib3's brotli hook, not in the tests themselves:

>           DECODER_ERROR_CLASSES += (brotli.error,)
E           AttributeError: module 'brotlicffi' has no attribute 'error'
.venv/lib/python3.11/site-packages/urllib3/response.py:461: AttributeError

This PR touches no dependencies (uv lock --check is green), the same file passes 15/15 locally on this branch, the other 7 test slices pass here, and main's current runs are green — so this looks like a transient brotlicffi/urllib3 skew on that one runner. Could a maintainer re-run the failed job when convenient? Happy to rebase instead if preferred.

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Comment

30 PRs reviewed (batch: 64044-63999). See aggregate summary at PR 64044.


Reviewed by Hermes Agent

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing a real current-head compression limit. The configuration approach fits the existing compression section and the patch applies cleanly, but one shared retry path remains hardcoded.

Problems

  • agent/conversation_loop.py:1046 still gates pre-API compaction with compression_attempts < 3, and agent/conversation_loop.py:1054 still logs /3. The proposed max_compression_attempts local is initialized only later in the loop. This is the pre-API path described by #62605, so compression.max_attempts: 6 would not permit more than three preflight compactions.
  • The added parser uses int(...), which accepts booleans and truncates fractional numeric input. That does not match the PR's stated fallback behavior for non-integer values.

Suggested changes

  • Resolve the cap before the pre-API guard and use it for both the guard and attempt logging; add an end-to-end loop test that proves a cap above three reaches a fourth pre-API compression.
  • Reject booleans and non-integral numeric values explicitly, with regression coverage.

Automated hermes-sweeper review.

max_compression_attempts = 3
# Config-driven via compression.max_attempts (parsed + validated in
# agent_init). Default 3 preserves the prior hardcoded behavior.
max_compression_attempts = getattr(agent, "max_compression_attempts", 3)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This local is initialized after the pre-API compaction branch, which still uses compression_attempts < 3 and logs /3 on current main (agent/conversation_loop.py:1046-1054). Please resolve the configured cap before that branch and use it there too; otherwise compression.max_attempts: 6 still allows only three preflight compressions on the #62605 path.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 16, 2026
@Kenmege

Kenmege commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

Updated this branch to current main and reran both the PR-specific coverage and the exact slice that previously failed.\n\nFresh locked-environment results:\n- tests/agent/test_compression_max_attempts_config.py: 6/6 pass\n- tests/tools/test_managed_browserbase_and_modal.py: 15/15 pass\n\nThe prior brotlicffi/urllib3 collection failure is no longer reproducible. The update is pushed in 2b520a6ca; CI has been retriggered.

@teknium1 teknium1 added the area/compression Context compression and continuation sessions label Jul 19, 2026
teknium1 added a commit that referenced this pull request Jul 22, 2026
Follow-up to the salvaged #64010 (Kenmege) and #63870 (dombejar) commits,
making one resolved compression.max_attempts cap govern ALL per-turn
compression attempt sites:

- conversation_loop: resolve max_compression_attempts ONCE at turn start
  (it was previously re-resolved inside the API-call loop) and route the
  pre-API pressure gate through it — that gate still hardcoded
  'compression_attempts < 3' and logged 'attempt=%s/3'.
- conversation_loop: the salvaged post-tool compaction gate now uses the
  resolved cap instead of a hardcoded 3.
- turn_context: the preflight compaction loop was 'for _pass in range(3)';
  it now sizes itself from the same resolved cap.
- agent_init: harden the max_attempts parser — reject booleans (bool
  subclasses int; 'true' would coerce to 1), reject fractional floats
  instead of truncating them, keep accepting integral floats and numeric
  strings; anything else falls back to 3 (floor 1, ceiling 10 unchanged).
- tests: replace #63870's inspect.getsource source-shape test with
  behavioral loop tests (post-tool compaction fires <= cap times per turn,
  shares its budget with the pre-API gate, resets between turns); add an
  e2e test proving a 4th preflight pass runs at config cap=6 while the
  unset default still stops at 3; extend the #64010 config tests with the
  bool/float parser semantics.

Salvages #64010 by @Kenmege and #63870 by @dombejar.
@teknium1

Copy link
Copy Markdown
Contributor

Merged via #69315 (commit 1c2faed). Your commit was cherry-picked with authorship preserved, unified with #63870 so all four attempt sites (pre-API gate, retry loop, preflight passes, post-tool gate) honor compression.max_attempts. Thanks!

@teknium1 teknium1 closed this Jul 22, 2026
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
Follow-up to the salvaged NousResearch#64010 (Kenmege) and NousResearch#63870 (dombejar) commits,
making one resolved compression.max_attempts cap govern ALL per-turn
compression attempt sites:

- conversation_loop: resolve max_compression_attempts ONCE at turn start
  (it was previously re-resolved inside the API-call loop) and route the
  pre-API pressure gate through it — that gate still hardcoded
  'compression_attempts < 3' and logged 'attempt=%s/3'.
- conversation_loop: the salvaged post-tool compaction gate now uses the
  resolved cap instead of a hardcoded 3.
- turn_context: the preflight compaction loop was 'for _pass in range(3)';
  it now sizes itself from the same resolved cap.
- agent_init: harden the max_attempts parser — reject booleans (bool
  subclasses int; 'true' would coerce to 1), reject fractional floats
  instead of truncating them, keep accepting integral floats and numeric
  strings; anything else falls back to 3 (floor 1, ceiling 10 unchanged).
- tests: replace NousResearch#63870's inspect.getsource source-shape test with
  behavioral loop tests (post-tool compaction fires <= cap times per turn,
  shares its budget with the pre-API gate, resets between turns); add an
  e2e test proving a 4th preflight pass runs at config cap=6 while the
  unset default still stops at 3; extend the NousResearch#64010 config tests with the
  bool/float parser semantics.

Salvages NousResearch#64010 by @Kenmege and NousResearch#63870 by @dombejar.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/compression Context compression and continuation sessions area/config Config system, migrations, profiles comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants