fix(event-backend): make expand_context return timeline-neighbor episodes by edwinyyyu · Pull Request #1541 · MemMachine/MemMachine · GitHub
Skip to content

fix(event-backend): make expand_context return timeline-neighbor episodes - #1541

Open
edwinyyyu wants to merge 4 commits into
MemMachine:mainfrom
edwinyyyu:fix/event-backend-expand-context
Open

fix(event-backend): make expand_context return timeline-neighbor episodes#1541
edwinyyyu wants to merge 4 commits into
MemMachine:mainfrom
edwinyyyu:fix/event-backend-expand-context

Conversation

@edwinyyyu

@edwinyyyu edwinyyyu commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Fixes #1540.

Problem

On the event backend, expand_context was silently inert on the search path: EventMemory.query fetched and materialized the expanded segment windows (a LATERAL query per direction), but LongTermMemory._search_scored_event read only the seed segment's _episode_uid and score from each window, and the response schema has no context field. Responses were byte-identical for expand_context 0 and 5, while every request paid for the expansion.

Fix

Bring the event backend to parity with the declarative backend's context handling (DeclarativeMemory._unify_scored_anchored_episode_contexts):

  • Each scored window contributes the episodes its segments belong to — chronological within the window, the seed's episode as the nucleus. (Every segment carries its _episode_uid, so segment-space windows map directly onto episode-space context; with the passthrough segmenter this is 1:1 timeline neighbors.)
  • Windows are unified best-score-first with the same fill algorithm: taken whole while they fit within num_episodes_limit, then filled by weighted index-proximity to the nucleus (forward recall preferred over backward, same weighting as declarative) until the limit is met. An episode keeps the score of the first window that contributed it.
  • The unified context is returned chronologically, matching the declarative backend's ordering contract for expanded results.
  • expand_context is clamped to num_episodes_limit - 1 (declarative parity).

expand_context == 0 behavior is unchanged — score-ordered seed episodes exactly as before, so existing callers see no difference unless they pass the parameter. Reranked configurations gain the same folding on top of reranker-scored windows (which already consumed the segments for scoring).

Follow-up self-review (third and fourth commits)

Two things the first two commits got wrong:

  • expand_context could go negative. The quota clamp min(max(0, expand_context), num_episodes_limit - 1) is -1 when num_episodes_limit == 0, which SearchMemoriesSpec.top_k allows (no lower bound). EventMemory._query then derives max_backward_segments = -1 and asks the segment store for a negative window — undefined by the SegmentStorePartition contract; the SQLAlchemy store happens to short-circuit on <= 0, the in-memory store computes an empty slice and drops the seed. The floor is now applied last.
  • Neither end-to-end test could tell the fix from its absence — both passed unmodified against the pre-fix long_term_memory.py. FakeEmbedder maps text to [len(text), -len(text)], so under cosine every document scores exactly 1.0 against every query: all seven timeline episodes are seeds of equal rank, ties keep insertion order (chronological), and num_episodes_limit=7 returns all seven with or without expansion. The "expansion adds episodes" assertion compared a limit-2 search against a limit-7 one, so the limit alone explained the difference.

The expansion tests now give each episode its own similarity from an explicit search rank, with the match's four timeline neighbours ranked last. That lets them assert the contract rather than an outcome — an exact episode list or exact score values would be pinned to the fixture's ranking and to the expand_context // 3 split, neither of which the fix claims:

  • No correct top-k can return those neighbours, and any nonzero window around the match reaches at least one of them whatever the backward/forward split, so "expansion returned a neighbour the search itself would not" survives a change of ranking or of the split. Chronological order and the episode limit are asserted alongside it.
  • The clamp is asserted on the call made to the segment store (0 <= backward + forward <= limit - 1, over several limit/expand_context pairs) rather than on which episodes come back.
  • expand_context == 0 is asserted as "matches only, best score first", without naming them.

Exact lists and score values stay in the unit tests, which own the fill algorithm and the score-retention rule. All three expansion tests fail against the code they cover.

Tests

  • End-to-end through the in-memory event-backend wiring: expansion returns neighbor episodes beyond the plain matches, contiguous on the timeline, chronologically ordered, and never exceeding num_episodes_limit.

  • Unit tests for the window→episode-uid extraction (dedup, nucleus identification) and the unification algorithm (whole-context fit, overflow proximity with forward preference, first-window score retention).

  • The expansion tests fail against the pre-fix long_term_memory.py; the unit tests cover the window→episode-uid extraction and the unification algorithm directly.

