# 백엔드 코드 리뷰

## 총평

현재 백엔드는 "새 구조가 도입됐지만 transition shim이 길게 남아 있는 상태"다. 그래서 파일 이름만 보면 분해가 끝난 것처럼 보이지만, 실제 코드를 읽으면 route와 shim이 여전히 중요한 orchestration 책임을 잡고 있다. 이번 리뷰에서는 특히 `steps.py`, `image_service.py`, `image_steps.py`를 중심으로 "왜 구조 정리가 체감되지 않는가"를 코드 레벨에서 확인했다.

## 실측 핫스팟

실측은 현재 checkout에서 `wc -l`로 확인했다.

| 파일 | LOC | 관찰 |
|---|---:|---|
| `backend/app/services/scene_image_service.py` | 2936 | 여전히 초대형 scene 경로 서비스 |
| `backend/app/services/image_service.py` | 1228 | 상단 docstring부터 스스로 helper shim이라고 밝히는 거대 shim |
| `backend/app/services/reference_image_service.py` | 1150 | 분리됐지만 여전히 큼 |
| `backend/app/api/v1/steps.py` | 383 | API + orchestration + legacy wrapper가 한 파일에 공존 |
| `backend/app/services/checkpoint_sync/scene_still_sync_service.py` | 398 | projection 핵심 로직이 집중된 병목 후보 |

리뷰 관점에서 지적된 "1,229-line shim"은 현재 checkout에서 `wc -l` 기준 1,228줄이다. 마지막 개행 차이를 감안해도 본질은 같다. `image_service.py`는 여전히 1.2k LOC급 shim으로 남아 있다.

## 코드 레벨 주요 문제

### 1. `image_service.py`는 아직 "작은 잔여 유틸"이 아니라 큰 shim이다

근거:

- 파일 헤더 `backend/app/services/image_service.py:1-6`은 레퍼런스/씬 이미지 생성 경로가 `ReferenceImageService` / `SceneImageService`로 이관됐고, 본 모듈은 `helper shim`만 유지한다고 직접 말한다.
- 그런데 실제 파일은 1,228줄이고 `class ImageService`가 `backend/app/services/image_service.py:82`부터 시작한다.
- 내부 메서드도 `backend/app/services/image_service.py:199`, `418`, `576`, `1223`에서 각각 스스로를 `Phase 3b.2 shim`이라고 적어 두었다.
- 동시에 `reference_image_service.py`는 1,150줄, `scene_image_service.py`는 2,936줄이다.

영향:

- 리팩터링이 끝난 줄 알고 `ReferenceImageService`나 `SceneImageService`만 읽으면 실제 엔드포인트 동작 맥락을 놓치기 쉽다.
- "분리됐다"는 메시지와 "실제로는 큰 shim이 남아 있다"는 현실이 충돌한다.
- 이미지 영역은 사실상 `image_service + reference_image_service + scene_image_service` 세 파일에 5,314줄이 분산된 상태다.

권고:

- `image_service.py`는 endpoint-specific facade만 남기거나, 아예 router/service 조합으로 재정렬해야 한다.
- 지금처럼 "완전히 이관되었다"는 docstring과 1.2k LOC 구현이 동시에 존재하는 상태는 유지보수자 판단을 흐린다.

### 2. `steps.py`는 아직 route 파일이 아니라 orchestration 전환 허브다

근거:

- `backend/app/api/v1/steps.py:35-126`의 `get_all_steps()`는 step status, gate, blocked_by, model 해석을 route 레벨에서 직접 계산한다.
- `backend/app/api/v1/steps.py:253-349`의 `run_step()`는 nested background worker를 내부 함수로 만들고, DB session 재생성, gate 검사, resume skip, post-sync까지 직접 관리한다.
- `backend/app/api/v1/steps.py:365-381`은 `_sync_checkpoints_to_db`, `_get_step_runner` 같은 legacy wrapper를 계속 노출한다.

영향:

- 이 파일은 HTTP adapter라기보다 "과도기 orchestration hub"다.
- step runtime 정책이 service/core로 내려가지 못한 상태라, 테스트와 신규 구현이 이 파일의 사적 helper에 계속 걸리게 된다.

권고:

- step read-model 계산은 별도 service로, background 실행 orchestration은 dispatch/run service로 내리는 편이 맞다.
- route 파일은 request validation과 response shaping만 담당해야 한다.

### 3. `_needs_presync`는 runtime 정책이 코드 상수로 굳어 있는 대표 사례다

근거:

- `backend/app/api/v1/steps.py:307-324`의 `_run_in_background()`는 `_needs_presync` 튜플을 직접 선언한다.
- 목록에는 `scene_director`, `outlook_extraction`, `outlook_phase1/2/3`, `scene_detail`, `scene_verify`가 들어 있다.
- 같은 저장소 안에서 `backend/app/core/step_manifest.py:21-30`은 step metadata 체계를 이미 도입했지만, 이 정책은 메타가 아니라 route 내부 하드코딩으로 남아 있다.

영향:

- step 추가/변경 시 manifest/catalog만 맞추면 된다는 기대가 깨진다.
- projection 의존 step을 빠뜨리면 테스트보다 실제 실행 시점에 먼저 터질 가능성이 높다.

