Skip to content

Commit 2f702d7

Browse files
feat: support multiple account IDs in feedback tool
- Allow account_id to be string or list of strings - Send same feedback to each account individually - Return combined results with success/failure counts - Add comprehensive tests for validation and execution - All 9 tests passing, linting clean
1 parent 59e792e commit 2f702d7

File tree

2 files changed

+207
-16
lines changed

2 files changed

+207
-16
lines changed

stackone_ai/feedback/tool.py

Lines changed: 91 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -21,18 +21,38 @@ class FeedbackInput(BaseModel):
2121
"""Input schema for feedback tool."""
2222

2323
feedback: str = Field(..., min_length=1, description="User feedback text")
24-
account_id: str = Field(..., min_length=1, description="Account identifier")
24+
account_id: str | list[str] = Field(..., description="Account identifier(s) - single ID or list of IDs")
2525
tool_names: list[str] = Field(..., min_length=1, description="List of tool names")
2626

27-
@field_validator("feedback", "account_id")
27+
@field_validator("feedback")
2828
@classmethod
29-
def validate_non_empty_trimmed(cls, v: str) -> str:
30-
"""Validate that string is non-empty after trimming."""
29+
def validate_feedback(cls, v: str) -> str:
30+
"""Validate that feedback is non-empty after trimming."""
3131
trimmed = v.strip()
3232
if not trimmed:
33-
raise ValueError("Field must be a non-empty string")
33+
raise ValueError("Feedback must be a non-empty string")
3434
return trimmed
3535

36+
@field_validator("account_id")
37+
@classmethod
38+
def validate_account_id(cls, v: str | list[str]) -> list[str]:
39+
"""Validate and normalize account ID(s) to a list."""
40+
if isinstance(v, str):
41+
trimmed = v.strip()
42+
if not trimmed:
43+
raise ValueError("Account ID must be a non-empty string")
44+
return [trimmed]
45+
46+
if isinstance(v, list):
47+
if not v:
48+
raise ValueError("At least one account ID is required")
49+
cleaned = [str(item).strip() for item in v if str(item).strip()]
50+
if not cleaned:
51+
raise ValueError("At least one valid account ID is required")
52+
return cleaned
53+
54+
raise ValueError("Account ID must be a string or list of strings")
55+
3656
@field_validator("tool_names")
3757
@classmethod
3858
def validate_tool_names(cls, v: list[str]) -> list[str]:
@@ -52,12 +72,14 @@ def execute(
5272
"""
5373
Execute the feedback tool with enhanced validation.
5474
75+
If multiple account IDs are provided, sends the same feedback to each account individually.
76+
5577
Args:
5678
arguments: Tool arguments as string or dict
5779
options: Execution options
5880
5981
Returns:
60-
Response from the API
82+
Combined response from all API calls
6183
6284
Raises:
6385
StackOneError: If validation or API call fails
@@ -72,16 +94,59 @@ def execute(
7294
# Validate with Pydantic
7395
parsed_params = FeedbackInput(**raw_params)
7496

75-
# Build validated request body
76-
validated_arguments = {
77-
"feedback": parsed_params.feedback,
78-
"account_id": parsed_params.account_id,
79-
"tool_names": parsed_params.tool_names,
97+
# Get list of account IDs (already normalized by validator)
98+
account_ids = parsed_params.account_id
99+
feedback = parsed_params.feedback
100+
tool_names = parsed_params.tool_names
101+
102+
# If only one account ID, use the parent execute method
103+
if len(account_ids) == 1:
104+
validated_arguments = {
105+
"feedback": feedback,
106+
"account_id": account_ids[0],
107+
"tool_names": tool_names,
108+
}
109+
return super().execute(validated_arguments, options=options)
110+
111+
# Multiple account IDs - send to each individually
112+
results = []
113+
errors = []
114+
115+
for account_id in account_ids:
116+
try:
117+
validated_arguments = {
118+
"feedback": feedback,
119+
"account_id": account_id,
120+
"tool_names": tool_names,
121+
}
122+
result = super().execute(validated_arguments, options=options)
123+
results.append({
124+
"account_id": account_id,
125+
"status": "success",
126+
"result": result
127+
})
128+
except Exception as exc:
129+
error_msg = str(exc)
130+
errors.append({
131+
"account_id": account_id,
132+
"status": "error",
133+
"error": error_msg
134+
})
135+
results.append({
136+
"account_id": account_id,
137+
"status": "error",
138+
"error": error_msg
139+
})
140+
141+
# Return combined results
142+
return {
143+
"message": f"Feedback sent to {len(account_ids)} account(s)",
144+
"total_accounts": len(account_ids),
145+
"successful": len([r for r in results if r["status"] == "success"]),
146+
"failed": len(errors),
147+
"results": results
80148
}
81149

82-
# Use the parent execute method with validated arguments
83-
return super().execute(validated_arguments, options=options)
84-
85150
except json.JSONDecodeError as exc:
86151
raise StackOneError(f"Invalid JSON in arguments: {exc}") from exc
87152
except ValueError as exc:
@@ -120,8 +185,18 @@ def create_feedback_tool(
120185
type="object",
121186
properties={
122187
"account_id": {
123-
"type": "string",
124-
"description": 'Account identifier (e.g., "acc_123456")',
188+
"oneOf": [
189+
{
190+
"type": "string",
191+
"description": 'Single account identifier (e.g., "acc_123456")',
192+
},
193+
{
194+
"type": "array",
195+
"items": {"type": "string"},
196+
"description": "List of account identifiers for multiple accounts",
197+
},
198+
],
199+
"description": "Account identifier(s) - single ID or list of IDs",
125200
},
126201
"feedback": {
127202
"type": "string",

tests/test_feedback.py

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,34 @@ def test_validation_errors(self) -> None:
5757
result = tool.execute(json_string)
5858
assert result["message"] == "Success"
5959

60+
def test_multiple_account_ids_validation(self) -> None:
61+
"""Test validation with multiple account IDs."""
62+
tool = create_feedback_tool(api_key="test_key")
63+
64+
# Test empty account ID list
65+
with pytest.raises(StackOneError, match="At least one account ID is required"):
66+
tool.execute({"feedback": "Great tools!", "account_id": [], "tool_names": ["test_tool"]})
67+
68+
# Test account ID list with empty strings
69+
with pytest.raises(StackOneError, match="At least one valid account ID is required"):
70+
tool.execute({"feedback": "Great tools!", "account_id": ["", " "], "tool_names": ["test_tool"]})
71+
72+
# Test valid multiple account IDs
73+
with patch("requests.request") as mock_request:
74+
mock_response = Mock()
75+
mock_response.status_code = 200
76+
mock_response.json.return_value = {"message": "Success"}
77+
mock_response.raise_for_status = Mock()
78+
mock_request.return_value = mock_response
79+
80+
result = tool.execute({
81+
"feedback": "Great tools!",
82+
"account_id": ["acc_123456", "acc_789012"],
83+
"tool_names": ["test_tool"]
84+
})
85+
assert "total_accounts" in result
86+
assert result["total_accounts"] == 2
87+
6088

6189
class TestFeedbackToolExecution:
6290
"""Test suite for feedback tool execution."""
@@ -151,6 +179,94 @@ def test_handles_api_errors(self) -> None:
151179
}
152180
)
153181

182+
def test_multiple_account_ids_execution(self) -> None:
183+
"""Test execution with multiple account IDs."""
184+
tool = create_feedback_tool(api_key="test_key")
185+
186+
api_response = {
187+
"message": "Feedback successfully stored",
188+
"key": "test-key.json",
189+
"submitted_at": "2025-10-08T11:44:16.123Z",
190+
"trace_id": "test-trace-id",
191+
}
192+
193+
with patch("requests.request") as mock_request:
194+
mock_response = Mock()
195+
mock_response.status_code = 200
196+
mock_response.json.return_value = api_response
197+
mock_response.raise_for_status = Mock()
198+
mock_request.return_value = mock_response
199+
200+
result = tool.execute({
201+
"feedback": "Great tools!",
202+
"account_id": ["acc_123456", "acc_789012", "acc_345678"],
203+
"tool_names": ["test_tool"]
204+
})
205+
206+
# Check combined result structure
207+
assert result["message"] == "Feedback sent to 3 account(s)"
208+
assert result["total_accounts"] == 3
209+
assert result["successful"] == 3
210+
assert result["failed"] == 0
211+
assert len(result["results"]) == 3
212+
213+
# Check individual results
214+
for i, account_id in enumerate(["acc_123456", "acc_789012", "acc_345678"]):
215+
assert result["results"][i]["account_id"] == account_id
216+
assert result["results"][i]["status"] == "success"
217+
assert result["results"][i]["result"] == api_response
218+
219+
# Verify API was called 3 times
220+
assert mock_request.call_count == 3
221+
222+
def test_multiple_account_ids_with_errors(self) -> None:
223+
"""Test execution with multiple account IDs where some fail."""
224+
tool = create_feedback_tool(api_key="test_key")
225+
226+
success_response = {"message": "Success"}
227+
228+
def mock_request_side_effect(*args, **kwargs):
229+
account_id = kwargs.get("json", {}).get("account_id")
230+
if account_id == "acc_123456":
231+
# Success case
232+
mock_response = Mock()
233+
mock_response.status_code = 200
234+
mock_response.json.return_value = success_response
235+
mock_response.raise_for_status = Mock()
236+
return mock_response
237+
else:
238+
# Error case
239+
mock_response = Mock()
240+
mock_response.status_code = 401
241+
mock_response.text = '{"error": "Unauthorized"}'
242+
mock_response.json.return_value = {"error": "Unauthorized"}
243+
mock_response.raise_for_status.side_effect = Exception("401 Client Error: Unauthorized")
244+
return mock_response
245+
246+
with patch("requests.request") as mock_request:
247+
mock_request.side_effect = mock_request_side_effect
248+
249+
result = tool.execute({
250+
"feedback": "Great tools!",
251+
"account_id": ["acc_123456", "acc_unauthorized"],
252+
"tool_names": ["test_tool"]
253+
})
254+
255+
# Check combined result structure
256+
assert result["total_accounts"] == 2
257+
assert result["successful"] == 1
258+
assert result["failed"] == 1
259+
assert len(result["results"]) == 2
260+
261+
# Check individual results
262+
success_result = next(r for r in result["results"] if r["account_id"] == "acc_123456")
263+
assert success_result["status"] == "success"
264+
assert success_result["result"] == success_response
265+
266+
error_result = next(r for r in result["results"] if r["account_id"] == "acc_unauthorized")
267+
assert error_result["status"] == "error"
268+
assert "401 Client Error: Unauthorized" in error_result["error"]
269+
154270

155271
class TestFeedbackToolIntegration:
156272
"""Test suite for feedback tool integration."""

0 commit comments

Comments
 (0)