AI PR 리뷰 검증 패턴

분류 프레임워크

AI 리뷰 코멘트가 전부 같은 무게를 가지는 건 아니에요. 진짜 버그도 있고, 스타일 선호도 있고, 완전히 틀린 것도 있어요. 단순한 VALID/INVALID 이분법으로는 부족하고, 빠른 분류 판단을 위해 더 세밀한 등급이 필요해요.

분류	기준	조치
VALID BUG	실제 버그, 보안 이슈, 장애 유발	즉시 수정
VALID IMPROVEMENT	올바른 제안, 코드 품질 향상	즉시 수정
GOOD-TO-HAVE	맞는 지적이지만 우선순위가 낮은 개선	쉬우면 수정, 위험하면 skip
CONTROVERSIAL	논쟁의 여지가 있음 — 타당하지만 비용 대비 효과 불명	건별로 판단
OPTIONAL	있으면 좋은 수준, 스타일 관련, 긴급하지 않음	사용자에게 확인
INVALID	틀림, context 오해, 해당 안 됨	문서화 + 보강 주석 추가

나중에 추가한 두 등급 — GOOD-TO-HAVE와 CONTROVERSIAL — 은 release PR에서 필수적이었어요. feature branch 리뷰에서는 모든 지적을 처리할 여유가 있지만, develop-to-main merge에서 19개 지적을 분류할 때는 “맞고 수정할 가치가 있는 것”과 “맞지만 지금 건드릴 가치가 없는 것”을 빠르게 분리해야 해요. GOOD-TO-HAVE는 수정이 쉽고 위험이 낮은 개선이에요. CONTROVERSIAL은 AI 지적이 타당하지만 현재 context에서 수정 비용이 이점을 넘는 경우예요. 예를 들어 release 중에 27개 test file에 type guard를 추가하라는 지적 같은 거죠.

AI가 흔히 혼동하는 패턴

1. 오래된 Diff / 이미 존재하는 기능

어떻게 보이나: 에이전트가 기능이 “없다”고 하지만 현재 코드에 존재해요.

왜 이런 일이 생기나: AI가 현재 file 상태가 아닌 PR diff를 review하기 때문이에요. 이전 commit에서 추가된 기능을 놓칠 수 있어요.

예시:

에이전트: "CRITICAL: Analytics 서비스 메서드가 Promise.reject('Not implemented')를 반환"
현실: 서비스에 1449줄의 완전한 구현이 있음

예방: 보강 주석을 추가해요.

// NOTE: This service IS FULLY IMPLEMENTED. All 5 analytics calculations
// are complete and production-ready via the consolidated getAnalytics() method.

2. Request 라이프사이클 오해

어떻게 보이나: 에이전트가 필요 없는 곳에 transaction/lock을 제안해요.

왜 이런 일이 생기나: AI가 framework별 request 라이프사이클(NestJS, Express)을 이해하지 못하기 때문이에요.

예시:

에이전트: "부모 조회와 이동 사이에 race condition — database locking 추가"
현실: NestJS HTTP 요청은 단일 스레드 event loop에서 동기적으로 실행

예방: 보강 주석을 추가해요.

// NOTE: NO RACE CONDITION exists between parent fetch and move operation.
// This entire method executes synchronously within a single HTTP request context.
// Node.js single-threaded event loop guarantees sequential execution.

3. Webhook 흐름 오해

어떻게 보이나: 에이전트가 webhook handler를 transaction으로 감싸라고 제안해요.

왜 이런 일이 생기나: AI가 외부 서비스에서 이미 상태를 commit했다는 걸 이해하지 못해요.

예시:

에이전트: "softDeleteAllByUserId가 구독 생성과 transaction으로 감싸져 있지 않음"
현실: LemonSqueezy가 이미 구독을 commit함. 우리 코드는 상태를 동기화할 뿐

예방: 보강 주석을 추가해요.

// NOTE: This is intentionally NOT wrapped in a transaction with subscription creation.
// External service already committed; webhook redelivery handles sync failures.

4. 변수 재할당 인식 실패

