Skip to content

test(lib-subprocess): drain_pipe_capped 境界値テスト追加 (順位 208) - #210

Merged
aloekun merged 1 commit into
masterfrom
rank-208-drain-pipe-boundary-test
Jun 16, 2026
Merged

test(lib-subprocess): drain_pipe_capped 境界値テスト追加 (順位 208)#210
aloekun merged 1 commit into
masterfrom
rank-208-drain-pipe-boundary-test

Conversation

@aloekun

@aloekun aloekun commented Jun 16, 2026

Copy link
Copy Markdown
Owner

Summary

  • drain_pipe_capped / drain_pipe_capped_reporting の MAX_LINES boundary に密着した 6 境界値テストを追加 (N-1 / N / N+1 行 × 2 variant)
  • PR refactor(lib): drain_pipe を 3 variant で lib-subprocess に extract (順位 173c) #207 で CR Major として検出された pr_size_check の silent disable (fail-open、ADR-043 violation) の root cause = drain_pipe_capped の off-by-one regression を test レベルで mechanical pin
  • 順位 173e (variant merge 検討) の前段として variant 判断の test ベース根拠を強化

設計

  • max_lines = 5 で固定、各境界値 (4 / 5 / 6 行) を独立 case として記述
  • drain_pipe_capped_reporting 側は truncated 報告 on/off の boundary も同時に pin (N 行 = 報告なし / N+1 行 = ... (1 line truncated) 付与)
  • memory feedback_test_dry_antipattern に従い helper 関数を作らず各 case を独立 setup

副次変更 (takt-fix iter 1 → iter 2 で auto-fix)

  • drain_pipe_capped_reporting の truncated 報告に英語単複処理を追加 (1 → "line" / 2+ → "lines")
  • 既存 test 1 件と新規 test 1 件の期待文字列を "... (1 line truncated)" に更新

Test plan

  • cargo test -p lib-subprocess 全 27 test pass (既存 21 + 新規 6)
  • cargo clippy -p lib-subprocess --tests -- -D warnings clean
  • 手動 mutation regression check: < max_lines<= max_lines で両 variant の n_plus_1 test が落ちることを確認
  • takt pre-push-review (iter 2 で fully_resolved)

関連

Summary by CodeRabbit

  • バグ修正

    • 処理結果の出力メッセージで、単数形と複数形の表記を正確に区別するよう改善しました。
  • テスト

    • 最大行数周辺の境界値条件に対する包括的なテストを追加しました。
  • ドキュメント

    • 完了したタスク関連のドキュメントを整理しました。

PR #207 (順位 173c) で CR が 🟠 Major として発見した silent truncate 由来 silent
disable (fail-open、ADR-043 violation) の root cause = drain_pipe_capped が
MAX_LINES boundary で off-by-one regression を起こしうる構造を test レベルで pin する。

追加内容 (lib-subprocess/src/lib.rs):
- drain_pipe_capped: N-1 / N / N+1 行の 3 boundary test
- drain_pipe_capped_reporting: 同 3 boundary test (truncated 報告 on/off 含む)
- max_lines=5 で固定、各 case は独立 setup (feedback_test_dry_antipattern 準拠)

検証:
- cargo test -p lib-subprocess 全 27 test pass (既存 21 + 新規 6)
- cargo clippy -p lib-subprocess --tests -- -D warnings clean
- 手動 mutation check: `< max_lines` → `<= max_lines` で両 variant の
  n_plus_1 test が落ちることを確認 (off-by-one regression 検出構造)
@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 553e13ef-f2ba-4b67-b94c-3f6f74408af5

📥 Commits

Reviewing files that changed from the base of the PR and between 2fce138 and f57ead0.

📒 Files selected for processing (3)
  • docs/todo-summary.md
  • docs/todo10.md
  • src/lib-subprocess/src/lib.rs
💤 Files with no reviewable changes (2)
  • docs/todo10.md
  • docs/todo-summary.md

📝 Walkthrough

Walkthrough

