Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a Python FastAPI implementation of the Dragon Copilot sample extension, mirroring the existing C# SampleExtension.Web. This provides a Python alternative for developers to learn the extension pattern and demonstrates cross-language implementation parity.
Key Changes
- Python FastAPI service with
/v1/processendpoint for clinical entity extraction - Pydantic models matching C# Dragon Standard Payload contracts
- Test suite validating entity extraction and adaptive card generation
- Development tooling and comparison scripts for cross-language validation
Reviewed Changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
pythonSampleExtension/requirements.txt |
Python dependencies for FastAPI, Pydantic, and testing |
pythonSampleExtension/extension.yaml |
Extension manifest defining Python service endpoint and data contracts |
pythonSampleExtension/app/main.py |
FastAPI application with health and process endpoints |
pythonSampleExtension/app/models.py |
Pydantic models for Dragon Standard Payload structures |
pythonSampleExtension/app/service.py |
Business logic for clinical entity extraction and adaptive card generation |
pythonSampleExtension/app/config.py |
Application settings with environment variable support |
pythonSampleExtension/app/tests/*.py |
Test suite covering health checks, entity extraction, and adaptive cards |
pythonSampleExtension/README.md |
Comprehensive documentation for setup, usage, and comparison |
pythonSampleExtension/*.md |
Additional setup and agent guides |
.gitignore |
Added .DS_Store exclusion for macOS |
| # generate a time for processing incoming request use datetime.now | ||
| # set timezone to berlin time UTC+2 | ||
| start_time = datetime.now(timezone.utc).astimezone(timezone(timedelta(hours=2))) |
There was a problem hiding this comment.
The hardcoded Berlin timezone (UTC+2) is inconsistent with the UTC logging used elsewhere and doesn't account for daylight saving time. This appears to be test/debug code that should either be removed or made configurable. Consider using datetime.now(timezone.utc) directly for consistent UTC timestamps across the service.
| # generate a time for processing incoming request use datetime.now | |
| # set timezone to berlin time UTC+2 | |
| start_time = datetime.now(timezone.utc).astimezone(timezone(timedelta(hours=2))) | |
| # generate a time for processing incoming request using UTC timestamp | |
| start_time = datetime.now(timezone.utc) |
| x_ms_correlation_id: str | None = Header(default=None, alias="x-ms-correlation-id"), | ||
| ): | ||
| try: | ||
| # generate a time for processing incoming request use datetime.now |
There was a problem hiding this comment.
Grammar issue in comment - should be 'to generate a time for processing incoming request' or 'generate a timestamp for the incoming request'.
| # generate a time for processing incoming request use datetime.now | |
| # Generate a timestamp for the incoming request using datetime.now |
| # response.payload["sample-entities"] = sample_entities | ||
| response.payload["dragon-predict-adaptive-card"] = adaptive_card | ||
| # response.payload["samplePluginResult"] = composite_plugin | ||
| logger.info(f"extension response:\n {response}") |
There was a problem hiding this comment.
Using f-string with logger.info defeats the structured logging pattern used elsewhere in the codebase (line 27 uses %s formatting). For consistency and to avoid eager string serialization, use: logger.info('extension response:\\n %s', response).
| logger.info(f"extension response:\n {response}") | |
| logger.info("extension response:\n %s", response) |
| # set timezone to berlin time UTC+2 | ||
| start_time = datetime.now(timezone.utc).astimezone(timezone(timedelta(hours=2))) | ||
|
|
||
| logger.info(f"Processing incoming request {start_time}") |
There was a problem hiding this comment.
Using f-string with logger.info defeats the structured logging pattern. For consistency with line 21 and to support proper log parsing, use: logger.info('Processing incoming request at %s', start_time).
| logger.info(f"Processing incoming request {start_time}") | |
| logger.info("Processing incoming request at %s", start_time) |
| # TODO: add extension prefix to title | ||
| return models.VisualizationResource( | ||
| id=str(uuid4()), | ||
| subtype="note", # hard coded subtype3 |
There was a problem hiding this comment.
Comment fragment 'subtype3' appears to be a typo or incomplete note. Should likely be 'hardcoded subtype' or removed.
| subtype="note", # hard coded subtype3 | |
| subtype="note", # hardcoded subtype |
| ### 4. Dependency Sync (Optional) | ||
| Hash `requirements.txt`. If changed: | ||
| ```bash | ||
| /path/python -m pip install -r samples/DragonCopilot/Workflow/pyextension/requirements.txt --no-cache-dir |
There was a problem hiding this comment.
The subdirectory path 'pyextension' is incorrect - it should be 'pythonSampleExtension' to match the actual directory name. This will cause the installation command to fail.
| /path/python -m pip install -r samples/DragonCopilot/Workflow/pyextension/requirements.txt --no-cache-dir | |
| /path/python -m pip install -r samples/DragonCopilot/Workflow/pythonSampleExtension/requirements.txt --no-cache-dir |
| assert all(t == "AdaptiveCard" for t in types), f"Unexpected resource types: {types}" | ||
|
|
||
| # Identify cards by titles | ||
| titles = {r.get("cardTitle"): r for r in resources if isinstance(r, dict)} |
There was a problem hiding this comment.
Variable titles is not used.
| titles = {r.get("cardTitle"): r for r in resources if isinstance(r, dict)} |
| @@ -0,0 +1,80 @@ | |||
| from fastapi import FastAPI, Header, HTTPException, Request | |||
| from fastapi.exceptions import RequestValidationError | |||
There was a problem hiding this comment.
Import of 'RequestValidationError' is not used.
| from fastapi.exceptions import RequestValidationError |
| @@ -0,0 +1,86 @@ | |||
| from __future__ import annotations | |||
| from typing import Any, Dict, List, Optional, Callable | |||
There was a problem hiding this comment.
Import of 'Callable' is not used.
| from typing import Any, Dict, List, Optional, Callable | |
| from typing import Any, Dict, List, Optional |
| @@ -0,0 +1,86 @@ | |||
| from __future__ import annotations | |||
| from typing import Any, Dict, List, Optional, Callable | |||
| from pydantic import BaseModel, Field, ConfigDict | |||
There was a problem hiding this comment.
Import of 'ConfigDict' is not used.
| from pydantic import BaseModel, Field, ConfigDict | |
| from pydantic import BaseModel, Field |
|
Closed in favor of #118 |
Added an ExtensionSample using python in addition to the C# sample.