Skip to content

Adding Current location via agument.#1

Open
LoPromise wants to merge 3 commits into
X-McKay:mainfrom
LoPromise:main
Open

Adding Current location via agument.#1
LoPromise wants to merge 3 commits into
X-McKay:mainfrom
LoPromise:main

Conversation

@LoPromise

Copy link
Copy Markdown

Description

Adds Current location via argument and provides it to the LLM
everything is tested and runs with lm sudio

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request introduces a new get_current_location tool and adds support for configuring a default location via command-line arguments or environment variables. It also renames existing tools for better consistency and updates documentation and configuration files to include JSON schemas and refined setup instructions. Review feedback suggests improving the CLI argument parsing to support standard space-separated formats, enhancing error handling within the new tool to prevent misleading success messages, and fixing tool naming inconsistencies in the configuration documentation.

Comment thread src/geocode_mcp/server.py Outdated
Comment thread src/geocode_mcp/server.py
Comment on lines +199 to +208
geo_result = await geocode_location(DEFAULT_LOCATION, limit=1)

response = {
"query": DEFAULT_LOCATION,
"results_count": len(geo_result.get("coordinates", [])),
"coordinates": geo_result.get("coordinates", []),
"message": f"Current default location: {DEFAULT_LOCATION}",
}

return [types.TextContent(type="text", text=json.dumps(response, indent=2))]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

If geocode_location fails (e.g., returns an error dictionary because the location wasn't found), the current implementation swallows the error and returns an empty results list with a misleading success message. It's better to propagate the error or include it in the response.

Suggested change
geo_result = await geocode_location(DEFAULT_LOCATION, limit=1)
response = {
"query": DEFAULT_LOCATION,
"results_count": len(geo_result.get("coordinates", [])),
"coordinates": geo_result.get("coordinates", []),
"message": f"Current default location: {DEFAULT_LOCATION}",
}
return [types.TextContent(type="text", text=json.dumps(response, indent=2))]
# Geocode the default location to get coordinates
geo_result = await geocode_location(DEFAULT_LOCATION, limit=1)
# Add a message to the result for context if successful
if "error" not in geo_result:
geo_result["message"] = f"Current default location: {DEFAULT_LOCATION}"
return [types.TextContent(type="text", text=json.dumps(geo_result, indent=2))]

Comment thread config/README.md

Once configured, the MCP server provides:

- **`mcp_geocoding_get_coordinates`**: Get latitude/longitude coordinates for any location

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The tool name here is inconsistent with the implementation in server.py and the root README.md, which use get_coordinates without the mcp_geocoding_ prefix.

Suggested change
- **`mcp_geocoding_get_coordinates`**: Get latitude/longitude coordinates for any location
- **`get_coordinates`**: Get latitude/longitude coordinates for any location

Comment thread config/README.md
- Uses OpenStreetMap Nominatim API (free, no API key required) No newline at end of file
- Uses OpenStreetMap Nominatim API (free, no API key required)

- **`mcp_geocoding_get_current_location`**: Get the current default location configured for this MCP server

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The tool name here is inconsistent with the implementation in server.py and the root README.md, which use get_current_location without the mcp_geocoding_ prefix.

Suggested change
- **`mcp_geocoding_get_current_location`**: Get the current default location configured for this MCP server
- **`get_current_location`**: Get the current default location configured for this MCP server

Comment thread README.md

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

duplicate maybe clean that up while we are at it.

LoPromise and others added 2 commits April 7, 2026 11:03
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
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.

1 participant