리뷰를 세 번 통과한 버그 — diff보다 범위가 문제였다

앞선 글에서 전역 Axios에 요청별 인증정보를 얹는 문제는 “사람 리뷰보다 정적 분석이 먼저 볼 문제”라고 썼다. 규칙으로 못 박을 수 있는 종류였기 때문이다.

이 글은 그 반대 사례다. 정적 분석만으로 정답을 못 박기 어렵고, 실제로 리뷰를 세 번 통과했다. 각 PR의 변경은 자기 티켓과 파일 안에서는 타당해 보였다. 그러나 공용 동작까지 펼쳐보면 조건이 충분히 좁지 않았다. 문제는 문법보다 적용 범위에 있었고, 그 범위는 diff만 봐서는 드러나지 않았다.

아래 사례는 구조만 남기고 지면·컴포넌트·작업 이름을 일반화했다.

증상

앱 웹뷰로 열리는 프로모션 목록 지면에서, 로그인 화면에 갔다 돌아오면 헤더 위에 빈 여백이 생긴다. Android에서는 보이지 않고 iOS에서만 재현됐기 때문에 처음에는 “iOS 이슈”로 접수됐다.

원인이 된 코드는 공용 상단 네비게이션 컴포넌트의 기본 스타일 한 줄이었다.

webview:pt-[calc(var(--safe-area-inset-top) + 1rem)]

“웹뷰면 노치 높이만큼 위에 여백을 준다.” 노치가 있는 기기에서 콘텐츠가 상단바에 가리지 않게 하는 흔한 safe-area 처리다. 값은 브라우저가 제공하는 환경 변수를 env()로 읽는다.

그런데 이 지면은 네이티브가 상단바를 직접 그리고 웹뷰는 그 아래에서 시작하는 모드로 열린다. 이미 상단바를 피해 있는데 노치 높이를 한 번 더 더한 것이다.

Android가 멀쩡한 이유는 고쳐져서가 아니었다. 다른 작업이 --safe-area-inset-top을 Android에서만 0px로 덮어쓰고 있었다. 값이 0이니 더해도 0. 증상이 사라진 게 아니라 가려진 것이다.

세 번의 리뷰

Git으로 이 한 줄의 이력을 추적하니 1년 넘게 이어진 세 작업이 나왔다.

① 도입 — “목록 지면 헤더의 safe-area 처리”

티켓 제목이 그랬다. 특정 지면 헤더 하나를 고치는 작업이었다. 그런데 변경은 그 헤더의 부모인 공용 컴포넌트의 기본 스타일에 들어갔다.

리뷰가 작동하지 않은 것도 아니다. 커밋 로그에 이런 게 남아 있다.

fix: webview인 경우에만 적용되도록 수정

처음엔 무조건 적용이었고, 리뷰 과정에서 “웹뷰일 때만”으로 좁혔다. 좋은 리뷰다. 그런데 한 칸 덜 좁혔다. 정확한 조건은 “웹뷰”가 아니라 “웹뷰가 상단바까지 덮는 전체 화면 모드”였다.

PR 설명에는 전체 화면 모드가 목적이라고 적혀 있었다. 의도는 fullscreen이었는데 조건은 webview로 걸렸다. 리뷰에서는 두 조건의 차이까지 질문하지 못했다.

② 은폐 — “Android 특정 웹뷰에서 하단 여백이 큰 이슈”

6개월 뒤. 티켓은 하단 여백 이야기다. 구현은 env() 직접 참조를 CSS 변수로 전면 치환하고, Android에서 safe-area 값을 0px로 강제하는 폴리필을 추가했다.

문제는 강제한 대상이 하단만이 아니라 상단도 포함이었다는 것이다. 그 두 줄은 수십 개 파일을 건드리는 치환 diff 속에 있었다.

이 변경 이후 Android에서 ①의 부작용이 안 보이게 됐다. 그리고 그것이 최초 변경으로부터 1년 뒤 이 건이 “iOS 전용 이슈”로 접수된 이유다.

③ 도구는 생겼는데 — “웹뷰 전체 화면 쿼리 파라미터 대응”

다시 6개월 뒤. 다른 기능을 하다가 “이 웹뷰가 상단바를 덮는 전체 화면인가”를 정확히 판정하는 유틸이 만들어졌다. ①이 처음부터 썼어야 할 바로 그 술어다.

폴리필은 새 술어로 옮겨갔다. 헤더는 안 옮겨갔다. 이 PR의 범위가 아니었으니까.

즉 이 버그가 접수되고 열흘 뒤, 이걸 고칠 도구가 저장소 안에 들어왔다. 다만 기존 헤더까지 이관 대상으로 연결되지 않았다.

