fold/CLAUDE.md

# CLAUDE.md — fold code conventions

Operating contract for Claude (and any other LLM-driven contributor) working
in this repo. Complements `AGENTS.md` (which covers backend/router
architecture). This file is the **structural** contract: how to keep code
readable, testable, and modular as the codebase grows.

## Core principle

**No new monoliths. Split before they grow.**

The encoder grew to 2800+ lines in a single file because each contributor
added "just one more arm" to the dispatch chain. Each addition was locally
reasonable; the cumulative result is unreviewable. Fight this drift on every
PR.

---

## File-size budgets

| Artifact            | Soft cap | Hard cap | Action at hard cap                   |
|---------------------|---------:|---------:|--------------------------------------|
| Header (.hpp)       | 300 lines | 500 lines | Split by responsibility              |
| Source (.cpp)       | 800 lines | 1500 lines | Split by responsibility              |
| Single function     | 50 lines | 150 lines | Extract helpers                      |
| Single class        | 300 lines | 600 lines | Split into composed components       |
| Switch/dispatch arm | 30 lines | 80 lines | Extract to named helper              |

If a file is **already** over the hard cap (e.g. `src/symbolic/encoder.cpp`
at 2800 lines), every PR that touches it must:

1. Not increase the line count beyond what's strictly needed for the
   functional change.
2. Carry along at least one extraction (one helper, one dispatch arm, one
   data structure moved to its own file) as scout-rule cleanup. The
   extraction goes in the same PR as the functional change.

When extracting from an oversized file, target the **anonymous-namespace
helper functions** first (low risk, no API change) before moving public
types.

---

## Function structure

### Dispatch functions
Functions that select behavior by type/kind/opcode (`encodeExpr`,
`encodeBinary`, `applyStmt`, etc.) should be **pure switches** that delegate
to per-arm helpers:

```cpp
// GOOD
z3::expr encodeExpr(ir::Expr e, const ASTContext& ctx, SymbolicEnv& env) {
    switch (e.kind) {
        case Kind::DeclRef:    return encodeDeclRef(e, ctx, env);
        case Kind::BinaryOp:   return encodeBinaryArm(e, ctx, env);
        case Kind::Call:       return encodeCallArm(e, ctx, env);
        // ...
    }
}

// BAD — bodies inline, function grows unbounded
z3::expr encodeExpr(...) {
    if (kind == DeclRef) { /* 30 lines */ }
    if (kind == BinaryOp) { /* 150 lines */ }
    if (kind == Call) { /* 200 lines */ }
}
```

A dispatch function's cyclomatic complexity should be O(number of cases),
not O(number of cases × average case complexity). If a single arm is over
30 lines, extract it.

### Helper extraction rule of thumb

Before duplicating a 4-line idiom, extract it. Three sites of the same
pattern → helper. Two sites → judgment call (usually leave it).

Before extracting, decide if the helper is **internal** (anonymous namespace
in the same .cpp, never tested directly) or **shared** (exposed in a header,
unit-testable). Default to internal; promote only when a second TU needs it
or when the logic is complex enough to deserve targeted tests.

---

## Layering

### Strict layer rules (no upward includes)

```
fold::ir          // dialect-agnostic types (Symbol, Expr, SourceLoc, ...)
   ↑
fold::report      // Finding, Location, structured output
   ↑
fold::symbolic    // SymbolicEnv, encoder, theory; consumes ir + report
   ↑
fold::detect      // detectors that subclass SafetyObserver
   ↑
fold::cli         // scan runner, MCP, CLI wiring
```

Headers in lower layers MUST NOT include headers from higher layers. The
compiler will catch most of these via include-what-you-use; the human cost
is treating "I just need access to FooDetector from SymbolicEnv" as a code
smell.

### Frontend abstraction (the M30 work)

`fold::ir` types (`Symbol`, `Expr`, `Call`, `MemberRef`, `SourceLoc`) are
**frontend-agnostic** by convention. Their header files include no clang
headers. Clang-aware implementations live in `*_clang.cpp` files (see
`src/symbolic/ir_expr_clang.cpp`, `src/symbolic/clang_location_mapper.cpp`).

A future Rust frontend constructs `ir::Expr{Kind::DeclRef, rust_handle}`
directly without going through the clang path. Detectors and hooks must
not break this invariant.

---

## Testability

### Test-first for new modules

A new file in `src/symbolic/` or `src/detect/` requires:
- Unit tests in `tests/unit/test_<module>.cpp`
- At least one positive test (intended behavior fires)
- At least one negative test (related-but-different input does NOT fire)
- Wired into `tests/unit/meson.build`

Don't add a detector without a test file. Don't add a transformation pass
without a counter-example test.

### Test-by-extraction for legacy modules

When extracting from a monolith, the extracted helper gains a unit test in
the same PR. If extracting `encodeArraySubscript` from `encodeExpr`, also
add `test_encode_array_subscript.cpp` with at least one happy-path case.
This converts "we're all-green" (integration coverage) into "all-green +
targeted helper coverage" (extraction enables regression isolation).

### Validation cadence on the hot path

