From e5ed8a19f9c2e109aa26da23d57be6d99f6d255b Mon Sep 17 00:00:00 2001 From: "Josef M. Gallmetzer" <64498081+galjos@users.noreply.github.com> Date: Thu, 10 Sep 2026 13:38:46 +0200 Subject: [PATCH] Prepare v1.2.0 --- .github/workflows/ci.yml | 13 ++- .github/workflows/release.yml | 29 +++++-- CHANGELOG.md | 31 +++++++ README.md | 34 +++++--- evals/agent/recipes.json | 44 ++++++++-- evals/agent/tasks.json | 22 ++++- pyproject.toml | 6 +- scripts/smoke-installed.py | 63 ++++++++++++++ skills/pitstop/SKILL.md | 13 ++- src/pitstop/cache.py | 36 ++++++++ src/pitstop/chargers.py | 42 ++++++++-- src/pitstop/cli.py | 126 +++++++++++++++++++--------- src/pitstop/core.py | 88 ++++++++++++++------ src/pitstop/cpo_tariffs.py | 4 +- src/pitstop/geocoding.py | 152 +++++++++++++++++++++++++++++----- src/pitstop/mcp_server.py | 104 ++++++++++++++--------- src/pitstop/overpass.py | 60 ++++++++++---- src/pitstop/results.py | 22 +++++ src/pitstop/validation.py | 52 ++++++++++++ src/pitstop/version.py | 2 +- tests/test_chargers.py | 49 ++++++++++- tests/test_core.py | 25 ++++++ tests/test_geocoding.py | 79 ++++++++++++++++++ tests/test_query_reuse.py | 39 +++++++++ tests/test_validation.py | 59 +++++++++++++ 25 files changed, 1009 insertions(+), 185 deletions(-) create mode 100644 scripts/smoke-installed.py create mode 100644 src/pitstop/cache.py create mode 100644 src/pitstop/results.py create mode 100644 src/pitstop/validation.py create mode 100644 tests/test_geocoding.py create mode 100644 tests/test_query_reuse.py create mode 100644 tests/test_validation.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2ea7aea..bcd67c6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,15 +20,20 @@ jobs: - run: pip install -e ".[dev]" - run: pytest -q - mcp-import: + mcp-protocol: runs-on: ubuntu-latest + strategy: + matrix: + mcp-requirement: ["mcp==1.12.0", "mcp<2"] steps: - uses: actions/checkout@v4 - uses: actions/setup-python@v5 with: python-version: "3.12" - - run: pip install -e ".[mcp]" - - run: python -c "import pitstop.mcp_server; print('mcp server imports OK')" + - run: pip install ".[mcp]" "$MCP_REQUIREMENT" + env: + MCP_REQUIREMENT: ${{ matrix.mcp-requirement }} + - run: python scripts/smoke-installed.py build: runs-on: ubuntu-latest @@ -39,6 +44,8 @@ jobs: python-version: "3.12" - run: pip install build - run: python -m build + - run: pip install --no-deps dist/*.whl + - run: python scripts/smoke-installed.py # Offline half of the eval suite: the eval files match the shipped parser, so a # recipe or task can never name a command the CLI cannot run. The live half diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index b590e4d..1ef4564 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -7,7 +7,7 @@ on: workflow_dispatch: inputs: version: - description: Existing tag to release, for example v1.0.2 + description: Existing tag to release, for example v1.2.0 required: true type: string @@ -15,6 +15,9 @@ permissions: contents: write id-token: write +env: + RELEASE_TAG: ${{ inputs.version || github.ref_name }} + jobs: build: name: Build distributions @@ -22,18 +25,34 @@ jobs: steps: - uses: actions/checkout@v4 with: - ref: ${{ github.event.inputs.version || github.ref_name }} + ref: ${{ github.event_name == 'workflow_dispatch' && format('refs/tags/{0}', inputs.version) || github.ref }} - uses: actions/setup-python@v5 with: python-version: "3.12" + - name: Check tag and package versions agree + run: | + python - <<'PY' + import os, runpy, tomllib + from pathlib import Path + tag = os.environ["RELEASE_TAG"] + package = tomllib.loads(Path("pyproject.toml").read_text())["project"]["version"] + runtime = runpy.run_path("src/pitstop/version.py")["__version__"] + if tag != f"v{package}" or runtime != package: + raise SystemExit(f"Version mismatch: tag={tag}, package={package}, runtime={runtime}") + PY + - run: python -m pip install --upgrade build twine - run: python -m build - run: python -m twine check dist/* + - run: python -m pip install --no-deps dist/*.whl + + - run: python scripts/smoke-installed.py + - uses: actions/upload-artifact@v4 with: name: pitstop-cli-dist @@ -51,11 +70,7 @@ jobs: - name: Resolve release tag run: | - if [[ "${{ github.event_name }}" == "workflow_dispatch" ]]; then - echo "VERSION=${{ inputs.version }}" >> "$GITHUB_ENV" - else - echo "VERSION=${GITHUB_REF_NAME}" >> "$GITHUB_ENV" - fi + echo "VERSION=$RELEASE_TAG" >> "$GITHUB_ENV" - name: Publish release env: diff --git a/CHANGELOG.md b/CHANGELOG.md index 2387e4d..a5b64e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,37 @@ All notable changes to this project are documented here. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project aims to follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.2.0] - 2026-09-10 + +### Added + +- Municipality discovery through `pitstop places` and the `find_places` MCP tool; + charger searches accept province and ISTAT ID and report the resolved location. +- Search coverage counts and cache timestamps in JSON and GeoJSON envelopes. +- Read-only MCP annotations and structured results; installed CLI/MCP checks + cover the minimum supported SDK (1.12) and the current 1.x SDK. +- Standard `--version` shortcut and charger download timeout/cache-age flags. + +### Fixed + +- Replace Alperia and Neogy tariff links that returned HTTP 404 with their + current official charging-offer and direct-payment pages. +- CLI and MCP searches reject non-finite or out-of-range coordinates, + nonpositive radii, and negative limits before fetching data. Invalid CLI + searches return exit code 2 instead of empty results or invalid JSON. +- Freshness filters exclude prices with missing, unparseable, or future update + dates. Unfiltered queries still include those reported prices. +- Charger municipality centers come from OSM administrative relations keyed by + ISTAT ID, correcting the misplaced Bolzano reference. Duplicate town names + require disambiguation instead of silently selecting a different province. +- Fuel queries no longer mutate shared station/price objects. Parsed datasets + and statistics are reused between MCP calls and invalidated after a refresh. +- Cache writes use unique temporary files; malformed Overpass caches are retried + and valid empty caches retain their provenance during an upstream failure. +- Charger results exclude invalid coordinates and centers outside the radius. +- Release workflows check out the requested tag and verify that tag, package, + and runtime versions agree before publication. + ## [1.1.1] - 2026-09-07 ### Fixed diff --git a/README.md b/README.md index b112e0a..49b64b0 100644 --- a/README.md +++ b/README.md @@ -6,11 +6,11 @@ A JSON-first CLI and MCP server for **Italian fuel-station prices** and **EV charging stations**, designed for AI agents, scripts, and humans. -Italy publishes per-station fuel prices daily (MIMIT *Osservaprezzi Carburanti* open data) and OpenStreetMap maps every EV charger in the country, but raw access means downloading multi-megabyte CSVs, joining files, sorting through misreports, and translating between Italian comune names and the ones a user actually types. `pitstop` does all of that locally and returns a small, well-formed JSON envelope. +Italy publishes per-station fuel prices daily (MIMIT *Osservaprezzi Carburanti* open data), and OpenStreetMap provides EV charger locations and capabilities. Raw access means downloading multi-megabyte CSVs, joining files, sorting through misreports, and translating municipality names. `pitstop` handles those steps locally and returns JSON or a readable table. Built for questions like: - *"What's the cheapest diesel near Rome right now?"* → `pitstop stations --fuel Gasolio --near 41.9,12.5 --cheapest` -- *"Where can I fast-charge my EV in Bolzano?"* → `pitstop chargers --comune Bozen --fast` +- *"Where can I fast-charge my EV in Bolzano?"* → `pitstop chargers --near 46.498,11.354 --fast` - *"Are these station prices statistically reliable?"* → every price says whether it was screened against its local market; screened ones carry a `regional_median`, a `deviation_pct`, and `outlier: true` when the price looks like a misreport. International city names work out of the box (`Rome`, `Milan`, `Bozen`, `Mailand`, `Venise`, …). @@ -26,8 +26,9 @@ Unofficial community project. Not affiliated with or endorsed by MIMIT. Fuel dat - **Coverage:** Italy only (by design, for now). - **Known caveats:** - Some operators report placeholder values (e.g. `1.000`); use `--min-price` (e.g. `1.2`) to drop them. - - Some price records are **stale** (a few were last updated years ago); use `--fresh-within-days` and check the `UPDATED` column / `updated` field. - - Some stations are **mis-geocoded** in the registry. As of v0.4.0 `pitstop` joins a second data source (ISTAT-derived comune coordinates from [opendatasicilia/comuni-italiani](https://github.com/opendatasicilia/comuni-italiani), 97.5% match) to validate each station's coordinate against its declared comune's *true* location. Stations >30 km off are flagged `coordinate_suspect` (`*` in the table), and `--near` excludes stations whose declared comune is geographically too far from the query point — even single-station comuni like RASUN-ANTERSELVA. Pass `--no-comune-validate` to disable. + - Some price records are **stale** (a few were last updated years ago); use `--fresh-within-days` and check the `UPDATED` column / `updated` field. When enabled, the freshness filter also excludes missing, unparseable, and future update dates. + - Some stations are **mis-geocoded** in the registry. `pitstop` uses station clusters and a second coordinate reference from [opendatasicilia/comuni-italiani](https://github.com/opendatasicilia/comuni-italiani) to flag discrepancies over 30 km as `coordinate_suspect` (`*` in the table). `--near` also excludes stations whose declared comune is geographically too far from the query point. Pass `--no-comune-validate` to disable the second reference. + - **Municipality reference coordinates can also be wrong.** Charger searches resolve a municipality's ISTAT ID to its mapped OpenStreetMap administrative center. JSON includes the selected `location`, its source link, cache age, and warnings when it differs from the reference coordinates. Ambiguous names require `--provincia` or `--comune-id`. If no mapped center is available, supply `--near lat,lon`. ## Install @@ -67,10 +68,19 @@ pitstop stations --near 46.498,11.354 --radius 5 --fuel Benzina --self --json pitstop fuels # Fast EV chargers (≥50 kW) within 5 km of Bolzano -pitstop chargers --near 46.498,11.354 --radius 5 --fast --json +pitstop chargers --comune Bozen --radius 5 --fast --public --json + +# Discover municipality IDs and disambiguate duplicate names +pitstop places Livo --json +pitstop chargers --comune Livo --provincia TN --json +pitstop chargers --comune-id 021008 --radius 5 --fast --json ``` -`stations` flags: `--comune`, `--provincia`, `--brand`, `--near "lat,lon"`, `--radius`, `--fuel` (substring, case-insensitive), `--self`, `--served`, `--cheapest` (needs `--fuel`), `--min-price` (drop values below a floor; e.g. `1.2` to skip placeholders), `--fresh-within-days` (drop stale prices), `--max-deviation-pct` (drop prices more than N% below their fuel's provincial median — catches misreports), `--no-comune-validate`, `--limit`, `--json`. Loading flags (`--refresh`, `--max-age`, `--timeout`) apply to the MIMIT data commands (`stations`, `fuels`, `stats`); `chargers` uses its own OSM cache and takes only `--refresh`. +`stations` flags: `--comune`, `--provincia`, `--brand`, `--near "lat,lon"`, `--radius`, `--fuel` (substring, case-insensitive), `--self`, `--served`, `--cheapest` (needs `--fuel`), `--min-price`, `--fresh-within-days`, `--max-deviation-pct`, `--no-comune-validate`, `--limit`, `--json`, `--geojson`. Choose one output format. `--limit 0` returns every match. + +Distances and radii are straight-line measurements, not driving distances or travel times. + +Loading flags (`--refresh`, `--max-age` in seconds, `--timeout` in seconds) apply to MIMIT commands and charger searches. The default caches last 24 hours for MIMIT, seven days for OSM, and 30 days for the municipality reference. `--refresh` also refreshes a charger's municipality lookup; `--max-age` controls its charger results. `--max-age 0` accepts cached files of any age. `places` supports `--refresh` and `--timeout`. Every returned price carries a `median_basis`. A `screened` price also carries `regional_median` and `deviation_pct`, plus `outlier: true` when it is >15% below the local median **or** below the Tukey lower fence Q1−1.5·IQR (the Tukey rule catches misreports in tight markets that the percent rule alone misses). The `outlier` key is emitted **only when it is true**, so read it as optional. Pass `--drop-outliers` to remove flagged prices entirely. @@ -78,7 +88,7 @@ A price is `unscreened` when its (fuel, provincia) bucket holds too few samples ## MCP server -For agents that speak MCP, the same data is exposed as tools (`list_fuels`, `find_stations`, `find_cheapest`, `find_chargers`, `get_stats`) over the shared core: +For agents that speak MCP, the same data is exposed as six tools (`list_fuels`, `find_stations`, `find_cheapest`, `find_chargers`, `find_places`, `get_stats`) over the shared core. Tools advertise read-only behavior and return structured JSON alongside text. Repeated fuel searches reuse parsed files and provincial statistics until the cached files change. ```bash pip install "pitstop-cli[mcp]" # or: uv tool install "pitstop-cli[mcp]" @@ -98,19 +108,23 @@ Machine-readable command recipes, with the caveats that belong with each answer, ```bash pip install -e ".[dev]" pytest -q +python scripts/smoke-installed.py # installed entry points and stdio MCP, offline ``` ## Automation contract - `stdout` is command output; `stderr` is diagnostics. -- `--json` emits a stable object with `source`, `*_extraction_date`, `generated_at`, `query`, `count`, `quality`, `stations[]`, and `disclaimer`. +- `stations --json` emits a stable object with `source`, `*_extraction_date`, `generated_at`, `query`, `count`, `quality`, `stations[]`, and `disclaimer`. +- Fuel and charger search envelopes include `coverage`: `fetched_count`, `matched_count` before the limit, `returned_count`, and `truncated`. These counts describe the downloaded data; OSM coverage is not an exhaustive charger inventory. +- `freshness` records local `fetched_at` and `age_seconds`: separately for the fuel registry and prices, or for the charger response. Charger `cache_status` is `hit`, `miss`, `stale_fallback`, `partial`, or `unavailable`. A partial/unavailable response has no successful fetch timestamp; an `error` explains degraded results. Fetch time does not establish current prices or charger availability. +- Charger municipality searches add `location` with the selected name, province, ISTAT ID, coordinates, provenance, and warnings. GeoJSON carries these envelope fields under `metadata`. - Exit codes: `0` success, `1` runtime error, `2` usage error. -- Source files are cached (default 24h) under `$XDG_CACHE_HOME/pitstop`; use `--refresh` to bypass. +- Source files are cached under `$XDG_CACHE_HOME/pitstop` (or `~/.cache/pitstop`); use `--refresh` to bypass. - Non-interactive; no hidden browser state or scraping. ## Status & roadmap -v1.1.1 — stable public release: fuel-price core (registry+price join, filters, proximity, cheapest, `--min-price` floor, `--fresh-within-days` freshness, combined 15% + Tukey IQR outlier rule, ISTAT comune-coordinate validation, JSON) + **EV charging stations via OSM Overpass** (operator, plug types, max kW, fee, access — `pitstop chargers`) + **operator tariff-page URLs** attached to each EV result. Includes **multi-fuel query support**, **international municipality mapping** (EN/FR/DE), **macro price statistics** (`pitstop stats`), and **navigation/GeoJSON support**. MCP server, agent skill, tests, CI. +v1.2.0 adds municipality discovery, province and ISTAT-ID selection for chargers, search coverage and cache timestamps, and structured MCP results. Fuel prices, EV charger locations, statistics, navigation links, and GeoJSON remain available through the CLI and MCP server. See [CHANGELOG.md](CHANGELOG.md) for fixes and compatibility details. Planned, roughly in order: - per-station **EV tariff data** if a source `pitstop` can read starts publishing per-kWh prices (today it parses only OSM's `fee` yes/no flag, no price field); diff --git a/evals/agent/recipes.json b/evals/agent/recipes.json index 18b9599..25a3703 100644 --- a/evals/agent/recipes.json +++ b/evals/agent/recipes.json @@ -22,7 +22,7 @@ }, { "id": "cheapest-fuel-near-place", - "question_pattern": "Cheapest fuel within driving distance of a coordinate.", + "question_pattern": "Cheapest fuel within a straight-line radius of a coordinate.", "commands": [ "pitstop stations --near 41.9028,12.4964 --radius 5 --fuel Gasolio --cheapest --min-price 1.2 --fresh-within-days 30 --limit 5 --json", "pitstop stations --near 41.9028,12.4964 --radius 5 --fuel Gasolio --cheapest --min-price 1.2 --fresh-within-days 30 --drop-outliers --limit 5 --json" @@ -43,7 +43,8 @@ "`--min-price 1.2` drops the 1.000 placeholder some operators report; leave it off for GPL, whose real prices sit below that floor.", "`--fresh-within-days` drops prices whose `updated` timestamp is older than N days; without it a years-old row can win the ranking.", "A station flagged `coordinate_suspect` sits more than 30 km from where its declared comune's other stations are (their median coordinate, or the ISTAT reference point when the comune has fewer than three stations), or outside Italy altogether. Its `distance_km` is unreliable even when its price is not.", - "Italy only." + "Italy only.", + "distance_km is straight-line distance, not a driving route or journey time." ] }, { @@ -130,14 +131,18 @@ "stations[].operator", "stations[].access", "stations[].distance_km", - "stations[].navigation_url" + "stations[].navigation_url", + "coverage", + "freshness", + "location.warnings[] (when a municipality was used)" ], "caveats": [ "Source is OpenStreetMap via Overpass. Coverage, power, and plug fields are what mappers entered; present them as unverified and name OSM.", "`--fast` is `--min-power 50` over the station's highest tagged socket output, so a charger whose mapper left the power untagged is filtered out rather than reported as slow.", "A populated `error` means the Overpass query failed. With `count: 0` that is \"unknown\", never \"no chargers nearby\"; retry or say the lookup failed.", - "`--comune` centres the search on the municipality's ISTAT reference point, not its town centre. For territorially large municipalities that is far off — Bolzano's sits about 20 km up-valley from the city — so prefer `--near lat,lon` whenever the location is known.", - "Availability is not in this data: it says where chargers are, not whether one is free now." + "`--comune` resolves an ISTAT municipality ID to its mapped OSM administrative center. Inspect `location` and its warnings. Duplicate names need `--provincia` or `--comune-id`; use explicit `--near` when the intended point is known.", + "Availability is not in this data: it says where chargers are, not whether one is free now.", + "Surface cache age and upstream errors. Counts describe inspected OSM data, not an exhaustive or live inventory." ] }, { @@ -151,13 +156,36 @@ "stations[].tariff_info_url", "stations[].operator", "disclaimer", - "error" + "error", + "coverage", + "freshness", + "location.warnings[] (when a municipality was used)" ], "caveats": [ "pitstop returns no per-kWh price for any charger: it parses OpenStreetMap's `fee` yes/no flag and no price field. There is no number to report and none to infer.", - "`fee` is absent when OSM carries no `fee` tag — that is unknown, not free. `--free` keeps every charger not marked `fee=yes`, untagged ones included, so check the field per station.", + "`fee` is absent when OSM carries no `fee` tag — that is unknown, not free. `--free` requires `fee=no`; unknown fees are excluded. This mapper-entered flag is not a current operator price quote.", "`tariff_info_url` links to the operator's own tariff page and is present only for operators in pitstop's curated list; it is the honest answer to a price question.", - "Never carry a figure over from `stations`: fuel prices and charging tariffs are different data from different sources." + "Never carry a figure over from `stations`: fuel prices and charging tariffs are different data from different sources.", + "Surface cache age and upstream errors. Counts describe inspected OSM data, not an exhaustive or live inventory." + ] + }, + { + "id": "municipality-discovery", + "question_pattern": "Find chargers in a municipality whose name may be shared by multiple provinces.", + "commands": [ + "pitstop places Livo --json", + "pitstop chargers --comune Livo --provincia TN --limit 5 --json" + ], + "parse": [ + "places[].comune_id", + "places[].provincia", + "location", + "coverage", + "freshness" + ], + "caveats": [ + "Select the intended province before searching. An ambiguous name is a usage error.", + "A missing mapped center is reported; provide explicit coordinates rather than substituting another municipality." ] } ] diff --git a/evals/agent/tasks.json b/evals/agent/tasks.json index 413ce8f..bd9c55b 100644 --- a/evals/agent/tasks.json +++ b/evals/agent/tasks.json @@ -197,8 +197,8 @@ ], "pass_criteria": [ "Checks each returned station's `fee` field instead of trusting the flag name.", - "States that `--free` keeps every charger not marked `fee=yes`, so results with no `fee` field are unknown rather than free.", - "Separates the chargers explicitly mapped `fee=no` from the untagged ones in the answer.", + "States that `--free` requires an explicit `fee=no`; missing fees are excluded.", + "If comparing against the unfiltered query, labels missing fee tags as unknown.", "Repeats that pitstop reports no per-kWh price and points at `tariff_info_url` where present." ], "common_failures": [ @@ -207,6 +207,24 @@ "Promises a free charge from a mapper-entered flag with no date behind it.", "Invents a tariff for the chargers that do charge." ] + }, + { + "id": "ambiguous-municipality", + "prompt": "Find fast public chargers near Livo in Trentino. Show the selected location and whether the results are complete.", + "expected_command_path": [ + "pitstop places Livo --json", + "pitstop chargers --comune Livo --provincia TN --fast --public --limit 5 --json" + ], + "pass_criteria": [ + "Selects LIVO in TN, ISTAT 022106, rather than LIVO in CO.", + "Reads the resolved location and source; does not invent coordinates when resolution fails.", + "Reports coverage, cache age, and upstream errors without calling OSM exhaustive or live." + ], + "common_failures": [ + "Selects the first substring match without checking province.", + "Presents an empty degraded response as proof that no chargers exist.", + "Uses the old Bolzano reference coordinate as a standing workaround." + ] } ] } diff --git a/pyproject.toml b/pyproject.toml index 516f91f..1c28dce 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "pitstop-cli" -version = "1.1.1" +version = "1.2.0" description = "JSON-first CLI + MCP server for Italian fuel-station prices and EV charging — MIMIT, OpenStreetMap, ISTAT." readme = "README.md" requires-python = ">=3.10" @@ -40,8 +40,8 @@ Issues = "https://github.com/galjos/pitstop-cli/issues" [project.optional-dependencies] # Upper bound is load-bearing: mcp 2.0.0 (2026-07-28) removed mcp.server.fastmcp, # which mcp_server.py imports. Lift it only together with that migration. -mcp = ["mcp>=1.0,<2"] -dev = ["pytest>=7", "mcp>=1.0,<2"] +mcp = ["mcp>=1.12,<2"] +dev = ["pytest>=7", "mcp>=1.12,<2"] [project.scripts] pitstop = "pitstop.cli:main" diff --git a/scripts/smoke-installed.py b/scripts/smoke-installed.py new file mode 100644 index 0000000..8df812f --- /dev/null +++ b/scripts/smoke-installed.py @@ -0,0 +1,63 @@ +"""Exercise the installed CLI and optional stdio MCP server without network access.""" + +import asyncio +import importlib.util +import json +import os +import subprocess +import sys +import tempfile +from pathlib import Path + + +async def check_mcp(env): + from mcp import ClientSession, StdioServerParameters + from mcp.client.stdio import stdio_client + + params = StdioServerParameters(command=sys.executable, args=["-m", "pitstop.mcp_server"], env=env) + async with stdio_client(params) as (read, write): + async with ClientSession(read, write) as session: + await session.initialize() + tools = (await session.list_tools()).tools + assert {t.name for t in tools} == { + "find_stations", "find_cheapest", "find_chargers", "find_places", "list_fuels", "get_stats" + } + assert all(t.annotations and t.annotations.readOnlyHint and + t.annotations.destructiveHint is False for t in tools) + result = await session.call_tool("find_places", {"query": "Livo"}) + assert not result.isError and result.structuredContent["matched_count"] == 2 + assert {p["provincia"] for p in result.structuredContent["places"]} == {"CO", "TN"} + result = await session.call_tool("find_chargers", {"near": "nan,11"}) + assert result.isError + print("Installed MCP: six read-only tools, structured discovery, invalid-input error passed") + + +def main(): + from pitstop.version import __version__ + + with tempfile.TemporaryDirectory(prefix="pitstop-install-smoke-") as directory: + cache = Path(directory) / "pitstop" + cache.mkdir() + (cache / "comuni_main.csv").write_text( + "comune,pro_com_t,lat,long,sigla\n" + "Livo,013130,46.17,9.30,CO\nLivo,022106,46.40,11.02,TN\n" + ) + env = dict(os.environ, XDG_CACHE_HOME=directory) + cli = [str(Path(sys.executable).parent / "pitstop")] + version = subprocess.run([*cli, "--version"], env=env, capture_output=True, text=True, check=True, timeout=15) + assert __version__ in version.stdout + places = subprocess.run([*cli, "places", "Livo", "--json"], env=env, + capture_output=True, text=True, check=True, timeout=15) + assert json.loads(places.stdout)["matched_count"] == 2 + invalid = subprocess.run([*cli, "stations", "--near", "nan,11", "--json"], + env=env, capture_output=True, text=True, timeout=15) + assert invalid.returncode == 2 and invalid.stdout == "" + print("Installed CLI: version, municipality discovery, invalid-input error passed") + if importlib.util.find_spec("mcp"): + asyncio.run(asyncio.wait_for(check_mcp(env), timeout=30)) + else: + print("MCP extra absent; base installation passed") + + +if __name__ == "__main__": + main() diff --git a/skills/pitstop/SKILL.md b/skills/pitstop/SKILL.md index aefa83a..680d7df 100644 --- a/skills/pitstop/SKILL.md +++ b/skills/pitstop/SKILL.md @@ -13,14 +13,14 @@ metadata: { "id": "uvx", "kind": "uvx", - "package": "pitstop-cli>=1.1.1", + "package": "pitstop-cli>=1.2.0", "bins": ["pitstop"], "label": "Run pitstop on demand (uvx)", }, { "id": "pipx", "kind": "pipx", - "package": "pitstop-cli>=1.1.1", + "package": "pitstop-cli>=1.2.0", "bins": ["pitstop"], "label": "Install pitstop (pipx)", }, @@ -58,6 +58,9 @@ pitstop stations --comune ROME --fuel Gasolio --geojson # Find EV chargers with error reporting pitstop chargers --comune Venice --json + +# Public CCS chargers rated at least 50 kW +pitstop chargers --comune Bozen --radius 5 --socket ccs --fast --public --json ``` Key flags: @@ -65,15 +68,21 @@ Key flags: - `--fuel`: Substring search. Supports **comma-separated lists** (e.g. `Benzina,Gasolio`). - `--geojson`: Emits a standard GeoJSON FeatureCollection with properties and geometry. - Other flags: `--provincia`, `--brand`, `--near`, `--radius`, `--self`/`--served`, `--cheapest`, `--min-price`, `--fresh-within-days`, `--limit`, `--json`. +- Charger power uses `--min-power` in kW; `--fast` means at least 50 kW and `--ultra-fast` at least 150 kW. Inspect the matching socket's power as well as the station maximum. ## JSON / GeoJSON contract - `stations --json` returns a stable envelope with `stations[]`, `query`, and a `quality` block (how many returned prices were screened against a local median). Each station includes a `navigation_url` (Google Maps). - `stations --geojson` returns a `FeatureCollection`. Geometry is `Point` [lon, lat]. Properties include all station metadata. - `chargers` output includes an `error` field in the envelope when the external Overpass API fails. +- Search `coverage` distinguishes fetched rows, matches before the limit, and returned rows. Read `freshness` for cache age; `generated_at` is response time, not source freshness. Surface stale-cache and partial-result errors. +- For chargers, use `pitstop places --json` to discover municipality IDs. Resolve duplicate names with `--provincia` or use `--comune-id`. The returned `location` identifies the mapped OSM administrative center and any coordinate discrepancy; surface its warnings. If no center is available, request explicit coordinates. ## Advice for Agents +- **Discover fuel names:** Run `pitstop fuels --json`; ordinary diesel is `Gasolio`. Substring matches also include premium blends, so compare exact `prices[].fuel` values when a specific fuel is requested. +- **Choose a useful stop:** When ranking within a bounded area, inspect every match (`--limit 0`) and break equal-price ties by distance. Distances are straight-line; a navigation link provides the next step for checking the driving route. +- **Follow the tariff handoff:** Open the official `tariff_info_url` when answering charging-cost questions. Check membership requirements, location, and payment method; a contract rate is not automatically the walk-up price. Report failed links rather than quoting an unverified tariff. - **Use International Names:** You can pass "Rome" or "Milan" directly to `--comune`; the tool handles the translation to the Italian dataset keys. - **Batch Fuel Queries:** To compare Petrol and Diesel, use `--fuel "Benzina,Gasolio"` in a single call. - **Surface Maps:** Always include the `navigation_url` in your response so the user can navigate to the station immediately. diff --git a/src/pitstop/cache.py b/src/pitstop/cache.py new file mode 100644 index 0000000..3628f67 --- /dev/null +++ b/src/pitstop/cache.py @@ -0,0 +1,36 @@ +"""Atomic cache writes and source-fetch timestamps.""" + +from __future__ import annotations + +import os +import tempfile +import time +from datetime import datetime, timezone +from pathlib import Path + + +def write_atomic(path: Path, data: bytes) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + name = None + try: + with tempfile.NamedTemporaryFile(dir=path.parent, prefix=path.name + ".", delete=False) as f: + name = f.name + f.write(data) + os.replace(name, path) + finally: + if name is not None: + Path(name).unlink(missing_ok=True) + + +def file_metadata(path: Path, status: str | None = None) -> dict: + return fetch_metadata(path.stat().st_mtime, status) + + +def fetch_metadata(fetched: float, status: str | None = None) -> dict: + result = { + "fetched_at": datetime.fromtimestamp(fetched, timezone.utc).isoformat(), + "age_seconds": max(0, int(time.time() - fetched)), + } + if status is not None: + result["cache_status"] = status + return result diff --git a/src/pitstop/chargers.py b/src/pitstop/chargers.py index b52e98a..d3565c5 100644 --- a/src/pitstop/chargers.py +++ b/src/pitstop/chargers.py @@ -13,6 +13,8 @@ from . import core, cpo_tariffs, overpass from .core import haversine_km, now_iso +from .validation import validate_search, validate_nonnegative, validate_download +from .results import SearchResults, coverage_of # Map OSM `socket:` to a human-readable plug name. _SOCKET_TYPES = { @@ -140,6 +142,13 @@ def parse_element(el: dict) -> EvStation | None: if lat is None or lon is None: return None + try: + lat, lon = float(lat), float(lon) + except (ValueError, TypeError): + return None + if not math.isfinite(lat) or not -90 <= lat <= 90 or not math.isfinite(lon) or not -180 <= lon <= 180: + return None + sockets: list[Socket] = [] for k, v in tags.items(): # match keys like socket:type2 (count) — ignore the :output / :voltage variants @@ -198,13 +207,21 @@ def find_chargers( free_only: bool = False, public_only: bool = False, refresh: bool = False, + timeout: int = overpass.DEFAULT_TIMEOUT, + max_age: int = overpass.DEFAULT_MAX_AGE, + limit: int = 0, ) -> tuple[list[EvStation], str | None]: """Fetch and filter EV charging stations from OSM around a point. Returns (stations, error_msg).""" + validate_search(near, radius_km, limit) + validate_nonnegative(min_power_kw=min_power_kw) + validate_download(timeout, max_age) # No Italy bbox check: that bound is for the fuel data, OSM EV lookups are not. - radius_m = int(max(100, radius_km * 1000)) + radius_m = max(1, math.ceil(radius_km * 1000)) + freshness: dict = {} elements, error = overpass.fetch_elements(_overpass_query(near[0], near[1], radius_m), - refresh=refresh) + refresh=refresh, timeout=timeout, + max_age=max_age, metadata=freshness) out: list[EvStation] = [] op_lc = operator.strip().lower() @@ -226,21 +243,27 @@ def find_chargers( continue if public_only and st.access.lower() not in ("public", "yes", "permissive"): continue - st.distance_km = round(haversine_km(near[0], near[1], st.lat, st.lon), 2) + distance = haversine_km(near[0], near[1], st.lat, st.lon) + if distance > radius_km: + continue + st.distance_km = round(distance, 2) st.tariff_info_url = cpo_tariffs.lookup(st.operator) out.append(st) out.sort(key=lambda s: s.distance_km if s.distance_km is not None else math.inf) - return out, error + return SearchResults(out, fetched_count=len(elements), limit=limit, freshness=freshness), error -def response_envelope(stations: list[EvStation], query: dict, error: str | None = None) -> dict: +def response_envelope(stations: list[EvStation], query: dict, error: str | None = None, + location: dict | None = None) -> dict: env = { "source": overpass.SOURCE_NAME, "source_url": overpass.SOURCE_URL, "generated_at": core.now_iso(), "query": query, "count": len(stations), + "coverage": coverage_of(stations), + "freshness": getattr(stations, "freshness", {}), "stations": [s.to_dict() for s in stations], "disclaimer": ( "Unofficial tool. EV-charger data from OpenStreetMap via Overpass API " @@ -255,16 +278,21 @@ def response_envelope(stations: list[EvStation], query: dict, error: str | None } if error: env["error"] = error + if location is not None: + env["location"] = location return env -def geojson_envelope(stations: list[EvStation], query: dict, error: str | None = None) -> dict: +def geojson_envelope(stations: list[EvStation], query: dict, error: str | None = None, + location: dict | None = None) -> dict: env = { "type": "FeatureCollection", "metadata": { "source": overpass.SOURCE_NAME, "generated_at": core.now_iso(), "query": query, + "coverage": coverage_of(stations), + "freshness": getattr(stations, "freshness", {}), "disclaimer": ( "Unofficial tool. EV-charger data from OpenStreetMap via Overpass API " "(© OpenStreetMap contributors, ODbL). Coverage and freshness vary. " @@ -276,4 +304,6 @@ def geojson_envelope(stations: list[EvStation], query: dict, error: str | None = } if error: env["metadata"]["error"] = error + if location is not None: + env["metadata"]["location"] = location return env diff --git a/src/pitstop/cli.py b/src/pitstop/cli.py index a6034e8..37dc673 100644 --- a/src/pitstop/cli.py +++ b/src/pitstop/cli.py @@ -9,7 +9,7 @@ import sys import urllib.error -from . import core +from . import core, validation from .version import __version__ # Must state both halves of the rule core.query_stations applies, or a Tukey-only @@ -35,6 +35,9 @@ def main(argv: list[str] | None = None) -> int: return 2 try: return args.func(args) + except validation.QueryError as e: + print(f"error: {e}", file=sys.stderr) + return 2 except urllib.error.URLError as e: print(f"error: could not fetch source data: {e}", file=sys.stderr) return 1 @@ -47,8 +50,8 @@ def _build_parser() -> argparse.ArgumentParser: p = argparse.ArgumentParser( prog="pitstop", description=( - "Unofficial JSON-first CLI for Italian fuel-station prices, backed by " - "MIMIT Osservaprezzi Carburanti open data." + "Italian fuel prices and EV charger discovery from MIMIT and " + "OpenStreetMap. Unofficial; JSON and readable tables." ), formatter_class=argparse.RawDescriptionHelpFormatter, epilog=( @@ -58,11 +61,12 @@ def _build_parser() -> argparse.ArgumentParser: " pitstop fuels" ), ) + p.add_argument("--version", action="version", version=f"pitstop {__version__}") p.set_defaults(func=None) sub = p.add_subparsers(dest="command") stations = sub.add_parser("stations", help="list and filter fuel stations with prices") - _add_load_args(stations) + _add_load_args(stations, geojson=True) stations.add_argument("--comune", default="", help="municipality name (case-insensitive)") stations.add_argument("--provincia", default="", help="2-letter province code, e.g. BZ, RM") stations.add_argument("--brand", default="", help="brand/bandiera substring (case-insensitive)") @@ -75,7 +79,7 @@ def _build_parser() -> argparse.ArgumentParser: stations.add_argument("--min-price", dest="min_price", type=float, default=0.0, help="drop prices below this floor (e.g. 1.2 to skip placeholder values); 0 = off") stations.add_argument("--fresh-within-days", dest="fresh_days", type=int, default=0, - help="drop prices last updated more than N days ago; 0 = off") + help="keep prices with a known update date within the last N days; 0 = off") stations.add_argument("--max-deviation-pct", dest="max_dev_pct", type=float, default=0.0, help="drop prices more than N%% below their (fuel, provincia) median; 0 = off") stations.add_argument("--drop-outliers", dest="drop_outliers", action="store_true", @@ -97,6 +101,8 @@ def _build_parser() -> argparse.ArgumentParser: chargers = sub.add_parser("chargers", help="find EV charging stations (OSM)") chargers.add_argument("--near", default="", help='proximity to "lat,lon" (or use --comune)') chargers.add_argument("--comune", default="", help="center the search on this Italian comune") + chargers.add_argument("--provincia", default="", help="province code to disambiguate a comune, e.g. TN") + chargers.add_argument("--comune-id", default="", help="six-digit ISTAT municipality code; discover with places") chargers.add_argument("--radius", type=float, default=10.0, help="radius in km (default 10)") chargers.add_argument("--operator", default="", help="operator substring (case-insensitive)") chargers.add_argument("--socket", default="", help="plug-type substring, e.g. ccs, chademo, type2") @@ -110,21 +116,35 @@ def _build_parser() -> argparse.ArgumentParser: chargers.add_argument("--public", action="store_true", help="only chargers with explicit public/yes/permissive access; unknown access is excluded") chargers.add_argument("--limit", type=int, default=20, help="max stations; 0 = no limit") - chargers.add_argument("--json", dest="as_json", action="store_true") - chargers.add_argument("--geojson", dest="as_geojson", action="store_true", help="emit GeoJSON FeatureCollection") + charger_format = chargers.add_mutually_exclusive_group() + charger_format.add_argument("--json", dest="as_json", action="store_true") + charger_format.add_argument("--geojson", dest="as_geojson", action="store_true", help="emit GeoJSON FeatureCollection") chargers.add_argument("--refresh", action="store_true", help="bypass the 7-day OSM cache") + chargers.add_argument("--timeout", type=int, default=60, help="per-request download timeout in seconds") + chargers.add_argument("--max-age", type=int, default=7 * 86400, help="maximum OSM cache age in seconds; 0 = unlimited") chargers.set_defaults(func=_cmd_chargers) + places = sub.add_parser("places", help="find municipality names, provinces, and ISTAT IDs") + places.add_argument("query", nargs="?", default="", help="name, international alias, or ISTAT ID") + places.add_argument("--provincia", default="", help="province code, e.g. TN") + places.add_argument("--limit", type=int, default=20, help="max results; 0 = no limit") + places.add_argument("--json", dest="as_json", action="store_true") + places.add_argument("--refresh", action="store_true") + places.add_argument("--timeout", type=int, default=60) + places.set_defaults(func=_cmd_places) + version = sub.add_parser("version", help="print version metadata") version.set_defaults(func=lambda _a: (print(f"pitstop {__version__}") or 0)) return p -def _add_load_args(sp: argparse.ArgumentParser) -> None: - sp.add_argument("--json", dest="as_json", action="store_true", help="emit JSON instead of a table") - sp.add_argument("--geojson", dest="as_geojson", action="store_true", help="emit GeoJSON FeatureCollection") +def _add_load_args(sp: argparse.ArgumentParser, *, geojson: bool = False) -> None: + output = sp.add_mutually_exclusive_group() + output.add_argument("--json", dest="as_json", action="store_true", help="emit JSON instead of a table") + if geojson: + output.add_argument("--geojson", dest="as_geojson", action="store_true", help="emit GeoJSON FeatureCollection") sp.add_argument("--refresh", action="store_true", help="bypass cache and re-download source files") sp.add_argument("--max-age", type=int, default=core.DEFAULT_MAX_AGE, help="seconds a cached file stays fresh") @@ -133,6 +153,7 @@ def _add_load_args(sp: argparse.ArgumentParser) -> None: def _load(args) -> core.Dataset: + validation.validate_download(args.timeout, args.max_age) return core.load(refresh=args.refresh, max_age=args.max_age, timeout=args.timeout) @@ -154,6 +175,9 @@ def _cmd_stations(args) -> int: print(f"error: invalid --near value: {e}", file=sys.stderr) return 2 + validation.validate_search((near_lat, near_lon) if use_near else None, args.radius, args.limit) + validation.validate_nonnegative(min_price=args.min_price, fresh_days=args.fresh_days, + max_deviation_pct=args.max_dev_pct) ds = _load(args) comune_norm = geocoding.normalize_comune(args.comune) @@ -248,25 +272,13 @@ def _print_stats_table(stats: dict) -> int: def _cmd_chargers(args) -> int: from . import chargers, geocoding - if not args.near.strip() and not args.comune.strip(): - print("error: pass --near \"lat,lon\" or --comune NAME", file=sys.stderr) - return 2 - - if args.near.strip(): - try: - lat, lon = _parse_latlon(args.near) - except ValueError as e: - print(f"error: invalid --near value: {e}", file=sys.stderr) - return 2 - else: - coords = geocoding.load_comune_coords() - comune_norm = geocoding.normalize_comune(args.comune) - true = coords.get(comune_norm) - if not true: - print(f"error: comune '{args.comune}' not found in the comune-coords reference", - file=sys.stderr) - return 1 - lat, lon = true + validation.validate_search(None, args.radius, args.limit) + validation.validate_nonnegative(min_power_kw=args.min_power_kw) + validation.validate_download(args.timeout, args.max_age) + (lat, lon), location = geocoding.resolve_search_location( + args.near, args.comune, args.provincia, args.comune_id, + refresh=args.refresh, timeout=args.timeout, + ) min_kw = args.min_power_kw if args.ultra_fast: @@ -283,13 +295,18 @@ def _cmd_chargers(args) -> int: free_only=args.free, public_only=args.public, refresh=args.refresh, + timeout=args.timeout, + max_age=args.max_age, + limit=args.limit, ) - if args.limit > 0: - stations = stations[: args.limit] query = {"near": f"{lat},{lon}", "radius_km": args.radius} if args.comune: query["comune"] = args.comune + if args.provincia: + query["provincia"] = args.provincia + if args.comune_id: + query["comune_id"] = args.comune_id if args.operator: query["operator"] = args.operator if args.socket: @@ -302,19 +319,46 @@ def _cmd_chargers(args) -> int: query["public"] = True if args.as_json: - _dump(chargers.response_envelope(stations, query, error=error)) + _dump(chargers.response_envelope(stations, query, error=error, location=location)) return 0 if args.as_geojson: - _dump(chargers.geojson_envelope(stations, query, error=error)) + _dump(chargers.geojson_envelope(stations, query, error=error, location=location)) return 0 if error: # The JSON paths carry `error` in the envelope; the table has nowhere to # put it, so an empty or stale result set would look like a complete one. print(f"warning: charger data may be incomplete: {error}", file=sys.stderr) + if location is not None: + print(f"Search center: {location['comune']} ({location['provincia']}, " + f"{location['comune_id']}) — {lat},{lon}") + for warning in location["warnings"]: + print(f"warning: {warning}", file=sys.stderr) return _print_chargers_table(stations) +def _cmd_places(args) -> int: + from . import geocoding + validation.validate_search(None, 1, args.limit) + validation.validate_download(args.timeout) + places = geocoding.find_municipalities(args.query, args.provincia, + refresh=args.refresh, timeout=args.timeout) + matched = len(places) + if args.limit: + places = places[:args.limit] + if args.as_json: + _dump({"source": geocoding.COMUNI_SOURCE_NAME, "source_url": geocoding.COMUNI_URL, + "query": args.query, "count": len(places), "matched_count": matched, + "truncated": len(places) < matched, "places": [p.to_dict() for p in places]}) + else: + print("ISTAT PR MUNICIPALITY") + for place in places: + print(f"{place.istat_id} {place.province:2} {place.name}") + print(f"\nShowing {len(places)} of {matched} matches. Use --comune-id for an exact selection.") + return 0 + + def _print_chargers_table(stations) -> int: + _print_coverage(stations) headers = ["DIST_KM", "OPERATOR", "MAX_KW", "PLUGS", "CAP", "FEE", "ACCESS", "NAME"] rows = [headers] for st in stations: @@ -350,6 +394,9 @@ def _print_chargers_table(stations) -> int: # ODbL attribution belongs on every surface, not only the JSON envelope. from . import overpass print(f"\nSource: {overpass.SOURCE_NAME}.") + freshness = getattr(stations, "freshness", {}) + if freshness.get("fetched_at"): + print(f"Downloaded: {freshness['fetched_at']} ({freshness['cache_status']}).") return 0 @@ -388,6 +435,7 @@ def _print_stations_geojson(ds: core.Dataset, stations: list[core.Station], quer def _print_stations_table(ds: core.Dataset, stations: list[core.Station], use_near: bool) -> int: + _print_coverage(stations) any_suspect = False any_outlier = False any_unscreened = False @@ -443,10 +491,14 @@ def _print_stations_table(ds: core.Dataset, stations: list[core.Station], use_ne def _parse_latlon(s: str) -> tuple[float, float]: - parts = s.split(",") - if len(parts) != 2: - raise ValueError('expected "lat,lon"') - return float(parts[0].strip()), float(parts[1].strip()) + return validation.parse_near(s) + + +def _print_coverage(stations) -> None: + coverage = getattr(stations, "coverage", None) + if coverage is not None: + print(f"Showing {coverage['returned_count']} of {coverage['matched_count']} " + "matches in the downloaded data.\n") def _dump(obj) -> None: diff --git a/src/pitstop/core.py b/src/pitstop/core.py index 0b9f1d0..ce4622f 100644 --- a/src/pitstop/core.py +++ b/src/pitstop/core.py @@ -11,10 +11,15 @@ import sys import time import urllib.request -from dataclasses import dataclass, field +from dataclasses import dataclass, field, replace from datetime import date, datetime, timezone +from functools import cached_property, lru_cache from pathlib import Path +from .validation import validate_search, validate_download, validate_nonnegative +from .cache import fetch_metadata, write_atomic +from .results import SearchResults, coverage_of + ANAGRAFICA_URL = "https://www.mimit.gov.it/images/exportCSV/anagrafica_impianti_attivi.csv" PREZZO_URL = "https://www.mimit.gov.it/images/exportCSV/prezzo_alle_8.csv" @@ -123,6 +128,19 @@ class Dataset: stations: dict[str, Station] registry_date: str price_date: str + source_fetches: dict[str, float] = field(default_factory=dict, repr=False) + + @cached_property + def market_stats(self) -> dict: + return fuel_provincia_stats(self) + + @cached_property + def centroids(self) -> dict: + return comune_centroids(self) + + @property + def freshness(self) -> dict: + return {source: fetch_metadata(fetched) for source, fetched in self.source_fetches.items()} def cache_dir() -> Path: @@ -158,9 +176,7 @@ def _cached_file( f"'Estrazione del ...'); MIMIT may be serving its maintenance page" ) - tmp = path.with_suffix(path.suffix + ".tmp") - tmp.write_bytes(data) - tmp.replace(path) # atomic; a partial download never clobbers a good cache file + write_atomic(path, data) return path @@ -239,6 +255,7 @@ def load( timeout: int = DEFAULT_TIMEOUT, ) -> Dataset: """Fetch (or read from cache) both files and return the joined dataset.""" + validate_download(timeout, max_age) ana = _cached_file( ANAGRAFICA_URL, "anagrafica_impianti_attivi.csv", @@ -253,9 +270,15 @@ def load( max_age=max_age, timeout=timeout, ) + return _load_parsed(ana, ana.stat().st_mtime_ns, prezzo, prezzo.stat().st_mtime_ns) + + +@lru_cache(maxsize=1) +def _load_parsed(ana: Path, registry_mtime: int, prezzo: Path, price_mtime: int) -> Dataset: stations, registry_date = _parse_registry(ana) price_date = _attach_prices(prezzo, stations) - return Dataset(stations=stations, registry_date=registry_date, price_date=price_date) + return Dataset(stations, registry_date, price_date, + {"registry": registry_mtime / 1e9, "prices": price_mtime / 1e9}) def filter_prices( @@ -282,7 +305,7 @@ def filter_prices( continue if max_age_days > 0: age = price_age_days(p.updated, today) - if age is not None and age > max_age_days: + if age is None or not 0 <= age <= max_age_days: continue out.append(p) return out @@ -340,11 +363,13 @@ def query_stations( validate_comune: bool = True, comune_coords: dict[str, tuple[float, float]] | None = None, ) -> list[Station]: - """Filter, sort, and limit stations. Mutates the dataset's Station objects - (narrows prices, sets distance_km), so pass a freshly loaded Dataset.""" + """Filter, sort, and limit stations without modifying the source dataset.""" + validate_search(near, radius_km, limit) + validate_nonnegative(min_price=min_price, max_age_days=max_age_days, + max_deviation_pct=max_deviation_pct) today = date.today() if max_age_days > 0 else None - centroids = comune_centroids(ds) - stats = fuel_provincia_stats(ds) + centroids = ds.centroids + stats = ds.market_stats if validate_comune and comune_coords is None: from . import geocoding comune_coords = geocoding.load_comune_coords() @@ -360,6 +385,8 @@ def query_stations( if brand and brand.lower() not in st.brand.lower(): continue + st = replace(st, distance_km=None, coordinate_suspect=False) + prices = filter_prices( st.prices, fuel, self_only, served_only, min_price, max_age_days, today ) @@ -371,6 +398,8 @@ def query_stations( prov = st.provincia.strip().upper() kept_prices: list[Price] = [] for p in prices: + p = replace(p, regional_median=None, deviation_pct=None, outlier=False, + median_basis="unscreened") s = stats.get((p.fuel.strip().lower(), prov)) if s is not None: med = s["median"] @@ -392,25 +421,26 @@ def query_stations( # Flag coordinates that are implausible or far from where they should be. # Prefer the data-derived centroid when available (robust if >=3 stations); - # fall back to the true ISTAT-derived coord (handles single-station comuni). - c = centroids.get(st.comune.upper()) - true_coord = comune_coords.get(st.comune.upper()) if comune_coords else None + # fall back to the municipality reference (handles single-station comuni). + key = (st.comune.strip().upper(), st.provincia.strip().upper()) + c = centroids.get(key) + reference_coord = (comune_coords.get(key) or comune_coords.get(key[0])) if comune_coords else None if not in_italy(st.lat, st.lon): st.coordinate_suspect = True elif c is not None: if haversine_km(c[0], c[1], st.lat, st.lon) > SUSPECT_DISTANCE_KM: st.coordinate_suspect = True - elif true_coord is not None: - if haversine_km(true_coord[0], true_coord[1], st.lat, st.lon) > SUSPECT_DISTANCE_KM: + elif reference_coord is not None: + if haversine_km(reference_coord[0], reference_coord[1], st.lat, st.lon) > SUSPECT_DISTANCE_KM: st.coordinate_suspect = True if near is not None: if not in_italy(st.lat, st.lon): continue # invalid coords cannot be reliably near anything # Reject stations whose declared comune is geographically too far - # from the query point. Prefer centroid, fall back to true_coord. - ref_comune_coord = c or true_coord + # from the query point. Prefer centroid, fall back to reference_coord. + ref_comune_coord = c or reference_coord if ref_comune_coord is not None: comune_dist = haversine_km(near[0], near[1], ref_comune_coord[0], ref_comune_coord[1]) if comune_dist > radius_km + 30.0: @@ -428,9 +458,7 @@ def query_stations( else: out.sort(key=lambda s: (s.comune, s.name)) - if limit > 0: - out = out[:limit] - return out + return SearchResults(out, fetched_count=len(ds.stations), limit=limit) def price_quality(stations: list[Station]) -> dict: @@ -464,6 +492,8 @@ def response_envelope(ds: Dataset, stations: list[Station], query: dict) -> dict "generated_at": now_iso(), "query": query, "count": len(stations), + "coverage": coverage_of(stations), + "freshness": ds.freshness, "quality": price_quality(stations), "stations": [s.to_dict() for s in stations], "disclaimer": DISCLAIMER, @@ -480,6 +510,8 @@ def geojson_envelope(ds: Dataset, stations: list[Station], query: dict) -> dict: "price_extraction_date": ds.price_date, "generated_at": now_iso(), "query": query, + "coverage": coverage_of(stations), + "freshness": ds.freshness, "disclaimer": DISCLAIMER, }, "features": [s.to_geojson_feature() for s in stations], @@ -579,19 +611,27 @@ def in_italy(lat: float, lon: float) -> bool: return ITALY_BBOX[0] <= lat <= ITALY_BBOX[1] and ITALY_BBOX[2] <= lon <= ITALY_BBOX[3] -def comune_centroids(ds: "Dataset", min_stations: int = 3) -> dict[str, tuple[float, float]]: +def comune_centroids(ds: "Dataset", min_stations: int = 3) -> dict: """Median (lat, lon) per comune, only for comuni with at least `min_stations` stations. The median resists individual mis-geocoded outliers.""" - groups: dict[str, list[tuple[float, float]]] = {} + groups: dict[tuple[str, str], list[tuple[float, float]]] = {} for st in ds.stations.values(): if not in_italy(st.lat, st.lon): continue - groups.setdefault(st.comune.upper(), []).append((st.lat, st.lon)) - return { + key = (st.comune.strip().upper(), st.provincia.strip().upper()) + groups.setdefault(key, []).append((st.lat, st.lon)) + result = { com: (statistics.median(p[0] for p in pts), statistics.median(p[1] for p in pts)) for com, pts in groups.items() if len(pts) >= min_stations } + name_counts: dict[str, int] = {} + for name, _province in groups: + name_counts[name] = name_counts.get(name, 0) + 1 + for key in list(result): + if name_counts[key[0]] == 1: + result[key[0]] = result[key] + return result def haversine_km(lat1: float, lon1: float, lat2: float, lon2: float) -> float: diff --git a/src/pitstop/cpo_tariffs.py b/src/pitstop/cpo_tariffs.py index b961dee..4f4b8a1 100644 --- a/src/pitstop/cpo_tariffs.py +++ b/src/pitstop/cpo_tariffs.py @@ -10,8 +10,8 @@ # match against the station's `operator` tag, so minor naming differences # (e.g. "Alperia Smart Mobility" vs "Alperia") still resolve. TARIFF_URLS: dict[str, str] = { - "alperia": "https://www.alperia.eu/it/elettrica-ricarica.html", - "neogy": "https://www.neogy.it/it/ricarica-pubblica.html", + "alperia": "https://www.alperia.eu/easycharge/", + "neogy": "https://www.neogy.it/rete-di-ricarica/direct-payment.html", "enel x way": "https://www.enelxway.com/it/it/privati/ricaricare-elettrica/pubblica", "enel x": "https://www.enelxway.com/it/it/privati/ricaricare-elettrica/pubblica", "enel": "https://www.enelxway.com/it/it/privati/ricaricare-elettrica/pubblica", diff --git a/src/pitstop/geocoding.py b/src/pitstop/geocoding.py index 9fe8531..f6276b4 100644 --- a/src/pitstop/geocoding.py +++ b/src/pitstop/geocoding.py @@ -1,7 +1,8 @@ -"""Second data source: authoritative Italian comune coordinates, used to +"""Second data source: Italian comune reference coordinates, used to validate MIMIT station coordinates. Self-contained centroid heuristics in `core` cannot catch mis-geocoded stations in single-station comuni (e.g. -RASUN-ANTERSELVA), so a true comune→(lat, lon) reference is required. +RASUN-ANTERSELVA). Reference coordinates can also be inaccurate; they are a +cross-check, not proof of a station's location. Source: opendatasicilia/comuni-italiani `main.csv`, derived from ISTAT. Runtime fetch + local cache; no redistribution.""" @@ -9,13 +10,19 @@ from __future__ import annotations import csv +import io +import math import os import sys import time import urllib.error import urllib.request +from dataclasses import dataclass from pathlib import Path +from .cache import write_atomic +from .validation import QueryError, parse_near, validate_download + COMUNI_URL = ( "https://raw.githubusercontent.com/opendatasicilia/comuni-italiani/main/dati/main.csv" ) @@ -96,6 +103,7 @@ def normalize_comune(name: str) -> str: def _cached_path(refresh: bool, max_age: int, timeout: int) -> Path | None: + validate_download(timeout, max_age) path = _cache_dir() / "comuni_main.csv" if not refresh and path.exists(): if max_age <= 0 or (time.time() - path.stat().st_mtime) < max_age: @@ -109,9 +117,12 @@ def _cached_path(refresh: bool, max_age: int, timeout: int) -> Path | None: print(f"pitstop: could not fetch comune coordinates ({e}); " f"falling back to self-contained heuristics", file=sys.stderr) return path if path.exists() else None - tmp = path.with_suffix(path.suffix + ".tmp") - tmp.write_bytes(data) - tmp.replace(path) + if not _read_municipalities(data.decode("utf-8-sig")): + if path.exists(): + print("pitstop: invalid municipality reference; using the cached copy", file=sys.stderr) + return path + raise OSError("municipality reference contained no usable records") + write_atomic(path, data) return path @@ -120,27 +131,124 @@ def load_comune_coords( refresh: bool = False, max_age: int = DEFAULT_COMUNI_MAX_AGE, timeout: int = DEFAULT_TIMEOUT, -) -> dict[str, tuple[float, float]]: - """Return {normalized_comune_name: (lat, lon)}. Empty dict on fetch failure - with no cache, so callers should treat it as best-effort.""" +) -> dict: + """Coordinates keyed by (name, province), plus unambiguous names. + + Empty on fetch failure with no cache. These reference coordinates are only + for station sanity checks; charger searches use the mapped OSM center. + """ path = _cached_path(refresh, max_age, timeout) if path is None: return {} return _parse_comuni(path) -def _parse_comuni(path: Path) -> dict[str, tuple[float, float]]: - out: dict[str, tuple[float, float]] = {} - with path.open("r", encoding="utf-8", errors="replace", newline="") as f: - reader = csv.DictReader(f) - for row in reader: - name = row.get("comune", "").strip() - if not name: - continue - try: - lat = float(row["lat"]) - lon = float(row["long"]) - except (KeyError, ValueError, TypeError): - continue - out[normalize_comune(name)] = (lat, lon) +@dataclass(frozen=True) +class Municipality: + name: str + province: str + istat_id: str + lat: float + lon: float + + def to_dict(self) -> dict: + return {"comune": self.name, "provincia": self.province, "comune_id": self.istat_id} + + +def _read_municipalities(text: str) -> list[Municipality]: + records = [] + for row in csv.DictReader(io.StringIO(text)): + name = normalize_comune(row.get("comune", "")) + code = (row.get("pro_com_t") or "").strip() + province = (row.get("sigla") or "").strip().upper() + if not name or not code.isdigit() or len(code) != 6 or not province: + continue + try: + lat, lon = float(row["lat"]), float(row["long"]) + except (KeyError, ValueError, TypeError): + continue + if math.isfinite(lat) and math.isfinite(lon) and -90 <= lat <= 90 and -180 <= lon <= 180: + records.append(Municipality(name, province, code, lat, lon)) + return records + + +def _parse_comuni(path: Path) -> dict: + out = {} + by_name: dict[str, list[Municipality]] = {} + for place in _read_municipalities(path.read_text(encoding="utf-8-sig")): + out[(place.name, place.province)] = (place.lat, place.lon) + by_name.setdefault(place.name, []).append(place) + for name, places in by_name.items(): + if len(places) == 1: + out[name] = (places[0].lat, places[0].lon) return out + + +def find_municipalities(query: str = "", provincia: str = "", *, + refresh: bool = False, timeout: int = 60) -> list[Municipality]: + path = _cached_path(refresh, DEFAULT_COMUNI_MAX_AGE, timeout) + if path is None: + raise OSError("municipality reference unavailable; use explicit --near coordinates") + records = _read_municipalities(path.read_text(encoding="utf-8-sig")) + if not records: + raise OSError("municipality reference contained no usable records; retry with --refresh or use --near") + query = normalize_comune(query) + province = provincia.strip().upper() + return sorted((p for p in records if (not query or query in p.name or query == p.istat_id) + and (not province or p.province == province)), key=lambda p: (p.name, p.province)) + + +def resolve_municipality(comune: str = "", provincia: str = "", comune_id: str = "", *, + refresh: bool = False, timeout: int = 60) -> dict: + from . import overpass + from .core import haversine_km + + name = normalize_comune(comune) + code = comune_id.strip() + if code and (not code.isdigit() or len(code) != 6): + raise QueryError("comune_id must be a six-digit ISTAT municipality code") + candidates = [p for p in find_municipalities(code or name, provincia, refresh=refresh, timeout=timeout) + if (not code or p.istat_id == code) and (not name or p.name == name)] + if not candidates: + raise QueryError(f"municipality {comune or comune_id!r} not found; use pitstop places or --near") + if len(candidates) != 1: + choices = ", ".join(f"{p.name} ({p.province}, {p.istat_id})" for p in candidates) + raise QueryError(f"ambiguous municipality: {choices}; select --provincia or --comune-id") + place = candidates[0] + query = ('[out:json][timeout:25];rel["boundary"="administrative"]' + f'["admin_level"="8"]["ref:ISTAT"="{place.istat_id}"];' + '(._;node(r:"admin_centre"););out body;') + freshness: dict = {} + elements, error = overpass.fetch_elements(query, refresh=refresh, timeout=timeout, + metadata=freshness) + relations = [e for e in elements if e.get("type") == "relation" + and e.get("tags", {}).get("ref:ISTAT") == place.istat_id] + center_ids = {m.get("ref") for r in relations for m in r.get("members", []) + if m.get("role") == "admin_centre" and m.get("type") == "node"} + centers = [e for e in elements if e.get("type") == "node" and e.get("id") in center_ids] + if len(relations) != 1 or len(centers) != 1: + raise OSError(f"no unique mapped center for {place.name} ({place.province}); " + f"use --near with known coordinates" + (f": {error}" if error else "")) + center = centers[0] + lat, lon = parse_near(f"{center.get('lat')},{center.get('lon')}") + distance = round(haversine_km(place.lat, place.lon, lat, lon), 2) + warnings = [error] if error else [] + if distance > 5: + warnings.append(f"reference coordinate differs by {distance:g} km; using the linked OSM administrative center") + return {**place.to_dict(), "lat": lat, "lon": lon, + "source": overpass.SOURCE_NAME, + "source_url": f"https://www.openstreetmap.org/node/{center['id']}", + "identity_source": COMUNI_SOURCE_NAME, + "reference_distance_km": distance, "freshness": freshness, "warnings": warnings} + + +def resolve_search_location(near: str, comune: str = "", provincia: str = "", comune_id: str = "", *, + refresh: bool = False, timeout: int = 60) -> tuple[tuple[float, float], dict | None]: + if near.strip(): + if comune.strip() or provincia.strip() or comune_id.strip(): + raise QueryError("pass either near coordinates or a municipality selector") + return parse_near(near), None + if not comune.strip() and not comune_id.strip(): + raise QueryError("pass --near, --comune, or --comune-id") + location = resolve_municipality(comune, provincia, comune_id, refresh=refresh, timeout=timeout) + return (location["lat"], location["lon"]), location diff --git a/src/pitstop/mcp_server.py b/src/pitstop/mcp_server.py index e3acfc8..8ae7397 100644 --- a/src/pitstop/mcp_server.py +++ b/src/pitstop/mcp_server.py @@ -1,16 +1,19 @@ """MCP server exposing pitstop's Italian fuel-price data as agent tools. Thin wrapper over pitstop.core (the same logic the CLI uses). Requires the -optional `mcp` extra: pip install "pitstop[mcp]". Run with `pitstop-mcp`.""" +optional `mcp` extra: pip install "pitstop-cli[mcp]". Run with `pitstop-mcp`.""" -from typing import Optional +from typing import Any, Optional from mcp.server.fastmcp import FastMCP +from mcp.types import ToolAnnotations from . import chargers as ev_chargers -from . import core, geocoding +from . import core, geocoding, validation mcp = FastMCP("pitstop") +_READ_ONLY = ToolAnnotations(readOnlyHint=True, destructiveHint=False, + idempotentHint=True, openWorldHint=True) _CAVEATS = ( " Data is daily (not real-time): prices are as of ~08:00 the day before " @@ -27,6 +30,8 @@ "a (fuel, provincia) bucket with too few samples for a median, so no outlier " "check ran on it — do not present it as verified. The envelope's `quality` block " "counts screened vs unscreened prices for the current answer." + " `coverage` separates fetched stations, matches before the limit, and returned " + "stations. `freshness` reports local download times, not price-update times." ) _FIND_STATIONS_DESC = ( @@ -54,12 +59,11 @@ def _parse_near(near: str) -> Optional[tuple]: if not near.strip(): return None - lat, lon = near.split(",") - return (float(lat.strip()), float(lon.strip())) + return validation.parse_near(near) -@mcp.tool() -def list_fuels() -> dict: +@mcp.tool(annotations=_READ_ONLY, structured_output=True) +def list_fuels() -> dict[str, Any]: """List the fuel-type names in the Italian MIMIT fuel dataset, with the number of price rows for each. Call this first to discover exact `fuel` values.""" ds = core.load() @@ -75,8 +79,8 @@ def list_fuels() -> dict: } -@mcp.tool() -def get_stats(fuel: str = "") -> dict: +@mcp.tool(annotations=_READ_ONLY, structured_output=True) +def get_stats(fuel: str = "") -> dict[str, Any]: """Get macro-level price statistics (median, min, max) per Italian province and a national aggregate. Use this to give advice on whether a region is generally cheaper or more expensive than others. `fuel` supports @@ -91,7 +95,7 @@ def get_stats(fuel: str = "") -> dict: } -@mcp.tool(description=_FIND_STATIONS_DESC) +@mcp.tool(description=_FIND_STATIONS_DESC, annotations=_READ_ONLY, structured_output=True) def find_stations( fuel: str = "", comune: str = "", @@ -107,7 +111,11 @@ def find_stations( max_deviation_pct: float = 0.0, drop_outliers: bool = False, limit: int = 20, -) -> dict: +) -> dict[str, Any]: + near_coords = _parse_near(near) + validation.validate_search(near_coords, radius_km, limit) + validation.validate_nonnegative(min_price=min_price, max_age_days=max_age_days, + max_deviation_pct=max_deviation_pct) ds = core.load() comune_norm = geocoding.normalize_comune(comune) stations = core.query_stations( @@ -115,7 +123,7 @@ def find_stations( comune=comune_norm, provincia=provincia, brand=brand, - near=_parse_near(near), + near=near_coords, radius_km=radius_km, fuel=fuel, self_only=self_only, @@ -141,13 +149,16 @@ def find_stations( "cheapest": cheapest or None, "min_price": min_price or None, "fresh_within_days": max_age_days or None, + "max_deviation_pct": max_deviation_pct or None, + "drop_outliers": drop_outliers or None, + "limit": limit, }.items() if v not in ("", None, False) } return core.response_envelope(ds, stations, query) -@mcp.tool(description=_FIND_CHEAPEST_DESC) +@mcp.tool(description=_FIND_CHEAPEST_DESC, annotations=_READ_ONLY, structured_output=True) def find_cheapest( fuel: str, comune: str = "", @@ -159,17 +170,21 @@ def find_cheapest( max_deviation_pct: float = 0.0, drop_outliers: bool = True, limit: int = 5, -) -> dict: - if min_price < 0: +) -> dict[str, Any]: + near_coords = _parse_near(near) + validation.validate_search(near_coords, radius_km, limit) + if min_price == -1: min_price = core.default_floor(fuel) - if max_age_days < 0: + if max_age_days == -1: max_age_days = 90 # ignore stale records when ranking by price + validation.validate_nonnegative(min_price=min_price, max_age_days=max_age_days, + max_deviation_pct=max_deviation_pct) ds = core.load() comune_norm = geocoding.normalize_comune(comune) stations = core.query_stations( ds, comune=comune_norm, - near=_parse_near(near), + near=near_coords, radius_km=radius_km, fuel=fuel, self_only=self_only, @@ -200,7 +215,10 @@ def find_cheapest( _FIND_CHARGERS_DESC = ( "Find EV charging stations near a coordinate or Italian comune, from " "OpenStreetMap. Pass either `near` (\"lat,lon\") or `comune` (Italian " - "municipality name; resolved via the comune-coords reference). Filter by " + "municipality name) or `comune_id` (six-digit ISTAT ID from find_places). " + "For duplicate names, specify provincia. Municipality IDs resolve to mapped " + "OpenStreetMap administrative centers; location records the center and warnings. " + "Surface location warnings, fetch errors, coverage, and cache freshness. Filter by " "operator substring, plug type (e.g. 'ccs', 'chademo', 'type2'), minimum " "max-power kW, free-only, and public-access-only. Returns a JSON envelope " "with operator, plug types, max kW, fee, access, distance, and (when the " @@ -212,7 +230,7 @@ def find_cheapest( ) -@mcp.tool(description=_FIND_CHARGERS_DESC) +@mcp.tool(description=_FIND_CHARGERS_DESC, annotations=_READ_ONLY, structured_output=True) def find_chargers( near: str = "", comune: str = "", @@ -223,31 +241,25 @@ def find_chargers( free_only: bool = False, public_only: bool = False, limit: int = 20, -) -> dict: - if not near.strip() and not comune.strip(): - return ev_chargers.response_envelope([], {}, error="pass either near or comune") - if near.strip(): - lat_s, lon_s = near.split(",") - lat, lon = float(lat_s.strip()), float(lon_s.strip()) - else: - comune_norm = geocoding.normalize_comune(comune) - coords = geocoding.load_comune_coords() - match = coords.get(comune_norm) - if not match: - return ev_chargers.response_envelope( - [], {"comune": comune}, error=f"comune '{comune}' not found" - ) - lat, lon = match + provincia: str = "", + comune_id: str = "", +) -> dict[str, Any]: + near_coords = _parse_near(near) + validation.validate_search(near_coords, radius_km, limit) + validation.validate_nonnegative(min_power_kw=min_power_kw) + (lat, lon), location = geocoding.resolve_search_location(near, comune, provincia, comune_id) stations, error = ev_chargers.find_chargers( near=(lat, lon), radius_km=radius_km, operator=operator, socket=socket, - min_power_kw=min_power_kw, free_only=free_only, public_only=public_only, + min_power_kw=min_power_kw, free_only=free_only, public_only=public_only, limit=limit, ) - if limit > 0: - stations = stations[:limit] query = {"near": f"{lat},{lon}", "radius_km": radius_km} if comune: query["comune"] = comune + if provincia: + query["provincia"] = provincia + if comune_id: + query["comune_id"] = comune_id if operator: query["operator"] = operator if socket: @@ -258,7 +270,23 @@ def find_chargers( query["free"] = True if public_only: query["public"] = True - return ev_chargers.response_envelope(stations, query, error=error) + return ev_chargers.response_envelope(stations, query, error=error, location=location) + + +@mcp.tool(annotations=_READ_ONLY, structured_output=True) +def find_places(query: str, provincia: str = "", limit: int = 20) -> dict[str, Any]: + """Find Italian municipality names, province codes, and six-digit ISTAT IDs. + Use before a charger search when a municipality name is ambiguous; pass the + chosen comune_id or provincia to find_chargers. Supports names such as Bozen. + """ + validation.validate_search(None, 1, limit) + places = geocoding.find_municipalities(query, provincia) + matched = len(places) + if limit: + places = places[:limit] + return {"source": geocoding.COMUNI_SOURCE_NAME, "source_url": geocoding.COMUNI_URL, + "query": query, "count": len(places), "matched_count": matched, + "truncated": len(places) < matched, "places": [p.to_dict() for p in places]} def main() -> None: diff --git a/src/pitstop/overpass.py b/src/pitstop/overpass.py index d5422a8..59459b9 100644 --- a/src/pitstop/overpass.py +++ b/src/pitstop/overpass.py @@ -15,6 +15,10 @@ import urllib.request from pathlib import Path +from .cache import file_metadata, write_atomic +from .validation import validate_download +from .version import __version__ + OVERPASS_URL = "https://overpass-api.de/api/interpreter" SOURCE_NAME = "OpenStreetMap (via Overpass API)" SOURCE_URL = "https://www.openstreetmap.org/copyright" @@ -40,21 +44,38 @@ def fetch_elements( refresh: bool = False, max_age: int = DEFAULT_MAX_AGE, timeout: int = DEFAULT_TIMEOUT, + metadata: dict | None = None, ) -> tuple[list[dict], str | None]: """Run an Overpass QL query and return (elements, error_msg). error_msg is None on success, or a string describing the failure. If a failure occurs but a stale cache exists, elements are returned - from cache and error_msg is still set.""" + from cache and error_msg is still set. Optional metadata receives the fetch + timestamp, cache age, and cache status for this response.""" + validate_download(timeout, max_age) path = _cache_dir() / f"{_cache_key(query)}.json" - if not refresh and path.exists(): + try: + cached_data = path.read_bytes() + except OSError: + cached_data = b"" + cache_valid = _unusable_reason(cached_data) is None + cached = _elements_of(cached_data) if cache_valid else [] + if metadata is not None: + metadata.clear() + def describe(status: str, from_cache: bool = True) -> None: + if metadata is not None: + metadata.update(file_metadata(path, status) if from_cache and path.exists() + else {"cache_status": status}) + + if not refresh and cache_valid: if max_age <= 0 or (time.time() - path.stat().st_mtime) < max_age: - return _read(path), None + describe("hit") + return cached, None req = urllib.request.Request( OVERPASS_URL, data=query.encode("utf-8"), - headers={"User-Agent": "pitstop/0.7 (https://github.com/galjos/pitstop-cli)"}, + headers={"User-Agent": f"pitstop/{__version__} (https://github.com/galjos/pitstop-cli)"}, method="POST", ) error = None @@ -64,19 +85,19 @@ def fetch_elements( except (urllib.error.URLError, OSError) as e: error = str(e) print(f"pitstop: Overpass fetch failed ({error}); " - f"{'using stale cache' if path.exists() else 'no data available'}", + f"{'using stale cache' if cache_valid else 'no data available'}", file=sys.stderr) - return (_read(path) if path.exists() else []), error + describe("stale_fallback" if cache_valid else "unavailable", cache_valid) + return cached, error # Overpass reports runtime failures as HTTP 200 with a `remark` in an otherwise # well-formed body, sometimes with a partial element set. Caching one would # replace a good cache and then serve "0 chargers, no error" for max_age. error = _unusable_reason(data) if error is not None: - cached = _read(path) if path.exists() else [] # Nothing cached: whatever the failed body carried beats zero chargers. - elements = cached or _elements_of(data) - if cached: + elements = cached if cache_valid else _elements_of(data) + if cache_valid: fallback = "using stale cache" elif elements: fallback = f"returning {len(elements)} partial element(s), not cached" @@ -84,11 +105,11 @@ def fetch_elements( fallback = "no data available" print(f"pitstop: Overpass returned no usable data ({error}); {fallback}", file=sys.stderr) + describe("stale_fallback" if cache_valid else "partial" if elements else "unavailable", cache_valid) return elements, error - tmp = path.with_suffix(path.suffix + ".tmp") - tmp.write_bytes(data) - tmp.replace(path) + write_atomic(path, data) + describe("miss") return _read(path), None @@ -104,6 +125,10 @@ def _unusable_reason(data: bytes) -> str | None: return f"response was not valid JSON: {e}" if not isinstance(parsed, dict): return "response was not a JSON object" + if not isinstance(parsed.get("elements"), list) or any( + not isinstance(element, dict) for element in parsed["elements"] + ): + return "response did not contain a valid elements list" remark = str(parsed.get("remark", "")).strip() if remark and any(m in remark.lower() for m in _ERROR_REMARK_MARKERS): return f"Overpass remark: {remark}" @@ -117,13 +142,14 @@ def _elements_of(data: bytes) -> list[dict]: except (json.JSONDecodeError, UnicodeDecodeError): return [] elements = parsed.get("elements") if isinstance(parsed, dict) else None - return elements if isinstance(elements, list) else [] + return [element for element in elements if isinstance(element, dict)] if isinstance(elements, list) else [] def _read(path: Path) -> list[dict]: try: - with path.open("r", encoding="utf-8") as f: - data = json.load(f) - return data.get("elements", []) if isinstance(data, dict) else [] - except (json.JSONDecodeError, OSError): + data = path.read_bytes() + if _unusable_reason(data) is not None: + return [] + return _elements_of(data) + except OSError: return [] diff --git a/src/pitstop/results.py b/src/pitstop/results.py new file mode 100644 index 0000000..6df4321 --- /dev/null +++ b/src/pitstop/results.py @@ -0,0 +1,22 @@ +"""List-compatible search results with counts before pagination.""" + + +class SearchResults(list): + def __init__(self, items, *, fetched_count: int, limit: int = 0, freshness=None): + self.matched_count = len(items) + self.fetched_count = fetched_count + self.freshness = freshness or {} + super().__init__(items[:limit] if limit > 0 else items) + + @property + def coverage(self) -> dict: + return { + "fetched_count": self.fetched_count, + "matched_count": self.matched_count, + "returned_count": len(self), + "truncated": len(self) < self.matched_count, + } + + +def coverage_of(items) -> dict: + return getattr(items, "coverage", {"returned_count": len(items)}) diff --git a/src/pitstop/validation.py b/src/pitstop/validation.py new file mode 100644 index 0000000..f9fc699 --- /dev/null +++ b/src/pitstop/validation.py @@ -0,0 +1,52 @@ +"""Search validation shared by the CLI, MCP tools, and query functions.""" + +from __future__ import annotations + +import math + + +class QueryError(ValueError): + """Invalid query input, reported as CLI exit code 2.""" + + +def parse_near(value: str) -> tuple[float, float]: + try: + lat, lon = value.split(",") + near = (float(lat.strip()), float(lon.strip())) + except ValueError as error: + raise QueryError('expected "lat,lon" with numeric coordinates') from error + _validate_coordinates(near) + return near + + +def _validate_coordinates(near: tuple[float, float]) -> None: + lat, lon = near + if not math.isfinite(lat) or not -90 <= lat <= 90: + raise QueryError("latitude must be finite and between -90 and 90") + if not math.isfinite(lon) or not -180 <= lon <= 180: + raise QueryError("longitude must be finite and between -180 and 180") + + +def validate_search( + near: tuple[float, float] | None, + radius_km: float, + limit: int = 0, +) -> None: + if near is not None: + _validate_coordinates(near) + if not math.isfinite(radius_km) or radius_km <= 0: + raise QueryError("radius must be finite and greater than zero") + if limit < 0: + raise QueryError("limit must not be negative; use 0 for no limit") + + +def validate_nonnegative(**values: float) -> None: + for name, value in values.items(): + if not math.isfinite(value) or value < 0: + raise QueryError(f"{name} must be finite and nonnegative") + + +def validate_download(timeout: int, max_age: int = 0) -> None: + if not math.isfinite(timeout) or timeout <= 0: + raise QueryError("timeout must be finite and greater than zero") + validate_nonnegative(max_age=max_age) diff --git a/src/pitstop/version.py b/src/pitstop/version.py index a82b376..c68196d 100644 --- a/src/pitstop/version.py +++ b/src/pitstop/version.py @@ -1 +1 @@ -__version__ = "1.1.1" +__version__ = "1.2.0" diff --git a/tests/test_chargers.py b/tests/test_chargers.py index a093e27..ac2bf0b 100644 --- a/tests/test_chargers.py +++ b/tests/test_chargers.py @@ -234,6 +234,44 @@ def test_overpass_error_remark_does_not_overwrite_a_good_cache(monkeypatch, tmp_ assert json.loads(path.read_bytes()) == json.loads(_good_body()), "cache was clobbered" +def test_overpass_metadata_preserves_empty_cache_on_failure(monkeypatch, tmp_path): + path = _serve(monkeypatch, tmp_path, _TIMEOUT_BODY) + path.write_text('{"elements": []}') + metadata = {} + assert overpass.fetch_elements(_QUERY, metadata=metadata) == ([], None) + assert metadata["cache_status"] == "hit" + fetched_at = metadata["fetched_at"] + elements, error = overpass.fetch_elements(_QUERY, refresh=True, metadata=metadata) + assert elements == [] and error + assert metadata["cache_status"] == "stale_fallback" + assert metadata["fetched_at"] == fetched_at + assert metadata["age_seconds"] >= 0 + + +def test_overpass_corrupt_cache_is_retried_without_false_freshness(monkeypatch, tmp_path): + path = _serve(monkeypatch, tmp_path, _TIMEOUT_BODY) + path.write_text('{"elements": [null]}') + metadata = {} + elements, error = overpass.fetch_elements(_QUERY, metadata=metadata) + assert elements == [] and error + assert metadata == {"cache_status": "unavailable"} + + +def test_charger_distance_filter_and_limit_report_coverage(monkeypatch): + elements = [_node(i, lat, 11.35, {"amenity": "charging_station"}) + for i, lat in enumerate([46.5, 46.501, 46.502, 46.8])] + monkeypatch.setattr(overpass, "fetch_elements", lambda *a, **k: (elements, None)) + stations, error = chargers.find_chargers(near=(46.5, 11.35), radius_km=1, limit=1) + assert error is None + assert stations.coverage == {"fetched_count": 4, "matched_count": 3, + "returned_count": 1, "truncated": True} + + +def test_invalid_upstream_charger_coordinates_are_skipped(): + for lat, lon in [("NaN", 11), (91, 11), (46, "bad"), (46, 181)]: + assert chargers.parse_element(_node(1, lat, lon, {"amenity": "charging_station"})) is None + + def test_overpass_error_remark_without_cache_reports_error_not_zero_results(monkeypatch, tmp_path): path = _serve(monkeypatch, tmp_path, _TIMEOUT_BODY) @@ -323,12 +361,16 @@ def test_mcp_find_chargers_normalizes_bilingual_comune(monkeypatch): from pitstop import mcp_server # No real network: serve a small coords table and a fake Overpass response. monkeypatch.setattr( - mcp_server.geocoding, "load_comune_coords", - lambda *a, **k: {"BOLZANO": (46.498, 11.354)}, + mcp_server.geocoding, "find_municipalities", + lambda *a, **k: [mcp_server.geocoding.Municipality("BOLZANO", "BZ", "021008", 46.65, 11.22)], ) monkeypatch.setattr( mcp_server.ev_chargers.overpass, "fetch_elements", - lambda *a, **k: ( + lambda query, **k: ( + [{"type": "relation", "id": 1, "tags": {"ref:ISTAT": "021008"}, + "members": [{"type": "node", "ref": 2, "role": "admin_centre"}]}, + {"type": "node", "id": 2, "lat": 46.498, "lon": 11.354}] + if "ref:ISTAT" in query else [_node(99, 46.498, 11.354, {"amenity": "charging_station", "operator": "Alperia", "socket:type2": "1"})], @@ -341,3 +383,4 @@ def test_mcp_find_chargers_normalizes_bilingual_comune(monkeypatch): assert "error" not in result, f"got error: {result.get('error')}" assert result["count"] == 1 assert result["stations"][0]["operator"] == "Alperia" + assert result["location"]["comune_id"] == "021008" diff --git a/tests/test_core.py b/tests/test_core.py index 818814b..188c7f3 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -76,6 +76,20 @@ def test_attach_prices_fields(registry_path, prices_path): assert gasolio.self_service is False +def test_load_reuses_parsing_and_invalidates_after_atomic_refresh(registry_path, prices_path, monkeypatch): + from pitstop.cache import write_atomic + monkeypatch.setattr(core, "_cached_file", lambda url, *a, **k: + registry_path if url == core.ANAGRAFICA_URL else prices_path) + first = core.load() + assert core.load() is first + write_atomic(prices_path, PRICES.replace("1.899", "1.999").encode()) + updated = core.load() + assert updated is not first + assert updated.stations["1"].prices[0].price == 1.999 + assert first.stations["1"].prices[0].price == 1.899 + assert updated.freshness["prices"]["age_seconds"] >= 0 + + def test_filter_prices_fuel_substring(): prices = [ core.Price("Gasolio", 1.799, False, ""), @@ -129,6 +143,17 @@ def test_filter_prices_max_age_drops_stale(): assert len(out_all) == 2 +def test_freshness_filter_requires_a_known_nonfuture_date(): + today = date(2026, 9, 10) + prices = [ + core.Price("Gasolio", 1.8, True, updated) + for updated in ("", "not-a-date", "2026-09-11T08:00:00", "2026-09-03T08:00:00") + ] + out = core.filter_prices(prices, max_age_days=7, today=today) + assert [p.updated for p in out] == ["2026-09-03T08:00:00"] + assert core.filter_prices(prices, today=today) == prices + + def test_default_floor(): # petrol/diesel/methane: real prices > placeholder 1.000, so apply a 1.2 floor assert core.default_floor("Gasolio") == 1.2 diff --git a/tests/test_geocoding.py b/tests/test_geocoding.py new file mode 100644 index 0000000..a2b3f90 --- /dev/null +++ b/tests/test_geocoding.py @@ -0,0 +1,79 @@ +import json + +import pytest + +from pitstop import cli, geocoding + + +REFERENCE = """comune,pro_com_t,lat,long,sigla +Bolzano,021008,46.655942,11.229637,BZ +Roma,058091,41.89332,12.482932,RM +Livo,013130,46.168845,9.304510,CO +Livo,022106,46.404711,11.019271,TN +""" + + +@pytest.fixture +def reference(tmp_path, monkeypatch): + path = tmp_path / "comuni.csv" + path.write_text(REFERENCE) + monkeypatch.setattr(geocoding, "_cached_path", lambda *a: path) + return path + + +def test_duplicate_names_preserve_province_and_id(reference): + places = geocoding.find_municipalities("Livo") + assert [(p.province, p.istat_id) for p in places] == [("CO", "013130"), ("TN", "022106")] + coords = geocoding._parse_comuni(reference) + assert "LIVO" not in coords + assert coords[("LIVO", "CO")] != coords[("LIVO", "TN")] + + +def test_ambiguous_name_requires_selection_before_overpass(reference, monkeypatch): + monkeypatch.setattr("pitstop.overpass.fetch_elements", lambda *a, **k: pytest.fail("unexpected fetch")) + with pytest.raises(ValueError, match="ambiguous.*013130.*022106"): + geocoding.resolve_municipality("Livo") + + +@pytest.mark.parametrize("name,province,code,lat,lon", [ + ("Bozen", "", "021008", 46.4984781, 11.3547399), + ("Rome", "", "058091", 41.8933203, 12.4829321), + ("Livo", "CO", "013130", 46.1688448, 9.30451), + ("Livo", "TN", "022106", 46.404711, 11.019271), + ("", "", "022106", 46.404711, 11.019271), +]) +def test_center_is_selected_by_municipality_identity(reference, monkeypatch, name, province, code, lat, lon): + def fetch(query, **kwargs): + assert f'["ref:ISTAT"="{code}"]' in query + return [ + {"type": "relation", "tags": {"ref:ISTAT": code}, + "members": [{"type": "node", "ref": 7, "role": "admin_centre"}]}, + {"type": "node", "id": 7, "lat": lat, "lon": lon}, + {"type": "node", "id": 8, "lat": 0, "lon": 0}, + ], None + monkeypatch.setattr("pitstop.overpass.fetch_elements", fetch) + location = geocoding.resolve_municipality(name, province, "" if name else code) + assert location["comune_id"] == code + assert (location["lat"], location["lon"]) == (lat, lon) + if code == "021008": + assert location["reference_distance_km"] > 19 + assert location["warnings"] + + +def test_unavailable_center_does_not_use_the_wrong_reference(reference, monkeypatch): + monkeypatch.setattr("pitstop.overpass.fetch_elements", lambda *a, **k: ([], "HTTP 503")) + with pytest.raises(OSError, match="no unique mapped center.*503"): + geocoding.resolve_municipality("Bozen") + + +def test_places_cli_makes_ambiguous_choices_reviewable(reference, capsys): + assert cli.main(["places", "Livo", "--json"]) == 0 + result = json.loads(capsys.readouterr().out) + assert result["count"] == 2 + assert {p["comune_id"] for p in result["places"]} == {"013130", "022106"} + + +def test_invalid_reference_is_not_reported_as_a_missing_municipality(reference): + reference.write_text("maintenance") + with pytest.raises(OSError, match="no usable records"): + geocoding.resolve_municipality("Bozen") diff --git a/tests/test_query_reuse.py b/tests/test_query_reuse.py new file mode 100644 index 0000000..d7140ed --- /dev/null +++ b/tests/test_query_reuse.py @@ -0,0 +1,39 @@ +from concurrent.futures import ThreadPoolExecutor +from datetime import date + +from pitstop import core + + +def dataset(): + return core.Dataset({str(i): core.Station(str(i), "", "", "", "Station", "", "ROMA", "RM", + 41.9, 12.5, + [core.Price("Benzina", 1.8 + i / 100, True, date.today().isoformat()), + core.Price("Gasolio", 1.7 + i / 100, True, date.today().isoformat())]) + for i in range(20)}, "2026-09-10", "2026-09-10") + + +def test_concurrent_queries_do_not_narrow_the_shared_dataset(): + ds = dataset() + def query(fuel): + return core.query_stations(ds, fuel=fuel, limit=3, validate_comune=False) + with ThreadPoolExecutor(max_workers=2) as pool: + petrol, diesel = list(pool.map(query, ["Benzina", "Gasolio"])) + assert {p.fuel for s in petrol for p in s.prices} == {"Benzina"} + assert {p.fuel for s in diesel for p in s.prices} == {"Gasolio"} + assert all(len(s.prices) == 2 for s in ds.stations.values()) + assert all(p.median_basis == "unscreened" for s in ds.stations.values() for p in s.prices) + petrol[0].prices[0].price = 0 + assert ds.stations["0"].prices[0].price == 1.8 + assert petrol.coverage == {"fetched_count": 20, "matched_count": 20, "returned_count": 3, "truncated": True} + + +def test_duplicate_municipality_centroids_stay_separate(): + ds = dataset() + for i, station in enumerate(ds.stations.values()): + station.comune = "LIVO" + station.provincia = "CO" if i < 10 else "TN" + station.lat, station.lon = (46.17, 9.30) if i < 10 else (46.40, 11.02) + centroids = core.comune_centroids(ds) + assert "LIVO" not in centroids + assert centroids[("LIVO", "CO")] == (46.17, 9.30) + assert centroids[("LIVO", "TN")] == (46.40, 11.02) diff --git a/tests/test_validation.py b/tests/test_validation.py new file mode 100644 index 0000000..b3f520c --- /dev/null +++ b/tests/test_validation.py @@ -0,0 +1,59 @@ +import pytest + +from pitstop import cli, core, mcp_server + + +@pytest.mark.parametrize("command", ["stations", "chargers"]) +@pytest.mark.parametrize("flags", [ + ["--near", "nan,11.354"], + ["--near", "46.498,inf"], + ["--near", "91,11.354"], + ["--near", "46.498,181"], + ["--near", "46.498,11.354", "--radius", "-1"], + ["--near", "46.498,11.354", "--radius", "0"], + ["--near", "46.498,11.354", "--radius", "nan"], + ["--near", "46.498,11.354", "--limit", "-1"], + ["--near", "46.498,11.354", "--timeout", "0"], + ["--near", "46.498,11.354", "--max-age", "-1"], +]) +def test_cli_rejects_invalid_search_before_fetch(command, flags, monkeypatch, capsys): + def unexpected_fetch(*args, **kwargs): + pytest.fail("invalid input must be rejected before fetching data") + + monkeypatch.setattr(core, "load", unexpected_fetch) + monkeypatch.setattr("pitstop.geocoding.load_comune_coords", unexpected_fetch) + monkeypatch.setattr("pitstop.overpass.fetch_elements", unexpected_fetch) + assert cli.main([command, *flags, "--json"]) == 2 + output = capsys.readouterr() + assert output.out == "" + assert "error:" in output.err + + +@pytest.mark.parametrize("tool,arguments", [ + (mcp_server.find_stations, {"near": "nan,11.354"}), + (mcp_server.find_cheapest, {"fuel": "Gasolio", "radius_km": -1}), + (mcp_server.find_cheapest, {"fuel": "Gasolio", "limit": -1}), + (mcp_server.find_chargers, {"near": "91,11.354"}), + (mcp_server.find_chargers, {"comune": "Bozen", "radius_km": float("inf")}), + (mcp_server.find_stations, {"min_price": float("nan")}), + (mcp_server.find_stations, {"max_age_days": -1}), + (mcp_server.find_cheapest, {"fuel": "Gasolio", "min_price": float("nan")}), + (mcp_server.find_cheapest, {"fuel": "Gasolio", "min_price": float("-inf")}), + (mcp_server.find_chargers, {"comune": "Bozen", "min_power_kw": -1}), +]) +def test_mcp_rejects_invalid_search_before_fetch(tool, arguments, monkeypatch): + def unexpected_fetch(*args, **kwargs): + pytest.fail("invalid input must be rejected before fetching data") + + monkeypatch.setattr(core, "load", unexpected_fetch) + monkeypatch.setattr("pitstop.geocoding.load_comune_coords", unexpected_fetch) + monkeypatch.setattr("pitstop.overpass.fetch_elements", unexpected_fetch) + with pytest.raises(ValueError): + tool(**arguments) + + +def test_valid_coordinate_boundaries_and_unlimited_results(): + assert cli._parse_latlon("-90,-180") == (-90, -180) + assert cli._parse_latlon("90,180") == (90, 180) + ds = core.Dataset(stations={}, registry_date="", price_date="") + assert core.query_stations(ds, near=(46.498, 11.354), limit=0, comune_coords={}) == []