Skip to content

feat(hermes): honor the pre-compress checkpoint API v2 contract - #1737

Open
AlexStocks wants to merge 2 commits into
oceanbase:masterfrom
AlexStocks:feat/hermes-pre-compress-checkpoint-v2
Open

AlexStocks wants to merge 2 commits into
oceanbase:masterfrom
AlexStocks:feat/hermes-pre-compress-checkpoint-v2

Conversation

@AlexStocks

@AlexStocks AlexStocks commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Which issue or RFC does this PR close?

No tracking issue. The Hermes host already dispatches the pre-compress checkpoint API v2 contract, but the
PowerContext provider still advertises v1.

Rationale for this change

The host exposes an opt-in, fail-closed contract for pre-compression checkpoints. Its memory manager reads the API
version advertised by the provider and only then does two things:

  • it hands the provider evidence_messages, a host-normalized transcript from which tool results, system messages,
    assistant tool_calls payloads and earlier compression summaries have already been removed;
  • it passes require_checkpoint and, when the provider raises while a checkpoint was required, propagates that
    exception so the caller keeps the uncompressed transcript.

The PowerContext provider advertises v1 (on_pre_compress(self, messages)), so neither happens:

  1. Evidence purity. The provider receives the raw transcript and filters roles itself. It cannot tell which
    user/assistant turns the host already replaced with a compression summary, so a summary can be stored as fresh
    evidence on the next compression window.
  2. Checkpoint strength. on_pre_compress is best-effort: a failed capture logs a diagnostic and returns "",
    which the manager counts as a successful checkpoint. With compression.checkpoint_required: true, the host then
    discards the uncompressed transcript even though nothing was stored.

What changes are included in this PR?

  • Declare pre_compress_checkpoint_api_version = 2 on PowerContextMemoryProvider.
  • Accept evidence_messages and require_checkpoint in on_pre_compress, and prefer the host-normalized evidence
    when the host supplies it. messages remains the fallback for hosts that only speak v1.
  • Raise PreCompressCheckpointError when a required checkpoint cannot be committed: capture disabled, provider not
    bound to a client and Scope, the Scope changed mid-capture, or the capture request failed.
  • Treat capture_content as the durable checkpoint. A later flush_memory failure no longer downgrades a committed
    checkpoint, because the transcript is already stored.
  • Document the v2 behavior and the checkpoint_required / capture_pre_compress combination in both READMEs.
  • Add 8 behavior tests covering the advertised version, evidence preference, the required-checkpoint failure and
    success paths, the already-captured-window case, and the committed-checkpoint-survives-flush-failure case.

Are there any user-facing changes?

Default behavior is unchanged: capture_pre_compress is off by default and require_checkpoint defaults to false,
so capture failures stay fail-open exactly as before.

One new interaction is worth calling out. If the host runs with compression.checkpoint_required: true while
capture_pre_compress is disabled, every compression now raises instead of silently discarding the transcript. That
is the intended fail-closed behavior of the v2 contract, and both READMEs now state that
compression.checkpoint_required: true requires capture_pre_compress: true.

How was this change tested?

  • uv run pytest tests/integrations/test_hermes_provider.py -q → 84 passed (76 before, 8 added)
  • uv run pytest tests/integrations/ tests/test_hermes_cli.py -q → 100 passed
  • uv run ruff format --check and uv run ruff check on both changed Python files → clean
  • uv run ty check --python-version 3.11 on both changed Python files → no diagnostics

The new tests assert the whole v2 surface through the provider's public methods: the advertised API version, the
preference for host-normalized evidence over the raw transcript, the required-checkpoint failure path, the required
checkpoint being committed, the already-captured window being skipped, and a committed checkpoint surviving a later
flush failure.

AI usage statement

Prepared by WorkBuddy (DeepSeek-V4.1-Flash) under human direction. The agent implemented the change and ran the
validation commands above. Human review of the diff is expected before merge.

Hermes' MemoryManager already dispatches the pre-compress checkpoint API v2
contract, but the PowerContext provider still advertises v1, so it never
receives the host-normalized evidence list or the require_checkpoint signal.

Declare pre_compress_checkpoint_api_version = 2, accept evidence_messages and
require_checkpoint, and prefer the host-normalized evidence: turns Hermes
already replaced with a compression summary are no longer stored as fresh
evidence.

When a checkpoint is required and cannot be committed, raise
PreCompressCheckpointError so Hermes keeps the uncompressed transcript. Without
a required checkpoint, capture failures stay fail-open as before.

