From e067943715007d988e42316c11d10413abc23e15 Mon Sep 17 00:00:00 2001 From: Brigs Date: Sat, 19 Sep 2026 18:06:17 -0400 Subject: [PATCH] Keep a conversation view's data values as the artifact wrote them lava_process_artifact turned every data_views value that matched a column header into that column's SQL name. Two of those keys carry data rather than a column name: LAVA reads directionSentValue and sentMessageStaticLabel as written and resolves every other key to a column. So a direction value of 'Sent' beside a 'Sent' time column reached the manifest as 'sent', no row matched it, and LAVA's conversation view showed every message as received. Those two keys now pass through unchanged, and column keys are sanitized as before. No shipped artifact's manifest changes: none of the 103 conversation views across the five tools uses a sent value or a static label that matches one of its own headers. The new test fails on three of its five cases against the old writer. Co-Authored-By: Claude Opus 5 --- .../test_lava_conversation_view_values.py | 153 ++++++++++++++++++ scripts/lavafuncs.py | 12 +- 2 files changed, 163 insertions(+), 2 deletions(-) create mode 100644 admin/test/scripts/test_lava_conversation_view_values.py diff --git a/admin/test/scripts/test_lava_conversation_view_values.py b/admin/test/scripts/test_lava_conversation_view_values.py new file mode 100644 index 00000000..58e2921d --- /dev/null +++ b/admin/test/scripts/test_lava_conversation_view_values.py @@ -0,0 +1,153 @@ +"""A conversation view's data values must reach the LAVA manifest as the artifact wrote them. + +`lava_process_artifact` turns the keys of a conversation data view that name a column into +that column's SQL name, so `timeColumn: 'Sent'` is stored as `sent`. It used to do that to +every value that matched a header, including the two keys LAVA reads as data rather than as +a column: `directionSentValue` and `sentMessageStaticLabel`. + +An artifact whose direction value is 'Sent' beside a 'Sent' time column therefore reached the +manifest with `directionSentValue: 'sent'`. LAVA classifies a message as sent by comparing the +direction column against that value, no row matched, and every message in the conversation view +was shown as received. Nothing errored, and the row counts were right. +""" +import datetime +import pathlib +import shutil +import sys +import tempfile +import unittest + +REPO_ROOT = pathlib.Path(__file__).resolve().parents[3] +sys.path.insert(0, str(REPO_ROOT)) + +from scripts import lavafuncs # pylint: disable=wrong-import-position +from scripts.context import Context # pylint: disable=wrong-import-position + +# 'Sent' is both the time column and the direction value, and 'Me' is both a column and the +# static label, which is the collision the writer has to get right. +HEADERS = [('Sent', 'datetime'), 'Direction', 'Author', 'Me', 'Body', 'Thread', 'Media'] + +SENT_AT = datetime.datetime(2026, 9, 19, 12, 0, tzinfo=datetime.timezone.utc) + + +class TestConversationViewValues(unittest.TestCase): + """Column keys are sanitized; data values are written as they were given.""" + + def setUp(self): + self.tmpdir = tempfile.mkdtemp() + lavafuncs.initialize_lava(self.tmpdir, self.tmpdir, 'fs') + Context.set_artifact_info({'name': 'Conversation Values', 'description': 'test artifact'}) + # Only the basename is read, so this path never has to exist. Keeping it synthetic + # lets the same test file be shared across the LEAPP tools unchanged. + Context.set_module_file_path( + str(REPO_ROOT / 'scripts' / 'artifacts' / 'conversation_values.py')) + + def tearDown(self): + if lavafuncs.lava_db is not None: + lavafuncs.lava_db.close() + lavafuncs.lava_db = None + lavafuncs.lava_data = None + Context.clear() + shutil.rmtree(self.tmpdir, ignore_errors=True) + + def _process(self, data_views): + table_name, object_columns, column_map = lavafuncs.lava_process_artifact( + 'Testing', 'conversation_values_module', 'Conversation Values', HEADERS, + record_count=2, func_name='conversation_values', data_views=data_views) + artifact = lavafuncs.lava_data['artifacts']['Testing'][0] + return artifact['data_views']['conversation'], table_name, object_columns, column_map + + def test_data_values_survive_matching_a_column_name(self): + view, _, _, _ = self._process({'conversation': { + 'conversationDiscriminatorColumn': 'Thread', + 'textColumn': 'Body', + 'directionColumn': 'Direction', + 'directionSentValue': 'Sent', + 'timeColumn': 'Sent', + 'senderColumn': 'Author', + 'sentMessageStaticLabel': 'Me', + }}) + + self.assertEqual(view['directionSentValue'], 'Sent') + self.assertEqual(view['sentMessageStaticLabel'], 'Me') + # The same text used as a column name is still sanitized. + self.assertEqual(view['timeColumn'], 'sent') + + def test_the_stored_sent_value_matches_the_rows(self): + """The check LAVA makes: direction column against the manifest's sent value.""" + view, table_name, object_columns, column_map = self._process({'conversation': { + 'conversationDiscriminatorColumn': 'Thread', + 'textColumn': 'Body', + 'directionColumn': 'Direction', + 'directionSentValue': 'Sent', + 'timeColumn': 'Sent', + 'senderColumn': 'Author', + }}) + rows = [ + (SENT_AT, 'Sent', 'me', 'me', 'first', 'thread-1', ''), + (SENT_AT, 'Received', 'them', 'me', 'second', 'thread-1', ''), + ] + lavafuncs.lava_insert_sqlite_data(table_name, rows, object_columns, HEADERS, column_map) + + cursor = lavafuncs.lava_db.cursor() + matched = cursor.execute( + f'SELECT COUNT(*) FROM "{table_name}" WHERE "{view["directionColumn"]}" = ?', + (view['directionSentValue'],)).fetchone()[0] + self.assertEqual(matched, 1, 'no row matches the sent value the manifest carries') + + def test_column_keys_are_still_sanitized(self): + view, _, _, _ = self._process({'conversation': { + 'conversationDiscriminatorColumn': 'Thread', + 'conversationLabelColumn': 'Thread', + 'textColumn': 'Body', + 'directionColumn': 'Direction', + 'directionSentValue': 'Outgoing', + 'timeColumn': 'Sent', + 'senderColumn': 'Author', + 'mediaColumn': 'Media', + 'sentMessageLabelColumn': 'Me', + }}) + + self.assertEqual(view, { + 'conversationDiscriminatorColumn': 'thread', + 'conversationLabelColumn': 'thread', + 'textColumn': 'body', + 'directionColumn': 'direction', + 'directionSentValue': 'Outgoing', + 'timeColumn': 'sent', + 'senderColumn': 'author', + 'mediaColumn': 'media', + 'sentMessageLabelColumn': 'me', + }) + + def test_a_value_that_names_no_column_is_untouched(self): + view, _, _, _ = self._process({'conversation': { + 'conversationDiscriminatorColumn': 'Thread', + 'textColumn': 'Body', + 'directionColumn': 'Direction', + 'directionSentValue': 'Outgoing', + 'timeColumn': 'Sent', + 'senderColumn': 'Author', + }}) + + self.assertEqual(view['directionSentValue'], 'Outgoing') + + def test_legacy_chat_view_keys_are_renamed_and_the_value_kept(self): + """The deprecated 'chat' view is upgraded to 'conversation' on the way through.""" + view, _, _, _ = self._process({'chat': { + 'threadDiscriminatorColumn': 'Thread', + 'threadLabelColumn': 'Thread', + 'textColumn': 'Body', + 'directionColumn': 'Direction', + 'directionSentValue': 'Sent', + 'timeColumn': 'Sent', + 'senderColumn': 'Author', + }}) + + self.assertEqual(view['conversationDiscriminatorColumn'], 'thread') + self.assertEqual(view['conversationLabelColumn'], 'thread') + self.assertEqual(view['directionSentValue'], 'Sent') + + +if __name__ == '__main__': + unittest.main() diff --git a/scripts/lavafuncs.py b/scripts/lavafuncs.py index c33eeb35..6d3dcea9 100644 --- a/scripts/lavafuncs.py +++ b/scripts/lavafuncs.py @@ -204,6 +204,13 @@ def initialize_lava(input_path, output_path, input_type, profile_filename=None): LEFT JOIN _lava_media_items as lmi ON lmr.media_item_id = lmi.id''') +# Conversation view keys whose value is data rather than a column name. LAVA reads these two +# as written and resolves every other key to a column, so the writer must not turn them into +# a column's SQL name when the value happens to match a header, as a 'Sent' direction value +# does beside a 'Sent' time column. +CONVERSATION_VALUE_KEYS = ('directionSentValue', 'sentMessageStaticLabel') + + def lava_process_artifact( category, module_name, @@ -309,8 +316,9 @@ def lava_process_artifact( # Remap old keys to new keys final_key = convert_map.get(key, key) - # Sanitize value if it's a column name, otherwise pass through - if value in column_names: + # Sanitize value if it's a column name, otherwise pass through. A value key + # carries data, so it passes through even when it equals a column name. + if final_key not in CONVERSATION_VALUE_KEYS and value in column_names: sanitized_params[final_key] = sanitize_sql_name(value) else: sanitized_params[final_key] = value