Skip to content

fix(compression): pass platform on compression-boundary session_start (#27633) - #27659

Closed
briandevans wants to merge 2 commits into
NousResearch:mainfrom
briandevans:fix/compression-boundary-platform-27633
Closed

fix(compression): pass platform on compression-boundary session_start (#27633)#27659
briandevans wants to merge 2 commits into
NousResearch:mainfrom
briandevans:fix/compression-boundary-platform-27633

Conversation

@briandevans

@briandevans briandevans commented May 17, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds platform=agent.platform or "cli" to the context-engine on_session_start call fired at the compression boundary in agent/conversation_compression.py. Mirrors the canonical pattern already used by the initial session-start at agent/agent_init.py:1351. Two new regression tests cover the explicit-platform and cli fallback paths.

Root cause: when _compress_context rotates session_id because of compression, the boundary notification at agent/conversation_compression.py:373 omits platform. Plugin context engines such as hermes-lcm cache per-session source lineage in _session_platform, and _apply_session_start_metadata unconditionally overwrites that field from kwargs.get("platform"). So when platform is absent on the compression call it gets reset to "" → normalized to "unknown". Every message ingested after the compression boundary is then attributed to source='unknown' instead of the real platform (discord, telegram, cli, …). The reporter saw 806 messages across 4 sessions in a real deployment attributed to source='unknown' despite originating from Discord; lcm_status source lineage degrades after each compression; lcm_grep source filters miss messages.

Mirrors agent/agent_init.py:1354 precedence: agent.platform or "cli".

Related Issue

Fixes #27633

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 — one-line change at line 377: pass platform=agent.platform or "cli" on the boundary on_session_start call, identical to the initial session-start call at agent/agent_init.py:1354.
  • tests/run_agent/test_compression_boundary_hook.py — two new cases: test_on_session_start_passes_platform_on_compression (agent.platform = "discord" flows through) and test_on_session_start_falls_back_to_cli_when_platform_unset (agent.platform = None falls back to "cli").

How to Test

  1. uv run --with pytest --with pytest-xdist --with pytest-asyncio python3 -m pytest tests/run_agent/test_compression_boundary_hook.py -v
  2. Expected: 5 passed, 0 failed (includes the 2 new regression cases).
  3. Regression guard: temporarily revert the production line; both new tests fail with the exact missing-kwarg signature. Restore — all 5 pass.

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 focused tests for the touched code and all pass (5/5)
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 15.x

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 — platform-independent
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

Related / Positioning

Sibling code paths that may need the same fix: the defensive companion on the plugin side — LCMEngine._apply_session_start_metadata in hermes-lcm should preserve _session_platform when platform is absent from kwargs, so a regression on this call doesn't silently clobber lineage again. Out of scope for this repo, but worth a follow-up in the LCM plugin.

Copilot AI review requested due to automatic review settings May 17, 2026 23:18

Copilot AI 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.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Ensures the compression boundary hook propagates the agent's platform to context engine plugins, preventing per-session source lineage from being reset on compression rollovers.

Changes:

  • Pass platform=agent.platform or "cli" to on_session_start during compression in agent/conversation_compression.py.
  • Add two tests verifying the platform is forwarded (and falls back to "cli" when unset).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
agent/conversation_compression.py Forwards platform (with "cli" fallback) to the compression boundary on_session_start call.
tests/run_agent/test_compression_boundary_hook.py Adds tests covering platform propagation and the "cli" fallback.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint labels May 17, 2026

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

Reviewed diff and verified locally with scripts/run_tests.sh tests/run_agent/test_compression_boundary_hook.py (5 passed). CI checks are green. I attempted to merge but GitHub denied MergePullRequest permission for bbernstein616.

@briandevans
briandevans force-pushed the fix/compression-boundary-platform-27633 branch from c238541 to 814de91 Compare May 19, 2026 01:55
@briandevans

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (was 82 commits behind) and pushed a follow-up commit that pins compressor._last_compress_aborted = False on the MagicMock-based compressors in this test file.

Background: PR #28102 added a new _last_compress_aborted flag on the real compressor, and the production compress_context now early-returns when it's truthy. On a bare MagicMock, attribute access auto-creates a truthy Mock instance, so the production code took the abort branch and session_id never rotated. That broke the two existing tests in this file on origin/main (test_on_session_start_called_with_compression_boundary, test_hook_failure_does_not_break_compression) and would have broken both new tests this PR adds.

Fix is one line per setup, same shape as the existing _last_summary_error = None pin. Focused tests pass locally:

tests/run_agent/test_compression_boundary_hook.py
5 passed in 177.64s

@briandevans
briandevans force-pushed the fix/compression-boundary-platform-27633 branch 4 times, most recently from 39da127 to d6b116a Compare May 28, 2026 22:16
briandevans and others added 2 commits May 29, 2026 22:10
…NousResearch#27633)

When _compress_context rotates session_id at the compression boundary,
the on_session_start call to the context engine omits the platform
kwarg. Plugin engines (e.g. hermes-lcm) that track per-session source
lineage via _session_platform get it overwritten with "" on every
compression rollover, which normalizes to "unknown". All messages
ingested after the boundary are then attributed to source='unknown'
instead of the actual platform (discord, telegram, cli, ...).

The initial session-start call in agent/agent_init.py:1351 already
passes platform=agent.platform or "cli". This change mirrors that
pattern on the compression-boundary call so the platform is preserved
across the rollover.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ests

Rebase onto main showed that PR NousResearch#28117 added a post-compress abort
branch that short-circuits before the session-id rotation that the new
platform-on-compression-boundary tests assert on. MagicMock attrs are
truthy by default, so the two new tests need to pin the abort flag the
same way `test_on_session_start_called_with_compression_boundary` already
does after teknium1's PR NousResearch#28465 caught up the older test on main.
@briandevans
briandevans force-pushed the fix/compression-boundary-platform-27633 branch from d6b116a to 2ead25b Compare May 30, 2026 05:11
@briandevans

Copy link
Copy Markdown
Contributor Author

Closing to focus the queue on security/file-safety work where civilian merges are landing. Happy to reopen if maintainers want this picked up.

@briandevans briandevans closed this Jun 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: compression boundary drops platform kwarg, causing source=unknown on all subsequent LCM messages

4 participants