Repository: instructlab/sdg

Issue:
Empty dataset error kills the workflow
When the qna.yaml is not appropriate or a wrong model is used, the generation fails to happen and throws an error --
`instructlab.sdg.pipeline.EmptyDatasetError: Pipeline stopped: Empty dataset after running pipe`


Proposed solution:

- [ ] We need this to fail gracefully and continue operations if there were multiple leaf nodes.
- [ ] Provide more information on the error so that the user knows what steps need to be followed to rectify the issue.

Hints from issue comments:
@relyt0925: +1 to this: I noticed this as well@marceloleitner: It is getting better with this patch but it would be nicer if it could have some hint on possible reasons. Like, "please ensure the number of examples is enough.", "please make sure it attends the guidelines at HTTP", or something like that. You will know better.

What I know is that I just spent a day debugging this issue. I could only understand the reason after I found the issue that led to this MR, https://github.com/instructlab/sdg/issues/240 @bbrowning: @marceloleitner Those are reasonable suggestions, although I'd ask that perhaps that be a separate issue because that's less about handling the case of a dataset being empty without crashing and more a request for better logging when something fails during the generation giving a user more indication of what potential causes of that type of failure may be.

Solution patch:
diff --git a/.github/mergify.yml b/.github/mergify.yml
index 081eeb40..77f1240b 100644
--- a/.github/mergify.yml
+++ b/.github/mergify.yml
@@ -17,7 +17,6 @@ pull_request_rules:
 
     # If workflow configuration files in .github/ are changed, the actionlint check must pass
     - or:
-      
       - and:
         - check-success=actionlint
         - or:
@@ -130,6 +129,15 @@ pull_request_rules:
       remove:
         - needs-rebase
 
+- name: release-branch-label
+  description: Automatically apply the release-branch label to release branch PRs
+  conditions:
+    - base~=^release-
+  actions:
+    label:
+      add:
+        - release-branch
+
 - name: Apply ci-failure label if any CI checks have failed
   conditions:
       - "#check-failure>0"
@@ -145,3 +153,30 @@ pull_request_rules:
     label:
       remove:
         - ci-failure
+
+- name: Apply 'one-approval' label if one of the maintainer approved the PR
+  conditions:
+      - "#approved-reviews-by=1"
+  actions:
+    label:
+      add:
+        - one-approval
+
+- name: Remove 'one-approval' label if the approval was reset
+  conditions:
+      - "#approved-reviews-by!=1"
+  actions:
+    label:
+      remove:
+        - one-approval
+
+- name: label-dependencies
+  description: Automatically apply dependencies label
+  conditions:
+    - or:
+      - files~=^requirements.*\.txt$
+      - files~=^requirements/
+  actions:
+    label:
+      add:
+        - dependencies
diff --git a/.github/workflows/actionlint.dockerfile b/.github/workflows/actionlint.dockerfile
index 79db008b..c082d8e7 100644
--- a/.github/workflows/actionlint.dockerfile
+++ b/.github/workflows/actionlint.dockerfile
@@ -1,3 +1,3 @@
 # Since dependabot cannot update workflows using docker,
 # we use this indirection since dependabot can update this file.
-FROM rhysd/actionlint:1.7.1@sha256:435ecdb63b1169e80ca3e136290072548c07fc4d76a044cf5541021712f8f344
+FROM rhysd/actionlint:1.7.2@sha256:89d3f90f82781dee3c8724651129634b08cf2241bbd67fcd02a1c5198119fc5e
diff --git a/.github/workflows/actionlint.yml b/.github/workflows/actionlint.yml
index e200d568..84fc0fd5 100644
--- a/.github/workflows/actionlint.yml
+++ b/.github/workflows/actionlint.yml
@@ -31,12 +31,12 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - name: "Harden Runner"
-        uses: step-security/harden-runner@5c7944e73c4c2a096b17a9cb74d65b6c2bbafbde # v2.9.1
+        uses: step-security/harden-runner@91182cccc01eb5e619899d80e4e971d6181294a7 # v2.10.1
         with:
           egress-policy: audit # TODO: change to 'egress-policy: block' after couple of runs
 
       - name: "Checkout"
-        uses: actions/checkout@a5ac7e51b41094c92402da3b24376905380afc29 # v4.1.6
+        uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0
         with:
           fetch-depth: 0
 
diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml
index 248fc8c3..e034f2ba 100644
--- a/.github/workflows/docs.yml
+++ b/.github/workflows/docs.yml
@@ -33,14 +33,14 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - name: "Harden Runner"
-        uses: step-security/harden-runner@5c7944e73c4c2a096b17a9cb74d65b6c2bbafbde # v2.9.1
+        uses: step-security/harden-runner@91182cccc01eb5e619899d80e4e971d6181294a7 # v2.10.1
         with:
           egress-policy: audit # TODO: change to 'egress-policy: block' after couple of runs
       - name: "Checkout"
-        uses: actions/checkout@a5ac7e51b41094c92402da3b24376905380afc29 # v4.1.6
+        uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0
         with:
           fetch-depth: 0
       - name: "Check Markdown documents"
-        uses: DavidAnson/markdownlint-cli2-action@b4c9feab76d8025d1e83c653fa3990936df0e6c8 # v16.0.0
+        uses: DavidAnson/markdownlint-cli2-action@db43aef879112c3119a410d69f66701e0d530809 # v17.0.0
         with:
           globs: '**/*.md'
diff --git a/.github/workflows/e2e-nvidia-a10g-x4.yml b/.github/workflows/e2e-nvidia-a10g-x4.yml
index b11ad257..f6affca5 100644
--- a/.github/workflows/e2e-nvidia-a10g-x4.yml
+++ b/.github/workflows/e2e-nvidia-a10g-x4.yml
@@ -53,7 +53,7 @@ jobs:
 
     steps:
       - name: Checkout
-        uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
+        uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0
         with:
           fetch-depth: 0
 
diff --git a/.github/workflows/e2e-nvidia-t4-x1.yml b/.github/workflows/e2e-nvidia-t4-x1.yml
index 9c709180..22dba961 100644
--- a/.github/workflows/e2e-nvidia-t4-x1.yml
+++ b/.github/workflows/e2e-nvidia-t4-x1.yml
@@ -83,14 +83,14 @@ jobs:
           ls -l
 
       - name: Checkout instructlab/instructlab
-        uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
+        uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0
         with:
           repository: "instructlab/instructlab"
           path: "instructlab"
           fetch-depth: 0
 
       - name: Checkout instructlab/sdg
-        uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
+        uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0
         with:
           repository: "instructlab/sdg"
           path: "sdg"
diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
index 07018a1a..7078d2c1 100644
--- a/.github/workflows/lint.yml
+++ b/.github/workflows/lint.yml
@@ -12,6 +12,7 @@ on:
       - 'pyproject.toml'
       - 'requirements*.txt'
       - 'tox.ini'
+      - '.pylintrc'
       - 'scripts/*.sh'
       - '.github/workflows/lint.yml'
       - 'src/instructlab/sdg/pipelines/**'
@@ -24,26 +25,56 @@ on:
       - 'pyproject.toml'
       - 'requirements*.txt'
       - 'tox.ini'
+      - '.pylintrc'
       - 'scripts/*.sh'
       - '.github/workflows/lint.yml'
       - 'src/instructlab/sdg/pipelines/**'
 
 env:
-  PYTHON_VERSION: 3.11
+  LC_ALL: en_US.UTF-8
+
+defaults:
+  run:
+    shell: bash
+
+permissions:
+  contents: read
 
 jobs:
   lint:
     runs-on: ubuntu-latest
+    name: "${{ matrix.lint.name }}"
+    strategy:
+      fail-fast: false
+      matrix:
+        lint:
+          - name: "ruff"
+            commands: |
+              tox -e ruff -- check
+          - name: "pylint"
+            commands: |
+              echo "::add-matcher::.github/workflows/matchers/pylint.json"
+              tox -e lint
+          - name: "mypy"
+            commands: |
+              tox -e mypy
+          - name: "pipelines"
+            commands: |
+              tox -e validate-pipelines
     steps:
-      - name: Checkout
-        uses: actions/checkout@v4
+      - name: "Harden Runner"
+        uses: step-security/harden-runner@91182cccc01eb5e619899d80e4e971d6181294a7 # v2.10.1
+        with:
+          egress-policy: audit # TODO: change to 'egress-policy: block' after couple of runs
+
+      - name: "Checkout"
+        uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0
         with:
           # https://github.com/actions/checkout/issues/249
-          fetch-depth: 0
-          submodules: true
+          fetch-depth: 0  
 
       - name: Setup Python 3.11
-        uses: actions/setup-python@v5
+        uses: actions/setup-python@f677139bbe7f9c59b41e40162b753c062f5d49a3 # v5.2.0
         with:
           python-version: 3.11
           cache: pip
@@ -51,28 +82,20 @@ jobs:
             **/pyproject.toml
             **/requirements*.txt
 
-      - name: Install dependencies
-        id: deps
+      - name: "Install tox"
         run: |
           python -m pip install --upgrade pip
