weiqingy commented on code in PR #984:
URL: https://github.com/apache/flink-agents/pull/984#discussion_r3763438217
##########
python/flink_agents/e2e_tests/e2e_tests_integration/long_term_memory_test.py:
##########
@@ -303,13 +303,45 @@ def check_result(*, result_dir: Path) -> None:
for record in actual_result:
records[f"{record.name}.{record.count}"] = record
+ assert "alice.2" in records, f"missing alice.2; got {sorted(records)}"
items = records["alice.2"].items
- # LLMs may treat different review comments as updates to the same
- # fact or as distinct facts.
- assert len(items) == 1
- item: MemorySetItem = items[-1]
- assert item.created_at < item.updated_at
- assert "bananas" in item.value
+ # The extraction model decides whether alice's two facts collapse into a
+ # 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
Review Comment:
Good catch, the `or []` made "no leak" and "never looked" the same result.
Fixed in 396efd96: `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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
