[test][python] Assert long-term memory content instead of an exact LLM item count - #984
[test][python] Assert long-term memory content instead of an exact LLM item count#984weiqingy wants to merge 3 commits into
Conversation
…M item count Generated-by: Claude Code 2.1.226
32b77a5 to
ace9a1c
Compare
The scoping check only covered bob's facts surfacing in alice's set, the direction that inflates alice's count. Misattribution the other way leaves alice's set short instead, so it is reported as an empty set rather than as a leak. Check bob's set for alice's facts ahead of the emptiness assertion. Generated-by: Claude Code 2.1.226
| # single item or stay separate, so the item count is not fixed. bob's set | ||
| # is reported alongside a failure to tell a per-key miss from a store-wide | ||
| # one. | ||
| bob_items = records["bob.2"].items if "bob.2" in records else None |
There was a problem hiding this comment.
bob.2 is optional here, so a missing output record, items is None, and an empty item list are all normalized into “no leaked values.” The new reverse-direction check can therefore pass without ever observing the Bob memory set. I verified that the current check_result passes both when bob.2 is absent and when bob.2.items == [], provided the Alice set is healthy. Since this PR is intended to check isolation in both directions—and zero items are deliberately treated as a failure for Alice—could we assert that bob.2 exists and that its item list is non-empty before scanning it for Alice facts?
There was a problem hiding this comment.
Good catch, the or [] made "no leak" and "never looked" the same result. Fixed in 396efd9: bob.2 now has to exist and be non-empty before the scan, same as alice's set. The three cases you named pass on d1aff9e0 and fail now, and alice's fact landing under bob's agent_id still surfaces as the leak rather than as her empty set.
One knock-on I'd like your read on. Checking bob's emptiness ahead of alice's means a both-empty run now fails on bob, so the store-wide vs per-key signal would have dropped out of the message. I kept it by reporting each key's set in the other's failure message. Does that read right to you?
…ng it The reverse-direction scope check read bob's items through a None-tolerant fallback, so a missing bob.2 record, a null item list, and an empty one all collapsed into an empty scan that passed. The check could therefore report isolation it never observed, while the same emptiness is a deliberate failure on alice's side. Assert that bob.2 exists and carries items before scanning it, and report each key's set in the other's failure message so a per-key miss stays distinguishable from a store-wide one. Generated-by: Claude Code 2.1.228
Linked issue: #983
Purpose of change
test_long_term_memory_async_execution_in_actionassertslen(items) == 1, which pins how many items mem0 stores after alice's two inputs. Whether those two facts collapse into one item or stay separate is the extraction model's decision, so the count is not guaranteed. The comment above the assertion already conceded the variability.On 2026-08-08 the test failed on
mainwithassert 0 == 1, on a commit that changed three markdown files underdocs/contentwith one line each. All three attempts failed, exhausting the--reruns 2budget. The run before it passed this test only on retry, and it was the only rerun in that job.This asserts on retrievable content instead of an exact count.
Two things this does not claim. Counts above
1have not been observed on this test, so loosening them is preventive rather than a response to a recorded failure. And the one failure that has been recorded, the empty set, still fails after this change, deliberately. The immediate value here is the diagnostic and the restored coverage rather than a greener CI.The empty case is deliberately kept as a failure rather than tolerated. Zero stored items means neither add landed, which may be a real defect in the add path rather than model variance, and it is the only one of the four observed outcomes that carries information. To make the next occurrence diagnosable, bob's memory set is reported alongside the failure: if bob has items and alice does not, the miss is per-key; if both are empty, extraction or the store failed wholesale. That matters because the e2e arm runs with
log_cli_level=OFFand pytest-rerunfailures reports only the final attempt, so today an occurrence leaves no trace.Two related changes come with it.
The
created_at < updated_atassertion is dropped. On a plain add mem0 setsupdated_atequal tocreated_at, so the strict comparison holds only when the model picked the update branch. That made it a second instance of the same flake, and it can also fail on a legitimate delete-then-add even when exactly one item survives.An explicit cross-key leak check is added. The exact count was incidentally covering this: alice and bob share a job id and a memory set name, so
agent_idis the only isolation boundary between them, and a scoping break would have surfaced as an inflated count. Asserting that bob's facts never appear in alice's set restores that coverage without depending on a count.The content match is case-insensitive because the stored text is the model's paraphrase rather than the input, so a title-cased "Bananas" would otherwise fail for the same reason the count does.
Timestamp coverage is kept, in a form that does not depend on which branch the model picked. The dropped ordering assertion was incidentally the only thing in the repo asserting that
created_atandupdated_atparse at all:_to_memory_set_itemsuppressesValueError/TypeErrorand yieldsNone, so an unrecognized timestamp format would otherwise go unnoticed. Asserting both fields are populated covers that and holds on both the create and the update path, since mem0 setsupdated_atequal tocreated_aton a plain add.Tests
Test-only change. The modified assertions were exercised against the observed and adjacent outcomes:
NoneThe first row is the failure seen on
main, which still fails. Rows two through four are the counts the model may legitimately produce. Rows five and six are cases the previous assertions failed on for reasons unrelated to the product. The last three are regressions that must still fail.ruff checkandruff format --checkpass on the file.The test is gated on
ACTION_API_KEYand needs a Flink cluster plus a local Ollama embedding model, so it does not run locally without those. Verification of the end-to-end path comes from CI.API
No public API change.
Documentation
doc-neededdoc-not-neededdoc-included