-
Notifications
You must be signed in to change notification settings - Fork 135
update #60
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
update #60
Conversation
- Add hindsight_agent.py: Main agent using Foundry Responses API with client-side tool execution - Add hindsight_agent_api.py: FastAPI wrapper for remote HTTP access - Add hindsight_client.py: Optimized memory API client with connection pooling - Add config.py: Centralized configuration with Azure App Configuration support - Add Dockerfile.agent-api: Container definition for Azure Container Apps - Add deploy-bicep.ps1: Deployment script with Bicep infrastructure - Add infra/agent-api.bicep: Infrastructure as Code for Container Apps - Update AGENTS.md: Comprehensive operations manual with Azure deployment details
… temperature/top_p
…ipts - Replace curated hindsight-tools-openapi.json with full hindsight-tools-openapi-full.json - Consolidate agent creation to single create_agent_full_spec.py script - Remove old update_agent_openapi.py (superseded) - Update AGENTS.md with new file structure and changelog - Agent Hindsight-v3:4 now has complete API access including all admin functions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds comprehensive Azure AI Foundry agent infrastructure and deployment automation for the Hindsight memory system. The changes introduce a new FastAPI-based agent API that can be deployed to Azure Container Apps, along with supporting infrastructure, client libraries, and extensive documentation.
Key changes:
- New remote agent API with FastAPI endpoints for chat and health checks
- Complete Azure infrastructure as code using Bicep templates
- Optimized Hindsight client with connection pooling and retry logic
- PowerShell deployment automation script
- Comprehensive operations documentation in AGENTS.md
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 26 comments.
Show a summary per file
| File | Description |
|---|---|
| requirements-agent-api.txt | Python dependencies for the agent API service |
| infra/agent-api.bicep | Infrastructure as Code defining Container Apps, ACR, Log Analytics, and RBAC |
| hindsight_client.py | Optimized HTTP client for Hindsight Memory API with connection pooling |
| hindsight_agent_api.py | FastAPI wrapper providing remote HTTP access to the agent |
| hindsight_agent.py | Main agent implementation using Foundry Responses API |
| hindsight-tools-openapi-full.json | Complete OpenAPI specification (24 endpoints) for agent tools |
| deploy-bicep.ps1 | PowerShell deployment automation script |
| create_agent_full_spec.py | Script to create/update agent with full OpenAPI spec |
| config.py | Centralized configuration with Azure App Configuration support |
| Dockerfile.agent-api | Multi-stage Docker build for agent API container |
| hindsight-api/hindsight_api/api/mcp.py | Added reflect tool to MCP server |
| AGENTS.md | Comprehensive 593-line operations manual covering deployment, auth, and troubleshooting |
| .env.example | Updated default model from o3-mini to gpt-5.2 |
| .dockerignore | Added exclusions for PDFs and documentation files |
Comments suppressed due to low confidence (1)
infra/agent-api.bicep:84
- CORS is configured to allow all origins ('*'), methods, and headers which creates a security vulnerability in production. This permissive configuration allows any website to make requests to your API, potentially enabling CSRF attacks and unauthorized access. For production deployments, configure specific allowed origins based on your actual client applications, and restrict methods and headers to only what's necessary.
corsPolicy: {
allowedOrigins: ['*']
allowedMethods: ['GET', 'POST', 'OPTIONS']
allowedHeaders: ['*']
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Assign RBAC to AI Project (cross-resource-group) | ||
| Write-Host "`nAssigning RBAC to AI Project..." -ForegroundColor Yellow | ||
| $subscriptionId = $account.id | ||
| $aiResourceId = "/subscriptions/$subscriptionId/resourceGroups/jacob-1216-resource/providers/Microsoft.CognitiveServices/accounts/jacob-1216-resource" | ||
|
|
||
| az role assignment create --assignee $principalId --role "Cognitive Services User" --scope $aiResourceId --output none 2>$null |
Copilot
AI
Dec 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded Azure resource URLs and IDs in deployment script reduce reusability. The AI resource ID at line 82 is hardcoded with a specific subscription ID and resource group 'jacob-1216-resource'. This couples the deployment script to a specific environment. Consider making these values parameters or loading them from configuration, which would allow the script to be used across different subscriptions and resource groups.
infra/agent-api.bicep
Outdated
| } | ||
| ] | ||
| scale: { | ||
| minReplicas: 0 |
Copilot
AI
Dec 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minimum replica count of 0 allows the container to scale to zero, which can cause cold start latency issues for the first request after being idle. When scaled to zero, the first user request will experience significant delay (10-30+ seconds) while the container starts up. For production workloads requiring consistent response times, consider setting minReplicas to 1 to maintain at least one warm instance.
| minReplicas: 0 | |
| minReplicas: 1 |
hindsight_client.py
Outdated
|
|
||
| # Default timeouts | ||
| self.default_timeout = 30 | ||
| self.reflect_timeout = 60 # Reflect operations take longer |
Copilot
AI
Dec 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reflect timeout is set to 60 seconds which may be insufficient for complex operations. The reflect operation is noted as taking longer (line 87), but 60 seconds might still timeout for complex queries involving large memory banks or extensive LLM processing. Consider making this timeout configurable or increasing it to 120-180 seconds based on observed behavior in production.
.env.example
Outdated
| HINDSIGHT_API_LLM_PROVIDER=openai | ||
| HINDSIGHT_API_LLM_API_KEY=your-api-key-here | ||
| HINDSIGHT_API_LLM_MODEL=o3-mini | ||
| HINDSIGHT_API_LLM_MODEL=gpt-5.2 |
Copilot
AI
Dec 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Model name updated to 'gpt-5.2' but documentation indicates 'gpt-5.2-chat'. The model name has been changed from 'o3-mini' to 'gpt-5.2', but according to AGENTS.md line 50, the actual deployment name is 'gpt-5.2-chat'. This inconsistency may cause configuration issues. Verify whether the deployment accepts 'gpt-5.2' or requires the full 'gpt-5.2-chat' name.
| HINDSIGHT_API_LLM_MODEL=gpt-5.2 | |
| HINDSIGHT_API_LLM_MODEL=gpt-5.2-chat |
| max_iterations = 10 | ||
| iteration = 0 |
Copilot
AI
Dec 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The maximum iteration limit of 10 for tool call loops may be insufficient for complex multi-step reasoning. If the agent needs to perform multiple recalls, retains, and reflects in sequence, it could hit this limit before completing the task. Consider either increasing this limit to 15-20 or making it configurable, and add logging when the limit is reached so you can detect if it's causing issues in production.
config.py
Outdated
| # Try to import Azure App Configuration provider | ||
| try: | ||
| from azure.appconfiguration.provider import load | ||
| from azure.identity import DefaultAzureCredential |
Copilot
AI
Dec 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'DefaultAzureCredential' is not used.
| from azure.identity import DefaultAzureCredential |
hindsight_agent.py
Outdated
| from azure.ai.projects import AIProjectClient | ||
| from azure.identity import AzureCliCredential, ManagedIdentityCredential, ChainedTokenCredential | ||
| from config import get_config | ||
| from hindsight_client import HindsightClient, MemoryBanks |
Copilot
AI
Dec 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'MemoryBanks' is not used.
| from hindsight_client import HindsightClient, MemoryBanks | |
| from hindsight_client import HindsightClient |
hindsight_agent_api.py
Outdated
| from typing import Optional | ||
| from contextlib import asynccontextmanager | ||
|
|
||
| from fastapi import FastAPI, HTTPException, Depends |
Copilot
AI
Dec 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'Depends' is not used.
| from fastapi import FastAPI, HTTPException, Depends | |
| from fastapi import FastAPI, HTTPException |
|
|
||
| import json | ||
| import logging | ||
| import time |
Copilot
AI
Dec 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'time' is not used.
| import time |
infra/agent-api.bicep
Outdated
| external: true | ||
| targetPort: 8080 | ||
| transport: 'http' | ||
| corsPolicy: { | ||
| allowedOrigins: ['*'] |
Copilot
AI
Dec 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The container app is configured with ingress.external: true and a permissive corsPolicy while hindsight_agent_api.py exposes /chat and memory operations without any authentication or authorization. This effectively publishes the agent and its default memory bank (hindsight_agent_bank) to the public internet, allowing any unauthenticated caller to POST to /chat and read or mutate stored memories as documented in AGENTS.md. Protect this by enforcing strong authn/z on the agent endpoints (e.g., API keys or tokens with per-user bank isolation), or by restricting ingress to private networks and tightening corsPolicy.allowedOrigins to only trusted origins.
| external: true | |
| targetPort: 8080 | |
| transport: 'http' | |
| corsPolicy: { | |
| allowedOrigins: ['*'] | |
| external: false | |
| targetPort: 8080 | |
| transport: 'http' | |
| corsPolicy: { | |
| allowedOrigins: [hindsightApiUrl] |
nicoloboschi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution!
it's not clear to me what the all the new files in the root dir are meant for. we need tp move them in a dedicated directory/package anyway.
Can you add more details on this implementation?
| return json.dumps({"error": str(e), "results": []}) | ||
|
|
||
| @mcp.tool() | ||
| async def reflect(query: str) -> str: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good idea!
| @@ -0,0 +1,192 @@ | |||
| // Hindsight Agent API - Azure Container Apps Infrastructure | |||
| // Bicep template for deploying the agent API alongside existing hindsight-api | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Examples should go in this repo: https://github.com/vectorize-io/hindsight-cookbook/
we can create a deployments directory
would you mind?
| HINDSIGHT_API_LLM_PROVIDER=openai | ||
| HINDSIGHT_API_LLM_API_KEY=your-api-key-here | ||
| HINDSIGHT_API_LLM_MODEL=o3-mini | ||
| HINDSIGHT_API_LLM_MODEL=gpt-5.2-chat |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5.2 is very slow and we don't want poor first-user experience
| @@ -1,151 +1,593 @@ | |||
| # AGENTS.md | |||
| # Hindsight Agent: Azure AI Foundry Operations Manual | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please revert changs to this file
No description provided.