drain_pipe_capped_reporting の切り捨てメッセージに単数/複数形の分岐(truncated == 1"line")を追加し、drain_pipe_cappeddrain_pipe_capped_reporting に対して max_lines の N-1/N/N+1 境界を検証するテストを追加。実装完了に伴い、対応する TODO エントリを docs/todo10.mddocs/todo-summary.md から削除。

Changes

drain_pipe_capped 境界値テストとメッセージ修正

Layer / File(s) Summary
単複形修正と境界値テスト追加
src/lib-subprocess/src/lib.rs
truncated == 1 のとき "line"、2 以上のとき "lines" を使う条件分岐を追加。drain_pipe_cappeddrain_pipe_capped_reporting に対して N-1/N/N+1 の入出力期待値を検証するテスト群を追加し、reporting 版は N+1 のときのみ "1 line truncated" 形式のサマリが付くことを確認。
TODO エントリの削除
docs/todo10.md, docs/todo-summary.md
実装完了に伴い、境界値テスト追加タスクのエントリを両ドキュメントから削除。todo10.md は「既知課題は無し」と記録するセクションが残る構成になっている。

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • aloekun/claude-code-hook-test#207: 同じ drain_pipe_capped_reporting の切り捨てサフィックス書式(単数・複数)の調整と drain_pipe* 系の境界テスト追加・拡張を行っており、本 PR はそのポストマージフィードバック対応に相当する。
  • aloekun/claude-code-hook-test#133: docs/todo-summary.md の推奨実行順序表から特定 TODO エントリを削除・調整しており、本 PR のドキュメント変更と同様の操作を行っている。
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed プルリクエストのタイトルは、メインの変更内容(drain_pipe_cappedの境界値テスト追加)を明確かつ具体的に要約しており、実装内容と完全に一致している。
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@aloekun
aloekun merged commit 6dc988f into master Jun 16, 2026
1 check passed
@aloekun
aloekun deleted the rank-208-drain-pipe-boundary-test branch June 16, 2026 03:17
aloekun added a commit that referenced this pull request Jun 16, 2026
PR #210 セッションで実観測した cron timing race + CR clean detection gap +
単複境界 test 教訓を踏まえ、post-merge-feedback report (`.claude/feedback-reports/210.md`)
の ✅ 採用候補 4 件 (T1-1 / T2-1 / T3-1 / T3-2) をユーザー承認 (2026-06-16) のもと
docs/todo10.md + docs/todo-summary.md に登録。

追加エントリ:
- 順位 208 (T1-1): cli-pr-monitor の CR walkthrough body parse で 'No actionable comments' 検出 (S, 🔧 Tier 2)
- 順位 209 (T2-1): PARK signal 出力を分単位 round-UP に変更 — Cron 秒解像度ミスマッチ修正 (S, 🔧 Tier 2)
- 順位 210 (T3-1): ~/.claude/rules/common/development-workflow.md + MEMORY.md に 'Cron 秒 → 分 round-UP' rule codify (XS, 💎 Tier 3)
- 順位 211 (T3-2): ~/.claude/rules/common/testing.md に '単複・閾値・時制で出力形式が変わる関数は N=0 / N=1 / N≥2 の 3 境界 variant 必須' guideline 追加 (XS, 💎 Tier 3)

