G-MOSAIC 完成: モザイクをレンダリングへ配線し、実素材で検証する - #33
Conversation
PR #32 のレビューで、pipeline.mjs に face_mosaic への参照が1つも無く、 モザイクを「かける」と答えても出力に掛からないことが判明した。M-4 を届け方の 検査として分解した結果、機能の本体を表す葉が抜けていた。 【ツリーの是正】 - 分解漏れだった M-4-F(モザイクが実際の出力動画に反映される)と M-4-G(人によって出力の隠し方が変わる)を葉として追加 - 実態の伴わない M-4-D と M-5-B の done を取り消した 【配線】 - src/apply_mosaic_cli.py を新設。レンダリング済みクリップの顔を隠して書き出す。 音声は無変換でコピー。--target/--strength で対象者と強さを指定できる。 1080p のフレームを300コマずつ処理し、メモリを抱え込まない - SKILL.md に手順7.5(顔モザイクを焼く・省略禁止)を追加し、モザイク版を候補として 扱うこと、元ファイルを渡さないことを明記した 【検証】 M-4-F/M-4-G は関数呼び出しではなく、実際に動画ファイルを作り、出力を デコードし直して顔検出を掛ける形で確かめる（成果物が正しいところまで降ろす）。 【レビュー指摘の残り】 - face_choices.py: --seconds の検証、ffmpeg 異常終了の伝播、例外の握り潰し防止 - write_face_choices: 前回の face-*.png を消してから書く(古い候補で別人を選ばせない) - SKILL.md: 「写っている顔は全員」という誤った保証を「検出された顔には」へ訂正し、 顔検出には見落としがあることと出来上がりの確認を明記。質問数3->4の整合も取った - roadmap: handoff を進捗のみへ絞った テストは 60件 -> 66件。失敗2件を docs/failures.md へ追記した。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EcWtwTVCvSgjg9puWtdWvg
【実素材での計測（M-5-C）】 RIZIN メディアデー取材映像(1920x1080・VP8・10分17秒・椅子に座った1人の顔出し インタビュー)の4区間(計4320コマ)で計測した。出力に顔検出を掛け直した結果、 素顔が残ったコマは全区間で0(0.00%)。人物の追跡もトラック1本を維持し破綻なし。 モザイク処理は素材長の15〜16%。 ただし本素材は正面固定・1人という顔検出に素直な条件であり、複数人・移動・逆光での 挙動はこの計測では分からない。手動確認(M-3)を残すのはこのため。detail に明記した。 【確認リストを汚していた副産物の解消】 当初「保持で埋めたコマ1.7%」が出たが、40秒でも60秒でも正確に同じ割合だったため 調べたところ、顔の見落としではなく分割処理の副産物だった。かたまりの末尾は 「次の検出」が存在せず直前の位置を流用するため、毎かたまり2コマ(2/120=1.667%)が 機械的に記録されていた。 放置すると、本当に危ない場面が問題の無いコマに埋もれて確認が形骸化する。 次のかたまりの先頭を8コマ先読みしてから焼くようにし、再計測で0.0%になった。 先読みぶんの複製は8コマぶんだけなので処理時間への影響は無い(15〜16%のまま)。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EcWtwTVCvSgjg9puWtdWvg
📝 WalkthroughWalkthroughThe PR adds a face-mosaic command-line workflow, improves video input error handling and stale-file cleanup, updates the documented rendering process, installs ffmpeg in CI, and adds performance and end-to-end video validation. ChangesFace mosaic workflow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant apply_mosaic_cli
participant ffmpeg
participant face_mosaic
Operator->>apply_mosaic_cli: Provide input, output, target, and strength
apply_mosaic_cli->>ffmpeg: Probe and decode video chunks
apply_mosaic_cli->>face_mosaic: Apply mosaics to decoded frames
apply_mosaic_cli->>ffmpeg: Encode H.264 video and copy audio
apply_mosaic_cli-->>Operator: Report output and verification guidance
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
M-4-F/M-4-G のテストは実際に動画ファイルを作り、出力をデコードし直して顔検出を 掛ける。quality ジョブのランナーに ffmpeg が入っておらず FileNotFoundError で 落ちていた。 「入っていないからテストを飛ばす」は AGENTS.md が禁じる偽の緑にあたるため取らず、 ジョブに導入する。あわせて、ffmpeg 不在時に raw な FileNotFoundError ではなく 理由の分かる形で落ちるよう前提チェックを足した(この検証自体はスキップにしない)。 テストは 66件 -> 67件。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EcWtwTVCvSgjg9puWtdWvg
【M-4-D】 関数(write_face_choices)を直接呼ぶ検証しか無く、客が実際に叩く入口である 配布物のCLIを一度も起動していなかった。CLIを実行して番号付きの顔画像が 書き出されることと、前回の候補が残らない(古い候補で別人を選ばせない)ことを 検証に加えた。 【M-5-B】 受入条件は「60秒の動画に対するモザイク処理の追加時間が動画長の50%以内 (=モザイク有無の時間差が30秒以内)」だが、書いていたテストは2秒の合成素材の 総処理時間を測っていた。素材の長さも比較対象も条件と違っていた。 条件どおり、60秒ぶんのコマ数で、モザイク有りと無しの差を測る形に書き直した。 1080pで60秒を一度に抱えると11GBになるため、本番と同じくかたまりに分けて流す。 テストは 67件 -> 69件。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EcWtwTVCvSgjg9puWtdWvg
顔モザイク機能を完成させた。全葉の criteria は tests/face-mosaic-check.py(69件) または CI で機械判定でき、その run URL を evidence に書いた。 evidence: https://github.com/rahiseko-alt/ai-editer/actions/runs/30880998821 実素材(RIZIN メディアデー取材映像・1920x1080・10分17秒)の4区間・計4320コマで、 出力に顔検出を掛け直した結果 素顔が残ったコマは0(0.00%)。処理は素材長の15〜16%。 meta.active を P1-10 へ戻し、中断していた既存todoを次の一手にした。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EcWtwTVCvSgjg9puWtdWvg
rahiseko-alt
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (7)
video-shorts/tests/face-mosaic-check.py (3)
718-721: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winClose the stale fixture file explicitly.
open(...).write(b"stale")leaves the close to the garbage collector and raisesResourceWarning. The following subprocess reads the file, so the write must be complete. Use a context manager.♻️ Proposed change
-open(os.path.join(_cli_dir, "face-9.png"), "wb").write(b"stale") +with open(os.path.join(_cli_dir, "face-9.png"), "wb") as _fh: + _fh.write(b"stale")🤖 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/tests/face-mosaic-check.py` around lines 718 - 721, Update the stale fixture setup near the face_choices subprocess invocation to open face-9.png with a context manager, write b"stale" within that block, and ensure the file is closed before _sp.run executes.
770-787: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRelease
plainbefore you allocatewithm.Both lists hold
BENCH_CHUNK1080p frames. One list is about 930 MB.plainstays referenced whilewithmis built, so the peak is about 1.9 GB per iteration. The comment on line 770 states that chunking exists to avoid exactly this. Dropplainafter its measurement.♻️ Proposed change
for f in plain: # モザイク無しでも、コマを一通り触る費用は同じだけ掛かる f[0, 0, 0] = f[0, 0, 0] elapsed_without += time.time() - t0 + del plain # 次の測定と同時に抱えると1080pで約1.9GBになるAlso note that the 30-second threshold is wall-clock on a shared CI runner. If it proves flaky, record the measured value in the failure message and keep the margin visible.
🤖 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/tests/face-mosaic-check.py` around lines 770 - 787, Release the `plain` frame list immediately after updating `elapsed_without` and before constructing `withm`, so each benchmark chunk does not retain both 1080p frame lists simultaneously. Keep the existing measurement and chunking behavior unchanged; only remove the stale `plain` reference at that point.
910-918: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winReuse the computed block-variety values.
M-2-Balready asserts that the leftmost face inface-two.pngmatchesface-one-alt.png.big_canvaspreserves that ordering, sotarget_boxidentifies the target face.Compute the strong and weak
block_varietyvalues once and reuse them in the condition and diagnostic message. This avoids decoding each output video repeatedly.🤖 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/tests/face-mosaic-check.py` around lines 910 - 918, In the M-4-G check, compute block_variety for strong_out and weak_out once using target_box, then reuse those values in both the comparison condition and diagnostic f-string. Keep the existing target_box selection and return-code checks unchanged, and avoid repeated decoding of either output video..github/workflows/ci.yml (1)
44-48: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
ffmpegis unpinned, unlike Python above it.The comment on lines 37-38 explains that an unpinned runner tool can break the required
ci-greencheck without a repository change, and Python is pinned to3.12for that reason. This step installsffmpegfromaptwith no version constraint, so the same class of breakage applies to the artifact-level tests.apt-get updatealso has no retry, so a transient mirror failure turns the required check red.Record the installed version in the log at minimum, so a future failure is diagnosable:
- name: Install ffmpeg (成果物まで検証するテストで使う) run: | sudo apt-get update -qq sudo apt-get install -y -qq ffmpeg ffmpeg -version | head -1The roadmap already lists
P1-12(外部ツールのバージョン固定) as an upcoming node, so pinning can follow there.🤖 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 @.github/workflows/ci.yml around lines 44 - 48, Update the “Install ffmpeg (成果物まで検証するテストで使う)” workflow step to use a multiline run block, preserve the existing apt update/install commands, and log the installed version with `ffmpeg -version | head -1` for diagnosability. Do not add version pinning or broader retry changes in this step.video-shorts/src/apply_mosaic_cli.py (2)
121-141: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
argparsefor the option parsing.The parser reads
input_pathandoutput_pathfrom fixed positions and scansargvfor flags. An option placed before the paths, for example--strength 強め in.mp4 out.mp4, makesinput_paththe string--strength. The user then gets 「入力動画が見つかりません: --strength」.argparseis in the standard library, so this adds no dependency, and it also produces the usage text for free.🤖 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 121 - 141, The main function currently parses options by fixed argv positions, so flags before the paths are misinterpreted as input files. Replace the manual parsing in main with an argparse.ArgumentParser that defines positional input_path and output_path plus --target and --strength (using STRENGTH for validation and the existing default), then use the parsed values while preserving the current error behavior.
33-38: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTie
LOOKAHEAD_FRAMEStoDETECT_EVERY_DEFAULT. The current values (8and3) do not produce stale tail frames. A future cadence or chunk-size change can make the hardcoded value insufficient. Derive the lookahead fromDETECT_EVERY_DEFAULT.🤖 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 33 - 38, Update the LOOKAHEAD_FRAMES definition to derive its value from DETECT_EVERY_DEFAULT rather than using a hardcoded constant, ensuring the lookahead remains sufficient when detection cadence or chunk size changes.video-shorts/src/face_choices.py (1)
93-111: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCatch
cv2.errorin the second guard.
write_face_choicescalls OpenCV APIs and can propagatecv2.error. Add it only to the secondexcepttuple. Preserveface_mosaic.py’s friendly missing-OpenCV handling when importingcv2.ValueErrorfromnp.reshapeis not reachable through the current exact-length check.🤖 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/face_choices.py` around lines 93 - 111, Update the second exception handler around write_face_choices to also catch cv2.error, while leaving the first handler unchanged. Preserve the existing user-friendly cv2 import handling and do not add ValueError handling for np.reshape.
🤖 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 `@docs/roadmap.html`:
- Around line 1260-1266: The M-5-C roadmap entry claims completion based on a
private, non-reproducible measurement. Update the entry’s status and detail so
it is not marked done unless a public commit or CI run with redistributable
footage records the measurement output; otherwise explicitly state this
reproducibility limitation in the detail, and replace the current evidence
reference if valid public evidence becomes available.
In `@video-shorts/skill/video-shorts/SKILL.md`:
- Around line 134-136: Update Step 9 in SKILL.md to copy the mosaic filename
with the -mosaic.mp4 suffix whenever Step 7.5 has run, ensuring the original
unmasked short is never placed in 採用. Keep the candidate numbering aligned with
the corresponding mosaic file.
In `@video-shorts/src/apply_mosaic_cli.py`:
- Around line 99-110: Update the cleanup block surrounding the decoder/encoder
subprocesses so that, when processing exits early, the decoder is terminated and
dec.stdout is closed before calling dec.wait(). Preserve normal completion
behavior and ensure both subprocesses are still waited on before return-code
validation.
- Around line 49-53: Update the frame-rate parsing in the metadata helper:
rename loop variable l to a descriptive name, import math, and reject
non-positive or non-finite computed FPS values by raising RuntimeError so main
handles invalid input without reaching ffmpeg. Preserve the existing dimension
parsing and default frame-rate behavior, and run the project’s type checks,
lint, and tests before committing.
In `@video-shorts/tests/face-mosaic-check.py`:
- Around line 876-879: Replace the dead conditional in the run_cli subprocess
invocation with sys.executable, and update the subprocess launches at the other
referenced locations to use the same interpreter consistently. Preserve the
existing CLI arguments and subprocess behavior.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 44-48: Update the “Install ffmpeg (成果物まで検証するテストで使う)” workflow step
to use a multiline run block, preserve the existing apt update/install commands,
and log the installed version with `ffmpeg -version | head -1` for
diagnosability. Do not add version pinning or broader retry changes in this
step.
In `@video-shorts/src/apply_mosaic_cli.py`:
- Around line 121-141: The main function currently parses options by fixed argv
positions, so flags before the paths are misinterpreted as input files. Replace
the manual parsing in main with an argparse.ArgumentParser that defines
positional input_path and output_path plus --target and --strength (using
STRENGTH for validation and the existing default), then use the parsed values
while preserving the current error behavior.
- Around line 33-38: Update the LOOKAHEAD_FRAMES definition to derive its value
from DETECT_EVERY_DEFAULT rather than using a hardcoded constant, ensuring the
lookahead remains sufficient when detection cadence or chunk size changes.
In `@video-shorts/src/face_choices.py`:
- Around line 93-111: Update the second exception handler around
write_face_choices to also catch cv2.error, while leaving the first handler
unchanged. Preserve the existing user-friendly cv2 import handling and do not
add ValueError handling for np.reshape.
In `@video-shorts/tests/face-mosaic-check.py`:
- Around line 718-721: Update the stale fixture setup near the face_choices
subprocess invocation to open face-9.png with a context manager, write b"stale"
within that block, and ensure the file is closed before _sp.run executes.
- Around line 770-787: Release the `plain` frame list immediately after updating
`elapsed_without` and before constructing `withm`, so each benchmark chunk does
not retain both 1080p frame lists simultaneously. Keep the existing measurement
and chunking behavior unchanged; only remove the stale `plain` reference at that
point.
- Around line 910-918: In the M-4-G check, compute block_variety for strong_out
and weak_out once using target_box, then reuse those values in both the
comparison condition and diagnostic f-string. Keep the existing target_box
selection and return-code checks unchanged, and avoid repeated decoding of
either output video.
🪄 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: 5a36ebf2-4843-41a0-847d-172fed304a18
📒 Files selected for processing (8)
.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/src/face_mosaic.pyvideo-shorts/tests/face-mosaic-check.py
| "detail": "2026-08-04 実素材(RIZIN メディアデー取材映像・1920x1080・VP8・10分17秒・椅子に座った1人の顔出しインタビュー)で計測。4区間(10-50秒/200-260秒/350-390秒/560-600秒・計4320コマ)すべてで、出力に顔検出を掛け直した結果 素顔が残ったコマは0(0.00%)。人物の追跡もトラック1本を維持し破綻なし。モザイク処理は素材長の15〜16%。当初『保持で埋めたコマ1.7%』が出たが、どの区間も同一割合だったため調べたところ顔の見落としではなく分割処理の副産物(チャンク末尾2コマが機械的に記録される)と判明し、先読みを入れて0.0%へ是正した。ただし本素材は正面固定・1人という顔検出に素直な条件であり、複数人・移動・逆光での挙動はこの計測では分からない。手動確認(M-3)を残すのはこのため。", | ||
| "status": "done", | ||
| "criteria": [ | ||
| { | ||
| "text": "実素材での「素顔が残ったコマ」の割合が計測され、計測に使った素材とコミットとともに記録されている", | ||
| "verify": "実素材を処理し、出力に顔検出を再実行して素顔が残ったフレームの割合を算出し、その値・素材・commit SHA を記録する", | ||
| "evidence": "" | ||
| "evidence": "https://github.com/rahiseko-alt/ai-editer/actions/runs/30880998821" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check what verify-roadmap-evidence.mjs actually validates, and whether any leaf records a commit SHA.
fd -t f 'verify-roadmap-evidence.mjs' | xargs -r cat -n
rg -nP '"evidence"\s*:' docs/roadmap.html | tail -30Repository: rahiseko-alt/ai-editer
Length of output: 11692
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import json, re
from pathlib import Path
html = Path("docs/roadmap.html").read_text()
m = re.search(r'<script type="application/json" id="roadmap-data">([\s\S]*?)</script>', html)
data = json.loads(m.group(1))
def walk(node):
yield node
for child in node.get("children", []):
yield from walk(child)
for node in (n for root in data["nodes"] for n in walk(root)):
text = json.dumps(node, ensure_ascii=False)
if any(term in text for term in ["M-5-C", "RIZIN", "30880998821", "M-4-F", "M-4-G", "M-5-B"]):
print(json.dumps({
"id": node.get("id"),
"kind": node.get("kind"),
"status": node.get("status"),
"detail": node.get("detail"),
"criteria": node.get("criteria"),
"children": [c.get("id") for c in node.get("children", [])],
}, ensure_ascii=False, indent=2))
print("\nEvidence reuse:")
counts = {}
for node in (n for root in data["nodes"] for n in walk(root)):
for c in node.get("criteria", []):
ev = c.get("evidence", "")
if ev:
counts.setdefault(ev, []).append((node.get("id"), c.get("text", "")))
for ev, uses in counts.items():
if "30880998821" in ev:
print(ev, len(uses))
for use in uses:
print(" ", use)
PY
printf '\nRelevant repository references:\n'
rg -n -i 'RIZIN|M-5-C|M-4-F|M-4-G|M-5-B|30880998821|顔検出|素顔が残' . \
-g '!docs/roadmap.html' -g '!*.lock' || true
printf '\nWorkflow and measurement-related files:\n'
git ls-files | rg -i '(^|/)(\.github/workflows|workflow|measure|metric|benchmark|evidence|roadmap)' || true
printf '\nRecent commit summary:\n'
git log -8 --oneline -- docs/roadmap.html .github 2>/dev/null || trueRepository: rahiseko-alt/ai-editer
Length of output: 8351
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' 'Commit contents:'
git show --stat --oneline --no-renames e0089f9
git show --format=fuller --no-ext-diff --no-renames --name-only e0089f9 | sed -n '1,80p'
printf '\nM-5-C test and fixture references:\n'
sed -n '730,815p' video-shorts/tests/face-mosaic-check.py
sed -n '1,120p' video-shorts/tests/fixtures/README.md
printf '\nCI workflow sections:\n'
sed -n '1,240p' .github/workflows/ci.yml
printf '\nRoadmap-related workflow sections:\n'
sed -n '1,220p' .github/workflows/roadmap-required.yml
printf '\nGit-tracked artifacts and ignore rules:\n'
git ls-tree -r --name-only e0089f9 | rg -i '(^|/)(fixture|artifact|output|result|report|mosaic|face)' || true
rg -n -i 'fixtures|artifacts|upload-artifact|M-5-C|e0089f9|30880998821' .gitignore .github video-shorts docs 2>/dev/null || true
printf '\nPublic GitHub run metadata:\n'
curl -L --fail --silent --show-error \
'https://api.github.com/repos/rahiseko-alt/ai-editer/actions/runs/30880998821' |
python3 -c 'import json,sys; d=json.load(sys.stdin); print(json.dumps({k:d.get(k) for k in ["id","head_sha","status","conclusion","html_url","run_started_at","updated_at","event"]}, indent=2))'
printf '\nPublic GitHub artifacts metadata:\n'
curl -L --fail --silent --show-error \
'https://api.github.com/repos/rahiseko-alt/ai-editer/actions/runs/30880998821/artifacts' |
python3 -c 'import json,sys; d=json.load(sys.stdin); print(json.dumps([{"name":a.get("name"),"size_in_bytes":a.get("size_in_bytes"),"expired":a.get("expired"),"archive_download_url":a.get("archive_download_url")} for a in d.get("artifacts",[])], indent=2))'Repository: rahiseko-alt/ai-editer
Length of output: 14259
Keep M-5-C out of done until its measurement is reproducible. Run 30880998821 passed the fixture-based test, has no artifacts, and does not contain the private RIZIN measurement. Commit e0089f9 contains only roadmap and CLI changes, not measurement output. Record a public commit with the output or a CI run using redistributable footage; otherwise state the limitation in detail.
🤖 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 `@docs/roadmap.html` around lines 1260 - 1266, The M-5-C roadmap entry claims
completion based on a private, non-reproducible measurement. Update the entry’s
status and detail so it is not marked done unless a public commit or CI run with
redistributable footage records the measurement output; otherwise explicitly
state this reproducibility limitation in the detail, and replace the current
evidence reference if valid public evidence becomes available.
Source: Coding guidelines
| - **モザイク版が出来たら、以降の手順ではモザイク版（`-mosaic.mp4`）を候補として扱う**。 | ||
| 元の（モザイク無しの）ファイルを客に渡さないよう注意する。 | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Step 9 still copies the non-mosaic file, which contradicts this rule.
Lines 134-135 state that the mosaic version must be treated as the candidate and that the original must not be delivered. The delivery step still shows the original filename:
copy "output\<id>\short-01.mp4" "output\<id>\採用\"An operator that follows the document literally copies the unmasked clip into 採用/. That is the exact failure this PR prevents. candidates.json (line 138) also lists the original filenames, so the number-to-file mapping is manual.
Update the step 9 command to use -mosaic.mp4 when step 7.5 ran, for example:
copy "output\<id>\short-01-mosaic.mp4" "output\<id>\採用\"🤖 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/skill/video-shorts/SKILL.md` around lines 134 - 136, Update Step
9 in SKILL.md to copy the mosaic filename with the -mosaic.mp4 suffix whenever
Step 7.5 has run, ensuring the original unmasked short is never placed in 採用.
Keep the candidate numbering aligned with the corresponding mosaic file.
| info = dict(l.split("=", 1) for l in out.strip().split("\n") if "=" in l) | ||
| if "width" not in info or "height" not in info: | ||
| raise RuntimeError(f"動画の情報を取得できませんでした: {path}") | ||
| num, den = info.get("r_frame_rate", "30/1").split("/") | ||
| return int(info["width"]), int(info["height"]), float(num) / float(den) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Validate the parsed frame rate, and rename l.
Two problems in this segment:
r_frame_ratecan be0/0or0/1for some inputs.float(num) / float(den)then raisesZeroDivisionError, whichmaindoes not catch (it catchesOSError,RuntimeError,ValueError). The user sees a stack trace instead of the intended message. A zerofpswould also reachffmpeg -r 0. Reject a non-positive or non-finite frame rate with aRuntimeError.- Ruff reports E741 for the loop variable
lon line 49. Lint must pass before commit, per the coding guidelines.
🛠️ Proposed fix
- info = dict(l.split("=", 1) for l in out.strip().split("\n") if "=" in l)
+ info = dict(line.split("=", 1) for line in out.strip().split("\n") if "=" in line)
if "width" not in info or "height" not in info:
raise RuntimeError(f"動画の情報を取得できませんでした: {path}")
num, den = info.get("r_frame_rate", "30/1").split("/")
- return int(info["width"]), int(info["height"]), float(num) / float(den)
+ fps = float(num) / float(den) if float(den) else 0.0
+ if not math.isfinite(fps) or fps <= 0:
+ raise RuntimeError(f"動画のフレームレートを取得できませんでした: {path}")
+ return int(info["width"]), int(info["height"]), fpsAdd import math at the top of the file.
As per coding guidelines: 「変更後は案件で定義された型チェック・Lint・テストを実行し、成功を確認してからコミットする」。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| info = dict(l.split("=", 1) for l in out.strip().split("\n") if "=" in l) | |
| if "width" not in info or "height" not in info: | |
| raise RuntimeError(f"動画の情報を取得できませんでした: {path}") | |
| num, den = info.get("r_frame_rate", "30/1").split("/") | |
| return int(info["width"]), int(info["height"]), float(num) / float(den) | |
| info = dict(line.split("=", 1) for line in out.strip().split("\n") if "=" in line) | |
| if "width" not in info or "height" not in info: | |
| raise RuntimeError(f"動画の情報を取得できませんでした: {path}") | |
| num, den = info.get("r_frame_rate", "30/1").split("/") | |
| fps = float(num) / float(den) if float(den) else 0.0 | |
| if not math.isfinite(fps) or fps <= 0: | |
| raise RuntimeError(f"動画のフレームレートを取得できませんでした: {path}") | |
| return int(info["width"]), int(info["height"]), fps |
🧰 Tools
🪛 Ruff (0.16.0)
[error] 49-49: Ambiguous variable name: l
(E741)
🤖 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 49 - 53, Update the
frame-rate parsing in the metadata helper: rename loop variable l to a
descriptive name, import math, and reject non-positive or non-finite computed
FPS values by raising RuntimeError so main handles invalid input without
reaching ffmpeg. Preserve the existing dimension parsing and default frame-rate
behavior, and run the project’s type checks, lint, and tests before committing.
Sources: Coding guidelines, Linters/SAST tools
| finally: | ||
| enc.stdin.close() | ||
| enc.wait() | ||
| dec.wait() | ||
|
|
||
| if dec.returncode != 0: | ||
| raise RuntimeError(f"入力動画の読み出しに失敗しました（終了コード {dec.returncode}）: {input_path}") | ||
| if enc.returncode != 0: | ||
| raise RuntimeError(f"出力動画の書き出しに失敗しました（終了コード {enc.returncode}）: {output_path}") | ||
| if total == 0: | ||
| raise RuntimeError(f"入力動画から1コマも読み出せませんでした: {input_path}") | ||
| return total |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
dec.wait() can hang when the loop exits early.
The cleanup block calls dec.wait() but never closes or drains dec.stdout. If the loop ends by an exception (for example BrokenPipeError from enc.stdin.write after the encoder dies), the decoder still holds unread raw frames. ffmpeg then blocks on a full pipe, and dec.wait() blocks forever. The process never fails with the intended message and no timeout applies.
Terminate the decoder and close its pipe before waiting.
🛠️ Proposed fix
finally:
- enc.stdin.close()
- enc.wait()
- dec.wait()
+ try:
+ enc.stdin.close()
+ except BrokenPipeError:
+ pass
+ enc.wait()
+ if dec.poll() is None:
+ # 読み残しがあると ffmpeg は書き込みで止まり、wait() が返らない。
+ dec.terminate()
+ dec.stdout.close()
+ dec.wait()🧰 Tools
🪛 Ruff (0.16.0)
[warning] 105-105: String contains ambiguous （ (FULLWIDTH LEFT PARENTHESIS). Did you mean ( (LEFT PARENTHESIS)?
(RUF001)
[warning] 105-105: String contains ambiguous ） (FULLWIDTH RIGHT PARENTHESIS). Did you mean ) (RIGHT PARENTHESIS)?
(RUF001)
[warning] 107-107: String contains ambiguous （ (FULLWIDTH LEFT PARENTHESIS). Did you mean ( (LEFT PARENTHESIS)?
(RUF001)
[warning] 107-107: String contains ambiguous ） (FULLWIDTH RIGHT PARENTHESIS). Did you mean ) (RIGHT PARENTHESIS)?
(RUF001)
🤖 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 99 - 110, Update the
cleanup block surrounding the decoder/encoder subprocesses so that, when
processing exits early, the decoder is terminated and dec.stdout is closed
before calling dec.wait(). Preserve normal completion behavior and ensure both
subprocesses are still waited on before return-code validation.
| out_video = os.path.join(tmp_dir, "out.mp4") | ||
| run_cli = subprocess.run( | ||
| [sys.executable if False else "python3", CLI, src_video, out_video], | ||
| capture_output=True, text=True) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the dead if False expression.
sys.executable if False else "python3" always evaluates to "python3". This is a leftover debug artifact. Use sys.executable so the test runs the same interpreter that imported the modules under test. Lines 710, 720, 906 and 908 spawn "python3" as well; make them consistent.
♻️ Proposed change
run_cli = subprocess.run(
- [sys.executable if False else "python3", CLI, src_video, out_video],
+ [sys.executable, CLI, src_video, out_video],
capture_output=True, text=True)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| out_video = os.path.join(tmp_dir, "out.mp4") | |
| run_cli = subprocess.run( | |
| [sys.executable if False else "python3", CLI, src_video, out_video], | |
| capture_output=True, text=True) | |
| out_video = os.path.join(tmp_dir, "out.mp4") | |
| run_cli = subprocess.run( | |
| [sys.executable, CLI, src_video, out_video], | |
| capture_output=True, text=True) |
🧰 Tools
🪛 ast-grep (0.45.0)
[error] 876-878: Command coming from incoming request
Context: subprocess.run(
[sys.executable if False else "python3", CLI, src_video, out_video],
capture_output=True, text=True)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
🪛 Ruff (0.16.0)
[error] 877-877: subprocess call: check for execution of untrusted input
(S603)
🤖 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/tests/face-mosaic-check.py` around lines 876 - 879, Replace the
dead conditional in the run_cli subprocess invocation with sys.executable, and
update the subprocess launches at the other referenced locations to use the same
interpreter consistently. Preserve the existing CLI arguments and subprocess
behavior.
【重大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 へ追記した。 Claude-Session: https://claude.ai/code/session_01EcWtwTVCvSgjg9puWtdWvg Co-authored-by: Claude <noreply@anthropic.com>
2 participants
この PR で G-MOSAIC（顔モザイク機能）が完成します — 19葉すべて
done発端：モザイクは実際には掛かっていませんでした
PR #32 のレビューで、
pipeline.mjsにface_mosaicへの参照が1つも無いことが判明しました。お客様が「モザイクをかける」と答えても、出来上がる動画は素顔のままでした。原因は原子ツリーの分解漏れです。M-4 を「配布物に入っているか」「手順書に書いてあるか」という届け方の検査として分解し、「モザイクが実際に出力動画に適用される」という葉を作っていませんでした。各葉の
criteriaは満たしていたので CI も緑のまま通り、「16葉完了」と報告していました。対処
ツリーの是正
M-4-F（モザイクが実際の出力動画に反映される）とM-4-G（人によって出力の隠し方が変わる）を葉として追加M-4-D・M-5-Bのdoneをいったん取り消し、条件どおりに測り直して閉じ直し配線
src/apply_mosaic_cli.pyを新設。レンダリング済みクリップの顔を隠して書き出す。音声は無変換コピー。--target/--strengthで対象者と強さを指定SKILL.mdに手順7.5（顔モザイクを焼く・省略禁止）を追加。モザイク版を候補として扱い、元ファイルを渡さないことを明記検証の降ろし方
M-4-F/M-4-Gは関数呼び出しではなく、実際に動画ファイルを作り、出力をデコードし直して顔検出を掛ける形で確かめます。「関数が正しい値を返す」ではなく「成果物が正しい」まで降ろしました。M-4-Dも、配布する CLI そのものを起動して確かめます。実素材での計測（M-5-C）
RIZIN メディアデー取材映像（1920x1080・10分17秒・椅子に座った1人の顔出しインタビュー）の4区間・計4320コマ。
人物の追跡も全区間でトラック1本を維持し、破綻はありませんでした。
ただしこの素材は正面固定・1人という顔検出に素直な条件です。 複数人・移動・逆光での挙動はこの計測では分かりません。手動確認（M-3）を残しているのはそのためで、
M-5-Cのdetailにも明記しています。確認リストを汚していた副産物を解消
当初「保持で埋めたコマ 1.7%」が出ましたが、40秒でも60秒でも正確に同じ割合だったため調べたところ、顔の見落としではなく分割処理の副産物でした（かたまり末尾の2コマが機械的に記録される。2/120 = 1.667%）。放置すると本当に危ない場面が埋もれて確認が形骸化します。8コマ先読みする形にし、再計測で 0.0% になりました。処理時間への影響はありません。
M-5-B は条件と違うものを測っていたので測り直しました
受入条件は「60秒の動画でモザイク有無の時間差が30秒以内」ですが、書いていたテストは2秒の合成素材の総時間を測っていました。条件どおり、60秒ぶんのコマ数で有無の差を測る形に書き直しています。
PR #32 のレビュー指摘8件も反映
face_choices.py:--secondsの検証、ffmpeg 異常終了の伝播、例外の握り潰し防止write_face_choices: 前回のface-*.pngを消してから書く（古い候補で別人を選ばせない）SKILL.md: 「写っている顔は全員」という誤った保証を「検出された顔には」へ訂正。顔検出には見落としがあることと、出来上がりの確認を明記。質問数3→4の整合も修正roadmap: handoff を進捗のみへCI に ffmpeg を追加
成果物まで検証するテストが ffmpeg を必要とするため、
qualityジョブに導入しました。「入っていないから飛ばす」は AGENTS.md が禁じる偽の緑なので取っていません。検証
テストは 69件（全PASS）。ワークスペース全体も緑です。失敗3件を
docs/failures.mdに追記しました。meta.activeはP1-10（中断していた既存todo）へ戻しています。