Skip to content

Commit a2b6388

Browse files
Lynskylateyiling
andauthored
feat(builtin): ,model <id> command for per-session model switching (#244)
* feat(builtin): add `,model <id>` command for per-session model switching Allow switching the model for the current session from chat via the `,model <provider:model>` command, persisted across restarts. - BuiltinImpl.run_model_stream now forwards state["model"] as the per-call model override to Agent.run_stream (one-line pass-through). - New builtin tool `model` (set_model) writes state["model"]; as a registered tool it is directly invocable as the `,model <id>` chat command, and also remains callable by the agent. - A minimal SessionStateStore persists state["model"] per session under bub.home/sessions/<id>.json via load_state/save_state (atomic write). Empty values clear the override; nothing is written to the process environment. - state["model"] is read once at turn start, so a mid-turn switch applies from the next turn (documented in the run_model_stream hookspec). Tests (LLM mocked) cover the pass-through, the tool/command, the end-to-end per-call override, persistence round-trip, and concurrent-session isolation. * Persist per-session model on the tape instead of a parallel store Drops the SessionStateStore (a second JSON persistence module under bub.home/sessions/) and reuses the existing per-session tape to carry state["model"]: - set_model now records each switch as a `model_switch` event on the session tape (context.tape), which merges back to the persisted store at end of turn — no new store. - load_state recovers the latest `model_switch` event from the session tape (reading the persisted store before the per-turn fork exists) and injects state["model"]; nothing recorded -> no injection (fresh session never inherits another's model). - save_state reverts to lifespan-only; the tool owns persistence now. Removes src/bub/builtin/session_state.py + tests/test_session_state.py; rewrites the persistence tests against the tape. Reuses two existing mechanisms (the in-memory state dict + the tape) rather than adding one. `uv run pytest` (205 passed) and `uv run ruff check .` are green. --------- Co-authored-by: yiling <yiling@ebay.com>
1 parent b12ada4 commit a2b6388

6 files changed

Lines changed: 231 additions & 7 deletions

File tree

src/bub/builtin/hook_impl.py

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,23 @@ def _get_agent(self) -> Agent:
7070
self._agent = Agent(self.framework)
7171
return self._agent
7272

73+
async def _recover_session_model(self, session_id: str) -> str | None:
74+
"""Recover the latest per-session model override recorded on the session tape.
75+
76+
The ``model`` tool records each switch as a ``model_switch`` event on the
77+
session's tape. Scanning that tape here (before the per-turn fork exists)
78+
reads the persisted store, so a choice from a prior turn or restart is
79+
restored. Returns ``None`` when nothing was recorded, so a fresh session
80+
never inherits another session's model.
81+
"""
82+
session = self._get_agent().tape.session_tape(session_id, self.framework.workspace)
83+
entries = list(await session.store.fetch_all(session.query().kinds("event")))
84+
for entry in reversed(entries):
85+
if entry.kind == "event" and entry.payload.get("name") == "model_switch":
86+
model = (entry.payload.get("data") or {}).get("model")
87+
return str(model) if model else None
88+
return None
89+
7390
@staticmethod
7491
async def _discard_message(_: ChannelMessage) -> None:
7592
return
@@ -118,6 +135,11 @@ async def load_state(self, message: ChannelMessage, session_id: str) -> State:
118135
state = {"session_id": session_id, "_runtime_agent": self._get_agent()}
119136
if context := field_of(message, "context_str"):
120137
state["context"] = context
138+
# Carry over a previously recorded per-session model override from the
139+
# session tape. Only set when a prior turn actually recorded one, so a
140+
# fresh/unknown session never inherits another session's model.
141+
if model := await self._recover_session_model(session_id):
142+
state["model"] = model
121143
return state
122144

123145
@hookimpl
@@ -126,6 +148,9 @@ async def save_state(self, session_id: str, state: State, message: ChannelMessag
126148
lifespan = field_of(message, "lifespan")
127149
if lifespan is not None:
128150
await lifespan.__aexit__(tp, value, traceback)
151+
# The per-session model override is persisted on the session tape by the
152+
# ``model`` tool itself (a ``model_switch`` event, merged back at end of
153+
# turn), so nothing to write here — this hook only closes the lifespan.
129154

130155
@hookimpl
131156
async def build_prompt(self, message: ChannelMessage, session_id: str, state: State) -> str | list[dict]:
@@ -158,7 +183,12 @@ async def build_prompt(self, message: ChannelMessage, session_id: str, state: St
158183

159184
@hookimpl
160185
async def run_model_stream(self, prompt: str | list[dict], session_id: str, state: State) -> AsyncStreamEvents:
161-
return await self._get_agent().run_stream(session_id=session_id, prompt=prompt, state=state)
186+
return await self._get_agent().run_stream(
187+
session_id=session_id,
188+
prompt=prompt,
189+
state=state,
190+
model=state.get("model"),
191+
)
162192

163193
@hookimpl
164194
def register_cli_commands(self, app: typer.Typer) -> None:

src/bub/builtin/tools.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,22 @@ async def quit_tool(*, context: ToolContext) -> str:
403403
return "Session tasks stopped."
404404

405405

406+
@tool(name="model", context=True)
407+
async def set_model(model_id: str, *, context: ToolContext) -> str:
408+
"""Switch the model for THIS session. Invoke as the `,model <model_id>` command.
409+
410+
Takes effect on the NEXT turn and persists across restarts. Pass any
411+
``provider:model`` string (for example ``openai:gpt-4o`` or
412+
``openrouter:openrouter/free``). An invalid model surfaces as an error on the
413+
next turn — run `,model <valid_id>` again to recover.
414+
"""
415+
context.state["model"] = model_id
416+
# Persist on the session tape (merged back at end of turn); load_state
417+
# recovers the latest `model_switch` event next turn / after restart.
418+
await context.tape.append_event("model_switch", {"model": model_id})
419+
return f"Session model set to {model_id} (applies from the next turn)."
420+
421+
406422
def _resolve_path(context: ToolContext, raw_path: str) -> Path:
407423
workspace = context.state.get("_runtime_workspace")
408424
path = Path(raw_path).expanduser()

src/bub/hookspecs.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,13 @@ def run_model(self, prompt: str | list[dict], session_id: str, state: State) ->
4343

4444
@hookspec(firstresult=True)
4545
def run_model_stream(self, prompt: str | list[dict], session_id: str, state: State) -> AsyncStreamEvents:
46-
"""Run model for one turn and return a stream of events. Should not be implemented if `run_model` is implemented."""
46+
"""Run model for one turn and return a stream of events. Should not be implemented if `run_model` is implemented.
47+
48+
Implementations may honor a runtime model override by reading
49+
``state["model"]`` (any ``provider:model`` string). The value takes
50+
effect on the turn in which it is read, so a model switched mid-turn via
51+
the `,model <id>` command applies from the *next* turn.
52+
"""
4753
raise NotImplementedError
4854

4955
@hookspec

tests/test_builtin_agent.py

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import pytest
99
from any_llm.types.completion import ChatCompletionChunk
1010

11+
import bub.builtin.tools # noqa: F401 — registers builtin tools (incl. `model`)
1112
from bub.builtin.agent import Agent
1213
from bub.builtin.model_runner import ModelRunner
1314
from bub.builtin.settings import AgentSettings
@@ -243,6 +244,35 @@ async def test_agent_run_model_defaults_to_none() -> None:
243244
assert completion_kwargs["model"] == "test:model"
244245

245246

247+
@pytest.mark.asyncio
248+
async def test_agent_run_model_override_does_not_mutate_default() -> None:
249+
"""A per-call model override must not leak into the agent's configured model.
250+
251+
The override is resolved per turn (``model or self.settings.model``) and
252+
forwarded to any-llm; it must never be written back to ``settings.model``.
253+
This is the agent-layer half of the guarantee that a session-scoped model
254+
switch (state['model'] -> run_stream(model=...)) cannot bleed across
255+
sessions the way a process-global env var would.
256+
"""
257+
agent = _make_agent()
258+
fork_capture = _ForkCapture()
259+
agent.tape = _FakeTapeFactory(fork_capture) # type: ignore[assignment]
260+
default_model = agent.settings.model
261+
262+
result = await agent.run_stream(
263+
session_id="user/s1",
264+
prompt="hello",
265+
state={"_runtime_workspace": "/tmp"}, # noqa: S108
266+
model="openai:gpt-4o",
267+
)
268+
[event async for event in result]
269+
270+
completion_kwargs = _model_runner(agent).completion_kwargs
271+
assert completion_kwargs is not None
272+
assert completion_kwargs["model"] == "openai:gpt-4o"
273+
assert agent.settings.model == default_model
274+
275+
246276
@pytest.mark.asyncio
247277
async def test_agent_run_resolves_allowed_tool_aliases_and_limits_prompt() -> None:
248278
allowed_name = "tests.allowed_agent_tool"
@@ -295,3 +325,27 @@ async def test_agent_run_rejects_unknown_allowed_tools() -> None:
295325

296326
with pytest.raises(ValueError, match="tests_missing_agent_tool"):
297327
[event async for event in stream]
328+
329+
330+
@pytest.mark.asyncio
331+
async def test_run_command_model_switches_session_model_directly() -> None:
332+
""",model <model_id> runs the `model` builtin as a chat command with no LLM call.
333+
334+
The command writes state['model'] on the same state object the framework
335+
hands to run_stream, so the override is picked up by run_model_stream on the
336+
next turn. Persistence is a `model_switch` event the tool records on the
337+
session tape (merged back at end of turn), not a side effect of save_state.
338+
"""
339+
agent = _make_agent()
340+
fork_capture = _ForkCapture()
341+
agent.tape = _FakeTapeFactory(fork_capture) # type: ignore[assignment]
342+
state: dict[str, Any] = {"_runtime_workspace": "/tmp"} # noqa: S108
343+
assert "model" not in REGISTRY or REGISTRY["model"].context is True
344+
345+
stream = await agent.run_stream(session_id="user/s1", prompt=",model openai:gpt-4o", state=state)
346+
events = [event async for event in stream]
347+
348+
# state["model"] is mutated in place (tool context shares the framework state).
349+
assert state["model"] == "openai:gpt-4o"
350+
deltas = [event.data.get("delta", "") for event in events if event.kind == "text"]
351+
assert any("Session model set to openai:gpt-4o" in delta for delta in deltas)

tests/test_builtin_hook_impl.py

Lines changed: 84 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,11 @@
99

1010
from bub.builtin.hook_impl import AGENTS_FILE_NAME, DEFAULT_SYSTEM_PROMPT, BuiltinImpl
1111
from bub.builtin.store import FileTapeStore
12+
from bub.builtin.tape import Tape
1213
from bub.channels.message import ChannelMessage
1314
from bub.framework import BubFramework
1415
from bub.runtime import AsyncStreamEvents, StreamEvent
16+
from bub.tape import AsyncTapeStoreAdapter, InMemoryTapeStore, TapeContext
1517

1618

1719
class RecordingLifespan:
@@ -26,18 +28,36 @@ async def __aexit__(self, exc_type, exc, traceback) -> None:
2628
self.exit_args = (exc_type, exc, traceback)
2729

2830

31+
def _fake_tape(home: Path) -> Tape:
32+
return Tape(
33+
archive_path=home / "tapes",
34+
store=AsyncTapeStoreAdapter(InMemoryTapeStore()),
35+
context=TapeContext(),
36+
)
37+
38+
2939
class FakeAgent:
30-
def __init__(self, home: Path) -> None:
40+
def __init__(self, home: Path, *, tape: Tape | None = None) -> None:
3141
self.settings = SimpleNamespace(home=home)
42+
# A real in-memory async tape so load_state's recovery path runs against
43+
# the same store the tests write `model_switch` events to.
44+
self.tape = tape if tape is not None else _fake_tape(home)
3245
self.run_calls: list[tuple[str, str, dict[str, object]]] = []
33-
self.run_stream_calls: list[tuple[str, str, dict[str, object]]] = []
46+
self.run_stream_calls: list[tuple[str, str, dict[str, object], str | None]] = []
3447

3548
async def run(self, *, session_id: str, prompt: str, state: dict[str, object]) -> str:
3649
self.run_calls.append((session_id, prompt, state))
3750
return "agent-output"
3851

39-
async def run_stream(self, *, session_id: str, prompt: str, state: dict[str, object]) -> AsyncStreamEvents:
40-
self.run_stream_calls.append((session_id, prompt, state))
52+
async def run_stream(
53+
self,
54+
*,
55+
session_id: str,
56+
prompt: str,
57+
state: dict[str, object],
58+
model: str | None = None,
59+
) -> AsyncStreamEvents:
60+
self.run_stream_calls.append((session_id, prompt, state, model))
4161

4262
async def iterator():
4363
yield StreamEvent("text", {"delta": "agent-output"})
@@ -108,6 +128,43 @@ async def test_load_state_and_save_state_manage_lifespan_and_context(tmp_path: P
108128
assert isinstance(lifespan.exit_args[1], ValueError)
109129

110130

131+
@pytest.mark.asyncio
132+
async def test_load_state_injects_model_recorded_on_session_tape(tmp_path: Path) -> None:
133+
"""A model_switch event recorded on the session tape is restored into state on load."""
134+
_, impl, agent = _build_impl(tmp_path)
135+
session = agent.tape.session_tape("resolved-session", impl.framework.workspace)
136+
await session.append_event("model_switch", {"model": "openai:gpt-4o"})
137+
138+
message = ChannelMessage(session_id="session", channel="cli", chat_id="room", content="hello")
139+
140+
state = await impl.load_state(message=message, session_id="resolved-session")
141+
142+
assert state["model"] == "openai:gpt-4o"
143+
144+
145+
@pytest.mark.asyncio
146+
async def test_load_state_does_not_inject_model_for_unknown_session(tmp_path: Path) -> None:
147+
"""A session with nothing recorded on its tape must not inherit any model (no leakage)."""
148+
_, impl, _ = _build_impl(tmp_path)
149+
150+
message = ChannelMessage(session_id="session", channel="cli", chat_id="room", content="hello")
151+
152+
state = await impl.load_state(message=message, session_id="fresh-session")
153+
154+
assert "model" not in state
155+
156+
157+
@pytest.mark.asyncio
158+
async def test_recover_session_model_returns_latest_recorded(tmp_path: Path) -> None:
159+
"""When several switches were recorded, the most recent one wins."""
160+
_, impl, agent = _build_impl(tmp_path)
161+
session = agent.tape.session_tape("resolved-session", impl.framework.workspace)
162+
await session.append_event("model_switch", {"model": "openai:gpt-4o"})
163+
await session.append_event("model_switch", {"model": "anthropic:claude-3"})
164+
165+
assert await impl._recover_session_model("resolved-session") == "anthropic:claude-3"
166+
167+
111168
@pytest.mark.asyncio
112169
async def test_build_prompt_marks_commands_and_prefixes_context(tmp_path: Path) -> None:
113170
_, impl, _ = _build_impl(tmp_path)
@@ -160,10 +217,32 @@ async def test_run_model_stream_delegates_to_agent(tmp_path: Path) -> None:
160217
events = [event async for event in stream]
161218

162219
assert [(event.kind, event.data) for event in events] == [("text", {"delta": "agent-output"})]
163-
assert agent.run_stream_calls == [("session", "prompt", state)]
220+
assert agent.run_stream_calls == [("session", "prompt", state, None)]
164221
assert agent.run_calls == []
165222

166223

224+
@pytest.mark.asyncio
225+
async def test_run_model_stream_forwards_state_model_override(tmp_path: Path) -> None:
226+
"""state['model'] must be forwarded as the per-call model override."""
227+
_, impl, agent = _build_impl(tmp_path)
228+
state = {"model": "openai:gpt-4o"}
229+
230+
await impl.run_model_stream(prompt="prompt", session_id="session", state=state)
231+
232+
assert agent.run_stream_calls == [("session", "prompt", state, "openai:gpt-4o")]
233+
234+
235+
@pytest.mark.asyncio
236+
async def test_run_model_stream_passes_none_when_state_has_no_model(tmp_path: Path) -> None:
237+
"""Without state['model'] the agent must fall back to its configured model."""
238+
_, impl, agent = _build_impl(tmp_path)
239+
state = {"context": "ctx"}
240+
241+
await impl.run_model_stream(prompt="prompt", session_id="session", state=state)
242+
243+
assert agent.run_stream_calls[-1][3] is None
244+
245+
167246
def test_system_prompt_appends_workspace_agents_file(tmp_path: Path) -> None:
168247
_, impl, _ = _build_impl(tmp_path)
169248
(tmp_path / AGENTS_FILE_NAME).write_text("local rules", encoding="utf-8")

tests/test_builtin_tools.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
quit_tool,
2121
render_tools_prompt,
2222
resolve_tool_names,
23+
set_model,
2324
)
2425
from bub.runtime import ErrorKind
2526
from bub.tape import AsyncTapeStoreAdapter, InMemoryTapeStore, TapeContext
@@ -151,6 +152,44 @@ def test_resolve_tool_names_rejects_unknown_names() -> None:
151152
resolve_tool_names(None, exclude={" tests_missing_tool "})
152153

153154

155+
def test_set_model_is_registered_with_context() -> None:
156+
assert "model" in REGISTRY
157+
tool_obj = REGISTRY["model"]
158+
assert tool_obj.context is True
159+
assert tool_obj.parameters == {
160+
"type": "object",
161+
"properties": {"model_id": {"type": "string"}},
162+
"required": ["model_id"],
163+
}
164+
165+
166+
@pytest.mark.asyncio
167+
async def test_set_model_writes_model_into_state_and_records_on_tape(tmp_path) -> None:
168+
context = _tool_context(tmp_path)
169+
assert "model" not in context.state
170+
171+
result = await set_model.run(model_id="openai:gpt-4o", context=context)
172+
173+
assert context.state["model"] == "openai:gpt-4o"
174+
assert "openai:gpt-4o" in result
175+
assert "next turn" in result.lower()
176+
# The switch is also persisted as a `model_switch` event on the session
177+
# tape, which load_state recovers on the next turn / after restart.
178+
entries = list(await context.tape.store.fetch_all(context.tape.query().kinds("event")))
179+
switches = [entry for entry in entries if entry.kind == "event" and entry.payload.get("name") == "model_switch"]
180+
assert len(switches) == 1
181+
assert switches[0].payload.get("data") == {"model": "openai:gpt-4o"}
182+
183+
184+
@pytest.mark.asyncio
185+
async def test_set_model_overwrites_previous_model(tmp_path) -> None:
186+
context = _tool_context(tmp_path, model="openai:gpt-4o")
187+
188+
await set_model.run(model_id="anthropic:claude-3", context=context)
189+
190+
assert context.state["model"] == "anthropic:claude-3"
191+
192+
154193
@pytest.mark.asyncio
155194
async def test_bash_returns_stdout_for_foreground_command(tmp_path) -> None:
156195
result = await bash.run(cmd=_python_shell("print('hello')"), context=_tool_context(tmp_path))

0 commit comments

Comments
 (0)