A committed capture is no longer downgraded by a later memory extraction
failure: capture_content is the durable checkpoint, flush_memory is not.

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.

Copilot review overview

🟡 Changes recommended

Address the scope-change race in checkpoint capture and clarify the incomplete README sentence.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 Medium severity · 1 Low severity

Open (2)
What changed in this PR

Updates the Hermes PowerContext provider to support the fail-closed pre-compression checkpoint API v2.

Changes:

  • Advertises API v2 and accepts normalized evidence.
  • Adds required-checkpoint failure handling.
  • Documents behavior and adds integration tests.
File Summary
tests/​integrations/​test_hermes_provider.py Adds checkpoint contract tests.
integrations/​hermes/​README.md Documents v2 checkpoint behavior.
integrations/​hermes/​plugins/​powercontext/​README.md Documents configuration and failure semantics.
integrations/​hermes/​plugins/​powercontext/​provider.py Implements v2 checkpoint handling.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +1023 to 1025
self._flush_memory_if_supported(scope_id=scope_id)
self._precompress_snapshot = [fingerprint for fingerprint, _message in entries]
return ""

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in da00759e. The provider now re-checks the active Scope after capture_content returns and before updating _precompress_snapshot; in required mode it raises if the Scope changed, so a stale checkpoint cannot be reported as successful for the new Scope.

Validation: python -m pytest tests/integrations/test_hermes_provider.py -q, ruff check, ruff format --check, and ty check --python-version 3.11 on the changed files.

Comment on lines +56 to +58
enable `capture_pre_compress` as well: a checkpoint PowerContext cannot commit
raises, and Hermes then keeps the uncompressed transcript instead of discarding
it behind a failed capture.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in da00759e. Reworded the README sentence to make the actor explicit: if PowerContext cannot commit the checkpoint, the provider raises and Hermes keeps the uncompressed transcript.

Validation: ruff format --check integrations/hermes/plugins/powercontext/README.md.

@AsperforMias AsperforMias left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The required checkpoint can report success after storing only a prefix of the transcript. Please address the inline finding before enabling the v2 success guarantee.


# Hermes' PRE_COMPRESS_CHECKPOINT_API_VERSION. Declaring v2 promises that a normal
# on_pre_compress() return means the captured transcript is stored, and that a
# checkpoint this provider cannot commit raises instead of reporting success.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P1] Do not advertise a successful required checkpoint for a truncated transcript

With capture_pre_compress=True and require_checkpoint=True, on_pre_compress() still serializes new entries with limit=30_000, then records every entry in _precompress_snapshot and returns normally. A direct public-hook reproduction with a 31,000-character user message followed by an assistant tail marker stores exactly 30,000 characters, omits the marker, and makes no additional capture on retry.

The truncation predates this PR, but advertising v2 now makes that normal return satisfy the host's required-checkpoint gate (Hermes contract). Compression can therefore proceed without a complete checkpoint, leaving the omitted tail unavailable from that checkpoint. Persist the complete window before reporting success, or raise in required mode when it cannot be fully stored; do not mark unsaved entries as captured. The reproduction used a recording client, not a full Hermes session.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Confirmed on 78b39e68 with the plugin's real HTTP client against a temporary SQLite-backed PowerContext server. With capture_turns=false, a 30,001-character user message followed by an assistant tail marker returned normally with require_checkpoint=True, but the persisted Source contained only 30,000 characters and no marker. Retrying created no additional Source, and the marker was still absent after restarting the server.

The required checkpoint should fully persist the filtered evidence or raise before advancing the snapshot. Otherwise the v2 success signal allows compression while part of the evidence remains unsaved.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in da00759e. Required pre-compress checkpoints now fail before snapshot advancement when the complete new transcript window cannot fit in the bounded capture payload. Optional captures can still persist a bounded prefix, but only the actually captured fingerprints are used for idempotency/snapshot advancement; required mode raises instead of advertising a successful partial checkpoint.

Added regression coverage for oversized required checkpoints and Scope changes during capture.

Validation: python -m pytest tests/integrations/test_hermes_provider.py -q (86 passed), ruff check, ruff format --check, and ty check --python-version 3.11 on the changed files.

Ensure required pre-compress checkpoints only report success after the complete bounded window is stored for the same active Scope. Keep optional captures from advancing the snapshot past persisted entries and clarify the documented fail-closed behavior.

Tested: python -m pytest tests/integrations/test_hermes_provider.py -q; ruff check provider/test files; ruff format --check provider/test/README; ty check provider/test files

This branch has not been deployed

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants