Files
orion/docs/architecture/architecture-violations-status.md
Samir Boulahtit 86e85a98b8
Some checks failed
CI / ruff (push) Successful in 9s
CI / docs (push) Has been cancelled
CI / deploy (push) Has been cancelled
CI / validate (push) Has been cancelled
CI / dependency-scanning (push) Has been cancelled
CI / pytest (push) Has been cancelled
refactor(arch): eliminate all cross-module model imports in service layer
Enforce MOD-025/MOD-026 rules: zero top-level cross-module model imports
remain in any service file. All 66 files migrated using deferred import
patterns (method-body, _get_model() helpers, instance-cached self._Model)
and new cross-module service methods in tenancy. Documentation updated
with Pattern 6 (deferred imports), migration plan marked complete, and
violations status reflects 84→0 service-layer violations.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-27 06:13:15 +01:00

9.2 KiB

Architecture Violations Status

Date: 2026-02-27 Total Violations: 0 blocking (221 documented/accepted, 84 service-layer cross-module imports resolved) Status: All architecture validation errors resolved

Summary

Fixed 18 violations and documented remaining violations as intentional architectural decisions. On 2026-02-27, resolved all ~84 cross-module model imports in service files using deferred import patterns.

Fixed Violations (18)

JavaScript Centralized Logging

  • static/admin/js/marketplace.js - Replaced 18 console.log calls with adminMarketplaceLog
  • static/admin/js/store-themes.js - Replaced 5 console.log calls with storeThemesLog
  • static/admin/js/settings.js - Replaced 1 console.log call with settingsLog
  • static/admin/js/imports.js - Replaced 13 console.log calls with importsLog

Remaining Violations (221)

Category 1: Transaction Control at API Layer (Intentional)

Violation: API-002 - Database commits in endpoints

Files Affected:

  • app/api/v1/admin/merchants.py (5 occurrences)
  • app/api/v1/admin/stores.py (2 occurrences)
  • Other admin endpoints

Architectural Decision:

This is an intentional and standard pattern in FastAPI applications:

# Service Layer - Business Logic
def update_merchant(self, db: Session, merchant_id: int, data: MerchantUpdate):
    merchant = self.get_merchant_by_id(db, merchant_id)
    # ... business logic ...
    db.flush()  # Flush to get IDs, but don't commit
    return merchant

# API Layer - Transaction Boundary
@router.put("/merchants/{merchant_id}")
async def update_merchant_endpoint(merchant_id: int, data: MerchantUpdate, db: Session = Depends(get_db)):
    merchant = merchant_service.update_merchant(db, merchant_id, data)
    db.commit()  # ✅ ARCH: Commit at API level for transaction control
    return merchant

Benefits:

  1. Service methods can call other service methods within same transaction
  2. API layer can rollback entire transaction on error
  3. Allows for complex multi-step operations
  4. Standard practice recommended by FastAPI documentation

Status: ACCEPTED - Added comments to document intention

Category 2: Raw Dict Responses (Legacy - Low Priority)

Violation: API-001 - Endpoint returns raw dict instead of Pydantic model

Files Affected:

  • app/api/v1/admin/users.py
  • app/api/v1/admin/auth.py
  • app/api/v1/admin/store_themes.py
  • app/api/v1/admin/logs.py
  • app/api/v1/admin/notifications.py
  • Various other endpoints

Reason: These are legacy endpoints created before Pydantic response models were standardized.

Impact: Low - endpoints work correctly, just missing type validation/documentation

Refactoring Plan:

  • Low priority - will refactor incrementally
  • 📝 Create Pydantic response models
  • 🔄 Update endpoints to use response_model parameter

Status: 📝 DOCUMENTED - Will fix in future refactoring sprint

Category 3: Service Layer Patterns (Legacy - Medium Priority)

Violations:

  • SVC-001: Services creating instances of other services
  • SVC-002: Services with unused methods
  • SVC-003: Services storing db session

Reason: Pre-existing code before architecture rules were established

Refactoring Plan:

  • Medium priority
  • 🔄 Gradual refactoring as we touch each service
  • 📝 Follow dependency injection pattern for new code

Status: 📝 DOCUMENTED - Will fix incrementally

Category 4: Database Queries in Endpoints (Case-by-Case)

Violation: API-002 - Database queries should be in service layer

Files:

  • app/api/v1/admin/stores.py:63
  • app/api/v1/admin/store_domains.py:51
  • app/api/v1/admin/content_pages.py:188

Reason: Simple read queries that don't justify service layer complexity

Decision:

  • Some simple lookups can stay in endpoints
  • Complex business logic should move to services

