Skip to content

Migrate model_router.rs LLM classifier onto api::OpenAiCompatClient (RC-CRATES-B 3/5) - #23

Merged
nuniesmith merged 1 commit into
mainfrom
claude/rc-crates-b-model-router-2026-05-20
May 20, 2026
Merged

Migrate model_router.rs LLM classifier onto api::OpenAiCompatClient (RC-CRATES-B 3/5)#23
nuniesmith merged 1 commit into
mainfrom
claude/rc-crates-b-model-router-2026-05-20

Conversation

@nuniesmith

@nuniesmith nuniesmith commented May 20, 2026

Copy link
Copy Markdown
Owner

Summary

Migrates ModelRouter::llm_classify from raw reqwest::Client against Ollama's native /api/chat to api::OpenAiCompatClient against Ollama's OpenAI-compatible /v1/chat/completions. Brings RC-CRATES-B from 2/5 → 3/5 done.

What changed

src/model_router.rs::llm_classify:

  • Removed: five inline serde structs (Req, Msg, Opts, Resp, RespMsg), the reqwest::Client::builder().timeout(...) plumbing, and the /api/chat URL construction.
  • Added: OpenAiCompatClient::new("", OpenAiCompatConfig::openai()).with_base_url(local_base_url + "/v1") (Ollama ignores the empty Bearer token), MessageRequest with max_tokens: 16 and the system prompt in system: Some(...), and a tokio::time::timeout(8s, ...) wrapper (the api crate doesn't expose a request-level timeout knob).
  • with_retry_policy(0, ...) to preserve the original one-shot semantics — the keyword classifier is the safety net, so transport retries here would just delay the inevitable fallback.

Behavioural caveat (documented in module header and TODO.md)

The old payload set options.temperature = 0.0 and options.num_predict = 16 on the Ollama-native call. api::MessageRequest carries max_tokens (kept as 16) but no temperature, so the classifier no longer pins determinism. The existing keyword fallback already catches misclassifications via the Unknown label arm, so the regression surfaces as a fallback rather than a wrong target.

Adding temperature to api::MessageRequest is the proper fix and is out of scope for this PR.

Note on the TODO description

The TODO had described this as a GET /api/tags health-check. It was actually a POST /api/chat LLM call (prompt classifier). The TODO entry has been corrected.

Test plan

  • cargo check -p api → clean
  • cargo check -p rustcode (blocked locally by the ort-sys CDN sandbox restriction; CI should verify)
  • Existing tests/test_grok_integration.rs::test_model_router_async_classification_matches_keyword_fallback still passes (it expects keyword fallback when Ollama is unreachable, which is unchanged behaviour)

Remaining RC-CRATES-B work

  • src/grok_reasoning.rs — uses xAI's /responses endpoint; needs api-crate extension
  • src/ollama_client.rs — depends on Ollama-native num_ctx and NDJSON streaming; needs api-crate extension or accepted feature loss

https://claude.ai/code/session_014DMg4gxA8VZv1MucmM7PV9


Generated by Claude Code

Summary by CodeRabbit

  • Refactor

    • Improved internal classification system reliability with explicit timeout and retry policies.
    • Updated migration documentation to reflect completed infrastructure improvements.
  • Documentation

    • Added determinism behavior notes regarding classification functionality limitations.

Review Change Stack

…RC-CRATES-B 3/5)

Switches `ModelRouter::llm_classify` from a raw `reqwest::Client` POST against
Ollama's native `/api/chat` endpoint to `api::OpenAiCompatClient` pointing at
the OpenAI-compatible `/v1/chat/completions` endpoint Ollama exposes.

What goes away:
- Five inline serde structs (`Req`, `Msg`, `Opts`, `Resp`, `RespMsg`)
- The inline `reqwest::Client::builder().timeout(...).build()` plumbing
- The `format!("{base}/api/chat")` URL construction

What replaces it:
- `OpenAiCompatClient::new("", OpenAiCompatConfig::openai()).with_base_url(...)`
  with `local_base_url + "/v1"` (Ollama ignores the empty Bearer token)
- `.with_retry_policy(0, ...)` to preserve one-shot semantics (the keyword
  classifier is the safety net, so transport retries here would just delay
  the inevitable fallback)
- `MessageRequest { max_tokens: 16, system: Some(CLASSIFY_SYSTEM), ... }`
- `tokio::time::timeout(Duration::from_secs(8), ...)` to replace the per-client
  reqwest timeout (the api crate doesn't expose a request-level timeout knob)

Behavioural caveat (documented in the module header and TODO.md):
the old payload set `options.temperature = 0.0` and `options.num_predict = 16`
on the Ollama-native call. `MessageRequest` carries `max_tokens` (kept) but
no `temperature`. The classifier therefore no longer pins determinism — the
keyword fallback already catches misclassifications via the `Unknown label`
arm, so accuracy degradation surfaces as a fallback rather than a wrong target.
Adding `temperature` to `api::MessageRequest` is the proper fix and is out
of scope here.

RC-CRATES-B is now 3/5 done. Remaining (`grok_reasoning.rs`,
`ollama_client.rs`) both need additional api-crate capabilities first
(xAI `/responses` endpoint and Ollama-native `num_ctx` / NDJSON streaming).

https://claude.ai/code/session_014DMg4gxA8VZv1MucmM7PV9
@coderabbitai

coderabbitai Bot commented May 20, 2026

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b150289a-77e7-425a-afb5-67d6aac14128

📥 Commits

Reviewing files that changed from the base of the PR and between ac70763 and 2ce17a2.

📒 Files selected for processing (2)
  • TODO.md
  • src/model_router.rs

📝 Walkthrough

Walkthrough

This PR migrates the Ollama task classifier in src/model_router.rs to use the OpenAI-compatible /v1/chat/completions endpoint via api::OpenAiCompatClient, replacing the native Ollama client call with an explicit timeout wrapper and one-shot retry policy. The refactored logic builds OpenAI-style message requests and parses text content blocks from the response. The project TODO checklist was updated to document the migration and reflect 3 of 5 completed migrations.

Changes

Ollama OpenAI-compatible endpoint migration

Layer / File(s) Summary
Classifier refactor to OpenAI-compatible endpoint
src/model_router.rs
The llm_classify function now constructs an OpenAiCompatClient with zero-retry policy and an 8-second timeout, sends a MessageRequest to Ollama's OpenAI-compatible chat endpoint with stream: false and max_tokens: 16, and parses OutputContentBlock::Text segments to derive the final label.
Migration tracking documentation
TODO.md
Migration notes document the endpoint switch, removed inline structs, explicit timeout/retry behavior, and the temperature determinism caveat; progress counter updated from 2/5 to 3/5 completed migrations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A classifier hops to OpenAI's way,
Ollama's compat endpoint saves the day,
No more native calls, just timeout and grace,
One-shot retry keeps up the pace! 🚀

✨ 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 claude/rc-crates-b-model-router-2026-05-20

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 and usage tips.

@nuniesmith
nuniesmith marked this pull request as ready for review May 20, 2026 15:16
@nuniesmith
nuniesmith merged commit 6b42bb9 into main May 20, 2026
1 of 2 checks passed
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