각 리뷰는 자기 범위에 갇혔다

세 PR을 리뷰어 자리에서 다시 보면, 나라도 승인했을 것 같다.

세 번 다 바뀐 줄은 자기 티켓과 파일 안에서 타당해 보였다. 문법도 자연스러웠고, 좁게 읽은 의도와도 맞았다. 다만 세 리뷰 중 어느 것도 공용 동작 전체를 기준으로 조건을 다시 재보지는 않았다.

이런 문제는 특정 팀에만 생기지 않는다. 현대 코드 리뷰를 관찰한 연구에 따르면, 결함 발견은 리뷰의 주요 동기지만 실제 리뷰는 예상보다 결함 발견의 비중이 낮고 지식 전달·팀 인식 향상·대안 도출 같은 효과도 낸다. 또한 리뷰의 핵심적인 어려움으로 코드와 변경의 맥락을 이해하는 일을 꼽는다. 맥락이 diff 밖에 있으면 리뷰가 그걸 볼 방법이 마땅치 않다.

그럼 왜 망가지나

1. 리뷰의 시야는 diff고, 버그는 diff 밖에 있었다

공용 컴포넌트의 기본 스타일 한 줄은 diff에서는 한 줄이다. 실제로는 그 컴포넌트를 쓰는 10곳 넘는 지면에 걸린다.

diff의 크기 ≠ 영향의 크기. 리뷰 시간은 눈에 보이는 diff 크기에 맞춰 배분되기 쉽다. 그래서 짧은 공용 변경이 실제 영향보다 가볍게 읽힐 수 있다.

2. 티켓 제목이 리뷰어의 시야를 좁힌다

“목록 지면 헤더의 safe-area 처리”를 읽은 리뷰어는 그 목록 지면을 본다. 실제 변경은 부모인 공용 컴포넌트였는데도.

티켓의 범위와 diff의 실제 범위가 어긋날 때가 가장 위험하다. 리뷰어는 티켓을 읽고 시야를 세팅한 채로 diff에 들어오기 때문에, 어긋난 만큼이 그대로 사각지대가 된다.

3. 가리는 것과 고치는 것은 둘 다 초록색이다

②는 증상을 없앴다. QA도 통과했다. 하지만 원인이 아니라 관측을 지웠다.

테스트가 초록이 되는 길에는 고치기뿐 아니라 가리기도 있다. 제한된 관측만으로는 리뷰와 QA가 둘을 구분하기 어렵다. 특히 값을 덮어쓰는 종류의 수정(전역 변수 override, 폴리필, !important)은 정당한 해결책일 수도 있지만, 다른 문제의 관측값까지 함께 지울 위험이 있다.

4. 술어를 만들고 이관하지 않았다

③에서 정확한 판정 함수가 생겼다. 그런데 당시 체크리스트에는 “새 술어를 만들었으면 옛 조건을 쓰던 곳을 전부 검색하고, 이관 여부를 기록한다”는 항목이 없었다.

리팩터링은 보통 “함수를 바꿨으면 호출처를 바꿔라”로 강제된다. 타입 시스템이 잡아준다. 그런데 개념이 정교해진 경우는 타입이 안 잡는다. isWebview도 isFullscreen도 똑같이 boolean이다.

정적 분석으로 잡히나

부분만.

잡히는 것 — 공용 패키지의 기본 스타일이나 기본값이 바뀌었다는 사실 자체는 기계가 안다. 경로 패턴과 변경 위치만 보면 된다. 소비처 개수를 세는 것도 기계가 낫다.

안 잡히는 것 — “이 조건은 webview가 아니라 fullscreen이어야 한다”는 판단. 두 술어 모두 유효한 boolean이고, 어느 쪽이 맞는지는 제품이 어떤 네이티브 모드로 열리는지를 알아야 한다. 현재 컴포넌트와 diff만으로는 그 계약이 드러나지 않는다.

그래서 앞선 Axios 건과 성격이 다르다. 그건 “전역에 요청별 상태를 쓰지 마라”라는 규칙으로 못 박히는 문제였다. 이건 하나의 문법 규칙으로 정답을 강제하기 어렵다.

기계가 할 수 있는 최선은 막는 게 아니라 리뷰어를 그 자리로 데려가는 것이다.

그래서 리뷰 질문 다섯 개

정적 분석이 못 하는 자리를 사람이 메우려면 질문이 구체적이어야 한다. “꼼꼼히 보자”는 아무것도 바꾸지 않는다.

1. 이 줄은 몇 군데에 적용되나? 공용 컴포넌트의 기본 스타일·기본 props·기본값을 건드리는 PR이면 소비처 수를 세서 본문에 적는다. “1줄 변경”과 “10곳 넘는 지면에 적용되는 변경”은 리뷰 시간이 달라야 한다.

2. 티켓의 범위와 diff의 범위가 같은가? 다르면 그 자리에서 짚는다. 티켓은 한 지면인데 변경은 공용 레이어면, 최소한 “왜 공용에 넣는가”에 답이 있어야 한다.

3. 이 조건의 이름이 맞나? 분기를 추가할 때 값이 아니라 술어의 이름을 본다. isWebview인가 isFullscreen인가. 좁히자는 지적은 리뷰에서 흔히 나오는데, 어디까지 좁혀야 하는지는 잘 안 따진다. ①은 정확히 그 한 칸에서 멈췄다.

4. 이건 고치는 건가 가리는 건가? 값을 덮어쓰는 수정이면 기본 질문으로 둔다. “이 override를 걷어내면 무엇이 다시 보이나?” 답을 모르면 원인을 모르는 것이다. ②는 하단 여백 티켓이었는데 상단 값까지 덮었고, 그래서 한쪽 OS의 증상이 반년 넘게 가려졌다.

5. 새 술어를 만들었으면 옛 소비처 목록이 있나? 개념을 정교하게 만든 PR은 기존 소비처 마이그레이션 목록을 첨부한다. 안 옮기는 것도 적는다 — 이유와 함께. ③에 이 목록이 있었다면 헤더가 거기 올라왔을 것이다.

질문 하나는 도구로 옮겼다

다섯 개 중 온전히 기계에 맡길 수 있는 것은 1번뿐이다. 그래서 그것만 스크립트로 만들어 PR 생성 절차에 붙였다.

하는 일은 셋이다. 변경된 파일에서 내보낸 심볼을 몇 곳이 쓰는지 세고, 소비처가 끌 수 없는 변경(공용 컴포넌트의 기본 스타일, 기본 variant, 전역 셀렉터)인지 보고, 값을 덮는 수정(CSS 변수 override, !important, 전역 defaults 대입)인지 본다. 뒤의 둘 중 하나라도 걸리면 경고를 띄우고, 그때만 PR 본문에 영향 범위와 다섯 질문의 답을 적게 한다.

만들고 나서 ①과 ②의 과거 커밋에 적용해 봤다. 둘 다 잡혔다.

### 공용 상단 네비게이션 컴포넌트
- ⚠️ 소비처가 끌 수 없는 변경 — 이 파일을 쓰는 곳 전부에 적용됩니다
  - 기본 스타일 — variant로 못 끈다
- 소비처: 10곳 넘음

### 웹뷰 폴리필
- ⚠️ 값을 덮는 수정 — 원인을 없앤 것인지 관측을 지운 것인지 확인이 필요합니다
  - CSS 변수를 인라인으로 덮어쓰기

만들면서 스스로 걸린 것도 있다. 수백 개 파일이 바뀐 브랜치에 돌렸더니 공용 심볼을 건드린 파일이 수십 개 떴다. 전부 뿌리니 정작 위험한 두 줄이 묻혔다 — ②가 당한 실패를 도구가 그대로 재현하고 있었다. 기본 출력을 경고 있는 것만으로 줄이고, 전부 보려면 플래그를 주게 바꿨다.

2번(범위가 같은가)·3번(술어 이름이 맞나)·5번(옛 소비처를 옮겼나)은 기계에 넘기지 않았다. 4번도 도구는 신호만 띄울 뿐, 판단은 사람 몫이다. 앞에서 쓴 대로 그 판단에는 코드에 안 적힌 계약이 필요하다. 도구는 그 자리까지만 데려다주고 멈춘다.

남는 것

이 버그의 진짜 비용은 수십 픽셀의 여백이 아니었다. 1년 넘게 세 작업이 각자 자기 범위에서는 타당하게 진행됐는데, 합쳐진 결과가 틀렸다는 것이다. 어느 한 리뷰의 diff만으로는 그 연결을 보기 어려웠다.

원인과 발현 사이가 몇 달씩 벌어지면 한 번의 리뷰로 연결하기 어렵다. 그러니 개인의 기억력에 기대기보다 연결이 필요한 자리에 표시를 남겨야 한다. 소비처 개수, 마이그레이션 목록, override의 근거가 그 표시다.

리뷰를 더 열심히 하자는 얘기가 아니다. 리뷰가 볼 수 있는 것을 diff 밖으로 조금 넓히자는 얘기다.