Skip to content

fix: feature-detect context focus compression support - #16450

Open
Tosko4 wants to merge 1 commit into
NousResearch:mainfrom
Tosko4:fix/context-compress-focus-topic-compat
Open

fix: feature-detect context focus compression support#16450
Tosko4 wants to merge 1 commit into
NousResearch:mainfrom
Tosko4:fix/context-compress-focus-topic-compat

Conversation

@Tosko4

@Tosko4 Tosko4 commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Keeps manual /compress <focus> and forced manual compression compatible with external context engines that were written against older compress(...) signatures, without masking real internal TypeError bugs from those engines.

Current main moved the compression call site from run_agent.py into agent/conversation_compression.py, but the bug class still existed there: Hermes caught any TypeError from context_compressor.compress(...) and retried without optional kwargs. That fallback preserved old plugins, but it could also hide a genuine engine bug as if the engine merely lacked focus_topic / force support.

This refresh feature-detects optional kwargs before calling the engine instead:

  • legacy engines receive only current_tokens
  • engines that accept focus_topic keep the /compress <focus> hint
  • engines that accept force receive the manual retry flag
  • engines that accept **kwargs receive both optional kwargs
  • internal TypeErrors now propagate instead of being treated as signature mismatch

Related Issue

Follow-up slice from the closed broad host-support PR #13370. Related context: #16306 salvaged and merged the compression-boundary signal from that same broader lane.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • agent/conversation_compression.py: add signature-based optional kwarg detection for context_compressor.compress(...) before invoking the engine.
  • tests/run_agent/test_compress_focus_plugin_fallback.py: cover legacy engines, **kwargs engines, focus-only engines, and propagation of internal TypeErrors.
  • tests/agent/test_compression_concurrent_fork.py: update the lock-refresher exception regression now that broad TypeError retry fallback is intentionally removed.

How to Test

  1. Run the focused regression and adjacent compression coverage:
    • ./scripts/run_tests.sh tests/run_agent/test_compress_focus_plugin_fallback.py tests/run_agent/test_413_compression.py tests/agent/test_compression_concurrent_fork.py tests/agent/test_compress_focus.py tests/agent/test_compression_rotation_state.py -q --tb=short
  2. Run static/syntax checks:
    • python3 -m compileall -q agent/conversation_compression.py tests/run_agent/test_compress_focus_plugin_fallback.py tests/agent/test_compression_concurrent_fork.py
    • git diff --check
    • ruff check agent/conversation_compression.py tests/run_agent/test_compress_focus_plugin_fallback.py tests/agent/test_compression_concurrent_fork.py

Validation

Local validation on refreshed head 0e746f2c43359774a147e4cebd8a9a7b7d653d1e:

  • python3 -m compileall -q agent/conversation_compression.py tests/run_agent/test_compress_focus_plugin_fallback.py tests/agent/test_compression_concurrent_fork.py — passed
  • git diff --check — passed
  • ruff check agent/conversation_compression.py tests/run_agent/test_compress_focus_plugin_fallback.py tests/agent/test_compression_concurrent_fork.py — passed
  • ./scripts/run_tests.sh tests/run_agent/test_compress_focus_plugin_fallback.py tests/run_agent/test_413_compression.py tests/agent/test_compression_concurrent_fork.py tests/agent/test_compress_focus.py tests/agent/test_compression_rotation_state.py -q --tb=short52 passed
  • contributor attribution check reproduced locally for the PR delta — passed

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Ubuntu 26.04 / Linux

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — no platform-specific paths or shell behavior changed
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

For New Skills

N/A

Screenshots / Logs

Focused local validation logs are summarized above. No UI screenshots; this is a host-side runtime compatibility fix.

@Tosko4
Tosko4 force-pushed the fix/context-compress-focus-topic-compat branch from 29df49c to 36e8966 Compare April 27, 2026 08:28
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/plugins Plugin system and bundled plugins labels Apr 27, 2026
@Tosko4

Tosko4 commented Apr 27, 2026

Copy link
Copy Markdown
Contributor Author

Context for reviewers: this is one of the small follow-up slices from the closed broad host-support PR #13370.

Teknium salvaged the compression-boundary signal into #16306, and the remaining host-side pieces from #13370 are being resubmitted as narrow, independent PRs instead of reviving the original large PR.

This slice is intentionally limited to its described scope; it does not vendor or bundle any external context-engine implementation.

