feat(pre-push-review): WP-07 findings handoff を markdown output-contract で標準化 — simplicity-review 契約新設・reviewer 間で列統一 (ADR-048、JSON 却下) - #252
Merged
Conversation
…act で標準化 — simplicity-review 契約新設・reviewer 間で列統一 (ADR-048、JSON 却下)
📝 WalkthroughWalkthroughreviewers から fix への findings handoff を markdown ベースの output-contract に統一する変更です。simplicity-review.md の新規output-contractを作成し、refutation-report.md に family_tag 列を追加、refute-finding.md instructionを更新、ADR-048 を新設して意思決定を記録し、CLAUDE.md と harness-improvement-plan.md を更新しました。 ChangesOutput-contract 統一とADR記録
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem うさぎ跳ねてmarkdown書く 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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)
docs/harness-improvement-plan.md (1)
69-69: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWP-07 のタイトルを実装結果に合わせて更新してください。
JSON 化という表現が overview row と WP-07 セクションに残っていますが、実装済みの内容は JSON ではなく markdown output-contract の標準化です。タイトルが古いと、後から読む人が「JSON handoff が入った」と誤解します。提案
-| WP-07 | 1-B | facet 間受け渡しの JSON 化 | M | なし | 実装済（...） | +| WP-07 | 1-B | facet 間受け渡しの output-contract 標準化 | M | なし | 実装済（...） |WP-07 セクション見出し側も同じ表現に揃えてください。
Also applies to: 163-170
🤖 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 `@docs/harness-improvement-plan.md` at line 69, Update the WP-07 title so it matches the implemented work: replace the outdated “JSON 化” wording with the markdown output-contract standardization reflected in ADR-048. Make the same terminology change in both the overview row and the WP-07 section heading so the labels stay consistent and don’t imply a JSON handoff; use the existing WP-07 and ADR-048 references to locate the affected text.
🤖 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 @.takt/facets/output-contracts/simplicity-review.md:
- Around line 19-38: The finding tables use inconsistent schemas across Current
Iteration, Carry-over, and Reopened, which breaks consumers that expect one
canonical set of columns. Update the markdown in simplicity-review.md so every
finding table uses the same ADR-048 column names and ordering, and adjust the
Carry-over/Reopened sections to match the Current Iteration table rather than
swapping in Previous/Current Evidence columns. Keep the schema uniform so
fix/refute logic can parse all finding tables with the same contract.
---
Nitpick comments:
In `@docs/harness-improvement-plan.md`:
- Line 69: Update the WP-07 title so it matches the implemented work: replace
the outdated “JSON 化” wording with the markdown output-contract standardization
reflected in ADR-048. Make the same terminology change in both the overview row
and the WP-07 section heading so the labels stay consistent and don’t imply a
JSON handoff; use the existing WP-07 and ADR-048 references to locate the
affected text.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ea07bfb1-657f-496f-b3ca-90825132691a
📒 Files selected for processing (6)
.takt/facets/instructions/refute-finding.md.takt/facets/output-contracts/refutation-report.md.takt/facets/output-contracts/simplicity-review.mdCLAUDE.mddocs/adr/adr-048-facet-findings-handoff-markdown-contract.mddocs/harness-improvement-plan.md
Comment on lines
+19
to
+38
| ## Current Iteration Findings (new) | ||
| | # | finding_id | family_tag | Severity | Type | Location | Issue | Fix Suggestion | | ||
| |---|------------|------------|----------|------|----------|-------|----------------| | ||
| | 1 | SIM-NEW-src-x-L10 | dead-code | Medium | dead-on-arrival | `src/x.ts:10` | Helper with no caller | Remove or wire up | | ||
|
|
||
| ## Carry-over Findings (persists) | ||
| | # | finding_id | family_tag | Previous Evidence | Current Evidence | Issue | Fix Suggestion | | ||
| |---|------------|------------|-------------------|------------------|-------|----------------| | ||
| | 1 | SIM-PERSIST-src-y-L30 | deep-nesting | `src/y.ts:30` | `src/y.ts:30` | Nesting persists | Flatten with guard clause | | ||
|
|
||
| ## Resolved Findings (resolved) | ||
| | finding_id | Resolution Evidence | | ||
| |------------|---------------------| | ||
| | SIM-RESOLVED-src-x-L10 | `src/x.ts:10` helper removed | | ||
|
|
||
| ## Reopened Findings (reopened) | ||
| | # | finding_id | family_tag | Prior Resolution Evidence | Recurrence Evidence | Issue | Fix Suggestion | | ||
| |---|------------|------------|--------------------------|---------------------|-------|----------------| | ||
| | 1 | SIM-REOPENED-src-y-L55 | deep-nesting | `Previously flattened at src/y.ts:30` | `Recurred at src/y.ts:55` | Nesting reintroduced | Flatten again | | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
列スキーマを全 finding table で統一してください。
Current Iteration だけが Severity / Type / Location を持ち、Carry-over / Reopened は Previous Evidence / Current Evidence に差し替わっています。さらに見出しの大小文字も ADR-048 の canonical 列名とズレています。fix / refute が表ごとに別スキーマを扱わずに済むよう、全 table を同じ列セットに揃えてください。
提案
-| # | finding_id | family_tag | Severity | Type | Location | Issue | Fix Suggestion |
+| # | finding_id | family_tag | severity | type | location | issue | fix suggestion |同じ列セットを Carry-over / Reopened にも適用してください。
🤖 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 @.takt/facets/output-contracts/simplicity-review.md around lines 19 - 38, The
finding tables use inconsistent schemas across Current Iteration, Carry-over,
and Reopened, which breaks consumers that expect one canonical set of columns.
Update the markdown in simplicity-review.md so every finding table uses the same
ADR-048 column names and ordering, and adjust the Carry-over/Reopened sections
to match the Current Iteration table rather than swapping in Previous/Current
Evidence columns. Keep the schema uniform so fix/refute logic can parse all
finding tables with the same contract.
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.
概要
ハーネス改善実行計画書 (
docs/harness-improvement-plan.md) の WP-07: facet 間受け渡しの JSON 化 を実装する。目的は pre-push review の reviewers → fix 間の findings 受け渡しでの parse 事故・読み落とし防止。当初計画 (findings の JSON 化 + Rust 検証 pre-step) を takt 公式仕様の調査に基づき却下し、takt idiomatic な markdown output-contract の標準化に方針転換した。判断過程は ADR-048 に記録。
公式調査の結論 (takt 0.35.3、ADR-017 で pin)
output_contracts.report[].format→reports/→{report:filename}参照)。OUTPUT_CONTRACT_STYLE_GUIDE.mdは```markdownブロックを必須とし、プレーンテキスト/非 markdown 契約を DON'T と明記。type/schema/jsonは無く、formatは検証されない markdown テンプレート。29 builtin 契約・36 workflow・251 report 宣言すべて markdown、JSON はゼロ。parse 事故の真因
「markdown だから」ではなく
simplicity-reviewに output-contract が存在せず、reviewer 間で finding テーブルの列が不統一だったこと。format: simplicity-reviewはファイル解決されずリテラル文字列に degrade し、simplicity reviewer は構造強制ゼロで free-form 出力していた。変更内容
.takt/facets/output-contracts/simplicity-review.md（新規契約）: builtinsecurity-reviewの finding テーブル構造を踏襲。simplicity reviewer に構造を強制し真因を解消。finding_id不変性の注記も追加（本 PR の dogfood レビューで surface した改善）。finding_id / family_tag / severity / type / location (file:line) / issue / fix suggestion。security は builtin 契約（同一列）を使用、simplicity が同構造を mirror。refutation-report.md/refute-finding.md: Survived Findings テーブルにfamily_tag追加、carry-over 指示も整合。format:名にそのまま解決されるため YAML 変更ゼロ。pre-push-review.yamlと refute variant の両方が自動的に裨益。post-pr-review は simplicity/security reviewer を持たず対象外。却下・見送りした代替案 (ADR-048 参照)
検証 (dogfood)
Result: APPROVE/Anomaly Scanテーブル）で出力することを実地確認。関連
Summary by CodeRabbit