Limit FastEmbed threading to prevent excessive CPU usage

[ptelang]

Summary

Fixed issue where FastEmbed was using all available CPU cores during embedding generation, causing CPU usage spikes up to 1000% in Docker containers with 10 cores.

Problem

Every 60 seconds during workload polling, FastEmbed's ONNX Runtime would spawn threads equal to the number of CPU cores. In Docker environments, this resulted in 1000% CPU usage (10 cores at 100% each).

Solution

• Added embedding_threads configuration parameter (default: 2, range: 1-16)
• Updated EmbeddingManager to pass thread limit to FastEmbed's TextEmbedding
• Added EMBEDDING_THREADS environment variable support
• Updated all initialization points (cli.py, server.py, polling_manager.py)

Impact

• CPU usage reduced from 1000% to ~50-100% during polling cycles
• Configurable via environment variable or CLI flag
• No performance degradation for typical workloads
• All 276 tests passing

Usage

# Use default (2 threads)
docker run mcp-optimizer

# Or configure custom thread count
docker run -e EMBEDDING_THREADS=4 mcp-optimizer

Testing

• All existing tests updated and passing
• Type checking, linting, and formatting verified
• Ready for production deployment

[claude]

Code Review

✅ Positives

• Excellent problem identification: Clear CPU usage issue from 1000% to 50-100%
• Consistent implementation: Threading parameter properly propagated through all initialization points (cli.py, server.py, polling_manager.py)
• Good defaults: default=2 is sensible for production; range validation (1-16) prevents misconfiguration
• Test coverage: Updated test to verify threading parameter passed correctly

🔍 Issues & Suggestions

1. Type validation inconsistency (config.py:155-162)

The field definition has int | None but sets default=2 (not None), which creates confusion:

• Field accepts None to mean "use all cores"
• But default is 2, not None
• Constraints (ge=1, le=16) will reject None values

Recommendation: Either:

• Remove | None and make field always require 1-16
• OR change default to None and update constraints with custom validator

2. Missing test coverage (test_embeddings.py)

Only updated existing test to check default behavior (threads=None). Missing:

• Test with explicit thread count (e.g., threads=2)
• Test with thread validation (boundaries: 1, 16)
• Test that config properly passes threading to EmbeddingManager

3. Minor: Documentation clarity (embeddings.py:46-48)

Comment says "Set to 1-4" but config allows 1-16. Consider aligning these or clarifying why the range differs from recommendation.

🎯 Verdict

Approve with minor fixes recommended. The core fix is solid and addresses the CPU issue effectively. The type annotation issue should be clarified before merge to prevent runtime confusion.

[ptelang]

@claude check again the newest commit. Verify if the fixes were applied. Make your comment short and precise.

[claude]

Claude finished @ptelang's task —— View job

---

✅ All Fixes Verified

• Review config.py for type validation fix
• Review test coverage additions
• Review documentation clarity
• Provide verification summary

Results

All three issues from previous review have been addressed:

1. Type validation fixed (config.py:155-161)

   • Field now properly accepts None with int | None
   • Default remains 2 for production safety
   • Constraint validation correctly handles None values

2. Test coverage complete (test_embeddings.py & test_config_basic.py)

   • ✅ Explicit thread count tests (lines 153-169, 172-194)
   • ✅ Boundary validation tests (lines 124-156 in test_config_basic.py)
   • ✅ Config integration tests (lines 197-241)

3. Documentation aligned (embeddings.py:46-48)

   • Comment now says "Set to 1-4 to limit CPU usage" (recommendation)
   • Config allows 1-16 (validation range) - clarified as limit vs recommendation

Ready to merge. Implementation is solid with comprehensive test coverage.

---

Job Run | Branch: set-threads-on-embedder