fix: keep chat_structured retry's nested seed in sync with the top-level one
beacon-reviewer caught a real gap: when a caller pins options["seed"], the retry loop set the new seed on ModelSettings' top-level "seed" field but left the same request's extra_body.options.seed (the Ollama-native passthrough) at the original pinned value. A backend that honors the nested field over the top-level OpenAI one would keep sending the identical effective seed on every retry, silently defeating this fix for exactly the pinned-seed case it needs to cover. Now both fields are kept in sync on every retry attempt, built via fresh copies rather than mutating the caller's options dict in place (that dict is shared across every attempt, and possibly across other calls). Added a regression test asserting the caller's options dict is untouched after a multi-attempt retry. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0132ojafeazQ3ephcBejEWFj
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
09a173e552
commit
1dcef77a2b
@@ -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,
|
||||
|
||||
@@ -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):
|
||||
|
||||
Reference in New Issue
Block a user