Skip to content

Comments

MODLD-510: Authority: Create Hub resource from marc field 100 when $t is present#199

Open
pkjacob wants to merge 2 commits intomasterfrom
pjacob/MODLD-510_2
Open

MODLD-510: Authority: Create Hub resource from marc field 100 when $t is present#199
pkjacob wants to merge 2 commits intomasterfrom
pjacob/MODLD-510_2

Conversation

@pkjacob
Copy link
Contributor

@pkjacob pkjacob commented Feb 17, 2026

No description provided.

Copilot AI review requested due to automatic review settings February 17, 2026 17:32
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds support for mapping MARC Authority field 100 records containing $t (title/uniform title) into HUB resources, updates punctuation-normalization rules for additional 100 subfields, and switches label generation to prefer the LD Dictionary label generator.

Changes:

  • Add a new authority mapping rule to produce HUB resources for 100 fields when $t is present (including CREATOR/TITLE edges).
  • Extend punctuation-normalization rules for additional 100 subfields in both bib and authority normalization configs.
  • Update label generation to use LabelGeneratorService, and adjust/add tests + test data for the new HUB behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/test/resources/authority/100/marc_100_with_$t.jsonl Updates test MARC authority record data for 100 with $t to reflect HUB scenario.
src/test/java/org/folio/marc4ld/authority/field100/MarcToLdAuthorityConcept100IT.java Removes the old “don’t map when $t present” test, aligning with new HUB mapping behavior.
src/test/java/org/folio/marc4ld/authority/field100/Marc2AuthorityHub100IT.java Adds an IT validating that 100+$t now yields a HUB resource.
src/main/resources/normalization/bib/marc2ld_bib_subfield_rules.yml Updates bib punctuation rules for additional 100 subfields.
src/main/resources/normalization/authority/marc2ld_authority_subfield_rules.yaml Adds authority punctuation rules for 100 subfields used by the new HUB mapping.
src/main/resources/marc4ldAuthority.yml Introduces an authority rule mapping 100+$t to HUB with CREATOR/TITLE edges.
src/main/java/org/folio/marc4ld/service/label/LabelServiceImpl.java Prefers dictionary-based label generation with fallback to existing label processor logic.
src/main/java/org/folio/marc4ld/configuration/LabelConfiguration.java Provides a Spring bean for LabelGeneratorService when missing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@pkjacob pkjacob changed the base branch from master to pjacob/MODLD-913 February 18, 2026 22:22
Base automatically changed from pjacob/MODLD-913 to master February 19, 2026 15:01
Copilot AI review requested due to automatic review settings February 19, 2026 15:09
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot AI review requested due to automatic review settings February 19, 2026 16:43
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

marc2ldCondition:
fieldsAllOf:
t: presented
subfields:
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

The subfield k (Form subheading) is included in the normalization rules for both bibliographic and authority records, but is not mapped in the marc4ldAuthority.yml configuration for the HUB mapping. In bibliographic field 600/610 hub mappings (lines 1069, 1162 in marc4ld.yml), subfield k is mapped as LABEL. Consider adding k: LABEL to the HUB subfields section to maintain consistency with bibliographic mappings and to ensure the subfield is properly processed.

Suggested change
subfields:
subfields:
k: LABEL

Copilot uses AI. Check for mistakes.
Copilot AI review requested due to automatic review settings February 19, 2026 17:43
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@pkjacob pkjacob force-pushed the pjacob/MODLD-510_2 branch 5 times, most recently from 47907c5 to cae6209 Compare February 19, 2026 19:42
@pkjacob pkjacob changed the title MODLD-510: Draft changes MODLD-510: Authority: Create Hub resource from marc field 100 when $t is present Feb 19, 2026
Copilot AI review requested due to automatic review settings February 19, 2026 20:21
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@sonarqubecloud
Copy link

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

protected void setConceptFields(Resource concept, DataField dataField) {
addNonRepeatableSubfield(concept, DATE.getValue(), dataField, F, marcFactory);
addNonRepeatableSubfield(concept, MUSIC_KEY.getValue(), dataField, R, marcFactory);
addNonRepeatableSubfield(concept, MEDIUM.getValue(), dataField, M, marcFactory);
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

MEDIUM values on a Resource are list-backed, but this uses addNonRepeatableSubfield, which will emit only the first medium value (and skip subsequent ones if present). If MARC $m should be repeatable/preserve all mediums, switch this to addRepeatableSubfield so multiple mediums aren’t silently dropped during LD→MARC mapping.

Suggested change
addNonRepeatableSubfield(concept, MEDIUM.getValue(), dataField, M, marcFactory);
addRepeatableSubfield(concept, MEDIUM.getValue(), dataField, M, marcFactory);

Copilot uses AI. Check for mistakes.
Comment on lines 1045 to 1051
q: NAME
t: NAME
p: NAME
m: MEDIUM
n: NAME
r: MUSIC_KEY
s: VERSION
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

The PR title is specific to authority 100+$t HUB creation, but this change also updates bib mapping rules (600/610/630 and shared X30/240 hub) and multiple bib mapping tests to add medium handling. Consider updating the PR title/description (or splitting changes) so the broader behavior change to bib mappings is clearly communicated to reviewers and release notes.

Copilot uses AI. Check for mistakes.
c: TITLES
d: DATE
g: MISC_INFO
j: ATTRIBUTION
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

In the new 100+$t HUB rule, the FAMILY creator edge doesn’t map subfield $q (name alternative), while the PERSON creator edge does. The provided test data for the family case includes $q, so this information will be dropped during mapping. Add q: NAME_ALTERNATIVE to the FAMILY creator subfields to keep parity with the other 100 mappings and avoid data loss.

Suggested change
j: ATTRIBUTION
j: ATTRIBUTION
q: NAME_ALTERNATIVE

Copilot uses AI. Check for mistakes.
Comment on lines +101 to +116
private void validateFamilyCreator(Resource hub) {
var creator = getFirstOutgoingEdge(hub, e -> e.getPredicate() == CREATOR).getTarget();
validateResource(
creator,
List.of(FAMILY),
Map.of(
"http://bibfra.me/vocab/lite/name", List.of("House of Windsor"),
"http://bibfra.me/vocab/library/numeration", List.of("Royal family"),
"http://bibfra.me/vocab/library/titles", List.of("Dynasty"),
"http://bibfra.me/vocab/lite/date", List.of("1917-"),
"http://bibfra.me/vocab/library/miscInfo", List.of("British royal house,"),
"http://bibfra.me/vocab/library/attribution", List.of("author of,"),
"http://bibfra.me/vocab/lite/label", List.of("Royal family, House of Windsor, Dynasty, 1917")
),
"Royal family, House of Windsor, Dynasty, 1917"
);
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

The family-fixture includes 100$q, but the test doesn’t assert anything about nameAlternative on the FAMILY creator resource. Once $q is mapped (or if it’s intentionally not mapped), add an explicit assertion here so the intended behavior is covered and doesn’t regress silently.

Copilot uses AI. Check for mistakes.
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.

1 participant