Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request migrates the application from Copilot SDK to PydanticAI Gateway (PAIG) for agent functionality, introduces multi-tenant support with usage tracking, and converts webhook message processing to asynchronous background task execution. Database schema is extended with tenants and usage_logs tables. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Facebook User
participant Webhook as Webhook Endpoint
participant BgTask as Background Task
participant AgentService as MessengerAgentService
participant Agent as PydanticAI Agent
participant Facebook as Facebook API
participant Repo as Repository/Database
Client->>Webhook: Send message
Webhook->>BgTask: Schedule process_message(background_tasks)
Webhook-->>Client: Return 200 OK
BgTask->>Repo: Fetch bot_configuration & reference_document
BgTask->>AgentService: Build AgentContext with tenant_id, recent_messages
AgentService->>Agent: Construct system_prompt, invoke agent.run()
Agent-->>AgentService: Return AgentResponse (message, confidence, escalation)
AgentService->>Facebook: Send response to user
AgentService->>Repo: Save message to message_history, log usage metrics
Repo-->>AgentService: Success
AgentService-->>BgTask: Complete
sequenceDiagram
participant Service as MessengerAgentService
participant PrimaryAgent as Agent (default_model)
participant FallbackAgent as Agent (fallback_model)
participant Error as Error Handler
Service->>PrimaryAgent: respond(context, user_message)
alt Primary Model Success
PrimaryAgent-->>Service: AgentResponse
else Primary Model Failure
PrimaryAgent->>Error: Exception caught
Service->>FallbackAgent: respond_with_fallback using fallback_model
FallbackAgent-->>Service: AgentResponse or escalation
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
❌ 5 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Prevent Tests Dashboard |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
|
|
||
| # Send response via Facebook | ||
| await send_message( | ||
| page_access_token=bot_config.facebook_page_access_token, |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Pull request overview
This PR migrates from GitHub Copilot SDK to PydanticAI Gateway, introducing structured AI responses, multi-tenant support, and improved observability. The migration replaces the custom CopilotService wrapper with native PydanticAI Gateway integration using the gateway/provider:model syntax.
Changes:
- Replaced CopilotService with PydanticAI Gateway-based MessengerAgentService with structured outputs
- Added multi-tenant database schema with tenants, usage tracking, and tenant_id foreign keys
- Updated configuration to use PydanticAI Gateway API keys and model specifications
- Implemented webhook endpoint for processing Facebook messages with agent responses
- Updated dependencies to pydantic-ai 1.16.0 and added pydantic-ai-slim with logfire integration
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 20 comments.
Show a summary per file
| File | Description |
|---|---|
| pyproject.toml | Updated pydantic-ai version to 1.16.0, added pydantic-ai-slim[logfire], added dependency-groups section |
| uv.lock | Reflects updated dependencies including pydantic-ai 1.16.0 and dev dependency groups |
| src/config.py | Removed Copilot SDK settings, added PydanticAI Gateway API key, default_model, and fallback_model configuration |
| src/services/copilot_service.py | Deleted entire file - no longer needed with PydanticAI Gateway |
| src/services/agent_service.py | Complete rewrite to use PydanticAI Agent with structured outputs (AgentResponse), tools, and fallback model support |
| src/services/reference_doc.py | Updated to use PydanticAI Agent for document synthesis instead of CopilotService |
| src/models/agent_models.py | Enhanced AgentResponse with Field validators, added tenant_id to AgentContext, added should_escalate helper method |
| src/api/webhook.py | Implemented full webhook handler with message processing, agent responses, and Facebook message sending |
| src/main.py | Removed Copilot service initialization, updated app metadata to reflect PAIG usage |
| src/logging_config.py | Updated logfire instrumentation API calls (instrument_fastapi, instrument_pydantic, instrument_pydantic_ai) |
| src/cli/setup_cli.py | Updated to use new build_reference_document function, removed CopilotService dependency |
| migrations/002_multi_tenant.sql | New migration adding tenants table, tenant_id columns, and usage_logs table for multi-tenant SaaS |
| .env.example | Replaced Copilot SDK variables with PydanticAI Gateway configuration |
| tests/conftest.py | Replaced mock_copilot_service with mock_agent_service, made imports conditional for optional dependencies |
| tests/unit/test_config.py | Updated tests to use new PydanticAI Gateway configuration fields |
| tests/unit/test_agent_service.py | Complete rewrite to test new PydanticAI-based agent service with mocked Agent instances |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Instrument PydanticAI for AI observability (pairs with PAIG) | ||
| try: | ||
| logfire.instrument_pydantic_ai() | ||
| except AttributeError: | ||
| # Fallback if instrument_pydantic_ai is not available | ||
| pass |
There was a problem hiding this comment.
The method logfire.instrument_pydantic_ai() may not exist yet in the logfire API. While there's a try/except to handle AttributeError, this suggests uncertainty about the API. Please verify this method exists in the version of logfire being used, or remove this call if it's not yet available.
|
|
||
| ALTER TABLE message_history | ||
| ADD COLUMN IF NOT EXISTS tenant_id uuid REFERENCES tenants(id); | ||
|
|
There was a problem hiding this comment.
The migration file creates foreign key relationships for tenant_id columns, but there's no migration or code to populate existing rows with tenant_id values. Existing bot_configurations, reference_documents, and message_history rows will have NULL tenant_id values after this migration runs. Consider adding a default tenant or providing a data migration strategy for existing rows.
| -- Backfill tenant_id for existing data using a default tenant | |
| -- Create a default tenant if none exists | |
| INSERT INTO tenants (id, name, email, plan) | |
| SELECT gen_random_uuid(), 'Default Tenant', NULL, 'free' | |
| WHERE NOT EXISTS (SELECT 1 FROM tenants); | |
| -- Assign all legacy records with NULL tenant_id to the default tenant | |
| UPDATE bot_configurations | |
| SET tenant_id = ( | |
| SELECT id FROM tenants | |
| WHERE name = 'Default Tenant' | |
| ORDER BY created_at | |
| LIMIT 1 | |
| ) | |
| WHERE tenant_id IS NULL; | |
| UPDATE reference_documents | |
| SET tenant_id = ( | |
| SELECT id FROM tenants | |
| WHERE name = 'Default Tenant' | |
| ORDER BY created_at | |
| LIMIT 1 | |
| ) | |
| WHERE tenant_id IS NULL; | |
| UPDATE message_history | |
| SET tenant_id = ( | |
| SELECT id FROM tenants | |
| WHERE name = 'Default Tenant' | |
| ORDER BY created_at | |
| LIMIT 1 | |
| ) | |
| WHERE tenant_id IS NULL; |
| save_message_history( | ||
| bot_id=bot_config.id, | ||
| sender_id=sender_id, | ||
| message_text=message_text, | ||
| response_text=response.message, | ||
| confidence=response.confidence, | ||
| requires_escalation=response.requires_escalation, | ||
| ) |
There was a problem hiding this comment.
The save_message_history call at the end of process_message happens after the message has already been sent to the user. If saving to the database fails, the message will have been sent but there will be no record of it. Consider either: 1) saving the message history before sending the response, 2) using a transaction that can be rolled back if the save fails, or 3) implementing a queuing system to ensure at-least-once delivery semantics.
| reference_doc_length=len(context.reference_doc), | ||
| recent_messages_count=len(context.recent_messages), | ||
| recent_messages=context.recent_messages, | ||
| tenant_id=getattr(context, 'tenant_id', None), |
There was a problem hiding this comment.
Using getattr with a default of None for tenant_id could hide issues where the tenant_id should be present but isn't. Since AgentContext now has tenant_id as an optional field (line 12 in src/models/agent_models.py), you should be able to access it directly as context.tenant_id without getattr. The current approach could mask bugs where tenant tracking isn't working properly.
| tenant_id=getattr(context, 'tenant_id', None), | |
| tenant_id=context.tenant_id, |
| reference_doc=ref_doc["content"], | ||
| tone=bot_config.tone, | ||
| recent_messages=[], # TODO: Load from message_history | ||
| tenant_id=getattr(bot_config, 'tenant_id', None), |
There was a problem hiding this comment.
Same issue as in agent_service.py - using getattr for tenant_id when it's a defined field on the model. This should be accessed directly as bot_config.tenant_id since multi-tenant support was added to the database schema.
| tenant_id=getattr(bot_config, 'tenant_id', None), | |
| tenant_id=bot_config.tenant_id, |
| agent_service = MessengerAgentService() | ||
| response = await agent_service.respond(context, message_text) |
There was a problem hiding this comment.
A new MessengerAgentService instance is created for every incoming message (line 93). Since the Agent initialization involves setting up the model connection and registering tools, this could be inefficient at scale. Consider implementing a singleton pattern or connection pooling to reuse agent instances across requests, or verify that PydanticAI handles connection pooling internally.
|
|
||
| [dependency-groups] | ||
| dev = [ | ||
| "faker>=40.1.2", | ||
| "hypothesis>=6.151.2", | ||
| "mypy>=1.19.1", | ||
| "pytest>=9.0.2", | ||
| "pytest-asyncio>=1.3.0", | ||
| "pytest-cov>=7.0.0", | ||
| "pytest-mock>=3.15.1", | ||
| "respx>=0.22.0", | ||
| "ruff>=0.14.14", | ||
| ] |
There was a problem hiding this comment.
The [dependency-groups] section is a PEP 735 feature that was introduced in late 2024. While this is valid syntax for tools that support it (like uv), ensure that your deployment environment and all developers are using compatible tooling. Traditional pip/setuptools won't recognize this section, which could cause issues. Consider documenting the requirement for uv or other PEP 735-compatible tools in the README.
| [dependency-groups] | |
| dev = [ | |
| "faker>=40.1.2", | |
| "hypothesis>=6.151.2", | |
| "mypy>=1.19.1", | |
| "pytest>=9.0.2", | |
| "pytest-asyncio>=1.3.0", | |
| "pytest-cov>=7.0.0", | |
| "pytest-mock>=3.15.1", | |
| "respx>=0.22.0", | |
| "ruff>=0.14.14", | |
| ] |
| MockAgent.return_value = mock_agent_instance | ||
|
|
||
| service = MessengerAgentService() | ||
| response = await service.respond(context, "Test message") |
There was a problem hiding this comment.
Variable response is not used.
| response = await service.respond(context, "Test message") | |
| await service.respond(context, "Test message") |
| mock_agent_instance.tool = MagicMock(return_value=lambda f: f) | ||
| MockAgent.return_value = mock_agent_instance | ||
|
|
||
| service = MessengerAgentService(model="gateway/anthropic:claude-3-5-sonnet-latest") |
There was a problem hiding this comment.
Variable service is not used.
|
|
||
| from src.services.agent_service import MessengerAgentService | ||
| from src.models.agent_models import AgentContext, AgentResponse | ||
| from src.services.agent_service import MessengerAgentService, MessengerAgentDeps |
There was a problem hiding this comment.
Import of 'MessengerAgentDeps' is not used.
| from src.services.agent_service import MessengerAgentService, MessengerAgentDeps | |
| from src.services.agent_service import MessengerAgentService |
All 12 todos completed PydanticAI Gateway integration implemented CopilotService removed New agent service with structured outputs Multi-tenant database migration created
jreakin
force-pushed
the
integrate-pydanticai-gateway
branch
from
6fe38e9 to
3cdd245
Compare
| start_time = time.time() | ||
|
|
||
| logfire.info( | ||
| "Processing agent response", | ||
| user_message_length=len(user_message), | ||
| # Build dependencies | ||
| deps = MessengerAgentDeps( | ||
| reference_doc=context.reference_doc, | ||
| tone=context.tone, |
There was a problem hiding this comment.
Bug: The pydantic_ai_gateway_api_key is loaded into settings but never passed to the PydanticAI Agent or set as an environment variable, which will cause authentication failures on every call.
Severity: CRITICAL
Suggested Fix
The API key from the settings object must be provided to the PydanticAI Gateway. This can be achieved by setting os.environ['PYDANTIC_AI_GATEWAY_API_KEY'] with the value from settings during initialization, or by passing the gateway credentials directly to the Agent constructor.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/services/agent_service.py#L38-L43
Potential issue: The `pydantic_ai_gateway_api_key` is loaded from the environment into
the `Settings` object but is never used to configure the PydanticAI `Agent`. The `Agent`
is initialized with a model string that relies on external authentication, but the API
key is not set as an environment variable or passed directly to the agent. This will
cause all calls to the PydanticAI Gateway to fail with authentication errors. As a
result, every webhook message will trigger a failed agent call, causing the system to
return a fallback response and defeating the purpose of the AI integration.
Migrate from GitHub Copilot SDK to PydanticAI Gateway
This PR replaces the GitHub Copilot SDK integration with PydanticAI Gateway for more reliable and structured AI responses. Key changes include:
CopilotServicewith a newMessengerAgentServiceusing PydanticAI GatewayThe new implementation provides better type safety, structured outputs, and multi-model fallback capabilities.
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.