Skip to content

Standardize and simplify move_one_step failure messages#306

Open
niteshver wants to merge 4 commits into
mesa:mainfrom
niteshver:move_onestep_tool_improve
Open

Standardize and simplify move_one_step failure messages#306
niteshver wants to merge 4 commits into
mesa:mainfrom
niteshver:move_onestep_tool_improve

Conversation

@niteshver

@niteshver niteshver commented Apr 26, 2026

Copy link
Copy Markdown
Contributor

This pr have a simple approach.

  1. Improved consistency and readability of failure messages in move_one_step.
  2. Replaced verbose and inconsistent outputs with concise, standardized one-line messages and removed redundant phrasing.

@coderabbitai

coderabbitai Bot commented Apr 26, 2026

Copy link
Copy Markdown
Contributor

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: 63b6f94d-566f-4398-bc05-6f526c60947f

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

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.

@niteshver niteshver changed the title Refactor move_one_step failure message for clarity and brevity Standardize and simplify move_one_step failure messages Apr 26, 2026

@sadrasabouri sadrasabouri left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @niteshver, thanks for contributing to mesa-llm.

I reviewed your PR as an external reviewer to make it ready for maintainers' review.
Try to scope down your PRs to the discussed changes (either in GitHub issues/discussions or the Mesa communication channel), or make it up to a simple refactoring change. In the latter case, reason in your PR description why your changes are a good one.

else:
target_cell = grid._cells.get(new_pos)
if target_cell is None:
return f"Agent {agent.unique_id} cannot move {direction}: boundary reached."

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try not to change messages before discussing the change with the team.

f"Agent {agent.unique_id} cannot move {direction} because "
"the target cell is full."
)
return f"Agent {agent.unique_id} cannot move {direction}: target cell is occupied."

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

f"Agent {agent.unique_id} cannot move {direction} because "
"the target cell is occupied."
)
return f"Agent {agent.unique_id} cannot move {direction}: target cell is occupied."

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

f"Agent {agent.unique_id} is at the boundary and cannot move "
f"{direction}. Try a different direction."
)
else:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you remove the elif new_pos not in grid._cells: condition? The proposed change alters the coordination check logic.

)

return f"agent {agent.unique_id} moved to {target_coordinates}."
return f"Agent {agent.unique_id} moved to {target_coordinates}."

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants