Conversation
📝 WalkthroughWalkthroughCondenses and rewrites Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ai_guidelines/frontend.md`:
- Line 28: The fenced code block in ai_guidelines/frontend.md (the
triple-backtick block around the src/ tree) is missing a language tag causing
markdownlint MD040; update the opening fence from ``` to ```text (or another
appropriate language identifier) so the block is explicitly labeled (e.g.,
change the opening fence of the src/ directory snippet in the code block to
```text).
- Line 233: The import example incorrectly includes a .ts extension; update the
documentation line that imports useCounter so it uses an extensionless import
(import { useCounter } from './useCounter';) to match the project's TypeScript
configuration and avoid implying allowImportingTsExtensions is enabled; locate
the example referencing useCounter and remove the ".ts" suffix from the import.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a909150b-f9f7-40cc-a2ea-f478f0a346b0
📒 Files selected for processing (1)
ai_guidelines/frontend.md
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 744d98f410
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Code Review: Condense frontend guidelinesSummaryThis PR reduces Blocking IssuesNone — this is a documentation-only change with no runtime impact. Non-blocking Suggestions1. Missing
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
ai_guidelines/frontend.md (2)
28-28:⚠️ Potential issue | 🟡 MinorAdd language tag to the fenced code block.
The code fence is missing a language identifier, causing markdownlint MD040 to fail.
Proposed fix
-``` +```text src/ components/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ai_guidelines/frontend.md` at line 28, The fenced code block closing/backtick is missing a language tag; update the opening fence for that snippet (the ``` fence that precedes the "src/ components/" lines) to include a language identifier such as "text" (i.e., change the opening ``` to ```text) so markdownlint MD040 passes and the block is properly labeled.
253-253:⚠️ Potential issue | 🟡 MinorRemove
.tsextension from import example.The import example includes a
.tsextension, but the repository's TypeScript configuration doesn't enableallowImportingTsExtensions. Use extensionless imports to match the project setup.Proposed fix
- import { useCounter } from './useCounter.ts'; + import { useCounter } from './useCounter';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ai_guidelines/frontend.md` at line 253, The import example currently uses a .ts extension which isn't supported by the project's TS config; update the example to use an extensionless import for useCounter by removing ".ts" from the import statement (look for the line importing useCounter and replace "import { useCounter } from './useCounter.ts';" with an extensionless import).
🧹 Nitpick comments (2)
ai_guidelines/frontend.md (2)
8-8: Consider the 100-line limit's practical implications.While concise components are generally beneficial, a strict 100-line limit may force artificial splits in cohesive logic. Consider adding guidance about when exceptions are acceptable (e.g., complex forms, state machines with many transitions).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ai_guidelines/frontend.md` at line 8, Update the guideline "Keep components under 100 lines" to acknowledge practical exceptions: modify the rule text to allow larger components when justified (e.g., complex forms, state machines with many transitions, performance-optimized components) and add a short section describing criteria for exemptions and examples of acceptable cases; reference the original rule text ("Keep components under 100 lines") and add two brief bullets: "When to split" and "When exceptions apply" with example scenarios and a note to prefer extracting utilities/hooks for testable parts (so reviewers can enforce intent rather than a hard line count).
202-202: Minor grammar suggestion.Consider hyphenating "data fetching" as "data-fetching" when used as a compound adjective before "operations."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ai_guidelines/frontend.md` at line 202, Update the sentence "Implement proper error handling for data fetching operations." in ai_guidelines/frontend.md to hyphenate the compound adjective by changing it to "Implement proper error handling for data-fetching operations." so the phrase is grammatically correct when used before "operations."
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ai_guidelines/frontend.md`:
- Around line 87-94: The guideline's blanket rule "Avoid exporting/importing
types" is too restrictive; update ai_guidelines/frontend.md to allow exporting
explicit type definitions for public APIs and shared domain models (e.g., User,
ApiResponse) while keeping utility-derived types (Parameters<typeof
useData>/ReturnType<typeof useData> like UseDataParams, UseDataReturn,
TitleOnly, WithoutTitle) limited to internal/private use; specifically, change
the wording to permit exporting/importing types for public contracts and shared
models, add a short rationale and examples showing when to export a type vs
derive it from functions like useData, and mark utility derivation as an
acceptable pattern only for internal implementation surfaces.
- Around line 160-165: The +error.svelte component imports page from $app/stores
which violates the guideline; update the import to import page from "$app/state"
in +error.svelte and verify any usages of page (e.g., page.params or page.route)
still work without a $ prefix, adjusting code accordingly; after change, run a
quick build/test to ensure the error route compiles and behaves the same.
---
Duplicate comments:
In `@ai_guidelines/frontend.md`:
- Line 28: The fenced code block closing/backtick is missing a language tag;
update the opening fence for that snippet (the ``` fence that precedes the "src/
components/" lines) to include a language identifier such as "text" (i.e.,
change the opening ``` to ```text) so markdownlint MD040 passes and the block is
properly labeled.
- Line 253: The import example currently uses a .ts extension which isn't
supported by the project's TS config; update the example to use an extensionless
import for useCounter by removing ".ts" from the import statement (look for the
line importing useCounter and replace "import { useCounter } from
'./useCounter.ts';" with an extensionless import).
---
Nitpick comments:
In `@ai_guidelines/frontend.md`:
- Line 8: Update the guideline "Keep components under 100 lines" to acknowledge
practical exceptions: modify the rule text to allow larger components when
justified (e.g., complex forms, state machines with many transitions,
performance-optimized components) and add a short section describing criteria
for exemptions and examples of acceptable cases; reference the original rule
text ("Keep components under 100 lines") and add two brief bullets: "When to
split" and "When exceptions apply" with example scenarios and a note to prefer
extracting utilities/hooks for testable parts (so reviewers can enforce intent
rather than a hard line count).
- Line 202: Update the sentence "Implement proper error handling for data
fetching operations." in ai_guidelines/frontend.md to hyphenate the compound
adjective by changing it to "Implement proper error handling for data-fetching
operations." so the phrase is grammatically correct when used before
"operations."
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0fe0bd80-1d34-419f-be67-0e0f3501fc91
📒 Files selected for processing (1)
ai_guidelines/frontend.md
|
/review |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
ai_guidelines/frontend.md (1)
28-28:⚠️ Potential issue | 🟡 MinorAdd a language identifier to the fenced code block.
The code fence is still unlabeled, so markdownlint
MD040will continue to warn.Proposed fix
-``` +```text src/ components/ ... -``` +```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ai_guidelines/frontend.md` at line 28, The fenced code block in ai_guidelines/frontend.md is missing a language identifier (triggering markdownlint MD040); update the opening fence from ``` to a labeled fence like ```text (or an appropriate language such as ```bash or ```json) so the block reads ```text ... ``` and removes the linter warning.
🧹 Nitpick comments (3)
ai_guidelines/frontend.md (3)
265-269: Add a minimal Context API code example.This section lists Context API as first choice but gives no example, which makes the guidance less actionable for assistants/readers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ai_guidelines/frontend.md` around lines 265 - 269, The section names Svelte Context API (setContext/getContext) but lacks an actionable example—add a minimal, typed example showing how to call setContext in a parent component to provide a value (e.g., an interface or store) and how a child uses getContext to consume it; reference the symbols setContext/getContext and optionally the Snippet type if demonstrating passing UI composition, ensure the example is small (one parent and one child) and includes type annotations for clarity.
288-302: Include a short integration-test example to match the stated test levels.You define unit/integration/E2E, but only show a unit sample. A compact interaction-based integration snippet would make the distinction concrete.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ai_guidelines/frontend.md` around lines 288 - 302, Add a compact integration-test example showing component interaction (e.g., rendering a parent or form component that composes child components and asserting user-driven state changes) to match the stated test levels; update the examples block near the unit test for MyComponent by adding a short integration snippet that uses render + user-event (or Svelte's fireEvent) to simulate user interaction across components, then assert resulting DOM changes or emitted events (reference MyComponent or a small ParentComponent/FormComponent composition) so readers can see how integration tests differ from the unit test.
158-158: Use explicit terminology to distinguish the two reactive declaration approaches.The phrase "do not mix the two approaches" is ambiguous and should name them directly. According to current Svelte 5 guidance, use:
- "derived stores" for
derived()fromsvelte/store- "
$derivedrune" (or "derived state") for the$derivedruneSuggested revision: "use
$derivedrune orderived()stores, not$:. Do not mix derived stores and$derivedrunes in the same file/module."Additionally, consider recommending that imports of the
derivedfunction be renamed/aliased (e.g.,import { derived as derivedStore } from 'svelte/store') to prevent mental confusion with the$derivedrune, especially during migrations to runes mode.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ai_guidelines/frontend.md` at line 158, Update the "Reactive declarations" line to explicitly name the two approaches — use "`$derived` rune" or "derived stores" (the derived() function from svelte/store) instead of the vague phrase "do not mix the two approaches"; change the sentence to: "use `$derived` rune or `derived()` stores, not `$:`. Do not mix derived stores and `$derived` runes in the same file/module." Also add a short recommendation to alias the imported function to avoid confusion during migrations (e.g., suggest importing derived as derivedStore with import { derived as derivedStore } from 'svelte/store') and ensure references to `$derived`, `derived()` and `$:` are used exactly as written so readers can find the symbols easily.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@ai_guidelines/frontend.md`:
- Line 28: The fenced code block in ai_guidelines/frontend.md is missing a
language identifier (triggering markdownlint MD040); update the opening fence
from ``` to a labeled fence like ```text (or an appropriate language such as
```bash or ```json) so the block reads ```text ... ``` and removes the linter
warning.
---
Nitpick comments:
In `@ai_guidelines/frontend.md`:
- Around line 265-269: The section names Svelte Context API
(setContext/getContext) but lacks an actionable example—add a minimal, typed
example showing how to call setContext in a parent component to provide a value
(e.g., an interface or store) and how a child uses getContext to consume it;
reference the symbols setContext/getContext and optionally the Snippet type if
demonstrating passing UI composition, ensure the example is small (one parent
and one child) and includes type annotations for clarity.
- Around line 288-302: Add a compact integration-test example showing component
interaction (e.g., rendering a parent or form component that composes child
components and asserting user-driven state changes) to match the stated test
levels; update the examples block near the unit test for MyComponent by adding a
short integration snippet that uses render + user-event (or Svelte's fireEvent)
to simulate user interaction across components, then assert resulting DOM
changes or emitted events (reference MyComponent or a small
ParentComponent/FormComponent composition) so readers can see how integration
tests differ from the unit test.
- Line 158: Update the "Reactive declarations" line to explicitly name the two
approaches — use "`$derived` rune" or "derived stores" (the derived() function
from svelte/store) instead of the vague phrase "do not mix the two approaches";
change the sentence to: "use `$derived` rune or `derived()` stores, not `$:`. Do
not mix derived stores and `$derived` runes in the same file/module." Also add a
short recommendation to alias the imported function to avoid confusion during
migrations (e.g., suggest importing derived as derivedStore with import {
derived as derivedStore } from 'svelte/store') and ensure references to
`$derived`, `derived()` and `$:` are used exactly as written so readers can find
the symbols easily.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 014379c9-1883-471f-8f15-a96267a2383e
📒 Files selected for processing (1)
ai_guidelines/frontend.md
michal-lightly
deleted the
michal-lig-9026-update-ai-instructions--4-condense-frontend
branch
3 participants
What has changed and why?
Condense the frontend guidelines. They were too verbose and repetitive.
How has it been tested?
N/A
Did you update CHANGELOG.md?
Summary by CodeRabbit