Code Quality

Pawrrtal Code Quality

Day-to-day patterns for readable, maintainable code across Pawrrtal's three stacks:

Hard gates are summarized in memory/semantic/DOMAIN_KNOWLEDGE.md and .agent/AGENTS.md; workflow in repo-operations; taste in memory/personal/PREFERENCES.md; path rules in .agent/rules/; Cursor plugin rules in .cursor/plugins/pawrrtal/rules/.

Quick Reference

Naming

Booleans

Verb prefix — no bare adjectives or nouns:

Never negative (isNotReady); invert. Derived booleans keep the prefix:

// Good
const canSubmit = isValid && !isLoading;

// Bad — bare adjective
const loading = query.status === "pending";

React callbacks

Match repo React rules: callback props are onSomething, local handlers are handleSomething. Do not mix them on the same surface.

Constants

UPPER_SNAKE_CASE vs camelCase depends on what the binding holds, not whether it is "config-related." See .cursor/plugins/pawrrtal/rules/clean-code/named-constants.mdc for magic-number extraction.

Descriptor names mirror the env setting in TypeScript style (Python database_url → databaseUrl). Keys in Config.all({ databaseUrl, sqliteDbFilename }) must match the descriptor const names. Add a Config suffix only when the bare name collides with a resolved value in the same scope.

const DEFAULT_SQLITE_DB_FILENAME = 'pawrrtal.db';

const databaseUrl = Config.string('DATABASE_URL').pipe(Config.withDefault(''));
const sqliteDbFilename = Config.string('SQLITE_DB_FILENAME').pipe(
  Config.withDefault(DEFAULT_SQLITE_DB_FILENAME),
);

export const DatabaseConfig: Config.Wrap<{ readonly filename: string }> = {
  filename: Config.all({ databaseUrl, sqliteDbFilename }).pipe(
    Config.map(({ databaseUrl: url, sqliteDbFilename: filename }) =>
      resolveFilename(url, filename),
    ),
  ),
};

User-facing strings

Tool names, buttons, status lines, and error copy are Title Case with spaces (List Folder, not list_folder). See user-facing-text.

Function Structure

Early returns

Guard clauses at the top, happy path at the bottom. Never bury the main path inside nested if/else. All if statements use braces. Nesting beyond three compound levels (if/for/while/try/switch) fails CI — extract helpers instead.

function finalizeTurn(turn: TurnState): TurnResult {
  if (!turn.isValid) {
    return TurnResult.invalid(turn.id);
  }
  if (turn.isCancelled) {
    return TurnResult.cancelled(turn.id);
  }
  if (!turn.hasProvider) {
    return TurnResult.missingProvider(turn.id);
  }

  const output = runPipeline(turn);
  return TurnResult.success(output);
}

Single responsibility

One job per function. Biome flags cognitive complexity via noExcessiveCognitiveComplexity — when it fires, extract; do not widen suppressions.

Delegate, don't duplicate

When an action and a guard share conditions, the action calls the guard:

export function publishConversation(conversation: Conversation): PublishResult {
  if (!canPublish(conversation)) {
    return PublishResult.error("Cannot publish");
  }
  return doPublish(conversation);
}

Parameters

• Max 3 positional parameters. A fourth value belongs in an options object.
• Destructure when reading 2+ properties from the same object. Single-property access does not need destructuring.
• No boolean positional parameters — they hide meaning at call sites. Use { immediate: true } instead.

Required payload uses input; optional config uses options. Event payloads may use payload; HTTP query objects may use params where the module already does.

Documentation

TSDoc / JSDoc (TypeScript)

Every function — exports and private helpers — gets a doc block that describes the interface (what callers pass and receive), never the implementation (algorithms, storage, downstream calls).

• Exported surfaces (components, hooks, lib helpers, Effect services): summary line plus @param for every parameter, @returns for non-void returns, and @template / @throws where they carry contract. Required even when types look obvious.
• Private helpers: single-line summary only — no @param/@returns boilerplate.
• Interface properties: inline /** */ above each field, not @property block tags.

Placement: directly above the declaration. Preserve existing docstrings when editing — update or delete only when the documented code changes. See .cursor/plugins/pawrrtal/rules/clean-code/preserve-documentation.mdc.

Brevity

Docstrings state the caller contract — what to pass and what comes back. They are not design docs, lesson notes, or commit messages.

Length budget

Do not put in docstrings

• Architecture diagrams, layer stack walkthroughs, or ASCII trees
• Tables that duplicate endpoints, method lists, or signatures the types already express
• Implementation roadmaps ("what to implement next"), lesson numbers, or phase labels
• Vendor file paths, line numbers, bean ids, or PR references
• Python-parity essays (link the source file in an ADR or test name if parity matters)
• Duplicate docs on the same symbol (pick file-level or export-level, not both)

When a longer note is justified

• A single non-obvious constraint the type system cannot express (e.g. "empty name becomes Untitled Project in service") — still cap at one sentence; put algorithmic WHY in a 1–2 line inline comment at the site, not a block above the function.
• OpenAPI description / summary on HttpApi endpoints — those are user-facing API docs, not code docstrings.

If deleting a docstring leaves nothing useful to say, delete it. Types and names are the default documentation.

Python docstrings

Public classes and functions get docstrings describing caller contract. Type hints on every function. Narrow except to specific types; log with context. See .cursor/plugins/pawrrtal/rules/clean-code/python-logging-exceptions.mdc.

Inline comments

