Skip to content

Add configurable LLM request headers - #118

Merged
IceCodeNew merged 2 commits into
masterfrom
codex/llm-extra-headers
Jul 25, 2026
Merged

Add configurable LLM request headers#118
IceCodeNew merged 2 commits into
masterfrom
codex/llm-extra-headers

Conversation

@IceCodeNew

@IceCodeNew IceCodeNew commented Jul 24, 2026

Copy link
Copy Markdown
Owner

Adds strict JSON configuration for primary and fallback LLM request headers, forwarding non-empty mappings through the existing any-llm compatibility layer. Covers DeepSeek, OpenAI, and OpenRouter with transport-level tests and privacy-safe validation. Stacked on #115. Verification: prek passed; 1187 tests passed; line coverage 99.90%; branch coverage 99.66%.

Summary by CodeRabbit

  • New Features

    • Added support for custom HTTP headers for primary and fallback LLM providers.
    • Headers can be configured through environment variables and routed to the appropriate provider.
    • Supported headers are kept out of API-client logs.
  • Bug Fixes

    • Added validation for malformed headers, unsupported providers, duplicate names, and unsafe values.
    • Provider compatibility and fallback configuration requirements are now enforced.
  • Documentation

    • Updated configuration and design documentation with header setup details and limitations.

@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds environment-based custom HTTP headers for primary and fallback LLM providers. Headers are strictly parsed, stored immutably, validated against provider compatibility, passed through composition, and forwarded to any-llm clients as default_headers.

Changes

LLM header configuration

Layer / File(s) Summary
Header parsing and settings validation
weather_briefing/config/http_headers.py, weather_briefing/config/settings.py, weather_briefing/data/any_llm_compatibility.py, tests/test_http_headers.py, tests/test_config.py
Parses JSON headers into immutable mappings, rejects invalid or duplicate headers, validates provider compatibility, and enforces fallback configuration requirements.
Any-llm forwarding and compatibility coverage
weather_briefing/llm/any_llm.py, tests/test_any_llm_provider.py, tests/test_any_llm_compatibility.py
Canonicalizes providers, conditionally forwards default_headers, rejects unsupported providers, and verifies outgoing headers and log redaction.
Primary and fallback provider composition
weather_briefing/composition/providers.py, tests/test_cli.py
Passes distinct primary and fallback header mappings into their respective provider constructions.
Configuration documentation
env.example, docs/design.md, docs/notes.md
Documents header environment variables, forwarding behavior, and provider compatibility constraints.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Environment
  participant Settings.from_env
  participant llm_provider
  participant create_any_llm_provider
  participant AnyLLM
  Environment->>Settings.from_env: LLM_EXTRA_HEADERS and fallback headers
  Settings.from_env-->>llm_provider: validated immutable mappings
  llm_provider->>create_any_llm_provider: primary or fallback extra_headers
  create_any_llm_provider->>AnyLLM: default_headers client argument
  AnyLLM-->>llm_provider: configured provider adapter
Loading

Possibly related PRs

Suggested labels: 🕐 40+ Minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: configurable LLM request headers for primary and fallback providers.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/llm-extra-headers

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@IceCodeNew

Copy link
Copy Markdown
Owner Author

/agentic_review

@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.86%. Comparing base (834bdd6) to head (c71033d).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff            @@
##           master     #118    +/-   ##
========================================
  Coverage   99.85%   99.86%            
========================================
  Files         111      115     +4     
  Lines       11989    12154   +165     
  Branches      716      729    +13     
========================================
+ Hits        11972    12137   +165     
  Misses         12       12            
  Partials        5        5            

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@qodo-code-review

qodo-code-review Bot commented Jul 24, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 46 rules

Grey Divider


Remediation recommended

1. Headers sent to unsupported providers ✓ Resolved 🐞 Bug ☼ Reliability
Description
create_any_llm_provider() forwards non-empty extra_headers as AnyLLM.create(default_headers=...) for
all providers, even though the repo’s documented compatibility contract limits default_headers to
DeepSeek/OpenAI/OpenRouter. With another provider selected, configuring
LLM_EXTRA_HEADERS/LLM_FALLBACK_EXTRA_HEADERS can cause provider creation to fail
(provider-dependent), preventing LLM usage at startup/runtime.
Code

weather_briefing/llm/any_llm.py[R270-273]

