chore: frontend tests - #42
Conversation
📝 WalkthroughWalkthroughAdds a centralized API interceptor (initializeApiInterceptors, unwrap/unwrapOr), new formatters, three UI components (Modal, Pagination, EventTypeIcon), widespread admin UI refactors to use them, many new/expanded tests, a lucide icon dependency, rollup resolve extensions, docs and tsconfig updates, plus test mocks and setup adjustments. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as Frontend Component
participant API as API Interceptor
participant Server as Backend API
participant Toast as Toast Notifier
participant Auth as Auth Store / Router
UI->>API: request (may include CSRF for non-GET)
API->>Server: forward HTTP request
Server-->>API: response (2xx / 401 / 403 / 422 / 429 / 5xx / network)
alt success (2xx)
API-->>UI: { data, error: undefined }
Note right of UI: UI calls unwrap/unwrapOr to obtain data
else 401
API->>Toast: show session-expired toast
API->>Auth: clear auth state, store redirect
API->>Auth: navigate to /login
API-->>UI: { data: undefined, error }
else 403/422/429/5xx
API->>Toast: show contextual error toast (validation/message)
API-->>UI: { data: undefined, error }
else Network Error
API->>Toast: show network error toast
API-->>UI: { data: undefined, error }
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
frontend/src/components/__tests__/Header.test.tsfrontend/src/components/__tests__/NotificationCenter.test.tsfrontend/src/components/__tests__/Spinner.test.tsfrontend/src/components/__tests__/ToastContainer.test.ts
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/components/__tests__/ToastContainer.test.ts (1)
frontend/src/stores/toastStore.ts (1)
addToast(16-25)
🪛 GitHub Actions: Frontend CI
frontend/src/components/__tests__/NotificationCenter.test.ts
[error] 835-835: AssertionError: expected "spy" to not be called at all, but actually been called 1 times. Failing test: 'keyboard navigation without action_url' in NotificationCenter.test.ts. Step: npm run test:coverage (vitest run --coverage).
🪛 GitHub Check: Unit Tests
frontend/src/components/__tests__/NotificationCenter.test.ts
[failure] 835-835: src/components/tests/NotificationCenter.test.ts > NotificationCenter > keyboard navigation without action_url > ignores non-Enter keydown events
AssertionError: expected "spy" to not be called at all, but actually been called 1 times
Received:
1st spy call:
Array [
"1",
]
Number of calls: 1
❯ src/components/tests/NotificationCenter.test.ts:835:58
⏰ 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). (1)
- GitHub Check: Scan Frontend
🔇 Additional comments (6)
frontend/src/components/__tests__/ToastContainer.test.ts (1)
123-213: LGTM! Comprehensive mouse interaction and timer edge case coverage.The new test suites effectively cover:
- Pause/resume timer behavior on mouseenter/mouseleave events
- Timer clearing and restarting logic
- Independent timers for multiple concurrent toasts
- Proper cleanup on component unmount
The timer management is correct, appropriately switching between fake and real timers as needed for each test scenario.
frontend/src/components/__tests__/NotificationCenter.test.ts (3)
90-129: LGTM! Robust MockEventSource implementation.The instance tracking and helper methods (
simulateMessage,simulateError,getLastInstance) enable precise testing of SSE behavior. The staticinstancesarray andclearInstances()method ensure proper test isolation.
379-622: Excellent SSE test coverage.The test suite comprehensively covers:
- Connection lifecycle (connect on auth, close on logout)
- Message handling (valid notifications, heartbeat, connected events)
- Error handling (parse errors, reconnection logic)
- Edge cases (max retries, auth state during errors)
624-826: LGTM! Comprehensive coverage of edge cases and interactions.The additional test suites effectively cover:
- External URL navigation with proper window.location mocking
- Already-read notification handling
- Icon rendering for various tag combinations
- Priority color classes
- Time formatting edge cases
- Auto-mark-as-read behavior with proper timer control
- Keyboard navigation scenarios
frontend/src/components/__tests__/Spinner.test.ts (1)
77-104: LGTM! Good defensive testing for invalid props.The fallback tests appropriately use
@ts-expect-errorto bypass TypeScript validation and verify runtime resilience. Testing that invalid props fall back to sensible defaults (medium size, primary color) ensures the component degrades gracefully.frontend/src/components/__tests__/Header.test.ts (1)
236-293: LGTM! Thorough coverage of responsive behavior and interaction patterns.The new tests effectively cover:
- Outside-click handling for dropdown closure using
document.body.click()- Responsive behavior: mobile menu closing on resize to desktop width
- Mobile detection on mount for narrow viewports
- Mobile menu logout flow with proper width configuration
The tests properly manipulate
window.innerWidthand dispatch resize events to simulate responsive scenarios.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/src/components/__tests__/NotificationCenter.test.ts (1)
399-424: Consider using literal RegExp patterns for test assertions (optional).Static analysis flags RegExp construction from variables at lines 410 and 455. While the ReDoS risk is negligible in test contexts with controlled input, you could eliminate the warning by using template literals in
getByRole:// Instead of: screen.getByRole('button', { name: new RegExp(`View notification: ${subject}`, 'i') }) // Consider: screen.getByRole('button', { name: /View notification: /i })However, this reduces test precision. The current approach is acceptable for tests where input is fully controlled.
Also applies to: 443-465
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/__tests__/NotificationCenter.test.ts
🧰 Additional context used
🪛 ast-grep (0.40.3)
frontend/src/components/__tests__/NotificationCenter.test.ts
[warning] 409-409: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(View notification: ${subject}, 'i')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 454-454: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(View notification: ${subject}, 'i')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ 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). (1)
- GitHub Check: E2E Tests
🔇 Additional comments (5)
frontend/src/components/__tests__/NotificationCenter.test.ts (5)
90-125: Excellent mock design for SSE testing.The
MockEventSourceimplementation with instance tracking is well-structured and provides comprehensive testing capabilities. The static methodsclearInstances()andgetLastInstance()enable proper test isolation and assertions on connection behavior.
131-185: Well-organized test helpers improve maintainability.The helper functions effectively reduce duplication and provide clear abstractions for common test scenarios. The
withMockedLocationhelper properly handles cleanup in the finally block, ensuring test isolation.
187-232: Data-driven test approach enhances coverage and maintainability.The test data arrays enable comprehensive coverage through parameterized tests while keeping the test logic DRY. The descriptive field names make the test cases easy to understand.
426-440: Previous review issue has been correctly addressed.The test now uses
vi.useFakeTimers()and properly configuresuserEvent.setup({ advanceTimers: vi.advanceTimersByTime })to prevent the auto-mark-as-read timer from interfering with the assertion. This fixes the flaky test behavior identified in the previous review.
483-600: Comprehensive SSE testing with proper timer management.The SSE test suites provide excellent coverage of connection lifecycle, message handling, and error scenarios. The use of fake timers is correctly applied, and the tests properly clean up with
vi.useRealTimers().The max reconnection attempts test (lines 568-582) simulates 4 error cycles effectively, though be aware it could become brittle if the reconnection timing logic changes in the component implementation.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
frontend/src/components/__tests__/NotificationCenter.test.ts (2)
434-445: Minor: Test description says "2s" but waits 2500ms.The test description says "marks notifications after 2s delay" but advances timers by 2500ms. If the component's actual delay is 2.5s, consider updating the description to match. This is a minor documentation clarity issue.
🔎 Proposed fix
describe('auto-mark as read', () => { - it('marks notifications after 2s delay', async () => { + it('marks notifications after delay when dropdown opens', async () => { vi.useFakeTimers();
493-501: Add a clarifying comment explaining the loop count.The test loops 4 times to verify that the component stops reconnecting after exceeding its max attempt limit. Since the component has
maxReconnectAttempts = 3, the 4th simulated error should trigger the final error log. A comment linking the test logic to the component constant improves maintainability.it('logs error after max reconnection attempts', async () => { await setupSSEWithFakeTimers(); + // Component has maxReconnectAttempts = 3, so 4 errors exhaust all attempts for (let i = 0; i < 4; i++) { MockEventSource.getLastInstance()!.simulateError(); await vi.advanceTimersByTimeAsync(30000); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/__tests__/NotificationCenter.test.ts
🧰 Additional context used
🪛 ast-grep (0.40.3)
frontend/src/components/__tests__/NotificationCenter.test.ts
[warning] 388-388: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(View notification: ${subject}, 'i')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 414-414: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(View notification: ${method}, 'i')
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ 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). (2)
- GitHub Check: Scan Frontend
- GitHub Check: E2E Tests
🔇 Additional comments (6)
frontend/src/components/__tests__/NotificationCenter.test.ts (6)
90-124: Well-structured MockEventSource implementation.The instance tracking, helper methods for simulating messages/errors, and proper cleanup in
beforeEachprovide good test isolation for SSE scenarios.
156-175: Clean SSE setup helpers with appropriate timer handling.The
setupSSEWithFakeTimerscorrectly leaves timer restoration to the caller, allowing tests to control timer advancement. Thevi.waitForusage (instead ofwaitForfrom testing-library) is correct when using fake timers.
201-243: Well-organized parameterized test data.The consolidated test case arrays enable comprehensive coverage with
it.eachwhile keeping the test code DRY. Good use ofas constassertions for type safety.
396-406: Previous review issue addressed: fake timers now prevent auto-mark interference.The test correctly uses
vi.useFakeTimers()withuserEvent.setup({ advanceTimers: vi.advanceTimersByTime })to prevent the auto-mark-as-read timer from interfering with the assertion.
422-432: Good accessibility test coverage.The tests verify essential accessibility attributes. Consider expanding in future PRs to cover
aria-expandedstate changes when dropdown opens/closes.
249-268: Thorough test setup with proper isolation.The
beforeEachblock correctly resets all mocks, clears EventSource instances, and suppresses console output. TheafterEachproperly restores mocks. This ensures reliable test isolation.
- excluded tests from build - instead of svg's using the icons from @lucide/svelte
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
frontend/src/lib/formatters.ts (1)
129-133: Consider clamping the size index to prevent out-of-bounds access.For extremely large byte values (petabytes+), the calculated index
icould exceed thesizesarray length, resulting inundefinedbeing displayed.🔎 Suggested fix
const k = 1024; const sizes = ['B', 'KB', 'MB', 'GB', 'TB']; - const i = Math.floor(Math.log(bytes) / Math.log(k)); + const i = Math.min(Math.floor(Math.log(bytes) / Math.log(k)), sizes.length - 1); return `${parseFloat((bytes / Math.pow(k, i)).toFixed(decimals))} ${sizes[i]}`;frontend/src/routes/admin/AdminSagas.svelte (1)
139-139: Effect dependency condition is always truthy.
autoRefresh || refreshRateis always truthy becauserefreshRatedefaults to5. The effect will trigger on any change to either variable, which works correctly, but the condition reads as if it's guarding against something.🔎 Clearer intent
- $effect(() => { if (autoRefresh || refreshRate) setupAutoRefresh(); }); + $effect(() => { autoRefresh; refreshRate; setupAutoRefresh(); });Or simply:
- $effect(() => { if (autoRefresh || refreshRate) setupAutoRefresh(); }); + $effect(() => { setupAutoRefresh(); });Since
setupAutoRefreshalready checksautoRefreshinternally.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
frontend/package.jsonfrontend/src/components/Modal.sveltefrontend/src/components/Pagination.sveltefrontend/src/lib/api-utils.tsfrontend/src/lib/formatters.tsfrontend/src/routes/admin/AdminSagas.sveltefrontend/tsconfig.json
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/lib/api-utils.ts (1)
frontend/src/stores/toastStore.ts (1)
addToast(16-25)
⏰ 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). (2)
- GitHub Check: E2E Tests
- GitHub Check: Build Frontend
🔇 Additional comments (9)
frontend/tsconfig.json (1)
23-23: Good practice to exclude test files from production builds.The exclusion patterns correctly cover all test files in the codebase. The
"src/**/__tests__/**"pattern handles test directories, and"src/**/*.test.ts"handles test files—all 16 test files use the.test.tsextension.frontend/package.json (1)
26-26: LGTM —@lucide/svelte@^0.562.0is the latest version with no known security vulnerabilities.The addition of
@lucide/svelteis a good choice for standardizing icon usage across UI components. Placement in runtime dependencies is correct.frontend/src/components/Pagination.svelte (1)
49-86: LGTM!The pagination controls are well-implemented with proper boundary checks (disabling first/previous at page 1, next/last at the final page) and good accessibility via
aria-labelattributes.frontend/src/components/Modal.svelte (1)
35-62: Well-structured modal with good accessibility attributes.The modal includes
role="dialog",aria-modal="true", andaria-labelledbyfor screen readers. The backdrop click and escape key handlers provide expected UX patterns.Consider generating a unique ID for
aria-labelledbyif multiple modals could coexist on the same page to avoid ID collisions.frontend/src/lib/api-utils.ts (1)
19-22: LGTM!The
handleApiErrorfunction provides a clean, centralized approach for error logging and user notification.frontend/src/lib/formatters.ts (2)
44-63: Clean implementation of duration formatting.The tiered approach handling milliseconds through hours with appropriate precision at each level is well thought out.
152-156: LGTM!The
truncatefunction handles edge cases properly and uses the correct slice length accounting for the ellipsis.frontend/src/routes/admin/AdminSagas.svelte (2)
348-459: Good integration of the new Modal component.The saga detail modal makes effective use of the new
Modalcomponent with proper snippet syntax for children content, and correctly integrates the formatting utilities for timestamps and durations.
337-339: Good integration of the new Pagination component.The pagination component is correctly wired with handlers for page and page-size changes.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
frontend/src/routes/admin/__tests__/AdminUsers.test.ts (1)
321-324: Consider asserting button existence before clicking.Using
!(non-null assertion) after.find()could mask test failures if the button isn't found. The test would fail with a less descriptive error.🔎 Proposed improvement
const allCreateButtons = screen.getAllByRole('button', { name: /Create User/i }); const submitBtn = allCreateButtons.find(btn => btn.closest('[role="dialog"]')); - await user.click(submitBtn!); + expect(submitBtn).toBeDefined(); + await user.click(submitBtn!);This pattern appears in multiple places (lines 324, 351, 439, 462, 681, 699). Consider applying consistently for better test diagnostics.
frontend/src/routes/admin/AdminUsers.svelte (2)
114-129: Consider distinguishing error types insaveRateLimits.
handleValidationErroris called for all errors, but network failures or server errors might not be validation errors. The test at line 568 in the test file expects this behavior for status 422, but other error types may produce confusing messages.🔎 Suggested approach
} catch (err) { - handleValidationError(err, 'Failed to save rate limits'); + if (err?.status === 422) { + handleValidationError(err, 'Failed to save rate limits'); + } else { + handleApiError(err, 'save rate limits'); + } } finally {This would provide more appropriate error messages for non-validation failures.
275-281: Advanced filters don't reset pagination.The
$effectresetscurrentPagewhensearchQuery,roleFilter, orstatusFilterchange, butadvancedFilterschanges are not tracked. This could leave users on an invalid page after applying advanced filters.🔎 Consider tracking advanced filters
-let prevFilters = { searchQuery: '', roleFilter: 'all', statusFilter: 'all' }; +let prevFilters = { searchQuery: '', roleFilter: 'all', statusFilter: 'all', advancedFilters: { bypassRateLimit: 'all', hasCustomLimits: 'all', globalMultiplier: 'all' } }; $effect(() => { - if (searchQuery !== prevFilters.searchQuery || roleFilter !== prevFilters.roleFilter || statusFilter !== prevFilters.statusFilter) { - prevFilters = { searchQuery, roleFilter, statusFilter }; + const advChanged = JSON.stringify(advancedFilters) !== JSON.stringify(prevFilters.advancedFilters); + if (searchQuery !== prevFilters.searchQuery || roleFilter !== prevFilters.roleFilter || statusFilter !== prevFilters.statusFilter || advChanged) { + prevFilters = { searchQuery, roleFilter, statusFilter, advancedFilters: { ...advancedFilters } }; currentPage = 1; } });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/src/routes/admin/AdminUsers.sveltefrontend/src/routes/admin/__tests__/AdminUsers.test.tsfrontend/src/routes/admin/__tests__/mocks/MockAdminLayout.svelte
⏰ 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). (2)
- GitHub Check: Scan Frontend
- GitHub Check: E2E Tests
🔇 Additional comments (4)
frontend/src/routes/admin/__tests__/mocks/MockAdminLayout.svelte (1)
1-14: Clean mock implementation for testing.The mock correctly accepts the same props as the real
AdminLayoutwhile providing a minimal wrapper for test rendering. Thedata-testidattribute enables easy assertions on layout presence.frontend/src/routes/admin/__tests__/AdminUsers.test.ts (1)
1-763: Comprehensive and well-structured test suite.The test file provides excellent coverage of the AdminUsers component including:
- Initial loading and error handling
- User list rendering with mobile/desktop views
- Search, filtering, and pagination
- CRUD operations via modals
- Rate limit management
- Form validation
The use of mock factories, hoisted mocks, and helper functions keeps tests DRY and maintainable.
frontend/src/routes/admin/AdminUsers.svelte (2)
599-606: Good defensive access pattern for varying usage structures.The fallback
usage.count || usage.tokens_remaining || 0correctly handles different rate limit algorithm response formats (sliding window vs token bucket).
1-666: Well-structured admin component with good refactoring.The component demonstrates good practices:
- Centralized error handling via
handleApiErrorandhandleValidationError- Reusable UI components (Modal, Pagination)
- Responsive design with mobile/desktop views
- Comprehensive rate limit management UI
- Proper Svelte 5 runes usage (
$state,$derived,$effect)
…d url paths fixes
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
frontend/src/routes/admin/__tests__/AdminEvents.test.ts (2)
163-176: Consider extracting the timer mode switching pattern.Most tests immediately call
vi.useRealTimers()afterbeforeEachsets up fake timers. This pattern is repeated ~30 times throughout the file. Consider either:
- Using real timers by default and only enabling fake timers for the specific tests that need them (like the auto-refresh interval test)
- Creating a helper that handles the timer mode
This would reduce boilerplate and make the intent clearer.
🔎 Example refactor approach
describe('AdminEvents', () => { beforeEach(() => { - vi.useFakeTimers(); setupMocks(); vi.clearAllMocks(); mocks.browseEventsApiV1AdminEventsBrowsePost.mockResolvedValue({ data: { events: [], total: 0 }, error: null }); mocks.getEventStatsApiV1AdminEventsStatsGet.mockResolvedValue({ data: null, error: null }); mocks.windowConfirm.mockReturnValue(true); }); afterEach(() => { - vi.useRealTimers(); cleanup(); vi.unstubAllGlobals(); }); + describe('auto-refresh interval', () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it('sets up auto-refresh interval', async () => { // ... test using fake timers + }); + });
817-842: Tests rely on DOM class selectors which can be fragile.These tests query by CSS class (
.text-green-600,.text-red-600,.text-blue-600) to verify event type coloring. While functional, this couples tests to Tailwind class names which could change. Consider addingdata-testidattributes to the color-coded elements in the component for more robust test selectors.frontend/src/routes/admin/AdminEvents.svelte (1)
362-374: Return type annotation is missing and misleading.The
getActiveFilterSummaryfunction returns an array (items) but has no explicit return type. The function name suggests it returns a string, but it actually returnsstring[]. Consider adding a return type annotation for clarity.🔎 Proposed fix
- function getActiveFilterSummary(): string { + function getActiveFilterSummary(): string[] { const items: string[] = []; // ... return items; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
frontend/rollup.config.jsfrontend/src/App.sveltefrontend/src/components/EventTypeIcon.sveltefrontend/src/components/Modal.sveltefrontend/src/routes/admin/AdminEvents.sveltefrontend/src/routes/admin/__tests__/AdminEvents.test.ts
⏰ 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). (2)
- GitHub Check: E2E Tests
- GitHub Check: Build Frontend
🔇 Additional comments (10)
frontend/rollup.config.js (1)
184-185: LGTM! Explicit.svelteextension support added.The addition of
extensions: ['.mjs', '.js', '.json', '.node', '.svelte']correctly enables resolution of Svelte components without requiring explicit file extensions in imports. This configuration aligns with current best practices for Svelte + Rollup projects (2024-2025) and complements the existingexportConditions: ['svelte'],dedupe: ['svelte'], and other resolve plugin settings already in place.frontend/src/components/EventTypeIcon.svelte (1)
1-51: Clean implementation of a reusable icon component.The component correctly maps event types to icons with appropriate fallback handling. The dual-format support (dot notation and underscore) ensures compatibility with different event naming conventions.
One minor consideration: In Svelte 5,
<svelte:component>is still supported but the newer pattern uses the component directly in the template. This works fine as-is.frontend/src/components/Modal.svelte (2)
25-34: Well-implemented Escape key handling with proper cleanup.The
$effect()pattern correctly addresses the previous review concern about the global keydown listener. The listener is now only attached whenopenis true, and the cleanup function properly removes it when the modal closes or the component unmounts.
41-67: Solid modal implementation with good accessibility.The component includes proper ARIA attributes (
aria-modal,aria-labelledby,role="dialog"), uses the fade transition for smooth UX, and correctly handles backdrop clicks. The Snippet-based children/footer pattern aligns with Svelte 5 conventions.frontend/src/routes/admin/__tests__/AdminEvents.test.ts (1)
1-893: Comprehensive test coverage for AdminEvents.The test suite thoroughly covers:
- Initial loading and auto-refresh
- Statistics display with conditional styling
- Event list rendering (desktop table and mobile cards)
- Filter panel toggle, input, apply, and clear
- Pagination controls and page size changes
- Event detail modal with related events
- Replay flow (dry-run preview, confirmation, progress tracking)
- Delete flow with confirmation and error handling
- Export dropdown (CSV/JSON)
- User overview modal
- Error handling paths
The mock infrastructure is well-organized with hoisted mocks and helper factories.
frontend/src/routes/admin/AdminEvents.svelte (4)
18-28: Good refactoring to centralized utilities and components.The imports for
handleApiError,formatTimestamp,Modal, andEventTypeIconconsolidate previously scattered logic into reusable modules. This improves maintainability and ensures consistent behavior across admin views.
1021-1108: Event Details Modal implementation looks good.The Modal component is used correctly with:
- Conditional rendering based on
selectedEvent- Proper content structure for event information
- Footer snippet with action buttons
- Related events navigation that loads details for clicked events
1110-1179: Replay Preview Modal provides good UX for dry-run confirmation.The modal clearly shows:
- Number of events to be replayed
- Preview of events with their details
- Warning about potential duplicate processing
- Clear action buttons for proceeding or canceling
The warning about duplicate results is an important user-facing safeguard.
1181-1265: User Overview Modal effectively displays user context.The modal provides a comprehensive view of:
- User profile information
- Rate limit settings
- Execution statistics with color-coded cards
- Recent events list
This is useful for administrators investigating user-related events.
frontend/src/App.svelte (1)
84-84: Regex route patterns are supported.The
@mateothegreat/svelte5-routeraccepts both string paths and RegExp patterns. The change to"^/admin$"is valid and will match exactly/admin, distinguishing it from the specific/admin/events,/admin/sagas,/admin/users, and/admin/settingsroutes defined earlier. This pattern serves as a default route for the/adminpath itself, displaying the AdminEvents component.
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (1)
frontend/src/routes/admin/AdminSagas.svelte (1)
60-67: Client-side filtering causes incorrect pagination counts.This was already flagged in a previous review. The
totalItemsreflects the API's unfiltered count whileexecutionIdFilterandsearchQueryare applied client-side, causing pagination to display incorrect totals.
🧹 Nitpick comments (3)
frontend/src/main.ts (1)
17-20: Consider settingappErrorfor unhandled promise rejections as well.The
onerrorhandler at lines 11-15 setsappError.setError(...), butonunhandledrejectiononly callsevent.preventDefault(). This inconsistency means unhandled promise rejections won't display in the UI error state, while synchronous errors will.If this is intentional (e.g., to avoid duplicate toasts from the API interceptor), a brief comment would clarify the design choice.
frontend/src/lib/api-interceptors.ts (1)
73-83: 401 debounce window may suppress legitimate re-authentication.The 1-second timeout on
isHandling401prevents concurrent 401 handling, but if a user re-authenticates within that window and immediately makes another request that fails with 401, it will be silently ignored.Consider resetting
isHandling401immediately after navigation completes rather than using a fixed timeout, or use a per-request token to track which 401 is being handled.frontend/src/routes/admin/AdminSagas.svelte (1)
123-123:$effectmay trigger unnecessarily whenrefreshRatechanges whileautoRefreshis false.The effect runs when either
autoRefreshorrefreshRatechanges, butsetupAutoRefresh()only sets up the interval whenautoRefreshis true. Consider refining the condition:-$effect(() => { if (autoRefresh || refreshRate) setupAutoRefresh(); }); +$effect(() => { autoRefresh; refreshRate; setupAutoRefresh(); });Or simply rely on the internal check in
setupAutoRefresh()and always call it when either dependency changes (current behavior is functionally correct, just slightly wasteful).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
docs/frontend/error-handling.mdfrontend/src/components/EventTypeIcon.sveltefrontend/src/components/Modal.sveltefrontend/src/lib/api-interceptors.tsfrontend/src/main.tsfrontend/src/routes/Editor.sveltefrontend/src/routes/admin/AdminEvents.sveltefrontend/src/routes/admin/AdminSagas.sveltefrontend/src/routes/admin/AdminUsers.sveltefrontend/src/routes/admin/__tests__/AdminEvents.test.tsfrontend/src/routes/admin/__tests__/AdminUsers.test.tsmkdocs.yml
✅ Files skipped from review due to trivial changes (2)
- mkdocs.yml
- docs/frontend/error-handling.md
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/components/EventTypeIcon.svelte
- frontend/src/components/Modal.svelte
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/src/lib/api-interceptors.ts (2)
frontend/src/stores/auth.ts (1)
csrfToken(48-48)frontend/src/stores/toastStore.ts (1)
addToast(16-25)
frontend/src/main.ts (1)
frontend/src/lib/api-interceptors.ts (1)
initializeApiInterceptors(60-125)
🪛 GitHub Actions: Frontend CI
frontend/src/routes/admin/__tests__/AdminEvents.test.ts
[error] 676-676: Cannot delete
[error] 893-893: Replay failed
[error] 676-676: Cannot delete
frontend/src/routes/admin/__tests__/AdminUsers.test.ts
[error] 458-458: Cannot delete
[error] 742-742: Failed to load
[error] 758-758: Reset failed
🪛 GitHub Check: Unit Tests
frontend/src/routes/admin/__tests__/AdminEvents.test.ts
[failure] 893-893: Unhandled error
Error: Replay failed
❯ src/routes/admin/tests/AdminEvents.test.ts:893:21
❯ node_modules/@vitest/runner/dist/chunk-hooks.js:155:11
❯ node_modules/@vitest/runner/dist/chunk-hooks.js:752:26
❯ node_modules/@vitest/runner/dist/chunk-hooks.js:1897:20
❯ runWithTimeout node_modules/@vitest/runner/dist/chunk-hooks.js:1863:10
❯ runTest node_modules/@vitest/runner/dist/chunk-hooks.js:1574:12
❯ runSuite node_modules/@vitest/runner/dist/chunk-hooks.js:1729:8
❯ runSuite node_modules/@vitest/runner/dist/chunk-hooks.js:1729:8
❯ runSuite node_modules/@vitest/runner/dist/chunk-hooks.js:1729:8
This error originated in "src/routes/admin/tests/AdminEvents.test.ts" test file. It doesn't mean the error was thrown inside the file itself, but while it was running.
The latest test that might've caused the error is "handles replay error and shows toast". It might mean one of the following:
- The error was thrown, while Vitest was running this test.
- If the error occurred after the test had been completed, this was the last documented test before it was thrown.
[failure] 676-676: Unhandled error
Error: Cannot delete
❯ src/routes/admin/tests/AdminEvents.test.ts:676:21
❯ node_modules/@vitest/runner/dist/chunk-hooks.js:155:11
❯ node_modules/@vitest/runner/dist/chunk-hooks.js:752:26
❯ node_modules/@vitest/runner/dist/chunk-hooks.js:1897:20
❯ runWithTimeout node_modules/@vitest/runner/dist/chunk-hooks.js:1863:10
❯ runTest node_modules/@vitest/runner/dist/chunk-hooks.js:1574:12
❯ runSuite node_modules/@vitest/runner/dist/chunk-hooks.js:1729:8
❯ runSuite node_modules/@vitest/runner/dist/chunk-hooks.js:1729:8
❯ runSuite node_modules/@vitest/runner/dist/chunk-hooks.js:1729:8
This error originated in "src/routes/admin/tests/AdminEvents.test.ts" test file. It doesn't mean the error was thrown inside the file itself, but while it was running.
The latest test that might've caused the error is "handles delete error and shows toast". It might mean one of the following:
- The error was thrown, while Vitest was running this test.
- If the error occurred after the test had been completed, this was the last documented test before it was thrown.
frontend/src/routes/admin/__tests__/AdminUsers.test.ts
[failure] 758-758: Unhandled error
Error: Reset failed
❯ src/routes/admin/tests/AdminUsers.test.ts:758:26
❯ node_modules/@vitest/runner/dist/chunk-hooks.js:155:11
❯ node_modules/@vitest/runner/dist/chunk-hooks.js:752:26
❯ node_modules/@vitest/runner/dist/chunk-hooks.js:1897:20
❯ runWithTimeout node_modules/@vitest/runner/dist/chunk-hooks.js:1863:10
❯ runTest node_modules/@vitest/runner/dist/chunk-hooks.js:1574:12
❯ runSuite node_modules/@vitest/runner/dist/chunk-hooks.js:1729:8
❯ runSuite node_modules/@vitest/runner/dist/chunk-hooks.js:1729:8
❯ runSuite node_modules/@vitest/runner/dist/chunk-hooks.js:1729:8
This error originated in "src/routes/admin/tests/AdminUsers.test.ts" test file. It doesn't mean the error was thrown inside the file itself, but while it was running.
The latest test that might've caused the error is "handles API error when resetting rate limits and shows toast". It might mean one of the following:
- The error was thrown, while Vitest was running this test.
- If the error occurred after the test had been completed, this was the last documented test before it was thrown.
[failure] 742-742: Unhandled error
Error: Failed to load
❯ src/routes/admin/tests/AdminUsers.test.ts:742:21
❯ node_modules/@vitest/runner/dist/chunk-hooks.js:155:11
❯ node_modules/@vitest/runner/dist/chunk-hooks.js:752:26
❯ node_modules/@vitest/runner/dist/chunk-hooks.js:1897:20
❯ runWithTimeout node_modules/@vitest/runner/dist/chunk-hooks.js:1863:10
❯ runTest node_modules/@vitest/runner/dist/chunk-hooks.js:1574:12
❯ runSuite node_modules/@vitest/runner/dist/chunk-hooks.js:1729:8
❯ runSuite node_modules/@vitest/runner/dist/chunk-hooks.js:1729:8
❯ runSuite node_modules/@vitest/runner/dist/chunk-hooks.js:1729:8
This error originated in "src/routes/admin/tests/AdminUsers.test.ts" test file. It doesn't mean the error was thrown inside the file itself, but while it was running.
The latest test that might've caused the error is "handles API error when loading rate limits and shows toast". It might mean one of the following:
- The error was thrown, while Vitest was running this test.
- If the error occurred after the test had been completed, this was the last documented test before it was thrown.
[failure] 458-458: Unhandled error
Error: Cannot delete
❯ src/routes/admin/tests/AdminUsers.test.ts:458:21
❯ node_modules/@vitest/runner/dist/chunk-hooks.js:155:11
❯ node_modules/@vitest/runner/dist/chunk-hooks.js:752:26
❯ node_modules/@vitest/runner/dist/chunk-hooks.js:1897:20
❯ runWithTimeout node_modules/@vitest/runner/dist/chunk-hooks.js:1863:10
❯ runTest node_modules/@vitest/runner/dist/chunk-hooks.js:1574:12
❯ runSuite node_modules/@vitest/runner/dist/chunk-hooks.js:1729:8
❯ runSuite node_modules/@vitest/runner/dist/chunk-hooks.js:1729:8
❯ runSuite node_modules/@vitest/runner/dist/chunk-hooks.js:1729:8
This error originated in "src/routes/admin/tests/AdminUsers.test.ts" test file. It doesn't mean the error was thrown inside the file itself, but while it was running.
The latest test that might've caused the error is "handles deletion error and shows toast". It might mean one of the following:
- The error was thrown, while Vitest was running this test.
- If the error occurred after the test had been completed, this was the last documented test before it was thrown.
⏰ 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). (1)
- GitHub Check: Build Frontend
🔇 Additional comments (8)
frontend/src/routes/admin/__tests__/AdminUsers.test.ts (1)
1-783: Excellent test coverage and structure.The test suite is comprehensive and well-organized, covering:
- Initial loading and error handling
- CRUD operations (create, edit, delete users)
- Search and filtering (basic and advanced)
- Pagination and user counts
- Rate limit management
- Form validation
- Modal interactions
The mock setup with hoisted functions and helper utilities (mock factories, renderWithUsers) follows best practices. The tests appropriately handle both mobile and desktop views.
frontend/src/main.ts (1)
22-38: Good resilience pattern for mount failures.Wrapping the mount in try/catch with a fallback
ErrorDisplayensures users see a meaningful error instead of a blank page if the main app fails to initialize.frontend/src/lib/api-interceptors.ts (1)
18-36: Well-structured error message extraction.The
getErrorMessagefunction handles various error shapes gracefully with sensible fallbacks. Good defensive coding.Also applies to: 38-40
frontend/src/routes/admin/__tests__/AdminEvents.test.ts (1)
162-174: Comprehensive test infrastructure with good mock patterns.The test file establishes solid patterns: hoisted mocks, factory functions for test data, proper cleanup, and coverage across loading, filtering, pagination, modals, and user interactions.
frontend/src/routes/admin/AdminSagas.svelte (1)
34-41: Clean state management pattern withsagaStatesmap.The structured
sagaStateslookup andgetStateInfohelper provide consistent styling and fallback handling across the component.Also applies to: 86-88
frontend/src/routes/Editor.svelte (1)
285-304: Good use of destructuring for API responses with explicit error checks.The K8s limits and example scripts fetching properly handles the error case with fallback defaults.
frontend/src/routes/admin/AdminEvents.svelte (2)
99-125: Good use ofunwrapOr()for read operations.The
loadEvents,loadStats, andloadEventDetailfunctions properly useunwrapOr()withnullfallbacks, ensuring graceful degradation on API failures.
929-1016: Clean Modal-based architecture for detail views.The refactoring to use the
Modalcomponent with{#snippet footer()}pattern provides consistent UX across Event Details, Replay Preview, and User Overview modals.Also applies to: 1018-1087, 1089-1173
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
frontend/src/routes/admin/__tests__/AdminUsers.test.ts (3)
451-470: Past issue resolved: Plain error object used correctly.The test now uses
{ message: 'Cannot delete' }instead ofnew Error(), addressing the previous pipeline failure where Vitest detected unhandled errors.
735-749: Past issue resolved: Plain error object used correctly.The test now uses
{ message: 'Failed to load' }instead ofnew Error(), consistent with the fix pattern.
751-775: Past issue resolved: Plain error object used correctly.The test now uses
{ message: 'Reset failed' }instead ofnew Error(), completing the fix for all flagged error handling tests.frontend/src/routes/admin/__tests__/AdminEvents.test.ts (1)
892-909: Same pattern issue as delete error test.The replay error test has the same concern: the mock implementation calls
addToast, and the mockunwrapdoesn't throw. Verify the component'sreplayEventfunction handles errors properly.#!/bin/bash # Check how replayEvent handles errors in AdminEvents.svelte ast-grep --pattern $'async function replayEvent($_) { $$$ }'frontend/src/lib/api-interceptors.ts (1)
133-135:unwrapOrmay returnundefinedinstead of fallback whendatais missing.When
result.erroris falsy butresult.datais alsoundefined, this returnsundefined as Trather than the fallback. This was flagged in a previous review.🔎 Proposed fix
export function unwrapOr<T>(result: { data?: T; error?: unknown }, fallback: T): T { - return result.error ? fallback : (result.data as T); + return result.error || result.data === undefined ? fallback : result.data; }
🧹 Nitpick comments (3)
frontend/src/lib/__mocks__/api-interceptors.ts (1)
12-15: Mockunwrapbehavior differs from production implementation.The mock returns
undefinedon error, but the realunwrap(inapi-interceptors.ts:128-131) throwsresult.error. This difference is likely intentional to avoid unhandled errors in tests, but it means tests won't catch code paths that rely on the throwing behavior.If any component code uses
try/catcharoundunwrap(), those catch blocks won't be exercised in tests. Consider documenting this difference or adding a throwing variant for specific test cases that need it.frontend/src/routes/admin/__tests__/AdminUsers.test.ts (2)
154-163: Test pattern has the mock callingaddToast, not the component.The mock implementation calls
mocks.addToast(...)directly, so this test verifies the mock behavior rather than the component's error handling. If the component fails to calladdToaston API errors, this test would still pass.A more robust approach would have the mock simply return the error, then assert the component calls
addToast:🔎 Alternative pattern
it('handles API error on load and shows toast', async () => { const error = { message: 'Network error' }; - mocks.listUsersApiV1AdminUsersGet.mockImplementation(async () => { - mocks.addToast('Failed to load users', 'error'); - return { data: null, error }; - }); + mocks.listUsersApiV1AdminUsersGet.mockResolvedValue({ data: null, error }); render(AdminUsers); await waitFor(() => expect(mocks.addToast).toHaveBeenCalledWith('Failed to load users', 'error'));This pattern repeats throughout the file (lines 336-341, 453-455, 578-580, 737-739, 760-762). If intentional due to centralized interceptor handling, consider adding a comment explaining the pattern.
320-323: Non-null assertion onfind()result could throw unclear errors.If the dialog structure changes,
submitBtncould be undefined andsubmitBtn!would throw an unclear error. Consider adding an explicit assertion for better test failure messages:🔎 Suggested improvement
const allCreateButtons = screen.getAllByRole('button', { name: /Create User/i }); const submitBtn = allCreateButtons.find(btn => btn.closest('[role="dialog"]')); + expect(submitBtn).toBeDefined(); await user.click(submitBtn!);This pattern appears in multiple places (lines 352, 440, 466, 686, 705).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
frontend/src/lib/__mocks__/api-interceptors.tsfrontend/src/lib/api-interceptors.tsfrontend/src/routes/admin/AdminUsers.sveltefrontend/src/routes/admin/__tests__/AdminEvents.test.tsfrontend/src/routes/admin/__tests__/AdminUsers.test.ts
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/src/lib/__mocks__/api-interceptors.ts (1)
frontend/src/lib/api-interceptors.ts (4)
getErrorMessage(18-36)initializeApiInterceptors(60-126)unwrap(128-131)unwrapOr(133-135)
frontend/src/lib/api-interceptors.ts (3)
frontend/src/lib/__mocks__/api-interceptors.ts (1)
initializeApiInterceptors(10-10)frontend/src/stores/auth.ts (1)
csrfToken(48-48)frontend/src/stores/toastStore.ts (1)
addToast(16-25)
⏰ 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). (2)
- GitHub Check: E2E Tests
- GitHub Check: Build Frontend
🔇 Additional comments (8)
frontend/src/lib/__mocks__/api-interceptors.ts (2)
3-8: SimplifiedgetErrorMessagemock is acceptable for testing.The mock omits the
detail(ValidationError) andErrorinstance handling from the real implementation. This is fine for unit tests where error shapes are controlled, but note that tests won't validate the full validation error formatting logic.
17-19:unwrapOrmatches production logic but inherits same edge case.This mock correctly mirrors the real implementation. Note that both return
result.data as Twhen there's no error, which could beundefinedifdatais missing—see past review comment on the real implementation for a potential fix.frontend/src/routes/admin/__tests__/AdminUsers.test.ts (1)
1-95: Well-structured test setup with comprehensive mocks.The test file demonstrates good practices:
- Clear mock data factories with sensible defaults and overrides
- Hoisted mocks for proper module mocking order
- Comprehensive Element.animate mock for Svelte transitions
- Clean helper functions for common render patterns
The module mocking approach correctly isolates the component from external dependencies.
frontend/src/lib/api-interceptors.ts (2)
73-83: 401 handling debounce pattern is sound.The
isHandling401flag with 1000ms reset prevents toast spam and multiple redirects during session expiration. The debounce window is reasonable.One minor note: the
finallyblock schedules the reset, but if the browser navigates to/loginsynchronously, subsequent 401s during the current page's pending requests are correctly ignored.
60-126: Comprehensive interceptor setup with proper error categorization.The error interceptor handles status codes appropriately:
- 401: Session expired with redirect (debounced)
- 403: Access denied toast
- 422: Validation errors with field-level formatting
- 429: Rate limiting warning
- 5xx: Generic server error
- Network errors: Connection check prompt
The request interceptor correctly injects CSRF tokens for non-GET requests.
frontend/src/routes/admin/__tests__/AdminEvents.test.ts (3)
164-182: Test setup correctly manages fake timers and global stubs.The
beforeEachusesvi.useFakeTimers()which is properly balanced withvi.useRealTimers()inafterEach. The window global stubs (open,confirm) are cleaned up viavi.unstubAllGlobals().Note: Several tests call
vi.useRealTimers()at their start, which is correct for tests that need real async behavior.
1-162: Comprehensive test setup with well-designed mock factories.The mock data factories (
createMockEvent,createMockStats,createMockEventDetail,createMockUserOverview) provide realistic test data with sensible defaults. The hoisted mock pattern ensures proper module initialization order.The
renderWithEventshelper simplifies test setup by handling both API mocks and render lifecycle.
543-626: Replay functionality tests cover key scenarios.Tests verify:
- Dry run preview behavior
- Confirmation before actual replay
- Cancellation handling
- Progress display during active sessions
The window.confirm mock is properly used to control user interaction.
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/src/lib/__mocks__/api-interceptors.tsfrontend/src/main.tsfrontend/vitest.setup.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/lib/mocks/api-interceptors.ts
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/main.ts (2)
frontend/src/lib/__mocks__/api-interceptors.ts (1)
initializeApiInterceptors(4-4)frontend/src/lib/api-interceptors.ts (1)
initializeApiInterceptors(60-126)
⏰ 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). (2)
- GitHub Check: E2E Tests
- GitHub Check: Build Frontend
🔇 Additional comments (2)
frontend/src/main.ts (1)
5-8: LGTM! Good centralization of API configuration.Centralizing the API interceptor initialization improves maintainability and ensures consistent error handling across all API calls. The interceptor properly handles authentication, validation, rate limiting, and server errors as shown in the relevant code snippets.
frontend/vitest.setup.ts (1)
4-6: Test setup mirrors main.ts rejection handling.This silences all unhandled rejections in tests, which is consistent with the new behavior in
main.ts. However, this shares the same concern raised formain.tslines 17-20: non-API promise rejections will be hidden during testing, potentially allowing bugs to slip through.If the
main.tshandler is updated to distinguish API errors from other rejections, consider updating this test handler accordingly to ensure tests catch non-API promise rejections.
1 participant
Summary by CodeRabbit
New Features
Improvements
Tests
Documentation
Dependencies
✏️ Tip: You can customize this high-level summary in your review settings.