Explain non-obvious WHY, never restate WHAT. If the code already reads clearly, delete the comment. Cap at 1–2 lines. No PR/ticket references — that belongs in the commit message.

Worth a comment: layer composition order that differs from sibling Effect.provide, duplicate-package hazards, vitest bundler workarounds, driver/SQL quirks, business rules the type system cannot express.

Not worth a comment: "provides auth middleware", "returns the list", anything the function name or type already states.

Type Declarations

Prefer type for first-party declarations. Use interface for object shapes that extend or need declaration merging, and for declare module. Unions, mapped, conditional, and derived types require type. Derive from source (z.infer, Pydantic models, SQLAlchemy schemas, Effect Schema.Type) instead of hand-duplicating fields.

No unsafe casts (as any, double casts). Fix the underlying type. No TypeScript enums — use unions or as const objects.

Return Types

Hooks, server actions, and Effect services follow the same rule: the signature documents the contract.

Immutability

Default to readonly for array and object parameters the function does not mutate:

export function summarizeMessages(messages: readonly Message[]): Summary;
export function applyTheme(theme: Readonly<ThemeTokens>): void;
export function shuffleIds(ids: string[]): void; // intentionally mutates

React: prefer immutable updates; avoid redundant mirrored state. See .cursor/plugins/pawrrtal/rules/clean-code/avoid-redundant-state.mdc.

File Organization

A concept is a component family, hook module, service, repo, or utility group. Multi-file features belong in a package directory (frontend/features/<feature>/, backend/app/providers/<provider>/, backend-ts/.../Modules/<Name>/) — not flat siblings with redundant prefixes.

Within a file: imports → types/constants → main exports → internal helpers. Blank lines between groups.

Split when:

• The file approaches 500 lines (CI fails above this).
• The filename needs "and".
• Tests need heavy setup for a single unit.
• You cannot describe the file in one sentence.
• Sentrux reports a layer violation — fix structure, do not bypass with clever imports.

Icons and SVGs live in dedicated files; components import them. Never inline glyph markup in feature components.

Readability

• Explaining variables. Name compound conditions (isRecentConversation, hasPendingToolCall) even when used once.
• Declare near first use. Do not stack unrelated const declarations at the top of a long function.

Tests

Test through public interfaces — behavior that should survive refactors. Every assertion encodes intent (what must stay true), not incidental implementation detail.

Agent-loop backend tests use scripted stream fns. Vitest strips types at runtime; run the suite after changing assertions.

Verifying Changes

A check counts only when its exit code proves the thing.

just check                              # Biome repo-wide (VCS-scoped — changed files only)
cd backend-ts && bun run check          # Biome on all backend-ts files (see Effect TS below)
cd frontend && bun run typecheck        # TS frontend
cd backend-ts && bun run typecheck      # Effect strangler (when touched)
just sentrux                            # architecture layers (when structure changes)
# plus scoped tests for the files you edited

• Gate on exit codes, not grepped tool output. Biome emits lint/complexity/noExcessiveCognitiveComplexity — pattern-matching stdout misses it.
• Lint changed files on the PR that introduces them. A violation can hide until the file is touched.
• Fix warnings in files you edit. Biome warnings and console warnings are latent errors — do not label them "pre-existing."
• When local and CI disagree, the clean CI run on the self-hosted runner is ground truth.

Effect TS (backend-ts/)

Repo-root just check runs Biome with VCS enabled, so it only lints git-changed files under backend-ts/. That can hide dozens of warnings in untouched files. Always run the workspace-local gate before claiming Effect TS is clean:

cd backend-ts && bun run check && bun run typecheck && bun run test

backend-ts/biome.json scopes Biome to the workspace (vcs.enabled: false) and relaxes two rules that conflict with our module layout:

Single effect install. Context.Service tags compare by reference. Nested node_modules/effect copies (e.g. api-core + apps/api each bundling their own) break HttpApi middleware wiring with Service not found: AuthenticationMiddlewareService. Fix:

• Hoist effect + @effect/* at backend-ts/package.json with overrides.
• Declare effect as a peer (plus dev) in @pawrrtal/api-core, not a runtime dependency.
• Vitest resolve.dedupe for Effect packages; ssr.noExternal: ['@effect/vitest'] so suite registration works under bun --bun vitest.

HttpApi test auth. Mirror production: pipe auth onto the handler layer (handlerLayer.pipe(Layer.provide(AuthMiddlewareStubLive))), same as HttpProjectsLive.pipe(Layer.provide([..., HttpAuthLive])). Sibling Effect.provide([auth, handler]) leaves middleware missing at route build time.

Test fixtures. Use new Project({...}) with UUID v4 ids (00000000-0000-4000-8000-…) — Schema rejects non-v4. Assert failures with Effect.exit + Cause.isFailReason, not Either (removed in Effect v4).

Config naming (Effect TS). Env strings inside Config.string('…') are UPPER_SNAKE; descriptor consts and Config.all keys are camelCase; shared literal defaults are DEFAULT_* / MAX_*. See Constants.

Inline comments (Effect TS). Add 1-line WHY only where the code diverges from the obvious pattern: duplicate-package hazards, layer composition order, vitest bundler quirks, SQL/driver workarounds. Do not narrate what Layer.provide does when the line already says it.

File layout. Tests live in apps/api/test/unit/** mirroring src/. No parallel test/Modules/ tree — it drifts and confuses typecheck scope.

Error Handling

Every async path handles failure and surfaces feedback — never fire-and-forget void promises.

If a function can fail, the return type or error channel says so. User-visible messages follow user-facing-text.