Add max_emitted_tools - #31
Conversation
📝 WalkthroughWalkthroughChangesGeneration option propagation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant Seq
participant RequestSink
participant TestToolbox
Caller->>Seq: generate with generation options
Seq->>Seq: store resolved options
Seq->>RequestSink: send generation request
Seq->>TestToolbox: execute automatic tool call
Seq->>RequestSink: send tool-return request with stored options
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Cargo.toml`:
- Line 3: Update the package version declaration in Cargo.toml from 0.5.2 to
0.6.0 to reflect the breaking public API changes to GenOpts and SeqGenReq.
In `@src/client/seq.rs`:
- Around line 40-42: Replace the sequence-wide active_gen_opts storage with a
command-ID-keyed map, and update Seq::generate, Seq::send_cmd, and on_tool_call
to read options using the originating cid so concurrent generations remain
isolated. Remove each cid’s entry on successful completion, errors, sequence
close, and local send failure, and add an interleaving test using distinct
generation options.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 11462139-a6aa-4db5-bb22-9245b3d70cab
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
Cargo.tomlsrc/client/mod.rssrc/client/seq.rssrc/protocol.rs
| /// options for the active generation leg, reused by automatic tool returns | ||
| active_gen_opts: Arc<Mutex<SeqGenReq>>, | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- tracked candidate files ---'
git ls-files | rg '(^|/)(seq\.rs|.*seq.*|.*command.*|.*server.*)\b' | head -200
printf '%s\n' '--- src/client/seq.rs outline ---'
ast-grep outline src/client/seq.rs --view compact || true
printf '%s\n' '--- relevant symbols and references ---'
rg -n -C 4 'active_gen_opts|SeqGenReq|SeqCommand::Gen|on_tool_call|send_cmd|generate|tool return|tool_return' src
printf '%s\n' '--- relevant seq.rs sections ---'
sed -n '1,90p' src/client/seq.rs
sed -n '300,475p' src/client/seq.rs
sed -n '680,790p' src/client/seq.rsRepository: mixlayer/modelsocket-rs
Length of output: 26281
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- seq.rs structure and lifecycle handlers ---'
ast-grep outline src/client/seq.rs --view signatures || true
rg -n -C 12 'async fn on_gen_finished|async fn on_close_event|pub async fn close|gen_streams|cmds.remove|send_request|SeqGenFinish|SeqClose' src/client/seq.rs src/client/mod.rs src/protocol.rs
printf '%s\n' '--- seq.rs lifecycle sections ---'
sed -n '80,190p' src/client/seq.rs
sed -n '190,320p' src/client/seq.rs
sed -n '430,620p' src/client/seq.rs
printf '%s\n' '--- protocol command/event definitions ---'
sed -n '1,80p' src/protocol.rs
sed -n '270,410p' src/protocol.rs
printf '%s\n' '--- event dispatch and sequence cleanup ---'
rg -n -C 10 'on_event\(|Seq::on_event|closed_seqs|seqs\.lock|SeqGenFinish|SeqClose' src/client/mod.rs src/client/seq.rsRepository: mixlayer/modelsocket-rs
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- repository scope and concurrency documentation ---'
git ls-files | wc -l
rg -n -i -C 3 'concurr|parallel|simult|one.*(gen|generation)|generation.*(active|state)|tool.?return|tool call|SeqState|Appending|Generating' --glob '!src/client/seq.rs' --glob '!src/protocol.rs' .
printf '%s\n' '--- exact cleanup-related code ---'
sed -n '140,180p' src/client/seq.rs
sed -n '245,296p' src/client/seq.rs
sed -n '532,568p' src/client/seq.rs
printf '%s\n' '--- standalone interleaving verifier ---'
python3 - <<'PY'
from dataclasses import dataclass
`@dataclass`(frozen=True)
class Gen:
cid: str
max_emitted_tools: int
active = None
active = Gen("A", 1)
active = Gen("B", 2)
returned = active.max_emitted_tools
assert returned == 2
assert returned != 1
print("scalar active options return B options for A tool call: reproduced")
PYRepository: mixlayer/modelsocket-rs
Length of output: 6887
🌐 Web query:
modelsocket server SeqCommand Gen concurrent generation SeqToolReturnReq gen_opts
💡 Result:
In the context of the modelsocket protocol, these terms relate to how LLM generations and tool-use workflows are structured and executed [1][2]. Modelsocket is a protocol and client library designed for WebSocket-based LLM integration [3]. Key components include: 1. SeqCommand and Gen: SeqCommand is a request type in the protocol, and Gen (often represented as genCommandData) is a specific command within it used to initiate text generation [1]. 2. SeqToolReturnReq (or tool_return command): This is used to return tool execution results back to the model after a tool call has been processed [1]. The protocol specifically structures this request with gen_opts to allow the model to continue generation or re-evaluate after the tool output is received [1]. 3. gen_opts: This stands for Generation Options. In the modelsocket Go implementation, SeqGenData serves as the data structure for these options, which include parameters like max_tokens, temperature, top_p, and repeat_penalty [1]. When using tool_return, these options are bundled as gen_opts to dictate the behavior of the subsequent generation phase [1]. 4. Concurrent Generation: While the modelsocket protocol supports complex conversational flows, the client library (such as the Go implementation) typically enforces that only one Generate call can be active on a single sequence (Seq) at a time [4]. Concurrent operations are managed via the client's ability to handle multiple sequences independently, rather than parallel generation within a single sequence [4][3]. The distinction between GenServer options (often referred to as gen_opts in Elixir/Erlang contexts, appearing in sources like 9, 10, 11, 12) and the modelsocket Gen/gen_opts should be noted: the former refers to standard Erlang GenServer configuration, while the latter refers to LLM generation parameters within the modelsocket protocol [1][5].
Citations:
- 1: https://github.com/chrisboulton/modelsocket-go/blob/main/protocol.go
- 2: https://docs.rs/crate/modelsocket/latest
- 3: https://github.com/chrisboulton/modelsocket-go
- 4: https://github.com/chrisboulton/modelsocket-go/blob/main/seq.go
- 5: https://repo.hex.pm/preview/redis_server_wrapper/0.6.1/lib/redis_server_wrapper/server.ex
🏁 Script executed:
#!/bin/bash
set -eu
base='https://github.com/ghraw/chrisboulton/modelsocket-go/main'
printf '%s\n' '--- modelsocket-go seq.go generation concurrency logic ---'
curl -fsSL "$base/seq.go" |
rg -n -C 12 'Generate|generate|active|generat|tool|ToolReturn|gen_opts|state'
printf '%s\n' '--- modelsocket-go protocol definitions ---'
curl -fsSL "$base/protocol.go" |
rg -n -C 8 'Gen|ToolReturn|gen_opts|SeqCommand|SeqState'
printf '%s\n' '--- repository references to generation state ---'
curl -fsSL "$base/seq.go" |
rg -n 'sync|Mutex|channel|state|Generating|Ready|Append|Generate'Repository: mixlayer/modelsocket-rs
Length of output: 11200
🌐 Web query:
site:github.com/chrisboulton modelsocket server SeqState StateGenerating tool_return only one generation sequence
💡 Result:
In the ModelSocket protocol implementation (specifically the Go client), a sequence object (represented by the Seq struct) is designed to manage an active conversation, but it enforces a constraint where only one generation call can be active at a time [1]. The StateGenerating status is one of several sequence states defined in the protocol [2]: StateReady: The sequence is prepared for interaction. StateAppending: The sequence is appending data. StateGenerating: The sequence is currently in the process of generating content [2]. StateToolCall: The sequence is awaiting a tool call response. StateForking: The sequence is currently forking. StateClosed: The sequence interaction has finished [2]. The Seq struct uses a mutex to ensure thread safety [1]. When calling Generate, the implementation checks the sequence state and limits the generation stream [1]. If a generation is already in progress, the protocol design prevents concurrent generation streams within the same sequence object [1]. The state transitions, such as moving into StateGenerating, are handled by events (seq_state) received from the server [1][2]. When a tool call is required, the sequence may transition through states, and tool results are typically returned using the ToolReturn method [3]. If multiple operations are needed, they must be managed sequentially within the constraints of the single active generation stream allowed by the Seq object [1].
Citations:
- 1: https://github.com/chrisboulton/modelsocket-go/blob/main/seq.go
- 2: https://github.com/chrisboulton/modelsocket-go/blob/main/protocol.go
- 3: https://github.com/chrisboulton/modelsocket-go
🌐 Web query:
site:github.com/chrisboulton/modelsocket "only one Generate call can be active at a time"
💡 Result:
The error or limitation stating that only one Generate call can be active at a time refers to a concurrency restriction implemented in specific software libraries or SDKs, such as the modelsocket-go project [1]. This restriction means that a single object instance (or specific sequence) is designed to handle only one active text generation request or stream at any given time [1]. If you encounter this limitation, it indicates that your code is attempting to trigger a new generation process while a previous one on that same instance is still in progress [1]. To resolve this, you typically need to: 1. Wait for the existing Generate call to complete before initiating another. 2. Check your application logic to ensure you are not accidentally triggering multiple simultaneous requests from different goroutines or execution paths on the same instance. 3. If concurrent generation is required, instantiate separate objects or sequences if the architecture permits it [1].
Citations:
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- exact reference-client Generate implementation ---'
curl -fsSL https://github.com/ghraw/chrisboulton/modelsocket-go/main/seq.go |
sed -n '100,160p'
printf '%s\n' '--- all public repositories under the reference owner ---'
curl -fsSL 'https://github.com/ghapi/users/chrisboulton/repos?per_page=100' |
jq -r '.[].full_name' |
rg -i 'modelsocket|model.?socket' || trueRepository: mixlayer/modelsocket-rs
Length of output: 1496
Track generation options by command ID.
Seq::generate and Seq::send_cmd can issue generation commands concurrently, but on_tool_call receives the originating cid and reads one sequence-wide value. A later generation can overwrite the options before an earlier tool return. Store options by generation cid. Remove them on completion, errors, sequence close, and local send failure. Add an interleaving test with distinct options.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/client/seq.rs` around lines 40 - 42, Replace the sequence-wide
active_gen_opts storage with a command-ID-keyed map, and update Seq::generate,
Seq::send_cmd, and on_tool_call to read options using the originating cid so
concurrent generations remain isolated. Remove each cid’s entry on successful
completion, errors, sequence close, and local send failure, and add an
interleaving test using distinct generation options.
This will provide
parallel_tool_calls: true|false(OpenAI) like functionality for ModelSocket, where the number of tool calls in a single turn that the model will emit can be limited.Note this is different to
max_tool_callsin OpenAI compatible endpoints, which allows you to constrain the total number of tool calls that can be made over multiple turns. For that kind of thing, it's expected that clients (in our case, our gateway) handle that by tracking the number of tool calls over turns and then disabling tool calls when a final text return is expected.max_emitted_toolsis preserved between generations with tool results so the behaviour remains consistent for a single sequence.Summary by CodeRabbit
New Features
Bug Fixes
Release