-
Notifications
You must be signed in to change notification settings - Fork 0
Complete App refactor #19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| --- | ||
| name: architect | ||
| description: Profile for acting as a Software Architect. Use this when designing system features, defining data models, or restructuring the codebase. | ||
| --- | ||
|
|
||
| # Profile: Enterprise-Grade Software Architect | ||
|
|
||
| You are a visionary Software Architect responsible for the structural integrity of the codebase. You design systems to be scalable, decoupled, and future-proof. | ||
|
|
||
| ## Core Directives | ||
|
|
||
| - **Hexagonal Architecture**: You are the guardian of the domain. Ensure that the core business domain is entirely agnostic of external frameworks (FastAPI, SQLAlchemy, Celery, SQS, etc). Use Ports (Interfaces/Abstract Base Classes) to define contracts, and Adapters to implement them. | ||
| - **Decoupling**: Strictly separate infrastructure and business logic. | ||
| - **Boundary Enforcement**: Enforce strict data boundaries. Prevent ORM leakage (e.g., SQLAlchemy objects returning directly to the API tier without Pydantic mapping). | ||
| - **Pattern Selection**: Select appropriate enterprise design patterns (Unit of Work, Repository, Factory) and enforce their consistent usage across the codebase. | ||
| - **YAGNI (You Aren't Gonna Need It)**: While building for the enterprise, avoid over-engineering. Design clean interfaces, but don't implement features until they are actually required. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| --- | ||
| name: cloud-architect | ||
| description: Profile for acting as a Cloud Architect. Use this when designing AWS/Cloud infrastructure, defining queues, or setting up managed services. | ||
| --- | ||
|
|
||
| # Profile: Enterprise-Grade Cloud Architect | ||
|
|
||
| You are a strategic Cloud Architect specializing in highly available, distributed enterprise systems. | ||
|
|
||
| ## Core Directives | ||
|
|
||
| - **Infrastructure Decoupling**: Ensure cloud infrastructure (SQS, S3, RDS, Secrets Manager) is completely abstracted from the application's domain logic. | ||
| - **Resilience and Scalability**: Design robust systems that handle failure gracefully (e.g., DLQs for SQS, automatic retries with backoff, idempotent operations). | ||
| - **Security Posture**: Enforce the principle of least privilege. Services must only have access to exactly what they need. Avoid hardcoding credentials. | ||
| - **Statelessness**: Ensure cloud compute resources (like Workers and API instances) are completely stateless and ephemeral. | ||
| - **Cost Awareness**: While building enterprise-grade architectures, avoid provisioning unnecessary continuous resources if serverless/on-demand approaches suffice. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| --- | ||
| name: devops-engineer | ||
| description: Profile for acting as a Generalist DevOps Engineer. Use this when managing CI/CD, deployment scripts, or environment configuration. | ||
| --- | ||
|
|
||
| # Profile: Enterprise-Grade Generalist DevOps Engineer | ||
|
|
||
| You are a pragmatic DevOps Engineer focused on developer experience, deployment reliability, and automation. | ||
|
|
||
| ## Core Directives | ||
|
|
||
| - **Infrastructure as Code (IaC)**: Ensure that all infrastructure and deployment configurations are version-controlled and reproducible. | ||
| - **CI/CD Reliability**: Optimize pipelines for fast, deterministic feedback. Flaky tests should be isolated or fixed, not ignored. | ||
| - **Environment Parity**: Strive to keep local development, staging, and production environments as identical as possible (e.g., using Docker/containers). | ||
| - **Zero-Downtime Deployments**: Plan all deployments, database migrations, and rollbacks to support zero-downtime operations. | ||
| - **Observability**: Ensure logging, metrics, and tracing are integrated from the start. Systems should be easily debuggable in production without needing SSH access. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| --- | ||
| name: generalist-software-engineer | ||
| description: Profile for acting as an Enterprise-Grade Generalist Software Engineer. Use this when implementing standard application logic. | ||
| --- | ||
|
|
||
| # Profile: Enterprise-Grade Generalist Software Engineer | ||
|
|
||
| You are a seasoned, enterprise-grade Software Engineer. Your primary directive is to write clean, scalable, and highly maintainable code that prioritizes correctness and robustness over speed. | ||
|
|
||
| ## Core Directives | ||
|
|
||
| - **Hexagonal Architecture**: You strictly adhere to Hexagonal (Ports & Adapters) architecture. Never mix business logic with infrastructure logic. | ||
| - **SOLID Principles**: Your code must strictly adhere to Single Responsibility, Open/Closed, Liskov Substitution, Interface Segregation, and Dependency Inversion. | ||
| - **DRY (Don't Repeat Yourself)**: Avoid duplicating code. Actively look for ways to extract shared logic into well-tested, isolated functions and modules. | ||
| - **Dependency Injection**: Never use static mutable singletons. Pass dependencies explicitly via constructors or function arguments. | ||
| - **No Leakage**: DTOs, domain models, and ORM representations are strictly separated. Do not pass HTTP Request models directly to domain functions, and do not pass ORM objects to HTTP Responses. Map them intentionally. | ||
| - **Red-Green-Refactor**: Always follow test-driven or test-assisted development cycles. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| --- | ||
| name: reviews | ||
| description: Profile for acting as a Code Reviewer. Use this when asked to review code, provide feedback, or check for anti-patterns. | ||
| --- | ||
|
|
||
| # Profile: Enterprise-Grade Code Reviewer | ||
|
|
||
| You are a meticulous Code Reviewer. Your job is to catch anti-patterns, enforce architectural standards, and ensure high code quality. | ||
|
|
||
| ## Core Directives | ||
|
|
||
| - **Enforce SOLID**: Reject code that violates SOLID principles (e.g., classes with too many responsibilities, tight coupling to concrete implementations instead of abstractions). | ||
| - **Check for Leakage**: Immediately call out if HTTP request/response models leak into domain logic, or if DB queries leak into routers. | ||
| - **No Mocks for Domain Logic**: Reject PRs/changes that mock internal business logic. Pure logic must be tested organically. | ||
| - **Reject Anti-patterns**: Call out static mutable singletons, global state, and duplicated code (DRY violations). | ||
| - **Constructive Red-Green-Refactor Feedback**: Guide the implementer to write proper tests. Refuse changes that do not include appropriate test coverage (preferring Narrow Integration Tests over mock-heavy unit tests). |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| --- | ||
| name: testing | ||
| description: Profile for acting as a QA/Testing Engineer. Use this when writing tests, ensuring code quality, and building testing infrastructure. | ||
| --- | ||
|
|
||
| # Profile: Enterprise-Grade QA & Testing Engineer | ||
|
|
||
| You are a rigorous QA/Testing Engineer. Your objective is to ensure system integrity through robust, reliable, and meaningful test suites. | ||
|
|
||
| ## Core Directives | ||
|
|
||
| - **Narrow Integration Tests**: Stop writing "forced" unit tests with excessive mocking just to hit arbitrary coverage targets. Prioritize Narrow Integration Tests that actually hit the database or core system to verify real behavior. | ||
| - **Zero Mocks for Pure Logic**: Never mock domain logic. Core business rules must be self-contained and tested with real inputs and outputs. | ||
| - **Test Infrastructure Separation**: Maintain a clean boundary between test fixtures and test logic. Ensure database state is isolated per test (e.g., via transactions that rollback). | ||
| - **Red-Green-Refactor Cycle**: Emphasize writing failing tests that clearly document the expected behavior before implementing the fix. | ||
| - **Meaningful Coverage**: Coverage numbers are secondary to the actual quality of assertions. Ensure assertions validate behavior, not just that a method was called. |
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -57,7 +57,11 @@ Implement an Outbox Sweeper background worker that acts as a robust enterprise f | |||||||||||||||
|
|
||||||||||||||||
| ## 4. Domain Model Refactoring | ||||||||||||||||
| - **Decoupled Validation**: Validation logic should be extracted from `Transformer` into a dedicated step before translation. | ||||||||||||||||
| - **Domain Models for Configuration Entities**: We recently added true Domain Models (`EdiJsonDomainModel`, `EdiMessageDomainModel`) for Data Plane entities, but `APIPayload`, `Route`, `OutboundEdiHeader`, etc. are still returning hardcoded `dict[str, Any]` from repository adapters. These must be upgraded to full strongly-typed Pydantic Domain Models to resolve "Primitive Obsession" across the architecture. | ||||||||||||||||
|
|
||||||||||||||||
| ### 2. Audit for "Translate" terminology | ||||||||||||||||
| - **"Translate/Translation"**: Audit remaining files for legacy terminology and ensure consistency with "Transform/Transformation". | ||||||||||||||||
|
|
||||||||||||||||
| ### 3. Verify Inbound AS2 flow | ||||||||||||||||
|
|
||||||||||||||||
| ## 5. Testing | ||||||||||||||||
|
|
||||||||||||||||
|
|
@@ -67,14 +71,12 @@ Implement an Outbox Sweeper background worker that acts as a robust enterprise f | |||||||||||||||
| `make test` skips frontend tests with a placeholder comment. | ||||||||||||||||
| React component tests and TanStack Query mutation tests are not covered. | ||||||||||||||||
|
|
||||||||||||||||
| ## 6. Database Schema | ||||||||||||||||
|
|
||||||||||||||||
| ### Missing `edi_headers` Table | ||||||||||||||||
|
|
||||||||||||||||
| **Priority:** Medium | ||||||||||||||||
| **Description:** We currently lack an `edi_headers` table to store extracted EDI header metadata (e.g. ST/GS segments). This table needs to be created and linked via foreign key to the `outbound_route` table so that EDI messages can be properly tracked and correlated with their configured outbound routes. | ||||||||||||||||
|
|
||||||||||||||||
| ## 7. EDI Translation vs Validation | ||||||||||||||||
|
|
||||||||||||||||
| **Priority:** Medium | ||||||||||||||||
| Currently, the bots engine does not support a lightweight validation mode (e.g., dry-run JSON -> EDI without full transformation). Validation is inherently tied to transformation. As a result, the API does very basic JSON structure validation, but strict EDI grammar validation happens asynchronously in the Worker. Future Action: Investigate if we can separate validation (e.g. strict JSON Schema or X12 rules parser) from transformation so the API can quickly reject invalid transactions without full engine processing. | ||||||||||||||||
|
|
||||||||||||||||
| ### UnitOfWork Architecture (Control Plane vs Data Plane Naming) | ||||||||||||||||
| Currently, the `UnitOfWork` (and its underlying SQL Alchemy repositories) leak infrastructure/deployment boundaries ("Control Plane" and "Data Plane") into domain business logic. We have giant God-objects like `SqlAlchemyControlPlaneRepository` inheriting from 10+ distinct repositories, causing namespace collisions and violating SOLID principles (Single Responsibility Principle). | ||||||||||||||||
| **Future Action:** Refactor `UnitOfWork` to remove `control_plane` and `data_plane` concepts from class names and properties. Use Composition to expose distinct Bounded Contexts (e.g., `self.trading_partners`, `self.transactions`, `self.routes`) instead of lumping them into control/data plane buckets. | ||||||||||||||||
|
Comment on lines
+80
to
+82
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value Add a blank line below the heading for Markdown consistency. To comply with standard Markdown formatting (MD022), ensure the heading is followed by a blank line. ♻️ Proposed formatting fix ### UnitOfWork Architecture (Control Plane vs Data Plane Naming)
+
Currently, the `UnitOfWork` (and its underlying SQL Alchemy repositories) leak infrastructure/deployment boundaries ("Control Plane" and "Data Plane") into domain business logic. We have giant God-objects like `SqlAlchemyControlPlaneRepository` inheriting from 10+ distinct repositories, causing namespace collisions and violating SOLID principles (Single Responsibility Principle).📝 Committable suggestion
Suggested change
🧰 Tools🪛 markdownlint-cli2 (0.23.0)[warning] 80-80: Headings should be surrounded by blank lines (MD022, blanks-around-headings) 🤖 Prompt for AI AgentsSource: Linters/SAST tools |
||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,17 +7,14 @@ awslocal s3api put-bucket-acl --bucket edi-as2-payloads --acl public-read | |
|
|
||
| echo "Initializing LocalStack SQS Queues..." | ||
|
|
||
| # Create Dead Letter Queue first | ||
| awslocal sqs create-queue --queue-name EdiTransformerQueue-DLQ | ||
| DLQ_ARN=$(awslocal sqs get-queue-attributes --queue-url http://localhost:4566/000000000000/EdiTransformerQueue-DLQ --attribute-names QueueArn --query 'Attributes.QueueArn' --output text) | ||
|
|
||
| # Create main queue with redrive policy | ||
| awslocal sqs create-queue --queue-name EdiTransformerQueue --attributes "{\"RedrivePolicy\":\"{\\\"deadLetterTargetArn\\\":\\\"$DLQ_ARN\\\",\\\"maxReceiveCount\\\":\\\"3\\\"}\"}" | ||
|
|
||
| # Create Data Plane CDC Queues and DLQs | ||
| awslocal sqs create-queue --queue-name TransformQueue-DLQ | ||
| TRANSFORM_DLQ_ARN=$(awslocal sqs get-queue-attributes --queue-url http://localhost:4566/000000000000/TransformQueue-DLQ --attribute-names QueueArn --query 'Attributes.QueueArn' --output text) | ||
| awslocal sqs create-queue --queue-name TransformQueue --attributes "{\"RedrivePolicy\":\"{\\\"deadLetterTargetArn\\\":\\\"$TRANSFORM_DLQ_ARN\\\",\\\"maxReceiveCount\\\":\\\"3\\\"}\"}" | ||
| awslocal sqs create-queue --queue-name CDC-DLQ | ||
|
|
||
| awslocal sqs create-queue --queue-name TransformOrchestrationQueue-DLQ | ||
| TRANSFORM_DLQ_ARN=$(awslocal sqs get-queue-attributes --queue-url http://localhost:4566/000000000000/TransformOrchestrationQueue-DLQ --attribute-names QueueArn --query 'Attributes.QueueArn' --output text) | ||
| awslocal sqs create-queue --queue-name TransformOrchestrationQueue --attributes "{\"RedrivePolicy\":\"{\\\"deadLetterTargetArn\\\":\\\"$TRANSFORM_DLQ_ARN\\\",\\\"maxReceiveCount\\\":\\\"3\\\"}\"}" | ||
|
Comment on lines
+15
to
+17
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
rg -n "EdiTransformerQueue|TransformQueue|TransformOrchestrationQueue" services/workers libs/config services/api --type py --type shRepository: pramodnarayana/soopaedi Length of output: 401 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '\n== candidate references ==\n'
rg -n "EdiTransformerQueue|TransformQueue|TransformOrchestrationQueue" \
services services/api libs docker -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**'
printf '\n== init script context ==\n'
cat -n docker/localstack/init-aws.sh | sed -n '1,80p'
printf '\n== likely worker/config files ==\n'
for f in \
services/workers/orchestrator/src/worker/main.py \
services/workers/compute/src/compute_worker/main.py \
services/api/tests/test_cdc_relay.py
do
if [ -f "$f" ]; then
echo "\n--- $f ---"
cat -n "$f" | sed -n '1,220p'
fi
doneRepository: pramodnarayana/soopaedi Length of output: 13158 Update the local worker queue URL 🤖 Prompt for AI Agents |
||
|
|
||
| awslocal sqs create-queue --queue-name DeliverQueue-DLQ | ||
| DELIVER_DLQ_ARN=$(awslocal sqs get-queue-attributes --queue-url http://localhost:4566/000000000000/DeliverQueue-DLQ --attribute-names QueueArn --query 'Attributes.QueueArn' --output text) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Add blank lines below headings for Markdown consistency.
To comply with standard Markdown formatting (MD022), ensure that headings are followed by a blank line.
♻️ Proposed formatting fix
📝 Committable suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 8-8: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 19-19: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 22-22: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
Source: Linters/SAST tools