enforce-no-pre-existing-issue
Don't deflect with 'pre-existing issue'. Either fix it as part of this change, defer with a clear rationale, or drop the disclaimer.
new_content_contains
A reader's guide to the rules that ship by default in
phr-mcp init --packs rust — the language-agnostic
platform packs are included automatically, and the
context pack installs scaffolding rather than rules.
Each entry carries its predicate, its severity, a one-line
distillation, and a worked example drawn from real-world
Rust code. The confidence pack adds two gate rules
that count grounded build/test/known-bug signals; the
journey pack ships taggers and aggregator-fact
references that let projects write their own trajectory
rules. The structural pack reasons over a
code graph built at the hook boundary, flagging untested
panicking paths and import cycles in the file you are editing.
Each rule is presented as a labelled specimen. The circular
glyph encodes the severity at a glance: a red ✕
is a block (pre-phase, exit 2, the host refuses the action);
an amber ! is a warning (post-phase, exit 1,
the action applies but the operator is told); a slate
○ is audit-only (silent at hook time, surfaced
only by phr-mcp audit).
Tags beneath each rule name show its phase and whether it participates in audits. The italic line is the rule's one-sentence point. From the field blocks show synthetic but plausible Rust code that would trip the rule, paired with a refactor that satisfies it.
Examples are illustrative, not extracted verbatim. They are written to clarify the rule, not to document any one project's actual state.
documents the default packs as of v0.26.0
Don't deflect with 'pre-existing issue'. Either fix it as part of this change, defer with a clear rationale, or drop the disclaimer.
new_content_containsDrop the 'not from our changes' disclaimer. Name the issue and decide: fix or defer.
new_content_containsDrop the 'not caused by our' disclaimer. Own the fix or own the decision to defer.
new_content_containsAvoid claiming a fix is complete without evidence. Run the verification (test, manual exercise, traced call chain) before reporting, or explicitly label the work 'untested' so the human knows to check.
new_content_containsDon't make repair claims without verifying. Run the failing case end-to-end before reporting a fix; otherwise mark it 'untested' so the user knows to verify.
new_content_containsAbout to commit. Trace the call chain end-to-end before reporting done. Half-fixes where one layer is wired but another is not are a recurring failure mode.
new_content_contains + __script__Stage files explicitly — git add -A / git add . sweeps unrelated changes into the commit. List the files you actually changed.
bash_command_matchesBuilds are I/O-bound: a rustc at 0% CPU is usually in disk-wait, not hung. Give it time or check `ps` state before killing the build.
bash_command_matchesAvoid .unwrap() in src/ — use ? for error propagation, or expect() with a clear message if truly unreachable.
new_content_contains + file_path_matchesDon't ship todo!() in src/ — finish the implementation or split it into a tracked task.
new_content_contains + file_path_matchesAvoid panic!() in src/ — return a Result and let the caller decide.
new_content_contains + file_path_matchesAvoid unimplemented!() in src/ — implement the path or remove it.
new_content_contains + file_path_matches`?fn` in ?file returns Result<_, String>. Define a proper error enum with thiserror.
function_returns_result_stringdbg!() in src/ — remove before committing, or use tracing::debug!() for diagnostics that stay.
new_content_contains + file_path_matchesPublic `?fn` takes `?param: &String` — prefer `&str` for ergonomics and to avoid forcing callers to own a String.
function_is_public + function_param_typeFunction `?fn` in ?file has ?count parameters. Consider grouping related params into a struct (builder/options pattern) or splitting the function — long signatures correlate with God-function debt.
function_param_count_highFile exceeds 800 lines — consider splitting into focused submodules. Long files correlate with God-object debt and slow down navigation. (Scoped to src/; test blocks excluded from the count; a top-of-file `//! phronesis-allow: audit-file-loc-high <reason>` doc-comment exempts intentional god-files.)
file_extension_is + file_path_matches + file_line_count_abovePublic `?fn` takes `?param: &Vec<T>` — prefer `&[T]` for ergonomics; a slice accepts arrays, slices, and Vecs alike. From the patterns guide §API Design 1.
function_is_public + function_param_is_vec_refRunning `?cmd` without `--workspace` only checks part of the workspace. Use `cargo <subcommand> --workspace --tests --examples` to catch sibling-crate breakage, or pass `-p <crate>` if scope was intentional.
cargo_command_lacks_workspace`execute_all_agenda_items()` is sync as of the 039 refactor — drop the `.await`. Cargo will reject the call site as `Result<Vec<Action>, String> is not a future`.
new_content_contains + file_extension_is`fire_all_consequences()` is sync — drop the `.await`. Cargo will reject as `Result<Vec<Consequence>, ReteError> is not a future`.
new_content_contains + file_extension_is`?fn` in ?file calls .clone() ?count times — review whether references or borrowed slices would work.
function_clone_count_highTest `?fn` in ?file has no assertions or `?` propagation — a placeholder test that always passes hides regressions.
test_without_assertion`impl Deref for` — Deref polymorphism is an anti-pattern for non-pointer types. Reserve Deref for smart-pointer wrappers (Box/Arc/Rc); for other types, prefer explicit delegation methods so the API surface is intentional.
new_content_contains + file_extension_isManual `=> return Err(...)` in a match arm — the `?` operator usually replaces this whole shape. Surface during one-time audit sweeps; deliberately silent at hook time so in-progress refactors aren't blocked.
new_content_contains + file_extension_isField named `*_id: String` — consider a newtype like `StateId(String)` for type safety so one ID kind can't be passed where another is expected. From the patterns guide §Design Patterns 2 (Newtype Pattern). (Scoped to src/ — test fixtures are exempt. A `///` doc-comment immediately above the field marks an intentional string ID, e.g. one crossing a JSON registry boundary, as an accepted exception.)
new_content_contains + file_extension_is + file_path_matchesField named `*_id: u64` — consider a newtype like `UserId(u64)` to prevent mixing different ID types. From the patterns guide §Design Patterns 2 (Newtype Pattern). (Scoped to src/ — test fixtures are exempt.)
new_content_contains + file_extension_is + file_path_matches`match` with a `None => {}` arm — `if let Some(x) = ...` is usually clearer. From the patterns guide §Idioms 2.
new_content_contains + file_extension_is`match` arm `Err(_) => {}` silently swallows errors. Either handle the error (log/return) or use `if let Ok(x) = ...` to make the intent explicit.
new_content_contains + file_extension_is`#![deny(warnings)]` breaks builds on toolchain upgrades, since each rustc release introduces new warnings. Move the policy to CI with `RUSTFLAGS="-D warnings"` instead. From the patterns guide §Anti-patterns (deny-warnings).
new_content_contains + file_extension_isParameter type `&Box<T>` adds a useless layer of indirection — prefer `&T` directly. From the patterns guide §Idioms (borrowed-types-for-arguments).
new_content_contains + file_extension_is`.expect("")` is strictly worse than `.unwrap()` — same panic, no explanation of the invariant. Either supply a real message or use `.unwrap()` and let the existing rule flag it.
new_content_contains + file_path_matches`Rc<RefCell<T>>` is the textbook 'fighting the borrow checker' shape — often a signal that an arena, index-based references, or a redesigned ownership model would be a better fit. Confirm intent.
new_content_contains + file_path_matchesString concatenation with `" + &` — prefer `format!("{}{}", a, b)` for readability and to avoid intermediate allocations. From the patterns guide §Idioms (concat-format).
new_content_contains + file_extension_is`#[allow(dead_code)]` in src/ — either delete the code or add a `///` doc-comment immediately above explaining why it's kept (planned API, generic-constraint trick, intentional placeholder). Documented exceptions are not flagged.
new_content_contains + file_path_matches`env::set_var(` in src/ — mutating process environment variables is unsound under concurrent reads (which is why edition 2024 marks the call unsafe). Verify the call site is genuinely single-threaded, or refactor to pass configuration explicitly through function arguments / a context struct. Tests where you control the thread count are usually fine; library code almost never is.
new_content_contains + file_path_matches`?fn` in ?file has ?count outer-scope `let` bindings — consider scoping intermediate temporaries into a block (`let result = { let raw = ...; let parsed = ...; ... }`) so only the final value is visible to the rest of the function. Block pattern: John Nunley, 'Rust's Block Pattern' (Dec 2025). (Scoped to src/ — examples/benches/tests are not production code and are exempt.)
file_path_matches + function_let_binding_count_high`?fn` in ?file has ?count outer-scope `let mut` declarations — consider John Nunley's block pattern: wrap the mutation in `let x = { let mut tmp = ...; ...; tmp }` so the surrounding scope sees an immutable binding. Block pattern: John Nunley, 'Rust's Block Pattern' (Dec 2025). (Scoped to src/ — examples/benches/tests are not production code and are exempt.)
file_path_matches + function_let_mut_count_high`?fn` in ?file calls `engine.eval(<string literal>)`. Inline string-eval can't be tested independently of the surrounding Rust code and bypasses any script registry. Move the script to a `.rhai` file and load it via `engine.compile_file(...)` (or `compile(...)` on `include_str!`-ed content) so the AST can be cached, values-checked at build time, and exercised in isolation.
engine_eval_string_literal + file_extension_is`print(` appears in a .rhai script. `print` is Rhai's equivalent of `dbg!()` — debug output that bypasses whatever response/logging channel your host has registered. Use the host-registered function for emitting output (commonly a `log`, `emit`, or `response_*` proxy your `Engine` exposes via `register_fn`) so script output flows through the same path as the rest of your application.
new_content_contains + file_extension_isprint() in ?fn (?file) — consider logging.info()/debug() instead. Remove debug prints before committing. Upstream: style guidance, not a correctness rule.
python_print_call + file_path_matchesDon't use bare `except:` — catch specific exception types. Bare except swallows KeyboardInterrupt and SystemExit. Upstream: PLE0704 (pylint), Bugbear B001.
python_bare_exceptMutable default `?param` in ?fn — defaults are created once at def time and shared across calls. Use None and create inside. Upstream: Bugbear B006.
python_mutable_default_argFunction call `?callee()` in default argument `?param` of ?fn — evaluated once at def time. If it returns a mutable or side-effectful value, this is a bug. Upstream: Bugbear B008 (narrower: only flags call expressions, not bare mutable literals which are covered by B006).
python_call_in_default_argHandler for ?exception in ?fn is empty (`pass`/`...`/comments) — the exception is silently swallowed. Recommend handling, re-raising, or documenting the intentional fallback. Upstream: Bugbear B110 (narrower: typed handlers only; bare handlers caught by enforce-no-bare-except).
python_exception_handler_passesFunction ?fn in ?file has ?count parameters — consider grouping into a config object or using the builder pattern. Exclude self/cls. Upstream: design smell (maintainability, not correctness).
python_function_param_count_highPublic def ?fn in ?file has no docstring. Upstream: documentation best practice; audit-only because docstring policy is project-dependent.
python_function_missing_docstring: any in src/ — narrow the type. Use `unknown` if you really don't know, then refine with type guards.
new_content_contains + file_path_matchesconsole.log in src/ — remove before committing, or use a proper logger.
new_content_contains + file_path_matches?count explicit `any` annotation(s) in ?fn (?file) — narrow the type, or use `unknown` and refine with type guards.
ts_explicit_any?count @ts-ignore/@ts-expect-error/@ts-nocheck comment(s) in ?file — each one turns the type checker off somewhere. Fix the type instead.
ts_suppression_comment?count non-null assertion(s) (`x!`) in ?fn (?file) — prefer explicit narrowing or optional chaining.
ts_non_null_assertionFunction `?fn` in ?file uses ?count force-unwrap(s). Prefer guard let or if let; reserve ! for invariants you can document.
function_uses_force_unwraptry! crashes on error — prefer try with do/catch, or try? when an Optional result is acceptable.
new_content_contains + file_extension_isForce-cast `as!` crashes on type mismatch — prefer `as?` with `if let`/`guard let`. Completes the force-bang trio with `!` and `try!`.
new_content_contains + file_extension_is`fatalError(` aborts the process — for recoverable conditions prefer a `throws` API and let the caller decide. Reserve `fatalError` for genuinely unreachable invariants (and prefer `precondition`/`assertionFailure` when the intent is a debug-only trap).
new_content_contains + file_extension_is`static var shared` is a mutable global — the Singleton pattern (eleev/swift-design-patterns §Creational/Singleton) uses `static let shared` so the instance can't be swapped at runtime. If mutability is intentional, add a comment or move state inside the instance.
new_content_contains + file_extension_isLegacy C-style constructor — prefer the modern Swift initializer (e.g. `CGRect(x:y:width:height:)`, `UIEdgeInsets(top:left:bottom:right:)`). Mirrors SwiftLint's `legacy_constructor` rule.
(new_content_contains | new_content_contains | new_content_contains | new_content_contains | new_content_contains | new_content_contains | new_content_contains | new_content_contains | new_content_contains) + file_extension_isLegacy random API — Swift 4.2+ ships `Int.random(in:)`, `Double.random(in:)`, and `Collection.randomElement()`, which work on all platforms (not just Darwin) and are uniformly distributed without modulo bias. Mirrors SwiftLint's `legacy_random` rule.
(new_content_contains | new_content_contains | new_content_contains) + file_extension_isLow confidence — compile/tests/known-bug not all green. Run the build and tests and resolve failing signals before committing.
bash_command_matches + __script__Medium confidence — one grounded signal is missing. Review before presenting this as done.
bash_command_matches + __script__`?func` (in ?file) calls a panicking API from the unwrap/expect/panic/todo/unimplemented family, and no test calls it directly. A panicking path with no test is where a crash reaches a user unnoticed — add a test that exercises this function, or handle the failure case explicitly. Coverage is matched by direct call only, so a function covered transitively may still be flagged.
edited_file + file_type + defines_fn + (calls_api | calls_api | calls_api | calls_api | calls_api) + untestedModule `?module` is part of import cycle `?cycle`. Mutually importing modules can't be understood, tested, or extracted independently, and the cycle tends to attract more coupling over time. Move the shared items into a third module both can depend on.
edited_file + declares_module + in_cycle`?func` uses a non-null assertion (`!`) and has no direct test. `!` tells the compiler a value cannot be null and produces no runtime check, so when the assumption is wrong the failure surfaces later and elsewhere, usually as a TypeError. Add a test that exercises the null case, or narrow the type so the assertion is unnecessary.
edited_file + file_type + defines_fn + calls_api + untested