Skip to content

Add matchedNotation field to search results for notation matches - #1933

Open
fvogel wants to merge 1 commit into
NatLibFi:mainfrom
fvogel:fix/notation-search-missing-bind
Open

fvogel wants to merge 1 commit into
NatLibFi:mainfrom
fvogel:fix/notation-search-missing-bind

Conversation

@fvogel

@fvogel fvogel commented Feb 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #1931 — adds a matchedNotation field to REST API search results when a concept is found via its skos:notation, parallel to the existing matchedPrefLabel for prefLabel matches.

Changes in src/model/sparql/GenericSparql.php:

  • Add BIND(IF((?pri = "5" || ?pri = "6"), ?match, ?unbound) as ?nlabel) in the $labelpriority block
  • Add ?nlabel to SELECT and GROUP BY clauses
  • Add elseif (isset($row->nlabel)) handler setting $hit['matchedNotation']

Before (notation search query=A.1):

{ "prefLabel": "Cat", "notation": "A.1" }

After:

{ "prefLabel": "Cat", "notation": "A.1", "matchedNotation": "A.1" }

Test plan

  • query=A.1 → result includes matchedNotation: "A.1"
  • query=A.1.1 → result includes matchedNotation: "A.1.1"
  • query=Cat → no matchedNotation field (prefLabel match, correct)
  • Existing PHPUnit tests pass

Tested against Skosmos 3.1 with Fuseki 5.4.0, searchByNotation true, sparqlDialect "JenaText".

When searchByNotation is enabled and a concept is matched by its
skos:notation, the labelpriority BIND block was missing priority 5/6
and transformConceptSearchResult() had no handler for notation matches.

Add the missing BIND, SELECT/GROUP BY entries, and a result handler
that populates a new matchedNotation field — parallel to the existing
matchedPrefLabel — so API consumers can see why a concept matched.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@fvogel
fvogel force-pushed the fix/notation-search-missing-bind branch from 407c898 to 1f36809 Compare February 12, 2026 22:55
@fvogel fvogel changed the title Fix notation search matches missing from labelpriority BIND Add matchedNotation field to search results for notation matches Feb 12, 2026
@sonarqubecloud

Copy link
Copy Markdown

@osma osma moved this to Under review in Skosmos 3.x Backlog Mar 3, 2026

@osma osma left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for the suggested enhancement and the PR to implement it.

It all looks good except for one rather minor detail: I don't like the SPARQL variable name ?nlabel because a notation is not a label. I understand this name is intended to match the style of ?alabel and ?hlabel for altLabels and hiddenLabels, respectively. In hindsight, maybe those variable names were not so clear from the start.

OTOH, the variable ?notation is already in use for returning the notation (regardless of whether it was matched) so renaming ?nlabel to ?notation is not possible either.

I suggest renaming ?nlabel to ?nmatch and then also renaming ?alabel to ?amatch and ?hlabel to ?hmatch. This needs to be done in a few different places in the code. Once this is done then all three variable names are clearer than they are currently and align well with each other.

@osma osma self-assigned this Mar 3, 2026
@osma

osma commented Mar 3, 2026

Copy link
Copy Markdown
Member

Also, we would like to have PHPUnit tests for features like this. It could go into GenericSparqlTest. Though I admit that we don't have 100% coverage for search functionality, at least it would be good not to decrease it.

@osma osma moved this from Under review to Skosmos 3.x Backlog (not this sprint) in Skosmos 3.x Backlog Apr 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Skosmos 3.x Backlog (not this sprint)

Development

Successfully merging this pull request may close these issues.

Enhancement: add matchedNotation field to search results for notation matches

2 participants