the mesh drive/navigate tools ship 13 placeholder parameter descriptions - convention 13's guard cannot see a @tool defined outside strands_robots/tools/, and the units are documented one layer below the schema

## What I measured Convention 13 says every parameter an agent tool exposes needs its own `Args:` entry, because `docstring_parser` derives the input schema from the docstring and the decorator substitutes `"Parameter <name>"` for anything it cannot find there. The convention names the harm exactly: *"a placeholder makes the parameter undiscoverable however carefully the source explains it."* That is the situation in `strands_robots/mesh/`. The four parameterised `@tool` closures returned by the `.tools` property have **no docstring at all**, so every one of their parameters reaches the model as a placeholder. Measured at `2efb05f` by instantiating each class and reading `tool_spec["inputSchema"]["json"]["properties"]`: | tool | placeholders | defined at | |---|---|---| | `drive_<suffix>` (`RosBridgedRobot`) | `linear`, `angular`, `duration` | `mesh/ros_bridge.py:338` | | `navigate_<suffix>` (`RosBridgedRobot`) | `x`, `y`, `yaw`, `timeout` | `mesh/ros_bridge.py:356` | | `drive_<suffix>` (`RosbridgeRobot`) | `linear`, `angular`, `duration` | `mesh/rosbridge_robot.py:301` | | `drive_<suffix>` (`RtpsRobot`) | `linear`, `angular`, `duration` | `mesh/rtps_robot.py:191` | **13 placeholder parameters across 4 tools.** Reproduced directly: ``` RtpsRobot("/turtle1", "/turtle1/cmd_vel").tools -> drive_turtle1 {'linear': 'Parameter linear', 'angular': 'Parameter angular', 'duration': 'Parameter duration'} RosBridgedRobot(node_name="/r2", ..., nav_action="/navigate_to_pose").tools -> navigate_r2 {'x': 'Parameter x', 'y': 'Parameter y', 'yaw': 'Parameter yaw', 'timeout': 'Parameter timeout'} ``` The remaining six guard-invisible tools (`stop`, `get_pose`, `get_scan`) take no parameters, so they cannot carry a placeholder and are not part of the defect. ## The information exists - one layer below where the schema is generated This is not a case of undocumented behaviour. Each closure delegates to a bound method whose docstring already carries a full, careful `Args:` section stating exactly what the schema is missing. `RosBridgedRobot.drive` (`ros_bridge.py:151`): ``` linear: Forward linear velocity (m/s), mapped to linear.x. Must be a finite number; both signs are valid (negative reverses). angular: Yaw angular velocity (rad/s), mapped to angular.z. ... duration: When given, hold the command for this many seconds ... must be > 0 and finite - a zero or negative hold has no message count that expresses it, and publishing a single velocity command anyway would start the robot moving. ``` and `RosBridgedRobot.navigate_to` (`ros_bridge.py:253`) documents `x` / `y` in meters, `yaw` in **radians**, `timeout` as a seconds budget, and that the goal is cancelled when it expires "so the robot does not keep navigating unattended". None of that reaches the model. The schema is the only thing the model reads, so on these tools it cannot discover that `linear` is m/s rather than a normalised -1..1 throttle, that `duration` is seconds, or that `yaw` is radians rather than degrees - on tools whose effect is to move a physical robot. ## The harm is graded, not uniform - 10 of the 13 are unrecoverable I checked whether the loss is recovered by the `description=` prose, which does reach the model: | tool | description carries units? | unrecoverable | |---|---|---| | `drive_r1` (`RosbridgeRobot`) | yes - "linear m/s up to 2.0, angular rad/s up to 1.0, optional duration s", plus the latch/auto-stop contract | 0 of 3 | | `drive_r2` (`RosBridgedRobot`) | no - "Drive the /r2 robot (linear/angular velocity)." | 3 of 3 | | `drive_turtle1` (`RtpsRobot`) | no - "Drive the /turtle1 robot over RTPS (linear/angular velocity)." | 3 of 3 | | `navigate_r2` (`RosBridgedRobot`) | frame only - names the map frame, no unit for any of the four | 4 of 4 | **10 of 13 have no unit or semantics anywhere in what the model reads.** `RosbridgeRobot.drive` is the control: it recovers all three in shared prose, which is why it should keep its description unchanged and is evidence the other three are an omission rather than a house style. The two `duration` cases are the sharpest. `RosbridgeRobot`'s description states the contract - "a command with duration stops automatically afterwards; without duration the last command latches until stop". For `drive_r2` and `drive_turtle1` the model is told neither the unit nor that omitting `duration` **latches a velocity until `stop`**, though both methods implement and document it. ## Root cause: the guard's discovery is scoped to a directory, not to the package `tests/tools/test_agent_tool_parameter_descriptions.py` is the convention-13 pin, and it is a good guard - no allowlist, no exemptions, both the consumer direction (no property description equals the placeholder) and the producer direction (every `Args:` name matches a declared parameter), with a 16-name `_EXPECTED_TOOLS` tripwire so a scan that resolves nowhere fails instead of passing over nothing. It reports `41 passed`. Its comment states the intent: *"Derived from the package object rather than a path literal, so a module added later is covered without an edit."* That defends against a new **module** inside `strands_robots/tools/`. It does not defend against a `@tool` defined **anywhere else**, because discovery is `pkgutil.iter_modules([_TOOLS_DIR])`. AST scan of the package: **26** `@tool` functions, **16** covered, **10** invisible - all 10 in `strands_robots/mesh/`. They are invisible for two independent reasons: they sit outside the scanned directory, and they are closures inside a `tools` property rather than module attributes, so `vars(module)` would not find them even if the directory were widened. No other test in `tests/` asserts on the placeholder string, so nothing catches these. So the convention is written down and pinned, and holds for everything the pin can see - and the tools it cannot see are the ones that drive robots. ## Proposed shape 1. Give each of the four closures a docstring with an `Args:` entry per parameter, sourced from the bound method's existing wording so the two cannot diverge in substance. 2. Widen the guard from "modules in `strands_robots/tools/`" to "every `@tool` in the package", including instance-bound ones, and keep it an exact expected set so a tool added later is triaged rather than joining an untested set silently. The fix is **purely additive by construction**: all four closures already pass an explicit `description=`, which takes precedence over the docstring's short description. Verified on the installed `strands` build - adding a docstring to a `@tool(description=...)` function leaves `tool_spec["description"]` byte-identical and changes only the per-parameter descriptions: ``` description: "EXPLICIT description stays." (identical with and without a docstring) linear: 'Parameter linear' -> 'Forward linear velocity (m/s).' ``` So no existing tool description moves, and the closures' narrower surface is preserved - note they deliberately expose only `linear` / `angular` / `duration` and not the methods' `count` / `frame_id`. That curation is exactly why the docstring has to be written rather than inherited from the method. ## One open design choice Widening the guard needs the tools to *exist*, and these are instance-bound: discovery requires constructing a `RosBridgedRobot` / `RosbridgeRobot` / `RtpsRobot`, and `navigate` only appears when `nav_action` is set (`get_scan` only when `scan_topic` is). Two candidate shapes: - **A** - AST-only: find every `@tool` in the package statically and assert each has an `Args:` entry per parameter. Needs no construction and no optional dependency, but checks the docstring rather than the schema the model actually receives, so it cannot catch a placeholder that arises any other way. - **B** - construct the three mesh classes with fully-populated optional wiring and read the real `tool_spec`. Exact, and covers the conditional tools, but the guard then carries construction knowledge and must be extended when a class is added. A and B fail on disjoint inputs, so a survey doing both is defensible; I would rather have the preference stated than pick silently. ## Sequencing Not proposing a PR yet on purpose. #2130 is open, green and unreviewed, and it modifies all three of these files (`mesh/ros_bridge.py`, `mesh/rosbridge_robot.py`, `mesh/rtps_robot.py`). Landing a second branch across the same three files would create exactly the base-overlap the "Detect an untested overlap with the base branch" check exists to flag, and would force a rebase and re-verification on whichever merges second. I will pick this up once #2130 is resolved. Measurements taken at `2efb05f`.