+    client_options: dict[str, object] = {"api_key": api_key, "api_base": api_base}
+    if extra_headers:
+        client_options["default_headers"] = extra_headers
+    sdk_client = AnyLLM.create(provider, **client_options)
Relevance

⭐⭐⭐ High

Team often gates provider-specific config to avoid startup/runtime failures when feature is unused
or unsupported.

PR-#108
PR-#114

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The adapter always forwards default_headers when headers are configured, but repo documentation
states only DeepSeek/OpenAI/OpenRouter accept default_headers in any-llm 1.x, so forwarding to
other providers violates the documented compatibility boundary and can break initialization.

weather_briefing/llm/any_llm.py[256-273]
docs/notes.md[51-57]
docs/design.md[170-177]
weather_briefing/composition/providers.py[45-69]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`create_any_llm_provider()` conditionally adds `default_headers` whenever `extra_headers` is non-empty, but does not check whether the selected any-llm provider supports that option. Repo docs explicitly describe `default_headers` support as limited to OpenAI-compatible providers (DeepSeek/OpenAI/OpenRouter).

## Issue Context
If a deployment selects a non-OpenAI-compatible provider (e.g., another any-llm provider) and sets `LLM_EXTRA_HEADERS` (or fallback headers), the factory will still pass `default_headers` into `AnyLLM.create(...)`. Depending on the provider’s constructor/options handling, this may raise (e.g., unexpected kwarg) or otherwise fail provider initialization.

## Fix Focus Areas
- weather_briefing/llm/any_llm.py[256-273]
- weather_briefing/config/settings.py[157-185]

## Recommended fix
Choose one explicit policy and implement it consistently:
1) **Fail fast at config load (preferred)**: In `Settings.from_env()`, if `llm_extra_headers` is non-empty and `llm_provider` is not in `{deepseek, openai, openrouter}`, raise `ConfigurationError` stating headers are only supported for those providers. Do the same for fallback (`llm_fallback_provider`).

or

2) **Gate in the factory**: In `create_any_llm_provider()`, only add `default_headers` for supported providers; otherwise ignore and (optionally) log a safe warning that headers were ignored due to provider incompatibility.

Add/adjust tests to cover the unsupported-provider behavior (either rejection with `ConfigurationError` or safe ignore).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. LLM headers note lacks assumptions ✓ Resolved 📘 Rule violation ⌂ Architecture
Description
The newly added docs/notes.md entry describes the LLM extra-header behavior but does not
explicitly state an assumption/constraint with a justification and clearly checkable revisit
trigger(s) as required. This makes the note harder to revisit/audit as the integration surface
changes.
Code

docs/notes.md[R51-57]

+## LLM 自定义 header 复用 SDK client 配置
+
+any-llm 1.x 没有跨 provider 的统一 HTTP header 接口,但官方镜像预装的 DeepSeek、OpenAI 和 OpenRouter 共用 OpenAI-compatible client,并接受 `default_headers`。应用只在用户配置非空 header 映射时传入这个 client 参数,不创建或接管额外 HTTP transport,因此未使用该功能的其他 any-llm provider 保持原有初始化行为和资源生命周期。
+
+自定义 header 可以承载网关凭据,也可以覆盖 SDK 生成的 `Authorization`、`User-Agent` 等字段。配置入口因此只接受合法 ASCII HTTP 字段,并且日志和错误不能包含 header 名称或值;覆盖认证字段后的正确性由部署者负责。
+
+如果 any-llm 提供统一且带能力声明的 header 配置接口,就改用该接口并重新评估官方支持范围。在此之前,不能把当前透传能力描述为适用于所有 provider。
Relevance

⭐⭐⭐ High

Repo enforces strict, revisitable documentation contracts; similar doc-clarity/compliance edits were
accepted recently.

PR-#92
PR-#102
PR-#114

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance IDs 2141673 and 2141676 require that each touched docs/notes.md entry explicitly
documents assumptions/constraints, why they are acceptable now, and concrete revisit conditions. The
added section (lines 51-57) describes current behavior and a possible future change, but does not
explicitly structure the entry around assumptions and justification with clearly checkable triggers.

