Skip to content

Commit bec5b03

Browse files
author
Mateusz
committed
feat(specs): update translation refactoring specs and add cli refactoring specs
- Update requirements, design, and tasks for cross-api-translation-refactoring - Add initial specs for cli-god-object-refactoring - Update CHANGELOG.md
1 parent 7ea8dd9 commit bec5b03

File tree

8 files changed

+1645
-738
lines changed

8 files changed

+1645
-738
lines changed

‎.kiro/specs/cli-god-object-refactoring/design.md‎

Lines changed: 458 additions & 0 deletions
Large diffs are not rendered by default.

‎.kiro/specs/cli-god-object-refactoring/requirements.md‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
# Requirements Document
2+
3+
## Introduction
4+
5+
This document specifies the requirements for refactoring `src/core/cli.py`, a 3.1k LOC "God Object" that violates multiple architectural principles. The CLI entry point currently contains significant business logic that should be distributed across specialized services following SOLID principles, DRY, proper DI container usage, and OOP design patterns. The refactoring must preserve all public APIs and maintain 100% backward compatibility while achieving a modular, layered architecture with strong separation of concerns.
6+
7+
## Glossary
8+
9+
- **CLI_Module**: The `src/core/cli.py` module serving as the command-line entry point for the LLM proxy server
10+
- **AppConfig**: The application configuration object (`src/core/config/app_config.py`) that holds all runtime settings
11+
- **ParameterResolution**: A tracking mechanism for recording the source of configuration parameters (CLI, env, config file)
12+
- **ArgumentParser**: The `argparse.ArgumentParser` instance that defines and parses CLI arguments
13+
- **ConfigurationApplicator**: A proposed service responsible for applying parsed CLI arguments to AppConfig
14+
- **ServerLifecycleManager**: A proposed service responsible for server startup, shutdown, and daemon mode handling
15+
- **PrivilegeChecker**: A proposed service responsible for cross-platform privilege/admin detection
16+
- **LoggingConfigurator**: A proposed service responsible for configuring logging based on AppConfig
17+
- **ErrorHandler**: A proposed service responsible for user-friendly error message formatting
18+
19+
## Requirements
20+
21+
### Requirement 1
22+
23+
**User Story:** As a developer, I want the CLI argument parsing to be separated from configuration application, so that I can test and maintain each concern independently.
24+
25+
#### Acceptance Criteria
26+
27+
1. WHEN the CLI_Module parses arguments THEN the ArgumentParser SHALL be constructed by a dedicated `ArgumentParserBuilder` class
28+
2. WHEN CLI arguments are parsed THEN the CLI_Module SHALL delegate to a `ConfigurationApplicator` service for applying arguments to AppConfig
29+
3. WHEN the `ConfigurationApplicator` applies arguments THEN the service SHALL record parameter sources via ParameterResolution
30+
4. WHEN argument validation fails THEN the CLI_Module SHALL receive a structured error from the validation service
31+
5. WHEN a new CLI argument is added THEN the developer SHALL only need to modify the `ArgumentParserBuilder` and `ConfigurationApplicator`
32+
33+
### Requirement 2
34+
35+
**User Story:** As a developer, I want server lifecycle management extracted into a dedicated service, so that startup, shutdown, and daemon mode logic are testable in isolation.
36+
37+
#### Acceptance Criteria
38+
39+
1. WHEN the server starts THEN the ServerLifecycleManager SHALL coordinate port availability checks, privilege verification, and uvicorn startup
40+
2. WHEN daemon mode is requested THEN the ServerLifecycleManager SHALL handle platform-specific daemonization (Unix fork, Windows subprocess)
41+
3. WHEN the server encounters a startup error THEN the ServerLifecycleManager SHALL delegate to ErrorHandler for user-friendly messages
42+
4. WHEN multiple servers start (main + Anthropic) THEN the ServerLifecycleManager SHALL coordinate their concurrent execution
43+
5. WHEN the server shuts down THEN the ServerLifecycleManager SHALL ensure graceful cleanup of all resources
44+
45+
### Requirement 3
46+
47+
**User Story:** As a developer, I want privilege checking extracted into a dedicated service, so that cross-platform admin detection is reusable and testable.
48+
49+
#### Acceptance Criteria
50+
51+
1. WHEN privilege checking is invoked THEN the PrivilegeChecker SHALL detect admin/root status on Windows, Linux, and macOS
52+
2. WHEN running as admin without `--allow-admin` THEN the PrivilegeChecker SHALL raise a structured exception
53+
3. WHEN the platform lacks privilege functionality THEN the PrivilegeChecker SHALL return a safe default without crashing
54+
4. WHEN privilege checking is tested THEN the PrivilegeChecker SHALL accept injectable platform detection for mocking
55+
56+
### Requirement 4
57+
58+
**User Story:** As a developer, I want logging configuration extracted into a dedicated service, so that log setup is consistent and testable.
59+
60+
#### Acceptance Criteria
61+
62+
1. WHEN logging is configured THEN the LoggingConfigurator SHALL apply log level, file path, and color settings from AppConfig
63+
2. WHEN log file paths need timestamp suffixes THEN the LoggingConfigurator SHALL apply them consistently
64+
3. WHEN logging configuration fails THEN the LoggingConfigurator SHALL provide clear error messages
65+
4. WHEN the LoggingConfigurator is tested THEN it SHALL accept injectable logging handlers for verification
66+
67+
### Requirement 5
68+
69+
**User Story:** As a developer, I want error handling extracted into a dedicated service, so that user-friendly error messages are consistent and maintainable.
70+
71+
#### Acceptance Criteria
72+
73+
1. WHEN an application build error occurs THEN the ErrorHandler SHALL format a user-friendly message with actionable guidance
74+
2. WHEN OAuth token expiration is detected THEN the ErrorHandler SHALL provide specific re-authentication instructions
75+
3. WHEN API key errors occur THEN the ErrorHandler SHALL list required environment variables
76+
4. WHEN an unknown error occurs THEN the ErrorHandler SHALL provide generic troubleshooting guidance
77+
5. WHEN error messages are displayed THEN the ErrorHandler SHALL write to stderr with consistent formatting
78+
79+
### Requirement 6
80+
81+
**User Story:** As a developer, I want the `apply_cli_args` function decomposed into smaller, focused functions, so that each configuration domain is handled independently.
82+
83+
#### Acceptance Criteria
84+
85+
1. WHEN CLI arguments are applied THEN the ConfigurationApplicator SHALL delegate to domain-specific applicators (server, logging, backends, session, auth, etc.)
86+
2. WHEN a domain applicator processes arguments THEN it SHALL only modify its relevant configuration section
87+
3. WHEN environment variables are set THEN the domain applicator SHALL handle them within its scope
88+
4. WHEN a new configuration domain is added THEN the developer SHALL create a new domain applicator without modifying existing ones
89+
5. WHEN domain applicators are tested THEN each SHALL be testable in isolation with mock AppConfig
90+
91+
### Requirement 7
92+
93+
**User Story:** As a developer, I want the refactored CLI to maintain 100% backward compatibility, so that existing scripts and integrations continue to work.
94+
95+
#### Acceptance Criteria
96+
97+
1. WHEN the refactored CLI is invoked THEN all existing command-line arguments SHALL be accepted with identical behavior
98+
2. WHEN the refactored CLI produces output THEN log messages and error formats SHALL match the original implementation
99+
3. WHEN the refactored CLI is tested THEN all existing CLI tests SHALL pass without modification
100+
4. WHEN the `main()` function is called THEN its signature and behavior SHALL remain unchanged
101+
5. WHEN `parse_cli_args()` or `apply_cli_args()` are called externally THEN their signatures and return types SHALL remain unchanged
102+
103+
### Requirement 8
104+
105+
**User Story:** As a developer, I want the refactored code to follow proper dependency injection patterns, so that services are loosely coupled and testable.
106+
107+
#### Acceptance Criteria
108+
109+
1. WHEN services are instantiated THEN they SHALL receive dependencies through constructor injection
110+
2. WHEN services need configuration THEN they SHALL receive AppConfig or relevant subsections through injection
111+
3. WHEN services are tested THEN mock dependencies SHALL be injectable without modifying production code
112+
4. WHEN the DI container is used THEN services SHALL be registered and resolved through the container
113+
114+
### Requirement 9
115+
116+
**User Story:** As a developer, I want the refactored code to have comprehensive test coverage, so that regressions are caught early.
117+
118+
#### Acceptance Criteria
119+
120+
1. WHEN new services are created THEN each SHALL have unit tests covering its public methods
121+
2. WHEN the refactoring is complete THEN the full test suite SHALL pass with zero regressions
122+
3. WHEN edge cases are identified THEN property-based tests SHALL verify behavior across input ranges
123+
4. WHEN integration points are modified THEN integration tests SHALL verify end-to-end behavior
124+
125+
### Requirement 10
126+
127+
**User Story:** As a developer, I want the refactored code organized into a clear module structure, so that related functionality is easy to locate.
128+
129+
#### Acceptance Criteria
130+
131+
1. WHEN the refactoring is complete THEN new services SHALL reside in `src/core/cli/` subdirectory
132+
2. WHEN the CLI module is imported THEN the public API SHALL be exposed through `src/core/cli/__init__.py`
133+
3. WHEN a developer looks for CLI-related code THEN the module structure SHALL clearly indicate each service's purpose
134+
4. WHEN the original `cli.py` is retained THEN it SHALL serve as a thin facade delegating to the new services

