[25.08.25 / TASK-247] Fix - Leaderboard 쿼리 결과값 버그 수정 #44
Conversation
Walkthrough리더보드 CTE가 정확한 일자(= date) 경계로 제한되고, start 통계의 NULL을 0으로 취급하도록 diff 계산이 변경되었습니다. 시작값이 없는 비정상 데이터를 제외하는 HAVING/WHERE 필터가 추가되었고, 관련 단위·통합 테스트가 확장·조정되었습니다. Changes
Sequence Diagram(s)sequenceDiagram autonumber participant C as Caller participant R as LeaderboardRepository participant Q as buildLeaderboardCteQuery participant DB as Database C->>R: getUserLeaderboard(metric, dateRange, limit) R->>R: compute nowDateKST (KST 보정), pastDateKST R->>Q: buildLeaderboardCteQuery(dateRange, nowDateKST, pastDateKST) Q-->>R: CTE (today_stats: date = nowDateKST, start_stats: date = pastDateKST) R->>DB: SELECT ... GROUP BY user ... HAVING SUM(COALESCE(ss.start_view,0)) != 0 DB-->>R: rows R-->>C: leaderboard Note over R,DB: NULL start_* → 0으로 취급하여 diff 계산 sequenceDiagram autonumber participant C as Caller participant R as LeaderboardRepository participant Q as buildLeaderboardCteQuery participant DB as Database C->>R: getPostLeaderboard(metric, dateRange, limit) R->>R: compute nowDateKST (KST 보정), pastDateKST R->>Q: buildLeaderboardCteQuery(dateRange, nowDateKST, pastDateKST) Q-->>R: CTE (date = 경계일로 제한) R->>DB: SELECT ... JOIN posts p ... WHERE p.released_at >= pastDateKST OR ss.post_id IS NOT NULL DB-->>R: rows R-->>C: leaderboard Note over R,DB: NULL start_* → 0으로 취급하여 diff 계산 Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code graph analysis (1)src/repositories/leaderboard.repository.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (5)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File ( |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (10)
src/repositories/leaderboard.repository.ts (7)
33-33: HAVING 절 의미를 명시적으로 단순화하고, 신규 유저 제외 여부를 명확히 하자현재 HAVING은 수학적으로
SUM(COALESCE(ss.start_view, 0)) != 0과 동치입니다. 가독성과 실행 계획 최적화를 위해 아래처럼 단순화하는 것을 권장합니다. 또한 이 조건은 기간 내 신규 게시물만 있는 유저(= start_view 합이 0)까지 제외합니다. 이것이 기획 의도라면 테스트로 고정하고, 아니라면 “기간 내 신규 게시물 보유”도 허용하도록 조건을 확장하세요.
- 단순화(동치):
- HAVING SUM(COALESCE(ts.today_view, 0)) != SUM(COALESCE(ts.today_view, 0) - COALESCE(ss.start_view, 0)) + HAVING SUM(COALESCE(ss.start_view, 0)) > 0
- 신규 게시물만 있는 유저 포함(의도 시):
- HAVING SUM(COALESCE(ts.today_view, 0)) != SUM(COALESCE(ts.today_view, 0) - COALESCE(ss.start_view, 0)) + HAVING + SUM(COALESCE(ss.start_view, 0)) > 0 + OR COUNT(DISTINCT CASE WHEN p.released_at >= '${pastDateKST}' AND p.is_active = true THEN p.id END) > 0
68-72: 비정상 데이터 제외 조건을 더 명확/일관되게 표현현 조건
COALESCE(ts.today_view, 0) != COALESCE(ts.today_view, 0) - COALESCE(ss.start_view, 0)은 본질적으로COALESCE(ss.start_view, 0) != 0과 동일합니다. 가독성과 옵티마이저를 위해 직접적으로 표현하세요. 또한 like 중심 정렬(likeCount)에서의 일관성을 위해 like 기준도 포함하는 안을 제안합니다.
- 명확화(동치):
- AND ( - p.released_at >= '${pastDateKST}' - OR - COALESCE(ts.today_view, 0) != COALESCE(ts.today_view, 0) - COALESCE(ss.start_view, 0) - ) + AND ( + p.released_at >= '${pastDateKST}' + OR COALESCE(ss.start_view, 0) > 0 + )
- like도 포함(선택):
- OR COALESCE(ss.start_view, 0) > 0 + OR COALESCE(ss.start_view, 0) > 0 + OR COALESCE(ss.start_like, 0) > 0
95-109: CTE 내 DISTINCT ON 사용과 파라미터 바인딩 개선 제안
- 동일 일자에 복수 레코드 가능성이 0이 아니라면
DISTINCT ON에ORDER BY가 없어서 비결정적입니다. 일자 고정이라면GROUP BY post_id와MAX로 안전하게 정규화하거나, (post_id, date) 유니크 인덱스로 무결성을 강제하는 방식을 권장합니다.- 날짜 리터럴을 문자열로 인라인하지 말고 바인딩 파라미터로 넘기면 계획 재사용과 안전성이 좋아집니다.
예시(선호):
today_stats AS ( - SELECT DISTINCT ON (post_id) - post_id, - daily_view_count AS today_view, - daily_like_count AS today_like - FROM posts_postdailystatistics - WHERE date = '${nowDateKST}' + SELECT + post_id, + MAX(daily_view_count) AS today_view, + MAX(daily_like_count) AS today_like + FROM posts_postdailystatistics + WHERE date = $2 + GROUP BY post_id ), start_stats AS ( - SELECT DISTINCT ON (post_id) - post_id, - daily_view_count AS start_view, - daily_like_count AS start_like - FROM posts_postdailystatistics - WHERE date = '${pastDateKST}' + SELECT + post_id, + MAX(daily_view_count) AS start_view, + MAX(daily_like_count) AS start_like + FROM posts_postdailystatistics + WHERE date = $3 + GROUP BY post_id )위 변경 시 상위 쿼리에서 파라미터 배열을
[limit, nowDateKST, pastDateKST]로 맞추어 주면 됩니다. 테스트는expect.stringContaining('WHERE date =')를 사용하고 있어 그대로 통과합니다.
86-91: buildLeaderboardCteQuery 시그니처 정리현재
dateRange와pastDateKST?를 동시에 받되, 항상 호출부에서pastDateKST를 넘깁니다. 이 경우dateRange는 사실상 무의미해집니다. 혼동 방지를 위해 한 가지로 통일하세요.
- 간소화(호출부가 계산):
- private buildLeaderboardCteQuery(dateRange: number, pastDateKST?: string) { - const nowDateKST = getCurrentKSTDateString(); - if (!pastDateKST) { - pastDateKST = getKSTDateStringWithOffset(-dateRange * 24 * 60); - } + private buildLeaderboardCteQuery(pastDateKST: string, nowDateKST = getCurrentKSTDateString()) {
- 또는 내부 계산만 하도록 하고 호출부에서
pastDateKST를 넘기지 않는 형태로 일원화하십시오.
13-13: cteQuery 인자 전달 일관화 필요위 시그니처 정리 제안과 결을 맞춰, 여기서도
dateRange없이pastDateKST만(또는 반대로) 전달하도록 통일하면 가독성과 유지보수성이 좋아집니다.
100-100: 오늘자 고정 일자 사용 시 데이터 수집 지연에 대한 방어 로직 검토
WHERE date = nowDateKST로 “오늘” 스냅샷이 아직 적재되지 않은 경우ts가 NULL이 되어view_diff = 0 - start_view로 음수가 될 수 있습니다. 이것이 의도라면 그대로 두되, 아니라면 다음 중 하나를 고려하세요:
- 오늘자 미존재 시 가장 최근 일자(<= nowDateKST)로 대체
- 오늘자 미존재 레코드는 제외
- 음수 diff를 0으로 클램프
필요 시 조건/테스트를 보강하는 것이 안전합니다.
108-108: 인덱스 점검 제안: (post_id, date) 또는 (date, post_id)본 쿼리는
date = ...필터와post_id조인을 반복 사용합니다.posts_postdailystatistics테이블에(date, post_id)또는(post_id, date)복합 인덱스가 없다면 추가를 권장합니다. CTE의 두 스캔 모두에 도움이 됩니다.src/repositories/__test__/leaderboard.repo.test.ts (3)
82-85: date 필터 검증을 더 견고하게현재는
'WHERE date ='포함 여부만 확인합니다. CTE에 두 번 등장하므로(오늘/기준일) 두 번 모두 존재하는지 검증하면 회귀에 강합니다.- expect(mockPool.query).toHaveBeenCalledWith( - expect.stringContaining('WHERE date ='), // pastDateKST를 사용하는 부분 확인 - [expect.any(Number)], // limit - ); + const sql = mockPool.query.mock.calls.at(-1)?.[0] as string; + expect(sql.match(/WHERE date\s*=/g)?.length).toBeGreaterThanOrEqual(2);
168-171: post 리더보드도 동일하게 date 필터 2회 검증 권장위 사용자 리더보드와 동일하게 두 번의
WHERE date =존재를 검증하면 안정적입니다.- expect(mockPool.query).toHaveBeenCalledWith( - expect.stringContaining('WHERE date ='), // pastDateKST를 사용하는 부분 확인 - [expect.any(Number)], // limit - ); + const sql = mockPool.query.mock.calls.at(-1)?.[0] as string; + expect(sql.match(/WHERE date\s*=/g)?.length).toBeGreaterThanOrEqual(2);
173-180: 문자열 매칭을 덜 취약하게(동등 의미로 테스트)현재 검증식은 수학적으로
COALESCE(ss.start_view, 0) != 0와 동치입니다. 구현 변경(동치 변환)에도 테스트가 깨지지 않도록 조건의 의미를 검증하는 방향을 권장합니다.- expect(mockPool.query).toHaveBeenCalledWith( - expect.stringContaining('COALESCE(ts.today_view, 0) != COALESCE(ts.today_view, 0) - COALESCE(ss.start_view, 0)'), - expect.anything() - ); + const sql = mockPool.query.mock.calls.at(-1)?.[0] as string; + expect(sql).toMatch(/COALESCE\(\s*ss\.start_view\s*,\s*0\s*\)\s*(!=|>|>=)\s*0/);또한 like 기준 정렬 케이스를 대비해 view/like 둘 다 커버하는 테스트를 추가하는 것을 고려하세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/repositories/__test__/leaderboard.repo.test.ts(2 hunks)src/repositories/leaderboard.repository.ts(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/repositories/leaderboard.repository.ts (1)
src/utils/date.util.ts (2)
getKSTDateStringWithOffset(44-57)getCurrentKSTDateString(10-21)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-and-test (20)
- GitHub Check: build-and-test (22)
🔇 Additional comments (3)
src/repositories/leaderboard.repository.ts (2)
24-25: diff 계산 시 시작값(NULL)→0 처리: 버그 원인 정확히 해결됨
COALESCE(ss.start_*, 0)로 기준값 부재 시 0을 사용하도록 바뀐 점이 핵심 이슈(초기 통계 레코드 무시)와 정합합니다. 이 변경으로 과소계산 문제가 제거됩니다.
61-63: 포스트 단위 diff 계산 로직 정합
today - start(없으면 0)계산으로 통일되어 사용자/게시물 모두 일관성 있습니다. 의도대로 동작합니다.src/repositories/__test__/leaderboard.repo.test.ts (1)
87-94: 비정상 유저 필터링 검증 추가: 의도 명확HAVING 절 존재 검증은 변경 의도를 잘 캡쳐합니다. 유지하세요.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/repositories/__test__/integration/leaderboard.repo.integration.test.ts (4)
12-12: KST 기준 경계 계산 유틸 도입은 적절합니다. 다만 문자열 포맷 의존성에 유의하세요.getKSTDateStringWithOffset이 반환하는
"YYYY-MM-DD 00:00:00+09"문자열은 ISO8601 표준 형식(T,+09:00)과 미묘하게 다릅니다. 본 파일 내 다른 검증(아래 released_at 비교)에서 문자열 비교·파싱을 수행할 경우 런타임/플랫폼에 따라 예외나 잘못된 비교가 발생할 수 있어요. 해당 비교 로직은 시간 객체(Date) 기반으로 다루는 방향을 권장합니다.
15-15: Jest 타임아웃(30초)과 DB statement_timeout(30초)이 동일 — 여유 버퍼를 두는 편이 안전합니다.쿼리 실행 + 네트워크 + 직렬화/역직렬화 + assertion까지의 오버헤드가 존재합니다. 동일한 30초로 맞추면 간헐적 타임아웃 플래키가 생길 수 있습니다. 테스트 안정성을 위해 Jest 타임아웃을 45~60초로 상향하거나, DB statement_timeout을 25초 수준으로 낮추는 것 중 하나를 권장합니다.
다음 중 하나의 패치를 제안합니다(둘 중 한 가지 방향만 적용):
- jest.setTimeout(30000); // 각 케이스당 30초 타임아웃 설정 + jest.setTimeout(45000); // 각 케이스당 45초 타임아웃(쿼리 타임아웃 대비 여유 15초)
46-46: statement_timeout 주석 문구와 의도 확인 필요 + Jest 타임아웃과의 간격 권장.
- 코멘트가 “쿼리 타임아웃 증가 (30초)”라고 되어 있으나, 상단 요약에 따르면 60→30초로 “감소”일 수 있습니다. 주석을 오해 없도록 정정하는 것이 좋습니다.
- Jest 타임아웃과 동일(30초)하게 두면 경계 여유가 없습니다. 아래 둘 중 하나를 고려해 주세요.
- statement_timeout: 30000, // 쿼리 타임아웃 증가 (30초) + statement_timeout: 25000, // 쿼리 타임아웃 25초 (Jest 45초 대비 여유 확보)또는 위 라인은 그대로 두고 Line 15의 Jest 타임아웃을 45초로 상향(별도 코멘트 참조).
81-92: afterAll의 1초 대기 두 번은 과합니다 — 종료 지연을 최소화하세요.
await testPool.end()는 내부적으로 커넥션 정리를 보장합니다. 추가로 2초 sleep은 테스트 러닝타임을 불필요하게 늘릴 수 있어요. 실제로 “Jest did not exit…” 경고를 완화하려는 의도라면, 우선 100~200ms 수준으로 축소해 보고, 필요 시에만 점진적으로 늘리는 쪽이 낫습니다.- // 모든 쿼리 완료 대기 - await new Promise(resolve => setTimeout(resolve, 1000)); + // 모든 쿼리 완료 대기 (최소 지연) + await new Promise(resolve => setTimeout(resolve, 150)); // 풀 완전 종료 if (testPool) { // 강제 종료: 모든 활성 쿼리와 연결 중지 await testPool.end(); } - // 추가 정리 시간 - await new Promise(resolve => setTimeout(resolve, 1000)); + // 추가 정리 시간 (최소 지연) + await new Promise(resolve => setTimeout(resolve, 150));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/repositories/__test__/integration/leaderboard.repo.integration.test.ts(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/repositories/__test__/integration/leaderboard.repo.integration.test.ts (1)
src/utils/date.util.ts (1)
getKSTDateStringWithOffset(44-57)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-test (22)
🔇 Additional comments (1)
src/repositories/__test__/integration/leaderboard.repo.integration.test.ts (1)
23-23: describe.skip 제거로 통합 테스트가 CI에 반영됩니다 — LGTM.실데이터·실DB 의존 테스트임을 상단 경고 주석으로 명확히 해둔 것도 좋습니다. isEnoughData 가드로 데이터 부족 시 스킵 처리하는 접근도 적절합니다.
src/repositories/__test__/integration/leaderboard.repo.integration.test.ts Show resolved Hide resolved
src/repositories/__test__/integration/leaderboard.repo.integration.test.ts Show resolved Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
src/repositories/leaderboard.repository.ts (6)
13-13: pastDateKST를 외부에서 계산해 CTE에 주입한 점은 적절합니다.호출부에서 범위를 명확히 고정해 CTE 내부 계산을 단순화했다는 점이 좋습니다. 다만 동일한 호출 내 결정론 보장을 위해 nowDateKST도 호출부에서 계산해 함께 주입하는 구조를 고려해볼 만합니다(아래 CTE 코멘트 참고).
86-95: KST 00:00~00:59(UTC 15시) 예외 처리 로직은 현실 문제를 정확히 겨냥합니다.단, 다음 2점은 추후 유지보수성/테스트 안정성을 위해 고려해볼 만합니다.
- nowDateKST도 호출부에서 파라미터로 넘기면(예:
buildLeaderboardCteQuery(dateRange, pastDateKST, nowDateKST?)) 단위 테스트에서 경계 시간을 안정적으로 주입·검증할 수 있습니다.- 이 판단 로직을
date.util.ts로 이동해 “리더보드 전용 기준일 계산 함수”로 공통화하면 다른 리포지토리/서비스에서도 일관되게 재사용 가능합니다.예시(선택):
- private buildLeaderboardCteQuery(dateRange: number, pastDateKST?: string) { - const nowDateKST = - new Date().getUTCHours() === 15 - ? getKSTDateStringWithOffset(-24 * 60) - : getCurrentKSTDateString(); + private buildLeaderboardCteQuery(dateRange: number, pastDateKST?: string, nowDateKST?: string) { + const resolvedNowDateKST = nowDateKST ?? ( + new Date().getUTCHours() === 15 + ? getKSTDateStringWithOffset(-24 * 60) + : getCurrentKSTDateString() + ); … - WHERE date = '${nowDateKST}' + WHERE date = '${resolvedNowDateKST}'
100-114: DISTINCT ON을 유지하려면 ORDER BY가 필요하고, 그렇지 않다면 DISTINCT ON 자체를 제거하는 편이 명확합니다.현재는
WHERE date = '...'로 단일 날짜만 조회하므로 (post_id, date)가 유니크라는 전제라면 DISTINCT ON이 불필요하고 쿼리 최적화에도 도움이 됩니다. 만약 데이터 품질상 중복 가능성이 있다면 명시적인ORDER BY post_id, <결정 기준>를 둬야 예측 가능합니다. 유니크 제약이 있다면 아래처럼 단순화하세요.today_stats AS ( - SELECT DISTINCT ON (post_id) + SELECT post_id, daily_view_count AS today_view, daily_like_count AS today_like FROM posts_postdailystatistics WHERE date = '${nowDateKST}' ), start_stats AS ( - SELECT DISTINCT ON (post_id) + SELECT post_id, daily_view_count AS start_view, daily_like_count AS start_like FROM posts_postdailystatistics WHERE date = '${pastDateKST}' )또는 데이터 정합성 측면에서 (post_id, date)에 유니크 인덱스/제약이 없다면 그 추가를 검토해 주세요.
15-36: 날짜 리터럴의 문자열 삽입은 안전하긴 하나(내부 생성값), 바인딩 파라미터로 전환하면 플랜 재사용/안전성이 향상됩니다.현재
${pastDateKST}/${nowDateKST}를 문자열로 인라인합니다. 공격면은 낮지만,pg의 바인딩($1,$2…)을 쓰면:
- 서버 측 준비된 플랜 재사용으로 CPU/파싱 비용이 줄고,
- 잠재적 SQL 인젝션 표면을 구조적으로 차단합니다.
구현 난이도를 낮추려면
buildLeaderboardCteQuery(...)가queryText와params를 함께 반환하게 리팩터링하는 방식을 추천합니다(예:{ cte, params: [nowDateKST, pastDateKST] }).Also applies to: 51-75
118-122: 정렬 매핑을 사용자/게시물로 분리해 타입·실행 시 안정성을 높이세요.현재 공용 매핑(
postCount포함)을 양쪽에서 재사용합니다. 만약PostLeaderboardSortType이postCount를 포함하지 않더라도, 런타임에 잘못 전달되면ORDER BY post_diff로 이어져 컬럼 미존재 오류가 날 수 있습니다. 매핑을 분리해 방어하는 편이 안전합니다.- private readonly SORT_COL_MAPPING = { - viewCount: 'view_diff', - likeCount: 'like_diff', - postCount: 'post_diff', - } as const; + private readonly USER_SORT_COL_MAPPING = { + viewCount: 'view_diff', + likeCount: 'like_diff', + postCount: 'post_diff', + } as const; + private readonly POST_SORT_COL_MAPPING = { + viewCount: 'view_diff', + likeCount: 'like_diff', + } as const;그리고 사용부:
- ORDER BY ${this.SORT_COL_MAPPING[sort]} DESC, u.id + ORDER BY ${this.USER_SORT_COL_MAPPING[sort]} DESC, u.id- ORDER BY ${this.SORT_COL_MAPPING[sort]} DESC, p.id + ORDER BY ${this.POST_SORT_COL_MAPPING[sort]} DESC, p.id타입 선언(
UserLeaderboardSortType,PostLeaderboardSortType)이 분리 매핑과 호환되는지 컴파일 체크를 통해 확인해 주세요.
105-105: 인덱스 관점 메모:WHERE date = '...'패턴을 많이 사용하므로(date, post_id)또는(post_id, date)복합 인덱스를 확인/추가하세요.조인 키가
post_id인 만큼, 워크로드 특성에 따라(date, post_id)가 더 유리할 때가 많습니다. 실제 쿼리 플랜(ANALYZE)로 선택도를 확인해 결정하시길 권장합니다.Also applies to: 113-113
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/controllers/user.controller.ts(0 hunks)src/repositories/__test__/integration/post.repo.integration.test.ts(0 hunks)src/repositories/leaderboard.repository.ts(6 hunks)
💤 Files with no reviewable changes (2)
- src/repositories/test/integration/post.repo.integration.test.ts
- src/controllers/user.controller.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/repositories/leaderboard.repository.ts (1)
src/utils/date.util.ts (2)
getKSTDateStringWithOffset(44-57)getCurrentKSTDateString(10-21)
🔇 Additional comments (3)
src/repositories/leaderboard.repository.ts (3)
24-25: NULL 시작값을 0으로 베이스라인하는 diff 계산 변경이 버그 원인과 일치합니다.
COALESCE(ss.start_view, 0)/COALESCE(ss.start_like, 0)사용으로 “시작 통계 부재=0” 처리가 명확해졌습니다. 이 변경은 총계 API와의 불일치를 해소하는 데 필요한 방향입니다.
48-50: post 리더보드도 pastDateKST를 호출부에서 계산해 주입 — 사용자 리더보드와의 일관성 확보 측면에서 좋습니다.
61-62: post 단위 diff 계산의 0 베이스라인 처리도 적절합니다.사용자 리더보드와 계산 정의가 일치하며, 누락된 시작 통계를 0으로 간주하려는 의도에 부합합니다.
| | ||
| type Token10 = string & { __lengthBrand: 10 }; | ||
| | ||
| // eslint-disable-next-line @typescript-eslint/naming-convention |
There was a problem hiding this comment.
혹시 이 부분 네이밍 컨벤션은 왜 ignore하셨을까요?
현우님 말대로 경고가 뜨는게 맞을 것 같은데, 혹시 강제로라도 무시할 이유가 있을까요?
There was a problem hiding this comment.
좋았던 점
- "존재하는 통계 데이터 중 n일 전부터 가장 과거의 데이터" 에서 대소 비교라,, 디테일한 부분에서 미싱인데 통합 테스팅 살려주시면서 동시에 해결해주셔서 감사해요!! order by 하나로 기대 수행시간이 많이 줄었다니,, scan range 가 중요한게 세삼 와닿네요.
- PR detail 에 세부 내용 작성해주셔서 F/U 이 쉬웠어요!! 감사해요!!
아쉬운 점
- "데이터 수집이 비정상적인 게시물" 에 대해 저희가 쉽게 인지할 방법이 있을까요? API 호출 시 비정상적인 것들에 대한 로깅이라던지 등의 방법으로요!
There was a problem hiding this comment.
코드 잘 읽었습니다. 고생하셨습니다!!🔥
좋았던 점
- 문제, 원인, 해결한 방법까지 잘 정리해주셔서 해당 PR에 대해 빠르게 이해할 수 있었던 것 같습니다!
- 데이터 수집이 비정상적인 유저 또는 게시물에 대한 처리를 꼼꼼하게 해주신 부분이 좋았습니다.
- KST 기준 00시~01시 사이라면 전날 데이터를 사용하도록 하신 부분도 인상 깊었습니다.
아쉬운 점
- 현우님 말씀처럼 비정상적인 게시물에 대해 로그를 남겨, 로깅 레벨이 심각한 로그만 슬랙을 보내는 것도 좋을 것 같아요!
🔥 변경 사항
view_diff와 leaderboard API에서 보는view_diff값의 불일치 (리더보드가 더 적게 표시)n일전 통계로존재하는 통계 데이터 중 n일 전부터 가장 과거의 데이터를 사용함. => 해당 기간 내 발행된 새 글의 첫 통계 데이터가 무시됨새로 발행된 글외에도토큰 만료 사용자가 기간 내 복구한 경우,vd 신규유저,기타 해당 일에 수집이 안 된 경우가 있기 때문에 HAVING, WHERE절 추가로 예외 처리함이번에 쿼리 수정하면서 당일의 데이터만 사용하기 때문에 불필요해진 order by 절을 삭제했더니 쿼리 실행 시간이 1/5 수준으로 줄어들었습니다. (참고) 그래서 통합 테스트도 살려두었습니다.
다만 문제는...
코드래빗 코멘트대로 정상이지만 필터링될 예외 케이스가 있긴 있음 (아주 드물 것이라고 생각합니다)
a. 해당 기간내에 발행한 글이 전체 글인 사용자 (신규 유저 등...)
b. 오래된 글이지만, 해당 기간내 집계된 조회수가 전체 조회수인 게시물
당일의 데이터만 사용하여, 전체 유저 데이터 수집이 덜 되었을 KST 00시~01시에는 값이 이상할 수 있음예외 로직 추가 완료🏷 관련 이슈
📸 스크린샷 (UI 변경 시 필수)
없음
📌 체크리스트
Summary by CodeRabbit
Bug Fixes
Tests
Style