Rule 2141673: Constrain docs/notes.md entries to documented, revisitable assumptions
Rule 2141676: Document assumptions and review triggers in docs/notes.md entries
docs/notes.md[51-57]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The newly added `docs/notes.md` section about `LLM_EXTRA_HEADERS`/`LLM_FALLBACK_EXTRA_HEADERS` does not explicitly include (1) an assumption/constraint, (2) a justification for why it's acceptable, and (3) concrete revisit trigger(s), as required by the docs/notes rules.

## Issue Context
This entry is meant to document a revisitable assumption/constraint about reusing `default_headers` via OpenAI-compatible any-llm providers. To be compliant, the note should make the assumption(s) and revisit conditions explicit and checkable.

## Fix Focus Areas
- docs/notes.md[51-57]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

3. Settings imports LLM module ✓ Resolved 🐞 Bug ⚙ Maintainability ⭐ New
Description
weather_briefing.config.settings imports DEFAULT_HEADERS_PROVIDERS from weather_briefing.llm.any_llm
just to validate header-provider allowlists, coupling config loading to the LLM module import graph.
This can make settings import (and any config-only tooling/tests) sensitive to LLM-side import-time
side effects like prompt resource loading and adds unnecessary startup overhead.
Code

weather_briefing/config/settings.py[16]

+from ..llm.any_llm import DEFAULT_HEADERS_PROVIDERS
Relevance

⭐ Low

Similar “avoid heavy import to prevent side effects/coupling” suggestion was explicitly rejected in
PR #99.

PR-#99

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Settings now directly imports a constant from the LLM adapter module. The LLM adapter module imports
prompt constants, and the prompt module loads prompt files at import time; therefore importing
Settings can trigger LLM/prompt import-time side effects even when only configuration is needed.

weather_briefing/config/settings.py[14-17]
weather_briefing/llm/any_llm.py[14-18]
weather_briefing/data/prompts.py[6-12]
weather_briefing/data/prompts.py[24-25]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`weather_briefing/config/settings.py` imports `DEFAULT_HEADERS_PROVIDERS` from `weather_briefing/llm/any_llm.py` only to validate header provider names. This creates an avoidable dependency from config parsing into the LLM implementation module graph.

Because `weather_briefing.llm.any_llm` imports `weather_briefing.data.prompts`, and `data.prompts` loads packaged prompt files at import time, importing `Settings` can now execute prompt-loading code even in config-only contexts.

### Issue Context
The allowlist is a small static set of provider IDs; it does not need to live in (or be imported from) the LLM adapter module. Keeping it in the LLM module also forces config code to pay for (and potentially fail due to) unrelated import-time behavior.

### Fix Focus Areas
- weather_briefing/config/settings.py[16-16]
- weather_briefing/llm/any_llm.py[29-30]

### Suggested fix
1. Move the provider allowlist to a lightweight module with no prompt/resource imports (e.g. `weather_briefing/config/llm_constants.py` or `weather_briefing/llm/constants.py`).
2. Import that constant from both `config/settings.py` and `llm/any_llm.py`.
3. Ensure the new constants module has no imports that trigger prompt/resource loading.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. OpenAI creds mis-scoped ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
In env.example, OPENAI_API_KEY/OPENAI_BASE_URL are introduced as “OpenAI fallback credentials”, even
though those variables are also relevant when LLM_PROVIDER=openai (not just when OpenAI is the
fallback). This wording/placement can lead to missing or incorrect OpenAI credential configuration
when users follow env.example, potentially causing authentication/config failures at runtime.
Code

env.example[R17-19]

+# OpenAI fallback credentials. OPENAI_BASE_URL is optional.
+# OPENAI_API_KEY=replace-in-runtime-environment
+# OPENAI_BASE_URL=
Relevance

⭐⭐⭐ High

Team has accepted fixes correcting misleading env.example scoping/wording to match actual config
behavior.

PR-#110
PR-#90

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The added comment explicitly scopes OpenAI environment variables as fallback-only and places them
under the fallback provider/model example block, which can miscommunicate their applicability.

env.example[13-21]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`env.example` labels `OPENAI_API_KEY` / `OPENAI_BASE_URL` as “fallback credentials”, which is misleading because these variables apply whenever the OpenAI provider is used (primary or fallback).

## Issue Context
The OpenAI variables are currently documented directly under the fallback configuration block, which visually implies they are fallback-only.

## Fix Focus Areas
- env.example[17-19]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Previous review results

Review updated until commit c71033d

