diff --git a/src/comfydv/_llm/chat.py b/src/comfydv/_llm/chat.py index b45283c..dca8bef 100644 --- a/src/comfydv/_llm/chat.py +++ b/src/comfydv/_llm/chat.py @@ -136,7 +136,26 @@ async def chat_structured( # top-level "seed" param, which works against both Ollama's and # llama-server's OpenAI-compatible endpoints) and give it a beat # via RETRY_BACKOFF_SECS in case it's still finishing loading. - attempt_settings["seed"] = next_seed(options, attempt) + seed = next_seed(options, attempt) + attempt_settings["seed"] = seed + if "extra_body" in attempt_settings: + # beacon-reviewer caught this: if a caller pinned options["seed"], + # it's also sitting in extra_body.options.seed (the Ollama-native + # passthrough). Left untouched, a backend that honors that nested + # field over the top-level OpenAI "seed" above would keep sending + # the same old seed on every retry — silently defeating this fix + # for exactly the pinned-seed case. Copy rather than mutate in + # place: extra_body/options here are the caller's own dicts, + # shared across every attempt (and possibly other calls). + # ModelSettings declares extra_body as `object` (it's an + # opaque passthrough field), so a plain dict() call on it + # doesn't type-check — cast first, this module always builds + # it as a dict (see model_settings above). + extra_body = dict(cast(dict, attempt_settings["extra_body"])) + nested_options = dict(extra_body.get("options") or {}) + nested_options["seed"] = seed + extra_body["options"] = nested_options + attempt_settings["extra_body"] = extra_body try: result = await agent.run( prompt, diff --git a/tests/test_llm_chat_structured.py b/tests/test_llm_chat_structured.py index b9f2ce2..ea57ed6 100644 --- a/tests/test_llm_chat_structured.py +++ b/tests/test_llm_chat_structured.py @@ -244,7 +244,33 @@ def test_chat_structured_retry_seed_starts_from_pinned_base(monkeypatch): assert fake.calls[0][2] == {"extra_body": {"options": {"seed": 42}}} assert fake.calls[1][2]["seed"] == 43 # base(42) + (attempt 2 - 1) - assert fake.calls[1][2]["extra_body"] == {"options": {"seed": 42}} + # The nested Ollama-native options.seed must track the same retry seed as + # the top-level one — a backend that honors the nested field over the + # top-level OpenAI "seed" must not keep seeing the stale pinned value. + assert fake.calls[1][2]["extra_body"] == {"options": {"seed": 43}} + + +def test_chat_structured_retry_does_not_mutate_callers_options_dict(monkeypatch): + """Regression guard for the fix above: syncing the nested seed must copy, + not mutate, the caller's options dict — otherwise a second call reusing + the same options object would start from the wrong base seed.""" + bad = ValidationError.from_exception_data("Widget", []) + fake = _FakeAgent([bad, _Widget(name="c", count=3)]) + monkeypatch.setattr(chat_mod, "_build_agent", lambda **kw: fake) + + caller_options = {"seed": 42} + _run_async( + chat_mod.chat_structured( + base_url="http://localhost:11434/v1", + model="llama3", + messages=_messages(), + schema=_Widget, + options=caller_options, + max_retries=2, + ) + ) + + assert caller_options == {"seed": 42} def test_chat_structured_retry_sleeps_between_attempts(monkeypatch):