Skip to content

Commit a088389

Browse files
refactor(llm): use typed access for extra_headers and document intended precedence in tests
- Replace getattr(..., 'extra_headers') with direct typed access - Add comments in tests to state intended behavior for each case Co-authored-by: openhands <openhands@all-hands.dev>
1 parent 568651a commit a088389

File tree

3 files changed

+15
-5
lines changed

3 files changed

+15
-5
lines changed

openhands-sdk/openhands/sdk/llm/options/chat_options.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ def select_chat_options(
2929
out["max_tokens"] = out.pop("max_completion_tokens")
3030

3131
# If user didn't set extra_headers, propagate from llm config
32-
if getattr(llm, "extra_headers", None) and "extra_headers" not in out:
32+
if llm.extra_headers is not None and "extra_headers" not in out:
3333
out["extra_headers"] = dict(llm.extra_headers)
3434

3535
# Reasoning-model quirks

openhands-sdk/openhands/sdk/llm/options/responses_options.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ def select_responses_options(
2626
out["tool_choice"] = "auto"
2727

2828
# If user didn't set extra_headers, propagate from llm config
29-
if getattr(llm, "extra_headers", None) and "extra_headers" not in out:
29+
if llm.extra_headers is not None and "extra_headers" not in out:
3030
out["extra_headers"] = dict(llm.extra_headers)
3131

3232
# Store defaults to False (stateless) unless explicitly provided

tests/sdk/llm/test_llm.py

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -356,9 +356,12 @@ def test_completion_merges_llm_extra_headers_with_extended_thinking_default(
356356
assert mock_completion.call_count == 1
357357
_, kwargs = mock_completion.call_args
358358
headers = kwargs.get("extra_headers") or {}
359-
# Should include default interleaved-thinking header
359+
# Intended behavior:
360+
# - No per-call headers provided.
361+
# - LLM.extra_headers should be used.
362+
# - Extended thinking default (anthropic-beta) should be merged in.
363+
# - Result keeps both the default and configured headers.
360364
assert headers.get("anthropic-beta") == "interleaved-thinking-2025-05-14"
361-
# Should preserve configured headers as well
362365
assert headers.get("X-Trace") == "1"
363366

364367

@@ -380,7 +383,10 @@ def test_completion_call_time_extra_headers_override_config_and_defaults(
380383
)
381384

382385
messages = [Message(role="user", content=[TextContent(text="Hi")])]
383-
# Call-time headers should take full precedence
386+
# Intended behavior:
387+
# - Per-call headers should replace any LLM.extra_headers.
388+
# - Extended thinking default should still be merged in.
389+
# - On conflicts, per-call headers win (anthropic-beta => custom-beta).
384390
call_headers = {"anthropic-beta": "custom-beta", "Header-Only": "H"}
385391
_ = llm.completion(messages=messages, extra_headers=call_headers)
386392

@@ -390,6 +396,7 @@ def test_completion_call_time_extra_headers_override_config_and_defaults(
390396
assert headers.get("anthropic-beta") == "custom-beta"
391397
assert headers.get("Header-Only") == "H"
392398
# LLM.config headers should not be merged when user specifies their own
399+
# (except defaults we explicitly add)
393400
assert "X-Trace" not in headers
394401

395402

@@ -427,6 +434,9 @@ def test_responses_call_time_extra_headers_override_config(mock_responses):
427434
)
428435

429436
messages = [Message(role="user", content=[TextContent(text="Hi")])]
437+
# Intended behavior:
438+
# - Per-call headers should replace any LLM.extra_headers for Responses path.
439+
# - No Anthropic default is currently added on the Responses path.
430440
call_headers = {"Header-Only": "H"}
431441
_ = llm.responses(messages=messages, extra_headers=call_headers)
432442

0 commit comments

Comments
 (0)