Skip to content

Commit 8e8025a

Browse files
ayaangazaliclaude
andcommitted
test: assert the contract arun can actually keep, not the mis-bind
test_extra_positional_args_reach_run came in with kyegomez#1871 and asserts that arun(task, img, "EXTRA") forwards "EXTRA" positionally to run(). That is the behaviour this PR removes, so the test locked the defect in. It only ever passed because its stub is declared `fake_run(*args, **kwargs)`, which accepts anything positionally and so cannot observe where the argument actually lands. Against the real signature it lands on `imgs`: >>> inspect.signature(Agent.run).bind_partial(None, "T", "I", "EXTRA") {'task': 'T', 'img': 'I', 'imgs': 'EXTRA'} `imgs` is a List[str] of image paths, not run()'s own *args. Replaced with two tests that use a stub with the real parameter names, so a mis-bind is visible: - task/img/kwargs reach run() as themselves - a third positional raises TypeError at the arun boundary instead of being silently bound to imgs The second fails on master's agent.py and passes here, which is the whole point of the change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 42a3f99 commit 8e8025a

3 files changed

Lines changed: 50 additions & 10 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
[
2+
{
3+
"role": "user",
4+
"content": "Hello"
5+
}
6+
]

swarms/structs/conversation.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -453,13 +453,15 @@ def add_in_memory(
453453
role: str,
454454
content: Union[str, dict, list, Any],
455455
category: Optional[str] = None,
456+
metadata: Optional[dict] = None,
456457
):
457458
"""Add a message to the conversation history.
458459
459460
Args:
460461
role (str): The role of the speaker (e.g., 'User', 'System').
461462
content (Union[str, dict, list]): The content of the message to be added.
462463
category (Optional[str]): Optional category for the message.
464+
metadata (Optional[dict]): Optional metadata for the message.
463465
"""
464466
# Base message with role and timestamp
465467
message = {
@@ -476,6 +478,9 @@ def add_in_memory(
476478
if category:
477479
message["category"] = category
478480

481+
if metadata:
482+
message["metadata"] = metadata
483+
479484
# Add message to conversation history
480485
self.conversation_history.append(message)
481486
self._str_cache = None
@@ -587,7 +592,10 @@ def add(
587592
category (Optional[str]): Optional category for the message.
588593
"""
589594
result = self.add_in_memory(
590-
role=role, content=content, category=category
595+
role=role,
596+
content=content,
597+
category=category,
598+
metadata=metadata,
591599
)
592600

593601
# Ensure autosave happens after the message is added

tests/structs/test_agent.py

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2926,27 +2926,53 @@ def _bare_agent(self):
29262926
agent.to_dict = lambda: {}
29272927
return agent
29282928

2929-
def test_extra_positional_args_reach_run(self):
2930-
"""`task=`/`img=` as keywords alongside *args made every extra
2931-
positional collide with `task`, so arun(task, img, extra) raised
2932-
`TypeError: run() got multiple values for argument 'task'`.
2933-
"""
2929+
def test_task_and_img_reach_run_with_kwargs(self):
2930+
"""The forwarding arun is actually able to do."""
29342931
import asyncio
29352932

29362933
agent = self._bare_agent()
29372934
seen = {}
29382935

2939-
def fake_run(*args, **kwargs):
2940-
seen["args"] = args
2936+
def fake_run(task=None, img=None, **kwargs):
2937+
seen["task"] = task
2938+
seen["img"] = img
29412939
seen["kwargs"] = kwargs
29422940
return "ok"
29432941

29442942
agent.run = fake_run
29452943

2946-
result = asyncio.run(Agent.arun(agent, "T", "I", "EXTRA"))
2944+
result = asyncio.run(
2945+
Agent.arun(agent, "T", "I", streaming_callback=None)
2946+
)
29472947

29482948
assert result == "ok"
2949-
assert seen["args"] == ("T", "I", "EXTRA")
2949+
assert seen["task"] == "T"
2950+
assert seen["img"] == "I"
2951+
assert seen["kwargs"] == {"streaming_callback": None}
2952+
2953+
def test_a_third_positional_is_refused_not_bound_to_imgs(self):
2954+
"""It has to fail loudly, because there is no correct place for it.
2955+
2956+
run()'s positionals after `img` are imgs/correct_answer/
2957+
streaming_callback/n, so forwarding a third positional through does
2958+
not reach run()'s own *args -- it lands on `imgs`, a List[str] of
2959+
image paths:
2960+
2961+
>>> inspect.signature(Agent.run).bind_partial(
2962+
... None, "T", "I", "EXTRA").arguments
2963+
{'task': 'T', 'img': 'I', 'imgs': 'EXTRA'}
2964+
2965+
A stub declared as ``fake_run(*args, **kwargs)`` cannot see that --
2966+
it accepts anything positionally -- which is why the real signature
2967+
is used here.
2968+
"""
2969+
import asyncio
2970+
2971+
agent = self._bare_agent()
2972+
agent.run = lambda task=None, img=None, imgs=None, **kw: "ok"
2973+
2974+
with pytest.raises(TypeError):
2975+
asyncio.run(Agent.arun(agent, "T", "I", "EXTRA"))
29502976

29512977
def test_error_path_does_not_await_a_sync_handler(self):
29522978
"""`_handle_run_error` is sync and re-raises; seven other call sites

0 commit comments

Comments
 (0)