🤖 Add code review and function declaration/expression preference to AGENTS.md #7577
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dosubot
bot
added
the
size:S
Contributor
📝 WalkthroughWalkthroughThe PR updates AGENTS.md to add a new coding guideline for function declarations and introduces comprehensive Code Review guidance sections with complexity checklists and design, test, naming, documentation, and style review items. The guidance appears in two separate locations within the document. Changes
Possibly related PRs
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 12/17/2025, 12:51:52 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results⏰ Completed at: 12/17/2025, 01:01:16 AM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
AGENTS.md(2 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:24.164Z
Learning: The project has CI automation that handles code formatting (pnpm format) automatically, so manual formatting suggestions should not be provided in code reviews for the Comfy-Org/ComfyUI_frontend repository.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.stories.ts:45-55
Timestamp: 2025-12-17T00:40:09.635Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, prefer pure function declarations (e.g., `function foo() { ... }`) over function expressions (e.g., `const foo = () => { ... }`) for pure functions, as function declarations are considered more functional than function expressions.
📚 Learning: 2025-12-09T20:22:23.620Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T20:22:23.620Z
Learning: Watch out for Code Smells and refactor to avoid them
Applied to files:
AGENTS.md
📚 Learning: 2025-12-09T20:22:23.620Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T20:22:23.620Z
Learning: Applies to **/*.{ts,tsx,vue} : Minimize nesting in code (e.g., deeply nested `if` or `for` statements); apply the Arrow Anti-Pattern principle
Applied to files:
AGENTS.md
📚 Learning: 2025-12-17T00:40:09.635Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.stories.ts:45-55
Timestamp: 2025-12-17T00:40:09.635Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, prefer pure function declarations (e.g., `function foo() { ... }`) over function expressions (e.g., `const foo = () => { ... }`) for pure functions, as function declarations are considered more functional than function expressions.
Applied to files:
AGENTS.md
📚 Learning: 2025-12-09T20:22:23.620Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T20:22:23.620Z
Learning: Applies to **/*.{ts,tsx,vue} : Favor pure functions, especially testable ones
Applied to files:
AGENTS.md
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Check tests-ui/README.md for test guidelines
Applied to files:
AGENTS.md
🪛 LanguageTool
AGENTS.md
[style] ~239-~239: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...lem that the developer speculates might need to be solved in the future. The future pro...
(REP_NEED_TO_VB)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: deploy-and-comment
- GitHub Check: lint-and-format
- GitHub Check: test
- GitHub Check: setup
- GitHub Check: collect
🔇 Additional comments (2)
AGENTS.md (2)
24-24: Good addition aligning with project practices.This codifies the preference documented in recent PR learnings and reinforces the functional programming approach.
220-233: Comprehensive and well-aligned code review guidance.These criteria reinforce the project's existing philosophy and best practices already established in the Development Guidelines section. The checklist is clear, actionable, and adds valuable structure to the review process.
Comment on lines
+235
to
+239
| #### [Complexity](https://google.github.io/eng-practices/review/reviewer/looking-for.html#complexity) | ||
|
|
||
| Is the CL more complex than it should be? Check this at every level of the CL—are individual lines too complex? Are functions too complex? Are classes too complex? “Too complex” usually means “can’t be understood quickly by code readers.” It can also mean “developers are likely to introduce bugs when they try to call or modify this code.” | ||
|
|
||
| A particular type of complexity is over-engineering, where developers have made the code more generic than it needs to be, or added functionality that isn’t presently needed by the system. Reviewers should be especially vigilant about over-engineering. Encourage developers to solve the problem they know needs to be solved now, not the problem that the developer speculates might need to be solved in the future. The future problem should be solved once it arrives and you can see its actual shape and requirements in the physical universe. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Polish the prose to reduce repetitive phrasing.
Lines 238–239 repeat "solve/solved" four times in close succession. While the guidance content is sound, consider rewording to improve readability.
- Encourage developers to solve the problem they know needs to be solved now, not the problem that the developer speculates might need to be solved in the future. The future problem should be solved once it arrives and you can see its actual shape and requirements in the physical universe.
+ Encourage developers to address the immediate problem they know needs solving now, rather than speculate about future problems. When a new problem actually emerges, they can see its concrete shape and requirements before solving it.Based on static analysis hint.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| #### [Complexity](https://google.github.io/eng-practices/review/reviewer/looking-for.html#complexity) | |
| Is the CL more complex than it should be? Check this at every level of the CL—are individual lines too complex? Are functions too complex? Are classes too complex? “Too complex” usually means “can’t be understood quickly by code readers.” It can also mean “developers are likely to introduce bugs when they try to call or modify this code.” | |
| A particular type of complexity is over-engineering, where developers have made the code more generic than it needs to be, or added functionality that isn’t presently needed by the system. Reviewers should be especially vigilant about over-engineering. Encourage developers to solve the problem they know needs to be solved now, not the problem that the developer speculates might need to be solved in the future. The future problem should be solved once it arrives and you can see its actual shape and requirements in the physical universe. | |
| #### [Complexity](https://google.github.io/eng-practices/review/reviewer/looking-for.html#complexity) | |
| Is the CL more complex than it should be? Check this at every level of the CL—are individual lines too complex? Are functions too complex? Are classes too complex? "Too complex" usually means "can't be understood quickly by code readers." It can also mean "developers are likely to introduce bugs when they try to call or modify this code." | |
| A particular type of complexity is over-engineering, where developers have made the code more generic than it needs to be, or added functionality that isn't presently needed by the system. Reviewers should be especially vigilant about over-engineering. Encourage developers to address the immediate problem they know needs solving now, rather than speculate about future problems. When a new problem actually emerges, they can see its concrete shape and requirements before solving it. |
🧰 Tools
🪛 LanguageTool
[style] ~239-~239: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...lem that the developer speculates might need to be solved in the future. The future pro...
(REP_NEED_TO_VB)
🤖 Prompt for AI Agents
In AGENTS.md around lines 235 to 239, the prose repeats "solve/solved" multiple
times in close succession, hurting readability; reword the passage to remove the
repeated phrasing by using a synonym or restructuring the sentence (e.g.,
replace one or two instances with words like "address", "resolve", or "implement
later", or combine clauses to avoid repetition), ensuring the original meaning
about avoiding over-engineering and favouring current solutions remains intact.
Contributor
Author
There was a problem hiding this comment.
It was pulled verbatim.
Contributor
There was a problem hiding this comment.
@DrJKL, understood! Since the text is quoted directly from the source, it makes sense to maintain the original wording. Thank you for clarifying.
🧠 Learnings used
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:24.164Z
Learning: The project has CI automation that handles code formatting (pnpm format) automatically, so manual formatting suggestions should not be provided in code reviews for the Comfy-Org/ComfyUI_frontend repository.
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.stories.ts:45-55
Timestamp: 2025-12-17T00:40:09.635Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, prefer pure function declarations (e.g., `function foo() { ... }`) over function expressions (e.g., `const foo = () => { ... }`) for pure functions, as function declarations are considered more functional than function expressions.
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.2 MB (baseline 3.2 MB) • ⚪ 0 BMain entry bundles and manifests
Graph Workspace — 992 kB (baseline 992 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Views & Navigation — 6.54 kB (baseline 6.54 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Panels & Settings — 297 kB (baseline 297 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
UI Components — 184 kB (baseline 184 kB) • ⚪ 0 BReusable component library chunks
Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Utilities & Hooks — 3.18 kB (baseline 3.18 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Vendor & Third-Party — 8.45 MB (baseline 8.45 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 3.41 MB (baseline 3.41 MB) • ⚪ 0 BBundles that do not match a named category
|
christian-byrne
approved these changes
Dec 17, 2025
| 22. Avoid mutable state, prefer immutability and assignment at point of declaration | ||
| 23. Favor pure functions (especially testable ones) | ||
| 24. Watch out for [Code Smells](https://wiki.c2.com/?CodeSmell) and refactor to avoid them | ||
| 24. Do not use function expressions if it's possible to use function declarations instead |
Contributor
There was a problem hiding this comment.
Is that a backtick or apostrophe? GitHub is rendering this line weirdly
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
From the Google Engineering Practices docs.
┆Issue is synchronized with this Notion page by Unito