diff --git a/.github/workflows/python_lint.yml b/.github/workflows/python_lint.yml index c6bb6c4..bcb8e83 100644 --- a/.github/workflows/python_lint.yml +++ b/.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. diff --git a/admin/scripts/check_artifact_descriptions.py b/admin/scripts/check_artifact_descriptions.py new file mode 100644 index 0000000..c900156 --- /dev/null +++ b/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()) diff --git a/admin/test/scripts/test_check_artifact_descriptions.py b/admin/test/scripts/test_check_artifact_descriptions.py new file mode 100644 index 0000000..53d7a56 --- /dev/null +++ b/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() diff --git a/scripts/artifacts/chrome.py b/scripts/artifacts/chrome.py index 4d2d4e8..f2ae27f 100644 --- a/scripts/artifacts/chrome.py +++ b/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", diff --git a/scripts/artifacts/takeoutSavedLinks.py b/scripts/artifacts/takeoutSavedLinks.py index 6a15ea7..baf049d 100755 --- a/scripts/artifacts/takeoutSavedLinks.py +++ b/scripts/artifacts/takeoutSavedLinks.py @@ -1,7 +1,9 @@ __artifacts_v2__ = { "takeoutSavedLinksDefault": { "name": "Saved Links - Default List", - "description": "Collections of saved links (images, places, web pages, etc.) from Google Search and Maps.", + "description": "Entries of the Default list in the Saved folder of a Google Takeout, " + "links saved from Google Search and Maps with their title, note, URL and " + "comment.", "author": "@KevinPagano3", "creation_date": "2021-09-25", "last_update_date": "2026-06-27", @@ -14,7 +16,8 @@ }, "takeoutSavedLinksFavImages": { "name": "Saved Links - Favorite Images", - "description": "Collections of saved links (images, places, web pages, etc.) from Google Search and Maps.", + "description": "Entries of the Favorite images list in the Saved folder of a Google " + "Takeout, with their title, note, URL and comment.", "author": "@KevinPagano3", "creation_date": "2021-09-25", "last_update_date": "2026-06-27", @@ -27,7 +30,8 @@ }, "takeoutSavedLinksFavPages": { "name": "Saved Links - Favorite Pages", - "description": "Collections of saved links (images, places, web pages, etc.) from Google Search and Maps.", + "description": "Entries of the Favorite pages list in the Saved folder of a Google " + "Takeout, with their title, note, URL and comment.", "author": "@KevinPagano3", "creation_date": "2021-09-25", "last_update_date": "2026-06-27", @@ -40,7 +44,8 @@ }, "takeoutSavedLinksWantToGo": { "name": "Saved Links - Want To Go", - "description": "Collections of saved links (images, places, web pages, etc.) from Google Search and Maps.", + "description": "Entries of the Want to go list in the Saved folder of a Google Takeout, " + "with their title, note, URL and comment.", "author": "@KevinPagano3", "creation_date": "2021-09-25", "last_update_date": "2026-06-27", diff --git a/scripts/artifacts/twitterReturns.py b/scripts/artifacts/twitterReturns.py index ca2a2ea..2c8a529 100755 --- a/scripts/artifacts/twitterReturns.py +++ b/scripts/artifacts/twitterReturns.py @@ -1,7 +1,9 @@ __artifacts_v2__ = { "tweets": { "name": "Tweets", - "description": "Processes tweets from a twitter return", + "description": "Tweets from the tweets file of a Twitter return, with time, text, the " + "image where the tweets media folder holds it, tweet id, edit info, " + "retweet flag and entities.", "author": "@AlexisBrignoni", "creation_date": "2025-06-23", "last_update_date": "2025-06-23", @@ -14,7 +16,9 @@ }, "deltweets": { "name": "Deleted Tweets", - "description": "Processes tweets from a twitter return", + "description": "Deleted tweets from the deleted-tweets file of a Twitter return, with " + "time, text, the image where the deleted-tweets media folder holds it, " + "tweet id, edit info, retweet flag and entities.", "author": "@AlexisBrignoni", "creation_date": "2025-06-24", "last_update_date": "2025-06-24", @@ -27,7 +31,9 @@ }, "dmtwitter": { "name": "Twitter DMs", - "description": "Processes direct messages from a twitter return", + "description": "Direct messages from the direct-messages file of a Twitter return, with " + "time, sender and recipient ids, text, conversation id, media URLs and " + "reactions.", "author": "@AlexisBrignoni", "creation_date": "2025-06-25", "last_update_date": "2025-06-25", @@ -40,7 +46,10 @@ }, "deleteddmtwitter": { "name": "Deleted Twitter DMs", - "description": "Processes direct messages from a twitter return", + "description": "Deleted direct messages from the deleted-direct-messages file of a " + "Twitter return, with time, sender and recipient ids, text, the image " + "where the deleted direct messages media folder holds it, conversation id, " + "media URLs and reactions.", "author": "@AlexisBrignoni", "creation_date": "2025-07-01", "last_update_date": "2025-07-01", @@ -53,7 +62,8 @@ }, "blocktwitter": { "name": "Blocked Twitter", - "description": "Processes direct messages from a twitter return", + "description": "Accounts listed in the block file of a Twitter return, with the account " + "id and user link.", "author": "@AlexisBrignoni", "creation_date": "2025-07-02", "last_update_date": "2025-07-02",