Skip to content

fix: async APIHub 페이지네이션 절단 경고·검증 누락 (2인 적대적 리뷰 재검증) - #28

Merged
digitie merged 1 commit into
mainfrom
review/asyncio-reverify-kma
Sep 10, 2026
Merged

digitie merged 1 commit into
mainfrom
review/asyncio-reverify-kma

Conversation

@digitie

@digitie digitie commented Sep 10, 2026

Copy link
Copy Markdown
Owner

배경

src/kma의 asyncio 전환은 이전 PR #25에서 이미 완료·머지되었다. 이번 작업은 그 전환이
실제로 안전한지 독립된 서브에이전트 2명(동시성/자원관리 관점, 보안/데이터 무결성 관점)으로
다시 적대적 리뷰를 받아 재검증한 결과다.

발견 및 수정

두 리뷰어가 독립적으로 동일한 버그에 수렴했다: ApiHubClient.aiter_pages()
(AsyncApiHubClient.iter_pages()로도 노출)가 공용 pagination.aiter_pages() 헬퍼에
위임하지 않고 for offset in range(max_pages) 루프를 직접 구현하고 있었다.
DataGoKrClient.aiter_pages()는 이미 공용 헬퍼에 위임하는데(동기/비동기 대칭 원칙,
AGENTS.md) APIHub 쪽만 예외였다.

결과적으로:

  • max_pages에 도달했는데 더 가져올 페이지가 남아있어도, 동기 iter_pages()와 달리
    PaginationLimitWarning을 내지 않고 조용히 데이터를 잘랐다.
  • start_page/max_pages/max_items 입력값 검증이 아예 없었다 (max_pages=0이 에러 없이
    빈 결과를 반환).
  • tests/*.py에 aiter_pages 테스트가 전무해 CI로는 잡히지 않았다.

변경 사항

  • src/kma/apihub.py: aiter_pages()를 pagination.aiter_pages()에 위임하도록 재작성
    (datagokr.py와 동일 패턴). 더 이상 쓰이지 않는 _body_item_count()/_has_next_page import 제거.
  • tests/test_apihub.py: PagingFakeSession/AsyncPagingFakeSession fixture + 4개 테스트
    추가 (동기/비동기 페이지 수집 대칭성, 경고 발생 대칭성, 인자 검증 대칭성). 수정 전 코드로
    되돌려 새 테스트 2개가 실제로 실패함을 확인했다 (회귀 방지 검증됨).
  • CHANGELOG.md/docs/journal.md/docs/resume.md 갱신.

보안/데이터 무결성 관점 리뷰에서는 실제 버그 없음 — 자격증명 마스킹, resultCode 예외 매핑,
재시도/백오프 로직이 동기/비동기 경로에서 동일한 공용 함수를 공유함을 확인했다. (informational로
보고된 completeness gap — DataGoKrClient의 타입화 helper 다수가 async facade에 대응 메서드가
없음 — 은 버그/취약점이 아니라 이번 PR 범위에서 다루지 않았다.)

검증

python -m pytest -q       # 153 passed, 12 skipped (was 149 passed; +4 new tests)
python -m ruff check .    # All checks passed
python -m mypy src/kma    # Success: no issues found in 22 source files

Live e2e (실 API):

KMA_RUN_LIVE=1 DATA_GO_KR_SERVICE_KEY=*** KMA_APIHUB_AUTH_KEY=*** python -m pytest -m integration
# 9 passed, 3 skipped (skip 사유는 서비스키 구독 범위 밖, 기존과 동일 — 회귀 없음)

🤖 Generated with Claude Code

https://claude.ai/code/session_01VMed8e6u2BBD5CuokBQRhv

…dation

Two independent adversarial-review subagents (concurrency/resource-
management angle, security/data-integrity angle) re-verified the
existing asyncio conversion of src/kma and both converged on the same
bug: ApiHubClient.aiter_pages() (also exposed as
AsyncApiHubClient.iter_pages()) hand-rolled its own pagination loop
instead of delegating to the shared pagination.aiter_pages() helper,
unlike DataGoKrClient.aiter_pages() which already follows that
pattern. As a result it silently dropped PaginationLimitWarning when
max_pages was hit with more data upstream, and skipped
start_page/max_pages/max_items validation, causing silent truncation
for async APIHub consumers with zero test coverage.

Rewired aiter_pages to delegate to pagination.aiter_pages the same
way the sync/datagokr paths do, dropped the now-dead local
_body_item_count helper and unused _has_next_page import, and added
sync/async pagination test coverage (PagingFakeSession /
AsyncPagingFakeSession) that fails against the pre-fix code.

The security/data-integrity review pass found no real bugs elsewhere
— credential masking, result-code mapping, and retry/backoff are
shared functions used identically by sync and async paths.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01VMed8e6u2BBD5CuokBQRhv
@digitie
digitie merged commit 1f8d8df into main Sep 10, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant