feat(cli-push-runner): Phase c MVP — pre-push lint-screen step (default OFF)#132
Merged
feat(cli-push-runner): Phase c MVP — pre-push lint-screen step (default OFF)#132
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
📝 Walkthrough概要PR は Diff に対して外部 lint-screen 分類器を実行し、マークダウン形式のレポートを生成する新しいパイプラインステージを追加します。このステージは設定で無効化でき、push パイプラインをブロックしません。 変更内容Lint-Screen Feature
推定コードレビュー難度🎯 3 (中程度) | ⏱️ ~20 分 関連する可能性のある PR
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 1
🧹 Nitpick comments (1)
src/cli-push-runner/src/config.rs (1)
113-227: ⚡ Quick win
[lint_screen]セクションのパース動作を検証するテストが不在
config_parses_with_diffが[diff]セクションのパースを検証しているように、[lint_screen]セクション全フィールドのパースをテストするケースがない。serde の field 名と TOML キー名は完全一致が必要なため、フィールドのリネームや追加が silent な None へのフォールバックに化けるリスクを防ぐためにも追加が望ましい。🧪 テスト追加の例
+ #[test] + fn config_parses_lint_screen_section() { + let toml_str = r#" +[quality_gate] +[[quality_gate.groups]] +name = "test" +commands = ["echo ok"] + +[takt] +workflow = "w" +task = "t" + +[push] +command = "echo push" + +[lint_screen] +enabled = true +exe_path = "custom/classifier.exe" +model = "mistral:7b" +endpoint = "http://localhost:11434" +timeout_secs = 30 +max_diff_lines = 1000 +output_path = ".takt/custom-report.md" +"#; + let config: Config = toml::from_str(toml_str).unwrap(); + let ls = config.lint_screen.unwrap(); + assert!(ls.enabled); + assert_eq!(ls.exe_path.as_deref(), Some("custom/classifier.exe")); + assert_eq!(ls.model.as_deref(), Some("mistral:7b")); + assert_eq!(ls.endpoint.as_deref(), Some("http://localhost:11434")); + assert_eq!(ls.timeout_secs, Some(30)); + assert_eq!(ls.max_diff_lines, Some(1000)); + assert_eq!(ls.output_path.as_deref(), Some(".takt/custom-report.md")); + }🤖 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/cli-push-runner/src/config.rs` around lines 113 - 227, Add a new unit test (similar to config_parses_with_diff) named e.g. config_parses_lint_screen that deserializes a TOML string containing a full [lint_screen] section and asserts every field on Config.lint_screen (or the LintScreen type) is Some/equals the expected values; locate the Config struct and existing tests (e.g. config_parses_with_diff, config_parses_full_without_diff) to mirror style, ensure you check optional fields unwrap_or defaults where applicable and assert that no field silently remains None due to a naming mismatch.
🤖 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 `@src/cli-push-runner/src/stages/lint_screen.rs`:
- Around line 199-216: The table-building code in render_findings_table leaves s
(severity), r (rule), file and line unescaped which can break the Markdown
table; wrap each of those values with sanitize_cell before formatting (i.e. pass
s, r, file and the computed line string through sanitize_cell) and ensure the
line value is produced as a deterministic string (use the existing
map/unwrap_or_else flow but sanitize its result) so all four fields are escaped
consistently alongside issue and suggestion.
---
Nitpick comments:
In `@src/cli-push-runner/src/config.rs`:
- Around line 113-227: Add a new unit test (similar to config_parses_with_diff)
named e.g. config_parses_lint_screen that deserializes a TOML string containing
a full [lint_screen] section and asserts every field on Config.lint_screen (or
the LintScreen type) is Some/equals the expected values; locate the Config
struct and existing tests (e.g. config_parses_with_diff,
config_parses_full_without_diff) to mirror style, ensure you check optional
fields unwrap_or defaults where applicable and assert that no field silently
remains None due to a naming mismatch.
🪄 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: c4d07d95-a1ce-45dd-92db-be62e6706fff
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
.takt/facets/instructions/review-simplicity.mdpush-runner-config.tomlsrc/cli-push-runner/Cargo.tomlsrc/cli-push-runner/src/config.rssrc/cli-push-runner/src/main.rssrc/cli-push-runner/src/stages/lint_screen.rssrc/cli-push-runner/src/stages/mod.rs
…lt OFF, gating なし、fallback graceful)
aloekun
force-pushed
the
feature/local-llm-phase-c-mvp
branch
from
73903d7 to
8416d3e
Compare
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 c MVP —
cli-push-runnerに lint-screen step を追加し、pre-push 時に diff を mistral:7b に流して lint 一次フィルタの所見 (.takt/lint-screen-report.md) を出力する経路を整備。Phase b' conditional GO (75%) のため gating せず、report 出力のみ (default OFF / 試験運用 opt-in)。アーキテクチャ
cli-push-runner(Rust) の新 stagestages/lint_screen.rs[lint_screen] enabled = true時のみ (default OFF)cli-finding-classifier.exe --mode lint-screen(stdin に diff、stdout に LintScreenResult JSON).takt/lint-screen-report.md(markdown table)review-simplicity.mdinstruction が同 path を optional context として読む (advisory 扱い)fallback 設計 (push を block しない)
human_review + fallback_reason、report に出力smoke dogfood 結果
868 行の現実 PR diff (Phase c 自身) を流したところ:
= mistral:7b は大規模 diff で JSON 構造を欠落させがちと判明。fallback path が graceful に処理。Phase d (PR-based dogfood) で頻度・対策方針を観測。
副次対応
step_timeout = 120 → 180(Phase b' end-to-end test の cargo test --ignored が境界 117s で timeout していた、commit73903d7で実証)Test plan
.takt/lint-screen-report.mdを context として認識するか (Phase d で観測)Out of scope (PR-B 行き)
Summary by CodeRabbit
リリースノート
新機能
ドキュメント
Chores