Skip to content

Bugfix/composite property namespace and boost - #1845

Merged
kwahlin merged 5 commits into
developfrom
bugfix/composite-property-namespace-and-boost
Oct 8, 2026
Merged

kwahlin merged 5 commits into
developfrom
bugfix/composite-property-namespace-and-boost

Conversation

@kwahlin

@kwahlin kwahlin commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Fixes a few things related to libris/definitions#628:

  • Score free-text queries against a composite property's combined sub-fields using query_string/cross_fields rather than simple_query_string, to get the best (dis_max-like) match, rather than summing the scores from each field. This avoids scores from e.g. both subject._str and subject.termComponentList._str, which would otherwise always favor complex subjects in the relevancy ranking with the new extended subject: filter.
  • Replace Property.getProperty(..), which included hard-coded namespace handling, with a more general Disambiguate.getPropertyByKey(..), which resolves a known property key against an ordered list of namespaces the same way Disambiguate.mapSingleKey(..) already does. This fixes an immediate bug where :subject was incorrectly parsed as ls:subject here, while also adding more flexibility for controlling which namespaces a query's keys should map against and in what preferred order. I think some ambiguity across different namespaces is inevitable (like :subject/ls:subject in this case) and making the namespace precedence configurable (still TODO) and/or overridable per request could be useful when consolidating the old and the new API. If we for example only want to query the "pure" records, we could simply set ['kbv'] as the precedence order, i.e. "map my keys only to kbv terms".

Keeping this as its own PR instead of adding even more to the base branch, since it's closely tied to libris/definitions#628.

@olovy olovy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

Having an additional boolean parameter is a bit unfortunate

@kwahlin

kwahlin commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

LGTM!

Having an additional boolean parameter is a bit unfortunate

You're right, better to use the enum instead. Fixed: 44389d2

Base automatically changed from feature/refactor-query-building to develop October 8, 2026 05:32
@kwahlin
kwahlin force-pushed the bugfix/composite-property-namespace-and-boost branch from 44389d2 to 8e45029 Compare October 8, 2026 14:37
@kwahlin
kwahlin merged commit 017bc96 into develop Oct 8, 2026
1 check failed
@kwahlin
kwahlin deleted the bugfix/composite-property-namespace-and-boost branch October 8, 2026 14:38
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