어떻게 보이나: 에이전트가 destructuring 이후의 할당 흐름을 잘못 읽어요.

왜 이런 일이 생기나: AI가 destructuring을 보고 모든 값이 같은 source에서 온다고 가정해요.

예시:

에이전트: "retry 후 resyncOccurred가 undefined일 수 있음"
현실: 327번 줄에서 resyncOccurred = true로 명시적 설정 (retryResult에서 온 게 아님)

예방: 보강 주석을 추가해요.

// NOTE: Explicitly set to true (not from retryResult) because 410 recovery IS a resync event.
resyncOccurred = true;

5. Process 모델 오해

어떻게 보이나: 에이전트가 module-level singleton을 thread-unsafe하다고 지적하거나 lock 추가를 제안해요.

왜 이런 일이 생기나: AI가 기본적으로 threading 모델을 가정하지만, 실제 production 환경에서는 process 기반 worker를 쓰는 경우가 많아요. Celery prefork, gunicorn worker process — 각 worker가 별도의 메모리 공간을 가지기 때문에 공유 상태 자체가 없어요.

예시:

에이전트: "Global _llm_service is not thread-safe — use threading.Lock"
현실: Celery prefork = 별도 process. 각각 자기만의 global namespace를 가짐

Python 프로젝트에서 CodeRabbit이 module-level 변수마다 thread-safety 이슈로 지적한 사례예요. Django/Celery stack에서 prefork worker를 쓰면 각 process가 global namespace의 자기 사본을 갖게 돼요. thread도 없고, 공유도 없고, 문제도 없어요.

예방: 보강 주석을 추가해요.

# NOTE: Celery prefork model = separate processes, not threads.
# Each worker process gets its own _llm_service singleton. No locking needed.

6. Pydantic 모델 mutability 가정

어떻게 보이나: 에이전트가 Pydantic 모델에 생성 후 속성 할당을 하면 실패하거나 error가 발생한다고 주장해요.

왜 이런 일이 생기나: AI가 Pydantic 모델이 기본적으로 immutable이라고 가정해요. 하지만 Pydantic v2 모델은 model_config = ConfigDict(frozen=True)를 명시적으로 설정하지 않는 한 mutable이에요. 해당 config 없이는 속성 할당이 정상 동작해요.

예시:

에이전트: "GenerateContentConfig assignment after construction won't work"
현실: frozen이 아닌 Pydantic v2 모델 — 속성 할당은 유효

Pydantic v2에서 frozen=True 없이 쓰고 있다면 생성 후 할당은 완전히 정상이에요. AI가 모델 config를 확인하지 않고 지적하는 거죠.

7. Factory pattern default 불일치

어떻게 보이나: 에이전트가 constructor의 default parameter 값이 production 설정과 불일치한다고 지적해요.

왜 이런 일이 생기나: AI가 constructor signature를 단독으로 비교하고, factory function이 항상 명시적 값을 전달한다는 걸 무시해요. default는 test나 직접 인스턴스 생성을 위한 fallback일 뿐이고 production에서는 실행되지 않아요.

예시:

에이전트: "WhisperSTTService default device='cpu' may not match production config"
현실: Factory가 항상 device=settings.WHISPER_DEVICE를 명시적으로 전달

constructor에 합리적인 default가 있고 factory가 이를 override하는 구조에서 자주 나타나는 패턴이에요. AI는 default와 config 값 사이의 불일치를 보지만, 실제 호출 경로를 추적하지는 않아요.

8. Cross-File 인식 실패

어떻게 보이나: 에이전트가 다른 file에 정의된 동작에 대해 질문해요.

왜 이런 일이 생기나: AI가 file을 단독으로 review하고 관련 file을 확인하지 않아요.

예방: 교차 참조 주석을 추가해요.

// NOTE: Related logic in sync-blocks.helper.ts:232 handles resyncRequired

보강 주석 템플릿

