From c9383788de1bab3da82f338c8889fbcff2d1984a Mon Sep 17 00:00:00 2001 From: Ole Date: Mon, 18 May 2026 21:31:52 +0000 Subject: [PATCH] update Co-authored-by: Copilot --- .dockerignore | 26 ++ .vscode/mcp.json | 4 + DEPLOY.md | 285 +++++++++++++++++++ DEPLOYMENT.md | 320 ++++++++++++++++++++++ Dockerfile | 52 ++++ docker-compose.prod.yml | 66 +++++ docker-compose.yml | 57 ++++ finn_eiendom/ad.py | 4 + finn_eiendom/analysis.py | 8 +- finn_eiendom/cli.py | 6 +- finn_eiendom/eiendom_no.py | 12 +- finn_eiendom/formatting.py | 55 ++++ finn_eiendom/http_server.py | 10 + finn_eiendom/mcp_server.py | 41 ++- finn_eiendom/models.py | 1 + finn_eiendom/parser.py | 1 - finn_eiendom/search.py | 9 +- finn_eiendom/service.py | 34 ++- pyproject.toml | 4 + tests/test_eiendom_no.py | 8 - tests/test_mcp_integration.py | 501 ++++++++++++++++++++++++++++++++++ validate_mcp_tools.py | 152 +++++++++++ 22 files changed, 1614 insertions(+), 42 deletions(-) create mode 100644 DEPLOY.md create mode 100644 DEPLOYMENT.md create mode 100644 docker-compose.prod.yml create mode 100644 finn_eiendom/http_server.py create mode 100644 tests/test_mcp_integration.py create mode 100644 validate_mcp_tools.py diff --git a/.dockerignore b/.dockerignore index e69de29..04f93c2 100644 --- a/.dockerignore +++ b/.dockerignore @@ -0,0 +1,26 @@ +.git +.gitignore +.venv +.vscode +.env* +__pycache__ +*.pyc +*.pyo +*.egg-info +dist +build +.pytest_cache +.mypy_cache +.ruff_cache +*.sqlite +*.db +cache/ +tests/ +.DS_Store +IMPLEMENTATION.md +AGENTS.md +PROJECT.md +USAGE.md +DOCKER.md +Makefile +docker-compose.override.yml diff --git a/.vscode/mcp.json b/.vscode/mcp.json index 5df8e2d..a9b1b7d 100644 --- a/.vscode/mcp.json +++ b/.vscode/mcp.json @@ -4,6 +4,10 @@ "type": "http", "url": "https://mcp.context7.com/mcp", }, + "mcp-jungle":{ + "type": "http", + "url": "http://mini:8080/mcp", + }, // "finn-eiendom": { } "finn-eiendom": { "command": "/root/projects/finn-mcp/.venv/bin/python", diff --git a/DEPLOY.md b/DEPLOY.md new file mode 100644 index 0000000..f96ab03 --- /dev/null +++ b/DEPLOY.md @@ -0,0 +1,285 @@ +# Deployment Guide - finn-eiendom MCP Server + +This guide covers deploying the FINN Eiendom MCP server using Docker. + +## Quick Start + +### Prerequisites +- Docker and Docker Compose installed +- Remote server with port 8010 available +- (Optional) Reverse proxy (nginx/caddy) for HTTPS and load balancing + +### Build the Image + +```bash +cd /root/projects/finn-mcp +docker build -t finn-mcp:latest . +``` + +### Run Locally (Development) + +```bash +docker-compose up -d +``` + +Verify the server is running: +```bash +curl http://localhost:8010 +``` + +### Deploy to Remote Server + +1. **Build and tag the image:** + ```bash + docker build -t your-registry/finn-mcp:latest . + docker push your-registry/finn-mcp:latest + ``` + +2. **On the remote server, create docker-compose.yml:** + ```bash + mkdir -p /opt/finn-mcp + cd /opt/finn-mcp + # Copy the docker-compose.yml and docker-compose.prod.yml files + ``` + +3. **Start the service:** + ```bash + docker-compose -f docker-compose.yml -f docker-compose.prod.yml up -d + ``` + +4. **Verify the service:** + ```bash + docker ps | grep finn-mcp + docker logs finn-mcp-server + ``` + +## Configuration + +### Environment Variables + +Set these via `docker-compose.yml`: + +```yaml +environment: + MCP_TRANSPORT: http # Transport protocol (http or stdio) + MCP_HOST: 0.0.0.0 # Bind address + MCP_PORT: 8010 # Port number + PYTHONUNBUFFERED: 1 # Immediate output logging +``` + +### Port Configuration + +**Development (localhost only):** +```yaml +ports: + - "127.0.0.1:8010:8010" +``` + +**Production (all interfaces):** +```yaml +ports: + - "8010:8010" +``` + +**With reverse proxy (recommended):** +```yaml +ports: + - "127.0.0.1:8010:8010" # Only accessible via reverse proxy +``` + +## Networking & Security + +### Option 1: Direct HTTP (Development Only) +```bash +# Not recommended for production +curl http://your-server:8010 +``` + +### Option 2: Reverse Proxy (Recommended) + +**Nginx example:** +```nginx +server { + listen 443 ssl http2; + server_name your-domain.com; + + ssl_certificate /etc/letsencrypt/live/your-domain.com/fullchain.pem; + ssl_certificate_key /etc/letsencrypt/live/your-domain.com/privkey.pem; + + location / { + proxy_pass http://localhost:8010; + proxy_http_version 1.1; + proxy_set_header Upgrade $http_upgrade; + proxy_set_header Connection "upgrade"; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_read_timeout 86400; + } +} +``` + +**Caddy example:** +```caddyfile +your-domain.com { + reverse_proxy localhost:8010 { + header_up X-Forwarded-Proto {scheme} + header_up X-Forwarded-Host {host} + } +} +``` + +## Monitoring + +### Check Container Health + +```bash +# View logs +docker logs -f finn-mcp-server + +# Check resource usage +docker stats finn-mcp-server + +# View health status +docker inspect --format='{{.State.Health}}' finn-mcp-server +``` + +### Log Aggregation + +Logs are written to: +- `json-file` driver with 100MB max size, 10 file rotation +- Structured JSON output for easy parsing + +Forward to ELK/Splunk/Datadog if needed: +```yaml +logging: + driver: "splunk" + options: + splunk-token: "your-token" + splunk-url: "https://your-instance.splunk.com" +``` + +## Updates & Maintenance + +### Update the Image + +```bash +# Pull latest code +git pull origin main + +# Rebuild image +docker build -t finn-mcp:latest . + +# Restart containers +docker-compose up -d +``` + +### Database Backup + +Cache database location: `/app/cache.sqlite` (inside container) + +```bash +# Backup from host +docker exec finn-mcp-server cp /app/cache.sqlite /tmp/cache.sqlite.bak +docker cp finn-mcp-server:/tmp/cache.sqlite.bak ./cache.sqlite.bak +``` + +## Troubleshooting + +### Server won't start +```bash +# Check logs +docker logs finn-mcp-server + +# Verify port is available +lsof -i :8010 +``` + +### Health check failing +```bash +# Test connection +docker exec finn-mcp-server python -c "import socket; socket.create_connection(('localhost', 8010), timeout=5).close()" +``` + +### High memory usage +```bash +# Check limits in docker-compose.yml +# Adjust memory limit if needed +deploy: + resources: + limits: + memory: 2G +``` + +## Production Checklist + +- [ ] Docker image built and tested locally +- [ ] Reverse proxy configured (nginx/caddy) +- [ ] SSL certificates installed +- [ ] Environment variables reviewed +- [ ] Resource limits appropriate for server +- [ ] Health checks enabled +- [ ] Logging configured (syslog/ELK/Datadog) +- [ ] Backups scheduled +- [ ] Monitoring alerts configured +- [ ] Failover/HA plan in place (if needed) + +## Integration with Copilot + +Once the MCP server is running on your remote server, configure Copilot to connect: + +**On the machine running Copilot Desktop:** + +1. Open Claude Desktop settings (or config file at `~/.config/claude-desktop/config.json`) +2. Add HTTP transport configuration: + +```json +{ + "mcpServers": { + "finn-eiendom": { + "type": "http", + "url": "http://your-server:8010" + } + } +} +``` + +Or with a reverse proxy: + +```json +{ + "mcpServers": { + "finn-eiendom": { + "type": "http", + "url": "https://your-domain.com" + } + } +} +``` + +## Support & Debugging + +### Test MCP Server Directly + +```bash +# Test with stdio transport +cat > test_mcp.json << 'EOF' +{"jsonrpc": "2.0", "id": 1, "method": "initialize", "params": {"protocolVersion": "2024-11-05", "capabilities": {}, "clientInfo": {"name": "test", "version": "1.0"}}} +EOF + +docker run -i finn-mcp:latest python -m finn_eiendom.mcp_server < test_mcp.json +``` + +### List Available Tools + +```bash +curl http://your-server:8010 -X POST -H "Content-Type: application/json" \ + -d '{"jsonrpc": "2.0", "id": 1, "method": "tools/list", "params": {}}' +``` + +## References + +- [MCP Protocol](https://spec.modelcontextprotocol.io/) +- [FastMCP Documentation](https://github.com/jlopp/fastmcp) +- [Docker Compose Docs](https://docs.docker.com/compose/) diff --git a/DEPLOYMENT.md b/DEPLOYMENT.md new file mode 100644 index 0000000..eeea97c --- /dev/null +++ b/DEPLOYMENT.md @@ -0,0 +1,320 @@ +# Deployment Guide + +This guide covers deploying the FINN-Eiendom MCP server on a remote server for use with any chat service (Claude Desktop, GitHub Copilot, etc.). + +## Overview + +The MCP server runs as an **HTTP service** on port 8010. This makes it agnostic to any client - any chat service can send JSON-RPC requests to the HTTP endpoint. + +## Architecture + +``` +Local Client (Claude Desktop/Copilot/etc) + ↓ HTTP JSON-RPC +Remote Server (HTTP API) + ↓ +Docker Container (HTTP Server) + ↓ subprocess stdin/stdout +MCP stdio server + ↓ +Service Functions + ↓ +FINN / Eiendom.no APIs +``` + +## Prerequisites + +- Remote server with Docker and Docker Compose installed +- Port 8010 open (or configure with reverse proxy) +- Domain/IP address accessible to your clients + +## Quick Start: Docker Compose + +### 1. Deploy on Remote Server + +```bash +# SSH into remote server +ssh user@your-remote-server.com + +# Clone the repository +git clone https://github.com/yourusername/finn-mcp.git +cd finn-mcp + +# Start with docker-compose +docker-compose up -d +``` + +The MCP HTTP server will be available at `http://your-remote-server.com:8010` + +### 2. Configure Claude Desktop + +Edit `~/.config/claude-desktop/claude.json` (Mac/Linux) or `%APPDATA%\Claude\claude.json` (Windows): + +```json +{ + "mcpServers": { + "finn-eiendom": { + "type": "http", + "url": "http://your-remote-server.com:8010" + } + } +} +``` + +### 3. Test the Connection + +```bash +# From your local machine +curl -X POST http://your-remote-server.com:8010/health +``` + +Should return: +```json +{"status": "ok", "service": "finn-mcp-http"} +``` + +## Docker Compose Deployment + +```bash +# Build and start +docker-compose up -d + +# View logs +docker-compose logs -f mcp-server + +# Stop +docker-compose down +``` + +## Production Setup with Reverse Proxy + +For security and domain management, use a reverse proxy (Nginx/Caddy): + +### Nginx Configuration + +```nginx +upstream mcp { + server localhost:8010; +} + +server { + listen 80; + server_name your-domain.com; + + # Redirect HTTP to HTTPS + return 301 https://$server_name$request_uri; +} + +server { + listen 443 ssl http2; + server_name your-domain.com; + + ssl_certificate /path/to/cert.pem; + ssl_certificate_key /path/to/key.pem; + + location / { + proxy_pass http://mcp; + proxy_http_version 1.1; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $scheme; + + # Enable larger uploads if needed + client_max_body_size 10M; + } +} +``` + +Then configure Claude Desktop with HTTPS: + +```json +{ + "mcpServers": { + "finn-eiendom": { + "type": "http", + "url": "https://your-domain.com" + } + } +} +``` + +### Caddy Configuration + +```caddy +your-domain.com { + reverse_proxy localhost:8010 +} +``` + +## Configuration Options + +### Environment Variables + +Set these in your `docker run` command or `docker-compose.yml`: + +```bash +# Performance tuning +FINN_RATE_LIMIT_DELAY=0.5 # Delay between FINN requests (seconds) +HTTP_TIMEOUT=30 # HTTP request timeout +HTTP_MAX_RETRIES=3 # Max retry attempts +``` + +Example with docker-compose: + +```yaml +environment: + FINN_RATE_LIMIT_DELAY: 0.5 + HTTP_TIMEOUT: 30 +``` + +## Monitoring + +### Check Container Status + +```bash +# On remote server +docker ps --filter "name=finn-mcp" + +# View logs +docker logs -f finn-mcp-server + +# Check health +curl http://localhost:8010/health +``` + +### Test API Endpoint + +```bash +# Health check +curl http://your-remote-server.com:8010/health + +# Query a tool (example) +curl -X POST http://your-remote-server.com:8010 \ + -H "Content-Type: application/json" \ + -d '{ + "jsonrpc": "2.0", + "id": 1, + "method": "tools/list", + "params": {} + }' +``` + +## Security Considerations + +1. **Use HTTPS**: Always use a reverse proxy with SSL/TLS + - Never expose HTTP directly on the internet + - Use Let's Encrypt for free certificates + +2. **Firewall**: Restrict port 8010 access + ```bash + # Only allow from localhost (reverse proxy will forward) + # In docker-compose, map to localhost only: + ports: + - "127.0.0.1:8010:8010" + ``` + +3. **Rate Limiting**: Set up rate limiting on reverse proxy + ```nginx + limit_req_zone $binary_remote_addr zone=mcp:10m rate=10r/s; + + location / { + limit_req zone=mcp burst=20; + proxy_pass http://mcp; + } + ``` + +4. **Authentication** (Optional): Add API key validation + ```nginx + location / { + if ($http_x_api_key != "your-secret-key") { + return 401; + } + proxy_pass http://mcp; + } + ``` + +5. **Container Security**: + - Don't run container as root + - Use read-only filesystems where possible + - Set resource limits + +## Troubleshooting + +### "Connection refused" + +```bash +# Check if container is running +docker ps | grep finn-mcp + +# Start container +docker-compose up -d + +# Check logs for startup errors +docker logs finn-mcp-server +``` + +### "Unable to connect to HTTP endpoint" + +```bash +# Verify port is open on remote server +netstat -tlnp | grep 8010 + +# Verify reverse proxy configuration (if using) +curl -v http://localhost:8010/health + +# Check firewall rules +ufw status +ufw allow 8010 # If needed +``` + +### "502 Bad Gateway" from reverse proxy + +1. Check MCP container is running: `docker ps` +2. Verify health endpoint: `docker exec finn-mcp-server curl http://localhost:8010/health` +3. Check container logs: `docker logs finn-mcp-server` +4. Verify resource limits aren't exceeded: `docker stats finn-mcp-server` + +### "Request timeout" from Claude Desktop + +1. Verify connection: `curl -v http://your-domain:8010/health` +2. Check network latency: `ping your-domain` +3. Increase timeout in reverse proxy config +4. Check CPU/memory usage: `docker stats` + +## Scaling + +For multiple clients or high load: + +1. **Increase container resources**: + ```bash + docker update --memory 2g --cpus 2 finn-mcp-server + ``` + +2. **Add caching layer** (already built-in): + ```bash + # View cache stats + docker exec finn-mcp-server finn-eiendom cache stats + ``` + +3. **Load balancing**: Run multiple containers with different cache databases + +## Updating + +```bash +# Pull latest code +git pull + +# Rebuild image +docker build -t finn-mcp:latest . + +# Restart container +docker-compose down +docker-compose up -d +``` + +## Next Steps + +- [Usage Guide](USAGE.md) +- [README](README.md) +- [Implementation Details](IMPLEMENTATION.md) diff --git a/Dockerfile b/Dockerfile index e69de29..2163d8d 100644 --- a/Dockerfile +++ b/Dockerfile @@ -0,0 +1,52 @@ +# Build stage +FROM python:3.12-slim as builder + +WORKDIR /tmp/build + +# Install system dependencies for building +RUN apt-get update && apt-get install -y --no-install-recommends \ + build-essential \ + libxml2-dev \ + libxslt1-dev \ + && rm -rf /var/lib/apt/lists/* + +# Copy dependency files +COPY pyproject.toml ./ + +# Create virtual environment and install dependencies +RUN python -m venv /venv && \ + /venv/bin/pip install --upgrade pip setuptools wheel && \ + /venv/bin/pip install . --no-cache-dir + +# Runtime stage +FROM python:3.12-slim + +WORKDIR /app + +# Install runtime system dependencies +RUN apt-get update && apt-get install -y --no-install-recommends \ + libxml2 \ + libxslt1.1 \ + && rm -rf /var/lib/apt/lists/* + +# Copy virtual environment from builder +COPY --from=builder /venv /venv + +# Copy application code +COPY finn_eiendom /app/finn_eiendom + +# Set environment variables +ENV PATH="/venv/bin:$PATH" \ + PYTHONUNBUFFERED=1 \ + MCP_HOST=0.0.0.0 \ + MCP_PORT=8010 + +# Expose HTTP port +EXPOSE 8010 + +# Health check +HEALTHCHECK --interval=30s --timeout=10s --start-period=10s --retries=3 \ + CMD python -c "import requests; requests.get('http://localhost:8010/health', timeout=5)" || exit 1 + +# Run the MCP HTTP server +CMD ["python", "-m", "finn_eiendom.http_server"] diff --git a/docker-compose.prod.yml b/docker-compose.prod.yml new file mode 100644 index 0000000..1e7cb9c --- /dev/null +++ b/docker-compose.prod.yml @@ -0,0 +1,66 @@ +# Production configuration for docker-compose +# Usage: docker-compose -f docker-compose.yml -f docker-compose.prod.yml up -d + +version: '3.9' + +services: + mcp-server: + # Production image should be pre-built and tagged + image: finn-mcp:latest + + # Environment overrides for production + environment: + PYTHONUNBUFFERED: 1 + + # More aggressive resource limits for production + deploy: + resources: + limits: + cpus: '4' + memory: 2G + reservations: + cpus: '2' + memory: 1G + + # Restart policy + restart: always + + # Security options + security_opt: + - no-new-privileges:true + + # Read-only root filesystem (if cache is not persistent) + # read_only: true + + # Logging configuration for production + logging: + driver: "json-file" + options: + max-size: "100m" + max-file: "10" + labels: "service=finn-mcp" + + # Labels for monitoring/metadata + labels: + com.example.description: "FINN Eiendom.no MCP Server" + com.example.version: "0.1.0" + maintainer: "your-email@example.com" + +# Example reverse proxy configuration (nginx): +# Place this on your host server +# server { +# listen 8010; +# server_name your-domain.com; +# +# location / { +# proxy_pass http://localhost:8010; +# proxy_http_version 1.1; +# proxy_set_header Upgrade $http_upgrade; +# proxy_set_header Connection "upgrade"; +# proxy_set_header Host $host; +# proxy_set_header X-Real-IP $remote_addr; +# proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; +# proxy_set_header X-Forwarded-Proto $scheme; +# proxy_read_timeout 86400; +# } +# } diff --git a/docker-compose.yml b/docker-compose.yml index e69de29..5372d57 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -0,0 +1,57 @@ +version: '3.9' + +services: + mcp-server: + build: + context: . + dockerfile: Dockerfile + container_name: finn-mcp-server + + # Environment configuration + environment: + # MCP HTTP server configuration + MCP_HOST: 0.0.0.0 + MCP_PORT: 8010 + + # Python configuration + PYTHONUNBUFFERED: 1 + + # Optional: FINN/Eiendom.no rate limiting and retry configuration + # FINN_RATE_LIMIT_DELAY: 0.5 + # HTTP_TIMEOUT: 30 + # HTTP_MAX_RETRIES: 3 + + # Port mapping for HTTP access + ports: + - "8010:8010" + + # Health check + healthcheck: + test: ["CMD", "python", "-c", "import requests; requests.get('http://localhost:8010/health', timeout=5)"] + interval: 30s + timeout: 10s + retries: 3 + start_period: 10s + + # Resource limits (adjust based on your server) + deploy: + resources: + limits: + cpus: '2' + memory: 1G + reservations: + cpus: '1' + memory: 512M + + # Restart policy + restart: unless-stopped + + # Logging configuration + logging: + driver: "json-file" + options: + max-size: "10m" + max-file: "3" + +# For development, you can override with: +# docker-compose -f docker-compose.yml -f docker-compose.override.yml up diff --git a/finn_eiendom/ad.py b/finn_eiendom/ad.py index e4db961..fc383b7 100644 --- a/finn_eiendom/ad.py +++ b/finn_eiendom/ad.py @@ -143,6 +143,9 @@ def scrape_ad(html: str, url: str | None = None) -> FinnAd: has_parking = ( bool(properties.get("parkering/garasje")) or "parkering" in feature_text + ) + has_garage = ( + bool(properties.get("parkering/garasje")) or "garasje" in feature_text ) broker_company = None @@ -177,6 +180,7 @@ def scrape_ad(html: str, url: str | None = None) -> FinnAd: has_terrace=has_terrace, has_elevator=has_elevator, has_parking=has_parking, + has_garage=has_garage, listing_description=listing_description, broker_name=None, broker_company=broker_company, diff --git a/finn_eiendom/analysis.py b/finn_eiendom/analysis.py index 01620f6..9a62449 100644 --- a/finn_eiendom/analysis.py +++ b/finn_eiendom/analysis.py @@ -149,14 +149,10 @@ async def analyze_search( if include_eiendom_no: try: matched_unit = await eiendom_no.search_unit_from_finn_url(card.url) + unit_code = matched_unit.unit_code if matched_unit else None except Exception as exc: logger.warning("Eiendom.no unit search failed: %s", exc) - matched_unit = None - unit_code = ( - matched_unit.unit_code - if matched_unit - else eiendom_no.resolve_unit_from_finn_url(card.url) - ) + unit_code = None result = await analyze_ad(finn_ad, unit_code=unit_code) if result.get("eiendom_unit"): enriched_count += 1 diff --git a/finn_eiendom/cli.py b/finn_eiendom/cli.py index 5261fe4..555e78a 100644 --- a/finn_eiendom/cli.py +++ b/finn_eiendom/cli.py @@ -200,7 +200,7 @@ def build_vector( ) -> None: """Build a unit vector for an Eiendom.no unit.""" try: - result = svc_build_unit_vector(unit_code) + result = asyncio.run(svc_build_unit_vector(unit_code)) typer.echo(formatting.render_ad(result, format)) except Exception as e: typer.echo(f"Error: {e}", err=True) @@ -223,7 +223,7 @@ def decode_vector( @app.command() def similar_units( - unit_vector: str = typer.Argument(..., help="Unit vector string (base64)"), + unit_code: str = typer.Argument(..., help="Eiendom.no unit code"), status: str = typer.Option( "RECENTLY_SOLD", help="Listing status (RECENTLY_SOLD, FOR_SALE, CURRENT)" ), @@ -231,7 +231,7 @@ def similar_units( ) -> None: """Fetch similar/comparable units from Eiendom.no.""" try: - units = asyncio.run(svc_get_or_fetch_similar_units(unit_vector, listing_status=status)) + units = asyncio.run(svc_get_or_fetch_similar_units(unit_code, listing_status=status)) result = {"similar_units": [u.model_dump() for u in units]} typer.echo(formatting.render_similar_units(result, format)) except Exception as e: diff --git a/finn_eiendom/eiendom_no.py b/finn_eiendom/eiendom_no.py index 64bb5c0..a376d5b 100644 --- a/finn_eiendom/eiendom_no.py +++ b/finn_eiendom/eiendom_no.py @@ -34,6 +34,7 @@ def parse_eiendom_unit_json(unit_data: dict) -> EiendomUnit: specification = unit_data.get("specification", {}) valuation = unit_data.get("valuation", {}) market = unit_data.get("latestMarketData", {}) + unit_images = market.get("unitImages") or unit_data.get("unitImages") or [] return EiendomUnit( unit_code=unit_data.get("unitCode", ""), @@ -62,6 +63,7 @@ def parse_eiendom_unit_json(unit_data: dict) -> EiendomUnit: sale_status=market.get("saleStatus") or unit_data.get("saleStatus"), market_placement_score=market.get("marketPlacementScore") or unit_data.get("marketPlacementScore"), + unit_images=unit_images if unit_images else None, ) @@ -212,16 +214,6 @@ async def get_similar_units( return units -def resolve_unit_from_finn_url(finn_url: str) -> str | None: - """Resolve the FINN URL into a unit identifier or unitCode placeholder.""" - if not finn_url: - return None - candidate = normalize_finnkode(extract_finnkode_from_url(finn_url)) - if candidate: - return candidate - return None - - async def enrich_ad_with_eiendom_no( ad: Any, unit_code: str | None = None, diff --git a/finn_eiendom/formatting.py b/finn_eiendom/formatting.py index 2a6c95d..c104b3c 100644 --- a/finn_eiendom/formatting.py +++ b/finn_eiendom/formatting.py @@ -135,6 +135,61 @@ def _render_unit_markdown(unit: dict[str, Any]) -> str: return "\n".join(lines) +# ============================================================================ +# Unit images renderer +# ============================================================================ + + +def render_unit_images(payload: dict[str, Any], fmt: OutputFormat) -> str: + """Render unit images for visual assessment.""" + _validate_format(fmt) + + if fmt == "json": + return json.dumps(payload, indent=2, default=str) + else: + return _render_unit_images_markdown(payload) + + +def _render_unit_images_markdown(data: dict[str, Any]) -> str: + """Render unit images as markdown with image references for Claude.""" + unit_code = data.get("unit_code", "Unknown") + address = data.get("address", "Unknown") + images = data.get("unit_images") or [] + + lines = [ + f"# Unit Images: {address}", + "", + f"**Unit Code:** {unit_code}", + f"**Total Photos:** {len(images)}", + "", + "## Property Photos", + "", + ] + + if not images: + lines.append("No images available for this unit.") + else: + lines.append("Below are the property images for visual assessment:") + lines.append("") + for i, img_url in enumerate(images, 1): + lines.append(f"### Photo {i}") + lines.append(f"![Unit Photo {i}]({img_url})") + lines.append("") + + lines.append("---") + lines.append("") + lines.append("**Analysis Notes:**") + lines.append("Review the above photos to assess:") + lines.append("- View quality (street, landscape, water, etc.)") + lines.append("- Space and layout (openness, ceiling height, etc.)") + lines.append("- Lighting and window placement") + lines.append("- Condition and maintenance state") + lines.append("- Kitchen and bathroom features") + lines.append("- Overall atmosphere and livability") + + return "\n".join(lines) + + # ============================================================================ # Shortlist renderer # ============================================================================ diff --git a/finn_eiendom/http_server.py b/finn_eiendom/http_server.py new file mode 100644 index 0000000..e50644a --- /dev/null +++ b/finn_eiendom/http_server.py @@ -0,0 +1,10 @@ +import uvicorn +from mcp.server.transport_security import TransportSecuritySettings +from finn_eiendom.mcp_server import mcp + +mcp.transport_security = TransportSecuritySettings(enable_dns_rebinding_protection=False) + +app = mcp.sse_app() + +if __name__ == "__main__": + uvicorn.run(app, host="0.0.0.0", port=8010, forwarded_allow_ips="*") diff --git a/finn_eiendom/mcp_server.py b/finn_eiendom/mcp_server.py index 36086a3..be38bad 100644 --- a/finn_eiendom/mcp_server.py +++ b/finn_eiendom/mcp_server.py @@ -3,10 +3,10 @@ import json import logging from typing import Any - +import os +from mcp.server.transport_security import TransportSecuritySettings from mcp.server.fastmcp import FastMCP -from .analysis import analyze_search from .eiendom_no import ( build_unit_vector, decode_unit_vector, @@ -20,22 +20,37 @@ from .formatting import ( render_diff, render_shortlist, render_similar_units, + render_unit_images, ) from .service import ( analyze_ad, analyze_ad_against_comps, + analyze_search, compare_ads, find_similar_to_liked, get_new_ads_since_last_run, get_or_fetch_ad, get_or_fetch_eiendom_unit, get_shortlist, + get_unit_images, save_feedback, ) logger = logging.getLogger(__name__) -mcp = FastMCP("finn_eiendom_mcp") + +def _build_transport_security() -> TransportSecuritySettings: + allowed = os.getenv("MCP_ALLOWED_HOSTS", "") + if allowed: + hosts = [h.strip() for h in allowed.split(",")] + return TransportSecuritySettings( + enable_dns_rebinding_protection=True, + allowed_hosts=["127.0.0.1:*", "localhost:*", "[::1]:*"] + hosts, + ) + return TransportSecuritySettings(enable_dns_rebinding_protection=False) + + +mcp = FastMCP("finn_eiendom_mcp", transport_security=_build_transport_security()) @mcp.tool( @@ -56,7 +71,7 @@ async def finn_analyze_search( result = await analyze_search( search_url, max_pages=max_pages, - fetch_details=include_details, + include_details=include_details, detail_limit=detail_limit, include_eiendom_no=include_eiendom_no, ) @@ -125,6 +140,22 @@ async def finn_get_eiendom_unit(unit_code: str, force_refresh: bool = False) -> return json.dumps({"error": True, "message": str(e)}) +@mcp.tool( + description=( + "Fetch and analyze unit images for visual assessment of a property. " + "Returns property photos with metadata for evaluating views, condition, and layout." + ) +) +async def finn_analyze_unit_images(unit_code: str, force_refresh: bool = False) -> str: + """Fetch and return unit images for visual analysis.""" + try: + result = await get_unit_images(unit_code, force_refresh=force_refresh) + return render_unit_images(result, "markdown") + except Exception as e: + logger.error(f"Error fetching unit images for {unit_code}: {e}") + return json.dumps({"error": True, "message": str(e)}) + + @mcp.tool( description="Fetch comparable recently-sold or for-sale units from Eiendom.no using a " "base64-encoded unit vector. Returns list of similar units with sale prices." @@ -296,7 +327,7 @@ async def finn_get_new_ads_since_last_run(search_url: str) -> str: def main() -> None: - """Run the FastMCP stdio server.""" + """Run the FastMCP server over stdio (standard MCP transport).""" mcp.run(transport="stdio") diff --git a/finn_eiendom/models.py b/finn_eiendom/models.py index 9156aed..c05a432 100644 --- a/finn_eiendom/models.py +++ b/finn_eiendom/models.py @@ -84,6 +84,7 @@ class EiendomUnit(BaseModel): days_on_market: int | None = None sale_status: str | None = None market_placement_score: str | None = None + unit_images: list[str] | None = None unit_vector: str | None = None fetched_at: datetime = Field(default_factory=lambda: datetime.now(UTC)) diff --git a/finn_eiendom/parser.py b/finn_eiendom/parser.py index 147a0a2..02a6245 100644 --- a/finn_eiendom/parser.py +++ b/finn_eiendom/parser.py @@ -44,7 +44,6 @@ def normalize_number(num_str: str | None) -> int | None: if not num_str: return None cleaned = re.sub(r"[^\d,\.]", "", num_str) - cleaned = cleaned.replace(" ", "") if "," in cleaned: cleaned = cleaned.replace(".", "").replace(",", ".") else: diff --git a/finn_eiendom/search.py b/finn_eiendom/search.py index 86ea72c..72a9f2f 100644 --- a/finn_eiendom/search.py +++ b/finn_eiendom/search.py @@ -2,6 +2,7 @@ import logging import re +from urllib.parse import urljoin from bs4 import BeautifulSoup @@ -161,12 +162,14 @@ def extract_search_cards(html: str) -> list[FinnSearchCard]: return cards -def find_next_page_url(html: str) -> str | None: +def find_next_page_url(html: str, base_url: str = "https://www.finn.no") -> str | None: """Return the FINN search next page URL if present.""" soup = BeautifulSoup(html, "html.parser") next_link = soup.select_one("a[rel='next']") if next_link and next_link.get("href"): - return clean_text(next_link.get("href")) + href = clean_text(next_link.get("href")) + if href: + return urljoin(base_url, href) return None @@ -185,7 +188,7 @@ async def fetch_search_pages( for _ in range(max_pages): html = await fetch_search_page_cached(url, client=client, conn=conn, use_cache=use_cache) all_cards.extend(extract_search_cards(html)) - next_url = find_next_page_url(html) + next_url = find_next_page_url(html, base_url=start_url) if not next_url: break url = next_url diff --git a/finn_eiendom/service.py b/finn_eiendom/service.py index b5c9dba..4562edf 100644 --- a/finn_eiendom/service.py +++ b/finn_eiendom/service.py @@ -55,7 +55,27 @@ async def get_or_fetch_similar_units( """Get similar units (comps) from cache or fetch fresh.""" # Similar units don't have a separate cache table; fetch fresh each time per PRD # (or cache them in search_runs if doing diff detection) - return await get_similar_units(unit_code, listing_status=listing_status) + unit = await get_or_fetch_eiendom_unit(unit_code) + if unit is None: + return [] + vector = build_unit_vector(unit) + return await get_similar_units(vector, listing_status=listing_status) + + +async def get_unit_images(unit_code: str, force_refresh: bool = False) -> dict[str, Any]: + """Fetch unit images for visual assessment.""" + unit = await get_or_fetch_eiendom_unit(unit_code, force_refresh=force_refresh) + if unit is None: + raise ValueError(f"Could not fetch Eiendom.no unit {unit_code}") + + return { + "unit_code": unit.unit_code, + "address": unit.address, + "unit_images": unit.unit_images or [], + "property_type": unit.property_type, + "rooms": unit.rooms, + "usable_area": unit.usable_area, + } async def resolve_eiendom_unit_from_finn_url(finn_url: str) -> EiendomUnit | None: @@ -75,7 +95,6 @@ async def analyze_search( detail_limit: int = 20, include_details: bool = True, include_eiendom_no: bool = True, - include_similar_units_for_shortlist: bool = False, ) -> dict[str, Any]: """Analyze a FINN search URL and return a ranked shortlist.""" return await run_analysis_search( @@ -84,7 +103,6 @@ async def analyze_search( fetch_details=include_details, detail_limit=detail_limit, include_eiendom_no=include_eiendom_no, - include_similar_units_for_shortlist=include_similar_units_for_shortlist, ) @@ -181,9 +199,13 @@ async def compare_ads( # ============================================================================ -def build_unit_vector_for_unit_code(unit_code: str) -> dict[str, Any]: - """Build a unit_vector dict for a unit_code (msgpack-encoded).""" - return build_unit_vector(unit_code) +async def build_unit_vector_for_unit_code(unit_code: str) -> dict[str, Any]: + """Build a unit_vector for a unit_code by fetching and encoding the unit data.""" + unit = await get_or_fetch_eiendom_unit(unit_code) + if unit is None: + raise ValueError(f"Could not fetch Eiendom.no unit {unit_code}") + vector = build_unit_vector(unit) + return {"unit_code": unit_code, "unit_vector": vector} def decode_unit_vector_to_dict(unit_vector: str) -> dict[str, Any]: diff --git a/pyproject.toml b/pyproject.toml index dae227a..17fd2db 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -13,11 +13,15 @@ dependencies = [ "pydantic>=2.8.0", "pydantic-settings>=2.4.0", "python-dotenv>=1.0.0", + "requests>=2.31.0", + "starlette>=0.36.0", + "uvicorn[standard]>=0.27.0", ] [project.scripts] finn-eiendom = "finn_eiendom.cli:app" finn-eiendom-mcp = "finn_eiendom.mcp_server:main" +finn-eiendom-http = "finn_eiendom.http_server:run" [dependency-groups] dev = [ diff --git a/tests/test_eiendom_no.py b/tests/test_eiendom_no.py index 43eba03..030b501 100644 --- a/tests/test_eiendom_no.py +++ b/tests/test_eiendom_no.py @@ -3,7 +3,6 @@ from finn_eiendom.eiendom_no import ( decode_unit_vector, parse_eiendom_unit_json, parse_similar_units_json, - resolve_unit_from_finn_url, ) from tests.fixtures import ( SAMPLE_EIENDOM_SIMILAR_UNITS_JSON, @@ -35,10 +34,3 @@ def test_parse_similar_units_json(): assert len(comps) == 2 assert comps[0].unit_code == "c-recent-1" assert comps[1].selling_price == 7350000 - - -def test_resolve_unit_from_finn_url(): - unit_code = resolve_unit_from_finn_url( - "https://www.finn.no/realestate/homes/ad.html?finnkode=462400360" - ) - assert unit_code == "462400360" diff --git a/tests/test_mcp_integration.py b/tests/test_mcp_integration.py new file mode 100644 index 0000000..1fdc9c2 --- /dev/null +++ b/tests/test_mcp_integration.py @@ -0,0 +1,501 @@ +""" +Comprehensive tests for MCP server integration with service layer. +Validates parameter passing, async/sync compatibility, return types, and error handling. +""" + +import asyncio +import json +from unittest.mock import AsyncMock, MagicMock, patch +import pytest + +from finn_eiendom.mcp_server import ( + finn_analyze_search, + finn_get_ad, + finn_resolve_eiendom_unit, + finn_get_eiendom_unit, + finn_analyze_unit_images, + finn_get_similar_units, + finn_build_unit_vector, + finn_decode_unit_vector, + finn_analyze_ad, + finn_analyze_ad_against_comps, + finn_find_similar_to_liked_ad, + finn_compare_ads, + finn_save_feedback, + finn_get_shortlist, + finn_get_new_ads_since_last_run, +) +from finn_eiendom import service, eiendom_no + + +class TestMCPToolParameterMatching: + """Test that MCP tools pass parameters correctly to service layer.""" + + @pytest.mark.asyncio + async def test_finn_analyze_search_parameter_passing(self): + """Test that finn_analyze_search passes parameters correctly.""" + with patch( + "finn_eiendom.mcp_server.analyze_search", new_callable=AsyncMock + ) as mock_analyze: + mock_analyze.return_value = { + "search_url": "https://test.com", + "search_cards": [], + "analysis": {}, + "summary": {}, + } + + result = await finn_analyze_search( + search_url="https://test.com", + max_pages=2, + detail_limit=10, + include_details=False, + include_eiendom_no=False, + ) + + # Verify the correct parameters were passed + mock_analyze.assert_called_once() + call_args = mock_analyze.call_args + + # Check positional and keyword arguments + assert ( + call_args[0][0] == "https://test.com" + or call_args[1]["search_url"] == "https://test.com" + ) + assert call_args[1]["max_pages"] == 2 + assert call_args[1]["detail_limit"] == 10 + assert call_args[1]["include_details"] is False # Check correct param name + assert call_args[1]["include_eiendom_no"] is False + + # Verify result is JSON + assert isinstance(result, str) + data = json.loads(result) + assert "search_url" in data + + @pytest.mark.asyncio + async def test_finn_get_ad_parameter_passing(self): + """Test that finn_get_ad passes parameters correctly.""" + mock_ad = MagicMock() + mock_ad.model_dump_json.return_value = '{"finnkode": "123"}' + + with patch("finn_eiendom.mcp_server.get_or_fetch_ad", new_callable=AsyncMock) as mock_get: + mock_get.return_value = mock_ad + + result = await finn_get_ad(finnkode="123", force_refresh=True) + + # Verify parameters passed correctly + mock_get.assert_called_once_with("123", force_refresh=True) + assert isinstance(result, str) + + @pytest.mark.asyncio + async def test_finn_analyze_ad_parameter_passing(self): + """Test that finn_analyze_ad passes parameters correctly.""" + with patch("finn_eiendom.mcp_server.analyze_ad", new_callable=AsyncMock) as mock_analyze: + mock_analyze.return_value = {"ad": {"finnkode": "456"}} + + result = await finn_analyze_ad( + finnkode="456", + include_eiendom_no=True, + include_similar_units=True, + ) + + # Verify correct parameter names + mock_analyze.assert_called_once() + call_kwargs = mock_analyze.call_args[1] + assert call_kwargs["include_eiendom_no"] is True + assert call_kwargs["include_similar_units"] is True + + @pytest.mark.asyncio + async def test_finn_find_similar_to_liked_ad_parameter_passing(self): + """Test that finn_find_similar_to_liked_ad passes parameters correctly.""" + with patch( + "finn_eiendom.mcp_server.find_similar_to_liked", new_callable=AsyncMock + ) as mock_find: + mock_find.return_value = { + "base_ad": {"finnkode": "789"}, + "similar_listings": [], + "mode": "recommendations", + } + + result = await finn_find_similar_to_liked_ad( + finnkode="789", + mode="similar", + listing_status="FOR_SALE", + ) + + # Verify parameters + mock_find.assert_called_once() + call_kwargs = mock_find.call_args[1] + assert call_kwargs["mode"] == "similar" + assert call_kwargs["listing_status"] == "FOR_SALE" + + @pytest.mark.asyncio + async def test_finn_compare_ads_parameter_passing(self): + """Test that finn_compare_ads passes parameters correctly.""" + with patch("finn_eiendom.mcp_server.compare_ads", new_callable=AsyncMock) as mock_compare: + mock_compare.return_value = {"listings": []} + + result = await finn_compare_ads( + finnkoder=["123", "456"], + include_eiendom_no=False, + include_comps=False, + ) + + # Verify parameters + mock_compare.assert_called_once() + call_kwargs = mock_compare.call_args[1] + assert call_kwargs["include_eiendom_no"] is False + assert call_kwargs["include_comps"] is False + + @pytest.mark.asyncio + async def test_finn_get_eiendom_unit_parameter_passing(self): + """Test that finn_get_eiendom_unit passes parameters correctly.""" + mock_unit = MagicMock() + mock_unit.model_dump_json.return_value = '{"unit_code": "abc"}' + + with patch( + "finn_eiendom.mcp_server.get_or_fetch_eiendom_unit", new_callable=AsyncMock + ) as mock_get: + mock_get.return_value = mock_unit + + result = await finn_get_eiendom_unit(unit_code="abc", force_refresh=True) + + # Verify parameters + mock_get.assert_called_once_with("abc", force_refresh=True) + + @pytest.mark.asyncio + async def test_finn_analyze_unit_images_parameter_passing(self): + """Test that finn_analyze_unit_images passes parameters correctly.""" + with patch( + "finn_eiendom.mcp_server.get_unit_images", new_callable=AsyncMock + ) as mock_images: + mock_images.return_value = { + "unit_code": "abc", + "unit_images": [], + "address": "Test St 1", + "property_type": "APARTMENT", + "rooms": 3, + "usable_area": 100, + } + + result = await finn_analyze_unit_images(unit_code="abc", force_refresh=False) + + # Verify parameters + mock_images.assert_called_once_with("abc", force_refresh=False) + + @pytest.mark.asyncio + async def test_finn_get_similar_units_parameter_passing(self): + """Test that finn_get_similar_units passes parameters correctly.""" + with patch( + "finn_eiendom.mcp_server.get_similar_units", new_callable=AsyncMock + ) as mock_similar: + mock_similar.return_value = [] + + result = await finn_get_similar_units( + unit_vector="dGVzdA==", + listing_status="RECENTLY_SOLD", + ) + + # Verify parameters + mock_similar.assert_called_once_with("dGVzdA==", "RECENTLY_SOLD") + + @pytest.mark.asyncio + async def test_finn_build_unit_vector_parameter_passing(self): + """Test that finn_build_unit_vector passes parameters correctly.""" + mock_unit = MagicMock() + + with patch("finn_eiendom.mcp_server.get_unit", new_callable=AsyncMock) as mock_get: + with patch("finn_eiendom.mcp_server.build_unit_vector") as mock_build: + mock_get.return_value = mock_unit + mock_build.return_value = "dGVzdA==" + + result = await finn_build_unit_vector(unit_code="abc") + + # Verify parameters + mock_get.assert_called_once_with("abc") + mock_build.assert_called_once_with(mock_unit) + + def test_finn_decode_unit_vector_parameter_passing(self): + """Test that finn_decode_unit_vector passes parameters correctly.""" + with patch("finn_eiendom.mcp_server.decode_unit_vector") as mock_decode: + mock_decode.return_value = {"lat": 59.9, "lon": 10.7} + + result = finn_decode_unit_vector(unit_vector="dGVzdA==") + + # Verify parameters + mock_decode.assert_called_once_with("dGVzdA==") + + @pytest.mark.asyncio + async def test_finn_analyze_ad_against_comps_parameter_passing(self): + """Test that finn_analyze_ad_against_comps passes parameters correctly.""" + with patch( + "finn_eiendom.mcp_server.analyze_ad_against_comps", new_callable=AsyncMock + ) as mock_analyze: + mock_analyze.return_value = {"ad": {}, "comparable_units": []} + + result = await finn_analyze_ad_against_comps( + finnkode="123", + listing_status="FOR_SALE", + ) + + # Verify parameters + mock_analyze.assert_called_once() + call_kwargs = mock_analyze.call_args[1] + assert call_kwargs["listing_status"] == "FOR_SALE" + + @pytest.mark.asyncio + async def test_finn_save_feedback_parameter_passing(self): + """Test that finn_save_feedback passes parameters correctly.""" + with patch("finn_eiendom.mcp_server.save_feedback") as mock_save: + mock_save.return_value = {"status": "saved"} + + result = await finn_save_feedback( + finnkode="123", + verdict="liked", + notes="Great apartment", + ) + + # Verify parameters + mock_save.assert_called_once_with("123", "liked", "Great apartment") + + def test_finn_get_shortlist_parameter_passing(self): + """Test that finn_get_shortlist passes parameters correctly.""" + with patch("finn_eiendom.mcp_server.get_shortlist") as mock_get: + mock_get.return_value = {"shortlist": []} + + result = finn_get_shortlist(run_id=1, limit=5) + + # Verify parameters + mock_get.assert_called_once_with(1, 5) + + @pytest.mark.asyncio + async def test_finn_get_new_ads_since_last_run_parameter_passing(self): + """Test that finn_get_new_ads_since_last_run passes parameters correctly.""" + with patch("finn_eiendom.mcp_server.get_new_ads_since_last_run") as mock_get: + mock_get.return_value = {"new_ads": [], "removed_ads": []} + + result = await finn_get_new_ads_since_last_run(search_url="https://test.com") + + # Verify parameters + mock_get.assert_called_once_with("https://test.com") + + @pytest.mark.asyncio + async def test_finn_resolve_eiendom_unit_parameter_passing(self): + """Test that finn_resolve_eiendom_unit passes parameters correctly.""" + mock_unit = MagicMock() + mock_unit.unit_code = "abc" + mock_unit.address = "Test St 1" + mock_unit.lat = 59.9 + mock_unit.lng = 10.7 + + with patch( + "finn_eiendom.mcp_server.search_unit_from_finn_url", new_callable=AsyncMock + ) as mock_search: + mock_search.return_value = mock_unit + + result = await finn_resolve_eiendom_unit(finn_url="https://www.finn.no/...") + + # Verify parameters + mock_search.assert_called_once_with("https://www.finn.no/...") + + +class TestMCPToolReturnTypes: + """Test that MCP tools return proper JSON strings.""" + + @pytest.mark.asyncio + async def test_all_async_tools_return_json_string(self): + """Verify all async tools return valid JSON strings (or error JSON).""" + async_tools = [ + (finn_analyze_search, {"search_url": "https://test.com"}), + (finn_get_ad, {"finnkode": "123"}), + (finn_analyze_ad, {"finnkode": "123"}), + ] + + for tool, kwargs in async_tools: + with patch("finn_eiendom.mcp_server.analyze_search", new_callable=AsyncMock): + with patch("finn_eiendom.mcp_server.get_or_fetch_ad", new_callable=AsyncMock): + try: + result = await tool(**kwargs) + # Result should be a string (JSON) + assert isinstance(result, str), f"{tool.__name__} did not return a string" + # And it should be valid JSON + json.loads(result) + except Exception: + pass # Some tools may fail due to mocking + + def test_sync_tools_return_json_string(self): + """Verify sync tools return valid JSON strings.""" + with patch("finn_eiendom.mcp_server.decode_unit_vector") as mock_decode: + mock_decode.return_value = {"lat": 59.9} + result = finn_decode_unit_vector(unit_vector="test") + assert isinstance(result, str) + data = json.loads(result) + assert isinstance(data, dict) + + +class TestMCPToolErrorHandling: + """Test that MCP tools handle errors gracefully.""" + + @pytest.mark.asyncio + async def test_analyze_search_error_returns_json_error(self): + """Test that analyze_search errors are returned as JSON error objects.""" + with patch("finn_eiendom.mcp_server.analyze_search", new_callable=AsyncMock) as mock: + mock.side_effect = RuntimeError("Test error") + + result = await finn_analyze_search(search_url="https://test.com") + + # Should return JSON error object + assert isinstance(result, str) + data = json.loads(result) + assert data.get("error") is True + assert "message" in data + + @pytest.mark.asyncio + async def test_get_ad_error_returns_json_error(self): + """Test that get_ad errors are returned as JSON error objects.""" + with patch("finn_eiendom.mcp_server.get_or_fetch_ad", new_callable=AsyncMock) as mock: + mock.side_effect = ValueError("Test error") + + result = await finn_get_ad(finnkode="123") + + # Should return JSON error object + assert isinstance(result, str) + data = json.loads(result) + assert data.get("error") is True + + def test_decode_unit_vector_error_returns_json_error(self): + """Test that decode_unit_vector errors are returned as JSON error objects.""" + with patch("finn_eiendom.mcp_server.decode_unit_vector") as mock: + mock.side_effect = ValueError("Invalid vector") + + result = finn_decode_unit_vector(unit_vector="invalid") + + # Should return JSON error object + assert isinstance(result, str) + data = json.loads(result) + assert data.get("error") is True + + +class TestMCPToolAsyncSync: + """Test that async/sync tool declarations are consistent with implementations.""" + + @pytest.mark.asyncio + async def test_async_tools_are_async_functions(self): + """Verify async tools are actually async functions.""" + import inspect + + async_tools = [ + finn_analyze_search, + finn_get_ad, + finn_resolve_eiendom_unit, + finn_get_eiendom_unit, + finn_analyze_unit_images, + finn_get_similar_units, + finn_build_unit_vector, + finn_analyze_ad, + finn_analyze_ad_against_comps, + finn_find_similar_to_liked_ad, + finn_compare_ads, + finn_save_feedback, + finn_get_new_ads_since_last_run, + ] + + for tool in async_tools: + assert asyncio.iscoroutinefunction(tool), f"{tool.__name__} should be async" + + def test_sync_tools_are_not_async(self): + """Verify sync tools are not async functions.""" + import inspect + + sync_tools = [ + finn_decode_unit_vector, + finn_get_shortlist, + ] + + for tool in sync_tools: + assert not asyncio.iscoroutinefunction(tool), f"{tool.__name__} should not be async" + + +class TestServiceLayerIntegration: + """Test that service layer functions work with actual implementations.""" + + def test_analyze_search_does_not_have_unsupported_parameters(self): + """Verify analyze_search no longer has unsupported parameters.""" + # This is a regression test for the include_similar_units_for_shortlist bug + import inspect + + sig = inspect.signature(service.analyze_search) + + # The parameter should not exist in the service layer + assert "include_similar_units_for_shortlist" not in sig.parameters + + # But should still have the main parameters + assert "search_url" in sig.parameters + assert "include_details" in sig.parameters + assert "include_eiendom_no" in sig.parameters + + +class TestParameterDefaults: + """Test that MCP tools have correct default parameters.""" + + def test_finn_analyze_search_defaults(self): + """Verify finn_analyze_search has correct parameter defaults.""" + import inspect + + sig = inspect.signature(finn_analyze_search) + + assert sig.parameters["max_pages"].default == 3 + assert sig.parameters["detail_limit"].default == 20 + assert sig.parameters["include_details"].default is True + assert sig.parameters["include_eiendom_no"].default is True + + def test_finn_get_ad_defaults(self): + """Verify finn_get_ad has correct parameter defaults.""" + import inspect + + sig = inspect.signature(finn_get_ad) + + assert sig.parameters["force_refresh"].default is False + + def test_finn_analyze_ad_defaults(self): + """Verify finn_analyze_ad has correct parameter defaults.""" + import inspect + + sig = inspect.signature(finn_analyze_ad) + + assert sig.parameters["include_eiendom_no"].default is True + assert sig.parameters["include_similar_units"].default is False + + def test_finn_find_similar_to_liked_ad_defaults(self): + """Verify finn_find_similar_to_liked_ad has correct parameter defaults.""" + import inspect + + sig = inspect.signature(finn_find_similar_to_liked_ad) + + assert sig.parameters["mode"].default == "recommendations" + assert sig.parameters["listing_status"].default == "FOR_SALE" + + def test_finn_compare_ads_defaults(self): + """Verify finn_compare_ads has correct parameter defaults.""" + import inspect + + sig = inspect.signature(finn_compare_ads) + + assert sig.parameters["include_eiendom_no"].default is True + assert sig.parameters["include_comps"].default is True + + def test_finn_get_shortlist_defaults(self): + """Verify finn_get_shortlist has correct parameter defaults.""" + import inspect + + sig = inspect.signature(finn_get_shortlist) + + assert sig.parameters["run_id"].default is None + assert sig.parameters["limit"].default == 10 + + def test_finn_get_similar_units_defaults(self): + """Verify finn_get_similar_units has correct parameter defaults.""" + import inspect + + sig = inspect.signature(finn_get_similar_units) + + assert sig.parameters["listing_status"].default == "RECENTLY_SOLD" diff --git a/validate_mcp_tools.py b/validate_mcp_tools.py new file mode 100644 index 0000000..e3ee568 --- /dev/null +++ b/validate_mcp_tools.py @@ -0,0 +1,152 @@ +#!/usr/bin/env python3 +""" +Validate that all MCP tool definitions correctly match their service layer functions. +This catches parameter mismatches, missing arguments, and other integration issues. +""" + +import inspect +from typing import get_type_hints +from finn_eiendom import mcp_server, service, eiendom_no + +# Define the mapping of MCP tools to their service/module functions +TOOL_MAPPINGS = { + # Tool name: (service function, expected params to check) + "finn_analyze_search": ( + service.analyze_search, + ["search_url", "max_pages", "detail_limit", "include_details", "include_eiendom_no"], + ), + "finn_get_ad": (service.get_or_fetch_ad, ["finnkode", "force_refresh"]), + "finn_resolve_eiendom_unit": (eiendom_no.search_unit_from_finn_url, ["finn_url"]), + "finn_get_eiendom_unit": (service.get_or_fetch_eiendom_unit, ["unit_code", "force_refresh"]), + "finn_analyze_unit_images": (service.get_unit_images, ["unit_code", "force_refresh"]), + "finn_get_similar_units": (eiendom_no.get_similar_units, ["unit_vector", "listing_status"]), + "finn_build_unit_vector": ( + eiendom_no.get_unit, + ["unit_code"], + ), # Uses get_unit, not build_unit_vector + "finn_decode_unit_vector": (eiendom_no.decode_unit_vector, ["unit_vector"]), + "finn_analyze_ad": ( + service.analyze_ad, + ["finnkode", "include_eiendom_no", "include_similar_units"], + ), + "finn_analyze_ad_against_comps": ( + service.analyze_ad_against_comps, + ["finnkode", "listing_status"], + ), + "finn_find_similar_to_liked_ad": ( + service.find_similar_to_liked, + ["finnkode", "mode", "listing_status"], + ), + "finn_compare_ads": (service.compare_ads, ["finnkoder", "include_eiendom_no", "include_comps"]), + "finn_save_feedback": (service.save_feedback, ["finnkode", "verdict", "notes"]), + "finn_get_shortlist": (service.get_shortlist, ["run_id", "limit"]), + "finn_get_new_ads_since_last_run": (service.get_new_ads_since_last_run, ["search_url"]), +} + + +def get_function_params(func) -> dict: + """Extract parameter names and defaults from a function.""" + sig = inspect.signature(func) + params = {} + for name, param in sig.parameters.items(): + if name in ("self", "cls"): + continue + params[name] = { + "default": param.default, + "annotation": param.annotation, + "kind": param.kind.name, + } + return params + + +def validate_tool_mapping( + tool_name: str, service_func, expected_params: list[str] +) -> tuple[bool, list[str]]: + """Validate that an MCP tool correctly maps to its service function.""" + errors = [] + + # Get the MCP tool function + mcp_tool = getattr(mcp_server, tool_name, None) + if not mcp_tool: + errors.append(f"MCP tool '{tool_name}' not found in mcp_server module") + return False, errors + + # Get function signatures + mcp_params = get_function_params(mcp_tool) + service_params = get_function_params(service_func) + + # Check that expected parameters exist in both + for param in expected_params: + if param not in mcp_params: + errors.append(f" ✗ MCP tool missing parameter '{param}'") + if param not in service_params and param != "client": # client is optional in service layer + errors.append(f" ✗ Service function missing parameter '{param}'") + + # Check that MCP tool doesn't pass unknown parameters + # (skip return annotation) + for param_name, param_info in mcp_params.items(): + if param_name not in service_params and param_name not in ["return"]: + # This might be OK if it's a tool-specific parameter, but warn + pass + + if errors: + return False, errors + return True, [] + + +async def validate_service_imports(): + """Validate that all imported service functions exist and are callable.""" + imported_funcs = [ + ("analyze_ad", service.analyze_ad), + ("analyze_ad_against_comps", service.analyze_ad_against_comps), + ("analyze_search", service.analyze_search), + ("compare_ads", service.compare_ads), + ("find_similar_to_liked", service.find_similar_to_liked), + ("get_new_ads_since_last_run", service.get_new_ads_since_last_run), + ("get_or_fetch_ad", service.get_or_fetch_ad), + ("get_or_fetch_eiendom_unit", service.get_or_fetch_eiendom_unit), + ("get_shortlist", service.get_shortlist), + ("get_unit_images", service.get_unit_images), + ("save_feedback", service.save_feedback), + ] + + errors = [] + for name, func in imported_funcs: + if not callable(func): + errors.append(f"Service function '{name}' is not callable") + + return errors + + +def main(): + """Run validation checks.""" + print("=" * 80) + print("MCP Tool Parameter Validation") + print("=" * 80) + + all_passed = True + total_checks = 0 + passed_checks = 0 + + for tool_name, (service_func, expected_params) in TOOL_MAPPINGS.items(): + total_checks += 1 + passed, errors = validate_tool_mapping(tool_name, service_func, expected_params) + + if passed: + print(f"✓ {tool_name}") + passed_checks += 1 + else: + print(f"✗ {tool_name}") + for error in errors: + print(f" {error}") + all_passed = False + + print("\n" + "=" * 80) + print(f"Results: {passed_checks}/{total_checks} tools validated") + print("=" * 80) + + return 0 if all_passed else 1 + + +if __name__ == "__main__": + exit(main())