‎.kiro/specs/cli-god-object-refactoring/tasks.md‎

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
1+
# Implementation Plan
2+
3+
- [ ] 1. Set up package structure and base interfaces
4+
- [ ] 1.1 Create `src/core/cli/` package directory structure
5+
- Create `src/core/cli/__init__.py` with public API exports
6+
- Create `src/core/cli/applicators/__init__.py` for domain applicators
7+
- _Requirements: 10.1, 10.2, 10.3_
8+
- [ ] 1.2 Define `DomainApplicator` protocol and base types
9+
- Create `src/core/cli/protocols.py` with `DomainApplicator` protocol
10+
- Define common type aliases and constants
11+
- _Requirements: 6.1, 8.1, 8.2_
12+
- [ ] 1.3 Write property test for domain applicator isolation
13+
- **Property 3: Domain Applicator Isolation**
14+
- **Validates: Requirements 6.2**
15+
16+
- [ ] 2. Extract ArgumentParserBuilder
17+
- [ ] 2.1 Create `ArgumentParserBuilder` class
18+
- Create `src/core/cli/argument_parser_builder.py`
19+
- Extract `build_cli_parser()` logic into builder methods organized by domain
20+
- Implement `_add_server_arguments`, `_add_backend_arguments`, `_add_logging_arguments`, etc.
21+
- _Requirements: 1.1, 1.5_
22+
- [ ] 2.2 Update `cli.py` to use `ArgumentParserBuilder`
23+
- Replace `build_cli_parser()` with delegation to `ArgumentParserBuilder().build()`
24+
- Maintain backward compatibility of `build_cli_parser()` function signature
25+
- _Requirements: 7.1, 7.5_
26+
- [ ] 2.3 Write unit tests for ArgumentParserBuilder
27+
- Test that all expected arguments are present
28+
- Test argument groups are correctly organized
29+
- _Requirements: 9.1_
30+
31+
- [ ] 3. Extract domain applicators
32+
- [ ] 3.1 Create ServerApplicator
33+
- Create `src/core/cli/applicators/server_applicator.py`
34+
- Extract host, port, timeout, command_prefix, context_window logic
35+
- _Requirements: 6.1, 6.2_
36+
- [ ] 3.2 Create LoggingApplicator
37+
- Create `src/core/cli/applicators/logging_applicator.py`
38+
- Extract log_file, log_level, capture settings, CBOR capture logic
39+
- _Requirements: 6.1, 6.2_
40+
- [ ] 3.3 Create BackendApplicator
41+
- Create `src/core/cli/applicators/backend_applicator.py`
42+
- Extract default_backend, API keys, static_route, hybrid settings, debugging overrides
43+
- _Requirements: 6.1, 6.2_
44+
- [ ] 3.4 Create SessionApplicator
45+
- Create `src/core/cli/applicators/session_applicator.py`
46+
- Extract session flags, planning phase, tool access, pytest settings
47+
- _Requirements: 6.1, 6.2_
48+
- [ ] 3.5 Create AuthApplicator
49+
- Create `src/core/cli/applicators/auth_applicator.py`
50+
- Extract auth flags, SSO settings, brute force protection
51+
- _Requirements: 6.1, 6.2_
52+
- [ ] 3.6 Create AssessmentApplicator
53+
- Create `src/core/cli/applicators/assessment_applicator.py`
54+
- Extract LLM assessment configuration
55+
- _Requirements: 6.1, 6.2_
56+
- [ ] 3.7 Create MemoryApplicator
57+
- Create `src/core/cli/applicators/memory_applicator.py`
58+
- Extract ProxyMem configuration
59+
- _Requirements: 6.1, 6.2_
60+
- [ ] 3.8 Create FailureHandlingApplicator
61+
- Create `src/core/cli/applicators/failure_handling_applicator.py`
62+
- Extract failure handling configuration
63+
- _Requirements: 6.1, 6.2_
64+
- [ ] 3.9 Create additional applicators for remaining domains
65+
- Create `EditPrecisionApplicator`, `IdentityApplicator`, `RoutingApplicator`, `CompactionApplicator`
66+
- _Requirements: 6.1, 6.2_
67+
- [ ] 3.10 Write unit tests for domain applicators
68+
- Test each applicator modifies only its domain
69+
- Test environment variable handling
70+
- _Requirements: 6.5, 9.1_
71+
72+
- [ ] 4. Checkpoint - Ensure all tests pass
73+
- Ensure all tests pass, ask the user if questions arise.
74+
75+
- [ ] 5. Extract ConfigurationApplicator
76+
- [ ] 5.1 Create `ConfigurationApplicator` class
77+
- Create `src/core/cli/configuration_applicator.py`
78+
- Implement coordination of domain applicators
79+
- Handle ParameterResolution recording
80+
- _Requirements: 1.2, 1.3, 6.1_
81+
- [ ] 5.2 Update `cli.py` to use `ConfigurationApplicator`
82+
- Replace `apply_cli_args()` internals with delegation to `ConfigurationApplicator`
83+
- Maintain backward compatibility of `apply_cli_args()` signature
84+
- _Requirements: 7.1, 7.5_
85+
- [ ] 5.3 Write property test for argument parsing round-trip
86+
- **Property 1: Argument Parsing Round-Trip Consistency**
87+
- **Validates: Requirements 1.1, 1.2, 7.1**
88+
- [ ] 5.4 Write property test for parameter source recording
89+
- **Property 2: Parameter Source Recording Completeness**
90+
- **Validates: Requirements 1.3**
91+
92+
- [ ] 6. Extract ErrorHandler
93+
- [ ] 6.1 Create `ErrorHandler` class
94+
- Create `src/core/cli/error_handler.py`
95+
- Extract `_handle_application_build_error()` logic
96+
- Implement `ErrorType` enum and `classify_error()` method
97+
- Implement specialized message formatters
98+
- _Requirements: 5.1, 5.2, 5.3, 5.4, 5.5_
99+
- [ ] 6.2 Update `cli.py` to use `ErrorHandler`
100+
- Replace `_handle_application_build_error()` with delegation to `ErrorHandler`
101+
- _Requirements: 5.1_
102+
- [ ] 6.3 Write property test for error classification
103+
- **Property 4: Error Classification Consistency**
104+
- **Validates: Requirements 5.1, 5.2, 5.3, 5.4**
105+
106+
- [ ] 7. Extract LoggingConfigurator
107+
- [ ] 7.1 Create `LoggingConfigurator` class
108+
- Create `src/core/cli/logging_configurator.py`
109+
- Extract `_configure_logging()`, `_with_timestamp_suffix()`, `_apply_pid_suffixes()` logic
110+
- _Requirements: 4.1, 4.2, 4.3_
111+
- [ ] 7.2 Update `cli.py` to use `LoggingConfigurator`
112+
- Replace logging configuration functions with delegation to `LoggingConfigurator`
113+
- _Requirements: 4.1_
114+
- [ ] 7.3 Write property test for timestamp suffix format
115+
- **Property 5: Timestamp Suffix Format Validity**
116+
- **Validates: Requirements 4.2**
117+
118+
- [ ] 8. Extract PrivilegeChecker
119+
- [ ] 8.1 Create `PrivilegeChecker` class with injectable platform detector
120+
- Create `src/core/cli/privilege_checker.py`
121+
- Extract `_is_admin()`, `_has_privilege_functionality()`, `_check_privileges()` logic
122+
- Define `PlatformDetector` protocol for testability
123+
- _Requirements: 3.1, 3.2, 3.3, 3.4_
124+
- [ ] 8.2 Update `cli.py` to use `PrivilegeChecker`
125+
- Replace privilege checking functions with delegation to `PrivilegeChecker`
126+
- _Requirements: 3.1_
127+
- [ ] 8.3 Write property test for privilege enforcement
128+
- **Property 6: Privilege Check Enforcement**
129+
- **Validates: Requirements 3.2**
130+
131+
- [ ] 9. Checkpoint - Ensure all tests pass
132+
- Ensure all tests pass, ask the user if questions arise.
133+
134+
- [ ] 10. Extract ServerLifecycleManager
135+
- [ ] 10.1 Create `ServerLifecycleManager` class
136+
- Create `src/core/cli/server_lifecycle_manager.py`
137+
- Extract `is_port_in_use()`, `_daemonize()`, `_maybe_run_as_daemon()` logic
138+
- Implement server startup coordination
139+
- _Requirements: 2.1, 2.2, 2.3, 2.4, 2.5_
140+
- [ ] 10.2 Update `cli.py` to use `ServerLifecycleManager`
141+
- Replace server lifecycle logic in `main()` with delegation to `ServerLifecycleManager`
142+
- _Requirements: 2.1_
143+
- [ ] 10.3 Write unit tests for ServerLifecycleManager
144+
- Test port availability checking
145+
- Test daemon mode handling
146+
- Test server coordination
147+
- _Requirements: 9.1_
148+
149+
- [ ] 11. Refactor main() to thin facade
150+
- [ ] 11.1 Simplify `main()` function
151+
- Reduce `main()` to orchestration of extracted services
152+
- Ensure all business logic is delegated to services
153+
- _Requirements: 10.4_
154+
- [ ] 11.2 Update `src/core/cli/__init__.py` with public API
155+
- Export `parse_cli_args`, `apply_cli_args`, `main`, `build_cli_parser`
156+
- Export `ArgumentParserBuilder`, `ConfigurationApplicator` for advanced usage
157+
- _Requirements: 10.2, 7.4, 7.5_
158+
- [ ] 11.3 Write property test for public API signature preservation
159+
- **Property 7: Public API Signature Preservation**
160+
- **Validates: Requirements 7.4, 7.5**
161+
162+
- [ ] 12. Checkpoint - Ensure all tests pass
163+
- Ensure all tests pass, ask the user if questions arise.
164+
165+
- [ ] 13. Backward compatibility verification
166+
- [ ] 13.1 Run existing CLI tests
167+
- Execute all tests in `tests/` related to CLI functionality
168+
- Verify zero test modifications required
169+
- _Requirements: 7.3, 9.2_
170+
- [ ] 13.2 Run full test suite
171+
- Execute `./.venv/Scripts/python.exe -m pytest -m "integration or unit"`
172+
- Verify zero regressions
173+
- _Requirements: 9.2_
174+
- [ ] 13.3 Write integration tests for end-to-end CLI behavior
175+
- Test full CLI invocation with various argument combinations
176+
- Verify output matches original implementation
177+
- _Requirements: 7.2, 9.4_
178+
179+
- [ ] 14. Final Checkpoint - Ensure all tests pass
180+
- Ensure all tests pass, ask the user if questions arise.

0 commit comments

Comments
 (0)