Repository navigation
refactor: fix file and class naming convention violations (OMN-1298) - #377
Merged
jonahgabriel merged 5 commits intoJan 12, 2026
Merged
jonahgabriel merged 5 commits into
jonahgabriel merged 5 commits into
Conversation
Rename files to follow directory-specific naming prefixes: - contracts/: diff_computer.py → contract_diff_computer.py - infrastructure/: infrastructure_bases.py → infra_bases.py, computation_cache.py → infra_computation_cache.py, load_balancer.py → infra_load_balancer.py, phase_sequencer.py → infra_phase_sequencer.py - pipeline/exceptions/: exception_*.py → pipeline_exception_*.py - pipeline/handlers/: model_capability_*.py → handler_capability_*.py - pipeline/replay/: executor_replay.py → runner_replay_executor.py, session_replay.py → runner_replay_session.py - services/diff/: store_diff_*.py → service_diff_*_store.py - services/replay/: recorder_effect.py → service_effect_recorder.py, injector_*.py → service_*_injector.py, sentinel_missing.py → service_sentinel_missing.py - services/sinks/: sink_*.py → service_sink_*.py - services/trace/: store_trace_*.py → service_trace_*_store.py, protocol_trace_store.py → protocols/storage/ - tools/mypy_plugins/: dict_any_checker.py → tool_dict_any_checker.py - validation/: visitor_any_type.py → checker_visitor_any_type.py, contract_validation_pipeline.py → validator_contract_pipeline.py - validation/phases/: *_validator.py → validator_*.py Update all imports and re-exports across source and test files.
Contributor
📝 WalkthroughWalkthroughThis PR systematically refactors module paths and class/entity names throughout the codebase, including renaming infrastructure modules (infrastructure_bases → infra_bases), renaming Model/Injector/Store/Sink classes to Service-prefixed variants, reorganizing validation modules with lazy exports, and updating documentation, tests, and scripts accordingly. No functional behavior changes are introduced. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/omnibase_core/pipeline/exceptions/pipeline_exception_duplicate_hook.py (1)
6-18: Remove the duplicateDuplicateHookErrorclass fromsrc/omnibase_core/errors/error_duplicate_hook.py.The codebase now contains two identical implementations of
DuplicateHookError:
src/omnibase_core/pipeline/exceptions/pipeline_exception_duplicate_hook.py(active, being imported and used)src/omnibase_core/errors/error_duplicate_hook.py(orphaned, no longer used by the pipeline module)The pipeline module and all tests import from
pipeline/exceptions/, making the version inerrors/redundant. Either delete the legacy file or haveerrors/error_duplicate_hook.pyre-export frompipeline/exceptions/pipeline_exception_duplicate_hook.pyto maintain a single source of truth and avoid maintenance issues.docs/guides/HANDLER_CONVERSION_CHECKLIST.md (1)
226-227: Inconsistent grep pattern in validation command.The validation command uses
ModelCapabilityto verify handler exports, but based on the naming convention refactor, handlers should now useHandlerCapabilityprefix. Update the grep pattern to match the new naming.📝 Suggested fix
### Check Handler Exports ```bash # Verify handlers are exported -grep -r "ModelCapability" src/omnibase_core/pipeline/handlers/__init__.py +grep -r "HandlerCapability" src/omnibase_core/pipeline/handlers/__init__.py</details> </blockquote></details> <details> <summary>docs/guides/HANDLER_CONVERSION_GUIDE.md (1)</summary><blockquote> `340-353`: **Documentation inconsistency: "Model" prefix requirement contradicts "Handler" prefix examples.** Lines 340-348 state that all handler model classes MUST start with "Model" prefix, but the examples immediately below show classes starting with "Handler" prefix (e.g., `HandlerCapabilityMetrics`, `HandlerCapabilityCaching`). The "WRONG" section mentions "Missing 'Model' prefix" but the "CORRECT" examples don't use "Model" prefix either. This section needs to be updated to reflect the new `Handler*` naming convention, or the examples need to be corrected to match the stated requirement. <details> <summary>📝 Suggested fix to align documentation with examples</summary> ```diff ### 4. "Model" Prefix for All Classes -All handler model classes MUST start with "Model" prefix: +All handler capability classes MUST follow the `Handler<Type><Name>` naming pattern: ```python # CORRECT class HandlerCapabilityMetrics(BaseModel): ... class HandlerCapabilityCaching(BaseModel): ... class HandlerCapabilityRetry(BaseModel): ... # WRONG -class MetricsHandler(BaseModel): ... # Missing "Model" prefix -class CachingCapability(BaseModel): ... # Missing "Model" prefix +class MetricsHandler(BaseModel): ... # Missing "Handler" prefix and wrong order +class CachingCapability(BaseModel): ... # Missing "Handler" prefix pattern</details> </blockquote></details> </blockquote></details>🤖 Fix all issues with AI agents
In @src/omnibase_core/pipeline/exceptions/__init__.py: - Around line 13-24: The pipeline/exceptions/ directory duplicates all eight exception classes; remove that directory and update its consumers to import the canonical exceptions from omnibase_core.errors instead. Delete the pipeline/exceptions package (or its __init__.py and modules) and replace internal re-exports such as DuplicateHookError, HookRegistryFrozenError, HookTimeoutError, HookTypeMismatchError, PipelineError, CallableNotFoundError, DependencyCycleError, and UnknownDependencyError so that files like pipeline/exceptions/__init__.py simply import and re-export these symbols from omnibase_core.errors (e.g., from omnibase_core.errors import DuplicateHookError, HookTimeoutError, ...), updating any import sites to reference omnibase_core.errors.* as needed and running tests to ensure no remaining references to the removed modules.🧹 Nitpick comments (3)
tests/unit/infrastructure/test_load_balancer.py (1)
14-21: Consider adding pytest markers for test categorization.Per coding guidelines, tests should use pytest markers (
@pytest.mark.unit,@pytest.mark.integration, etc.) for test categorization. These unit tests forLoadBalancerwould benefit from@pytest.mark.unitmarkers.♻️ Example marker addition
+@pytest.mark.unit def test_load_balancer_initialization(): """Test that LoadBalancer initializes with correct defaults.""" lb = LoadBalancer()tests/unit/infrastructure/execution/test_phase_sequencer.py (1)
29-39: Import path correctly updated.The import from
omnibase_core.infrastructure.execution.infra_phase_sequenceris valid and all functions remain unchanged.Consider renaming the test file to
test_infra_phase_sequencer.pyfor consistency with the source module naming convention. The codebase follows a pattern where source files with theinfra_prefix have corresponding test files with thetest_infra_prefix (e.g.,infra_bases.py→test_infra_bases.py).tests/unit/infrastructure/test_infra_bases.py (1)
1-35: Add@pytest.mark.unitmarkers for test categorization.As per coding guidelines, tests should use pytest markers for categorization. These unit tests are missing the
@pytest.mark.unitdecorator.♻️ Proposed fix to add pytest markers
""" Tests for infra_bases module. Validates that infrastructure base imports are accessible and properly exposed. """ +import pytest + +@pytest.mark.unit def test_can_import_infra_bases(): """Test that infra_bases module imports successfully.""" from omnibase_core.infrastructure import infra_bases assert infra_bases is not None +@pytest.mark.unit def test_service_effect_available(): """Test that ModelServiceEffect is accessible.""" from omnibase_core.infrastructure.infra_bases import ModelServiceEffect assert ModelServiceEffect is not None +@pytest.mark.unit def test_service_compute_available(): """Test that ModelServiceCompute is accessible.""" from omnibase_core.infrastructure.infra_bases import ModelServiceCompute assert ModelServiceCompute is not None +@pytest.mark.unit def test_all_exports_defined(): """Test that __all__ contains expected exports.""" from omnibase_core.infrastructure import infra_bases assert hasattr(infra_bases, "__all__") assert "ModelServiceEffect" in infra_bases.__all__ assert "ModelServiceCompute" in infra_bases.__all__📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (106)
docs/architecture/ECOSYSTEM_DIRECTORY_STRUCTURE.mddocs/architecture/NODE_CLASS_HIERARCHY.mddocs/architecture/SUBCONTRACT_ARCHITECTURE.mddocs/guides/HANDLER_CONVERSION_CHECKLIST.mddocs/guides/HANDLER_CONVERSION_GUIDE.mddocs/guides/node-building/03_COMPUTE_NODE_TUTORIAL.mddocs/guides/node-building/04_EFFECT_NODE_TUTORIAL.mddocs/reference/SERVICE_WRAPPERS.mdscripts/validate_class_naming.pyscripts/validation/validate-file-locations.pyscripts/validation/validate_naming.pysrc/omnibase_core/cli/cli_contract.pysrc/omnibase_core/contracts/contract_diff_computer.pysrc/omnibase_core/enums/replay/enum_recorder_mode.pysrc/omnibase_core/infrastructure/__init__.pysrc/omnibase_core/infrastructure/execution/__init__.pysrc/omnibase_core/infrastructure/execution/infra_phase_sequencer.pysrc/omnibase_core/infrastructure/infra_bases.pysrc/omnibase_core/infrastructure/infra_computation_cache.pysrc/omnibase_core/infrastructure/infra_load_balancer.pysrc/omnibase_core/models/contracts/diff/__init__.pysrc/omnibase_core/models/diff/__init__.pysrc/omnibase_core/models/diff/model_diff_storage_configuration.pysrc/omnibase_core/models/replay/model_effect_record.pysrc/omnibase_core/models/trace_query/model_trace_query.pysrc/omnibase_core/models/trace_query/model_trace_summary.pysrc/omnibase_core/pipeline/__init__.pysrc/omnibase_core/pipeline/exceptions/__init__.pysrc/omnibase_core/pipeline/exceptions/pipeline_exception.pysrc/omnibase_core/pipeline/exceptions/pipeline_exception_callable_not_found.pysrc/omnibase_core/pipeline/exceptions/pipeline_exception_dependency_cycle.pysrc/omnibase_core/pipeline/exceptions/pipeline_exception_duplicate_hook.pysrc/omnibase_core/pipeline/exceptions/pipeline_exception_hook_registry_frozen.pysrc/omnibase_core/pipeline/exceptions/pipeline_exception_hook_timeout.pysrc/omnibase_core/pipeline/exceptions/pipeline_exception_hook_type_mismatch.pysrc/omnibase_core/pipeline/exceptions/pipeline_exception_unknown_dependency.pysrc/omnibase_core/pipeline/handlers/__init__.pysrc/omnibase_core/pipeline/handlers/handler_capability_caching.pysrc/omnibase_core/pipeline/handlers/handler_capability_metrics.pysrc/omnibase_core/pipeline/replay/__init__.pysrc/omnibase_core/pipeline/replay/runner_replay_executor.pysrc/omnibase_core/pipeline/replay/runner_replay_session.pysrc/omnibase_core/protocols/replay/protocol_effect_recorder.pysrc/omnibase_core/protocols/replay/protocol_rng_service.pysrc/omnibase_core/protocols/replay/protocol_time_service.pysrc/omnibase_core/protocols/storage/__init__.pysrc/omnibase_core/protocols/storage/protocol_diff_store.pysrc/omnibase_core/protocols/storage/protocol_trace_store.pysrc/omnibase_core/services/diff/__init__.pysrc/omnibase_core/services/diff/service_diff_file_store.pysrc/omnibase_core/services/diff/service_diff_history.pysrc/omnibase_core/services/diff/service_diff_in_memory_store.pysrc/omnibase_core/services/replay/__init__.pysrc/omnibase_core/services/replay/service_config_override_injector.pysrc/omnibase_core/services/replay/service_corpus_replay_orchestrator.pysrc/omnibase_core/services/replay/service_effect_recorder.pysrc/omnibase_core/services/replay/service_rng_injector.pysrc/omnibase_core/services/replay/service_sentinel_missing.pysrc/omnibase_core/services/replay/service_time_injector.pysrc/omnibase_core/services/service_contract_validation_event_emitter.pysrc/omnibase_core/services/sinks/__init__.pysrc/omnibase_core/services/sinks/service_sink_file.pysrc/omnibase_core/services/sinks/service_sink_memory.pysrc/omnibase_core/services/trace/__init__.pysrc/omnibase_core/services/trace/service_trace_in_memory_store.pysrc/omnibase_core/services/trace/service_trace_recording.pysrc/omnibase_core/tools/mypy_plugins/__init__.pysrc/omnibase_core/tools/mypy_plugins/tool_dict_any_checker.pysrc/omnibase_core/validation/__init__.pysrc/omnibase_core/validation/checker_visitor_any_type.pysrc/omnibase_core/validation/phases/__init__.pysrc/omnibase_core/validation/phases/validator_expanded_contract.pysrc/omnibase_core/validation/phases/validator_expanded_contract_graph.pysrc/omnibase_core/validation/phases/validator_merge.pysrc/omnibase_core/validation/validator_any_type.pysrc/omnibase_core/validation/validator_contract_pipeline.pytests/integration/pipeline/replay/test_replay_e2e.pytests/integration/validation/test_contract_validation_events_e2e.pytests/unit/api_snapshots/test_infrastructure_api_snapshot.pytests/unit/contracts/test_contract_diff_computer.pytests/unit/infrastructure/execution/test_phase_sequencer.pytests/unit/infrastructure/test_computation_cache.pytests/unit/infrastructure/test_infra_bases.pytests/unit/infrastructure/test_infrastructure_bases.pytests/unit/infrastructure/test_load_balancer.pytests/unit/pipeline/handlers/test_capability_caching.pytests/unit/pipeline/handlers/test_capability_metrics.pytests/unit/pipeline/replay/test_executor_replay.pytests/unit/pipeline/replay/test_injector_rng.pytests/unit/pipeline/replay/test_injector_time.pytests/unit/pipeline/replay/test_recorder_effect.pytests/unit/services/diff/test_service_diff_history.pytests/unit/services/diff/test_store_diff_file.pytests/unit/services/diff/test_store_diff_in_memory.pytests/unit/services/replay/test_service_config_override_injector.pytests/unit/services/replay/test_service_corpus_replay_orchestrator.pytests/unit/services/test_service_contract_validation_event_emitter.pytests/unit/services/trace/__init__.pytests/unit/services/trace/test_service_trace_recording.pytests/unit/services/trace/test_store_trace_in_memory.pytests/unit/validation/phases/test_contract_validation_performance.pytests/unit/validation/phases/test_merge_validator.pytests/unit/validation/test_contract_validation_pipeline.pytests/unit/validation/test_contract_validation_pipeline_events.pytests/unit/validation/test_validator_any_type.pytests/unit/validation/test_visitor_any_type.py💤 Files with no reviewable changes (2)
- tests/unit/infrastructure/test_infrastructure_bases.py
- scripts/validation/validate-file-locations.py
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Always use Poetry for Python package management and task execution (poetry run pytest, poetry add, poetry install) - never use pip or python directly
Use structured error handling with ModelOnexError and EnumCoreErrorCode, never generic Exception - provide message, error_code, and context
Use @standard_error_handling decorator to eliminate try/catch boilerplate in async operations
Use standardized exception comment markers (# fallback-ok:, # catch-all-ok:, # cleanup-resilience-ok:, # boundary-ok:, # init-errors-ok:, # tool-resilience-ok:) when using catch-all or broad exception handlers to document intent
Order exception tuples alphabetically by exception class name when catching multiple exceptions
Use centralized exception groups from omnibase_core.errors.exception_groups (VALIDATION_ERRORS, PYDANTIC_MODEL_ERRORS, FILE_IO_ERRORS, etc.) for exception handling
Use PEP 604 union syntax (str | None) instead of legacy Union/Optional syntax - enforced by ruff UP007
Use FileRegistry from omnibase_core.runtime.runtime_file_registry for loading YAML contracts - handle error codes: FILE_NOT_FOUND, FILE_READ_ERROR, CONFIGURATION_PARSE_ERROR, CONTRACT_VALIDATION_ERROR, DUPLICATE_REGISTRATION, DIRECTORY_NOT_FOUND
Maintain minimum 60% test coverage requirement across the codebase measured with pytest-covFiles:
tests/unit/services/trace/__init__.pysrc/omnibase_core/models/diff/model_diff_storage_configuration.pysrc/omnibase_core/protocols/storage/protocol_diff_store.pytests/unit/infrastructure/execution/test_phase_sequencer.pysrc/omnibase_core/pipeline/handlers/handler_capability_caching.pysrc/omnibase_core/cli/cli_contract.pysrc/omnibase_core/models/diff/__init__.pysrc/omnibase_core/validation/validator_contract_pipeline.pysrc/omnibase_core/pipeline/exceptions/pipeline_exception_duplicate_hook.pytests/unit/services/diff/test_store_diff_file.pytests/unit/pipeline/replay/test_executor_replay.pytests/unit/validation/test_validator_any_type.pysrc/omnibase_core/protocols/replay/protocol_rng_service.pytests/unit/validation/test_contract_validation_pipeline_events.pytests/unit/services/replay/test_service_config_override_injector.pytests/unit/api_snapshots/test_infrastructure_api_snapshot.pysrc/omnibase_core/pipeline/handlers/__init__.pysrc/omnibase_core/enums/replay/enum_recorder_mode.pysrc/omnibase_core/services/trace/service_trace_recording.pysrc/omnibase_core/models/trace_query/model_trace_summary.pysrc/omnibase_core/services/trace/__init__.pysrc/omnibase_core/protocols/replay/protocol_effect_recorder.pysrc/omnibase_core/services/sinks/__init__.pyscripts/validation/validate_naming.pysrc/omnibase_core/services/diff/service_diff_in_memory_store.pytests/unit/services/test_service_contract_validation_event_emitter.pysrc/omnibase_core/protocols/replay/protocol_time_service.pytests/unit/infrastructure/test_infra_bases.pysrc/omnibase_core/models/trace_query/model_trace_query.pytests/unit/contracts/test_contract_diff_computer.pysrc/omnibase_core/pipeline/__init__.pytests/unit/validation/phases/test_contract_validation_performance.pysrc/omnibase_core/services/replay/service_corpus_replay_orchestrator.pytests/unit/services/replay/test_service_corpus_replay_orchestrator.pysrc/omnibase_core/infrastructure/execution/__init__.pysrc/omnibase_core/tools/mypy_plugins/__init__.pysrc/omnibase_core/services/replay/service_effect_recorder.pytests/unit/services/diff/test_service_diff_history.pysrc/omnibase_core/services/replay/service_config_override_injector.pysrc/omnibase_core/pipeline/exceptions/pipeline_exception_hook_timeout.pysrc/omnibase_core/pipeline/handlers/handler_capability_metrics.pysrc/omnibase_core/services/diff/service_diff_history.pysrc/omnibase_core/pipeline/exceptions/pipeline_exception_hook_registry_frozen.pysrc/omnibase_core/services/sinks/service_sink_memory.pysrc/omnibase_core/validation/validator_any_type.pytests/integration/validation/test_contract_validation_events_e2e.pysrc/omnibase_core/contracts/contract_diff_computer.pysrc/omnibase_core/services/trace/service_trace_in_memory_store.pytests/unit/pipeline/handlers/test_capability_caching.pysrc/omnibase_core/models/replay/model_effect_record.pysrc/omnibase_core/services/diff/__init__.pytests/unit/pipeline/replay/test_injector_rng.pytests/unit/pipeline/handlers/test_capability_metrics.pysrc/omnibase_core/pipeline/exceptions/pipeline_exception_unknown_dependency.pysrc/omnibase_core/pipeline/replay/__init__.pytests/unit/validation/test_visitor_any_type.pytests/unit/infrastructure/test_computation_cache.pysrc/omnibase_core/protocols/storage/protocol_trace_store.pysrc/omnibase_core/pipeline/exceptions/pipeline_exception_callable_not_found.pysrc/omnibase_core/protocols/storage/__init__.pysrc/omnibase_core/services/replay/__init__.pysrc/omnibase_core/services/sinks/service_sink_file.pysrc/omnibase_core/validation/checker_visitor_any_type.pyscripts/validate_class_naming.pytests/unit/validation/test_contract_validation_pipeline.pytests/unit/services/trace/test_store_trace_in_memory.pysrc/omnibase_core/models/contracts/diff/__init__.pysrc/omnibase_core/pipeline/exceptions/pipeline_exception_hook_type_mismatch.pytests/unit/validation/phases/test_merge_validator.pysrc/omnibase_core/pipeline/exceptions/pipeline_exception_dependency_cycle.pytests/unit/infrastructure/test_load_balancer.pysrc/omnibase_core/infrastructure/infra_bases.pytests/unit/pipeline/replay/test_injector_time.pytests/integration/pipeline/replay/test_replay_e2e.pysrc/omnibase_core/services/replay/service_time_injector.pysrc/omnibase_core/services/replay/service_rng_injector.pytests/unit/services/trace/test_service_trace_recording.pysrc/omnibase_core/validation/__init__.pysrc/omnibase_core/services/service_contract_validation_event_emitter.pytests/unit/pipeline/replay/test_recorder_effect.pysrc/omnibase_core/infrastructure/__init__.pysrc/omnibase_core/services/diff/service_diff_file_store.pysrc/omnibase_core/pipeline/replay/runner_replay_session.pysrc/omnibase_core/validation/phases/__init__.pysrc/omnibase_core/pipeline/replay/runner_replay_executor.pysrc/omnibase_core/pipeline/exceptions/__init__.pytests/unit/services/diff/test_store_diff_in_memory.pytests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use pytest markers (@pytest.mark.unit, @pytest.mark.integration, @pytest.mark.slow, @pytest.mark.smoke, @pytest.mark.performance) for test categorization
Files:
tests/unit/services/trace/__init__.pytests/unit/infrastructure/execution/test_phase_sequencer.pytests/unit/services/diff/test_store_diff_file.pytests/unit/pipeline/replay/test_executor_replay.pytests/unit/validation/test_validator_any_type.pytests/unit/validation/test_contract_validation_pipeline_events.pytests/unit/services/replay/test_service_config_override_injector.pytests/unit/api_snapshots/test_infrastructure_api_snapshot.pytests/unit/services/test_service_contract_validation_event_emitter.pytests/unit/infrastructure/test_infra_bases.pytests/unit/contracts/test_contract_diff_computer.pytests/unit/validation/phases/test_contract_validation_performance.pytests/unit/services/replay/test_service_corpus_replay_orchestrator.pytests/unit/services/diff/test_service_diff_history.pytests/integration/validation/test_contract_validation_events_e2e.pytests/unit/pipeline/handlers/test_capability_caching.pytests/unit/pipeline/replay/test_injector_rng.pytests/unit/pipeline/handlers/test_capability_metrics.pytests/unit/validation/test_visitor_any_type.pytests/unit/infrastructure/test_computation_cache.pytests/unit/validation/test_contract_validation_pipeline.pytests/unit/services/trace/test_store_trace_in_memory.pytests/unit/validation/phases/test_merge_validator.pytests/unit/infrastructure/test_load_balancer.pytests/unit/pipeline/replay/test_injector_time.pytests/integration/pipeline/replay/test_replay_e2e.pytests/unit/services/trace/test_service_trace_recording.pytests/unit/pipeline/replay/test_recorder_effect.pytests/unit/services/diff/test_store_diff_in_memory.py**/src/omnibase_core/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/src/omnibase_core/**/*.py: Follow directory-specific file naming conventions enforced by checker_naming_convention.py: cli_, container_, decorator_, enum_, error_, factory_, mixin_, model_, node_, runtime_, service_*, etc.
Ensure 100% mypy strict mode compliance (0 errors) and pyright basic mode compliance across all source files in src/omnibase_coreFiles:
src/omnibase_core/models/diff/model_diff_storage_configuration.pysrc/omnibase_core/protocols/storage/protocol_diff_store.pysrc/omnibase_core/pipeline/handlers/handler_capability_caching.pysrc/omnibase_core/cli/cli_contract.pysrc/omnibase_core/models/diff/__init__.pysrc/omnibase_core/validation/validator_contract_pipeline.pysrc/omnibase_core/pipeline/exceptions/pipeline_exception_duplicate_hook.pysrc/omnibase_core/protocols/replay/protocol_rng_service.pysrc/omnibase_core/pipeline/handlers/__init__.pysrc/omnibase_core/enums/replay/enum_recorder_mode.pysrc/omnibase_core/services/trace/service_trace_recording.pysrc/omnibase_core/models/trace_query/model_trace_summary.pysrc/omnibase_core/services/trace/__init__.pysrc/omnibase_core/protocols/replay/protocol_effect_recorder.pysrc/omnibase_core/services/sinks/__init__.pysrc/omnibase_core/services/diff/service_diff_in_memory_store.pysrc/omnibase_core/protocols/replay/protocol_time_service.pysrc/omnibase_core/models/trace_query/model_trace_query.pysrc/omnibase_core/pipeline/__init__.pysrc/omnibase_core/services/replay/service_corpus_replay_orchestrator.pysrc/omnibase_core/infrastructure/execution/__init__.pysrc/omnibase_core/tools/mypy_plugins/__init__.pysrc/omnibase_core/services/replay/service_effect_recorder.pysrc/omnibase_core/services/replay/service_config_override_injector.pysrc/omnibase_core/pipeline/exceptions/pipeline_exception_hook_timeout.pysrc/omnibase_core/pipeline/handlers/handler_capability_metrics.pysrc/omnibase_core/services/diff/service_diff_history.pysrc/omnibase_core/pipeline/exceptions/pipeline_exception_hook_registry_frozen.pysrc/omnibase_core/services/sinks/service_sink_memory.pysrc/omnibase_core/validation/validator_any_type.pysrc/omnibase_core/contracts/contract_diff_computer.pysrc/omnibase_core/services/trace/service_trace_in_memory_store.pysrc/omnibase_core/models/replay/model_effect_record.pysrc/omnibase_core/services/diff/__init__.pysrc/omnibase_core/pipeline/exceptions/pipeline_exception_unknown_dependency.pysrc/omnibase_core/pipeline/replay/__init__.pysrc/omnibase_core/protocols/storage/protocol_trace_store.pysrc/omnibase_core/pipeline/exceptions/pipeline_exception_callable_not_found.pysrc/omnibase_core/protocols/storage/__init__.pysrc/omnibase_core/services/replay/__init__.pysrc/omnibase_core/services/sinks/service_sink_file.pysrc/omnibase_core/validation/checker_visitor_any_type.pysrc/omnibase_core/models/contracts/diff/__init__.pysrc/omnibase_core/pipeline/exceptions/pipeline_exception_hook_type_mismatch.pysrc/omnibase_core/pipeline/exceptions/pipeline_exception_dependency_cycle.pysrc/omnibase_core/infrastructure/infra_bases.pysrc/omnibase_core/services/replay/service_time_injector.pysrc/omnibase_core/services/replay/service_rng_injector.pysrc/omnibase_core/validation/__init__.pysrc/omnibase_core/services/service_contract_validation_event_emitter.pysrc/omnibase_core/infrastructure/__init__.pysrc/omnibase_core/services/diff/service_diff_file_store.pysrc/omnibase_core/pipeline/replay/runner_replay_session.pysrc/omnibase_core/validation/phases/__init__.pysrc/omnibase_core/pipeline/replay/runner_replay_executor.pysrc/omnibase_core/pipeline/exceptions/__init__.py🧠 Learnings (14)
📓 Common learnings
Learnt from: CR Repo: OmniNode-ai/omnibase_core PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-01-11T18:12:47.310Z Learning: Applies to **/src/omnibase_core/**/*.py : Follow directory-specific file naming conventions enforced by checker_naming_convention.py: cli_*, container_*, decorator_*, enum_*, error_*, factory_*, mixin_*, model_*, node_*, runtime_*, service_*, etc.Learnt from: CR Repo: OmniNode-ai/omnibase_core PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-01-11T18:12:47.310Z Learning: Applies to **/src/omnibase_core/**/*.py : Ensure 100% mypy strict mode compliance (0 errors) and pyright basic mode compliance across all source files in src/omnibase_core📚 Learning: 2026-01-11T18:12:47.310Z
Learnt from: CR Repo: OmniNode-ai/omnibase_core PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-01-11T18:12:47.310Z Learning: Applies to **/nodes/node_*.py : Use NodeCompute, NodeReducer, NodeOrchestrator, and NodeEffect base classes - import from omnibase_core.nodes and always call super().__init__(container)Applied to files:
docs/guides/node-building/03_COMPUTE_NODE_TUTORIAL.mddocs/guides/node-building/04_EFFECT_NODE_TUTORIAL.mddocs/reference/SERVICE_WRAPPERS.mddocs/architecture/NODE_CLASS_HIERARCHY.mdsrc/omnibase_core/infrastructure/infra_bases.py📚 Learning: 2026-01-11T18:12:47.310Z
Learnt from: CR Repo: OmniNode-ai/omnibase_core PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-01-11T18:12:47.310Z Learning: Applies to **/*.py : Use structured error handling with ModelOnexError and EnumCoreErrorCode, never generic Exception - provide message, error_code, and contextApplied to files:
docs/guides/node-building/03_COMPUTE_NODE_TUTORIAL.mddocs/guides/node-building/04_EFFECT_NODE_TUTORIAL.md📚 Learning: 2026-01-11T18:12:47.310Z
Learnt from: CR Repo: OmniNode-ai/omnibase_core PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-01-11T18:12:47.310Z Learning: Applies to **/nodes/node_*.py : Use ModelONEXContainer for dependency injection in node __init__ methods, never ModelContainer - they are different types with different purposesApplied to files:
docs/guides/node-building/03_COMPUTE_NODE_TUTORIAL.mddocs/guides/node-building/04_EFFECT_NODE_TUTORIAL.md📚 Learning: 2026-01-11T18:12:47.310Z
Learnt from: CR Repo: OmniNode-ai/omnibase_core PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-01-11T18:12:47.310Z Learning: Applies to **/models/model_*.py : Add from_attributes=True to ConfigDict for immutable value objects nested in other Pydantic models used with pytest-xdist parallel executionApplied to files:
tests/unit/contracts/test_contract_diff_computer.py📚 Learning: 2026-01-11T18:12:47.310Z
Learnt from: CR Repo: OmniNode-ai/omnibase_core PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-01-11T18:12:47.310Z Learning: Applies to **/src/omnibase_core/**/*.py : Ensure 100% mypy strict mode compliance (0 errors) and pyright basic mode compliance across all source files in src/omnibase_coreApplied to files:
src/omnibase_core/tools/mypy_plugins/__init__.py📚 Learning: 2026-01-11T18:12:47.310Z
Learnt from: CR Repo: OmniNode-ai/omnibase_core PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-01-11T18:12:47.310Z Learning: Applies to **/src/omnibase_core/**/*.py : Follow directory-specific file naming conventions enforced by checker_naming_convention.py: cli_*, container_*, decorator_*, enum_*, error_*, factory_*, mixin_*, model_*, node_*, runtime_*, service_*, etc.Applied to files:
src/omnibase_core/validation/checker_visitor_any_type.pyscripts/validate_class_naming.pydocs/guides/HANDLER_CONVERSION_GUIDE.mdsrc/omnibase_core/validation/__init__.py📚 Learning: 2026-01-11T18:12:47.310Z
Learnt from: CR Repo: OmniNode-ai/omnibase_core PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-01-11T18:12:47.310Z Learning: Applies to **/*.py : Use PEP 604 union syntax (str | None) instead of legacy Union/Optional syntax - enforced by ruff UP007Applied to files:
src/omnibase_core/validation/checker_visitor_any_type.py📚 Learning: 2026-01-11T18:12:47.310Z
Learnt from: CR Repo: OmniNode-ai/omnibase_core PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-01-11T18:12:47.310Z Learning: Applies to **/nodes/node_*.py : Use protocol names for service resolution (container.get_service('ProtocolEventBus')) - never use concrete class namesApplied to files:
scripts/validate_class_naming.py📚 Learning: 2026-01-11T18:12:47.310Z
Learnt from: CR Repo: OmniNode-ai/omnibase_core PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-01-11T18:12:47.310Z Learning: Applies to **/nodes/node_*.py : Use duck typing with protocols for dependency resolution - never use isinstance checks, only protocol-based resolutionApplied to files:
scripts/validate_class_naming.py📚 Learning: 2026-01-11T18:12:47.310Z
Learnt from: CR Repo: OmniNode-ai/omnibase_core PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-01-11T18:12:47.310Z Learning: Applies to **/*.py : Always use Poetry for Python package management and task execution (poetry run pytest, poetry add, poetry install) - never use pip or python directlyApplied to files:
docs/guides/HANDLER_CONVERSION_GUIDE.md📚 Learning: 2026-01-11T18:12:47.310Z
Learnt from: CR Repo: OmniNode-ai/omnibase_core PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-01-11T18:12:47.310Z Learning: Applies to **/*.py : Use FileRegistry from omnibase_core.runtime.runtime_file_registry for loading YAML contracts - handle error codes: FILE_NOT_FOUND, FILE_READ_ERROR, CONFIGURATION_PARSE_ERROR, CONTRACT_VALIDATION_ERROR, DUPLICATE_REGISTRATION, DIRECTORY_NOT_FOUNDApplied to files:
src/omnibase_core/validation/__init__.py📚 Learning: 2026-01-11T18:12:47.310Z
Learnt from: CR Repo: OmniNode-ai/omnibase_core PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-01-11T18:12:47.310Z Learning: Applies to **/*.py : Use centralized exception groups from omnibase_core.errors.exception_groups (VALIDATION_ERRORS, PYDANTIC_MODEL_ERRORS, FILE_IO_ERRORS, etc.) for exception handlingApplied to files:
src/omnibase_core/pipeline/exceptions/__init__.py📚 Learning: 2026-01-11T18:12:47.310Z
Learnt from: CR Repo: OmniNode-ai/omnibase_core PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-01-11T18:12:47.310Z Learning: Applies to **/nodes/node_*.py : Use mixin composition for cross-cutting concerns: MixinDiscoveryResponder, MixinEventHandler, MixinEventListener, MixinNodeExecutor, MixinNodeLifecycle, MixinRequestResponseIntrospection, MixinWorkflowExecutionApplied to files:
docs/guides/HANDLER_CONVERSION_CHECKLIST.md🧬 Code graph analysis (38)
tests/unit/validation/test_validator_any_type.py (1)
src/omnibase_core/validation/validator_any_type.py (1)
ValidatorAnyType(75-207)tests/unit/services/replay/test_service_config_override_injector.py (1)
src/omnibase_core/services/replay/service_sentinel_missing.py (1)
_Missing(15-41)src/omnibase_core/pipeline/handlers/__init__.py (2)
src/omnibase_core/pipeline/handlers/handler_capability_caching.py (1)
HandlerCapabilityCaching(34-171)src/omnibase_core/pipeline/handlers/handler_capability_metrics.py (1)
HandlerCapabilityMetrics(27-107)src/omnibase_core/services/trace/service_trace_recording.py (1)
src/omnibase_core/protocols/storage/protocol_trace_store.py (1)
ProtocolTraceStore(38-183)src/omnibase_core/services/sinks/__init__.py (2)
src/omnibase_core/services/sinks/service_sink_file.py (1)
ServiceFileSink(31-177)src/omnibase_core/services/sinks/service_sink_memory.py (1)
ServiceMemorySink(29-122)src/omnibase_core/services/diff/service_diff_in_memory_store.py (1)
src/omnibase_core/protocols/storage/protocol_diff_store.py (1)
ProtocolDiffStore(40-216)tests/unit/services/test_service_contract_validation_event_emitter.py (5)
src/omnibase_core/services/sinks/service_sink_file.py (1)
ServiceFileSink(31-177)src/omnibase_core/services/sinks/service_sink_memory.py (1)
ServiceMemorySink(29-122)src/omnibase_core/protocols/validation/protocol_event_sink.py (3)
sink_type(168-178)is_ready(181-192)close(151-165)src/omnibase_core/enums/enum_event_sink_type.py (1)
EnumEventSinkType(35-106)src/omnibase_core/protocols/event_bus/protocol_event_message.py (1)
value(33-35)tests/unit/infrastructure/test_infra_bases.py (2)
src/omnibase_core/models/services/model_service_effect.py (1)
ModelServiceEffect(61-125)src/omnibase_core/models/services/model_service_compute.py (1)
ModelServiceCompute(106-168)src/omnibase_core/pipeline/__init__.py (1)
src/omnibase_core/services/replay/service_rng_injector.py (1)
ServiceRNGInjector(64-185)tests/unit/validation/phases/test_contract_validation_performance.py (2)
src/omnibase_core/validation/phases/validator_expanded_contract.py (1)
ExpandedContractValidator(91-609)src/omnibase_core/validation/phases/validator_merge.py (1)
MergeValidator(162-611)src/omnibase_core/services/replay/service_corpus_replay_orchestrator.py (1)
src/omnibase_core/pipeline/replay/runner_replay_executor.py (1)
ExecutorReplay(103-384)tests/unit/services/replay/test_service_corpus_replay_orchestrator.py (2)
src/omnibase_core/services/replay/service_corpus_replay_orchestrator.py (1)
replay(182-272)src/omnibase_core/pipeline/replay/runner_replay_executor.py (1)
ExecutorReplay(103-384)src/omnibase_core/tools/mypy_plugins/__init__.py (1)
src/omnibase_core/tools/mypy_plugins/tool_dict_any_checker.py (1)
DictAnyCheckerPlugin(68-779)src/omnibase_core/services/replay/service_effect_recorder.py (1)
src/omnibase_core/protocols/replay/protocol_effect_recorder.py (1)
ProtocolEffectRecorder(69-254)tests/unit/services/diff/test_service_diff_history.py (2)
src/omnibase_core/services/diff/service_diff_in_memory_store.py (1)
ServiceDiffInMemoryStore(60-300)tests/unit/services/diff/test_store_diff_in_memory.py (1)
store(50-52)src/omnibase_core/validation/validator_any_type.py (2)
src/omnibase_core/models/core/model_capability.py (1)
validation(230-232)src/omnibase_core/validation/validator_base.py (1)
ValidatorBase(108-1036)src/omnibase_core/services/trace/service_trace_in_memory_store.py (1)
src/omnibase_core/protocols/storage/protocol_trace_store.py (1)
ProtocolTraceStore(38-183)tests/unit/pipeline/handlers/test_capability_caching.py (1)
src/omnibase_core/pipeline/handlers/handler_capability_caching.py (1)
HandlerCapabilityCaching(34-171)tests/unit/pipeline/replay/test_injector_rng.py (2)
src/omnibase_core/services/replay/service_rng_injector.py (1)
ServiceRNGInjector(64-185)src/omnibase_core/protocols/replay/protocol_rng_service.py (5)
seed(110-129)ProtocolRNGService(76-194)random(131-147)randint(149-169)choice(171-194)tests/unit/pipeline/handlers/test_capability_metrics.py (1)
src/omnibase_core/pipeline/handlers/handler_capability_metrics.py (1)
HandlerCapabilityMetrics(27-107)src/omnibase_core/pipeline/replay/__init__.py (6)
src/omnibase_core/services/replay/service_corpus_replay_orchestrator.py (1)
replay(182-272)src/omnibase_core/pipeline/replay/runner_replay_executor.py (1)
ExecutorReplay(103-384)src/omnibase_core/pipeline/replay/runner_replay_session.py (1)
ReplaySession(74-135)src/omnibase_core/services/replay/service_effect_recorder.py (1)
ServiceEffectRecorder(103-483)src/omnibase_core/services/replay/service_rng_injector.py (1)
ServiceRNGInjector(64-185)src/omnibase_core/services/replay/service_time_injector.py (1)
ServiceTimeInjector(62-172)tests/unit/validation/test_visitor_any_type.py (1)
src/omnibase_core/validation/checker_visitor_any_type.py (1)
AnyTypeVisitor(65-527)tests/unit/infrastructure/test_computation_cache.py (1)
src/omnibase_core/infrastructure/infra_computation_cache.py (1)
ComputationCache(11-79)src/omnibase_core/protocols/storage/__init__.py (1)
src/omnibase_core/protocols/storage/protocol_trace_store.py (1)
ProtocolTraceStore(38-183)tests/unit/services/trace/test_store_trace_in_memory.py (5)
src/omnibase_core/protocols/storage/protocol_trace_store.py (1)
ProtocolTraceStore(38-183)src/omnibase_core/services/trace/service_trace_in_memory_store.py (1)
ServiceTraceInMemoryStore(61-306)src/omnibase_core/models/trace/model_execution_trace.py (1)
ModelExecutionTrace(32-361)src/omnibase_core/models/trace_query/model_trace_query.py (1)
ModelTraceQuery(44-228)src/omnibase_core/enums/enum_execution_status.py (1)
EnumExecutionStatus(10-158)tests/unit/infrastructure/test_load_balancer.py (1)
src/omnibase_core/infrastructure/infra_load_balancer.py (1)
LoadBalancer(17-86)tests/unit/pipeline/replay/test_injector_time.py (2)
src/omnibase_core/services/replay/service_time_injector.py (1)
ServiceTimeInjector(62-172)src/omnibase_core/protocols/replay/protocol_time_service.py (3)
now(87-104)utc_now(106-123)ProtocolTimeService(50-123)tests/integration/pipeline/replay/test_replay_e2e.py (3)
src/omnibase_core/services/replay/service_effect_recorder.py (1)
ServiceEffectRecorder(103-483)src/omnibase_core/services/replay/service_rng_injector.py (1)
ServiceRNGInjector(64-185)src/omnibase_core/services/replay/service_time_injector.py (1)
ServiceTimeInjector(62-172)src/omnibase_core/services/replay/service_rng_injector.py (1)
src/omnibase_core/protocols/replay/protocol_rng_service.py (1)
ProtocolRNGService(76-194)tests/unit/services/trace/test_service_trace_recording.py (2)
src/omnibase_core/services/trace/service_trace_in_memory_store.py (1)
ServiceTraceInMemoryStore(61-306)src/omnibase_core/services/trace/service_trace_recording.py (1)
ServiceTraceRecording(65-327)src/omnibase_core/validation/__init__.py (4)
src/omnibase_core/validation/checker_visitor_any_type.py (1)
AnyTypeVisitor(65-527)src/omnibase_core/validation/validator_contract_pipeline.py (1)
ContractValidationPipeline(111-881)src/omnibase_core/models/validation/model_expanded_contract_result.py (1)
ModelExpandedContractResult(31-91)src/omnibase_core/validation/validator_contract_linter.py (1)
ValidatorContractLinter(192-884)src/omnibase_core/services/service_contract_validation_event_emitter.py (2)
src/omnibase_core/services/sinks/service_sink_file.py (1)
ServiceFileSink(31-177)src/omnibase_core/services/sinks/service_sink_memory.py (1)
ServiceMemorySink(29-122)src/omnibase_core/services/diff/service_diff_file_store.py (1)
src/omnibase_core/protocols/storage/protocol_diff_store.py (1)
ProtocolDiffStore(40-216)src/omnibase_core/pipeline/replay/runner_replay_session.py (3)
src/omnibase_core/services/replay/service_effect_recorder.py (1)
ServiceEffectRecorder(103-483)src/omnibase_core/services/replay/service_rng_injector.py (1)
ServiceRNGInjector(64-185)src/omnibase_core/services/replay/service_time_injector.py (1)
ServiceTimeInjector(62-172)src/omnibase_core/validation/phases/__init__.py (2)
src/omnibase_core/validation/phases/validator_expanded_contract.py (1)
ExpandedContractValidator(91-609)src/omnibase_core/validation/phases/validator_expanded_contract_graph.py (1)
ExpandedContractGraphValidator(44-266)src/omnibase_core/pipeline/replay/runner_replay_executor.py (5)
src/omnibase_core/pipeline/replay/runner_replay_session.py (2)
ReplaySession(74-135)mode(124-135)src/omnibase_core/services/replay/service_effect_recorder.py (1)
ServiceEffectRecorder(103-483)src/omnibase_core/services/replay/service_rng_injector.py (1)
ServiceRNGInjector(64-185)src/omnibase_core/services/replay/service_time_injector.py (1)
ServiceTimeInjector(62-172)src/omnibase_core/enums/replay/enum_recorder_mode.py (1)
EnumRecorderMode(45-67)src/omnibase_core/pipeline/exceptions/__init__.py (3)
src/omnibase_core/errors/error_duplicate_hook.py (1)
DuplicateHookError(10-18)src/omnibase_core/errors/error_hook_timeout.py (1)
HookTimeoutError(10-21)src/omnibase_core/errors/error_hook_type_mismatch.py (1)
HookTypeMismatchError(10-25)tests/unit/services/diff/test_store_diff_in_memory.py (3)
src/omnibase_core/services/diff/service_diff_in_memory_store.py (1)
ServiceDiffInMemoryStore(60-300)src/omnibase_core/protocols/storage/protocol_diff_store.py (7)
ProtocolDiffStore(40-216)put(91-113)get(115-132)query(134-159)delete(161-178)exists(180-194)count(196-216)src/omnibase_core/models/contracts/diff/model_contract_diff.py (1)
ModelContractDiff(27-307)
…vention-violations Resolve merge conflict in services/replay/__init__.py and fix PR review issues: - Resolve merge conflict: Keep origin/main naming convention (InjectorRNG, InjectorTime, etc.) - Fix duplicate exceptions: Remove 8 pipeline_exception*.py files, re-export from errors/ - Add @pytest.mark.unit markers to test_infra_bases.py and test_load_balancer.py
Code Review - PR #377: Naming Convention RefactorSummaryThis PR performs a large-scale refactoring to enforce directory-specific file naming conventions across the codebase. The changes affect 106 files with 1,263 additions and 1,298 deletions, primarily consisting of file renames and corresponding import updates. ✅ Strengths1. Excellent Consistency & Standards Adherence
2. Comprehensive Import Updates
3. Zero Logic Changes
4. Test Coverage Maintained
5. Documentation Quality
🔍 Observations & Considerations1. Exception File ConsolidationThe
These have been consolidated into a single
2. Handler Naming Pattern ChangeFiles in
This is correct per CLAUDE.md which specifies 3. Infrastructure File NamingNotable renames in
The 4. Service File ReorganizationServices reorganized with clearer prefixes:
This makes the service role more explicit in the filename. 🎯 Code Quality AssessmentType Safety: ✅ EXCELLENT
Performance: ✅ NO IMPACT
Security: ✅ NO CONCERNS
Testing: ✅ COMPREHENSIVE
Documentation: ✅ THOROUGH
🚨 Potential Issues & Recommendations1. CI Pipeline Status
The naming convention checker should now pass with these changes. 2. Import Cache InvalidationWhen developers pull this PR, they should: # Clean Python cache files
find . -type d -name '__pycache__' -exec rm -rf {} + 2>/dev/null || true
find . -type f -name '*.pyc' -delete
# Reinstall with Poetry to rebuild import resolution
poetry installConsider adding a note in the PR description or merge commit message. 3. IDE Import CachingDevelopers using PyCharm, VSCode, or other IDEs may need to:
4. External ProjectsIf any external projects import from # OLD (will break)
from omnibase_core.infrastructure.infrastructure_bases import ModelServiceCompute
from omnibase_core.contracts.diff_computer import ContractDiffComputer
# NEW (required)
from omnibase_core.infrastructure.infra_bases import ModelServiceCompute
from omnibase_core.contracts.contract_diff_computer import ContractDiffComputerRecommendation: Coordinate with teams maintaining:
Consider creating a MIGRATION.md document listing all renamed files for downstream consumers. 📋 Pre-Merge Checklist
🎉 Final VerdictAPPROVE ✅ This is an excellent refactoring PR that:
The refactoring is clean, systematic, and well-executed. The only concern is ensuring downstream projects are notified of the breaking import changes. Quality Score: 9/10
Great work on maintaining code quality and consistency! 🚀 |
Update test_service_replay_safety_enforcer.py to use renamed modules and classes: - injector_time -> service_time_injector - injector_rng -> service_rng_injector - InjectorTime -> ServiceTimeInjector - InjectorRNG -> ServiceRNGInjector
Code Review: File Naming Convention Refactor (OMN-1298)SummaryThis PR performs a comprehensive refactoring to enforce directory-specific naming conventions across the codebase. The changes rename 106 files and update all imports/re-exports accordingly. ✅ Strengths
🔍 Areas of Concern1. Handler Naming Convention Change (Medium Priority)The PR renames
Impact: This creates a disconnect between documentation and implementation. The documentation extensively references Recommendation: Either update CLAUDE.md's "Common Pitfalls" section and Handler Conversion Guide to reflect 2. Test File Naming (Low Priority)Test files follow the new conventions (e.g.,
The diff shows "+39/-35" which suggests minor content changes. Verify these are intentional. 3. Validation Module Complexity (Observation)The
Not a blocker, but worth noting for future refactoring. 🧪 Testing Considerations
📊 Code Quality
🔐 Security AssessmentNo security concerns. This is a pure refactoring with no logic changes. 📝 Recommendations
✅ VerdictAPPROVE with minor documentation updates recommended. This is a well-executed refactoring that improves naming consistency across the codebase. The systematic approach and comprehensive import updates demonstrate careful attention to detail. The only concern is the documentation mismatch, which should be addressed before or immediately after merge. Estimated Risk: Low Great work on maintaining consistency across such a large codebase! 🎉 |
- Add types-psutil dev dependency for proper type checking - Add jsonpath_ng to mypy ignore list (no stubs available)
Pull Request Review: File and Class Naming Convention Fixes (OMN-1298)SummaryThis PR addresses naming convention violations across the codebase by renaming files and classes to follow directory-specific prefix rules. This is a large refactoring PR with 108 files changed (+1292/-1304 lines), primarily focused on enforcing consistent naming patterns. ✅ Strengths1. Excellent Scope and Consistency
2. Follows CLAUDE.md Conventions
3. Class Name Fixes (Critical)
4. Test Coverage
5. Dependencies Updated
|
- Update HANDLER_CONVERSION_CHECKLIST.md grep pattern to use HandlerCapability - Fix HANDLER_CONVERSION_GUIDE.md documentation inconsistency (Model vs Handler prefix) - Rename test_phase_sequencer.py to test_infra_phase_sequencer.py for consistency
PR Review: File and Class Naming Convention Refactor (OMN-1298)SummaryThis PR performs a large-scale refactoring to enforce directory-specific naming conventions across the omnibase_core codebase. The changes are mechanically correct and align with the project standards. Overall, this is a well-executed refactoring with comprehensive test and documentation updates. Strengths
Issues Found1. Documentation InconsistenciesLocation: docs/guides/HANDLER_CONVERSION_GUIDE.md:340-353 The documentation has conflicting guidance - states handlers MUST use Model prefix but examples show HandlerCapabilityMetrics with Handler prefix. Recommendation: Update section 4 to align with the actual Handler naming convention. Location: docs/guides/HANDLER_CONVERSION_CHECKLIST.md:226-227 The grep validation command searches for ModelCapability but should search for HandlerCapability. 2. Missing pytest markers (Minor)Location: tests/unit/infrastructure/test_infra_bases.py Some test functions are missing @pytest.mark.unit markers per project convention. Code Quality Assessment
RecommendationAPPROVE with minor documentation fixes The refactoring is mechanically sound and achieves its goal of enforcing naming conventions. Documentation inconsistencies should be corrected before merge, but they do not affect functional correctness. Review conducted following CLAUDE.md guidelines. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/guides/HANDLER_CONVERSION_GUIDE.md (1)
313-315: Fix inconsistent import path.The import statement references the old filename
model_capability_examplebut should use the new naming conventionhandler_capability_exampleto align with the refactored file structure.📝 Proposed fix
-from omnibase_core.pipeline.handlers.model_capability_example import ( +from omnibase_core.pipeline.handlers.handler_capability_example import ( HandlerCapabilityExample, )
🧹 Nitpick comments (1)
docs/guides/HANDLER_CONVERSION_GUIDE.md (1)
704-709: Consider aligning pattern description with earlier definition.Line 704 uses the pattern
Handler<Capability>while line 342 earlier specifies the more precise patternHandler<Type><Name>. The table correctly showsHandlerCapability<Name>. For consistency, consider using the same pattern format throughout.♻️ Suggested refinement
-All handler classes MUST follow the pattern `Handler<Capability>`: +All handler classes MUST follow the pattern `HandlerCapability<Name>`:
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/guides/HANDLER_CONVERSION_CHECKLIST.mddocs/guides/HANDLER_CONVERSION_GUIDE.mdtests/unit/infrastructure/execution/test_infra_phase_sequencer.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Always use Poetry for Python package management and task execution (poetry run pytest, poetry add, poetry install) - never use pip or python directly
Use structured error handling with ModelOnexError and EnumCoreErrorCode, never generic Exception - provide message, error_code, and context
Use @standard_error_handling decorator to eliminate try/catch boilerplate in async operations
Use standardized exception comment markers (# fallback-ok:, # catch-all-ok:, # cleanup-resilience-ok:, # boundary-ok:, # init-errors-ok:, # tool-resilience-ok:) when using catch-all or broad exception handlers to document intent
Order exception tuples alphabetically by exception class name when catching multiple exceptions
Use centralized exception groups from omnibase_core.errors.exception_groups (VALIDATION_ERRORS, PYDANTIC_MODEL_ERRORS, FILE_IO_ERRORS, etc.) for exception handling
Use PEP 604 union syntax (str | None) instead of legacy Union/Optional syntax - enforced by ruff UP007
Use FileRegistry from omnibase_core.runtime.runtime_file_registry for loading YAML contracts - handle error codes: FILE_NOT_FOUND, FILE_READ_ERROR, CONFIGURATION_PARSE_ERROR, CONTRACT_VALIDATION_ERROR, DUPLICATE_REGISTRATION, DIRECTORY_NOT_FOUND
Maintain minimum 60% test coverage requirement across the codebase measured with pytest-cov
Files:
tests/unit/infrastructure/execution/test_infra_phase_sequencer.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use pytest markers (@pytest.mark.unit, @pytest.mark.integration, @pytest.mark.slow, @pytest.mark.smoke, @pytest.mark.performance) for test categorization
Files:
tests/unit/infrastructure/execution/test_infra_phase_sequencer.py
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: OmniNode-ai/omnibase_core PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T18:12:47.310Z
Learning: Applies to **/src/omnibase_core/**/*.py : Follow directory-specific file naming conventions enforced by checker_naming_convention.py: cli_*, container_*, decorator_*, enum_*, error_*, factory_*, mixin_*, model_*, node_*, runtime_*, service_*, etc.
Learnt from: CR
Repo: OmniNode-ai/omnibase_core PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T18:12:47.310Z
Learning: Applies to **/src/omnibase_core/**/*.py : Ensure 100% mypy strict mode compliance (0 errors) and pyright basic mode compliance across all source files in src/omnibase_core
📚 Learning: 2026-01-11T18:12:47.310Z
Learnt from: CR
Repo: OmniNode-ai/omnibase_core PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T18:12:47.310Z
Learning: Applies to **/*.py : Always use Poetry for Python package management and task execution (poetry run pytest, poetry add, poetry install) - never use pip or python directly
Applied to files:
docs/guides/HANDLER_CONVERSION_GUIDE.md
📚 Learning: 2026-01-11T18:12:47.310Z
Learnt from: CR
Repo: OmniNode-ai/omnibase_core PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T18:12:47.310Z
Learning: Applies to **/src/omnibase_core/**/*.py : Follow directory-specific file naming conventions enforced by checker_naming_convention.py: cli_*, container_*, decorator_*, enum_*, error_*, factory_*, mixin_*, model_*, node_*, runtime_*, service_*, etc.
Applied to files:
docs/guides/HANDLER_CONVERSION_GUIDE.md
📚 Learning: 2026-01-11T18:12:47.310Z
Learnt from: CR
Repo: OmniNode-ai/omnibase_core PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-11T18:12:47.310Z
Learning: Applies to **/nodes/node_*.py : Use mixin composition for cross-cutting concerns: MixinDiscoveryResponder, MixinEventHandler, MixinEventListener, MixinNodeExecutor, MixinNodeLifecycle, MixinRequestResponseIntrospection, MixinWorkflowExecution
Applied to files:
docs/guides/HANDLER_CONVERSION_CHECKLIST.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: Tests (Split 20/20)
- GitHub Check: Tests (Split 14/20)
- GitHub Check: Tests (Split 19/20)
- GitHub Check: Tests (Split 8/20)
- GitHub Check: Tests (Split 13/20)
- GitHub Check: Tests (Split 10/20)
- GitHub Check: Tests (Split 16/20)
- GitHub Check: Tests (Split 15/20)
- GitHub Check: Tests (Split 17/20)
- GitHub Check: Tests (Split 12/20)
- GitHub Check: Tests (Split 3/20)
- GitHub Check: Tests (Split 18/20)
- GitHub Check: Tests (Split 9/20)
- GitHub Check: Tests (Split 7/20)
- GitHub Check: Tests (Split 6/20)
- GitHub Check: Tests (Split 1/20)
- GitHub Check: Tests (Split 5/20)
- GitHub Check: Tests (Split 2/20)
- GitHub Check: claude-review
🔇 Additional comments (5)
tests/unit/infrastructure/execution/test_infra_phase_sequencer.py (3)
4-21: LGTM!The docstring correctly reflects the renamed module
infra_phase_sequencer, and the comprehensive test category documentation is well-maintained.
29-39: LGTM!The import path update correctly reflects the module rename from
phase_sequencertoinfra_phase_sequencer, consistent with the directory-specific naming convention (infra_*prefix for infrastructure modules). Based on learnings, this aligns with the naming conventions enforced bychecker_naming_convention.py.
57-91: Well-structured test suite.All test classes properly use the
@pytest.mark.unitmarker as per coding guidelines. The tests comprehensively cover the phase sequencer's public API including determinism verification and error handling withModelOnexError.docs/guides/HANDLER_CONVERSION_CHECKLIST.md (2)
44-69: LGTM! Naming convention updates are consistent.The checklist has been properly updated to reflect the
HandlerCapability*naming convention throughout. Table entries, file paths, and section headers all align with the refactored naming pattern.
215-227: Validation commands correctly updated.The grep commands have been properly updated to verify the absence of old naming patterns and presence of new
HandlerCapability*exports. These commands will help ensure naming consistency across the codebase.
jonahgabriel
deleted the
jonah/omn-1298-fix-file-and-class-naming-convention-violations
branch
January 12, 2026 23:44
jonahgabriel
added a commit
that referenced
this pull request
Apr 25, 2026
andywu42
pushed a commit
to andywu42/omnibase_core
that referenced
this pull request
Jun 10, 2026
….venv (OmniNode-ai#907) * fix(OMN-9537): add source_only flag to compliance scanner to exclude .venv Without this flag, onex compliance check scans all .venv installed packages producing ~1258 false-positive handler_resolution failures (94% noise). The source_only=True kwarg filters out any contract.yaml whose path contains a .venv component, reducing to source-only contracts. Default behaviour (source_only=False) is unchanged for backwards compatibility. Two unit tests added covering the exclude and include cases. Closes OMN-9537 * fix(OMN-9537): prune venv dirs during walk instead of post-filtering Replace rglob+filter with os.walk that prunes venv dirs in-place before descending. This avoids traversing tens of thousands of .venv files entirely, fixing the I/O performance issue flagged in hostile review. Also extends the exclusion set from only .venv to .venv, venv, env, .env to cover all common virtualenv naming conventions. Adds a third test case covering the bare 'venv' (no dot) case. * test(OMN-9537): add no-arg default regression assertion for backwards-compat Addresses CodeRabbit finding: verify that scan() with no args includes .venv contracts (default source_only=False preserved). * contract(OMN-9537): add ticket contract with deploy evidence for compliance-scanner source_only Closes OMN-9537. * fix(OMN-9537): replace docker exec deploy-gate check with echo pattern Mirrors OMN-9624 precedent — echo with deploy keyword satisfies the deploy-gate text scan without requiring a live container in CI. Closes OMN-9537. * chore(OMN-9537): re-trigger CI after OCC OmniNode-ai#377 merged
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.
Rename files to follow directory-specific naming prefixes:
Update all imports and re-exports across source and test files.
Summary by CodeRabbit
Release Notes
Documentation
API Changes
HandlerCapabilityMetrics,HandlerCapabilityCaching).ServiceTimeInjector,ServiceRNGInjector,ServiceEffectRecorder).Service*prefix (e.g.,ServiceDiffInMemoryStore,ServiceTraceInMemoryStore).Dependencies
types-psutildevelopment dependency.✏️ Tip: You can customize this high-level summary in your review settings.