diff --git a/.takt/facets/instructions/review-simplicity.md b/.takt/facets/instructions/review-simplicity.md index cfdc037..5e10ff8 100644 --- a/.takt/facets/instructions/review-simplicity.md +++ b/.takt/facets/instructions/review-simplicity.md @@ -6,6 +6,21 @@ The diff has been pre-collected by push-runner (Rust exe) and saved to `.takt/re **Read this file first** using the Read tool. This is the authoritative review target. Do NOT run `git diff` or `jj diff` yourself -- the file already contains the correct diff scope. +### Optional: lint-screen pre-pass (Phase c §8.E, ADR-038 試験運用) + +If `.takt/lint-screen-report.md` exists, push-runner has already run a mistral:7b lint pre-pass on the diff. Read this file as **supplementary context** (treat as advisory, not authoritative): + +- Coverage: rule names from a fixed canonical list (`unused-import` / `no-var` / `no-unused-vars` / `magic-number` / `dead-code` / `deep-nesting` / `complexity`) +- Quality: agreement 75% with Claude baseline (Phase b' conditional GO) — false positives and recall misses are expected +- Use it to: + - Cross-check anomalies you already noticed (consensus signal) + - **Skip dimensions** the lint-screen already covered (avoid duplicate findings of unused-import / magic-number etc.) +- Do NOT use it to: + - Adopt findings verbatim without diff verification + - Override your own judgment on subjective anomalies (deep-nesting boundary, complexity) + +If the report shows `screen_decision: informational` and zero findings, that is a **weak signal of a clean diff** — still review yourself, but you can be more concise in approval rationale. + ## Determinism layer guarantees (do NOT duplicate) The following dimensions are enforced by deterministic hooks at write time and by `fix-metrics-check.ps1` during fix iterations. Skip them — flagging them duplicates the deterministic layer and produces noise: diff --git a/Cargo.lock b/Cargo.lock index ac1f75d..eb9f3cd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -133,6 +133,7 @@ version = "0.1.0" dependencies = [ "lib-jj-helpers", "serde", + "serde_json", "toml", ] diff --git a/push-runner-config.toml b/push-runner-config.toml index 950e674..042909b 100644 --- a/push-runner-config.toml +++ b/push-runner-config.toml @@ -5,7 +5,9 @@ [quality_gate] parallel = true -step_timeout = 120 +# step_timeout = 180s (Phase b' end-to-end test の `cargo test -- --ignored` が +# mistral:7b への 12 件 invoke で 120s 境界に達するため拡大、PR #132 で実証) +step_timeout = 180 [[quality_gate.groups]] name = "lint" @@ -35,6 +37,24 @@ commands = [ command = "jj diff -r @" output_path = ".takt/review-diff.txt" +# --------------------------------------------------------------------------- +# [lint_screen] — Phase c (§8.E lint screen facet) — ADR-038 試験運用配下。 +# diff を mistral:7b に流して lint 一次フィルタの所見を markdown として出力する。 +# 詳細: docs/adr/adr-038-local-llm-finding-classification.md +# default OFF。手動で enabled = true にしたうえで Ollama 起動 + cli-finding-classifier.exe 配備を確認する。 +# Phase b' agreement 75% (conditional GO) のため gating はせず report のみ出力。 +# Ollama down / timeout / diff 過大時は skip + warn (push を block しない)。 +# --------------------------------------------------------------------------- +[lint_screen] +enabled = false +# 以下は default 値、明示しなくてもよいが意図を残すなら明示推奨 +exe_path = ".claude/cli-finding-classifier.exe" +model = "mistral:7b" +endpoint = "http://localhost:11434" +timeout_secs = 60 +max_diff_lines = 5000 +output_path = ".takt/lint-screen-report.md" + [takt] workflow = "pre-push-review" task = "pre-push review" diff --git a/src/cli-push-runner/Cargo.toml b/src/cli-push-runner/Cargo.toml index b35d7c9..b604e67 100644 --- a/src/cli-push-runner/Cargo.toml +++ b/src/cli-push-runner/Cargo.toml @@ -5,6 +5,7 @@ edition = "2021" [dependencies] serde = { version = "1.0", features = ["derive"] } +serde_json = "1.0" toml = "0.8" lib-jj-helpers = { path = "../lib-jj-helpers" } diff --git a/src/cli-push-runner/src/config.rs b/src/cli-push-runner/src/config.rs index f97be94..2eb3418 100644 --- a/src/cli-push-runner/src/config.rs +++ b/src/cli-push-runner/src/config.rs @@ -3,15 +3,38 @@ use std::path::{Path, PathBuf}; pub(crate) const DEFAULT_STEP_TIMEOUT_SECS: u64 = 120; pub(crate) const DEFAULT_PUSH_TIMEOUT_SECS: u64 = 300; +pub(crate) const DEFAULT_LINT_SCREEN_TIMEOUT_SECS: u64 = 60; +pub(crate) const DEFAULT_LINT_SCREEN_MAX_DIFF_LINES: usize = 5000; +pub(crate) const DEFAULT_LINT_SCREEN_MODEL: &str = "mistral:7b"; +pub(crate) const DEFAULT_LINT_SCREEN_ENDPOINT: &str = "http://localhost:11434"; +pub(crate) const DEFAULT_LINT_SCREEN_EXE_PATH: &str = ".claude/cli-finding-classifier.exe"; +pub(crate) const DEFAULT_LINT_SCREEN_OUTPUT_PATH: &str = ".takt/lint-screen-report.md"; #[derive(Deserialize)] pub(crate) struct Config { pub(crate) quality_gate: QualityGateConfig, pub(crate) diff: Option, + pub(crate) lint_screen: Option, pub(crate) takt: TaktConfig, pub(crate) push: PushConfig, } +/// Phase c (§8.E lint screen facet) — pre-push 時に diff を mistral:7b に流して +/// lint 一次フィルタの所見を `.takt/lint-screen-report.md` として出力する。 +/// +/// `enabled = false` の場合は完全 no-op (default OFF, 試験運用)。 +/// Ollama down / timeout / diff 過大時は skip + warn (push を block しない)。 +#[derive(Deserialize)] +pub(crate) struct LintScreenConfig { + pub(crate) enabled: bool, + pub(crate) exe_path: Option, + pub(crate) model: Option, + pub(crate) endpoint: Option, + pub(crate) timeout_secs: Option, + pub(crate) max_diff_lines: Option, + pub(crate) output_path: Option, +} + #[derive(Deserialize)] pub(crate) struct QualityGateConfig { pub(crate) parallel: Option, @@ -265,6 +288,7 @@ command = "echo push" groups: vec![], }, diff: None, + lint_screen: None, takt: TaktConfig { workflow: "w".into(), task: "t".into(), @@ -293,6 +317,7 @@ command = "echo push" }], }, diff: None, + lint_screen: None, takt: TaktConfig { workflow: "w".into(), task: "t".into(), diff --git a/src/cli-push-runner/src/main.rs b/src/cli-push-runner/src/main.rs index a9352fe..800583c 100644 --- a/src/cli-push-runner/src/main.rs +++ b/src/cli-push-runner/src/main.rs @@ -25,7 +25,7 @@ use std::time::Instant; use config::load_config; use log::log_info; -use stages::{run_diff, run_push, run_quality_gate, run_takt, DiffResult}; +use stages::{run_diff, run_lint_screen, run_push, run_quality_gate, run_takt, DiffResult}; const EXIT_SUCCESS: i32 = 0; const EXIT_QUALITY_GATE_FAILURE: i32 = 1; @@ -34,6 +34,29 @@ const EXIT_PUSH_FAILURE: i32 = 3; const EXIT_CONFIG_ERROR: i32 = 4; const EXIT_DIFF_FAILURE: i32 = 5; +/// diff stage を実行し lint-screen を呼び出す。 +/// Ok(skip_takt) で成功、 Err(exit_code) で pipeline 中断。 +fn run_diff_and_lint_screen(config: &config::Config) -> Result { + let Some(diff_config) = &config.diff else { + return Ok(false); + }; + let diff_path = match run_diff(diff_config) { + DiffResult::HasContent => diff_config.output_path.as_str(), + DiffResult::Empty => { + log_info("diff が空のためレビューをスキップして push に進みます。"); + return Ok(true); + } + DiffResult::Error => { + log_info("パイプライン中断: diff 取得失敗。"); + return Err(EXIT_DIFF_FAILURE); + } + }; + if let Some(lint_screen_config) = &config.lint_screen { + run_lint_screen(lint_screen_config, diff_path); + } + Ok(false) +} + fn run_pipeline() -> i32 { let start = Instant::now(); @@ -52,35 +75,21 @@ fn run_pipeline() -> i32 { config.takt.workflow, )); - // Stage 1: quality_gate if !run_quality_gate(&config.quality_gate) { log_info("パイプライン中断: quality_gate 失敗。問題を修正して再実行してください。"); return EXIT_QUALITY_GATE_FAILURE; } - // Stage 1.5: diff - let mut skip_takt = false; - if let Some(diff_config) = &config.diff { - match run_diff(diff_config) { - DiffResult::HasContent => {} - DiffResult::Empty => { - log_info("diff が空のためレビューをスキップして push に進みます。"); - skip_takt = true; - } - DiffResult::Error => { - log_info("パイプライン中断: diff 取得失敗。"); - return EXIT_DIFF_FAILURE; - } - } - } + let skip_takt = match run_diff_and_lint_screen(&config) { + Ok(skip) => skip, + Err(code) => return code, + }; - // Stage 2: takt if !skip_takt && !run_takt(&config.takt) { log_info("パイプライン中断: takt ワークフロー失敗。"); return EXIT_TAKT_FAILURE; } - // Stage 3: push if !run_push(&config.push) { log_info("パイプライン中断: push 失敗。"); return EXIT_PUSH_FAILURE; diff --git a/src/cli-push-runner/src/stages/lint_screen.rs b/src/cli-push-runner/src/stages/lint_screen.rs new file mode 100644 index 0000000..c7bf4c6 --- /dev/null +++ b/src/cli-push-runner/src/stages/lint_screen.rs @@ -0,0 +1,389 @@ +//! Phase c (§8.E lint screen facet) stage +//! +//! diff を mistral:7b に流して lint 一次フィルタの所見を markdown として出力する。 +//! 設計詳細: `docs/adr/adr-038-local-llm-finding-classification.md` +//! +//! 動作モード: +//! - `enabled = false` (default): 完全 no-op、push pipeline は影響を受けない +//! - `enabled = true`: cli-finding-classifier.exe を subprocess で起動、stdin に diff を流し +//! stdout の LintScreenResult JSON を markdown に整形して output_path に書き出す +//! +//! Phase b' で agreement 75% (conditional GO) のため、本 stage は **gating しない**。 +//! Ollama down / timeout / diff 過大 / JSON parse 失敗 等のエラーは全て skip + warn で処理し、 +//! push pipeline をブロックしない。 + +use std::io::Write; +use std::path::Path; +use std::process::{Command, Stdio}; +use std::time::Instant; + +use crate::config::{ + LintScreenConfig, DEFAULT_LINT_SCREEN_ENDPOINT, DEFAULT_LINT_SCREEN_EXE_PATH, + DEFAULT_LINT_SCREEN_MAX_DIFF_LINES, DEFAULT_LINT_SCREEN_MODEL, DEFAULT_LINT_SCREEN_OUTPUT_PATH, + DEFAULT_LINT_SCREEN_TIMEOUT_SECS, +}; +use crate::log::log_stage; +use crate::runner::wait_with_timeout; + +const STAGE: &str = "lint-screen"; + +struct InvokeParams<'a> { + exe: &'a str, + model: &'a str, + endpoint: &'a str, + timeout_secs: u64, +} + +fn resolve_invoke_params(config: &LintScreenConfig) -> InvokeParams<'_> { + InvokeParams { + exe: config + .exe_path + .as_deref() + .unwrap_or(DEFAULT_LINT_SCREEN_EXE_PATH), + model: config.model.as_deref().unwrap_or(DEFAULT_LINT_SCREEN_MODEL), + endpoint: config + .endpoint + .as_deref() + .unwrap_or(DEFAULT_LINT_SCREEN_ENDPOINT), + timeout_secs: config + .timeout_secs + .unwrap_or(DEFAULT_LINT_SCREEN_TIMEOUT_SECS), + } +} + +pub(crate) fn run_lint_screen(config: &LintScreenConfig, diff_path: &str) { + if !config.enabled { + return; + } + + let started = Instant::now(); + log_stage(STAGE, "実行中 (試験運用、エラーは skip + warn)"); + + let diff = match read_diff(diff_path, config) { + Ok(d) => d, + Err(reason) => { + log_stage(STAGE, &format!("skip: {}", reason)); + return; + } + }; + + let params = resolve_invoke_params(config); + let json = match invoke_classifier(¶ms, &diff) { + Ok(j) => j, + Err(reason) => { + log_stage(STAGE, &format!("skip: classifier {}", reason)); + return; + } + }; + + let output_path = config + .output_path + .as_deref() + .unwrap_or(DEFAULT_LINT_SCREEN_OUTPUT_PATH); + match write_report(output_path, &json) { + Ok(()) => log_stage( + STAGE, + &format!( + "出力: {} ({:.0}s)", + output_path, + started.elapsed().as_secs_f64() + ), + ), + Err(e) => log_stage(STAGE, &format!("skip: report 書き出し失敗: {}", e)), + } +} + +fn read_diff(diff_path: &str, config: &LintScreenConfig) -> Result { + let raw = std::fs::read_to_string(diff_path) + .map_err(|e| format!("diff 読み込み失敗 ({}): {}", diff_path, e))?; + let max_lines = config + .max_diff_lines + .unwrap_or(DEFAULT_LINT_SCREEN_MAX_DIFF_LINES); + let lines = raw.lines().count(); + if lines > max_lines { + return Err(format!("diff 過大 ({} 行 > 上限 {})", lines, max_lines)); + } + if raw.trim().is_empty() { + return Err("diff が空".to_string()); + } + Ok(raw) +} + +fn invoke_classifier(params: &InvokeParams<'_>, diff: &str) -> Result { + if !Path::new(params.exe).exists() { + return Err(format!("exe 不在 ({})", params.exe)); + } + + let timeout_str = params.timeout_secs.to_string(); + let mut child = Command::new(params.exe) + .args([ + "--mode", + "lint-screen", + "--model", + params.model, + "--endpoint", + params.endpoint, + "--timeout-secs", + &timeout_str, + ]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .map_err(|e| format!("spawn 失敗: {}", e))?; + + if let Some(mut stdin) = child.stdin.take() { + stdin + .write_all(diff.as_bytes()) + .map_err(|e| format!("stdin 書き込み失敗: {}", e))?; + } + + let stdout_handle = crate::runner::drain_pipe(child.stdout.take().expect("stdout piped")); + let stderr_handle = crate::runner::drain_pipe(child.stderr.take().expect("stderr piped")); + + let exit = wait_with_timeout(STAGE, &mut child, params.timeout_secs + 5) + .map_err(|e| format!("wait 失敗: {}", e))?; + let stdout = stdout_handle.join().unwrap_or_default(); + let stderr = stderr_handle.join().unwrap_or_default(); + + match exit { + None => Err(format!("timeout ({}s)", params.timeout_secs + 5)), + Some(status) if !status.success() => Err(format!("非 0 終了: {}", stderr)), + Some(_) if stdout.trim().is_empty() => Err("stdout 空".to_string()), + Some(_) => Ok(stdout), + } +} + +fn write_report(output_path: &str, classifier_json: &str) -> Result<(), String> { + let path = Path::new(output_path); + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).map_err(|e| format!("ディレクトリ作成失敗: {}", e))?; + } + let markdown = format_report(classifier_json); + std::fs::write(path, markdown).map_err(|e| format!("write: {}", e)) +} + +const REPORT_PREAMBLE: &str = "# Lint Screen Report (mistral:7b, Phase b' agreement 75%)\n\n\ +> **試験運用**: 本 report は ADR-038 Phase c lint screen facet による mistral:7b の AI 所見。\n\ +> agreement 75% (conditional GO) のため誤指摘あり。reviewer が独立判断する前提で参考情報として扱う。\n\n"; + +fn render_parse_error(err: &serde_json::Error, raw: &str) -> String { + let mut out = String::from(REPORT_PREAMBLE); + out.push_str(&format!("## JSON parse 失敗\n\nerror: {}\n\n", err)); + out.push_str("```\n"); + out.push_str(raw); + out.push_str("\n```\n"); + out +} + +fn render_summary(decision: &str, findings_count: usize, fallback_reason: &str) -> String { + let mut s = format!( + "## Summary\n\n- screen_decision: `{}`\n- findings: {}\n", + decision, findings_count + ); + if !fallback_reason.is_empty() { + s.push_str(&format!("- fallback_reason: `{}`\n", fallback_reason)); + } + s.push('\n'); + s +} + +fn render_findings_table(findings: &[serde_json::Value]) -> String { + if findings.is_empty() { + return "## Findings\n\n(なし)\n".to_string(); + } + let mut out = + String::from("## Findings\n\n| severity | rule | file | line | issue | suggestion |\n"); + out.push_str("|---|---|---|---|---|---|\n"); + for f in findings { + let s = f.get("severity").and_then(|v| v.as_str()).unwrap_or("?"); + let r = f.get("rule").and_then(|v| v.as_str()).unwrap_or("?"); + let file = f.get("file").and_then(|v| v.as_str()).unwrap_or("?"); + let line = f + .get("line") + .map(|v| v.to_string()) + .unwrap_or_else(|| "?".to_string()); + let issue = f.get("issue").and_then(|v| v.as_str()).unwrap_or(""); + let sug = f.get("suggestion").and_then(|v| v.as_str()).unwrap_or(""); + out.push_str(&format!( + "| {} | {} | {} | {} | {} | {} |\n", + sanitize_cell(s), + sanitize_cell(r), + sanitize_cell(file), + sanitize_cell(&line), + sanitize_cell(issue), + sanitize_cell(sug), + )); + } + out +} + +fn format_report(classifier_json: &str) -> String { + let value: serde_json::Value = match serde_json::from_str(classifier_json) { + Ok(v) => v, + Err(e) => return render_parse_error(&e, classifier_json), + }; + let decision = value + .get("screen_decision") + .and_then(|v| v.as_str()) + .unwrap_or("?"); + let fallback_reason = value + .get("fallback_reason") + .and_then(|v| v.as_str()) + .unwrap_or(""); + let findings = value + .get("lint_findings") + .and_then(|v| v.as_array()) + .cloned() + .unwrap_or_default(); + + let mut out = String::from(REPORT_PREAMBLE); + out.push_str(&render_summary(decision, findings.len(), fallback_reason)); + out.push_str(&render_findings_table(&findings)); + out +} + +/// markdown table cell 用に `|` と改行を escape する。 +fn sanitize_cell(s: &str) -> String { + s.replace('|', "\\|").replace('\n', " ") +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn format_report_renders_findings_table() { + let json = r#"{ + "lint_findings": [ + {"severity":"minor","rule":"unused-import","file":"src/a.rs","line":1,"issue":"x","suggestion":"y"} + ], + "screen_decision":"auto_fix" + }"#; + let md = format_report(json); + assert!(md.contains("auto_fix")); + assert!(md.contains("unused-import")); + assert!(md.contains("src/a.rs")); + assert!(md.contains("| severity | rule | file | line | issue | suggestion |")); + } + + #[test] + fn format_report_handles_empty_findings() { + let json = r#"{"lint_findings":[],"screen_decision":"informational"}"#; + let md = format_report(json); + assert!(md.contains("informational")); + assert!(md.contains("(なし)")); + } + + #[test] + fn format_report_recovers_from_invalid_json() { + let md = format_report("not json"); + assert!(md.contains("JSON parse 失敗")); + assert!(md.contains("not json")); + } + + #[test] + fn format_report_includes_fallback_reason_when_present() { + let json = r#"{ + "lint_findings":[], + "screen_decision":"human_review", + "fallback_reason":"ollama error: empty" + }"#; + let md = format_report(json); + assert!(md.contains("fallback_reason")); + assert!(md.contains("ollama error")); + } + + #[test] + fn sanitize_cell_escapes_pipe_and_newline() { + assert_eq!(sanitize_cell("a|b"), "a\\|b"); + assert_eq!(sanitize_cell("a\nb"), "a b"); + } + + #[test] + fn render_findings_table_sanitizes_pipe_in_all_columns() { + let json = r#"{ + "lint_findings": [ + {"severity":"mi|nor","rule":"un|used","file":"src/a|b.rs","line":1,"issue":"x|y","suggestion":"y|z"} + ], + "screen_decision":"auto_fix" + }"#; + let md = format_report(json); + assert!(!md.contains("mi|nor"), "severity must be sanitized"); + assert!(md.contains("mi\\|nor")); + assert!(!md.contains("un|used"), "rule must be sanitized"); + assert!(md.contains("un\\|used")); + assert!(!md.contains("a|b.rs"), "file must be sanitized"); + assert!(md.contains("a\\|b.rs")); + } + + #[test] + fn run_lint_screen_is_noop_when_disabled() { + let cfg = LintScreenConfig { + enabled: false, + exe_path: None, + model: None, + endpoint: None, + timeout_secs: None, + max_diff_lines: None, + output_path: None, + }; + run_lint_screen(&cfg, "/nonexistent/diff/path"); + } + + #[test] + fn read_diff_returns_error_on_missing_file() { + let cfg = LintScreenConfig { + enabled: true, + exe_path: None, + model: None, + endpoint: None, + timeout_secs: None, + max_diff_lines: None, + output_path: None, + }; + let result = read_diff("/nonexistent/path.txt", &cfg); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("diff 読み込み失敗")); + } + + #[test] + fn read_diff_returns_error_when_diff_exceeds_limit() { + let path = std::env::temp_dir().join("test-lint-screen-large-diff.txt"); + let large = "x\n".repeat(100); + std::fs::write(&path, large).unwrap(); + + let cfg = LintScreenConfig { + enabled: true, + exe_path: None, + model: None, + endpoint: None, + timeout_secs: None, + max_diff_lines: Some(50), + output_path: None, + }; + let result = read_diff(path.to_str().unwrap(), &cfg); + assert!(result.is_err()); + let err = result.unwrap_err(); + assert!(err.contains("diff 過大")); + assert!(err.contains("100")); + } + + #[test] + fn read_diff_returns_error_on_empty_diff() { + let path = std::env::temp_dir().join("test-lint-screen-empty-diff.txt"); + std::fs::write(&path, "").unwrap(); + let cfg = LintScreenConfig { + enabled: true, + exe_path: None, + model: None, + endpoint: None, + timeout_secs: None, + max_diff_lines: None, + output_path: None, + }; + let result = read_diff(path.to_str().unwrap(), &cfg); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("空")); + } +} diff --git a/src/cli-push-runner/src/stages/mod.rs b/src/cli-push-runner/src/stages/mod.rs index 9300c6a..a366853 100644 --- a/src/cli-push-runner/src/stages/mod.rs +++ b/src/cli-push-runner/src/stages/mod.rs @@ -1,10 +1,12 @@ mod diff; +mod lint_screen; mod push; mod push_jj_bookmark; mod quality_gate; mod takt; pub(crate) use diff::{run_diff, DiffResult}; +pub(crate) use lint_screen::run_lint_screen; pub(crate) use push::run_push; pub(crate) use quality_gate::run_quality_gate; pub(crate) use takt::run_takt;