Spatial API Unification by hillhack · Pull Request #151 · mesa/mesa-llm

hillhack · 2026-03-05T17:00:24Z

Fix for:#150
Changes

Standardized LLMAgent.pos
Added a @property in LLMAgent that resolves the correct coordinates for both:
- legacy mesa.space (using self.pos)
- new mesa.discrete_space (using self.cell.coordinate)
Unified Movement
Added LLMAgent.move_to(target_coordinates) to encapsulate movement logic for different Mesa space types.
Refactored Inbuilt Tools
Simplified move_one_step and teleport_to_location to use agent.pos and agent.move_to().
Full Compatibility
Verified that all 203 tests pass, including test_inbuilt_tools.py across different environment setups.

coderabbitai · 2026-03-05T17:00:54Z

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7c4d7459-b9b1-41e0-88b0-f69266deab79

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

codecov · 2026-03-05T17:27:21Z

Codecov Report

❌ Patch coverage is 98.03922% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 92.29%. Comparing base (4ea91ed) to head (82282b9).

Files with missing lines	Patch %	Lines
mesa_llm/tools/tool_decorator.py	94.11%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #151      +/-   ##
==========================================
+ Coverage   90.08%   92.29%   +2.20%     
==========================================
  Files          19       19              
  Lines        1503     1531      +28     
==========================================
+ Hits         1354     1413      +59     
+ Misses        149      118      -31

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

for more information, see https://pre-commit.ci

colinfrisch · 2026-03-07T14:19:44Z

That's a big PR, and I need to spend more time on it... what do you think @wang-boyu @sanika-n ?

Copilot

Pull request overview

This PR aims to unify spatial APIs across legacy mesa.space and newer mesa.discrete_space by standardizing how LLMAgent exposes position and performs movement, and by refactoring inbuilt movement tools to rely on that unified interface.

Changes:

Added a standardized LLMAgent.pos property and LLMAgent.move_to(...) method to abstract over Agent.pos vs cell.coordinate and different space types.
Refactored inbuilt movement tools (move_one_step, teleport_to_location) to use agent.pos/agent.move_to(...) and introduced a @requires decorator intended to express tool preconditions.
Added/updated tests to exercise the new position/movement behaviors and some tool-decorator schema parsing.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`mesa_llm/llm_agent.py`	Adds unified `pos` property and `move_to(...)`; simplifies observation location to use `self.pos`.
`mesa_llm/tools/inbuilt_tools.py`	Removes `_get_agent_position`, switches to `agent.pos` + `agent.move_to(...)`, adds `@requires` annotations.
`mesa_llm/tools/tool_decorator.py`	Introduces `requires(...)` and extends `_python_to_json_type` parsing for string annotations.
`mesa_llm/__init__.py`	Exposes `requires` at the package level.
`tests/test_tools/test_inbuilt_tools.py`	Updates `DummyAgent` to provide `pos`/`move_to` consistent with the new unified API.
`tests/test_coverage_gap.py`	Adds tests for `LLMAgent.pos`/`move_to`, async wrappers, and tool schema parsing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copilot · 2026-03-07T14:24:04Z

mesa_llm/tools/tool_decorator.py

+    def decorator(func: Callable):
+        if not hasattr(func, "__tool_requirements__"):
+            func.__tool_requirements__ = []
+        func.__tool_requirements__.append(
+            {"precondition": precondition, "reason": reason}
+        )
+        return func


requires stores preconditions on __tool_requirements__, but nothing in the codebase reads/enforces this attribute (e.g., ToolManager executes tools unconditionally). As a result, the new @requires usage in tools is currently a no-op and won't prevent invalid tool execution. Either integrate requirement checking into ToolManager (before calling the tool) and surface the optional reason, or remove requires to avoid a misleading public API.

Copilot · 2026-03-07T14:24:04Z

mesa_llm/tools/inbuilt_tools.py

@@ -82,7 +74,7 @@ def move_one_step(agent: "LLMAgent", direction: str) -> str:

    grid = getattr(agent.model, "grid", None)
    if isinstance(grid, OrthogonalMooreGrid | OrthogonalVonNeumannGrid):


move_one_step now unconditionally unpacks agent.pos (e.g., row, col = agent.pos). If agent.pos is None or not a 2-tuple, this will raise a TypeError instead of returning a clear error as before. Since @requires(...) is not enforced anywhere, this needs an explicit runtime check (raise ValueError with a helpful message) or the requirements mechanism needs to be wired into tool execution.

Suggested change

if isinstance(grid, OrthogonalMooreGrid | OrthogonalVonNeumannGrid):

if isinstance(grid, OrthogonalMooreGrid | OrthogonalVonNeumannGrid):

# Validate agent.pos before unpacking to avoid obscure TypeError.

if not isinstance(agent.pos, (tuple, list)) or len(agent.pos) != 2:

raise ValueError(

"Agent position must be a 2-tuple (row, col) for orthogonal grids; "

f"got: {agent.pos!r}"

)

Copilot · 2026-03-07T14:24:05Z

tests/test_coverage_gap.py

+    loop = asyncio.new_event_loop()
+    asyncio.set_event_loop(loop)
+    loop.run_until_complete(run_astep())
+    assert agent.step_called is True
+
+


This test creates and sets a new global event loop but never closes it (and doesn't restore the previous loop). That can leak resources and cause cross-test interference. Prefer asyncio.run(...), pytest.mark.asyncio with await, or ensure the loop is closed and the prior loop is restored in a try/finally.

Suggested change

loop = asyncio.new_event_loop()

asyncio.set_event_loop(loop)

loop.run_until_complete(run_astep())

assert agent.step_called is True

asyncio.run(run_astep())

assert agent.step_called is True

Copilot · 2026-03-07T14:24:05Z

tests/test_coverage_gap.py

+@tool
+def string_annotated_tool(agent, data: "list[int]") -> str:
+    """A tool with string annotations.
+    Args:
+        data: describe data
+    """
+    return str(data)
+
+
+def test_string_annotated_tool_schema():


Defining a @tool at module import time mutates the global tool registry (_GLOBAL_TOOL_REGISTRY) as a side effect of importing this test module, which can make unrelated tests order-dependent. Consider defining this tool inside the test and/or clearing/restoring _GLOBAL_TOOL_REGISTRY around the assertions.

Suggested change

@tool

def string_annotated_tool(agent, data: "list[int]") -> str:

"""A tool with string annotations.

Args:

data: describe data

"""

return str(data)

def test_string_annotated_tool_schema():

def test_string_annotated_tool_schema():

@tool

def string_annotated_tool(agent, data: "list[int]") -> str:

"""A tool with string annotations.

Args:

data: describe data

"""

return str(data)

wang-boyu · 2026-03-07T15:26:54Z

I would prefer to have the unified spatial API provided by Mesa, not part of Mesa-LLM.

Standardized LLMAgent.pos
Added a @property in LLMAgent that resolves the correct coordinates for both:

legacy mesa.space (using self.pos)

new mesa.discrete_space (using self.cell.coordinate)

Unified Movement
Added LLMAgent.move_to(target_coordinates) to encapsulate movement logic for different Mesa space types.

Hi @quaquel @EwoutH, are functions like these of interest to you in Mesa?

mesa.experimental.continuous_space.ContinuousSpaceAgent has position property, but no move_to(position) method
mesa.discrete_space.CellAgent has move_to(cell) method, but no position property or a move_to(position) method. Its position can be accessed through self.cell.coordinate or self.cell.position.

Would it be feasible & useful to have a unifed position property and move_to(position) method for all agent types in Mesa?

hillhack · 2026-03-07T15:53:57Z

i made these changes when i have old version of mesa on my system. As in new version space is removed and lots of development happen. so i need to revise the changes again.

feat: standardize spatial access across mesa space types

66e36ea

hillhack closed this Mar 5, 2026

hillhack reopened this Mar 5, 2026

hillhack force-pushed the jyoti_standardize-spatial branch from a251c82 to 2b3fa66 Compare March 6, 2026 02:16

test: update inbuilt tool tests for spatial standardization

002c82c

hillhack force-pushed the jyoti_standardize-spatial branch from addbfb9 to 002c82c Compare March 6, 2026 02:20

[pre-commit.ci] auto fixes from pre-commit.com hooks

82282b9

for more information, see https://pre-commit.ci

hillhack changed the title ~~Fix: issues/150~~ Spatial API Unification Mar 6, 2026

colinfrisch requested a review from Copilot March 7, 2026 14:19

colinfrisch assigned wang-boyu, colinfrisch and sanika-n Mar 7, 2026

Copilot started reviewing on behalf of colinfrisch March 7, 2026 14:20 View session

Copilot AI reviewed Mar 7, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Spatial API Unification#151

Spatial API Unification#151
hillhack wants to merge 3 commits intomesa:mainfrom
hillhack:jyoti_standardize-spatial

hillhack commented Mar 5, 2026 •

edited

Loading

Uh oh!

coderabbitai bot commented Mar 5, 2026 •

edited

Loading

Review skipped

Uh oh!

codecov bot commented Mar 5, 2026 •

edited

Loading

Uh oh!

colinfrisch commented Mar 7, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

wang-boyu commented Mar 7, 2026

Uh oh!

hillhack commented Mar 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		@@ -82,7 +74,7 @@ def move_one_step(agent: "LLMAgent", direction: str) -> str:

		grid = getattr(agent.model, "grid", None)
		if isinstance(grid, OrthogonalMooreGrid \| OrthogonalVonNeumannGrid):

-    if isinstance(grid, OrthogonalMooreGrid | OrthogonalVonNeumannGrid):
+    if isinstance(grid, OrthogonalMooreGrid | OrthogonalVonNeumannGrid):
+        # Validate agent.pos before unpacking to avoid obscure TypeError.
+        if not isinstance(agent.pos, (tuple, list)) or len(agent.pos) != 2:
+            raise ValueError(
+                "Agent position must be a 2-tuple (row, col) for orthogonal grids; "
+                f"got: {agent.pos!r}"
+            )

Uh oh!

Conversation

hillhack commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

codecov bot commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

colinfrisch commented Mar 7, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

wang-boyu commented Mar 7, 2026

Uh oh!

hillhack commented Mar 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

hillhack commented Mar 5, 2026 •

edited

Loading

coderabbitai bot commented Mar 5, 2026 •

edited

Loading

codecov bot commented Mar 5, 2026 •

edited

Loading