Skip to content

Fix llama provider alias in character chat#2586

Merged
rmusser01 merged 1 commit into
devfrom
codex/llama-provider-alias-character-chat
Jul 3, 2026
Merged

Fix llama provider alias in character chat#2586
rmusser01 merged 1 commit into
devfrom
codex/llama-provider-alias-character-chat

Conversation

@rmusser01

@rmusser01 rmusser01 commented Jul 3, 2026

Copy link
Copy Markdown
Owner

Summary

  • Canonicalizes the WebUI/catalog provider id llama to llama.cpp in ChatCompletionRequest validation.
  • Applies the same provider alias normalization in shared chat provider/model resolution so character chat /complete-v2 routes raw provider: "llama" through the llama.cpp adapter instead of the missing-credentials path.
  • Adds focused regression coverage plus Backlog task TASK-12118.

Verification

  • /Users/macbook-dev/Documents/GitHub/tldw_server2/.venv/bin/python -m pytest tldw_Server_API/tests/Chat_NEW/unit/test_chat_schemas.py tldw_Server_API/tests/Chat_NEW/unit/test_provider_model_resolution.py -q
  • /Users/macbook-dev/Documents/GitHub/tldw_server2/.venv/bin/python -m bandit -r tldw_Server_API/app/api/v1/schemas/chat_request_schemas.py tldw_Server_API/app/core/Chat/chat_service.py -f json -o /tmp/bandit_task_12118_pr.json
  • git diff --check
  • Live check: POST /api/v1/chats/{id}/complete-v2?scope_type=global with provider: "llama" returned 200 OK text/event-stream from the configured llama.cpp server.

Change summary

AI-authored PR. Per repo policy, a human-authored Change summary explaining what changed and why is required before merge.


Summary by cubic

Fixes character chat provider alias so llama resolves to llama.cpp, ensuring /api/v1/chats/{id}/complete-v2 uses the llama.cpp adapter instead of failing with missing credentials. Addresses TASK-12118.

  • Bug Fixes
    • Canonicalize WebUI provider llama to llama.cpp in ChatCompletionRequest validation.
    • Normalize provider in metrics parsing and shared provider/model resolution (including inline provider/model) for character chat.
    • Add regression tests for schema normalization and provider/model alias handling; document in backlog for TASK-12118.

Written for commit 75072c7. Summary will update on new commits.

Review in cubic

@coderabbitai

coderabbitai Bot commented Jul 3, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 04526c5b-c15c-422f-bf7c-55d35fca0091

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/llama-provider-alias-character-chat

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

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Canonicalize llama provider alias to llama.cpp for character chat

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

Grey Divider

AI Description

• Normalize catalog provider id llama to canonical llama.cpp during request validation.
• Apply the same normalization in shared provider/model resolution used by /complete-v2.
• Add regression tests plus backlog task record for TASK-12118.
Diagram

graph TD
A["WebUI / Client"] --> B["/complete-v2 API"] --> C["ChatCompletionRequest"] --> D["Provider alias normalize"] --> E["Provider/model resolver"] --> F["llama.cpp adapter"]
Loading
High-Level Assessment

The approach is appropriate: reuse the existing catalog-provider normalization utility and apply it at both schema validation and shared resolver boundaries to cover standard chat and character chat code paths. This minimizes behavioral drift and ensures metrics/provider dispatch stay consistent.

Files changed (5) +99 / -8

Bug fix (2) +13 / -8
chat_request_schemas.pyNormalize 'api_provider' catalog aliases during ChatCompletionRequest validation +4/-2

Normalize 'api_provider' catalog aliases during ChatCompletionRequest validation

• Routes provider id validation through normalize_catalog_provider_for_chat so 'llama' (and related aliases) are canonicalized to 'llama.cpp' before checking supported providers. Prevents valid WebUI catalog ids from being rejected or misrouted.

tldw_Server_API/app/api/v1/schemas/chat_request_schemas.py

chat_service.pyCanonicalize provider names in shared provider/model parsing and normalization +9/-6

Canonicalize provider names in shared provider/model parsing and normalization

• Applies normalize_catalog_provider_for_chat when parsing provider/model strings for metrics and when normalizing request provider/model selection. Ensures raw 'provider: llama' in character chat resolves to 'llama.cpp' and avoids the missing-credentials path.

tldw_Server_API/app/core/Chat/chat_service.py

Tests (2) +38 / -0
test_chat_schemas.pyAdd schema regression test for 'llama' → 'llama.cpp' canonicalization +12/-0

Add schema regression test for 'llama' → 'llama.cpp' canonicalization

• Adds a parametrized unit test asserting that both 'llama' and 'llama.cpp' inputs result in 'request.api_provider == 'llama.cpp'' after validation.

tldw_Server_API/tests/Chat_NEW/unit/test_chat_schemas.py

test_provider_model_resolution.pyAdd resolver regression test for catalog alias normalization +26/-0

Add resolver regression test for catalog alias normalization

• Adds a unit test that forces 'request.api_provider = 'llama'' and asserts resolve_provider_and_model normalizes metrics and selected provider to 'llama.cpp', with debug_info reflecting raw vs normalized values.

