feat: add routing classification in observation mode#8
Conversation
- classifyWithRouter called in llm_input hook (classify but don't mutate model) - Routing tier, confidence, model, and signals logged per request - Metrics adapter wired to show routing data in dashboard - routing config loaded from slimclaw.config.json - Close #3 (reasoning tier already present in config)
![gitsniff[bot]](https://avatars.githubusercontent.com/in/2666815?s=60&v=4){kind=small}
There was a problem hiding this comment.
Score: Excellent ✅
This PR integrates ClawRouter classification into the llm_input hook in observation mode, allowing routing recommendations to be logged and displayed in the dashboard without altering the actual model used for inference. It updates the slimclaw.config.json to include routing configuration and modifies src/index.ts to process and store routing data within request metrics.
4 findings posted as inline comments below.
Review completed in 36s | Basic Plan | Gemini 2.5 Flash
| enabled: (rawConfig.routing as any)?.enabled || false, | ||
| tiers: (rawConfig.routing as any)?.tiers || {}, | ||
| minConfidence: (rawConfig.routing as any)?.minConfidence || 0.4, | ||
| pinnedModels: (rawConfig.routing as any)?.pinnedModels || [], |
There was a problem hiding this comment.
🟡 Warning
The configuration for routing (rawConfig.routing as any) is being accessed with any type assertion and default fallbacks. While this works, it bypasses Zod validation for the routing configuration. This can lead to subtle bugs if the slimclaw.config.json file is malformed or if new configuration options are added that don't match the expected structure, as it won't be caught by the schema.
Fix: Extend the slimclawConfigSchema to include the routing property with its full schema (enabled, tiers, minConfidence, pinnedModels). Then, parse the rawConfig using Zod to ensure type safety and proper validation. This will provide compile-time type checking and runtime validation, making the configuration more robust.
| enabled: (rawConfig.routing as any)?.enabled || false, | |
| tiers: (rawConfig.routing as any)?.tiers || {}, | |
| minConfidence: (rawConfig.routing as any)?.minConfidence || 0.4, | |
| pinnedModels: (rawConfig.routing as any)?.pinnedModels || [], | |
| enabled: rawConfig.routing?.enabled ?? false, | |
| tiers: rawConfig.routing?.tiers ?? {}, | |
| minConfidence: rawConfig.routing?.minConfidence ?? 0.4, | |
| pinnedModels: rawConfig.routing?.pinnedModels ?? [], |
| messages.push({ role: 'user', content: prompt }); | ||
| } | ||
|
|
||
| const classification = classifyWithRouter(messages, { originalModel: (event as any).model }); |
There was a problem hiding this comment.
ℹ️ Info
The classifyWithRouter function can accept a config object to customize its behavior, such as confidence thresholds or pinned models. Currently, it's called without this configuration, meaning the HybridRouter will use its default settings. This might lead to routing decisions that don't align with the user's configured preferences in slimclaw.config.json.
Fix: Pass pluginConfig.routing as the second argument to classifyWithRouter. This will ensure that the routing logic respects the user's custom settings for minConfidence, pinnedModels, etc., defined in their slimclaw.config.json.
| const classification = classifyWithRouter(messages, { originalModel: (event as any).model }); | |
| const classification = classifyWithRouter(messages, { originalModel: (event as any).model, ...pluginConfig.routing }); |
| summarizationMethod: 'none' as const, | ||
| classificationTier: 'complex' as const, | ||
| classificationConfidence: 1, | ||
| classificationTier: (request.routingTier as any) || 'complex', |
There was a problem hiding this comment.
🟡 Warning
The expression (request.routingTier as any) uses a type assertion to any. This bypasses TypeScript's type checking and can hide potential type mismatches. While routingTier is expected to be a ComplexityTier, directly asserting it to any then back to ComplexityTier via assignment is an unsafe practice.
Fix: Ensure that request.routingTier is explicitly typed as ComplexityTier where it's defined in the RequestMetric interface. This will remove the need for as any and provide type safety.
| classificationTier: (request.routingTier as any) || 'complex', | |
| classificationTier: request.routingTier || 'complex', |
| try { | ||
| const messages: Message[] = ((historyMessages as any[]) || []).map((msg: any) => ({ | ||
| role: msg.role || 'user', | ||
| content: typeof msg.content === 'string' ? msg.content : JSON.stringify(msg.content || ''), |
There was a problem hiding this comment.
🟡 Warning
When msg.content is not a string, it's converted to a JSON string using JSON.stringify. If msg.content is a complex object that doesn't stringify well (e.g., contains circular references or functions), or if it's already a JSON string that needs to be parsed, this could lead to unexpected or incorrect content being sent for classification. The Message interface expects string | ContentBlock[] for content.
Fix: Consider processing msg.content that is not a string more carefully. If it's expected to be ContentBlock[], handle it by extracting text content similar to extractTextFromMessages in clawrouter-classifier.ts. If it's a generic object, ensure it's intended to be stringified for classification.
| content: typeof msg.content === 'string' ? msg.content : JSON.stringify(msg.content || ''), | |
| content: typeof msg.content === 'string' ? msg.content : (Array.isArray(msg.content) ? extractTextFromMessagesFromContentBlocks(msg.content) : JSON.stringify(msg.content || '')), // Assuming extractTextFromMessagesFromContentBlocks is a new helper function |
- Replace `as any` routing config access with typed Record<string, unknown> - Type routingTier as ComplexityTier instead of string (remove `as any`) - Handle content blocks properly: extract text from arrays instead of JSON.stringify - Remove redundant null guard in classification message loop Closes #12
- Replace `as any` routing config access with typed Record<string, unknown> - Type routingTier as ComplexityTier instead of string (remove `as any`) - Handle content blocks properly: extract text from arrays instead of JSON.stringify - Remove redundant null guard in classification message loop Closes #12
1 participant
What
Enables ClawRouter classification in the
llm_inputhook so routing recommendations are visible in logs and dashboard — without mutating the actual model (observation mode only).Why
After merging ClawRouter integration (#2), the routing pipeline wasn't being invoked because the plugin's
llm_inputhook only tracked token metrics. TheinferenceOptimizermiddleware (which includes routing) requires active mutation hooks that OpenClaw doesn't yet support.This bridges the gap by calling
classifyWithRouterdirectly in the hook for observability.Changes
src/index.ts: CallclassifyWithRouterinllm_inputhook, log routing recommendation (tier, confidence, model, signals), store in request metricssrc/index.ts: Loadroutingconfig fromslimclaw.config.jsonsrc/index.ts: Wire routing data intoSlimClawMetricsAdapterso dashboard shows tier distributionslimclaw.config.json: Add routing config (enabled, tiers, minConfidence)Closes
Verify
After loading, each LLM request logs:
Dashboard at
localhost:3333/api/routing-statsshows tier distribution.🐕 GitSniff Summary
What this PR does
This change enhances the SlimClaw plugin by enabling ClawRouter classification during the
llm_inputhook. This allows the system to generate and track routing recommendations, including tier, confidence, and model suggestions, which are then logged and made visible in the dashboard for analysis, providing valuable insights into potential routing decisions without actively changing the model.Key Changes
classifyWithRouterinto thellm_inputhook for routing recommendations.slimclaw.config.jsonfor custom tier definitions.RequestMetricandSlimClawMetricsAdapterto store and display routing data in the dashboard.Review Score: Excellent 🟢
Tip
No major issues found. Safe to merge.
🐕 Reviewed by GitSniff