Encoder and detector changes are precision-sensitive: unit tests can pass
while curl/yams audit findings drift by hundreds. After any encoder or
detector edit:

1. `ninja -C build/release` — must succeed
2. `meson test -C build/release` — must be fully green (Fail: 0)
3. For changes touching `encodeExpr` / `encodeBinary` / `encodeUnary` /
   `applyStmt` / `SymbolicEnv` / `SafetyObserver`: re-run a curl null-deref
   audit and diff finding counts against the last known-good baseline.
   Document any delta.

### What "green" means

`meson test -C build/release` returning `Fail: 0` is necessary
but not sufficient. The full bar is:

- Build green with `-Werror`-equivalent warnings inspected
- All tests pass
- For symbolic-encoder changes: spot-check finding count parity on at
  least one OSS target (curl is the canonical baseline)
- No `// TODO` left in code for "I'll handle this in a follow-up" — either
  do it now or write a tracked issue

---

## API surface discipline

### When adding a public API

Before adding to a header (`include/fold/...`):

1. **Is one caller enough?** If only one TU needs it, keep it in the .cpp
   anonymous namespace.
2. **Does it deserve a test?** Public APIs need targeted unit tests.
3. **Is the name self-documenting?** `compute()` is a smell; `compute<x>()`
   for some specific `x` is the right name.
4. **Does it leak frontend types?** A header in `fold/ir/` must not name
   `clang::*`; a header in `fold/detect/` may name clang because detectors
   are clang-frontend-coupled today, but each new such header makes a Rust
   frontend harder.

### When adding an observer hook

`SafetyObserver` hooks are a contract that every detector implements. Each
new hook is:
- A signature update across 7+ detectors
- A new clang→IR conversion site in the encoder
- A risk of subtle ordering bugs

Before adding a hook, ask: can this be derived from an existing hook +
SymbolicEnv state? If yes, don't add. If no, add it with parameter types
already abstract (`ir::Expr`, `ir::Symbol`, `ir::SourceLoc`) — never with
`clang::*` types.

---

## Editing oversized files (the encoder rule)

`src/symbolic/encoder.cpp` is 2800+ lines. When a change is necessary
there:

### DO

- Add new logic as a **named helper** in the same file, called from one
  dispatch site. Don't inline new logic into an existing 100-line arm.
- Use the existing `encodeBinaryOperatorArm` / `encodeCallExprArm` pattern
  for any new big-body arm: forward-declare in an anonymous namespace
  near the top, define after `encodeExpr`.
- Carry along **one extraction** with the functional change (scout rule):
  pull one anonymous-namespace helper into a sibling .cpp file, or split
  the file along a natural boundary if the helper is large.

### DON'T

- Add a new dispatch arm directly to the legacy if-chain — it's empty now;
  use the `switch(ir::Expr::Kind)` pattern.
- Grow `encodeBinaryOperatorArm` or `encodeCallExprArm` beyond their
  current size. If a new case is needed, factor sub-helpers first.
- Add new clang dyn_cast chains. Detector-side dispatch should go through
  `ir::Expr::Kind` and the typed accessors (`binaryOpcode()`,
  `declRefSymbol()`, etc.).

### Planned splits (for future PRs)

- `encoder.cpp` should eventually become:
  - `encoder_dispatch.cpp` — `encodeExpr` switch + small leaf arms
  - `encoder_binary.cpp` — `encodeBinary` + `encodeBinaryOperatorArm`
  - `encoder_unary.cpp` — `encodeUnary` + `encodeUnaryOperatorArm`
  - `encoder_call.cpp` — `encodeCallExprArm` + library models
  - `encoder_stmt.cpp` — `applyStmt` + DeclStmt handling
  - `encoder_types.cpp` — `coerceToBv`, `coerceToType`, type-system glue
  - `encoder_runtime.cpp` — small utility helpers (`freshUnknown`,
    `markEncoded`, `recordPrecision`)

Each split is a separate PR with explicit test parity verification.

---

## Naming

- Anonymous-namespace helper functions: `camelCase`, descriptive of intent
  (`encodeArraySubscriptArm`, not `handleArraySubscript`).
- Public types: `PascalCase` (`SymbolicEnv`, `SafetyObserver`).
- Constants: `kPascalCase` (`kBitvecWidth`, `kMaxModeledCopyBytes`).
- Member fields: trailing underscore (`current_`, `definitely_non_null_`).
- IR types: in `fold::ir` namespace, short names (`Expr`, `Symbol`,
  `SourceLoc`) — they're the most-used vocabulary.

---

## Why this file exists

The M30 refactor (Phases A–D3) made the detector + observer surface
frontend-agnostic. But the encoder itself is still 2800 lines of
mostly-clang code that grew because nobody enforced size budgets. The
refactor cost ~12 sessions of careful work to undo.

Don't repeat that. New code should be split as it's written. Old code
should shrink with every PR that touches it.

If you're an LLM working in this repo: when you're about to add code, ask
"would a human reviewer be happy reading this 6 months from now?" If the
answer is "they'd skim past it because the file is too big," extract first.