ruff (pinned 0.15.14) format + check clean; ty check packages/server reports no diagnostics from the touched files. Episodic + server test suites pass (514 passed, 1 skipped).

The same change, based on speedkick, is #1547.

🤖 Generated with Claude Code

edwinyyyu and others added 2 commits August 28, 2026 12:29
…odes

Fixes MemMachine#1540.

On the event backend, expand_context was silently inert: EventMemory
fetched and materialized the expanded segment windows, but
LongTermMemory._search_scored_event read only the seed segment's
_episode_uid and score from each window, and the response schema has no
context field - so responses were byte-identical for expand_context 0
and 5 while every request paid the LATERAL fetch.

The declarative backend, by contrast, folds neighbor episodes into the
returned list (_unify_scored_anchored_episode_contexts). This brings
the event backend to parity:

- Each scored window now contributes the episodes its segments belong
  to (chronological within the window, the seed's episode as nucleus).
- Windows are unified best-score-first with the same fill algorithm as
  the declarative backend: taken whole while they fit within
  num_episodes_limit, then filled by weighted index-proximity to the
  nucleus (forward recall preferred) until the limit is met; an episode
  keeps the score of the first window that contributed it.
- The unified context is returned chronologically, matching the
  declarative backend's ordering contract for expanded results.
- expand_context is clamped to num_episodes_limit - 1 (declarative
  parity).

expand_context == 0 behavior is unchanged (score-ordered seeds, exactly
as before). Reranked configurations gain the same folding on top of
reranker-scored windows.

Tests: end-to-end via the in-memory event-backend wiring (neighbors
returned, chronological order, limit respected) and unit tests for the
window-to-episode-uid extraction and the unification algorithm
(whole-context fit, overflow proximity with forward preference,
first-window score retention).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
edwinyyyu and others added 2 commits August 28, 2026 16:02
expansion tests actually discriminate

Self-review of the two commits above turned up one defect and one hole.

Defect: the quota clamp `min(max(0, expand_context), num_episodes_limit - 1)`
goes negative when `num_episodes_limit == 0` -- reachable, since
`SearchMemoriesSpec.top_k` carries no lower bound. `EventMemory._query`
then derives `max_backward_segments = -1 // 3 = -1` and hands the segment
store a negative window, which the SegmentStorePartition contract does not
define: the SQLAlchemy store happens to short-circuit on `<= 0`, the
in-memory store computes an empty slice and drops the seed. Apply the floor
last so the clamp can only ever produce a non-negative window.

Hole: neither end-to-end test could tell the fix from its absence -- both
pass unmodified against the pre-fix `long_term_memory.py`. `FakeEmbedder`
maps text to `[len(text), -len(text)]`, so under cosine every document
scores exactly 1.0 against every query; all seven timeline episodes become
seeds of equal rank, ties keep insertion order (which is chronological),
and `num_episodes_limit=7` returns all seven with or without expansion. The
"expansion adds episodes" assertion compared a limit-2 search against a
limit-7 one, so the limit alone explained the difference.

Embed on a keyword instead: only `tl-3` matches the query, so `tl-4` and
`tl-5` -- which score zero -- can reach the result only through the
expansion. The tests now pin the exact window (`[tl-3, tl-4, tl-5]`,
chronological, each keeping the window's score), the clamp against an
oversized `expand_context`, and the non-negative window above. All three
fail against the code they cover.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PVtg6Zea292Pb9L7GXnTJp
The tests added in the previous commit discriminate, but they pin an
outcome: an exact episode list (`[tl-3, tl-4, tl-5]`) and exact score
values. Both are properties of the fixture's ranking and of the
backward/forward split `expand_context // 3`, neither of which the fix
claims -- change the split or the scoring and the tests fail while the
behaviour under test is still correct.

Restate them as the contract. Each episode now gets its own similarity from
an explicit search rank, with the match's four timeline neighbours ranked
last, so:

- no correct top-k can return those neighbours, and any nonzero window
  around the match reaches at least one of them whatever the split. The
  assertion is "expansion returned a neighbour the search itself would not",
  plus chronological order and the episode limit.
- the clamp is asserted on the call made to the segment store
  (0 <= backward + forward <= limit - 1, over several limit/expand_context
  pairs) rather than on which episodes come back.
- `expand_context == 0` is asserted as "matches only, best score first",
  without naming them.

Exact lists and score values stay in the unit tests, which own the fill
algorithm and the score-retention rule and are meant to track them. All
three expansion tests still fail against the code they cover.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PVtg6Zea292Pb9L7GXnTJp
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.

expand_context is silently ignored by the event backend's REST search path

1 participant