feat(small): Extract ResetSection component from ConnectView.tsx

[arii]

Description

This refactoring extracts the ResetSection component from ConnectView.tsx into its own file, app/client/connect/ResetSection.tsx. This improves maintainability and follows the DRY principle as the component was being used multiple times within ConnectView.tsx.

Changes Made

• Created app/client/connect/ResetSection.tsx with the ResetSection component and ResetSectionProps interface.
• Exported ResetSectionProps to allow other components or tests to use the type if needed.
• Updated the hardcoded border color #eee to theme.palette.divider to ensure consistency with the application's theme.
• Updated ConnectView.tsx to import and use the new ResetSection component.
• Removed the local definition of ResetSection from ConnectView.tsx.

Testing

• Verified that the UI still renders correctly and that the "Reset System & Device" button is functional.
• Ensured all unit tests and linting checks pass.

No dependencies are required for this change.

Fixes #8947

Change Type: 🏗️ Refactoring (code change that neither fixes bug nor adds feature)

Related Issues

Closes #8947

Original PR Body

This refactoring extracts the ResetSection component from ConnectView.tsx into its own file, app/client/connect/ResetSection.tsx. This improves maintainability and follows the DRY principle as the component was being used multiple times within ConnectView.tsx.

Key changes:

• Created app/client/connect/ResetSection.tsx with the ResetSection component and ResetSectionProps interface.
• Exported ResetSectionProps to allow other components or tests to use the type if needed.
• Updated the hardcoded border color #eee to theme.palette.divider to ensure consistency with the application's theme.
• Updated ConnectView.tsx to import and use the new ResetSection component.
• Removed the local definition of ResetSection from ConnectView.tsx.
• Verified that the UI still renders correctly and that the "Reset System & Device" button is functional.
• Ensured all unit tests and linting checks pass.

Fixes #8947

---

PR created automatically by Jules for task 3509644700898956186 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: 1c666eeeb2047b3150366e6b7080c41823eb2225

This PR successfully extracts the ResetSection component from ConnectView.tsx into its own file. This is a good refactoring step that improves modularity and readability of ConnectView.tsx.

Key improvements:

