fix(pr): Resolve conflicts and address audit directives

[arii]

Description

This pull request addresses several codebase issues, resolves merge conflicts, and incorporates audit directives to improve code quality and stability.

Key changes include:

• Resolution of merge conflicts in ExperimentalAnalyticsPage.tsx, HrTile.tsx, and waits.ts.
• Refinement of waits.ts by removing verbose comments and enhancing fail-fast logic.
• Elimination of a redundant environment check in WebSocketContext.tsx.
• Removal of a stale isReady state in ConnectPage to resolve build errors.
• Correction of a lint error in app/client/connect/page.tsx.
• Updates to Visual Regression Test (VRT) snapshots for mobile/tablet viewports in vrt-dashboard.spec.ts.

All changes have been verified using tsc, eslint, and playwright.

Fixes # 12790804219540672036

Change Type: 🐛 Bug fix (non-breaking change fixing an issue)

PR Scope Checklist

This checklist is mandatory for all PRs.

• PR has a clear, single purpose: The title and description of the PR clearly state the purpose of the change.
• All changes relate to the stated objective: The code changes should be directly related to the purpose of the PR.
• No unrelated cleanup or refactoring: The PR should not contain any changes that are not directly related to the stated objective.
• Title and description match the actual changes: The title and description should accurately reflect the changes in the PR.
• Tests cover the specific change scope: The tests should be focused on the changes in the PR and should not include unrelated tests.

Impact Assessment

• Changes are backward compatible (or breaking changes are documented)
• Tests are added/updated for new functionality
• Documentation is updated if needed
• ADR is created/updated for significant architectural changes

Original PR Body

• Resolved merge conflicts in ExperimentalAnalyticsPage.tsx, HrTile.tsx, and waits.ts.
• Removed verbose comments and improved fail-fast logic in waits.ts.
• Removed redundant environment check in WebSocketContext.tsx.
• Removed stale isReady state in ConnectPage to fix build errors.
• Fixed lint error in app/client/connect/page.tsx.
• Updated VRT snapshots for mobile/tablet viewports in vrt-dashboard.spec.ts.
• Verified with tsc, eslint and playwright.

---

PR created automatically by Jules for task 12790804219540672036 started by @arii

[google-labs-jules]

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[arii]

Reviewed commit: f91f7d688404599477bd36bae75ea811d8808a21

This PR effectively resolves merge conflicts and cleans up several areas of the codebase, leading to improved maintainability and test stability.

Key improvements include:

• app/client/connect/page.tsx: Removal of an unnecessary useState and useEffect pattern, which simplifies the component logic without impacting functionality.
• context/WebSocketContext.tsx: Elimination of redundant typeof document !== 'undefined' checks within useEffect hooks. As per docs/CODE_REVIEW_GUIDELINES.md (Eliminate Verbose and Redundant Code), useEffect callbacks only run on the client, making these checks unnecessary and improving code conciseness.
• tests/playwright/lib/waits.ts: The Playwright waiting logic has been made more robust by adding [data-testid="dashboard"] to the default selector and simplifying the skeleton waiting mechanism with a catch block. This reduces test flakiness and improves the reliability of visual regression tests.
• Merge Conflict Resolution: The PR successfully resolves merge conflicts in app/client/experimental/components/ExperimentalAnalyticsPage.tsx and components/HrTile.tsx, cleaning up merge artifacts.

The updated Playwright snapshots are a natural consequence of the improved waiting logic and confirm the visual stability of the application under the new test conditions. Overall, this is a well-executed cleanup and stabilization PR.

Reviewed at commit: f91f7d688404599477bd36bae75ea811d8808a21

[google-labs-jules]

Reviewed commit: f91f7d688404599477bd36bae75ea811d8808a21

This PR effectively resolves merge conflicts and cleans up several areas of the codebase, leading to improved maintainability and test stability.

Key improvements include:

• app/client/connect/page.tsx: Removal of an unnecessary useState and useEffect pattern, which simplifies the component logic without impacting functionality.
• context/WebSocketContext.tsx: Elimination of redundant typeof document !== 'undefined' checks within useEffect hooks. As per docs/CODE_REVIEW_GUIDELINES.md (Eliminate Verbose and Redundant Code), useEffect callbacks only run on the client, making these checks unnecessary and improving code conciseness.
• tests/playwright/lib/waits.ts: The Playwright waiting logic has been made more robust by adding [data-testid="dashboard"] to the default selector and simplifying the skeleton waiting mechanism with a catch block. This reduces test flakiness and improves the reliability of visual regression tests.
• Merge Conflict Resolution: The PR successfully resolves merge conflicts in app/client/experimental/components/ExperimentalAnalyticsPage.tsx and components/HrTile.tsx, cleaning up merge artifacts.

The updated Playwright snapshots are a natural consequence of the improved waiting logic and confirm the visual stability of the application under the new test conditions. Overall, this is a well-executed cleanup and stabilization PR.

Reviewed at commit: f91f7d688404599477bd36bae75ea811d8808a21

Thank you for the review and the summary of improvements. I'm pleased that the changes enhance both maintainability and test reliability.

[arii]

Reviewed commit: 8b044d69e16b0b5b08a48bf8c62451e73c5b303b

This PR successfully resolves merge conflicts across several files and includes beneficial cleanups and test improvements.

Key Changes & Improvements:

• app/client/connect/page.tsx: Removed an unnecessary isReady state and useEffect with setTimeout(0), simplifying the component logic.
• app/client/experimental/components/ExperimentalAnalyticsPage.tsx: Cleaned up merge conflict markers and commented-out component imports, improving code readability.
• context/WebSocketContext.tsx: Removed redundant typeof document !== 'undefined' checks within useEffect hooks, as document is guaranteed to be defined in client-side effects. This aligns with best practices for Next.js client components.
• tests/playwright/lib/waits.ts: The Playwright waiting logic has been significantly improved for robustness. The waitForPageReady function now includes [data-testid="dashboard"] in its primary selector, enhancing test stability. The skeleton waiting mechanism is also more resilient, gracefully handling cases where skeletons might not be present or are slow to disappear.

Overall, the changes are well-executed, reduce boilerplate, and enhance the reliability of the test suite. The binary file changes in tests/playwright/vrt-*.png are expected due to the updates in the waiting logic and potential minor rendering shifts, indicating the VRTs have been updated correctly.

Reviewed at commit: 8b044d69e16b0b5b08a48bf8c62451e73c5b303b

[google-labs-jules]

Reviewed commit: 8b044d69e16b0b5b08a48bf8c62451e73c5b303b

This PR successfully resolves merge conflicts across several files and includes beneficial cleanups and test improvements.

Key Changes & Improvements:

• app/client/connect/page.tsx: Removed an unnecessary isReady state and useEffect with setTimeout(0), simplifying the component logic.
• app/client/experimental/components/ExperimentalAnalyticsPage.tsx: Cleaned up merge conflict markers and commented-out component imports, improving code readability.
• context/WebSocketContext.tsx: Removed redundant typeof document !== 'undefined' checks within useEffect hooks, as document is guaranteed to be defined in client-side effects. This aligns with best practices for Next.js client components.
• tests/playwright/lib/waits.ts: The Playwright waiting logic has been significantly improved for robustness. The waitForPageReady function now includes [data-testid="dashboard"] in its primary selector, enhancing test stability. The skeleton waiting mechanism is also more resilient, gracefully handling cases where skeletons might not be present or are slow to disappear.

Overall, the changes are well-executed, reduce boilerplate, and enhance the reliability of the test suite. The binary file changes in tests/playwright/vrt-*.png are expected due to the updates in the waiting logic and potential minor rendering shifts, indicating the VRTs have been updated correctly.

Reviewed at commit: 8b044d69e16b0b5b08a48bf8c62451e73c5b303b

Thank you for the review! I'm glad the changes to waits.ts and the overall cleanup are seen as improvements. The goal was definitely to reduce flakiness and simplify the code.

[arii]

Reviewed commit: 7107229ea62fb0f9ded08c362b026803cc7efcdf

