Standardize and simplify move_one_step failure messages#306
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
sadrasabouri
left a comment
There was a problem hiding this comment.
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." |
There was a problem hiding this comment.
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." |
| 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." |
| f"Agent {agent.unique_id} is at the boundary and cannot move " | ||
| f"{direction}. Try a different direction." | ||
| ) | ||
| else: |
There was a problem hiding this comment.
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}." |
This pr have a simple approach.