Refactor web_search to reuse agent instance instead of recreating on each invocation

[Copilot]

Fix Web Search Agent Resource Management

This PR addresses all feedback from the initial implementation, fixing the resource leak issue by implementing the proper persistent agent pattern using the Agent Framework's recommended approach.

Problem

The original web_search() function created a new Azure AI agent on every call. While using async context managers, this pattern was inefficient and didn't properly leverage the Agent Framework's persistent agent capabilities.

Solution

Implemented persistent agent pattern following Agent Framework best practices:

1. Agent Created Once: First call creates agent with store=True on Azure AI service
2. ID Cached: Only agent ID stored at module level (lightweight, no resource leak)
3. Agent Reused: Subsequent calls retrieve agent by ID using proper context managers
4. Proper Cleanup: Each client connection uses async context manager for cleanup

Changes

Core Implementation

• src/spec_to_agents/tools/bing_search.py

  • Added _get_or_create_web_search_agent_id() helper for agent lifecycle management
  • Refactored web_search() to use persistent agent pattern
  • Added comprehensive module docstring explaining architecture
  • Added optional cleanup_web_search_agent() for production deployments

• src/spec_to_agents/utils/clients.py

  • Added agent_id parameter to create_agent_client() for agent retrieval
  • Updated documentation with examples for both creating and retrieving agents

Testing

• tests/test_tools_bing_search.py
  • Added reset_web_search_agent autouse fixture for test isolation
  • Updated all tests to work with persistent agent pattern
  • Added test_web_search_agent_persistence to verify ID caching
  • Added test_web_search_full_lifecycle integration test verifying:
    • Agent created once on first call
    • Agent ID stored and reused
    • Context managers properly balanced (no leaks)

Dependencies

• pyproject.toml
  • Added ruff>=0.14.3 to dev dependencies

Before/After

Before (Resource Inefficiency)

async def web_search(query: str) -> str:
    async with create_agent_client() as client:
        agent = client.create_agent(...)  # ← Created every time
        response = await agent.run(query)
        return response.text
    # Agent deleted on exit

After (Persistent Agent Pattern)

_web_search_agent_id: str | None = None

async def _get_or_create_web_search_agent_id() -> str:
    if _web_search_agent_id is None:
        async with create_agent_client() as client:
            agent = client.create_agent(..., store=True)
            _web_search_agent_id = agent.id  # ← Cache ID
    return _web_search_agent_id

async def web_search(query: str) -> str:
    agent_id = await _get_or_create_web_search_agent_id()
    async with create_agent_client() as client:
        agent = client.create_agent(agent_id=agent_id, ...)  # ← Retrieve by ID
        response = await agent.run(query)
        return response.text

Addresses Original Feedback

✅ Resource Leak Fixed: Agent created once, reused by ID (no repeated creation)
✅ Async Context Manager Pattern: Properly used for client cleanup
✅ Test Inconsistencies Fixed: Autouse fixture prevents state leakage
✅ Import Style Fixed: Single module-level import statement
✅ Comprehensive Documentation: Architecture and production deployment guidance

Verification

• ✅ All 9 tests passing (100% test success rate)
• ✅ Linting: ruff check passes
• ✅ Formatting: ruff format --check passes
• ✅ Type checking: mypy passes with no errors
• ✅ Integration test verifies context manager cleanup (enter/exit balanced)

Test Coverage

tests/test_tools_bing_search.py::test_web_search_success PASSED          [ 11%]
tests/test_tools_bing_search.py::test_web_search_no_results PASSED       [ 22%]
tests/test_tools_bing_search.py::test_web_search_with_custom_count PASSED [ 33%]
tests/test_tools_bing_search.py::test_web_search_api_error PASSED        [ 44%]
tests/test_tools_bing_search.py::test_web_search_formatting PASSED       [ 55%]
tests/test_tools_bing_search.py::test_web_search_empty_results_list PASSED [ 66%]
tests/test_tools_bing_search.py::test_web_search_result_numbering PASSED [ 77%]
tests/test_tools_bing_search.py::test_web_search_agent_persistence PASSED [ 88%]
tests/test_tools_bing_search.py::test_web_search_full_lifecycle PASSED   [100%]

Production Deployment

The implementation includes an optional cleanup function for production:

import atexit
import asyncio

def shutdown():
    asyncio.run(cleanup_web_search_agent())

atexit.register(shutdown)

Notes

• Maintains full backward compatibility - same public API and behavior
• Follows Agent Framework persistent agent pattern as documented
• No breaking changes to existing functionality

---

Fixes #53 (Refactor web_search to reuse agent instance as function tool)

[Copilot]

Pull Request Overview

This PR refactors the Bing web search tool to use a singleton pattern for agent reuse instead of creating a new agent for each search query. This improves efficiency by caching the agent instance at module level.

Key changes:

• Introduced module-level agent caching with _web_search_agent and _agent_client variables
• Added _get_web_search_agent() function to implement singleton pattern
• Updated web_search() to use cached agent instead of creating new instances

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
src/spec_to_agents/tools/bing_search.py	Implemented singleton pattern for web search agent with module-level caching
tests/test_tools_bing_search.py	Added fixture to reset agent cache between tests and new test for agent reuse behavior

[Copilot]

The create_agent_client() is designed to be used as an async context manager for automatic cleanup (as documented in clients.py lines 42-44). By storing the client at module level without using the context manager, agents created with this client will never be cleaned up, leading to resource leaks. The original implementation correctly used async with create_agent_client() as client: to ensure cleanup. Consider either: (1) reverting to the per-call pattern with proper cleanup, or (2) if caching is necessary for performance, implement a proper cleanup mechanism (e.g., using atexit or providing an explicit cleanup function).

[Copilot]

The comment says "Explicitly disable the fixture" but the code doesn't actually disable the autouse=True fixture. The fixture will still run and reset the cache before this test. The manual reset on lines 300-301 is redundant with the fixture. Either remove the comment and manual reset (relying on the fixture), or if you need to test caching across multiple calls within a single test, clarify that the comment is explaining why manual reset is done at the start.

Suggested change

	# Explicitly disable the fixture for this test to verify caching
	import spec_to_agents.tools.bing_search as bing_search_module
	bing_search_module._web_search_agent = None
	bing_search_module._agent_client = None

[Copilot]

Module 'spec_to_agents.tools.bing_search' is imported with both 'import' and 'import from'.

[Copilot]

Module 'spec_to_agents.tools.bing_search' is imported with both 'import' and 'import from'.

[lavaman131]

Comprehensive Fix Applied

I've addressed all the code review feedback and implemented a complete solution following the Agent Framework's persistent agent pattern.

What Changed

Replaced the original implementation with a properly architected solution that addresses all feedback points:

1. ✅ Fixed Resource Management: Properly uses async context managers for clients while keeping agents persistent
2. ✅ Correct API Usage: Agent created once with store=True, retrieved by ID on subsequent calls
3. ✅ Fixed Test Issues: Added autouse fixture, removed redundant imports, consistent mock patterns
4. ✅ Added Integration Tests: Comprehensive lifecycle test verifying context manager cleanup
5. ✅ Production-Ready Documentation: Module docstring + optional cleanup function with atexit example

Implementation Details

Persistent Agent Pattern:

• Agent created once on Azure AI service (store=True)
• Only agent ID cached at module level (lightweight, no leak)
• Each call uses fresh context manager for client (proper cleanup)
• Agent persists on service side between calls

Quality Verification:

• All 9 tests passing
• ruff check - no issues
• ruff format --check - compliant
• mypy - no type errors

Key Files Changed

• src/spec_to_agents/tools/bing_search.py - Complete refactor with persistent pattern
• src/spec_to_agents/utils/clients.py - Added agent_id parameter support
• tests/test_tools_bing_search.py - Updated all tests + new lifecycle test
• pyproject.toml - Added ruff dependency

Ready for review! 🚀