tldw_Server_API/tests/Chat_NEW/unit/test_provider_model_resolution.py

Documentation (1) +48 / -0
task-12118 - Fix-llama-provider-alias-rejection-in-character-chat-completions.mdAdd completed backlog task documenting the alias regression and fix +48/-0

Add completed backlog task documenting the alias regression and fix

• Introduces TASK-12118 describing the 'llama' vs 'llama.cpp' provider-id regression in character chat completions, acceptance criteria, verification steps, and final summary.

backlog/tasks/task-12118 - Fix-llama-provider-alias-rejection-in-character-chat-completions.md

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request resolves an issue where the WebUI catalog provider ID llama was not correctly canonicalized to llama.cpp in character chat completions, causing credential errors. The fix applies provider normalization across the chat request schema and the shared provider/model resolver, backed by new unit tests. The review feedback highlights that replacing the original .lower() and .strip() operations with the normalization function could introduce case-sensitivity issues, whitespace bugs, or None type errors, and suggests preserving these defensive string manipulations.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread tldw_Server_API/app/core/Chat/chat_service.py Outdated
Comment thread tldw_Server_API/app/core/Chat/chat_service.py
@qodo-code-review

qodo-code-review Bot commented Jul 3, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

Context used
✅ Compliance rules (platform): 74 rules

Grey Divider


Action required

1. Untyped test_llamacpp_provider_aliases... ✓ Resolved 📘 Rule violation ✧ Quality
Description
The new test methods test_llamacpp_provider_aliases_normalize_to_canonical_provider and
test_resolve_provider_and_model_normalizes_llamacpp_catalog_alias are missing required type hints,
including an untyped parameter and absent explicit return type annotations. This violates the
repository requirement for fully-annotated function signatures (including -> None for pytest
tests) and reduces the usefulness of static analysis.
Code

tldw_Server_API/tests/Chat_NEW/unit/test_chat_schemas.py[121]

+    def test_llamacpp_provider_aliases_normalize_to_canonical_provider(self, api_provider):
Evidence
PR Compliance ID 380614 requires explicit type hints on all function parameters and return values
for new/modified functions. In test_llamacpp_provider_aliases_normalize_to_canonical_provider, the
api_provider parameter is untyped and the function lacks an explicit -> None return annotation,
and in test_resolve_provider_and_model_normalizes_llamacpp_catalog_alias the function also has no
-> ... return annotation; these omissions directly contradict the compliance requirement.

Rule 380614: Require type hints on function parameters and return values
tldw_Server_API/tests/Chat_NEW/unit/test_chat_schemas.py[121-121]
tldw_Server_API/tests/Chat_NEW/unit/test_provider_model_resolution.py[50-50]

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/modified pytest test functions are missing required type hints in their signatures: one has an untyped parameter and both are missing explicit return type annotations. Update the function signatures to include complete type hints (including `-> None` where appropriate) to satisfy compliance and improve static analysis.

## Issue Context
PR Compliance ID 380614 requires explicit type hints for all function parameters and return values on newly added/modified functions. For pytest tests, the return type should typically be explicitly annotated as `-> None`.

## Fix Focus Areas
- tldw_Server_API/tests/Chat_NEW/unit/test_chat_schemas.py[119-130]
- tldw_Server_API/tests/Chat_NEW/unit/test_provider_model_resolution.py[49-73]

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


2. Long test function signature ✓ Resolved 📘 Rule violation ✧ Quality
Description
The added test function definition line is overly long and introduces a new PEP 8 line-length/style
issue. This can cause formatter/linter failures under standard project tooling.
Code

tldw_Server_API/tests/Chat_NEW/unit/test_chat_schemas.py[121]

+    def test_llamacpp_provider_aliases_normalize_to_canonical_provider(self, api_provider):
Evidence
PR Compliance ID 380616 requires PEP 8-compliant formatting for changed Python code. The newly added
def test_llamacpp_provider_aliases_normalize_to_canonical_provider(self, api_provider): line
introduces a style/line-length risk.

Rule 380616: Enforce PEP 8 for Python code formatting
tldw_Server_API/tests/Chat_NEW/unit/test_chat_schemas.py[121-121]

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 new test definition line exceeds typical project PEP 8 line-length limits due to the long function name.

## Issue Context
Compliance requires new/changed Python code to be PEP 8 compliant.

## Fix Focus Areas
- tldw_Server_API/tests/Chat_NEW/unit/test_chat_schemas.py[121-121]

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



Remediation recommended

3. Unreachable warning log ✓ Resolved 🐞 Bug ◔ Observability
Description
In chat_service.normalize_request_provider_and_model(), the _CHAT_NONCRITICAL_EXCEPTIONS handler
executes pass before logger.warning(...), making the warning unreachable and silently hiding
unexpected alias-resolution failures.
Code

tldw_Server_API/app/core/Chat/chat_service.py[R1439-1442]

        # Unexpected exceptions should be visible in production logs
        pass
        logger.warning(f"Unexpected error during model alias resolution: {_unexpected}")
-    provider = (api_provider or default_provider).lower()
+    provider = normalize_catalog_provider_for_chat(api_provider or default_provider)
Evidence
The pass statement immediately precedes logger.warning(...) in the exception handler, so
execution cannot reach the warning call, meaning unexpected exceptions in alias resolution will not
be logged.

tldw_Server_API/app/core/Chat/chat_service.py[1435-1443]

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

### Issue description
`normalize_request_provider_and_model()` intends to surface unexpected exceptions during model alias resolution, but the current `except _CHAT_NONCRITICAL_EXCEPTIONS` block has a `pass` before `logger.warning(...)`, so the warning never executes.

### Issue Context
This reduces production diagnosability when alias resolution fails unexpectedly.

### Fix Focus Areas
- tldw_Server_API/app/core/Chat/chat_service.py[1438-1442]

### Suggested fix
- Remove the stray `pass` line before `logger.warning(...)` (or move the warning above it).
- Consider logging with stack context (e.g., `logger.warning(..., exc_info=True)` if supported by the logger wrapper) while still swallowing the exception as intended.

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



Informational

4. Fragile provider-alias test ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
The new unit test mutates ChatCompletionRequest.api_provider after model construction to force a
raw "llama" value, which bypasses schema validation and will break if assignment validation is
enabled later (or if the model becomes frozen).
Code

tldw_Server_API/tests/Chat_NEW/unit/test_provider_model_resolution.py[R52-58]

+    request = ChatCompletionRequest(
+        api_provider="llama.cpp",
+        model="local-model",
+        messages=[{"role": "user", "content": "Hello"}],
+    )
+    request.api_provider = "llama"
+
Evidence
The test explicitly mutates api_provider post-construction, while the schema validator normalizes
api_provider during construction and the model config does not indicate assignment validation. The
production character-chat path uses a lightweight request object for resolution, not a Pydantic
request model mutation.

tldw_Server_API/tests/Chat_NEW/unit/test_provider_model_resolution.py[49-58]
tldw_Server_API/app/api/v1/schemas/chat_request_schemas.py[833-842]
tldw_Server_API/app/api/v1/schemas/chat_request_schemas.py[1028-1038]
tldw_Server_API/app/api/v1/endpoints/character_chat_sessions.py[5388-5437]

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

### Issue description
`test_resolve_provider_and_model_normalizes_llamacpp_catalog_alias()` sets `request.api_provider = "llama"` after creating a `ChatCompletionRequest`. This relies on Pydantic not validating assignments, making the test less representative and potentially brittle.

### Issue Context
The character chat `/complete-v2` path passes an unvalidated lightweight object into `resolve_provider_and_model` (not a `ChatCompletionRequest`). The test can mirror that without mutating a Pydantic model.

### Fix Focus Areas
- tldw_Server_API/tests/Chat_NEW/unit/test_provider_model_resolution.py[49-73]

### Suggested fix
- Replace the `ChatCompletionRequest` + post-init mutation with a minimal stub object (like the `_ProviderModelResolutionRequest` used in `character_chat_sessions.py`) that has `api_provider="llama"` and `model="local-model"`.
- Keep the assertions about `debug_info["raw"]["api_provider"] == "llama"` and normalized provider == `"llama.cpp"`.

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


Grey Divider

Qodo Logo

Comment thread tldw_Server_API/tests/Chat_NEW/unit/test_chat_schemas.py Outdated
Comment thread tldw_Server_API/tests/Chat_NEW/unit/test_chat_schemas.py Outdated
Comment thread tldw_Server_API/app/core/Chat/chat_service.py Outdated
@rmusser01 rmusser01 force-pushed the codex/llama-provider-alias-character-chat branch from db14254 to 75072c7 Compare July 3, 2026 16:39
@rmusser01

Copy link
Copy Markdown
Owner Author

Addressed the review feedback and rebased on latest dev at origin/dev commit 51372f5014.

Changes made in the amended PR commit:

  • Preserved defensive .strip().lower() normalization and None guards before catalog-provider canonicalization.
  • Removed the unreachable pass before the alias-resolution warning log.
  • Added explicit pytest type hints / -> None and wrapped the long test signature.
  • Replaced the brittle post-construction ChatCompletionRequest.api_provider mutation with a lightweight request stub for resolver coverage.

Verification after rebase:

  • git diff --check origin/dev...HEAD
  • python -m pytest tldw_Server_API/tests/Chat_NEW/unit/test_chat_schemas.py tldw_Server_API/tests/Chat_NEW/unit/test_provider_model_resolution.py -q -> 40 passed
  • python -m bandit -r tldw_Server_API/app/api/v1/schemas/chat_request_schemas.py tldw_Server_API/app/core/Chat/chat_service.py -f json -o /tmp/bandit_task_12118_review_after_rebase.json -> 0 findings

All inline review threads are resolved.

@rmusser01 rmusser01 merged commit 6c1c575 into dev Jul 3, 2026
16 of 17 checks passed
@rmusser01 rmusser01 deleted the codex/llama-provider-alias-character-chat branch July 3, 2026 16:42
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.

1 participant