refactor: reduce codebase complexity (~3,000 lines net reduction)#108
Open
ElNiak wants to merge 2 commits intoproductionfrom
Open
refactor: reduce codebase complexity (~3,000 lines net reduction)#108ElNiak wants to merge 2 commits intoproductionfrom
ElNiak wants to merge 2 commits intoproductionfrom
Conversation
…d classes Track 1 - God class splits: - Split DockerBuilder into platform_detection, buildx_operations, image_operations - Split MetricsCollector into metric_types, metric_analysis - Split ExperimentManager into experiment_phases, experiment_cleanup Track 2 - Event/observer simplification: - Replace ~130 event subclasses with factory classmethods on 8 base classes - Simplify ITypedObserver from 881 to ~80 lines with dynamic dispatch - Remove all backward-compat alias functions from events - Clean up events/__init__.py from 404 to ~100 lines - Update all emitters to use factory methods directly Track 3 - Validator cleanup: - Remove dead validator exports from pydantic_factories.py - Simplify validators/__init__.py (remove __getattr__ machinery) Net reduction: ~5,600 lines across 40 files. 870 tests passing.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Sorry @ElNiak, your pull request is larger than the review limit of 150000 diff characters
|
✅ PR Validation Passed Your changes look good! The quick validation checks have passed:
The full CI pipeline will run when this PR is merged or when targeting the main branches. |
Contributor
There was a problem hiding this comment.
Pull request overview
Large-scale refactor to reduce codebase complexity by consolidating the event system around factory-created base event classes, splitting “god classes” into mixins/modules, and simplifying observer dispatch/validator exports.
Changes:
- Replaced many per-event subclasses with factory classmethods on a smaller set of domain base event classes; updated emitters/observers/tests accordingly.
- Introduced new mixins/modules for experiment lifecycle phases and cleanup, plus DockerBuilder mixins for build/platform/buildx responsibilities.
- Simplified validator package exports by removing lazy
__getattr__import machinery.
Reviewed changes
Copilot reviewed 50 out of 50 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_core/test_event_system_integration.py | Updates tests to use ExperimentEvent/ServiceEvent factories instead of removed subclasses. |
| tests/unit/test_core/test_command_audit_observer.py | Updates tests to create service events via factories; adjusts expectations around supported event types/routing. |
| tests/integration/test_early_termination.py | Updates environment error event creation to factory-based EnvironmentEvent.error. |
| panther/plugins/services/service_event_mixin.py | Switches test-start emission to TestEvent.execution_started factory. |
| panther/plugins/environments/network_environment/shadow_ns/shadow_ns.py | Removes unused ExperimentFinishedEarlyEvent import. |
| panther/plugins/environments/network_environment/localhost_single_container/localhost_single_container.py | Removes unused ExperimentFinishedEarlyEvent import. |
| panther/core/observer/workflow/init.py | Docstring formatting cleanup. |
| panther/core/observer/impl/storage_observer.py | Moves toward BaseEvent-based handler signatures; trims imports; docstring changes. |
| panther/core/observer/impl/state_observer.py | Simplifies imports and shifts handlers to BaseEvent signatures for name-based dispatch. |
| panther/core/observer/impl/metrics_observer.py | Shifts step/test-start handlers to BaseEvent signatures; reduces event-class imports. |
| panther/core/observer/impl/logger_observer.py | Shifts error handlers to BaseEvent signatures and trims removed event imports. |
| panther/core/observer/impl/experiment_observer.py | Replaces class-based dispatch table with (entity_type, name) dispatch map and service fallback. |
| panther/core/observer/impl/command_audit_observer.py | Updates to BaseEvent-based handling and removes dependency on old event subclasses. |
| panther/core/metrics/resource_monitor.py | Docstring formatting cleanup. |
| panther/core/metrics/enums.py | Docstring formatting cleanup. |
| panther/core/metrics/metric_types.py | Adds Metric and TimingContext dataclasses for shared metrics structures. |
| panther/core/metrics/metric_analysis.py | Adds analysis/query mixin for filtering and summarizing collected metrics. |
| panther/core/experiment_phases.py | Introduces ExperimentPhasesMixin for initialization and test execution workflow. |
| panther/core/experiment_cleanup.py | Introduces ExperimentCleanupMixin for teardown, observer unregister, metrics export, and Docker cleanup. |
| panther/core/events/test/events.py | Refactors many test event subclasses into TestEvent factory classmethods. |
| panther/core/events/test/emitter.py | Updates emitter to create/notify factory-based test events. |
| panther/core/events/test/init.py | Trims exports to factory-based API + kept subclasses. |
| panther/core/events/step/events.py | Refactors many step event subclasses into StepEvent factory classmethods. |
| panther/core/events/step/emitter.py | Updates emitter to create/notify factory-based step events. |
| panther/core/events/step/init.py | Trims exports to factory-based API. |
| panther/core/events/service/events.py | Refactors many service event subclasses into ServiceEvent factories; keeps Docker build subclasses. |
| panther/core/events/service/emitter.py | Updates service emitter to use ServiceEvent factory methods. |
| panther/core/events/service/init.py | Trims exports to factory-based API + kept subclasses. |
| panther/core/events/plugin/events.py | Refactors many plugin event subclasses into PluginEvent factory classmethods. |
| panther/core/events/plugin/emitter.py | Updates plugin emitter to use PluginEvent factories. |
| panther/core/events/plugin/init.py | Trims exports to factory-based API. |
| panther/core/events/experiment/events.py | Refactors many experiment event subclasses into ExperimentEvent factories; keeps select subclasses for compatibility. |
| panther/core/events/experiment/emitter.py | Updates experiment emitter to use ExperimentEvent factories. |
| panther/core/events/experiment/init.py | Trims exports to factory-based API + kept subclasses. |
| panther/core/events/environment/emitter.py | Updates environment emitter to use factory-based EnvironmentEvent creation. |
| panther/core/events/environment/init.py | Trims exports to factory-based environment event API. |
| panther/core/events/assertion/events.py | Refactors many assertion event subclasses into AssertionEvent factory classmethods. |
| panther/core/events/assertion/emitter.py | Updates assertion emitter to use AssertionEvent factories. |
| panther/core/events/assertion/init.py | Trims exports to factory-based API. |
| panther/core/events/init.py | Updates public exports to match new factory-based event API; removes many subclass exports. |
| panther/core/docker_builder/platform_detection.py | Adds platform/arch detection mixin for Docker builds and cache platform isolation. |
| panther/core/docker_builder/image_operations.py | Adds image build/validation operations mixin for Docker builds. |
| panther/core/docker_builder/buildx_operations.py | Adds buildx operations mixin for cross-platform builds and builder/context management. |
| panther/config/core/validators/init.py | Replaces lazy __getattr__ exports with direct imports/__all__. |
Comments suppressed due to low confidence (3)
panther/core/observer/impl/storage_observer.py:6
- There are two top-level string literals: the module docstring at line 1 and a second triple-quoted string starting at line 5. The second one is not treated as the module docstring and becomes a no-op statement; it should be removed or merged into the real module docstring to avoid confusing docs/linting.
panther/core/observer/impl/command_audit_observer.py:7 - This file has both a module docstring (line 1) and an additional standalone triple-quoted string starting at line 5. The second string is not a docstring and will be executed as a no-op statement; please delete it or merge it into the actual module docstring.
panther/core/observer/impl/logger_observer.py:495 on_experiment_failedlogsevent.data['failure_reason'], butExperimentEvent.failed(...)populateserror_message/error_type(andsummary). This will log "Unknown reason" for real failed events; please switch to the correct data key(s).
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
54
to
76
| @@ -85,7 +72,7 @@ def handle_command_generation_started( | |||
| event.data.get("phase"), | |||
| ) | |||
|
|
|||
| def handle_command_generated(self, event: CommandGeneratedEvent) -> None: | |||
| def handle_command_generated(self, event: BaseEvent) -> None: | |||
| """Handle command generated event.""" | |||
…, and mixin inlining Wire hidden core modules to CLI (report, build-metrics, debug commands), delete orphaned tools and unused mixins, inline single-use mixins into their consumers, replace empty protocol placeholders with intermediate base classes (ClientServerProtocolBase, PeerToPeerProtocolBase). Net: -1,746 LOC, 13 files deleted, 5 created, 10 new CLI commands.
|
✅ PR Validation Passed Your changes look good! The quick validation checks have passed:
The full CI pipeline will run when this PR is merged or when targeting the main branches. |
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
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
__getattr__lazy import machinery50 files changed, 4,755 insertions, 7,663 deletions (net ~2,900 lines removed).
Test plan
from panther.core.events import *andfrom panther.core.observer import *