Skip to content

fix(pi): dedupe active_memory injection across turns (#1007) - #1008

Open
Nithin745 wants to merge 1 commit into
mksglu:mainfrom
Nithin745:fix/1007-dedupe-active-memory-injection
Open

fix(pi): dedupe active_memory injection across turns (#1007)#1008
Nithin745 wants to merge 1 commit into
mksglu:mainfrom
Nithin745:fix/1007-dedupe-active-memory-injection

Conversation

@Nithin745

Copy link
Copy Markdown

Fixes #1007

Root cause

before_agent_start unconditionally rebuilt _pendingContext from
db.getEvents(sessionId, { minPriority: 3, limit: 50 }) on every turn where
that query returned any rows — even when the returned events were identical
to what had already been injected on a prior turn.

The "context" hook pushes _pendingContext as a transient message that is
never persisted back into context.messages (transformContext only
mutates a local copy of the message array for that one outgoing call), so a
repeated-but-never-cached-in-this-exact-form message lands right where
Anthropic's cache_control breakpoint sits — a full cache-miss rewrite
instead of a cache-read on every such turn. This is the exact reproduction
documented in #1007 (session events vs. Anthropic usage deltas in a live
transcript).

Fix

The resume-snapshot injection right next to this code already solves the
identical problem with a "consumed" flag (db.markResumeConsumed), shown
exactly once. This PR applies the same high-water-mark pattern to
active_memory:

  • A new module-level _lastShownActiveMemoryEventId (mirroring the existing
    single-session-per-process module state already in this file, e.g.
    _sessionId / _pendingContext) tracks the max session_events.id
    already shown as active_memory in the current session.
  • before_agent_start now only considers events newer than that mark. If
    nothing qualifies, no active_memory block is built and no cache-miss
    message is added for that reason.
  • The mark advances to cover every newly-considered event once
    memoryContext is non-empty, and resets on session_start /
    session_shutdown.

All existing behavior is preserved: the minPriority: 3 filter, the
limit: 50 cap, the role category exclusion (#856), the
buildAutoInjection 500-token-budget helper with its inline fallback
formatting, and the resume-snapshot logic (untouched).

Note: the always-on routing anchor (Pi-1) still fires every turn regardless
— that's an intentional, separate, fixed-size injection out of scope for
this issue, which is specifically about active_memory repetition.

Tests

Added an "Issue #1007" describe block in tests/pi-extension.test.ts with
two cases, following the file's existing conventions:

  1. Reproduces the bug: seeds one priority-3 non-role session event,
    confirms it's injected on the first before_agent_start turn, then
    confirms a second turn with no new qualifying events does not repeat
    that content.
  2. Confirms the fix scales: a new qualifying event landing between two
    turns causes the second turn's injection to contain only the new
    event, not the previously-shown one again.

Both new tests were confirmed to fail against the pre-fix code (verified
by temporarily reverting the src/adapters/pi/extension.ts change and
re-running) and pass with the fix applied.

Ran the full suite: npm test → 210 test files / 4719 tests passed, 25
skipped (pre-existing skips, unrelated to this change). npm run build and
npm run typecheck both clean.

before_agent_start unconditionally rebuilt _pendingContext from
db.getEvents(sessionId, { minPriority: 3, limit: 50 }) on every turn
where that query returned any rows, even when the returned events
were identical to what had already been injected on a prior turn.

The "context" hook pushes _pendingContext as a transient message that
is never persisted back into context.messages (transformContext only
mutates a local copy for that one outgoing call), so a repeated but
never-cached-in-this-exact-form message lands right where Anthropic's
cache_control breakpoint sits, causing a full cache-miss rewrite
instead of a cache-read on every such turn.

The resume-snapshot injection right next to this code already solves
the identical problem with a "consumed" flag (db.markResumeConsumed),
shown exactly once. This adds the same high-water-mark treatment for
active_memory: a module-level _lastShownActiveMemoryEventId (mirroring
the existing single-session-per-process module state already in this
file, e.g. _sessionId and _pendingContext) tracks the max
session_events.id already shown. Only events newer than that mark are
considered on each turn; if nothing qualifies, no active_memory block
is built and no cache-miss message is added for that reason. The mark
resets on session_start and session_shutdown.

All existing behavior is preserved: the minPriority:3 filter, the
limit:50 cap, the role category exclusion, the buildAutoInjection
500-token-budget helper with its inline fallback formatting, and the
resume-snapshot logic (untouched).

Tests: added two cases in tests/pi-extension.test.ts (the "Issue mksglu#1007"
describe block) that reproduce the bug against the pre-fix code (both
fail without this change, confirmed by temporarily reverting the fix)
and verify the fix:
  - a turn with no new qualifying events since the last injection does
    not repeat the previously-shown active_memory content
  - a turn with a genuinely new qualifying event injects only the new
    event, not the full set again

Fixes mksglu#1007
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.

"context" hook injection still breaks prompt cache on active-memory updates — regression of #598's fix intent

2 participants