Business rules sit in the repository layer; introduce a ReviewService #125

New issue

Open

opened 2026-07-28 06:06:11 +00:00 by claude-bot · 0 comments

claude-bot commented

2026-07-28 06:06:11 +00:00

Severity: LOW - refactor, do it alongside other work

The observation

Layering is real in one direction: routes never touch raw SQL, sessions flow correctly, and keyset
pagination lives where it should. But responsibilities have not been assigned to the layers:

Routes construct ORM entities and own commits
"Services" is only projection-application plus ingest
The decision-to-status mapping — genuine business logic — sits in
PhotoRepository.apply_decision_projection
EvidenceRepository.supersede and DecisionRepository.get_by_id are dead code
CommentRepository and UserRepository are thin CRUD ceremony

Recommendation

Not worth a rework for its own sake. Do it as a side effect of the concurrency fix (#77): a
ReviewService owning submit_decision, update_notes, update_flags, add_manual_evidence,
and supersede_evidence — each doing conflict check → history write → projection → audit → commit.

That collapses the route bodies to request parsing and makes #6 and #9 testable without going
through HTTP.

Done when

Write paths go through a service that owns the transaction
Business rules live in exactly one layer
Dead repository methods are used or removed
Tests can exercise review operations without HTTP

References

backend/app/repositories/photos.py:80-101
backend/app/api/routes/photos.py

Best done with: #77 (concurrency fix), #79 (projection rebuild).

## Severity: LOW - refactor, do it alongside other work ## The observation Layering is real in one direction: routes never touch raw SQL, sessions flow correctly, and keyset pagination lives where it should. But responsibilities have not been assigned to the layers: - Routes construct ORM entities and own commits - "Services" is only projection-application plus ingest - The decision-to-status mapping — genuine business logic — sits in `PhotoRepository.apply_decision_projection` - `EvidenceRepository.supersede` and `DecisionRepository.get_by_id` are dead code - `CommentRepository` and `UserRepository` are thin CRUD ceremony ## Recommendation Not worth a rework for its own sake. Do it as a **side effect** of the concurrency fix (#77): a `ReviewService` owning `submit_decision`, `update_notes`, `update_flags`, `add_manual_evidence`, and `supersede_evidence` — each doing conflict check → history write → projection → audit → commit. That collapses the route bodies to request parsing and makes #6 and #9 testable without going through HTTP. ## Done when - [ ] Write paths go through a service that owns the transaction - [ ] Business rules live in exactly one layer - [ ] Dead repository methods are used or removed - [ ] Tests can exercise review operations without HTTP ## References - `backend/app/repositories/photos.py:80-101` - `backend/app/api/routes/photos.py` Best done with: #77 (concurrency fix), #79 (projection rebuild).

claude-bot added this to the v0.6.0 milestone

2026-07-28 06:06:11 +00:00

claude-bot added the

backend

label

2026-07-28 06:06:11 +00:00