PR #33 レビュー12件を是正する（素顔が納品されうる手順の欠陥を含む） - #34
Merged
Merged
Conversation
【重大2件】 1. SKILL.md 手順9 が素顔のファイルを指したままだった。手順7.5(モザイクを焼く)を 足して「以降はモザイク版を候補として扱う」と書きながら、納品の手順は short-01.mp4 のままで、手順書どおりに従うと素顔のクリップが採用/に入る状態だった。 モザイク機能が防ごうとしていた失敗そのものを手順書が引き起こしていた。 モザイク版と非モザイク版の2通りに分けて明記し、納品手順がモザイク版を指している ことを検証する回帰テストを追加した。 2. M-5-C の evidence が計測を裏付けていなかった。書いた CI run は固定素材のテストを 回しただけで、第三者素材(Gazeta Esportiva/MMA PLUS)での計測は含んでいない。素材は リポジトリにも CI にも置けないため誰も再現できない。done を取り消して doing に戻し、 なぜ evidence が無いのか・done にするには何が必要かを detail に明記した。 計測値そのものは判断材料として detail に残す。 【安定性3件】 - probe(): r_frame_rate が 0/0 の素材で ZeroDivisionError になり、main が拾えず トレースバックが客に出ていた。非正・非有限を RuntimeError で弾く - run(): 途中で例外終了すると、読み残しのある decoder が書き込みで止まり dec.wait() が 返らずハングしうる。先に terminate して stdout を閉じる - face_choices: write_face_choices が上げうる cv2.error を捕捉する 【使い勝手・保守2件】 - apply_mosaic_cli の引数解析を argparse へ。位置決め打ちだったため 「--strength 強め in.mp4 out.mp4」で入力パスがフラグ名になっていた - LOOKAHEAD_FRAMES を DETECT_EVERY_DEFAULT から導出（間隔変更に追随させる） 【テスト・CI 5件】 - `if False` の残骸を除去し、子プロセスの起動を sys.executable に統一 - 速度計測で plain を解放してから withm を作る（1080pで約1.9GB同時保持を避ける） - block_variety を1回だけ計算して使い回す（毎回デコードし直さない） - 固定ファイルの書き込みを with で閉じる - CI に ffmpeg の版をログ出力（将来ランナー更新で壊れたとき原因を追えるように） テストは 69件 -> 70件。失敗2件を docs/failures.md へ追記した。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EcWtwTVCvSgjg9puWtdWvg
📝 WalkthroughWalkthroughChangesMosaic workflow
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@video-shorts/src/apply_mosaic_cli.py`:
- Around line 115-120: Update the decoder cleanup flow around the short-read
handling and the `dec.poll()` check to track whether processing reached clean
EOF versus exiting due to an exception. Only call `dec.terminate()` for
exception-driven termination; after normal EOF, close `dec.stdout` and let
`dec.wait()` collect the decoder without sending SIGTERM or producing a
failed-input status.
- Around line 149-152: Update the SystemExit handling around parser.parse_args
in the CLI argument parsing flow to return int(e.code) directly, preserving
argparse’s exit code so --help returns 0 while genuine parse failures retain
their nonzero code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0e40e257-ede8-4b9c-a8dc-ddf75070dc54
📒 Files selected for processing (7)
.github/workflows/ci.ymldocs/failures.mddocs/roadmap.htmlvideo-shorts/skill/video-shorts/SKILL.mdvideo-shorts/src/apply_mosaic_cli.pyvideo-shorts/src/face_choices.pyvideo-shorts/tests/face-mosaic-check.py
Comment on lines
+115
to
120
| if dec.poll() is None: | ||
| # 読み残しがあると ffmpeg はパイプへの書き込みで止まり wait() が返らない。 | ||
| # 例外で途中終了したときに、ここで永久に固まらないよう先に止める。 | ||
| dec.terminate() | ||
| dec.stdout.close() | ||
| dec.wait() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Do not terminate the decoder after a clean EOF.
A short read at Line 92 marks normal decoder EOF. The decoder can close stdout before it exits. If dec.poll() is still None, Line 118 sends SIGTERM. Lines 122-123 then report a failed input with return code -15 after successful processing.
Track clean EOF separately. Terminate dec only when processing exits by exception. Let dec.wait() collect a decoder that reached EOF normally.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@video-shorts/src/apply_mosaic_cli.py` around lines 115 - 120, Update the
decoder cleanup flow around the short-read handling and the `dec.poll()` check
to track whether processing reached clean EOF versus exiting due to an
exception. Only call `dec.terminate()` for exception-driven termination; after
normal EOF, close `dec.stdout` and let `dec.wait()` collect the decoder without
sending SIGTERM or producing a failed-input status.
Comment on lines
+149
to
+152
| try: | ||
| args = parser.parse_args(argv[1:]) | ||
| except SystemExit as e: | ||
| return int(e.code or 2) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/sh
python - <<'PY'
import argparse
parser = argparse.ArgumentParser()
try:
parser.parse_args(["--help"])
except SystemExit as exc:
assert exc.code == 0
assert int(exc.code or 2) == 2
PYRepository: rahiseko-alt/ai-editer
Length of output: 230
🏁 Script executed:
#!/bin/sh
set -eu
printf '%s\n' '--- target file ---'
sed -n '120,180p' video-shorts/src/apply_mosaic_cli.py
printf '%s\n' '--- related entry points and parser calls ---'
rg -n -C 3 'def main|parse_args|SystemExit|apply_mosaic_cli' video-shortsRepository: rahiseko-alt/ai-editer
Length of output: 9846
Return the argparse help exit code.
parse_args(["--help"]) raises SystemExit(0), but int(e.code or 2) returns 2. Return int(e.code) instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@video-shorts/src/apply_mosaic_cli.py` around lines 149 - 152, Update the
SystemExit handling around parser.parse_args in the CLI argument parsing flow to
return int(e.code) directly, preserving argparse’s exit code so --help returns 0
while genuine parse failures retain their nonzero code.
rahiseko-alt
added a commit
that referenced
this pull request
Aug 4, 2026
* PR #34 レビュー2件を是正し、M-5-C の計測を再現可能にする レビュー是正（いずれも回帰テスト付き。是正前のコードで実際に落ちることを確認）: - 読み切った後のデコーダに SIGTERM を送っていた。ffmpeg が stdout を閉じてから 終了するまでの隙に当たると終了コードが -15 になり、全コマを正しく焼き終えたのに 「入力動画の読み出しに失敗しました」と落ちる。読み切ったかどうかを追跡して、 例外で抜けたときだけ止めるようにした。 - `int(e.code or 2)` により `--help` が異常終了(2)を返していた。argparse が決めた 終了コードをそのまま返す。 M-5-C（実素材での見落とし率）の再現性: - 第三者素材で測った結果は再現できず evidence にできなかったため、パブリック ドメインの実写映像(NASA・Wikimedia Commons 経由)で測る scripts/measure-leak-rate.py と専用ワークフローを追加した。お客様と同じ経路で mp4 を書き出し、その出来上がりを 復号し直して顔検出を掛ける。顔の無い素材で「見落とし0%」を出さないよう、 元素材に顔が無ければ計測失敗として落とす。 - 計測結果: インタビュー場面 0.58%、管制室の引きの画 100%。後者は追従の破綻ではなく 顔が小さすぎて検出に掛からないため。この限界を手順書にも明記した。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EcWtwTVCvSgjg9puWtdWvg * PR #35 レビュー3件を是正する（計測ワークフローが一度も動かない欠陥を含む） 重大: 計測ワークフローが計測スクリプトの知らない引数(--start)を渡しており、 回した瞬間に argparse の使い方エラーで落ちる状態だった。M-5-C の evidence を 作るための仕組みが、一度も動かせないまま「これが証拠になります」と説明されていた。 - ワークフローの入力を --case に揃えた（既定の長さも実測と同じ20秒に合わせた） - build_parser() を切り出し、**ワークフローが組み立てるコマンド行をそのまま 受け口に食わせる回帰テスト**を追加。未知の引数を渡すワークフローに差し替えて 実際に落ちることも確認済み。 その他2件: - 復号器が途中で失敗しても部分的な数え上げを返していた。途中までしか見ていない 結果を「見落とし率」として記録しうるため、終了コードが非ゼロなら落とす。 - ワークフローのトークン権限を contents: read に絞り、checkout の資格情報を 残さないようにした。dispatch 入力も run 本文へ直接展開せず env 経由で渡す。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EcWtwTVCvSgjg9puWtdWvg * コミットに混ざった .pyc を外し、使用方法の記述を実際の引数に合わせる - scripts/__pycache__/*.pyc が追跡されていた。ルートの .gitignore に __pycache__/ と *.pyc が無く、リポジトリ直下に Python を置いたことで漏れた。 - 計測スクリプトの冒頭の使用方法が、廃止した --start を案内したままだった。 実際の引数（--case）に合わせる。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EcWtwTVCvSgjg9puWtdWvg * 計測ワークフローの引数検査を、入力の取りこぼしまで見るようにする 「旗が1つでもあれば合格」だったため、--case の受け渡しを消してもテストが素通り していた。その状態では、画面で crowd を選んで実行しても既定の all を測ることに なり、黙って別のものを計測する。 期待する組み合わせを書き写すのではなく、workflow_dispatch で宣言された入力の 集合と、実際にスクリプトへ渡している入力の集合を突き合わせる。書き写す方式だと 入力を増やしたときにテスト側の更新漏れで同じ穴が開くため。 --case の受け渡しだけを削ったワークフローで、実際にこの検査が落ちることを確認済み。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EcWtwTVCvSgjg9puWtdWvg --------- Co-authored-by: Claude <noreply@anthropic.com>
This was referenced Aug 4, 2026
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
2 participants
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.
PR #33 のレビュー指摘12件が、反映前にマージされたため追いかけて是正します。うち2件は重大です。
重大① 手順書どおりに従うと素顔が納品される
SKILL.mdに手順7.5（モザイクを焼く）を足して「以降はモザイク版を候補として扱う」と書きながら、納品の手順9はshort-01.mp4のままでした。手順書どおりに従うと、素顔のクリップが
採用/に入ります。この機能が防ごうとしていた失敗そのものを、手順書自身が引き起こす状態でした。原因は、新しい手順を「足す」ことに意識が向き、その手順が下流に与える影響を確認しなかったことです。手順7.5でファイル名が変わるのだから、下流でファイル名を使う箇所はすべて見直す必要がありました。
モザイク版・非モザイク版の2通りに分けて明記し、納品手順がモザイク版を指していることを検証する回帰テストを追加しました。
重大②
M-5-Cの evidence が計測を裏付けていなかったevidence に書いた CI run は固定素材のテストを回しただけで、第三者素材（Gazeta Esportiva / MMA PLUS）での計測は含んでいませんでした。素材はリポジトリにも CI にも置けないため、誰も再現・検証できません。
「計測を実際にやった」ことと「第三者が検証できる形で残した」ことを区別していませんでした。AGENTS.md の「evidence は偽造不能な外部事実のみ・自己申告は証拠にしない」は、まさにこの区別のためにあります。
doneを取り消してdoingに戻し、なぜ evidence が無いのか・doneにするには何が必要かをdetailに明記しました。計測値そのものは判断材料として残しています。これにより G-MOSAIC は 19葉中18葉 done になります。
安定性 3件
probe():r_frame_rateが0/0の素材でZeroDivisionErrorになり、mainが拾えずトレースバックが客に出ていましたrun(): 途中で例外終了すると、読み残しのある decoder が書き込みで止まりdec.wait()が返らずハングしえましたface_choices:write_face_choicesが上げうるcv2.errorを捕捉使い勝手・保守 2件
apply_mosaic_cliの引数解析をargparseへ。位置決め打ちだったため--strength 強め in.mp4 out.mp4で入力パスがフラグ名になっていましたLOOKAHEAD_FRAMESをDETECT_EVERY_DEFAULTから導出（間隔変更に追随）テスト・CI 5件
if Falseの残骸を除去し、子プロセス起動をsys.executableに統一plainを解放してからwithmを作る（1080pで約1.9GB同時保持を回避）block_varietyを1回だけ計算して使い回すwithで閉じる検証
テストは 69件 → 70件（全PASS）。ワークスペース全体も緑です。失敗2件を
docs/failures.mdに追記しました。Generated by Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation