update the repo
This commit is contained in:
51
AGENT_SKILL/design_pattern_compliance/CHECKLIST.md
Normal file
51
AGENT_SKILL/design_pattern_compliance/CHECKLIST.md
Normal file
@@ -0,0 +1,51 @@
|
||||
# Checklist: design_pattern_compliance
|
||||
|
||||
## Layered Architecture (Backend)
|
||||
- [ ] No direct calls from presentation layer to data access layer
|
||||
- [ ] Controllers only handle HTTP concerns (validation, serialization, routing)
|
||||
- [ ] Business logic resides in service layer
|
||||
- [ ] Data access layer handles only persistence concerns
|
||||
- [ ] No upward dependencies (data layer does not depend on service layer)
|
||||
|
||||
## Dependency Injection
|
||||
- [ ] No direct instantiation of infrastructure dependencies (e.g., `new MongoClient()`)
|
||||
- [ ] Dependencies are injected (constructor, setter, or framework DI)
|
||||
- [ ] Classes depend on interfaces/protocols, not concrete implementations
|
||||
- [ ] Framework-appropriate DI mechanisms are used (e.g., FastAPI Depends, Spring @Autowired)
|
||||
|
||||
## Interface-Driven Development
|
||||
- [ ] Business logic depends on abstractions (interfaces/repositories)
|
||||
- [ ] No direct infrastructure calls in business logic (no `boto3`, `psycopg2.connect()`, etc. in services)
|
||||
- [ ] Concrete implementations exist for interfaces but are injected, not instantiated internally
|
||||
|
||||
## Frontend State Management (Zustand)
|
||||
- [ ] Global shared state is managed in Zustand stores
|
||||
- [ ] Component state (`useState`) is used only for:
|
||||
- Form inputs and UI controls
|
||||
- Temporary UI state (loading states for local effects, animation states)
|
||||
- Data that is truly isolated to a single component
|
||||
- [ ] Store follows naming convention: `use[Domain]Store`
|
||||
- [ ] Store actions are named as verbs
|
||||
- [ ] No business logic in components that should be in store actions
|
||||
|
||||
## Infrastructure as Code
|
||||
- [ ] Terraform configuration uses a remote backend (GCS, S3, etc.)
|
||||
- [ ] No local `terraform.tfstate` files committed
|
||||
- [ ] Resources are named with environment identifier (e.g., `${var.environment}`)
|
||||
- [ ] IAM roles follow least privilege principle (specific permissions, no wildcards)
|
||||
- [ ] Separate configurations or workspaces for different environments
|
||||
- [ ] All providers and modules have explicit version pins
|
||||
- [ ] Terraform files are formatted (`terraform fmt`)
|
||||
|
||||
## Dependency Pinning
|
||||
- [ ] Python: `requirements.txt` or `pyproject.toml` contains exact versions (no `~`, `>`, `<` without corresponding lock file)
|
||||
- [ ] Python: If using `pyproject.toml` with poetry or pipenv, lock file (`poetry.lock` or `Pipfile.lock`) is present
|
||||
- [ ] Node.js: `package-lock.json` or `yarn.lock` is present and matches `package.json`
|
||||
- [ ] Other languages: Equivalent lock files are present and committed
|
||||
- [ ] No unpinned dependencies that could cause non-reproducible builds
|
||||
|
||||
## Code Quality
|
||||
- [ ] Duplicated code has been considered for extraction
|
||||
- [ ] Preference given to composition over inheritance where applicable
|
||||
- [ ] Utility functions are placed in appropriate shared locations
|
||||
- [ ] No "God classes" or objects with too many responsibilities
|
||||
363
AGENT_SKILL/design_pattern_compliance/EXAMPLES.md
Normal file
363
AGENT_SKILL/design_pattern_compliance/EXAMPLES.md
Normal file
@@ -0,0 +1,363 @@
|
||||
# Examples: design_pattern_compliance
|
||||
|
||||
## Correct Examples
|
||||
|
||||
### Layered Architecture (Python/FastAPI)
|
||||
```python
|
||||
# GOOD: Proper layer separation
|
||||
# presentation layer (api/patients.py)
|
||||
from fastapi import APIRouter, Depends, HTTPException
|
||||
from services.patient_service import PatientService
|
||||
from models.patient import PatientResponse
|
||||
|
||||
router = APIRouter(prefix="/patients", tags=["patients"])
|
||||
|
||||
def get_patient_service() -> PatientService:
|
||||
# Dependency injection - could come from a DI container
|
||||
return PatientService(
|
||||
repository=PatientRepository() # In practice, this would be injected
|
||||
)
|
||||
|
||||
@router.get("/{patient_id}", response_model=PatientResponse)
|
||||
def get_patient(
|
||||
patient_id: str,
|
||||
service: PatientService = Depends(get_patient_service)
|
||||
):
|
||||
"""Controller handles only HTTP concerns."""
|
||||
patient = service.get_patient_by_id(patient_id)
|
||||
if not patient:
|
||||
raise HTTPException(status_code=404, detail="Patient not found")
|
||||
return patient
|
||||
|
||||
|
||||
# service layer (services/patient_service.py)
|
||||
from repositories.patient_repository import PatientRepository
|
||||
from models.patient import PatientModel
|
||||
|
||||
class PatientService:
|
||||
def __init__(self, repository: PatientRepository):
|
||||
self.repository = repository # Dependency injected
|
||||
|
||||
def get_patient_by_id(self, patient_id: str) -> Optional[PatientModel]:
|
||||
# Business logic lives here
|
||||
return self.repository.get_by_id(patient_id)
|
||||
|
||||
|
||||
# data access layer (repositories/patient_repository.py)
|
||||
from models.patient import PatientModel
|
||||
|
||||
class PatientRepository:
|
||||
def get_by_id(self, patient_id: str) -> Optional[PatientModel]:
|
||||
# Data access concerns only
|
||||
# In real implementation, this would query a database
|
||||
pass
|
||||
```
|
||||
|
||||
### Dependency Injection with Interfaces
|
||||
```python
|
||||
# GOOD: Interface-based dependencies
|
||||
from abc import ABC, abstractmethod
|
||||
from typing import Protocol
|
||||
|
||||
# Define interface (protocol)
|
||||
class PatientRepository(Protocol):
|
||||
def get_by_id(self, patient_id: str) -> Optional[PatientModel]: ...
|
||||
def save(self, patient: PatientModel) -> bool: ...
|
||||
|
||||
# Concrete implementation
|
||||
class PostgreSQLPatientRepository:
|
||||
def __init__(self, connection_string: str):
|
||||
self.connection_string = connection_string
|
||||
|
||||
def get_by_id(self, patient_id: str) -> Optional[PatientModel]:
|
||||
# Implementation details
|
||||
pass
|
||||
|
||||
# Service depends on interface, not concrete class
|
||||
class PatientService:
|
||||
def __init__(self, repository: PatientRepository): # Accepts any implementation
|
||||
self.repository = repository
|
||||
|
||||
def get_patient(self, patient_id: str) -> Optional[PatientModel]:
|
||||
return self.repository.get_by_id(patient_id)
|
||||
|
||||
# Usage with dependency injection
|
||||
def get_patient_service() -> PatientService:
|
||||
repo = PostgreSQLPatientRepository(os.getenv("DB_CONNECTION"))
|
||||
return PatientService(repository)
|
||||
```
|
||||
|
||||
### Frontend Zustand Store
|
||||
```javascript
|
||||
// GOOD: Proper Zustand store usage
|
||||
import { create } from 'zustand';
|
||||
|
||||
// Patient store slice
|
||||
const usePatientStore = create((set, get) => ({
|
||||
// State
|
||||
patients: [],
|
||||
loading: false,
|
||||
error: null,
|
||||
|
||||
// Actions
|
||||
setPatients: (patients) => set({ patients }),
|
||||
fetchPatients: async () => {
|
||||
set({ loading: true, error: null });
|
||||
try {
|
||||
const response = await api.getPatients();
|
||||
set({ patients: response.data, loading: false });
|
||||
} catch (err) {
|
||||
set({ error: err.message, loading: false });
|
||||
}
|
||||
},
|
||||
addPatient: (patient) => set((state) => ({
|
||||
patients: [...state.patients, patient],
|
||||
loading: false,
|
||||
error: null
|
||||
})),
|
||||
}));
|
||||
|
||||
// Component using store - ONLY for UI state
|
||||
import { useState } from 'react';
|
||||
import { usePatientStore } from '../store/patientStore';
|
||||
|
||||
export const PatientForm: React.FC = () => {
|
||||
// LOCAL UI state only - correct use of useState
|
||||
const [name, setName] = useState('');
|
||||
const [dob, setDob] = useState('');
|
||||
const [submitting, setSubmitting] = useState(false);
|
||||
|
||||
// GLOBAL state from store
|
||||
const { patients, loading, error, addPatient } = usePatientStore();
|
||||
|
||||
const handleSubmit = async (e) => {
|
||||
e.preventDefault();
|
||||
setSubmitting(true);
|
||||
try {
|
||||
await addPatient({ id: Date.now(), name, dob });
|
||||
setName('');
|
||||
setDob('');
|
||||
} finally {
|
||||
setSubmitting(false);
|
||||
}
|
||||
};
|
||||
|
||||
return (
|
||||
<form onSubmit={handleSubmit}>
|
||||
{/* Form fields using local state */}
|
||||
<input value={name} onChange={(e) => setName(e.target.value)} placeholder="Name" />
|
||||
<input value={dob} onChange={(e) => setDob(e.target.value)} placeholder="Date of Birth" />
|
||||
<button type="submit" disabled={submitting}>
|
||||
{submitting ? 'Adding...' : 'Add Patient'}
|
||||
</button>
|
||||
|
||||
{/* Displaying global state */}
|
||||
{loading && <p>Saving...</p>}
|
||||
{error && <p>Error: {error}</p>}
|
||||
</form>
|
||||
);
|
||||
};
|
||||
```
|
||||
|
||||
### Infrastructure as Code (Terraform)
|
||||
```hcl
|
||||
# GOOD: Proper Terraform practices
|
||||
terraform {
|
||||
# REQUIRED: Remote backend
|
||||
backend "gcs" {
|
||||
bucket = "vkist-terraform-state"
|
||||
prefix = "env/${var.environment}"
|
||||
}
|
||||
|
||||
# REQUIRED: Versioned providers
|
||||
required_providers {
|
||||
google = {
|
||||
source = "hashicorp/google"
|
||||
version = "~> 4.0"
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
variable "environment" {
|
||||
description = "Deployment environment"
|
||||
type = string
|
||||
validation {
|
||||
condition = ["dev", "staging", "prod"].contains(var.upper)
|
||||
error_message = "Environment must be either dev, staging, or prod."
|
||||
}
|
||||
}
|
||||
|
||||
# RESOURCE NAMING with environment
|
||||
resource "google_compute_instance" "vm" {
|
||||
name = "vkist-${var.environment}-backend"
|
||||
machine_type = "e2-medium"
|
||||
|
||||
# ... other configuration
|
||||
}
|
||||
|
||||
# LEAST PRIVILEGE IAM
|
||||
resource "google_service_account" "backend" {
|
||||
account_id = "vkist-backend-${var.environment}"
|
||||
display_name = "VKIST Backend Service Account"
|
||||
}
|
||||
|
||||
# Grant only necessary roles
|
||||
resource "google_project_iam_member" "storage_reader" {
|
||||
role = "roles/storage.objectViewer"
|
||||
member = "serviceAccount:${google_service_account.backend.email}"
|
||||
}
|
||||
|
||||
# VERSIONED MODULES
|
||||
module "network" {
|
||||
source = "terraform-google-modules/network/google"
|
||||
version = "~> 5.0"
|
||||
|
||||
project_id = var.project_id
|
||||
network_name = "vkist-${var.environment}-net"
|
||||
# ... other parameters
|
||||
}
|
||||
```
|
||||
|
||||
### Dependency Pinning
|
||||
```txt
|
||||
# GOOD: requirements.txt with exact versions
|
||||
fastapi==0.104.1
|
||||
uvicorn[standard]==0.24.0
|
||||
pydantic==2.5.0
|
||||
sqlalchemy==2.0.23
|
||||
alembic==1.13.1
|
||||
python-multipart==0.0.6
|
||||
```
|
||||
|
||||
```json
|
||||
// GOOD: package.json with package-lock.json
|
||||
{
|
||||
"name": "vkist-frontend",
|
||||
"version": "1.0.0",
|
||||
"dependencies": {
|
||||
"react": "^18.2.0",
|
||||
"react-dom": "^18.2.0",
|
||||
"zustand": "^4.4.1",
|
||||
"axios": "^1.6.2"
|
||||
}
|
||||
}
|
||||
// package-lock.json ensures exact versions are installed
|
||||
```
|
||||
|
||||
## Incorrect Examples (to avoid)
|
||||
|
||||
### Layer Violation
|
||||
```python
|
||||
# BAD: Controller directly accessing repository (skipping service layer)
|
||||
from fastapi import APIRouter, Depends
|
||||
from repositories.patient_repository import PatientRepository
|
||||
|
||||
router = APIRouter()
|
||||
|
||||
@router.get("/patients/{id}")
|
||||
def get_patient(patient_id: str, repo: PatientRepository = Depends()):
|
||||
# VIOLATION: Presentation layer accessing data layer directly
|
||||
return repo.get_by_id(patient_id)
|
||||
```
|
||||
|
||||
### Direct Infrastructure Instantiation
|
||||
```python
|
||||
# BAD: Service creating its own database connection
|
||||
import psycopg2
|
||||
import os
|
||||
|
||||
class PatientService:
|
||||
def __init__(self):
|
||||
# VIOLATION: Direct instantiation of infrastructure
|
||||
self.conn = psycopg2.connect(
|
||||
host=os.getenv("DB_HOST"),
|
||||
database=os.getenv("DB_NAME"),
|
||||
user=os.getenv("DB_USER"),
|
||||
password=os.getenv("DB_PASSWORD")
|
||||
)
|
||||
|
||||
def get_patient(self, patient_id: str):
|
||||
cursor = self.conn.cursor()
|
||||
cursor.execute("SELECT * FROM patients WHERE id = %s", (patient_id,))
|
||||
return cursor.fetchone()
|
||||
```
|
||||
|
||||
### Frontend Anti-pattern: State in Components
|
||||
```javascript
|
||||
// BAD: Using useState for shared data that should be in store
|
||||
import { useState, useEffect } from 'react';
|
||||
import { fetchPatients } from '../api/patientApi';
|
||||
|
||||
export const PatientList: React.FC = () => {
|
||||
// WRONG: This state is shared and should be in a store
|
||||
const [patients, setPatients] = useState([]);
|
||||
const [loading, setLoading] = useState(false);
|
||||
|
||||
useEffect(() => {
|
||||
const loadPatients = async () => {
|
||||
setLoading(true);
|
||||
try {
|
||||
const data = await fetchPatients();
|
||||
setPatients(data); // This state might be needed by other components
|
||||
setLoading(false);
|
||||
} catch (err) {
|
||||
setError(err.message);
|
||||
setLoading(false);
|
||||
}
|
||||
};
|
||||
|
||||
loadPatients();
|
||||
}, []);
|
||||
|
||||
// ... render logic
|
||||
};
|
||||
|
||||
// If another component needs patients data, it would have to fetch again
|
||||
// or receive it via props - inefficient and not scalable
|
||||
```
|
||||
|
||||
### Infrastructure Mistakes
|
||||
```hcl
|
||||
# BAD: Local state storage (should be remote)
|
||||
terraform {
|
||||
# MISSING: backend configuration
|
||||
# state will be stored locally - VIOLATION
|
||||
}
|
||||
|
||||
# BAD: Hard-coded environment (should be variable)
|
||||
resource "google_compute_network" "vpc" {
|
||||
name = "vkist-prod-network" # What if we want to deploy to staging?
|
||||
}
|
||||
|
||||
# BAD: Overly permissive IAM
|
||||
resource "google_project_iam_member" "admin" {
|
||||
role = "roles/editor" # Too permissive - VIOLATION of least privilege
|
||||
member = "user:admin@example.com"
|
||||
}
|
||||
|
||||
# BAD: Unpinned module version
|
||||
module "network" {
|
||||
source = "terraform-google-modules/network/google"
|
||||
# MISSING: version parameter - could get breaking changes on next run
|
||||
}
|
||||
```
|
||||
|
||||
### Dependency Issues
|
||||
```txt
|
||||
# BAD: Unpinned dependencies in requirements.txt
|
||||
fastapi
|
||||
uvicorn
|
||||
pydantic
|
||||
# Any of these could break on next pip install
|
||||
```
|
||||
|
||||
```json
|
||||
// BAD: Missing or outdated lock file
|
||||
{
|
||||
"name": "my-app",
|
||||
"dependencies": {
|
||||
"lodash": "^4.17.21"
|
||||
// No package-lock.json - versions not locked
|
||||
}
|
||||
}
|
||||
```
|
||||
96
AGENT_SKILL/design_pattern_compliance/SKILL.md
Normal file
96
AGENT_SKILL/design_pattern_compliance/SKILL.md
Normal file
@@ -0,0 +1,96 @@
|
||||
# Skill: design_pattern_compliance
|
||||
|
||||
## Trigger Conditions
|
||||
- Creation of a new class, function, or module
|
||||
- Significant refactoring of existing code (architectural changes)
|
||||
- Adding new infrastructure (Terraform modules)
|
||||
- Creating new state slices
|
||||
)
|
||||
|
||||
## Rules
|
||||
|
||||
### 1. Layered Architecture (Backend)
|
||||
- **Direction of Dependencies**:
|
||||
- Presentation layer (API controllers) → Service layer → Data Access layer
|
||||
- No skipping layers (e.g., controller directly accessing repository)
|
||||
- No upward dependencies (data layer should not depend on service layer)
|
||||
- **Separation of Concerns**:
|
||||
- Controllers handle HTTP concerns only (validation, serialization, routing)
|
||||
- Services contain business logic and orchestration
|
||||
- Data Access/Repositories handle persistence concerns only
|
||||
- Severity: `[ERROR]`
|
||||
|
||||
### 2. Dependency Injection and Inversion of Control
|
||||
- **No Direct Instantiation**:
|
||||
- Classes should not instantiate their dependencies directly (especially infrastructure clients)
|
||||
- Dependencies should be injected via constructor or setter
|
||||
- **Interface-Based Dependencies**:
|
||||
- Depend on abstractions (interfaces/protocols) rather than concrete implementations
|
||||
- Example: Service depends on `DatabaseRepository` interface, not `PostgreSQLRepository` directly
|
||||
- **Framework-Compatible DI**:
|
||||
- Use framework-provided dependency injection (e.g., FastAPI's `Depends`, Spring `@Autowired`)
|
||||
- Avoid service locator pattern
|
||||
- Severity: `[ERROR]`
|
||||
|
||||
### 3. Interface-Driven Development
|
||||
- **Program to Interfaces**:
|
||||
- Concrete classes should implement defined interfaces
|
||||
- Clients should depend on interfaces, not implementations
|
||||
- **No Concrete Infra in Business Logic**:
|
||||
- Business logic should not contain direct calls to infrastructure (e.g., `boto3.client()`, `psycopg2.connect()`)
|
||||
- Infrastructure access should be abstracted behind repository/service interfaces
|
||||
- Severity: `[ERROR]`
|
||||
|
||||
### 4. Frontend State Management (Zustand)
|
||||
- **Store Slices**:
|
||||
- Global state should be split into logical slices using Zustand
|
||||
- Each slice corresponds to a domain (e.g., `usePatientStore`, `useAppointmentStore`)
|
||||
- **No Inline State for Shared Data**:
|
||||
- Component state (`useState`) should only be used for:
|
||||
- UI-specific state (form inputs, toggles, animation states)
|
||||
- Data that is not shared across components
|
||||
- Any data that is shared across components or needed by multiple hooks should be in a store
|
||||
- **Store Structure**:
|
||||
- Follow consistent naming: `use[Domain]Store`
|
||||
- Actions should be named as verbs (e.g., `setPatients`, `fetchAppointments`)
|
||||
- Selectors should be derived and memoized where appropriate
|
||||
- Severity: `[ERROR]`
|
||||
|
||||
### 5. Infrastructure as Code (IaC) Best Practices
|
||||
- **Terraform State**:
|
||||
- All Terraform configurations must use a remote backend (GCS recommended for this project)
|
||||
- No local `terraform.tfstate` files in the repository
|
||||
- **Resource Naming**:
|
||||
- Use consistent naming conventions that include environment (e.g., `vkist-${var.environment}-service`)
|
||||
- **Least Privilege IAM**:
|
||||
- IAM roles and policies must grant only the permissions necessary
|
||||
- Avoid using wildcard permissions (`*`) or overly broad roles
|
||||
- **Environment Separation**:
|
||||
- Use separate workspaces or directories for different environments (e.g., `env/staging`, `env/prod`)
|
||||
- Do not use the same resources for different environments
|
||||
- **Version Pinning**:
|
||||
- All external dependencies must be version-pinned:
|
||||
- Terraform modules: `source = "terraform-google-modules/network/google//version"`
|
||||
- Provider versions explicitly set
|
||||
- Severity: `[ERROR]`
|
||||
|
||||
### 6. Dependency Pinning
|
||||
- **Python**:
|
||||
- `requirements.txt` or `pyproject.toml` must contain exact versions (no wildcards)
|
||||
- Use `pip freeze > requirements.txt` for deterministic builds
|
||||
- **Node.js**:
|
||||
- `package-lock.json` or `yarn.lock` must be committed and up to date
|
||||
- Dependencies in `package.json` should use caret (`^`) or tilde (`~`) ranges appropriately, but lock file ensures reproducibility
|
||||
- **Other Ecosystems**:
|
||||
- Equivalent lock files must be present and committed (e.g., `Pipfile.lock`, `poetry.lock`, `Cargo.lock`)
|
||||
- Severity: `[ERROR]`
|
||||
|
||||
### 7. Code Reusability and Abstraction
|
||||
- **Avoid Duplication**:
|
||||
- Similar code in two or more places should be extracted to a shared utility or component
|
||||
- **Prefer Composition Over Inheritance**:
|
||||
- Especially in complex domain models, favor composing behavior over deep inheritance hierarchies
|
||||
- **Utility Functions**:
|
||||
- Place truly generic utilities in shared locations (e.g., `utils/`, `lib/`)
|
||||
- Avoid creating utility classes with only static methods if not needed in the language
|
||||
- Severity: `[WARN]`
|
||||
Reference in New Issue
Block a user