Results up to commit 0b96bcd ⚖️ Balanced


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)


Remediation recommended
1. LLM headers note lacks assumptions ✓ Resolved 📘 Rule violation ⌂ Architecture
Description
The newly added docs/notes.md entry describes the LLM extra-header behavior but does not
explicitly state an assumption/constraint with a justification and clearly checkable revisit
trigger(s) as required. This makes the note harder to revisit/audit as the integration surface
changes.
Code

docs/notes.md[R51-57]

+## LLM 自定义 header 复用 SDK client 配置
+
+any-llm 1.x 没有跨 provider 的统一 HTTP header 接口,但官方镜像预装的 DeepSeek、OpenAI 和 OpenRouter 共用 OpenAI-compatible client,并接受 `default_headers`。应用只在用户配置非空 header 映射时传入这个 client 参数,不创建或接管额外 HTTP transport,因此未使用该功能的其他 any-llm provider 保持原有初始化行为和资源生命周期。
+
+自定义 header 可以承载网关凭据,也可以覆盖 SDK 生成的 `Authorization`、`User-Agent` 等字段。配置入口因此只接受合法 ASCII HTTP 字段,并且日志和错误不能包含 header 名称或值;覆盖认证字段后的正确性由部署者负责。
+
+如果 any-llm 提供统一且带能力声明的 header 配置接口,就改用该接口并重新评估官方支持范围。在此之前,不能把当前透传能力描述为适用于所有 provider。
Relevance

⭐⭐⭐ High

Repo enforces strict, revisitable documentation contracts; similar doc-clarity/compliance edits were
accepted recently.

PR-#92
PR-#102
PR-#114

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance IDs 2141673 and 2141676 require that each touched docs/notes.md entry explicitly
documents assumptions/constraints, why they are acceptable now, and concrete revisit conditions. The
added section (lines 51-57) describes current behavior and a possible future change, but does not
explicitly structure the entry around assumptions and justification with clearly checkable triggers.

Rule 2141673: Constrain docs/notes.md entries to documented, revisitable assumptions
Rule 2141676: Document assumptions and review triggers in docs/notes.md entries
docs/notes.md[51-57]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The newly added `docs/notes.md` section about `LLM_EXTRA_HEADERS`/`LLM_FALLBACK_EXTRA_HEADERS` does not explicitly include (1) an assumption/constraint, (2) a justification for why it's acceptable, and (3) concrete revisit trigger(s), as required by the docs/notes rules.

## Issue Context
This entry is meant to document a revisitable assumption/constraint about reusing `default_headers` via OpenAI-compatible any-llm providers. To be compliant, the note should make the assumption(s) and revisit conditions explicit and checkable.

## Fix Focus Areas
- docs/notes.md[51-57]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Headers sent to unsupported providers ✓ Resolved 🐞 Bug ☼ Reliability
Description
create_any_llm_provider() forwards non-empty extra_headers as AnyLLM.create(default_headers=...) for
all providers, even though the repo’s documented compatibility contract limits default_headers to
DeepSeek/OpenAI/OpenRouter. With another provider selected, configuring
LLM_EXTRA_HEADERS/LLM_FALLBACK_EXTRA_HEADERS can cause provider creation to fail
(provider-dependent), preventing LLM usage at startup/runtime.
Code

weather_briefing/llm/any_llm.py[R270-273]

+    client_options: dict[str, object] = {"api_key": api_key, "api_base": api_base}
+    if extra_headers:
+        client_options["default_headers"] = extra_headers
+    sdk_client = AnyLLM.create(provider, **client_options)
Relevance

⭐⭐⭐ High

Team often gates provider-specific config to avoid startup/runtime failures when feature is unused
or unsupported.

PR-#108
PR-#114

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The adapter always forwards default_headers when headers are configured, but repo documentation
states only DeepSeek/OpenAI/OpenRouter accept default_headers in any-llm 1.x, so forwarding to
other providers violates the documented compatibility boundary and can break initialization.

weather_briefing/llm/any_llm.py[256-273]
docs/notes.md[51-57]
docs/design.md[170-177]
weather_briefing/composition/providers.py[45-69]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`create_any_llm_provider()` conditionally adds `default_headers` whenever `extra_headers` is non-empty, but does not check whether the selected any-llm provider supports that option. Repo docs explicitly describe `default_headers` support as limited to OpenAI-compatible providers (DeepSeek/OpenAI/OpenRouter).

