|
1 | | -Run a comprehensive code review using CKB's deterministic analysis + your semantic review. |
| 1 | +Run a CKB-augmented code review optimized for minimal token usage. |
2 | 2 |
|
3 | 3 | ## Input |
4 | 4 | $ARGUMENTS - Optional: base branch (default: main), or "staged" for staged changes, or a PR number |
5 | 5 |
|
6 | | -## MCP vs CLI |
| 6 | +## Philosophy |
7 | 7 |
|
8 | | -CKB runs as an MCP server in this environment. MCP mode is strongly preferred for interactive review because the SCIP index stays loaded between calls — drill-down tools like `findReferences`, `analyzeImpact`, and `explainSymbol` execute instantly against the in-memory index. CLI mode reloads the index on every invocation. |
| 8 | +CKB already answered the structural questions (secrets? breaking? dead code? test gaps?). |
| 9 | +The LLM's job is ONLY what CKB can't do: semantic reasoning about correctness, design, |
| 10 | +and intent. Every source line you read costs tokens — read only what CKB says is risky. |
9 | 11 |
|
10 | | -## The Three Phases |
| 12 | +## Phase 1: Structural scan (~1k tokens into context) |
11 | 13 |
|
12 | | -### Phase 1: CKB structural scan (5 seconds, 0 tokens) |
13 | | - |
14 | | -Call the `reviewPR` MCP tool with compact mode: |
15 | | -``` |
16 | | -reviewPR(baseBranch: "main", compact: true) |
| 14 | +```bash |
| 15 | +ckb review --base=main --format=json --compact 2>/dev/null |
17 | 16 | ``` |
18 | 17 |
|
19 | | -This returns ~1k tokens instead of ~30k — just the verdict, non-pass checks, top 10 findings, and action items. Use `compact: false` only if you need the full raw data. |
20 | | - |
21 | | -If a PR number was given, get the base branch first: |
| 18 | +If a PR number was given: |
22 | 19 | ```bash |
23 | 20 | BASE=$(gh pr view $ARGUMENTS --json baseRefName -q .baseRefName) |
| 21 | +ckb review --base=$BASE --format=json --compact 2>/dev/null |
24 | 22 | ``` |
25 | | -Then pass it: `reviewPR(baseBranch: BASE, compact: true)` |
26 | 23 |
|
27 | | -> **If CKB is not running as an MCP server** (last resort), use the CLI instead: |
28 | | -> ```bash |
29 | | -> ./ckb review --base=main --format=json |
30 | | -> ``` |
31 | | -> Note: CLI mode reloads the SCIP index on every call, so drill-down steps will be slower. |
| 24 | +From the output, build three lists: |
| 25 | +- **SKIP**: passed checks — don't touch these files or topics |
| 26 | +- **INVESTIGATE**: warned/failed checks — these are your review scope |
| 27 | +- **READ**: hotspot files + files with warn/fail findings — the only files you'll read |
32 | 28 |
|
33 | | -From CKB's output, immediately note: |
34 | | -- **Passed checks** → skip these categories. Don't waste tokens re-checking secrets, breaking changes, test coverage, etc. |
35 | | -- **Warned checks** → your review targets |
36 | | -- **Top hotspot files** → read these first |
37 | | -- **Test gaps** → functions to evaluate |
| 29 | +**Early exit**: If verdict=pass and score≥80, write a one-line approval and stop. No source reading needed. |
38 | 30 |
|
39 | | -### Phase 2: Drill down on CKB findings (0 tokens via MCP) |
| 31 | +## Phase 2: Targeted source reading (the only token-expensive step) |
40 | 32 |
|
41 | | -Before reading source code, use CKB's MCP tools to investigate specific findings. These calls are instant because the SCIP index is already loaded from Phase 1. |
| 33 | +Do NOT read the full diff. Do NOT read every changed file. |
42 | 34 |
|
43 | | -| CKB finding | Drill-down tool | What to check | |
44 | | -|---|---|---| |
45 | | -| Dead code | `findReferences(symbolId: "...")` or `searchSymbols` → `findReferences` | Does it actually have references? CKB's SCIP index can miss cross-package refs | |
46 | | -| Blast radius | `analyzeImpact(symbolId: "...")` | Are the "callers" real logic or just framework registrations? | |
47 | | -| Coupling gap | `explainSymbol(name: "...")` on the missing file | What does the co-change partner do? Does it actually need updates? | |
48 | | -| Bug patterns | Already verified by differential analysis | Just check the specific line CKB flagged | |
49 | | -| Complexity | `explainFile(path: "...")` | What functions are driving the increase? | |
50 | | -| Test gaps | `getAffectedTests(baseBranch: "main")` | Which tests exist? Which functions are actually untested? | |
51 | | -| Hotspots | `getHotspots(limit: 10)` | Full churn history for the flagged files | |
| 35 | +Read ONLY: |
| 36 | +1. Files that appear in INVESTIGATE findings (just the changed hunks via `git diff main...HEAD -- <file>`) |
| 37 | +2. New files (CKB has no history for these) — but only if <500 lines each |
| 38 | +3. Skip generated files, test files for existing tests, and config/CI files |
52 | 39 |
|
53 | | -### Phase 3: Semantic review of high-risk files |
| 40 | +For each file you read, look for exactly: |
| 41 | +- Logic errors (wrong condition, off-by-one, nil deref) |
| 42 | +- Security issues (injection, auth bypass, secrets) |
| 43 | +- Design problems (wrong abstraction, leaky interface) |
| 44 | +- Missing edge cases the tests don't cover |
54 | 45 |
|
55 | | -Now read the actual source — but only for: |
56 | | -1. Files CKB ranked as top hotspots |
57 | | -2. Files with warned findings that survived drill-down |
58 | | -3. New files (CKB can't assess design quality of new code) |
| 46 | +Do NOT look for: style, naming, formatting, documentation, test coverage — |
| 47 | +CKB already checked these structurally. |
59 | 48 |
|
60 | | -For each file, look for things CKB CANNOT detect: |
61 | | -- Logic bugs (wrong conditions, off-by-one, race conditions) |
62 | | -- Security issues (injection, auth bypass, data exposure) |
63 | | -- Design problems (wrong abstraction, unclear naming, leaky interfaces) |
64 | | -- Edge cases (nil inputs, empty collections, concurrent access) |
65 | | -- Error handling quality (not just missing — wrong strategy) |
66 | | -
|
67 | | -### Phase 4: Write the review |
68 | | -
|
69 | | -Format: |
| 49 | +## Phase 3: Write the review (be terse) |
70 | 50 |
|
71 | 51 | ```markdown |
72 | | -## Summary |
73 | | -One paragraph: what the PR does, overall assessment. |
| 52 | +## [APPROVE|REQUEST CHANGES|DISCUSS] — CKB score: [N]/100 |
74 | 53 |
|
75 | | -## Must Fix |
76 | | -Findings that should block merge. File:line references. |
| 54 | +[One sentence: what the PR does] |
77 | 55 |
|
78 | | -## Should Fix |
79 | | -Issues worth addressing but not blocking. |
| 56 | +### Issues |
| 57 | +1. **[must-fix|should-fix]** `file:line` — [issue in one sentence] |
| 58 | +2. ... |
80 | 59 |
|
81 | | -## CKB Analysis |
82 | | -- Verdict: [pass/warn/fail], Score: [0-100] |
83 | | -- [N] checks passed, [N] warned |
84 | | -- Key findings: [top 3] |
85 | | -- False positives identified: [any CKB findings you disproved] |
86 | | -- Test gaps: [N] untested functions — [your assessment of which matter] |
| 60 | +### CKB passed (no review needed) |
| 61 | +[comma-separated list of passed checks] |
87 | 62 |
|
88 | | -## Recommendation |
89 | | -Approve / Request changes / Needs discussion |
| 63 | +### CKB flagged (verified above) |
| 64 | +[for each warn/fail finding: confirmed/false-positive + one-line reason] |
90 | 65 | ``` |
91 | 66 |
|
92 | | -## Tips |
| 67 | +If no issues found: just the header line + CKB passed list. Nothing else. |
| 68 | + |
| 69 | +## Anti-patterns (token waste) |
93 | 70 |
|
94 | | -- If CKB says "secrets: pass" — trust it, don't re-scan 100+ files |
95 | | -- If CKB says "breaking: pass" — trust it, SCIP-verified API comparison |
96 | | -- If CKB says "dead-code: FormatSARIF" — DON'T trust blindly, verify with `findReferences` or grep |
97 | | -- CKB's hotspot scores are based on git churn history — higher score = more volatile file = review more carefully |
98 | | -- CKB's complexity delta shows WHERE cognitive load increased — read those functions |
| 71 | +- Reading files CKB marked as pass → waste |
| 72 | +- Reading generated files → waste |
| 73 | +- Summarizing what the PR does in detail → waste (git log exists) |
| 74 | +- Explaining why passed checks passed → waste |
| 75 | +- Running MCP drill-down tools when CLI already gave enough signal → waste |
| 76 | +- Reading test files to "verify test quality" → waste unless CKB flagged test-gaps |
| 77 | +- Reading hotspot-only files with no findings → high churn ≠ needs review right now |
0 commit comments