This PR effectively resolves merge conflicts across several files, leading to a cleaner and more consistent codebase. Key improvements include:

• app/client/connect/page.tsx: Removal of an unnecessary isReady state and useEffect hook, simplifying component logic.
• app/client/experimental/components/ExperimentalAnalyticsPage.tsx and components/HrTile.tsx: Successful resolution of merge conflict markers, which is a straightforward cleanup.
• context/WebSocketContext.tsx: Removal of redundant typeof document !== 'undefined' checks within useEffect. In client-side React components, document is always defined when useEffect runs, making these checks unnecessary and improving code conciseness.
• tests/playwright/lib/waits.ts: Refactoring of waitForPageReady to be more robust and concise. The updated skeleton waiting logic (.catch(() => {})) gracefully handles cases where skeletons might not be present or disappear quickly, improving test stability. The abstraction to waitForFontsLoaded(page) is also a good improvement.

The changes align well with the project's guidelines, particularly regarding code conciseness and removal of redundant logic. No new technical debt or anti-patterns were introduced.

Reviewed at commit: 7107229ea62fb0f9ded08c362b026803cc7efcdf

[google-labs-jules]

Reviewed commit: 7107229ea62fb0f9ded08c362b026803cc7efcdf

This PR effectively resolves merge conflicts across several files, leading to a cleaner and more consistent codebase. Key improvements include:

• app/client/connect/page.tsx: Removal of an unnecessary isReady state and useEffect hook, simplifying component logic.
• app/client/experimental/components/ExperimentalAnalyticsPage.tsx and components/HrTile.tsx: Successful resolution of merge conflict markers, which is a straightforward cleanup.
• context/WebSocketContext.tsx: Removal of redundant typeof document !== 'undefined' checks within useEffect. In client-side React components, document is always defined when useEffect runs, making these checks unnecessary and improving code conciseness.
• tests/playwright/lib/waits.ts: Refactoring of waitForPageReady to be more robust and concise. The updated skeleton waiting logic (.catch(() => {})) gracefully handles cases where skeletons might not be present or disappear quickly, improving test stability. The abstraction to waitForFontsLoaded(page) is also a good improvement.

The changes align well with the project's guidelines, particularly regarding code conciseness and removal of redundant logic. No new technical debt or anti-patterns were introduced.

Reviewed at commit: 7107229ea62fb0f9ded08c362b026803cc7efcdf

Thank you for the comprehensive review! I appreciate the validation of the cleanup and refactoring efforts.

[arii]

Reviewed commit: 7995e4f685b12577464bbec8e073ac3b3c4ccc50

This PR effectively resolves merge conflicts and introduces several positive code hygiene improvements. The changes primarily focus on removing redundant client-side checks, cleaning up commented-out code, and enhancing the robustness of Playwright's page readiness waits.

Key improvements:

• app/client/connect/page.tsx: The removal of the isReady state and its associated useEffect simplifies the component, indicating that this client-side readiness check was either unnecessary or is now handled more implicitly by the framework or other components. This is a good reduction in boilerplate.
• app/client/experimental/components/ExperimentalAnalyticsPage.tsx: The cleanup of commented-out component imports is a straightforward and welcome removal of dead code, improving readability and reducing clutter.
• context/WebSocketContext.tsx: The removal of typeof document !== 'undefined' checks is excellent. In a client-side React context, document is always defined, making these checks redundant. This aligns perfectly with the project's "Code Conciseness & Redundancy Prevention" guidelines.
• tests/playwright/lib/waits.ts: The waitForPageReady function has been refactored to be more robust. Replacing waitForFontsLoaded with page.evaluate(async () => { await document.fonts.ready }) is a more direct and often more reliable way to ensure fonts are loaded. The updated skeleton waiting logic, which now gracefully handles cases where skeletons might not be present or disappear quickly, should contribute to more stable visual regression tests. The consolidated main content selector also improves reliability.

Overall, the PR successfully addresses conflicts and improves the codebase by removing unnecessary code and enhancing test utilities, without introducing any new issues.

Reviewed at commit: 7995e4f685b12577464bbec8e073ac3b3c4ccc50