Extract StreamDiscoveryContext to executor/stream_discovery.rs module #15

2025-07-05T14:37:27-07:00

jwilger commented

2025-07-05 14:37:27 -07:00

(Migrated from github.com)

Summary

Extract StreamDiscoveryContext type to dedicated executor/stream_discovery.rs module
Third small step in systematic executor.rs refactoring (2,956 lines → smaller modules)
Updates field accesses to use proper getter methods for better encapsulation
Maintains all existing APIs and functionality - no breaking changes

Type of Change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Refactoring (non-breaking change that improves code organization)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation update
Performance improvement
Test improvement

Testing Checklist

Unit tests pass locally
Integration tests pass locally
Property-based tests pass locally
All examples compile and run successfully
Benchmarks show no performance regressions
Memory usage remains stable under load

Performance Impact

No performance impact expected - pure code organization change with identical runtime behavior.

Security Checklist

Input validation requirements reviewed
Authentication/authorization impacts assessed
Data encryption requirements verified
Sensitive data exposure risks evaluated
Audit logging requirements confirmed

Code Quality

Code follows established patterns and conventions
Complex logic is well-documented
Error handling is comprehensive
Resource cleanup is handled properly
Thread safety considerations addressed

Reviewer Checklist

Business logic changes reviewed for correctness
Performance implications assessed
Security implications reviewed
Test coverage adequate for changes
Documentation updated as needed
Breaking changes properly communicated
Migration guide provided if needed

Review Focus

This is a pure refactoring with no functional changes. Focus on:

Verify module organization makes sense
Confirm no behavioral changes
Check that imports/exports are correct
Validate this follows the systematic refactoring plan from PLANNING.md
This builds on PR #13 (retry) and #14 (context) extractions

🤖 Generated with Claude Code

## Summary - Extract StreamDiscoveryContext type to dedicated executor/stream_discovery.rs module - Third small step in systematic executor.rs refactoring (2,956 lines → smaller modules) - Updates field accesses to use proper getter methods for better encapsulation - Maintains all existing APIs and functionality - no breaking changes ## Type of Change - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [x] Refactoring (non-breaking change that improves code organization) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] Documentation update - [ ] Performance improvement - [ ] Test improvement ## Testing Checklist - [ ] Unit tests pass locally - [ ] Integration tests pass locally - [ ] Property-based tests pass locally - [ ] All examples compile and run successfully - [ ] Benchmarks show no performance regressions - [ ] Memory usage remains stable under load ## Performance Impact No performance impact expected - pure code organization change with identical runtime behavior. ## Security Checklist - [ ] Input validation requirements reviewed - [ ] Authentication/authorization impacts assessed - [ ] Data encryption requirements verified - [ ] Sensitive data exposure risks evaluated - [ ] Audit logging requirements confirmed ## Code Quality - [ ] Code follows established patterns and conventions - [ ] Complex logic is well-documented - [ ] Error handling is comprehensive - [ ] Resource cleanup is handled properly - [ ] Thread safety considerations addressed ## Reviewer Checklist - [ ] Business logic changes reviewed for correctness - [ ] Performance implications assessed - [ ] Security implications reviewed - [ ] Test coverage adequate for changes - [ ] Documentation updated as needed - [ ] Breaking changes properly communicated - [ ] Migration guide provided if needed ## Review Focus This is a pure refactoring with no functional changes. Focus on: - Verify module organization makes sense - Confirm no behavioral changes - Check that imports/exports are correct - Validate this follows the systematic refactoring plan from PLANNING.md - This builds on PR #13 (retry) and #14 (context) extractions 🤖 Generated with [Claude Code](https://claude.ai/code)

copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed

2025-07-05 14:39:13 -07:00

copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull Request Overview

This PR refactors the executor module by extracting key types and logic into dedicated submodules to improve organization and encapsulation without changing existing behavior.

Extracted StreamDiscoveryContext into executor/stream_discovery.rs
Extracted retry logic (RetryConfig, RetryPolicy) into executor/retry.rs
Extracted ExecutionContext into executor/context.rs
Updated executor.rs to use new modules and split large methods into focused helper functions
Updated PLANNING.md to reflect the refactoring progress and process rules

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
executor/stream_discovery.rs	New module for stream discovery iteration state and APIs
executor/retry.rs	New module for retry configuration and policy
executor/context.rs	New module for execution context management
executor.rs	Refactored to import new modules, replace inlined logic, and add helper methods
PLANNING.md	Updated refactoring plan, added priorities and process rules

