Updated the SDK Operations for Agents

[MinuraPunchihewa]

This PR updates the SDK operations for agents based on the new syntax supported via SQL.

Fixes https://linear.app/mindsdb/issue/RES-119/agents-into-python-sdk

Depends on mindsdb/mindsdb#11255

[entelligence-ai-pr-reviews]

Review Summary

🏷️ Draft Comments (4)

Skipped posting 4 drafted comments based on your review threshold. Feel free to update them here.

mindsdb_sdk/agents.py (4)

86-88: skills and data default to mutable objects ([], {}) in Agent.__init__, causing shared state and potential data corruption between instances.

Scores:

• Production Impact: 0
• Fix Specificity: 0
• Urgency Impact: 0
• Total Score: 0

Reason for filtering: The line numbers 86-88 don't match the actual code. The Agent.init method starts at line 80, and the parameters mentioned (skills with default [], data with default {}) don't exist in the actual code. The actual parameters at those lines are 'provider: str = None' and 'collection: CollectionBase = None'. There is no 'data' parameter in the Agent.init method.

Analysis: This comment is based on incorrect understanding of the code structure. The reported issue doesn't exist at the specified line numbers or anywhere in the actual file content.

---

97-99: Agent.__init__ does not handle skills=None or data=None, leading to TypeError if not provided and breaking the fix for mutable defaults.

Scores:

• Production Impact: 0
• Fix Specificity: 0
• Urgency Impact: 0
• Total Score: 0

Reason for filtering: The line numbers 97-99 don't correspond to the described issue. At those lines, the actual code is 'self.updated_at = updated_at' and 'self.collection = collection'. There is no 'self.data = data' assignment in the Agent.init method. This comment is based on the previous incorrect comment about non-existent mutable defaults.

Analysis: This comment references code that doesn't exist at the specified line numbers. The issue it's trying to fix is based on the previous incorrect comment.

---

189-191: Agent.__eq__ compares self.skills != other.skills directly, which is O(n) and can be very slow for large skill lists; this can cause major slowdowns in equality checks for agents with many skills.

Scores:

• Production Impact: 0
• Fix Specificity: 0
• Urgency Impact: 0
• Total Score: 0

Reason for filtering: The line numbers 189-191 don't match the described code. At those lines, the actual code is about comparing provider and skills, but the snippet shows 'if self.skills != other.skills:' which doesn't appear at those exact lines. The actual skills comparison is at line 190 'if self.skills != other.skills:', but the performance concern about O(n) comparison is questionable since Python's list comparison is already optimized and the suggested fix doesn't provide meaningful performance improvement.

Analysis: Line numbers don't precisely match, and the performance optimization suggested is not meaningful since Python's built-in list comparison is already efficient.

---

326-336: add_files and add_file methods accept arbitrary file paths from user input and pass them directly to self.api.upload_file without sanitization, enabling path traversal or unauthorized file access if user input is not strictly controlled.

Scores:

• Production Impact: 4
• Fix Specificity: 4
• Urgency Impact: 4
• Total Score: 12

Reason for filtering: This is a valid security concern. The code at lines 326-336 does process file paths from user input and passes them to upload_file without validation. The suggested validation for path traversal attacks is appropriate and the commitable suggestion is technically sound.

Analysis: Valid security issue identifying potential path traversal vulnerability. The file path validation is important for preventing unauthorized file access. The fix is specific and addresses the security concern appropriately.

---

[MinuraPunchihewa]

Hey @ea-rus,
For the data parameter of agents, do you think we should allow objects (Database, Table, KnowledegBase) to be passed instead of a dict? Or both?

If we allow dicts, should we represent them as objects in the Agent object?

I wanted to confirm this before I complete this PR.

[entelligence-ai-pr-reviews]

Review Summary

🏷️ Draft Comments (2)

Skipped posting 2 drafted comments based on your review threshold. Feel free to update them here.

mindsdb_sdk/agents.py (2)

199-209: Agent.from_json does not convert created_at and updated_at from string to datetime, causing runtime errors or incorrect attribute types.

Scores:

• Production Impact: 4
• Fix Specificity: 5
• Urgency Impact: 3
• Total Score: 12

Reason for filtering: This is a valid issue. The Agent constructor expects datetime objects for created_at and updated_at parameters (lines 86-87), but from_json passes these values directly from JSON without conversion. JSON typically contains string representations of dates, which would cause type mismatches. The suggested fix properly handles string-to-datetime conversion using fromisoformat.

Analysis: High production impact as this could cause runtime errors when Agent objects are created from JSON responses. The fix is very specific and immediately applicable. Medium urgency as it affects core functionality but may not manifest in all usage patterns.

---

326-336: add_files uploads each file individually in a loop, causing multiple sequential network requests and high latency for large file lists.

Scores:

• Production Impact: 1
• Fix Specificity: 1
• Urgency Impact: 1
• Total Score: 3

Reason for filtering: The commitable suggestion references a non-existent API method 'upload_files' (batch upload) that doesn't exist in the codebase. The suggestion would break the code by calling self.api.upload_files() which is not defined. While the performance concern is valid, the proposed solution is not implementable without first adding the batch upload API method.

Analysis: The suggestion would introduce a breaking change by calling undefined methods. While the performance optimization idea has merit, the implementation assumes infrastructure that doesn't exist in the current codebase.

---

[entelligence-ai-pr-reviews]

Review Summary

🏷️ Draft Comments (1)

Skipped posting 1 drafted comments based on your review threshold. Feel free to update them here.

mindsdb_sdk/agents.py (1)

548-549: existing_agent['skills'] is accessed directly without checking if the key exists, which can cause a KeyError and crash if the API response lacks the 'skills' key.

Scores:

• Production Impact: 3
• Fix Specificity: 5
• Urgency Impact: 2
• Total Score: 10

Reason for filtering: This is a valid defensive programming improvement. The code at lines 548-549 does access existing_agent['skills'] directly, which could cause a KeyError if the API response lacks the 'skills' key. The suggested fix using .get('skills', []) is safe and prevents potential runtime crashes.

Analysis: The comment identifies a legitimate defensive programming issue where direct dictionary access could cause KeyError exceptions. While the likelihood of the API returning malformed responses may be low, the fix is trivial and eliminates a potential crash point. The suggestion is specific and immediately applicable with no risk of breaking existing functionality.

---

[entelligence-ai-pr-reviews]

Review Summary

🏷️ Draft Comments (2)

Skipped posting 2 drafted comments based on your review threshold. Feel free to update them here.

mindsdb_sdk/agents.py (2)

36-212: Agent class is missing the data and prompt_template attributes, but methods in Agents expect them to exist, causing AttributeError at runtime.

Scores:

• Production Impact: 1
• Fix Specificity: 1
• Urgency Impact: 1
• Total Score: 3

Reason for filtering: The comment claims Agent class is missing 'data' and 'prompt_template' attributes that methods expect, but examining the full file content shows no evidence of any methods accessing agent.data or agent.prompt_template. The commitable suggestion uses getattr(self, 'data', {}) which is nonsensical - it's checking if the current instance has an attribute that hasn't been set yet. This appears to be a false positive based on incorrect analysis of the codebase.

Analysis: False positive - no evidence in the actual code that these attributes are needed or accessed anywhere

---

330-349: In add_files, the code uploads each file individually and updates agent data in a loop, causing multiple API calls and updates for large file lists, leading to significant performance overhead.

Scores:

• Production Impact: 1
• Fix Specificity: 1
• Urgency Impact: 1
• Total Score: 3

Reason for filtering: The line numbers 330-349 don't match the actual add_files method implementation. The snippet shows code that doesn't exist in the actual file - there's no agent.data access, no prompt_template manipulation, and no loop that updates agent data. The actual implementation (lines 313-358) works with knowledge bases and skills, not direct agent data manipulation. The commitable suggestion references non-existent code patterns.

Analysis: Incorrect line numbers and the described code pattern doesn't exist in the actual implementation - this is a false positive

---

[ea-rus]

yes, it is a question: how to set up embedding model parameters.
maybe to add option to update config (default embedding model via SDK?)

[MinuraPunchihewa]

Yes, that's a great idea, @ea-rus. Let me add that in.

[MinuraPunchihewa]

@ea-rus I added a few operations to update the configuration here:
#202

The error message for creating KBs in agents has also been updated.

[github-actions]

Coverage Report

File	Stmts	Miss	Cover	Missing
mindsdb_sdk
agents.py	210	64	70%	30, 104, 107, 110, 113, 121, 129, 149, 170, 181, 184, 188, 190, 192, 194, 196, 198, 205, 268, 281, 292, 303, 306–310, 325, 334–338, 382, 390–391, 449, 457–458, 462, 492–500, 508–510, 512–513, 539–552, 564–565, 567
databases.py	49	2	96%	118, 146
handlers.py	39	1	97%	77
jobs.py	97	7	93%	40, 52, 80, 84, 146–149
knowledge_bases.py	133	18	86%	70–73, 132–136, 160, 189, 196, 200–202, 206, 228–232, 242
ml_engines.py	42	3	93%	94, 126, 128
models.py	210	19	91%	109, 140–141, 222, 231, 233, 303, 339, 348, 372, 397, 491, 499, 518, 534, 542, 567, 571, 584
projects.py	63	1	98%	160
query.py	13	1	92%	14
skills.py	53	9	83%	43, 45, 49, 58–60, 76–80, 122
tables.py	130	15	88%	140–142, 145, 165, 192, 203–204, 209, 224, 227, 321, 342–347, 356, 376
views.py	37	2	95%	105, 138
mindsdb_sdk/connectors
rest_api.py	255	52	80%	19–29, 35–36, 51, 55, 58–59, 79–81, 102, 105, 112–115, 148–156, 177–178, 213–216, 230–231, 285–290, 294–306
mindsdb_sdk/utils
agents.py	50	4	92%	72, 79–81
mind.py	47	47	0%	1–128
openai.py	85	30	65%	37–40, 83–85, 107, 148–158, 215–216, 234–240, 258–276
table_schema.py	21	21	0%	1–54
TOTAL	1621	296	82%

Tests	Skipped	Failures	Errors	Time
28	0 💤	0 ❌	0 🔥	10.760s ⏱️

[entelligence-ai-pr-reviews]

Review Summary

🏷️ Draft Comments (2)

Skipped posting 2 drafted comments based on your review threshold. Feel free to update them here.

mindsdb_sdk/agents.py (2)

316-358: add_files method is missing the knowledge_base parameter, causing a runtime error when called from add_file with four arguments.

Scores:

• Production Impact: 0
• Fix Specificity: 0
• Urgency Impact: 0
• Total Score: 0

Reason for filtering: The bug description is factually incorrect. Looking at the actual code at lines 313-321, the add_files method DOES include the knowledge_base: str = None parameter in its signature. The method signature is correct and matches what add_file calls it with. This is a false positive based on incorrect understanding of the existing code.

Analysis: The review comment claims the knowledge_base parameter is missing, but examining the actual file content at line 313 shows: def add_files(self, name: str, file_paths: List[str], description: str, knowledge_base: str = None):. The parameter exists and is properly implemented throughout the method body (lines 340-344). This is a clear technical accuracy issue.

---

316-368: add_files and add_file methods do not validate or sanitize file paths, allowing path traversal (e.g., ../../etc/passwd) and potential unauthorized file access or overwriting on the server.

Scores:

• Production Impact: 0
• Fix Specificity: 0
• Urgency Impact: 0
• Total Score: 0

Reason for filtering: The commitable suggestion would break the code by adding an import os statement inside the method body, which is invalid Python syntax. Import statements must be at the module level, not inside function definitions. Additionally, the suggestion introduces incomplete code with '...' placeholders that would cause syntax errors.

Analysis: While path traversal validation is a legitimate security concern, the suggested fix is technically flawed. The import os statement inside the function body would cause a syntax error, and the incomplete code with '...' placeholders would break the method. The suggestion fails the commitable_suggestion safety check.

---