Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 33 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,40 @@

Config is not code, and reviewing it as though it were produced confident
nonsense. Reported from the field: every file under `.claude/` came back HIGH.
Rule sets had the mirror-image problem — they could stop applying without ever
saying so.

### Fixed
### Fixed — rule sets

- **A rule set dropped for prompt budget is now reported**, with the budget and
the reason. Previously it vanished silently — the failure this feature was
otherwise built to avoid.
- **The lowest severity is dropped first.** Rule sets load in filename order, so
overflow used to shed whichever sorted last: a `high` security set lost to a
style guide on the letter `s`. Severity is the only ranking the author
declared, so it decides. Surviving sets still *render* in filename order, so
the cache key does not move.

### Added

- **`!` exclusion globs in `scope`** — `["src/**", "!src/legacy/**"]`, the way
CodeRabbit's `path_filters` reads. An exclusion beats every include.
- **`always: true`** to apply a rule set to every file regardless of `scope`.
An empty `scope` already meant this; now it can be said out loud, and
`rules list` marks it.
- **`description:`** — one line for humans, shown by `rules list`, never sent to
the model.
- **`globs:` and `alwaysApply:` accepted as aliases**, so a rule set ported from
`.cursor/rules/` loads unedited.
- **`diffmind rules check`** — validates every rule set and pattern rule and
exits non-zero, for CI. Catches parse errors, duplicate ids, identical bodies,
scopes that match nothing, and invalid regex in `rules.toml`.
- **A `scope` of only `!` exclusions is now a parse error.** It matched no file
at all, which is a rule set that silently does no work.
- **`examples/nextjs-app-router/`** — a complete, working rule set for a
React/Next.js project: three scoped `.md` rule sets and 13 regex rules.

### Fixed — what gets reviewed

- **Coding-agent and editor config is no longer reviewed.** `.claude/`,
`.cursor/`, `.windsurf/`, `.aider/`, `.vscode/`, `.idea/`, `.zed/` and
Expand Down
89 changes: 83 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,42 @@ Your repository's own `.gitignore` is never touched.

## Your team's standards

Everything lives in `.diffmind/`, committed to your repo:

```
.diffmind/
├── config.toml # settings — model, base branch, gate thresholds
├── rules.toml # regex rules: free, deterministic, never vary
└── rules/*.md # written rules: judgement, read by the model
```

### Set it up

```bash
diffmind rules init # writes a starter .diffmind/rules/default.md
# edit it, then:
diffmind rules list # shows what loads, its severity ceiling and its globs
diffmind # review with them applied
```

**A complete worked example lives in
[`examples/nextjs-app-router/`](examples/nextjs-app-router/)** — three scoped
rule sets plus 13 regex rules for a real React/Next.js project. Copy the folder
and edit.

### Which file does a rule go in?

**If a regex can decide it, it goes in `rules.toml`.** It costs nothing, never
changes between runs, and is safe to block a build on. Only judgement goes in a
`.md`.

| | `rules.toml` | `rules/*.md` |
| --- | --- | --- |
| Read by | regex, before the model runs | the model, in the prompt |
| Cost | free | tokens, every review |
| Same answer every run | always | depends on the model |
| Good for | `next/router` in `app/`, `@ts-ignore`, hardcoded secrets | "is this abstraction earning its keep" |

### Written rules — `.diffmind/rules/*.md`

For rules that need judgement instead of a pattern. They live in the repo and
Expand All @@ -405,7 +441,8 @@ people carry.

```markdown
---
scope: ["src/api/**/*.ts"]
description: Conventions for the public API layer
scope: ["src/api/**/*.ts", "!src/api/legacy/**"]
severity: high
---

Expand All @@ -416,11 +453,25 @@ severity: high
- Reject changes that widen a response struct without a version bump.
```

`scope` is a glob for the files the rule set applies to (leave it out for the
whole repo). `severity` is a **maximum** for findings from that rule set — it can
lower a finding's severity but never raise it. `id` defaults to the filename.
Create a starter file with `diffmind rules init`, and see what loads with
`diffmind rules list`.
| Field | Meaning |
| --- | --- |
| `id` | Suppression handle, `rulebook.<id>`. Defaults to the filename. |
| `description` | One line, for humans. Shown by `rules list`; never sent to the model. |
| `scope` | Globs this governs. Omit for the whole repo. A `!` prefix excludes, and beats every include. |
| `always` | Apply to every file regardless of `scope`. Paid for on every review — make it deliberate. |
| `severity` | A **maximum** for findings from this set. Can lower a finding's severity, never raise it. Also the drop order when the budget is tight. |

