Spaces:
Build error
fix(tests): add missing skip decorator and tests for exclude_tools
Browse files## What this PR fixes
1. **CI Python 3.12 failure**: Added skip decorator to `TestLocalBackend`
in `test_memory_system.py` - these tests require hnswlib which is not
available on all CI runners.
2. **Missing test coverage**: Added 6 tests for the `exclude_tools` feature
in `test_content_router.py`. Tests use existing helper functions
`generate_python_code()`, `generate_json_data()`, and
`generate_search_results()` defined at lines 57-95 of the same file.
3. **Anthropic/OpenAI inconsistency**: Fixed `_process_content_blocks()`
to add `router:excluded:tool` marker for Anthropic format, matching
the OpenAI format behavior at line 1157.
4. **Dead code removal**: Removed unused `exclude_tools` field from
`SmartCrusherConfig` - the actual implementation uses
`ContentRouterConfig.exclude_tools` in content_router.py.
AI review: code-reviewer (2 iterations), adversarial-reviewer (2 iterations)
Issues fixed: missing test coverage, format inconsistency, dead code
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
@@ -425,10 +425,6 @@ class SmartCrusherConfig:
|
|
| 425 |
# many items have identical content (e.g., repeated status messages).
|
| 426 |
dedup_identical_items: bool = True
|
| 427 |
|
| 428 |
-
# Tools to exclude from compression (output passed through unmodified)
|
| 429 |
-
# Set to None to use DEFAULT_EXCLUDE_TOOLS, or provide custom set
|
| 430 |
-
exclude_tools: set[str] | None = None
|
| 431 |
-
|
| 432 |
|
| 433 |
@dataclass
|
| 434 |
class CacheOptimizerConfig:
|
|
|
|
| 425 |
# many items have identical content (e.g., repeated status messages).
|
| 426 |
dedup_identical_items: bool = True
|
| 427 |
|
|
|
|
|
|
|
|
|
|
|
|
|
| 428 |
|
| 429 |
@dataclass
|
| 430 |
class CacheOptimizerConfig:
|
|
@@ -1250,6 +1250,7 @@ class ContentRouter(Transform):
|
|
| 1250 |
tool_use_id = block.get("tool_use_id", "")
|
| 1251 |
if tool_use_id in excluded_tool_ids:
|
| 1252 |
new_blocks.append(block)
|
|
|
|
| 1253 |
continue
|
| 1254 |
|
| 1255 |
tool_content = block.get("content", "")
|
|
|
|
| 1250 |
tool_use_id = block.get("tool_use_id", "")
|
| 1251 |
if tool_use_id in excluded_tool_ids:
|
| 1252 |
new_blocks.append(block)
|
| 1253 |
+
transforms_applied.append("router:excluded:tool")
|
| 1254 |
continue
|
| 1255 |
|
| 1256 |
tool_content = block.get("content", "")
|
|
@@ -23,6 +23,14 @@ from typing import Any
|
|
| 23 |
|
| 24 |
import pytest
|
| 25 |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 26 |
from headroom.memory.adapters.graph import InMemoryGraphStore
|
| 27 |
from headroom.memory.adapters.graph_models import (
|
| 28 |
Entity,
|
|
@@ -1370,6 +1378,7 @@ class TestMemoryTools:
|
|
| 1370 |
# =============================================================================
|
| 1371 |
|
| 1372 |
|
|
|
|
| 1373 |
class TestLocalBackend:
|
| 1374 |
"""Integration tests for LocalBackend."""
|
| 1375 |
|
|
|
|
| 23 |
|
| 24 |
import pytest
|
| 25 |
|
| 26 |
+
# Check if hnswlib is available for LocalBackend tests
|
| 27 |
+
try:
|
| 28 |
+
from headroom.memory.adapters.hnsw import _check_hnswlib_available
|
| 29 |
+
|
| 30 |
+
HNSW_AVAILABLE = _check_hnswlib_available()
|
| 31 |
+
except ImportError:
|
| 32 |
+
HNSW_AVAILABLE = False
|
| 33 |
+
|
| 34 |
from headroom.memory.adapters.graph import InMemoryGraphStore
|
| 35 |
from headroom.memory.adapters.graph_models import (
|
| 36 |
Entity,
|
|
|
|
| 1378 |
# =============================================================================
|
| 1379 |
|
| 1380 |
|
| 1381 |
+
@pytest.mark.skipif(not HNSW_AVAILABLE, reason="hnswlib not available")
|
| 1382 |
class TestLocalBackend:
|
| 1383 |
"""Integration tests for LocalBackend."""
|
| 1384 |
|
|
@@ -543,3 +543,251 @@ class TestSummary:
|
|
| 543 |
|
| 544 |
# Should be a string
|
| 545 |
assert summary is not None
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 543 |
|
| 544 |
# Should be a string
|
| 545 |
assert summary is not None
|
| 546 |
+
|
| 547 |
+
|
| 548 |
+
# =============================================================================
|
| 549 |
+
# TestExcludeTools
|
| 550 |
+
# =============================================================================
|
| 551 |
+
|
| 552 |
+
|
| 553 |
+
class TestExcludeTools:
|
| 554 |
+
"""Tests for exclude_tools feature - bypassing compression for specific tools."""
|
| 555 |
+
|
| 556 |
+
@pytest.fixture
|
| 557 |
+
def tokenizer(self):
|
| 558 |
+
"""Get a tokenizer for tests."""
|
| 559 |
+
from headroom.providers import OpenAIProvider
|
| 560 |
+
from headroom.tokenizer import Tokenizer
|
| 561 |
+
|
| 562 |
+
provider = OpenAIProvider()
|
| 563 |
+
token_counter = provider.get_token_counter("gpt-4o")
|
| 564 |
+
return Tokenizer(token_counter, "gpt-4o")
|
| 565 |
+
|
| 566 |
+
def test_default_exclude_tools_uses_defaults(self, tokenizer):
|
| 567 |
+
"""Default config excludes DEFAULT_EXCLUDE_TOOLS (Read, Glob, etc)."""
|
| 568 |
+
config = ContentRouterConfig(min_section_tokens=10)
|
| 569 |
+
router = ContentRouter(config)
|
| 570 |
+
|
| 571 |
+
# Create message with tool call from "Read" tool (should be excluded)
|
| 572 |
+
messages = [
|
| 573 |
+
{
|
| 574 |
+
"role": "assistant",
|
| 575 |
+
"content": None,
|
| 576 |
+
"tool_calls": [
|
| 577 |
+
{
|
| 578 |
+
"id": "call_read_1",
|
| 579 |
+
"type": "function",
|
| 580 |
+
"function": {"name": "Read", "arguments": "{}"},
|
| 581 |
+
}
|
| 582 |
+
],
|
| 583 |
+
},
|
| 584 |
+
{
|
| 585 |
+
"role": "tool",
|
| 586 |
+
"tool_call_id": "call_read_1",
|
| 587 |
+
"content": generate_python_code(20), # Large content that would normally compress
|
| 588 |
+
},
|
| 589 |
+
]
|
| 590 |
+
|
| 591 |
+
result = router.apply(messages, tokenizer)
|
| 592 |
+
|
| 593 |
+
# Content should be unchanged (passed through, not compressed)
|
| 594 |
+
assert result.messages[1]["content"] == messages[1]["content"]
|
| 595 |
+
# Check transform was marked as excluded
|
| 596 |
+
assert "router:excluded:tool" in result.transforms_applied
|
| 597 |
+
|
| 598 |
+
def test_custom_exclude_tools(self, tokenizer):
|
| 599 |
+
"""Custom exclude_tools set is respected."""
|
| 600 |
+
config = ContentRouterConfig(
|
| 601 |
+
min_section_tokens=10,
|
| 602 |
+
exclude_tools={"MyCustomTool"}, # Only exclude this tool
|
| 603 |
+
)
|
| 604 |
+
router = ContentRouter(config)
|
| 605 |
+
|
| 606 |
+
# Create message with MyCustomTool (should be excluded)
|
| 607 |
+
messages = [
|
| 608 |
+
{
|
| 609 |
+
"role": "assistant",
|
| 610 |
+
"content": None,
|
| 611 |
+
"tool_calls": [
|
| 612 |
+
{
|
| 613 |
+
"id": "call_custom_1",
|
| 614 |
+
"type": "function",
|
| 615 |
+
"function": {"name": "MyCustomTool", "arguments": "{}"},
|
| 616 |
+
}
|
| 617 |
+
],
|
| 618 |
+
},
|
| 619 |
+
{
|
| 620 |
+
"role": "tool",
|
| 621 |
+
"tool_call_id": "call_custom_1",
|
| 622 |
+
"content": generate_json_data(50),
|
| 623 |
+
},
|
| 624 |
+
]
|
| 625 |
+
|
| 626 |
+
result = router.apply(messages, tokenizer)
|
| 627 |
+
|
| 628 |
+
# Content should be unchanged
|
| 629 |
+
assert result.messages[1]["content"] == messages[1]["content"]
|
| 630 |
+
assert "router:excluded:tool" in result.transforms_applied
|
| 631 |
+
|
| 632 |
+
def test_non_excluded_tools_are_compressed(self, tokenizer):
|
| 633 |
+
"""Tools not in exclude_tools set are still compressed."""
|
| 634 |
+
config = ContentRouterConfig(
|
| 635 |
+
min_section_tokens=10,
|
| 636 |
+
exclude_tools={"Read"}, # Only exclude Read, not OtherTool
|
| 637 |
+
)
|
| 638 |
+
router = ContentRouter(config)
|
| 639 |
+
|
| 640 |
+
original_content = generate_json_data(100) # Large JSON array
|
| 641 |
+
|
| 642 |
+
messages = [
|
| 643 |
+
{
|
| 644 |
+
"role": "assistant",
|
| 645 |
+
"content": None,
|
| 646 |
+
"tool_calls": [
|
| 647 |
+
{
|
| 648 |
+
"id": "call_other_1",
|
| 649 |
+
"type": "function",
|
| 650 |
+
"function": {"name": "OtherTool", "arguments": "{}"},
|
| 651 |
+
}
|
| 652 |
+
],
|
| 653 |
+
},
|
| 654 |
+
{
|
| 655 |
+
"role": "tool",
|
| 656 |
+
"tool_call_id": "call_other_1",
|
| 657 |
+
"content": original_content,
|
| 658 |
+
},
|
| 659 |
+
]
|
| 660 |
+
|
| 661 |
+
result = router.apply(messages, tokenizer)
|
| 662 |
+
|
| 663 |
+
# Content should be compressed (different from original)
|
| 664 |
+
# Note: Compression may or may not change the content depending on strategy
|
| 665 |
+
# But it should NOT have the excluded marker
|
| 666 |
+
assert "router:excluded:tool" not in result.transforms_applied
|
| 667 |
+
|
| 668 |
+
def test_empty_exclude_tools_compresses_all(self, tokenizer):
|
| 669 |
+
"""Empty exclude_tools set means no tools are excluded."""
|
| 670 |
+
config = ContentRouterConfig(
|
| 671 |
+
min_section_tokens=10,
|
| 672 |
+
exclude_tools=set(), # Empty set - exclude nothing
|
| 673 |
+
)
|
| 674 |
+
router = ContentRouter(config)
|
| 675 |
+
|
| 676 |
+
messages = [
|
| 677 |
+
{
|
| 678 |
+
"role": "assistant",
|
| 679 |
+
"content": None,
|
| 680 |
+
"tool_calls": [
|
| 681 |
+
{
|
| 682 |
+
"id": "call_read_1",
|
| 683 |
+
"type": "function",
|
| 684 |
+
"function": {"name": "Read", "arguments": "{}"},
|
| 685 |
+
}
|
| 686 |
+
],
|
| 687 |
+
},
|
| 688 |
+
{
|
| 689 |
+
"role": "tool",
|
| 690 |
+
"tool_call_id": "call_read_1",
|
| 691 |
+
"content": generate_python_code(20),
|
| 692 |
+
},
|
| 693 |
+
]
|
| 694 |
+
|
| 695 |
+
result = router.apply(messages, tokenizer)
|
| 696 |
+
|
| 697 |
+
# Should NOT be excluded (empty set means compress everything)
|
| 698 |
+
assert "router:excluded:tool" not in result.transforms_applied
|
| 699 |
+
|
| 700 |
+
def test_anthropic_format_tool_result_exclusion(self, tokenizer):
|
| 701 |
+
"""Anthropic format tool_result blocks are also excluded."""
|
| 702 |
+
config = ContentRouterConfig(
|
| 703 |
+
min_section_tokens=10,
|
| 704 |
+
exclude_tools={"Glob"},
|
| 705 |
+
)
|
| 706 |
+
router = ContentRouter(config)
|
| 707 |
+
|
| 708 |
+
# Anthropic format with tool_use and tool_result in content blocks
|
| 709 |
+
messages = [
|
| 710 |
+
{
|
| 711 |
+
"role": "assistant",
|
| 712 |
+
"content": [
|
| 713 |
+
{
|
| 714 |
+
"type": "tool_use",
|
| 715 |
+
"id": "toolu_glob_1",
|
| 716 |
+
"name": "Glob",
|
| 717 |
+
"input": {"pattern": "*.py"},
|
| 718 |
+
}
|
| 719 |
+
],
|
| 720 |
+
},
|
| 721 |
+
{
|
| 722 |
+
"role": "user",
|
| 723 |
+
"content": [
|
| 724 |
+
{
|
| 725 |
+
"type": "tool_result",
|
| 726 |
+
"tool_use_id": "toolu_glob_1",
|
| 727 |
+
"content": generate_search_results(50),
|
| 728 |
+
}
|
| 729 |
+
],
|
| 730 |
+
},
|
| 731 |
+
]
|
| 732 |
+
|
| 733 |
+
result = router.apply(messages, tokenizer)
|
| 734 |
+
|
| 735 |
+
# Find the tool_result block and verify content unchanged
|
| 736 |
+
user_msg = result.messages[1]
|
| 737 |
+
tool_result_block = next(
|
| 738 |
+
(b for b in user_msg["content"] if b.get("type") == "tool_result"), None
|
| 739 |
+
)
|
| 740 |
+
assert tool_result_block is not None
|
| 741 |
+
assert tool_result_block["content"] == messages[1]["content"][0]["content"]
|
| 742 |
+
# Verify exclusion was tracked (consistent with OpenAI format)
|
| 743 |
+
assert "router:excluded:tool" in result.transforms_applied
|
| 744 |
+
|
| 745 |
+
def test_mixed_excluded_and_non_excluded_tools(self, tokenizer):
|
| 746 |
+
"""Multiple tools in same conversation - only excluded ones pass through."""
|
| 747 |
+
config = ContentRouterConfig(
|
| 748 |
+
min_section_tokens=10,
|
| 749 |
+
exclude_tools={"Read"}, # Only exclude Read
|
| 750 |
+
)
|
| 751 |
+
router = ContentRouter(config)
|
| 752 |
+
|
| 753 |
+
read_content = generate_python_code(20)
|
| 754 |
+
other_content = generate_json_data(100)
|
| 755 |
+
|
| 756 |
+
messages = [
|
| 757 |
+
{
|
| 758 |
+
"role": "assistant",
|
| 759 |
+
"content": None,
|
| 760 |
+
"tool_calls": [
|
| 761 |
+
{
|
| 762 |
+
"id": "call_read_1",
|
| 763 |
+
"type": "function",
|
| 764 |
+
"function": {"name": "Read", "arguments": "{}"},
|
| 765 |
+
},
|
| 766 |
+
{
|
| 767 |
+
"id": "call_other_1",
|
| 768 |
+
"type": "function",
|
| 769 |
+
"function": {"name": "OtherTool", "arguments": "{}"},
|
| 770 |
+
},
|
| 771 |
+
],
|
| 772 |
+
},
|
| 773 |
+
{
|
| 774 |
+
"role": "tool",
|
| 775 |
+
"tool_call_id": "call_read_1",
|
| 776 |
+
"content": read_content,
|
| 777 |
+
},
|
| 778 |
+
{
|
| 779 |
+
"role": "tool",
|
| 780 |
+
"tool_call_id": "call_other_1",
|
| 781 |
+
"content": other_content,
|
| 782 |
+
},
|
| 783 |
+
]
|
| 784 |
+
|
| 785 |
+
result = router.apply(messages, tokenizer)
|
| 786 |
+
|
| 787 |
+
# Read tool content should be unchanged (excluded)
|
| 788 |
+
read_result = next(m for m in result.messages if m.get("tool_call_id") == "call_read_1")
|
| 789 |
+
assert read_result["content"] == read_content
|
| 790 |
+
|
| 791 |
+
# OtherTool may or may not be compressed, but should be processed
|
| 792 |
+
# (we just verify it wasn't excluded)
|
| 793 |
+
assert "router:excluded:tool" in result.transforms_applied
|