feat: add turn admission hook#214
Conversation
6b1602b to
8deff5c
Compare
frostming
left a comment
There was a problem hiding this comment.
Not complete reviewed yet. I think we should first resolve the fundamental design choice.
| class SteeringHandle: | ||
| """Control surface exposed to model hooks through turn state.""" | ||
|
|
||
| session_id: str | ||
| buffer: SteeringBuffer |
There was a problem hiding this comment.
Looks like too many abstraction layers, consider uniting SteeringHandle and SteeringBuffer
There was a problem hiding this comment.
The split between SteeringHandle and SteeringBuffer is probably premature for this PR.
This PR currently treats steering as session-scoped, but there is an unresolved design question around whether steering should eventually target a session, a specific active turn, or a forked tape/turn branch. If that becomes first-class later, a public handle over a different internal routing/storage model might make sense.
For now, since the PR only implements session-scoped steering, I agree we can collapse this into one simpler type and avoid the extra abstraction.
| @hookspec(firstresult=True) | ||
| def admit_message( | ||
| self, | ||
| session_id: str, | ||
| message: Envelope, | ||
| turn: TurnSnapshot, | ||
| ) -> AdmitDecision | None: | ||
| """Decide how to handle an inbound channel message for a session. | ||
| Return ``None`` to keep Bub's default concurrent scheduling behavior. | ||
| """ | ||
| raise NotImplementedError |
There was a problem hiding this comment.
Is it better to implement this as a method of Channel?
Because steering is highly related to channels, each channel should be able to define its own steering logic. The current approach only takes the first implemented hook.
There was a problem hiding this comment.
I see admit_message as more than channel-side input handling. Some decisions, especially STEER, only become meaningful when they are paired with a run_model implementation that knows how to consume steering input. If the active model hook never drains the steering queue, a channel-level STEER decision cannot actually steer the running turn.
Motivation
ChannelManagercurrently schedules each inbound channel message as a new turn immediately. The default concurrent behavior is simple, but plugins do not have a hook to decide whether the next message for an already-active session should be processed now, queued, dropped, or offered to the running turn as steering input.This PR adds an optional scheduling decision point while keeping the default behavior unchanged.
Design
Add a new first-result hook:
Nonemeans no decision; if every implementation returnsNone, Bub keeps the current default concurrent scheduling behavior.AdmitDecisionsupports four actions:PROCESS: schedule immediatelyDROP: discard explicitlyWAIT: process after the active turn finishesSTEER: enqueue as per-session steering input for model hooks to consume viastate["_runtime_steering"]Undrained steering messages are promoted to the front of the pending queue when the active turn finishes, preserving FIFO order.
Scope
ChannelManagerpath; directprocess_inbound()calls are unaffectedDROPexplicitlyVerification
uv run pytest -quv run ruff check .uv run mypy src