diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index 22f3ba70..f1c4d056 100644 --- a/.github/workflows/benchmark.yml +++ b/.github/workflows/benchmark.yml @@ -53,18 +53,34 @@ jobs: github.ref == 'refs/heads/main' && 'medium' || 'smoke' }} - BENCHMARK_PROFILE: >- - ${{ - contains(github.event.pull_request.labels.*.name, 'profile:grobid_crf') && 'grobid_crf' || - contains(github.event.pull_request.labels.*.name, 'profile:biorxiv_elife') && 'biorxiv_elife' || - inputs.profile || - 'grobid_crf' - }} BENCHMARK_RUN: benchmarks/runs/${{ github.sha }}/validation steps: - uses: actions/checkout@v5 + - name: Write GCS model reader key (temporary access to private model bucket) + id: gcp_auth + env: + GCS_MODEL_READER_SA_KEY_B64: ${{ secrets.GCS_MODEL_READER_SA_KEY_B64 }} + run: | + KEY_PATH="${RUNNER_TEMP}/gcs-model-reader-key.json" + printf '%s' "$GCS_MODEL_READER_SA_KEY_B64" | base64 -d > "$KEY_PATH" + chmod 600 "$KEY_PATH" + echo "key_path=$KEY_PATH" >> "$GITHUB_OUTPUT" + + - name: Resolve benchmark profile + env: + PR_LABELS_JSON: ${{ toJson(github.event.pull_request.labels.*.name) }} + DISPATCH_PROFILE: ${{ inputs.profile }} + run: | + LABEL_PROFILE=$(echo "$PR_LABELS_JSON" | jq -r '.[] | select(startswith("profile:")) | sub("^profile:"; "")' | head -n1) + PROFILE="${LABEL_PROFILE:-${DISPATCH_PROFILE:-grobid_crf}}" + if ! [[ "$PROFILE" =~ ^[A-Za-z0-9_]+$ ]]; then + echo "::error::Invalid profile '$PROFILE' (from label or input) - only letters, digits and underscores are allowed" + exit 1 + fi + echo "BENCHMARK_PROFILE=$PROFILE" >> "$GITHUB_ENV" + - name: Set image tag id: image_tag run: | @@ -139,6 +155,8 @@ jobs: fi docker run -d --name sciencebeam-parser -p 8080:8070 \ -e SCIENCEBEAM_PARSER__PRELOAD_ON_STARTUP=true \ + -v "${{ steps.gcp_auth.outputs.key_path }}:/creds/gcp-key.json:ro" \ + -e GOOGLE_APPLICATION_CREDENTIALS=/creds/gcp-key.json \ "${PROFILE_ENV[@]}" ${{ steps.image_tag.outputs.value }} - name: Wait for parser diff --git a/sciencebeam_parser/config/config.py b/sciencebeam_parser/config/config.py index 05a860dc..4b0bae36 100644 --- a/sciencebeam_parser/config/config.py +++ b/sciencebeam_parser/config/config.py @@ -2,7 +2,7 @@ import os import copy from pathlib import Path -from typing import Any, Optional, Union +from typing import Any, FrozenSet, Optional, Union import yaml @@ -27,6 +27,29 @@ def _deep_merge(base: dict, overlay: dict) -> dict: return result +def _resolve_sequence_model_profile( + seq_profiles: dict, + name: str, + _seen: FrozenSet[str] = frozenset() +) -> dict: + if name not in seq_profiles: + raise ValueError( + f'Unknown sequence_model_profile {name!r}. Available: {sorted(seq_profiles)}' + ) + if name in _seen: + raise ValueError( + f'Circular extends detected for sequence_model_profile {name!r} ' + f'(chain: {" -> ".join([*_seen, name])})' + ) + profile = seq_profiles[name] + base_name = profile.get('extends') + overlay = {key: value for key, value in profile.items() if key != 'extends'} + if not base_name: + return overlay + base = _resolve_sequence_model_profile(seq_profiles, base_name, _seen | {name}) + return _deep_merge(base, overlay) + + class AppConfig: def __init__(self, props: dict): self.props = props @@ -87,7 +110,7 @@ def resolve_profile(self, profile_name: Optional[str] = None) -> 'AppConfig': f'Profile {resolved!r} references unknown sequence_model_profile ' f'{seq_name!r}. Available: {sorted(seq_profiles)}' ) - overlay['models'] = seq_profiles[seq_name] + overlay['models'] = _resolve_sequence_model_profile(seq_profiles, seq_name) for key, value in profile.items(): if key != 'sequence_models': diff --git a/sciencebeam_parser/resources/default_config/config.yml b/sciencebeam_parser/resources/default_config/config.yml index 45a205fb..f73a3b52 100644 --- a/sciencebeam_parser/resources/default_config/config.yml +++ b/sciencebeam_parser/resources/default_config/config.yml @@ -179,6 +179,14 @@ sequence_model_profiles: citation: path: 'https://github.com/kermitt2/grobid/raw/0.9.0/grobid-home/models/citation' engine: 'wapiti' + grobid_custom_hybrid: + extends: grobid_crf_0_9_0 + reference_segmenter: + # TODO: switch back once the release is public + # path: 'https://github.com/eLifePathways/sciencebeam-models/releases/download/dev/2026-07-01-delft-grobid-reference-segmenter-CustomBidLSTM_CRF-grobid_090-scielo_preprints-v1.tar.gz' + # path: 'gs://sciencebeam-v2-models/delft/reference-segmenter/models/2026-07-01_CustomBidLSTM_CRF_grobid_090_scielo_preprints_ore/reference-segmenter/' + path: 'gs://sciencebeam-v2-models/wapiti/reference-segmenter/models/2026-07-01_wapiti_grobid_090_scielo_preprints_ore/reference-segmenter' + engine: 'wapiti' profiles: biorxiv_elife: @@ -188,6 +196,11 @@ profiles: processors: fulltext: noise_filter_enabled: false + grobid_custom_hybrid: + sequence_models: grobid_custom_hybrid + processors: + fulltext: + noise_filter_enabled: false profile_aliases: grobid_crf: grobid_crf_0_9_0 diff --git a/tests/config/config_test.py b/tests/config/config_test.py index 22faa2d8..3ebc1bf3 100644 --- a/tests/config/config_test.py +++ b/tests/config/config_test.py @@ -5,7 +5,11 @@ import pytest import yaml -from sciencebeam_parser.config.config import AppConfig, _deep_merge +from sciencebeam_parser.config.config import ( + AppConfig, + _deep_merge, + _resolve_sequence_model_profile +) MINIMAL_PROFILE_CONFIG = { @@ -18,10 +22,15 @@ 'segmentation': {'path': 'path_b/segmentation', 'engine': 'wapiti'}, 'header': {'path': 'path_b/header', 'engine': 'wapiti'}, }, + 'profile_b_extended': { + 'extends': 'profile_b', + 'header': {'path': 'path_b_extended/header', 'engine': 'wapiti'}, + }, }, 'profiles': { 'profile_a': {'sequence_models': 'profile_a'}, 'profile_b': {'sequence_models': 'profile_b'}, + 'profile_b_extended': {'sequence_models': 'profile_b_extended'}, 'profile_with_extra': { 'sequence_models': 'profile_a', 'processors': {'fulltext': {'use_cv_model': True}}, @@ -67,6 +76,59 @@ def test_does_not_mutate_base(self): assert base['a']['b'] == 1 +class TestResolveSequenceModelProfile: + def test_returns_profile_without_extends_unchanged(self): + seq_profiles = { + 'base': {'segmentation': {'path': 'base/segmentation'}}, + } + result = _resolve_sequence_model_profile(seq_profiles, 'base') + assert result == {'segmentation': {'path': 'base/segmentation'}} + + def test_merges_extended_profile(self): + seq_profiles = { + 'base': { + 'segmentation': {'path': 'base/segmentation', 'engine': 'wapiti'}, + 'header': {'path': 'base/header', 'engine': 'wapiti'}, + }, + 'child': { + 'extends': 'base', + 'header': {'path': 'child/header'}, + }, + } + result = _resolve_sequence_model_profile(seq_profiles, 'child') + assert result['segmentation'] == {'path': 'base/segmentation', 'engine': 'wapiti'} + assert result['header'] == {'path': 'child/header', 'engine': 'wapiti'} + assert 'extends' not in result + + def test_supports_chained_extends(self): + seq_profiles = { + 'grandparent': {'segmentation': {'path': 'gp/segmentation'}}, + 'parent': {'extends': 'grandparent', 'header': {'path': 'p/header'}}, + 'child': {'extends': 'parent', 'table': {'path': 'c/table'}}, + } + result = _resolve_sequence_model_profile(seq_profiles, 'child') + assert result['segmentation'] == {'path': 'gp/segmentation'} + assert result['header'] == {'path': 'p/header'} + assert result['table'] == {'path': 'c/table'} + + def test_raises_on_unknown_profile(self): + with pytest.raises(ValueError, match='Unknown sequence_model_profile'): + _resolve_sequence_model_profile({}, 'missing') + + def test_raises_on_unknown_extends_target(self): + seq_profiles = {'child': {'extends': 'missing'}} + with pytest.raises(ValueError, match='Unknown sequence_model_profile'): + _resolve_sequence_model_profile(seq_profiles, 'child') + + def test_raises_on_circular_extends(self): + seq_profiles = { + 'a': {'extends': 'b'}, + 'b': {'extends': 'a'}, + } + with pytest.raises(ValueError, match='Circular extends'): + _resolve_sequence_model_profile(seq_profiles, 'a') + + class TestAppConfigResolveProfile: def _make_config(self, extra: Optional[dict] = None) -> AppConfig: props = dict(MINIMAL_PROFILE_CONFIG) @@ -80,6 +142,11 @@ def test_applies_sequence_model_profile(self): assert config['models']['segmentation']['engine'] == 'wapiti' assert config['models']['header']['path'] == 'path_b/header' + def test_applies_extended_sequence_model_profile(self): + config = self._make_config().resolve_profile('profile_b_extended') + assert config['models']['segmentation']['path'] == 'path_b/segmentation' + assert config['models']['header']['path'] == 'path_b_extended/header' + def test_inherits_base_model_keys_not_in_profile(self): config = self._make_config().resolve_profile('profile_a') assert config['models']['segmentation']['path'] == 'path_a/segmentation'