Skip to content

fix: rewrite tool descriptions flagged by Claude connector directory review - #273

Merged
mattpodwysocki merged 5 commits into
mainfrom
ctufts/claude-connector-changes
Oct 13, 2026
Merged

mattpodwysocki merged 5 commits into
mainfrom
ctufts/claude-connector-changes

Conversation

@ctufts

@ctufts ctufts commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Description

The Claude connector directory review flagged three tool descriptions that told Claude to avoid other tools or sources, or to call a tool when nobody asked for it. This PR rewrites them so each one says what the tool does and when it's useful.

  • render_map_tool no longer calls itself the preferred map tool, tells Claude to avoid built-in map tools, or says to run as the final step after every Mapbox result. It now says it's for when someone wants to see a location, route or dataset on a map.
  • ground_location_tool no longer says to use it as the first and only tool, or to skip web search. Instead it says it already does reverse geocoding, so Claude doesn't need a separate reverse_geocode_tool call for the same coordinate.
  • category_list_tool (already deprecated) no longer tells Claude to page through every category before doing anything else. It just says it supports limit/offset pagination.

Tool names, schemas and behavior don't change, so nothing breaks for MCP clients or for code that imports the tools directly.

What users will notice: Claude stops drawing a map after every search or route, and draws one when someone asks or when a map clearly helps. When someone does ask for a map, Claude might occasionally pick the host's built-in map tool, but the server instructions still send map requests to render_map_tool, so that should be rare. For questions about a place, Claude may now run a web search alongside ground_location_tool, which means more sources in the answer.


Testing

  • npm run build passes.
  • npm test: 979 of 980 pass. The one failure is in test/utils/urlSafety.test.ts (IPv4-in-IPv6 encoding check). It fails the same way on main and isn't related to this change.
  • npm run lint: no errors. The 35 warnings were already there.
  • Grepped src to confirm none of the flagged phrases are left.

Checklist

  • Tests added or updated (npm test passes): no new tests, since only description text changed and no test asserts on it
  • Lint passes (npm run lint)
  • CHANGELOG.md updated under Unreleased
  • Documentation updated if needed (README, JSDoc)

🤖 Generated with Claude Code

…review

render_map_tool, ground_location_tool and category_list_tool descriptions
no longer tell Claude to avoid other tools or sources, or to call a tool
unprompted. Tool names, schemas and behavior are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ctufts ctufts added the locationai-team-skills Opened via the location-ai PR creation skill label Oct 11, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ctufts
ctufts marked this pull request as ready for review October 11, 2026 16:43
@ctufts
ctufts requested a review from a team as a code owner October 11, 2026 16:43

@mattpodwysocki mattpodwysocki left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice, this covers everything they flagged. I grepped the rest of the tool descriptions for always/never/only/prefer/instead/etc. and everything left is just routing between our own tools, which seems like exactly what they want.

One thing I'd fix before merging:

  • docs/render-map-tool.md:120 still says the LLM is "instructed (via each tool's own output) to call render_map_tool as the final step whenever a mapboxRender field is present". That's the exact behavior we're pulling out here, so if a reviewer reads the docs it'll look like nothing changed. Maybe just reword it to say Claude calls render_map_tool when someone wants to see the result on a map.

A few smaller things, not blocking:

  • src/utils/storeMapPayload.ts:106 appends "To show this on a map, call render_map_tool (not static_map_image_tool)..." to every result that has a map payload. It's conditional and only compares our own tools so I think it's fine, but text in tool results gets read the same way as descriptions, so just flagging it in case they come back on it.
  • Server instructions: "To show anything on a map, use render_map_tool" is the broadest routing line we have left. They didn't flag it and it doesn't name anyone else's tool, so I'd leave it. If they push back, something like "render_map_tool shows Mapbox results on an interactive map" would probably do it.
  • DirectionsTool.input.schema.ts:147: the geometries description says "The map preview UI still renders the route on its own", which kind of implies a map shows up automatically. Now that we're not forcing a map, maybe something like "the mapboxRender.ref works with render_map_tool either way".

Optional: nothing tests description text, so this stuff could creep back in. A quick test that runs every tool description, param description, and the server instructions through a list of banned phrases (FIRST and ONLY, FINAL step, PREFERRED, search the web, etc.) would keep us honest.

@mattpodwysocki

Copy link
Copy Markdown
Contributor

Ran this PR through the new tool-selection eval (#274) to see if it actually changes what Claude does. Good news: no regressions. Nothing that passed on main fails here, and requested maps still render fine.

It doesn't fully fix the "maps nobody asked for" thing though. Drive times, museum lists, multi-stop ordering and review questions still end in a render_map_tool call at about the same rate as main. In every one of those transcripts the trigger is the hint our data tools append to their results ("📍 To show this on a map, call render_map_tool..."), which this PR doesn't touch.

I tried making the hint conditional, one line in src/utils/storeMapPayload.ts:

-    `\n\n📍 To show this on a map, call render_map_tool (not static_map_image_tool) with this ref unchanged:\n` +
+    `\n\nIf the user wants to see this on a map, pass this ref unchanged to render_map_tool (not static_map_image_tool):\n` +

I kept the literal render_map_tool({ "payload_refs": [...] }) line since that's what stops Claude from inventing refs.

Results (Opus 5.5, 17 prompts x 2 loading modes x 3 reps):

main this PR this PR + hint change
Pass 75/102 80/102 89/102
Map drawn when nobody asked 27 22 13
Requested maps still drawn 24/24 24/24 24/24
Map ref passed unchanged 102/102 102/102 102/102

Drive time went from 0/6 to 6/6 and reviews from 0/3 to 2/3. The remaining misses are museum lists and multi-stop routes, where Claude decides a map helps, which seems fine to me.

If the reviewer tests anything like this, the hint change seems worth folding in before we resubmit. No tests assert the hint text, so it's a one-line change. Happy to push it to your branch if you want.

mattpodwysocki
mattpodwysocki previously approved these changes Oct 13, 2026

@mattpodwysocki mattpodwysocki left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. The rewrites cover everything the connector review called out, and I didn't find anything else in the descriptions that steers away from other tools or web search. What's left (the MUST/IMPORTANT bits in optimization, the FIRST ring in polygonSchema) are parameter rules, not routing.

The eval from #274 backs it up: no regressions on Claude or chat-latest, requested maps still render, and unrequested maps go down (27 to 22 rows on Claude, 13 to 4 on chat-latest).

Two things:

  • This conflicts with main now that #274 landed, just the CHANGELOG. Keep both entries under Unreleased (your Bug Fixes, the eval's Other Features). Merge main in rather than rebasing so the approval sticks.
  • The render hint in storeMapPayload.ts ("📍 To show this on a map, call render_map_tool...") is basically the old FINAL step instruction coming back through tool results, and it's where most of the remaining unrequested maps come from. Fine as a follow-up if you'd rather keep this PR to what the reviewer listed, but I'd do the conditional wording from my earlier comment before resubmitting.

ctufts and others added 2 commits October 13, 2026 14:51
- storeMapPayload render hint now applies only when the user wants a map,
  keeping the literal payload_refs call so refs are passed unchanged
- docs/render-map-tool.md no longer says render_map_tool is the final step
- directions geometries description no longer implies an automatic map

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@mattpodwysocki mattpodwysocki left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice, thanks for picking up the hint change. The wording in storeMapPayload.ts is exactly what I ran through the eval (89/102 vs 75 on main, requested maps still 24/24), and the directions_tool and docs tweaks go the same direction.

One small nit: the CHANGELOG mentions the hint but not the directions_tool description change ("if the user wants to see the route on a map, pass it to render_map_tool"). Worth a line so it's in the release notes with the rest. Not blocking.

@mattpodwysocki
mattpodwysocki merged commit deb5404 into main Oct 13, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

locationai-team-skills Opened via the location-ai PR creation skill

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants