-
Notifications
You must be signed in to change notification settings - Fork 852
Improve AGENTS.md #927
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Improve AGENTS.md #927
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -141,7 +141,10 @@ Usage notes: | |
| - ALWAYS run `npm run lint` before committing - CI will fail otherwise | ||
| - ALWAYS run `npm run pretty` to format code consistently | ||
| - ESLint configuration in `.eslintrc.json` enforces React/JSX standards | ||
| - Prettier configuration in `.prettierrc` handles formatting | ||
| - Prettier configuration in `.prettierrc` handles formatting (100 char width, no semicolons, single quotes, trailing commas) | ||
|
|
||
| ✅ Good: `import Browser from 'webextension-polyfill'` (single quotes, no semicolon) | ||
| ❌ Bad: `import Browser from "webextension-polyfill";` (double quotes, semicolon) | ||
|
|
||
| - Naming conventions: component directories use PascalCase; feature folders use kebab-case; entry files are typically `index.jsx` or `index.mjs` | ||
| - Avoid heavy dependencies; if necessary, justify and keep bundle size under control | ||
|
|
@@ -166,6 +169,7 @@ Usage notes: | |
| - Commit subject: imperative, capitalize first word; separate subject/body with a blank line; wrap at ~72 characters; explain what and why. | ||
| - PRs: link related issues, summarize scope/behavior changes; include screenshots for UI changes. | ||
| - Note i18n updates in PR description when `src/_locales/` changes. | ||
| - If any validation step is skipped, include `Validation skipped: <reason>; <runtime impact or "no runtime files touched">` in the PR description. | ||
|
|
||
| ### Directory Structure | ||
|
|
||
|
|
@@ -204,7 +208,7 @@ src/ | |
|
|
||
| ## Localization | ||
|
|
||
| - Source of truth: `src/_locales/en/main.json`; do not change keys | ||
| - Source of truth: `src/_locales/en/main.json`; do not change existing keys (only add new ones) | ||
| - Add new strings to `en/main.json` first, then propagate to other locales | ||
| - Register new locales in `src/_locales/resources.mjs` | ||
| - Preserve placeholders and product names; keep punctuation/quotes intact | ||
|
|
@@ -240,6 +244,8 @@ The extension supports multiple AI providers: | |
| - Updating API integration: Modify files in `src/services/apis/` | ||
| - Adding new UI component: Create in `src/components/` | ||
|
|
||
| **Note:** Ask before deleting/renaming files, modifying build config/manifests, or making changes that affect multiple site adapters. If the user explicitly requests one of these changes, proceed and document scope and risk in the PR summary. | ||
|
|
||
| ## Time Expectations | ||
|
|
||
| - Do not interrupt builds or long-running commands unless they appear hung or unresponsive. | ||
|
|
@@ -252,12 +258,10 @@ The extension supports multiple AI providers: | |
|
|
||
| ## Critical Validation Steps | ||
|
|
||
| 1. ALWAYS run `npm run build` after any code changes | ||
| 2. ALWAYS manually load and test the built extension in a browser (no automated testing available) | ||
| 3. ALWAYS verify the build creates expected file structure (non-empty bundles, no missing files) | ||
| 4. ALWAYS test core extension functionality (popup, content script injection, keyboard shortcuts) | ||
|
|
||
| Always build and manually test the extension in a browser before considering any change complete. Simply running the build is NOT sufficient - you must load the extension and test actual functionality. | ||
| 1. Runtime-impacting changes (`src/**`, `src/manifest*.json`, `build.mjs`, `package*.json`, `safari/**`): run `npm run build`, verify expected build artifacts, and run manual browser smoke tests. | ||
| 2. Behavior-adjacent localization changes (`src/_locales/**` only): run `npm run build` and manual browser smoke tests. | ||
| 3. Docs-only changes (`*.md`, `screenshots/**`): build/manual browser tests may be skipped, but the PR description must include `Validation skipped: docs-only change; no runtime files touched`. | ||
| 4. When in doubt, treat the change as runtime-impacting and execute the full validation flow. | ||
|
Comment on lines
+261
to
+264
|
||
|
|
||
| --- | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's an inconsistency in how the validation skip format is presented. Line 172 uses angle brackets as placeholders (
<reason>,<runtime impact or "no runtime files touched">), while line 263 provides a concrete example without angle brackets (docs-only change; no runtime files touched.). While angle brackets typically indicate placeholders in documentation, it would be clearer to explicitly state this or use a consistent format. Consider adding text like "(replace with actual values)" after the template on line 172, or reformatting the template to match the concrete example style more clearly.