Skip to content

fix(scoring): rank ckan_find_relevant_datasets by DCAT-AP_IT holder and publisher, not just organization#34

Merged
aborruso merged 1 commit into
ondata:mainfrom
piersoft:fix/relevance-scoring-holder-publisher
May 25, 2026
Merged

fix(scoring): rank ckan_find_relevant_datasets by DCAT-AP_IT holder and publisher, not just organization#34
aborruso merged 1 commit into
ondata:mainfrom
piersoft:fix/relevance-scoring-holder-publisher

Conversation

@piersoft

@piersoft piersoft commented May 24, 2026

Copy link
Copy Markdown
Contributor

Fix ckan_find_relevant_datasets scoring on federated CKAN catalogs (notably dati.gov.it). Adds holder and publisher as separate weighted fields in scoreDatasetRelevance(), sourced from DCAT-AP_IT extras[] first (the authoritative location on Italian portals via ckanext-dcatapit), with fallback to root-level for non-DCAT-AP_IT portals. The bug On dati.gov.it, organization in package_search results is the harvesting catalog, not the data owner. A dataset owned by a Comune that publishes via Regione/GAL/Unione gets organization=regione-puglia (or similar), so a query like "datasets from Comune di Lecce" scores 0 on the owner field and misses the actual dct:rightsHolder. Compounding this: package_search exposes holder_name / publisher_name in two places that often disagree: inside extras[] → the authoritative DCAT-AP_IT value (correct owner) at root level → on aggregator catalogs, overwritten by the harvester with the organization name (wrong) Reading only the root field silently misses the real owner. The fix reads from extras[] first. Validated on real datasets (live package_search on dati.gov.it/opendata) Datasetextras.holderroot.holderbeforeafterdefibrillatori-esterniComune di Mesagne ✓GAL Terra dei Messapi ✗612defibrillatori-dae-progetto-comune-cardioprotettoComune di Lecce ✓Regione Puglia ✗1013 Verified end-to-end on the AgID deployment (ckan-mcp-server.agid.workers.dev) after merging the same fix downstream. Compatibility Backward compatible. weights object accepts two new optional fields (holder, publisher); clients that don't set them get the improved ranking by default. CkanPackage interface gains typed holder_name and publisher_name. New internal helper readDcatExtra() encapsulates the extras-first, root-fallback lookup. Score breakdown markdown and JSON outputs include holder and publisher per dataset.

@piersoft

Copy link
Copy Markdown
Contributor Author

FYI: the claude-code-action check failed with "Could not fetch an OIDC token" — this is expected for PRs from forks because GitHub Actions denies id-token: write permission to fork-originated workflows for security. Not related to the patch itself. The codebase patch applies cleanly on main.

…ization

On federated CKAN catalogs (notably dati.gov.it and any portal harvesting
from sub-publishers), the 'organization' field represents the harvesting
catalog, not the data owner. Datasets owned by small/medium Comuni that
publish via aggregators (Regione, GAL, Unione di Comuni, ASL) score 0
on the owner-related field, breaking queries like 'datasets from Comune
di Lecce'.

DCAT-AP_IT distinguishes three distinct concepts:
  - dct:publisher       -> CKAN field publisher_name
  - dct:rightsHolder    -> CKAN field holder_name (actual owner)
  - catalog membership  -> CKAN organization (harvester on aggregators)

scoreDatasetRelevance() now scores holder_name and publisher_name as
two separate weighted fields, additive to the existing scoring. Defaults
holder=4 (peer with title; institutional owner), publisher=2 (lower
weight because it can be a technical role e.g. 'Redazione OD' rather
than the institution).

The fields are read from extras[] (the authoritative DCAT-AP_IT location
on Italian portals via ckanext-dcatapit) with fallback to the root-level
field. On dati.gov.it, package_search exposes holder_name and
publisher_name BOTH in extras[] (correct DCAT-AP_IT values) AND at root
level (often overwritten by the harvester with the organization name);
reading only the root would silently miss the actual owner. The
root-level fallback preserves correct behavior on non-DCAT-AP_IT portals
(data.gov, open.canada.ca) that expose these fields plainly.