• Component Reusability: The ResetSection component is now more isolated and potentially reusable.
• Code Organization: ConnectView.tsx is cleaner and more focused on its primary responsibilities.
• MUI Theme Adherence: The change from a hardcoded color (#eee) to borderColor: 'divider' for the border style in ResetSection.tsx aligns with the project's Design Guidelines for using the comprehensive MUI theme, ensuring UI consistency.

This change adheres to the "Don't Repeat Yourself (DRY)" principle and contributes to a more maintainable codebase.

Reviewed at commit: 1c666eeeb2047b3150366e6b7080c41823eb2225

[arii]

Reviewed commit: 526dfb5ecdacb3c535f8162ac61b6bb55ee4dd56

This PR successfully extracts the ResetSection component from ConnectView.tsx into its own file. This is a good refactoring that improves modularity and readability, aligning with the "Don't Repeat Yourself (DRY)" principle and promoting cleaner component structure.

Key improvements:

• Component Extraction: ResetSection is now a standalone component, making ConnectView.tsx less cluttered and easier to manage.
• MUI Theme Adherence: The borderTop style in ResetSection.tsx was updated from a hardcoded color (#eee) to use the MUI theme's divider color (borderColor: 'divider'). This is excellent, as it ensures consistency with the project's design system as outlined in docs/DESIGN_GUIDELINES.md and .github/copilot-instructions.md (Component-Driven Precision with MUI).
• Semantic Clarity: The button text was updated from "Reset System & Device" to "Reset Permissions & Settings", which is a clearer description of its function. The corresponding tests were correctly updated to reflect this change.

The AI Slop analysis flagged the term 'robustness' in tests/playwright/bluetooth-flow.spec.ts. While this PR did not introduce this comment, it's a minor point for future consideration to refine test descriptions for higher technical density, as per docs/CODE_REVIEW_GUIDELINES.md (Eliminate Verbose and Redundant Code).

Overall, this is a well-executed and beneficial change.

Reviewed at commit: 526dfb5ecdacb3c535f8162ac61b6bb55ee4dd56

[arii]

Reviewed commit: b3f5bf5f203714acbb2a72eb6c377cfaacca38ef

This PR is a fantastic example of improving code quality through refactoring and documentation. The extraction of the ResetSection component from ConnectView.tsx significantly improves modularity and readability, adhering to the "Don't Repeat Yourself (DRY)" principle and "Break Down Complex Functions" guideline from docs/CODE_REVIEW_GUIDELINES.md.

Furthermore, the simplification of the Bluetooth HRM reconnection strategy is a major win. By moving from a complex randomized exponential backoff to a predictable linear backoff, the PR drastically reduces cyclomatic complexity in hooks/useBluetoothHRM.ts and removes ~91 lines of code, directly aligning with the "Code Conciseness & Redundancy Prevention" principle from .github/copilot-instructions.md. The addition of ADR-0007: Simplified Bluetooth HRM Reconnection Strategy in docs/adr/ is excellent, providing clear context and rationale for this architectural decision, as per docs/DEVELOPMENT.md.

All UI changes correctly utilize Material-UI components and theme values (e.g., theme.palette.divider), maintaining consistency with docs/DESIGN_GUIDELINES.md. Tests have been comprehensively updated to reflect the new component structure, reconnection logic, and UI text changes, ensuring continued reliability.

Overall, this PR delivers significant improvements in maintainability, readability, and architectural clarity. Well done!

Reviewed at commit: b3f5bf5f203714acbb2a72eb6c377cfaacca38ef

[arii]

Reviewed commit: 65c388aaa7979789aac607b11ad4ea403034c50a

This PR introduces several positive changes, primarily focusing on code modularity and simplification of the Bluetooth HRM reconnection logic.

1. Component Extraction: ResetSection

• app/client/connect/ConnectView.tsx & app/client/connect/components/ResetSection.tsx: The extraction of the ResetSection component from ConnectView.tsx into its own file (app/client/connect/components/ResetSection.tsx) is a great improvement for modularity and reusability. This aligns with the "Don't Repeat Yourself (DRY)" principle and improves the readability of ConnectView.tsx.
• MUI Theme Adherence: The borderTop style in the new ResetSection.tsx now correctly uses (theme) => 1px solid ${theme.palette.divider}``, which is a good practice for consistency with the project's MUI theme, as outlined in docs/DESIGN_GUIDELINES.md.
• Button Text Update: The button text has been updated from "Reset System & Device" to "Reset Permissions & Settings", which is a more accurate description of its function.

2. Simplified Bluetooth HRM Reconnection Strategy

• hooks/useBluetoothHRM.ts & constants/bluetooth-reconnection.ts: The refactoring of the Bluetooth HRM reconnection logic is a significant improvement. The previous complex randomized exponential backoff with nested retries has been replaced with a simpler, predictable linear backoff strategy. This change is well-justified by the new ADR.
• docs/adr/0007-simplified-bluetooth-reconnection.md: The addition of ADR-0007 provides excellent documentation for this architectural decision, clearly outlining the context, problem statement, decision drivers, outcome, and pros/cons. This adherence to ADRs is crucial for maintaining architectural clarity.
• Code Conciseness: As noted in the ADR, this simplification has led to a significant reduction in lines of code (~91 lines removed from useBluetoothHRM.ts), which directly aligns with the "Code Conciseness & Redundancy Prevention" guidelines in docs/CODE_REVIEW_GUIDELINES.md.
• Centralized Resilience: Moving the retry logic to the primary reconnection loop and removing internal retries from connectToGatt improves the state machine's cleanliness and separation of concerns.

3. Test Updates

• All relevant unit and Playwright tests have been updated to reflect the component extraction, button text changes, and the new reconnection logic and constants. This ensures continued test coverage and reliability.

Overall, this PR delivers substantial improvements in code quality, maintainability, and documentation. The changes are well-executed and align with project guidelines.

Reviewed at commit: 65c388aaa7979789aac607b11ad4ea403034c50a

[arii]

Reviewed commit: 004da8084f5541f8a3d7f4343bb71f0964e4ae49

This PR introduces excellent refactoring and significant architectural improvements, particularly in the Bluetooth HRM reconnection strategy. The changes align well with the project's core principles of quality, consistency, and continuous improvement, as outlined in docs/DEVELOPMENT.md.

1. Component Extraction: ResetSection

• The extraction of the ResetSection component from ConnectView.tsx into its own file (app/client/connect/components/ResetSection.tsx) is a great move. It improves modularity, readability, and reusability of the ConnectView component. This directly adheres to the "Don't Repeat Yourself (DRY)" principle and "Code Conciseness & Redundancy Prevention" guidelines.
• The component correctly uses MUI components and theme values (e.g., theme.palette.divider for the border), maintaining UI consistency as per docs/DESIGN_GUIDELINES.md.
• The updated button text from "Reset System & Device" to "Reset Permissions & Settings" is a clear UX improvement, providing more accurate context to the user.

2. Simplified Bluetooth HRM Reconnection Strategy

• This is a substantial and well-justified architectural decision. The move from a complex randomized exponential backoff to a predictable linear backoff strategy significantly reduces cyclomatic complexity and improves maintainability of the useBluetoothHRM hook.
• The new ADR (docs/adr/0007-simplified-bluetooth-reconnection.md) provides excellent documentation for this change, detailing the context, decision drivers, and outcomes. This is crucial for understanding significant architectural choices.
• The connectToGatt function is now cleaner, focusing on a single connection attempt, with the reconnect function handling the retry logic. This separation of concerns is a strong improvement.
• The adjustment of BLUETOOTH_MAX_RECONNECT_ATTEMPTS to 8 and RECONNECT_BASE_DELAY_MS to 2000ms provides a robust 72-second reconnection window, as explained in the ADR.
• The removal of the deviceBusy message and associated complex retry logic from constants/bluetooth-messages.ts and useBluetoothHRM.ts directly contributes to code conciseness.

3. Testing and Code Hygiene

• All relevant unit and Playwright tests have been updated to reflect the changes in component text and reconnection logic, ensuring continued correctness and stability.
• The removal of old Python verification scripts (verification/verify_stale.py, verify_empty_dashboard.py) is a good step for code hygiene, assuming they are no longer needed.

Overall, this PR is a high-quality contribution that enhances the codebase's maintainability, readability, and architectural soundness.

Reviewed at commit: 004da8084f5541f8a3d7f4343bb71f0964e4ae49

[arii]

Reviewed commit: 0c37db68e063923aaf8717176482d89c31db1349

This PR introduces several positive changes, including the extraction of the ResetSection component and a significant simplification of the Bluetooth HRM reconnection strategy. The reconnection changes are well-documented with a new ADR, which is excellent.

However, this PR combines multiple distinct logical changes, which violates our 'One Logical Change Per PR' guideline as outlined in docs/DEVELOPMENT.md (PR Scope).

Specifically:

1. Component Extraction: The ResetSection component extraction is a good refactor, improving modularity. The change in button text from 'Reset System & Device' to 'Reset Permissions & Settings' is a functional change that should be explicitly mentioned in the PR description.
2. Bluetooth Reconnection Strategy: The simplification of the Bluetooth HRM reconnection logic is a significant architectural improvement, reducing complexity and improving maintainability, as detailed in docs/adr/0007-simplified-bluetooth-reconnection.md. The corresponding updates to constants, useBluetoothHRM hook, and tests are well-executed.
3. Deletion of Python Verification Scripts: The removal of verification/verify_stale.py and verify_empty_dashboard.py is a substantial change that is unrelated to the PR's stated purpose. If these scripts are obsolete, their removal should be a separate chore PR with a clear justification. If they are still relevant, their deletion needs to be reverted.

To maintain a clean and reviewable commit history, please split this PR into separate, focused PRs. I've created a technical-debt issue for this scope violation.

Reviewed at commit: 0c37db68e063923aaf8717176482d89c31db1349

[arii]

Reviewed commit: a79705b5dbdcc765d0cc6eff7709afec77f41cf2

This PR introduces several positive changes, including the successful extraction of the ResetSection component, which improves modularity and maintainability. The simplification of the Bluetooth HRM reconnection strategy, along with the accompanying ADR (docs/adr/0007-simplified-bluetooth-reconnection.md), is an excellent step towards reducing complexity and technical debt in useBluetoothHRM.ts. The updates to constants, environment variables, and tests are consistent with these changes.

However, the PR's scope is broader than indicated by its title "feat(small): Extract ResetSection component from ConnectView.tsx". The Bluetooth reconnection refactor is a significant architectural change, and the deletion of two Python verification scripts (verification/verify_stale.py and verify_empty_dashboard.py) also represents a substantial change that is not directly related to the component extraction.

Per docs/DEVELOPMENT.md (PR Scope), "Each PR should address a single concern (e.g., one bug fix, one feature)" and "Architectural changes and large-scale refactoring should be done in separate PRs from feature work or bug fixes." This PR combines a small feature extraction with a large refactoring effort and a cleanup of verification scripts.

I am requesting changes to address the PR scope and to clarify the deletion of the verification scripts.

Reviewed at commit: a79705b5dbdcc765d0cc6eff7709afec77f41cf2

[arii]

🤖 AI Technical Audit

Principal Engineer Review

⚠️ CRITICAL BLOCKERS: Merge Conflicts & Scope Creep

This PR cannot be merged or reviewed in depth in its current state due to the presence of Git Merge Markers (<<<<<<< HEAD) and significant Scope Creep.

ANTI-AI-SLOP DIRECTIVES

1. CODE RATIO (CRITICAL): This PR is ostensibly about a "small" UI refactor (Extract ResetSection), yet ~80% of the diff relates to a complex rewrite of the Bluetooth reconnection logic. You MUST remove the following files from this PR to restore the ratio to the stated scope:
   • hooks/useBluetoothHRM.ts
   • constants/bluetooth-reconnection.ts
   • docs/ARCHITECTURE_DECISIONS.md
   • docs/adr/0007-simplified-bluetooth-reconnection.md
   • All unrelated test updates.
2. OVER-ENGINEERING: The inclusion of docs/adr/0007-simplified-bluetooth-reconnection.md in a UI refactoring PR is the definition of process failure. Architectural decisions for backend/hardware logic do not belong in a frontend component extraction PR.
3. STALE FEATURES: The description claims "Verified that... the 'Reset System & Device' button is functional", but the code changes the text to "Reset Permissions & Settings". The description appears generated or copy-pasted without verifying the actual code changes.

---

File-by-File Analysis

📄 app/client/connect/components/ResetSection.tsx (The Intended Change)

Status: Needs Correction

Problem: Implicit Copy Change.
You changed the button label from "Reset System & Device" to "Reset Permissions & Settings". While the extracted component structure is correct and the use of borderColor: 'divider' is a good improvement, the copy change contradicts your PR description.

Implementation Sample (If copy change was unintentional):

// Revert to original text if not explicitly required by a ticket
{isResetting ? 'Resetting...' : 'Reset System & Device'}

📄 hooks/useBluetoothHRM.ts

Status: 🚫 DELETE FROM PR

Problem: Broken Code / Merge Conflicts.
This file contains raw merge markers. This code is syntax-invalid and will crash the build.

Evidence:

<<<<<<< HEAD
import {
  BLUETOOTH_MAX_RECONNECT_ATTEMPTS,
  RECONNECT_BASE_DELAY_MS,
} from '@/constants/bluetooth-reconnection'
// ...
=======
import { BLUETOOTH_MAX_RECONNECT_ATTEMPTS } from '@/constants/bluetooth-reconnection'
>>>>>>> origin/leader

📄 docs/ARCHITECTURE_DECISIONS.md

Status: 🚫 DELETE FROM PR

Problem: Merge Conflicts.
Similar to the hook, this documentation file is corrupted with merge markers.

📄 constants/bluetooth-reconnection.ts

Status: 🚫 DELETE FROM PR

Problem: Unrelated Logic Change.
Changing BLUETOOTH_MAX_RECONNECT_ATTEMPTS from 5 to 8 is a logic change that has nothing to do with extracting a React component. Do not mix configuration tuning with refactoring.

Architectural Impact

The intended change (UI extraction) has low impact and improves modularity. However, the accidental inclusion of the Bluetooth rewrite radically changes the device connectivity state machine (switching from exponential to linear backoff). This creates a high risk of introducing bugs into the connectivity layer under the guise of a "small" UI task.

GitHub Checks

Automated checks are not listed, but if they ran, they would FAIL immediately due to syntax errors in useBluetoothHRM.ts caused by <<<<<<< HEAD markers.

Review automatically published via RepoAuditor.

[arii]

Reviewed commit: 011e926a7b7be0ee1d2827491f101d906fa7e389

The extraction of the ResetSection component into its own file (app/client/connect/components/ResetSection.tsx) is a good refactoring step. It improves modularity and readability, and the component correctly uses MUI's sx prop with borderColor: 'divider' for theme consistency, as outlined in docs/DESIGN_GUIDELINES.md.

However, this PR also includes a change in .github/actions/setup-env/action.yml which removes the installation of GitHub CLI. While this might be a valid cleanup, it is unrelated to the component extraction. According to our Development Guide, 'each PR should address a single concern'.

Please consider reverting the changes to .github/actions/setup-env/action.yml in this PR and submitting them as a separate 'chore' PR. This will help maintain clear PR scope and facilitate easier reviews.

Reviewed at commit: 011e926a7b7be0ee1d2827491f101d906fa7e389

[arii]

🤖 AI Technical Audit

Critical Code Review: PR #8955

Verdict: ⛔ CHANGES REQUESTED (Critical)

This PR is currently in a broken state. You have committed Git merge conflict markers (<<<<<<< HEAD, =======, >>>>>>>) directly into the codebase. This renders the code syntactically invalid and breaks the CI/CD pipeline.

Furthermore, the diff provided does not include ConnectView.tsx, which the description claims was modified to remove the old code. Without seeing that file, I cannot verify that the code was actually extracted and not just duplicated.

🚫 ANTI-AI-SLOP DIRECTIVES

1. CODE RATIO: The diff is polluted with Git conflict markers. Approximately 20% of the changed lines are generated conflict metadata that must be removed immediately.
2. STALE FEATURES: The presence of >>>>>>> origin/leader indicates you are attempting to merge into a branch that has diverged significantly, or you resolved a rebase incorrectly. The conflicting content suggests functional divergence (e.g., Button text "Reset System & Device" vs "Reset Permissions & Settings").
3. OVER-ENGINEERING: N/A - The current issue is under-engineering (lack of diligence in resolving merge conflicts).
4. DUPLICATE HOOKS/TYPES: Cannot be fully assessed until syntax errors are resolved, but ResetSectionProps is defined inside the conflict blocks.
5. OVERLY VERBOSE COMMENTS: N/A.

---

📁 File-by-File Analysis

1. .github/actions/setup-env/action.yml

🔴 PROBLEM: Unresolved Merge Conflicts
This file controls the CI environment. Committing conflict markers here breaks the build for everyone.

Implementation Sample (Current Broken State):

<<<<<<< HEAD
=======
    - name: Install GitHub CLI
      shell: bash
      run: |
>>>>>>> origin/leader

Fix: Rebase correctly against origin/leader and choose the correct blocks.

2. app/client/connect/components/ResetSection.tsx

🔴 PROBLEM: Unresolved Merge Conflicts & Logic Divergence
The component file contains multiple conflict blocks. You need to decide which logic to keep. The PR description says "Reset System & Device", but the origin/leader branch seems to have updated this to "Reset Permissions & Settings".

Implementation Sample (Conflict):

<<<<<<< HEAD
      {isResetting ? 'Resetting...' : 'Reset System & Device'}
=======
      {isResetting ? 'Resetting...' : 'Reset Permissions & Settings'}
>>>>>>> origin/leader

Architectural Note:
The HEAD version uses borderTop: '1px solid' while origin/leader uses MUI system props borderTop: 1. The latter (borderTop: 1) is the correct implementations for MUI System to work with borderColor: 'divider' correctly without defining style type (solid) explicitly often.

Recommendation:

1. Reset your branch to the tip of origin/leader.
2. Apply your extraction changes again carefully.
3. Ensure ResetSectionProps is exported.
4. Align the text copy with the latest product requirements (check if "Permissions & Settings" is the new standard over "System & Device").

3. ConnectView.tsx (Missing from Diff)

🔴 PROBLEM: Missing Cleanup
The PR description states: "Removed the local definition of ResetSection from ConnectView.tsx". However, this file is not present in the diff. If this file is not updated, you are introducing Duplicate Code rather than refactoring it.

---

🚨 Architectural Impact

• System Stability: The current state breaks the build process due to invalid YAML and TSX syntax.
• Maintainability: The extraction (once fixed) is good, but the current conflicting text descriptions (System/Device vs Permissions/Settings) create ambiguity about the feature's scope.

Review automatically published via RepoAuditor.

[arii]

Reviewed commit: 006f5692f2c09566fd5f64460abd8455c272cbaa

The changes in ConnectView.test.tsx are well-executed and align with the PR's goal of extracting the ResetSection component. The test updates correctly reflect the new prop structure of ConnectView and include appropriate tests for the ResetSection's functionality and interaction with onForgetDevice and onReset callbacks. The use of waitFor for asynchronous actions and getByLabelText for accessibility-focused element selection are good practices.

Specifically, the refactoring of prop handling (e.g., userHeight as an object, explicit error props) and the updated unit toggle test demonstrate an improvement in the test suite's robustness and clarity. The new tests for the Reset Permissions & Settings section ensure that the extracted component's functionality is covered.

While the PR focuses on extracting a small component, the extensive defaultProps in the test file highlight that ConnectView still manages a significant amount of state and logic. This is not a blocker for this PR, but it indicates a potential area for future refactoring.

Reviewed at commit: 006f5692f2c09566fd5f64460abd8455c272cbaa