Refactor validation logic in press release handlers; replace Pydantic… - #22
Conversation
… with explicit checks for title and content
WalkthroughPydantic v2を使用した検証からカスタム検証関数への移行。SavePressReleaseRequestモデルを廃止し、parse_press_release_id()およびparse_save_request()という明示的な検証ヘルパー関数を導入。エンドポイント署名を更新し、環境変数ドキュメントを追加。 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
webapp/python/main.py (1)
1-211:⚠️ Potential issue | 🟡 MinorCLAUDE.mdのPython実装ガイドラインを更新してください
CLAUDE.mdではPythonバリデーション方法として「Pydantic for validation」と記載されていますが、webapp/python/README.mdおよび実装コードでは「実装内での明示的なJSON/必須項目チェック」を明確に採用しています。
実装がガイドラインと異なるため、CLAUDE.mdの以下の部分を修正してください:
Language Framework/Router Database Driver Key Libraries Python FastAPI psycopg 明示的なバリデーション(HTTPException) AGENTS.md「変更ルール」に従い、実装の意図的な設計変更が反映されるようにガイドラインを更新することで、他の開発者の混乱を防げます。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webapp/python/main.py` around lines 1 - 211, CLAUDE.md currently states "Pydantic for validation" for Python but the actual FastAPI implementation uses explicit in-handler validation and HTTPException handling (see parse_save_request, parse_press_release_id and http_exception_handler), so update the table entry for Python to reflect "FastAPI | psycopg | explicit validation (HTTPException)" (or similar wording) and add a short note referencing the implemented pattern and AGENTS.md change rules so the guideline matches the code intent.
🧹 Nitpick comments (2)
webapp/python/README.md (1)
190-190: pydantic依存関係の説明を明確化することを検討してくださいバリデーションがPydanticベースから明示的チェックに変更されましたが、依存関係にはまだpydanticが記載されています。FastAPIが内部的にPydanticを使用するため依存関係として必要ですが、読者が混乱する可能性があります。
📝 説明の追加案
- **pydantic**: データバリデーション + - 注: FastAPI内部で使用。本実装のリクエストバリデーションは明示的チェックで実装🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webapp/python/README.md` at line 190, Update the README entry for "pydantic" to clarify why it remains listed: mention that FastAPI depends on pydantic internally (so pydantic must be a dependency) even though this project replaced Pydantic-based validation with explicit checks; change the line "**pydantic**: データバリデーション" to a brief explanatory sentence referencing "pydantic" and "FastAPI" so readers understand it's retained for FastAPI runtime compatibility rather than direct use in the codebase.webapp/python/main.py (1)
85-85: コメントの全角括弧を半角に変更することを検討してくださいRuffが全角括弧(
(/))について警告しています。コードの一貫性のため半角括弧を使用することを推奨します。✏️ 括弧の修正
- # 空白のみタイトルも許容(PHP/Goと同じ) + # 空白のみタイトルも許容(PHP/Goと同じ)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webapp/python/main.py` at line 85, The inline comment contains full-width Japanese parentheses ( and ); replace them with ASCII/half-width parentheses ( and ) to satisfy Ruff and maintain consistency — update the comment text "# 空白のみタイトルも許容(PHP/Goと同じ)" to use half-width parentheses, e.g. "# 空白のみタイトルも許容(PHP/Goと同じ)" so the change applies to the comment in webapp/python/main.py.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@webapp/python/main.py`:
- Around line 65-71: The JSON decode exception handler around json.loads(body)
currently discards the original exception; change the except block to capture
the exception (except json.JSONDecodeError as err) and re-raise the
HTTPException using exception chaining (raise HTTPException(status_code=400,
detail={"code":"INVALID_JSON","message":"Invalid JSON"}) from err) so the
original json.JSONDecodeError is preserved for debugging; keep references to
json.loads, payload, body, json.JSONDecodeError and HTTPException when making
the change.
---
Outside diff comments:
In `@webapp/python/main.py`:
- Around line 1-211: CLAUDE.md currently states "Pydantic for validation" for
Python but the actual FastAPI implementation uses explicit in-handler validation
and HTTPException handling (see parse_save_request, parse_press_release_id and
http_exception_handler), so update the table entry for Python to reflect
"FastAPI | psycopg | explicit validation (HTTPException)" (or similar wording)
and add a short note referencing the implemented pattern and AGENTS.md change
rules so the guideline matches the code intent.
---
Nitpick comments:
In `@webapp/python/main.py`:
- Line 85: The inline comment contains full-width Japanese parentheses ( and );
replace them with ASCII/half-width parentheses ( and ) to satisfy Ruff and
maintain consistency — update the comment text "# 空白のみタイトルも許容(PHP/Goと同じ)" to use
half-width parentheses, e.g. "# 空白のみタイトルも許容(PHP/Goと同じ)" so the change applies to
the comment in webapp/python/main.py.
In `@webapp/python/README.md`:
- Line 190: Update the README entry for "pydantic" to clarify why it remains
listed: mention that FastAPI depends on pydantic internally (so pydantic must be
a dependency) even though this project replaced Pydantic-based validation with
explicit checks; change the line "**pydantic**: データバリデーション" to a brief
explanatory sentence referencing "pydantic" and "FastAPI" so readers understand
it's retained for FastAPI runtime compatibility rather than direct use in the
codebase.
…e original exception in response
… with explicit checks for title and content
Summary by CodeRabbit
リリースノート
Documentation
Bug Fixes