Add the artifact description gate to the lint job

.github/workflows/python_lint.yml

@@ -46,6 +46,15 @@ jobs:
         if: steps.changed-files-py.outputs.any_changed == 'true'
         run: python admin/scripts/check_claim_language.py
 
+      # An artifact's description is the one line the report, the LAVA manifest and
+      # casework quote for it. One that is missing, runs to several lines, only repeats
+      # the name, or duplicates a sibling's in the same module says nothing about the
+      # rows it fronts. Whether a description claims past its own notes is a judgement
+      # the script cannot make; its --review mode lays the pair out for that pass.
+      - name: Guard against an artifact description that says nothing
+        if: steps.changed-files-py.outputs.any_changed == 'true'
+        run: python admin/scripts/check_artifact_descriptions.py
+
       # A conversation artifact's columns are ordered from the roles it declares in
       # data_views: timestamp, other dates, direction, sender, conversation label,
       # message text, media, then the rest. See admin/docs/conversation_column_order.md.

admin/scripts/check_artifact_descriptions.py

@@ -0,0 +1,192 @@
+"""Fail CI when an artifact's description is missing, runs to more than one line,
+only repeats the artifact's name, or duplicates another artifact's in the same module.
+
+Every artifact module declares an `__artifacts_v2__` dict whose `description` is the
+one line the HTML report, the LAVA manifest and the module index quote for it. It is
+the sentence somebody reads to decide what the artifact is. Four defects in it are
+purely mechanical and this script fails on them:
+
+* missing, or not a string;
+* empty once stripped;
+* more than one line;
+* the same text as the artifact's `name`, which says nothing the name did not;
+* the same text as another artifact's description in the same module, which cannot be
+  right for both, since two artifacts exist because they report different things.
+  The commonest cause is a block copied for a sibling and not finished.
+
+The other half of a description audit is not mechanical and this script does not try
+to be a gate for it: whether a description claims past a limit the artifact's own notes
+already concede. Seven merged artifacts were found doing exactly that in one day by
+reading each description beside the sentences in its notes that concede a limit. That
+is a judgement pass and it belongs before the pull request. `--review` lays the pair
+out for it: for each artifact in the named modules it prints the description and every
+sentence of the notes that carries limiting vocabulary, and never fails.
+
+Usage:
+  check_artifact_descriptions.py [--root REPO_ROOT]
+  check_artifact_descriptions.py --review MODULE [MODULE ...]
+
+Exit status 0 when every description passes the mechanical rules, 1 when any fails,
+2 when the artifact tree cannot be found.
+"""
+import argparse
+import ast
+import os
+import re
+import sys
+
+# Vocabulary that marks a sentence in `notes` as conceding a limit. This drives the
+# review listing only; nothing here is a claim in itself.
+CONCESSION = re.compile(
+    r"\b(?:not established|not evidence|is not|are not|was not|were not|does not|"
+    r"do not|did not|cannot|could not|never|no row|not reported|not parsed|"
+    r"not decoded|not resolved|ships with|already present|only|absence of|"
+    r"blank|empty|unexercised|as stored)\b", re.IGNORECASE)
+
+STANDARD_NOTE = (
+    'A description is the one line quoted for the artifact. Say what the rows are and '
+    'where they come from; a description that repeats the name or a sibling says '
+    'nothing, and one that claims past its own notes is the defect --review exists '
+    'to surface.')
+
+
+def artifact_blocks(path):
+    """The __artifacts_v2__ dict of one module as {key: info}, or (None, problem)."""
+    try:
+        with open(path, encoding='utf-8', errors='replace') as handle:
+            tree = ast.parse(handle.read())
+    except (OSError, SyntaxError) as err:
+        return None, f'{os.path.basename(path)}: could not parse ({err})'
+    for node in tree.body:
+        if isinstance(node, ast.Assign) and any(
+                isinstance(t, ast.Name) and t.id == '__artifacts_v2__' for t in node.targets):
+            try:
+                value = ast.literal_eval(node.value)
+            except ValueError:
+                return None, f'{os.path.basename(path)}: __artifacts_v2__ is not a literal'
+            return (value if isinstance(value, dict) else {}), None
+    return {}, None
+
+
+def _normal(text):
+    return ' '.join(str(text).split()).lower()
+
+
+def check_module(path):
+    """(violations, problem) for one module. Each violation is (module, key, reason)."""
+    blocks, problem = artifact_blocks(path)
+    if blocks is None:
+        return [], problem
+    module = os.path.basename(path)
+    violations = []
+    seen = {}
+    for key, info in blocks.items():
+        if not isinstance(info, dict):
+            continue
+        description = info.get('description')
+        if not isinstance(description, str):
+            violations.append((module, key, 'description is missing or not a string'))
+            continue
+        if not description.strip():
+            violations.append((module, key, 'description is empty'))
+            continue
+        if '\n' in description:
+            violations.append((module, key, 'description runs to more than one line'))
+        name = info.get('name')
+        if isinstance(name, str) and _normal(name) == _normal(description):
+            violations.append((module, key, 'description only repeats the name'))
+        normal = _normal(description)
+        if normal in seen:
+            violations.append((module, key,
+                               f'description duplicates {seen[normal]!r} in the same module'))
+        else:
+            seen[normal] = key
+    return violations, None
+
+
+def concession_sentences(notes):
+    """The sentences of a notes field that concede a limit, for the review listing."""
+    if not isinstance(notes, str):
+        return []
+    sentences = [s.strip() for s in re.split(r'(?<=[.])\s+', notes) if s.strip()]
+    return [s for s in sentences if CONCESSION.search(s)]
+
+
+def review(paths):
+    """Print each description beside the limits its own notes concede. Never fails."""
+    for path in paths:
+        blocks, problem = artifact_blocks(path)
+        print(f'\n{"=" * 78}\n{os.path.basename(path)}')
+        if blocks is None:
+            print(f'  {problem}')
+            continue
+        for key, info in blocks.items():
+            if not isinstance(info, dict):
+                continue
+            print(f'\n  {key}')
+            print(f'     description: {info.get("description")!r}')
+            limits = concession_sentences(info.get('notes'))
+            for sentence in limits[:8]:
+                print(f'     notes limit: {sentence[:160]}')
+            if not limits:
+                print('     notes limit: (none conceded)')
+    print('\nJudge each description against the limits beside it: it must not claim past them.')
+
+
+def main():
+    parser = argparse.ArgumentParser(
+        description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter)
+    parser.add_argument('--root', default=None, help='repository root')
+    parser.add_argument('--review', nargs='+', metavar='MODULE',
+                        help='print each description beside its notes\' conceded limits '
+                             'for the named artifact modules, and exit 0')
+    args = parser.parse_args()
+
+    root = args.root or os.path.dirname(
+        os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
+    artifacts = os.path.join(root, 'scripts', 'artifacts')
+    if not os.path.isdir(artifacts):
+        print(f'No scripts/artifacts under {root}', file=sys.stderr)
+        return 2
+
+    if args.review:
+        paths = []
+        for name in args.review:
+            base = name if name.endswith('.py') else name + '.py'
+            candidate = base if os.path.isabs(base) else os.path.join(artifacts, os.path.basename(base))
+            if not os.path.isfile(candidate):
+                print(f'No such artifact module: {name}', file=sys.stderr)
+                return 2
+            paths.append(candidate)
+        review(paths)
+        return 0
+
+    violations, unreadable, modules = [], [], 0
+    for name in sorted(os.listdir(artifacts)):
+        if not name.endswith('.py'):
+            continue
+        modules += 1
+        found, problem = check_module(os.path.join(artifacts, name))
+        violations.extend(found)
+        if problem:
+            unreadable.append(problem)
+
+    if violations:
+        print(f'Artifact descriptions that say nothing ({len(violations)}):')
+        for module, key, reason in violations:
+            print(f'  {module}::{key}  {reason}')
+        print()
+        print(STANDARD_NOTE)
+        return 1
+
+    summary = f'Checked {modules} artifact module(s): every description is present, one line, and its own.'
+    if unreadable:
+        summary += f' {len(unreadable)} module(s) NOT checked.'
+    print(summary)
+    for problem in unreadable:
+        print(f'  {problem}')
+    return 0
+
+
+if __name__ == '__main__':
+    sys.exit(main())

admin/test/scripts/test_check_artifact_descriptions.py

@@ -0,0 +1,141 @@
+"""Prove the description checker fails on each shape it exists to catch and passes
+the shapes it must leave alone.
+
+The mechanical defects are exact: a description that is missing, empty, more than one
+line, identical to the artifact's name, or identical to a sibling's in the same module.
+Each gets a fixture that fails and a neighbouring fixture that passes, because a gate
+that has only ever been seen green has not been shown to gate anything.
+
+Two negative cases matter as much as the positives. Identical descriptions in two
+different modules are not flagged, since the rule is scoped to one module, where a
+duplicate is a copy left unfinished. And the review listing is not a gate: its helper
+returns the conceding sentences and nothing here asserts on them as failures.
+
+Expected values are written out as literals rather than derived from the module under
+test, so a fixture cannot move with a bug in the code it checks.
+"""
+import importlib.util
+import pathlib
+import sys
+import tempfile
+import textwrap
+import unittest
+
+REPO_ROOT = pathlib.Path(__file__).resolve().parents[3]
+_MODULE_PATH = REPO_ROOT / 'admin' / 'scripts' / 'check_artifact_descriptions.py'
+
+# admin/scripts is not a package, so load the module from its path.
+_spec = importlib.util.spec_from_file_location('check_artifact_descriptions', _MODULE_PATH)
+cad = importlib.util.module_from_spec(_spec)
+sys.modules['check_artifact_descriptions'] = cad
+_spec.loader.exec_module(cad)
+
+
+def violations_for(source, filename='sample.py'):
+    with tempfile.TemporaryDirectory() as folder:
+        path = pathlib.Path(folder) / filename
+        path.write_text(textwrap.dedent(source), encoding='utf-8')
+        found, problem = cad.check_module(str(path))
+    if problem:
+        raise AssertionError(problem)
+    return [(key, reason) for _module, key, reason in found]
+
+
+def block(entries):
+    """An __artifacts_v2__ module source from {key: {field: value}}."""
+    lines = ['__artifacts_v2__ = {']
+    for key, fields in entries.items():
+        lines.append(f'    {key!r}: {{')
+        for field, value in fields.items():
+            lines.append(f'        {field!r}: {value!r},')
+        lines.append('    },')
+    lines.append('}')
+    return '\n'.join(lines) + '\n'
+
+
+class MechanicalDefects(unittest.TestCase):
+    def test_missing_description_fails(self):
+        found = violations_for(block({'a': {'name': 'Thing'}}))
+        self.assertEqual(found, [('a', 'description is missing or not a string')])
+
+    def test_non_string_description_fails(self):
+        found = violations_for(block({'a': {'name': 'Thing', 'description': 7}}))
+        self.assertEqual(found, [('a', 'description is missing or not a string')])
+
+    def test_empty_description_fails(self):
+        found = violations_for(block({'a': {'name': 'Thing', 'description': '   '}}))
+        self.assertEqual(found, [('a', 'description is empty')])
+
+    def test_multiline_description_fails(self):
+        found = violations_for(block({'a': {'name': 'Thing',
+                                             'description': 'Rows from x.\nMore.'}}))
+        self.assertEqual(found, [('a', 'description runs to more than one line')])
+
+    def test_description_equal_to_name_fails(self):
+        found = violations_for(block({'a': {'name': 'Samsung Notes',
+                                             'description': 'Samsung Notes'}}))
+        self.assertEqual(found, [('a', 'description only repeats the name')])
+
+    def test_name_match_ignores_case_and_spacing(self):
+        found = violations_for(block({'a': {'name': 'Samsung  Notes',
+                                             'description': 'samsung notes '}}))
+        self.assertEqual(found, [('a', 'description only repeats the name')])
+
+    def test_duplicate_within_module_fails_on_the_second(self):
+        found = violations_for(block({
+            'contacts': {'name': 'Romeo Contacts', 'description': 'Parses Romeo contacts'},
+            'accounts': {'name': 'Romeo Accounts', 'description': 'Parses Romeo contacts'},
+        }))
+        self.assertEqual(found, [('accounts',
+                                  "description duplicates 'contacts' in the same module")])
+
+    def test_duplicate_match_ignores_case(self):
+        found = violations_for(block({
+            'a': {'name': 'A', 'description': 'Kik notifications from FCM'},
+            'b': {'name': 'B', 'description': 'kik Notifications from fcm'},
+        }))
+        self.assertEqual([k for k, _ in found], ['b'])
+
+
+class ShapesLeftAlone(unittest.TestCase):
+    def test_distinct_one_line_descriptions_pass(self):
+        found = violations_for(block({
+            'a': {'name': 'JusTalk - Calls', 'description': 'Calls from the JusTalk store'},
+            'b': {'name': 'JusTalk Kids - Calls',
+                  'description': 'Calls from the JusTalk Kids store'},
+        }))
+        self.assertEqual(found, [])
+
+    def test_description_that_extends_the_name_passes(self):
+        found = violations_for(block({'a': {'name': 'Samsung Notes',
+                                             'description': 'Notes from Samsung Notes, with media'}}))
+        self.assertEqual(found, [])
+
+    def test_same_description_in_two_modules_is_not_flagged(self):
+        source = block({'a': {'name': 'A', 'description': 'Chess database'}})
+        self.assertEqual(violations_for(source, 'one.py'), [])
+        self.assertEqual(violations_for(source, 'two.py'), [])
+
+    def test_unparseable_module_is_reported_not_crashed(self):
+        with tempfile.TemporaryDirectory() as folder:
+            path = pathlib.Path(folder) / 'broken.py'
+            path.write_text('__artifacts_v2__ = {\n', encoding='utf-8')
+            found, problem = cad.check_module(str(path))
+        self.assertEqual(found, [])
+        self.assertIn('could not parse', problem)
+
+
+class ReviewListing(unittest.TestCase):
+    def test_conceding_sentences_are_selected(self):
+        notes = ('One row per entry. Whether every file is recorded was not established. '
+                 'Times are UTC. A blank value is not evidence of absence.')
+        limits = cad.concession_sentences(notes)
+        self.assertEqual(limits, ['Whether every file is recorded was not established.',
+                                  'A blank value is not evidence of absence.'])
+
+    def test_non_string_notes_yield_nothing(self):
+        self.assertEqual(cad.concession_sentences(None), [])
+
+
+if __name__ == '__main__':
+    unittest.main()

scripts/artifacts/chrome.py

@@ -92,7 +92,8 @@
     },
     "chrome_os_settings": {
         "name": "Chrome - OS Settings",
-        "description": "Parses OS Settings from Google Takeout",
+        "description": "Preference names with the user's gender and birth year from the Chrome OS "
+                       "Settings.json of a Google Takeout.",
         "author": "@stark4n6 & @upintheairsheep",
         "creation_date": "2023-08-18",
         "last_update_date": "2026-06-22",
@@ -105,7 +106,9 @@
     },
     "chrome_arc_packages": {
         "name": "Chrome - ARC Packages",
-        "description": "Parses OS Settings from Google Takeout",
+        "description": "Android (ARC) packages backed up from a Chrome OS device, from the Chrome "
+                       "OS Settings.json of a Google Takeout, with package name, version, last "
+                       "backup time and Android id.",
         "author": "@stark4n6 & @upintheairsheep",
         "creation_date": "2023-08-18",
         "last_update_date": "2026-06-22",