`globs:` and `alwaysApply:` are accepted as aliases, so a rule ported from
`.cursor/rules/` loads without an edit.

Create a starter file with `diffmind rules init`, see what loads with
`diffmind rules list`, and validate the lot with `diffmind rules check` —
which exits non-zero, so it belongs in CI:

```bash
diffmind rules check # parse errors, duplicate ids, identical bodies,
# dead globs, invalid regex in rules.toml
```

A finding from a rule set gets the ID `rulebook.<id>` and can be silenced like
any other. If the model credits a rule set that does not apply to that file, the
Expand All @@ -432,6 +483,32 @@ which rule sets apply, so every review in a group starts with an identical
prefix. That is what makes prompt-prefix caching possible later. A rule set that
fails to parse is reported and skipped, never ignored quietly.

#### Scope them, or they get dropped

Every rule set matching a file is pasted into that file's prompt, and the prompt
shares a byte budget with the diff and the symbol context — the diff always keeps
at least half. **When the rule sets do not all fit, whole ones are dropped**
(never truncated: half a rule reads as a complete rule that says something else).

Two things make that safe rather than mysterious:

- **Lowest `severity` goes first**, so a `high` security set outlives a style
guide. Unranked sets go before ranked ones.
- **You are told.** The run prints which sets did not fit and why.

Still, the real lever is `scope`, not brevity. Scoped well, only two or three
sets ever apply at once:

```
react.md scope: ["**/*.tsx"]
nextjs-app-router.md scope: ["app/**", "src/app/**"]
server-actions-security.md scope: ["**/actions.ts", "**/route.ts"]
```

Two rules of thumb: keep each set to a page or two — attention dilutes, and
twenty pages of rules makes the model worse at each one — and run
`diffmind rules list` after editing to confirm the globs are what you meant.

### Pattern rules — `.diffmind/rules.toml`

Regex, checked against added lines before the model runs: instant, always the
Expand Down
2 changes: 2 additions & 0 deletions apps/tui-cli/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,8 @@ pub enum RulesAction {
Init,
/// List the rule sets that would be loaded, and what they govern
List,
/// Validate every rule set and pattern rule. Exit 1 on a problem — for CI.
Check,
}