## Issue Context
If a deployment selects a non-OpenAI-compatible provider (e.g., another any-llm provider) and sets `LLM_EXTRA_HEADERS` (or fallback headers), the factory will still pass `default_headers` into `AnyLLM.create(...)`. Depending on the provider’s constructor/options handling, this may raise (e.g., unexpected kwarg) or otherwise fail provider initialization.

## Fix Focus Areas
- weather_briefing/llm/any_llm.py[256-273]
- weather_briefing/config/settings.py[157-185]

## Recommended fix
Choose one explicit policy and implement it consistently:
1) **Fail fast at config load (preferred)**: In `Settings.from_env()`, if `llm_extra_headers` is non-empty and `llm_provider` is not in `{deepseek, openai, openrouter}`, raise `ConfigurationError` stating headers are only supported for those providers. Do the same for fallback (`llm_fallback_provider`).

or

2) **Gate in the factory**: In `create_any_llm_provider()`, only add `default_headers` for supported providers; otherwise ignore and (optionally) log a safe warning that headers were ignored due to provider incompatibility.

Add/adjust tests to cover the unsupported-provider behavior (either rejection with `ConfigurationError` or safe ignore).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Results up to commit 974df1c ⚖️ Balanced


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)


Informational
1. OpenAI creds mis-scoped ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
In env.example, OPENAI_API_KEY/OPENAI_BASE_URL are introduced as “OpenAI fallback credentials”, even
though those variables are also relevant when LLM_PROVIDER=openai (not just when OpenAI is the
fallback). This wording/placement can lead to missing or incorrect OpenAI credential configuration
when users follow env.example, potentially causing authentication/config failures at runtime.
Code

env.example[R17-19]

+# OpenAI fallback credentials. OPENAI_BASE_URL is optional.
+# OPENAI_API_KEY=replace-in-runtime-environment
+# OPENAI_BASE_URL=
Relevance

⭐⭐⭐ High

Team has accepted fixes correcting misleading env.example scoping/wording to match actual config
behavior.

PR-#110
PR-#90

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The added comment explicitly scopes OpenAI environment variables as fallback-only and places them
under the fallback provider/model example block, which can miscommunicate their applicability.

env.example[13-21]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`env.example` labels `OPENAI_API_KEY` / `OPENAI_BASE_URL` as “fallback credentials”, which is misleading because these variables apply whenever the OpenAI provider is used (primary or fallback).

## Issue Context
The OpenAI variables are currently documented directly under the fallback configuration block, which visually implies they are fallback-only.

## Fix Focus Areas
- env.example[17-19]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Qodo Logo

Comment thread docs/notes.md Outdated
Comment thread weather_briefing/llm/any_llm.py Outdated
@IceCodeNew

Copy link
Copy Markdown
Owner Author

/agentic_review

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 6a00c0f

@IceCodeNew

Copy link
Copy Markdown
Owner Author

/agentic_review

Comment thread env.example Outdated
@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 974df1c

@IceCodeNew
IceCodeNew force-pushed the codex/llm-extra-headers branch from 974df1c to 77db7f9 Compare July 25, 2026 05:52
@IceCodeNew

Copy link
Copy Markdown
Owner Author

/agentic_review

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 77db7f9

@IceCodeNew
IceCodeNew force-pushed the codex/configurable-llm-fallback branch from f500cfd to 77a2a24 Compare July 25, 2026 06:00
@IceCodeNew
IceCodeNew force-pushed the codex/llm-extra-headers branch 2 times, most recently from 00d9707 to ce273ef Compare July 25, 2026 06:04
Base automatically changed from codex/configurable-llm-fallback to master July 25, 2026 06:09
@IceCodeNew
IceCodeNew force-pushed the codex/llm-extra-headers branch 2 times, most recently from 69110da to 74fff44 Compare July 25, 2026 07:20
@IceCodeNew
IceCodeNew force-pushed the codex/llm-extra-headers branch from 74fff44 to c71033d Compare July 25, 2026 07:52
@IceCodeNew
IceCodeNew marked this pull request as ready for review July 25, 2026 07:54
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add configurable LLM request headers with strict JSON parsing

✨ Enhancement 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Parse LLM_EXTRA_HEADERS/LLM_FALLBACK_EXTRA_HEADERS into strict, immutable JSON header maps.
• Validate headers only for providers that support any-llm default_headers passthrough.
• Forward headers via AnyLLM.create() and verify transport + privacy with new tests.
Diagram

graph TD
  Env[/"Env vars"/] --> Parser["Header parser"] --> Settings["Settings loader"] --> Compose["Provider composer"] --> Factory["Any-LLM factory"] --> SDK{{"any-llm SDK"}} --> LLM{{"LLM HTTP API"}}
  Factory --> Compat["SDK compat list"]
  subgraph Legend
    direction LR
    _cfg[/Config/] ~~~ _mod[Module] ~~~ _ext{{External}}
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Inject headers at the HTTP transport layer
  • ➕ Avoids relying on provider constructors accepting default_headers
  • ➕ Would work uniformly across all providers, including currently blacklisted ones
  • ➖ More invasive; risks affecting retries/auth handling and any-llm internal assumptions
  • ➖ Harder to reason about per-provider behavior and may be brittle across SDK updates
2. Use any-llm stateless API with client_args on each request
  • ➕ Aligns directly with any-llm’s newer client_args passthrough semantics
  • ➕ Avoids maintaining a local compatibility blacklist for persistent clients
  • ➖ Conflicts with the app’s lifecycle requirement to hold/close provider clients
  • ➖ May increase overhead by recreating clients per request
3. Provider-specific header env vars (per provider)
  • ➕ Can tailor validation and supported headers per provider SDK
  • ➕ No need for a shared compatibility set
  • ➖ Configuration surface grows quickly and is harder to document
  • ➖ Harder to support primary/fallback symmetry and general any-llm providers

Recommendation: The current approach is the best tradeoff: strict, privacy-safe JSON parsing + an explicit compatibility gate, then forwarding via the existing any-llm provider client constructor path. It keeps the change localized (config + factory) while remaining explicit about pinned-SDK limitations and avoiding transport-level hacks.

Files changed (13) +403 / -5

Enhancement (5) +111 / -3
providers.pyPass primary/fallback extra headers into any-llm provider creation +2/-0

Pass primary/fallback extra headers into any-llm provider creation

• Threads Settings-derived extra header mappings into create_any_llm_provider for both primary and fallback providers during composition.

weather_briefing/composition/providers.py

http_headers.pyImplement strict JSON-to-immutable header parsing +49/-0

Implement strict JSON-to-immutable header parsing

• Adds headers_from_env() to parse optional JSON objects into MappingProxyType mappings while enforcing valid header names, unique case-insensitive keys, ASCII-only string values, and no control characters. Error messages are designed to be privacy-safe.

weather_briefing/config/http_headers.py

settings.pyAdd Settings fields and validation for extra headers +30/-0

Add Settings fields and validation for extra headers

• Introduces llm_extra_headers and llm_fallback_extra_headers fields, loads them via headers_from_env, enforces fallback dependency constraints, and rejects configuring headers for providers that do not support default_headers passthrough.

weather_briefing/config/settings.py

any_llm_compatibility.pyAdd pinned-SDK compatibility blacklist for default_headers +18/-0

Add pinned-SDK compatibility blacklist for default_headers

• Defines UNSUPPORTED_DEFAULT_HEADER_PROVIDERS as a frozenset used by both settings validation and provider factory guards.

weather_briefing/data/any_llm_compatibility.py

any_llm.pyForward configured headers into AnyLLM.create() with canonical provider validation +12/-3

Forward configured headers into AnyLLM.create() with canonical provider validation

• Extends create_any_llm_provider() with an extra_headers parameter, validates against the provider class canonical PROVIDER_NAME and the unsupported-provider set, and conditionally passes default_headers through to AnyLLM.create(). Also normalizes error reporting and stored provider identity to the canonical provider name.

weather_briefing/llm/any_llm.py

Tests (5) +278 / -2
test_any_llm_compatibility.pyPin-test unsupported default_headers providers against current SDK +27/-0

Pin-test unsupported default_headers providers against current SDK

• Adds a regression test asserting the UNSUPPORTED_DEFAULT_HEADER_PROVIDERS set matches the pinned any-llm SDK and that there are completion providers outside the unsupported set.

tests/test_any_llm_compatibility.py

test_any_llm_provider.pyTest header forwarding, validation, and privacy-safe logging +146/-2

