| Category | Key Rule | Section |
|---|---|---|
| Testing | No #[cfg(test)] in source files → use tests/ directory |
Testing |
| Errors | Use derive_more::Display + derive_more::Error |
Error Handling |
| Tracing | All functions have #[instrument] |
Logging |
| Builders | Always use builders, never struct literals | Type Construction |
| Imports | use crate::{Type} not use crate::module::Type |
Module Organization |
| lib.rs | Only mod and pub use statements |
Module Organization |
| Workspace | No re-exports between workspace crates | Workspace Organization |
| Commits | Fix all errors/warnings before committing | Workflow |
| Linting | Never use #[allow] - fix root cause instead |
Linting |
- Plan → Use planning document (.md) with implementation steps
- Add the planning document to PLANNING_INDEX.md for tracking.
- For each step:
- Generate code
- Fix cargo check errors/warnings
- Run all checks (see below)
- Commit with audit-friendly message
- Push to branch
- Update planning document to serve as user guide
Fix all issues before committing, even if "unrelated":
# only run these after code changes:
# prefer running on package when possible
just check [package] # Basic compilation
# check-all recipe takes too long on full workspace
# prefer testing the package in isolation
just test-package [package]
just check-all [package] # clippy, fmt & test
# only run this if markdown files changed:
markdownlint-cli2 "**/*.md" # Markdown (if changed)Use just recipes (not raw cargo commands) to ensure justfile stays current.
Pre-merge commits, in addition:
- run
just check-all - run
just audit - run
just check-features
Zero tolerance: all tests passing, zero clippy warnings, zero errors.
// You export Input at crate level
pub use input::Input;
// Existing doctest breaks (was using module path):
/// ```
/// use crate::module::Input; // ❌ Now ambiguous!
/// ```
// Fix immediately:
/// ```
/// use crate::Input; // ✅ Crate-level import
/// ```
Common pitfalls:
- Export changes → doctest import paths break
- New exports → name conflicts with existing types
- Feature additions → missing
#[cfg(feature)]gates - Struct field additions → doctests missing new required fields
Only run when:
- Explicitly requested
- Before merge to main
- Targeted integration testing
just test-apiAlways use builders for struct construction. Never use struct literals.
// ❌ BAD: Struct literal (breaks on field additions, order-dependent)
let config = Config {
host: "localhost".to_string(),
port: 8080,
timeout: Duration::from_secs(30),
};
// ✅ GOOD: Builder pattern (self-documenting, future-proof)
let config = Config::builder()
.host("localhost")
.port(8080)
.timeout(Duration::from_secs(30))
.build();Benefits: Self-documenting, optional fields, validation, IDE support
1. derive_builder (#[derive(derive_builder::Builder)]):
use crate::MessageBuilder; // Import Builder struct
let msg = MessageBuilder::default()
.role(Role::User)
.content(vec![Input::Text("test".to_string())])
.build()
.expect("Valid message");2. derive_new (#[derive(derive_new::new)]):
For simple constructors with few arguments, use derive_new to generate a new() method:
use derive_new::new;
#[derive(Debug, Clone, new)]
pub struct Config {
host: String,
port: u16,
}
// Usage:
let config = Config::new("localhost".to_string(), 8080);3. Manual builders (impl Type { pub fn builder() }):
For complex types with many optional fields or validation:
// Do not import Builder struct
let request = GenerateRequest::builder() // Type provides builder()
.messages(vec![msg])
.build()
.expect("Valid request");Test pattern: Extract .build() calls that return Result:
// ✅ GOOD: Separate statements
let message = MessageBuilder::default()
.role(Role::User)
.build()
.expect("Valid");
let request = GenerateRequest::builder()
.messages(vec![message])
.build()
.expect("Valid");
// ❌ BAD: Nested builds (Result handling fails)
let request = GenerateRequest::builder()
.messages(vec![MessageBuilder::default()...build()])
.build();Data structures:
#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
pub struct MyType { /* ... */ }Use derive_more for:
Display,FromStr,From,Deref,DerefMut,AsRef,AsMut
Enums (no fields):
#[derive(Debug, Clone, strum::EnumIter)]
pub enum Status { Active, Inactive }Private fields + derive-based access:
use derive_getters::Getters;
use derive_setters::Setters;
#[derive(Debug, Clone, Getters, Setters)]
#[setters(prefix = "with_")] // Avoid getter/setter name conflicts
pub struct SecurityContext {
/// User ID (propagated to getter docs)
#[setters(doc = "Sets user ID")] // Separate setter docs
user_id: Option<UserId>,
#[setters(skip)] // Read-only field
created_at: DateTime<Utc>,
}
// Usage with manual constructor:
impl SecurityContext {
pub fn new(user_id: Option<UserId>) -> Self {
Self {
user_id,
created_at: Utc::now(),
}
}
}
ctx.user_id(); // Getter
ctx.with_user_id(new_id); // SetterWhen to use:
- derive_getters: Always for private fields
- derive_setters: Mutable config/state objects
- Manual constructors: For initialization (connections, validation, resources)
ErrorKind (specific conditions):
#[derive(Debug, Clone, PartialEq, Eq, Hash, derive_more::Display)]
pub enum StorageErrorKind {
#[display("Media not found: {}", _0)]
NotFound(String),
}Wrapper (error + location):
#[derive(Debug, Clone, derive_more::Display, derive_more::Error)]
#[display("Storage: {} at {}:{}", kind, file, line)]
pub struct StorageError {
pub kind: StorageErrorKind,
pub line: u32,
pub file: &'static str, // ✅ &'static str, not String
}Do not derive PartialEq, Eq, Hash, PartialOrd, Ord on wrapper errors (location tracking makes comparison confusing).
Do not write manual impl Display or impl Error for error types.
Use derive_more::Display + derive_more::Error on all errors.
Audit checklist:
- ✅ All error structs use
derive_more::Displaywith#[display(...)] - ✅ All error structs use
derive_more::Error - ✅ All ErrorKind variants have
#[display(...)] - ✅ No manual
impl std::fmt::Display - ✅ No manual
impl std::error::Error - ✅ All constructors use
#[track_caller] - ✅ Error
filefields use&'static str
#[derive(Debug, Clone, derive_more::Display, derive_more::Error)]
#[display("HTTP Error: {} at {}:{}", message, file, line)]
pub struct HttpError {
pub message: String,
pub line: u32,
pub file: &'static str,
}
impl HttpError {
#[track_caller]
pub fn new(message: impl Into<String>) -> Self {
let loc = std::panic::Location::caller();
Self {
message: message.into(),
line: loc.line(),
file: loc.file(),
}
}
}#[derive(Debug, Clone, PartialEq, Eq, Hash, derive_more::Display)]
pub enum StorageErrorKind {
#[display("Media not found: {}", _0)]
NotFound(String), // Tuple: use _0, _1
#[display("Hash mismatch: expected {expected}, got {actual}")]
HashMismatch { expected: String, actual: String }, // Struct: use names
}#[derive(Debug, Clone, derive_more::Display, derive_more::Error)]
#[display("Storage: {} at {}:{}", kind, file, line)]
pub struct StorageError {
pub kind: StorageErrorKind,
pub line: u32,
pub file: &'static str,
}
impl StorageError {
#[track_caller]
pub fn new(kind: StorageErrorKind) -> Self {
let loc = std::panic::Location::caller();
Self { kind, line: loc.line(), file: loc.file() }
}
}#[derive(Debug, derive_more::From, derive_more::Display, derive_more::Error)]
pub enum CrateErrorKind {
#[from(HttpError)]
Http(HttpError),
#[from(StorageError)]
Storage(StorageError),
}
#[derive(Debug, derive_more::Display, derive_more::Error)]
#[display("Crate Error: {}", _0)]
pub struct CrateError(Box<CrateErrorKind>);
// Generic blanket From
impl<T> From<T> for CrateError
where T: Into<CrateErrorKind>
{
fn from(err: T) -> Self {
Self(Box::new(err.into()))
}
}Problem: Blanket impl<T> From<T> creates type inference ambiguity,
requiring manual conversions everywhere.
Solution: Macros generate monomorphized From impls for perfect
inference and ergonomic ? operator.
Two-macro system:
/// Bridge external errors through wrappers into ErrorKind
macro_rules! bridge_error {
($external:ty => $wrapper:ty) => {
impl From<$external> for ErrorKind {
#[track_caller]
fn from(err: $external) -> Self {
<$wrapper>::from(err).into()
}
}
};
}
/// Complete the chain: external error → ErrorKind → Error
macro_rules! error_from {
($source:ty) => {
impl From<$source> for Error {
#[track_caller]
fn from(err: $source) -> Self {
let kind = ErrorKind::from(err);
tracing::error!(error_kind = %kind, "Error created");
Self(Box::new(kind))
}
}
};
}
// Usage: One line per external error type
bridge_error!(reqwest::Error => HttpError);
bridge_error!(serde_json::Error => JsonError);
bridge_error!(url::ParseError => UrlError);
// Complete the conversion chain
error_from!(reqwest::Error);
error_from!(serde_json::Error);
error_from!(url::ParseError);
// From<ErrorKind> for Error (manual, not macro)
impl From<ErrorKind> for Error {
#[track_caller]
fn from(kind: ErrorKind) -> Self {
tracing::error!(error_kind = %kind, "Error created");
Self(Box::new(kind))
}
}Why macros are required:
Without macros, the compiler cannot infer which Into<ErrorKind> impl to use:
// ❌ Blanket impl causes ambiguity
impl<T> From<T> for Error where T: Into<ErrorKind> { ... }
// At call sites:
let response = reqwest::get(url).await?;
// ^ ERROR: type annotations needed
// Forces manual conversion everywhere:
let response = reqwest::get(url)
.await
.map_err(|e| Error::from(ErrorKind::from(HttpError::from(e))))?; // Boilerplate!With macros (monomorphized):
// ✅ Concrete implementations via macros
error_from!(reqwest::Error); // Generates impl From<reqwest::Error> for Error
// At call sites:
let response = reqwest::get(url).await?; // ✅ Just works! Perfect inference.Conversion chain:
reqwest::Error
↓ (error_from! macro generates From impl)
ErrorKind::from(reqwest::Error)
↓ (bridge_error! impl: reqwest::Error → HttpError → ErrorKind)
ErrorKind::Http(HttpError)
↓ (From<ErrorKind> for Error)
Error(Box<ErrorKind>)
Benefits:
- Zero boilerplate at call sites (just use
?) - Perfect type inference (compiler knows exact impl)
- Explicit error type list (self-documenting)
#[track_caller]preserved on all conversions- Type-safe (only declared types convert)
Reference: See crates/botticelli_error for complete implementation.
ALL functions have #[instrument] - public, private, methods, free functions.
Observability is critical for debugging, performance monitoring, and error tracking. Missing instrumentation is a defect. If a private function has a bug, you need tracing to find it.
#[instrument(skip(conn), fields(table_name, limit))]
pub fn list_content(
conn: &mut PgConnection,
table_name: &str,
limit: i64,
) -> DatabaseResult<Vec<ContentRow>> {
debug!("Querying content table");
debug!(sql = %query, "Executing query");
match result {
Ok(rows) => {
debug!(count = rows.len(), "Retrieved rows");
Ok(rows)
}
Err(e) => {
error!(error = ?e, "Query failed");
Err(e.into())
}
}
}ALL functions (public, private, methods, free functions):
- Use
#[instrument]for automatic span creation - Skip large params:
skip(connection, data) - Include context:
fields(table_name, limit) - Emit events at key points (debug/info/warn/error)
- Log SQL at debug level
- Track errors before returning
trace!()- Loop iterations, fine-grained detaildebug!()- Function entry/exit, state changes, SQLinfo!()- Major events, initializationwarn!()- Recoverable unusual conditionserror!()- Errors requiring investigation
debug!(count = items.len(), "Processing"); // Field
debug!(value = ?self.field(), "State"); // Debug format
info!(table = %table_name, "Creating"); // Display format
#[instrument(skip(connection, large_json))] // Skip large dataFeature-gated code: Use full path tracing::warn!() instead of use tracing::warn; to avoid orphaned imports when features are disabled.
- ✅ Every function (public and private) has
#[instrument] - ✅ Span fields include context (IDs, counts)
- ✅ Large structures skipped
- ✅ Key operations emit events
- ✅ Errors logged before return
- ✅ SQL at debug level
- ✅ Span names follow
module.function
Without tracing:
Human: "Bot command failed"
AI: "Run with RUST_LOG=debug"
Human: <500 lines>
AI: "Check guild_id?"
Human: "Oh, forgot that"
Result: 3+ messages, 10+ minutes
With tracing:
Human: "Bot command failed"
Human: <5 lines>
ERROR discord.execute: Missing arg command="server.get_stats" missing_arg="guild_id"
AI: "Add guild_id = '123456' to TOML [bot.args]"
Result: 1 message, instant fix
Benefits:
- Location tracking → AI jumps to code
- Structured fields → AI sees exact problem
- Spans → AI understands which layer failed
- Error context → AI knows what was attempted
- Performance data → AI diagnoses slowness
- Cache observability → AI debugs stale data
Design principle: Write traces for an AI reader. Ask: "Would this single trace give enough info to diagnose?"
#[cfg(test)] mod tests in source files is not allowed.
All tests go in tests/ directory.
Rationale: Centralized tests, easier to maintain, no source file clutter
Audit: Flag any #[cfg(test)] or mod tests in source as violations
Naming: {module}_{component}_test.rs
- Examples:
storage_filesystem_test.rs,narrative_executor_test.rs
Imports: Use crate-level exports
use botticelli::{Type, OtherType}; // ✅ Crate-level
// NOT: use botticelli::module::Type;Independence: Self-contained, no inter-test dependencies
Helpers: Create within test files to reduce duplication
Design tests to minimize:
- Tokens: Minimal prompts, low
max_tokens(e.g., 10) - Requests: Fewest calls possible (1-3, not 20+)
- Time: Short duration
Feature gating:
#[test]
#[cfg_attr(not(feature = "api"), ignore)]
fn test_gemini_api() {
// Uses API tokens
}Run with: just test-api
Do not use #[ignore] - reserved for:
- Unimplemented features
- Broken tests needing fixes
- Temporarily disabled during refactoring
Consider:
- Mocking API responses
- Separate "expensive" test suite with warnings
- Local test doubles
lib.rs: Only mod and pub use statements. No type definitions, traits, or impl blocks.
Module declarations: Private (not pub mod)
Crate-level exports: Re-export all public types at crate root
Imports: Always use crate::{Type}, never use crate::module::Type or super::Type
// src/lib.rs (✅ ONLY mod + pub use)
mod error;
mod models;
pub use error::{MyError, MyResult};
pub use models::{Model, NewModel};
// src/models.rs
use crate::{MyError, MyResult}; // ✅ Crate-level imports
pub struct Model { /* ... */ }Guideline: When file exceeds ~500-1000 lines
Structure:
src/mymodule/
├── mod.rs # ONLY mod + pub use
├── core.rs
├── io.rs
└── helpers.rs # Internal, not exported
mod.rs pattern:
mod core;
mod io;
mod helpers; // Internal
pub use core::{Type1, Type2};
pub use io::{Reader, Writer};
// helpers not exportedCrate-level types:
use crate::{Type1, Type2}; // ✅ AlwaysInternal helpers:
use crate::module::helper::function; // ✅ OK for internal
use crate::database::schema::users; // ✅ OK for schemaForbidden:
use crate::module::Type; // ❌ Module path
use super::Type; // ❌ Super path
use module::*; // ❌ Wildcard- Single import path per type
- No ambiguity
- Hidden internal structure
- Easy refactoring
- Better IDE support
lib.rs only contains mod declarations and pub use exports.
Even small crates (100-200 lines) should separate into modules:
crates/my_crate/src/
├── lib.rs # mod + pub use only
├── role.rs
├── input.rs
└── output.rs
Forbidden:
// crates/botticelli_database/src/lib.rs
pub use botticelli_error::DatabaseError; // ❌ Creates ambiguity
// Now users have two import paths:
use botticelli_error::DatabaseError; // Source
use botticelli_database::DatabaseError; // Re-exported ❌Required:
// crates/botticelli_database/src/lib.rs
// NO re-exports of dependency types
// Users import from source:
use botticelli_database::Repository; // Database's own types
use botticelli_error::DatabaseError; // Error from error crateRationale:
- Creates ambiguity (two paths to same type)
- Breaks "single import path" principle
- Makes refactoring difficult
- IDE confusion
- Unclear ownership
Type aliases OK:
use botticelli_error::DatabaseError;
/// Convenience alias for database results.
pub type DatabaseResult<T> = Result<T, DatabaseError>; // ✅ Alias OKException: Only top-level binary/library crate may re-export for user convenience:
// crates/botticelli/src/lib.rs (top-level only)
pub use botticelli_core::{Role, Input};
pub use botticelli_error::BotticelliError;
// Internal crates NEVER re-export from each otherOne clear responsibility per module:
- Single type (simple case)
- Related types (common case - e.g., enum + helper struct)
- Shared dependencies (used by multiple modules)
Same rules as single crates:
use crate::{Type}; // ✅ Crate-level
// NOT: use crate::module::Type;Cross-crate imports:
use other_crate::{Type}; // ✅ Direct from source crateDerive: Serialize, Deserialize for persisted/transmitted types
Attributes:
#[serde(skip)]- Runtime state, caches, UI state, handles#[serde(default)]- UseDefaultwhen missing#[serde(default = "fn_name")]- Custom default#[serde(rename = "name")]- Different serialized name
Group skipped fields:
// Runtime state (not serialized)
#[serde(skip)]
cache: HashMap<K, V>,
#[serde(skip)]
texture: TextureHandle,Complex needs: Implement custom Serialize/Deserialize
Usage: #[cfg(feature = "feature-name")]
Documentation: /// Available with thefeature-namefeature.
Available features:
backend-eframe- eframe/wgpu (default)text-detection- OpenCV text detectionlogo-detection- OpenCV logo detectionocr- Tesseract OCRdev- All optional featuresapi- Empty marker for API tests
Verification:
cargo check --no-default-features # Without optionals
cargo check --all-features # All together
just check-features # All combinationsNever use #[allow(dead_code)] or any #[allow(...)] directive. Fix the root cause instead.
Why this matters:
dead_codewarnings expose missing#[cfg(feature)]gates- Unused code = design problem or missing functionality
- Each
allowpostpones a real problem
Fix it properly:
// ❌ Never do this
#[allow(dead_code)]
field: String,
// ✅ Add feature gate
#[cfg(feature = "database")]
field: String,
// ✅ Or add getter to use it
pub fn field(&self) -> &str { &self.field }Solutions (in order):
- Feature gate the code:
#[cfg(feature = "...")]if truly conditional - Make it public with getters: Expose via proper encapsulation
- Use
pub(crate): Limit visibility appropriately - Delete it: If genuinely unused, remove it
Never use #[allow] directives. If you think you need one, you're solving the wrong problem. Fix the root cause instead.
AI systems operate on probabilistic pattern matching, not deterministic rules. #[allow(dead_code)] appears frequently in Rust training data, creating strong learned associations. This documentation shifts probability distributions but cannot guarantee compliance - the AI will still reach for anti-patterns when trained heuristics dominate.
Human review is critical:
- Search for
#[allowin diffs - reject any occurrence - Run
just check-featuresto catch hidden feature gate issues - Verify
#[cfg]usage over suppression - The AI's decisions blend: training data (strongest), project docs, immediate context, statistical patterns
No amount of documentation creates "hard rules" in probabilistic systems.
- Let linter complete (don't deny all warnings immediately)
- Fix all issues in single pass - address root causes, never add
#[allow] - After markdown edits:
markdownlint-cli2(notmarkdownlint) - Don't run cargo clippy/test after markdown-only changes
Style: /// for items, //! for modules
Required: All public items (enforced by #![warn(missing_docs)])
Content:
- What (concise first line)
- Why (when non-obvious)
- Parameters/returns (when not obvious from types)
- Examples (complex APIs)
- Errors (Result-returning functions)
Keep concise - avoid stating the obvious from signature
In Cargo.toml:
>=1.0→"x"(major only)>=0.1.0→"x.y"(major.minor)<0.1.0→"x.y.z"(full version)
Before testing: cargo update (update Cargo.lock)
First-class document: Maintain alongside code changes
Forcing function principle: Always reference just recipes in instructions, never raw cargo commands. If the justfile is required for basic workflow, it must stay current.
When to update:
- New workflows/tools → add recipes
- Changed dependencies/flags → update recipes
- New linters → add check recipes
- Build optimizations → update recipes
- CI/CD changes → sync justfile
Test recipes after changes
State machine extraction: Multiple booleans representing mutually exclusive states → extract to enum
Borrow checker: Simultaneous immutable/mutable borrows → extract needed values before mutable borrow:
let value = *self.field(); // Extract first
self.mutate(); // Then mutably borrowForbidden: Use #![forbid(unsafe_code)] in lib.rs
When to commit:
- Only when user explicitly requests
- After all checks pass
- All tests passing, zero warnings
Commit message format:
git commit -m "$(cat <<'EOF'
type(scope): Brief description
Detailed explanation of what and why.
Key changes:
- Bullet point 1
- Bullet point 2
Testing:
- Test coverage details
Files modified:
- path/to/file.rs - what changed
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
EOF
)"Git safety:
- Never update git config
- Never destructive commands (force push, hard reset) unless explicit
- Never skip hooks (--no-verify) unless explicit
- Never force push to main/master
- Check authorship before amend:
git log -1 --format='%an %ae'
Files:
dist-workspace.toml- config.github/workflows/release.yml- auto-generated
Commands:
just dist-build # Build artifacts
just dist-check # Verify without upload
just dist-plan # Preview release
just dist-generate # Update CI workflowProcess:
- Update version in
Cargo.toml just dist-planjust pre-release- Create/push tag (e.g.,
v0.1.0) - GitHub Actions publishes
just audit # Check vulnerabilities
just omnibor # Generate artifact tree
just security # All checksBefore release:
just security- Update deps:
just update-deps - Re-run full test suite
Top priorities:
- ✅ Fix all issues before commit
- ✅ All functions instrumented (public and private)
- ✅ Use derive_more for all errors
- ✅ Always use builders, never literals
- ✅ Tests in
tests/, never inline - ✅ lib.rs only has mod + pub use
- ✅ Import as
use crate::{Type} - ✅ No re-exports between workspace crates