Repository navigation
Add validation framework with omnibase_core integration - #21
jonahgabriel merged 6 commits into
Conversation
- Add validation module reusing validators from omnibase_core: - Architecture validator (one-model-per-file) - Contracts validator (YAML contracts) - Patterns validator (naming conventions) - Union usage validator - Circular import validator - Add CLI validation commands (omni-infra validate) - Add pre-commit hooks matching omnibase_core configuration - Update pyproject.toml with consistent linting/formatting settings - Add standalone validation script (scripts/validate.py) - Copy .gitignore from omnibase_core
WalkthroughAdds infra validation tooling: repo ignores and pre-commit hooks, tooling and dependency updates, validation orchestration script, Click+Rich CLI, infra-specific validator wrappers re-exporting core validators, documentation, tests, and a GitHub Actions workflow. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as CLI (Click/Rich)
participant Infra as Infra Validators\n(src/omnibase_infra/validation)
participant Core as Core Validators\n(omnibase_core.validation)
participant Script as Validate Script\n(scripts/validate.py)
participant Result as Validation Result
User->>CLI: run validate <type> [options]
CLI->>Infra: call validate_infra_<type>(directory, options)
Infra->>Core: delegate to core validate_<type>(...)
Core->>Result: perform checks and return result object
Result-->>Infra: return result
Infra-->>CLI: return result or aggregated summary
CLI->>User: print styled output and exit status
Note right of CLI: scripts/validate.py can be invoked directly\nand follows similar delegation to core via infra wrappers
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Poem
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (11)
CLAUDE.md (1)
1-670: Documentation file requires validation framework integration context.This comprehensive CLAUDE.md documentation updates infrastructure patterns and migration planning but doesn't reference the new validation framework being introduced in this PR (validation module, CLI commands, pre-commit hooks, validation scripts).
Consider adding a section documenting:
- New validation CLI commands (
omni-infra validate architecture|contracts|patterns|unions|imports|all)- Pre-commit validation hooks integration
- Validation script usage (
scripts/validate.py --verbose/--quick)- How these fit into the agent-driven development workflow
This ensures developers understand the new validation tooling available to them.
.gitignore (2)
222-222: Refine the environment file ignore pattern to preserve.env.example.Line 222's
.env.*pattern is overly broad and will exclude.env.example, which should be committed as a template per environment variable standards. Based on learnings, use a more restrictive pattern.- .env.* + .env.local + .env.*.localAlternatively, if you want to be more explicit:
- .env.* + .env.production + .env.staging + .env.development.localThis ensures
.env.exampleremains committed as a template while excluding local overrides.
52-56: Consolidate duplicated ignore patterns across sections.The file contains significant duplication of ignore patterns, which reduces maintainability:
.onex_cache/appears at lines 53 and 245.envappears at lines 135 and 221venv/,.venv/,env/,ENV/,env.bak/duplicated at lines 136-141 vs 209-212reports/duplicated at lines 200 and 234.cursor/duplicated at lines 238 and 251Consider reorganizing into logical sections without redundancy.
Also applies to: 135-141, 209-212, 221-222, 234-235, 238-239, 245-245, 251-251
pyproject.toml (2)
164-175: Consider removing security-sensitive rule ignores.Several ignored rules have security implications that warrant attention:
S301(pickle usage) - deserialization vulnerabilitiesS311(non-cryptographic random) - weak randomness in security contextsS324(insecure hash functions) - weak cryptographyS105(hardcoded passwords) - credential exposureWhile labeled as "technical debt," these rules help catch genuine security issues in new code. Consider enabling them and fixing violations incrementally, or at minimum documenting which existing code requires these exceptions.
226-231: Track removal of globalignore_missing_imports.The global
ignore_missing_imports = trueis pragmatic for now, but it can mask legitimate type errors from third-party packages that do have type stubs. Onceomnibase_coreaddspy.typed, consider:
- Removing the global setting
- Adding specific overrides only for packages without stubs
.pre-commit-config.yaml (2)
80-126: Consider incremental validation for performance.All ONEX validation hooks use
pass_filenames: false, running full codebase validation on every commit. This is fine for the current codebase size, but as the project grows, consider:
- Using
pass_filenames: truewith validators that support file-specific checks- Implementing caching in validators to skip unchanged files
133-143: CI validation gap is documented but worth tracking.Skipping all ONEX validation hooks in CI means validation failures are only caught locally. While this is necessary if
omnibase_coreisn't available in CI, consider adding a CI job that installs dependencies and runsscripts/validate.py allto provide a safety net.scripts/validate.py (2)
85-88: Consider makingmax_unionsconfigurable.The hardcoded
max_unions=20is documented but may need adjustment as the codebase evolves. Consider:
- Moving to a configuration file
- Accepting as a CLI argument
This is a minor improvement for maintainability.
165-170: Variable shadowing reduces readability.The variable
passedis used both as the success count (line 161) and as the loop variable (line 169), which shadows the earlier meaning.if all(results.values()): print("All validations PASSED") return True else: - failed = [name for name, passed in results.items() if not passed] + failed = [name for name, success in results.items() if not success] print(f"FAILED: {', '.join(failed)}") return Falsesrc/omnibase_infra/validation/infra_validators.py (2)
11-17: Clarify type aliasing for result types.Lines 12-14 alias
ModelValidationResultfrommodel_import_validation_resultasCircularImportValidationResultto distinguish it from the standardModelValidationResult(line 11). While functionally correct, this aliasing is subtle and could confuse maintainers.Consider adding a clarifying comment:
+# Import validation uses a specialized result type distinct from standard validation from omnibase_core.models.model_import_validation_result import ( ModelValidationResult as CircularImportValidationResult, )
189-225: Consider duck typing over isinstance for result type checking.Line 206 uses
isinstanceto distinguishCircularImportValidationResultfrom standardModelValidationResult. While functional, checking for thehas_circular_importsattribute (duck typing) would be more robust and explicit about the distinction.Consider this more explicit approach:
for name, result in results.items(): - if isinstance(result, CircularImportValidationResult): + if hasattr(result, "has_circular_imports"): # Circular import validator uses has_circular_imports if not result.has_circular_imports:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (10)
.gitignore(1 hunks).pre-commit-config.yaml(1 hunks)CLAUDE.md(4 hunks)pyproject.toml(3 hunks)scripts/validate.py(1 hunks)src/omnibase_infra/cli/__init__.py(1 hunks)src/omnibase_infra/cli/commands.py(1 hunks)src/omnibase_infra/validation/__init__.py(1 hunks)src/omnibase_infra/validation/infra_validators.py(1 hunks)tests/conftest.py(1 hunks)
🧰 Additional context used
🧠 Learnings (47)
📓 Common learnings
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/canonical_patterns.mdc:0-0
Timestamp: 2025-11-28T18:58:53.781Z
Learning: All code must pass pre-commit validation hooks including string version detection, backward compatibility checks, fallback pattern removal, single class per file, error raising validation, Pydantic pattern validation, union usage validation, and enum/model import prevention
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/canonical_patterns.mdc:0-0
Timestamp: 2025-11-28T18:58:53.781Z
Learning: Organize models under `src/omnibase_core/models/` by domain including: base, cli, common, config, core, contracts, discovery, health, infrastructure, logging, metadata, nodes, operations, results, security, service, tools, validation, and workflows
Learnt from: CR
Repo: OmniNode-ai/omnibase_core PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T21:48:22.867Z
Learning: Applies to src/omnibase_core/models/**/*.py : Use Pydantic models for data validation and serialization - leverage Pydantic 2.11+ features for type safety
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/canonical_patterns.mdc:0-0
Timestamp: 2025-11-24T17:22:32.195Z
Learning: Applies to **/v[0-9]_[0-9]_[0-9]/{tools,registry}/**.py : Constructor-based dependency injection must be used for all tool and node classes; validate that required dependencies are not None, raising OnexError with specific error code if missing
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/node_standards.mdc:0-0
Timestamp: 2025-11-24T17:23:49.777Z
Learning: All ONEX nodes must conform to the canonical structure, code generation, and interface patterns established in the `node_cli` node, using it as the primary source of truth for directory structure, contract schema patterns, linked document architecture, base state patterns, shared schema references, extensibility patterns, CLI interface declarations, code generation, dependency injection, error handling, testing, and documentation
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/pr.mdc:0-0
Timestamp: 2025-11-24T17:24:10.209Z
Learning: Applies to **/*.py : Automatically perform code standards checks on all Python files in the PR, including: Any/Any imports and Dict[str, Any] usage violations, naming convention violations (tool_, model_, enum_ prefixes), anti-pattern detection (direct tool instantiation, telescoping constructors), and type safety enforcement (strongest typing possible). Fix all violations immediately with proper commit messages.
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/standards.mdc:0-0
Timestamp: 2025-11-24T17:24:41.687Z
Learning: CI checks must validate naming compliance for all ONEX standards (prefixes, file locations, class names)
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-24T16:33:51.604Z
Learning: All Python code must have comprehensive test coverage following ONEX Core testing patterns with tests organized by domain, using proper fixtures, and achieving high coverage while maintaining code quality
📚 Learning: 2025-11-28T18:58:53.781Z
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/canonical_patterns.mdc:0-0
Timestamp: 2025-11-28T18:58:53.781Z
Learning: All code must pass pre-commit validation hooks including string version detection, backward compatibility checks, fallback pattern removal, single class per file, error raising validation, Pydantic pattern validation, union usage validation, and enum/model import prevention
Applied to files:
.pre-commit-config.yaml
📚 Learning: 2025-11-24T17:24:10.209Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/pr.mdc:0-0
Timestamp: 2025-11-24T17:24:10.209Z
Learning: Applies to **/*.py : Automatically perform code standards checks on all Python files in the PR, including: Any/Any imports and Dict[str, Any] usage violations, naming convention violations (tool_, model_, enum_ prefixes), anti-pattern detection (direct tool instantiation, telescoping constructors), and type safety enforcement (strongest typing possible). Fix all violations immediately with proper commit messages.
Applied to files:
.pre-commit-config.yamlpyproject.toml
📚 Learning: 2025-11-24T16:32:55.606Z
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/node_standards.mdc:0-0
Timestamp: 2025-11-24T16:32:55.606Z
Learning: Applies to **/*contract*.yaml : All ONEX nodes must have validated YAML contracts following the contract-driven development pattern with input_state and output_state schema definitions
Applied to files:
.pre-commit-config.yamlCLAUDE.md
📚 Learning: 2025-11-25T21:48:22.867Z
Learnt from: CR
Repo: OmniNode-ai/omnibase_core PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T21:48:22.867Z
Learning: Install and run pre-commit hooks before pushing code to catch formatting and type checking issues early
Applied to files:
.pre-commit-config.yaml
📚 Learning: 2025-11-24T17:23:49.777Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/node_standards.mdc:0-0
Timestamp: 2025-11-24T17:23:49.777Z
Learning: Applies to **/node_*/v[0-9]*_[0-9]*_[0-9]*/contracts/contract_*.yaml : All ONEX node contract definitions must use the new subcontract architecture pattern, breaking down complex contracts into separate contract_actions.yaml, contract_models.yaml, contract_validation.yaml, and optional contract_cli.yaml and contract_capabilities.yaml files for separation of concerns, maintainability, reusability, modularity, and future tool-as-a-service readiness
Applied to files:
.pre-commit-config.yamlscripts/validate.pyCLAUDE.md
📚 Learning: 2025-11-25T21:48:22.867Z
Learnt from: CR
Repo: OmniNode-ai/omnibase_core PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T21:48:22.867Z
Learning: Applies to **/*.py : Run code through black formatter, isort for import sorting, and ruff linter before committing
Applied to files:
.pre-commit-config.yamlpyproject.toml
📚 Learning: 2025-11-29T22:07:25.230Z
Learnt from: CR
Repo: OmniNode-ai/omniintelligence PR: 0
File: migration_sources/omniarchon/CLAUDE.md:0-0
Timestamp: 2025-11-29T22:07:25.230Z
Learning: Applies to migration_sources/omniarchon/**/.env* : Never commit `.env` files to version control. Use `.env.example` as template. All configuration must be manageable via environment variables with sensible defaults. Document all required variables with their purposes.
Applied to files:
.gitignore
📚 Learning: 2025-11-24T17:23:49.777Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/node_standards.mdc:0-0
Timestamp: 2025-11-24T17:23:49.777Z
Learning: All ONEX nodes must conform to the canonical structure, code generation, and interface patterns established in the `node_cli` node, using it as the primary source of truth for directory structure, contract schema patterns, linked document architecture, base state patterns, shared schema references, extensibility patterns, CLI interface declarations, code generation, dependency injection, error handling, testing, and documentation
Applied to files:
src/omnibase_infra/cli/__init__.pysrc/omnibase_infra/cli/commands.py
📚 Learning: 2025-11-24T17:22:32.195Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/canonical_patterns.mdc:0-0
Timestamp: 2025-11-24T17:22:32.195Z
Learning: Applies to **/cli_tools/*/v[0-9]_[0-9]_[0-9]/commands/*.py : CLI commands must use Click framework with error handling: catch OnexError and exit with status code 1
Applied to files:
src/omnibase_infra/cli/__init__.pysrc/omnibase_infra/cli/commands.py
📚 Learning: 2025-11-29T17:13:38.776Z
Learnt from: CR
Repo: OmniNode-ai/omniarchon PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-29T17:13:38.776Z
Learning: Applies to scripts/verify_environment.py : Run `python3 scripts/verify_environment.py --verbose` before deployment to validate 9 critical environment checks.
Applied to files:
scripts/validate.py
📚 Learning: 2025-11-28T18:58:53.781Z
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/canonical_patterns.mdc:0-0
Timestamp: 2025-11-28T18:58:53.781Z
Learning: Organize models under `src/omnibase_core/models/` by domain including: base, cli, common, config, core, contracts, discovery, health, infrastructure, logging, metadata, nodes, operations, results, security, service, tools, validation, and workflows
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-24T17:22:32.195Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/canonical_patterns.mdc:0-0
Timestamp: 2025-11-24T17:22:32.195Z
Learning: Follow canonical patterns from reference implementations: use node_cli/v1_0_0/ as primary reference and node_kafka_event_bus/v1_0_0/ for complex backend patterns
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-29T22:07:25.230Z
Learnt from: CR
Repo: OmniNode-ai/omniintelligence PR: 0
File: migration_sources/omniarchon/CLAUDE.md:0-0
Timestamp: 2025-11-29T22:07:25.230Z
Learning: Applies to migration_sources/omniarchon/**/*.py : For Redpanda/Kafka connection patterns: Docker services use `omninode-bridge-redpanda:9092` (DNS resolves via /etc/hosts to 192.168.86.200:9092), host scripts use `192.168.86.200:29092` (direct IP with external port), remote server access uses `localhost:29092`. Never mix these contexts.
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-29T17:13:38.776Z
Learnt from: CR
Repo: OmniNode-ai/omniarchon PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-29T17:13:38.776Z
Learning: Use event-driven architecture with Kafka topics for asynchronous processing: enrichment, code analysis, manifest processing, and entity embedding pipelines.
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-30T21:55:10.284Z
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-30T21:55:10.284Z
Learning: Applies to src/omninode_bridge/nodes/**/contract.yaml : All contract YAML files for ONEX v2.0 nodes MUST define subcontract references, input/output models, and FSM configurations. Use YAML 1.2 syntax.
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-24T17:23:49.777Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/node_standards.mdc:0-0
Timestamp: 2025-11-24T17:23:49.777Z
Learning: Applies to **/node_*/v[0-9]*_[0-9]*_[0-9]*/contract.yaml : All ONEX node contract definitions must follow the linked document architecture pattern with contract.yaml linking to node_config.yaml and deployment_config.yaml as associated documents
Applied to files:
CLAUDE.md
📚 Learning: 2025-12-03T03:23:43.633Z
Learnt from: CR
Repo: OmniNode-ai/omniintelligence PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T03:23:43.633Z
Learning: Refer to legacy source code in `migration_sources/omniarchon/` for reference during migration work, including `MIGRATION_SUMMARY.md`, `OMNIARCHON_MIGRATION_INVENTORY.md`, and `QUICK_REFERENCE.md`
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-24T17:23:49.777Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/node_standards.mdc:0-0
Timestamp: 2025-11-24T17:23:49.777Z
Learning: Applies to **/node_*/v[0-9]*_[0-9]*_[0-9]*/models/model_contract_*.py : All ONEX node auto-generated Pydantic models must be organized in a `models/` directory with files for state.py, model_contract_actions.py, model_contract_models.py, model_contract_validation.py, model_contract_cli.py (optional), model_contract_capabilities.py (optional), and error_codes.py, generated from the corresponding contract definitions
Applied to files:
CLAUDE.md
📚 Learning: 2025-12-03T03:23:43.633Z
Learnt from: CR
Repo: OmniNode-ai/omniintelligence PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T03:23:43.633Z
Learning: Applies to nodes/**/v*/ : Each node follows a versioned canonical structure with directories for contracts (YAML definitions), models (Pydantic models), node.py (main implementation), introspection.py (introspection support), scenarios (integration test scenarios), and node_tests (node-specific tests)
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-24T17:23:49.777Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/node_standards.mdc:0-0
Timestamp: 2025-11-24T17:23:49.777Z
Learning: Applies to **/node_*/v[0-9]*_[0-9]*_[0-9]*/contracts/contract_cli.yaml : All ONEX node CLI interface definitions, if applicable, must be included in contract_cli.yaml with entrypoint and commands specifications
Applied to files:
CLAUDE.mdsrc/omnibase_infra/cli/commands.py
📚 Learning: 2025-11-24T17:24:41.687Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/standards.mdc:0-0
Timestamp: 2025-11-24T17:24:41.687Z
Learning: Applies to **/v[0-9]_[0-9]_[0-9]/contract.yaml : Main contract files must be named `contract.yaml` and serve as the interface definition (source of truth) for the node
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-24T17:22:32.195Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/canonical_patterns.mdc:0-0
Timestamp: 2025-11-24T17:22:32.195Z
Learning: Applies to **/v[0-9]_[0-9]_[0-9]/contracts/ : Organize contract subcomponents into separate files (contract_actions.yaml, contract_models.yaml, contract_validation.yaml, etc.) and reference them from main contract.yaml
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-25T21:48:22.867Z
Learnt from: CR
Repo: OmniNode-ai/omnibase_core PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T21:48:22.867Z
Learning: Applies to src/omnibase_core/**/*.py : Use container.get_service('ProtocolName') for dependency resolution by protocol name, never by concrete class name
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-24T16:33:32.747Z
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/standards.mdc:0-0
Timestamp: 2025-11-24T16:33:32.747Z
Learning: Use Protocol for interface definitions when implementations may live outside core codebase; use Pydantic models only for base classes with shared logic
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-24T17:24:41.687Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/standards.mdc:0-0
Timestamp: 2025-11-24T17:24:41.687Z
Learning: Applies to **/*.py : Import protocols from `omnibase.protocol.protocol_<name>` module paths
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-24T17:24:41.687Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/standards.mdc:0-0
Timestamp: 2025-11-24T17:24:41.687Z
Learning: Applies to **/protocols/protocol_*.py : Protocol class names must follow the pattern `Protocol<Name>` (e.g., `ProtocolFileGenerator`)
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-24T16:33:32.747Z
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/standards.mdc:0-0
Timestamp: 2025-11-24T16:33:32.747Z
Learning: Applies to **/protocols/protocol_*.py : Protocol class names must follow the pattern `Protocol<Name>` using PascalCase
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-24T17:24:41.687Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/standards.mdc:0-0
Timestamp: 2025-11-24T17:24:41.687Z
Learning: Applies to **/protocols/protocol_*.py : Use Protocol for tool interfaces and plugin APIs based on method shape (structural typing), not Pydantic models with inheritance
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-24T16:33:32.747Z
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/standards.mdc:0-0
Timestamp: 2025-11-24T16:33:32.747Z
Learning: Applies to **/*.py : Import protocols from `omnibase.protocol.protocol_*` paths
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-24T16:33:32.747Z
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/standards.mdc:0-0
Timestamp: 2025-11-24T16:33:32.747Z
Learning: Applies to **/protocols/protocol_*.py : Protocol files must follow the naming pattern `protocol_<name>.py` and be located in `*/protocols/` directories
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-24T16:33:32.747Z
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/standards.mdc:0-0
Timestamp: 2025-11-24T16:33:32.747Z
Learning: Applies to **/*.py : Import models from shared core paths using `omnibase.model.core.model_*` pattern
Applied to files:
CLAUDE.md
📚 Learning: 2025-10-14T12:06:38.965Z
Learnt from: jonahgabriel
Repo: OmniNode-ai/omninode_bridge PR: 0
File: :0-0
Timestamp: 2025-10-14T12:06:38.965Z
Learning: In pyproject.toml for OmniNode Bridge: Dev dependencies are pytest ^8.4.0, pytest-asyncio ^0.25.0, mypy ^1.13.0, black ^24.10.0, and ruff ^0.8.0, all compatible with Python 3.12.
Applied to files:
pyproject.toml
📚 Learning: 2025-10-14T12:06:38.965Z
Learnt from: jonahgabriel
Repo: OmniNode-ai/omninode_bridge PR: 0
File: :0-0
Timestamp: 2025-10-14T12:06:38.965Z
Learning: In pyproject.toml for OmniNode Bridge: Core dependencies are pydantic ^2.11.7, fastapi ^0.115.0, uvicorn ^0.32.0, asyncpg ^0.29.0, and redis ^6.0.0 (for Redis/Valkey compatibility).
Applied to files:
pyproject.toml
📚 Learning: 2025-10-14T12:06:38.965Z
Learnt from: jonahgabriel
Repo: OmniNode-ai/omninode_bridge PR: 0
File: :0-0
Timestamp: 2025-10-14T12:06:38.965Z
Learning: OmniNode Bridge project uses Python ^3.12 as the baseline version, upgraded from 3.11 for performance improvements, enhanced type system, and better asyncio support.
Applied to files:
pyproject.toml
📚 Learning: 2025-11-24T17:23:49.777Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/node_standards.mdc:0-0
Timestamp: 2025-11-24T17:23:49.777Z
Learning: Applies to **/node_*/v[0-9]*_[0-9]*_[0-9]*/*.py : All ONEX node implementations must follow dependency injection and protocol-first design patterns as established in the node_cli canonical reference
Applied to files:
pyproject.toml
📚 Learning: 2025-11-24T17:22:32.195Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/canonical_patterns.mdc:0-0
Timestamp: 2025-11-24T17:22:32.195Z
Learning: Applies to **/v[0-9]_[0-9]_[0-9]/{tools,registry}/**.py : Constructor-based dependency injection must be used for all tool and node classes; validate that required dependencies are not None, raising OnexError with specific error code if missing
Applied to files:
pyproject.toml
📚 Learning: 2025-11-25T21:48:22.867Z
Learnt from: CR
Repo: OmniNode-ai/omnibase_core PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T21:48:22.867Z
Learning: Applies to **/*.py : Always use 100% strict mypy type checking - all functions must have type annotations and no untyped definitions are allowed
Applied to files:
pyproject.toml
📚 Learning: 2025-12-03T03:23:43.633Z
Learnt from: CR
Repo: OmniNode-ai/omniintelligence PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T03:23:43.633Z
Learning: Applies to src/**/*.py : Run code quality checks using `ruff check src tests`, `ruff check --fix src tests` for auto-fix, `black src tests` for formatting, `isort src tests` for import sorting, and `mypy src` for type checking
Applied to files:
pyproject.toml
📚 Learning: 2025-12-03T03:23:43.633Z
Learnt from: CR
Repo: OmniNode-ai/omniintelligence PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T03:23:43.633Z
Learning: Applies to tests/**/*.py : Use pytest markers: `pytest.mark.unit` for unit tests, `pytest.mark.integration` for integration tests, `pytest.mark.slow` for slow tests, and `pytest.mark.performance` for performance benchmarks
Applied to files:
pyproject.toml
📚 Learning: 2025-11-29T17:13:38.776Z
Learnt from: CR
Repo: OmniNode-ai/omniarchon PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-29T17:13:38.776Z
Learning: Applies to {**/*.py,!docs/**,!scripts/examples/**} : Ensure 100% test coverage for production code, with fail-closed security configuration as documented in `IMPROVEMENTS.md`.
Applied to files:
pyproject.toml
📚 Learning: 2025-12-03T16:55:49.744Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T16:55:49.744Z
Learning: Applies to **/*.py : Use type-safe configuration via Pydantic Settings from config/settings.py with 90+ type-safe variables organized into External Service Discovery, Shared Infrastructure, AI Provider API Keys, Local Services, and Feature Flags
Applied to files:
pyproject.toml
📚 Learning: 2025-11-24T17:24:54.193Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-24T17:24:54.193Z
Learning: Applies to **/*test*.py : Apply pytest markers (mock, integration) ONLY to fixture parameters using pytest.param, never directly on test functions or classes
Applied to files:
pyproject.toml
📚 Learning: 2025-11-29T17:13:38.776Z
Learnt from: CR
Repo: OmniNode-ai/omniarchon PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-29T17:13:38.776Z
Learning: Applies to **/*.py : NO environment variables hardcoded in code. ALL configuration must be provided via `.env` file. Use Pydantic Settings with `BaseSettings` and `Field` with `env` parameter for configuration management.
Applied to files:
pyproject.toml
📚 Learning: 2025-11-24T16:33:51.604Z
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-24T16:33:51.604Z
Learning: Applies to tests/**/conftest.py : Test fixtures must be defined in `conftest.py` and should provide reusable sample data, UUIDs, semantic versions, and model data
Applied to files:
tests/conftest.py
📚 Learning: 2025-12-03T03:23:43.633Z
Learnt from: CR
Repo: OmniNode-ai/omniintelligence PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T03:23:43.633Z
Learning: Applies to tests/**/*.py : Organize tests into unit tests (no infrastructure), integration tests (requires Kafka and databases), and node-specific tests with shared fixtures for Kafka mocks, sample data, correlation IDs, and intelligence client mocks
Applied to files:
tests/conftest.py
📚 Learning: 2025-11-24T17:24:54.193Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-24T17:24:54.193Z
Learning: Applies to **/*test*.py : Use context-based fixtures with pytest.param and conditional dependency injection (e.g., UNIT_CONTEXT vs INTEGRATION_CONTEXT) for mock and integration tests
Applied to files:
tests/conftest.py
🧬 Code graph analysis (4)
scripts/validate.py (1)
src/omnibase_infra/cli/commands.py (1)
validate(22-23)
src/omnibase_infra/validation/__init__.py (1)
src/omnibase_infra/validation/infra_validators.py (8)
get_validation_summary(189-225)validate_infra_all(152-186)validate_infra_architecture(32-48)validate_infra_circular_imports(132-149)validate_infra_contract_deep(90-108)validate_infra_contracts(51-65)validate_infra_patterns(68-87)validate_infra_union_usage(111-129)
src/omnibase_infra/validation/infra_validators.py (1)
src/omnibase_infra/cli/commands.py (1)
validate(22-23)
src/omnibase_infra/cli/commands.py (1)
src/omnibase_infra/validation/infra_validators.py (7)
validate_infra_architecture(32-48)validate_infra_contracts(51-65)validate_infra_patterns(68-87)validate_infra_union_usage(111-129)validate_infra_circular_imports(132-149)get_validation_summary(189-225)validate_infra_all(152-186)
🔇 Additional comments (19)
.gitignore (1)
183-189: Validation result patterns align well with PR objectives.The patterns for validation and test artifacts (lines 183-189) are well-suited for the validation framework being introduced in this PR. These will appropriately exclude generated validation reports from version control.
tests/conftest.py (1)
1-15: LGTM!The import reordering (stdlib before third-party) aligns with PEP8 and the isort configuration in
pyproject.toml. Themock_containerfixture provides a reasonable baseline for testing ONEX container interactions.pyproject.toml (2)
195-215: Good pytest configuration.The marker definitions and strict options provide a solid testing foundation. The
asyncio_mode = "auto"simplifies async test writing, and--strict-markerswill catch marker typos early.
252-279: Coverage configuration looks solid.The
fail_under = 60threshold is a pragmatic starting point for a new validation framework. Branch coverage and parallel execution are good practices. The exclusion patterns appropriately skip non-production code paths.src/omnibase_infra/cli/__init__.py (1)
1-1: Minimal package initializer is acceptable.The docstring-only
__init__.pyfollows Python conventions. The CLI entry point is correctly defined inpyproject.tomlpointing tocommands:cli..pre-commit-config.yaml (2)
33-78: Python development hooks are well-configured.The hook ordering (black → isort → ruff → mypy) follows best practices. Scoping mypy to
src/omnibase_infra/prevents type-checking test files, which is appropriate. The tmp directory cleanup with error suppression handles edge cases gracefully.
10-29: Bothyamlfmt(v0.17.2) andpre-commit-hooks(v6.0.0) are already at their latest available versions. No version updates are needed.Likely an incorrect or invalid review comment.
scripts/validate.py (3)
26-39: Fail-open behavior on ImportError is intentional but risky.Returning
Truewhenomnibase_coreisn't available means validation silently passes. While this enables gradual adoption, it could mask issues if the import fails for unexpected reasons (e.g., version mismatch, corrupted package).Consider differentiating between "module not installed" (acceptable skip) vs "import error due to bug" (should fail).
42-61: LGTM!The directory existence check before validation is a sensible guard for projects that may not have a
nodes/directory yet.
174-210: LGTM!The CLI implementation is clean with proper exit codes. The
argparsesetup matches the documented usage in the module docstring.src/omnibase_infra/validation/__init__.py (1)
1-47: Well-structured validation module interface.The re-export pattern cleanly exposes both core validators and infrastructure-specific wrappers. The separation in
__all__between direct re-exports and infra wrappers enhances clarity.src/omnibase_infra/cli/commands.py (5)
16-23: Clean CLI structure using Click groups.The two-level group hierarchy (cli → validate) provides a clear namespace for validation commands.
84-106: Robust handling of circular import results.The command correctly checks
has_circular_importsand safely accesses optional attributes (cycles,errors) withhasattrguards.
109-145: Comprehensive validation orchestration with clear reporting.The Rich table presentation and summary logic provide excellent user experience. The helpers handle heterogeneous result types gracefully.
148-178: Defensive helper functions handle type variance well.The
hasattrchecks accommodate the different result types (CircularImportValidationResultvsModelValidationResult) without brittle type checks.
26-66: Consistent command implementation pattern.All three commands follow the same structure: print status → call validator → print result → exit with appropriate code. Defaults align with
infra_validators.py.src/omnibase_infra/validation/infra_validators.py (3)
32-48: Strict architecture validation appropriate for infrastructure.The zero-tolerance default (
max_violations=0) correctly enforces one-model-per-file for infrastructure nodes.
51-87: Appropriate defaults for infrastructure validation.Both wrappers set sensible defaults: contracts target the nodes directory, and patterns enforce strict mode suitable for infrastructure code quality.
90-149: Specialized validators address infrastructure concerns.
validate_infra_contract_deepenables autonomous code generation by providing comprehensive contract validation, whilevalidate_infra_circular_importstackles infrastructure-specific dependency complexity.
PR Review: Add validation framework with omnibase_core integration✅ SummaryThis PR successfully adds a comprehensive validation framework to 🎯 Code Quality Assessment✅ Strengths
🔍 Issues Found🐛 Critical IssuesNone identified - No critical bugs or security vulnerabilities found.
|
Add GitHub Actions CI workflow matching omnibase_core patterns: - Smoke tests for fast failure on basic issues - Full test suite with parallel execution - Code quality checks (black, isort, ruff, mypy) - Test result artifact upload and summary Also add pytest-xdist and pytest-timeout dev dependencies.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
30-30: Remove redundant pip cache when using Poetry with in-project virtualenvs.Lines 30, 89, and 153 specify
cache: 'pip'in the setup-python action, but this is redundant when using Poetry as the dependency manager withvirtualenvs-in-project: true. The explicitactions/cache@v4steps in each job handle caching the.venvdirectory, which is where Poetry stores dependencies. The pip cache won't be used and adds unnecessary overhead.Apply this diff to all three jobs (example shown for smoke-test):
- name: Set up Python uses: actions/setup-python@v5 with: python-version: ${{ env.PYTHON_VERSION }} - cache: 'pip'Also applies to: 89-89, 153-153
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
.github/workflows/test.yml(1 hunks)pyproject.toml(4 hunks)
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/canonical_patterns.mdc:0-0
Timestamp: 2025-11-28T18:58:53.781Z
Learning: All code must pass pre-commit validation hooks including string version detection, backward compatibility checks, fallback pattern removal, single class per file, error raising validation, Pydantic pattern validation, union usage validation, and enum/model import prevention
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/dag_based_tool_bootstrapping.mdc:0-0
Timestamp: 2025-11-24T16:32:06.947Z
Learning: Maintain 100% validation success rate for all tool bootstrapping operations; all DAG scenarios (bootstrap_all_tools, extract_node_to_tool, extract_specific_tool) must achieve complete validation before artifacts are committed
📚 Learning: 2025-10-14T12:06:38.965Z
Learnt from: jonahgabriel
Repo: OmniNode-ai/omninode_bridge PR: 0
File: :0-0
Timestamp: 2025-10-14T12:06:38.965Z
Learning: In pyproject.toml for OmniNode Bridge: Dev dependencies are pytest ^8.4.0, pytest-asyncio ^0.25.0, mypy ^1.13.0, black ^24.10.0, and ruff ^0.8.0, all compatible with Python 3.12.
Applied to files:
pyproject.toml
📚 Learning: 2025-10-14T12:06:38.965Z
Learnt from: jonahgabriel
Repo: OmniNode-ai/omninode_bridge PR: 0
File: :0-0
Timestamp: 2025-10-14T12:06:38.965Z
Learning: In pyproject.toml for OmniNode Bridge: Core dependencies are pydantic ^2.11.7, fastapi ^0.115.0, uvicorn ^0.32.0, asyncpg ^0.29.0, and redis ^6.0.0 (for Redis/Valkey compatibility).
Applied to files:
pyproject.toml
📚 Learning: 2025-12-03T22:36:46.855Z
Learnt from: CR
Repo: OmniNode-ai/omnibase_infra PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T22:36:46.855Z
Learning: Applies to **/*.py : Update all import references from `omnibase.` to `omnibase_core.` and from `omnibase.exceptions` to `omnibase_core.exceptions`. Update all imports: `omnibase.enums` to `omnibase_core.enums`.
Applied to files:
pyproject.toml
📚 Learning: 2025-10-14T12:06:38.965Z
Learnt from: jonahgabriel
Repo: OmniNode-ai/omninode_bridge PR: 0
File: :0-0
Timestamp: 2025-10-14T12:06:38.965Z
Learning: OmniNode Bridge project uses Python ^3.12 as the baseline version, upgraded from 3.11 for performance improvements, enhanced type system, and better asyncio support.
Applied to files:
pyproject.toml
📚 Learning: 2025-11-24T17:23:49.777Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/node_standards.mdc:0-0
Timestamp: 2025-11-24T17:23:49.777Z
Learning: Applies to **/node_*/v[0-9]*_[0-9]*_[0-9]*/*.py : All ONEX node implementations must follow dependency injection and protocol-first design patterns as established in the node_cli canonical reference
Applied to files:
pyproject.toml
📚 Learning: 2025-11-24T17:22:32.195Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/canonical_patterns.mdc:0-0
Timestamp: 2025-11-24T17:22:32.195Z
Learning: Applies to **/v[0-9]_[0-9]_[0-9]/{tools,registry}/**.py : Constructor-based dependency injection must be used for all tool and node classes; validate that required dependencies are not None, raising OnexError with specific error code if missing
Applied to files:
pyproject.toml
📚 Learning: 2025-12-03T22:36:46.855Z
Learnt from: CR
Repo: OmniNode-ai/omnibase_infra PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T22:36:46.855Z
Learning: Applies to **/*.py : All dependencies must be injected via container: `def __init__(self, container: ONEXContainer)`. Use container injection pattern for dependency management.
Applied to files:
pyproject.toml
📚 Learning: 2025-11-25T21:48:22.867Z
Learnt from: CR
Repo: OmniNode-ai/omnibase_core PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T21:48:22.867Z
Learning: Applies to **/*.py : Always use 100% strict mypy type checking - all functions must have type annotations and no untyped definitions are allowed
Applied to files:
pyproject.toml
📚 Learning: 2025-11-25T21:48:22.867Z
Learnt from: CR
Repo: OmniNode-ai/omnibase_core PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T21:48:22.867Z
Learning: Applies to **/*.py : Run code through black formatter, isort for import sorting, and ruff linter before committing
Applied to files:
pyproject.toml
📚 Learning: 2025-12-03T03:23:43.660Z
Learnt from: CR
Repo: OmniNode-ai/omniintelligence PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T03:23:43.660Z
Learning: Applies to src/**/*.py : Run code quality checks using `ruff check src tests`, `ruff check --fix src tests` for auto-fix, `black src tests` for formatting, `isort src tests` for import sorting, and `mypy src` for type checking
Applied to files:
pyproject.toml
📚 Learning: 2025-11-24T17:24:10.209Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/pr.mdc:0-0
Timestamp: 2025-11-24T17:24:10.209Z
Learning: Applies to **/*.py : Automatically perform code standards checks on all Python files in the PR, including: Any/Any imports and Dict[str, Any] usage violations, naming convention violations (tool_, model_, enum_ prefixes), anti-pattern detection (direct tool instantiation, telescoping constructors), and type safety enforcement (strongest typing possible). Fix all violations immediately with proper commit messages.
Applied to files:
pyproject.toml
📚 Learning: 2025-12-03T03:23:43.660Z
Learnt from: CR
Repo: OmniNode-ai/omniintelligence PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T03:23:43.660Z
Learning: Applies to tests/**/*.py : Use pytest markers: `pytest.mark.unit` for unit tests, `pytest.mark.integration` for integration tests, `pytest.mark.slow` for slow tests, and `pytest.mark.performance` for performance benchmarks
Applied to files:
pyproject.toml
📚 Learning: 2025-11-29T17:13:38.776Z
Learnt from: CR
Repo: OmniNode-ai/omniarchon PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-29T17:13:38.776Z
Learning: Applies to {**/*.py,!docs/**,!scripts/examples/**} : Ensure 100% test coverage for production code, with fail-closed security configuration as documented in `IMPROVEMENTS.md`.
Applied to files:
pyproject.toml
📚 Learning: 2025-12-03T16:55:49.755Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T16:55:49.755Z
Learning: Applies to **/*.py : Use type-safe configuration via Pydantic Settings from config/settings.py with 90+ type-safe variables organized into External Service Discovery, Shared Infrastructure, AI Provider API Keys, Local Services, and Feature Flags
Applied to files:
pyproject.toml
📚 Learning: 2025-11-24T17:24:54.193Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-24T17:24:54.193Z
Learning: Applies to **/*test*.py : Apply pytest markers (mock, integration) ONLY to fixture parameters using pytest.param, never directly on test functions or classes
Applied to files:
pyproject.toml
📚 Learning: 2025-11-29T17:13:38.776Z
Learnt from: CR
Repo: OmniNode-ai/omniarchon PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-29T17:13:38.776Z
Learning: Applies to **/*.py : NO environment variables hardcoded in code. ALL configuration must be provided via `.env` file. Use Pydantic Settings with `BaseSettings` and `Field` with `env` parameter for configuration management.
Applied to files:
pyproject.toml
🔇 Additional comments (4)
.github/workflows/test.yml (1)
191-218: Test summary aggregation logic is sound.The test-summary job properly depends on all three upstream jobs, uses
if: always()to ensure it runs regardless of prior failures, and correctly extracts and aggregates individual job results before reporting the final outcome.pyproject.toml (3)
254-281: Verify coverage threshold alignment with project standards.The coverage configuration sets
fail_under = 60, which is relatively permissive. Learnings reference a "100% test coverage for production code" requirement. Confirm whether the 60% threshold is intentional for this infra project or if it should be raised to match project standards.Does the omnibase_infra project require 100% test coverage for production code, or is 60% acceptable as an initial baseline?
97-99: Configuration standards alignment looks good overall.Black, isort (aside from the noted issue), pytest, mypy, and ruff configurations align well with project standards: Python 3.12, strict mypy type checking (line 224), proper tool versions, and consistent line length (88). The ruff ignore list, while extensive, intentionally matches omnibase_core for consistency across the ONEX ecosystem.
Also applies to: 241-252
20-22: omnibase-core version 0.3.5 is not available on PyPI — maximum available version is 0.3.2.The PR pins omnibase-core to 0.3.5, but PyPI only has releases up to 0.3.2 (uploaded Nov 14, 2025). This will cause installation failures. Update the dependency to 0.3.2 or confirm that 0.3.5 will be released before merge.
omnibase-spi 0.2.0 and fastapi 0.120.1 are both available on PyPI and valid.
⛔ Skipped due to learnings
Learnt from: jonahgabriel Repo: OmniNode-ai/omninode_bridge PR: 0 File: :0-0 Timestamp: 2025-10-14T12:06:38.965Z Learning: In pyproject.toml for OmniNode Bridge: Dev dependencies are pytest ^8.4.0, pytest-asyncio ^0.25.0, mypy ^1.13.0, black ^24.10.0, and ruff ^0.8.0, all compatible with Python 3.12.Learnt from: jonahgabriel Repo: OmniNode-ai/omninode_bridge PR: 0 File: :0-0 Timestamp: 2025-10-14T12:06:38.965Z Learning: In pyproject.toml for OmniNode Bridge: Core dependencies are pydantic ^2.11.7, fastapi ^0.115.0, uvicorn ^0.32.0, asyncpg ^0.29.0, and redis ^6.0.0 (for Redis/Valkey compatibility).Learnt from: CR Repo: OmniNode-ai/omninode_bridge PR: 0 File: .cursor/rules/node_standards.mdc:0-0 Timestamp: 2025-11-24T16:32:55.606Z Learning: Node services must implement dependency injection using `ModelONEXContainer` from `omnibase_core.models.container.model_onex_container`Learnt from: CR Repo: OmniNode-ai/omniclaude PR: 0 File: .cursor/rules/canonical_patterns.mdc:0-0 Timestamp: 2025-11-24T17:22:32.195Z Learning: Applies to **/v[0-9]_[0-9]_[0-9]/{tools,registry}/**.py : Constructor-based dependency injection must be used for all tool and node classes; validate that required dependencies are not None, raising OnexError with specific error code if missingLearnt from: CR Repo: OmniNode-ai/omniclaude PR: 0 File: .cursor/rules/node_standards.mdc:0-0 Timestamp: 2025-11-24T17:23:49.777Z Learning: Applies to **/node_*/v[0-9]*_[0-9]*_[0-9]*/*.py : All ONEX node implementations must follow dependency injection and protocol-first design patterns as established in the node_cli canonical referenceLearnt from: CR Repo: OmniNode-ai/omninode_bridge PR: 0 File: .cursor/rules/standards.mdc:0-0 Timestamp: 2025-11-24T16:33:32.747Z Learning: Applies to **/*.py : Import enums from `omnibase.enums` packageLearnt from: CR Repo: OmniNode-ai/omniclaude PR: 0 File: .cursor/rules/node_standards.mdc:0-0 Timestamp: 2025-11-24T17:23:49.777Z Learning: All ONEX nodes must conform to the canonical structure, code generation, and interface patterns established in the `node_cli` node, using it as the primary source of truth for directory structure, contract schema patterns, linked document architecture, base state patterns, shared schema references, extensibility patterns, CLI interface declarations, code generation, dependency injection, error handling, testing, and documentationLearnt from: CR Repo: OmniNode-ai/omnibase_infra PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-03T22:36:46.855Z Learning: Applies to **/*.py : All dependencies must be injected via container: `def __init__(self, container: ONEXContainer)`. Use container injection pattern for dependency management.
| key: >- | ||
| venv-${{ runner.os }}-${{ env.PYTHON_VERSION }}- ${{ hashFiles('**/poetry.lock') }}- ${{ env.CACHE_VERSION }} | ||
| restore-keys: | | ||
| venv-${{ runner.os }}-${{ env.PYTHON_VERSION }}-${{ hashFiles('**/poetry.lock') }}-${{ env.CACHE_VERSION }} |
There was a problem hiding this comment.
Fix cache key format inconsistency that breaks cache restoration.
The cache key format (line 45, 104, 168) has a space before ${{ hashFiles() }} but the restore-keys (line 47, 106, 170) do not. This mismatch prevents the cache from being properly restored, defeating the caching strategy and causing unnecessary dependency re-installations.
All three jobs have this issue:
- Smoke-test: lines 44-47
- Test: lines 103-106
- Lint: lines 167-170
Apply this diff to all three jobs (example shown for smoke-test; apply to test and lint jobs identically):
- name: Load cached venv
id: cached-poetry-dependencies
uses: actions/cache@v4
with:
path: .venv
key: >-
- venv-${{ runner.os }}-${{ env.PYTHON_VERSION }}- ${{ hashFiles('**/poetry.lock') }}- ${{ env.CACHE_VERSION }}
+ venv-${{ runner.os }}-${{ env.PYTHON_VERSION }}-${{ hashFiles('**/poetry.lock') }}-${{ env.CACHE_VERSION }}
restore-keys: |
venv-${{ runner.os }}-${{ env.PYTHON_VERSION }}-${{ hashFiles('**/poetry.lock') }}-${{ env.CACHE_VERSION }}Also applies to: 103-106, 167-170
🤖 Prompt for AI Agents
.github/workflows/test.yml around lines 44-47 (and similarly at 103-106 and
167-170): the cache key string has an extra space before `${{ hashFiles(...) }}`
in the primary key but not in the restore-keys, causing mismatched keys and
failed cache restores; remove the stray space so the primary key exactly matches
the restore-keys format (i.e., ensure no space before `${{
hashFiles('**/poetry.lock') }}`) in the smoke-test, test, and lint job key
definitions.
PR Review: Validation Framework with omnibase_core IntegrationOverall Assessment: APPROVED with RecommendationsThis PR adds a solid validation framework that aligns well with ONEX infrastructure standards. The implementation demonstrates good architecture and follows the DRY principle by reusing validators from omnibase_core. Strengths1. Strong Typing and Architecture Compliance
2. DRY Principle Applied
3. Comprehensive Validation Coverage
4. Developer Experience
5. CI/CD Integration
Critical Issues Found1. CRITICAL: Inconsistent Union Type LimitLocation: infra_validators.py:113 vs validate.py:86 vs commands.py:71 The max_unions parameter has different defaults across interfaces:
Impact: CLI users will get different validation results than pre-commit hooks and CI Recommendation: Standardize to max_unions=20 across all interfaces with a constant 2. Documentation MismatchLocation: infra_validators.py:123 The docstring says "Defaults to 10" but the parameter default is 20. Recommendation: Update docstring to match actual default 3. Poetry Version MismatchLocation: .github/workflows/test.yml:11 POETRY_VERSION is set to "2.2.1" but Poetry 2.2.1 does not exist yet (latest stable is 1.8.x) Impact: CI will fail when trying to install non-existent Poetry version Recommendation: Change to "1.8.3" or "latest" 4. Cache Key Formatting IssueLocation: .github/workflows/test.yml:45 The cache key has extra spaces between version components which may reduce cache hit rate. Recommendations1. Add Constants for Magic NumbersCreate constants in infra_validators.py to avoid magic numbers:
2. Add Validation to CIThe pre-commit hooks skip ONEX validation in CI. Consider adding a separate CI job that runs the validation script. 3. Test CoverageNo tests were added for the validation module itself. Consider adding:
4. Performance OptimizationRunning all 5 validators on every commit may be slow. Consider adding --quick mode to pre-commit hooks for local development. Security ReviewNo security concerns identified:
Code Quality Checklist
Action ItemsMust Fix Before Merge:
Should Fix:
Nice to Have:
SummaryThis is a high-quality PR that adds essential infrastructure validation tooling. The code follows ONEX principles well and provides multiple interfaces for developers. The issues found are mostly configuration inconsistencies rather than architectural problems. Once the critical fixes are applied, this will be ready to merge. Recommendation: Approve with required changes |
CI was failing with pytest exit code 5 (no tests collected) because tests/unit/ only contained an empty __init__.py. Added 7 smoke tests that verify basic package imports and module structure. Tests verify: - Package import (omnibase_infra) - CLI module structure - Validation module imports - Errors module structure - Event bus module structure - All 9 submodules are importable - Validation constants exist
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
tests/unit/test_smoke.py (5)
12-18: Basic package import smoke test is appropriateSimple import + non-
Noneassertion is enough for a top-level smoke. If you later expose a stable attribute like__version__, you might also assert its presence, but not required for this PR.
30-44: Strengthen validation module smoke test by asserting callabilityRight now you only assert the imported validators are non-
None. It’s slightly more robust to assert they’re callable, which still keeps the test cheap:- assert validate_infra_all is not None - assert validate_infra_architecture is not None - assert validate_infra_contracts is not None - assert validate_infra_patterns is not None + assert callable(validate_infra_all) + assert callable(validate_infra_architecture) + assert callable(validate_infra_contracts) + assert callable(validate_infra_patterns)
46-53: Align errors-module assertions with the docstringThe docstring says “structured correctly” but the test only checks that
errorsimports. Either:
- add at least one structural assertion (e.g., presence of a canonical error type or exported symbol), or
- relax the docstring to say it just verifies importability.
As-is, the docstring slightly over-promises what’s being tested.
63-88: Good cross-subsystem import smoke; watch for future heavinessThis is a useful “can I import all major submodules?” check and should catch packaging/namespace regressions early. Just be mindful over time that any heavy side effects in these modules (network, DB clients, etc.) will directly affect smoke-test speed and reliability.
This broad coverage pairs well with having more focused per-subsystem unit tests under
tests/unit/as the suite grows, matching the usual subsystem-based organization. Based on learnings, ...
90-99: Path-constant checks are fine; note potential brittlenessAsserting exact string values for
INFRA_SRC_PATHandINFRA_NODES_PATHis a clear contract about repo layout. Just be aware this will need updating if you ever:
- switch these constants to
Pathobjects, or- make them OS-separator–aware via
os.path.join.If that evolution is likely, you could instead assert relative semantics (e.g., suffix) rather than the full literal, but for now this is acceptable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (1)
tests/unit/test_smoke.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: NEVER useAny- Always use specific types. Use Pydantic Models for all data structures as proper Pydantic models.
Use CamelCase for model class names:ModelUserData. All model classes follow CamelCase naming pattern starting with 'Model' prefix.
Use snake_case for all Python filenames:model_user_data.py. All filenames must use lowercase with underscores.
Each file contains exactly oneModel*class. One model per file pattern for all Pydantic models.
All tools/services follow contract-driven patterns. Use Pydantic Models as proper data structures following contract definitions.
All dependencies must be injected via container:def __init__(self, container: ONEXContainer). Use container injection pattern for dependency management.
Use duck typing through protocols, never isinstance. Protocol resolution for dependency resolution instead of type checking.
All exceptions converted to OnexError with chaining:raise OnexError(...) from e. Use OnexError only for exception handling.
Update all import references fromomnibase.toomnibase_core.and fromomnibase.exceptionstoomnibase_core.exceptions. Update all imports:omnibase.enumstoomnibase_core.enums.
Files:
tests/unit/test_smoke.py
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/canonical_patterns.mdc:0-0
Timestamp: 2025-11-28T18:58:53.781Z
Learning: All code must pass pre-commit validation hooks including string version detection, backward compatibility checks, fallback pattern removal, single class per file, error raising validation, Pydantic pattern validation, union usage validation, and enum/model import prevention
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/canonical_patterns.mdc:0-0
Timestamp: 2025-11-28T18:58:53.781Z
Learning: Organize models under `src/omnibase_core/models/` by domain including: base, cli, common, config, core, contracts, discovery, health, infrastructure, logging, metadata, nodes, operations, results, security, service, tools, validation, and workflows
Learnt from: CR
Repo: OmniNode-ai/omnibase_core PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T21:48:22.867Z
Learning: Applies to src/omnibase_core/models/**/*.py : Use Pydantic models for data validation and serialization - leverage Pydantic 2.11+ features for type safety
📚 Learning: 2025-12-03T03:23:43.660Z
Learnt from: CR
Repo: OmniNode-ai/omniintelligence PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T03:23:43.660Z
Learning: Applies to tests/**/*.py : Organize tests into unit tests (no infrastructure), integration tests (requires Kafka and databases), and node-specific tests with shared fixtures for Kafka mocks, sample data, correlation IDs, and intelligence client mocks
Applied to files:
tests/unit/test_smoke.py
📚 Learning: 2025-11-28T18:58:53.781Z
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/canonical_patterns.mdc:0-0
Timestamp: 2025-11-28T18:58:53.781Z
Learning: Applies to tests/**/*.py : Write comprehensive test coverage following the test structure under `tests/unit/` organized by subsystem (enums, models, mixins, utils)
Applied to files:
tests/unit/test_smoke.py
🔇 Additional comments (3)
tests/unit/test_smoke.py (3)
1-10: Module-level smoke-test setup looks solidClear docstring, no external deps, and
from __future__ import annotationskeeps typing modern without side effects. Nothing to change here.
20-28: CLI module import and interface check are sufficient for smoke coverageVerifying that
omnibase_infra.cli.commandsimports and exposes a callableclientrypoint is a good, lightweight smoke test for the CLI surface.
55-61: Event bus structure check matches intentImporting
event_busand asserting__all__exists gives a nice, low-cost contract check for the public surface.
Pull Request Review: Validation Framework with omnibase_core IntegrationOverall Assessment: APPROVE with Minor RecommendationsThis PR successfully adds a comprehensive validation framework that aligns omnibase_infra with omnibase_core standards. The implementation is solid, well-structured, and follows ONEX principles effectively. Code Quality AnalysisStrengths
Issues & RecommendationsMedium Priority Recommendations
Low Priority Suggestions
Security Considerations - No Issues Detected
Performance Considerations - Well Optimized
ONEX Compliance Check - Fully Compliant
SummaryThis is high-quality, production-ready code that successfully establishes validation infrastructure for omnibase_infra. Key Achievements:
Recommendations Priority:
Verdict: APPROVE This PR is ready to merge. The recommendations above are enhancements, not blockers. |
Critical fixes: - Fix CI cache key format inconsistency that broke cache restoration - Fix CLI --max-unions default (10→20) to match validator - Fix bare exception catch in validate.py (now specific types) Major fixes: - Add error handling to lint job install steps for consistency - Remove external packages from isort known_first_party - Fix docstrings: max_unions defaults to 20, not 10 Test fixes: - Remove smoke tests for uncommitted modules (errors, event_bus, handlers, runtime) - Keep only tests for committed modules (5 tests pass)
PR Review: Validation Framework with omnibase_core Integration✅ Overall Assessment: APPROVED with Minor SuggestionsThis PR successfully adds a comprehensive validation framework that integrates with omnibase_core. The implementation is well-structured, follows ONEX principles, and provides excellent developer experience through CLI commands, pre-commit hooks, and standalone scripts. 🎯 Strengths1. Excellent Code Reuse Pattern ⭐The validation wrappers in infra_validators.py demonstrate proper DRY principles by reusing validators from omnibase_core rather than reimplementing them. This is exactly the right approach for infrastructure code. 2. Strong Type Safety ✅
3. Developer Experience 🚀Three complementary interfaces make validation accessible:
4. Well-Structured Documentation 📚
🔍 Code Quality AnalysisValidation Module (src/omnibase_infra/validation/infra_validators.py)Strengths:
Observations:
CLI Commands (src/omnibase_infra/cli/commands.py)Strengths:
Minor Suggestion: Validation Script (scripts/validate.py)Strengths:
Excellent Error Handling: 🔒 Security Analysis✅ No Security Concerns Identified
🚀 Performance ConsiderationsPre-commit Hook Ordering ⚡The pre-commit configuration follows optimal ordering: black → isort → ruff → mypy → ONEX validators This is correct because:
CI Workflow Efficiency 🎯The GitHub Actions workflow (test.yml) is well-optimized:
📊 Test Coverage✅ Smoke Tests are AppropriateThe test_smoke.py file provides:
These are exactly the right tests for a validation framework. More comprehensive tests should come when actual infrastructure nodes are migrated. 🎨 Alignment with CLAUDE.md Standards✅ Perfect ONEX Compliance
🐛 Potential Issues
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (7)
pyproject.toml (1)
249-249: Past issue resolved: isort configuration is now correct.The
known_first_partyconfiguration now correctly includes only locally-owned packages. The previous issue with external packages has been addressed..github/workflows/test.yml (2)
44-46: Past issue resolved: cache key format is now consistent.The cache keys no longer have the space formatting issue that was previously flagged. All three jobs (smoke-test, test, lint) now use consistent key formats that will properly restore cached dependencies.
Also applies to: 102-104, 165-167
169-182: Past issue resolved: lint job now has proper error handling.The lint job's dependency and project installation steps now include the same error handling patterns used in smoke-test and test jobs. Failed installations will produce clear error messages.
scripts/validate.py (1)
130-134: Past issue partially resolved: now catches specific exceptions.The exception handling now catches specific exception types (AttributeError, RuntimeError, OSError) instead of a bare
except Exception, which addresses the previous concern. However, note that returningTrue(passing validation) when these errors occur could potentially mask legitimate bugs in the validator itself.src/omnibase_infra/cli/commands.py (1)
68-82: Past issue resolved: CLI default now matches underlying validator.The
--max-unionsoption now correctly defaults to 20, matching the default invalidate_infra_union_usage. This ensures consistent behavior between CLI and programmatic usage.src/omnibase_infra/validation/infra_validators.py (2)
111-129: Past issue resolved: docstring now matches code default.The docstring correctly states that
max_unions"Defaults to 20", which matches the function signature on line 113. The documentation is now consistent with the implementation.
152-186: Past issue resolved: docstring now accurately documents union limit.The docstring correctly states "Union usage (max 20)" on line 163, which matches the default value used by
validate_infra_union_usagewhen called without arguments on line 183.
🧹 Nitpick comments (1)
src/omnibase_infra/cli/commands.py (1)
7-7: ReplaceAnywith specific types.The code uses
Anytype in function signatures (lines 157, 168), which violates the coding guideline "NEVER useAny- Always use specific types."The result objects have known types. Apply this diff to use specific types:
-from typing import Any +from typing import Union + +from omnibase_core.models.common.model_validation_result import ModelValidationResult +from omnibase_core.models.model_import_validation_result import ( + ModelValidationResult as CircularImportValidationResult, +) + +ValidationResult = Union[ModelValidationResult[None], CircularImportValidationResult]Then update the function signatures:
-def _get_error_count(result: Any) -> int: +def _get_error_count(result: ValidationResult) -> int: """Get the error count from a validation result.""" -def _print_result(name: str, result: Any) -> None: +def _print_result(name: str, result: ValidationResult) -> None: """Print validation result with rich formatting."""As per coding guidelines, all functions must use specific types instead of Any.
Also applies to: 157-157, 168-168
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (6)
.github/workflows/test.yml(1 hunks)pyproject.toml(4 hunks)scripts/validate.py(1 hunks)src/omnibase_infra/cli/commands.py(1 hunks)src/omnibase_infra/validation/infra_validators.py(1 hunks)tests/unit/test_smoke.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/unit/test_smoke.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: NEVER useAny- Always use specific types. Use Pydantic Models for all data structures as proper Pydantic models.
Use CamelCase for model class names:ModelUserData. All model classes follow CamelCase naming pattern starting with 'Model' prefix.
Use snake_case for all Python filenames:model_user_data.py. All filenames must use lowercase with underscores.
Each file contains exactly oneModel*class. One model per file pattern for all Pydantic models.
All tools/services follow contract-driven patterns. Use Pydantic Models as proper data structures following contract definitions.
All dependencies must be injected via container:def __init__(self, container: ONEXContainer). Use container injection pattern for dependency management.
Use duck typing through protocols, never isinstance. Protocol resolution for dependency resolution instead of type checking.
All exceptions converted to OnexError with chaining:raise OnexError(...) from e. Use OnexError only for exception handling.
Update all import references fromomnibase.toomnibase_core.and fromomnibase.exceptionstoomnibase_core.exceptions. Update all imports:omnibase.enumstoomnibase_core.enums.
Files:
src/omnibase_infra/cli/commands.pyscripts/validate.pysrc/omnibase_infra/validation/infra_validators.py
🧠 Learnings (24)
📓 Common learnings
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/canonical_patterns.mdc:0-0
Timestamp: 2025-11-28T18:58:53.781Z
Learning: All code must pass pre-commit validation hooks including string version detection, backward compatibility checks, fallback pattern removal, single class per file, error raising validation, Pydantic pattern validation, union usage validation, and enum/model import prevention
📚 Learning: 2025-10-14T12:06:38.965Z
Learnt from: jonahgabriel
Repo: OmniNode-ai/omninode_bridge PR: 0
File: :0-0
Timestamp: 2025-10-14T12:06:38.965Z
Learning: In pyproject.toml for OmniNode Bridge: Dev dependencies are pytest ^8.4.0, pytest-asyncio ^0.25.0, mypy ^1.13.0, black ^24.10.0, and ruff ^0.8.0, all compatible with Python 3.12.
Applied to files:
pyproject.toml
📚 Learning: 2025-10-14T12:06:38.965Z
Learnt from: jonahgabriel
Repo: OmniNode-ai/omninode_bridge PR: 0
File: :0-0
Timestamp: 2025-10-14T12:06:38.965Z
Learning: In pyproject.toml for OmniNode Bridge: Core dependencies are pydantic ^2.11.7, fastapi ^0.115.0, uvicorn ^0.32.0, asyncpg ^0.29.0, and redis ^6.0.0 (for Redis/Valkey compatibility).
Applied to files:
pyproject.toml
📚 Learning: 2025-12-03T22:36:46.869Z
Learnt from: CR
Repo: OmniNode-ai/omnibase_infra PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T22:36:46.869Z
Learning: Applies to **/*.py : Update all import references from `omnibase.` to `omnibase_core.` and from `omnibase.exceptions` to `omnibase_core.exceptions`. Update all imports: `omnibase.enums` to `omnibase_core.enums`.
Applied to files:
pyproject.toml
📚 Learning: 2025-10-14T12:06:38.965Z
Learnt from: jonahgabriel
Repo: OmniNode-ai/omninode_bridge PR: 0
File: :0-0
Timestamp: 2025-10-14T12:06:38.965Z
Learning: OmniNode Bridge project uses Python ^3.12 as the baseline version, upgraded from 3.11 for performance improvements, enhanced type system, and better asyncio support.
Applied to files:
pyproject.toml
📚 Learning: 2025-11-24T17:23:49.777Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/node_standards.mdc:0-0
Timestamp: 2025-11-24T17:23:49.777Z
Learning: Applies to **/node_*/v[0-9]*_[0-9]*_[0-9]*/*.py : All ONEX node implementations must follow dependency injection and protocol-first design patterns as established in the node_cli canonical reference
Applied to files:
pyproject.toml
📚 Learning: 2025-11-24T17:22:32.195Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/canonical_patterns.mdc:0-0
Timestamp: 2025-11-24T17:22:32.195Z
Learning: Applies to **/v[0-9]_[0-9]_[0-9]/{tools,registry}/**.py : Constructor-based dependency injection must be used for all tool and node classes; validate that required dependencies are not None, raising OnexError with specific error code if missing
Applied to files:
pyproject.toml
📚 Learning: 2025-12-03T22:36:46.869Z
Learnt from: CR
Repo: OmniNode-ai/omnibase_infra PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T22:36:46.869Z
Learning: Applies to **/*.py : All dependencies must be injected via container: `def __init__(self, container: ONEXContainer)`. Use container injection pattern for dependency management.
Applied to files:
pyproject.toml
📚 Learning: 2025-11-25T21:48:22.867Z
Learnt from: CR
Repo: OmniNode-ai/omnibase_core PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T21:48:22.867Z
Learning: Applies to **/*.py : Always use 100% strict mypy type checking - all functions must have type annotations and no untyped definitions are allowed
Applied to files:
pyproject.toml
📚 Learning: 2025-11-25T21:48:22.867Z
Learnt from: CR
Repo: OmniNode-ai/omnibase_core PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T21:48:22.867Z
Learning: Applies to **/*.py : Run code through black formatter, isort for import sorting, and ruff linter before committing
Applied to files:
pyproject.toml
📚 Learning: 2025-12-03T03:23:43.660Z
Learnt from: CR
Repo: OmniNode-ai/omniintelligence PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T03:23:43.660Z
Learning: Applies to src/**/*.py : Run code quality checks using `ruff check src tests`, `ruff check --fix src tests` for auto-fix, `black src tests` for formatting, `isort src tests` for import sorting, and `mypy src` for type checking
Applied to files:
pyproject.toml
📚 Learning: 2025-12-03T03:23:43.660Z
Learnt from: CR
Repo: OmniNode-ai/omniintelligence PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T03:23:43.660Z
Learning: Applies to tests/**/*.py : Use pytest markers: `pytest.mark.unit` for unit tests, `pytest.mark.integration` for integration tests, `pytest.mark.slow` for slow tests, and `pytest.mark.performance` for performance benchmarks
Applied to files:
pyproject.toml
📚 Learning: 2025-11-24T17:24:10.209Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/pr.mdc:0-0
Timestamp: 2025-11-24T17:24:10.209Z
Learning: Applies to **/*.py : Automatically perform code standards checks on all Python files in the PR, including: Any/Any imports and Dict[str, Any] usage violations, naming convention violations (tool_, model_, enum_ prefixes), anti-pattern detection (direct tool instantiation, telescoping constructors), and type safety enforcement (strongest typing possible). Fix all violations immediately with proper commit messages.
Applied to files:
pyproject.toml
📚 Learning: 2025-11-29T17:13:38.776Z
Learnt from: CR
Repo: OmniNode-ai/omniarchon PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-29T17:13:38.776Z
Learning: Applies to {**/*.py,!docs/**,!scripts/examples/**} : Ensure 100% test coverage for production code, with fail-closed security configuration as documented in `IMPROVEMENTS.md`.
Applied to files:
pyproject.toml
📚 Learning: 2025-12-03T16:55:49.755Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T16:55:49.755Z
Learning: Applies to **/*.py : Use type-safe configuration via Pydantic Settings from config/settings.py with 90+ type-safe variables organized into External Service Discovery, Shared Infrastructure, AI Provider API Keys, Local Services, and Feature Flags
Applied to files:
pyproject.toml
📚 Learning: 2025-11-24T17:24:54.193Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-24T17:24:54.193Z
Learning: Applies to **/*test*.py : Apply pytest markers (mock, integration) ONLY to fixture parameters using pytest.param, never directly on test functions or classes
Applied to files:
pyproject.toml
📚 Learning: 2025-11-29T17:13:38.776Z
Learnt from: CR
Repo: OmniNode-ai/omniarchon PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-29T17:13:38.776Z
Learning: Applies to **/*.py : NO environment variables hardcoded in code. ALL configuration must be provided via `.env` file. Use Pydantic Settings with `BaseSettings` and `Field` with `env` parameter for configuration management.
Applied to files:
pyproject.toml
📚 Learning: 2025-11-24T17:22:32.195Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/canonical_patterns.mdc:0-0
Timestamp: 2025-11-24T17:22:32.195Z
Learning: Applies to **/cli_tools/*/v[0-9]_[0-9]_[0-9]/commands/*.py : CLI commands must use Click framework with error handling: catch OnexError and exit with status code 1
Applied to files:
src/omnibase_infra/cli/commands.py
📚 Learning: 2025-11-24T17:23:49.777Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/node_standards.mdc:0-0
Timestamp: 2025-11-24T17:23:49.777Z
Learning: Applies to **/node_*/v[0-9]*_[0-9]*_[0-9]*/contracts/contract_cli.yaml : All ONEX node CLI interface definitions, if applicable, must be included in contract_cli.yaml with entrypoint and commands specifications
Applied to files:
src/omnibase_infra/cli/commands.py
📚 Learning: 2025-11-24T17:23:49.777Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/node_standards.mdc:0-0
Timestamp: 2025-11-24T17:23:49.777Z
Learning: All ONEX nodes must conform to the canonical structure, code generation, and interface patterns established in the `node_cli` node, using it as the primary source of truth for directory structure, contract schema patterns, linked document architecture, base state patterns, shared schema references, extensibility patterns, CLI interface declarations, code generation, dependency injection, error handling, testing, and documentation
Applied to files:
src/omnibase_infra/cli/commands.py
📚 Learning: 2025-11-24T17:24:10.209Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/pr.mdc:0-0
Timestamp: 2025-11-24T17:24:10.209Z
Learning: When user requests PR description creation or pull request submission (e.g., 'create a PR description', 'create a PR', 'submit for review'), automatically execute a complete workflow: (1) gather git statistics, (2) analyze changes, (3) check work tickets, (4) generate content using template, (5) create full PR via `gh pr create` targeting `development` branch by default unless user specifies otherwise, (6) automatically switch to code review mode and perform standards checks on Python files, (7) fix any violations found, and (8) report results to user.
Applied to files:
.github/workflows/test.yml
📚 Learning: 2025-11-29T17:13:38.776Z
Learnt from: CR
Repo: OmniNode-ai/omniarchon PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-29T17:13:38.776Z
Learning: Applies to scripts/verify_environment.py : Run `python3 scripts/verify_environment.py --verbose` before deployment to validate 9 critical environment checks.
Applied to files:
scripts/validate.py
📚 Learning: 2025-11-28T18:58:53.781Z
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/canonical_patterns.mdc:0-0
Timestamp: 2025-11-28T18:58:53.781Z
Learning: Applies to **/*.py : Do not use bare `except:` without re-raise
Applied to files:
scripts/validate.py
📚 Learning: 2025-11-28T18:58:53.781Z
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/canonical_patterns.mdc:0-0
Timestamp: 2025-11-28T18:58:53.781Z
Learning: Applies to **/*.py : Do not use `except ValueError: ... = Enum.UNKNOWN` patterns; raise explicit errors instead
Applied to files:
scripts/validate.py
🧬 Code graph analysis (3)
src/omnibase_infra/cli/commands.py (1)
src/omnibase_infra/validation/infra_validators.py (7)
validate_infra_architecture(32-48)validate_infra_contracts(51-65)validate_infra_patterns(68-87)validate_infra_union_usage(111-129)validate_infra_circular_imports(132-149)get_validation_summary(189-225)validate_infra_all(152-186)
scripts/validate.py (1)
src/omnibase_infra/cli/commands.py (1)
validate(22-23)
src/omnibase_infra/validation/infra_validators.py (1)
src/omnibase_infra/cli/commands.py (1)
validate(22-23)
🔇 Additional comments (10)
pyproject.toml (2)
197-217: LGTM! Well-structured pytest configuration.The pytest markers (unit, integration, slow, smoke, performance, infrastructure, validation) align with ONEX testing standards, and the strict configuration options will catch marker typos early.
219-239: LGTM! Comprehensive mypy and Pydantic configuration.The mypy configuration with Pydantic plugin and explicit handling of omnibase_core/omnibase_spi missing stubs is appropriate. The Pydantic-mypy settings enforce strict validation patterns.
.github/workflows/test.yml (1)
1-223: LGTM! Well-structured CI workflow with appropriate job dependencies.The workflow provides:
- Fast-failing smoke tests (5min timeout)
- Comprehensive test suite with parallelization (-n auto)
- Code quality checks (Black, isort, Ruff, MyPy)
- Clear summary reporting
The dependency chain (smoke-test → test) ensures quick feedback on basic issues before running the full suite.
scripts/validate.py (2)
26-102: LGTM! Consistent validator function pattern.All validator functions follow a consistent pattern:
- Import from omnibase_core with ImportError fallback
- Run validation with infrastructure-appropriate defaults
- Print results with clear PASS/FAIL indicators
- Return boolean for exit code
The defaults align with the infra_validators.py configuration (e.g., max_unions=20).
174-210: LGTM! Clean CLI interface with argparse.The main entry point provides:
- Clear validator choices (all, architecture, contracts, patterns, unions, imports)
- Verbose and quick mode flags
- Standard exit codes (0 for success, 1 for failure)
- Clean dispatch via validator_map
src/omnibase_infra/cli/commands.py (2)
26-107: LGTM! Well-structured CLI commands with consistent patterns.All validation commands follow a consistent pattern:
- Import the appropriate validator
- Execute validation with user-provided or default arguments
- Use Rich for formatted console output
- Exit with appropriate status codes (0 for success, 1 for failure)
The imports command correctly handles the different result type (CircularImportValidationResult) with appropriate property checks.
148-178: LGTM! Type-safe helper functions with proper fallbacks.The helper functions handle both result types (ModelValidationResult and CircularImportValidationResult) using duck typing with hasattr checks. The fail-closed default behavior in
_is_result_valid(returning False) is appropriate for validation tooling.src/omnibase_infra/validation/infra_validators.py (3)
8-25: LGTM! Proper imports and type annotations throughout.All imports correctly reference
omnibase_core(notomnibase.), following the coding guidelines. The module uses specific types throughout with noAnyusage, and all functions have complete type annotations.
189-225: LGTM! Type-safe summary function with proper handling of both result types.The function correctly handles both result types:
- CircularImportValidationResult: checks
has_circular_importsproperty- ModelValidationResult: checks
is_validpropertyThe return type is properly typed as
dict[str, int | list[str]].
228-239: LGTM! Clean public API with appropriate exports.The
__all__definition clearly documents the public API surface:
- Constants for default paths
- Individual validators for each validation type
- Orchestrator function (validate_infra_all)
- Summary generator
All exported names are defined in the module and properly typed.
Complete systematic fix of all 32 review issues (2 critical, 8 major, 2 minor, 20 nitpick). 🔴 CRITICAL FIXES: - Extract INFRA_MAX_UNIONS constant for consistent union type limits - Verify cache key consistency across all CI jobs (already correct) 🟠 MAJOR FIXES: - Enhance exception handling with specific error types and actionable messages - Add validator metadata display for better debugging - Standardize validation parameters (INFRA_MAX_VIOLATIONS, INFRA_PATTERNS_STRICT) - Create comprehensive validator defaults test suite (33 tests) - Refactor to use raise SystemExit() following Click best practices - Document dependency versioning strategy with evidence-based analysis - Remove redundant pip cache from GitHub Actions workflow 🟡 MINOR FIXES: - Improve circular import validator error handling with recovery suggestions - Analyze and document pre-commit CI skip configuration ⚪ NITPICK FIXES: - Add ValidationResult type alias using Python 3.12+ type keyword - Strengthen smoke tests with callability assertions - Consolidate duplicate .gitignore patterns - Refine .env ignore pattern to preserve .env.example - Document security-related ruff ignores with justifications - Add TODO for global ignore_missing_imports removal - Remove empty TYPE_CHECKING block (TC005) - Add ONEX validation job to GitHub Actions CI - Create comprehensive validation framework documentation (5 guides) 📁 FILES MODIFIED (14): - Code: infra_validators.py, test_smoke.py, validate.py, wiring.py - Config: .gitignore, pyproject.toml, test.yml - Tests: test_validator_defaults.py (new) - Docs: 6 validation guides (new) ✅ VALIDATION: - mypy: Success (22 files) - ruff: All checks passed - black: 31 files unchanged - isort: All imports sorted - pytest: 5/5 smoke tests passed - ONEX validators: 5/5 passed All quality gates passing. Production ready.
🔍 PR Review: Validation Framework with omnibase_core IntegrationOverall Assessment: ✅ EXCELLENT - Ready to Merge This PR adds a comprehensive validation framework that properly integrates with 🎯 Strengths✅ Architecture & Design
✅ Code Quality
✅ Testing
✅ CI/CD Integration
✅ Documentation
📋 Code Quality Detailsinfra_validators.py (277 lines)
scripts/validate.py (314 lines)
pyproject.toml
.github/workflows/test.yml
🔒 Security Review✅ No Security Concerns FoundReviewed security-sensitive areas:
🚀 Performance Considerations✅ Well Optimized
Performance metrics:
✅ ONEX CompliancePerfectly Aligned with CLAUDE.md Standards
📊 Test Coverage AnalysisTest files:
Coverage areas:
Missing coverage (acceptable for MVP):
🎯 Recommendations for Future EnhancementsPriority: LOW (current implementation is production-ready)
🎉 SummaryThis is excellent work that sets a solid foundation for ONEX infrastructure development. The validation framework is:
No blocking issues found. Ready to merge. 📝 Commit History QualityAll 6 commits are well-structured with clear messages:
Special recognition for the final commit message - it's a model example of how to document comprehensive changes with clear categorization (CRITICAL/MAJOR/MINOR/NITPICK) and complete file listing. Reviewed by: Claude Code (ONEX Infrastructure Standards) |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
docs/validation/framework_integration.md (1)
224-224: Clarify isinstance usage justification in context of coding guidelines.The coding guidelines state "Use duck typing through protocols, never isinstance," but this line justifies isinstance for result type discrimination. While union type handling for different result models (ValidationResult vs CircularImportValidationResult) may warrant this exception, the justification should be more explicit about why protocols aren't suitable here and when isinstance is acceptable.
Consider expanding this to: "Different validators return different result types with distinct attributes (
is_validvshas_circular_imports). Protocol-based discrimination isn't suitable here because the result types come from the core framework. isinstance check is justified for discriminating between concrete result model types in aggregation scenarios.".github/workflows/test.yml (1)
242-244: Consider performance impact of --verbose flag in CI.The ONEX validators run with
--verboseflag, which may produce excessive output and slow down the CI job, especially as the codebase grows. Consider:
- Using
--verboseonly on failure for debugging- Defaulting to quiet mode with summary-only output
- Adding a conditional flag based on workflow dispatch inputs
Example conditional verbosity:
- - name: Run ONEX validators - run: | - poetry run python scripts/validate.py all --verbose + - name: Run ONEX validators + run: | + poetry run python scripts/validate.py all ${{ github.event_name == 'workflow_dispatch' && '--verbose' || '' }}tests/unit/validation/test_validator_defaults.py (1)
274-330: Refactor brittle script content checks for maintainability.The direct string matching in
scripts/validate.pycontent (lines 280-290, 298-299, 306-313, 320-330) is fragile and will break on formatting changes (whitespace, line breaks, import reordering) even when functionality remains unchanged. This creates maintenance burden and false test failures.Consider these more robust alternatives:
Option 1: Test script behavior instead of content (preferred)
def test_architecture_script_uses_correct_defaults(self) -> None: """Verify architecture validator called with correct defaults.""" with patch('omnibase_infra.validation.validate_architecture') as mock_validate: mock_validate.return_value = MagicMock(is_valid=True, errors=[]) # Import and call the script function from scripts.validate import run_architecture run_architecture(verbose=False) # Verify correct defaults were used mock_validate.assert_called_with( INFRA_SRC_PATH, max_violations=INFRA_MAX_VIOLATIONS )Option 2: Use AST parsing instead of string matching
def test_script_imports_constants(self) -> None: """Verify script imports required constants.""" import ast script_path = Path("scripts/validate.py") tree = ast.parse(script_path.read_text()) imports = {alias.name for node in ast.walk(tree) if isinstance(node, ast.ImportFrom) for alias in node.names} assert 'INFRA_MAX_VIOLATIONS' in importsOption 3: Integration test
def test_validate_script_cli_integration(self) -> None: """Integration test: verify script executes with defaults.""" result = subprocess.run( ["poetry", "run", "python", "scripts/validate.py", "architecture", "--help"], capture_output=True, text=True ) assert result.returncode == 0scripts/validate.py (1)
26-35: Avoid duplicating infra paths; reuseinfra_validatorsconstants/wrappers.All the
run_*helpers hard-code"src/omnibase_infra/"and"src/omnibase_infra/nodes/"instead of usingINFRA_SRC_PATH/INFRA_NODES_PATH(and, where appropriate, thevalidate_infra_*wrappers you already defined). This makes the script slightly brittle if the default locations ever change.Consider refactoring along these lines to centralize defaults:
-from omnibase_core.validation import validate_architecture -from omnibase_infra.validation.infra_validators import INFRA_MAX_VIOLATIONS +from omnibase_infra.validation.infra_validators import ( + INFRA_SRC_PATH, + INFRA_NODES_PATH, + INFRA_MAX_VIOLATIONS, + INFRA_PATTERNS_STRICT, + INFRA_MAX_UNIONS, + INFRA_UNIONS_STRICT, + validate_infra_architecture, + validate_infra_contracts, + validate_infra_patterns, + validate_infra_union_usage, +) ... - result = validate_architecture( - "src/omnibase_infra/", max_violations=INFRA_MAX_VIOLATIONS - ) + result = validate_infra_architecture( + INFRA_SRC_PATH, max_violations=INFRA_MAX_VIOLATIONS + )and similarly for contracts/patterns/unions/imports. That keeps the script behavior aligned with the infra wrappers and tests as paths evolve.
Also applies to: 52-63, 81-89, 106-120, 137-140
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (16)
.github/workflows/test.yml(1 hunks).gitignore(1 hunks)docs/validation/README.md(1 hunks)docs/validation/circular_import_validator_improvements.md(1 hunks)docs/validation/framework_integration.md(1 hunks)docs/validation/performance_notes.md(1 hunks)docs/validation/troubleshooting.md(1 hunks)docs/validation/validator_reference.md(1 hunks)pyproject.toml(4 hunks)scripts/validate.py(1 hunks)src/omnibase_infra/cli/commands.py(1 hunks)src/omnibase_infra/runtime/wiring.py(1 hunks)src/omnibase_infra/validation/infra_validators.py(1 hunks)tests/unit/test_smoke.py(1 hunks)tests/unit/validation/__init__.py(1 hunks)tests/unit/validation/test_validator_defaults.py(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- docs/validation/performance_notes.md
- tests/unit/validation/init.py
🚧 Files skipped from review as they are similar to previous changes (1)
- .gitignore
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: NEVER useAny- Always use specific types. Use Pydantic Models for all data structures as proper Pydantic models.
Use CamelCase for model class names:ModelUserData. All model classes follow CamelCase naming pattern starting with 'Model' prefix.
Use snake_case for all Python filenames:model_user_data.py. All filenames must use lowercase with underscores.
Each file contains exactly oneModel*class. One model per file pattern for all Pydantic models.
All tools/services follow contract-driven patterns. Use Pydantic Models as proper data structures following contract definitions.
All dependencies must be injected via container:def __init__(self, container: ONEXContainer). Use container injection pattern for dependency management.
Use duck typing through protocols, never isinstance. Protocol resolution for dependency resolution instead of type checking.
All exceptions converted to OnexError with chaining:raise OnexError(...) from e. Use OnexError only for exception handling.
Update all import references fromomnibase.toomnibase_core.and fromomnibase.exceptionstoomnibase_core.exceptions. Update all imports:omnibase.enumstoomnibase_core.enums.
Files:
tests/unit/test_smoke.pytests/unit/validation/test_validator_defaults.pyscripts/validate.pysrc/omnibase_infra/cli/commands.pysrc/omnibase_infra/runtime/wiring.pysrc/omnibase_infra/validation/infra_validators.py
🧠 Learnings (29)
📓 Common learnings
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/canonical_patterns.mdc:0-0
Timestamp: 2025-11-28T18:58:53.781Z
Learning: All code must pass pre-commit validation hooks including string version detection, backward compatibility checks, fallback pattern removal, single class per file, error raising validation, Pydantic pattern validation, union usage validation, and enum/model import prevention
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/canonical_patterns.mdc:0-0
Timestamp: 2025-11-28T18:58:53.781Z
Learning: Organize models under `src/omnibase_core/models/` by domain including: base, cli, common, config, core, contracts, discovery, health, infrastructure, logging, metadata, nodes, operations, results, security, service, tools, validation, and workflows
Learnt from: CR
Repo: OmniNode-ai/omnibase_core PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T21:48:22.867Z
Learning: Applies to src/omnibase_core/models/**/*.py : Use Pydantic models for data validation and serialization - leverage Pydantic 2.11+ features for type safety
📚 Learning: 2025-11-24T17:23:49.777Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/node_standards.mdc:0-0
Timestamp: 2025-11-24T17:23:49.777Z
Learning: Applies to **/node_*/v[0-9]*_[0-9]*_[0-9]*/SCHEMA_DECISIONS.md : Each versioned ONEX node implementation directory must include a `SCHEMA_DECISIONS.md` file documenting schema-specific design decisions, implementation notes, and validation strategies
Applied to files:
docs/validation/README.mddocs/validation/validator_reference.mddocs/validation/troubleshooting.md
📚 Learning: 2025-12-03T16:55:49.755Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T16:55:49.755Z
Learning: Applies to config/README.md : Document all configuration variables in config/README.md with type information, validation rules, and usage examples
Applied to files:
docs/validation/README.md
📚 Learning: 2025-11-24T17:23:49.777Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/node_standards.mdc:0-0
Timestamp: 2025-11-24T17:23:49.777Z
Learning: All ONEX node deviations from canonical patterns must be documented and justified in the node's root-level README.md and subject to maintainer review
Applied to files:
docs/validation/README.md
📚 Learning: 2025-11-24T17:23:49.777Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/node_standards.mdc:0-0
Timestamp: 2025-11-24T17:23:49.777Z
Learning: Applies to **/node_*/README.md : All ONEX nodes must include a `README.md` file at the node root directory level with node-level documentation (required)
Applied to files:
docs/validation/README.md
📚 Learning: 2025-11-24T17:23:49.777Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/node_standards.mdc:0-0
Timestamp: 2025-11-24T17:23:49.777Z
Learning: Applies to **/node_*/v[0-9]*_[0-9]*_[0-9]*/contracts/contract_*.yaml : All ONEX node contract definitions must use the new subcontract architecture pattern, breaking down complex contracts into separate contract_actions.yaml, contract_models.yaml, contract_validation.yaml, and optional contract_cli.yaml and contract_capabilities.yaml files for separation of concerns, maintainability, reusability, modularity, and future tool-as-a-service readiness
Applied to files:
docs/validation/validator_reference.mdscripts/validate.py
📚 Learning: 2025-12-03T03:23:43.660Z
Learnt from: CR
Repo: OmniNode-ai/omniintelligence PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T03:23:43.660Z
Learning: Applies to tests/**/*.py : Organize tests into unit tests (no infrastructure), integration tests (requires Kafka and databases), and node-specific tests with shared fixtures for Kafka mocks, sample data, correlation IDs, and intelligence client mocks
Applied to files:
tests/unit/test_smoke.py
📚 Learning: 2025-10-14T12:06:38.965Z
Learnt from: jonahgabriel
Repo: OmniNode-ai/omninode_bridge PR: 0
File: :0-0
Timestamp: 2025-10-14T12:06:38.965Z
Learning: In pyproject.toml for OmniNode Bridge: Dev dependencies are pytest ^8.4.0, pytest-asyncio ^0.25.0, mypy ^1.13.0, black ^24.10.0, and ruff ^0.8.0, all compatible with Python 3.12.
Applied to files:
pyproject.toml
📚 Learning: 2025-10-14T12:06:38.965Z
Learnt from: jonahgabriel
Repo: OmniNode-ai/omninode_bridge PR: 0
File: :0-0
Timestamp: 2025-10-14T12:06:38.965Z
Learning: In pyproject.toml for OmniNode Bridge: Core dependencies are pydantic ^2.11.7, fastapi ^0.115.0, uvicorn ^0.32.0, asyncpg ^0.29.0, and redis ^6.0.0 (for Redis/Valkey compatibility).
Applied to files:
pyproject.toml
📚 Learning: 2025-12-03T22:36:46.869Z
Learnt from: CR
Repo: OmniNode-ai/omnibase_infra PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T22:36:46.869Z
Learning: Applies to **/*.py : Update all import references from `omnibase.` to `omnibase_core.` and from `omnibase.exceptions` to `omnibase_core.exceptions`. Update all imports: `omnibase.enums` to `omnibase_core.enums`.
Applied to files:
pyproject.toml
📚 Learning: 2025-10-14T12:06:38.965Z
Learnt from: jonahgabriel
Repo: OmniNode-ai/omninode_bridge PR: 0
File: :0-0
Timestamp: 2025-10-14T12:06:38.965Z
Learning: OmniNode Bridge project uses Python ^3.12 as the baseline version, upgraded from 3.11 for performance improvements, enhanced type system, and better asyncio support.
Applied to files:
pyproject.toml
📚 Learning: 2025-11-24T17:23:49.777Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/node_standards.mdc:0-0
Timestamp: 2025-11-24T17:23:49.777Z
Learning: Applies to **/node_*/v[0-9]*_[0-9]*_[0-9]*/*.py : All ONEX node implementations must follow dependency injection and protocol-first design patterns as established in the node_cli canonical reference
Applied to files:
pyproject.toml
📚 Learning: 2025-11-24T17:22:32.195Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/canonical_patterns.mdc:0-0
Timestamp: 2025-11-24T17:22:32.195Z
Learning: Applies to **/v[0-9]_[0-9]_[0-9]/{tools,registry}/**.py : Constructor-based dependency injection must be used for all tool and node classes; validate that required dependencies are not None, raising OnexError with specific error code if missing
Applied to files:
pyproject.toml
📚 Learning: 2025-12-03T22:36:46.869Z
Learnt from: CR
Repo: OmniNode-ai/omnibase_infra PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T22:36:46.869Z
Learning: Applies to **/*.py : All dependencies must be injected via container: `def __init__(self, container: ONEXContainer)`. Use container injection pattern for dependency management.
Applied to files:
pyproject.toml
📚 Learning: 2025-11-25T21:48:22.867Z
Learnt from: CR
Repo: OmniNode-ai/omnibase_core PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T21:48:22.867Z
Learning: Applies to **/*.py : Always use 100% strict mypy type checking - all functions must have type annotations and no untyped definitions are allowed
Applied to files:
pyproject.toml
📚 Learning: 2025-11-25T21:48:22.867Z
Learnt from: CR
Repo: OmniNode-ai/omnibase_core PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T21:48:22.867Z
Learning: Applies to **/*.py : Run code through black formatter, isort for import sorting, and ruff linter before committing
Applied to files:
pyproject.toml
📚 Learning: 2025-11-24T17:24:10.209Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/pr.mdc:0-0
Timestamp: 2025-11-24T17:24:10.209Z
Learning: Applies to **/*.py : Automatically perform code standards checks on all Python files in the PR, including: Any/Any imports and Dict[str, Any] usage violations, naming convention violations (tool_, model_, enum_ prefixes), anti-pattern detection (direct tool instantiation, telescoping constructors), and type safety enforcement (strongest typing possible). Fix all violations immediately with proper commit messages.
Applied to files:
pyproject.toml
📚 Learning: 2025-12-03T03:23:43.660Z
Learnt from: CR
Repo: OmniNode-ai/omniintelligence PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T03:23:43.660Z
Learning: Applies to src/**/*.py : Run code quality checks using `ruff check src tests`, `ruff check --fix src tests` for auto-fix, `black src tests` for formatting, `isort src tests` for import sorting, and `mypy src` for type checking
Applied to files:
pyproject.toml
📚 Learning: 2025-12-03T16:55:49.755Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T16:55:49.755Z
Learning: Applies to **/*.py : Use type-safe configuration via Pydantic Settings from config/settings.py with 90+ type-safe variables organized into External Service Discovery, Shared Infrastructure, AI Provider API Keys, Local Services, and Feature Flags
Applied to files:
pyproject.toml
📚 Learning: 2025-11-29T17:13:38.776Z
Learnt from: CR
Repo: OmniNode-ai/omniarchon PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-29T17:13:38.776Z
Learning: Applies to {**/*.py,!docs/**,!scripts/examples/**} : Ensure 100% test coverage for production code, with fail-closed security configuration as documented in `IMPROVEMENTS.md`.
Applied to files:
pyproject.toml
📚 Learning: 2025-12-03T03:23:43.660Z
Learnt from: CR
Repo: OmniNode-ai/omniintelligence PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T03:23:43.660Z
Learning: Applies to tests/**/*.py : Use pytest markers: `pytest.mark.unit` for unit tests, `pytest.mark.integration` for integration tests, `pytest.mark.slow` for slow tests, and `pytest.mark.performance` for performance benchmarks
Applied to files:
pyproject.toml
📚 Learning: 2025-11-29T17:13:38.776Z
Learnt from: CR
Repo: OmniNode-ai/omniarchon PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-29T17:13:38.776Z
Learning: Applies to **/*.py : NO environment variables hardcoded in code. ALL configuration must be provided via `.env` file. Use Pydantic Settings with `BaseSettings` and `Field` with `env` parameter for configuration management.
Applied to files:
pyproject.toml
📚 Learning: 2025-12-03T22:36:46.869Z
Learnt from: CR
Repo: OmniNode-ai/omnibase_infra PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-03T22:36:46.869Z
Learning: Applies to **/*.py : All tools/services follow contract-driven patterns. Use Pydantic Models as proper data structures following contract definitions.
Applied to files:
pyproject.toml
📚 Learning: 2025-11-29T17:13:38.776Z
Learnt from: CR
Repo: OmniNode-ai/omniarchon PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-29T17:13:38.776Z
Learning: Applies to scripts/verify_environment.py : Run `python3 scripts/verify_environment.py --verbose` before deployment to validate 9 critical environment checks.
Applied to files:
scripts/validate.py
📚 Learning: 2025-11-28T18:58:53.781Z
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/canonical_patterns.mdc:0-0
Timestamp: 2025-11-28T18:58:53.781Z
Learning: Applies to **/*.py : Do not use bare `except:` without re-raise
Applied to files:
scripts/validate.py
📚 Learning: 2025-11-28T18:58:53.781Z
Learnt from: CR
Repo: OmniNode-ai/omninode_bridge PR: 0
File: .cursor/rules/canonical_patterns.mdc:0-0
Timestamp: 2025-11-28T18:58:53.781Z
Learning: Applies to **/*.py : Do not use `except ValueError: ... = Enum.UNKNOWN` patterns; raise explicit errors instead
Applied to files:
scripts/validate.py
📚 Learning: 2025-11-24T17:22:32.195Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/canonical_patterns.mdc:0-0
Timestamp: 2025-11-24T17:22:32.195Z
Learning: Applies to **/cli_tools/*/v[0-9]_[0-9]_[0-9]/commands/*.py : CLI commands must use Click framework with error handling: catch OnexError and exit with status code 1
Applied to files:
src/omnibase_infra/cli/commands.py
📚 Learning: 2025-11-24T17:23:49.777Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/node_standards.mdc:0-0
Timestamp: 2025-11-24T17:23:49.777Z
Learning: Applies to **/node_*/v[0-9]*_[0-9]*_[0-9]*/contracts/contract_cli.yaml : All ONEX node CLI interface definitions, if applicable, must be included in contract_cli.yaml with entrypoint and commands specifications
Applied to files:
src/omnibase_infra/cli/commands.py
📚 Learning: 2025-11-24T17:23:49.777Z
Learnt from: CR
Repo: OmniNode-ai/omniclaude PR: 0
File: .cursor/rules/node_standards.mdc:0-0
Timestamp: 2025-11-24T17:23:49.777Z
Learning: All ONEX nodes must conform to the canonical structure, code generation, and interface patterns established in the `node_cli` node, using it as the primary source of truth for directory structure, contract schema patterns, linked document architecture, base state patterns, shared schema references, extensibility patterns, CLI interface declarations, code generation, dependency injection, error handling, testing, and documentation
Applied to files:
src/omnibase_infra/cli/commands.py
🧬 Code graph analysis (4)
tests/unit/test_smoke.py (2)
src/omnibase_infra/cli/commands.py (1)
cli(17-18)src/omnibase_infra/validation/infra_validators.py (4)
validate_infra_all(180-212)validate_infra_architecture(60-76)validate_infra_contracts(79-93)validate_infra_patterns(96-115)
scripts/validate.py (1)
src/omnibase_infra/cli/commands.py (1)
validate(22-23)
src/omnibase_infra/cli/commands.py (1)
src/omnibase_infra/validation/infra_validators.py (7)
validate_infra_architecture(60-76)validate_infra_contracts(79-93)validate_infra_patterns(96-115)validate_infra_union_usage(139-157)validate_infra_circular_imports(160-177)get_validation_summary(215-255)validate_infra_all(180-212)
src/omnibase_infra/validation/infra_validators.py (1)
src/omnibase_infra/cli/commands.py (1)
validate(22-23)
🔇 Additional comments (10)
src/omnibase_infra/runtime/wiring.py (1)
1-44: LGTM! Well-documented placeholder module.The docstring clearly outlines the module's future purpose, responsibilities, design principles, handler categories, and integration points. The
from __future__ import annotationsimport on line 39 is appropriate for when type hints are added during implementation.docs/validation/README.md (1)
1-222: LGTM! Comprehensive validation framework documentation.The documentation clearly covers quick start, available validators with priorities, validation results structure, CI/CD integration, configuration options, and practical examples. The organization by priority (HIGH/MEDIUM) helps users understand criticality, and the justifications for infrastructure-specific settings (e.g., max_unions=20 for typed handlers) provide valuable context.
.github/workflows/test.yml (1)
44-46: LGTM! Cache key consistency issue resolved.The cache key format is now consistent across all jobs (smoke-test, test, lint, onex-validation), with no space before
${{ hashFiles('**/poetry.lock') }}. The restore-keys pattern correctly matches the primary key format, ensuring proper cache restoration.Also applies to: 102-104, 165-167, 223-225
docs/validation/circular_import_validator_improvements.md (1)
1-154: LGTM! Clear documentation of error handling improvements.The documentation effectively describes the enhanced error detection, actionable error messages, improved output display for verbose/non-verbose modes, and comprehensive error handling coverage. The concrete examples for each error scenario (configuration, missing dependencies, API incompatibility, permissions, unexpected errors) with corresponding fixes provide excellent troubleshooting guidance.
docs/validation/troubleshooting.md (1)
1-647: LGTM! Excellent troubleshooting guide with actionable solutions.The guide comprehensively covers all major issue categories (import errors, validation failures, performance issues, CI/CD issues, integration issues) with clear symptom descriptions, root cause analysis, concrete solutions, and prevention strategies. The inclusion of specific commands, configuration examples, and restructuring guidance makes this highly practical for developers encountering validation issues.
docs/validation/validator_reference.md (1)
1-628: LGTM! Comprehensive validator reference documentation.The reference provides detailed documentation for all eight validators (validate_infra_architecture, validate_infra_contracts, validate_infra_patterns, validate_infra_contract_deep, validate_infra_union_usage, validate_infra_circular_imports, validate_infra_all, get_validation_summary) with clear signatures, parameter descriptions, return types, validation coverage, common violations with fixes, example usage, and CI/CD integration. The constants reference section at the end serves as a helpful quick reference.
tests/unit/validation/test_validator_defaults.py (1)
1-507: Strong test coverage for default parameter consistency.The test suite comprehensively validates that default parameters are consistent across function signatures, CLI commands, and script usage. The mock-based tests (lines 81-93, 107-116, 137-148, 176-188, 206-218, 241-268) effectively verify correct parameter propagation without executing actual validation logic. The consistency checks (lines 419-507) ensure all entry points use the same defaults, which is critical for a consistent user experience.
pyproject.toml (1)
20-22: Tooling and validation config looks consistent.The dependency pins, Ruff/mypy/pytest settings, isort
known_first_party=["omnibase_infra"], and coverage config all line up and match the intended ONEX ecosystem standards. The globalignore_missing_imports = trueis clearly marked as temporary with per-module overrides in place, which is a reasonable transitional setup.If you haven’t already, please run the full toolchain (
poetry lock,ruff,mypy,pytest) once more on this branch to confirm there are no version- or config-specific surprises withomnibase-core==0.3.5/omnibase-spi==0.2.0under Python 3.12.Also applies to: 26-26, 79-80, 97-105, 108-200, 208-222, 225-237, 239-247, 253-261, 262-289
tests/unit/test_smoke.py (1)
1-115: Smoke tests correctly cover import surface and infra defaults.The smoke suite is lightweight, fast, and validates the expected package/CLI/validation importability plus the infra path constants. The explicit string checks for
INFRA_SRC_PATH/INFRA_NODES_PATHare acceptable here given the “fail fast on structure changes” intent documented in the test.src/omnibase_infra/validation/infra_validators.py (1)
32-77: Infra validator wrappers and defaults are well-structured and consistent.The infra-specific wrappers correctly centralize defaults (
INFRA_SRC_PATH,INFRA_NODES_PATH, union/violation limits, strict flags) and delegate cleanly to the core validators.validate_infra_allandget_validation_summaryprovide a clear, typed orchestration layer that the CLI and scripts can depend on, and the updated docstrings now match the actual defaults (e.g., union max of 20).Also applies to: 79-157, 160-213, 215-277
| Provides CLI interface for infrastructure management and validation. | ||
| """ | ||
|
|
||
| from typing import Any |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the overall structure of the file
wc -l src/omnibase_infra/cli/commands.pyRepository: OmniNode-ai/omnibase_infra
Length of output: 108
🏁 Script executed:
# Inspect the import section (lines 1-20)
head -20 src/omnibase_infra/cli/commands.py | cat -nRepository: OmniNode-ai/omnibase_infra
Length of output: 515
🏁 Script executed:
# Inspect the functions mentioned around lines 7, 181-201
sed -n '1,50p' src/omnibase_infra/cli/commands.py | cat -nRepository: OmniNode-ai/omnibase_infra
Length of output: 1787
🏁 Script executed:
# Inspect lines 175-210 where the function definitions should be
sed -n '175,210p' src/omnibase_infra/cli/commands.py | cat -nRepository: OmniNode-ai/omnibase_infra
Length of output: 1612
🏁 Script executed:
# Check for the infra_validators module
find src/omnibase_infra -name "*validator*" -type fRepository: OmniNode-ai/omnibase_infra
Length of output: 119
🏁 Script executed:
# Check the infra_validators module to see available classes and types
head -100 src/omnibase_infra/validation/infra_validators.py | cat -nRepository: OmniNode-ai/omnibase_infra
Length of output: 4607
🏁 Script executed:
# Search for class definitions and return types in infra_validators
rg "^class |^def " src/omnibase_infra/validation/infra_validators.py -A 2Repository: OmniNode-ai/omnibase_infra
Length of output: 1071
🏁 Script executed:
# Search for ValidationResult and CircularImportValidationResult types
rg "ValidationResult|CircularImportValidationResult" src/omnibase_infra/validation/infra_validators.pyRepository: OmniNode-ai/omnibase_infra
Length of output: 1444
🏁 Script executed:
# Check all imports in commands.py to see if infra_validators is already imported
grep -n "import\|from" src/omnibase_infra/cli/commands.py | head -20Repository: OmniNode-ai/omnibase_infra
Length of output: 1143
🏁 Script executed:
# Check the actual line numbers of the functions to confirm comment's "lines 181-201"
grep -n "def _is_result_valid\|def _get_error_count\|def _print_result" src/omnibase_infra/cli/commands.pyRepository: OmniNode-ai/omnibase_infra
Length of output: 220
🏁 Script executed:
# Check if there are any circular import patterns (commands.py loaded by validators)
rg "from.*commands|import.*commands" src/omnibase_infra/validation/Repository: OmniNode-ai/omnibase_infra
Length of output: 52
Replace Any in helper signatures with concrete validation result types.
Per the repo guidelines ("NEVER use Any"), the Any import on line 7 and its usage in _get_error_count (line 190) and _print_result (line 201) should be replaced with specific types from the infra validators:
-from typing import Any
+from typing import Union
+
+from omnibase_infra.validation.infra_validators import (
+ ValidationResult,
+ CircularImportValidationResult,
+)
+
+ResultType = Union[ValidationResult, CircularImportValidationResult]Then update the function signatures:
-def _get_error_count(result: Any) -> int:
+def _get_error_count(result: ResultType) -> int:-def _print_result(name: str, result: Any) -> None:
+def _print_result(name: str, result: ResultType) -> None:The duck-typing logic (hasattr checks) remains unchanged; this only adds type information for static analysis. The union accurately reflects that these helpers accept either validation result type.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/omnibase_infra/cli/commands.py around line 7 (and usages at lines ~190
and ~201), replace the typing.Any import with concrete validation result types
from infra.validators (import the two result classes used by the validators) and
update the helper function signatures for _get_error_count and _print_result to
accept a Union of those two specific result types instead of Any; keep the
existing duck-typing (hasattr) checks intact so only the static type annotations
change, and update the import statement and function annotations accordingly so
flake/typing checks no longer use Any.
| @validate.command("architecture") | ||
| @click.argument("directory", default="src/omnibase_infra/") | ||
| @click.option( | ||
| "--max-violations", | ||
| default=None, | ||
| help="Maximum allowed violations (default: INFRA_MAX_VIOLATIONS)", | ||
| ) | ||
| def validate_architecture_cmd(directory: str, max_violations: int | None) -> None: |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/omnibase_infra/cli/commands.py | head -150Repository: OmniNode-ai/omnibase_infra
Length of output: 6324
🏁 Script executed:
cat -n src/omnibase_infra/cli/commands.py | tail -n +150Repository: OmniNode-ai/omnibase_infra
Length of output: 2741
🏁 Script executed:
rg "def validate_infra_architecture|def validate_infra_union_usage" src/omnibase_infra/validation/ -A 3Repository: OmniNode-ai/omnibase_infra
Length of output: 776
🏁 Script executed:
# Verify Click's actual behavior without type=int by checking Click documentation
# and testing what type Click passes when no type is specified
python3 << 'EOF'
import click
# Simulate what Click does without type=int
# When no type is specified, Click treats the value as a string
@click.command()
@click.option("--max-violations", default=None)
def test_cmd(max_violations):
print(f"Type: {type(max_violations)}, Value: {max_violations!r}")
# This demonstrates Click's default behavior
ctx = click.Context(test_cmd)
print("Click without type=int specification:")
print("- Parses arguments as strings by default")
print("- User input '--max-violations 5' becomes string '5'")
print("- This would cause TypeError when passed to int-expecting functions")
EOFRepository: OmniNode-ai/omnibase_infra
Length of output: 156
Add type=int to numeric Click options to avoid passing strings to validators.
Both --max-violations and --max-unions options lack type=int specification. Without it, Click will parse user-supplied values as strings and pass them to validate_infra_architecture() and validate_infra_union_usage(), which expect int parameters. This causes a TypeError at runtime.
Apply to lines 28-32 and 86-90:
@click.option(
"--max-violations",
+ type=int,
default=None,
help="Maximum allowed violations (default: INFRA_MAX_VIOLATIONS)",
) @click.option(
"--max-unions",
+ type=int,
default=None,
help="Maximum allowed complex unions (default: INFRA_MAX_UNIONS)",
)Also remove Any imports and type hints from helper functions (lines 7, 190, 201). Replace with a concrete Protocol or union of result types to comply with the "NEVER use Any" guideline.
🤖 Prompt for AI Agents
In src/omnibase_infra/cli/commands.py around lines 26-33 and 86-90, the Click
numeric options --max-violations and --max-unions are missing type=int so user
input is passed as strings; update both click.option declarations to include
type=int so validators receive ints. Also in the same file remove the Any import
and replace Any type hints on helper functions at lines 7, 190, and 201 with a
concrete Protocol or a specific union of result types (define a small Protocol
or explicit result dataclass/union to represent the expected return shape) and
update the function signatures to use that concrete type instead of Any.
| @validate.command("imports") | ||
| @click.argument("directory", default="src/omnibase_infra/") | ||
| def validate_imports_cmd(directory: str) -> None: | ||
| """Check for circular imports.""" | ||
| from omnibase_infra.validation.infra_validators import ( | ||
| validate_infra_circular_imports, | ||
| ) | ||
|
|
||
| console.print(f"[bold blue]Checking circular imports in {directory}...[/bold blue]") | ||
| result = validate_infra_circular_imports(directory) | ||
|
|
||
| # CircularImportValidationResult uses has_circular_imports property | ||
| if not result.has_circular_imports: | ||
| console.print("[bold green]Circular Imports: PASS[/bold green]") | ||
| raise SystemExit(0) | ||
| console.print("[bold red]Circular Imports: FAIL[/bold red]") | ||
| if hasattr(result, "cycles") and result.cycles: | ||
| for cycle in result.cycles: | ||
| console.print(f" [red]Cycle: {cycle}[/red]") | ||
| if hasattr(result, "errors") and result.errors: | ||
| for error in result.errors: | ||
| console.print(f" [red]{error}[/red]") | ||
| raise SystemExit(1) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "*model*import*validation*" -o -name "*validation*result*" | head -20Repository: OmniNode-ai/omnibase_infra
Length of output: 52
🏁 Script executed:
fd -e py . | xargs grep -l "CircularImportValidationResult" | head -10Repository: OmniNode-ai/omnibase_infra
Length of output: 154
🏁 Script executed:
rg "class CircularImportValidationResult" -A 30Repository: OmniNode-ai/omnibase_infra
Length of output: 52
🏁 Script executed:
cat -n src/omnibase_infra/validation/infra_validators.py | head -50Repository: OmniNode-ai/omnibase_infra
Length of output: 2597
🏁 Script executed:
rg "CircularImportValidationResult" -B 5 -A 5Repository: OmniNode-ai/omnibase_infra
Length of output: 14580
🏁 Script executed:
cat -n scripts/validate.py | grep -A 20 -B 5 "circular_imports\|import_errors\|unexpected_errors"Repository: OmniNode-ai/omnibase_infra
Length of output: 3827
🏁 Script executed:
cat -n src/omnibase_infra/cli/commands.py | sed -n '117,139p'Repository: OmniNode-ai/omnibase_infra
Length of output: 1249
🏁 Script executed:
rg -A 10 "if hasattr.*cycles" src/omnibase_infra/cli/commands.pyRepository: OmniNode-ai/omnibase_infra
Length of output: 825
Fix circular-import field names in CLI to match the actual model.
The CLI uses incorrect attribute names that will never match the model fields. Update validate_imports_cmd (lines 133–138) to use the canonical fields from CircularImportValidationResult:
- Replace
result.cycleswithresult.circular_imports - Replace generic
result.errorswithresult.import_errorsandresult.unexpected_errors
The hasattr() checks currently mask this issue—the fields won't be found, so users see no diagnostic details from the CLI despite the model providing them. Align with scripts/validate.py (lines 161–193) which correctly uses circular_imports, import_errors, and unexpected_errors.
🤖 Prompt for AI Agents
In src/omnibase_infra/cli/commands.py around lines 117–139, the CLI is printing
cycle and error details using incorrect field names (result.cycles and
result.errors) that don't exist on CircularImportValidationResult; update the
code to use the actual fields: iterate result.circular_imports (instead of
result.cycles) to print each cycle, and iterate result.import_errors and
result.unexpected_errors (instead of result.errors) to print diagnostics; keep
the presence checks (e.g., if result.circular_imports) or hasattr checks but
target the correct attribute names so users see the real diagnostic output
before exiting.
1 participant
Summary
omni-infra validate)Changes
Validation Module (
src/omnibase_infra/validation/)CLI Commands (
src/omnibase_infra/cli/)Pre-commit Hooks (
.pre-commit-config.yaml)Matches omnibase_core configuration:
Standalone Script (
scripts/validate.py)python scripts/validate.py --verbose python scripts/validate.py --quick # Skip medium priorityTest plan
poetry run pre-commit run --all-filessucceedsSummary by CodeRabbit
New Features
Chores
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.