-          python -m pip install tox
-
-      - name: Run Ruff check
-        run: |
-          tox -e ruff -- check
+          python -m pip install tox tox-gh
 
-      - name: Run linting
-        if: ${{ !cancelled() && (steps.deps.outcome == 'success') }}
+      - name: "${{ matrix.lint.name }}"
         run: |
-          echo "::add-matcher::.github/workflows/matchers/pylint.json"
-          tox -e lint
+          ${{ matrix.lint.commands }}
+        env:
+          RUFF_OUTPUT_FORMAT: github
 
-      - name: Run mypy type checks
-        if: ${{ !cancelled() && (steps.deps.outcome == 'success') }}
-        run: |
-          tox -e mypy
-
-      - name: Validate pipeline schemas
-        if: ${{ !cancelled() && (steps.deps.outcome == 'success') }}
-        run: |
-          tox -e validate-pipelines
+  lint-workflow-complete:
+    needs: ["lint"]
+    runs-on: ubuntu-latest
+    steps:
+      - name: Lint Workflow Complete
+        run: echo "Lint Workflow Complete"
diff --git a/.github/workflows/pypi.yaml b/.github/workflows/pypi.yaml
index 242ffbfb..6df80872 100644
--- a/.github/workflows/pypi.yaml
+++ b/.github/workflows/pypi.yaml
@@ -37,19 +37,19 @@ jobs:
         runs-on: ubuntu-latest
         steps:
             - name: "Harden Runner"
-              uses: step-security/harden-runner@5c7944e73c4c2a096b17a9cb74d65b6c2bbafbde # v2.9.1
+              uses: step-security/harden-runner@91182cccc01eb5e619899d80e4e971d6181294a7 # v2.10.1
               with:
                   egress-policy: audit # TODO: change to 'egress-policy: block' after couple of runs
 
 
             - name: "Checkout"
-              uses: actions/checkout@a5ac7e51b41094c92402da3b24376905380afc29 # v4.1.6
+              uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0
               with:
                   # for setuptools-scm
                   fetch-depth: 0
 
             - name: "Build and Inspect"
-              uses: hynek/build-and-inspect-python-package@2dbbf2b252d3a3c7cec7a810e3ed5983bd17b13a # v2.8.0
+              uses: hynek/build-and-inspect-python-package@73aea398b9c8de9ea9e4464c6b13cb8b1f3d6294 # v2.9.0
 
     # push to Test PyPI on
     # - a new GitHub release is published
@@ -67,7 +67,7 @@ jobs:
 
         steps:
             - name: "Harden Runner"
-              uses: step-security/harden-runner@5c7944e73c4c2a096b17a9cb74d65b6c2bbafbde # v2.9.1
+              uses: step-security/harden-runner@91182cccc01eb5e619899d80e4e971d6181294a7 # v2.10.1
               with:
                   egress-policy: audit # TODO: change to 'egress-policy: block' after couple of runs
 
@@ -78,7 +78,7 @@ jobs:
                   path: dist
 
             - name: "Upload to Test PyPI"
-              uses: pypa/gh-action-pypi-publish@0ab0b79471669eb3a4d647e625009c62f9f3b241 # v1.10.1
+              uses: pypa/gh-action-pypi-publish@897895f1e160c830e369f9779632ebc134688e1b # v1.10.2
               with:
                   repository-url: https://test.pypi.org/legacy/
 
@@ -99,7 +99,7 @@ jobs:
 
         steps:
             - name: "Harden Runner"
-              uses: step-security/harden-runner@5c7944e73c4c2a096b17a9cb74d65b6c2bbafbde # v2.9.1
+              uses: step-security/harden-runner@91182cccc01eb5e619899d80e4e971d6181294a7 # v2.10.1
               with:
                   egress-policy: audit # TODO: change to 'egress-policy: block' after couple of runs
 