권고:

- pre-sync 요구사항을 manifest/catalog 메타로 올리고, `steps.py`는 그 메타만 읽어야 한다.

### 4. `image_steps.py`에 unreachable legacy block이 그대로 남아 있다

근거:

- `backend/app/core/steps/image_steps.py:52-61`의 `_sync_analysis_to_db()`는 `orchestrate_full_sync(...)` 호출 직후 `return`한다.
- 바로 아래 `backend/app/core/steps/image_steps.py:63` 이후 블록은 예전 entity/scene/outlook sync 구현인데, 현재 제어흐름상 도달할 수 없다.

영향:

- 유지보수자가 "이 코드도 아직 실행되나?"를 확인하는 데 시간을 쓴다.
- 코드 리뷰 시 실제 경로와 유사 레거시 경로를 구분해야 해서 인지 비용이 올라간다.

권고:

- dead code는 history로 보내고, 현행 wrapper만 남기는 편이 낫다.

### 5. 구현 계약 변화가 adapter 없이 바로 드러나고 있다

이 항목은 "테스트가 낡았다"는 말보다 더 정확하다. 실제 구현이 과거 helper/constructor/shape 계약을 흡수하지 않고 바로 바뀌었다.

근거:

- `backend/app/modules/pipeline/outlook_extractor_v2.py:69-84`는 `load_schema()` 결과를 deepcopy한 뒤 곧바로 `schema["properties"]["scene_assignments"]...`를 수정한다. minimal schema mock에는 바로 `KeyError`가 난다.
- `backend/app/core/steps/scene_steps.py:96-105`는 `SceneSegmentationStep._execute()`에서 `self.build_opik_metadata(...)`를 호출한다. `StepRunner.__init__`를 우회해 부분 객체를 만드는 기존 테스트는 `opik_context` 부재로 바로 깨진다.
- `backend/app/modules/pipeline/text_cleaner.py:40-77`은 현재 `extract_text_from_pdf_llm()`만 노출한다. 과거 테스트가 import하던 `clean_text` symbol은 더 이상 없다.
- `backend/app/modules/variation_recommender.py:103-157`의 `VariationRecommender` 생성자는 `project_llm_config`만 받는다. 예전 테스트가 기대하던 `llm_client` dependency injection path는 사라졌다.

영향:

- 구현이 진화하는 것은 맞지만, 호환 shim이나 명시적 migration 없이 바로 contract가 바뀌어 테스트 붕괴 폭이 커졌다.
- 이 패턴은 단지 테스트 부채가 아니라, 내부 호출자도 비슷한 식으로 갑자기 깨질 수 있음을 뜻한다.

권고:

- 전환 중인 모듈은 최소한 adapter shim, deprecated alias, 또는 테스트 갱신 PR을 같이 가져가야 한다.
- "리팩터링 완료" 커밋에서 contract change와 cleanup를 한 번에 묶지 않는 편이 좋다.

### 6. scene/image 영역은 아직 "세 조각으로 나뉜 큰 덩어리"에 가깝다

근거:

- `scene_image_service.py` 2,936줄, `reference_image_service.py` 1,150줄, `image_service.py` 1,228줄을 합치면 이미지 도메인만 5,314줄이다.
- 파일 헤더는 분리를 말하지만, 실제로는 scene path, reference path, endpoint shim이 모두 크고 서로 긴밀히 연결돼 있다.

영향:

- 이미지 품질 이슈, provenance 이슈, variation 이슈가 생기면 추적 범위가 여전히 넓다.
- "어느 서비스가 canonical owner인가"가 코드 독해만으로는 분명하지 않다.

권고:

- 다음 분해 축이 필요하다.
- reference resolution
- variation recommendation/edit
- validation/selection
- persistence/provenance
- endpoint facade

## 좋은 코드 포인트

### 1. `checkpoint_sync` 분해 자체는 옳은 방향이다

- route와 step 쪽에 legacy wrapper가 남아 있어도, sync 핵심을 별도 오케스트레이터로 밀어낸 방향 자체는 맞다.

### 2. `step_catalog`는 아직 덜 채택됐지만 설계 의도는 좋다

- 적어도 metadata와 class binding을 한 view로 모으려는 축은 생겼다.
- 지금 필요한 것은 새 abstraction을 더 만드는 것이 아니라, 기존 소비자를 그 abstraction으로 실제로 옮기는 일이다.

## 우선순위 높은 코드 정리 항목

1. `steps.py`에서 `_needs_presync`, `_sync_checkpoints_to_db`, `_get_step_runner` 같은 전환용 glue를 service 계층으로 내린다.
2. `image_steps.py`의 unreachable legacy block을 제거한다.
3. `image_service.py`를 진짜 facade 크기로 줄이거나, 남겨둘 책임을 명확히 재정의한다.
4. contract가 바뀐 모듈(`outlook_extractor_v2`, `text_cleaner`, `variation_recommender`, `scene_steps`)에는 transition shim 또는 테스트 동시 갱신 규칙을 적용한다.
5. scene/image 도메인에서 "누가 canonical owner인가"를 파일 경계만으로도 드러나게 다시 분해한다.