feat(skill): route Portolan catalogs, without reimplementing them - #30
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7028b1135
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| cadastre_url = "https://s3.pilw.io/rp-kemit-kataster/ANDMED/Tartu_maakond_KATASTER_GPKG.zip" | ||
|
|
||
| if not cadastre_gpkg.exists(): | ||
| if not _reuse_cached("Cadastral GeoPackage", cadastre_gpkg, cadastre_gpkg.exists(), refresh): |
There was a problem hiding this comment.
When --refresh downloads a newer mutable cadastre archive here, the manifest still hard-codes download_timestamp and version to 2026-08-25 at lines 390-391, while project.yaml retains the old ETag. finalize_run then copies that timestamp into the source access fields, so a pre-commit refresh can record new bytes and counts as an older snapshot, defeating the example's provenance and reproducibility guarantees. Capture the response/archive timestamp, version, and ETag during this download instead.
AGENTS.md reference: AGENTS.md:L90-L91
Useful? React with 👍 / 👎.
| self.assertIn("Portolan", self.skill) | ||
| data_sources = (REPO_ROOT / "references/data-sources.md").read_text(encoding="utf-8") | ||
| self.assertIn("## Portolan catalogs", data_sources) | ||
| self.assertIn("AGENTS.md", data_sources) |
There was a problem hiding this comment.
These assertions only check for three strings, so they remain green if the shipped guidance no longer causes an agent to inspect collection metadata, select assets by role, or produce a correctly pinned result. Because this commit materially changes shipped agent behavior, add an eval case that actually exercises a representative Portolan catalog task rather than treating keyword presence as behavioral coverage.
AGENTS.md reference: AGENTS.md:L87-L90
Useful? React with 👍 / 👎.
| and roads_meta.get("matched") == roads_meta.get("returned") | ||
| and roads_meta.get("returned") == len(roads_raw.get("features", [])) | ||
| ) | ||
| roads_cache_valid = _reuse_cached("ETAK main roads", roads_geojson, roads_cache_valid, refresh) |
There was a problem hiding this comment.
If an existing roads cache or its metadata is truncated or invalid JSON, execution fails in the preceding json.loads calls before this new refresh check runs; the education cache follows the same pattern at lines 282-294. Thus pipeline.py --refresh cannot recover from a damaged cache despite promising to discard cached sources. Skip reading cached files when refresh is true, or remove them before validating them.
Useful? React with 👍 / 👎.
Portolan publishes geospatial data as a static STAC catalog on object storage -- GeoParquet plus PMTiles for vector, COG for raster, plain Parquet for tabular -- with no API server. Reading one is the access pattern this toolkit already defaults to, so almost nothing new is needed: DuckDB with httpfs over remote GeoParquet already works. What was missing is recognition and routing. An agent handed a catalog root had no way to know what it was looking at, that `AGENTS.md` beside every collection is instruction meant for it, or that dedicated skills exist. So `data-sources.md` gains a Portolan section covering the layout, the working rules that are easy to get wrong (read AGENTS.md before querying, select assets by `roles` rather than key, use the https href), and pointers to `portolan-sdi/portolan-skills` and the spec as ground truth. `SKILL.md` gains the term in three places an agent routes through, and nothing more -- the detail belongs in the reference. The additive part is pinning, which is this toolkit's concern and not the reading skill's. A collection already carries an SPDX license, providers, version identity and `via` links, so the section maps them onto the manifest fields rather than inventing provenance, and names two traps: - a catalog whose producer and host differ is a mirror, so its `updated` is a sync time and the authority is behind the `via` link; - `file:checksum` is multihash-encoded (PORTO-CORE-029), so copying it into a `sha256:` pin records a value that can never match the bytes. Coverage: SKILL.md is a router whose pointers nothing checked. The new tests assert every reference it names exists, every reference is reachable from it, and that Portolan routes to the section documenting it. Both failure paths were confirmed by breaking them. Closes #26 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DqoLGyKY5opHGjNMSerpHg
Review was right that the routing test proves only that three strings are present. It stays green if the guidance stops changing what an agent does, which is the thing the change is for. So add `017-portolan-catalog`, a live case over a fixture catalog that grades two behaviours by consequence: - `parcels/AGENTS.md` carries the count rule. `record_status` holds one superseded historical row, and every count must exclude it. Three rows match ARIMAA; two are current. An agent that queried the GeoParquet without reading the collection's agent guide answers 3 and fails `geodata.row_count`; - the GeoParquet holds the `data` role under an opaque asset key, while the pre-conversion GeoJSON is keyed `parcels` and carries `source`. Picking by key lands on the upstream extract, whose columns are the publisher's originals, so a wrong pick surfaces rather than quietly returning the right number. The provenance assertions grade the rest: a collection publishes an SPDX license, producer/host providers and a version identity, so a project built from one has no excuse for an unpinned or unattributed source. The fixture is local, so the case needs no third-party catalog to be reachable and cannot fail on someone else's outage. It runs in live mode only -- an agent is the thing under test -- so fixture CI skips it, which is honest rather than free: `not_testable` is not `passed`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DqoLGyKY5opHGjNMSerpHg
c1f1278 to
ec7f314
Compare
The fixture catalog cannot falsify the guidance -- we wrote both, so they agree by construction. These tests check the same claims against the published Fields of the World catalog instead, the one the Portolan browser links to. It immediately earned its place. `predictions/vectors` carries no `data` asset at all: its roles are visual, style, thumbnail, documentation, metadata, and the GeoParquet sits behind a `partition:glob` and 210 item links, with the `data` role on each item. An agent following the reference as written -- "select assets by roles, `data` is the primary Parquet" -- would have found no data and concluded the collection was empty. The reference now says a collection without a `data` asset is partitioned rather than empty, and where to look instead. The multihash note is confirmed rather than corrected: every `file:checksum` on the Estonia item begins `1220`, sha2-256 in multihash, so the warning against copying one into a `sha256:` pin field describes something real. Opt-in, because a third-party outage must never redden this repository: the tests skip unless OPENMAPSTACK_NETWORK_TESTS=1, and skip rather than fail when the service cannot be reached. A skip is `not_testable`, not a pass. Wiring them into a scheduled job would give them recurring value, but that is a separate decision from having them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DqoLGyKY5opHGjNMSerpHg
1 participant
Closes #26.
What I found first
The issue asks for a skill to consume Portolan datasets, and suggests consulting
portolan-sdi/portolan-skillswhen needed. That suggestion is the right call, and more so than it looks:reading-portolanalready exists there and is exactly the consumer skill described — metadata andAGENTS.md, assets by role, DuckDB over GeoParquet, COGs, cross-dataset joins, partitioned collections, PMTiles maps. It is Apache-2.0, built on the Agent Skills open standard, and tracks the spec version inpins.tomlwith a drift checker.So this PR does not reimplement it. Writing a second, thinner copy of someone else's maintained skill would drift within a release.
What was actually missing
Reading a Portolan catalog is the access pattern this toolkit already defaults to — static files on object storage, queried in place with DuckDB and
httpfs. The gap was recognition and routing: an agent handed a catalog root had no way to know what it was looking at, that theAGENTS.mdbeside every collection is instruction addressed to it, or that dedicated skills exist.references/data-sources.mdgains a## Portolan catalogssection:catalog.jsonroot,AGENTS.md/README.mdper collection, theschemas.portolan-sdi.orgversion URI) and the directory layout;AGENTS.mdbefore writing a query, select assets byrolesand never by asset key, use thehttpshref rather than hand-rewriting it tos3://;portolan-skills(preferred when installed) andportolan-specas ground truth.SKILL.mdgains the term in the three places an agent routes through — the module table, the discovery triage line, and the frontmatter description — and nothing else. The detail belongs in the reference, perAGENTS.md.The part that is genuinely ours
Pinning. A Portolan collection already carries an SPDX license, providers, version identity and
vialinks, so the section maps those ontoproject.yamlfields rather than inventing provenance — and names two traps that would otherwise produce a confident wrong manifest:producerandhostdiffer is a mirror, not the authority. Its top-levelupdatedis the last sync time, so pinning it records the mirror's freshness and attributes the wrong party;file:checksumis multihash-encoded, not a raw sha256 (PORTO-CORE-029). Copying it into asha256:pin field records a value that can never match the bytes, andlocal_snapshotpins are verified against real content.Neutrality
No install command is embedded.
portolan-skillsships as Agent Skills and works across tools, but installing a plugin is agent-specific, so the reference names the repo and leaves the mechanism to the host — consistent withAGENTS.mdon keeping shared instructions vendor-neutral.Coverage
SKILL.mdis a router whose pointers nothing checked, and fixture evals grade produced projects rather than the routing table.tests/test_skill_routing.pyasserts every reference it names exists, every reference is reachable from it, and that Portolan routes to the section documenting it. Both failure paths were confirmed by deliberately breaking them.464 unit tests pass; fixture evals 16/16 contract_ci and 26/26 mutation.
Not included
A live eval exercising an agent against a real catalog. That needs a public catalog plus model credentials and would be stochastic; the deterministic layers cannot measure whether an agent uses the guidance. Worth a follow-up issue if you want it graded rather than documented.
🤖 Generated with Claude Code
https://claude.ai/code/session_01DqoLGyKY5opHGjNMSerpHg