Skip to content

Commit 100e490

Browse files
andrewschenckandrewschenck
committed
ragl.manager, ragl.ragstore logging update
1 parent 013d14f commit 100e490

File tree

3 files changed

+54
-36
lines changed

3 files changed

+54
-36
lines changed

‎ragl/manager.py‎

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,7 @@ def __init__(
243243
self.split = split
244244
self.timestamp = batch_timestamp or int(time.time_ns() // 1000)
245245
self.text_index = text_index
246+
_LOG.info('Initialized %s', self)
246247

247248
def chunk_text_unit(self, unit: TextUnit) -> Iterator[TextUnit]:
248249
"""
@@ -263,6 +264,7 @@ def chunk_text_unit(self, unit: TextUnit) -> Iterator[TextUnit]:
263264
Yields:
264265
Chunked TextUnit instances.
265266
"""
267+
_LOG.debug('Chunking TextUnit: %d characters', len(unit))
266268
if not self.split:
267269
# Create a copy with proper chunk_position and text_id
268270
text_id = (f'{TEXT_ID_PREFIX}{unit.parent_id}-'
@@ -469,6 +471,7 @@ def __init__(
469471
self.min_chunk_size = config.min_chunk_size
470472
self.paranoid = config.paranoid
471473
self._metrics = defaultdict(RAGTelemetry)
474+
_LOG.info('Initialized %s', self)
472475

473476
def add_text(
474477
self,
@@ -503,7 +506,7 @@ def add_text(
503506
Returns:
504507
List of stored TextUnit instances.
505508
"""
506-
_LOG.debug('Adding text: %s', text_or_unit)
509+
_LOG.debug('Adding text: %d characters', len(text_or_unit))
507510

508511
with self.track_operation('add_text'):
509512
return self.add_texts(
@@ -537,18 +540,11 @@ def add_texts(
537540
split:
538541
Whether to split the text into chunks.
539542
540-
Raises:
541-
ValidationError:
542-
If texts are empty or params invalid.
543-
DataError:
544-
If no chunks are stored.
545-
546543
Returns:
547544
List of stored TextUnit instances.
548545
"""
546+
_LOG.debug('Adding texts: %d items', len(texts_or_units))
549547
with self.track_operation('add_texts'):
550-
_LOG.debug('Adding texts: %d items', len(texts_or_units))
551-
552548
self._validate_text_input(texts_or_units)
553549

554550
_chunk_size, _overlap = self._resolve_chunk_params(
@@ -801,8 +797,8 @@ def track_operation(self, operation_name: str) -> Iterator[None]:
801797
duration = time.time() - start
802798
record_failure = self._metrics[operation_name].record_failure
803799
record_failure(duration)
804-
_LOG.critical('Operation failed: %s (%.3fs) - %s', operation_name,
805-
duration, e)
800+
_LOG.error('Operation failed: %s (%.3fs) - %s', operation_name,
801+
duration, e)
806802
raise
807803

808804
def _convert_to_text_unit(self, item: str | TextUnit) -> TextUnit:
@@ -1014,7 +1010,7 @@ def _sanitize_text(self, text: str) -> str:
10141010
limit = self.MAX_INPUT_LENGTH
10151011
if len(text.encode('utf-8')) > limit:
10161012
msg = 'text too long'
1017-
_LOG.critical(msg)
1013+
_LOG.error(msg)
10181014
raise ValidationError(msg)
10191015

10201016
if self.paranoid:
@@ -1077,15 +1073,15 @@ def _validate_chunking(chunk_size: int, overlap: int) -> None:
10771073

10781074
if cs <= 0:
10791075
msg = 'Chunk_size must be positive'
1080-
_LOG.critical(msg)
1076+
_LOG.error(msg)
10811077
raise ValidationError(msg)
10821078
if ov < 0:
10831079
msg = 'Overlap must be non-negative'
1084-
_LOG.critical(msg)
1080+
_LOG.error(msg)
10851081
raise ValidationError(msg)
10861082
if ov >= cs:
10871083
msg = 'Overlap must be less than chunk_size'
1088-
_LOG.critical(msg)
1084+
_LOG.error(msg)
10891085
raise ValidationError(msg)
10901086

10911087
def _validate_query(self, query: str) -> None:
@@ -1108,12 +1104,12 @@ def _validate_query(self, query: str) -> None:
11081104
_LOG.debug('Validating query')
11091105
if not query or not query.strip():
11101106
msg = 'Query cannot be empty'
1111-
_LOG.critical(msg)
1107+
_LOG.error(msg)
11121108
raise ValidationError(msg)
11131109

11141110
if len(query) > self.MAX_QUERY_LENGTH:
11151111
msg = f'Query too long: {len(query)} > {self.MAX_QUERY_LENGTH}'
1116-
_LOG.critical(msg)
1112+
_LOG.error(msg)
11171113
raise ValidationError(msg)
11181114

11191115
@staticmethod
@@ -1149,7 +1145,7 @@ def _validate_top_k(top_k: int) -> None:
11491145
_LOG.debug('Validating top_k parameter')
11501146
if not isinstance(top_k, int) or top_k < 1:
11511147
msg = 'top_k must be a positive integer'
1152-
_LOG.critical(msg)
1148+
_LOG.error(msg)
11531149
raise ValidationError(msg)
11541150

11551151
def __str__(self) -> str:

‎ragl/ragstore.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,8 @@ def __init__(
7474
store and retrieval.
7575
7676
Raises:
77-
TypeError: If args don’t implement protocols.
77+
TypeError:
78+
If args don’t implement protocols.
7879
"""
7980
if not isinstance(embedder, EmbedderProtocol):
8081
msg = 'embedder must implement EmbedderProtocol'
@@ -86,6 +87,7 @@ def __init__(
8687
raise TypeError(msg)
8788
self.embedder = embedder
8889
self.storage = storage
90+
_LOG.info('Initialized %s', self)
8991

9092
def clear(self) -> None:
9193
"""Clear all data from store."""

‎tests/functional/ragl/test_manager.py‎

Lines changed: 37 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -657,7 +657,7 @@ def test_add_text_string_success(self, mock_log):
657657
self.assertEqual(len(result), 1)
658658
self.assertIsInstance(result[0], TextUnit)
659659
self.mock_ragstore.store_texts.assert_called_once()
660-
mock_log.debug.assert_any_call('Adding text: %s', text)
660+
mock_log.debug.assert_any_call('Adding text: %d characters', 19)
661661

662662
@patch('ragl.manager._LOG')
663663
def test_add_text_textunit_success(self, mock_log):
@@ -681,7 +681,8 @@ def test_add_text_textunit_success(self, mock_log):
681681

682682
self.assertEqual(len(result), 1)
683683
self.assertIsInstance(result[0], TextUnit)
684-
mock_log.debug.assert_any_call('Adding text: %s', text_unit)
684+
mock_log.debug.assert_any_call('Adding text: %d characters',
685+
len(text_unit))
685686

686687
def test_add_text_empty_string(self):
687688
"""Test adding empty text raises ValidationError."""
@@ -693,8 +694,8 @@ def test_add_text_empty_string(self):
693694
manager.add_text("")
694695

695696
self.assertIn('text cannot be whitespace-only or zero-length', str(cm.exception))
696-
mock_log.critical.assert_called()
697-
call_args = mock_log.critical.call_args
697+
mock_log.error.assert_called()
698+
call_args = mock_log.error.call_args
698699
self.assertEqual(call_args[0][0],
699700
'Operation failed: %s (%.3fs) - %s')
700701
self.assertEqual(call_args[0][1], 'add_text')
@@ -753,7 +754,7 @@ def test_add_text_no_valid_chunks(self):
753754
manager.add_text(text)
754755

755756
self.assertIn('No valid chunks stored', str(cm.exception))
756-
call_args = mock_log.critical.call_args
757+
call_args = mock_log.error.call_args
757758
self.assertEqual(call_args[0][0],
758759
'Operation failed: %s (%.3fs) - %s')
759760
self.assertEqual(call_args[0][1], 'add_text')
@@ -862,7 +863,17 @@ def test_add_texts_empty_list(self):
862863
manager.add_texts([])
863864

864865
self.assertIn('texts_or_units cannot be empty', str(cm.exception))
865-
mock_log.error.assert_called_with('texts_or_units cannot be empty')
866+
867+
# Check that the ValidationError is logged within the operation failure
868+
mock_log.error.assert_called()
869+
call_args = mock_log.error.call_args
870+
self.assertEqual(call_args[0][0],
871+
'Operation failed: %s (%.3fs) - %s')
872+
self.assertEqual(call_args[0][1], 'add_texts')
873+
self.assertIsInstance(call_args[0][2], float) # execution time
874+
self.assertIsInstance(call_args[0][3], ValidationError)
875+
self.assertIn('texts_or_units cannot be empty',
876+
str(call_args[0][3]))
866877

867878
def test_add_texts_contains_empty_string(self):
868879
"""Test adding texts with empty string raises ValidationError."""
@@ -893,8 +904,17 @@ def test_add_texts_contains_invalid_type(self):
893904

894905
self.assertIn('Invalid text type, must be str or TextUnit',
895906
str(cm.exception))
896-
mock_log.error.assert_called_with(
897-
'Invalid text type, must be str or TextUnit')
907+
908+
# Check that the ValidationError is logged within the operation failure
909+
mock_log.error.assert_called()
910+
call_args = mock_log.error.call_args
911+
self.assertEqual(call_args[0][0],
912+
'Operation failed: %s (%.3fs) - %s')
913+
self.assertEqual(call_args[0][1], 'add_texts')
914+
self.assertIsInstance(call_args[0][2], float) # execution time
915+
self.assertIsInstance(call_args[0][3], ValidationError)
916+
self.assertIn('Invalid text type, must be str or TextUnit',
917+
str(call_args[0][3]))
898918

899919
def test_add_texts_custom_chunk_params(self):
900920
"""Test adding texts with custom chunk parameters."""
@@ -1715,7 +1735,7 @@ def test_get_context_query_too_long(self):
17151735

17161736
expected_msg = f'Query too long: {len(long_query)} > {RAGManager.MAX_QUERY_LENGTH}'
17171737
self.assertIn('Query too long', str(cm.exception))
1718-
call_args = mock_log.critical.call_args
1738+
call_args = mock_log.error.call_args
17191739
self.assertEqual(call_args[0][0],
17201740
'Operation failed: %s (%.3fs) - %s')
17211741
self.assertEqual(call_args[0][1], 'get_context')
@@ -1733,7 +1753,7 @@ def test_get_context_invalid_top_k(self):
17331753

17341754
self.assertIn('top_k must be a positive integer',
17351755
str(cm.exception))
1736-
call_args = mock_log.critical.call_args
1756+
call_args = mock_log.error.call_args
17371757
self.assertEqual(call_args[0][0],
17381758
'Operation failed: %s (%.3fs) - %s')
17391759
self.assertEqual(call_args[0][1], 'get_context')
@@ -1956,7 +1976,7 @@ def test_sanitize_text_input_too_long(self):
19561976
manager._sanitize_text(long_text)
19571977

19581978
self.assertIn('text too long', str(cm.exception))
1959-
mock_log.critical.assert_called_with('text too long')
1979+
mock_log.error.assert_called_with('text too long')
19601980

19611981
def test_sanitize_text_input_paranoid_mode(self):
19621982
"""Test sanitizing text input in paranoid mode."""
@@ -2011,7 +2031,7 @@ def test_validate_chunking_zero_chunk_size(self):
20112031
RAGManager._validate_chunking(0, 20)
20122032

20132033
self.assertIn('Chunk_size must be positive', str(cm.exception))
2014-
mock_log.critical.assert_called_with('Chunk_size must be positive')
2034+
mock_log.error.assert_called_with('Chunk_size must be positive')
20152035

20162036
def test_validate_chunking_negative_chunk_size(self):
20172037
"""Test chunking validation with negative chunk size."""
@@ -2025,7 +2045,7 @@ def test_validate_chunking_negative_overlap(self):
20252045
RAGManager._validate_chunking(100, -5)
20262046

20272047
self.assertIn('Overlap must be non-negative', str(cm.exception))
2028-
mock_log.critical.assert_called_with(
2048+
mock_log.error.assert_called_with(
20292049
'Overlap must be non-negative')
20302050

20312051
def test_validate_chunking_overlap_too_large(self):
@@ -2036,7 +2056,7 @@ def test_validate_chunking_overlap_too_large(self):
20362056

20372057
self.assertIn('Overlap must be less than chunk_size',
20382058
str(cm.exception))
2039-
mock_log.critical.assert_called_with(
2059+
mock_log.error.assert_called_with(
20402060
'Overlap must be less than chunk_size')
20412061

20422062
def test_validate_chunking_overlap_greater_than_chunk_size(self):
@@ -2064,7 +2084,7 @@ def test_validate_query_none(self):
20642084
manager._validate_query(None)
20652085

20662086
self.assertIn('Query cannot be empty', str(cm.exception))
2067-
mock_log.critical.assert_called_with('Query cannot be empty')
2087+
mock_log.error.assert_called_with('Query cannot be empty')
20682088

20692089
def test_validate_query_empty(self):
20702090
"""Test query validation with empty query."""
@@ -2094,7 +2114,7 @@ def test_validate_query_too_long(self):
20942114

20952115
expected_msg = f'Query too long: {len(long_query)} > {RAGManager.MAX_QUERY_LENGTH}'
20962116
self.assertIn('Query too long', str(cm.exception))
2097-
mock_log.critical.assert_called_with(expected_msg)
2117+
mock_log.error.assert_called_with(expected_msg)
20982118

20992119
@patch('ragl.manager._LOG')
21002120
def test_validate_top_k_valid(self, mock_log):
@@ -2110,7 +2130,7 @@ def test_validate_top_k_zero(self):
21102130

21112131
self.assertIn('top_k must be a positive integer',
21122132
str(cm.exception))
2113-
mock_log.critical.assert_called_with(
2133+
mock_log.error.assert_called_with(
21142134
'top_k must be a positive integer')
21152135

21162136
def test_validate_top_k_negative(self):

0 commit comments

Comments
 (0)