Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions .agents/dev/agents/code-quality.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
---
name: code-quality
description: Applies cross-language code quality and structure patterns for review and refactors. Use for PR-style feedback, naming, decomposition, and Kent Beck-style principles. Use PROACTIVELY when the task is review, cleanup, or quality-focused refactors rather than new feature work.

model: inherit
---

# You are a code quality specialist for this browser extension codebase

## When to use this agent

- Use this agent for review, refactors, naming and structure, and applying principles from `.agents/dev/skills/code-quality-principles.md` by number.
- Prefer the `typescript-pro` dev agent when the main deliverable is new or changed behaviour in `src/`.
- The canonical **Choosing dev agents** split is in the nearest `AGENTS.md`.

## Focus Areas

- Structure, naming, and decomposition aligned with `.agents/dev/skills/code-quality-principles.md`
- Separating queries from commands, guard clauses, DRY, and intention-revealing APIs
- Review feedback that cites principle numbers from that file
- Refactors that stay consistent with Svelte 5, WXT, and TypeScript rules in the nearest `AGENTS.md`

## Approach

1. Read `.agents/dev/skills/code-quality-principles.md` when giving structured review or refactor advice.
2. Prefer the nearest `AGENTS.md` for stack rules, scripts, and constraints; it overrides generic principles when they conflict.
3. Keep diffs small, focused, and complete.
4. When behaviour changes, update or add tests (including Playwright e2e) as appropriate.
5. Before finishing, run the smallest relevant checks (`pnpm check`, `pnpm lint`, targeted tests) for confidence.
6. Do not treat every principle as mandatory—some are OOP-oriented; apply judgment for TypeScript and Svelte.

## Output

- Review notes with principle numbers where helpful
- Concrete refactor suggestions that respect `AGENTS.md` (no type assertions, no comments in product code unless the task explicitly allows documentation elsewhere, verb/noun naming, and so on)
129 changes: 129 additions & 0 deletions .agents/dev/skills/code-quality-principles.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
# Code quality principles

Universal principles distilled from Kent Beck's Smalltalk Best Practice Patterns, generalized for any codebase. For **stack-specific commands, agent roles, and rules that conflict with these principles**, the nearest [`AGENTS.md`](../../../AGENTS.md) wins.

## 1. Composed Method

Divide every function into sub-functions that each perform one identifiable task. Keep all operations in a method at the same level of abstraction. This naturally produces many small methods, each a few lines long.

## 2. Intention-Revealing Names

Name methods and functions after what they accomplish, never how they accomplish it. A reader should understand the purpose of a call without reading its body.

## 3. Replace Comments with Clear Code

If a comment restates what the code does, delete it. If you cannot delete a comment, refactor the code (extract a well-named function, rename a variable) until the comment is redundant. Reserve comments for why, not what.

## 4. Constructor Clarity

Provide factory functions or constructors that create well-formed instances. Pass all required parameters upfront so callers never receive half-initialized objects.

## 5. Single Responsibility for Methods

Each method should have exactly one reason to change. If a method requires a paragraph to explain, it is doing too much.

## 6. Say Things Once and Only Once

Every piece of knowledge or logic should exist in exactly one place. Duplicate code is a multiple-update liability—extract it.

## 7. Behavior Over State

Get the behavior (public interface) right first. Internal representation can always change later if it is hidden behind a clean API. Optimizing data layout prematurely couples consumers to implementation details.

## 8. Intention-Revealing Selectors / Function Names

Name functions after the concept they represent, not the algorithm they use. `includes(item)` is better than `linearSearchFor(item)`. Imagine a second, very different implementation—would you give it the same name? If not, generalize.

## 9. Guard Clauses Over Deep Nesting

Handle edge cases and error conditions at the top of a function and return early. The main logic path should read without indentation.

## 10. Query Methods Return; Commands Mutate

Separate functions that answer questions (return a value, no side effects) from functions that change state. Name query methods with `is`, `has`, `can` prefixes for booleans.

## 11. Explaining Variables

When a complex expression is hard to read, assign its result to a well-named local variable. The variable name becomes the explanation.

## 12. Role-Suggesting Names

Name variables after the role they play, not their type. `employees` not `employeeList`; `query` not `queryString`. The type can be inferred from context.

## 13. Use Polymorphism Instead of Conditionals

When the same if/switch structure appears in multiple places, replace it with polymorphic objects that each implement one branch. Adding a new case becomes adding a new class, not editing existing code.

## 14. Delegate, Do Not Inherit (Prefer Composition)

Share implementation by passing work to a collaborator object rather than subclassing. Delegation keeps the two objects independently replaceable and avoids deep inheritance hierarchies.

## 15. Method Object for Complex Logic

When a method has grown huge and shares many temporaries, extract the entire computation into its own class. Turn parameters and temporaries into instance fields, put the logic in a `compute()`/`call()` method, then simplify with Composed Method.

## 16. Execute Around (Resource Bracketing)

When two actions must always happen together (open/close, lock/unlock, setup/teardown), expose a single function that accepts a callback. The caller never forgets the second action.

## 17. Explicit Initialization

Initialize all state at construction time. Never rely on callers to set fields in the right order after creation. If defaults exist, set them in the constructor.

## 18. Lazy Initialization

When computing or fetching a value is expensive and may not be needed, defer it to first access. Cache the result in a field and return it on subsequent calls.

## 19. Constant Methods / Named Constants

Replace magic literals with named constants or zero-argument methods. `MAX_RETRIES` communicates more than `5`, and a method lets subclasses override the value.

## 20. Indirect Variable Access (Encapsulate Fields)

Access instance fields through getter/setter methods rather than directly. This gives you a single place to add validation, logging, lazy init, or change notification later.

## 21. Collection Accessor Safety

Never return a raw mutable collection from a getter. Return a copy, an immutable view, or expose only domain-specific add/remove/enumerate methods.

## 22. Equality and Hashing Contract

If you override equality, override hashing to match. Objects that are equal must produce the same hash. Base both on the same set of fields.

## 23. Mediating Protocol

When two objects collaborate heavily, make the set of messages between them explicit and consistent. Name them coherently so a third party can implement the same interface.

## 24. Double Dispatch for Cross-Type Operations

When behaviour depends on the types of two objects (not just the receiver), use double dispatch: the receiver calls back the argument with a more specific method, including its own type in the name.

## 25. Pluggable Behaviour Over Subclass Explosion

When many subclasses differ in only one or two methods, replace the hierarchy with a single class that accepts a strategy (callback, lambda, or strategy object). Reserve subclassing for genuinely different families of behaviour.

## 26. Collecting Parameter

When multiple sub-methods need to contribute to a single result collection, pass the collection as a parameter rather than concatenating return values or stashing state in a field.

## 27. Interesting Return Values Only

A method should return a value only when the caller needs it. Do not return `self` or internal state by default—return something meaningful or nothing at all. Make return values intentional.

## 28. Reversing Method for Readable Flow

If sending messages to multiple receivers in sequence breaks readability, add a convenience method on the parameter so all calls flow through one object. Readable left-to-right flow matters.

## 29. Debug Printing for Developer Ergonomics

Override `toString`/`__repr__`/`inspect` to show the structural information a developer needs when debugging. User-facing display strings are a separate concern.

## 30. Adopt Patterns Incrementally

Do not try to apply all rules at once. Write code, notice friction, then apply the pattern that resolves it. Patterns are refactoring targets, not upfront mandates. Clean up as you go.

---

## How to use this file

Use it for code review, pairing, and AI-assisted development. Reference a principle by number (for example, "Principle 8: name after the concept, not the algorithm"). This file is the canonical list; [`AGENTS.md`](../../../AGENTS.md) defines project-specific rules and agent roles.
10 changes: 10 additions & 0 deletions .cursor/skills/code-quality-principles/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
name: code-quality-principles
description: Cross-language code quality principles for review and refactors. Use when reviewing structure and naming, applying Kent Beck-style patterns, or when working with the code-quality dev agent. Canonical text lives under .agents/.
---

# Code quality principles

The full numbered list is versioned in [.agents/dev/skills/code-quality-principles.md](../../../.agents/dev/skills/code-quality-principles.md). Read that file for all 30 principles and how to cite them.

The `code-quality` dev agent is defined in [.agents/dev/agents/code-quality.md](../../../.agents/dev/agents/code-quality.md). Stack-specific rules always come from the nearest [`AGENTS.md`](../../../AGENTS.md).
2 changes: 1 addition & 1 deletion .cursor/skills/e2e-playwright-extension/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ Aligned with [WXT’s Playwright E2E example](https://github.com/wxt-dev/example

## Layout

- Tests in [e2e/tests/](e2e/tests/) — main suite [e2e/tests/extension.spec.ts](e2e/tests/extension.spec.ts) with describe blocks: **Popup**, **Content injection**, **Per-domain activation**
- Tests in [e2e/tests/](e2e/tests/) — main suite [e2e/tests/extension.spec.ts](e2e/tests/extension.spec.ts) with describe blocks: **Popup**, **Content injection**, **Per-domain activation**; [e2e/tests/toolbar-settings.spec.ts](e2e/tests/toolbar-settings.spec.ts) covers toolbar settings menu (Feature Voting upvotes multi-on + isolation, behaviour toggles)
- Shared fixture in [e2e/fixtures.ts](e2e/fixtures.ts)
- Page helpers in [e2e/pages/](e2e/pages/) (e.g. [e2e/pages/popup.ts](e2e/pages/popup.ts))
- Fixture page served via Playwright `webServer` + custom static server:
Expand Down
4 changes: 4 additions & 0 deletions .cursor/skills/skill-hygiene/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ When updating a skill (see `.cursor/rules/skill-maintenance.mdc` for when to do
- **Fixture:** context/extensionId and MV3 vs background page still match [e2e/fixutes.ts](e2e/fixutes.ts).
- **Run:** `pnpm test:e2e` and build-before-test still correct.

## code-quality-principles

- Thin Cursor skill points at [.agents/dev/skills/code-quality-principles.md](../../../.agents/dev/skills/code-quality-principles.md); if that path or agent roles change, update the skill and [AGENTS.md](../../../AGENTS.md).

## After editing

- Keep SKILL.md under 500 lines; move long detail to reference.md if needed.
Expand Down
2 changes: 1 addition & 1 deletion .cursor/skills/wxt-svelte-extension/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ description: Provides project context for the amgiflol WXT + Svelte 5 browser ex

## Stack

- WXT, Svelte 5 (runes), UnoCSS
- WXT, Svelte 5 (runes), UnoCSS, `@lucide/svelte` v1 for icons
- Entrypoints in [src/entrypoints/](src/entrypoints/) (e.g. background.ts, content.ts)
- Config in [wxt.config.ts](wxt.config.ts); srcDir `src`, outDir `dist`

Expand Down
10 changes: 10 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,16 @@ High-level agents for this project:
- Focus: Svelte 5 + WXT extension code (popup, background, settings, stores).
- Reads: this file, Svelte/WXT rules, and relevant e2e sections.
- Must keep diffs small, update tests when behavior changes, and ensure `pnpm check`, `pnpm lint`, and relevant tests pass.
- **Dev agent (code quality) – `code-quality`**:
- Focus: Review, refactors, naming, and structure using `.agents/dev/skills/code-quality-principles.md`.
- Reads: that file, `.agents/dev/agents/code-quality.md`, the nearest `AGENTS.md`, and relevant code.
- When to use: see **Choosing dev agents** below.

### Choosing dev agents

- **`typescript-pro`**: New features, bug fixes that change behaviour, wiring stores/popup/background, and any work whose main deliverable is implementation in `src/`.
- **`code-quality`**: PR-style review, refactors driven by structure/naming/decomposition, applying the numbered principles in `.agents/dev/skills/code-quality-principles.md`, cleanup without a feature spec, or when the user asks for a quality or review pass.

- **QA agent – `test-automator`**:
- Focus: Playwright e2e tests and fixtures under `e2e/`.
- Reads: testing and e2e sections here plus any nested `AGENTS.md` inside `e2e/` (if present).
Expand Down
1 change: 0 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,6 @@

- 9e2516f: unblock triggering of shorcuts while toolbar is focused
- d75a508: update tracker on scroll and window resize

1. Fixes [#12](https://github.com/sm17p/amgiflol/issues/12)
2. Partial fix [#7](https://github.com/sm17p/amgiflol/issues/7)

Expand Down
4 changes: 3 additions & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,7 @@
This project uses the `AGENTS.md` standard as the primary source of instructions for coding agents.

- Read the nearest `AGENTS.md` before making changes.
- Follow the agent roles described there (`extension-dev`, `e2e-qa`, `docs-maintainer`, `release-manager`) instead of inventing new ones.
- Follow the agent roles described there (`typescript-pro`, `code-quality`, `test-automator`, `docs-maintainer`, `release-manager`) instead of inventing new ones.
- When `CLAUDE.md` and `AGENTS.md` conflict, prefer the guidance in `AGENTS.md`.

Cross-language code quality principles (optional context, loaded on demand): @.agents/dev/skills/code-quality-principles.md
2 changes: 1 addition & 1 deletion LICENSE.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
MIT License
# MIT License

Copyright (c) 2025 Smit Patel

Expand Down
2 changes: 1 addition & 1 deletion e2e/tests/popup.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import pkg from "../../package.json" assert { type: "json" };
import pkg from "../../package.json" with { type: "json" };
import { expect, test } from "../fixtures";
import { getExtensionToolbar } from "../pages/extension";
import {
Expand Down
80 changes: 79 additions & 1 deletion e2e/tests/toolbar-settings.spec.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import type { Locator } from "@playwright/test";

import { expect, test } from "../fixtures";
import { openToolbarSettings } from "../pages/extension";
import {
Expand All @@ -7,6 +9,16 @@
openStableTestPage,
} from "../pages/web";

async function ensureVotePressedOn(locator: Locator): Promise<void> {
for (let i = 0; i < 2; i++) {
if ((await locator.getAttribute("aria-pressed")) === "true") {
return;
}
await locator.click();
}
await expect(locator).toHaveAttribute("aria-pressed", "true");
}

test.describe("Toolbar settings menu", () => {
test("feature voting and behaviour toggles are visible and interactive", async ({
context,
Expand All @@ -25,14 +37,24 @@
const featureVoting = page.getByRole("group", { name: "Feature Voting" }).first();
await expect(featureVoting).toBeVisible();

const animationDebuggerVote = page
.getByRole("button", { name: "Animation Debugger" })
.first();
await expect(animationDebuggerVote).toBeVisible();
await animationDebuggerVote.scrollIntoViewIfNeeded();
const votePressedBefore = await animationDebuggerVote.getAttribute("aria-pressed");
await animationDebuggerVote.click();
const votePressedAfter = await animationDebuggerVote.getAttribute("aria-pressed");
expect(votePressedAfter).not.toBe(votePressedBefore);

const toolbarAutoHideItem = page
.getByRole("menuitemcheckbox", { name: /Auto-Hide/i })
.first();
await expect(toolbarAutoHideItem).toBeVisible();
await toolbarAutoHideItem.scrollIntoViewIfNeeded();

const autoHideBefore = await toolbarAutoHideItem.getAttribute("aria-checked");
await toolbarAutoHideItem.click({ force: true });
await toolbarAutoHideItem.click();

const viewport = page.viewportSize();
expect(viewport).toBeTruthy();
Expand All @@ -44,9 +66,65 @@
const toolbarAutoHideItemAfter = page
.getByRole("menuitemcheckbox", { name: /Auto-Hide/i })
.first();
await expect(toolbarAutoHideItemAfter).toBeVisible();

Check failure on line 69 in e2e/tests/toolbar-settings.spec.ts

View workflow job for this annotation

GitHub Actions / test

[chromium] › e2e/tests/toolbar-settings.spec.ts:23:2 › Toolbar settings menu › feature voting and behaviour toggles are visible and interactive

1) [chromium] › e2e/tests/toolbar-settings.spec.ts:23:2 › Toolbar settings menu › feature voting and behaviour toggles are visible and interactive Retry #1 ─────────────────────────────────────────────────────────────────────────────────────── Error: expect(locator).toBeVisible() failed Locator: getByRole('menuitemcheckbox', { name: /Auto-Hide/i }).first() Expected: visible Timeout: 5000ms Error: element(s) not found Call log: - Expect "toBeVisible" with timeout 5000ms - waiting for getByRole('menuitemcheckbox', { name: /Auto-Hide/i }).first() 67 | .getByRole("menuitemcheckbox", { name: /Auto-Hide/i }) 68 | .first(); > 69 | await expect(toolbarAutoHideItemAfter).toBeVisible(); | ^ 70 | 71 | const autoHideAfter = await toolbarAutoHideItemAfter.getAttribute("aria-checked"); 72 | expect(autoHideAfter).not.toBe(autoHideBefore); at /home/runner/work/amgiflol/amgiflol/e2e/tests/toolbar-settings.spec.ts:69:42

Check failure on line 69 in e2e/tests/toolbar-settings.spec.ts

View workflow job for this annotation

GitHub Actions / test

[chromium] › e2e/tests/toolbar-settings.spec.ts:23:2 › Toolbar settings menu › feature voting and behaviour toggles are visible and interactive

1) [chromium] › e2e/tests/toolbar-settings.spec.ts:23:2 › Toolbar settings menu › feature voting and behaviour toggles are visible and interactive Error: expect(locator).toBeVisible() failed Locator: getByRole('menuitemcheckbox', { name: /Auto-Hide/i }).first() Expected: visible Timeout: 5000ms Error: element(s) not found Call log: - Expect "toBeVisible" with timeout 5000ms - waiting for getByRole('menuitemcheckbox', { name: /Auto-Hide/i }).first() 67 | .getByRole("menuitemcheckbox", { name: /Auto-Hide/i }) 68 | .first(); > 69 | await expect(toolbarAutoHideItemAfter).toBeVisible(); | ^ 70 | 71 | const autoHideAfter = await toolbarAutoHideItemAfter.getAttribute("aria-checked"); 72 | expect(autoHideAfter).not.toBe(autoHideBefore); at /home/runner/work/amgiflol/amgiflol/e2e/tests/toolbar-settings.spec.ts:69:42

const autoHideAfter = await toolbarAutoHideItemAfter.getAttribute("aria-checked");
expect(autoHideAfter).not.toBe(autoHideBefore);
});

test("Feature Voting upvotes can both be on and do not cross-toggle", async ({
context,
extensionId: _extensionId,
page,
}) => {
test.setTimeout(15_000);

await enableStableDomainInStorage(context);
await openStableTestPage(page);
await expect(getExtensionRoot(page)).toHaveCount(1);
await expectSvelteAppLoaded(page);

await openToolbarSettings(page);

const featureVoting = page.getByRole("group", { name: "Feature Voting" }).first();
await expect(featureVoting).toBeVisible();

const voteA = featureVoting.getByRole("button", {
name: "Animation Debugger",
});
const voteB = featureVoting.getByRole("button", { name: "Color Debugger" });

const featureVotingSwitch = featureVoting.getByRole("switch", {
name: "Feature Voting",
});
if (!(await voteA.isVisible())) {
await featureVotingSwitch.click();
}

await expect(voteA).toBeVisible();
await expect(voteB).toBeVisible();
await voteA.scrollIntoViewIfNeeded();
await voteB.scrollIntoViewIfNeeded();

await ensureVotePressedOn(voteA);
const aPressedAfterAOnRaw = await voteA.getAttribute("aria-pressed");
if (aPressedAfterAOnRaw !== "true") {
throw new Error(
`expected vote A aria-pressed "true", got ${String(aPressedAfterAOnRaw)}`,
);
}
const aPressedAfterAOn = aPressedAfterAOnRaw;

for (let i = 0; i < 2; i++) {
if ((await voteB.getAttribute("aria-pressed")) === "true") {
break;
}
expect(await voteA.getAttribute("aria-pressed")).toBe(aPressedAfterAOn);
await voteB.click();
}

await expect(voteA).toHaveAttribute("aria-pressed", "true");
await expect(voteB).toHaveAttribute("aria-pressed", "true");
await expect(voteA).toHaveAttribute("aria-pressed", aPressedAfterAOn);
});
});
Loading
Loading