test: add unit tests for BedrockClassifier

[hashwnath]

Addresses #448

Adds comprehensive unit tests for BedrockClassifier covering:

• Initialization: client injection, boto3 auto-creation, default/custom model ID, inference config defaults and overrides
• Tool schema structure: validates the analyzePrompt tool spec shape and required fields
• Classification flow: happy path agent selection, confidence conversion, toolChoice behavior for anthropic vs non-anthropic models, custom inference config propagation
• Error handling: no output, empty tool use, invalid tool input, BotoCoreError, ClientError
• Callbacks: verifies on_classifier_start and on_classifier_stop are invoked with correct arguments

Follows existing test patterns from test_anthropic_classifier.py (pytest, MockAgent, @patch, @pytest.mark.asyncio).

All 22 tests pass.

[brnaba-aws]

@claude review this

[claude]

Claude finished @brnaba-aws's task in 1m 58s —— View job

---

PR Review: test: add unit tests for BedrockClassifier

• Read CLAUDE.md for repo guidelines
• Read the test file under review
• Read the BedrockClassifier source to compare
• Analyze tests and provide feedback

---

Overall this is a solid addition — the test structure mirrors test_anthropic_classifier.py well, coverage is broad, and the error-path tests match the actual exception handling in the source. A few issues worth addressing before merging:

---

Critical: Decorator ordering on all async tests

Every async test uses this ordering:

@patch('agent_squad.classifiers.bedrock_classifier.user_agent')
@pytest.mark.asyncio
async def test_process_request_success(self, mock_user_agent):

Python decorators are applied bottom-up, so here @pytest.mark.asyncio is applied first to the async def, then @patch wraps it — meaning pytest sees the sync wrapper from @patch, not the marked async function. The asyncio mark may not propagate through, causing tests to silently not be awaited in certain pytest-asyncio versions/modes.

The conventional and safe order is:

@pytest.mark.asyncio
@patch('agent_squad.classifiers.bedrock_classifier.user_agent')
async def test_process_request_success(self, mock_user_agent):

This applies to all 12 async tests (lines 181, 221, 235, 258, 288, 313, 325, 344, 392, 426, 460, 496, 534). Fix this →

---

Missing test: mistral-large toolChoice branch

The source at bedrock_classifier.py:91 checks:

if "anthropic" in self.model_id or 'mistral-large' in self.model_id:

There's a test for anthropic models (line 460) and non-anthropic models (line 496), but no test for the mistral-large branch. Adding one would complete coverage of this conditional. Fix this →

---

Unused import

MagicMock is imported on line 2 but never used anywhere in the file:

from unittest.mock import Mock, patch, AsyncMock, MagicMock  # MagicMock unused

Fix this →

---

Missing coverage: region fallback to AWS_REGION env var

bedrock_classifier.py:30 uses:

self.region = options.region or os.environ.get('AWS_REGION')

test_init_boto3_auto_creation (line 93) verifies explicit region, but there's no test for the fallback path where both options.region is None and AWS_REGION is set in the environment. This edge case is worth covering.

---

No coverage for classify() entry point

All tests call process_request() directly, bypassing the classify() method in the base class which is responsible for setting history and updating the system prompt. A test calling classify() would verify the full real-world path including system prompt assembly. This isn't a blocker but is worth noting as a gap.

---

Minor: system_prompt is empty string during all tests

Since tests bypass classify(), self.system_prompt is always "" when converse is called (the base Classifier.__init__ sets it to ""). The system field in the Bedrock call therefore carries an empty string. Tests like test_process_request_tool_choice_for_anthropic_model (line 460) verify the toolConfig but don't assert on the system field. This is fine, but worth being aware of.

---

What's well done

• Full error path coverage: BotoCoreError, ClientError, empty output, no toolUse, None toolUse, and invalid tool input all tested.
• Callback argument verification (lines 382–388) correctly matches the positional and keyword args from the source.
• Confidence string-to-float conversion test is a nice edge case.
• test_process_request_agent_not_found correctly tests the get_agent_by_id fallback to None.
• Custom inference config propagation verified end-to-end through to the converse call kwargs.

---

|

[cornelcroi]

Hey @hashwnath, is the default classifier and had no unit tests, so filling this gap is genuinely valuable. The tests follow existing patterns and cover the key scenarios well. Only thing needed before merging: CI has not run yet — needs to go green first.

[hashwnath]

Hi, Addressed the previous comments, hope CI runs this time. It might need a manual approval from your end since it's my first PR here.
https://github.com/2FastLabs/agent-squad/pull/457/checks