Skip to content

T045: iris-adapter-llm-openai#44

Open
likith1908 wants to merge 8 commits into
T044-llm-azure-openaifrom
T045-llm-openai
Open

T045: iris-adapter-llm-openai#44
likith1908 wants to merge 8 commits into
T044-llm-azure-openaifrom
T045-llm-openai

Conversation

@likith1908

@likith1908 likith1908 commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements T045 - the iris-adapter-llm-openai workspace package providing the OpenAI LLM adapter. Extends OpenAICompatProvider from T043 (iris_adapter_llm_shared). Stacked on #43.

  • packages/iris-adapters/llm-openai/src/iris_adapter_llm_openai/client.py: OpenAIProvider implementing id, _base_url(), _auth_headers(), and _model_for_hint(). Single endpoint https://api.openai.com/v1/chat/completions. Auth via Authorization: Bearer header. Two model slots (chat + extract) selected by model_hint field, no ContextVar needed (model goes in request body, not URL).
  • packages/iris-adapters/llm-openai/tests/test_unit.py: 23 unit tests covering all C-LLM-001..010 contract points using a mock httpx.AsyncClient.
  • packages/iris-adapters/llm-openai/tests/test_live.py: 3 live tests gated on IRIS_LLM_LIVE_OPENAI=1, verified against real OpenAI endpoint.
  • pyproject.toml: iris_adapter_llm_openai registered in importlinter root_packages, layers, independence, forbidden, mypy strict overrides, and coverage source.
  • tests/001-project-scaffold/: workspace, import-linter, and mypy scaffold tests updated for the renamed module.
  • .env.example: IRIS_LLM_OPENAI_API_KEY and optional model env vars documented.

Task reference

  • Task ID: T045
  • Workstream: 004-llm-adapter-set

Acceptance criteria

T045

  • OpenAIProvider.from_env() constructs from env vars; raises RuntimeError if IRIS_LLM_OPENAI_API_KEY is missing
  • Endpoint: https://api.openai.com/v1/chat/completions
  • Auth: Authorization: Bearer <key> header, no api-key header
  • model_hint="extraction" routes to model_extract; all other hints route to model_chat
  • All C-LLM-001..010 contract points covered by unit tests
  • Live tests pass against real OpenAI (IRIS_LLM_LIVE_OPENAI=1): 3/3 passed

PR checklist

  • Every acceptance criterion in the task's tasks.md entry is satisfied.
  • docs-ci workflow passes (markdown lint + tasks structural check).
  • No em-dashes introduced in new prose.
  • All internal links resolve.
  • No secrets, credentials, or personal names introduced.

Rebase note

This PR is stacked on #43. Once #43 merges, rebase onto main before merging this one.

Notes for the reviewer

Module rename: Scaffold was iris_llm_openai; renamed to iris_adapter_llm_openai to match the iris_adapter_* naming convention.

No ContextVar: Unlike Azure, OpenAI sends the model in the request body JSON field, so no per-request routing state is needed. _model_for_hint() simply returns the model string and _base_url() always returns the same constant URL.

Live tests: Verified against real OpenAI endpoint with gpt-4o-mini. All 3 tests passed.

…s/LlmParams gap fix

- Add packages/iris-adapters/llm-shared/ with OpenAICompatProvider abstract base
  class and RetryConfig/with_retry exponential backoff helper. Internal workspace
  package; consumed only by llm-azure-openai, llm-openai, and llm-local.
- Add OcrParams and LlmParams to AdaptersSchema in iris-config, closing a gap
  from WS003/WS004 where both were specified as "consumed from WS002" but never
  defined. All fields have safe defaults so existing product YAMLs remain valid.
- Regenerate docs/schemas/product.schema.json to include the new param blocks.
- Register iris_llm_shared in root_packages (pyproject.toml) and update workspace
  membership tests; exclude it from concrete-adapter contracts since it is a
  shared helper, not an independent adapter.
…C base

Address two required changes and one optional nit from Anmol's review on PR #41.

- retry.py: with_retry now returns (result, retry_count) where retry_count is the
  number of retries performed (0 means first attempt succeeded)
- openai_compat.py: unpack tuple from with_retry and set llm.retry_count on the
  OTEL span; inherit abc.ABC so missing abstract methods fail at instantiation
- pyproject.toml: add iris_adapter_llm_shared to [tool.coverage.run] source
- tests/test_unit.py: update with_retry tests to unpack and assert retry_count;
  add two span-attribute tests (retry_count 0 and 1)
- tests/conftest.py: span_exporter fixture that attaches to the active TracerProvider
Implements AzureOpenAIProvider via OpenAICompatProvider base class.
Two-deployment routing (chat/extract) via ContextVar; api-key auth;
api-version configurable via IRIS_LLM_AZURE_API_VERSION env var.
23 unit tests + 3 live tests (gated on IRIS_LLM_LIVE_AZURE=1).
@likith1908 likith1908 marked this pull request as ready for review June 25, 2026 04:37
@likith1908 likith1908 requested a review from anmolg1997 as a code owner June 25, 2026 04:37
Implements OpenAIProvider via OpenAICompatProvider base class.
Bearer auth; single api.openai.com/v1 endpoint; model routing via
model_hint (extraction vs chat). 23 unit tests covering C-LLM-001..010;
3 live tests gated on IRIS_LLM_LIVE_OPENAI=1 (untested pending API key).

@anmolg1997 anmolg1997 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed on the stack (own diff against T044-llm-azure-openai). Same verdict as the Azure adapter: a clean minimal subclass that proves out the base class.

  • Constant endpoint https://api.openai.com/v1/chat/completions and Authorization: Bearer header. Correct.
  • _model_for_hint routes extraction vs chat to the two configured models; everything else inherited from the tested base.
  • from_env / _require_env mirror the Azure adapter, no credential leak.

Coverage 94% with the single miss being the _require_env raise, consistent with the others. Error mapping, retry, and structured output are all inherited from llm-shared and already covered there, so there is nothing adapter-specific left untested here.

Stacked on #43, so it rebases up the chain as the zipper proceeds. Approving the content; will re-confirm on rebase onto main.

@likith1908 likith1908 force-pushed the T044-llm-azure-openai branch from f1f0d45 to 34184cf Compare June 25, 2026 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants