diff --git a/.claude/agents/validator.md b/.claude/agents/validator.md new file mode 100644 index 00000000..a6124ecb --- /dev/null +++ b/.claude/agents/validator.md @@ -0,0 +1,52 @@ +--- +name: validator +description: Runs the repository quality gates (prettier, eslint, tsc, jest, build, markdownlint) and fixes what fails. Use proactively after any logic change and before reporting work complete. +tools: Bash, Read, Edit, Write, Grep, Glob +background: false +color: green +--- + +You run this repository's quality gates and return a verdict. A full run emits verbose Jest output and a Next build, so the point of running here is that the calling context receives a short report instead of several thousand lines. + +## Gates + +Run `npm run validate`, which chains all seven gates, and capture the exit code with `echo "EXIT: $?"`. Do not judge it by reading its output. + +If it fails partway, run the remaining gates individually so every one is exercised before you report: + +1. `npm run prettier` +2. `npm run eslint` +3. `npm run tsc` +4. `npm run test:jest` +5. `npm run test:cypress:e2e` +6. `npm run build` +7. `npm run lint:markdown` + +The chain is `&&`, so a failure at position 5 means `build` and `lint:markdown` never ran. Never treat those as passed. + +If `test:cypress:e2e` fails, quote the actual error. Treat it as an environment limit only when the Cypress **binary fails to launch**, an Electron or window-server error raised before any spec runs, since Cypress needs a GUI session a headless agent shell may not have. A failing assertion inside a spec is a real failure. Either way, report which gates actually ran (see [`code-qa.yaml`](../../.github/workflows/code-qa.yaml) for what CI covers). + +Two ordering notes. `npm run prettier` and `npm run eslint` both write; run Prettier again after any ESLint fix, because the `curly` fix inserts braces inline where Prettier would break the statement across lines. Finish with `npm run prettier:check`, which is what CI runs. + +`npm run test:jest` carries `--passWithNoTests`, so exit code 0 alone does not prove tests ran. Report the test count. + +## Fixing + +Fix the cause, not the symptom. Specifically: + +- Never weaken, skip, or delete a test to make a gate pass. Read the test, read the source, find the cause. See [`testing.md`](../rules/testing.md). +- Never add a fallback in production code to satisfy a failing test. +- Never silence a type error with `any`, `unknown`, `@ts-ignore`, or an `eslint-disable`. Replace it with a concrete type. See [`code-style.md`](../rules/code-style.md). +- Re-run the failing gate after each fix, then re-run the gates that precede it if your fix touched files they check. + +If a failure is pre-existing and unrelated to the change under test, fix it anyway when it is small, and report it plainly when it is not. Do not present it as passing. + +## Reporting + +Do not stage, commit, push, or otherwise touch git state. + +Return a short report, not a transcript: + +- One line per gate: name, exit code, and the test or file count where the gate reports one. +- For each failure that survives: the file and line, the cause in one sentence, and what you changed. +- A final verdict line: every gate at exit code 0, or the list of gates still failing. diff --git a/.claude/hooks/validate-gate.mts b/.claude/hooks/validate-gate.mts new file mode 100644 index 00000000..8253970c --- /dev/null +++ b/.claude/hooks/validate-gate.mts @@ -0,0 +1,219 @@ +#!/usr/bin/env node +// Enforces the validation mandate in CLAUDE.md: a session that changed code, tests, +// config, or docs may not finish until the quality gates have been run. +// +// Registered on three events in `.claude/settings.json`: +// PostToolUse (Write|Edit|MultiEdit|Bash) - mark the session dirty, record gates run +// SubagentStop (validator) - credit every gate to the delegated run +// Stop - block once while gates are outstanding +// +// The hook is structural, never evaluative. It records whether a gate was *run*, not +// whether it *passed*, because inferring pass or fail from tool output would either nag +// after a clean run or clear after a red one. Confirming exit codes is the agent's job. +// +// Run via `node --experimental-strip-types` (no build step, no dependencies). +// Type-stripping-safe TypeScript only: type annotations / interfaces, no enums, +// namespaces, or parameter properties. + +import { mkdirSync, readFileSync, rmSync, writeFileSync } from 'fs'; +import { tmpdir } from 'os'; +import { isAbsolute, join, relative } from 'path'; + +interface ToolInput { + file_path?: string; + command?: string; +} + +interface HookPayload { + hook_event_name?: string; + session_id?: string; + cwd?: string; + tool_name?: string; + tool_input?: ToolInput; + stop_hook_active?: boolean; +} + +interface GateState { + dirty: boolean; + gates: string[]; +} + +/** Every gate `npm run validate` runs that can also run on a developer machine. */ +const GATES = ['prettier', 'eslint', 'tsc', 'jest', 'build', 'markdown']; + +/** Maps a shell command to the gates it runs. */ +const GATE_PATTERNS: [RegExp, string[]][] = [ + [/npm run validate\b/, GATES], + [/npm run prettier\b/, ['prettier']], + [/npm run eslint\b/, ['eslint']], + [/npm run tsc(?![:\w])/, ['tsc']], + [/npm run test:jest\b/, ['jest']], + [/npm run build\b/, ['build']], + [/npm run lint:markdown\b/, ['markdown']], +]; + +const REMINDER = + 'This change requires validation. Before you finish, run the quality gates and confirm ' + + 'each reaches exit code 0: `npm run prettier`, `npm run eslint`, `npm run tsc`, ' + + '`npm run test:jest`, `npm run build`, `npm run lint:markdown`. Check the actual exit ' + + 'code rather than scrolling the output, and fix any failure rather than reporting around ' + + 'it. Delegating the run to the `validator` subagent keeps the output out of this context.'; + +/** Path to this session's marker, kept outside the repository so it never reaches `git status`. */ +function statePath(sessionId: string): string { + return join(tmpdir(), 'claude-validate-gate', `${sessionId}.json`); +} + +function readState(sessionId: string): GateState | null { + try { + return JSON.parse(readFileSync(statePath(sessionId), 'utf-8')) as GateState; + } catch { + return null; + } +} + +function writeState(sessionId: string, state: GateState): void { + const target = statePath(sessionId); + + try { + mkdirSync(join(tmpdir(), 'claude-validate-gate'), { recursive: true }); + writeFileSync(target, JSON.stringify(state), 'utf-8'); + } catch { + // A marker that cannot be written degrades the gate to a no-op rather than + // breaking the session. + } +} + +function clearState(sessionId: string): void { + try { + rmSync(statePath(sessionId), { force: true }); + } catch { + // Nothing to clean up. + } +} + +/** + * Whether editing this file should require validation. + * + * Markdown counts because `lint:markdown` is one of the gates. The agent-tooling tree is + * excluded: `.claude/` is ignored by Prettier, ESLint, and markdownlint alike, so no gate + * can fail because of it. + */ +function requiresValidation(filePath: string, cwd: string): boolean { + if (!filePath) return false; + + const rel = isAbsolute(filePath) ? relative(cwd, filePath) : filePath; + + if (rel.startsWith('..') || rel.startsWith('.claude/') || rel.includes('/.claude/')) return false; + + if (rel.startsWith('src/') || rel.startsWith('cypress/') || rel.startsWith('jest/')) return true; + + if (rel.endsWith('.md')) return true; + + // Root-level configuration: `package.json`, `eslint.config.js`, `next.config.js`, and so on. + return !rel.includes('/') && /\.(ts|tsx|js|mjs|cjs|json)$/.test(rel); +} + +/** Records gates run by a shell command, or marks the session dirty after an edit. */ +function handlePostToolUse(payload: HookPayload, sessionId: string): void { + const toolName = payload.tool_name ?? ''; + + if (toolName === 'Bash') { + const state = readState(sessionId); + + if (!state?.dirty) process.exit(0); + + const command = payload.tool_input?.command ?? ''; + const matched = GATE_PATTERNS.filter(([pattern]) => pattern.test(command)).flatMap(([, gates]) => gates); + + if (matched.length === 0) process.exit(0); + + writeState(sessionId, { dirty: true, gates: [...new Set([...state.gates, ...matched])] }); + process.exit(0); + } + + if (toolName !== 'Write' && toolName !== 'Edit' && toolName !== 'MultiEdit') process.exit(0); + + if (!requiresValidation(payload.tool_input?.file_path ?? '', payload.cwd ?? process.cwd())) { + process.exit(0); + } + + const wasClean = !readState(sessionId)?.dirty; + + // A fresh change invalidates whatever was validated before it, so the gate list resets. + writeState(sessionId, { dirty: true, gates: [] }); + + // Remind once per clean-to-dirty transition rather than on every edit. + if (wasClean) { + process.stdout.write( + JSON.stringify({ + hookSpecificOutput: { + hookEventName: 'PostToolUse', + additionalContext: REMINDER, + }, + }), + ); + } + + process.exit(0); +} + +/** Blocks once while gates are outstanding, then releases so the gate can never deadlock. */ +function handleStop(payload: HookPayload, sessionId: string): void { + const state = readState(sessionId); + + if (!state?.dirty) process.exit(0); + + const missing = GATES.filter((gate) => !state.gates.includes(gate)); + + if (missing.length === 0) { + clearState(sessionId); + process.exit(0); + } + + // `stop_hook_active` means this hook already blocked this turn. Releasing keeps a gate + // that cannot be satisfied (Cypress will not launch on macOS here) from looping. + if (payload.stop_hook_active) process.exit(0); + + process.stderr.write( + `This session changed code, tests, config, or docs, and ${missing.length} of ${GATES.length} ` + + `quality gates have not been run: ${missing.join(', ')}. Run \`npm run validate\` and confirm ` + + 'it reaches exit code 0 before finishing. The chain is `&&`, so if it stops partway, the gates ' + + 'after the failure did not run: finish them individually (`npm run prettier`, `npm run eslint`, ' + + '`npm run tsc`, `npm run test:jest`, `npm run build`, `npm run lint:markdown`) rather than ' + + 'treating them as passed.', + ); + process.exit(2); +} + +function main(): void { + let payload: HookPayload; + + try { + payload = JSON.parse(readFileSync(0, 'utf-8')) as HookPayload; + } catch { + process.exit(0); + } + + const sessionId = payload.session_id ?? ''; + + if (!sessionId) process.exit(0); + + const event = payload.hook_event_name ?? ''; + + if (event === 'PostToolUse') handlePostToolUse(payload, sessionId); + + if (event === 'SubagentStop') { + const state = readState(sessionId); + + if (state?.dirty) writeState(sessionId, { dirty: true, gates: [...GATES] }); + + process.exit(0); + } + + if (event === 'Stop') handleStop(payload, sessionId); + + process.exit(0); +} + +main(); diff --git a/.claude/rules/code-style.md b/.claude/rules/code-style.md index e0f8ccdc..54683265 100644 --- a/.claude/rules/code-style.md +++ b/.claude/rules/code-style.md @@ -6,21 +6,24 @@ paths: # Code style -These rules mirror [`.github/copilot-instructions.md`](../../.github/copilot-instructions.md) (the Copilot-side source); keep both in sync. Enforced in CI by `npm run prettier:check`, `npm run eslint:check`, and `npm run tsc`. +These rules mirror [`.github/copilot-instructions.md`](../../.github/copilot-instructions.md) (the Copilot-side source); keep both in sync. Enforced in CI by `npm run prettier:check`, `npm run eslint:check`, and `npm run tsc`. Testing rules live in [`testing.md`](testing.md); every change ends with `npm run validate` at exit code 0, per [`CLAUDE.md`](../../CLAUDE.md). ## Formatting -- **Tabs, not spaces** for indentation (ESLint errors otherwise). -- Semicolons required; single quotes (including JSX); `printWidth` 120; trailing commas everywhere. +- **Tabs, not spaces** for indentation, enforced by Prettier (`useTabs`, `tabWidth` 4 in [`.prettierrc`](../../.prettierrc)). +- Semicolons required; single quotes including JSX (`jsxSingleQuote`); `printWidth` 120; trailing commas everywhere. - Imports are sorted automatically by `@trivago/prettier-plugin-sort-imports` - don't hand-order them. -- Prefix intentionally-unused variables/args with `_` (e.g. `_event`) so ESLint ignores them. +- Prefix intentionally-unused variables/args with `_` (e.g. `_event`), which `no-unused-vars` ignores. This is a deliberate departure from the Google guide, which bans `_` on identifiers. +- ESLint applies **one rule set to every file**, JavaScript and TypeScript alike, parsed by `@typescript-eslint/parser` (see [`eslint.config.js`](../../eslint.config.js)). ## Imports - **Always use path aliases, never relative paths.** Aliases are defined in [`tsconfig.json`](../../tsconfig.json) and mirrored in [`jest.config.js`](../../jest.config.js): `@/`, `@components/`, `@configs/`, `@constants/`, `@data/`, `@helpers/`, `@images/`, `@layouts/`, `@styles/`, `@util/`. -- Example: `import Avatar from '@components/banner/Avatar';` - not `'../banner/Avatar'`. +- Example: `import Avatar from '@components/banner/Avatar';` - not `'../banner/Avatar'`. Tests importing their own subject are the one exception; see [`testing.md`](testing.md). - Import Node built-in modules with the **bare specifier** (`import { readFileSync } from 'fs'`), never the `node:` prefix (`'node:fs'`). Matches the existing convention - e.g. `require('util')` in [`jest/setup.ts`](../../jest/setup.ts). +- Use `import type { Foo }` when a symbol is used only as a type, and `export type { Foo }` when re-exporting one. [`tsconfig.json`](../../tsconfig.json) sets `isolatedModules`, which requires the latter. +- Export style follows the kind of module. Components, layouts, App Router route files such as [`page.tsx`](../../src/app/page.tsx), the data modules such as [`projects.ts`](../../src/data/projects.ts), and [`theme.ts`](../../src/styles/theme.ts) default-export their subject. Configs, constants, helpers, utilities, instrumentation, and the SVG components in [`icons.tsx`](../../src/images/icons.tsx) use named exports. One module can carry both: [`layout.tsx`](../../src/app/layout.tsx) default-exports `RootLayout` beside named `metadata` and `viewport`. Never `export let`. ## Components & styling @@ -32,3 +35,54 @@ These rules mirror [`.github/copilot-instructions.md`](../../.github/copilot-ins - Strict mode is on; types must be explicit (no implicit `any`). - Do **not** "fix" an existing `any` by swapping it to `unknown` or adding an `eslint-disable` - replace it with a specific concrete type, and respect an `any` that is intentional. + +### Google style guide + +Follow the [Google TypeScript Style Guide](https://google.github.io/styleguide/tsguide.html) except where this file or the framework overrides it. The rules below are the ones Prettier and ESLint do not already cover; the `/google-ts-style` skill holds the fuller digest for a deliberate style pass. + +- `UpperCamelCase` for types and components, `lowerCamelCase` for values, `CONSTANT_CASE` for module-level constants and enum values. +- Treat acronyms as words: `loadHttpUrl`, not `loadHTTPURL`. +- `===` and `!==` always, except `== null` when both `null` and `undefined` should match. +- Annotate object literals (`const config: Foo = { ... }`) rather than asserting them (`{ ... } as Foo`). The assertion suppresses excess-property checking. +- `as` and `!` are unsafe. Prefer a runtime check, and say why in a comment when one is not possible. Use `as`, never the angle-bracket form. +- No `@ts-ignore` or `@ts-nocheck`. `@ts-expect-error` is permitted in tests only, with a comment. +- `interface` for object shapes, not a `type` alias of an object literal. +- Optional properties and parameters (`href?: string`) rather than `href: string | undefined`. Add nullability at the use site, not inside a type alias. +- `T[]` for simple element types, `Array` for complex ones. Never `String`, `Number`, or `Boolean` as types. +- Throw only `Error` or a subclass, always via `new Error(...)`. An empty `catch` needs a comment saying why. +- Prefer `for...of`; never unfiltered `for...in`. + +Not adopted from the guide: `snake_case` filenames (this repo uses kebab-case directories with PascalCase components), the ban on `_` identifier prefixes (unused arguments require it here), and any expectation of mandatory return-type annotations (the guide leaves that to the author). + +## Readability + +Prefer the readable form wherever it costs nothing at runtime. + +- Braced blocks for anything that is not a single-line early exit. `if (!data) return;` may stay unbraced on one line, as may `break`, `continue`, and `throw`. Everything else takes `{ }`, including a single-statement body that spans lines. `curly` enforces this. +- A blank line before `return`, `break`, `continue`, and `throw` when it is not the first statement in its block. `padding-line-between-statements` enforces this for `return`, `continue`, and `throw`; `break` is written discipline, because the rule cannot tell a loop `break` from a `switch` `break`. +- No blank lines between `switch` cases. +- Separate groups that do different work with a blank line: setup, action, assertion; or fetch, transform, render. +- JSX props sorted alphabetically, or grouped by purpose (identity, data, behaviour, styling). Choose one per component and do not mix the two. + +`npm run validate` runs Prettier before ESLint, and the `curly` fix inserts braces inline. After any ESLint fix sweep, run `npm run prettier` again and finish with `npm run prettier:check`, which is what CI runs. + +## Comments + +- **Comments describe the code as it stands.** Never narrate a change, a fix, or a prior state ("now uses", "changed to", "previously", "no longer", "restored"). Git history and pull requests carry that, and the comment outlives the change that prompted it. +- **Never argue that the code is correct or safe.** A note defending a decision, such as "`createStars` is declared below and is already initialized by the time this callback runs", documents the edit rather than the code. Say what something does or why it exists; do not justify that it works. +- A comment that contradicts the code is corrected, not deleted. When the two disagree, the code is the truth. +- Delete commented-out code rather than leaving it in place. +- Redundancy is not a defect on a public surface, but inside a function body a comment that restates the line beneath it is noise. Delete those; keep anything carrying a constraint, hazard, or non-obvious behaviour. +- Compiler and tooling directives are never comments to delete: `//@ts-check`, `/// `, `// @ts-expect-error`, and `eslint-disable` lines. + +## JSDoc + +- **Every exported symbol carries a [JSDoc](https://jsdoc.app/) `/** */` block, without exception**, and so do the members of an exported structure: interface properties, object keys, enum values. Write for a reader meeting it for the first time. Reach for what the signature cannot express (why it exists, a constraint, an invariant, a caller obligation); where nothing better exists, a plain restatement is correct. Being obvious is not a defect on a public surface; being absent is. +- A private helper gets a block when its behaviour is not evident from its name and signature. A binding declared inside a function body does not: the name and type already carry it. +- **In a block you are writing, do not put types in JSDoc.** TypeScript ignores `@param {string}`, `@returns {number}`, `@type`, and `@typedef` in `.ts`/`.tsx` files, so they become prose that drifts from the signature. Skip `@implements`, `@enum`, `@private`, and `@override` beside the corresponding keyword too, and add `@param`/`@returns` lines where they say more than the name and type already do. +- **Leave existing tags alone unless they are wrong.** A `@param` or `@returns` already in the tree was added deliberately, annotation and all. Read the surrounding code, correct what is factually wrong, and change nothing else: do not strip a `{type}` annotation, reword accurate prose, or delete a tag for looking redundant. Delete one only when it is wrong and uncorrectable, such as documenting a parameter the signature no longer has. +- `@throws`, `@example`, `@deprecated`, and `@see` are encouraged: none of them are expressible in the type system. `@deprecated` names its replacement. +- Open a function or component block with a third-person verb phrase ("Returns the parsed config"), not an imperative. +- One tag per line, tag at line start. A block stays on one line until it overflows, at which point `/**` and `*/` move to their own lines. Bodies are Markdown, so an enumeration needs a real list rather than indented text. +- **No Markdown link syntax in JSDoc.** `[text](url)` is Markdown's, not JSDoc's, and `[name](#anchor)` is worse still: there is no document to anchor into, so it renders as dead text. JSDoc has its own forms, so use them. Reference a symbol with `{@link SvgIconProps}`, which TypeScript resolves through its symbol table into working hover and Go to Definition. Point at an external page with `@see https://example.com`, or inline it as `{@link https://example.com Display text}`. +- `//` line comments for implementation notes; a multi-line note uses consecutive `//` lines. No `/* */` block inside a function body, with one exception: naming an argument at a call site, `someFunction(/* shouldRender= */ true)`. diff --git a/.claude/rules/docs-authoring.md b/.claude/rules/docs-authoring.md index 2e2d5c56..ff9e3f1e 100644 --- a/.claude/rules/docs-authoring.md +++ b/.claude/rules/docs-authoring.md @@ -24,14 +24,18 @@ When creating or editing any markdown file, follow the discipline below. These a - Document a tunable value by the **name a consumer changes it by** (env var, config key, CLI flag, or a named member of a centralized constants/config module that other code reads), judging by role, not location. Don't document an ephemeral local variable as the config surface. - **Acronyms** in prose you write or edit use capitals (ID, URL) and are expanded on first use per doc ("Deoxyribonucleic acid (DNA)"). Keep exact casing for brand/tool/package names (npm, iOS), domain terms (snRNA), and direct code references (an `id` field). - No placeholders, TODOs, or empty "add details here" sections. +- A document opens with a single H1 named for its file, then a one to three sentence introduction for a reader who does not yet know the subject, then H2s. Headings are unique and fully descriptive ("Retry backoff limits", not "Limits"), because anchors are generated from them, and use sentence case. +- **Tables only for uniform data scanned quickly.** If columns repeat across rows, cells sit empty, or a cell holds a sentence of prose, use a list instead. +- Prefer Markdown to raw HTML for layout or styling. +- Link text names the destination: never "here", "link", "this", or a bare URL. ## Links & code -- Every file reference is a **clickable markdown link to a file**, never a bare filename and never a link to a directory. Link to a file inside the directory (e.g. its `index.md`/`README.md`) instead. +- Every file reference is a **clickable markdown link to a file**, never a bare filename and never a link to a directory. Link to a file inside the directory (e.g. its `index.md`/`README.md`) instead. A generic reference, where no particular file is meant, is a code span rather than a link: "update your `README.md`". - Use relative links (GitHub-compatible) and verify the path resolves from the doc's own location. - Don't paste full definitions/class bodies; link to the file. Inline snippets only for a short usage example, a critical config line, or logic that text can't convey (3-10 lines max). ## Mermaid -- Every diagram **must** include both `accTitle` (specific) and `accDescr` (a real description, not "a diagram showing…"). No exceptions. +- Every diagram **must** include both `accTitle` (specific) and `accDescr` (a real description, not "a diagram showing…"). No exceptions. Images are held to the same bar: real alt text, never "screenshot" or "diagram". - Valid Mermaid only; reflect actual current code; pick the diagram type that fits (don't default to `flowchart`). diff --git a/.claude/rules/testing.md b/.claude/rules/testing.md new file mode 100644 index 00000000..ef8829ea --- /dev/null +++ b/.claude/rules/testing.md @@ -0,0 +1,116 @@ +--- +paths: + - "**/*.test.ts" + - "**/*.test.tsx" + - "cypress/**/*.ts" + - "jest/**/*.ts" + - "jest.config.js" + - "cypress.config.ts" +--- + +# Testing + +Jest with [`@testing-library/react`](https://testing-library.com/docs/react-testing-library/intro) covers units; Cypress with `cypress-axe` covers end-to-end and accessibility. Style rules that are not test-specific live in [`code-style.md`](code-style.md). + +## Test mandate + +- **Logic changes, bug fixes, and new features land with their tests in the same change.** The test asserts the specific behaviour the change introduces or repairs, so it locks the change against regression. +- **Pure refactors, renames, and file moves need no new tests, but every existing test must still pass.** A diff that skips or weakens a test is a behaviour change, not a refactor. +- Run `npm run validate` before reporting any change complete. See [`CLAUDE.md`](../../CLAUDE.md). + +## One test file per source file + +Colocated, same name: [`Banner.tsx`](../../src/components/banner/Banner.tsx) gives [`Banner.test.tsx`](../../src/components/banner/Banner.test.tsx). No orphan test file without a same-named source beside it, no test file named after a function (`useProjectHover.test.ts` for a hook that lives in another file), and no second test file for one source. + +Exempt from the rule: static data modules such as [`projects.ts`](../../src/data/projects.ts), type-only modules, metadata route exports ([`manifest.ts`](../../src/app/manifest.ts), [`robots.ts`](../../src/app/robots.ts)), and instrumentation entry points. Components are **not** exempt. + +## Never + +- Skip, gut, or delete a failing test. Read the test, read the source, find the cause, fix it, confirm it passes with real assertions. +- Use `it.skip`, `describe.skip`, or `test.skip`. Remove a skipped test rather than leaving it. +- Write no-op assertions (`expect(true).toBe(true)`), assertions that restate the implementation, or type assertions of already-typed values (`expect(typeof name).toBe('string')` where `name: string`). +- Build a one-row `it.each` table. Make it a plain `it()`. +- Add a fallback in production code (`?? defaultValue`) to make a test pass. Fix the test. + +Every test answers one question: what behaviour does this lock in that a real future change could break? If the answer is nothing, delete it. + +## Mocking + +**The default is not to mock.** A mock is a claim about how a dependency behaves, written by the person whose code is under test, and it keeps passing after the real dependency changes. Every one you add subtracts from what the test proves. A test whose collaborators are all mocked asserts only that mocks were called. + +This applies to every substitution technique, not just `jest.mock`: stubs, fakes, spies that replace behaviour, hand-written doubles, and monkey-patching a module's export. + +**Reach for a mock only when the real thing cannot run in the test.** Exhaust these first, in order: + +1. Use the real implementation with real inputs. Most helpers, utilities, hooks, and components run fine in jsdom. +2. Pass a value in rather than replacing a module. A function that takes its dependency as an argument needs no mock. +3. Build a real object or fixture and assert on the real output. +4. Move the assertion to a level where the seam is real, or cover it in Cypress instead. + +**Never mock code that holds logic**, whoever wrote it. Helpers, utilities, domain logic, components, hooks, constants, and the modules in [`src/data/`](../../src/data/projects.ts) are exercised for real. Never mock the subject under test, in whole or in part: a partial mock of the module you are testing means the test no longer tests it. + +**Mock only at an input/output boundary**, and only the outermost one the test needs. The permitted boundaries are closed, and each is here because the real thing cannot run in jsdom: + +| Boundary | What that means here | +| --- | --- | +| A third-party SDK that reaches the network | `firebase/app`, `firebase/analytics`, and `firebase/performance`, mocked in [`firebase.test.ts`](../../src/configs/firebase.test.ts) because the wrapper under test sits directly on them | +| This repo's own wrapper around such an SDK, when testing a consumer of it | [`@configs/firebase`](../../src/configs/firebase.ts) from a component test, so rendering does not fire live analytics | +| Framework context the test renderer cannot supply | `next/navigation` | +| The clock | `jest.useFakeTimers()`, which replaces the environment rather than your code | +| Browser APIs jsdom omits | `navigator` and similar | + +A wrapper qualifies only because its whole job is to reach the outside world. That is the narrow exception to the rule above, not a licence to mock a repo module that computes something. + +Anything outside that table needs a one-line comment above the mock naming which boundary it crosses. If you cannot write that sentence, the mock is not justified: use the real thing. + +**Never mock to make a failing test pass.** A mock introduced while chasing a red test is hiding the failure, not fixing it. + +Retrieve mock state with `jest.requireMock('@configs/firebase').logAnalyticsEvent` or `usePathname as jest.MockedFunction`, never with `require()`. + +`next/image` is left unmocked, and is the pattern to follow. It rewrites `src` through its loader, so the test asserts with `expect.stringContaining('profile_pic_drawn.webp')` rather than mocking the component to get an exact path. + +## Naming + +`describe('')` names the component or module; `it('')` names the behaviour: + +```tsx +describe('ProjectsGrid', () => { + it('logs analytics on project hover and click', () => {}); +}); +``` + +New titles do not start with "should". Titles already written that way are grandfathered; do not rewrite them in an unrelated change. A second sibling `describe` separates a distinct concern (`describe('ProjectsGrid responsive columns')`). + +## Table-driven tests + +Use `it.each` when rows vary input and expected output across the **same** code path, as [`ProjectsGrid.test.tsx`](../../src/components/projects/ProjectsGrid.test.tsx) does for breakpoints: + +```tsx +it.each([ + { breakpoint: 'sm', expectedColumns: 2, minWidth: '600px' }, + { breakpoint: 'md', expectedColumns: 3, minWidth: '900px' }, +] as const)('renders $expectedColumns columns from $minWidth ($breakpoint)', ({ expectedColumns, minWidth }) => {}); +``` + +Name every field; no positional rows. Rows that differ in the assertion body rather than the data belong in separate `it()` blocks, because a table whose rows each run different code is a noisier loop. + +## House patterns + +- `render` and `screen` from `@testing-library/react`. Use `fireEvent`; `@testing-library/user-event` is not a dependency. +- Shared setup goes in `beforeEach(() => { jest.clearAllMocks(); render(); })`. +- Debounced or delayed behaviour uses `jest.useFakeTimers()` in `beforeEach` with `jest.runOnlyPendingTimers()` then `jest.useRealTimers()` in `afterEach`. +- Assert accessibility through roles and accessible names (`screen.getByRole('button', { name: /view more projects/i })`), label text, and `aria-*` attributes. `jest-axe` is not installed; axe runs in Cypress. +- Wrap the subject in [`ThemeRegistry`](../../src/components/ThemeRegistry.tsx) when the assertion depends on the theme, and render it bare when it does not. +- **Import carve-out:** the subject under test is imported relatively (`import Banner from './Banner';`) while collaborators use path aliases (`@components/ThemeRegistry`). This is the one documented exception to the alias rule in [`code-style.md`](code-style.md). Prettier's `importOrder` places the relative import last automatically. + +## Cypress + +Specs live in [`cypress/e2e/`](../../cypress/e2e/landing.cy.ts). Every `describe` closes with `afterEach(() => { cy.a11yCheck(); })`, the custom command defined in [`commands.ts`](../../cypress/support/commands.ts). No `baseUrl` is configured, so specs call `cy.visit('http://localhost:3000')` directly. Header and policy checks use `cy.request({ failOnStatusCode: false })` rather than driving the UI. + +## Running tests + +- One file: `npx jest src/components/banner/Banner.test.tsx` +- One case: `npx jest -t 'partial title'` +- `npm run test:jest` carries `--passWithNoTests`, so a green run alone does not prove any test executed. Confirm the reported test count. +- End to end: `npm run test:cypress:e2e` runs headless against a dev server; `npm run test:cypress:open` opens the runner. CI runs the headless form, see [`code-qa.yaml`](../../.github/workflows/code-qa.yaml). +- If `test:cypress:e2e` fails, quote the actual error. Treat it as an environment limit only when the Cypress **binary fails to launch**, an Electron or window-server error raised before any spec runs, since Cypress needs a GUI session that a headless agent shell may not have. A failing assertion inside a spec is a real failure and is never environmental. diff --git a/.claude/settings.json b/.claude/settings.json index 585928f4..f034dee7 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -11,6 +11,39 @@ "statusMessage": "Checking docs-authoring rules" } ] + }, + { + "matcher": "Write|Edit|MultiEdit|Bash", + "hooks": [ + { + "type": "command", + "command": "node --disable-warning=ExperimentalWarning --experimental-strip-types \"$CLAUDE_PROJECT_DIR/.claude/hooks/validate-gate.mts\"", + "statusMessage": "Tracking validation state" + } + ] + } + ], + "SubagentStop": [ + { + "matcher": "validator", + "hooks": [ + { + "type": "command", + "command": "node --disable-warning=ExperimentalWarning --experimental-strip-types \"$CLAUDE_PROJECT_DIR/.claude/hooks/validate-gate.mts\"", + "statusMessage": "Recording validation run" + } + ] + } + ], + "Stop": [ + { + "hooks": [ + { + "type": "command", + "command": "node --disable-warning=ExperimentalWarning --experimental-strip-types \"$CLAUDE_PROJECT_DIR/.claude/hooks/validate-gate.mts\"", + "statusMessage": "Checking the validation gate" + } + ] } ] } diff --git a/.claude/skills/google-ts-style/SKILL.md b/.claude/skills/google-ts-style/SKILL.md new file mode 100644 index 00000000..54fe1706 --- /dev/null +++ b/.claude/skills/google-ts-style/SKILL.md @@ -0,0 +1,95 @@ +--- +name: google-ts-style +description: Digest of the Google TypeScript Style Guide with this repository's Next.js carve-outs marked. Use for a deliberate style pass over TypeScript, or to settle a style question that .claude/rules/code-style.md does not answer. +--- + +# Google TypeScript style + +Digest of the [Google TypeScript Style Guide](https://google.github.io/styleguide/tsguide.html), limited to rules that Prettier and ESLint do not already enforce here. The day-to-day subset lives in [`code-style.md`](../../rules/code-style.md); this file is the fuller reference. + +Anything Prettier settles (quotes, semicolons, line width, blank lines at block edges, import order, trailing commas) is out of scope: run `npm run prettier`, do not hand-adjust. + +## Carve-outs, read first + +Four of Google's rules do not apply here. Do not "fix" code to match them. + +- **Default exports.** Google bans them. This repository uses them for components, layouts, App Router route files such as [`page.tsx`](../../../src/app/page.tsx), the data modules such as [`projects.ts`](../../../src/data/projects.ts), and [`theme.ts`](../../../src/styles/theme.ts). Configs, constants, helpers, utilities, instrumentation, and the SVG components in [`icons.tsx`](../../../src/images/icons.tsx) use named exports. +- **Filenames.** Google specifies `snake_case`. This repository uses kebab-case directories with PascalCase component files ([`cookie-snackbar/CookieSnackbar.tsx`](../../../src/components/cookie-snackbar/CookieSnackbar.tsx)). +- **Underscore prefixes.** Google bans `_` on identifiers. Here, intentionally unused variables and arguments require it, because [`eslint.config.js`](../../../eslint.config.js) configures `no-unused-vars` with `argsIgnorePattern: '^_'` and `varsIgnorePattern: '^_'`. +- **Return-type annotations.** Google leaves these to the author rather than mandating them. Treat them as optional and add them where a complex return benefits. + +## Naming + +- `UpperCamelCase` for classes, interfaces, types, enums, decorators, and type parameters. +- `lowerCamelCase` for variables, parameters, functions, methods, properties, and module aliases. +- `CONSTANT_CASE` for module-level constants and enum values that are genuinely immutable, not for every `const`. +- Treat acronyms as words: `loadHttpUrl`, not `loadHTTPURL`. +- Names must be clear to a new reader. Do not abbreviate by deleting letters. Variables in scope for ten lines or fewer may use short names. +- A local alias of an existing symbol keeps the original's naming format. + +## Type system + +- Rely on inference. Omit annotations for values initialized to a literal or a `new` expression. +- `interface` for object shapes, not a `type` alias of an object literal. +- Use interfaces rather than classes to define structural types, and state the type at the symbol's declaration. +- Optional fields and parameters (`href?: string`) rather than `href: string | undefined`. +- Do not bake `| null` or `| undefined` into a type alias; add nullability at the use site. +- Google expresses no preference between `null` and `undefined`. This codebase uses `undefined`, matching React and Next.js. +- Avoid `any`. Provide a concrete type, or use `unknown` and narrow it with a type guard. Never launder an existing `any` into `unknown` to quiet a linter, and respect an `any` that is deliberate. +- Do not use `{}` as a type. Use `unknown`, `object`, or `Record`. +- `T[]` and `readonly T[]` for simple element types; `Array` for anything more complex. +- Give index signatures a meaningful key label (`{ [userName: string]: number }`), and consider `Map`, `Set`, or `Record` instead. +- Use the simplest type construct that expresses the code. Repetition costs less than clever conditional and mapped types. +- Never `String`, `Boolean`, or `Number` as types or constructors. Use the lowercase primitives. +- Avoid APIs whose generic appears only in the return type; when consuming one, pass the generic explicitly. + +## Assertions and suppressions + +- `as` and `!` are unsafe. Prefer a runtime check, and comment why when one is impossible. +- Use `as`, never the angle-bracket form. +- Annotate object literals (`const config: Foo = { ... }`) rather than asserting them. The assertion silences excess-property checking, which is where this rule earns its keep. +- No `@ts-ignore` or `@ts-nocheck`: a specific compiler error usually signals a larger problem. `@ts-expect-error` is permitted in tests, with a comment. + +## Imports and exports + +- Named exports, exporting only what is used outside the module. The default-export carve-out above overrides this for components, layouts, route files, data modules, and the theme. +- `export let` is not allowed; expose a getter instead. +- `import type` when a symbol is used only as a type; `export type` when re-exporting one. [`tsconfig.json`](../../../tsconfig.json) sets `isolatedModules`, which requires the latter. +- Prefer named imports for frequently used symbols; prefer a namespace import when pulling many symbols from a large API. +- Renaming on import (`{ X as Y }`) is fine for collisions or clarity. +- Side-effect-only loads use `import '...';`. +- This repository additionally requires path aliases over relative paths, and bare specifiers over the `node:` prefix. See [`code-style.md`](../../rules/code-style.md). + +## Language features + +- `const` by default, `let` when reassignment is needed, never `var`. One variable per declaration. +- `===` and `!==` always, except `== null` when both `null` and `undefined` should match. +- Braced blocks for control flow. See the readability rules in [`code-style.md`](../../rules/code-style.md), which are narrower than Google's. +- Every `switch` has a `default`, placed last, and non-empty groups do not fall through. +- Prefer `for...of`. Never unfiltered `for...in`; use `Object.keys()` or a `hasOwnProperty` check. +- Spread objects into objects and arrays into arrays only; never spread a primitive, `null`, or `undefined`. +- Prefer function declarations for named functions; use arrow functions rather than function expressions. Use a concise arrow body only when the return value is used. +- Classes should not hold properties initialized to arrow functions, which obscures `this`. +- Convert types with `String()`, `Boolean()`, `Number()`, template literals, or `!!`, never with `new`. Do not use unary `+` for string to number. Check for `NaN` explicitly. Reserve `parseInt` for non-decimal bases. +- Do not write an explicit boolean coercion where the context already coerces, such as an `if` or `while` condition. Enum values are the exception: compare them explicitly. +- No `const enum`. No `eval`, `with`, `debugger` in production, builtin prototype modification, `Array()` or `Object()` constructors, `require()` imports, or `namespace Foo {}`. +- Do not set non-numeric properties on an array; use a `Map` or an object. + +## Errors + +- Prefer throwing exceptions to ad hoc error handling. +- Always `new Error()`, never `Error()`. +- Throw only `Error` or a subclass; other values carry no stack trace. +- Treat caught values as `Error`, narrowing where needed. Doing nothing in a `catch` is rarely correct and requires a comment explaining why. +- Keep the body of a `try` small where that does not hurt readability. + +## Comments and JSDoc + +- `/** JSDoc */` for what a consumer of the code needs to know. `//` for implementation notes. Multi-line implementation comments use several `//` lines, never a `/* */` block. +- Document all top-level exports. +- Do not restate types in JSDoc. TypeScript ignores `@param {string}`, `@returns {number}`, `@type`, `@typedef`, `@implements`, and `@enum` in `.ts` and `.tsx` files, so they become prose that drifts from the signature. +- `@param` and `@returns` lines are required only where they add information beyond the name and type. A block with a good summary and no tags is idiomatic. +- Do not use `@override`; the compiler does not enforce it, so it drifts. +- Document a non-obvious argument at the call site with a block comment (`foo(/* silent= */ true)`), or prefer an options object with destructuring. +- Write the JSDoc block before a decorator, not between the decorator and the declaration. +- JSDoc is Markdown. Use `-` lists rather than whitespace alignment, and give each tag its own line. diff --git a/.claude/skills/write-tests/SKILL.md b/.claude/skills/write-tests/SKILL.md new file mode 100644 index 00000000..4908f034 --- /dev/null +++ b/.claude/skills/write-tests/SKILL.md @@ -0,0 +1,44 @@ +--- +name: write-tests +description: Author or repair a Jest or Cypress test in this repository's house style. Use when adding a test, when a source change needs coverage, or when a test is failing and needs a root-cause fix rather than a weakened assertion. +--- + +# Write tests + +The rules are in [`.claude/rules/testing.md`](../../rules/testing.md); this skill is the procedure for applying them. Style rules that are not test-specific are in [`code-style.md`](../../rules/code-style.md). + +**Scope.** One test file per source file, colocated and same-named. Adding a test never means adding a second test file for a source that already has one. + +## Phases (run in order) + +### 1. Read before writing + +Read the source **and** its existing test. A failing test needs both before you touch either: read the test, read the source, then name the cause. If the source is at fault, fix the source. Weakening the assertion, adding `.skip`, or adding a fallback in production code to make the test pass are all prohibited. + +Check whether the file is exempt from needing a test at all (static data, type-only modules, metadata route exports). Components are never exempt. + +### 2. Choose the shape + +Use `it.each` when rows vary input and expected output across the same code path; name every field, and never write a one-row table. Use a plain `it()` when the cases differ in what they assert rather than in their data, because a table whose rows run different code is a noisier loop. + +Title the `describe` after the subject and the `it` after the behaviour, in third person: `renders the ProjectsGrid title`. Do not open a new title with "should". + +### 3. Assert behaviour + +Every test answers one question: what behaviour does this lock in that a real future change could break? If the answer is nothing, do not write it. + +Reach for roles and accessible names (`getByRole('button', { name: /view more projects/i })`) before test IDs. Do not assert the types of already-typed values, restate the implementation, or write `expect(true).toBe(true)`. + +### 4. Do not mock + +Start from zero mocks and add one only when the real dependency cannot run in the test. Before writing any substitute, whether a mock, stub, fake, or behaviour-replacing spy, try in order: the real implementation with real inputs; passing the dependency in as an argument; a real fixture asserted on its real output; or moving the assertion to a level where the seam is real. + +Never mock code that holds logic (helpers, utilities, domain logic, components, hooks, constants, `src/data/`), and never mock the subject under test, even partially. Mock only at an input/output boundary, and only the outermost one the test needs: a third-party SDK that reaches the network, this repo's own wrapper around one when testing a consumer of it, framework context the renderer cannot supply (`next/navigation`), the clock, and browser APIs jsdom omits. Anything else needs a comment above it naming which boundary it crosses; if you cannot write that sentence, use the real thing. + +A mock added while chasing a red test hides the failure rather than fixing it. Read mock state with `jest.requireMock(...)` or `as jest.MockedFunction`, never `require()`. + +### 5. Validate + +Run the file first (`npx jest path/to/file.test.tsx`), then the suite (`npm run test:jest`), and confirm each exit code with `echo "EXIT: $?"`. `--passWithNoTests` means exit code 0 alone does not prove your test ran, so check the reported test count. + +Then run the rest of the gates per [`CLAUDE.md`](../../../CLAUDE.md), or delegate to the `validator` subagent. diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 442a6d4e..9cf78e84 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -24,14 +24,25 @@ npm run test:cypress:e2e # E2E tests headless npm run build # Production build ``` -**Always run `npm run validate` before committing.** This is the comprehensive quality gate used in CI/CD. +**Always run `npm run validate` before committing**, and frequently while making changes. This is the quality gate CI runs. ### Testing Requirements -- **Unit tests**: Every component requires a `.test.tsx` file (see `src/components/banner/Banner.test.tsx`) -- **Test setup**: Uses `jest/setup.ts` with jsdom environment -- **E2E tests**: Located in `cypress/e2e/`, include accessibility tests (cypress-axe) -- **Coverage**: Run `npm run test:jest:coverage` for coverage reports +- **One test file per source file**, colocated and same-named: `Banner.tsx` gives `Banner.test.tsx` (see `src/components/banner/Banner.test.tsx`). No orphan tests, no test file named after a function, no second test file for one source. +- **Logic changes, bug fixes, and new features land with their tests in the same change.** Pure refactors need no new tests, but every existing test must still pass. A diff that skips or weakens a test is a behaviour change, not a refactor. +- **Exempt** from needing a test: static data (`src/data/*`), type-only modules, metadata routes (`manifest.ts`, `robots.ts`), instrumentation entry points. Components are not exempt. +- **Never** skip, gut, or delete a failing test: read the test, read the source, fix the cause. No `it.skip`/`describe.skip`, no `expect(true).toBe(true)`, no assertions that restate the implementation, no `expect(typeof x).toBe('string')` on an already-typed value, no one-row `it.each` table, and never add a `?? fallback` in production code to make a test pass. +- **Do not mock.** Start from zero and add one only when the real dependency cannot run in the test. This covers every substitution technique: mocks, stubs, fakes, behaviour-replacing spies, hand-written doubles. A test whose collaborators are all mocked asserts only that mocks were called. Try first, in order: the real implementation with real inputs; passing the dependency in as an argument instead of replacing a module; a real fixture asserted on its real output; moving the assertion to where the seam is real, or into Cypress +- **Never mock code that holds logic**, whoever wrote it: helpers, utilities, domain logic, components, hooks, constants, `src/data/*`. Never mock the subject under test, even partially +- **Mock only at an I/O boundary**, outermost one needed, from this closed list: a third-party SDK that reaches the network (`firebase/*` in the wrapper's own test); this repo's wrapper around one when testing a consumer (`@configs/firebase` from a component test); framework context the renderer cannot supply (`next/navigation`); the clock (`jest.useFakeTimers()`); browser APIs jsdom omits (`navigator`). A wrapper qualifies only because its whole job is reaching the outside world. Anything else needs a comment naming which boundary it crosses; if you cannot write that sentence, use the real thing +- **Never mock to make a failing test pass** - that hides the failure. Read mock state with `jest.requireMock(...)` or `as jest.MockedFunction`, never `require()` +- **Naming**: `describe('')` plus `it('')`, e.g. `renders the ProjectsGrid title`. New titles do not start with "should"; existing ones are grandfathered. +- **Table-driven**: use `it.each` when rows vary input and expected output across the same code path, and name every field. Rows that differ in the assertion body belong in separate `it()` blocks. +- **House patterns**: `render`/`screen` from `@testing-library/react`; `fireEvent` (`@testing-library/user-event` is not a dependency); `beforeEach(() => { jest.clearAllMocks(); render(); })`; fake timers via `jest.useFakeTimers()` with `jest.runOnlyPendingTimers()` in `afterEach`. Assert accessibility through roles and accessible names. `next/image` is not mocked, so assert its rewritten `src` with `expect.stringContaining`. +- **Import carve-out**: the subject under test is imported relatively (`./Banner`) while collaborators use path aliases. This is the one exception to the alias rule below. +- **Setup and E2E**: `jest/setup.ts` with the jsdom environment; Cypress specs in `cypress/e2e/`, every `describe` closing with `afterEach(() => { cy.a11yCheck(); })` (cypress-axe). No `baseUrl` is set, so specs call `cy.visit('http://localhost:3000')`. +- `npm run test:jest` carries `--passWithNoTests`, so exit code 0 alone does not prove a test ran. Check the reported count. +- **Coverage**: `npm run test:jest:coverage`. No threshold is configured, so it is a report rather than a gate. ## Project-Specific Conventions @@ -47,6 +58,10 @@ import { DELAYS } from '@constants/index'; import { isNetworkFast } from '@util/isNetworkFast'; ``` +Also: import Node built-ins with the **bare specifier** (`import { readFileSync } from 'fs'`), never the `node:` prefix. Use `import type { Foo }` when a symbol is used only as a type, and `export type { Foo }` when re-exporting one (`isolatedModules` is on). + +Export style follows the kind of module. Components, layouts, App Router route files (`src/app/page.tsx`), the data modules (`src/data/projects.ts`), and `src/styles/theme.ts` default-export their subject. Configs, constants, helpers, utilities, instrumentation, and the SVG components in `src/images/icons.tsx` use named exports. One module can carry both: `src/app/layout.tsx` default-exports `RootLayout` beside named `metadata` and `viewport`. Never `export let`. + ### Component Patterns #### Material-UI Styling @@ -93,20 +108,25 @@ THRESHOLDS.SNEEZE_TRIGGER_INTERVAL; // Easter egg triggers NETWORK.SLOW_DOWNLINK_THRESHOLD; // Network performance checks ``` +The module also exports `ANIMATIONS` and `MAX_STARS`. + ## Code Style & Quality ### Linting & Formatting -- **Indentation**: Tabs (not spaces) - enforced by ESLint +- **Indentation**: Tabs (not spaces) - enforced by Prettier (`useTabs`, `tabWidth` 4 in `.prettierrc`) +- **Quotes and punctuation**: single quotes including JSX (`jsxSingleQuote`), semicolons required, trailing commas everywhere - **Line length**: Prettier wraps at `printWidth` 120 (`.prettierrc`) - **Import sorting**: Handled by `@trivago/prettier-plugin-sort-imports` -- **Unused vars**: Prefix with `_` to ignore (e.g., `_unusedParam`) ### ESLint Rules (see `eslint.config.js`) -- Tabs for indentation (indent: ['error', 'tab']) +**One rule set for every file**, JavaScript and TypeScript alike, parsed by `@typescript-eslint/parser`: + +- Tabs for indentation (indent: ['error', 'tab']) and required semicolons - Console logs allowed (`no-console: off`) -- Unused vars with `_` prefix ignored +- Unused vars are an error, with `_`-prefixed names ignored (e.g. `_unusedParam`) +- `curly` and `padding-line-between-statements` enforce the readability rules below ### TypeScript @@ -114,6 +134,38 @@ NETWORK.SLOW_DOWNLINK_THRESHOLD; // Network performance checks - **No implicit any**: All types must be explicit - **React 19**: Uses new `react-jsx` transform - Run `npm run tsc` to check types (no emit) +- Do **not** "fix" an existing `any` by swapping it to `unknown` or adding an `eslint-disable`. Replace it with a concrete type, and respect an `any` that is intentional. + +Follow the [Google TypeScript Style Guide](https://google.github.io/styleguide/tsguide.html) except where this file or the framework overrides it. The deltas that matter (fuller digest in [`google-ts-style`](../.claude/skills/google-ts-style/SKILL.md)): + +- **Naming**: `UpperCamelCase` types and components, `lowerCamelCase` values, `CONSTANT_CASE` module-level constants and enum values. Acronyms are words: `loadHttpUrl`, not `loadHTTPURL` +- **Types**: `interface` for object shapes, not a `type` alias of an object literal; optional properties (`href?: string`) over `href: string | undefined`, with nullability added at the use site; `T[]` for simple element types and `Array` for complex ones; never `String`, `Number`, or `Boolean` as types +- **Assertions**: annotate object literals (`const config: Foo = { ... }`) rather than asserting them, since `as` suppresses excess-property checking. `as` and `!` are unsafe, so prefer a runtime check and say why in a comment when one is impossible. Use `as`, never angle brackets +- **Suppressions**: no `@ts-ignore` or `@ts-nocheck`. `@ts-expect-error` is permitted in tests only, with a comment +- **Control flow**: `===` and `!==` always, except `== null` when both `null` and `undefined` should match. Prefer `for...of`, never unfiltered `for...in` +- **Errors**: throw only `Error` or a subclass, always via `new Error(...)`. An empty `catch` needs a comment saying why + +Not adopted: `snake_case` filenames (kebab-case directories with PascalCase components here), the ban on `_` identifier prefixes (unused arguments require it), and mandatory return-type annotations. + +### Readability + +- Braced blocks for anything that is not a single-line early exit. `if (!data) return;` may stay unbraced on one line, as may `break`, `continue`, and `throw`; everything else takes `{ }` +- A blank line before `return`, `break`, `continue`, and `throw` when it is not the first statement in its block +- No blank lines between `switch` cases +- Separate groups that do different work with a blank line: setup, action, assertion; or fetch, transform, render +- JSX props sorted alphabetically, or grouped by purpose (identity, data, behaviour, styling). Choose one per component and do not mix + +### Comments & JSDoc + +- **Comments describe the code as it stands.** Never narrate a change, fix, or prior state ("now uses", "previously", "no longer", "restored"); git history carries that. Never argue that the code is correct or safe, which documents the edit rather than the code. Delete commented-out code. A comment contradicting the code is corrected, not deleted +- **Every exported symbol carries a `/** */` block, without exception**, as do the members of an exported structure (interface properties, object keys, enum values). Write for a reader meeting it for the first time; where nothing beyond a restatement is true, restate. Being obvious is not a defect on a public surface, being absent is +- A private helper gets a block when its name and signature do not carry it; a binding inside a function body does not, and a comment there that restates the next line is noise +- **In a block you write, do not put types in JSDoc.** TypeScript ignores `@param {string}`, `@returns {number}`, `@type`, and `@typedef` in `.ts`/`.tsx`, so they drift from the signature. Skip `@implements`, `@enum`, `@private`, and `@override` beside the keyword, and add `@param`/`@returns` where they say more than the name and type do +- **Leave existing tags alone unless wrong.** A `@param`/`@returns` already in the tree was added deliberately, annotation and all. Read the surrounding code, fix what is factually wrong, change nothing else: do not strip a `{type}`, reword accurate prose, or delete a tag for looking redundant. Delete only when wrong and uncorrectable, such as documenting a parameter the signature no longer has +- `@throws`, `@example`, `@deprecated` (naming its replacement), and `@see` are encouraged: none are expressible in the type system. Open a block with a third-person verb phrase; one tag per line; bodies are Markdown +- **No Markdown link syntax in JSDoc.** `[text](url)` is Markdown's, not JSDoc's, and `[name](#anchor)` has no document to anchor into so it renders as dead text. Reference a symbol with `{@link SvgIconProps}`, which TypeScript resolves into working hover and Go to Definition; point at an external page with `@see https://example.com` or `{@link https://example.com Display text}` +- `//` for implementation notes, consecutive `//` for multi-line. No `/* */` inside a function body except to name an argument at a call site: `someFunction(/* shouldRender= */ true)` +- Never delete a directive: `//@ts-check`, `/// `, `// @ts-expect-error`, `eslint-disable` ## Next.js App Router Specifics @@ -184,8 +236,10 @@ Architecture docs in `docs/architecture/`: - `index.md`: System overview - Component-specific docs for Avatar, Projects, Publications, etc. -## Common Gotchas +When writing or editing any Markdown, the canonical spec is [`audit-docs.prompt.md`](prompts/audit-docs.prompt.md). The always-apply subset: -1. **Don't** import from relative paths - use path aliases -2. **Don't** forget to update tests when changing components -3. **Always** run `npm run validate` frequently when making changes +- **Zero hallucination**: document only what the code provably does. Know the file that proves a claim before writing it +- **No em-dashes or en-dashes**: replace each with a comma, parenthesis, colon, separate sentence, or a spaced hyphen, including existing ones in any file you edit +- **Canadian English** for prose you write or change (colour, behaviour, standardize), never for code identifiers, config keys, or package names +- **No subjective adjectives** (important, robust, seamless). State the fact that would earn the adjective +- Every file reference is a clickable Markdown link to a **file**, never a bare filename or a directory, and every Mermaid diagram carries both `accTitle` and `accDescr` diff --git a/.github/prompts/audit-docs.prompt.md b/.github/prompts/audit-docs.prompt.md index af55b1b9..2566a8f3 100644 --- a/.github/prompts/audit-docs.prompt.md +++ b/.github/prompts/audit-docs.prompt.md @@ -11,17 +11,15 @@ labels: Act as a **Strictly Factual Technical Writer and Auditor**. Make the `docs/` directory an objective, verifiable reflection of the current #codebase. Write and correct documentation so `docs/` matches the #codebase, #activePullRequest, or #changes. Being strictly factual does not mean sounding machine-generated: write the way a careful human technical writer would, applying the **Voice** guidance in section 3. -**Scope: documentation only.** Unless the invoking task explicitly asks for code or behaviour changes, this run edits documentation (markdown, text files, and in-code comments, docstrings, JSDoc, module headers) and never changes executable code or behaviour. See Rule 1. +**Scope: documentation only.** Unless the invoking task explicitly asks for code or behaviour changes, this run edits documentation (markdown, text files, and in-code comments, docstrings, and file-level headers) and never changes executable code or behaviour. See Rule 1. **Core philosophy:** -- **Reporter, not editor.** Convert code facts into documentation. Do not editorialize, which means do not make value judgments you cannot cite. Objective means no unverified claims. -- **Document value, not narration.** Code is self-documenting for _what_ it does; docs must add what code cannot show: _why_ something exists (decisions, constraints, trade-offs), _how_ parts interact (boundaries, data flows, integration points), and _when_ to use it (context, prerequisites). If a sentence only restates the code, cut it. _Exception:_ consumer-facing API/tool docs must state _what_ the code does, since external readers cannot see the source. +- **Reporter, not editor.** Convert code facts into documentation. Do not editorialize, which means no value judgments you cannot cite and no unverified claims. +- **Document value, not narration.** Code is self-documenting for _what_ it does; `docs/` prose must add what code cannot show: _why_ something exists (decisions, constraints, trade-offs), _how_ parts interact (boundaries, data flows, integration points), and _when_ to use it (context, prerequisites). If a sentence only restates the code, cut it. _Exception:_ consumer-facing API/tool docs must state _what_ the code does, since external readers cannot see the source. - **Link, do not duplicate.** Point to source files; never copy code into markdown. -**Dual audience:** every document serves internal developers (maintaining the architecture) and/or external developers (consuming the APIs/tools). Prefer content useful to both. Serve human skimmers and LLM/coding-assistant readers with the same prose: use one canonical term per concept (no synonym-swapping for the same thing), and replace an ambiguous `it`/`this`/`these` with the actual noun when the referent could drift. - -**Tone:** approachable for concepts, precise for details, objective always (Rule 3). The register stays formal and neutral, never stiff or machine-like; see **Voice** in section 3. Do not add contractions or a conversational register. +**Audience and tone:** every document serves internal developers maintaining the architecture and external developers consuming the APIs, so prefer content useful to both. Serve human skimmers and coding-assistant readers with the same prose: one canonical term per concept, and an ambiguous `it`/`this`/`these` replaced by the actual noun when the referent could drift. Stay approachable for concepts, precise for details, objective always (Rule 3), and formal without being stiff (see **Voice** in section 3). No contractions. --- @@ -31,15 +29,13 @@ Execute all three phases in order. ### Phase 1: PR sync -- **Condition:** only if #activePullRequest or #changes exist. -- Treat the PR diff as the **source of truth**. Identify code-level changes (added, removed, modified behaviour). +- **Condition:** only if #activePullRequest or #changes exist. Treat the diff as the **source of truth** and identify code-level changes (added, removed, modified behaviour). - **Update `docs/`** to document those changes, even where the PR did not touch docs. Document only behaviour the PR changed. - **Output:** state whether you made changes or found docs already accurate. ### Phase 2: general audit -- Audit all of `docs/` against the current #codebase. -- **Correct** pre-existing content that contradicts the code. Preserve accurate content's phrasing and style. +- Audit all of `docs/` against the current #codebase. **Correct** pre-existing content that contradicts the code, preserving accurate content's phrasing and style. - **Delete** pre-existing content only if it is massively duplicated, describes removed features, or fundamentally cannot be corrected. Default to correcting, not deleting. Your own generated content may be edited or removed freely when wrong. - **Create new files** only when needed: check the existing structure first and reuse a home when one fits; for a genuinely new directory apply the **Diátaxis** framework (Tutorials, How-To Guides, Reference, Explanation); create for new components/systems, external API guides, or missing structures. - **Output:** state whether you made changes or found docs already accurate. @@ -48,9 +44,17 @@ Execute all three phases in order. **Mandatory.** Execute regardless of Phase 1 and 2 results. -- **Scope:** every `.md` file outside `docs/`, plus docstrings, JSDoc, inline comments, and module headers across the target. -- **Actions:** scan the target for documentation and comments; read the current implementation of each documented element; verify it against actual code behaviour; update or remove anything inaccurate or outdated; add missing docs only for exported/public APIs that lack them or for complex internal logic a maintainer could not follow; remove bloat (over-verbose AI comments, narration of obvious code), keeping only "why" explanations, non-obvious "what" descriptions, and essential "how" for complex algorithms. -- **Standards:** exported elements get a concise docstring (what, params, returns, only where non-obvious from the names); internal elements are documented only for complex logic, gotchas, or edge cases; inline comments only for non-obvious business logic, workarounds, or complex transformations. Remove noise such as the comment `// Increment counter` above `counter++` (delete the comment, keep the `counter++`), `@param id - The id`, verbose AI docstrings, outdated comments, and orphaned TODO comments. +- **Scope:** every `.md` file outside `docs/`, plus documentation comments, inline comments, and file-level headers across the target. +- **Actions:** scan for documentation and comments; read the current implementation of each documented element; verify it against actual code behaviour; correct or remove anything inaccurate or outdated; document every public symbol that lacks it; remove bloat, keeping "why" explanations, non-obvious "what" descriptions, and essential "how" for complex algorithms. Removing bloat means deleting comments that restate the code, never comments that explain a non-obvious internal. +- **Always document the public surface.** Every public or exported symbol carries a documentation comment, without exception, as do the members of a public structure: fields, properties, keys, enum values. Write for a reader meeting the symbol for the first time, assuming they can infer nothing from its name. Reach for what the declaration cannot express, such as why it exists, a constraint, an invariant, or a caller obligation. Where no such explanation exists, a plain restatement of what the symbol does is correct: being obvious is not a defect on a public surface, being absent is. **Rule 2 still governs.** This rule obliges you to read the implementation, never to infer a description from the symbol's name. If you cannot verify what it does, say so in your output and leave it undocumented rather than writing a plausible guess, which is how drift starts. +- **Do not restate what the language's own syntax declares**, such as a type, a visibility modifier, or an override marker. This governs what you write in a **new** documentation comment and never licenses removing an existing one. +- **Correct an existing documentation tag; do not strip or delete it.** A parameter, return, throws, or example entry was written deliberately. Read enough surrounding code to judge it, then fix what is factually wrong and leave what is right, including parts a convention would omit in new code. Removing a tag, or a piece of one, because it looks redundant is restyling someone else's work, not auditing it. Delete a whole tag only when it is wrong and uncorrectable, such as one documenting a parameter the signature no longer has. Phase 2's "default to correcting, not deleting" governs in-code documentation too. +- **Internal elements** are documented where the logic is complex or carries a gotcha or edge case. Delete an internal comment only when it restates the line beneath it, such as `// Increment counter` above a counter increment (delete the comment, keep the code). +- **Comments describe the code as it stands.** Never narrate a change, a fix, or a prior state ("now uses", "previously", "no longer", "restored"): version control carries that, and the comment outlives the change that prompted it. Never argue that the code is correct or safe, which documents the edit rather than the code. Delete commented-out code rather than leaving it in place. +- **Form:** a documentation comment is a complete sentence, capitalized and punctuated; a short trailing comment may be a fragment. Wrap long comment lines to the width the file already uses, letting an unbreakable URL exceed it. Use the documentation format's own list syntax for enumerations, since indented plain text collapses into one run-on sentence when rendered. Never box a comment in asterisks or other decorative characters. Documentation precedes an annotation or decorator and never sits between it and the declaration. +- **Contracts worth stating:** any cleanup the caller owns (a handle to close, a listener to remove, a subscription to cancel), the error values or exception types a caller can branch on, and a deprecation marker naming its replacement. A deprecation without migration directions is incomplete; add one only where it is provable under Rule 2. +- **File-level headers:** where the language provides one, it states the file's contents, uses, or dependencies. Notes aimed at maintainers rather than consumers go with the implementation instead. +- **Also remove:** outdated comments and orphaned TODO comments. - **Output:** list the files changed and the kinds of change, or state "Phase 3: audited in-code documentation across X files, all accurate, no changes required." --- @@ -59,7 +63,7 @@ Execute all three phases in order. ### Rule 1: Documentation only (no behaviour changes) -Edit **documentation, never code behaviour**. In scope: markdown, text files, and in-code documentation (comments, docstrings, JSDoc, module headers). Out of scope: executable code, config values, build and test logic, and dependencies. Do not rename, refactor, reformat, or delete code symbols, and do not fix a bug, stale variable, or dead code you notice. Editing a comment is allowed; changing the code it describes is not. A stale comment is fixed by correcting the comment, not the code. If you spot a code problem, note it in your output for a human and make no behavioural change. +Edit **documentation, never code behaviour**. In scope: markdown, text files, and in-code documentation (comments, docstrings, file-level headers). Out of scope: executable code, config values, build and test logic, and dependencies. Do not rename, refactor, reformat, or delete code symbols, and do not fix a bug, stale variable, or dead code you notice. Editing a comment is allowed; changing the code it describes is not. A stale comment is fixed by correcting the comment, not the code. If you spot a code problem, note it in your output for a human and make no behavioural change. **Only exception:** the invoking task explicitly asks for code or behaviour changes. Absent that, this run is documentation-only. @@ -81,16 +85,15 @@ Every statement must be grounded in code you have **opened and read in full duri ### Rule 3: Strict objectivity - **Correct falsehoods.** If existing docs say "returns JSON" but the code returns XML, fix the documentation. -- **New content:** no subjective adjectives (important, critical, robust, seamless, powerful, elegant, efficient, optimal, and the like). State facts. +- **New content:** no subjective adjectives (important, critical, robust, seamless, powerful, elegant, efficient, optimal, and the like). State facts. _Bad:_ "The `auth.ts` middleware is a critical component." _Good:_ "The `auth.ts` middleware blocks unauthorized requests." - **Objective is not flat.** Banning subjective adjectives does not mandate robotic prose. Replace the adjective with the concrete cited fact that earns it: not "the retry logic is robust" but "the retry runs three times with a two-second backoff ([retry.ts](../src/retry.ts) lines 12-19)." (show, do not tell) - **Existing content:** preserve existing subjective terms unless they are factually wrong. -- _Bad (AI):_ "The `auth.ts` middleware is a critical component." _Good (AI):_ "The `auth.ts` middleware blocks unauthorized requests." ### Rule 4: No placeholders or TODOs No empty sections, stubs, or "add details here" comments. If the code does not exist, the documentation should not either. -### Rule 5: Mermaid accessibility (zero tolerance) +### Rule 5: Mermaid diagram and image accessibility (zero tolerance) Every Mermaid diagram MUST include both: @@ -99,6 +102,8 @@ Every Mermaid diagram MUST include both: No exceptions. Do not output any diagram missing either field. +Images are held to the same bar: every image carries alt text conveying what it shows. Generic alt text ("screenshot", "diagram") fails exactly as an absent `accDescr` does. Use an image only where showing is easier than describing. + --- ## 3. Writing Guidelines @@ -107,18 +112,16 @@ No exceptions. Do not output any diagram missing either field. Write as a careful human technical writer: formal and neutral, never robotic. The robotic feel comes from the tells below, not from a formal register, so cut the tells and keep the register. -- **Lead with the point.** Put the conclusion, answer, or action in the first sentence; detail follows. -- **Show, do not tell.** Demonstrate with a command, number, cited line, or named edge case instead of asserting significance. -- **Vary sentence length where natural.** Do not force a cadence target; reference and spec material may run uniform. +- **Lead with the point**, putting the conclusion, answer, or action in the first sentence. **Show, do not tell:** demonstrate with a command, number, cited line, or named edge case instead of asserting significance. Vary sentence length where natural, without forcing a cadence target. - **Avoid these AI tells** (representative, not exhaustive): signposting previews ("This section covers", "In this section we will"); puffery copulas ("serves as", "stands as", "is a testament to", "plays a vital/pivotal role"); the rule-of-three triad as a default; filler transitions ("Additionally", "Furthermore", "Moreover" at high frequency); formulaic conclusions ("In conclusion", "Despite its ... it faces challenges"); and padded words such as delve, leverage, underscore, showcase, intricate, vibrant, foster, tapestry, seamless. Keep a word when it is factually correct in context (a test `harness`, an OAuth `realm`). -- **A why-claim is still a claim (Rule 2).** Cite the comment, ADR, commit, test, or config that proves a rationale or trade-off, or state the _what_ and stop. -- **Scope.** Apply this guidance only to prose you add or change; do not rewrite accurate existing prose for rhythm or voice (Phase 2, Rule 1). It applies to `docs/` prose, not in-code documentation (Phase 3 keeps docstrings and comments terse). -- **Stay formal.** No contractions, no casual asides, no emoji, and no "add imperfections" or detector-evasion tricks. Naturalness comes from cutting tells, not from informality. +- **A why-claim is still a claim (Rule 2).** Cite the comment, design record, commit, test, or config that proves a rationale or trade-off, or state the _what_ and stop. +- **Scope.** Apply this only to prose you add or change; do not rewrite accurate existing prose for rhythm (Phase 2, Rule 1). It governs `docs/` prose, not in-code documentation, which Phase 3 keeps terse. +- **Stay formal.** No contractions, casual asides, emoji, or detector-evasion tricks. Naturalness comes from cutting tells, not from informality. ### Brevity & style -- Use prose to carry reasoning and explanation (the _why_ and _how_); reserve bullets and numbered lists for genuine enumerations (steps, options, fields, parameters). Do not force explanation into parallel bullet fragments, and do not de-list a real list: genuine enumerations stay lists (scannable for people, and easy for an LLM to retrieve). No walls of text. -- **Concise, not choppy:** no line-by-line narration, but keep the connective prose that carries logic; cutting every transition reads as robotic. Lead each paragraph and section with its point (inverted pyramid), then give the detail. +- Use prose to carry reasoning (the _why_ and _how_); reserve bullets and numbered lists for genuine enumerations (steps, options, fields, parameters). Do not force explanation into parallel bullet fragments, and do not de-list a real list: enumerations stay lists, scannable for people and easy to retrieve. No walls of text. **Concise, not choppy:** no line-by-line narration, but keep the connective prose that carries logic. Lead each paragraph and section with its point, then give the detail. +- **Tables only for uniform data scanned quickly**, meaning many parallel items with distinct attributes. If columns repeat across rows, cells sit empty, or a cell holds a sentence of prose, use a list with sub-headings instead. ### Language @@ -128,8 +131,7 @@ Write as a careful human technical writer: formal and neutral, never robotic. Th ### Configuration references -- Document a tunable value by the **name a consumer changes it by**, judging by role, not location. That surface includes external interfaces (env vars, config-file keys, CLI flags) **and** named members of a centralized or exported constants/configuration module that other code reads as tunable values: if a named, stable value is read elsewhere and changing it changes behaviour, document it by that name even when it is an internal `const`. -- Format: "Set or change `` in `` to control ``." Name the consumer-facing value, for example `LIMITS.MAX_RETRIES` in the constants module, not a transient internal such as a `dbUrl` local. +- Document a tunable value by the **name a consumer changes it by**, judging by role, not location. That surface includes external interfaces (env vars, config-file keys, CLI flags) and named members of a centralized or exported constants module that other code reads: if a named, stable value is read elsewhere and changing it changes behaviour, document it by that name even when it is internal. Format: "Set or change `` in `` to control ``." Name the consumer-facing value, for example `LIMITS.MAX_RETRIES`, not a transient local. ### File citations & references (strictly enforced) @@ -140,6 +142,7 @@ Write as a careful human technical writer: formal and neutral, never robotic. Th - **Links target files, not directories.** If the text refers to a directory, link to a file inside it such as its `index.md` or `README.md`. - ❌ "[`/design`](../design)" - ✅ "[`/design`](../design/index.md)" +- **Link text names the destination.** Never "here", "link", "this", or a bare URL: write the sentence first, then wrap the phrase that names what it points at. - Weave links into prose; use a footer `Implementation:` only when inline is unnatural. Do not link the same file twice in adjacent sentences. - Verify every path resolves from the doc's own location. If a referenced file does not exist, correct or remove the statement. @@ -149,7 +152,9 @@ Write as a careful human technical writer: formal and neutral, never robotic. Th ### Formatting -- Always use relative links (for GitHub compatibility). New directories must have an `index.md`. +- Always use relative links, including `../` paths, for GitHub compatibility. Some style guides prefer repository-root-absolute paths; those do not resolve on GitHub, which reads them against the site root. New directories must have an `index.md`. +- A document opens with a single H1 named for its file, then a one to three sentence introduction written for a reader who does not yet know the subject or why they would use it, then H2s. Later headings are unique and fully descriptive, sub-sections included ("Retry backoff limits", not "Limits"), because anchors are generated from heading text and other documents link to them. Use sentence case. +- Prefer standard markup to raw HTML. If the markup cannot express it, reconsider whether the document needs it. - Add a `## Related Documentation` section at the file bottom only when genuinely relevant links exist (not in `index.md` or `README.md`). --- @@ -168,11 +173,9 @@ Exclude logging, metrics, telemetry, trivial validation, internal utilities, and ## 5. Mermaid Diagrams -**Create for:** multi-service interactions, state machines, data pipelines, flows of 5+ steps, user journeys, dependency graphs. **Skip for:** trivial logic, basic CRUD, or repeating a short list. +**Create for:** multi-service interactions, state machines, data pipelines, flows of 5+ steps, user journeys, dependency graphs. **Skip for:** trivial logic, basic CRUD, or repeating a short list. Apply the significance filter from §4. -- Valid Mermaid syntax only. No ASCII art, static images, or `style`/colour customizations. -- Include `accTitle` and `accDescr` (Rule 5). Reflect actual current code, never hypothetical structures. Apply the significance filter from §4. -- Choose the fitting type (`flowchart`, `sequenceDiagram`, `classDiagram`, `stateDiagram`, `journey`, `C4Context`, `mindmap`, `xychart`, `kanban`, `architecture-beta`, `treemap-beta`); do not default to `flowchart`. +- Valid Mermaid syntax only, reflecting current code and never hypothetical structures. No ASCII art, static images, or `style`/colour customizations. Include `accTitle` and `accDescr` (Rule 5). Choose the fitting type (`flowchart`, `sequenceDiagram`, `classDiagram`, `stateDiagram`, `journey`, `C4Context`, `mindmap`, `xychart`, `kanban`, `architecture-beta`, `treemap-beta`), never defaulting to `flowchart` unless it is the best fit. --- @@ -184,14 +187,13 @@ Before finalizing, review your own work and fix everything below. No exceptions. Then confirm: -- Only documentation changed: no executable code, config values, tests, or dependencies were modified (unless the invoking task explicitly asked for code changes). -- No hedging words ("appears to", "seems to", "likely", "probably", "should", "will"). -- Pre-existing content changed only to fix factual errors; accurate phrasing and voice preserved (voice guidance applies to prose you added or changed only). -- Every file reference is a clickable link that resolves to a file, not a directory. -- Configuration references name the value a consumer changes it by (env/config/CLI flag or a constants-module member), not an ephemeral internal variable. -- Acronyms in prose you wrote are capitalized and expanded on first use (exceptions: brand/tool/package names, domain terms, code references). -- No new subjective adjectives and no code dumps (link instead); reasoning sits in prose, genuine enumerations stay lists. -- New or changed prose reads as a careful human wrote it: leads with the point, no signposting previews or banned AI tells, one canonical term per concept, no ambiguous `it`/`this`/`these`. -- Architecture flows include only significant steps (§4); every Mermaid diagram has `accTitle` and `accDescr`. -- No em-dashes (`—`) or en-dashes (`–`) anywhere you wrote (a grammatically correct hyphen `-` is fine); new or changed prose uses Canadian English. +- Only documentation changed: no executable code, config values, tests, or dependencies (unless the invoking task explicitly asked for code changes). Pre-existing content changed only to fix factual errors, with accurate phrasing and voice left alone. +- No hedging ("appears to", "seems to", "likely", "probably", "should", "will"), no new subjective adjectives, and no code dumps. +- Every file reference is a clickable link resolving to a file, not a directory. Configuration references name the value a consumer changes it by. +- Acronyms you wrote are capitalized and expanded on first use (exceptions: brand/tool/package names, domain terms, code references). +- New or changed prose reads as a careful human wrote it: leads with the point, no signposting or banned AI tells, one canonical term per concept, no ambiguous `it`/`this`/`these`. +- Architecture flows include only significant steps (§4); every diagram has `accTitle` and `accDescr`, and every image has real alt text. +- No em-dashes (`—`) or en-dashes (`–`) anywhere you wrote; new or changed prose uses Canadian English. +- Every public symbol you touched carries a documentation comment written from its implementation, not from its name, and no comment narrates a change, argues the code is safe, or sits commented out. +- Rendered output was checked, not only the source: diagrams parse, nested lists and tables render, and documentation comments display the intended text. - Phase 3 ran and its result is reported. diff --git a/CLAUDE.md b/CLAUDE.md index 78b6233e..cc9e144a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -6,14 +6,14 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co This repo is worked on by **both** GitHub Copilot and Claude Code. Keep these authoritative: -- [`.github/copilot-instructions.md`](.github/copilot-instructions.md) — canonical, shared conventions. Copilot cannot read `CLAUDE.md`, so when conventions change, update that file too. -- [`docs/architecture/`](docs/architecture/index.md) and [`docs/usage/`](docs/usage/index.md) — per-area detail (read these instead of re-deriving structure). -- [`.claude/rules/`](.claude/rules/code-style.md) — Claude-specific rules that load automatically. [`code-style.md`](.claude/rules/code-style.md) loads when editing `.ts`/`.tsx`; [`docs-authoring.md`](.claude/rules/docs-authoring.md) loads when editing markdown. +- [`.github/copilot-instructions.md`](.github/copilot-instructions.md) - canonical, shared conventions. Copilot cannot read `CLAUDE.md`, and the automated code reviews read that file rather than `.claude/`, so when conventions change, update it too. +- [`docs/architecture/`](docs/architecture/index.md) and [`docs/usage/`](docs/usage/index.md) - per-area detail (read these instead of re-deriving structure). +- [`.claude/rules/`](.claude/rules/code-style.md) - Claude-specific rules that load automatically. [`code-style.md`](.claude/rules/code-style.md) loads when editing `.ts`/`.tsx`, [`testing.md`](.claude/rules/testing.md) when editing tests or test tooling, and [`docs-authoring.md`](.claude/rules/docs-authoring.md) when editing markdown. ## Commands -- `npm run dev` — dev server at localhost:3000 -- `npm run validate` — full quality gate (prettier → eslint → tsc → jest → cypress → build → markdownlint). **Run before committing.** +- `npm run dev` - dev server at localhost:3000 +- `npm run validate` - full quality gate (prettier, eslint, tsc, jest, cypress, build, markdownlint) - Individual gates: `npm run prettier:check`, `npm run eslint:check`, `npm run tsc`, `npm run test:jest`, `npm run test:cypress:e2e`, `npm run build`, `npm run lint:markdown` - Run a **single** Jest test: - one file: `npx jest src/components/banner/Banner.test.tsx` @@ -21,16 +21,30 @@ This repo is worked on by **both** GitHub Copilot and Claude Code. Keep these au - (path aliases resolve in tests via `moduleNameMapper` in [`jest.config.js`](jest.config.js)) - Install with `npm ci`. CI runs on **Node 22.x** ([`.github/workflows/code-qa.yaml`](.github/workflows/code-qa.yaml)); there are no pre-commit hooks, so `npm run validate` is the manual equivalent. +## Validation + +**Every change to logic, tests, configuration, or documentation ends with the quality gates run and green.** This is not optional and not deferrable. + +- Confirm the **actual exit code** (`echo "EXIT: $?"`) after each gate. The output is long and failures surface at the end, so scrolling it is not a check. +- A single gate is never a substitute for the full set. Running `npm run test:jest` alone skips type checking, linting, the build, and markdown linting. +- If a gate fails, fix the cause and re-run until it passes. Never report work complete, or describe validation as passing, before that point. Report a pre-existing failure honestly rather than presenting it as unrelated and therefore fine. +- Run `npm run prettier` again after any ESLint fix, and finish with `npm run prettier:check`: `eslint --fix` inserts braces inline where Prettier would break the statement across lines. +- If `test:cypress:e2e` fails, quote the actual error. Treat it as an environment limit only when the Cypress **binary fails to launch**, an Electron or window-server error raised before any spec runs, since Cypress needs a GUI session that a headless agent shell may not have. A failing assertion inside a spec is a real failure. Either way, run the remaining gates (`build` and `lint:markdown` come after Cypress in the chain) and say plainly that e2e was not run. +- Delegate the run to the `validator` subagent to keep verbose Jest and build output out of this context. + +A `Stop` hook blocks the first attempt to finish while gates are outstanding, and names which ones. + ## Architecture -A single-page Next.js **App Router** portfolio: the whole site is [`src/app/layout.tsx`](src/app/layout.tsx) (metadata, SEO/JSON-LD, providers) plus [`src/app/page.tsx`](src/app/page.tsx). Stack is React 19 + TypeScript + Material-UI (Emotion). Portfolio content is **static TypeScript data** in [`src/data/`](src/data/projects.ts) (projects, publications, socials, keywords) imported and rendered directly — there is no CMS, database, or content fetching. State is minimal local React hooks (no Redux/global store). Cross-cutting concerns: Sentry (client/server/edge configs), Firebase analytics, Vercel Speed Insights, and a PWA service worker ([`public/sw.js`](public/sw.js) registered by [`src/components/ServiceWorkerRegister.tsx`](src/components/ServiceWorkerRegister.tsx)); security headers live in [`next.config.js`](next.config.js). For per-area detail see [`docs/architecture/`](docs/architecture/index.md). +A single-page Next.js **App Router** portfolio: the whole site is [`src/app/layout.tsx`](src/app/layout.tsx) (metadata, SEO/JSON-LD, providers) plus [`src/app/page.tsx`](src/app/page.tsx). Stack is React 19 + TypeScript + Material-UI (Emotion). Portfolio content is **static TypeScript data** in [`src/data/`](src/data/projects.ts) (projects, publications, socials, keywords) imported and rendered directly: there is no CMS, database, or content fetching. State is minimal local React hooks (no Redux/global store). Cross-cutting concerns: Sentry (client/server/edge configs), Firebase analytics, Vercel Speed Insights, and a PWA service worker ([`public/sw.js`](public/sw.js) registered by [`src/components/ServiceWorkerRegister.tsx`](src/components/ServiceWorkerRegister.tsx)); security headers live in [`next.config.js`](next.config.js). For per-area detail see [`docs/architecture/`](docs/architecture/index.md). ## Conventions -Two rules trip people up most — **use tabs, not spaces**, and **import via path aliases (`@components/...`), never relative paths**. The full set (MUI `sx`-only styling, strict TypeScript, Server Components by default, colocated `.test.tsx`, etc.) lives in [`.claude/rules/code-style.md`](.claude/rules/code-style.md) and is enforced by `prettier`/`eslint`/`tsc`. +Two rules trip people up most: **use tabs, not spaces**, and **import via path aliases (`@components/...`), never relative paths**. The full set (MUI `sx`-only styling, strict TypeScript, Server Components by default, JSDoc on exports, the readability rules, and the [Google TypeScript Style Guide](https://google.github.io/styleguide/tsguide.html) deltas) lives in [`.claude/rules/code-style.md`](.claude/rules/code-style.md). Testing conventions (one colocated test per source, mocking policy, `it.each` tables, naming) live in [`.claude/rules/testing.md`](.claude/rules/testing.md). Both are enforced by `prettier`, `eslint`, and `tsc`. ## Claude Code extras -- The [`.github/prompts/`](.github/prompts/readme.md) files (`audit-docs`, `audit-pr`, `audit-quality`) are **Copilot coding-agent** prompts (need an active PR + Copilot Chat) — not Claude Code commands. -- Claude Code equivalents: the `/audit-docs` skill (documentation audit, mirrors [`.github/prompts/audit-docs.prompt.md`](.github/prompts/audit-docs.prompt.md)), plus the built-in `/code-review` and `/security-review`. -- A PostToolUse hook reminds you of the doc-authoring rules whenever you edit a markdown file. +- The [`.github/prompts/`](.github/prompts/readme.md) files (`audit-docs`, `audit-pr`, `audit-quality`) are **Copilot coding-agent** prompts (need an active PR + Copilot Chat), not Claude Code commands. +- Skills: `/audit-docs` (documentation audit, mirrors [`.github/prompts/audit-docs.prompt.md`](.github/prompts/audit-docs.prompt.md)), `/write-tests` (author or repair a test to house style), and `/google-ts-style` (fuller style digest for a deliberate style pass). Plus the built-in `/code-review` and `/security-review`. +- Subagent: `validator` runs the six local quality gates in its own context and returns a verdict instead of several thousand lines of output. +- Hooks ([`.claude/hooks/`](.claude/hooks/validate-gate.mts)): `markdown-audit-reminder` restates the doc-authoring rules whenever you edit a markdown file; `validate-gate` tracks which gates have run and blocks the first attempt to finish while any are outstanding. diff --git a/cypress.config.ts b/cypress.config.ts index a88ff536..261b9747 100644 --- a/cypress.config.ts +++ b/cypress.config.ts @@ -2,7 +2,6 @@ import { defineConfig } from 'cypress'; export default defineConfig({ e2e: { - setupNodeEvents(on, config) {}, // Include shadow DOM elements in command results includeShadowDom: true, // Allow certain Content Security Policies diff --git a/cypress/e2e/landing.cy.ts b/cypress/e2e/landing.cy.ts index 4e7580f5..f6d87887 100644 --- a/cypress/e2e/landing.cy.ts +++ b/cypress/e2e/landing.cy.ts @@ -1,32 +1,24 @@ -// This test suite is for the landing page describe('Landing Page', () => { afterEach(() => { - // Accessibility check cy.a11yCheck(); }); - // This test checks that the page renders correctly it('should render page', () => { cy.visit('http://localhost:3000'); - // Check that the profile picture exists on the page cy.get('[data-testid="profile_pic"]').should('exist'); }); it('should show cookie snackbar on first load and not after accepting', () => { cy.visit('http://localhost:3000'); - // The snackbar should be visible on first load cy.get('.MuiSnackbar-root').should('exist').and('be.visible'); cy.contains('This website uses cookies to enhance the user experience.').should('be.visible'); - // Click the close button to accept cookies cy.get('.MuiSnackbar-root button[aria-label="close"]').click(); cy.get('.MuiSnackbar-root').should('not.exist'); - // Reload the page cy.reload(); - // The snackbar should not appear again cy.get('.MuiSnackbar-root').should('not.exist'); }); diff --git a/cypress/e2e/security/middleware.cy.ts b/cypress/e2e/security/headers.cy.ts similarity index 69% rename from cypress/e2e/security/middleware.cy.ts rename to cypress/e2e/security/headers.cy.ts index 6278cb9c..9d81c483 100644 --- a/cypress/e2e/security/middleware.cy.ts +++ b/cypress/e2e/security/headers.cy.ts @@ -1,4 +1,4 @@ -describe('Middleware Security Tests', () => { +describe('Security headers', () => { beforeEach(() => { cy.visit('http://localhost:3000'); }); @@ -7,8 +7,7 @@ describe('Middleware Security Tests', () => { cy.a11yCheck(); }); - it('should prevent SSRF through middleware headers', () => { - // Test that sensitive headers are not reflected back + it('should not reflect forwarded-host headers back in the response', () => { const sensitiveHeaders = { 'X-Forwarded-Host': 'evil.com', 'X-Forwarded-Proto': 'http', @@ -25,14 +24,10 @@ describe('Middleware Security Tests', () => { }, failOnStatusCode: false, }).then((response) => { - // Check that sensitive headers are not reflected in response - const responseHeaders = Object.keys(response.headers).map((key) => key.toLowerCase()); const responseHeaderValues = Object.values(response.headers).join(' '); - // Ensure the malicious header value is not reflected back expect(responseHeaderValues).to.not.include(headerValue); - // Check that response doesn't contain redirect to external domain if (response.status >= 300 && response.status < 400) { const location = response.headers.location; if (location) { @@ -44,8 +39,7 @@ describe('Middleware Security Tests', () => { }); }); - it('should validate redirect destinations in middleware', () => { - // Test that redirects don't allow SSRF + it('should not redirect to an off-origin destination', () => { const maliciousRedirects = [ 'http://evil.com', 'https://attacker.site/steal-data', @@ -63,12 +57,10 @@ describe('Middleware Security Tests', () => { if (response.status >= 300 && response.status < 400) { const location = response.headers.location; if (location) { - // Should not redirect to external or malicious URLs expect(location).to.not.include('evil.com'); expect(location).to.not.include('attacker.site'); expect(location).to.not.match(/^(ftp|file|gopher):/); - // Should only redirect to same origin or relative paths if (typeof location === 'string' && location.startsWith('http')) { expect(location).to.match(/^https?:\/\/localhost(:\d+)?/); } @@ -78,8 +70,7 @@ describe('Middleware Security Tests', () => { }); }); - it('should sanitize request headers in middleware processing', () => { - // Test that middleware doesn't process dangerous header combinations + it('should not leak internal paths from rewritten request headers', () => { cy.request({ url: 'http://localhost:3000', headers: { @@ -90,10 +81,9 @@ describe('Middleware Security Tests', () => { }, failOnStatusCode: false, }).then((response) => { - // Verify response doesn't expose internal paths or dangerous content expect(response.body).to.not.include('/etc/passwd'); expect(response.body).to.not.include('/admin/secret'); - expect(response.status).to.not.equal(500); // Should handle gracefully + expect(response.status).to.not.equal(500); }); }); }); diff --git a/cypress/e2e/security/image.cy.ts b/cypress/e2e/security/image.cy.ts index c6b562a2..f0c658ac 100644 --- a/cypress/e2e/security/image.cy.ts +++ b/cypress/e2e/security/image.cy.ts @@ -8,14 +8,13 @@ describe('Image Security Tests', () => { }); it('should only allow configured remote image domains', () => { - // Based on your next.config.js, only alexjsully.me should be allowed + // next.config.js allows only alexjsully.me as a remote image host. const disallowedDomain = 'https://random-external-site.com/image.jpg'; cy.request({ url: `/_next/image?url=${encodeURIComponent(disallowedDomain)}&w=640&q=75`, failOnStatusCode: false, }).then((response) => { - // Should return error status, not serve the image expect(response.status).to.not.equal(200); }); }); @@ -29,8 +28,8 @@ describe('Image Security Tests', () => { url: `/_next/image?url=${encodeURIComponent(param)}&w=640&q=75`, failOnStatusCode: false, }).then((response) => { - // Should return error status, not serve files expect(response.status).to.not.equal(200); + const contentType = response.headers['content-type'] || ''; expect(contentType).to.not.include('text/plain'); expect(contentType).to.not.include('application/octet-stream'); diff --git a/cypress/e2e/security/security.cy.ts b/cypress/e2e/security/security.cy.ts index 75fdc160..6a429fb9 100644 --- a/cypress/e2e/security/security.cy.ts +++ b/cypress/e2e/security/security.cy.ts @@ -4,31 +4,24 @@ describe('General Security Tests', () => { }); afterEach(() => { - // Accessibility check cy.a11yCheck(); }); - // This test checks for XSS vulnerability it('should not execute malicious scripts', () => { - // Define a malicious input that attempts to inject a script const maliciousInput = { regex: /<\/script>