Skip to content

Commit a8abc90

Browse files
Add Copilot instructions file for project guidelines and conventions (#378)
1 parent 9e0046c commit a8abc90

1 file changed

Lines changed: 350 additions & 0 deletions

File tree

.github/copilot-instructions.md

Lines changed: 350 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,350 @@
1+
# Copilot Instructions — Prisma Decision API
2+
3+
This repository contains two sub-projects that work together:
4+
5+
- **`PrismaDotnetApi/`** — A C# ASP.NET Core Web API (Clean Architecture)
6+
- **`PrismaFastApi/`** — A Python FastAPI service for Bayesian inference / influence-diagram solving
7+
8+
The .NET API calls the Python API over HTTP for all solver operations.
9+
10+
---
11+
12+
## Architecture
13+
14+
### .NET — Clean Architecture layers
15+
16+
| Layer | Project | Responsibility |
17+
|---|---|---|
18+
| Presentation | `PrismaApi.Api` | Controllers, attributes, security policies |
19+
| Application | `PrismaApi.Application` | Services, repositories, mapping, background jobs |
20+
| Domain | `PrismaApi.Domain` | Entities, DTOs, interfaces, constants |
21+
| Infrastructure | `PrismaApi.Infrastructure` | EF Core context, caching, DB utilities |
22+
23+
Dependencies always point inward. Infrastructure and Application implement interfaces defined in Domain.
24+
25+
### Python — FastAPI layers
26+
27+
| Layer | Location | Responsibility |
28+
|---|---|---|
29+
| Routes | `src/routes/` | FastAPI routers, request/response handling |
30+
| Services | `src/services/` | Business and solver logic |
31+
| DTOs | `src/dtos/` | Pydantic models |
32+
| Utils | `src/utils/` | Shared helpers |
33+
34+
---
35+
36+
## C# Conventions
37+
38+
### Naming
39+
40+
- **Classes, interfaces, methods, properties, enums**: PascalCase
41+
- **Interfaces**: `I` prefix — `IProjectService`, `ICrudRepository<T, TId>`
42+
- **Async methods**: `*Async` suffix — `GetAsync()`, `CreateAsync()`
43+
- **Private fields**: `_camelCase``_projectRepository`, `_cache`
44+
- **Method parameters & locals**: camelCase
45+
- **`CancellationToken` parameter**: always named `ct`
46+
- **Constants** in static classes: PascalCase — `MaxShortStringLength`
47+
48+
### DTOs
49+
50+
Each entity has a family of DTOs using inheritance:
51+
52+
```csharp
53+
public class ProjectDto { /* shared base fields */ }
54+
public class ProjectCreateDto : ProjectDto { /* POST body */ }
55+
public class ProjectIncomingDto : ProjectDto { /* PUT body */ }
56+
public class ProjectOutgoingDto : ProjectDto { /* response */ }
57+
public class PopulatedProjectDto : ProjectDto { /* response with nested entities */ }
58+
```
59+
60+
JSON serialization uses `[JsonPropertyName("snake_case")]` attributes.
61+
62+
### Entities
63+
64+
```csharp
65+
// Hierarchy
66+
IBaseEntity<TId>
67+
└── BaseEntity // Id, CreatedAt, UpdatedAt
68+
└── AuditableEntity // + CreatedById, UpdatedById, navigation properties
69+
70+
public class Project : AuditableEntity
71+
{
72+
// EF Core fluent config lives in the entity class
73+
public static void OnModelConfiguring(ModelBuilder modelBuilder) { ... }
74+
75+
// Navigation collections initialised inline
76+
public ICollection<ProjectRole> ProjectRoles { get; set; } = new List<ProjectRole>();
77+
}
78+
```
79+
80+
### Repository pattern
81+
82+
```csharp
83+
// Generic base interface — defined in Domain
84+
public interface ICrudRepository<TEntity, TId>
85+
{
86+
Task<TEntity?> GetByIdAsync(TId id, bool withTracking = true,
87+
Expression<Func<TEntity, bool>>? filterPredicate = null, CancellationToken ct = default);
88+
Task<TEntity> AddAsync(TEntity entity, CancellationToken ct);
89+
Task UpdateRangeAsync(IEnumerable<TEntity> entities, CancellationToken ct);
90+
Task DeleteAsync(TEntity entity, CancellationToken ct);
91+
// ...
92+
}
93+
94+
// Entity-specific interface extends the generic one
95+
public interface IProjectRepository : ICrudRepository<Project, Guid>
96+
{
97+
Task<ICollection<Project>> GetProjectsWhereUserHasAccess(string userId, CancellationToken ct);
98+
}
99+
```
100+
101+
The `filterPredicate` parameter is used for row-level authorization — pass it instead of filtering after the fact.
102+
103+
### Service pattern
104+
105+
```csharp
106+
public class ProjectService : IProjectService
107+
{
108+
private readonly IProjectRepository _projectRepository;
109+
private readonly IMemoryCache _cache;
110+
111+
public ProjectService(IProjectRepository projectRepository, IMemoryCache cache)
112+
{
113+
_projectRepository = projectRepository;
114+
_cache = cache;
115+
}
116+
117+
// Every public method: async, accepts UserOutgoingDto for authorization, accepts CancellationToken
118+
public async Task<List<ProjectOutgoingDto>> CreateAsync(
119+
List<ProjectCreateDto> dtos,
120+
UserOutgoingDto userDto,
121+
CancellationToken ct = default)
122+
{ ... }
123+
}
124+
```
125+
126+
- All methods are `async Task<T>`.
127+
- Every mutating method receives the current `UserOutgoingDto` for authorization checks.
128+
- Methods accept and return `List<Dto>` (batch operations).
129+
- Use `IMemoryCache` for caching; invalidate on mutations.
130+
131+
### Controller pattern
132+
133+
```csharp
134+
[ApiController]
135+
[Route("")]
136+
public class ProjectsController : PrismaBaseEntityController
137+
{
138+
[HttpPost("projects")]
139+
public async Task<ActionResult<List<ProjectOutgoingDto>>> CreateProjects(
140+
[FromBody] List<ProjectCreateDto> dtos,
141+
CancellationToken ct = default)
142+
{
143+
UserOutgoingDto user = HttpContext.GetLoadedUser();
144+
await BeginTransactionAsync(ct);
145+
try
146+
{
147+
var result = await _projectService.CreateAsync(dtos, user, ct);
148+
await CommitTransactionAsync(ct);
149+
return Ok(result);
150+
}
151+
catch
152+
{
153+
await RollbackTransactionAsync(CancellationToken.None);
154+
throw;
155+
}
156+
}
157+
}
158+
```
159+
160+
- Always inherit `PrismaBaseController` or `PrismaBaseEntityController`.
161+
- `[LoadUser]` and `[ApiExceptionFilter]` are applied on the base controller — do not add them individually.
162+
- Always wrap write operations in a transaction (Begin / Commit / Rollback in catch).
163+
- Retrieve the current user with `HttpContext.GetLoadedUser()`.
164+
- Return `ActionResult<List<Dto>>` — never raw types.
165+
166+
### Mapping pattern
167+
168+
Conversions live in static extension-method classes named `*MappingExtensions`:
169+
170+
```csharp
171+
public static class ProjectMappingExtensions
172+
{
173+
public static ProjectOutgoingDto ToOutgoingDto(this Project entity) { ... }
174+
public static List<ProjectOutgoingDto> ToOutgoingDtos(this IEnumerable<Project> entities)
175+
=> entities.Select(e => e.ToOutgoingDto()).ToList();
176+
public static Project ToEntity(this ProjectCreateDto dto, UserOutgoingDto user) { ... }
177+
}
178+
```
179+
180+
Never put mapping logic in controllers or services directly.
181+
182+
### Testing
183+
184+
- xUnit with `[Fact]` (single case) and `[Theory]` + `[InlineData]` (parameterised).
185+
- Tests share infrastructure via `IClassFixture<PrismaApiFixture>` and `[Collection(nameof(PrismaCollection))]`.
186+
- Test method naming: `MethodName_StateUnderTest_ExpectedResult` — e.g. `GetProject_ReturnsProject`, `GetProjectWithoutAccess_ReturnsNotFound`.
187+
- Use `TestClientGetAsync<T>` / `TestClientPostAsync<T>` extension helpers rather than raw `HttpClient`.
188+
- Assert HTTP status code first, then payload properties.
189+
190+
---
191+
192+
## Python Conventions
193+
194+
### Naming
195+
196+
- **Classes**: PascalCase — `SolverService`, `DecisionTreeCreator`
197+
- **Functions & methods**: snake_case — `find_optimal_decisions`, `build_inference_engine`
198+
- **Variables & parameters**: snake_case — `project_id`, `issue_dtos`
199+
- **Module files**: snake_case — `solver_service.py`, `decision_dtos.py`
200+
- **Constants / enum values**: UPPER_SNAKE_CASE
201+
202+
### DTOs (Pydantic)
203+
204+
Mirror the C# DTO family pattern:
205+
206+
```python
207+
class IssueDto(BaseModel):
208+
id: uuid.UUID = Field(default_factory=uuid.uuid4)
209+
project_id: uuid.UUID
210+
211+
class IssueIncomingDto(IssueDto):
212+
type: str
213+
214+
class IssueOutgoingDto(IssueDto):
215+
type: str
216+
decision: Optional[DecisionOutgoingDto]
217+
uncertainty: Optional[UncertaintyOutgoingDto]
218+
```
219+
220+
### FastAPI routes
221+
222+
```python
223+
router = APIRouter(tags=["solvers"])
224+
225+
@router.post("/solvers/project/{project_id}")
226+
async def get_optimal_decisions(
227+
issues: list[IssueOutgoingDto],
228+
edges: list[EdgeOutgoingDto],
229+
solver_service: SolverService = Depends(get_solver_service),
230+
) -> SolutionDto:
231+
return await solver_service.find_optimal_decision_pyagrum_from_dtos(issues, edges)
232+
```
233+
234+
- Use `Depends()` for all service injection.
235+
- Declare the return type annotation on every route handler — FastAPI uses it for serialization and OpenAPI docs.
236+
- One `APIRouter` per resource file.
237+
238+
---
239+
240+
## Python Code Quality Standards
241+
242+
These standards apply to all new and modified Python code. Existing code may not yet meet them — always apply them when touching a file.
243+
244+
### Typing — use modern Python 3.11+ style
245+
246+
Use built-in generic types and `typing.Optional` for nullable values. **Never** import `List`, `Dict`, `Tuple`, or `Set` from `typing` — use the built-in lowercase equivalents instead.
247+
248+
```python
249+
# Wrong
250+
from typing import List
251+
def process(items: List[str]) -> str | None: ...
252+
253+
# Correct
254+
from typing import Optional
255+
def process(items: list[str]) -> Optional[str]: ...
256+
```
257+
258+
Use `Optional[T]` for nullable values rather than `T | None`. Use `Union[X, Y]` for non-nullable unions of multiple types.
259+
260+
Every function and method **must** have a return type annotation, including `-> None` for void functions and `-> Self` where appropriate.
261+
262+
Always import `Optional` and `Union` from `typing`. Only import other items from `typing` that have no built-in equivalent: `TypeVar`, `Protocol`, `overload`, `TYPE_CHECKING`, `cast`, `Final`, `Literal`, `TypeAlias`.
263+
264+
### No `Any`
265+
266+
Avoid `Any` as a type. Use `TypeVar`, specific unions, `object`, or `Protocol` instead. When interfacing with untyped third-party libraries (e.g. `pyagrum`), constrain `# type: ignore` to the narrowest possible line and add a comment explaining why.
267+
268+
```python
269+
# Wrong
270+
def get_labels(node: Any) -> Any: ...
271+
272+
# Correct
273+
def get_labels(node: int | str) -> tuple[str, ...]: ...
274+
```
275+
276+
### Avoid imperative for-loops for collection transforms
277+
278+
Use comprehensions and `itertools` for building, filtering, and transforming collections.
279+
280+
**Flattening** — use `itertools.chain.from_iterable`, not a for-loop with `.extend()`:
281+
282+
```python
283+
# Wrong — side-effect-in-comprehension antipattern
284+
states: list[OptionOutgoingDto] = []
285+
[states.extend(issue.decision.options) for issue in issues if issue.decision]
286+
287+
# Correct
288+
from itertools import chain
289+
states = list(chain.from_iterable(
290+
issue.decision.options for issue in issues if issue.decision is not None
291+
))
292+
```
293+
294+
**Filtering + transforming** — use a comprehension or `map`/`filter`:
295+
296+
```python
297+
# Wrong
298+
result = []
299+
for issue in issues:
300+
if issue.type == "Decision":
301+
result.append(issue.id)
302+
303+
# Correct
304+
result = [issue.id for issue in issues if issue.type == "Decision"]
305+
```
306+
307+
**Finding the first match** — use `next()` with a default:
308+
309+
```python
310+
# Wrong — allocates the full filtered list
311+
state = [s for s in states if str(s.id) == state_id][0]
312+
313+
# Correct
314+
state = next((s for s in states if str(s.id) == state_id), None)
315+
```
316+
317+
Imperative loops are acceptable when the body has genuine side effects (e.g. mutating a CPT in-place via a third-party API), is too complex to read as a comprehension, or when building a result that requires intermediate mutable state across iterations.
318+
319+
### No mutable default arguments
320+
321+
```python
322+
# Wrong — the default list is shared across all calls
323+
async def get_solutions(self, evidence: list[list[uuid.UUID]] = []) -> list[SolutionDto]: ...
324+
325+
# Correct
326+
async def get_solutions(self, evidence: Optional[list[list[uuid.UUID]]] = None) -> list[SolutionDto]:
327+
if evidence is None:
328+
evidence = []
329+
```
330+
331+
### No class-level mutable state
332+
333+
Mutable collections must be instance attributes, initialised in `__init__`, not class-level attributes (which are shared across all instances):
334+
335+
```python
336+
# Wrong
337+
class Manager:
338+
all_ids: set[str] = set() # shared across all instances!
339+
340+
# Correct
341+
class Manager:
342+
def __init__(self) -> None:
343+
self.all_ids: set[str] = set()
344+
```
345+
346+
### Async
347+
348+
- Route handlers and service methods that perform I/O must be `async def`.
349+
- Do not mix sync blocking calls inside `async def` — use `asyncio.to_thread()` if you must call a sync library.
350+
- Do not use `asyncio.sleep(0)` as a polling mechanism.

0 commit comments

Comments
 (0)