経路の特殊性:
- 順位 208/209/210 は session 内で AI が改善案として独自抽出 → ユーザーが post-merge-feedback report を待たずに採用予告 → analyzer も同一 3 件 + T3-2 を独立抽出 (二重確認)
- 通常の post-merge-feedback flow (merge → analyzer → ユーザー承認) と異なり、session-derived findings を report が裏打ちする形になったため、専用 docs PR として通常 land フローと分離
aloekun added a commit that referenced this pull request Jun 17, 2026
…0) (#212)

* fix(pr-monitor): CR check flow defects (Bundle CR-CHECK 順位 208/209/210)

PR #210/#211 セッションで実観測した cli-pr-monitor の CR check flow 不具合 3 件を
バンドル修正:

順位 208 (CR walkthrough body parse):
- check-ci-coderabbit に WALKTHROUGH_CLEAN_MARKER / WALKTHROUGH_HEADER_MARKER 定数 +
  parse_walkthrough_clean_marker + is_clean_walkthrough_comment helper を追加
- CodeRabbitStatus に walkthrough_clean: bool フィールド追加
- decide() に 'walkthrough_clean && !has_unresolved → complete' 分岐追加
- formal Review object が無くても walkthrough body の clean marker を信頼して
  recheck loop を構造的に終了。memory feedback_coderabbit_no_actionable_merge_signal
  の手動 workaround を機械化
- 7 fixture test (header あり / rate-limit overlay / marker なし / CR 以外 / push_time
  前 / decide 2 経路)

順位 209 (PARK signal output round-UP):
- cli-pr-monitor/stages/poll.rs に round_up_to_next_minute pure helper +
  compute_safe_minute_for_park_signal helper 追加
- ReviewParkSignalFields に safe_minute_unix / safe_minute_iso_utc 追加
- format_review_park_signal template に新 field 出力 + ACTION REQUIRED block で
  2 制約 (秒→分 round-UP + :00/:30 minute 衝突回避) を sequential check として明示
- AI agent consumer が round-DOWN 罠を踏まない構造的防御
- 10 test (round-UP 5 件 / compute 4 件 / signal 1 件)

順位 210 (rule codify):
- ~/.claude/rules/common/development-workflow.md に 'Cron スケジューリングの秒 → 分
  round-UP' section 追加、analyzer 観点の '30s jitter vs 90s 前倒し制約' 混在を
  2 直交制約として別個に明示 (Constraint 1 = scheduling minimum lead time /
  Constraint 2 = execution jitter ≤90s pre-fire)
- feedback_cron_round_up_for_second_timestamps.md 新規 memory + MEMORY.md pointer
- feedback_coderabbit_no_actionable_merge_signal.md に '順位 208 で機械化済' 追記

検証:
- cargo build --release clean (30.72s)
- cargo test -p check-ci-coderabbit: 89 passed (82 既存 + 7 新規)
- cargo test -p cli-pr-monitor: 203 passed (193 既存 + 10 新規)
- cargo clippy ... -D warnings clean
- markdownlint clean

scope 外 (別 PR で対応):
- poll.rs (1988→2090 lines) / check-ci-coderabbit/main.rs (2143→2356 lines) /
  run_check (147 lines) の file/function-length lint 違反は touch-trigger ratchet
  情報 (block されない soft-feedback) として残置、ファイル責務分割 refactor で対応

global 編集 (jj 管理外):
- ~/.claude/rules/common/development-workflow.md
- ~/.claude/projects/.../memory/feedback_cron_round_up_for_second_timestamps.md
- ~/.claude/projects/.../memory/MEMORY.md
- ~/.claude/projects/.../memory/feedback_coderabbit_no_actionable_merge_signal.md

* test(check-ci-coderabbit): walkthrough_clean header-only test 追加 (PR #212 CR Nitpick 採用)

CR Nitpick 指摘 (PR #212 review 1 件) を採用。既存 test
walkthrough_clean_skipped_when_header_missing_to_avoid_user_post_false_positive
が humanreviewer ユーザーで user filter + header check の 2 責務を同時テストして
いたため、header marker 単独の検証を coderabbitai[bot] + clean marker + header 欠落
の独立 test として補完。

追加: walkthrough_clean_skipped_when_coderabbitai_post_lacks_header_marker

検証:
- cargo test -p check-ci-coderabbit: 90 passed (89 既存 + 1 新規)
- memory feedback_test_dry_antipattern.md と整合 (テスト独立性を優先、新規 test は
  fixture を inline で独立 setup)

CR 評価: 🧹 Nitpick / Low value / Effort XS / Risk None。memory
feedback_test_dry_antipattern の精神と一致する提案 (DRY ではなく test responsibility
分離) のため採用。
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant