From 1745091c822089eebbd0a0f613eae3893846bb8a Mon Sep 17 00:00:00 2001 From: Kaushik Kumar Date: Mon, 7 Sep 2026 19:08:49 +0530 Subject: [PATCH 1/4] Add model prediction and rule integration Signed-off-by: Kaushik Kumar --- CHANGELOG.rst | 1 + README.rst | 14 + docs/source/index.rst | 1 + docs/source/model_rules.rst | 23 ++ setup.cfg | 1 + src/scancode_required_phrases/inference.py | 46 +++- src/scancode_required_phrases/model_rules.py | 220 ++++++++++++++++ tests/test_inference.py | 29 +- tests/test_model_rules.py | 264 +++++++++++++++++++ 9 files changed, 585 insertions(+), 14 deletions(-) create mode 100644 docs/source/model_rules.rst create mode 100644 src/scancode_required_phrases/model_rules.py create mode 100644 tests/test_model_rules.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 2f482ee..b8c0418 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -10,3 +10,4 @@ v0.0.0 - Add required phrase dataset extraction. - Add composite rule required phrase updates. - Add required phrase model training and ONNX export. +- Add model prediction and rule integration. diff --git a/README.rst b/README.rst index a4c7050..e014a36 100644 --- a/README.rst +++ b/README.rst @@ -66,6 +66,20 @@ changing a ScanCode rule or file: Predictions require human review before they are added to license rules. +Add predicted phrases to rules +============================== + +Review predictions before modifying rules. Then run the integration command on +a final model directory or Hugging Face repository: + +.. code-block:: console + + add-model-required-phrases --model model-output/final-model --dry-run --verbose + +The command validates each candidate with ScanCode's required-phrase helpers and +writes each changed rule once. Rebuild the ScanCode license index after applying +changes without ``--dry-run``. + Development =========== diff --git a/docs/source/index.rst b/docs/source/index.rst index 779611d..208dc8c 100644 --- a/docs/source/index.rst +++ b/docs/source/index.rst @@ -11,6 +11,7 @@ license rules. dataset composite_rules training + model_rules contribute/contrib_doc Indices and tables diff --git a/docs/source/model_rules.rst b/docs/source/model_rules.rst new file mode 100644 index 0000000..31f98e1 --- /dev/null +++ b/docs/source/model_rules.rst @@ -0,0 +1,23 @@ +Add model-predicted required phrases +==================================== + +Use the command after reviewing predictions from a validated final model: + +.. code-block:: console + + add-model-required-phrases \ + --model model-output/final-model \ + --dry-run \ + --verbose + +``--model`` accepts a local final-model directory or a Hugging Face repository. +The model must pass the hardened publication checks before inference starts. + +The command skips rules that cannot receive generated required phrases and +rules that already contain required-phrase markers. Each prediction must pass +ScanCode's candidate and locatability checks. Accepted phrases are applied in +memory and each changed rule is written once. + +Use ``--license-expression`` to process one expression and ``--limit`` for a +small review run. Remove ``--dry-run`` only after reviewing the predictions. +Rebuild the ScanCode license index after writing rules. diff --git a/setup.cfg b/setup.cfg index 14d02c4..7aec05c 100644 --- a/setup.cfg +++ b/setup.cfg @@ -55,6 +55,7 @@ where = src [options.entry_points] console_scripts = add-composite-required-phrases = scancode_required_phrases.composite_rules:add_composite_required_phrases + add-model-required-phrases = scancode_required_phrases.model_rules:add_model_required_phrases build-required-phrases-dataset = scancode_required_phrases.dataset:main export-required-phrase-model = scancode_required_phrases.export:main train-required-phrase-model = scancode_required_phrases.training:main diff --git a/src/scancode_required_phrases/inference.py b/src/scancode_required_phrases/inference.py index 6d758fd..b5413fc 100644 --- a/src/scancode_required_phrases/inference.py +++ b/src/scancode_required_phrases/inference.py @@ -41,6 +41,41 @@ def words_from_text(text): return required_phrase_splitter(unicodedata.normalize("NFKC", text)) +def _word_counts(word_ids): + counts = {} + for word_id in word_ids: + if word_id is not None: + counts[word_id] = counts.get(word_id, 0) + 1 + return counts + + +def encode_words(tokenizer, words, max_length): + """Encode the longest complete-word prefix and report truncation.""" + call = dict( + is_split_into_words=True, + add_special_tokens=True, + return_tensors="pt", + ) + full = tokenizer(words, truncation=False, **call) + encoding = tokenizer(words, truncation=True, max_length=max_length, **call) + + full_counts = _word_counts(full.word_ids()) + retained_counts = _word_counts(encoding.word_ids()) + covered_words = max(retained_counts, default=-1) + 1 + complete_words = covered_words + + if covered_words and retained_counts[covered_words - 1] != full_counts[covered_words - 1]: + complete_words -= 1 + encoding = tokenizer(words[:complete_words], truncation=False, **call) + + if not complete_words: + raise ValueError("Tokenizer retained no complete words") + if encoding["input_ids"].shape[1] > max_length: + raise ValueError("Complete-word encoding exceeds the model maximum length") + + return encoding, complete_words < len(words) + + def span_confidence(crf, word_emissions, tags, mask, free, span): """Return the CRF probability mass agreeing with one decoded span.""" start, end = span @@ -82,12 +117,10 @@ def predict(self, text): if not words: return PredictionResult(words=(), phrases=(), truncated=False) - encoding = self.tokenizer( - words, - is_split_into_words=True, - truncation=True, + encoding, truncated = encode_words( + tokenizer=self.tokenizer, + words=words, max_length=self.max_length, - return_tensors="pt", ) positions = first_subword_positions(encoding.word_ids()) if not positions: @@ -110,11 +143,8 @@ def predict(self, text): free = self.model.crf(word_emissions, tags, mask=mask, reduction="none") labels = [ID2LABEL[int(label)] for label in decoded] - truncated = len(labels) < len(words) predictions = [] for start, end in extract_spans(labels): - if truncated and end == len(labels) - 1: - continue predictions.append( PhrasePrediction( text=" ".join(words[start : end + 1]), diff --git a/src/scancode_required_phrases/model_rules.py b/src/scancode_required_phrases/model_rules.py new file mode 100644 index 0000000..796b25d --- /dev/null +++ b/src/scancode_required_phrases/model_rules.py @@ -0,0 +1,220 @@ +# Copyright (c) nexB Inc. and others. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Add model-predicted required phrases to ScanCode license rules.""" + +import os +from pathlib import Path + +import click + +from licensedcode.models import rules_data_dir +from licensedcode.required_phrases import add_required_phrase_to_rule +from licensedcode.required_phrases import find_phrase_spans_in_text +from licensedcode.required_phrases import get_base_rules_by_expression +from licensedcode.required_phrases import RequiredPhraseRuleCandidate +from licensedcode.tokenize import get_existing_required_phrase_spans + +from scancode_required_phrases.inference import RequiredPhrasePredictor + + +MIN_TOKENS = 2 +MIN_SINGLE_TOKEN_LEN = 5 +MAX_RULE_TEXT = 4000 + + +def load_predictor(model, hf_token=None): + """Load a predictor from a local directory or Hugging Face repository.""" + model_dir = Path(model) + if not model_dir.is_dir(): + from huggingface_hub import snapshot_download + + model_dir = Path(snapshot_download(repo_id=model, token=hf_token)) + return RequiredPhrasePredictor.from_model_dir(model_dir) + + +def is_updatable(rule): + """Return True if a rule can receive predicted required phrases.""" + if rule.is_from_license: + return False + if len(rule.text) > MAX_RULE_TEXT: + return False + if not rule.is_approx_matchable: + return False + if rule.skip_for_required_phrase_generation: + return False + return not get_existing_required_phrase_spans(rule.text) + + +def select_rules(license_expression=None): + """Return eligible rules grouped by license expression.""" + try: + rules_by_expression = get_base_rules_by_expression(license_expression) + except KeyError: + raise click.ClickException( + f"No rules for license expression: {license_expression}" + ) from None + + selected = {} + for expression, rules in rules_by_expression.items(): + updatable = [rule for rule in rules if is_updatable(rule)] + if updatable: + selected[expression] = updatable + return selected + + +def new_counts(): + return dict( + rules=0, + truncated=0, + rejected=0, + not_found=0, + injected=0, + skipped=0, + written=0, + ) + + +def add_predicted_phrases(rule, phrases, counts, dry_run=False, verbose=False): + """Validate and add predicted phrases, writing the rule at most once.""" + candidates = [] + for phrase in phrases: + candidate = RequiredPhraseRuleCandidate.create(rule.license_expression, phrase) + if not candidate.is_good(rule, MIN_TOKENS, MIN_SINGLE_TOKEN_LEN): + counts["rejected"] += 1 + continue + if not find_phrase_spans_in_text(rule.text, phrase): + counts["not_found"] += 1 + continue + candidates.append(phrase) + + if not candidates: + return False + + original_text = rule.text + original_source = rule.source + source = f"{original_source} ml_model" if original_source else "ml_model" + + for phrase in candidates: + updated = add_required_phrase_to_rule( + rule=rule, + required_phrase=phrase, + source=source, + debug=verbose, + dry_run=True, + ) + if updated: + counts["injected"] += 1 + else: + counts["skipped"] += 1 + + if rule.text == original_text: + return False + if not dry_run: + rule.dump(rules_data_dir) + return True + + +def update_rules_from_predictions( + selected, + predictor, + dry_run=False, + limit=0, + verbose=False, +): + """Predict and add phrases to selected rules and return run counts.""" + counts = new_counts() + total = sum(len(rules) for rules in selected.values()) + click.echo(f"Predicting required phrases for {total} rules") + + for expression, rules in selected.items(): + if verbose: + click.echo(f"{expression}: {len(rules)} rules") + + for rule in rules: + if limit and counts["rules"] >= limit: + click.echo(f"Stopping at {limit} rules") + return counts + + counts["rules"] += 1 + result = predictor.predict(rule.text) + if result.truncated: + counts["truncated"] += 1 + phrases = [prediction.text for prediction in result.phrases] + if not phrases: + continue + + if verbose: + click.echo(f" {rule.identifier}: {phrases}") + if add_predicted_phrases( + rule=rule, + phrases=phrases, + counts=counts, + dry_run=dry_run, + verbose=verbose, + ): + counts["written"] += 1 + + return counts + + +@click.command(name="add-model-required-phrases") +@click.option( + "--model", + required=True, + help="Final model directory or Hugging Face repository.", +) +@click.option( + "--license-expression", + help="Only update rules for this license expression.", +) +@click.option( + "--dry-run", + is_flag=True, + help="Predict and validate phrases without saving rules.", +) +@click.option( + "--limit", + default=0, + type=click.IntRange(min=0), + help="Stop after this many rules; zero processes all rules.", +) +@click.option( + "-v", + "--verbose", + is_flag=True, + help="Print predictions for each rule.", +) +@click.help_option("-h", "--help") +def add_model_required_phrases(model, license_expression, dry_run, limit, verbose): + """Add model-predicted required phrases to license rules.""" + selected = select_rules(license_expression=license_expression) + if not selected: + click.echo("No eligible rules found") + return + + predictor = load_predictor(model, hf_token=os.environ.get("HF_TOKEN")) + counts = update_rules_from_predictions( + selected=selected, + predictor=predictor, + dry_run=dry_run, + limit=limit, + verbose=verbose, + ) + + click.echo(f"\nrules processed : {counts['rules']}") + click.echo(f" truncated : {counts['truncated']}") + click.echo(f"phrases injected : {counts['injected']}") + click.echo(f" rejected : {counts['rejected']}") + click.echo(f" not found : {counts['not_found']}") + click.echo(f" nothing to add : {counts['skipped']}") + click.echo(f"rules written : {counts['written']}") + + if dry_run: + click.echo("Dry run: no rules were saved") + elif counts["written"]: + click.echo("Run scancode-reindex-licenses to use the new required phrases") + + +if __name__ == "__main__": + add_model_required_phrases() diff --git a/tests/test_inference.py b/tests/test_inference.py index 30cb1ed..c88e574 100644 --- a/tests/test_inference.py +++ b/tests/test_inference.py @@ -13,6 +13,7 @@ pytest.importorskip("transformers") from scancode_required_phrases import inference +from scancode_required_phrases.inference import encode_words from scancode_required_phrases.inference import RequiredPhrasePredictor from scancode_required_phrases.inference import words_from_text from scancode_required_phrases.model import ConstrainedCRF @@ -35,9 +36,17 @@ def word_ids(self): class FakeTokenizer: - def __call__(self, words, max_length=512, **kwargs): - word_ids = [None] + list(range(len(words))) + [None] - return FakeEncoding(word_ids[:max_length]) + def __init__(self, subwords=None): + self.subwords = subwords or {} + + def __call__(self, words, truncation, max_length=None, **kwargs): + word_ids = [None] + for index, word in enumerate(words): + word_ids.extend([index] * self.subwords.get(word, 1)) + word_ids.append(None) + if truncation and len(word_ids) > max_length: + word_ids = word_ids[: max_length - 1] + [None] + return FakeEncoding(word_ids) class StubTagger(torch.nn.Module): @@ -85,6 +94,14 @@ def test_loads_predictor_from_validated_model(tmp_path, monkeypatch): assert predictor.max_length == 384 +def test_encode_words_removes_a_partially_truncated_word(): + tokenizer = FakeTokenizer({"many": 3}) + encoding, truncated = encode_words(tokenizer, ["one", "many", "three"], 4) + + assert encoding.word_ids() == [None, 0, None] + assert truncated + + def test_predicts_bioes_phrase_without_mutation(): text = "Granted under the MIT License to everyone" predictor = RequiredPhrasePredictor( @@ -115,9 +132,9 @@ def test_predicts_single_word_phrase(): assert [phrase.text for phrase in result.phrases] == ["MIT"] -def test_drops_phrase_cut_by_truncation(): +def test_keeps_valid_phrase_at_truncation_boundary(): predictor = RequiredPhrasePredictor( - model=StubTagger({2: "B-REQ", 3: "I-REQ"}), + model=StubTagger({2: "S-REQ"}), tokenizer=FakeTokenizer(), max_length=5, ) @@ -125,7 +142,7 @@ def test_drops_phrase_cut_by_truncation(): result = predictor.predict("one two three four five six") assert result.truncated - assert result.phrases == () + assert [phrase.text for phrase in result.phrases] == ["three"] def test_empty_text_does_not_run_model(): diff --git a/tests/test_model_rules.py b/tests/test_model_rules.py new file mode 100644 index 0000000..0000f8c --- /dev/null +++ b/tests/test_model_rules.py @@ -0,0 +1,264 @@ +# Copyright (c) nexB Inc. and others. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +import sys +from types import SimpleNamespace + +import click +from click.testing import CliRunner +import pytest + +from licensedcode.models import Rule + +from scancode_required_phrases import model_rules +from scancode_required_phrases.inference import PhrasePrediction +from scancode_required_phrases.inference import PredictionResult +from scancode_required_phrases.model_rules import add_model_required_phrases +from scancode_required_phrases.model_rules import add_predicted_phrases +from scancode_required_phrases.model_rules import is_updatable +from scancode_required_phrases.model_rules import new_counts +from scancode_required_phrases.model_rules import select_rules +from scancode_required_phrases.model_rules import update_rules_from_predictions + + +class FakeRule: + def __init__( + self, + text="some license text here", + is_from_license=False, + is_approx_matchable=True, + skip=False, + ): + self.text = text + self.is_from_license = is_from_license + self.is_approx_matchable = is_approx_matchable + self.skip_for_required_phrase_generation = skip + + +class FakePredictor: + def __init__(self, phrases, truncated=False): + self.phrases = phrases + self.truncated = truncated + + def predict(self, text): + predictions = tuple( + PhrasePrediction( + text=phrase, + start_word=0, + end_word=0, + confidence=1.0, + ) + for phrase in self.phrases + ) + return PredictionResult( + words=tuple(text.split()), + phrases=predictions, + truncated=self.truncated, + ) + + +def make_rule(text, source=None): + rule = Rule( + license_expression="mit", + identifier="mit_test.RULE", + text=text, + source=source, + is_license_reference=True, + relevance=100, + ) + return rule + + +TEXT = "Permission is granted under the MIT License to do things with this" + + +@pytest.mark.parametrize( + "rule", + [ + FakeRule(is_from_license=True), + FakeRule(text="x" * 4001), + FakeRule(is_approx_matchable=False), + FakeRule(skip=True), + FakeRule(text="under the {{mit license}} terms"), + ], +) +def test_is_updatable_excludes_ineligible_rules(rule): + assert not is_updatable(rule) + + +def test_is_updatable_accepts_a_plain_rule(): + assert is_updatable(FakeRule()) + + +def test_select_rules_filters_and_groups_rules(monkeypatch): + monkeypatch.setattr( + model_rules, + "get_base_rules_by_expression", + lambda expression: { + "mit": [FakeRule(), FakeRule(is_from_license=True)], + "bsd-new": [FakeRule(skip=True)], + }, + ) + + selected = select_rules() + + assert list(selected) == ["mit"] + assert len(selected["mit"]) == 1 + + +def test_select_rules_reports_an_unknown_expression(monkeypatch): + def get_rules(expression): + raise KeyError(expression) + + monkeypatch.setattr(model_rules, "get_base_rules_by_expression", get_rules) + with pytest.raises(click.ClickException, match="No rules"): + select_rules("unknown") + + +def test_add_predicted_phrases_marks_all_candidates_and_preserves_source(): + rule = make_rule(TEXT, source="mit_1.RULE") + counts = new_counts() + + updated = add_predicted_phrases( + rule=rule, + phrases=["MIT License", "do things"], + counts=counts, + dry_run=True, + ) + + assert updated + assert counts["injected"] == 2 + assert "{{MIT License}}" in rule.text + assert "{{do things}}" in rule.text + assert rule.source == "mit_1.RULE ml_model" + + +def test_add_predicted_phrases_rejects_an_unsuitable_candidate(): + rule = make_rule(TEXT) + counts = new_counts() + + assert not add_predicted_phrases(rule, ["is"], counts, dry_run=True) + assert counts["rejected"] == 1 + assert "{{" not in rule.text + + +def test_add_predicted_phrases_counts_a_candidate_not_in_the_rule(): + rule = make_rule(TEXT) + counts = new_counts() + + assert not add_predicted_phrases( + rule, + ["Apache License"], + counts, + dry_run=True, + ) + assert counts["not_found"] == 1 + + +def test_add_predicted_phrases_writes_once(tmp_path, monkeypatch): + rule = make_rule(TEXT) + original_dump = Rule.dump + writes = [] + + def dump(rule, rules_data_dir): + writes.append(rule.identifier) + original_dump(rule, rules_data_dir) + + monkeypatch.setattr(Rule, "dump", dump) + monkeypatch.setattr(model_rules, "rules_data_dir", str(tmp_path)) + + assert add_predicted_phrases(rule, ["MIT License", "do things"], new_counts()) + saved = Rule.from_file(str(tmp_path / rule.identifier)) + assert writes == [rule.identifier] + assert "{{MIT License}}" in saved.text + assert "{{do things}}" in saved.text + + +def test_update_rules_from_predictions_processes_selected_rules(): + rule = make_rule(TEXT) + + counts = update_rules_from_predictions( + selected={"mit": [rule]}, + predictor=FakePredictor(["MIT License"]), + dry_run=True, + ) + + assert counts["rules"] == 1 + assert counts["injected"] == 1 + assert counts["written"] == 1 + + +def test_update_rules_from_predictions_counts_truncation_and_honors_limit(): + rules = [make_rule(TEXT) for _ in range(3)] + + counts = update_rules_from_predictions( + selected={"mit": rules}, + predictor=FakePredictor([], truncated=True), + dry_run=True, + limit=2, + ) + + assert counts["rules"] == 2 + assert counts["truncated"] == 2 + + +def test_command_does_not_load_a_model_without_eligible_rules(monkeypatch): + monkeypatch.setattr(model_rules, "select_rules", lambda **kwargs: {}) + + def fail(*args, **kwargs): + raise AssertionError("model should not load") + + monkeypatch.setattr(model_rules, "load_predictor", fail) + result = CliRunner().invoke(add_model_required_phrases, ["--model", "unused"]) + + assert result.exit_code == 0 + assert "No eligible rules found" in result.output + + +def test_command_wires_selection_prediction_and_dry_run(monkeypatch, tmp_path): + selected = {"mit": [object()]} + predictor = object() + calls = [] + monkeypatch.setattr(model_rules, "select_rules", lambda **kwargs: selected) + monkeypatch.setattr(model_rules, "load_predictor", lambda *args, **kwargs: predictor) + + def update(**kwargs): + calls.append(kwargs) + return new_counts() + + monkeypatch.setattr(model_rules, "update_rules_from_predictions", update) + result = CliRunner().invoke( + add_model_required_phrases, + ["--model", str(tmp_path), "--license-expression", "mit", "--dry-run"], + ) + + assert result.exit_code == 0 + assert calls == [ + { + "selected": selected, + "predictor": predictor, + "dry_run": True, + "limit": 0, + "verbose": False, + } + ] + + +def test_load_predictor_downloads_a_hugging_face_snapshot(tmp_path, monkeypatch): + model_dir = tmp_path / "model" + model_dir.mkdir() + downloads = [] + predictor = object() + hub = SimpleNamespace( + snapshot_download=lambda repo_id, token: downloads.append((repo_id, token)) + or str(model_dir) + ) + monkeypatch.setitem(sys.modules, "huggingface_hub", hub) + monkeypatch.setattr( + model_rules.RequiredPhrasePredictor, + "from_model_dir", + lambda path: predictor, + ) + + assert model_rules.load_predictor("owner/model", "token") is predictor + assert downloads == [("owner/model", "token")] From 2ca4841dfafd324f54db2a022983f6c3c99c95bf Mon Sep 17 00:00:00 2001 From: Kaushik Date: Sun, 13 Sep 2026 10:47:57 +0000 Subject: [PATCH 2/4] Harden model prediction and rule updates Signed-off-by: Kaushik --- .github/workflows/tests.yml | 60 +++++ README.rst | 16 +- azure-pipelines.yml | 3 +- docs/source/model_rules.rst | 12 +- setup.cfg | 8 + src/scancode_required_phrases/inference.py | 48 +--- src/scancode_required_phrases/model_rules.py | 212 ++++++++++----- src/scancode_required_phrases/training.py | 60 +++-- tests/test_inference.py | 17 +- tests/test_model_rules.py | 258 ++++++++++++++----- tests/test_training.py | 13 +- 11 files changed, 495 insertions(+), 212 deletions(-) create mode 100644 .github/workflows/tests.yml diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml new file mode 100644 index 0000000..4aa2469 --- /dev/null +++ b/.github/workflows/tests.yml @@ -0,0 +1,60 @@ +name: Tests + +on: [push, pull_request] + +permissions: {} + +jobs: + base: + runs-on: ubuntu-24.04 + strategy: + matrix: + python-version: ["3.10", "3.13"] + steps: + - name: Checkout code + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd + with: + persist-credentials: false + - name: Set up Python + uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 + with: + python-version: ${{ matrix.python-version }} + - name: Install development dependencies + run: ./configure --dev + - name: Run code checks + run: | + venv/bin/ruff check + make doc8 + - name: Run tests + run: venv/bin/pytest -q + - name: Build distributions + if: matrix.python-version == '3.13' + run: | + venv/bin/python -m build + venv/bin/twine check dist/* + + ml: + runs-on: ubuntu-24.04 + steps: + - name: Checkout code + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd + with: + persist-credentials: false + - name: Set up Python + uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 + with: + python-version: "3.12" + - name: Install ML dependencies + run: | + ./configure --dev + venv/bin/pip install -e ".[training]" + - name: Run ML and integration tests + env: + USE_TF: "0" + run: | + venv/bin/pytest -q \ + tests/test_training.py \ + tests/test_model.py \ + tests/test_export.py \ + tests/test_inference.py \ + tests/test_model_rules.py diff --git a/README.rst b/README.rst index e014a36..49e5eab 100644 --- a/README.rst +++ b/README.rst @@ -54,8 +54,12 @@ reload and validation succeed. See ``docs/source/training.rst`` for details. Run read-only prediction ======================== -Load a validated final model and return candidate required phrases without -changing a ScanCode rule or file: +Install the inference dependencies, then load a validated final model and +return candidate required phrases without changing a ScanCode rule or file: + +.. code-block:: console + + python -m pip install ".[inference]" .. code-block:: python @@ -76,9 +80,11 @@ a final model directory or Hugging Face repository: add-model-required-phrases --model model-output/final-model --dry-run --verbose -The command validates each candidate with ScanCode's required-phrase helpers and -writes each changed rule once. Rebuild the ScanCode license index after applying -changes without ``--dry-run``. +For a Hugging Face repository, also provide its full commit hash with +``--model-revision``. The command rejects phrase text found more than once in a +rule because ScanCode's mutation helper would mark every occurrence. It validates +the complete rule update and writes each changed rule once. Rebuild the ScanCode +license index after applying changes without ``--dry-run``. Development =========== diff --git a/azure-pipelines.yml b/azure-pipelines.yml index bfea243..3be6213 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -28,7 +28,8 @@ jobs: tests/test_training.py \ tests/test_model.py \ tests/test_export.py \ - tests/test_inference.py + tests/test_inference.py \ + tests/test_model_rules.py displayName: Run training unit tests - template: etc/ci/azure-posix.yml diff --git a/docs/source/model_rules.rst b/docs/source/model_rules.rst index 31f98e1..5ee8e18 100644 --- a/docs/source/model_rules.rst +++ b/docs/source/model_rules.rst @@ -10,13 +10,15 @@ Use the command after reviewing predictions from a validated final model: --dry-run \ --verbose -``--model`` accepts a local final-model directory or a Hugging Face repository. -The model must pass the hardened publication checks before inference starts. +Install the ``inference`` extra before using this command. ``--model`` accepts +a local final-model directory or a Hugging Face repository. Remote models also +require their full commit hash through ``--model-revision``. The model must pass +the publication checks before inference starts. The command skips rules that cannot receive generated required phrases and -rules that already contain required-phrase markers. Each prediction must pass -ScanCode's candidate and locatability checks. Accepted phrases are applied in -memory and each changed rule is written once. +rules that already contain required-phrase markers. It rejects phrase text +found more than once because ScanCode would mark every occurrence. The complete +rule update is checked in memory and each changed rule is written once. Use ``--license-expression`` to process one expression and ``--limit`` for a small review run. Remove ``--dry-run`` only after reviewing the predictions. diff --git a/setup.cfg b/setup.cfg index 7aec05c..07f289d 100644 --- a/setup.cfg +++ b/setup.cfg @@ -62,6 +62,13 @@ console_scripts = [options.extras_require] +inference = + huggingface-hub == 0.36.2 + pytorch-crf == 0.7.2 + safetensors >= 0.4 + sentencepiece >= 0.2 + torch >= 2.0 + transformers == 4.57.3 training = accelerate >= 0.33 huggingface-hub == 0.36.2 @@ -78,6 +85,7 @@ onnx = onnx >= 1.16 onnxruntime >= 1.18 dev = + build pytest >= 7.0.1 pytest-xdist >= 2 aboutcode-toolkit >= 7.0.2 diff --git a/src/scancode_required_phrases/inference.py b/src/scancode_required_phrases/inference.py index b5413fc..c55eec8 100644 --- a/src/scancode_required_phrases/inference.py +++ b/src/scancode_required_phrases/inference.py @@ -10,6 +10,7 @@ from licensedcode.tokenize import required_phrase_splitter +from scancode_required_phrases.training import encode_complete_words from scancode_required_phrases.training import extract_spans from scancode_required_phrases.training import first_subword_positions from scancode_required_phrases.training import ID2LABEL @@ -41,41 +42,6 @@ def words_from_text(text): return required_phrase_splitter(unicodedata.normalize("NFKC", text)) -def _word_counts(word_ids): - counts = {} - for word_id in word_ids: - if word_id is not None: - counts[word_id] = counts.get(word_id, 0) + 1 - return counts - - -def encode_words(tokenizer, words, max_length): - """Encode the longest complete-word prefix and report truncation.""" - call = dict( - is_split_into_words=True, - add_special_tokens=True, - return_tensors="pt", - ) - full = tokenizer(words, truncation=False, **call) - encoding = tokenizer(words, truncation=True, max_length=max_length, **call) - - full_counts = _word_counts(full.word_ids()) - retained_counts = _word_counts(encoding.word_ids()) - covered_words = max(retained_counts, default=-1) + 1 - complete_words = covered_words - - if covered_words and retained_counts[covered_words - 1] != full_counts[covered_words - 1]: - complete_words -= 1 - encoding = tokenizer(words[:complete_words], truncation=False, **call) - - if not complete_words: - raise ValueError("Tokenizer retained no complete words") - if encoding["input_ids"].shape[1] > max_length: - raise ValueError("Complete-word encoding exceeds the model maximum length") - - return encoding, complete_words < len(words) - - def span_confidence(crf, word_emissions, tags, mask, free, span): """Return the CRF probability mass agreeing with one decoded span.""" start, end = span @@ -117,9 +83,9 @@ def predict(self, text): if not words: return PredictionResult(words=(), phrases=(), truncated=False) - encoding, truncated = encode_words( + encoding, truncated = encode_complete_words( + tokens=words, tokenizer=self.tokenizer, - words=words, max_length=self.max_length, ) positions = first_subword_positions(encoding.word_ids()) @@ -127,8 +93,12 @@ def predict(self, text): return PredictionResult(words=tuple(words), phrases=(), truncated=False) device = next(self.model.parameters()).device - input_ids = encoding["input_ids"].to(device) - attention_mask = encoding["attention_mask"].to(device) + input_ids = torch.tensor([encoding["input_ids"]], dtype=torch.long, device=device) + attention_mask = torch.tensor( + [encoding["attention_mask"]], + dtype=torch.long, + device=device, + ) with torch.inference_mode(): emissions = self.model.emissions(input_ids, attention_mask) diff --git a/src/scancode_required_phrases/model_rules.py b/src/scancode_required_phrases/model_rules.py index 796b25d..c2c7a98 100644 --- a/src/scancode_required_phrases/model_rules.py +++ b/src/scancode_required_phrases/model_rules.py @@ -3,64 +3,121 @@ """Add model-predicted required phrases to ScanCode license rules.""" +from copy import copy +import json import os from pathlib import Path +from pathlib import PurePosixPath +import shutil +import tempfile import click from licensedcode.models import rules_data_dir from licensedcode.required_phrases import add_required_phrase_to_rule from licensedcode.required_phrases import find_phrase_spans_in_text -from licensedcode.required_phrases import get_base_rules_by_expression +from licensedcode.required_phrases import get_updatable_rules_by_expression from licensedcode.required_phrases import RequiredPhraseRuleCandidate from licensedcode.tokenize import get_existing_required_phrase_spans from scancode_required_phrases.inference import RequiredPhrasePredictor +from scancode_required_phrases.training import IMMUTABLE_REVISION MIN_TOKENS = 2 MIN_SINGLE_TOKEN_LEN = 5 -MAX_RULE_TEXT = 4000 -def load_predictor(model, hf_token=None): - """Load a predictor from a local directory or Hugging Face repository.""" +def _artifact_names(success_marker): + """Return safe artifact paths listed in a model success marker.""" + try: + files = success_marker["files"] + except (KeyError, TypeError) as error: + raise ValueError("Remote model has no valid file inventory") from error + if type(files) is not dict or not files: + raise ValueError("Remote model has no valid file inventory") + + names = ["SUCCESS.json", *files] + for name in names: + if type(name) is not str or not name: + raise ValueError(f"Remote model contains an unsafe artifact path: {name!r}") + path = PurePosixPath(name) + if path.is_absolute() or ".." in path.parts: + raise ValueError(f"Remote model contains an unsafe artifact path: {name!r}") + return names + + +def _load_remote_predictor(repository, revision, hf_token): + """Download only declared model artifacts and load them locally.""" + from huggingface_hub import hf_hub_download + from huggingface_hub import snapshot_download + + marker_path = hf_hub_download( + repo_id=repository, + filename="SUCCESS.json", + revision=revision, + token=hf_token, + ) + try: + marker = json.loads(Path(marker_path).read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError) as error: + raise ValueError("Remote model has no valid success marker") from error + + artifact_names = _artifact_names(marker) + snapshot = Path( + snapshot_download( + repo_id=repository, + revision=revision, + token=hf_token, + allow_patterns=artifact_names, + ) + ) + with tempfile.TemporaryDirectory() as temporary_directory: + model_dir = Path(temporary_directory) + for name in artifact_names: + source = snapshot / name + if not source.is_file(): + raise ValueError(f"Remote model is missing artifact: {name}") + target = model_dir / name + target.parent.mkdir(parents=True, exist_ok=True) + try: + target.hardlink_to(source) + except OSError: + shutil.copy2(source, target) + return RequiredPhrasePredictor.from_model_dir(model_dir) + + +def load_predictor(model, hf_token=None, revision=None): + """Load a predictor from a local directory or pinned Hugging Face revision.""" model_dir = Path(model) - if not model_dir.is_dir(): - from huggingface_hub import snapshot_download + if model_dir.is_dir(): + if revision: + raise ValueError("A model revision cannot be used with a local model directory") + return RequiredPhrasePredictor.from_model_dir(model_dir) - model_dir = Path(snapshot_download(repo_id=model, token=hf_token)) - return RequiredPhrasePredictor.from_model_dir(model_dir) - - -def is_updatable(rule): - """Return True if a rule can receive predicted required phrases.""" - if rule.is_from_license: - return False - if len(rule.text) > MAX_RULE_TEXT: - return False - if not rule.is_approx_matchable: - return False - if rule.skip_for_required_phrase_generation: - return False - return not get_existing_required_phrase_spans(rule.text) + if not revision or not IMMUTABLE_REVISION.fullmatch(revision): + raise ValueError("A 40-character model revision is required for a remote model") + return _load_remote_predictor(model, revision, hf_token) def select_rules(license_expression=None): - """Return eligible rules grouped by license expression.""" + """Return rules eligible for model prediction, grouped by expression.""" try: - rules_by_expression = get_base_rules_by_expression(license_expression) + rules_by_expression = get_updatable_rules_by_expression( + license_expression=license_expression, + simple_expression=False, + ) except KeyError: raise click.ClickException( f"No rules for license expression: {license_expression}" ) from None - selected = {} + selected_rules_by_expression = {} for expression, rules in rules_by_expression.items(): - updatable = [rule for rule in rules if is_updatable(rule)] - if updatable: - selected[expression] = updatable - return selected + selected_rules = [rule for rule in rules if not get_existing_required_phrase_spans(rule.text)] + if selected_rules: + selected_rules_by_expression[expression] = selected_rules + return selected_rules_by_expression def new_counts(): @@ -69,49 +126,64 @@ def new_counts(): truncated=0, rejected=0, not_found=0, + ambiguous=0, + conflicts=0, injected=0, - skipped=0, + changed=0, written=0, ) +def candidate_issue(rule, phrase): + """Return why ``phrase`` cannot be safely added, or None.""" + candidate = RequiredPhraseRuleCandidate.create(rule.license_expression, phrase) + if not candidate.is_good(rule, MIN_TOKENS, MIN_SINGLE_TOKEN_LEN): + return "rejected" + + spans = find_phrase_spans_in_text(rule.text, phrase) + if not spans: + return "not_found" + if len(spans) != 1: + return "ambiguous" + + def add_predicted_phrases(rule, phrases, counts, dry_run=False, verbose=False): - """Validate and add predicted phrases, writing the rule at most once.""" - candidates = [] + """Validate a complete rule update and write the rule at most once.""" + accepted_phrases = [] for phrase in phrases: - candidate = RequiredPhraseRuleCandidate.create(rule.license_expression, phrase) - if not candidate.is_good(rule, MIN_TOKENS, MIN_SINGLE_TOKEN_LEN): - counts["rejected"] += 1 + issue = candidate_issue(rule, phrase) + if issue: + counts[issue] += 1 continue - if not find_phrase_spans_in_text(rule.text, phrase): - counts["not_found"] += 1 - continue - candidates.append(phrase) + accepted_phrases.append(phrase) - if not candidates: + if not accepted_phrases: return False - original_text = rule.text - original_source = rule.source - source = f"{original_source} ml_model" if original_source else "ml_model" - - for phrase in candidates: - updated = add_required_phrase_to_rule( - rule=rule, + accepted_phrases.sort(key=lambda phrase: (-len(phrase), phrase)) + updated_rule = copy(rule) + source = f"{rule.source} ml_model" if rule.source else "ml_model" + for phrase in accepted_phrases: + if not add_required_phrase_to_rule( + rule=updated_rule, required_phrase=phrase, source=source, debug=verbose, dry_run=True, - ) - if updated: - counts["injected"] += 1 - else: - counts["skipped"] += 1 + ): + counts["conflicts"] += 1 + return False - if rule.text == original_text: + if updated_rule.text == rule.text: return False - if not dry_run: - rule.dump(rules_data_dir) + + counts["injected"] += len(accepted_phrases) + if dry_run: + return True + + updated_rule.dump(rules_data_dir) + rule.text = updated_rule.text + rule.source = updated_rule.source return True @@ -153,7 +225,9 @@ def update_rules_from_predictions( dry_run=dry_run, verbose=verbose, ): - counts["written"] += 1 + counts["changed"] += 1 + if not dry_run: + counts["written"] += 1 return counts @@ -164,6 +238,10 @@ def update_rules_from_predictions( required=True, help="Final model directory or Hugging Face repository.", ) +@click.option( + "--model-revision", + help="Full commit hash required for a Hugging Face model.", +) @click.option( "--license-expression", help="Only update rules for this license expression.", @@ -186,14 +264,28 @@ def update_rules_from_predictions( help="Print predictions for each rule.", ) @click.help_option("-h", "--help") -def add_model_required_phrases(model, license_expression, dry_run, limit, verbose): +def add_model_required_phrases( + model, + model_revision, + license_expression, + dry_run, + limit, + verbose, +): """Add model-predicted required phrases to license rules.""" selected = select_rules(license_expression=license_expression) if not selected: click.echo("No eligible rules found") return - predictor = load_predictor(model, hf_token=os.environ.get("HF_TOKEN")) + try: + predictor = load_predictor( + model, + hf_token=os.environ.get("HF_TOKEN"), + revision=model_revision, + ) + except ValueError as error: + raise click.ClickException(str(error)) from error counts = update_rules_from_predictions( selected=selected, predictor=predictor, @@ -204,10 +296,12 @@ def add_model_required_phrases(model, license_expression, dry_run, limit, verbos click.echo(f"\nrules processed : {counts['rules']}") click.echo(f" truncated : {counts['truncated']}") - click.echo(f"phrases injected : {counts['injected']}") + click.echo(f"phrases accepted : {counts['injected']}") click.echo(f" rejected : {counts['rejected']}") click.echo(f" not found : {counts['not_found']}") - click.echo(f" nothing to add : {counts['skipped']}") + click.echo(f" ambiguous : {counts['ambiguous']}") + click.echo(f" conflicts : {counts['conflicts']}") + click.echo(f"rules changed : {counts['changed']}") click.echo(f"rules written : {counts['written']}") if dry_run: diff --git a/src/scancode_required_phrases/training.py b/src/scancode_required_phrases/training.py index b84bb42..4886cdb 100644 --- a/src/scancode_required_phrases/training.py +++ b/src/scancode_required_phrases/training.py @@ -528,16 +528,17 @@ def _coverage_counts(word_ids): return counts -def align_labels(tokens, word_labels, tokenizer, max_length): - """Align unchanged labels to a tokenizer-verified complete word prefix.""" - if type(tokens) is not list or type(word_labels) is not list: - raise TypeError("tokens and word_labels must be lists") - if len(tokens) != len(word_labels) or not tokens: - raise ValueError("tokens and word_labels must have equal non-zero lengths") - if validate_bioes(word_labels): - raise ValueError("word_labels must be a valid BIOES sequence") +def encode_complete_words( + tokens, + tokenizer, + max_length, + required_positions=(), +): + """Encode and validate the longest complete-word prefix of ``tokens``.""" + if type(tokens) is not list or not tokens: + raise ValueError("tokens must be a non-empty list") if getattr(tokenizer, "is_fast", True) is not True: - raise ValueError("Training requires a fast tokenizer with word IDs") + raise ValueError("A fast tokenizer with word IDs is required") call = { "is_split_into_words": True, @@ -562,12 +563,7 @@ def align_labels(tokens, word_labels, tokenizer, max_length): f"full encoding: dataset words have zero subwords at positions {missing}", ) - encoding = tokenizer( - tokens, - truncation=True, - max_length=max_length, - **call, - ) + encoding = tokenizer(tokens, truncation=True, max_length=max_length, **call) word_ids, covered = _validated_word_ids( encoding, len(tokens), "retained encoding", special_ids, vocab_size ) @@ -583,10 +579,8 @@ def align_labels(tokens, word_labels, tokenizer, max_length): ) complete_words = word_id - omitted_positions = list(range(complete_words, len(tokens))) - omitted_required = [ - position for position in omitted_positions if word_labels[position] != "O" - ] + omitted_positions = set(range(complete_words, len(tokens))) + omitted_required = sorted(omitted_positions.intersection(required_positions)) if omitted_required: raise AlignmentError( "omitted-non-o", @@ -622,6 +616,27 @@ def align_labels(tokens, word_labels, tokenizer, max_length): if len(encoding["attention_mask"]) != len(encoding["input_ids"]): raise AlignmentError("shape-mismatch", "tokenizer input and attention lengths differ") + return encoding, bool(omitted_positions) + + +def align_labels(tokens, word_labels, tokenizer, max_length): + """Align unchanged labels to a tokenizer-verified complete word prefix.""" + if type(tokens) is not list or type(word_labels) is not list: + raise TypeError("tokens and word_labels must be lists") + if len(tokens) != len(word_labels) or not tokens: + raise ValueError("tokens and word_labels must have equal non-zero lengths") + if validate_bioes(word_labels): + raise ValueError("word_labels must be a valid BIOES sequence") + + required_positions = [position for position, label in enumerate(word_labels) if label != "O"] + encoding, truncated = encode_complete_words( + tokens, + tokenizer, + max_length, + required_positions=required_positions, + ) + word_ids = encoding.word_ids() + label_ids = [] previous_word = None for word_id in word_ids: @@ -633,7 +648,7 @@ def align_labels(tokens, word_labels, tokenizer, max_length): label_ids.append(IGNORE_INDEX) previous_word = word_id encoding["labels"] = label_ids - return encoding, bool(omitted_positions), False + return encoding, truncated, False def first_subword_positions(word_ids): @@ -1290,7 +1305,10 @@ def _load_local_model(model_dir, offline=True): "loaded", ) tokenizer = AutoTokenizer.from_pretrained( - str(model_dir), use_fast=True, local_files_only=True + str(model_dir), + use_fast=True, + local_files_only=True, + fix_mistral_regex=False, ) if not tokenizer.is_fast: raise ValueError("Final_Model tokenizer is not fast") diff --git a/tests/test_inference.py b/tests/test_inference.py index c88e574..332a1f1 100644 --- a/tests/test_inference.py +++ b/tests/test_inference.py @@ -13,7 +13,6 @@ pytest.importorskip("transformers") from scancode_required_phrases import inference -from scancode_required_phrases.inference import encode_words from scancode_required_phrases.inference import RequiredPhrasePredictor from scancode_required_phrases.inference import words_from_text from scancode_required_phrases.model import ConstrainedCRF @@ -25,8 +24,8 @@ class FakeEncoding(dict): def __init__(self, word_ids): super().__init__( - input_ids=torch.zeros((1, len(word_ids)), dtype=torch.long), - attention_mask=torch.ones((1, len(word_ids)), dtype=torch.long), + input_ids=[0] * len(word_ids), + attention_mask=[1] * len(word_ids), ) self._word_ids = word_ids @@ -36,6 +35,10 @@ def word_ids(self): class FakeTokenizer: + is_fast = True + all_special_ids = [0] + vocab_size = 100 + def __init__(self, subwords=None): self.subwords = subwords or {} @@ -94,14 +97,6 @@ def test_loads_predictor_from_validated_model(tmp_path, monkeypatch): assert predictor.max_length == 384 -def test_encode_words_removes_a_partially_truncated_word(): - tokenizer = FakeTokenizer({"many": 3}) - encoding, truncated = encode_words(tokenizer, ["one", "many", "three"], 4) - - assert encoding.word_ids() == [None, 0, None] - assert truncated - - def test_predicts_bioes_phrase_without_mutation(): text = "Granted under the MIT License to everyone" predictor = RequiredPhrasePredictor( diff --git a/tests/test_model_rules.py b/tests/test_model_rules.py index 0000f8c..395a93b 100644 --- a/tests/test_model_rules.py +++ b/tests/test_model_rules.py @@ -1,6 +1,8 @@ # Copyright (c) nexB Inc. and others. All rights reserved. # SPDX-License-Identifier: Apache-2.0 +import hashlib +import json import sys from types import SimpleNamespace @@ -15,24 +17,14 @@ from scancode_required_phrases.inference import PredictionResult from scancode_required_phrases.model_rules import add_model_required_phrases from scancode_required_phrases.model_rules import add_predicted_phrases -from scancode_required_phrases.model_rules import is_updatable from scancode_required_phrases.model_rules import new_counts from scancode_required_phrases.model_rules import select_rules from scancode_required_phrases.model_rules import update_rules_from_predictions class FakeRule: - def __init__( - self, - text="some license text here", - is_from_license=False, - is_approx_matchable=True, - skip=False, - ): + def __init__(self, text="some license text here"): self.text = text - self.is_from_license = is_from_license - self.is_approx_matchable = is_approx_matchable - self.skip_for_required_phrase_generation = skip class FakePredictor: @@ -72,50 +64,35 @@ def make_rule(text, source=None): TEXT = "Permission is granted under the MIT License to do things with this" -@pytest.mark.parametrize( - "rule", - [ - FakeRule(is_from_license=True), - FakeRule(text="x" * 4001), - FakeRule(is_approx_matchable=False), - FakeRule(skip=True), - FakeRule(text="under the {{mit license}} terms"), - ], -) -def test_is_updatable_excludes_ineligible_rules(rule): - assert not is_updatable(rule) +def test_select_rules_reuses_scancode_selection_and_excludes_marked_rules(monkeypatch): + calls = [] + def get_rules(**kwargs): + calls.append(kwargs) + return { + "mit": [FakeRule(), FakeRule(text="under the {{mit license}} terms")], + "bsd-new": [], + } -def test_is_updatable_accepts_a_plain_rule(): - assert is_updatable(FakeRule()) + monkeypatch.setattr(model_rules, "get_updatable_rules_by_expression", get_rules) + selected = select_rules("mit") -def test_select_rules_filters_and_groups_rules(monkeypatch): - monkeypatch.setattr( - model_rules, - "get_base_rules_by_expression", - lambda expression: { - "mit": [FakeRule(), FakeRule(is_from_license=True)], - "bsd-new": [FakeRule(skip=True)], - }, - ) - - selected = select_rules() - + assert calls == [{"license_expression": "mit", "simple_expression": False}] assert list(selected) == ["mit"] assert len(selected["mit"]) == 1 def test_select_rules_reports_an_unknown_expression(monkeypatch): - def get_rules(expression): - raise KeyError(expression) + def get_rules(**kwargs): + raise KeyError(kwargs["license_expression"]) - monkeypatch.setattr(model_rules, "get_base_rules_by_expression", get_rules) + monkeypatch.setattr(model_rules, "get_updatable_rules_by_expression", get_rules) with pytest.raises(click.ClickException, match="No rules"): select_rules("unknown") -def test_add_predicted_phrases_marks_all_candidates_and_preserves_source(): +def test_add_predicted_phrases_dry_run_does_not_mutate_rule(): rule = make_rule(TEXT, source="mit_1.RULE") counts = new_counts() @@ -128,9 +105,8 @@ def test_add_predicted_phrases_marks_all_candidates_and_preserves_source(): assert updated assert counts["injected"] == 2 - assert "{{MIT License}}" in rule.text - assert "{{do things}}" in rule.text - assert rule.source == "mit_1.RULE ml_model" + assert rule.text == TEXT + assert rule.source == "mit_1.RULE" def test_add_predicted_phrases_rejects_an_unsuitable_candidate(): @@ -155,8 +131,60 @@ def test_add_predicted_phrases_counts_a_candidate_not_in_the_rule(): assert counts["not_found"] == 1 -def test_add_predicted_phrases_writes_once(tmp_path, monkeypatch): +def test_add_predicted_phrases_rejects_ambiguous_occurrences(): + text = "MIT License applies here. MIT License applies there." + rule = make_rule(text) + counts = new_counts() + + assert not add_predicted_phrases(rule, ["MIT License"], counts, dry_run=True) + assert counts["ambiguous"] == 1 + assert rule.text == text + + +def test_add_predicted_phrases_rejects_an_ambiguous_shorter_phrase(): + text = "Redistribution clause and binary Redistribution clause" + rule = make_rule(text) + counts = new_counts() + + assert add_predicted_phrases( + rule, + ["Redistribution clause", "binary Redistribution clause"], + counts, + dry_run=True, + ) + assert counts["ambiguous"] == 1 + assert counts["injected"] == 1 + assert rule.text == text + + +def test_add_predicted_phrases_rolls_back_a_conflicting_update(monkeypatch): rule = make_rule(TEXT) + calls = [] + + def add_phrase(rule, required_phrase, **kwargs): + calls.append(required_phrase) + if len(calls) == 2: + return False + rule.text = f"{{{{{required_phrase}}}}} " + rule.text + return True + + monkeypatch.setattr(model_rules, "add_required_phrase_to_rule", add_phrase) + counts = new_counts() + + assert not add_predicted_phrases( + rule, + ["MIT License", "do things"], + counts, + dry_run=True, + ) + assert calls == ["MIT License", "do things"] + assert counts["conflicts"] == 1 + assert counts["injected"] == 0 + assert rule.text == TEXT + + +def test_add_predicted_phrases_writes_exact_rule_once(tmp_path, monkeypatch): + rule = make_rule(TEXT, source="mit_1.RULE") original_dump = Rule.dump writes = [] @@ -170,8 +198,12 @@ def dump(rule, rules_data_dir): assert add_predicted_phrases(rule, ["MIT License", "do things"], new_counts()) saved = Rule.from_file(str(tmp_path / rule.identifier)) assert writes == [rule.identifier] - assert "{{MIT License}}" in saved.text - assert "{{do things}}" in saved.text + assert saved.text == ( + "Permission is granted under the {{MIT License}} to {{do things}} with this" + ) + assert saved.source == "mit_1.RULE ml_model" + assert rule.text == saved.text + assert rule.source == saved.source def test_update_rules_from_predictions_processes_selected_rules(): @@ -185,7 +217,9 @@ def test_update_rules_from_predictions_processes_selected_rules(): assert counts["rules"] == 1 assert counts["injected"] == 1 - assert counts["written"] == 1 + assert counts["changed"] == 1 + assert counts["written"] == 0 + assert rule.text == TEXT def test_update_rules_from_predictions_counts_truncation_and_honors_limit(): @@ -218,47 +252,131 @@ def fail(*args, **kwargs): def test_command_wires_selection_prediction_and_dry_run(monkeypatch, tmp_path): selected = {"mit": [object()]} predictor = object() - calls = [] - monkeypatch.setattr(model_rules, "select_rules", lambda **kwargs: selected) - monkeypatch.setattr(model_rules, "load_predictor", lambda *args, **kwargs: predictor) + selections = [] + loads = [] + updates = [] + monkeypatch.delenv("HF_TOKEN", raising=False) + + monkeypatch.setattr( + model_rules, + "select_rules", + lambda **kwargs: selections.append(kwargs) or selected, + ) + monkeypatch.setattr( + model_rules, + "load_predictor", + lambda *args, **kwargs: loads.append((args, kwargs)) or predictor, + ) def update(**kwargs): - calls.append(kwargs) + updates.append(kwargs) return new_counts() monkeypatch.setattr(model_rules, "update_rules_from_predictions", update) result = CliRunner().invoke( add_model_required_phrases, - ["--model", str(tmp_path), "--license-expression", "mit", "--dry-run"], + [ + "--model", + str(tmp_path), + "--license-expression", + "mit", + "--dry-run", + "--limit", + "7", + "--verbose", + ], ) assert result.exit_code == 0 - assert calls == [ + assert selections == [{"license_expression": "mit"}] + assert loads == [ + ( + (str(tmp_path),), + {"hf_token": None, "revision": None}, + ) + ] + assert updates == [ { "selected": selected, "predictor": predictor, "dry_run": True, - "limit": 0, - "verbose": False, + "limit": 7, + "verbose": True, } ] + assert "rules written : 0" in result.output + assert "Dry run: no rules were saved" in result.output -def test_load_predictor_downloads_a_hugging_face_snapshot(tmp_path, monkeypatch): - model_dir = tmp_path / "model" - model_dir.mkdir() - downloads = [] - predictor = object() +def test_load_predictor_rejects_a_revision_for_a_local_model(tmp_path): + with pytest.raises(ValueError, match="local model directory"): + model_rules.load_predictor(tmp_path, revision="a" * 40) + + +def test_load_predictor_requires_a_remote_revision(): + with pytest.raises(ValueError, match="40-character"): + model_rules.load_predictor("owner/model") + + +def test_remote_artifact_names_reject_unsafe_paths(): + with pytest.raises(ValueError, match="unsafe artifact path"): + model_rules._artifact_names({"files": {"../model.safetensors": "digest"}}) + + +def test_load_predictor_stages_only_declared_remote_artifacts(tmp_path, monkeypatch): + snapshot = tmp_path / "snapshot" + snapshot.mkdir() + artifact = snapshot / "model.safetensors" + artifact.write_bytes(b"weights") + marker = { + "files": { + "model.safetensors": hashlib.sha256(b"weights").hexdigest(), + } + } + marker_path = snapshot / "SUCCESS.json" + marker_path.write_text(json.dumps(marker), encoding="utf-8") + (snapshot / ".gitattributes").write_text("metadata", encoding="utf-8") + + marker_downloads = [] + snapshot_downloads = [] hub = SimpleNamespace( - snapshot_download=lambda repo_id, token: downloads.append((repo_id, token)) - or str(model_dir) + hf_hub_download=lambda **kwargs: marker_downloads.append(kwargs) or str(marker_path), + snapshot_download=lambda **kwargs: snapshot_downloads.append(kwargs) or str(snapshot), ) monkeypatch.setitem(sys.modules, "huggingface_hub", hub) - monkeypatch.setattr( - model_rules.RequiredPhrasePredictor, - "from_model_dir", - lambda path: predictor, - ) + predictor = object() - assert model_rules.load_predictor("owner/model", "token") is predictor - assert downloads == [("owner/model", "token")] + def load(model_dir): + assert sorted(path.name for path in model_dir.iterdir()) == [ + "SUCCESS.json", + "model.safetensors", + ] + return predictor + + monkeypatch.setattr(model_rules.RequiredPhrasePredictor, "from_model_dir", load) + revision = "a" * 40 + + assert ( + model_rules.load_predictor( + "owner/model", + hf_token="token", + revision=revision, + ) + is predictor + ) + assert marker_downloads == [ + { + "repo_id": "owner/model", + "filename": "SUCCESS.json", + "revision": revision, + "token": "token", + } + ] + assert snapshot_downloads == [ + { + "repo_id": "owner/model", + "revision": revision, + "token": "token", + "allow_patterns": ["SUCCESS.json", "model.safetensors"], + } + ] diff --git a/tests/test_training.py b/tests/test_training.py index 7f7a687..8ded15d 100644 --- a/tests/test_training.py +++ b/tests/test_training.py @@ -936,14 +936,25 @@ class LocalTokenizer: "from_config", lambda config: LocalBackbone(), ) + tokenizer_calls = [] monkeypatch.setattr( transformers.AutoTokenizer, "from_pretrained", - lambda *args, **kwargs: LocalTokenizer(), + lambda *args, **kwargs: tokenizer_calls.append((args, kwargs)) or LocalTokenizer(), ) loaded, tokenizer = training._load_local_model(model_dir) + assert tokenizer_calls == [ + ( + (str(model_dir),), + { + "use_fast": True, + "local_files_only": True, + "fix_mistral_regex": False, + }, + ) + ] assert loaded.weight.item() == 3.0 assert loaded.backbone.position_ids.device.type == "cpu" assert tokenizer.is_fast From 423bc2c0f0099381103d23bfc807de6cc1ad97e3 Mon Sep 17 00:00:00 2001 From: Kaushik Date: Sun, 13 Sep 2026 13:48:09 +0000 Subject: [PATCH 3/4] Harden prediction artifact and rule writes Signed-off-by: Kaushik --- src/scancode_required_phrases/model_rules.py | 49 ++++++++++++++--- tests/test_model_rules.py | 55 +++++++++++++++++++- 2 files changed, 94 insertions(+), 10 deletions(-) diff --git a/src/scancode_required_phrases/model_rules.py b/src/scancode_required_phrases/model_rules.py index c2c7a98..a528b86 100644 --- a/src/scancode_required_phrases/model_rules.py +++ b/src/scancode_required_phrases/model_rules.py @@ -39,10 +39,15 @@ def _artifact_names(success_marker): names = ["SUCCESS.json", *files] for name in names: - if type(name) is not str or not name: + if type(name) is not str or not name or "\\" in name: raise ValueError(f"Remote model contains an unsafe artifact path: {name!r}") path = PurePosixPath(name) - if path.is_absolute() or ".." in path.parts: + if ( + not path.parts + or path.is_absolute() + or ".." in path.parts + or path.as_posix() != name + ): raise ValueError(f"Remote model contains an unsafe artifact path: {name!r}") return names @@ -147,8 +152,8 @@ def candidate_issue(rule, phrase): return "ambiguous" -def add_predicted_phrases(rule, phrases, counts, dry_run=False, verbose=False): - """Validate a complete rule update and write the rule at most once.""" +def prepare_predicted_phrases(rule, phrases, counts, verbose=False): + """Return a completely validated updated rule, or None.""" accepted_phrases = [] for phrase in phrases: issue = candidate_issue(rule, phrase) @@ -158,7 +163,7 @@ def add_predicted_phrases(rule, phrases, counts, dry_run=False, verbose=False): accepted_phrases.append(phrase) if not accepted_phrases: - return False + return accepted_phrases.sort(key=lambda phrase: (-len(phrase), phrase)) updated_rule = copy(rule) @@ -172,16 +177,44 @@ def add_predicted_phrases(rule, phrases, counts, dry_run=False, verbose=False): dry_run=True, ): counts["conflicts"] += 1 - return False + return if updated_rule.text == rule.text: - return False + return counts["injected"] += len(accepted_phrases) + return updated_rule + + +def write_rule_atomically(rule): + """Write rule through a same-filesystem temporary file.""" + rules_directory = Path(rules_data_dir) + with tempfile.TemporaryDirectory( + prefix=".required-phrases-", + dir=rules_directory.parent, + ) as temporary_directory: + temporary_directory = Path(temporary_directory) + rule.dump(str(temporary_directory)) + staged = temporary_directory / rule.identifier + with staged.open("rb+") as stream: + os.fsync(stream.fileno()) + os.replace(staged, rules_directory / rule.identifier) + + +def add_predicted_phrases(rule, phrases, counts, dry_run=False, verbose=False): + """Validate a complete rule update and write the rule at most once.""" + updated_rule = prepare_predicted_phrases( + rule=rule, + phrases=phrases, + counts=counts, + verbose=verbose, + ) + if not updated_rule: + return False if dry_run: return True - updated_rule.dump(rules_data_dir) + write_rule_atomically(updated_rule) rule.text = updated_rule.text rule.source = updated_rule.source return True diff --git a/tests/test_model_rules.py b/tests/test_model_rules.py index 395a93b..2d8d9be 100644 --- a/tests/test_model_rules.py +++ b/tests/test_model_rules.py @@ -18,6 +18,7 @@ from scancode_required_phrases.model_rules import add_model_required_phrases from scancode_required_phrases.model_rules import add_predicted_phrases from scancode_required_phrases.model_rules import new_counts +from scancode_required_phrases.model_rules import prepare_predicted_phrases from scancode_required_phrases.model_rules import select_rules from scancode_required_phrases.model_rules import update_rules_from_predictions @@ -92,6 +93,25 @@ def get_rules(**kwargs): select_rules("unknown") +def test_prepare_predicted_phrases_returns_updated_copy(): + rule = make_rule(TEXT, source="mit_1.RULE") + counts = new_counts() + + updated_rule = prepare_predicted_phrases( + rule=rule, + phrases=["MIT License", "do things"], + counts=counts, + ) + + assert updated_rule.text == ( + "Permission is granted under the {{MIT License}} to {{do things}} with this" + ) + assert updated_rule.source == "mit_1.RULE ml_model" + assert counts["injected"] == 2 + assert rule.text == TEXT + assert rule.source == "mit_1.RULE" + + def test_add_predicted_phrases_dry_run_does_not_mutate_rule(): rule = make_rule(TEXT, source="mit_1.RULE") counts = new_counts() @@ -206,6 +226,28 @@ def dump(rule, rules_data_dir): assert rule.source == saved.source +def test_add_predicted_phrases_keeps_existing_file_when_atomic_replace_fails( + tmp_path, + monkeypatch, +): + rule = make_rule(TEXT) + rule.dump(str(tmp_path)) + rule_path = tmp_path / rule.identifier + before = rule_path.read_bytes() + monkeypatch.setattr(model_rules, "rules_data_dir", str(tmp_path)) + monkeypatch.setattr( + model_rules.os, + "replace", + lambda *args: (_ for _ in ()).throw(OSError("simulated replace failure")), + ) + + with pytest.raises(OSError, match="simulated replace failure"): + add_predicted_phrases(rule, ["MIT License"], new_counts()) + + assert rule_path.read_bytes() == before + assert rule.text == TEXT + + def test_update_rules_from_predictions_processes_selected_rules(): rule = make_rule(TEXT) @@ -318,9 +360,18 @@ def test_load_predictor_requires_a_remote_revision(): model_rules.load_predictor("owner/model") -def test_remote_artifact_names_reject_unsafe_paths(): +@pytest.mark.parametrize( + "name", + [ + "../model.safetensors", + "models/../model.safetensors", + r"..\model.safetensors", + r"C:\model.safetensors", + ], +) +def test_remote_artifact_names_reject_unsafe_paths(name): with pytest.raises(ValueError, match="unsafe artifact path"): - model_rules._artifact_names({"files": {"../model.safetensors": "digest"}}) + model_rules._artifact_names({"files": {name: "digest"}}) def test_load_predictor_stages_only_declared_remote_artifacts(tmp_path, monkeypatch): From 43dbc4f78a6e17fdcbd398405a87134eea5f3f51 Mon Sep 17 00:00:00 2001 From: Kaushik Date: Sun, 13 Sep 2026 17:14:11 +0000 Subject: [PATCH 4/4] Fix Linux model artifact staging Signed-off-by: Kaushik --- src/scancode_required_phrases/model_rules.py | 1 + tests/test_model_rules.py | 34 ++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/src/scancode_required_phrases/model_rules.py b/src/scancode_required_phrases/model_rules.py index a528b86..597f711 100644 --- a/src/scancode_required_phrases/model_rules.py +++ b/src/scancode_required_phrases/model_rules.py @@ -83,6 +83,7 @@ def _load_remote_predictor(repository, revision, hf_token): source = snapshot / name if not source.is_file(): raise ValueError(f"Remote model is missing artifact: {name}") + source = source.resolve(strict=True) target = model_dir / name target.parent.mkdir(parents=True, exist_ok=True) try: diff --git a/tests/test_model_rules.py b/tests/test_model_rules.py index 2d8d9be..a58306e 100644 --- a/tests/test_model_rules.py +++ b/tests/test_model_rules.py @@ -374,6 +374,40 @@ def test_remote_artifact_names_reject_unsafe_paths(name): model_rules._artifact_names({"files": {name: "digest"}}) +def test_load_predictor_resolves_snapshot_symlinks(tmp_path, monkeypatch): + snapshot = tmp_path / "snapshot" + blobs = tmp_path / "blobs" + snapshot.mkdir() + blobs.mkdir() + marker = {"files": {"model.safetensors": hashlib.sha256(b"weights").hexdigest()}} + marker_blob = blobs / "marker" + marker_blob.write_text(json.dumps(marker), encoding="utf-8") + weights_blob = blobs / "weights" + weights_blob.write_bytes(b"weights") + try: + (snapshot / "SUCCESS.json").symlink_to(marker_blob) + (snapshot / "model.safetensors").symlink_to(weights_blob) + except OSError: + pytest.skip("symbolic links are unavailable") + + hub = SimpleNamespace( + hf_hub_download=lambda **kwargs: str(snapshot / "SUCCESS.json"), + snapshot_download=lambda **kwargs: str(snapshot), + ) + monkeypatch.setitem(sys.modules, "huggingface_hub", hub) + + def load(model_dir): + for name in ("SUCCESS.json", "model.safetensors"): + path = model_dir / name + assert path.is_file() + assert not path.is_symlink() + return "predictor" + + monkeypatch.setattr(model_rules.RequiredPhrasePredictor, "from_model_dir", load) + + assert model_rules.load_predictor("owner/model", revision="a" * 40) == "predictor" + + def test_load_predictor_stages_only_declared_remote_artifacts(tmp_path, monkeypatch): snapshot = tmp_path / "snapshot" snapshot.mkdir()