nimmo/forgejo-auto-pr-review
Watch
1
0
Fork
You've already forked forgejo-auto-pr-review
 0
Code Issues 5 Pull requests Projects Releases Packages 1 Activity Actions

feat: add OpenAI-compatible provider #14

Merged
nimmo merged 2 commits from issue-5-provider-interface into main 2026-07-20 20:06:35 +01:00
Conversation 1 Commits 2 Files changed 5 +97 -12
nimmo commented 2026-07-20 19:36:27 +01:00
Owner
Copy link

Implements #5.

  • introduce a structured provider request contract
  • isolate Ollama behind the contract
  • add an OpenAI-compatible chat-completions adapter with environment-only credentials
  • document provider configuration, privacy, and workflow secret setup

Validation: python3 -W error::ResourceWarning -m unittest discover -s tests (29 passing).

Implements #5. - introduce a structured provider request contract - isolate Ollama behind the contract - add an OpenAI-compatible chat-completions adapter with environment-only credentials - document provider configuration, privacy, and workflow secret setup Validation: `python3 -W error::ResourceWarning -m unittest discover -s tests` (29 passing).
feat: add OpenAI-compatible provider
All checks were successful
AI pull-request review / review (pull_request) Successful in 1m8s
Details
accf46e019
nimmo commented 2026-07-20 19:37:36 +01:00
Author
Owner
Copy link

Automated code review

Reviewed commit: 1695dbd81c3176b464de26666ca1156f08967092

Verdict: Requires further work

Resolve the blocking or important findings and investigate failed deterministic checks before merging.

Overall assessment

The pull request adds an OpenAI-compatible provider alongside the existing Ollama provider, abstracting the provider interface via a ReviewRequest dataclass and a unified review_with_provider() dispatcher. Documentation is updated in README.md to reflect the new model name (qwen3.6:35b-a3b-q8_0) and provider configuration options. All 29 unit tests pass.

Blocking findings

None.

Important findings

  • Missing validation of provider.base_url format for openai-compatible (reviewer.py:261; high confidence)
    base = provider.get("base_url", "").rstrip("/")
    if not base:
    raise ValueError("provider.base_url is required for openai-compatible")
    request = urllib.request.Request(base + "/chat/completions", ...)
    Impact: If base_url lacks a path prefix (e.g., /v1) and the provider requires it, the request will fail with an ambiguous HTTP error. The current validation only checks for non-empty base_url but does not enforce presence of required API version paths.
    Suggested fix: Add explicit validation that base_url ends with a known API path (e.g., '/v1') or raise a clear error if missing, matching the documented requirement in README.md.

Suggestions

None.

Tests and validation

  • unit-tests: success

Questions

  • Is there a plan to support dynamic provider discovery (e.g., via entry points) instead of hardcoding 'ollama' and 'openai-compatible'?
  • Should the OpenAI-compatible provider also support context_size and temperature overrides from config, or are those strictly Ollama-specific?

Review limitations

  • The code review is based only on supplied diffs and configuration files; runtime behavior of the new provider cannot be verified without integration tests.
  • No evidence was provided for how environment variable precedence works when both provider.api_key_env and hardcoded defaults like 'OPENAI_API_KEY' are present.
Review metadata
  • Reviewer: forgejo-ai-review v1
  • Model: qwen3-coder-next:q4_K_M
  • Guidance loaded: .forgejo/ai-review.yml, README.md
  • Commits considered: 2
  • Previous automated review: updated
  • Findings: 1
<!-- forgejo-ai-review:v1 --> ## Automated code review Reviewed commit: `1695dbd81c3176b464de26666ca1156f08967092` ## Verdict: Requires further work Resolve the blocking or important findings and investigate failed deterministic checks before merging. ### Overall assessment The pull request adds an OpenAI-compatible provider alongside the existing Ollama provider, abstracting the provider interface via a ReviewRequest dataclass and a unified review_with_provider() dispatcher. Documentation is updated in README.md to reflect the new model name (qwen3.6:35b-a3b-q8_0) and provider configuration options. All 29 unit tests pass. ### Blocking findings None. ### Important findings - **Missing validation of provider.base_url format for openai-compatible** (reviewer.py:261; high confidence) base = provider.get("base_url", "").rstrip("/") if not base: raise ValueError("provider.base_url is required for openai-compatible") request = urllib.request.Request(base + "/chat/completions", ...) Impact: If base_url lacks a path prefix (e.g., /v1) and the provider requires it, the request will fail with an ambiguous HTTP error. The current validation only checks for non-empty base_url but does not enforce presence of required API version paths. Suggested fix: Add explicit validation that base_url ends with a known API path (e.g., '/v1') or raise a clear error if missing, matching the documented requirement in README.md. ### Suggestions None. ### Tests and validation - unit-tests: **success** ### Questions - Is there a plan to support dynamic provider discovery (e.g., via entry points) instead of hardcoding 'ollama' and 'openai-compatible'? - Should the OpenAI-compatible provider also support `context_size` and `temperature` overrides from config, or are those strictly Ollama-specific? ### Review limitations - The code review is based only on supplied diffs and configuration files; runtime behavior of the new provider cannot be verified without integration tests. - No evidence was provided for how environment variable precedence works when both `provider.api_key_env` and hardcoded defaults like 'OPENAI_API_KEY' are present. <details><summary>Review metadata</summary> - Reviewer: forgejo-ai-review v1 - Model: qwen3-coder-next:q4_K_M - Guidance loaded: .forgejo/ai-review.yml, README.md - Commits considered: 2 - Previous automated review: updated - Findings: 1 </details>
config: use Qwen 3.6 for reviews
All checks were successful
AI pull-request review / review (pull_request) Successful in 1m0s
Details
1695dbd81c
nimmo merged commit 021d3eb7c2 into main 2026-07-20 20:06:35 +01:00
nimmo deleted branch issue-5-provider-interface 2026-07-20 20:06:36 +01:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
nimmo/forgejo-auto-pr-review!14
No description provided.