- Fix memory-embeddings-llm-gateway tests: add queryInstruction field to
expected config objects after loadGatewayConfigFromEnv was updated to
return it
- Add STYLEGUIDE.md: SF code standards adapted from ace-coder patterns
(purpose doctrine, principles, anti-patterns STY001-012, thresholds,
naming, patterns, documentation sections)
- Phase 2 /sf prefix removal: update all web components, browser dispatch,
and tests to use direct commands (/autonomous, /stop, /next, /discuss,
/init, /new-milestone) instead of /sf-prefixed forms
- workflow-actions.ts: all command strings updated
- chat-mode.tsx: SF_ACTIONS array updated
- project-welcome.tsx: primaryCommand values updated
- command-surface.tsx: fallback display updated
- remaining-command-panels.tsx: usage examples updated
- browser-slash-command-dispatch.ts: add stop/new-milestone/init to
SF_PASSTHROUGH_COMMANDS so they route correctly to the extension
- recovery-diagnostics-service.ts: suggestion commands updated
- welcome-screen.ts: hint text updated
- All affected tests updated to match new command strings
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
11 KiB
SF Code Standards
Code patterns for AI-assisted development. Full rules: AGENTS.md · Planning contract: 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 | The #1 rule: every symbol must answer why it exists |
| 2. Principles | Core coding principles |
| 3. Anti-Patterns | Blocked patterns and required replacements |
| 4. Thresholds | Code quality limits |
| 5. Naming | Naming conventions |
| 6. Patterns | Architectural patterns |
| 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
/**
* 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
// 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:
embeddingModelnotembModel;queryInstructionnotqueryInstr - 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.
// ✅ 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).
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_MSafterTHRESHOLDfailures - 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.
// 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.
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.
// ✅ Contract claim
test("claimUnit_whenLeaseExpired_returnsTrue", () => { ... });
// ❌ Not a contract
test("claimUnit works", () => { ... });
Three tiers:
- Behaviour contracts — what the consumer receives. Primary. Spec.
- Degradation contracts — what happens when dependencies fail (DB down, gateway unreachable).
- 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
// 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 — planning conventions, spec-first TDD, test naming
- docs/adr/0000-purpose-to-software-compiler.md — foundational product contract
- docs/SPEC_FIRST_TDD.md — test-first constitution
- biome.json — linter config (
npm run lint) - scripts/tech-debt-scan.mjs — TODO/FIXME threshold tracking