Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 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
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
.claude/**
.idea/**

site/

*.zip

.vscode/**
Expand Down
4 changes: 4 additions & 0 deletions .llm/context.md
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,7 @@ if !valid { return Err(FortressError::InvalidState); } // Explicit error
- **Exhaustive matches** — Never use `_ =>` wildcards on enums
- **Enums over booleans** — `Compression::Enabled` not `true`
- **Type safety** — Make invalid states unrepresentable
- **Doc examples too** — Rustdoc examples must use `?` and `Result`, never `panic!` or `unwrap()`

**See also:** [type-driven-design.md](skills/type-driven-design.md), [rust-pitfalls.md](skills/rust-pitfalls.md)

Expand Down Expand Up @@ -749,6 +750,7 @@ The changelog (`CHANGELOG.md`) is for **users of the library**, not developers.
- New features, APIs, or configuration options
- Bug fixes that affect user-visible behavior
- Breaking changes (with migration guidance)
- New enum variants on exhaustively matchable enums (**Breaking:** — see skill doc)
- Performance improvements users would notice
- Dependency updates that affect compatibility

Expand Down Expand Up @@ -828,6 +830,8 @@ Workflow syntax errors are easy to introduce and tedious to debug in CI. Always

### Documentation Changes

> **See also:** [documentation-code-consistency.md](skills/documentation-code-consistency.md) — Keeping docs and code in sync, verifying CHANGELOG accuracy, error documentation patterns.

**Run after modifying any rustdoc comments:**

```bash
Expand Down
89 changes: 87 additions & 2 deletions .llm/skills/changelog-practices.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,70 @@ Examples:
- Adding required trait bounds
- Changing return types
- Changing default behavior
- **Adding enum variants to exhaustively matchable enums** (see below)

### Enum Variants Are Breaking Changes (Unless `#[non_exhaustive]`)

**Critical:** Adding a new variant to a public enum is a **breaking change** if users can exhaustively match on it.

```rust
// ❌ Exhaustively matchable — adding variants is BREAKING
pub enum ConnectionState {
Disconnected,
Connecting,
Connected,
}

// ✅ Non-exhaustive — adding variants is NOT breaking
#[non_exhaustive]
pub enum ConnectionState {
Disconnected,
Connecting,
Connected,
}
```

**Why it breaks:** Users with exhaustive matches will get compile errors:

```rust
// User's code that breaks when you add a variant
match state {
ConnectionState::Disconnected => { ... }
ConnectionState::Connecting => { ... }
ConnectionState::Connected => { ... }
// ERROR: non-exhaustive patterns: `NewVariant` not covered
}
```

**CHANGELOG entries for new enum variants:**

```markdown
# ❌ WRONG — Listed as "Added" but it's breaking
### Added
- `ConnectionState::Syncing` variant for synchronization phase

# ✅ CORRECT — Marked as breaking with migration guidance
### Changed
- **Breaking:** Added `ConnectionState::Syncing` variant. Update exhaustive matches to handle this new state.
```

**Prevention:** When creating public enums that may grow, use `#[non_exhaustive]`:

```rust
/// Connection states for peer sessions.
///
/// This enum is `#[non_exhaustive]`; new variants may be added
/// in future versions without a breaking change.
#[non_exhaustive]
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum ConnectionState {
Disconnected,
Connecting,
Connected,
}
```

> **See also:** [workspace-organization.md](workspace-organization.md) for `#[non_exhaustive]` best practices.

### New Features (SHOULD document)

Expand Down Expand Up @@ -217,8 +281,9 @@ When making changes, ask:
1. **Is this `pub`?** If yes, consider changelog entry
2. **Does behavior change?** If yes, document it
3. **Could user code break?** If yes, mark as **Breaking:**
4. **Is this a bug fix users would care about?** If yes, document it
5. **Is this purely internal?** If yes, skip changelog
4. **Adding enum variants?** Check if enum is `#[non_exhaustive]` — if not, it's **Breaking:**
5. **Is this a bug fix users would care about?** If yes, document it
6. **Is this purely internal?** If yes, skip changelog

---

Expand Down Expand Up @@ -281,8 +346,28 @@ rg 'changed_function|ChangedStruct' --type rust --type md

---

## Verification Before Committing

**Always verify CHANGELOG claims match actual code:**

```bash
# Verify derives exist before claiming them
rg '#\[derive.*Hash' src/lib.rs

# Verify method/type exists
rg 'pub fn method_name|pub struct TypeName' --type rust

# Build docs to catch broken links
RUSTDOCFLAGS="-D warnings" cargo doc --no-deps
```

> **See also:** [documentation-code-consistency.md](documentation-code-consistency.md) for comprehensive verification commands and common pitfalls.

---

## References

- [Keep a Changelog](https://keepachangelog.com/en/1.1.0/)
- [Semantic Versioning](https://semver.org/)
- [Rust API Guidelines - Documentation](https://rust-lang.github.io/api-guidelines/documentation.html)
- [documentation-code-consistency.md](documentation-code-consistency.md) — Keeping docs and code in sync
90 changes: 90 additions & 0 deletions .llm/skills/defensive-programming.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

> **This document defines the defensive programming standards for Fortress Rollback.**
> All production code, including library code and any editor/tooling code, MUST follow these practices.
> **This also applies to documentation examples** — rustdoc examples are compiled and should demonstrate correct error handling.

## Core Philosophy

Expand Down Expand Up @@ -55,6 +56,43 @@ unreachable!();
unreachable!("this should never happen");
```

### Documentation Examples Must Also Follow Zero-Panic

**Rustdoc examples are compiled code.** They must demonstrate proper error handling, not panic shortcuts:

```rust
// ❌ FORBIDDEN in doc examples — teaches bad habits
/// # Examples
///
/// ```
/// let session = SessionBuilder::new().build().unwrap();
/// let result = session.advance_frame();
/// if result.is_err() {
/// panic!("frame advance failed"); // NEVER show panic! as error handling
/// }
/// ```

// ✅ REQUIRED in doc examples — demonstrates proper patterns
/// # Examples
///
/// ```
/// # use fortress_rollback::*;
/// let session = SessionBuilder::new().build()?;
/// let requests = session.advance_frame()?;
/// for request in requests {
/// // Handle each request...
/// }
/// # Ok::<(), FortressError>(())
/// ```
```

**Why this matters:**

- Doc examples are often copy-pasted by users
- Examples with `panic!` or `unwrap()` teach incorrect patterns
- Users learn from examples — show them the RIGHT way
- The `# Ok::<(), Error>(())` pattern enables `?` in doc tests

### Required Patterns

All fallible operations MUST return `Result`:
Expand Down Expand Up @@ -511,6 +549,58 @@ return Err(FortressError::InvalidPlayerIndex {
});
```

### Error Categorization: `InvalidRequest*` vs `InternalError*`

Choosing the correct error category is critical for debugging and API contracts.

**`InvalidRequestStructured` / `InvalidRequest`**: Use for **caller-provided invalid arguments**.
The caller made a mistake; this is expected and recoverable.

**`InternalErrorStructured` / `InternalError`**: Use for **library bugs or invariant violations**.
Something went wrong inside the library that should never happen under normal API usage.

**Decision tree:**

```
Is the invalid value provided by the caller as an argument?
├─ YES → InvalidRequestStructured (caller's responsibility)
└─ NO → Is it derived from internal library state?
├─ YES → InternalErrorStructured (library bug)
└─ NO → Trace back: who created this value?
└─ Usually leads to one of the above
```

**Example: Division by zero**

```rust
// Function signature: pub fn try_buffer_index(&self, buffer_size: usize) -> Result<...>

// ❌ WRONG: buffer_size == 0 is caller's fault, not a library bug
if buffer_size == 0 {
return Err(FortressError::InternalErrorStructured {
kind: InternalErrorKind::DivisionByZero, // Wrong category!
});
}

// ✅ CORRECT: Caller passed invalid argument
if buffer_size == 0 {
return Err(FortressError::InvalidRequestStructured {
kind: InvalidRequestKind::ZeroBufferSize,
});
}
```

**Why this matters:**

- `InternalError` tells users: "Report this as a bug" — wrong if it's their fault
- `InvalidRequest` tells users: "Fix your input" — actionable guidance
- Incorrect categorization erodes trust and wastes debugging time

**Quick test:** Ask "If a user follows the documented API correctly, can they ever trigger this error?"

- **NO** (impossible with correct usage) → `InternalError` (indicates library bug)
- **YES** (possible with incorrect arguments) → `InvalidRequest` (user error)

### Unknown/Fallback Variants in Error Reason Enums

When creating "reason" enums for structured errors, include an `Unknown` or fallback variant
Expand Down
Loading
Loading