refactor(routing): rename hint:reasoning-quick → hint:chat#1801
Conversation
📝 WalkthroughWalkthroughThis PR introduces a new ChangesReasoning-quick model tier integration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/openhuman/routing/provider.rs (1)
90-105: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd explicit trace/debug log for remote model normalization.
Line 90-105 adds new
hint:chatnormalization but does not emit a direct normalization log. Add onedebug/traceevent with requested hint/category/resolved model to make routing regressions grepable.🧭 Suggested patch
fn resolve_remote_model(&self, requested_model: &str, category: TaskCategory) -> String { if category != TaskCategory::Heavy { return self.remote_fallback_model.clone(); } - // Keep remote model naming aligned with backend modelRegistry. - match requested_model.strip_prefix("hint:") { + // Keep remote model naming aligned with backend modelRegistry. + let resolved = match requested_model.strip_prefix("hint:") { Some("reasoning") => MODEL_REASONING_V1.to_string(), // Orchestrator's low-TTFT chat tier — Kimi K2.6 Turbo on the // backend's `reasoning-quick-v1`. Backend support added in // tinyhumansai/backend#760. Some("chat") => MODEL_REASONING_QUICK_V1.to_string(), Some("agentic") => MODEL_AGENTIC_V1.to_string(), Some("coding") => MODEL_CODING_V1.to_string(), _ => requested_model.to_string(), - } + }; + tracing::debug!( + requested_model, + category = category.as_str(), + resolved_model = resolved.as_str(), + "[routing] normalized remote model" + ); + resolved }As per coding guidelines, “In Rust, use
log/tracingatdebugortracelevel for development-oriented diagnostics on new/changed flows, including logs at … branch decisions …”.🤖 Prompt for 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. In `@src/openhuman/routing/provider.rs` around lines 90 - 105, The resolve_remote_model function currently normalizes hint-prefixed models but lacks an observable log; add a tracing debug/trace call inside resolve_remote_model that logs the input requested_model, the TaskCategory (category), and the final resolved model string (the value returned or remote_fallback_model) so routing changes are grepable — place the log right before each return (or once before the final return) and use the existing tracing/log crate at debug/trace level; reference resolve_remote_model, requested_model, category, remote_fallback_model, and the constants like MODEL_REASONING_QUICK_V1 in the log message.
🧹 Nitpick comments (1)
src/openhuman/providers/router.rs (1)
9-13: ⚡ Quick winUse the canonical model constant in tier mapping.
Line 12 hardcodes
"reasoning-quick-v1"; preferMODEL_REASONING_QUICK_V1to prevent drift between routing layers.♻️ Suggested patch
+use crate::openhuman::config::MODEL_REASONING_QUICK_V1; + fn openhuman_tier_to_hint(model: &str) -> Option<&'static str> { match model { "reasoning-v1" => Some("reasoning"), - "reasoning-quick-v1" => Some("chat"), + MODEL_REASONING_QUICK_V1 => Some("chat"), "agentic-v1" => Some("agentic"), "coding-v1" => Some("coding"), "summarization-v1" => Some("summarization"),🤖 Prompt for 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. In `@src/openhuman/providers/router.rs` around lines 9 - 13, The match in openhuman_tier_to_hint uses a hardcoded "reasoning-quick-v1"; replace that literal with the canonical constant MODEL_REASONING_QUICK_V1 in the match arm inside the openhuman_tier_to_hint function, and ensure the constant is in scope (import or fully qualify it) so the mapping uses the single source-of-truth symbol instead of a string literal.
🤖 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.
Outside diff comments:
In `@src/openhuman/routing/provider.rs`:
- Around line 90-105: The resolve_remote_model function currently normalizes
hint-prefixed models but lacks an observable log; add a tracing debug/trace call
inside resolve_remote_model that logs the input requested_model, the
TaskCategory (category), and the final resolved model string (the value returned
or remote_fallback_model) so routing changes are grepable — place the log right
before each return (or once before the final return) and use the existing
tracing/log crate at debug/trace level; reference resolve_remote_model,
requested_model, category, remote_fallback_model, and the constants like
MODEL_REASONING_QUICK_V1 in the log message.
---
Nitpick comments:
In `@src/openhuman/providers/router.rs`:
- Around line 9-13: The match in openhuman_tier_to_hint uses a hardcoded
"reasoning-quick-v1"; replace that literal with the canonical constant
MODEL_REASONING_QUICK_V1 in the match arm inside the openhuman_tier_to_hint
function, and ensure the constant is in scope (import or fully qualify it) so
the mapping uses the single source-of-truth symbol instead of a string literal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4c6b6a54-f946-4e78-94d5-f52e1a657d93
📒 Files selected for processing (8)
src/openhuman/agent/agents/loader.rssrc/openhuman/agent/agents/orchestrator/agent.tomlsrc/openhuman/config/mod.rssrc/openhuman/config/schema/types.rssrc/openhuman/providers/router.rssrc/openhuman/routing/policy.rssrc/openhuman/routing/provider.rssrc/openhuman/routing/provider_tests.rs
…trator Builds on tinyhumansai#1761 (which introduced the `reasoning-quick-v1` tier and wired it up as `hint:reasoning-quick`). Renames the hint slot to `hint:chat` because the orchestrator's actual job — front-line conversational TTFT — is described better by "chat" than by the backend model id leaking into the hint namespace. Same end-to-end behavior as tinyhumansai#1761: hint:chat ↔ reasoning-quick-v1 (Kimi K2.6 Turbo on Fireworks). - `agent/agents/orchestrator/agent.toml`: `hint = "chat"`. - `agent/agents/loader.rs`: test asserts `h == "chat"`. - `providers/router.rs`: `openhuman_tier_to_hint("reasoning-quick-v1")` now returns `Some("chat")` so custom `model_routes` round-trip. - `routing/provider.rs:resolve_remote_model`: `Some("chat") => MODEL_REASONING_QUICK_V1` instead of `Some("reasoning-quick")`. - `routing/policy.rs`: rustdoc documents `hint:chat` as Heavy (always remote — local model can't meet the TTFT budget). - `routing/provider_tests.rs`: new regression `regression_chat_hint_routes_remote_as_reasoning_quick_v1`. - `config/schema/types.rs`: docstring on `MODEL_REASONING_QUICK_V1` references `hint:chat`. Tests: `regression_chat_hint_routes_remote_as_reasoning_quick_v1`, `orchestrator_has_chat_hint_and_named_tools` — both pass.
2be851c to
b54ee2b
Compare
|
Note on red CI checks:
This PR only touches |
Summary
Follow-up to #1761 — renames the orchestrator's hint slot from
reasoning-quicktochat. Same end-to-end behavior; same backend model (reasoning-quick-v1/ Kimi K2.6 Turbo via backend#760).agent/agents/orchestrator/agent.toml:hint = "chat".agent/agents/loader.rs: loader test assertsh == "chat".providers/router.rs:openhuman_tier_to_hint("reasoning-quick-v1") = Some("chat")for custommodel_routesround-trip.routing/provider.rs:resolve_remote_model:Some("chat") => MODEL_REASONING_QUICK_V1.routing/policy.rs: rustdoc documentshint:chatas Heavy (always remote).routing/provider_tests.rs: new regressionregression_chat_hint_routes_remote_as_reasoning_quick_v1.config/schema/types.rs: docstring onMODEL_REASONING_QUICK_V1referenceshint:chat.Problem
hint:reasoning-quick(introduced in #1761) lets the backend model id leak into the hint namespace. The orchestrator's actual job is front-line conversational TTFT, which is named better bychat. Hints should describe the workload, not the backend tier — the same wayhint:reasoningdoesn't sayhint:deepseek-v4-pro.Solution
Rename the single hint slot. Backend mapping (
hint:chat→reasoning-quick-v1) lives entirely client-side inrouting::provider::resolve_remote_model, exactly where the other hint-to-model translations live. No backend changes needed — the constantMODEL_REASONING_QUICK_V1and the backend model id are unchanged.Submission Checklist
diff-cover) meet the gate enforced by.github/workflows/coverage.yml.docs/TEST-COVERAGE-MATRIX.mdreflect this change## Relateddocs/RELEASE-MANUAL-SMOKE.md)Closes #NNNin the## RelatedsectionImpact
reasoning-quick-v1on the backend. Only the hint string inagent.tomland the route-table key change.model_routesneed achatentry instead ofreasoning-quick. Without one, the request falls through to the default provider model (same fallback as any other unmapped hint).Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
Validation Blocked
Behavior Changes
Parity Contract
reasoning-quick-v1) and constant (MODEL_REASONING_QUICK_V1) are unchanged. Cost rows and identity-cost defaults from feat(orchestrator): use reasoning-quick-v1 for low-latency chat #1761 stay put.routing::policy::classifystill bucketshint:chatasHeavy;routing::provider::resolve_remote_modeltranslateshint:chatto the same backend model id feat(orchestrator): use reasoning-quick-v1 for low-latency chat #1761 used. Regression test confirms the end-to-end remote-model string.Duplicate / Superseded PR Handling