fix: 릴리즈 리뷰 반영 (SDL 동작 단정 전수 정정) - #295
Conversation
릴리즈 PR #289의 Codex 지적 13건 중 12건이 같은 유형이었다 —
"SDL description이 코드가 보장하지 않는 동작을 단정한다".
지적된 자리만 막는 대응을 멈추고 해당 유형의 입력 공간을 전수 대조했다.
지적 2건 직접 반영:
- WriteReviewInput: "이미지 최대 10장 또는 영상 1개"는 배타로 읽히지만
validateMedia는 IMAGE/VIDEO를 독립적으로 세므로 혼합 11개까지 유효하다.
- WriteReviewMediaInput.sortOrder: writeReview가 `sort_order: i`(배열 인덱스)로
저장해 입력값을 버린다. 동작 변경은 릴리즈 범위 밖이라 계약을 명시하는 쪽으로 정정.
전수 대조로 추가 발견:
- sortOrder 미지정 기본값 2건 — SellerCreateFaqTopicInput("서버가 정한 기본값"이라
모호), SellerCreateBannerInput(누락). 실제로는 둘 다 `?? 0`이라 맨 뒤가 아니다.
SellerAddProductImageInput은 `?? count`라 "맨 뒤" 서술이 정확해 유지.
- limit 상한 16건 — SDL 입력 31개 중 16개가 @max를 안 적고 있었다. 상한은 DTO에만
있어 SDL에서 보이지 않는데 초과하면 ValidationPipe가 BAD_REQUEST를 낸다.
"기본 N, 1~M만 허용하며 벗어나면 BAD_REQUEST"로 통일.
- statusHistories 정렬 1건 — 구매자 상세는 changed_at asc, 판매자 상세는 desc로
서로 반대인데 판매자 쪽만 "최신순"이 적혀 있었다. 구매자 쪽에 방향을 명시.
대조했으나 코드와 일치해 손대지 않은 것:
스냅샷/시점 단정 59건, null 배타 열거 2건, 정렬 단정 62건, 배타/택일 12건.
docs:check BASELINE을 현재 수치로 갱신(#290~#294 머지로 요소가 늘어 스냅샷이
낡아 있었다). 동작 변경 없음 — SDL description과 자동생성 타입 주석만 바뀐다.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🩺 NestJS Doctor — 90/100 (Excellent)진단 313건 (error 0).
architecture / security 상위 항목
|
🧹 knip — dead-code 리포트전체 리포트
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ea195e553
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| """EVENT 카테고리 ID. 비우면 '전체' 칩(필터 미적용).""" | ||
| categoryId: ID | ||
| """이미지 수(3열 그리드 9개 단위 — figma 명세 기준).""" | ||
| """이미지 수(기본 9 — 3열 그리드 9개 단위, figma 명세 기준). 1~30만 허용하며 벗어나면 BAD_REQUEST.""" |
There was a problem hiding this comment.
When a client supplies an out-of-range limit, the ValidationPipe raises an HTTP 400 exception, but src/global/filters/graphql-exception.filter.ts maps that status to GraphQL extensions.code = BAD_USER_INPUT, not BAD_REQUEST. The same new wording appears on every limit description changed here, so generated schema documentation directs clients to handle an error code they will never receive; use BAD_USER_INPUT or explicitly describe statusCode: 400 instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
반영: graphql-exception.filter.ts:21이 HttpStatus.BAD_REQUEST → 'BAD_USER_INPUT'으로 매핑한다. FE가 받는 extensions.code는 BAD_USER_INPUT이므로 지적대로 SDL 문구가 틀렸다.
같은 유형 반복이라 자리별 대응을 멈추고 SDL 전체의 에러 코드 표기를 전수 대조했다.
BAD_REQUEST32건 — 이번 PR에서 내가 넣은 16건 + 기존에 이미 있던 16건. 전부BAD_USER_INPUT(400)으로 교체. statusCode를 괄호에 붙여 둘을 혼동하지 않게 했다.NOT_FOUND56건 ·FORBIDDEN50건 — 매핑 테이블과 이름이 같아 정확. 그대로 둠.- 매핑에 없는 status를 쓰는 곳이 있는지도 확인했다 → 아래 별건 보고.
별건(이 PR 범위 밖, 문서가 아니라 동작 문제): ConflictException(409)이 4곳에서 던져지는데(user-profile.service.ts:53,88 닉네임 중복 · account.repository.ts:105 · user-review.service.ts:113 리뷰 중복) 409는 STATUS_TO_CODE에 없어 mapStatusToCode가 INTERNAL_SERVER_ERROR를 돌려준다. FE가 "이미 리뷰함"과 서버 장애를 구분할 수 없다. 릴리즈 중인 문서 PR에서 에러 계약을 바꾸는 건 부적절해서 별도 이슈로 올린다.
| """FAQ 답변 본문(HTML).""" | ||
| answerHtml: String! | ||
| """노출 순서. 미지정 시 서버가 정한 기본값이 들어간다.""" | ||
| """노출 순서. 미지정 시 0이 들어간다(맨 뒤가 아니다).""" |
There was a problem hiding this comment.
When existing FAQ topics also have the default sortOrder of 0, omitting this field gives the new topic 0 and SellerRepository.listFaqTopics orders ties by ascending id, placing the newly created topic last. Negative sort orders are also accepted, so 0 cannot generally guarantee a non-final position. The equivalent banner wording is likewise unsupported; describe only the actual default value rather than asserting (맨 뒤가 아니다).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
반영: 지적이 맞다. listFaqTopics는 orderBy: [{ sort_order: 'asc' }, { id: 'asc' }]라 기존 항목이 전부 0이면 새 항목이 맨 뒤로 간다. @IsInt()만 있고 @Min이 없어 음수도 허용되므로 0이 비최종 위치를 보장하지도 못한다.
(맨 뒤가 아니다)는 앞선 라운드에서 내가 쓴 표현인데, 이번 PR에서 2자리에 더 퍼뜨렸다. 전수로 걷어내고 목록별 실제 정렬에 맞춰 다시 썼다 — 5자리가 같은 문구였지만 뒤의 정렬은 3종으로 달랐다.
| 자리 | 실제 정렬 | 새 문구 |
|---|---|---|
| 옵션 그룹 · 옵션 아이템 · 커스텀 문구 토큰 | sort_order asc (동률 tie-break 없음) |
동률 순서는 보장하지 않는다고 명시 |
| FAQ 토픽 | [sort_order asc, id asc] |
기존이 모두 0이면 맨 뒤에 온다 |
| 배너 | 구매자 조회가 [sort_order asc, id asc]로 1건만 선택 |
기존이 모두 0이면 새 배너는 뽑히지 않는다 |
배너는 판매자 목록(listBanners)이 id desc라 sort_order를 아예 안 쓴다는 것도 이번에 확인해서, 구매자 노출 기준임을 문구에 박았다.
SellerAddProductImageInput.sortOrder는 ?? count라 "맨 뒤에 붙는다"가 실제로 정확해 그대로 뒀다.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Coverage report
Test suite run success1875 tests passing in 225 suites. Report generated by 🧪jest coverage report action from 558c6a0 |
Codex 2차 지적 2건. 둘 다 이 PR에서 내가 여러 자리에 동시에 퍼뜨린 문구라 지적된 자리가 아니라 문구 전체를 훑었다. - BAD_REQUEST → BAD_USER_INPUT (32건): graphql-exception.filter가 400을 BAD_USER_INPUT으로 매핑하므로 FE가 받는 extensions.code와 달랐다. 이 PR에서 넣은 16건 외에 기존 16건이 더 있었다. NOT_FOUND·FORBIDDEN은 매핑과 이름이 같아 그대로. - "(맨 뒤가 아니다)" 제거 (5건): FAQ·배너는 [sort_order asc, id asc]라 기존이 전부 0이면 새 항목이 맨 뒤로 가고, @min이 없어 음수도 허용된다. 5자리가 같은 문구였지만 실제 정렬은 3종이라 각각에 맞춰 다시 썼다. (옵션 3종: tie-break 없음 / FAQ: id asc / 배너: 구매자 조회 1건 선택) 하우스 스타일에 맞춰 장황한 문구도 걷어냈다 — 반복 수정으로 붙은 (400) 32건, WriteReviewMediaInput 타입·필드 설명 중복, 3줄짜리 sortOrder 설명 5건.
1 participant
배경
릴리즈 PR #289의 Codex 지적 13건을 분류한 결과 12건이 같은 유형이었다 — "SDL description이 코드가 보장하지 않는 동작을 단정한다". 유일한 예외인 첫 지적(seller 대화 커서 정렬 불일치)만 실제 코드 버그였고 #290에서 해소했다.
그 12건 안에서도 하위 유형이 반복됐다: sortOrder 의미 2회, null 배타 열거 2회, 본문/링크 필드 배타성 2회. 지적된 자리만 막고 유형을 안 훑은 게 라운드가 늘어난 원인이라, CLAUDE.md 절차대로 개별 대응을 멈추고 전수 대조로 전환했다.
대조 방법
SDL description 1,352건에서 검증 가능한 동작 단정 376건을 추출해 코드와 대조했다. 검출기는 양극성 probe(잡혀야 할 자리 / 안 잡혀야 할 자리)로 자체 검증한 뒤 결과를 인용했다 — 초기 시도에서 정규식 이스케이프 오류로 정상 자리까지 전부 위반으로 찍힌 적이 있어, "0건"이든 "전건"이든 검출기부터 반증했다.
변경
지적 2건 직접 반영
WriteReviewInput— "이미지 최대 10장 또는 영상 1개"는 배타로 읽히지만validateMedia는 IMAGE/VIDEO를 독립적으로 센다. 혼합 11개까지 유효.WriteReviewMediaInput.sortOrder—writeReview가sort_order: i(배열 인덱스)로 저장해 입력값을 버린다. 값을 존중하도록 바꾸면 중복·결번 입력의 정규화 규칙을 새로 정해야 해서 릴리즈 범위를 넘는다. 배열 순서가 곧 노출 순서라는 현재 계약을 명시하는 쪽을 택했다.전수 대조로 추가 발견 17건
@Max를 안 적고 있었다. 상한은 DTO에만 있어 SDL에서 안 보이는데 초과 시ValidationPipe가 BAD_REQUEST를 낸다SellerCreateFaqTopicInput(모호),SellerCreateBannerInput(누락). 실제로는 둘 다?? 0이라 맨 뒤가 아니다changed_at asc, 판매자 상세는desc로 서로 반대인데 판매자 쪽만 "최신순"이 적혀 있었다limit 16건 + sortOrder 2건 + 정렬 1건 = 19건이나,
MyReviewableOrderItemsInput/MyReviewsInput이 같은 파일이라 파일 수는 13개.반대 방향 확인도 했다 —
SellerAddProductImageInput.sortOrder는?? count라 "맨 뒤에 붙는다"가 정확해서 그대로 뒀다.대조했으나 코드와 일치해 손대지 않은 것
*_snapshot컬럼을 읽는 게 맞았다 (NotificationItem.productName의 "주문 알림만 스냅샷" 분기까지 일치)orderBy와 일치기타
docs:checkBASELINE을 현재 수치로 갱신했다. #290~#294 머지로 요소가 늘어(input 타입 72→73, input 필드 227→229, 출력 필드 603→618) 스냅샷이 낡아 있었다. 스크립트 주석이 "커버리지가 바뀌면 BASELINE도 함께 갱신한다"고 정해 둔 대로다.검증
yarn validate통과 — 225 suites / 1875 testsbuildSchema통과 (42개 파일, 232개 타입) — 파일 단위 파싱이 놓치는 타입 누락 없음docs:check8개 카테고리 전부 100% 유지동작 변경 없음. SDL description과 그로부터 생성되는 타입 주석만 바뀐다. 테스트를 추가하지 않은 이유도 같다.
2차 반영 (Codex 리뷰 2건)
둘 다 이 PR에서 내가 여러 자리에 동시에 퍼뜨린 문구라, 지적된 자리가 아니라 문구 전체를 훑었다.
BAD_REQUEST는 FE가 받는 코드가 아니다graphql-exception.filter.ts:21이 400 →BAD_USER_INPUT매핑NOT_FOUND·FORBIDDEN은 매핑과 이름이 같아 유지(맨 뒤가 아니다)는 보장 못 한다[sort_order asc, id asc]라 기존이 전부 0이면 새 항목이 맨 뒤.@Min없어 음수도 허용반복 수정으로 붙은 군더더기도 함께 걷어냈다:
(400)32건,WriteReviewMediaInput타입·필드 설명 중복, 3줄짜리 sortOrder 설명 5건 → 기존 하우스 스타일(한 줄)로.별건 발견 (이 PR 범위 밖):
ConflictException(409)이 4곳에서 던져지는데 409가STATUS_TO_CODE에 없어INTERNAL_SERVER_ERROR로 나간다. FE가 "이미 리뷰함"과 서버 장애를 구분할 수 없다. 릴리즈 중인 문서 PR에서 에러 계약을 바꾸지 않고 별도 이슈로 올린다.최종 변경 규모: 22개 파일, +46 / −44.