Comments suppressed due to low confidence (2)

PLANNING.md:102

[nitpick] This line contains an internal instruction for an LLM agent rather than human developers. Consider removing or rephrasing it so the plan focuses on actionable steps for the team.

**CRITICAL**: Claude must continue working through ALL refactoring tasks systematically without stopping. Always include "Review @PLANNING.md to identify next refactoring task and create new todo list to continue systematic refactoring" as the FINAL item in every refactoring todo list.

PLANNING.md:327

[nitpick] This final todo list entry duplicates the item at line 102. Consider deduplicating to avoid confusion and keep the plan concise.

5. **"Review @PLANNING.md to identify next refactoring task and create new todo list to continue systematic refactoring"** (FINAL ITEM for refactoring work)

## Pull Request Overview This PR refactors the `executor` module by extracting key types and logic into dedicated submodules to improve organization and encapsulation without changing existing behavior. - Extracted `StreamDiscoveryContext` into `executor/stream_discovery.rs` - Extracted retry logic (`RetryConfig`, `RetryPolicy`) into `executor/retry.rs` - Extracted `ExecutionContext` into `executor/context.rs` - Updated `executor.rs` to use new modules and split large methods into focused helper functions - Updated `PLANNING.md` to reflect the refactoring progress and process rules ### Reviewed Changes Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments. <details> <summary>Show a summary per file</summary> | File | Description | |-------------------------------------|------------------------------------------------------------------| | executor/stream_discovery.rs | New module for stream discovery iteration state and APIs | | executor/retry.rs | New module for retry configuration and policy | | executor/context.rs | New module for execution context management | | executor.rs | Refactored to import new modules, replace inlined logic, and add helper methods | | PLANNING.md | Updated refactoring plan, added priorities and process rules | </details> <details> <summary>Comments suppressed due to low confidence (2)</summary> **PLANNING.md:102** * [nitpick] This line contains an internal instruction for an LLM agent rather than human developers. Consider removing or rephrasing it so the plan focuses on actionable steps for the team. ``` **CRITICAL**: Claude must continue working through ALL refactoring tasks systematically without stopping. Always include "Review @PLANNING.md to identify next refactoring task and create new todo list to continue systematic refactoring" as the FINAL item in every refactoring todo list. ``` **PLANNING.md:327** * [nitpick] This final todo list entry duplicates the item at line 102. Consider deduplicating to avoid confusion and keep the plan concise. ``` 5. **"Review @PLANNING.md to identify next refactoring task and create new todo list to continue systematic refactoring"** (FINAL ITEM for refactoring work) ``` </details>

eventcore/src/executor/stream_discovery.rs

					
				@ -0,0 +54,4 @@

				    }

				    /// Checks if the iteration limit has been exceeded.

				    pub(crate) fn is_limit_exceeded(&self) -> bool {

The is_limit_exceeded method is not currently used; consider removing it or using it inside validate_iteration_limit to keep the iteration check logic DRY and consistent.

The `is_limit_exceeded` method is not currently used; consider removing it or using it inside `validate_iteration_limit` to keep the iteration check logic DRY and consistent.

eventcore/src/executor/stream_discovery.rs

					
				@ -0,0 +55,4 @@

				    /// Checks if the iteration limit has been exceeded.

				    pub(crate) fn is_limit_exceeded(&self) -> bool {

				        self.iteration >= self.max_iterations

The iteration check here (>=) and the one in validate_iteration_limit (>) differ by one; unify these to prevent an off-by-one in stopping conditions.

        self.iteration > self.max_iterations

The iteration check here (`>=`) and the one in `validate_iteration_limit` (`>`) differ by one; unify these to prevent an off-by-one in stopping conditions. ```suggestion self.iteration > self.max_iterations ```

jwilger commented

2025-07-06 14:10:08 -07:00

(Migrated from github.com)

Closing this PR as obsolete. PR #10 has been merged and already implemented this extraction! The StreamDiscoveryContext type has been successfully extracted to executor/stream_discovery.rs with a complete type-state implementation as part of the functional refactoring in PR #10.

Closing this PR as obsolete. PR #10 has been merged and already implemented this extraction! The `StreamDiscoveryContext` type has been successfully extracted to `executor/stream_discovery.rs` with a complete type-state implementation as part of the functional refactoring in PR #10.