Skip to content

Commit 4bf766d

Browse files
authored
🤖 Address comments from #7194 (#7198)
## Summary Addresses Christian's fantastic comments. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7198-Address-comments-from-7194-2c16d73d365081aaa7a1ea06c2d86ec0) by [Unito](https://www.unito.io)
1 parent 10fddc9 commit 4bf766d

File tree

1 file changed

+31
-49
lines changed

1 file changed

+31
-49
lines changed

‎AGENTS.md‎

Lines changed: 31 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333

3434
## Monorepo Architecture
3535

36-
The project now uses **Nx** for build orchestration and task management:
36+
The project uses **Nx** for build orchestration and task management:
3737

3838
- **Task Orchestration**: Commands like `dev`, `build`, `lint`, and `test:browser` run via Nx
3939
- **Caching**: Nx provides intelligent caching for faster rebuilds
@@ -97,13 +97,29 @@ Key Nx features:
9797
- aim to cover critical logic and new features
9898
- Playwright:
9999
- optional tags like `@mobile`, `@2x` are respected by config
100+
- Tests to avoid
101+
- Change detector tests
102+
- e.g. a test that just asserts that the defaults are certain values
103+
- Tests that are dependent on non-behavioral features like utility classes or styles
104+
- Redundant tests
100105

101106
## Commit & Pull Request Guidelines
102107

103-
- Commits: Use `[skip ci]` for locale-only updates when appropriate
104-
- PRs: Include clear description, linked issues (`- Fixes #123`), and screenshots/GIFs for UI changes
105-
- Quality gates: `pnpm lint`, `pnpm typecheck`, and relevant tests must pass
108+
- PRs:
109+
- Include clear description
110+
- Reference linked issues (e.g. `- Fixes #123`)
111+
- Keep it extremely concise and information-dense
112+
- Don't use emojis or add excessive headers/sections
113+
- Follow the PR description template in the `.github/` folder.
114+
- Quality gates:
115+
- `pnpm lint`
116+
- `pnpm typecheck`
117+
- `pnpm knip`
118+
- Relevant tests must pass
119+
- Never use `--no-verify` to bypass failing tests
120+
- Identify the issue and present root cause analysis and possible solutions if you are unable to solve quickly yourself
106121
- Keep PRs focused and small
122+
- If it looks like the current changes will have 300+ lines of non-test code, suggest ways it could be broken into multiple PRs
107123

108124
## Security & Configuration Tips
109125

@@ -150,6 +166,10 @@ Key Nx features:
150166
9. Use Vite for fast development and building
151167
10. Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
152168
11. Avoid new usage of PrimeVue components
169+
12. Write tests for all changes, especially bug fixes to catch future regressions
170+
13. Write code that is expressive and self-documenting to the furthest degree possible. This reduces the need for code comments which can get out of sync with the code itself. Try to avoid comments unless absolutely necessary
171+
14. Whenever a new piece of code is written, the author should ask themselves 'is there a simpler way to introduce the same functionality?'. If the answer is yes, the simpler course should be chosen
172+
15. Refactoring should be used to make complex code simpler
153173

154174
## External Resources
155175

@@ -188,55 +208,17 @@ When referencing Comfy-Org repos:
188208
2. Use GitHub API for branches/PRs/metadata
189209
3. Curl GitHub website if needed
190210

191-
## Settings and Feature Flags Quick Reference
192-
193-
### Settings Usage
194-
195-
```typescript
196-
const settingStore = useSettingStore()
197-
const value = settingStore.get('Comfy.SomeSetting') // Get setting
198-
await settingStore.set('Comfy.SomeSetting', newValue) // Update setting
199-
```
200-
201-
### Dynamic Defaults
202-
203-
```typescript
204-
{
205-
id: 'Comfy.Example.Setting',
206-
defaultValue: () => window.innerWidth < 1024 ? 'small' : 'large' // Runtime context
207-
}
208-
```
209-
210-
### Version-Based Defaults
211-
212-
```typescript
213-
{
214-
id: 'Comfy.Example.Feature',
215-
defaultValue: 'legacy',
216-
defaultsByInstallVersion: { '1.25.0': 'enhanced' } // Gradual rollout
217-
}
218-
```
219-
220-
### Feature Flags
221-
222-
```typescript
223-
if (api.serverSupportsFeature('feature_name')) { // Check capability
224-
// Use enhanced feature
225-
}
226-
const value = api.getServerFeature('config_name', defaultValue) // Get config
227-
```
228-
229-
**Documentation:**
230-
231-
- Settings system: `docs/SETTINGS.md`
232-
- Feature flags system: `docs/FEATURE_FLAGS.md`
233-
234211
## Common Pitfalls
235212

236213
- NEVER use `any` type - use proper TypeScript types
237214
- NEVER use `as any` type assertions - fix the underlying type issue
238215
- NEVER use `--no-verify` flag when committing
239216
- NEVER delete or disable tests to make them pass
240217
- NEVER circumvent quality checks
241-
- NEVER use `dark:` or `dark-theme:` tailwind variants. Instead use a semantic value from the `style.css` theme, e.g. `bg-node-component-surface`
242-
- NEVER use `:class="[]"` to merge class names - always use `import { cn } from '@/utils/tailwindUtil'`, for example: `<div :class="cn('text-node-component-header-icon', hasError && 'text-danger')" />`
218+
- NEVER use the `dark:` tailwind variant
219+
- Instead use a semantic value from the `style.css` theme
220+
- e.g. `bg-node-component-surface`
221+
- NEVER use `:class="[]"` to merge class names
222+
- Always use `import { cn } from '@/utils/tailwindUtil'`
223+
- e.g. `<div :class="cn('text-node-component-header-icon', hasError && 'text-danger')" />`
224+
- Use `cn()` inline in the template when feasible instead of creating a `computed` to hold the value

0 commit comments

Comments
 (0)