Address code review feedback: improve documentation and code organization

Co-authored-by: arii <342438+arii@users.noreply.github.com>

Author: Copilot

tests/playwright/lib/auth.ts

@@ -154,19 +154,26 @@ export async function verifySpotifyTokenStatus(
  * Create an authenticated browser context with stored credentials.
  * Useful for tests that require pre-authenticated state.
  *
+ * Note: This function provides a foundation for custom auth state management.
+ * For production use, implement storage state handling with context.storageState().
+ *
  * @param context - The Playwright BrowserContext object
- * @param storageState - Optional path to saved storage state
- * @returns The configured context
+ * @param _storageState - Reserved for future storage state path parameter
+ * @returns The configured context (passthrough for now)
+ * @example
+ * ```typescript
+ * // Save authenticated state
+ * await context.storageState({ path: 'auth.json' })
+ *
+ * // Create context with saved state
+ * const context = await browser.newContext({ storageState: 'auth.json' })
+ * ```
  */
 export async function createAuthenticatedContext(
   context: BrowserContext,
   _storageState?: string,
 ): Promise<BrowserContext> {
-  // If storage state is provided, apply it
-  // Note: This is a placeholder for custom auth state management
-  // In practice, you would use context.storageState() to save and restore
-
-  // For now, just return the context as-is
+  // Returns context as-is - implement storage state handling as needed
   return context
 }
 

tests/playwright/lib/setup.ts

@@ -170,7 +170,8 @@ export async function setupVisualRegressionTest(pages: {
   const baseUrl = getBaseURL()
 
   // Navigate all pages in parallel
-  // Note: Uses legacy routes for backward compatibility with existing tests
+  // Note: Uses LEGACY_ROUTES for control/mock/connect for backward compatibility with existing tests.
+  // Dashboard uses HRM_ROUTES.DASHBOARD since it's just '/'.
   await Promise.all([
     dashboardPage.goto(`${baseUrl}${HRM_ROUTES.DASHBOARD}`),
     controlPage.goto(`${baseUrl}${LEGACY_ROUTES.PHONE}`),
@@ -227,6 +228,8 @@ export async function setupComprehensiveTest(options: {
   const mockTab = await context.newPage()
   const connectTab = await context.newPage()
 
+  // Note: Uses LEGACY_ROUTES for control/mock/connect for backward compatibility with existing tests.
+  // Dashboard uses HRM_ROUTES.DASHBOARD since it's just '/'.
   await Promise.all([
     dashboardTab.goto(`${baseUrl}${HRM_ROUTES.DASHBOARD}`),
     controlTab.goto(`${baseUrl}${LEGACY_ROUTES.PHONE}`),
@@ -345,14 +348,17 @@ export async function setupMockHrStreaming(
 ): Promise<void> {
   const { bpm = 155, zone = 4 } = options
 
+  // Get the BPM input element
+  const bpmInput = mockPage.getByLabel('Current BPM')
+
   // Set HR value
-  await mockPage.getByLabel('Current BPM').fill(String(bpm))
+  await bpmInput.fill(String(bpm))
 
   // Set zone
   await mockPage.getByRole('button', { name: `Zone ${zone}` }).click()
 
   // Verify BPM is set
-  await expect(mockPage.getByLabel('Current BPM')).toHaveValue(String(bpm))
+  await expect(bpmInput).toHaveValue(String(bpm))
 }
 
 /**

tests/playwright/lib/waits.ts

@@ -8,9 +8,7 @@
  * - WebSocket connection verification
  * - Font loading guarantees
  */
-import type { Page } from '@playwright/test'
-
-import type { Response as PlaywrightResponse } from '@playwright/test'
+import type { Page, Response as PlaywrightResponse } from '@playwright/test'
 
 /**
  * Default timeout values for wait operations (in milliseconds)