Skip to content

Conversation

@aeft
Copy link
Contributor

@aeft aeft commented Sep 9, 2025

What type of PR is this?

test/refactor: add more test cases and refactor SelectBestModelForCategory/SelectBestModelFromList/InitializeJailbreakClassifier for testability

What this PR does / why we need it:

Follow my previous plan to improve the unit tests for the remaining methods in classifier.go
#57

  • SelectBestModelForCategory and SelectBestModelFromList contained duplicated and complex logic. I refactored by extracting key methods and added comprehensive test cases
  • Refactor InitializeJailbreakClassifier with dependency injection
  • Achieving 100% coverage for all main methods: InitializeJailbreakClassifier, CheckForJailbreak, AnalyzeContentForJailbreak, ClassifyCategory, ClassifyPII, DetectPIIInContent, AnalyzeContentForPII, ClassifyAndSelectBestModel, SelectBestModelForCategory, SelectBestModelFromList, GetModelsForCategory.
  • Refactor: remove load aware algorithm

Which issue(s) this PR fixes:

Fixes #

Release Notes: No

…ectBestModelFromList for testability

Signed-off-by: Alex Wang <[email protected]>
@netlify
Copy link

netlify bot commented Sep 9, 2025

Deploy Preview for vllm-semantic-router ready!

Name Link
🔨 Latest commit d78e0e7
🔍 Latest deploy log https://app.netlify.com/projects/vllm-semantic-router/deploys/68c073a32479d300087e416e
😎 Deploy Preview https://deploy-preview-101--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 Sep 9, 2025

👥 vLLM Semantic Team Notification

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

📁 config

Owners: @rootfs
Files changed:

  • config/config.yaml

📁 deploy

Owners: @rootfs, @Xunzhuo
Files changed:

  • deploy/kubernetes/config.yaml

📁 src

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

  • src/semantic-router/pkg/config/config.go
  • src/semantic-router/pkg/config/config_test.go
  • src/semantic-router/pkg/extproc/request_handler.go
  • src/semantic-router/pkg/extproc/response_handler.go
  • src/semantic-router/pkg/extproc/router.go
  • src/semantic-router/pkg/extproc/security_test.go
  • src/semantic-router/pkg/extproc/test_utils_test.go
  • src/semantic-router/pkg/utils/classification/classifier.go
  • src/semantic-router/pkg/utils/classification/classifier_test.go
  • src/training/model_eval/result_to_config.py

vLLM

🎉 Thanks for your contributions!

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

@aeft aeft changed the title test: add more test cases and refactor SelectBestModelForCategory/SelectBestModelFromList for testability test: add more test cases and refactor SelectBestModelForCategory/SelectBestModelFromList/InitializeJailbreakClassifier for testability Sep 9, 2025
@aeft aeft force-pushed the test/classifier-add-tests branch from 0a8827e to ad93419 Compare September 9, 2025 18:20
@aeft
Copy link
Contributor Author

aeft commented Sep 9, 2025

@rootfs Updated. I also removed:

  • load_aware in the config;
  • unused fields (ModelLoad, ModelLoadLock, ModelTTFT) in Classifier;
  • return value quality when select models.

@rootfs rootfs merged commit fe0b5b5 into vllm-project:main Sep 9, 2025
9 checks passed
@aeft aeft deleted the test/classifier-add-tests branch September 9, 2025 18:45
@rootfs
Copy link
Collaborator

rootfs commented Sep 9, 2025

@aeft thanks for contributing!

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.

4 participants