Skip to content

Code quality improvements and faster tunnel detection - #145

Merged
elyxlz merged 13 commits into
masterfrom
refactor/code-quality-improvements
Nov 23, 2025
Merged

Code quality improvements and faster tunnel detection#145
elyxlz merged 13 commits into
masterfrom
refactor/code-quality-improvements

Conversation

@elyxlz

@elyxlz elyxlz commented Nov 22, 2025 •
edited
Loading

Copy link
Copy Markdown
Owner

Summary

This PR includes two major improvements:

  1. Code Quality Refactoring - Reduced codebase by 74 lines while adding features and fixing bugs
  2. Fast SSH Tunnel Detection - Reduced stale tunnel detection from 3+ minutes to 30-40 seconds with automatic recovery

Code Quality Improvements

Refactoring Changes

  • Extracted helper functions to reduce duplication
  • Better type safety with tp.Literal types
  • Named constants for maintainability (ELLIPSIS_LENGTH, STATUS_ICONS)
  • Clearer error messaging throughout
  • Merged conflicts from master while preserving improvements

Bug Fixes

  • Fixed notification validation to properly handle user cancellation
  • Improved error handling consistency

Net Result: -74 lines of code with better functionality

SSH Tunnel Detection Improvements

Problem

Users experiencing stale SSH tunnels faced:

  • 180 seconds to detect dead connection (SSH keepalive)
  • Commands hanging forever on stale tunnels (no timeouts)
  • Manual retry required after failures

Solution - Phase 1: Fast Detection

1. Aggressive SSH Keepalive (tunnel_manager.py)

# Changed from: 60s interval, 3 attempts = 180s detection
# To: 15s interval, 2 attempts = 30s detection
"-o", "ServerAliveInterval=15",
"-o", "ServerAliveCountMax=2",
"-o", "TCPKeepAlive=yes",  # NEW

2. Fast-Fail Validation (tunnel_manager.py)

  • Control socket check: 5s → 2s
  • Port connectivity: 1.0s → 0.5s
  • Total validation: 6s+ → 2.5s

3. Request Timeouts (api_client.py - 12 locations)

  • Short operations (queries): 5s timeout
  • Medium operations (logs, status): 10s timeout
  • Long operations (artifact upload): 30s timeout

Solution - Phase 2: Automatic Recovery

Auto-Retry Logic (api_client.py)

  • Catches ConnectionError and Timeout exceptions
  • Automatically stops stale control master
  • Retries request after tunnel recreation (up to 2 attempts)
  • Shows user-friendly progress: Connection failed, recreating tunnel (attempt 1/2)...

Impact

Metric Before After
Stale detection time 180s 30-40s
Hung command timeout ∞ (infinite) 5-30s
Recovery Manual retry Automatic
User experience Frustrating hangs Seamless recovery

Files Modified

Code Quality:

  • src/nexus/cli/api_client.py - Type improvements, helper function
  • src/nexus/cli/constants.py - Added STATUS_ICONS
  • src/nexus/cli/jobs.py - Helper functions, better error messages
  • src/nexus/cli/utils.py - Named constants, formatting
  • src/nexus/server/api/router.py - Formatting
  • src/nexus/server/core/job.py - Formatting
  • src/nexus/server/external/system.py - Formatting

Tunnel Detection:

  • src/nexus/cli/tunnel_manager.py - SSH keepalive and validation
  • src/nexus/cli/api_client.py - Timeouts and retry logic

Test Plan

  • Resolve merge conflicts while preserving refactored code
  • Implement aggressive SSH keepalive settings
  • Add timeouts to all API requests
  • Implement automatic retry with tunnel recreation
  • Test with actual stale tunnel scenario
  • Verify commands recover automatically
  • Check that timeout values are appropriate for production use

🤖 Generated with Claude Code

elyxlz and others added 9 commits November 20, 2025 00:24
@elyxlz @claude
Phase 2: Deep code refactoring (-94 LOC total)
5acf5cb 
Optimizations implemented:
- Removed dead code (format_resource_str)
- Moved STATUS_COLOR_MAP to module level
- Extracted _is_git_repo() helper (2 duplicates → 1)
- Created _build_job_info() helper (6 duplicates → 1)
- Simplified verbose conditionals (-12 lines)
- Extracted _should_skip_wandb_check() for clarity

Results:
- LOC: -94 lines (318 insertions, 412 deletions)
- Tests: 40/42 passing ✓
- Type safety: 0 pyright errors ✓

Combined with Phase 1, achieved significant code quality
improvements while reducing total codebase size.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@elyxlz @claude
Fix critical bugs from code review
e03f590 
Critical fixes:
- Fix notification validation logic preventing silent job submission failures
- Fix CPU job display inconsistency between preview and success messages

High-priority fixes:
- Add Literal type annotation to format_gpu_info style parameter
- Extract magic numbers to named constants (WANDB_CHECK_*, JOB_INIT_MAX_ATTEMPTS, COMPLETED_JOB_LOG_TAIL_LINES)
- Improve wandb check logic with minimum delay (skip jobs < 30s and > 12min)
- Use frozenset for TERMINAL_STATUSES (O(1) vs O(n) lookups)