@Tosko4
Tosko4 force-pushed the fix/context-compress-focus-topic-compat branch 5 times, most recently from 7f160fb to ffc83f2 Compare May 1, 2026 20:20
@Tosko4
Tosko4 force-pushed the fix/context-compress-focus-topic-compat branch from ffc83f2 to e49ede9 Compare May 14, 2026 20:48
@Tosko4
Tosko4 force-pushed the fix/context-compress-focus-topic-compat branch from e49ede9 to d26eb87 Compare July 5, 2026 09:00
@Tosko4
Tosko4 force-pushed the fix/context-compress-focus-topic-compat branch from d26eb87 to 0e746f2 Compare July 5, 2026 09:14
@Tosko4

Tosko4 commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

@teknium1 refreshed this against current main.

This is still relevant: the compression path moved from run_agent.py into agent/conversation_compression.py, but current main still had the broad TypeError fallback around context_compressor.compress(...). That could still mask a real internal engine bug as a legacy signature mismatch.

The refresh now feature-detects optional compression kwargs before calling the engine:

  • legacy engines still get current_tokens only
  • engines accepting focus_topic keep the focused /compress <topic> hint
  • engines accepting force get the manual retry flag
  • engines accepting **kwargs get both optional kwargs
  • internal TypeErrors now propagate instead of being retried as compatibility fallback

Validation on refreshed head 0e746f2c43359774a147e4cebd8a9a7b7d653d1e:

  • python3 -m compileall -q agent/conversation_compression.py tests/run_agent/test_compress_focus_plugin_fallback.py tests/agent/test_compression_concurrent_fork.py
  • git diff --check
  • ruff check agent/conversation_compression.py tests/run_agent/test_compress_focus_plugin_fallback.py tests/agent/test_compression_concurrent_fork.py
  • ./scripts/run_tests.sh tests/run_agent/test_compress_focus_plugin_fallback.py tests/run_agent/test_413_compression.py tests/agent/test_compression_concurrent_fork.py tests/agent/test_compress_focus.py tests/agent/test_compression_rotation_state.py -q --tb=short52 passed
  • contributor attribution check reproduced locally for the PR delta — passed

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for narrowing this to the compression-call boundary. The premise remains present on current main: agent/conversation_compression.py:639-644 catches every TypeError from an engine and retries without focus_topic or force, which can mask an engine-internal TypeError.

The proposed signature-based call construction in 0e746f2c43359774a147e4cebd8a9a7b7d653d1e preserves compatibility for strict legacy engines while leaving the existing outer exception path responsible for lock release. A repository-wide production search found no second direct context_compressor.compress(...) call path requiring the same change.

Automated hermes-sweeper review.

@teknium1 teknium1 added 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 12, 2026
@teknium1 teknium1 added the area/compression Context compression and continuation sessions label Jul 19, 2026

@GottZ GottZ 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.

This was generated by AI during triage.

Summary

Three PRs cover distinct context-compression failure modes: #9050 injects automatic mid-task resumption, #16450 feature-detects optional compressor arguments instead of masking internal TypeErrors, and #21161 exposes compression activity in gateway busy acknowledgements. Only #16450 directly fixes the target call-boundary compatibility bug without conflicting with the repository’s current stale-task policy or touching the separate gateway-status concern.

Related pull requests

  • #9050 [closed] related — (+97/-13) — remain closed: The diff rewrites the summary prefix and injects a synthetic resume message after a tool result, but this conflicts with the contributor-documented design that historical work must not resume unless the latest real user message requests it. It remains relevant as the rejected automatic-resume alternative and must not be reopened over that blocking design decision.
  • #16450 related — (+188/-43) — merge: The diff replaces the broad TypeError retry with signature-based optional-argument selection, preserving legacy compressor compatibility while allowing engine-internal TypeErrors to propagate; tests cover legacy, focus-only, **kwargs, failure, and lock-cleanup paths. This agrees with the keep_open review on #16450, which confirmed the bug on current main and found no second production call path.
  • #21161 related — (+120/-2) — keep open pending rebase: The gateway diff removes misleading iteration 0/N output and adds useful compression activity details, addressing a separate observability defect. Consistent with the keep_open review on #21161, its activity lifecycle must first be moved from the now-obsolete run_agent.py body into agent/conversation_compression.py with cleanup covering every return and exception path.

Suggested consolidation

Merge #16450 because it is the current-path, narrowly scoped fix for compressor capability detection and TypeError masking. Keep #9050 closed due to the explicit contributor-backed design conflict, and retain #21161 as an independent rebase/port candidate rather than treating it as a duplicate; no PRs in this set should be closed as duplicates.

Cross-PR triage: Reviewed 3 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 29 kB of PR diffs, 9 kB of issue/PR text, 6 kB of discussion (6 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

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 comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have 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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants