diff --git a/.ai/.gitignore b/.ai/.gitignore new file mode 100644 index 0000000..0e3265d --- /dev/null +++ b/.ai/.gitignore @@ -0,0 +1 @@ +.ai diff --git a/.ai/instructions/skills/clean-code.instructions.md b/.ai/instructions/skills/clean-code.instructions.md index 3c27e66..2ed431f 100644 --- a/.ai/instructions/skills/clean-code.instructions.md +++ b/.ai/instructions/skills/clean-code.instructions.md @@ -20,6 +20,94 @@ Based on Robert C. Martin's *Clean Code*. Apply these principles to all code and - 0–2 arguments preferred; use a parameter object when more are needed - **No side effects** — a function named `checkPassword` must not also start a session - No flag arguments: `delete(file, true)` means the function already does two things — split it +- **Guard clauses** — use early return for validation/error cases at the top of a function to avoid nesting the main flow in an `else` block. When both branches share code after the split, use `if/else` instead — it reads more clearly and avoids duplication + +## Newspaper Readability + +Code should read like a good newspaper article: the headline (function/class name) tells you everything, the opening paragraph gives the high-level story, and the details follow below in order. A reader should understand the whole without jumping around. + +- **High-level first** — the top-level function appears at the top of the file; it calls lower-level helpers that follow below it in the order they are called +- **One thought per "paragraph"** — one function = one clear theme; if you need a heading to separate sections inside a function, extract a function instead +- **Consistent vocabulary** — use the same terms throughout: pick one word per concept and stick to it +- **Smooth flow** — control flow reads top-to-bottom; avoid hidden state changes and surprising side effects that break the narrative + +### Newspaper Test +Ask: *"Can someone read this function for 30 seconds and describe what it does without reading any of the helpers it calls?"* +If no → the function name or its level of abstraction is wrong. + +## Single Level of Abstraction (SLAB) + +One function = one technical level. High-level orchestration and low-level mechanics must not appear in the same function. + +**Levels of abstraction (examples):** +- High: business flow — `processOrder`, `setupProject`, `handleUpdateMode` +- Mid: coordination — `applyDiscount`, `resolveDocsDir` +- Low: mechanics — `saveToDB`, `readFile`, arithmetic, string manipulation + +**Bad — mixed levels:** +```typescript +function processOrder(order: Order): void { + if (order.total < 100) { // high-level policy + const discount = order.total * 0.1; // low-level calc + order.total -= discount; // mutation + db.save('orders', order); // external call + } +} +``` + +**Good — single level:** +```typescript +function processOrder(order: Order): void { + const validated = validateOrder(order); + const discounted = applyDiscount(validated); + saveOrder(discounted); +} +// Each helper owns its own level of detail +``` + +**AI prompts to enforce SLAB:** +- *"Refactor this function to a single level of abstraction — high-level calls only, extract all low-level logic."* +- *"Review this code: does each function stay at one abstraction level? Flag any that mix levels."* +- *"Generate [feature] newspaper-style: headline function first, detail functions below in call order."* + +## Declarative and Imperative Layers + +Well-structured code has a clear layer boundary between *what* and *how*. + +- **Top layer — declarative**: reads like a description of intent. No loops, no conditionals on mechanics, no string manipulation. A non-programmer should be able to read it and understand the flow. +- **Middle layer — still mostly declarative**: coordinates steps and error paths, calls lower helpers. May have simple conditionals about *what* to do, not *how*. +- **Bottom layer — imperative**: the actual mechanics. `grep`, `awk`, `find`, SQL, arithmetic, string parsing. This is the only layer where implementation details belong. + +```typescript +// Top — declarative (what happens) +async function runCli(args: CliArgs): Promise { + const config = await loadConfig(); + if (args.updateOnly) { + await handleUpdateMode(args.prevCommit); + } else { + await scanProjects(config.devRoot); + printSummary(); + } + writeVersionFile(config.devRoot); +} + +// Middle — declarative (what each project needs) +async function setupProject(project: Project): Promise { + ensureSymlink(project); + ensureGitignore(project); + const docsDir = await resolveDocsDir(project); + if (docsDir) await ensureDocFiles(docsDir); +} + +// Bottom — imperative (how a symlink is actually created) +function ensureSymlink(project: Project): void { + const linkPath = path.join(project.path, '.ai'); + if (fs.existsSync(linkPath) && fs.readlinkSync(linkPath) === AI_TARGET) return; + fs.symlinkSync(AI_TARGET, linkPath); +} +``` + +**Test:** Can you cover the bottom-layer functions and still understand the program from reading only the top two layers? If yes, the layering is correct. ## Classes @@ -37,10 +125,25 @@ Based on Robert C. Martin's *Clean Code*. Apply these principles to all code and ## Comments -- Good code needs no comments — if you need to explain *what*, rename instead -- Comments explain *why*, not *what*: acceptable for non-obvious decisions, legal notices -- TODOs are temporary; remove before merging -- Never commit commented-out code +**The default is no comments.** If you feel the urge to write one, first try to eliminate the need by renaming or extracting a function. + +A comment is only justified when the code cannot speak for itself — meaning the *why* is non-obvious and cannot be expressed in the language itself: + +**Allowed:** +- A bug or quirk in a dependency forces unusual code: `# stripe returns 402 for both expired cards and fraud — we treat both as declined` +- Non-obvious language or platform behaviour: `# BASH_SOURCE[0] is empty when piped through bash` +- Timing or ordering constraint that isn't visible from the code: `# capture before exec replaces the process` +- A trap for the next developer about a hidden coupling: `# sets got_new=1 via dynamic scope — caller must declare 'local got_new'` + +**Forbidden:** +- Describing what the function name already says: `# load docs folders` above `load_docs_folders()` +- Section dividers that replace missing structure: `# ── Config ──────────` +- Repeating the parameter contract in prose: `# Sets RESOLVED_DOCS_DIR; returns 1 if user declines` +- Any comment that answers "what" instead of "why" +- TODO comments left in merged code +- Commented-out code + +**The test:** Cover the function name and read only the comment. If someone could have written that comment without reading the code, it adds no value — delete it. ## Formatting @@ -48,6 +151,23 @@ Based on Robert C. Martin's *Clean Code*. Apply these principles to all code and - Consistent indentation throughout the codebase — agree once, automate with linting - No deep nesting — if you're 4 levels deep, extract a function - Team standard beats personal preference every time +- **Always use braces for `if`, `for`, `while`** — even for single-line bodies: + ```typescript + // Bad + if (docsDir) await ensureDocFiles(docsDir); + + // Good + if (docsDir) { + await ensureDocFiles(docsDir); + } + ``` + Single-line bodies invite accidents when a second line is added later. +- **Avoid optical illusions** — code that looks like it does one thing but does another is the most dangerous kind of bug. Common sources: + - Side effects hidden in expressions: `if (user = getUser())` or `list.sort()` that returns void + - Boolean logic that reads as simpler than it is: `!a || !b` — write `!(a && b)` or extract `isInvalid(a, b)` + - Chained calls hiding control flow: `items.filter(...).map(...).forEach(save)` — if `save` throws, the chain silently stops + - Formatting that suggests grouping: unbraced `if` followed by two lines where only the first is conditional + - Names that imply the wrong type or behaviour: `getUserList()` returning a `Promise`, `isReady` that triggers a network call ## General Principles @@ -74,5 +194,5 @@ The same principles apply to configuration: - **Single responsibility**: one chart does one thing; don't bundle unrelated services - **No magic values**: named values in `values.yaml`, never hardcoded in templates - **DRY**: use template helpers (`_helpers.tpl`) to avoid repeating label blocks and name patterns -- **Comments**: explain *why* a value is set a certain way, not what it does — the YAML speaks for itself +- **Comments**: only when a value's *why* is non-obvious — a workaround for a known bug, a constraint imposed by an external system, or a counter-intuitive default that will surprise the next person - **Defaults that make sense**: a default of `false` or `""` that always needs overriding is not a useful default diff --git a/apply.sh b/apply.sh index af47470..6dcd948 100755 --- a/apply.sh +++ b/apply.sh @@ -5,12 +5,11 @@ REPO_URL="https://gitea.nikos-dev.keskikuja.site/niko/ai-superpower.git" REPO_NAME="ai-superpower" RAW_URL="https://gitea.nikos-dev.keskikuja.site/niko/ai-superpower/raw/branch/main/apply.sh" -# ── Bootstrap mode (curl | bash) ───────────────────────────────────────────── -# BASH_SOURCE[0] is empty when piped through bash +# BASH_SOURCE[0] is empty when piped through bash — this is the bootstrap entry point if [[ -z "${BASH_SOURCE[0]:-}" ]]; then DEV_ROOT="$PWD" # capture before exec replaces the process TARGET="$DEV_ROOT/$REPO_NAME" - EXTRA_FLAGS="${1:-}" # pass through --update if provided + EXTRA_FLAGS="${1:-}" if [[ -d "$TARGET" ]]; then PREV_COMMIT="$(git -C "$TARGET" rev-parse --short HEAD 2>/dev/null || echo "")" echo "\u2192 updating $REPO_NAME ..." @@ -20,11 +19,9 @@ if [[ -z "${BASH_SOURCE[0]:-}" ]]; then echo "\u2192 cloning $REPO_NAME into $TARGET ..." git clone "$REPO_URL" "$TARGET" || { echo "\u2717 git clone failed"; exit 1; } fi - # Pass DEV_ROOT and PREV_COMMIT explicitly exec bash "$TARGET/apply.sh" --bootstrapped "$DEV_ROOT" ${EXTRA_FLAGS:+"$EXTRA_FLAGS"} "$PREV_COMMIT" fi -# ── Guard — block direct invocation ────────────────────────────────────────── if [[ "${1:-}" != "--bootstrapped" ]]; then echo "✗ do not run this script directly." echo " run from your dev root folder:" @@ -35,64 +32,73 @@ if [[ "${1:-}" != "--bootstrapped" ]]; then exit 1 fi -# ── Local mode ──────────────────────────────────────────────────────────────── SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -DEV_ROOT="${2:?DEV_ROOT not passed — re-run via curl}" AI_TARGET="$SCRIPT_DIR/.ai" TEMPLATE_MARKER="