#[derive(Subcommand, Debug)]
Expand Down
2 changes: 2 additions & 0 deletions apps/tui-cli/src/daemon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -667,6 +667,8 @@ mod tests {
rules: vec![],
rulebooks: vec![core_engine::Rulebook {
id: "api".into(),
description: None,
always: false,
scope: vec!["src/api/**".into()],
severity: Some(core_engine::Severity::High),
body: "- Handlers return ApiError.".into(),
Expand Down
43 changes: 42 additions & 1 deletion apps/tui-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,16 +297,22 @@ fn run_command(
return Ok(0);
}
for b in &books {
let scope = if b.scope.is_empty() {
let scope = if b.always {
"always (every file)".to_string()
} else if b.scope.is_empty() {
"whole repository".to_string()
} else {
b.scope.join(", ")
};
let severity = b.severity.map(|s| s.as_str()).unwrap_or("unset");
println!(" {:<24} {severity:<7} {scope}", b.id);
if let Some(d) = &b.description {
println!(" {:<24} {:<7} {d}", "", "");
}
}
Ok(0)
}
cli::RulesAction::Check => rules::check(project_root),
},

cli::Commands::Cache { action } => {
Expand Down Expand Up @@ -573,6 +579,39 @@ impl ProjectRules {
}
}

/// Say so when the prompt budget cannot fit every rule set.
///
/// The alternative is what shipped before: a rule set stops applying and
/// nothing anywhere says it did. The check is deliberately at the *widest*
/// budget — depth 0, before any retry shrinks it — so this warns about the
/// rule sets that can never apply, not about a transient squeeze on one
/// oversized unit.
fn warn_on_dropped_rulebooks(backend: &dyn ReviewBackend, settings: &Settings, books: &[Rulebook]) {
if books.is_empty() {
return;
}
let budget = core_engine::section_budget_bytes(
backend.context_tokens(),
settings.max_tokens as usize,
0,
);
let dropped = core_engine::rulebooks_dropped(books, budget);
if dropped.is_empty() {
return;
}

eprintln!(
" ! {} rule set(s) do not fit the prompt budget ({budget} bytes) and were \
dropped: {}",
dropped.len(),
dropped.join(", ")
);
eprintln!(
" Lowest severity is dropped first. Narrow their `scope`, shorten them, \
or use a backend with a larger context window."
);
}

pub fn build_analyzer(
backend: Box<dyn ReviewBackend>,
settings: &Settings,
Expand All @@ -581,6 +620,8 @@ pub fn build_analyzer(
ticket: Option<String>,
project: ProjectRules,
) -> ReviewAnalyzer {
warn_on_dropped_rulebooks(&*backend, settings, &project.books);

let mut analyzer = ReviewAnalyzer::new(backend)
.with_unit_grouper(unit_grouper(project_root))
.with_languages(detect_languages(diff))
Expand Down
110 changes: 110 additions & 0 deletions apps/tui-cli/src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,116 @@ fn warn_on_duplicate_ids(rules: &[CustomRule]) {
}
}

/// Validate `.diffmind/rules/` and `.diffmind/rules.toml`. Returns the exit code.
///
/// Exists because every other check in this module warns on stderr during a
/// review, where it scrolls past behind the findings. A rule set that stopped
/// working deserves to fail CI, not to be mentioned in passing.
pub fn check(project_root: &Path) -> anyhow::Result<i32> {
let mut problems = 0usize;

let dir = project_root.join(".diffmind").join("rules");
// Name files, not ids: when two rule sets collide the ids are identical, so
// an id-based message tells the reader nothing about which file to edit.
let mut parsed: Vec<(std::path::PathBuf, Rulebook)> = Vec::new();

if dir.is_dir() {
let mut entries: Vec<std::path::PathBuf> = walkdir::WalkDir::new(&dir)
.into_iter()
.filter_map(|e| e.ok())
.filter(|e| e.file_type().is_file())
.map(|e| e.path().to_path_buf())
.filter(|p| p.extension().is_some_and(|x| x == "md"))
.collect();
entries.sort();

for path in entries {
let path = path.as_path();
let stem = path
.file_stem()
.map(|s| s.to_string_lossy().to_string())
.unwrap_or_default();
let text = match std::fs::read_to_string(path) {
Ok(t) => t,
Err(e) => {
println!(" ✗ {}: {e}", path.display());
problems += 1;
continue;
}
};
match core_engine::rulebook::parse(&stem, &text) {
Ok(book) => {
// A glob nobody can satisfy is the silent-no-op this whole
// command exists to surface.
for glob in &book.scope {
if glob.trim().is_empty() {
println!(" ✗ {}: empty glob in `scope`", path.display());
problems += 1;
}
}
println!(" ✓ {:<28} {}", book.id, path.display());
parsed.push((path.to_path_buf(), book));
}
Err(e) => {
println!(" ✗ {}: {e}", path.display());
problems += 1;
}
}
}
}

// Two rule sets saying the same thing cost tokens twice and give the model
// two chances to report the same finding.
for i in 0..parsed.len() {
for j in (i + 1)..parsed.len() {
if parsed[i].1.body == parsed[j].1.body {
println!(
" ✗ {} and {} have identical bodies — delete one.",
parsed[i].0.display(),
parsed[j].0.display()
);
problems += 1;
}
}
}

// Counted from what was parsed above rather than by calling
// `load_rulebooks`, which would re-parse every file and print its own copy
// of the warnings this command has already reported.
let mut ids: std::collections::HashMap<&str, Vec<&std::path::Path>> = Default::default();
for (path, book) in &parsed {
ids.entry(book.id.as_str()).or_default().push(path);
}
let mut collisions: Vec<_> = ids.iter().filter(|(_, v)| v.len() > 1).collect();
collisions.sort_by_key(|(id, _)| *id);
for (id, paths) in collisions {
let names: Vec<String> = paths.iter().map(|p| p.display().to_string()).collect();
println!(
" ✗ {} rule sets share the id '{id}' ({}) — set an explicit `id`.",
paths.len(),
names.join(", ")
);
problems += 1;
}

// Pattern rules: an invalid regex is skipped at review time with a warning
// nobody reads. Here it is a failure.
for rule in load_custom_rules(project_root) {
if let Err(e) = regex::Regex::new(&rule.pattern) {
println!(" ✗ rule '{}': invalid regex — {e}", rule.effective_id());
problems += 1;
}
}

if problems == 0 {
println!("\n {} rule set(s), no problems.", parsed.len());
Ok(0)
} else {
println!("\n {problems} problem(s).");
Ok(1)
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
10 changes: 10 additions & 0 deletions examples/nextjs-app-router/.diffmind/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# Written by diffmind. Generated state — not worth committing.
# Deliberately absent: rules/, rules.toml, config.toml, baseline.json.
cache/
runs/
models/
graph.db
graph.db-wal
graph.db-shm
symbols.json
daemon.json
Loading
Loading