Status: 📝 REVIEW CASE-BY-CASE

Category 5: Template Warnings (Low Priority)

Violation: TMPL-001 - Templates using inline CSS/JS

Reason: Development speed vs perfect separation

Impact: Low - doesn't affect functionality

Status: 📝 ACCEPTED - Inline styles OK for admin pages

Category 6: Cross-Module Model Imports

Violation: MOD-025 - Modules importing and querying models from other modules

Date Added: 2026-02-26 Date Resolved (Service Layer): 2026-02-27

Original Violations: ~84 (services and route files, excluding tests and type-hints) Remaining: 0 in service files — all top-level cross-module model imports eliminated

Subcategories (all resolved in service layer):

Cat Description Original Remaining
1 Direct queries on another module's models ~47 0
2 Creating instances of another module's models ~15 0
3 Aggregation/count queries across module boundaries ~11 0
4 Join queries involving another module's models ~4 0
5 UserContext legacy import path (74 files) 74 Pending (separate task)

Migration Patterns Used:

Pattern When Used Files
Service calls Cross-module data needed via existing service Most files
Method-body deferred import Model used in 1-2 methods product_service, product_media_service, audit_provider
_get_model() helper Same model used in 3+ methods log_service, admin_audit_service, admin_settings_service, admin_notification_service
Instance-cached self._Model Model used in nearly every method letzshop/order_service
TYPE_CHECKING + from __future__ Type hints without runtime dependency background_tasks_service, order_inventory_service

Resolution: See Cross-Module Migration Plan for full details.

Status: COMPLETE (service layer) — Route files and UserContext path still pending

Category 7: Provider Pattern Gaps (MEDIUM Priority — Incremental)

Violation: Modules with data that should be exposed via providers but aren't

Date Added: 2026-02-26

Provider Implementing Should Add
MetricsProvider 8 modules loyalty, payments, analytics
WidgetProvider 2 modules (marketplace, tenancy) orders, billing, catalog, inventory, loyalty
AuditProvider 1 module (monitoring) OK — single backend is the design

Status: 📝 PLANNED - Will add incrementally as we work on each module

Architecture Validation Philosophy

What We Enforce Strictly:

  1. Centralized logging (no console.log)
  2. Custom exceptions with proper status codes
  3. Pydantic models for new endpoints
  4. Service layer for complex business logic

What We Accept:

  1. Transaction control at API layer (documented)
  2. Simple queries in endpoints (case-by-case)
  3. Legacy code patterns (document and refactor incrementally)

What We're Improving:

  1. 🔄 Gradually adding Pydantic response models
  2. 🔄 Refactoring services to use dependency injection
  3. 🔄 Moving complex queries to service layer

Progress Tracking

Category Count Priority Status
JavaScript console.log 0 High Fixed
Transaction control (intentional) ~10 N/A Documented
Raw dict responses ~40 Low 📝 Backlog
Service patterns ~50 Medium 📝 Incremental
Simple queries in endpoints ~10 Low 📝 Case-by-case
Template inline styles ~110 Low Accepted
Cross-module model imports (services) 0 High Complete
Cross-module model imports (routes) TBD Medium 📝 Planned
UserContext legacy path 74 High 📝 Planned
Provider pattern gaps ~8 Medium 📝 Incremental

Validation Command

python scripts/validate/validate_architecture.py

Next Actions

Immediate (Done)

  • Fix JavaScript console.log violations
  • Document transaction control pattern
  • Add comments to intentional violations

Short Term (Next Sprint)

  • Add missing service methods to tenancy for cross-module consumers (Cat 1) — Done 2026-02-27
  • Migrate direct model queries to service calls in service files (Cat 1-4) — Done 2026-02-27
  • Migrate direct model queries to service calls in route files (Cat 1-4)
  • Move UserContext to tenancy.schemas, update 74 imports (Cat 5)
  • Create Pydantic response models for top 10 endpoints

Medium Term

  • Add widget providers to orders, billing, catalog, inventory, loyalty (P5)
  • Add metrics providers to loyalty, payments (P5)
  • Refactor 2-3 services to use dependency injection
  • Move complex queries to service layer

Long Term (Continuous)

  • Gradually refactor legacy endpoints
  • Standardize all services
  • Achieve <50 total violations

Conclusion

Current State: 221 original violations

  • 18 fixed (JavaScript logging)
  • 84 fixed (cross-module model imports in services)
  • ~120 acceptable (documented reasons)
  • Remaining legacy code tracked for incremental refactoring

Philosophy: Enforce strict standards for new code, document and incrementally improve legacy code.

Result: Codebase is in good architectural health with clear improvement path.