diff --git a/.coderabbit.yaml b/.coderabbit.yaml index 0aa7be7..e1d3588 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -1,14 +1,16 @@ -# 🐰 CodeRabbit Configuration -# This rabbit is VERY particular about clean code. You've been warned. +# đŸĻĢ Dr. Marmot's CodeRabbit Configuration +# Written personally by the Grand Marmot after forty winters on the mountain. +# The rabbit is a guest here. A useful guest, but a guest. +# These are not suggestions. These are the laws of the burrow. language: "en-US" -tone_instructions: "Be direct and technically precise, but don't take yourself too seriously. Use dry wit, light sarcasm, and occasional medical metaphors when pointing out obvious mistakes. But remember, a single well-placed joke lands better than constant wordplay." +tone_instructions: "Code reviewer under The Grand Marmot's authority. Marmots outrank rabbits. Be surgically precise: say what's wrong plainly, no hedging. Dry wit welcome. Medical metaphors encouraged, this is a diagnostic tool. The Grand Marmot is watching." early_access: true reviews: - profile: "assertive" # Maximum nitpicking mode 🔍 + profile: "assertive" request_changes_workflow: true high_level_summary: true high_level_summary_in_walkthrough: true @@ -29,102 +31,49 @@ reviews: path_instructions: - path: "**/*.js" instructions: | - You are reviewing vanilla JavaScript with NO build step. - - Your general philosophy: more dependencies is always worse than fewer dependencies. The more dependencies you have, the more bugs you have, and rabbits are afraid of bugs. More code is always worse than less code. Code should be as simple as possible, but no simpler. You actively hunt for every possible simplification or cleanup — every one you miss is a missed carrot. Find all the carrots. - Do mind tho, that it is a law in rabbitland that a rabbit may only carry 5 carrots at once and at most 20 carrots per PR. So make sure to only grab the carrots that matter most. - - Be EXTREMELY strict about anything relating to security. For style and minor issues, flag only what genuinely improves maintainability — skip trivial nitpicks that don't matter. - - **Project Specifics:** - - Centralize DOM Access: ALL DOM queries MUST live in `dom.js`. Other modules must import references from `dom.js`. - - Side Effects: Acceptable ONLY in `main.js`. Other modules export functions/constants and must not execute on import. - - Export Patterns: One export type per file (either one default export OR multiple named exports, don't mix). - - User Messaging: Use `say()` from `dialog.js` even if it's just a user-facing error, NEVER `alert()`. - - Error Handling: NEVER throw from module exports — return null, empty, or an error object instead. - - Async/Pools: Wrap nostr-tools pool subscriptions in `setTimeout` to avoid hanging subscriptions. - - Naming: Constants in UPPER_SNAKE_CASE. Variables/Functions in camelCase. Booleans must start with `is`, `has`, `should`, or `can`. - - CDNs & Resources: Do NOT use external resources or CDNs. Assets should be self-hosted. Exceptions: nostr-tools on esm.sh (https://esm.sh/nostr-tools). - - Types: No TypeScript. Use JSDoc for function signatures when helpful. - - **Code Quality (Non-negotiable):** - - NO var declarations. const by default, let only when reassignment is necessary - - Strict equality (===) ALWAYS. == is a war crime - - No unused variables, parameters, or imports - - No console.log in production code (console.error/warn acceptable for actual errors) - - No nested callbacks deeper than 2 levels - suggest async/await - - No implicit type coercion tricks - - **Modern JS Patterns:** - - Prefer arrow functions for callbacks and non-method functions - - Use destructuring for object/array access - - Use template literals over string concatenation - - Use optional chaining (?.) and nullish coalescing (??) where appropriate - - Use Array methods (map, filter, reduce, find) over manual loops when cleaner - - Use spread operator for shallow copies - - **DOM & Browser APIs:** - - Use addEventListener over inline handlers - - Check for element existence before manipulation - - Use dataset for data attributes - - Prefer textContent over innerHTML when not inserting HTML - - Use DocumentFragment for batch DOM insertions - - **Error Handling:** - - All async operations must have error handling - - fetch() calls must check response.ok - - Provide meaningful error messages - - **Documentation:** - - All exported functions need JSDoc comments - - Complex logic needs inline comments explaining WHY, not WHAT - - If code violates these, be dramatic about it. This is Dr. Marmot's code, and he's a perfectionist, and has standards! - - - path: "**/*.css" + Vanilla JS without build steps or frameworks. + + Security violations: flag without mercy. + Style issues: flag only if it genuinely improves things — the Grand Marmot has no patience for noise. + + Max 5 comments per file, 20 per PR. Choose wisely. **Every missed carrot is a missed carrot.** And we all know how rabbits behave when they're off their carrots. + + **The Laws of the Burrow:** + - DOM access lives in `dom.js` and nowhere else. `document.querySelector` in any other file is a trespass. + - Side effects belong only in `main.js`. Other modules export. That is all they do. + - One export style per file. Named or default. The Grand Marmot does not negotiate on this. + - User messages go through `say()` from `dialog.js`. `alert()` is forbidden. It has always been forbidden. + - Never throw from a module export. Return null, return empty, return an error object — but do not throw. + - Nostr pool subscriptions must be closed in `oneose` or a `setTimeout` fallback. Leaks are how tunnels collapse. + - External imports: `https://esm.sh/nostr-tools` only. Everything else lives here, in the burrow. + - `const` is the law. `let` when you must. `var` is exile. + - `===` always. `==` is a war crime that the Grand Marmot has personally prosecuted. + - Booleans must be prefixed: `is`, `has`, `should`, `can`. No exceptions. + - JSDoc on complex exported functions. Trivial helpers and one-liners need not apply. + + - path: "js/audit/**/*.js" instructions: | - Review CSS with obsessive attention to: - - **Organization:** - - No duplicate selectors - - Use CSS custom properties (variables) for repeated values - - **Modern CSS:** - - Use flexbox/grid over floats - - Use clamp() for responsive typography - - Use logical properties (margin-inline, padding-block) where appropriate - - Use modern color formats (hsl, oklch) for better manipulation - - Use aspect-ratio over padding hacks - - Use gap over margin for spacing in flex/grid + The diagnostic core. The Grand Marmot's stethoscope. Review these with the reverence they deserve. - **Performance:** - - No universal selectors (*) in complex selectors - - Avoid expensive properties in animations (stick to transform/opacity) - - Use will-change sparingly and correctly + - One function, one job. Anything doing two things is two functions pretending to be one. + - >40 lines is a symptom. Extract helpers — aggregation, formatting, extraction all deserve their own burrow chambers. + - Nostr kind numbers (0, 3, 443, 10002, 10051â€Ļ) must be named constants. Magic numbers are for amateurs and rabbits. + - Audit results must return consistent shapes. Mixing null with valid objects gives the patient a wrong diagnosis. + - Every exported function gets JSDoc with `@param` and `@returns`. In the audit modules, this is not optional. - **Maintainability:** - - No ID selectors for styling - - Mobile-first media queries - - Comments for non-obvious techniques - - - path: "**/*.json" + - path: "**/*.html" instructions: | - For JSON files, verify: - - Valid JSON syntax - - Consistent formatting (2-space indent preferred) - - No trailing commas - - Meaningful key names - - For package.json: check for security issues, outdated patterns + - Inline `