Test header forwarding, validation, and privacy-safe logging

• Extends factory tests to ensure default_headers is only passed when configured, validates canonical provider name handling, and rejects unsupported providers. Adds transport-level coverage for OpenAI-compatible providers (DeepSeek/OpenAI/OpenRouter) to assert headers are sent while sensitive header data is not logged.

tests/test_any_llm_provider.py

test_cli.pyPlumb extra_headers through CLI/provider composition tests +8/-0

Plumb extra_headers through CLI/provider composition tests

• Updates CLI/provider composition tests to include empty extra_headers by default and verifies primary/fallback extra_headers are forwarded into provider creation calls.

tests/test_cli.py

test_config.pyAdd Settings env parsing tests for extra header JSON +57/-0

Add Settings env parsing tests for extra header JSON

• Adds settings coverage for loading primary and fallback extra headers as immutable mappings, rejecting unsupported providers, and requiring fallback provider/model when fallback headers are set.

tests/test_config.py

test_http_headers.pyUnit tests for strict header JSON validation and redaction +40/-0

Unit tests for strict header JSON validation and redaction

• Adds tests for JSON/object type validation, header-name/value rules (ASCII, no control chars, no duplicates), and ensures error messages never disclose header names/values.

tests/test_http_headers.py

Documentation (2) +8 / -0
design.mdDocument header env vars and any-llm default_headers wiring +2/-0

Document header env vars and any-llm default_headers wiring

• Adds design documentation describing how primary and fallback extra headers are parsed as immutable mappings and passed to any-llm provider clients via default_headers, including the constraint that unsupported providers are rejected and the parameter is omitted when empty.

docs/design.md

notes.mdExplain reuse of any-llm provider client args for headers +6/-0

Explain reuse of any-llm provider client args for headers

• Documents the relationship to any-llm’s client_args passthrough work and clarifies why the app uses the underlying provider client construction path to set default_headers. Notes the need to re-validate provider constructors when SDKs/providers change.

docs/notes.md

Other (1) +6 / -0
env.exampleAdd examples for LLM_EXTRA_HEADERS and fallback headers +6/-0

Add examples for LLM_EXTRA_HEADERS and fallback headers

• Introduces annotated examples for configuring primary and fallback header JSON objects, warning that values may contain credentials and are not logged.

env.example

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit c71033d

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@weather_briefing/config/settings.py`:
- Around line 311-319: The _validate_llm_headers_provider function checks the
raw provider spelling instead of the canonical identifier used by
create_any_llm_provider. Canonicalize the provider before comparing it with
UNSUPPORTED_DEFAULT_HEADER_PROVIDERS, and add primary coverage for an
unsupported uppercase provider plus fallback coverage for
LLM_FALLBACK_EXTRA_HEADERS while preserving the existing ConfigurationError
behavior.

In `@weather_briefing/llm/any_llm.py`:
- Around line 264-280: The provider/header validation in create_any_llm_provider
must use the same canonical provider representation as settings validation.
Update the relevant settings validation for LLM_PROVIDER and
LLM_FALLBACK_PROVIDER to resolve each value through AnyLLM.get_provider_class
and validate header support against PROVIDER_NAME, preserving existing behavior
for unsupported providers.
🪄 Autofix (Beta)

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 Plus

Run ID: c9d359b7-1067-439b-a491-91b876ce63db

📥 Commits

Reviewing files that changed from the base of the PR and between 834bdd6 and c71033d.

📒 Files selected for processing (13)
  • docs/design.md
  • docs/notes.md
  • env.example
  • tests/test_any_llm_compatibility.py
  • tests/test_any_llm_provider.py
  • tests/test_cli.py
  • tests/test_config.py
  • tests/test_http_headers.py
  • weather_briefing/composition/providers.py
  • weather_briefing/config/http_headers.py
  • weather_briefing/config/settings.py
  • weather_briefing/data/any_llm_compatibility.py
  • weather_briefing/llm/any_llm.py

Comment thread weather_briefing/config/settings.py
Comment thread weather_briefing/llm/any_llm.py
@IceCodeNew
IceCodeNew merged commit ebdd307 into master Jul 25, 2026
18 checks passed
@IceCodeNew
IceCodeNew deleted the codex/llm-extra-headers branch July 25, 2026 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant