feat(hooks): takt persona-without-model lint rule (順位 39 / D-4)#150
Open
aloekun wants to merge 3 commits into
Open
feat(hooks): takt persona-without-model lint rule (順位 39 / D-4)#150aloekun wants to merge 3 commits into
aloekun wants to merge 3 commits into
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)。
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (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 Tip CodeRabbit can scan for known vulnerabilities in your dependencies using OSV Scanner.OSV Scanner will automatically detect and report security vulnerabilities in your project's dependencies. No additional configuration is required. |
![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 する。
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
New Features
persona:フィールドに対してmodel:の明示的指定を検証する新しいカスタムリントルールTests
Documentation