From b8b0249c90aa8a69e8ee42b751779b6d011e3c9f Mon Sep 17 00:00:00 2001 From: Daniel Ecer Date: Tue, 18 Aug 2026 13:24:52 +0100 Subject: [PATCH 1/3] Use one label for every citation identifier The citation model's generated training data labelled DOIs and every other identifier . GROBID's corpus has no label, and CitationSemanticExtractor had no mapping for it, so a DOI predicted by a model trained on generated data became a note: never a SemanticExternalIdentifier, never scored as reference_doi, and not counted towards is_reference_valid. Over the committed corpus that was 5431 of 5446 identifier tokens, and 583 of 589 identifier elements. models/citation/labels.py now states the label set, the identifier label and the labels that legitimately stay notes; the training TEI paths, the JATS sub-field map, the extractor and the benchmark's reference_doi analysis all read from it, and a test fails if they drift apart. Type detection stays at extraction, so no training label depends on a regex having matched - which is what gave those five truncated DOIs a type="DOI" they could not earn. CitationTrainingTeiParser maps any idno element back to the identifier label rather than enumerating the types get_post_processed_xml_root can write, so a sixth type cannot make the existing corpus unparseable. SimpleModelSemanticExtractor warns once per label when a model emits something outside its expected note set; only citation opts in. Over the same corpus, typed DOI identifiers rise from 38 - all of them -derived - to 616 across 3304 references. --- .../analyze_field_regressions/_models.py | 4 +- sciencebeam_parser/models/citation/extract.py | 8 +- sciencebeam_parser/models/citation/labels.py | 44 ++++++ .../models/citation/training_data.py | 35 ++--- sciencebeam_parser/models/extract.py | 24 +++- sciencebeam_parser/models/training_data.py | 9 +- .../training/jats/field_vocab.py | 8 +- sciencebeam_parser/utils/labels.py | 5 +- tests/models/citation/labels_test.py | 59 +++++++++ tests/models/citation/training_data_test.py | 125 +++++++++++++++--- tests/models/extract_test.py | 79 ++++++++++- 11 files changed, 352 insertions(+), 48 deletions(-) create mode 100644 sciencebeam_parser/models/citation/labels.py create mode 100644 tests/models/citation/labels_test.py diff --git a/benchmarks/analyze_field_regressions/_models.py b/benchmarks/analyze_field_regressions/_models.py index a82ea7d3..306d2d8b 100644 --- a/benchmarks/analyze_field_regressions/_models.py +++ b/benchmarks/analyze_field_regressions/_models.py @@ -2,6 +2,8 @@ from typing import Dict, List, Optional +from sciencebeam_parser.models.citation.labels import IDENTIFIER_LABEL + _REFERENCE_MODEL_LABELS: Dict[str, frozenset] = { 'segmentation': frozenset({''}), 'reference-segmenter': frozenset({''}), @@ -13,7 +15,7 @@ MODEL_RELEVANT_LABELS: Dict[str, Dict[str, frozenset]] = { 'reference_doi': {**_REFERENCE_MODEL_LABELS, - 'citation': frozenset({'', ''})}, + 'citation': frozenset({IDENTIFIER_LABEL, ''})}, 'reference_title': {**_REFERENCE_MODEL_LABELS, 'citation': frozenset({''})}, 'first_reference_text': _REFERENCE_MODEL_LABELS, 'title': {**_HEADER_MODEL_LABELS, 'header': frozenset({'<title>'})}, diff --git a/sciencebeam_parser/models/citation/extract.py b/sciencebeam_parser/models/citation/extract.py index 9ba98c59..dd604215 100644 --- a/sciencebeam_parser/models/citation/extract.py +++ b/sciencebeam_parser/models/citation/extract.py @@ -25,6 +25,7 @@ T_SemanticContentFactory ) from sciencebeam_parser.document.layout_document import LayoutBlock +from sciencebeam_parser.models.citation.labels import IDENTIFIER_LABEL, NOTE_CITATION_LABELS from sciencebeam_parser.models.extract import SimpleModelSemanticExtractor @@ -181,7 +182,10 @@ def get_invalid_reference(ref: SemanticReference) -> SemanticInvalidReference: class CitationSemanticExtractor(SimpleModelSemanticExtractor): def __init__(self): - super().__init__(semantic_content_class_by_tag=SIMPLE_SEMANTIC_CONTENT_CLASS_BY_TAG) + super().__init__( + semantic_content_class_by_tag=SIMPLE_SEMANTIC_CONTENT_CLASS_BY_TAG, + expected_note_tags=NOTE_CITATION_LABELS + ) def get_semantic_content_for_entity_name( # pylint: disable=too-many-return-statements self, @@ -192,7 +196,7 @@ def get_semantic_content_for_entity_name( # pylint: disable=too-many-return-sta return parse_page_range(layout_block) if name == '<web>': return parse_web(layout_block) - if name == '<pubnum>': + if name == IDENTIFIER_LABEL: return parse_pubnum(layout_block) if name == '<date>': return parse_date(layout_block) diff --git a/sciencebeam_parser/models/citation/labels.py b/sciencebeam_parser/models/citation/labels.py new file mode 100644 index 00000000..b3ae603e --- /dev/null +++ b/sciencebeam_parser/models/citation/labels.py @@ -0,0 +1,44 @@ +from typing import AbstractSet, FrozenSet + + +# The label set the citation model is trained and served against, matching GROBID's +# citation corpus (grobid-trainer/resources/dataset/citation). +# +# Every identifier carries IDENTIFIER_LABEL, whatever kind it is; the kind is detected +# from the text at extraction, so no label depends on a regex having matched. +IDENTIFIER_LABEL = '<pubnum>' + +OTHER_LABEL = '<other>' + +CITATION_LABELS: FrozenSet[str] = frozenset({ + '<author>', + '<booktitle>', + '<collaboration>', + '<date>', + '<editor>', + '<institution>', + '<issue>', + '<journal>', + '<location>', + '<note>', + OTHER_LABEL, + '<pages>', + '<publisher>', + '<series>', + '<tech>', + '<title>', + '<volume>', + '<web>', + IDENTIFIER_LABEL +}) + +# Labels with no semantic counterpart, which CitationSemanticExtractor keeps as notes. +# Anything else in CITATION_LABELS has to map to semantic content. +NOTE_CITATION_LABELS: AbstractSet[str] = frozenset({ + '<booktitle>', + '<collaboration>', + '<institution>', + '<note>', + '<series>', + '<tech>' +}) diff --git a/sciencebeam_parser/models/citation/training_data.py b/sciencebeam_parser/models/citation/training_data.py index d84b6a1f..ca62d780 100644 --- a/sciencebeam_parser/models/citation/training_data.py +++ b/sciencebeam_parser/models/citation/training_data.py @@ -3,25 +3,19 @@ from lxml import etree -from sciencebeam_parser.document.semantic_document import SemanticExternalIdentifierTypes from sciencebeam_parser.document.tei.common import TEI_NS_PREFIX, tei_xpath from sciencebeam_parser.models.training_data import ( AbstractTeiTrainingDataGenerator, - AbstractTrainingTeiParser + AbstractTrainingTeiParser, + TeiTrainingElementPath ) from sciencebeam_parser.models.citation.extract import ( get_detected_external_identifier_type_for_text ) +from sciencebeam_parser.models.citation.labels import IDENTIFIER_LABEL from sciencebeam_parser.utils.xml import get_text_content -# get_post_processed_xml_root tags <idno> elements with any detected identifier type -# (not just DOI); all but DOI should still resolve back to the <pubnum> label -_PUBNUM_EXTERNAL_IDENTIFIER_TYPES = ( - SemanticExternalIdentifierTypes.ARXIV, - SemanticExternalIdentifierTypes.PII, - SemanticExternalIdentifierTypes.PMCID, - SemanticExternalIdentifierTypes.PMID, -) +_IDNO_TAG = TEI_NS_PREFIX + 'idno' # Matches "388 - 412", "281-282", "1199 -1207" etc. _PAGE_RANGE_RE = re.compile(r'^(\S+)\s*[-–]\s*(\S+)$') @@ -52,12 +46,15 @@ '<location>': ROOT_TRAINING_XML_ELEMENT_PATH + ['pubPlace'], '<tech>': ROOT_TRAINING_XML_ELEMENT_PATH + ['note[@type="report"]'], '<web>': ROOT_TRAINING_XML_ELEMENT_PATH + ['ptr[@type="web"]'], - '<idno>': ROOT_TRAINING_XML_ELEMENT_PATH + ['idno[@type="DOI"]'], - '<pubnum>': ROOT_TRAINING_XML_ELEMENT_PATH + ['idno'], + IDENTIFIER_LABEL: ROOT_TRAINING_XML_ELEMENT_PATH + ['idno'], '<note>': ROOT_TRAINING_XML_ELEMENT_PATH + ['note'] } +def _is_idno_path_step(path_step: str) -> bool: + return path_step == _IDNO_TAG or path_step.startswith(_IDNO_TAG + '[') + + class CitationTeiTrainingDataGenerator(AbstractTeiTrainingDataGenerator): DEFAULT_TEI_FILENAME_SUFFIX = '.references.tei.xml' @@ -101,7 +98,13 @@ def __init__(self) -> None: ), use_tei_namespace=True ) - for external_identifier_type in _PUBNUM_EXTERNAL_IDENTIFIER_TYPES: - self.label_by_relative_element_path_map[ - ('{}idno[@type="{}"]'.format(TEI_NS_PREFIX, external_identifier_type),) - ] = '<pubnum>' + + def get_label_for_element_path( + self, + tei_training_element_path: TeiTrainingElementPath, + text: str + ) -> str: + element_path = tei_training_element_path.get_path() + if element_path and _is_idno_path_step(element_path[-1]): + return IDENTIFIER_LABEL + return super().get_label_for_element_path(tei_training_element_path, text=text) diff --git a/sciencebeam_parser/models/extract.py b/sciencebeam_parser/models/extract.py index 8bab6d0d..e47f5355 100644 --- a/sciencebeam_parser/models/extract.py +++ b/sciencebeam_parser/models/extract.py @@ -1,7 +1,7 @@ from abc import ABC, abstractmethod import logging import re -from typing import Iterable, Mapping, Optional, Tuple +from typing import AbstractSet, Iterable, Mapping, Optional, Set, Tuple from sciencebeam_parser.document.semantic_document import ( SemanticContentWrapper, @@ -9,6 +9,7 @@ T_SemanticContentFactory ) from sciencebeam_parser.document.layout_document import EMPTY_BLOCK, LayoutBlock, LayoutTokensText +from sciencebeam_parser.utils.labels import OTHER_LABELS LOGGER = logging.getLogger(__name__) @@ -57,10 +58,28 @@ def __init__( self, semantic_content_class_by_tag: Optional[ Mapping[str, T_SemanticContentFactory] - ] = None + ] = None, + *, + expected_note_tags: Optional[AbstractSet[str]] = None ): super().__init__() self.semantic_content_class_by_tag = semantic_content_class_by_tag or {} + self.expected_note_tags = expected_note_tags + self._reported_unexpected_note_tags: Set[str] = set() + + def _report_unexpected_note_tag(self, name: str) -> None: + if self.expected_note_tags is None: + return + if name in self.expected_note_tags or name in OTHER_LABELS: + return + if name in self._reported_unexpected_note_tags: + return + self._reported_unexpected_note_tags.add(name) + LOGGER.warning( + 'no semantic content for label %r, keeping it as a note' + ' (it will not appear in any extracted field)', + name + ) def get_semantic_content_for_entity_name( self, @@ -70,6 +89,7 @@ def get_semantic_content_for_entity_name( semantic_content_class = self.semantic_content_class_by_tag.get(name) if semantic_content_class: return semantic_content_class(layout_block=layout_block) + self._report_unexpected_note_tag(name) return SemanticNote( layout_block=layout_block, note_type=name diff --git a/sciencebeam_parser/models/training_data.py b/sciencebeam_parser/models/training_data.py index 874bd9cf..aed4d56d 100644 --- a/sciencebeam_parser/models/training_data.py +++ b/sciencebeam_parser/models/training_data.py @@ -7,7 +7,7 @@ from lxml.builder import ElementMaker from sciencebeam_parser.utils.xml_writer import XmlTreeWriter -from sciencebeam_parser.utils.labels import get_split_prefix_label +from sciencebeam_parser.utils.labels import OTHER_LABELS, get_split_prefix_label from sciencebeam_parser.utils.tokenizer import get_tokenized_tokens from sciencebeam_parser.document.tei.common import TEI_E, TEI_NS_PREFIX, tei_xpath from sciencebeam_parser.document.layout_document import ( @@ -30,9 +30,6 @@ NO_NS_TEI_E = ElementMaker() -OTHER_LABELS = {'<other>', 'O'} - - class ExtractInstruction: pass @@ -568,7 +565,7 @@ def __init__( self.root_training_xml_xpath = './' + '/'.join(root_training_xml_element_path) self.line_as_token = line_as_token - def _get_label_for_element_path( + def get_label_for_element_path( self, tei_training_element_path: TeiTrainingElementPath, text: str @@ -605,7 +602,7 @@ def iter_parse_training_tei_to_flat_labeled_layout_tokens( continue token_count = 0 if text.path.element_list: - label = self._get_label_for_element_path(text.path, text=text.text) + label = self.get_label_for_element_path(text.path, text=text.text) if prev_label != label: prefix = 'B-' if text.is_start else 'I-' else: diff --git a/sciencebeam_parser/training/jats/field_vocab.py b/sciencebeam_parser/training/jats/field_vocab.py index a5c6761e..b29e7b9f 100644 --- a/sciencebeam_parser/training/jats/field_vocab.py +++ b/sciencebeam_parser/training/jats/field_vocab.py @@ -1,5 +1,7 @@ from typing import Dict +from sciencebeam_parser.models.citation.labels import IDENTIFIER_LABEL + class JatsFieldNames: TITLE = 'title' @@ -116,9 +118,9 @@ class JatsSubFieldNames: JatsSubFieldNames.REFERENCE_ISSUE: '<issue>', JatsSubFieldNames.REFERENCE_FPAGE: '<pages>', JatsSubFieldNames.REFERENCE_LPAGE: '<pages>', - JatsSubFieldNames.REFERENCE_DOI: '<idno>', - JatsSubFieldNames.REFERENCE_PMID: '<pubnum>', - JatsSubFieldNames.REFERENCE_PMCID: '<pubnum>', + JatsSubFieldNames.REFERENCE_DOI: IDENTIFIER_LABEL, + JatsSubFieldNames.REFERENCE_PMID: IDENTIFIER_LABEL, + JatsSubFieldNames.REFERENCE_PMCID: IDENTIFIER_LABEL, JatsSubFieldNames.REFERENCE_WEB: '<web>', JatsSubFieldNames.REFERENCE_LABEL: '<note>', JatsSubFieldNames.REFERENCE_PUBLISHER_NAME: '<publisher>', diff --git a/sciencebeam_parser/utils/labels.py b/sciencebeam_parser/utils/labels.py index 293a3a74..47c7fa8f 100644 --- a/sciencebeam_parser/utils/labels.py +++ b/sciencebeam_parser/utils/labels.py @@ -1,4 +1,7 @@ -from typing import Tuple +from typing import FrozenSet, Tuple + + +OTHER_LABELS: FrozenSet[str] = frozenset({'<other>', 'O'}) def strip_tag_prefix(tag: str) -> str: diff --git a/tests/models/citation/labels_test.py b/tests/models/citation/labels_test.py new file mode 100644 index 00000000..f9af731b --- /dev/null +++ b/tests/models/citation/labels_test.py @@ -0,0 +1,59 @@ +import logging + +import pytest + +from sciencebeam_parser.document.layout_document import LayoutBlock +from sciencebeam_parser.document.semantic_document import SemanticNote +from sciencebeam_parser.models.citation.extract import CitationSemanticExtractor +from sciencebeam_parser.models.citation.labels import ( + CITATION_LABELS, + IDENTIFIER_LABEL, + NOTE_CITATION_LABELS, + OTHER_LABEL +) +from sciencebeam_parser.models.citation.training_data import ( + TRAINING_XML_ELEMENT_PATH_BY_LABEL +) +from sciencebeam_parser.training.jats.field_vocab import CITATION_LABEL_BY_SUB_FIELD + + +LOGGER = logging.getLogger(__name__) + + +TEXT_1 = 'text 1' + + +class TestCitationLabels: + def test_should_declare_note_labels_within_the_label_set(self): + assert NOTE_CITATION_LABELS <= CITATION_LABELS + + def test_should_declare_identifier_label_within_the_label_set(self): + assert IDENTIFIER_LABEL in CITATION_LABELS + + def test_should_generate_training_tei_for_every_label_except_other(self): + assert set(TRAINING_XML_ELEMENT_PATH_BY_LABEL.keys()) == CITATION_LABELS - {OTHER_LABEL} + + def test_should_only_use_known_labels_for_jats_sub_fields(self): + assert set(CITATION_LABEL_BY_SUB_FIELD.values()) <= CITATION_LABELS + + def test_should_use_the_identifier_label_for_every_jats_identifier_sub_field(self): + identifier_labels = { + label + for sub_field, label in CITATION_LABEL_BY_SUB_FIELD.items() + if sub_field.endswith(('-doi', '-pmid', '-pmcid')) + } + assert identifier_labels == {IDENTIFIER_LABEL} + + @pytest.mark.parametrize("label", sorted(CITATION_LABELS - {OTHER_LABEL})) + def test_should_extract_semantic_content_for_every_label_that_is_not_a_declared_note( + self, + label: str + ): + semantic_content = CitationSemanticExtractor().get_semantic_content_for_entity_name( + label, layout_block=LayoutBlock.for_text(TEXT_1) + ) + LOGGER.debug('semantic_content: %r', semantic_content) + if label in NOTE_CITATION_LABELS: + assert isinstance(semantic_content, SemanticNote) + else: + assert not isinstance(semantic_content, SemanticNote) diff --git a/tests/models/citation/training_data_test.py b/tests/models/citation/training_data_test.py index 3119e98e..93055373 100644 --- a/tests/models/citation/training_data_test.py +++ b/tests/models/citation/training_data_test.py @@ -1,5 +1,5 @@ import logging -from typing import Iterable, Optional, Sequence +from typing import Iterable, Optional, Sequence, Tuple import pytest from lxml import etree @@ -9,7 +9,11 @@ LayoutDocument, LayoutLine ) -from sciencebeam_parser.document.semantic_document import SemanticExternalIdentifierTypes +from sciencebeam_parser.document.semantic_document import ( + SemanticExternalIdentifier, + SemanticExternalIdentifierTypes, + SemanticReference +) from sciencebeam_parser.document.tei.common import ( TEI_E, get_tei_xpath_text_content_list, @@ -20,7 +24,12 @@ DEFAULT_DOCUMENT_FEATURES_CONTEXT, LayoutModelData ) +from sciencebeam_parser.models.model import ( + iter_entity_layout_blocks_for_labeled_layout_tokens +) from sciencebeam_parser.models.citation.data import CitationDataGenerator +from sciencebeam_parser.models.citation.extract import CitationSemanticExtractor +from sciencebeam_parser.models.citation.labels import IDENTIFIER_LABEL from sciencebeam_parser.models.citation.training_data import ( ROOT_TRAINING_XML_ELEMENT_PATH, TRAINING_XML_ELEMENT_PATH_BY_LABEL, @@ -78,6 +87,32 @@ def get_training_tei_xml_for_model_data_iterable( return xml_root +def get_semantic_references_for_training_tei_xml( + xml_root: etree.ElementBase +) -> Sequence[SemanticReference]: + extractor = CitationSemanticExtractor() + labeled_layout_tokens_list = ( + get_training_tei_parser().parse_training_tei_to_labeled_layout_tokens_list(xml_root) + ) + return [ + semantic_content + for labeled_layout_tokens in labeled_layout_tokens_list + for semantic_content in extractor.iter_semantic_content_for_entity_blocks( + iter_entity_layout_blocks_for_labeled_layout_tokens(labeled_layout_tokens) + ) + if isinstance(semantic_content, SemanticReference) + ] + + +def get_external_identifier_types_and_values( + semantic_reference: SemanticReference +) -> Sequence[Tuple[Optional[str], Optional[str]]]: + return [ + (external_identifier.external_identifier_type, external_identifier.value) + for external_identifier in semantic_reference.iter_by_type(SemanticExternalIdentifier) + ] + + def get_training_tei_xml_for_layout_document( layout_document: LayoutDocument ) -> etree.ElementBase: @@ -450,17 +485,17 @@ def test_should_parse_single_label_with_multiple_tokens_on_multiple_lines(self): "external_identifier_type", [ SemanticExternalIdentifierTypes.ARXIV, + SemanticExternalIdentifierTypes.DOI, SemanticExternalIdentifierTypes.PII, SemanticExternalIdentifierTypes.PMCID, - SemanticExternalIdentifierTypes.PMID + SemanticExternalIdentifierTypes.PMID, + 'other' ] ) - def test_should_parse_non_doi_idno_type_as_pubnum( + def test_should_parse_any_idno_type_as_identifier_label( self, external_identifier_type: str ): - # get_post_processed_xml_root tags <pubnum>-derived <idno> elements with the - # detected identifier type (e.g. PMCID); parsing must still recover <pubnum> tei_root = _get_training_tei_with_references([ TEI_E('bibl', *[ TEI_E('idno', {'type': external_identifier_type}, TOKEN_1, TEI_E('lb')), @@ -471,12 +506,26 @@ def test_should_parse_non_doi_idno_type_as_pubnum( tei_root ) assert tag_result == [[ - (TOKEN_1, 'B-<pubnum>') + (TOKEN_1, f'B-{IDENTIFIER_LABEL}') ]] - def test_should_round_trip_pubnum_with_detected_external_identifier_type(self): + @pytest.mark.parametrize( + "text,expected_type,expected_value", + [ + ('10.1234/test', SemanticExternalIdentifierTypes.DOI, '10.1234/test'), + ('arXiv: 0706.0001', SemanticExternalIdentifierTypes.ARXIV, '0706.0001'), + ('PMID: 1234567', SemanticExternalIdentifierTypes.PMID, '1234567'), + ('PMC1234567', SemanticExternalIdentifierTypes.PMCID, 'PMC1234567') + ] + ) + def test_should_round_trip_identifier_to_typed_semantic_external_identifier( + self, + text: str, + expected_type: str, + expected_value: str + ): label_and_layout_line_list = [ - ('<pubnum>', get_next_layout_line_for_text('PMC1234567')) + (IDENTIFIER_LABEL, get_next_layout_line_for_text(text)) ] labeled_model_data_list = get_labeled_model_data_list( label_and_layout_line_list, @@ -486,14 +535,58 @@ def test_should_round_trip_pubnum_with_detected_external_identifier_type(self): labeled_model_data_list ) assert get_tei_xpath_text_content_list( - xml_root, f'{BIBL_XPATH}/tei:idno[@type="{SemanticExternalIdentifierTypes.PMCID}"]' - ) == ['PMC1234567'] - tag_result = get_training_tei_parser().parse_training_tei_to_tag_result( - xml_root + xml_root, f'{BIBL_XPATH}/tei:idno[@type="{expected_type}"]' + ) == [text] + references = get_semantic_references_for_training_tei_xml(xml_root) + assert len(references) == 1 + assert get_external_identifier_types_and_values(references[0]) == [ + (expected_type, expected_value) + ] + + def test_should_round_trip_multiple_identifiers_of_one_reference(self): + label_and_layout_line_list = [ + ('<title>', get_next_layout_line_for_text('Title 1')), + (IDENTIFIER_LABEL, get_next_layout_line_for_text('10.1234/test')), + ('O', get_next_layout_line_for_text('and')), + (IDENTIFIER_LABEL, get_next_layout_line_for_text('arXiv: 0706.0001')), + ('O', get_next_layout_line_for_text('and')), + (IDENTIFIER_LABEL, get_next_layout_line_for_text('PMID: 1234567')) + ] + labeled_model_data_list = get_labeled_model_data_list( + label_and_layout_line_list, + data_generator=get_data_generator() ) - assert tag_result == [[ - ('PMC1234567', 'B-<pubnum>') - ]] + xml_root = get_training_tei_xml_for_model_data_iterable( + labeled_model_data_list + ) + references = get_semantic_references_for_training_tei_xml(xml_root) + assert len(references) == 1 + assert get_external_identifier_types_and_values(references[0]) == [ + (SemanticExternalIdentifierTypes.DOI, '10.1234/test'), + (SemanticExternalIdentifierTypes.ARXIV, '0706.0001'), + (SemanticExternalIdentifierTypes.PMID, '1234567') + ] + + def test_should_merge_directly_adjacent_identifiers_of_one_reference(self): + # the parser reconstructs B-/I- from label changes rather than element boundaries, + # so two idno elements with only whitespace between them come back as one identifier + label_and_layout_line_list = [ + (IDENTIFIER_LABEL, get_next_layout_line_for_text('10.1234/test')), + (IDENTIFIER_LABEL, get_next_layout_line_for_text('PMID: 1234567')) + ] + labeled_model_data_list = get_labeled_model_data_list( + label_and_layout_line_list, + data_generator=get_data_generator() + ) + xml_root = get_training_tei_xml_for_model_data_iterable( + labeled_model_data_list + ) + assert len(tei_xpath(xml_root, f'{BIBL_XPATH}/tei:idno')) == 2 + references = get_semantic_references_for_training_tei_xml(xml_root) + assert len(references) == 1 + assert get_external_identifier_types_and_values(references[0]) == [ + (SemanticExternalIdentifierTypes.DOI, '10.1234/testPMID:1234567') + ] @pytest.mark.parametrize( "tei_label,element_path", diff --git a/tests/models/extract_test.py b/tests/models/extract_test.py index 7a8b58e9..3a513f8e 100644 --- a/tests/models/extract_test.py +++ b/tests/models/extract_test.py @@ -1,5 +1,13 @@ +import logging + +import pytest + from sciencebeam_parser.document.layout_document import LayoutBlock -from sciencebeam_parser.models.extract import get_regex_cleaned_layout_block_with_prefix_suffix +from sciencebeam_parser.document.semantic_document import SemanticNote, SemanticTitle +from sciencebeam_parser.models.extract import ( + SimpleModelSemanticExtractor, + get_regex_cleaned_layout_block_with_prefix_suffix +) class TestGetRegexCleanedLayoutBlockWithPrefixSuffix: @@ -62,3 +70,72 @@ def test_should_return_prefix_suffix_for_prefix_suffix_match(self): assert prefix_block.text == 'a' assert cleaned_block.text == 'b c' assert suffix_block.text == 'd' + + +class SimpleExtractor(SimpleModelSemanticExtractor): + def iter_semantic_content_for_entity_blocks(self, entity_tokens, **kwargs): + return [ + self.get_semantic_content_for_entity_name(name, layout_block=layout_block) + for name, layout_block in entity_tokens + ] + + +def _get_semantic_content_for_entity_name( + extractor: SimpleModelSemanticExtractor, + name: str +): + return extractor.get_semantic_content_for_entity_name( + name, layout_block=LayoutBlock.for_text('text 1') + ) + + +class TestSimpleModelSemanticExtractor: + def test_should_use_mapped_semantic_content_class(self): + extractor = SimpleExtractor({'<title>': SemanticTitle}) + assert isinstance( + _get_semantic_content_for_entity_name(extractor, '<title>'), + SemanticTitle + ) + + def test_should_keep_unmapped_label_as_note(self): + extractor = SimpleExtractor({'<title>': SemanticTitle}) + semantic_content = _get_semantic_content_for_entity_name(extractor, '<other-label>') + assert isinstance(semantic_content, SemanticNote) + assert semantic_content.note_type == '<other-label>' + + def test_should_not_warn_about_unmapped_label_without_expected_note_tags( + self, + caplog: pytest.LogCaptureFixture + ): + extractor = SimpleExtractor({'<title>': SemanticTitle}) + with caplog.at_level(logging.WARNING): + _get_semantic_content_for_entity_name(extractor, '<other-label>') + assert not caplog.records + + @pytest.mark.parametrize("name", ['<note>', 'O', '<other>']) + def test_should_not_warn_about_expected_note_or_other_tags( + self, + name: str, + caplog: pytest.LogCaptureFixture + ): + extractor = SimpleExtractor( + {'<title>': SemanticTitle}, + expected_note_tags={'<note>'} + ) + with caplog.at_level(logging.WARNING): + _get_semantic_content_for_entity_name(extractor, name) + assert not caplog.records + + def test_should_warn_once_about_an_unexpected_note_tag( + self, + caplog: pytest.LogCaptureFixture + ): + extractor = SimpleExtractor( + {'<title>': SemanticTitle}, + expected_note_tags={'<note>'} + ) + with caplog.at_level(logging.WARNING): + _get_semantic_content_for_entity_name(extractor, '<idno>') + _get_semantic_content_for_entity_name(extractor, '<idno>') + assert len(caplog.records) == 1 + assert '<idno>' in caplog.records[0].getMessage() From 3a995867a5c8afaaed71fdb3b0c147c3e01a096e Mon Sep 17 00:00:00 2001 From: Daniel Ecer <d.ecer@elifesciences.org> Date: Tue, 18 Aug 2026 13:40:08 +0100 Subject: [PATCH 2/3] Cite where GROBID draws the identifier line TEICitationSaxParser maps both idno and pubnum to <pubnum> and leaves the type unused ("TBD: keep the idno type for further exploitation"); CitationParser types the prediction afterwards through BiblioItem.checkIdentifier(). Its training TEI spells the kinds type="arXiv" and type="PMC" and covers ISSN and ISBN, none of which SemanticExternalIdentifierTypes has - so an enumerated type list would raise on GROBID's own corpus. --- sciencebeam_parser/models/citation/labels.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/sciencebeam_parser/models/citation/labels.py b/sciencebeam_parser/models/citation/labels.py index b3ae603e..a57674c9 100644 --- a/sciencebeam_parser/models/citation/labels.py +++ b/sciencebeam_parser/models/citation/labels.py @@ -2,10 +2,15 @@ # The label set the citation model is trained and served against, matching GROBID's -# citation corpus (grobid-trainer/resources/dataset/citation). +# citation corpus. # # Every identifier carries IDENTIFIER_LABEL, whatever kind it is; the kind is detected -# from the text at extraction, so no label depends on a regex having matched. +# from the text at extraction, so no label depends on a regex having matched. This is +# what GROBID does: TEICitationSaxParser maps both idno and pubnum to <pubnum>, and +# CitationParser types the prediction afterwards via BiblioItem.checkIdentifier(). Its +# training TEI carries the kind as an idno attribute, spelled type="arXiv" and +# type="PMC" and also covering ISSN and ISBN, which is why the parser accepts any +# idno type rather than the ones detection can produce here. IDENTIFIER_LABEL = '<pubnum>' OTHER_LABEL = '<other>' From 53c4a473ba27e95a2ac2a7163ee3a6b78d7d4a35 Mon Sep 17 00:00:00 2001 From: Daniel Ecer <d.ecer@elifesciences.org> Date: Wed, 19 Aug 2026 13:12:38 +0100 Subject: [PATCH 3/3] Start a new identifier when the JATS sub-field kind changes One label for every identifier leaves nothing to separate a DOI from a PMID that follows it directly: the JATS label function returns bare labels, so the generator kept writing into the open <idno> and produced a single element whose value carried both identifiers, typed as a DOI. The previous split avoided that only because the two kinds had different element paths. Emitting B-<pubnum> at a sub-field change within a reference gives each identifier its own element again. Same shape as the reference segmenter's instance transitions. Two identifiers of one kind in a reference stay undistinguished - the annotation carries sub-field kind and reference instance, not sub-field instance. The delft conversion still merges the two elements into one training span, because the training TEI parser rebuilds B-/I- from label changes rather than element boundaries. That is a shared-parser defect, recorded separately, and it has to land before the artifact is regenerated. --- .../training/cli/generate_data.py | 20 +++- tests/training/cli/generate_data_test.py | 92 +++++++++++++++++++ 2 files changed, 111 insertions(+), 1 deletion(-) diff --git a/sciencebeam_parser/training/cli/generate_data.py b/sciencebeam_parser/training/cli/generate_data.py index e5488cec..4b46cf23 100644 --- a/sciencebeam_parser/training/cli/generate_data.py +++ b/sciencebeam_parser/training/cli/generate_data.py @@ -41,6 +41,7 @@ iter_data_lines_for_model_data_iterables, iter_labeled_layout_token_for_layout_model_label ) +from sciencebeam_parser.models.citation.labels import IDENTIFIER_LABEL from sciencebeam_parser.models.training_data import TeiTrainingDataGenerator from sciencebeam_parser.processors.fulltext.models import FullTextModels from sciencebeam_parser.resources.default_config import DEFAULT_CONFIG_FILE @@ -1166,16 +1167,33 @@ def get_main_model(self, document_context: TrainingDataDocumentContext) -> Model return document_context.fulltext_models.citation_model def get_jats_label_fn(self) -> Optional[JatsLabelFn]: + previous_identifier: Optional[Tuple[int, Optional[str]]] = None + def fn( annotated: JatsAnnotatedLayoutDocument, _seg_labels: Dict[int, str], md: LayoutModelData, ) -> Optional[str]: + nonlocal previous_identifier token = md.layout_token if not token: return None sub_field = annotated.get_token_sub_field(token) - return CITATION_LABEL_BY_SUB_FIELD.get(sub_field or '') if sub_field else None + label = CITATION_LABEL_BY_SUB_FIELD.get(sub_field or '') if sub_field else None + if label != IDENTIFIER_LABEL: + previous_identifier = None + return label + # Identifiers of different kinds share one label, so without a B- prefix a DOI + # directly followed by a PMID would be written as a single <idno>, and typed as + # one identifier whose value carries both. + identifier = (annotated.get_token_instance(token), sub_field) + is_new_identifier = ( + previous_identifier is not None + and previous_identifier[0] == identifier[0] + and previous_identifier[1] != identifier[1] + ) + previous_identifier = identifier + return 'B-' + label if is_new_identifier else label return fn def iter_model_layout_documents( diff --git a/tests/training/cli/generate_data_test.py b/tests/training/cli/generate_data_test.py index 64b1cacb..34343806 100644 --- a/tests/training/cli/generate_data_test.py +++ b/tests/training/cli/generate_data_test.py @@ -47,6 +47,7 @@ from sciencebeam_parser.models.citation.training_data import ( CitationTeiTrainingDataGenerator ) +from sciencebeam_parser.models.citation.labels import IDENTIFIER_LABEL from sciencebeam_parser.training.jats.annotated_document import JatsAnnotatedLayoutDocument from sciencebeam_parser.training.jats.field_vocab import JatsFieldNames, JatsSubFieldNames import sciencebeam_parser.training.cli.generate_data as generate_data_module @@ -865,6 +866,97 @@ def _make_md(line: LayoutLine, token_idx: int = 0) -> LayoutModelData: ) +def _get_citation_label_list_for_sub_fields( + text: str, + sub_field_by_token_index: Dict[int, str] +) -> List[Optional[str]]: + line = LayoutLine.for_text(text) + citation_doc = LayoutDocument(pages=[LayoutPage(blocks=[LayoutBlock(lines=[line])])]) + annotated = JatsAnnotatedLayoutDocument(layout_document=citation_doc) + for token_index, sub_field in sub_field_by_token_index.items(): + annotated.set_token_label( + line.tokens[token_index], JatsFieldNames.REFERENCE, + sub_field_name=sub_field, instance_id=1, + ) + label_fn = CitationModelTrainingDataGenerator().get_jats_label_fn() + assert label_fn is not None + return [ + label_fn(annotated, {}, _make_md(line, token_index)) + for token_index in range(len(line.tokens)) + ] + + +@log_on_exception +class TestCitationJatsLabelFn: + def test_should_label_identifier_sub_fields_with_the_identifier_label(self): + assert _get_citation_label_list_for_sub_fields( + 'doi 10 unrelated', + {1: JatsSubFieldNames.REFERENCE_DOI} + ) == [None, IDENTIFIER_LABEL, None] + + def test_should_start_a_new_identifier_when_the_kind_changes(self): + # 'doi' and 'pmid' tokens stand for the identifier values; without a B- prefix on the + # second one the generator would write both into a single <idno> + assert _get_citation_label_list_for_sub_fields( + 'doi pmid', + { + 0: JatsSubFieldNames.REFERENCE_DOI, + 1: JatsSubFieldNames.REFERENCE_PMID + } + ) == [IDENTIFIER_LABEL, 'B-' + IDENTIFIER_LABEL] + + def test_should_not_start_a_new_identifier_within_one_kind(self): + assert _get_citation_label_list_for_sub_fields( + 'doi doi doi', + { + 0: JatsSubFieldNames.REFERENCE_DOI, + 1: JatsSubFieldNames.REFERENCE_DOI, + 2: JatsSubFieldNames.REFERENCE_DOI + } + ) == [IDENTIFIER_LABEL, IDENTIFIER_LABEL, IDENTIFIER_LABEL] + + def test_should_not_start_a_new_identifier_after_unlabelled_text(self): + # the unlabelled token already closes the <idno> element + assert _get_citation_label_list_for_sub_fields( + 'doi and pmid', + { + 0: JatsSubFieldNames.REFERENCE_DOI, + 2: JatsSubFieldNames.REFERENCE_PMID + } + ) == [IDENTIFIER_LABEL, None, IDENTIFIER_LABEL] + + def test_should_not_start_a_new_identifier_across_references(self): + # each reference is its own training document, so the first identifier of the next + # one needs no prefix even though its kind differs from the previous reference's + line = LayoutLine.for_text('pmid doi') + citation_doc = LayoutDocument(pages=[LayoutPage(blocks=[LayoutBlock(lines=[line])])]) + annotated = JatsAnnotatedLayoutDocument(layout_document=citation_doc) + annotated.set_token_label( + line.tokens[0], JatsFieldNames.REFERENCE, + sub_field_name=JatsSubFieldNames.REFERENCE_PMID, instance_id=1, + ) + annotated.set_token_label( + line.tokens[1], JatsFieldNames.REFERENCE, + sub_field_name=JatsSubFieldNames.REFERENCE_DOI, instance_id=2, + ) + label_fn = CitationModelTrainingDataGenerator().get_jats_label_fn() + assert label_fn is not None + assert [ + label_fn(annotated, {}, _make_md(line, 0)), + label_fn(annotated, {}, _make_md(line, 1)) + ] == [IDENTIFIER_LABEL, IDENTIFIER_LABEL] + + def test_should_not_start_a_new_identifier_after_another_label(self): + assert _get_citation_label_list_for_sub_fields( + 'doi 2020 pmid', + { + 0: JatsSubFieldNames.REFERENCE_DOI, + 1: JatsSubFieldNames.REFERENCE_YEAR, + 2: JatsSubFieldNames.REFERENCE_PMID + } + ) == [IDENTIFIER_LABEL, '<date>', IDENTIFIER_LABEL] + + @log_on_exception class TestReferenceSegmenterJatsLabelFn: def test_labels_whole_line_as_reference_when_any_token_labeled(self):