@@ -130,4 +130,4 @@ jobs:
                   rm ./dist/*.sigstore.json
 
             - name: "Upload to PyPI"
-              uses: pypa/gh-action-pypi-publish@0ab0b79471669eb3a4d647e625009c62f9f3b241 # v1.10.1
+              uses: pypa/gh-action-pypi-publish@897895f1e160c830e369f9779632ebc134688e1b # v1.10.2
diff --git a/.github/workflows/spellcheck.yml b/.github/workflows/spellcheck.yml
index 060abc88..4c0f854b 100644
--- a/.github/workflows/spellcheck.yml
+++ b/.github/workflows/spellcheck.yml
@@ -32,14 +32,14 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - name: "Harden Runner"
-        uses: step-security/harden-runner@5c7944e73c4c2a096b17a9cb74d65b6c2bbafbde # v2.9.1
+        uses: step-security/harden-runner@91182cccc01eb5e619899d80e4e971d6181294a7 # v2.10.1
         with:
          egress-policy: audit # TODO: change to 'egress-policy: block' after couple of runs
 
       - name: "Checkout"
-        uses: actions/checkout@a5ac7e51b41094c92402da3b24376905380afc29 # v4.1.6
+        uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0
         with:
           fetch-depth: 0
 
       - name: Spellcheck
-        uses: rojopolis/spellcheck-github-actions@a0fba0ca8b9e552d4241ea5ccfaa4ca4162622d0 # v0.41.0
+        uses: rojopolis/spellcheck-github-actions@b83ca7c1b5c285e4f2b43e209a455c74872ec341 # v0.42.0
diff --git a/.github/workflows/stale_bot.yml b/.github/workflows/stale_bot.yml
new file mode 100644
index 00000000..b621e396
--- /dev/null
+++ b/.github/workflows/stale_bot.yml
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: Apache-2.0
+
+name: 'Close stale issues and PRs'
+
+on:
+  schedule:
+    - cron: '30 1 * * *'
+
+env:
+  LC_ALL: en_US.UTF-8
+
+defaults:
+  run:
+    shell: bash
+
+permissions:
+  contents: read
+
+jobs:
+  stale:
+    permissions:
+      issues: write
+      pull-requests: write
+    runs-on: ubuntu-latest
+    steps:
+      - name: "Harden Runner"
+        uses: step-security/harden-runner@91182cccc01eb5e619899d80e4e971d6181294a7 # v2.10.1
+        with:
+          egress-policy: audit # TODO: change to 'egress-policy: block' after couple of runs
+
+      - name: "Stale Action"
+        uses: actions/stale@28ca1036281a5e5922ead5184a1bbf96e5fc984e # v9.0.0
+        with:
+          stale-issue-label: 'stale'
+          stale-issue-message: >
+            This issue has been automatically marked as stale because it has not had activity within 90 days.
+            It will be automatically closed if no further activity occurs within 30 days.
+          close-issue-message: >
+            This issue has been automatically closed due to inactivity. Please feel free to reopen if you feel it is still relevant!
+          days-before-issue-stale: 90
+          days-before-issue-close: 30
+          stale-pr-label: 'stale'
+          stale-pr-message: >
+            This pull request has been automatically marked as stale because it has not had activity within 90 days.
+            It will be automatically closed if no further activity occurs within 30 days.
+          close-pr-message: >
+            This pull request has been automatically closed due to inactivity. Please feel free to reopen if you intend to continue working on it!
+          days-before-pr-stale: 90
+          days-before-pr-close: 30
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index fbb6149e..ee410389 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -51,12 +51,12 @@ jobs:
             platform: "macos-latest"
     steps:
       - name: "Harden Runner"
-        uses: step-security/harden-runner@5c7944e73c4c2a096b17a9cb74d65b6c2bbafbde # v2.9.1
+        uses: step-security/harden-runner@91182cccc01eb5e619899d80e4e971d6181294a7 # v2.10.1
         with:
           egress-policy: audit # TODO: change to 'egress-policy: block' after couple of runs
 
       - name: Checkout
-        uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
+        uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0
         with:
           # https://github.com/actions/checkout/issues/249
           fetch-depth: 0
@@ -76,7 +76,7 @@ jobs:
           brew install expect coreutils bash
 
       - name: Setup Python ${{ matrix.python }}
-        uses: actions/setup-python@82c7e631bb3cdc910f68e0081d67478d79c6982d # v5.1.0
+        uses: actions/setup-python@f677139bbe7f9c59b41e40162b753c062f5d49a3 # v5.2.0
         with:
           python-version: ${{ matrix.python }}
           cache: pip
diff --git a/.pylintrc b/.pylintrc
index 5cde4c71..612f2258 100644
--- a/.pylintrc
+++ b/.pylintrc
@@ -434,6 +434,7 @@ disable=raw-checker-failed,
         inconsistent-return-statements,
         no-member,
         too-many-arguments,
+        too-many-positional-arguments,
         too-many-locals,
         too-many-branches,
         too-many-statements,
diff --git a/src/instructlab/sdg/generate_data.py b/src/instructlab/sdg/generate_data.py
index 61934535..d7bfd9f2 100644
--- a/src/instructlab/sdg/generate_data.py
+++ b/src/instructlab/sdg/generate_data.py
@@ -25,7 +25,6 @@
 from instructlab.sdg.pipeline import (
     FULL_PIPELINES_PACKAGE,
     SIMPLE_PIPELINES_PACKAGE,
-    EmptyDatasetError,
     Pipeline,
     PipelineContext,
 )
@@ -359,6 +358,7 @@ def generate_data(
         )
 
     generated_data = None
+    empty_sdg_leaf_nodes = []
     for leaf_node in leaf_nodes.values():
         is_knowledge = False
         leaf_node_path = leaf_node[0]["taxonomy_path"].replace("->", "_")
@@ -382,9 +382,9 @@ def generate_data(
         logger.debug("Dataset: %s", ds)
         new_generated_data = pipe.generate(ds, leaf_node_path)
         if len(new_generated_data) == 0:
-            raise EmptyDatasetError(
-                "Pipeline stopped: Empty dataset after running pipe"
-            )
+            empty_sdg_leaf_nodes.append(leaf_node_path)
+            logger.warning("Empty dataset for qna node: %s", leaf_node_path)
+            continue
         generated_data = (
             [new_generated_data]
             if generated_data is None
@@ -418,3 +418,9 @@ def generate_data(
 
     generate_duration = time.time() - generate_start
     logger.info(f"Generation took {generate_duration:.2f}s")
+    if len(empty_sdg_leaf_nodes) > 0:
+        logger.warning(
+            "Leaf nodes with empty sdg output: {}".format(
+                " ".join(empty_sdg_leaf_nodes)
+            )
+        )


Test patch:
diff --git a/tests/test_generate_data.py b/tests/test_generate_data.py
index 0eb50d08..b5dc7c6f 100644
--- a/tests/test_generate_data.py
+++ b/tests/test_generate_data.py
@@ -9,6 +9,7 @@
 import glob
 import json
 import os
+import re
 import shutil
 import tempfile
 import unittest
@@ -267,6 +268,11 @@ def strip_q(q):
     return output
 
 
+def _empty_llmblock_generate(self, samples):
+    """Return an empty set of generated samples."""
+    return []
+
+
 @patch.object(LLMBlock, "_generate", _noop_llmblock_generate)
 class TestGenerateCompositionalData(unittest.TestCase):
     @pytest.fixture(autouse=True)
@@ -452,6 +458,61 @@ def __exit__(self, *args):
         self.teardown()
 
 
+@patch.object(LLMBlock, "_generate", _empty_llmblock_generate)
+class TestGenerateEmptyDataset(unittest.TestCase):
+    @pytest.fixture(autouse=True)
+    def _init_taxonomy(self, taxonomy_dir):
+        self.test_taxonomy = taxonomy_dir
+
+    def setUp(self):
+        self.tmp_path = tempfile.TemporaryDirectory().name
+        test_valid_knowledge_skill_file = os.path.join(
+            TEST_DATA_DIR, "test_valid_knowledge_skill.yaml"
+        )
+        tracked_knowledge_file = os.path.join("knowledge  ", "tracked", "qna.yaml")
+        untracked_knowledge_file = os.path.join("knowledge", "new", "qna.yaml")
+        test_valid_knowledge_skill = load_test_skills(test_valid_knowledge_skill_file)
+        self.test_taxonomy.add_tracked(
+            tracked_knowledge_file, test_valid_knowledge_skill
+        )
+        self.test_taxonomy.create_untracked(
+            untracked_knowledge_file, test_valid_knowledge_skill
+        )
+
+    def test_generate(self):
+        with patch("logging.Logger.info") as mocked_logger:
+            generate_data(
+                client=MagicMock(),
+                logger=mocked_logger,
+                model_family="merlinite",
+                model_name="models/merlinite-7b-lab-Q4_K_M.gguf",
+                num_instructions_to_generate=10,
+                taxonomy=self.test_taxonomy.root,
+                taxonomy_base=TEST_TAXONOMY_BASE,
+                output_dir=self.tmp_path,
+                chunk_word_count=1000,
+                server_ctx_size=4096,
+                pipeline="simple",
+            )
+        mocked_logger.warning.assert_called()
+        assert re.search(
+            "empty sdg output: knowledge_new", mocked_logger.warning.call_args.args[0]
+        )
+
+    def teardown(self) -> None:
+        """Recursively remove the temporary repository and all of its
+        subdirectories and files.
+        """
+        shutil.rmtree(self.tmp_path)
+        return
+
+    def __enter__(self):
+        return self
+
+    def __exit__(self, *args):
+        self.teardown()
+
+
 def test_context_init_batch_size_optional():
     """Test that the _context_init function can handle a missing batch size by
     delegating to the default in PipelineContext.