Skip to content

Commit e2c38e1

Browse files
dpageclaude
andcommitted
Address remaining CodeRabbit review feedback for compaction module.
- Expand protected set to cover full tool groups, preventing orphaned tool call/result messages when a turn straddles the recent window. - Add input validation in deserialize_history() for non-list/non-dict data. - Strengthen test assertion for preserved recent window tail. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 7083d7a commit e2c38e1

File tree

2 files changed

+23
-6
lines changed

2 files changed

+23
-6
lines changed

web/pgadmin/llm/compaction.py

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,12 @@ def compact_history(
280280
# Find tool groups
281281
tool_groups = _find_tool_pair_indices(messages)
282282

283+
# Expand protected set to include entire tool groups so we never
284+
# split a tool-use turn (leaving orphaned call or result messages).
285+
for i in list(protected):
286+
if i in tool_groups:
287+
protected |= set(tool_groups[i])
288+
283289
# Classify and score all non-protected messages
284290
candidates = []
285291
for i in range(len(messages)):
@@ -361,8 +367,14 @@ def deserialize_history(
361367
Returns:
362368
List of Message objects.
363369
"""
370+
if not isinstance(history_data, list):
371+
return []
372+
364373
messages = []
365374
for item in history_data:
375+
if not isinstance(item, dict):
376+
continue
377+
366378
role_str = item.get('role', '')
367379
content = item.get('content', '')
368380

@@ -373,7 +385,9 @@ def deserialize_history(
373385

374386
# Reconstruct tool calls if present
375387
tool_calls = []
376-
for tc_data in item.get('tool_calls', []):
388+
for tc_data in item.get('tool_calls') or []:
389+
if not isinstance(tc_data, dict):
390+
continue
377391
tool_calls.append(ToolCall(
378392
id=tc_data.get('id', ''),
379393
name=tc_data.get('name', ''),
@@ -383,7 +397,9 @@ def deserialize_history(
383397
# Reconstruct tool results if present
384398
from pgadmin.llm.models import ToolResult
385399
tool_results = []
386-
for tr_data in item.get('tool_results', []):
400+
for tr_data in item.get('tool_results') or []:
401+
if not isinstance(tr_data, dict):
402+
continue
387403
tool_results.append(ToolResult(
388404
tool_call_id=tr_data.get('tool_call_id', ''),
389405
content=tr_data.get('content', ''),

web/pgadmin/llm/tests/test_compaction.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -130,10 +130,11 @@ def test_preserves_first_and_recent(self):
130130
# First message should be preserved
131131
self.assertEqual(result[0].content, 'First message')
132132
# Last 4 messages should be preserved
133-
last_4_original = messages[-4:]
134-
last_4_result = result[-4:]
135-
for orig, res in zip(last_4_original, last_4_result):
136-
self.assertEqual(orig.content, res.content)
133+
self.assertGreaterEqual(len(result), 5)
134+
self.assertEqual(
135+
[m.content for m in result[-4:]],
136+
[m.content for m in messages[-4:]],
137+
)
137138

138139
def test_drops_low_value(self):
139140
"""Low-value messages should be dropped first."""

0 commit comments

Comments
 (0)