feat(hooks): takt persona-without-model lint rule (順位 39 / D-4)#150
Merged
Conversation
順位 47 (>vs>= boundary lint) は PR #126 (commit b677b9d, 2026-05-08) で no-time-field-strict-greater rule として既に land 済 (custom-lint-rules.toml line 208-243) を D-4 着手前検証で発見。memory rule feedback_verify_task_not_already_done を適用して stale todo entry を削除し、D-4 を Round 2 計画から 順位 39 (takt workflow `model` 必須化 lint rule) に繰上げ、D-5 を 順位 56 + 119 bundle に再構成。 変更: - docs/local-llm-offload-analysis.md: D-4/D-5 row 更新 + 想定リスク に re-pivot 経緯追記 - docs/todo7.md: stale 順位 47 entry (38 行) 削除 - docs/todo-summary.md: 順位 47 row 削除 Phase E 着手前提 (5 PR 累積 dogfood) は変わらず、D-4〜D-7 で 4 PR 追加観測の方針継続。
…t rule (順位 39 / Phase D D-4) PR #98 post-merge-feedback Tier 1 #1 採用。takt workflow yaml で persona: を持つ step に model: 未指定の場合を検出する custom lint rule (ルール⑨)。Bundle Z #B-α と同じ 決定論的防止層 (ADR-007 正規表現層)。 実装: - .claude/custom-lint-rules.toml: 新規 rule takt-workflow-persona-without-model - pattern: multi-line regex で persona: 直後の field 行が model: 以外なら fire - field enumeration 方式 (Rust regex lookahead 非対応の pragmatic 対処) - extensions=[yaml] + paths=[.takt/workflows/*.yaml] で範囲限定 (順位 102 PR #148 で 実装した paths filter を利用) - .takt/workflows/post-pr-review.yaml: 2 site に model: sonnet 追加で clean baseline - line 23: judge block (loop_monitor) の persona: supervisor 直下 - line 117: supervise step の persona: supervisor 直下 (Bundle Y2 完全性) - .takt/workflows/pre-push-review.yaml: 1 site に model: sonnet 追加で clean baseline - line 26: judge block (loop_monitor) の persona: supervisor 直下 - src/hooks-post-tool-linter/src/main.rs: unit tests 6 件追加 - takt_workflow_persona_detects_judge_block_violation - takt_workflow_persona_detects_supervise_step_violation - takt_workflow_persona_skips_when_model_directly_follows (clean baseline) - takt_workflow_persona_detects_multiple_violations_in_same_file - takt_workflow_persona_skips_non_yaml_extension (extensions filter) - deployed_takt_workflows_have_clean_baseline_for_persona_model_rule (regression test) Phase D D-4 dogfood: real diff として lint_screen の dogfood data 蓄積を目的。 Round 2 計画 (D-3 + D-4〜D-7 = 累積 5 PR) の 2 PR 目。 cargo test: 108 passed (前 102 + 新 6)。
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthrough新しいカスタムlintルール ChangesPersona Model Lint Rule and Enforcement
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
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. 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
🧹 Nitpick comments (1)
src/hooks-post-tool-linter/src/main.rs (1)
2036-2042: ⚡ Quick winテスト用ルールの正規表現ハードコードは設定とのドリフト源です
ここで正規表現を複製すると、
.claude/custom-lint-rules.toml更新時にテストが古いまま残るリスクがあります。id = "takt-workflow-persona-without-model"を TOML から読み出してテストに使う形へ寄せるのが安全です。差分イメージ
- fn takt_workflow_persona_without_model_rule() -> CustomRule { - make_test_rule( - "takt-workflow-persona-without-model", - r"...", - &["yaml"], - ) - } + fn deployed_rule_by_id(id: &str) -> CustomRule { + let path = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("..").join("..").join(".claude").join("custom-lint-rules.toml"); + let content = std::fs::read_to_string(&path).expect("read custom-lint-rules.toml"); + let config: CustomRulesConfig = toml::from_str(&content).expect("parse custom rules"); + config.rules + .unwrap_or_default() + .into_iter() + .find(|r| r.id == id) + .expect("rule id not found") + }🤖 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 `@src/hooks-post-tool-linter/src/main.rs` around lines 2036 - 2042, The test currently hardcodes the rule id and regex in takt_workflow_persona_without_model_rule; change it to read the rule definition (at least the id, preferably the regex) from the custom rules TOML so the test stays in sync: modify takt_workflow_persona_without_model_rule to lookup the entry with id "takt-workflow-persona-without-model" from the parsed .claude/custom-lint-rules.toml (reuse existing TOML parsing logic if available), then call make_test_rule using the retrieved id and pattern instead of the inline string/regex.
🤖 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 @.claude/custom-lint-rules.toml:
- Line 318: The regex pattern variable (pattern) used to detect a persona:
followed by certain keys is missing several valid keys (e.g., output_contracts,
pass_previous_response, required_permission_mode, parallel), causing false
negatives; update the pattern string so the alternation after persona: includes
those missing keys (and any other current keys used in persona blocks) so that
lines like "persona: <name>" followed by those keys are matched; ensure the
updated pattern still uses the same (?m) and anchors and preserves whitespace
handling used in the original pattern.
In `@docs/local-llm-offload-analysis.md`:
- Line 5: The document asserts future progress on "2026-05-13" (e.g., references
to "Phase d Round 2", "着手", "着手中", "追加済み前提" under the "状態" / Phase sections) as
if already completed; change those statements into planned language or separate
planned vs actual sections: replace assertive verbs like "着手/追加済み" with
"予定/着手予定" (or add "(予定)") for every occurrence of the date string "2026-05-13",
add a clear "更新基準日: 2026-05-12" header, and explicitly split content into "実績"
(completed items) and "計画/予定" (future items) so the Phase d/Phase d Round 2 and
Phase c+ mentions are not presented as accomplished.
---
Nitpick comments:
In `@src/hooks-post-tool-linter/src/main.rs`:
- Around line 2036-2042: The test currently hardcodes the rule id and regex in
takt_workflow_persona_without_model_rule; change it to read the rule definition
(at least the id, preferably the regex) from the custom rules TOML so the test
stays in sync: modify takt_workflow_persona_without_model_rule to lookup the
entry with id "takt-workflow-persona-without-model" from the parsed
.claude/custom-lint-rules.toml (reuse existing TOML parsing logic if available),
then call make_test_rule using the retrieved id and pattern instead of the
inline string/regex.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 53779dbf-24fe-4e94-9d71-06ae9c33a798
📒 Files selected for processing (7)
.claude/custom-lint-rules.toml.takt/workflows/post-pr-review.yaml.takt/workflows/pre-push-review.yamldocs/local-llm-offload-analysis.mddocs/todo-summary.mddocs/todo7.mdsrc/hooks-post-tool-linter/src/main.rs
💤 Files with no reviewable changes (2)
- docs/todo-summary.md
- docs/todo7.md
…ds 追加 (PR #150 CR Major) CodeRabbit Major finding (#150 (comment)): persona 直後の field 列挙 から output_contracts / pass_previous_response / required_permission_mode / parallel が漏れていた = false negative リスク。 4 fields を pattern alternation に追加し regression test も同梱。 変更: - .claude/custom-lint-rules.toml: ルール⑨ pattern alternation に 4 fields 追加 - src/hooks-post-tool-linter/src/main.rs: 同期更新 + 新 test takt_workflow_persona_detects_required_permission_mode_violation で pre-push-review.yaml fix step (persona: coder → required_permission_mode:) pattern が検出されることを assert cargo test: 109 passed (前 108 + 新 1、CR Major regression test) CR Minor (line 5、未来日進捗) は project local time (JST) と CR UTC の時差解釈 ギャップに起因 (CR 時点 = 2026-05-12 20:04 UTC = 2026-05-13 05:04 JST)。 2026-05-13 は session-start hook が認識する local 今日であり、'着手' は本 PR で 発生した事実。project 内既存 PR land 日付も全て local time (timezone qualifier 無し) で記載しており、本 PR の追記のみ qualifier 付与は逆に不整合を生む。 別 reply で resolved として close する。
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
aloekun
added a commit
that referenced
this pull request
May 13, 2026
… test (#151) * feat(hooks): Phase D D-5 — comment-lint test 拡充 + MAX_CUSTOM_VIOLATIONS outer/inner break test (順位 56 + 119) ## D-5 実装 (順位 56 + 119 bundle) ### 順位 56: comment-lint hook test 拡充 (PR #104 T2-1+T2-2 bundle) `src/hooks-post-tool-comment-lint-rust/src/main.rs`: - UTF-8 multi-byte test 5 パターン追加: 漢字 + ASCII 混合 / 漢字単独 / emoji / BMP 外 文字 (𝕊) / 結合文字 (e + U+0301) - Block comment boundary test 6 パターン追加: 複数行 (start/end/middle 被覆) + 単行 block comment (exact / starts-at / ends-at) で `span_overlaps_ranges` の inclusive-bounds 区間交差判定を体系化 - 既存 1 パターンずつのテストは保持 (regression 防止) ### 副産物: `byte_offset_to_line` char-boundary panic fix UTF-8 漢字単独パターン test 着手時に発見した production bug を修正: - 旧: `source[..clamped].bytes()` は `clamped` が char boundary でない場合 panic - 新: `source.as_bytes()[..clamped].iter()` で byte slice 経由に変更 (char boundary 不要) - 影響: needle 末尾が multi-byte 文字の場合 (例: 「漢字のみのコメント」を Edit/Write の new_string として渡す現実シナリオ) で hook が panic していたのを修正 - direct unit test `byte_offset_to_line_handles_mid_multibyte_offset` を追加して bug location を pinpoint ### 順位 119: MAX_CUSTOM_VIOLATIONS outer/inner loop break scope explicit test `src/hooks-post-tool-linter/src/main.rs`: - `run_custom_rules_outer_break_skips_subsequent_rules`: rule_a 単独で 21 件 → cap で 20 件、rule_b は呼ばれない (全 violation が "RULE_A") を assert - `run_custom_rules_inner_cap_after_partial_first_rule`: rule_a 19 件 + rule_b 1 件 = 20 件 (rule_b の inner break が mid-rule で発火) を type 分布で検証 - PR #148 で発見した `run_custom_rules` refactor の outer/inner loop break scope を test net で固定化 ## test 結果 - hooks-post-tool-comment-lint-rust: 85 passed (前 84 + 新 1 direct unit + 11 integration) - hooks-post-tool-linter: 111 passed (前 109 + 新 2) - 合計 196 tests pass ## docs 整理 - docs/todo7.md: 順位 56 entry 削除 - docs/todo8.md: 順位 119 entry 削除 - docs/todo-summary.md: 順位 56 / 119 行削除 - docs/local-llm-offload-analysis.md: D-5 row を ✅ 着手済 に更新、bug fix を副産物として明記 ## 並行: PR #150 post-merge-feedback 登録 (本セッション前から @ に含まれていた変更) - docs/todo8.md: 順位 120 (rule comment + ADR-007 case study) / 順位 121 (4 fields 個別 fixture test) / 順位 122 (development-workflow.md Step 0 stale entry 確認 step) 登録 - docs/todo-summary.md: 3 行追加 ## Phase D 累積状況 D-3 (PR #148) + D-4 (PR #150) + D-5 (本 PR) = 3 PR 完了、Phase E 着手要件 (5 PR 累積) まであと 2 PR (D-6 + D-7)。本 PR の dogfood は push 時に `LINT_SCREEN_ENABLED=true` で opt-in 観測予定。 * docs(analysis): Phase D D-5 dogfood outcome を記録 (lint_screen 4 件目の data point)
aloekun
added a commit
that referenced
this pull request
May 18, 2026
…検出 hook 2 段構え) ## 順位 39 削除 順位 39 (takt workflow `model` フィールド必須化 lint rule) は PR #150 で `takt-workflow-persona-without-model` rule として実装済のため、todo-summary.md table 行と todo4.md 詳細 entry を削除。 調査の過程で、stale entry 候補 3 件のうち以下 2 件は既に削除済と判明: - 順位 104 (ADR-007 amendment): PR #161 で既削除 - 順位 126 (ADR-038 hallucinate codify): PR #156 (Phase E) で ADR-038 migrate + 詳細 entry 削除済 ## 順位 136 新規追加 (Tier 1) 本セッション (PR cleanup-stale-rank-39 作業中) で実証された failure mode: local working copy が stale parent (master と sibling) のまま docs/todo*.md を 読み込み、master 上で既に削除済の entry 2 件を「stale entry として削除する」と 誤判定。memory rule `feedback_verify_task_not_already_done.md` は強制力ゼロで 再発確実 (memory rule 全般の限界、`feedback_no_unenforced_rules.md` 原則の 自己事例)。Claude Code Web との並列セッション運用前提下では構造的再発確実。 structural defense として 2 hook 二段構え: - 案 A (予防層): SessionStart hook で `jj git fetch` + master との lineage を additional context として AI 出力 - 案 B (最終 backstop): PreToolUse hook で stale parent 時の docs/todo*.md edit を hard block `feedback_no_unenforced_rules.md` 例外条件 = 2 つの hook で機械強制可能。 ADR-039 experimental pattern 適用予定。
4 tasks
aloekun
added a commit
that referenced
this pull request
May 18, 2026
…検出 hook 2 段構え) (#162) ## 順位 39 削除 順位 39 (takt workflow `model` フィールド必須化 lint rule) は PR #150 で `takt-workflow-persona-without-model` rule として実装済のため、todo-summary.md table 行と todo4.md 詳細 entry を削除。 調査の過程で、stale entry 候補 3 件のうち以下 2 件は既に削除済と判明: - 順位 104 (ADR-007 amendment): PR #161 で既削除 - 順位 126 (ADR-038 hallucinate codify): PR #156 (Phase E) で ADR-038 migrate + 詳細 entry 削除済 ## 順位 136 新規追加 (Tier 1) 本セッション (PR cleanup-stale-rank-39 作業中) で実証された failure mode: local working copy が stale parent (master と sibling) のまま docs/todo*.md を 読み込み、master 上で既に削除済の entry 2 件を「stale entry として削除する」と 誤判定。memory rule `feedback_verify_task_not_already_done.md` は強制力ゼロで 再発確実 (memory rule 全般の限界、`feedback_no_unenforced_rules.md` 原則の 自己事例)。Claude Code Web との並列セッション運用前提下では構造的再発確実。 structural defense として 2 hook 二段構え: - 案 A (予防層): SessionStart hook で `jj git fetch` + master との lineage を additional context として AI 出力 - 案 B (最終 backstop): PreToolUse hook で stale parent 時の docs/todo*.md edit を hard block `feedback_no_unenforced_rules.md` 例外条件 = 2 つの hook で機械強制可能。 ADR-039 experimental pattern 適用予定。
7 tasks
aloekun
added a commit
that referenced
this pull request
May 21, 2026
…e-feedback 3 件統合) (#167) * docs(todo): 順位 139 新規追加 — PR #166 post-merge-feedback 採用 (exe-help-block system exe FP negative test) * test(hooks-pre-tool-validate): 順位 139 — exe-help-block system exe negative test 追加 PR #166 post-merge-feedback で 3 独立ソース (PR diff / Session / Pre-push:simplicity) で同時指摘された systemic pattern への対策。takt-fix で regex は `(?:cli-[\w-]+|hooks-[\w-]+|check-ci-[\w-]+)\.exe` に narrowed 済だが、 既存 `exe_help_block_allows_unrelated_exe` は `foo.exe` の架空名のみで実在 system exe を未検証。将来 regex を broad pattern に戻した場合の silent regression を test 層で明示的に検出する目的で、cargo/python/node/notepad の 4 件 negative test を追加。 * test(hooks-post-tool-linter): 順位 121 — takt_workflow_persona 3 fields 個別 fixture test 追加 + doc 修正 PR #150 CR Major fix で persona: alternation に追加された 4 fields のうち、 従来 regression test は `required_permission_mode` の 1 case のみで、 doc comment は「4 fields regression test」と主張しつつ実態と乖離していた。 本 commit で: - 既存 `takt_workflow_persona_detects_required_permission_mode_violation` の doc を 実態に合わせて「代表 case」表現に修正、残り 3 fields は個別 test で検証する旨を明記 - `pass_previous_response` / `output_contracts` / `parallel` の個別 fixture test を追加 - `.claude/custom-lint-rules.toml` の `[rules.test_coverage.main_ext_tests.yaml]` に 3 件の新 test 名を登録 (rule test coverage check で機械強制される宣言) 将来 alternation から個別 field を誤って削除した場合に test fail で検出される safety net を確保。 (順位 121、PR #150 T2-#1 採用) * docs(adr): 順位 120 — rule⑨ コメント拡張 + ADR-007 に enumeration-based pattern case study 追記 PR #150 CR Major fix で「persona: alternation の列挙漏れ」が発生した経験を踏まえ、 takt yaml schema 拡張時の rule 更新フローを 2 箇所に codify: (1) `.claude/custom-lint-rules.toml` rule⑨ 上部に「Field 拡張手順」4 ステップを追記 (grep で sibling field を抽出 → alternation 追加 → test helper 追加 → fixture test + [rules.test_coverage] 宣言追加 → cargo test 確認) (2) `docs/adr/adr-007-custom-linter-layer-boundary.md` に Case study section を追加 (Rust regex の lookahead 非対応 → enumeration-based negation 採用の判断経緯 + 列挙漏れリスクへの多層防御 + 同型 rule 設計時の checklist) 順位 121 (個別 fixture test) と 順位 120 (本 commit) で「規約 → test → 保守手順」の 3 層が完成し、次回 takt yaml schema 拡張時の rule 更新フローが文書化される。 (順位 120、PR #150 T1-#1 採用、analyzer Tier 1 → ユーザー判断で Tier 3 reclassify) * docs(todo): 順位 120 / 121 / 139 完了に伴い削除 (Bundle l land) PR #150 + #166 post-merge-feedback 採用 3 件 (Bundle l: Custom lint rule test gap closure) が同 PR で land 完了したため、対応する todo entry を削除: - 順位 139 (exe-help-block system exe negative test): commit 3049f56 で 4 件 negative test 追加 - 順位 121 (takt_workflow_persona 3 fields fixture test): commit bfab173 で 3 件 fixture test 追加 + doc 修正 + .claude/custom-lint-rules.toml の [rules.test_coverage.main_ext_tests.yaml] 宣言更新 - 順位 120 (rule⑨ コメント拡張 + ADR-007 case study): commit a2df865 で field 拡張手順 4 ステップ追記 + ADR-007 に enumeration-based pattern case study section 追加 todo-summary.md / todo8.md から該当 entry を一括削除。残存 task の優先順位に影響なし。
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
1 participant
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.
Summary
Phase D D-4 として 順位 39 (PR #98 post-merge-feedback Tier 1 #1) の takt-workflow-persona-without-model lint rule を実装。Bundle Z #B-α と同じ決定論的防止層 (ADR-007 正規表現層)。
当初 D-4 = 順位 47 (
>vs>=boundary lint) を予定していたが、着手前検証 (memory rulefeedback_verify_task_not_already_done) で PR #126 (b677b9d4f54d) で既に land 済 を発見し、D-5 から 順位 39 を D-4 に繰上げ、D-5 を 順位 56 + 119 bundle に再構成。詳細は docs/local-llm-offload-analysis.md L295-299 参照。Commit 構成
12d11c96— docs(todo,analysis): D-4 を 順位 39 に re-pivot + stale 順位 47 entry 削除0c2cc07d— feat(hooks-post-tool-linter): takt-workflow-persona-without-model lint rule変更内容 (commit 2)
新規 lint rule ⑨ (
.claude/custom-lint-rules.toml)takt-workflow-persona-without-modelpersona:直後の field 行がmodel:以外なら fireClean baseline (3 yaml site に
model: sonnet追加).takt/workflows/post-pr-review.yaml:24(judge block in loop_monitor).takt/workflows/post-pr-review.yaml:119(supervise step、Bundle Y2 完全性).takt/workflows/pre-push-review.yaml:27(judge block in loop_monitor)Unit tests (6 件、
src/hooks-post-tool-linter/src/main.rs)takt_workflow_persona_detects_judge_block_violationtakt_workflow_persona_detects_supervise_step_violationtakt_workflow_persona_skips_when_model_directly_follows(clean baseline)takt_workflow_persona_detects_multiple_violations_in_same_filetakt_workflow_persona_skips_non_yaml_extensiondeployed_takt_workflows_have_clean_baseline_for_persona_model_rule(regression test)Phase D D-4 dogfood metrics
LINT_SCREEN_ENABLED=true で push を実行。
.takt/lint-screen-report.mdの結果:informational## DiagnosticsectionD-4 観測の意義:
auto_fix+ 1 false positive、D-4 はinformational+ 0 findings = lint-screen が「指摘なし」と判定する経路を実証Test plan
cargo test -p hooks-post-tool-linter --bin hooks-post-tool-linterで 108 tests pass (前 102 + 新 6)cargo fmt --checkで新規追加コードは rustfmt cleanpnpm build:allで deploy 用 exe 更新成功LINT_SCREEN_ENABLED=true pnpm pushで pre-push pipeline 完走 (simplicity + security 両 APPROVED、1 iteration 3m 28s).takt/workflows/*.yaml全 file が clean baseline 維持 (deployed_takt_workflows_have_clean_baseline_for_persona_model_ruletest で機械検証)Out of scope
pnpm deploy:hooks実行で対応関連
Summary by CodeRabbit
persona:に対するmodel:明示を検証するカスタムリントルールmodel: sonnetを明示