Skip to content

Conversation

@yossiovadia
Copy link
Collaborator

Summary

This PR fixes inconsistent model selection behavior by ensuring the ExtProc router uses the same UnifiedClassifier (LoRA-based) instance as the Classification API.

Problem

  • Classification API (port 8080): Used UnifiedClassifier (LoRA models) ✅
  • ExtProc router (port 8801): Used legacy Classifier (traditional BERT) ❌
  • This caused different classification results for the same query, leading to incorrect model selection in category-based routing

Example Issue

Math queries were classified correctly by the API but routed to the wrong model through Envoy:

Query: "What is 234 + 567?"

Classification API (8080):
- Category: math, Confidence: 0.886 ✅
- Expected model: Model-B (score 1.0)

Envoy Routing (8801):
- Actual model: Model-A ❌
- Reason: Using uninitialized legacy classifier → fallback to default

Root Cause

In router.go, the UnifiedClassifier was never wired to the legacy Classifier:

// Before fix - UnifiedClassifier not connected ❌
autoSvc, err := services.NewClassificationServiceWithAutoDiscovery(cfg)
if err != nil {
    services.NewClassificationService(classifier, cfg)  // Not stored
} else {
    _ = autoSvc  // UnifiedClassifier never wired to Classifier
}

Solution

Wire the UnifiedClassifier to enable delegation:

// After fix - UnifiedClassifier properly wired ✅
var classificationSvc *services.ClassificationService
autoSvc, err := services.NewClassificationServiceWithAutoDiscovery(cfg)
if err != nil {
    classificationSvc = services.NewClassificationService(classifier, cfg)
} else {
    classificationSvc = autoSvc
    if classificationSvc.HasUnifiedClassifier() {
        unifiedClassifier := classificationSvc.GetUnifiedClassifier()
        if unifiedClassifier != nil {
            classifier.UnifiedClassifier = unifiedClassifier  // Now wired!
        }
    }
}

Changes

  • router.go: Wire UnifiedClassifier to Classifier during initialization, store ClassificationService reference
  • classifier.go: Add UnifiedClassifier field, delegate to it before trying in-tree classifier, add classifyWithUnifiedClassifier() helper method
  • classification.go: Add GetUnifiedClassifier() getter method

Testing

Before fix:

Query: What is 789 + 123?
Category: math, Confidence: 0.886
Expected: Model-B
Actual: Model-A ❌ FAIL

After fix:

Query: What is 789 + 123?
Category: math, Confidence: 0.896
Expected: Model-B
Actual: Model-B ✅ PASS

Related Issues

Closes #640

…ed classification

This change ensures the ExtProc router uses the same UnifiedClassifier
(LoRA-based) instance as the Classification API, fixing inconsistent
model selection behavior.

**Problem:**
- Classification API (port 8080) used UnifiedClassifier (LoRA models)
- ExtProc router (port 8801) used legacy Classifier (traditional BERT)
- This caused different classification results for the same query,
  leading to incorrect model selection in category-based routing

**Solution:**
1. Wire UnifiedClassifier from ClassificationService to legacy Classifier
2. Add delegation in Classifier.ClassifyCategoryWithEntropy() to use
   UnifiedClassifier when available
3. Add GetUnifiedClassifier() method to ClassificationService

**Changes:**
- router.go: Wire UnifiedClassifier to Classifier during initialization
- classifier.go: Delegate to UnifiedClassifier before trying in-tree
  classifier, add classifyWithUnifiedClassifier() helper method
- classification.go: Add GetUnifiedClassifier() getter method

Related to vllm-project#640

Co-Authored-By: Claude <[email protected]>
Signed-off-by: Yossi Ovadia <[email protected]>
@netlify
Copy link

netlify bot commented Nov 11, 2025

Deploy Preview for vllm-semantic-router ready!

Name Link
🔨 Latest commit 62e4158
🔍 Latest deploy log https://app.netlify.com/projects/vllm-semantic-router/deploys/691a915fd43289000847fd1a
😎 Deploy Preview https://deploy-preview-641--vllm-semantic-router.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@github-actions
Copy link

github-actions bot commented Nov 11, 2025

👥 vLLM Semantic Team Notification

The following members have been identified for the changed files in this PR and have been automatically assigned:

📁 src

Owners: @rootfs, @Xunzhuo, @wangchen615
Files changed:

  • src/semantic-router/pkg/classification/classifier.go
  • src/semantic-router/pkg/extproc/router.go
  • src/semantic-router/pkg/services/classification.go

vLLM

🎉 Thanks for your contributions!

This comment was automatically generated based on the OWNER files in the repository.

@github-actions github-actions bot deleted a comment Nov 15, 2025
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.

ExtProc router uses legacy Classifier instead of LoRA-based classifier

4 participants