Conversation
There was a problem hiding this comment.
This PR introduces a fixed banner system to replace on-demand generation. The approach is sound for reducing generation overhead and ensuring consistent visual identity.
Critical Issues Found:
Two defects that could cause runtime failures have been identified:
-
Crash risk in example code: The
next()function in the README example lacks error handling and will raiseStopIterationif the manifest becomes corrupted or out of sync. -
Data consistency issue: The manifest structure has 4 candidates but 5 assets, creating a structural mismatch. While intentional for the current use case (00-developer is excluded), this needs defensive validation in any consuming code to prevent index errors.
Note: This review focuses on the documented examples and data structures. The actual implementation code that consumes this manifest isn't part of this PR, so ensure proper error handling and validation are added when integrating this feature into the application logic.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
|
|
||
| manifest = json.loads(Path("assets/follow-banners/manifest.json").read_text()) | ||
| selected_id = secrets.choice(manifest["candidates"]) | ||
| selected = next(a for a in manifest["assets"] if a["id"] == selected_id) |
There was a problem hiding this comment.
🛑 Crash Risk: The next() function will raise StopIteration if no matching asset ID is found in the manifest. Add a default value or error handling to prevent runtime crashes when the manifest is corrupted or candidates list is out of sync with assets.
| selected = next(a for a in manifest["assets"] if a["id"] == selected_id) | |
| selected = next((a for a in manifest["assets"] if a["id"] == selected_id), None) | |
| if selected is None: | |
| raise ValueError(f"Selected ID '{selected_id}' not found in assets") | |
| print(selected["id"], selected["url"]) |
| "candidates": [ | ||
| "01-reading", | ||
| "02-meeting", | ||
| "03-cafe", | ||
| "04-travel" | ||
| ], |
There was a problem hiding this comment.
🛑 Logic Error: The candidates array contains 4 IDs but the assets array has 5 items. If this manifest is used programmatically without cross-validation, there's a mismatch risk. The code in README.md line 21 could fail if a candidate ID doesn't exist in assets, or if asset indexing logic assumes array positions match.
Closes #12
フォローバナーを毎回生成する運用から、承認済みの固定画像を再利用する運用へ変更します。読書・打ち合わせ・カフェ・旅行の4種類から記事ごとに1枚をランダム選択し、同じ記事の再試行では選択を維持します。
原本PNG5枚(開発版は保管のみ)、CDN URL付きマニフェスト、利用規則、生成プロンプト、記事スキル参照を追加しました。既存ChatGPT週次タスクの指示も更新・保存済みです。
検証: PNG5枚の読み込み・1983×793・SHA256、候補4件、CDN4件のHTTP 200とJPEG読込、抽選例、週次設定の保存後読み戻しが成功。TC-001は手動検証。サーバーコード変更なしのためbuild/testは対象外。
制約: 次回の自動投稿は未実行。クラウド側で乱数機能が使えない場合は制約を報告する設定です。公開時に記録されたpost_idと画像IDを以後の再試行で再利用します。