refactor: Tech debt cleanup and code quality improvements#31
Merged
Conversation
- Create TermQShared target with Sendable domain models (Tag, Column, Card, Board) - Add shared OutputTypes (TerminalOutput, ColumnOutput, PendingOutput, ErrorOutput) - Add shared BoardLoader for consistent board loading/writing across CLI and MCP - Extract generic ComponentInstaller protocol to eliminate CLI/MCP installer duplication - Migrate termq-cli to use TermQShared (remove ~200 lines of duplicate models) - Migrate MCPServerLib to use TermQShared with type aliases for compatibility - Add TermQSharedTests for shared model coverage Net reduction: ~1000 lines of duplicated code Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Split BoardViewModel from 768 to 441 lines by extracting: - TabManager: Session tab state, navigation, attention indicators, transient cards - BoardPersistence: Save/load operations, file monitoring for external changes - FileMonitor: Helper class for dispatch source-based file monitoring BoardViewModel now coordinates these managers while maintaining backwards compatible API through proxy properties and methods. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Phase 3 of tech debt cleanup - split the TerminalSessionManager god object by extracting theme management into a dedicated TerminalThemeManager class. Changes: - Create TerminalThemeManager for theme state and application logic - TerminalSessionManager now delegates theme operations to themeManager - Proxy properties maintain API compatibility (themeId, currentTheme) - Theme callback mechanism for applying changes to all sessions Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Improve test coverage from 66% to 78%: - Add testResource* tests for all MCP resource handlers (terminals, columns, pending, context) - Add testPrompt* tests for all MCP prompt handlers (session_start, workflow_guide, terminal_summary) - Add test.coverage Makefile target for easy coverage reporting Coverage improvements: - ResourceHandlers.swift: 0% → 95% - PromptHandlers.swift: 0% → 94% Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Create .claude/commands/code-style.md capturing patterns learned: - Swift 6 Sendable conformance patterns - Actor-isolated class with dispatch source helpers - God object decomposition strategy - Init order with callback configure() pattern - Generic type extraction for code deduplication - Observable vs Sendable model separation - MCP testing patterns with type extraction helpers - Refactoring checklist for future upgrades Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Extract MCPStatusView from ContentView (23 lines) - Extract SafePasteAnalyzer from TerminalHostView (63 lines) - Extract LLMVendor enum from CardEditorView (54 lines) Addresses Issue #6 from forensic code review. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Create Constants.swift with Shell, Terminal, Columns, ColorPalette, LLMTokens, and Activity configuration constants - Update TerminalSessionManager to use Terminal and Activity constants - Update ColumnEditorView to use ColorPalette constants - Update ContentView to use Columns.fallbackName constant Addresses Issue #7 from forensic code review. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Define context-specific patterns for CLI, MCP, ViewModels, and file ops - Document when to use try?, throw, or return isError - Add typed error definition examples - List errors that should never be silently ignored - Update refactoring checklist with error handling items Addresses Issue #9 from forensic code review. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Create SchemaBuilder enum with type-safe property definitions
- Add convenience methods: string(), bool(), int() for common types
- Refactor SchemaDefinitions to use builder pattern
- Replace verbose nested dictionary literals with clean DSL syntax
Example:
inputSchema: S.objectSchema([
S.string("identifier", "Terminal name or UUID", required: true),
S.string("column", "Target column name", required: true),
])
Addresses forensic review Issue #10.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Create InputValidator enum with type-safe parameter extraction - Add validation methods: requireString, requireNonEmptyString, requireUUID - Add optional extractors: optionalString, optionalBool, optionalUUID, optionalPath - Define ValidationError enum with descriptive error messages - Refactor all MCP tool handlers to use centralized validation - Consistent error handling: return isError:true instead of throwing Benefits: - Type-safe parameter extraction reduces runtime errors - Consistent error messages improve LLM experience - Path validation with tilde expansion - UUID format validation with clear error messages Addresses forensic review Issue #11. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Audit findings: - No actual memory leaks found in codebase - All closures properly use [weak self] where needed - Event monitors have cleanup in onDisappear - Timers and dispatch sources cleaned up in deinit - Callback-based coordination avoids retain cycles Documentation added: - Closure capture rules for classes vs structs - Event monitor cleanup pattern - Timer/dispatch source cleanup - Callback-based coordination guidance - Checklist item for memory management Addresses forensic review Issue #12. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add comprehensive InputValidator tests (93% line coverage) - Add TermQError error description tests (100% coverage) - Add ColumnOutput, PendingOutput, PendingSummary tests - Add SetResponse and MoveResponse tests - Fix test assertions for order-independent badge comparisons - Fix path validation tests for trailing slash normalization Total coverage improved from ~75% to ~80% lines. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add termqcli and TermQ_TermQ.bundle to gitignore - these are generated during the build process and shouldn't be tracked. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
NSAlert requires main actor isolation in Xcode 16.4+ with stricter Swift concurrency checking. Add @mainactor annotation to satisfy the compiler's concurrency requirements. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR addresses accumulated tech debt and improves code quality across the codebase:
Architecture Improvements:
MCP Server Improvements:
Testing:
Documentation:
Housekeeping:
Test plan
🤖 Generated with Claude Code