Results:
- LOC: -74 lines (340 insertions, 414 deletions)
- Type safety: 0 pyright errors ✓
- Tests: 40/42 passing ✓

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@elyxlz @claude
Fix screen permission test failures
bcf3e2d 
Set SCREENDIR to /tmp/nexus-screen with mode 700 to avoid permission errors in test environments where /run/screen is not accessible.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@elyxlz @claude
Phase 3: Code cleanup and CLI-server separation
2113114 
- Created nexus/cli/ids.py to duplicate generate_job_id() (eliminate server dependency)
- Created nexus/cli/constants.py with status constants and magic numbers
- Removed all imports from nexus.server in CLI code (complete module separation)
- Extracted _format_job_details() helper to consolidate duplicate formatting
- Extracted _resolve_job_target() helper to consolidate job lookup logic
- Extracted _format_gpu_status_part() helper to consolidate GPU status formatting
- Replaced all hardcoded status strings with constants
- Replaced all magic numbers with named constants
- Added missing type annotations to inline functions

Net impact: -31 LOC, improved maintainability, zero CLI-server coupling

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@elyxlz @claude
Additional code quality improvements and refactoring
8b8ef17 
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@elyxlz @claude
Productive LOC reduction: eliminate duplication with helper functions
17f8c98 
## Phase 1: CLI Job Preparation Logic
- Extract _build_notification_lists() - eliminates 20+ lines of duplicate list building
- Extract _load_jobrc() - eliminates 10 lines of duplicate file reading
- Extract _build_job_request() - eliminates 36 lines of duplicate job request construction
- Replace verbose loops with comprehensions for cleaner code

## Phase 2: Display Logic
- Add STATUS_ICONS dict - replace 7-line nested ternary with 1-line lookup
- Improve readability in show_history()

## Phase 3: Server-Side Improvements
- Extract _require_env() in notifications.py - reduce 32 lines to 7 (25-line reduction)
- Extract _clear_cache_if_refresh() in system.py - reduce 15 lines to 4 across 4 functions

## Quality Metrics
- Files Modified: 4
- Net Change: +3 lines (135 insertions, 132 deletions)
- Eliminated ~90 lines of duplication, added ~87 lines of reusable helpers
- DRY Compliance: Single source of truth for job preparation, env validation, cache clearing
- Type Safety: 0 pyright errors with proper overloads
- Tests: 42/42 passing

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@elyxlz
ruff
81b4bda
@elyxlz @claude
Resolve merge conflicts from master
e80c134 
Resolved conflicts in 7 files by keeping refactored code with:
- Better type safety (Literal types)
- Helper functions to reduce duplication
- Named constants for maintainability
- Clearer error messaging

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@elyxlz @claude
Improve SSH tunnel stale detection speed
5e9541d 
Reduces stale tunnel detection time from 3+ minutes to ~30-40 seconds
and adds automatic recovery.

**Phase 1: Fast Detection**
- Aggressive SSH keepalive: 15s interval (was 60s), 2 attempts (was 3)
  - Detects dead connections in 30s instead of 180s
- Fast-fail tunnel validation: 2.5s total (was 6s+)
  - Control socket check: 2s timeout (was 5s)
  - Port connectivity: 0.5s timeout (was 1s)
- Add timeouts to ALL API requests (12 locations):
  - Short operations: 5s timeout
  - Medium operations: 10s timeout
  - Long operations: 30s timeout
  - Prevents infinite hangs on stale tunnels

**Phase 2: Automatic Recovery**
- Auto-retry with tunnel recreation on connection failures
- Catches ConnectionError and Timeout exceptions
- Stops stale control master and retries (up to 2 attempts)
- User-friendly "recreating tunnel" progress messages

**Impact:**
- Before: 180s to detect, infinite hang, manual retry required
- After: 30-40s to detect, 5-30s fail-fast, automatic recovery

Files modified:
- src/nexus/cli/tunnel_manager.py: SSH keepalive and validation timeouts
- src/nexus/cli/api_client.py: Request timeouts and retry logic

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@elyxlz elyxlz changed the title refactor/code quality improvements Code quality improvements and faster tunnel detection Nov 23, 2025
elyxlz and others added 4 commits November 23, 2025 10:50
@elyxlz @claude
Fix pyright type error in retry logic
b036fcb 
Remove unreachable code and fix type checking issue where last_error
could be None.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@elyxlz @claude
ruff
963f73c 
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@elyxlz @claude
Bump version to 0.5.32
3bc8bd9 
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@elyxlz @claude
Update lock file for version 0.5.32
deb9984 
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@elyxlz
elyxlz merged commit 5ce9983 into master Nov 23, 2025
4 checks passed
@elyxlz elyxlz mentioned this pull request Nov 24, 2025
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@elyxlz