패턴	템플릿
기능 존재	`// NOTE: [Feature] IS [implemented/handled] [here/below] - [brief description]`
Race Condition 없음	`// NOTE: NO RACE CONDITION - [framework] executes [operation] synchronously within single request`
의도적 설계	`// NOTE: Intentionally [omitted/designed this way] - [reason]`
Cross-File 참조	`// NOTE: Related logic in [file:line] handles [concern]`

9. Cross-Branch 혼동

어떻게 보이나: 에이전트가 코드가 assertion과 모순된다고 주장하면서, PR branch와 일치하지 않는 line 번호를 인용해요.

왜 이런 일이 생기나: 미묘한 케이스예요. PR target이 main이 아닌 develop일 때, AI reviewer가 실제 target branch 대신 main branch 코드를 분석하는 경우가 있어요. 이전 PR에서 이미 develop에 merge된 변경이 있으면, reviewer는 main의 이전 코드를 보고 존재하지 않는 모순을 지적해요.

예시:

에이전트: "Lines 796-800 set deletedAt unconditionally — toBeNull() should fail"
현실: 해당 줄은 develop에서 NOTE 주석임. if 블록은 PR #711에서 제거됨

PR #712에서 실제로 겪은 사례예요. Claude가 main branch 코드를 분석해서 test assertion이 잘못됐다고 지적했어요. 하지만 develop에서는 인용한 줄이 이미 주석으로 교체되어 있었고, production 코드는 이전 PR에서 변경된 상태였어요. assertion은 현재 develop 상태에서는 정확했어요.

예방: test 위치에 보강 주석을 추가해요.

// NOTE: The deletedAt-setting code was removed in PR #711. This test verifies
// the post-removal behavior: Event blocks only get itemStatus=Deleted, no deletedAt.

10. Large Diff Blindness (GitHub API 406)

어떻게 보이나: AI reviewer가 release PR에서 코멘트를 반환하지 않거나 극히 적은 피드백만 줘요.

왜 이런 일이 생기나: GitHub API가 PR diff가 20,000줄을 넘으면 HTTP 406을 반환해요. Release PR(develop to main)은 이 제한을 자주 넘어요. diff API endpoint에 의존하는 AI reviewer는 분석할 데이터 자체가 없어서, 빈 review를 만들거나 file 이름만 보고 추측해요.

우회 방법: git diff origin/main..origin/develop으로 diff를 로컬에서 생성하고 도메인별로 review agent에 나눠서 전달해요. API 제한을 완전히 우회하면서 각 reviewer에게 처리 가능한 크기의 chunk를 보낼 수 있어요.

11. Release PR에서의 Authorship 범위 지정

어떻게 보이나: review 지적이 다른 팀원이 작성한 코드를 지적해요.

왜 중요한가: Release PR은 보통 여러 작성자의 squash merge예요. 팀 전체가 작성한 모든 줄을 review하면 시간 낭비이고 처리할 수 없는 noise만 생겨요. 다른 사람의 구현 결정에 대한 full context가 없으니까요. severity가 CRITICAL인 경우를 제외하고는 자기 commit에 집중하는 게 좋아요.

방법: 분류 전에 authorship을 확인해요.

git log origin/main..HEAD -- {file} --format="%h %ae %s"

다른 작성자의 지적은 finding registry에서 N/A로 표시해요. 작성자와 관계없이 CRITICAL 지적만 에스컬레이션하면 돼요. PR #710에서는 15개 고유 지적 중 7개가 authorship scoping으로 N/A 처리돼서 상당한 시간을 절약했어요.

12. 마크다운 포매팅 환각

어떻게 보이나: Reviewer가 마크다운 테이블에 포매팅 문제가 있다고 주장해요(예: “앞에 ||가 와서 빈 첫 번째 열이 생김”). 실제로는 테이블이 완벽하게 유효한데도요.

왜 이런 일이 생기나: Copilot이 실제 file 내용을 파싱하지 않고, 흔한 마크다운 문제에 대한 패턴 매칭으로 포매팅 지적을 만들어내는 경우가 있어요. 다른 혼동 패턴이 코드 로직을 잘못 읽는 것과 달리, 이건 순수하게 날조된 거예요 — file 어디에도 없는 구문 문제를 만들어내는 거죠.

