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:

// 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.