Skip to content

Commit d5bd4fd

Browse files
authored
fix(session): pass memory_policy through CreateSessionOptions (#4495)
AsyncHTTPClient.create_session() takes (session_id, options); memory_policy is a key of CreateSessionOptions, not a keyword argument. Three production call sites still pass it as a keyword and raise TypeError: AsyncHTTPClient.create_session() got an unexpected keyword argument 'memory_policy' on every session that does not already exist: - openviking/ingest/replay.py, ConversationReplayClient.ensure_session: `ingest backfill` fails on every new session. The orchestrator catches per-session exceptions, so a first backfill prints one error per session and finishes with 0 commits. - bot/vikingbot/openviking_mount/ov_server.py, VikingClient.ensure_session. - openviking/session/train/components/session_commit.py, SessionCommitPolicyTrainer._commit_one, which swallows the TypeError and returns a failed commit record with an empty task_id. All three now pass options={"memory_policy": policy}, and options=None when no policy is configured. benchmark/locomo/vikingbot/import_to_ov.py already used that form. The three test fakes accepted the obsolete keyword, so none of the paths had regression coverage. They now mirror the real SDK signature: reverting any one of the three fixes fails its tests. Fixes #4493
1 parent cb7fe2a commit d5bd4fd

6 files changed

Lines changed: 26 additions & 17 deletions

File tree

bot/tests/test_openviking_api_key_type.py

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,11 @@ def __init__(self, **kwargs):
5454
async def initialize(self):
5555
return None
5656

57-
async def create_session(self, session_id=None, memory_policy=None):
58-
return {"session_id": session_id or "s-1", "memory_policy": memory_policy}
57+
async def create_session(self, session_id=None, options=None):
58+
return {
59+
"session_id": session_id or "s-1",
60+
"memory_policy": (options or {}).get("memory_policy"),
61+
}
5962

6063
async def session_exists(self, _session_id):
6164
return False
@@ -1896,9 +1899,9 @@ class NotFoundError(Exception):
18961899
async def _get_session(_session_id):
18971900
raise NotFoundError("Resource not found")
18981901

1899-
async def _create_session(session_id=None, memory_policy=None):
1900-
created.append((session_id, memory_policy))
1901-
return {"session_id": session_id, "memory_policy": memory_policy}
1902+
async def _create_session(session_id=None, options=None):
1903+
created.append((session_id, options))
1904+
return {"session_id": session_id, "memory_policy": (options or {}).get("memory_policy")}
19021905

19031906
monkeypatch.setattr(client.client, "get_session", _get_session)
19041907
monkeypatch.setattr(client.client, "create_session", _create_session)
@@ -1909,7 +1912,7 @@ async def _create_session(session_id=None, memory_policy=None):
19091912
)
19101913

19111914
assert result == {"session_id": "session-1", "memory_policy": {"strategy": "compact"}}
1912-
assert created == [("session-1", {"strategy": "compact"})]
1915+
assert created == [("session-1", {"memory_policy": {"strategy": "compact"}})]
19131916

19141917

19151918
@pytest.mark.asyncio

bot/vikingbot/openviking_mount/ov_server.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1169,7 +1169,7 @@ async def ensure_session(
11691169

11701170
return await client.create_session(
11711171
session_id=session_id,
1172-
memory_policy=memory_policy,
1172+
options={"memory_policy": memory_policy} if memory_policy else None,
11731173
)
11741174

11751175
@staticmethod

openviking/ingest/replay.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,8 @@ async def ensure_session(
6868
return await self._client.get_session(ov_session_id)
6969
except NotFoundError:
7070
return await self._client.create_session(
71-
session_id=ov_session_id, memory_policy=memory_policy or None
71+
session_id=ov_session_id,
72+
options={"memory_policy": memory_policy} if memory_policy else None,
7273
)
7374

7475
async def append(self, ov_session_id: str, messages: List[Dict[str, Any]]) -> int:

openviking/session/train/components/session_commit.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ async def _commit_one(
134134
stage = "create_session"
135135
await self.client.create_session(
136136
session_id=session_id,
137-
memory_policy=_training_commit_memory_policy(),
137+
options={"memory_policy": _training_commit_memory_policy()},
138138
)
139139
stage = "batch_add_messages"
140140
await self._batch_add_messages(session_id, messages)

tests/ingest/test_replay.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@ async def get_session(self, sid, *, auto_create=False):
2222
raise NotFoundError(sid, "session")
2323
return {"session_id": sid, "pending_tokens": 0, "message_count": 0}
2424

25-
async def create_session(self, session_id=None, memory_policy=None):
26-
self.created.append(session_id)
25+
async def create_session(self, session_id=None, options=None):
26+
self.created.append((session_id, options))
2727
self.existing.add(session_id)
2828
return {"session_id": session_id}
2929

@@ -40,9 +40,12 @@ async def test_ensure_session_get_or_create():
4040
sdk = _FakeSDK()
4141
client = ConversationReplayClient(sdk)
4242
await client.ensure_session("s1") # not present -> create
43-
assert sdk.created == ["s1"]
43+
assert sdk.created == [("s1", None)]
4444
await client.ensure_session("s1") # now present -> no second create
45-
assert sdk.created == ["s1"]
45+
assert sdk.created == [("s1", None)]
46+
# a memory policy travels inside CreateSessionOptions, not as a keyword
47+
await client.ensure_session("s2", {"working_memory": {"enabled": False}})
48+
assert sdk.created[-1] == ("s2", {"memory_policy": {"working_memory": {"enabled": False}}})
4649

4750

4851
async def test_append_chunks_at_100():

tests/session/train/test_train_framework.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1206,8 +1206,8 @@ def __init__(self):
12061206
self.committed_sessions = []
12071207
self.task_poll_counts = {}
12081208

1209-
async def create_session(self, *, session_id, memory_policy=None):
1210-
self.created_sessions.append((session_id, memory_policy))
1209+
async def create_session(self, session_id=None, options=None):
1210+
self.created_sessions.append((session_id, options))
12111211

12121212
async def batch_add_messages(self, session_id, messages):
12131213
self.messages.setdefault(session_id, []).extend(messages)
@@ -1257,8 +1257,10 @@ async def test_session_commit_policy_trainer_records_commit_trace_id():
12571257
(
12581258
commit_result["session_id"],
12591259
{
1260-
"memory_types": ["cases", "trajectories", "experiences"],
1261-
"working_memory": {"enabled": False},
1260+
"memory_policy": {
1261+
"memory_types": ["cases", "trajectories", "experiences"],
1262+
"working_memory": {"enabled": False},
1263+
}
12621264
},
12631265
)
12641266
]

0 commit comments

Comments
 (0)