Surfaced on real Puglia datasets (validated against live package_search):
  - defibrillatori-esterni:
      extras.holder='Comune di Mesagne' (correct, DCAT-AP_IT)
      root.holder='GAL Terra dei Messapi' (wrong, harvester)
      organization='GAL Terra dei Messapi'
      score: 6 -> 12
  - defibrillatori-dae-progetto-comune-cardioprotetto:
      extras.holder='Comune di Lecce'
      organization='Regione Puglia'
      score: 10 -> 13

Backward compatible: clients that don't set 'holder'/'publisher' in
weights get the improved ranking by default. CkanPackage interface
gains typed holder_name and publisher_name (previously accessed via
index signature). New internal helper readDcatExtra() encapsulates
the extras-first lookup.
@piersoft
piersoft force-pushed the fix/relevance-scoring-holder-publisher branch from 30c703a to cf96e0c Compare May 25, 2026 00:03
@piersoft

piersoft commented May 25, 2026

Copy link
Copy Markdown
Contributor Author

The original commit (v1) was tested on package_show responses and worked, but on package_search results — which is what ckan_find_relevant_datasets actually consumes — holder_name and publisher_name are exposed in two locations that disagree on aggregator catalogs (extras[] vs root level). On dati.gov.it the root-level value is overwritten by the harvester with the catalog's name, while extras[] preserves the authoritative DCAT-AP_IT value.
Force-pushed a consolidated single commit with both the original scoring change and the readDcatExtra() helper that prefers extras[] first. Diff is now 3 files, +97 -6.
Validated end-to-end against live https://dati.gov.it/opendata and deployed on the AgID Worker (ckan-mcp-server.agid.workers.dev) following the same patch merged downstream (AgID PRs #1 and #3).

@aborruso

Copy link
Copy Markdown
Member

Review + additions before merge

Thanks for the fix @piersoft — the extras-first logic is correct and well-documented. We ran a full audit before merging and added what was missing.


What was reviewed

  • ✅ Logic in readDcatExtra: extras-first → root-fallback is safe and correct
  • ✅ No regression on non-DCAT portals: tested on open.canada.ca, data.gov.ukholder=0, publisher=0 for all results
  • ✅ Fix verified on dati.gov.it: query "defibrillatori Comune di Lecce" now returns the dataset with Holder: 4 (from extras[holder_name]="Comune di Lecce"), despite organization="Regione Puglia" (the harvester)
  • ✅ Italian portals audit: dati.comune.messina.it and dati.regione.sicilia.it have DCAT-AP_IT extras that agree with root fields (no divergence found) → harmless; dati.arpae.it has no holder/publisher → harmless; bdap-opendata.rgs.mef.gov.it has extras[publisher_name]="Open" (internal label) → minor false-positive risk on generic queries, acceptable
  • ✅ All 399 existing tests passing

What was missing (blocking)

Zero tests for readDcatExtra and the new scoring behavior. The core bug — extras[holder_name] wins over root[holder_name] on federated portals — had no test protecting it from future regressions.


What was added (commit 07d44f8)

readDcatExtra was made export to allow direct unit testing.

10 tests for readDcatExtra covering:

  • extras value returned when key is present
  • extras wins over root when both are present (the key invariant)
  • falls back to root when key not in extras
  • falls back to root when extras is empty or absent
  • returns "" when neither extras nor root have the key
  • skips extras entry with empty string value
  • skips extras entry with non-string value
  • works correctly for publisher_name key

7 tests for scoreDatasetRelevance — holder and publisher covering:

  • scores holder from extras (DCAT-AP_IT pattern)
  • extras holder wins over root holder when they differ — the exact federated-portal bug scenario (extras.holder="Comune di Mesagne" vs root.holder="GAL Terra dei Messapi")
  • scores publisher from extras
  • holder and publisher contribute to total correctly
  • non-DCAT portal: holder and publisher score 0 when fields absent
  • default weight 4 for holder
  • default weight 2 for publisher

3 existing tests updated to account for the new fields:

  • sums scores from multiple fields: added + breakdown.holder + breakdown.publisher to the expected sum
  • uses custom weights correctly: added holder: 4, publisher: 2 to the weights object
  • returns structure: added toHaveProperty('holder') and toHaveProperty('publisher') checks on breakdown

Result: 416/416 tests passing (was 399).


The changes were pushed to ondata/ckan-mcp-server:fix/relevance-scoring-holder-publisher and will be merged from there.

@aborruso
aborruso merged commit fccf8db into ondata:main May 25, 2026
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants