Skip to content

feat: expose compression no-op status - #309

Merged
stephenschoettler merged 4 commits into
stephenschoettler:mainfrom
TurgutKural:fix/public-compression-noop-status
Jul 5, 2026
Merged

stephenschoettler merged 4 commits into
stephenschoettler:mainfrom
TurgutKural:fix/public-compression-noop-status

Conversation

@TurgutKural

@TurgutKural TurgutKural commented Jul 4, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Expose public last_compression_status, last_compression_noop_reason, and last_compression_was_noop properties.
  • Update the fresh-tail preflight regression test to assert through the public API instead of private _last_compression_* fields.

Why

  • Hermes host runtimes need a stable public contract to distinguish real LCM compactions from no-op compression attempts.
  • The underlying no-op decision already exists in LCM; this PR makes that decision observable without requiring hosts to reach into private implementation state.

Validation

  • Focused validation: PYTHONPATH=/path/to/Hermes-Agent python3 -m pytest tests/test_lcm_engine.py::test_lcm_tool_status_includes_optional_cache_usage_metrics tests/test_lcm_engine.py::TestEngineABC::test_preflight_does_not_request_compaction_when_only_fresh_tail_is_over_threshold -q -> 2 passed
  • Default validation:
    • pytest tests/test_lcm_core.py tests/test_lcm_engine.py tests/test_packaging_install.py -q
    • pytest -q
    • bash -lc 'ulimit -n 1024 && pytest -q'
    • python -m compileall -q .
    • python -m py_compile scripts/import_lossless_claw.py
    • bash -n scripts/install.sh scripts/update.sh
    • git diff --check -> clean
  • Workflow validation, if workflows changed: N/A (no workflow changes)

Notes

Refs #

@TurgutKural
TurgutKural force-pushed the fix/public-compression-noop-status branch from ec6e942 to c82d228 Compare July 4, 2026 21:14
@stephenschoettler

Copy link
Copy Markdown
Owner

Thanks for the PR. Could you update the description to match the repo template before I review?

Please add:

  • Validation

The code can stay as-is. This just makes review, release notes, and future archaeology easier.

@Tosko4

Tosko4 commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator

@TurgutKural @stephenschoettler I checked this against current main and did a local validation pass.

My read: this is a legit small contract fix, not placebo. The underlying no-op state already exists and is already set by LCM in the relevant paths (should_compress_preflight() and compress()), especially the high-pressure-but-only-fresh-tail/raw-backlog-below-leaf-threshold case. On current main, there is no public last_compression_* property surface, so a host that needs to distinguish “LCM actually compacted” from “LCM deliberately no-oped” has to either inspect private fields or guess from the returned context. This PR exposes that existing decision as a stable public read surface.

That is in line with the goal of LCM: the engine owns deterministic context-boundary decisions, and the host should be able to observe whether a compaction boundary was real or intentionally skipped without poking private implementation state. It does not change compression behavior or make LCM semantically smarter; it just makes the existing decision inspectable by host/runtime code.

Validation I ran on c82d228ce0896d8e4e7973a232c368c4392f8189:

  • python3 -m pytest tests/test_lcm_engine.py::TestEngineABC::test_preflight_does_not_request_compaction_when_only_fresh_tail_is_over_threshold tests/test_lcm_engine.py::test_lcm_tool_status_includes_optional_cache_usage_metrics -q -o addopts= → 2 passed
  • python3 -m pytest tests/test_lcm_engine.py -q -o addopts= → 554 passed
  • python3 -m pytest -q -o addopts= → 1179 passed, 12 xfailed
  • git diff --check origin/main...HEAD → clean
  • Synthetic probe: tokens over threshold, preflight returns False, public status is noop, public bool is True, and the public reason includes the leaf-threshold explanation.

One process note: the PR body still uses ## Test; Stephen already asked for the repo-template ## Validation heading, so that should be updated before final review.

@TurgutKural

Copy link
Copy Markdown
Contributor Author

@Tosko4 @stephenschoettler updated.

@Tosko4

Tosko4 commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator

@TurgutKural thanks, confirmed the PR description is updated.

One clarification for review: the code head for this PR is still c82d228ce0896d8e4e7973a232c368c4392f8189, so this PR itself remains the LCM-side public contract/plumbing change. It exposes the existing no-op decision through public read-only properties, but it does not change compression behavior by itself.

The actual host-side bug fix for “plugin no-op should not be treated as a successful compression boundary / session split” is the linked Hermes Agent PR: NousResearch/hermes-agent#58495.

So my read is:

  • this PR is still valid and useful as the LCM contract surface
  • the runtime bug is only fixed when the host-side PR consumes that contract
  • the two PRs should be reviewed as related, but not as the same fix

@stephenschoettler flagging that distinction explicitly so the LCM-side review stays scoped correctly.

@stephenschoettler

Copy link
Copy Markdown
Owner

Please rebase this onto current main and rerun the focused validation. I want to review this on a current branch before deciding whether it is merge-ready.

@stephenschoettler

Copy link
Copy Markdown
Owner

@codex review

Current head: 5fb3d9e9227b.

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5fb3d9e922

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread engine.py
Comment on lines +861 to +863
def last_compression_was_noop(self) -> bool:
"""Whether the most recent compression/preflight decision was a no-op."""
return self._last_compression_status == "noop"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reset stale no-op status before positive preflights

If a session first hits a preflight no-op (for example, only the fresh tail is over threshold) and a later call to should_compress_preflight(...) becomes eligible, this property still reports True because the positive preflight paths return without clearing _last_compression_status or the no-op reason. A host using the new public flag immediately after preflight can therefore misclassify the next real compaction boundary as an LCM no-op until compress() starts. Please clear or update the status/reason on positive preflight decisions before exposing this flag.

Useful? React with 👍 / 👎.

@stephenschoettler stephenschoettler left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Approved for merge under Stephen approval: reviewer gate t_9eb57f4e passed final head and verified the stale no-op blocker is fixed; CI is green. The remaining Codex thread is stale/non-material against this head.

@stephenschoettler
stephenschoettler merged commit 376c442 into stephenschoettler:main Jul 5, 2026
5 checks passed
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.

3 participants