이 패턴이 특히 비용이 큰 이유는 비슷한 file 수에 비례해서 늘어나기 때문이에요. 비슷한 구조의 file이 많은 문서 전용 PR(예: 11개 README)을 review할 때, 환각이 모든 file에서 반복되면서 지적 수가 급증해요.

예시:

에이전트: "이 README의 테이블은 각 행이 `||`로 시작해서 GitHub이 빈 첫 번째 열을 렌더링한다."
현실: 테이블은 표준 `| col | col |` 형식 — 이중 파이프는 어디에도 없음.

crucio PR #40에서 실제로 겪은 사례예요. 18개 Copilot 지적 중 12개(67%)가 11개 README file에 걸쳐 이 똑같은 환각이었어요. 테이블 포매팅 문제는 단 하나도 없었어요. 처음에는 각 file을 개별적으로 확인하려 했지만, 두 번째 지적이 동일하다는 걸 확인한 후 나머지를 일괄 기각해서 한 시간 넘게 절약했어요.

예방: 문서 PR에서는 지적된 file 하나를 먼저 스팟 체크하세요. 첫 번째 지적이 false positive면, 개별 review 없이 비슷한 지적을 일괄 기각하세요.

워크플로

issue 코멘트(claude[bot])와 review 스레드(Copilot) 모두 가져오기
위 프레임워크로 각 항목 분류
INVALID인 경우: 패턴 식별 → 보강 주석 추가 → 문서화
OPTIONAL인 경우: 사용자에게 확인(수정/건너뛰기/나중에)
review 검증을 참조하는 설명적인 메시지로 commit

실제 사례

사례 1: moba-nestjs PR #629 (claude[bot])

통계: 12개 코멘트, 3개 INVALID, 5개 OPTIONAL, 4개 VALID IMPROVEMENT

주요 INVALID:

기능이 이미 존재(analytics 서비스 완전 구현됨)
request 라이프사이클 오해(단일 스레드 event loop에서 race condition 없음)
webhook 흐름 오해(외부 서비스가 이미 commit)

사례 2: moba-etl PR #5 (GitHub Copilot)

통계: 10개 코멘트, 0개 INVALID, 4개 VALID BUG, 3개 VALID IMPROVEMENT, 1개 ALREADY FIXED, 2개 OPTIONAL

주요 VALID BUG:

json.dumps() encoding — put_object()는 str이 아닌 bytes 필요
Manifest 키 불일치 — 읽기/쓰기에 다른 키 사용
S3 prefix 정규화 — trailing slash 없는 경로가 잘못된 키 생성

결과: 모든 버그 수정, 오탐 없음. 인프라/데이터 코드에서 Copilot review가 매우 정확했어요.

사례 3: crucio PR #6 Round 2 (CodeRabbit + Claude Bot)

통계: 14개 항목(6 CodeRabbit, 8 Claude Bot), 1 VALID BUG, 2 CONTROVERSIAL→FIX, 3 GTH→FIX, 1 SKIP, 6 INVALID, 1 DUP

세 가지 새로운 혼동 패턴이 동시에 나타난 PR이에요. Python 프로젝트에서 Celery prefork worker, Pydantic v2 모델, factory pattern을 사용한 서비스 초기화 — AI reviewer가 일관되게 틀리는 세 가지였어요.

주요 VALID BUG:

extract_tags에 ValueError handler 누락 — 영구적 실패(잘못된 config, safety filter)를 fast-fail 대신 재시도

주요 INVALID (새 패턴):

Process 모델(#5): Celery prefork = 별도 process, thread 아님
Pydantic mutability(#6): frozen 아닌 모델은 속성 할당 가능
Factory default(#7): factory가 명시적 값을 전달하므로 constructor default는 무관
GitHub Actions format: 쉼표 구분 "Tool1,Tool2"는 공식 문서와 일치

결과: 6개 수정, 6개 INVALID 근거와 함께 기각. 정확도가 혼재 — CodeRabbit 4/6 INVALID, Claude Bot 1 VALID BUG + 2 INVALID.

사례 4: moba-nestjs PR #710 Round 1+2 (Copilot + Claude)

통계: 19개 raw 지적 → dedup 후 15개 고유. 1 VALID BUG, 2 GTH→FIX, 3 CONTROVERSIAL→SKIP, 1 INVALID, 3 DEFER, 7 N/A(authorship)

GitHub API 406 이슈(#10)를 유발한 release PR이에요. 로컬에서 diff를 생성하고 도메인별로 agent에 나누는 우회 방법은 잘 동작했지만, authorship scoping 문제(#11)가 새로 발생했어요. 지적의 거의 절반이 다른 팀원이 작성한 코드를 지적한 N/A였어요.

주요 VALID BUG:

moveCrossIntegration에서 blockRepo.count()에 withDeleted: true 누락 — soft-deleted T block(취소된 반복 인스턴스)이 count되지 않아서 parent가 moveCrossIntegrationSingle로 잘못 라우팅

주요 INVALID:

오래된 Diff(#1): 한국어 README “삭제”는 실제로 이름 변경(git이 rename을 delete+create로 표시)

주요 SKIP 결정 (CONTROVERSIAL):

Sync용 soft-deleted record: WebSocket event가 삭제를 처리하지 getBlocksByIds가 아님
Google API type assertion: null conferenceData clearing에 type-safe 대안이 없음
test의 non-null assertion: 올바른 지적이지만 27개 = release PR 시점에 적절하지 않음

프로세스 학습: Claude의 구조화된 review는 finding별로 개별 파싱(STEP 1C)이 필요하고, 하나의 CR-1으로 합치면 안 돼요. Round 1에서 이걸 놓쳤고, Round 2에서 수정했어요.

사례 5: moba-nestjs PR #712 Round 1+2 (Claude)

통계: Round 1: 8개(5 INVALID, 2 CONTROVERSIAL→FIX, 1 GTH→FIX). Round 2: 2개(1 INVALID, 1 GTH→FIX)

Cross-Branch 혼동(#9)이 처음 나타난 PR이에요. Claude가 PR target(develop) 대신 main branch 코드를 분석해서, test assertion이 796-800번 줄의 구현과 모순된다고 주장했어요. develop에서는 해당 줄이 이미 NOTE 주석이었고, deletedAt 설정 코드는 이전 PR(#711)에서 제거된 상태였어요.

주요 INVALID (새 패턴):

Cross-Branch 혼동(#9): Reviewer가 PR target(develop) 대신 main branch 코드를 분석. toBeNull()이 796-800번 줄 구현과 모순된다고 주장했지만, develop에서 해당 줄은 NOTE 주석(deletedAt 설정 코드는 PR #711에서 제거됨)

결과: 양쪽 round에서 3개 수정, 6개 INVALID 기각. 새 패턴 문서화: Cross-Branch 혼동 — AI reviewer가 PR target이 develop인데도 main branch context를 기본으로 사용.

분류 프레임워크

AI가 흔히 혼동하는 패턴

1. 오래된 Diff / 이미 존재하는 기능

2. Request 라이프사이클 오해

3. Webhook 흐름 오해

4. 변수 재할당 인식 실패

5. Process 모델 오해

6. Pydantic 모델 mutability 가정

7. Factory pattern default 불일치

8. Cross-File 인식 실패

보강 주석 템플릿

9. Cross-Branch 혼동

10. Large Diff Blindness (GitHub API 406)

11. Release PR에서의 Authorship 범위 지정

12. 마크다운 포매팅 환각

워크플로

실제 사례

사례 1: moba-nestjs PR #629 (claude[bot])

사례 2: moba-etl PR #5 (GitHub Copilot)

사례 3: crucio PR #6 Round 2 (CodeRabbit + Claude Bot)

사례 4: moba-nestjs PR #710 Round 1+2 (Copilot + Claude)

사례 5: moba-nestjs PR #712 Round 1+2 (Claude)

Comments