# SF Code Standards

Code patterns for AI-assisted development. Full rules: [AGENTS.md](AGENTS.md) · Planning contract: [docs/adr/0000-purpose-to-software-compiler.md](docs/adr/0000-purpose-to-software-compiler.md)

---

## Quick Index

Agent-facing docs are for model consumption first: terse, structured, low-ceremony. Compress wording, not semantics — never remove purpose, value, consumer, consequence, invariants, or action thresholds to save tokens.

| Section | Description |
|---------|-------------|
| [1. Purpose Doctrine](#1-purpose-doctrine) | The #1 rule: every symbol must answer why it exists |
| [2. Principles](#2-principles) | Core coding principles |
| [3. Anti-Patterns](#3-anti-patterns) | Blocked patterns and required replacements |
| [4. Thresholds](#4-thresholds) | Code quality limits |
| [5. Naming](#5-naming) | Naming conventions |
| [6. Patterns](#6-patterns) | Architectural patterns |
| [7. Documentation](#7-documentation) | JSDoc / comment standards |

---

## 1. Purpose Doctrine

**Purpose is the most important thing in any symbol.**

Every exported function, class, constant, and module must answer:

- **why** it exists (not what it does — the signature shows that)
- **what value** it creates or protects
- **who** calls it in production (a real consumer, not just tests)
- **what breaks** if it returns the wrong answer

If any answer is missing: `BLOCKED: purpose unclear — [field]`.

### JSDoc format

```js
/**
 * Acquire a unit claim atomically. Returns true on success, false if another
 * worker already holds an unexpired lease.
 *
 * Purpose: prevent two workers from dispatching the same unit when the
 * run-lock is unavailable — the conditional UPDATE is the safety net.
 *
 * Consumer: autonomous dispatch.ts when picking the next eligible unit per
 * poll tick.
 */
export function claimUnit(unitId, leaseMs) { ... }
```

Required sections for non-trivial exports:

- **First line** — what it returns / does, present tense.
- **Purpose:** — why it exists; the value it protects.
- **Consumer:** — who calls it in production. No consumer = symbol shouldn't exist yet.

A bare `/** Helper. */` is a code smell. Either write the purpose or delete the symbol.

### Module-level JSDoc

```js
// session-recorder.js — per-process session lifecycle manager
//
// Purpose: capture the session/turn/file-touch/ref stream into DB rows so
// the memory pipeline has structured data to embed and cross-session search
// has rows to query.
//
// Consumer: bootstrap/register-hooks.js wires all 7 lifecycle events here.
```

---

## 2. Principles

| Principle | Rule |
|-----------|------|
| **Purpose first** | No symbol ships without a clear why, value, consumer, and falsifier. |
| **Single responsibility** | One concern per module/function. Adding a second concern = split or extract. |
| **DRY** | Single source of truth for mappings, defaults, and shared logic. |
| **Self-documenting names** | Names reveal intent. A comment explaining *what* something is = rename it. |
| **Constants over magic values** | No raw defaults, timeouts, or limits in logic. Named constants only. |
| **Observability** | Failures log at `logWarning` / `logError`. Happy path stays silent. |
| **Dead code zero** | No unused exports, no commented-out blocks, no unreachable branches. |
| **Small units** | Stay within thresholds (§ 4). Extract or split when approaching limits. |
| **Fallbacks only when real** | A fallback that can't deliver working behavior is noise. Omit it. |
| **Finish bounded refactors** | Rewire and remove the old path in the same PR. No shims, no dual paths. |
| **Single writer** | `sf-db.js` is the only file that issues write SQL. All others call its exports. |
| **Spec-first TDD** | Write the failing test before implementing. Test name = contract claim. |

---

## 3. Anti-Patterns

| Anti-pattern | Why | Required replacement | Rule |
|---|---|---|---|
| `throw new Error(...)` bare in business logic | Callers can't distinguish failure classes | Throw with a descriptive prefix: `throw new Error("session-recorder.initSessionRecorder: db unavailable")` | **STY001** |
| Silent `catch` swallowing | Hides breakage | `logWarning(module, msg)` then decide: re-throw or return explicit failure | **STY002** |
| Magic status strings inline | Spreads typo-prone comparisons | Named constant or exported string literal at definition site | **STY003** |
| Generic names: `utils`, `helpers`, `common`, `misc` | Unsearchable, no domain signal | Name by capability: `memory-source-store.js`, `embed-circuit.js` | **STY004** |
| `// TODO: fix later` without ticket / owner | Permanent invisible debt | Fix now, or add a dated `// TODO(owner): <why>` with `node scripts/tech-debt-scan.mjs` visibility | **STY005** |
| Calling `db.prepare(...)` outside `sf-db.js` | Breaks single-writer invariant | Add an exported wrapper in `sf-db.js` | **STY006** |
| Embedding logic in hook wiring | Blurs responsibilities; untestable | Extract to a purpose-named module; wire only the call in `register-hooks.js` | **STY007** |
| Docstring = "Helper." or no docstring | Purpose is invisible to RAG and reviewers | Full JSDoc with Purpose + Consumer (§ 1) | **STY008** |
| Bare `process.env.FOO` scattered in logic | Config not auditable or testable | Named constant + `loadXxxConfigFromEnv()` function with null-guard | **STY009** |
| Test name = `"test X"` / `"works"` | Not a contract claim | `what_when_expected` form: `claimUnit_whenLeaseExpired_returnsTrue` | **STY010** |
| Mechanical test (counts mocks, not behavior) | Breaks on refactors that don't change behavior | Test what the *consumer receives*; label implementation guards `// guard:` | **STY011** |
| Committing to `dist/` or `~/.sf/agent/` | Generated output, not source | `dist/` is gitignored build output; run `npm run copy-resources` to rebuild | **STY012** |

---

## 4. Thresholds

Two-tier: **Warn** = flag in review; **Error** = blocks merge.

| Metric | Warn | Error |
|--------|------|-------|
| Function lines | 50 | 75 |
| File lines | 800 | 1500 |
| Function arguments | 5 | 8 |
| Nesting depth | 4 | 6 |
| Dead code | 0 tolerance | — |
| `TODO`/`FIXME` count | per `tech-debt-scan.mjs` thresholds | — |

Infrastructure files (`sf-db.js`, generated schemas) may exceed file-line limits when extraction would harm clarity. Add a comment explaining why.

---

## 5. Naming

### Files

| Kind | Convention | Example |
|------|-----------|---------|
| Module | `kebab-case.js` | `session-recorder.js`, `memory-embeddings-llm-gateway.js` |
| Test | `kebab-case.test.mjs` / `.test.ts` | `sf-db-migration.test.mjs` |
| Prompt template | `kebab-case.md` | `execute-task.md` |
| Bootstrap/wiring | `register-hooks.js`, `init-*.js` | — |

### Functions and variables

- **Verb + noun**: `createGatewayEmbedFn`, `recordTurnStart`, `listUnembeddedMemoryIds`
- **No vague verbs alone**: not `run`, `do`, `handle` — add the object
- **No marketing words**: not `simple`, `unified`, `enhanced`, `smart`
- **Verbose over abbreviated**: `embeddingModel` not `embModel`; `queryInstruction` not `queryInstr`
- **Predicate booleans**: `embedCircuitIsOpen()`, `isDbAvailable()` — reads as a question

### Constants

| Pattern | Use for | Example |
|---------|---------|---------|
| `DEFAULT_*` | Default values | `DEFAULT_EMBEDDING_MODEL`, `DEFAULT_TIMEOUT_MS` |
| `MAX_*`, `MIN_*` | Bounds | `MAX_PER_INVOCATION`, `MIN_INTERVAL_MS` |
| `*_THRESHOLD` | Trigger limits | `EMBED_CIRCUIT_THRESHOLD` |
| `*_TO_*`, `*_MAP` | Domain A → B mappings | `UNIT_TYPE_TO_LABEL` |
| `ENV_*` | Env var name strings | `ENV_KEY`, `ENV_EMBED_MODEL` |
| `SCHEMA_VERSION` | Single integer, bumped per migration | — |

---

## 6. Patterns

### Single-writer DB

`sf-db.js` is the only file that prepares and executes write SQL. All other modules call exported wrappers. This makes the write surface auditable, testable, and migration-safe.

```js
// ✅ Correct — call the exported wrapper
import { upsertSession } from "./sf-db.js";
upsertSession({ id, cwd, branch });

// ❌ Wrong — raw SQL outside sf-db.js
const stmt = db.prepare("INSERT INTO sessions ...");
```

### Config from env

Always read env vars through a named `loadXxxConfigFromEnv()` function that returns `null` when required keys are absent (opt-in) or throws with a clear message (required).

```js
export function loadGatewayConfigFromEnv() {
  const keyEntry = firstEnvEntry(KEY_ALIASES);
  if (!keyEntry) return null; // opt-in: absent = no-op
  ...
  return { url, apiKey, embeddingModel, queryInstruction };
}
```

### Circuit breaker

When a remote dependency can stall (timeout), implement a circuit breaker that:
- Counts consecutive failures
- Opens for `CIRCUIT_OPEN_MS` after `THRESHOLD` failures
- Logs once per open period (throttled)
- Half-opens automatically after cooldown

See `embedCircuit` in `memory-embeddings-llm-gateway.js` as the reference.

### Asymmetric embeddings (Qwen3)

Qwen3-Embedding uses asymmetric retrieval. Always pass `instruction` for queries; omit for documents.

```js
// Query embedding — instruction required
const embedFn = createGatewayEmbedFn(cfg, { instruction: cfg.queryInstruction });

// Document/backfill embedding — no instruction
const embedFn = createGatewayEmbedFn(cfg);
```

### Hook wiring

`bootstrap/register-hooks.js` wires lifecycle events to module functions. Keep each hook body thin: import, call, done. No business logic in hooks.

```js
pi.on("agent_end", async (event) => {
  const text = event.messages?.at(-1)?.content?.find(b => b.type === "text")?.text ?? "";
  await recordTurnEnd(text);
});
```

### Test contracts

Test names are contract claims: `what_when_expected`.

```js
// ✅ Contract claim
test("claimUnit_whenLeaseExpired_returnsTrue", () => { ... });

// ❌ Not a contract
test("claimUnit works", () => { ... });
```

Three tiers:
1. **Behaviour contracts** — what the consumer receives. Primary. Spec.
2. **Degradation contracts** — what happens when dependencies fail (DB down, gateway unreachable).
3. **Implementation guards** — labelled `// guard:` — protect specific failure modes. Refactors may update these.

---

## 7. Documentation

### When to comment

- **Always**: exported symbols with non-trivial behavior (full JSDoc per § 1)
- **Rarely**: inline comments only when the *why* is genuinely non-obvious from reading the code
- **Never**: comments that restate what the code does; comments as TODO parking

### Keeping docs current

When you change behavior, update the JSDoc Purpose and Consumer in the same commit. A stale Purpose is worse than no Purpose — it actively misleads the next reader.

### Module headers

```js
// module-name.js — one-line description
//
// Purpose: why this module exists as a separable unit.
//
// Consumer: who imports this at runtime (or "internal" if only tests).
```

---

## See Also

- [AGENTS.md](AGENTS.md) — planning conventions, spec-first TDD, test naming
- [docs/adr/0000-purpose-to-software-compiler.md](docs/adr/0000-purpose-to-software-compiler.md) — foundational product contract
- [docs/SPEC_FIRST_TDD.md](docs/SPEC_FIRST_TDD.md) — test-first constitution
- [biome.json](biome.json) — linter config (`npm run lint`)
- [scripts/tech-debt-scan.mjs](scripts/tech-debt-scan.mjs) — TODO/FIXME threshold tracking