Skip to content

fix(compaction): skip compression when it cannot reduce the token count - #354

Open
hashbender wants to merge 1 commit into
mainfrom
mirror/pr-56415
Open

fix(compaction): skip compression when it cannot reduce the token count#354
hashbender wants to merge 1 commit into
mainfrom
mirror/pr-56415

Conversation

@hashbender

Copy link
Copy Markdown
Owner

What does this PR do?

Skips trajectory compaction when it cannot actually reduce the token count. compress_trajectory (and its async twin) replaced a compressible middle region with a summary without checking that the summary is smaller than what it replaces. When the region was already small, the "summary" was larger than the original, so the trajectory grew while being marked was_compressed=True (observed: 406 → 465 tokens, tokens_saved = -59).

The fix adds a net-savings guard: when the compressible region's token count is <= the summary target, return the trajectory unchanged instead of expanding it. Applied to both the sync and async paths (sibling call path included).

Related Issue

No separate issue filed; reproduces on current main.

Type of Change

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

Changes Made

  • trajectory_compressor.py — guard both compress_trajectory and compress_trajectory_async: skip compression when region_tokens <= summary_target_tokens, returning the trajectory unchanged.
  • tests/test_trajectory_compressor.py — sync and async tests asserting a tiny compressible region is left unchanged (no token growth, was_compressed not set).

How to Test

  1. On main: compressing a trajectory with a small compressible middle grows the token count yet reports was_compressed=True.
  2. Apply this PR.
  3. scripts/run_tests.sh tests/test_trajectory_compressor.py -q → the added net-savings tests pass (34 passed). Note: 3 tests (*_omits_temperature, kimi/moonshot) fail identically on unmodified main — they are unrelated to this change (temperature-omission for kimi models), not a regression from this PR.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(compaction): ...)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix (no unrelated commits)
  • I've run scripts/run_tests.sh tests/test_trajectory_compressor.py — the net-savings tests pass; the only failures are the 3 pre-existing *_omits_temperature cases that also fail on main
  • I've added tests for my changes (sync + async)
  • I've tested on my platform: macOS 15

Documentation & Housekeeping

  • Documentation — N/A
  • cli-config.yaml.example — N/A
  • CONTRIBUTING.md / AGENTS.md — N/A
  • Cross-platform impact — N/A (pure token-count arithmetic)
  • Tool descriptions/schemas — N/A

Mirror-of: NousResearch#56415
NousResearch#56415

@tenki-reviewer

tenki-reviewer Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review Complete
No issues found!

Risk: 🟢 Low (5/100) — no findings · 88 LOC across 2 files


PR #354 introduces tests for trajectory_compressor.py but does not modify the compressor itself. No issues found across correctness, performance, or training-data preservation lenses.

Files Reviewed (2 files)
tests/test_trajectory_compressor.py
trajectory_compressor.py

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant