Repository navigation
적대적 코드 리뷰: src/krex 정확성·보안·성능 수정 + CI 추가 - #12
Merged
Merged
Conversation
…, performance fixes 4 independent reviewer subagents (correctness/concurrency, security, API design, performance/reliability) audited src/krex; every finding was adversarially re-verified by a separate skeptic agent (reading the actual code + reproducing the failure) before being applied. 37 raw findings. Highlights: - client.py: _parse_page() discarded the ENTIRE page when just one row failed to parse -- for traffic.incident() (real-time accident/incident SMS feed), one unrelated malformed field on one incident wiped out every other, currently-active incident in the same response. Now skips unparseable rows and only raises if the whole page fails to parse. - client.py: _get()'s multi-key fallback only checked whether a key was *present* in the row dict, not whether its value was non-blank, so a blank entrpsNm masked a populated restAreaNm and aborted the whole national rest-area listing. Now falls through to the next alias when a value is blank. - client.py: debug_call() now validates the function exists in the catalog before invoking it; latest_weather() gained a max-lookback cap and clarified naive-datetime (KST-assumed) docstring; _enum_or_none()'s TypeVar was unbound (using the wrong T instead of a bound E), requiring a type: ignore[call-arg] that's no longer needed. - _http.py: additional secret-redaction and error-handling fixes (see the full diff for _mask_params/_ex_items changes). Fixed a regression the fix pass itself introduced: _env.py's load_local_env() was wrapped in @lru_cache keyed only on the `path` argument, but the function resolves relative to cwd when path is None -- tests that monkeypatch.chdir() to an isolated tmp_path and expect a fresh .env read got a stale cached result from whichever directory called it first (verified: this session's own earlier exploration of the real .env leaked into test_client_loads_local_dotenv_keys_by_default's isolated fixture, returning the real 10-digit KEX_EX_API_KEY instead of the test's "exkey"). Reverted the cache -- this function's whole purpose is reading a file that can legitimately change between calls, so caching it is unsafe regardless of cwd. Also fixed 4 pre-existing ruff line-length violations the fix pass introduced while editing _http.py (E501, one auto-fixed, three manually reflowed). Also adds .github/workflows/ci.yml (lint/typecheck/test on Python 3.11-3.13). Verified against real servers: `KEX_LIVE=1 pytest -m live` with the KEX_EX_API_KEY/KEX_GO_API_KEY/DATA_GO_KR_SERVICE_KEY present in this checkout's .env -- 5 passed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XF9V2q4mAmhmXn6t5G9Hfe
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
개요
src/krex전체를 4명의 독립된 전문 리뷰어 서브에이전트가 적대적으로 리뷰했습니다. 각 발견은 별도 검증 에이전트가 실제 코드를 다시 읽고 반박을 시도한 뒤 확정했습니다. (방법론: digitie/python-mois-api#9)가장 중요한 수정 — 행 하나가 페이지 전체를 날림 (실시간 사고정보에 특히 위험)
client.py:_parse_page()가 행 하나만 파싱에 실패해도 페이지 전체를 버렸습니다.traffic.incident()(실시간 사고 문자정보)에서 무관한 필드 하나가 깨지면 실제로 활성 중인 다른 사고 정보까지 전부 사라졌습니다. 이제 파싱 불가능한 행만 건너뛰고, 전부 실패했을 때만 예외를 던집니다.client.py:_get()의 다중 키 fallback이 값이 아니라 키의 존재 여부만 확인해,entrpsNm이 빈 문자열이면 값이 채워진restAreaNm으로 넘어가지 않고 전국 휴게소 목록 조회 전체가 실패했습니다. 값이 비어 있으면 다음 별칭 키로 넘어가도록 수정했습니다.수정 패스 자체가 만든 회귀 1건도 바로잡았습니다
_env.py의load_local_env()에@lru_cache를 추가했는데, 이 함수는path=None이면 현재 작업 디렉터리 기준으로.env를 찾습니다.monkeypatch.chdir()로 격리된 임시 디렉터리에서 새.env를 읽으려는 테스트가, 이전에 다른 디렉터리에서 호출된 캐시된 결과를 그대로 돌려받는 문제를 발견했습니다(실제로 이번 세션에서 진짜.env를 미리 조회했던 게 테스트의 격리된 fixture로 새어 들어가KEX_EX_API_KEY가 테스트 값"exkey"대신 실제 10자리 값으로 나왔습니다). 캐시를 제거했습니다 — 이 함수의 목적 자체가 "호출마다 달라질 수 있는 파일을 읽는 것"이라 캐싱이 애초에 안전하지 않습니다.수정 패스 중
_http.py에서 발생한 ruff E501(줄 길이) 위반 4건도 함께 정리했습니다.검증
python -m pytest -q -m "not live": 84 passed, 5 deselectedKEX_LIVE=1 python -m pytest -m live(.env의KEX_EX_API_KEY/KEX_GO_API_KEY/DATA_GO_KR_SERVICE_KEY) → 5 passedpython -m ruff check .: 통과python -m mypy src/krex: 통과 (strict)추가: CI 워크플로
.github/workflows/ci.yml신규 추가 —lint/typecheck/test(Python 3.11/3.12/3.13,not live마커 제외) 3개 job.🤖 Generated with Claude Code
Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com