Repository navigation
docs: comprehensive documentation audit with 141 files reviewed - #509
Merged
Merged
Conversation
added 15 commits
February 14, 2026 08:49
…iewed Parallel audit of all docs/ using 8 agents identified: - 15 files requiring full rewrite (wrong architecture patterns) - 4 files requiring deletion (stale/duplicate) - 62 files requiring updates - 42 files clean Key findings: entire onboarding path teaches imperative patterns instead of handler-based architecture. ModelHandlerOutput constraints undocumented. Threading doc contradicts stateless node architecture. ADR-006 and ADR-013 actively conflict on canonical enums. Includes severity rubric, source-of-truth hierarchy, handler architecture reference spec, example policy, CI automation recommendations, golden path acceptance test, and risk analysis.
…lines Comprehensive documentation overhaul based on DOCS_AUDIT_REPORT.md findings across 141 audited files. Aligns all docs with handler architecture, contract-driven patterns, and current codebase standards. Rewrites (15 files): - FIRST_NODE.md, QUICK_START.md: handler architecture tutorials - Templates (COMPUTE, EFFECT, ORCHESTRATOR): thin shell + handler + contract - PATTERNS_CATALOG.md: 10 handler-based patterns - ANTI_PATTERNS.md: expanded from 1 to 10 anti-patterns - EVENT_DRIVEN_ARCHITECTURE.md: DLQ, idempotency, backpressure - CONTRACT_SYSTEM.md, DEPENDENCY_INJECTION.md, TYPE_SYSTEM.md: modern patterns - THREADING.md: 2028→399 lines, async/event-bus model - API_DOCUMENTATION.md: rewritten from current source Deletions (7 files): - ADR-013 (conflicted with ADR-006), ENHANCED_NODE_PATTERNS.md - ONEX_MIXIN_ARCHITECTURE_PATTERNS.md + 2 RAG artifacts - ONEX_MIXIN_SYSTEM_RESEARCH.md, INHERITANCE_MODES_VALIDATION_RULES.md New (1 file): - HANDLER_ARCHITECTURE.md: canonical handler reference guide Updates (79 files): - Handler delegation in all node examples - ModelHandlerOutput constraints tables in tutorials/templates - PEP 604 throughout (Optional→X|None, Dict→dict, List→list) - black/isort→ruff, ModelContainer→ModelONEXContainer - Fixed fabricated class names, stale versions, broken links - 21-directory naming convention table in NAMING_CONVENTIONS.md - Historical banners on stale migration/CI docs - ADR-006 declared canonical after ADR-013 deletion
…s, stale references - Fix 4 orchestrator examples returning result (forbidden by architecture) - Fix wrong import path in ANTI_PATTERNS.md container example - Fix 3 case-mismatch links (INSTALLATION.md → installation.md) - Remove stale reference to legacy imperative nodes (removed in v0.4.0) - Fix ModelAction description from "state transitions" to "workflow commands"
- Fix wrong ModelContainer import path in 08_COMMON_PITFALLS.md - Clarify orchestrator Quick Reference stub to use events/intents, not result
…ncorrect claims - Fix models.handler -> models.dispatch import path (6 occurrences, 5 files) - Fix models.event -> models.events import path (2 files) - Fix models.model_event_envelope -> models.events.model_event_envelope (2 files) - Replace non-existent ModelEvent class with ModelEventEnvelope (6 occurrences) - Fix broken test: pytest.raises(ValueError) -> pytest.raises(ModelOnexError) - Fix incorrect claim "Only orchestrators can publish events" (EFFECT can too)
…odelOnexError - Add input_envelope_id/correlation_id to ModelHandlerOutput.for_*() examples - Fix ValueError references to ModelOnexError in constraint violation docs - 10 files with missing params, 2 files with wrong error type
…eError refs - Fix models.model_event_envelope -> models.events.model_event_envelope (2 files) - Replace ModelEvent with ModelEventEnvelope in THREADING.md (3 occurrences) - Fix ValueError -> ModelOnexError for constraint violations (6 files)
…all for_*() calls - Add required input_envelope_id and correlation_id params to ModelHandlerOutput factory calls across 10 documentation files - Update handler signatures to receive envelope parameter where needed - Fix accessor patterns to use envelope.metadata.envelope_id
…rs, handler pattern violations - Replace EnumIntentType/EnumIntentPriority phantom enums with string literals - Fix ModelIntent import path to omnibase_core.models.reducer.model_intent - Add input_envelope_id/correlation_id params to all handler examples - Refactor node-level business logic to handler pattern - Replace FSMTransitionResult with ModelFSMTransitionResult - Fix Pydantic v1 Config to v2 model_config = ConfigDict(...) - Convert events/intents list syntax to tuple syntax - Add handler_id to all for_*() factory calls
…mports and undefined vars - Refactor OrderProcessingReducerService and IntentExecutorEffectService from node-level business logic to handler delegation pattern - Fix ModelOrchestratorInput import to orchestrator subpackage - Fix ModelAction/EnumActionType import paths - Add input_envelope_id/correlation_id params to all template handlers - Add handler_id to all for_*() calls in templates - Fix ProtocolDatabase -> ProtocolDatabaseConnection (actual class) - Fix ModelComputeOutput/ModelComputeCache import paths - Remove dead ModelWorkflowDefinition import
…tutorial banners - Add input_envelope_id/correlation_id to handler signatures in tutorials - Add tutorial simplification banners before node-level logic sections - Add DI comments for direct handler instantiation in tutorials - Fix template test calls to include required handler params - Fix api/nodes.md handler examples with proper param signatures - Remove unused EnumNodeKind import - Add handler_id to orchestrator tutorial for_orchestrator() call
… phantom imports/methods
… methods, fix tuple syntax
… hook consumer attribution - Remove entire Bridge Repository section from ECOSYSTEM_DIRECTORY_STRUCTURE.md - Remove Bridge Pattern code examples and Bridge Mixins section - Fix dependency graph: omninode_bridge → omnibase_infra - Make omniintelligence section conservatively generic (unverifiable tree removed) - Fix CLAUDE_CODE_HOOKS.md: hook models consumed by omniclaude, not omniintelligence - Fix source_node prefix in mixin_intent_publisher.py: omninode_bridge → omnibase_core - Extract ProtocolKafkaClient to own file (single-class-per-file compliance) - Fix comment in model_projection_base.py: omninode_bridge → omnibase_infra
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Documentation Audit Report and shifts documentation to a handler-centric, contract-driven architecture: node examples become thin shells delegating to handlers, ModelHandlerOutput per-node constraints are formalized, typing and DI guidance modernized, legacy mixin research assets removed; all changes are documentation-only. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Node as Thin Node Shell
participant Container as DI Container / Registry
participant Handler as Handler (compute/effect/reducer/orchestrator)
participant External as External System (DB/API/EventBus)
Client->>Node: send input / event
Node->>Container: resolve handler (by contract / handler_id)
Container-->>Node: handler instance
Node->>Handler: invoke execute/handle(input, metadata)
Handler->>External: perform I/O or emit events/intents
External-->>Handler: response / ack
Handler-->>Node: return ModelHandlerOutput(events/intents/result)
Node-->>Client: acknowledge / forward emitted items
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (10)
docs/architecture/MIXIN_CLASSIFICATION.md (1)
85-115:⚠️ Potential issue | 🟡 MinorRemove duplicate
MixinContractStateReducerentry and fix totals.
MixinContractStateReduceris listed at Line 85 and again at Line 115. This duplicates the mixin in the classification table and likely inflates the Domain and Total counts. Please keep a single entry and recompute the summary counts accordingly.docs/guides/MIGRATING_FROM_DICT_ANY.md (1)
5-7:⚠️ Potential issue | 🟡 MinorResolve conflicting “Last Updated” dates.
The header shows 2025-12-26 while the footer shows 2025-12-25. Pick one date to avoid confusion.
🔧 Proposed fix
-**Last Updated**: 2025-12-25 +**Last Updated**: 2025-12-26Also applies to: 729-730
docs/architecture/MIXINS_TO_HANDLERS_REFACTOR.md (1)
70-79:⚠️ Potential issue | 🟡 MinorAdd missing imports in the descriptor example.
The descriptor snippet uses
ModelIdentifier,ModelSemVer,EnumHandlerRole,EnumHandlerType, andEnumHandlerTypeCategorybut doesn’t import them. Please add the import block so the example is runnable.📌 Suggested doc fix
from omnibase_core.models.handlers import ModelHandlerDescriptor +from omnibase_core.models.handlers import ( + ModelIdentifier, + ModelSemVer, + EnumHandlerRole, + EnumHandlerType, + EnumHandlerTypeCategory, +)docs/guides/mixin-development/03_PYDANTIC_MODELS.md (1)
41-46:⚠️ Potential issue | 🟡 MinorAdd a language identifier to the command fence.
Use
bashto satisfy MD040 and improve syntax highlighting.🛠️ Suggested fix
-``` +```bash cd /Volumes/PRO-G40/Code/omnibase_core/src/omnibase_core/models/contracts/subcontracts/docs/guides/MIGRATING_TO_DECLARATIVE_NODES.md (1)
586-605:⚠️ Potential issue | 🟡 MinorAdjust the troubleshooting snippets to validate the correct nested keys (and show imports).
As written,
model_validate(yaml.safe_load(...))may pass the full YAML document rather than thestate_transitions/workflow_definitionobject, which can cause validation errors depending on schema shape. Also, the snippet omits theyaml/Pathimports.🛠️ Suggested doc fix
- node.fsm_contract = ModelFSMSubcontract.model_validate( - yaml.safe_load(Path("contract.yaml").read_text()) - ) + from pathlib import Path + import yaml + data = yaml.safe_load(Path("contract.yaml").read_text()) + node.fsm_contract = ModelFSMSubcontract.model_validate(data["state_transitions"])- node.workflow_definition = ModelWorkflowDefinition.model_validate( - yaml.safe_load(Path("workflow.yaml").read_text()) - ) + from pathlib import Path + import yaml + data = yaml.safe_load(Path("workflow.yaml").read_text()) + node.workflow_definition = ModelWorkflowDefinition.model_validate( + data["workflow_coordination"]["workflow_definition"] + )docs/architecture/MODEL_INTENT_ARCHITECTURE.md (1)
552-595:⚠️ Potential issue | 🟡 MinorRetry path references a helper that isn’t defined in this updated flow.
_execute_single_intent()is referenced, but the earlier section states inline executors were removed and the execution now delegates to the handler registry. Update the retry path to use the same registry-based handler call (or document the helper explicitly).✅ Suggested fix (reuse registry-based handler)
- try: - await asyncio.sleep(1.0) # Backoff - await self._execute_single_intent(intent) + try: + await asyncio.sleep(1.0) # Backoff + handler = self.handler_registry.get(intent.intent_type) + await handler.execute(intent) except Exception as retry_error:docs/architecture/ONEX_FOUR_NODE_ARCHITECTURE.md (1)
586-630:⚠️ Potential issue | 🟡 MinorFix REDUCER constraint inconsistency (intents are emitted in the example).
The constraint block says intents are forbidden, but the example returns
intents=intents. Update the constraints or the example so they match.✅ Suggested doc fix
- REDUCER output constraints: - - Allowed: projections[] - - Forbidden: events[], intents[], result + REDUCER output constraints: + - Allowed: projections[], intents[] + - Forbidden: events[], resultdocs/architecture/MESSAGE_TOPIC_MAPPING.md (1)
98-125:⚠️ Potential issue | 🟡 MinorAlign intent topic domain with the updated naming.
Examples still use
omninode-bridgein topic names and the intent executor domain. If the renamed domain is nowomnibase_core/omnibase_infra, update these examples for consistency with the rest of the docs (or explicitly note that this domain is intentionally retained).Also applies to: 311-314
docs/guides/TESTING_GUIDE.md (1)
32-46:⚠️ Potential issue | 🟡 MinorAdd missing
Anyimport in typed examples.
Anyis referenced but not imported, which breaks copy/paste usability. Please addfrom typing import Anyhere and mirror the fix in other snippets that useAny.✅ Suggested patch for this block
import pytest +from typing import Any from omnibase_core.models.container.model_onex_container import ModelONEXContainer from omnibase_core.nodes.node_compute import NodeComputedocs/guides/MIXIN_SUBCONTRACT_MAPPING.md (1)
917-921:⚠️ Potential issue | 🟡 MinorExample contradicts the “non-existent mixins” note.
This section imports
MixinRetry, but the note above says it doesn’t exist as a Python class. Either replace the example with an existing mixin or explicitly mark it as hypothetical to avoid confusion.
🤖 Fix all issues with AI agents
In `@DOCS_AUDIT_REPORT.md`:
- Around line 158-191: The fenced code block in DOCS_AUDIT_REPORT.md is missing
a language tag; update the opening fence from ``` to ```text (or another
appropriate language like ```markdown) so markdownlint and renderers recognize
it, leaving the block content unchanged and keeping the closing ``` as-is;
locate the "Handler Architecture Guide" fenced block and modify its opening
fence accordingly.
In `@docs/architecture/CANONICAL_EXECUTION_SHAPES.md`:
- Line 297: Several fenced code blocks in the document (e.g., the block
immediately following the "class HandlerIntentExecutor" header and the other
similar blocks later) are missing language identifiers; update each
triple-backtick fence to include an appropriate language tag (for example
```python, ```text, or ```json) so markdownlint MD040 is satisfied—locate the
fences around the "HandlerIntentExecutor" section and the other reported blocks
and add the explicit language specifier to each opening ``` line.
In `@docs/architecture/CONTAINER_DI_DESIGN.md`:
- Around line 616-617: The checklist item "- [ ] Pre-commit hooks passing (ruff,
mypy)" is inaccurate and incomplete; update that line to either list the actual
critical hooks from .pre-commit-config.yaml (e.g., ruff-format, ruff-fix,
mypy-type-check, pyright-type-check, yamlfmt, validate-repository-structure,
validate-naming-conventions, validate-file-naming-conventions,
validate-contracts, etc.) or replace the hardcoded list with a reference to the
canonical file (".pre-commit-config.yaml") so it stays in sync, and correct the
placement of mypy by noting it runs at pre-push (mypy-type-check) rather than
pre-commit.
In `@docs/architecture/CONTAINER_TYPES.md`:
- Around line 235-259: The fenced code block showing the NodeDataProcessor
example is missing a language tag; update the opening fence from ``` to
```python to satisfy MD040 and enable syntax highlighting for the block that
contains NodeDataProcessor, ModelONEXContainer, ProtocolLogger, ProtocolEventBus
and the process method; ensure the closing fence remains ``` unchanged.
In `@docs/architecture/CONTRACT_DRIVEN_NODEREDUCER_V1_0.md`:
- Around line 30-42: The docs currently contradict: the Output Constraints table
marks intents[] as Forbidden for REDUCER while the API mapping and spec require
intents via output.intents; update the text to resolve this by either (A)
clarifying that the "Forbidden" rule on ModelHandlerOutput applies only to the
NodeReducer instance scope (mentioning NodeReducer and ModelHandlerOutput
explicitly) and add a sentence stating FSM executor functions may still emit
intents via output.intents, or (B) change the table entry for `intents[]` to
**Allowed** and ensure the prose references `output.intents` consistently; pick
one approach and update the Output Constraints section and any sentences near
lines referencing NodeReducer/FSM executor to remove the contradiction.
In `@docs/architecture/DEPENDENCY_INJECTION.md`:
- Around line 264-281: The fenced resolution flow block is missing a Markdown
language tag; update the code fence that begins with the ASCII diagram of
container.get_service(ProtocolLogger) to use the "text" language identifier
(i.e., change ``` to ```text) so markdownlint MD040 is satisfied—this affects
the block containing container.get_service,
_service_cache["ProtocolLogger:default"], registry.resolve_service,
_interface_map["ProtocolLogger"], and _resolve_by_lifecycle.
- Around line 55-96: Several fenced-code blocks in the DEPENDENCY_INJECTION
documentation are missing language identifiers (markdownlint MD040); update the
decision-rule diagram block and the architecture diagram block to use fenced
blocks with a language tag (use `text` for the ASCII decision tree and
architecture diagram, or `python` where code is present) so linting passes;
specifically add the language identifiers to the blocks around the "Writing a
node class?" decision rule and the "ModelONEXContainer" architecture diagram
(references: the decision rule fenced block and the ModelONEXContainer
architecture fenced block).
In `@docs/architecture/ECOSYSTEM_DIRECTORY_STRUCTURE.md`:
- Around line 231-238: The fenced code block containing the import "from
omnibase_core.infrastructure.infra_bases import ModelServiceEffect" and the
class "MyInfraEffect(ModelServiceEffect)" is missing a language specifier;
update the opening fence to include "python" (i.e., ```python) so the block is
properly highlighted and MD040 is resolved.
In `@docs/architecture/MIXIN_ARCHITECTURE.md`:
- Around line 383-396: The markdown code fence is missing a language identifier;
update the fenced block containing the example of MyComputeNode and its
execute_compute implementation (which imports NodeCompute and delegates to
MyComputeHandler) by adding the language tag "python" after the opening triple
backticks so the snippet reads as a Python code block (e.g., ```python).
In `@docs/architecture/MODEL_INTENT_ARCHITECTURE.md`:
- Around line 508-546: The markdown code fence for the NodeIntentExecutor
example (containing class NodeIntentExecutor and method execute_intents, plus
imports like ModelIntent and ModelONEXContainer) is missing a language tag;
update the opening fence from ``` to ```python so the block is marked as Python
(addressing markdownlint MD040) and ensure the rest of the fenced content
remains unchanged.
In `@docs/architecture/NODE_CLASS_HIERARCHY.md`:
- Around line 300-317: The handler references undefined parameters
input_envelope_id and correlation_id inside async def handle(...), so update the
DatabaseWriter.handle signature to accept them (e.g., async def handle(self,
operation: dict[str, Any], input_envelope_id: str, correlation_id: str) ->
ModelHandlerOutput[None]) or make them optional with defaults
(input_envelope_id: Optional[str] = None, correlation_id: Optional[str] = None);
adjust any callers to pass these values through and keep the existing usage
where it returns ModelHandlerOutput.for_effect(...). Ensure type hints/imports
are updated accordingly.
- Around line 203-229: The PriceCalculator handler references input_envelope_id
and correlation_id but they are not defined in async def handle; update the
signature of PriceCalculator.handle to accept these parameters (e.g., add
input_envelope_id and correlation_id with appropriate types/annotations) and
ensure any callers pass them through; modify the function signature and any
related type hints or ModelHandlerInput usage so ModelHandlerOutput.for_compute
can use input_envelope_id and correlation_id without NameError.
In `@docs/architecture/NODE_PURITY_GUARANTEES.md`:
- Around line 325-331: The example's handle method references input_envelope_id
and correlation_id but they aren't declared; update the async def handle
signature (the handle method) to accept input_envelope_id and correlation_id
(e.g., async def handle(self, data: dict[str, Any], input_envelope_id: str,
correlation_id: str) -> ModelHandlerOutput[dict[str, Any]]), then pass those
parameters unchanged into ModelHandlerOutput.for_compute so the names used in
the return call are defined and type-consistent with
ModelHandlerOutput.for_compute.
In `@docs/architecture/overview.md`:
- Around line 311-339: The handler example references input_envelope_id and
correlation_id but the handle signature lacks an envelope or those IDs; update
the HandlerMyCompute.handle signature to accept an envelope (or explicit
input_envelope_id and correlation_id) and extract the IDs before calling
ModelHandlerOutput.for_compute, e.g., derive input_envelope_id = envelope.id and
correlation_id = envelope.correlation_id (or use provided params), then call
_process(input_data) and return the result as before; ensure the signature
change is applied to any examples that call HandlerMyCompute.handle and that
_process remains unchanged.
In `@docs/guides/MIGRATING_FROM_DICT_ANY.md`:
- Around line 704-711: The example is misleading because it mutates original
then copies it; update the snippet to show the correct usage path by creating
and passing an unmutated copy to ModelErrorDetails.from_dict (e.g., make a copy
of original before any mutations and call ModelErrorDetails.from_dict on that
copy) or alternatively show only the non-mutating usage where original is passed
directly without subsequent modification; refer to the variable original and the
constructor ModelErrorDetails.from_dict when making the change.
In `@docs/guides/mixin-development/01_CREATING_MIXINS.md`:
- Around line 80-88: Add the shell language tag to the fenced code block
containing the commands (the block with the cd and touch commands for
mixin_error_handling.yaml and model_error_handling_subcontract.py); update the
opening ``` to ```bash so the block that includes "cd .../mixins/" and "touch
mixin_error_handling.yaml" / "touch model_error_handling_subcontract.py" is
explicitly marked as bash for proper Markdown linting and readability.
In `@docs/guides/mixin-development/README.md`:
- Around line 78-84: The docs use a machine-specific absolute path in the "Check
existing mixins" example (the ls /Volumes/PRO-G40/... line); change that to a
portable, repo-relative command such as ls ./src/omnibase_core/mixins/ (or show
a fallback using git root: ls "$(git rev-parse
--show-toplevel)/src/omnibase_core/mixins/") so readers can list the mixins
without relying on a local volume; update the README.md example that follows the
ModelHealthCheckSubcontract import accordingly.
In `@docs/guides/node-building/03_COMPUTE_NODE_TUTORIAL.md`:
- Around line 62-123: The example's handler and node are misaligned:
HandlerPriceCalculator.execute currently is synchronous and takes (envelope,
input_data) but NodePriceCalculatorCompute.process calls it as
process(input_data) without awaiting; fix by making the handler async and
matching the call signature — change HandlerPriceCalculator.execute to async def
execute(self, envelope: ModelOnexEnvelope, input_data:
ModelPriceCalculatorInput) -> ModelHandlerOutput and update
NodePriceCalculatorCompute.process to async def process(self, envelope:
ModelOnexEnvelope, input_data: ModelPriceCalculatorInput): return await
self._handler.execute(envelope, input_data), ensuring types and the use of
envelope.metadata in ModelHandlerOutput.for_compute remain correct.
In `@docs/guides/node-building/04_EFFECT_NODE_TUTORIAL.md`:
- Around line 74-123: The NodeFileBackupEffect.process currently calls the
handler with only input_data but HandlerFileBackup.execute requires (envelope,
input_data); update NodeFileBackupEffect.process to accept the envelope and
input_data parameters (matching the node runtime signature) and invoke
self._handler.execute(envelope, input_data), returning its result; this involves
changing the process signature and the call site inside NodeFileBackupEffect to
pass both the envelope (e.g., envelope or input_envelope) and input_data to
HandlerFileBackup.execute so the handler receives the required envelope
metadata.
In `@docs/guides/node-building/06_ORCHESTRATOR_NODE_TUTORIAL.md`:
- Around line 90-143: The NodePipelineOrchestrator.process currently calls the
handler with only input_data but HandlerPipelineOrchestrator.execute expects
(envelope, input_data); update NodePipelineOrchestrator.process to accept the
envelope and pass it through to the handler (i.e., change the signature to async
def process(self, envelope, input_data) and call await
self._handler.execute(envelope, input_data)) so the handler receives the
required envelope parameter.
🧹 Nitpick comments (2)
docs/architecture/MIXIN_CLASSIFICATION.md (1)
52-52: Clarify Domain migration target scope.“
omnibase_core/utils/” reads as the sole target, but Domain mixins are also shown underdomain/fsm/. Consider broadening this line (e.g.,omnibase_core/utils/andomnibase_core/domain/) to avoid misleading readers.docs/guides/node-building/05_REDUCER_NODE_TUTORIAL.md (1)
191-203: Clarify intent emission terminology for REDUCER nodes.The Output Constraints section states that
intents[]is Forbidden for REDUCER nodes (line 197), and line 200 clarifies that "They do NOT return typed results, events, or intents viaModelHandlerOutput." However, the tutorial extensively demonstrates REDUCER nodes emitting intents (e.g., lines 273-294 in the full file showModelIntentemission patterns).This could be confusing to readers. Consider clarifying:
- Whether the constraint means intents cannot be returned in
ModelHandlerOutput.intents[]field specifically- How intent emission actually works if not through
ModelHandlerOutput- The relationship between "ModelIntent objects within the output model" (line 200) and the forbidden
intents[]field📝 Suggested clarification
-REDUCER nodes return **projections** (state snapshots). They do NOT return typed results, events, or intents via `ModelHandlerOutput`. Side effects are described as `ModelIntent` objects within the output model and routed to EFFECT nodes by an ORCHESTRATOR. +REDUCER nodes return **projections** (state snapshots) via `ModelHandlerOutput.projections[]`. They do NOT return typed results, events, or intents in the standard `ModelHandlerOutput` fields. Instead, side effects are described as `ModelIntent` objects in reducer-specific output models (e.g., `ModelMetricsAggregationOutput.intents`) and routed to EFFECT nodes by an ORCHESTRATOR.
| # The handler owns the business logic for intent execution. | ||
| # The node (NodeIntentExecutorEffect) is a thin shell that delegates to this handler. | ||
|
|
||
| class HandlerIntentExecutor: |
Contributor
There was a problem hiding this comment.
Add language identifiers to fenced code blocks (MD040).
markdownlint flags these fenced blocks as missing a language. Please add an explicit language (e.g., text or python) to each of these fences to satisfy MD040.
Also applies to: 321-321, 381-381, 404-404
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 297-297: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@docs/architecture/CANONICAL_EXECUTION_SHAPES.md` at line 297, Several fenced
code blocks in the document (e.g., the block immediately following the "class
HandlerIntentExecutor" header and the other similar blocks later) are missing
language identifiers; update each triple-backtick fence to include an
appropriate language tag (for example ```python, ```text, or ```json) so
markdownlint MD040 is satisfied—locate the fences around the
"HandlerIntentExecutor" section and the other reported blocks and add the
explicit language specifier to each opening ``` line.
added 3 commits
February 14, 2026 13:59
- Fix handler envelope passing in all node tutorials (compute, effect, orchestrator) - Resolve REDUCER output constraints contradiction (ModelHandlerOutput vs ModelReducerOutput) - Fix broken handler signatures missing input_envelope_id/correlation_id params - Add language tags to all fenced code blocks missing them (MD040) - Replace machine-specific absolute paths with relative paths - Remove duplicate MixinContractStateReducer entry, recompute counts - Add missing imports in examples (handlers, typing.Any, yaml/Path) - Fix workaround example ordering in MIGRATING_FROM_DICT_ANY - Update retry path to registry-based handler in MODEL_INTENT_ARCHITECTURE - Clarify intent emission terminology for REDUCER nodes - Fix troubleshooting snippets to extract correct nested YAML keys
…en test in HANDLER_ARCHITECTURE
…ocumentation - Fix 5x `omnibase_core.core.node_*` → `omnibase_core.nodes.node_*` import paths - Fix 5x phantom `ModelErrorHandlingSubcontract` imports to tutorial-local imports - Replace phantom `ServiceLogging` class with actual `emit_log_event_sync` API
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
docs/guides/TESTING_GUIDE.md (1)
31-45:⚠️ Potential issue | 🟡 MinorAdd a language tag to the fenced code block.
The code block starting at Line 31 is missing a language specifier, which triggers MD040 and reduces readability.
📝 Suggested fix
-``` +```pythondocs/guides/node-building/05_REDUCER_NODE_TUTORIAL.md (2)
728-742:⚠️ Potential issue | 🟡 MinorAvoid accessing
reducer_output.completed_at(likely not on ModelReducerOutput).
reducer_outputis produced byNodeReducer.process()and appears to be aModelReducerOutput. Unless that model exposescompleted_at, this example will break. Prefer a locally computed timestamp or omit the field.🧾 Suggested fix
- "completed_at": reducer_output.completed_at.isoformat() if hasattr(reducer_output, 'completed_at') else None, + "completed_at": datetime.now(UTC).isoformat(),
760-768:⚠️ Potential issue | 🔴 CriticalCritical: EnumConflictResolution members do not exist in the mapping.
The mapping uses non-existent enum members.
EnumConflictResolution(fromenum_reducer_types) only has:FIRST_WINS,LAST_WINS,MERGE,ERROR,CUSTOM. The tutorial referencesSUM,AVERAGE,TAKE_MAX,TAKE_MIN,TAKE_LATESTwhich will causeAttributeErrorat runtime. Update the mapping to use valid enum members. OnlyMERGEis correct; the other five must be replaced with available options.docs/guides/MIXIN_SUBCONTRACT_MAPPING.md (1)
635-695:⚠️ Potential issue | 🟠 MajorThe mixin count and list contain significant discrepancies that require correction.
The documentation claims "32 Total" imperative mixins, but verification reveals:
- The table actually documents 33 mixins
- The codebase contains 40 actual mixin classes
- 7 mixins are missing from the documented list:
MixinCaching,MixinEventHandler,MixinHealthCheck,MixinMetrics,MixinNodeIntrospection,MixinSensitiveFieldRedaction, andMixinWorkflowExecution- The documented name
MixinRedactionshould beMixinSensitiveFieldRedactionto match the actual implementationThe phantom mixins verification is correct—all 19 listed phantom mixins (
MixinRetry,MixinTimeout, etc.) are correctly confirmed as not existing as Python classes.Update the header count, add the 7 missing mixins to the table, and correct the redaction mixin name.
🤖 Fix all issues with AI agents
In `@docs/architecture/HANDLER_ARCHITECTURE.md`:
- Around line 300-305: Add a language tag (e.g., text) to the fenced code block
that contains the ModelOnexError snippet so markdownlint MD040 is satisfied and
formatting improves; change the block from ``` to ```text for the snippet
starting with "ModelOnexError: ORCHESTRATOR cannot set result - use events[] and
intents[] only." and apply the same `text` tag to any other fence-only blocks in
HANDLER_ARCHITECTURE.md to keep formatting consistent.
In `@docs/architecture/MIXIN_CLASSIFICATION.md`:
- Around line 58-63: The table counts are inconsistent with the enumerated
lists: update the "Runtime (R)" row to show 21 mixins and a total of 22 (since
Runtime has 1 data model), update the "Domain (D)" row to show 15 mixins and a
total of 17 (15 mixins + 2 data models), and ensure the summary totals remain
correct (Mixins 39, Data Models 3, Total 42); also reconcile the matching
summary/header lines that repeat these totals (the other occurrence around the
summary lines referenced) so "Runtime (R)", "Handler (H)", "Domain (D)" and the
final "Total" row all reflect the same corrected numbers.
In `@docs/architecture/overview.md`:
- Around line 375-378: The example test calls handler.handle(...) without the
required keyword-only IDs; update the call to handler.handle to pass
input_envelope_id and correlation_id as keyword arguments (e.g., UUID strings or
fixture-provided IDs) in addition to the payload dict so the signature of
handler.handle(input, *, input_envelope_id, correlation_id) is satisfied; ensure
the test provides deterministic or fixture-generated values for
input_envelope_id and correlation_id and keeps the same assertion on
output.result["result"].
In `@docs/architecture/SUBCONTRACT_ARCHITECTURE.md`:
- Around line 195-206: The fenced code block showing the handler delegation
example lacks a Markdown language tag; update the block by adding the "python"
language identifier to the opening fence so the example (the class
AggregationReducerNode extending NodeReducer and its async method
execute_reduction) is syntax-highlighted correctly.
In `@docs/guides/node-building/04_EFFECT_NODE_TUTORIAL.md`:
- Around line 66-78: The example is missing imports used by the
HandlerFileBackup example; add import statements for ModelOnexEnvelope,
NodeEffect, ModelONEXContainer, and ModelFileBackupInput at the top of the
snippet so the HandlerFileBackup class and its execute method (execute(self,
envelope: ModelOnexEnvelope, input_data: ModelFileBackupInput) ->
ModelHandlerOutput) compile when copy‑pasted; ensure the new imports are placed
alongside the existing imports (e.g., with ModelHandlerOutput and
ModelEventEnvelope) and reference the same omnibase_core models package as the
other imports.
added 2 commits
February 15, 2026 07:29
Replace non-existent `omnibase_core.models.envelope.model_onex_envelope` with correct `omnibase_core.models.core.model_onex_envelope` in compute, effect, and orchestrator tutorial files.
…doc files - models.model_intent -> models.reducer.model_intent (1 file) - utils.circuit_breaker -> models.configuration.model_circuit_breaker (2 files) - ProtocolLogger (type-based) -> ProtocolLoggerLike, preserving string DI keys (7 files) - MixinIntrospection -> MixinNodeIntrospection (8 files) - models.services import -> infrastructure.infra_bases import (3 files) Also includes code fence language hints and mixin name/count corrections from prior uncommitted review fixes.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/guides/mixin-development/README.md (1)
268-272:⚠️ Potential issue | 🟡 MinorFix inconsistent import path in the test snippet.
The example imports from
your_project.model.subcontracts, but the guide standardizes onmodels/contracts/subcontracts. This will mislead readers.🔧 Suggested fix
-from your_project.model.subcontracts import ModelYourFeatureSubcontract +from your_project.models.contracts.subcontracts.model_your_feature_subcontract import ( + ModelYourFeatureSubcontract, +)docs/guides/mixin-development/01_CREATING_MIXINS.md (1)
526-545:⚠️ Potential issue | 🟡 MinorFix malformed Markdown fences in the README example.
The blocks are closed with
markdown, which breaks rendering. Close withand start the next block explicitly.🔧 Suggested fix
```yaml subcontracts: - path: "../../mixins/mixin_error_handling.yaml" integration_field: "error_handling_configuration" -```markdown +``` ### Advanced Configuration ```python @@ error_config = ModelErrorHandlingSubcontract( enable_circuit_breaker=True, @@ error_retry_delay_ms=2000 ) -```markdown +```
🤖 Fix all issues with AI agents
In `@DOCS_AUDIT_REPORT.md`:
- Around line 158-191: The Markdown code block that begins with the opening
fence "```text" is incorrectly closed with "```text" instead of the proper
closing fence "```"; edit the closing fence for that Handler Architecture Guide
block so the final fence is just "```" (search for the block that starts with
"```text" and replace its trailing "```text" with "```" to restore proper
Markdown rendering).
In
`@docs/architecture/architecture-research/RESEARCH_REPORT_4_NODE_ARCHITECTURE.md`:
- Line 31: Replace the machine-specific absolute path string
"/Volumes/PRO-G40/Code/omnibase_3/.cursor/rules/node_standards.mdc" in the
document RESEARCH_REPORT_4_NODE_ARCHITECTURE.md with a repo-relative placeholder
(e.g. "<repo-root>/...") or clearly mark it as a placeholder; update the
occurrence at line 31 (and the similar occurrence at line 59) so historical docs
do not contain environment-specific absolute paths.
In `@docs/architecture/ECOSYSTEM_DIRECTORY_STRUCTURE.md`:
- Around line 16-18: The docs file ECOSYSTEM_DIRECTORY_STRUCTURE.md contains
machine-specific absolute paths in the "Location" fields (e.g.,
`/Volumes/PRO-G40/Code/omnibase_core`) which breaks portability; update each
such Location entry (including the shown block and the other occurrences
mentioned) to use repo-relative or placeholder paths like
`<repo-root>/omnibase_core` or `./omnibase_core`, keeping the "Purpose" and
"Package Name" lines unchanged and apply the same replacement consistently for
all similar entries across the document.
In `@docs/guides/MIXIN_SUBCONTRACT_MAPPING.md`:
- Around line 521-524: The import path for MixinNodeIntrospection is incorrect
in the example; update the import to reference the module that matches the class
name by importing MixinNodeIntrospection from
omnibase_core.mixins.mixin_node_introspection, and keep the example class
declaration IntrospectableNode(NodeReducer, MixinNodeIntrospection) unchanged so
docs reflect the correct module location.
In `@docs/guides/mixin-development/01_CREATING_MIXINS.md`:
- Around line 75-88: The documentation instructs running a validator against a
planned YAML mixin file in nodes/canary/mixins/ that does not exist; update the
guide to first present the current workflow using the Pydantic subcontract
models (models/contracts/subcontracts/) and show validation commands targeting
the actual Python model (e.g., model_error_handling_subcontract.py / the
Pydantic class name), then add a clearly marked “planned” section demonstrating
the future YAML file example and validator command that will apply once
nodes/canary/mixins/mixin_error_handling.yaml exists; ensure you reference the
Pydantic subcontract module names and the planned mixin filename
(mixin_error_handling.yaml) so readers know which items are current vs. planned.
Comment on lines
+158
to
+191
| ```text | ||
| # Handler Architecture Guide | ||
|
|
||
| ## 1. Goal and Invariants | ||
| - Nodes are thin shells; handlers own logic | ||
| - Output constraints matrix | ||
| - Contract-driven behavior | ||
|
|
||
| ## 2. Minimal Example | ||
| - YAML contract | ||
| - Handler class | ||
| - Thin node shell | ||
| - Input/output Pydantic models | ||
|
|
||
| ## 3. DI and Registry Resolution | ||
| - Protocol-based resolution: container.get_service(ProtocolHandler) | ||
| - Capability-based resolution | ||
| - No manual instantiation | ||
|
|
||
| ## 4. Handler Lifecycle | ||
| - Construction, execution, teardown | ||
| - Stateless vs. stateful handlers | ||
| - Thread safety: handlers are single-request-scoped | ||
|
|
||
| ## 5. Testing Pattern | ||
| - Handler unit test (no container needed) | ||
| - Contract validation test (golden fixtures) | ||
| - Integration test with container | ||
|
|
||
| ## 6. Migration from Imperative Nodes | ||
| - Before: logic in process() | ||
| - After: logic in handler, node delegates | ||
| - Checklist | ||
| ```text |
Contributor
There was a problem hiding this comment.
Fix malformed code fence closing tag.
The block opened with text is closed with text, which breaks Markdown rendering. Close with ``` only.
🔧 Suggested fix
-```text
+```text
# Handler Architecture Guide
@@
-```text
+```🤖 Prompt for AI Agents
In `@DOCS_AUDIT_REPORT.md` around lines 158 - 191, The Markdown code block that
begins with the opening fence "```text" is incorrectly closed with "```text"
instead of the proper closing fence "```"; edit the closing fence for that
Handler Architecture Guide block so the final fence is just "```" (search for
the block that starts with "```text" and replace its trailing "```text" with
"```" to restore proper Markdown rendering).
| ### 1.1 Reference Implementation Standards (omnibase_3 -- Historical) | ||
|
|
||
| **Source**: `/Volumes/PRO-G40/Code/omnibase_3/.cursor/rules/node_standards.mdc` | ||
| **Source**: `/Volumes/PRO-G40/Code/omnibase_3/.cursor/rules/node_standards.mdc` *(historical path, no longer available)* |
Contributor
There was a problem hiding this comment.
Use repo-relative placeholders for historical paths.
Even in historical docs, absolute /Volumes/PRO-G40/... paths are machine-specific. Prefer <repo-root>/... or mark as a placeholder.
🔧 Suggested fix
-**Source**: `/Volumes/PRO-G40/Code/omnibase_3/.cursor/rules/node_standards.mdc` *(historical path, no longer available)*
+**Source**: `<repo-root>/omnibase_3/.cursor/rules/node_standards.mdc` *(historical path, no longer available)*
@@
-**Location**: `/Volumes/PRO-G40/Code/omnibase_core/src/omnibase_core/core/`
+**Location**: `<repo-root>/omnibase_core/src/omnibase_core/core/`Also applies to: 59-59
🤖 Prompt for AI Agents
In
`@docs/architecture/architecture-research/RESEARCH_REPORT_4_NODE_ARCHITECTURE.md`
at line 31, Replace the machine-specific absolute path string
"/Volumes/PRO-G40/Code/omnibase_3/.cursor/rules/node_standards.mdc" in the
document RESEARCH_REPORT_4_NODE_ARCHITECTURE.md with a repo-relative placeholder
(e.g. "<repo-root>/...") or clearly mark it as a placeholder; update the
occurrence at line 31 (and the similar occurrence at line 59) so historical docs
do not contain environment-specific absolute paths.
Comment on lines
+521
to
+524
| ```python | ||
| from omnibase_core.mixins.mixin_introspection import MixinNodeIntrospection | ||
|
|
||
| class IntrospectableNode(NodeReducer, MixinIntrospection): | ||
| class IntrospectableNode(NodeReducer, MixinNodeIntrospection): |
Contributor
There was a problem hiding this comment.
Fix the import path for MixinNodeIntrospection.
The import points to mixin_introspection, but the class name suggests the module should be mixin_node_introspection. This doc example will mislead users if the module was renamed along with the class.
✅ Proposed fix
-from omnibase_core.mixins.mixin_introspection import MixinNodeIntrospection
+from omnibase_core.mixins.mixin_node_introspection import MixinNodeIntrospection🤖 Prompt for AI Agents
In `@docs/guides/MIXIN_SUBCONTRACT_MAPPING.md` around lines 521 - 524, The import
path for MixinNodeIntrospection is incorrect in the example; update the import
to reference the module that matches the class name by importing
MixinNodeIntrospection from omnibase_core.mixins.mixin_node_introspection, and
keep the example class declaration IntrospectableNode(NodeReducer,
MixinNodeIntrospection) unchanged so docs reflect the correct module location.
added 6 commits
February 15, 2026 09:49
…t path, add warning logging Major: - Add frozen=True, extra="forbid", from_attributes=True to ModelProjectionBase ConfigDict - Add missing ConfigDict to ModelIntentPublishResult (frozen, extra=forbid) - Update test_model_intent_publish_result to expect ValidationError for extra fields - Fix phantom test path in 01_CREATING_MIXINS.md to use real test directory Minor: - Add warning logging for silent ImportError fallback in mixin_intent_publisher - Clarify ProtocolKafkaClient as publish-only protocol in docstring - Remove stale file counts from ECOSYSTEM_DIRECTORY_STRUCTURE.md - Fix cd prefix in mixin guide code example
…roject Pydantic standards
…ent, remove residual count
- Add isinstance(kafka_client, ProtocolKafkaClient) runtime check in MixinIntentPublisher to catch misconfigured containers at init time - Document _depth recursion guard parameter in compute_similarity() docstring
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/guides/mixin-development/01_CREATING_MIXINS.md (1)
548-574:⚠️ Potential issue | 🔴 CriticalFix malformed markdown code block closures.
Lines 552, 566, and 574 have incorrect code block closures with
markdown which breaks rendering. Code blocks should close with justand then content continues normally.📝 Proposed fix
### Basic Configuration -```yaml +```yaml subcontracts: - path: "../../mixins/mixin_error_handling.yaml" integration_field: "error_handling_configuration" -```markdown +``` ### Advanced Configuration -```python +```python # Tutorial example - you create this class as part of the exercise above from model_error_handling_subcontract import ModelErrorHandlingSubcontract error_config = ModelErrorHandlingSubcontract( enable_circuit_breaker=True, circuit_failure_threshold=10, error_retry_attempts=5, error_retry_delay_ms=2000 ) -```markdown +``` ## Best Practices 1. **Always Enable Circuit Breakers**: For EFFECT nodes calling external systems 2. **Configure Retry Types**: Specify which errors are retriable 3. **Set Appropriate Thresholds**: Based on your SLA requirements 4. **Monitor Metrics**: Track error rates and circuit breaker states -``` +
🤖 Fix all issues with AI agents
In `@docs/architecture/ECOSYSTEM_DIRECTORY_STRUCTURE.md`:
- Around line 42-52: Update the tree diagram to use the plural directory name
"models/services/" instead of "models/service/": find the entry currently
showing "models/service/" (and its child files like model_service_compute.py,
model_service_effect.py, model_service_reducer.py,
model_service_orchestrator.py) and change it to "models/services/"; also make
the same plural correction where the diagram repeats the path later (the
occurrences referenced near the other mentions of "services" at the lower
sections) so the diagram matches the actual directory structure and the
references at lines ~210 and ~239.
In `@docs/guides/mixin-development/01_CREATING_MIXINS.md`:
- Around line 473-474: The import "from model_error_handling_subcontract import
ModelErrorHandlingSubcontract" uses a bare module name and will fail; change it
to the full package path (e.g., replace the bare import with the
package-qualified import for ModelErrorHandlingSubcontract) so the module is
resolved correctly — update the import line that references
ModelErrorHandlingSubcontract to use the full package dotted path.
- Line 427: The import for ModelErrorHandlingSubcontract is using a bare module
name; update the import to use the full package path consistent with the other
imports (the same package path used on line 306) so the symbol
ModelErrorHandlingSubcontract is resolved correctly at runtime—replace the bare
import with the fully qualified package import referencing
ModelErrorHandlingSubcontract.
- Around line 557-558: The import uses a bare module name; change "from
model_error_handling_subcontract import ModelErrorHandlingSubcontract" to use
the full package path (e.g. from
your_package.submodule.model_error_handling_subcontract import
ModelErrorHandlingSubcontract) so the module resolves consistently (same fix
applied previously for the imports around lines with other occurrences).
- Line 366: Remove the unused type import by deleting the line "from typing
import Any" in the example; ensure no other code in this snippet references Any
so the import can be safely removed and the example's imports remain clean.
- Around line 471-472: The import style is inconsistent: keep package-level
imports like the existing NodeCompute line by changing the ModelContractCompute
import; update the import statement that references ModelContractCompute so it
uses "from omnibase_core.models.contracts import ModelContractCompute" (matching
the NodeCompute pattern) to rely on the contracts package export and maintain
consistent import conventions for ModelContractCompute alongside NodeCompute.
In `@docs/INDEX.md`:
- Line 349: The table row "| Testing | Complete | 7 testing guides |" has a
mismatched count vs the "Testing & CI" list; either update that row to "6
testing guides" or add the missing guide to the "Testing & CI" section so the
counts match. Locate the table entry string and the "Testing & CI" list (the
table row text and the "Testing & CI" section heading) and keep them consistent
by changing the number in the table to 6 if no new guide is added, or add the
absent guide to the list and ensure the table remains "7 testing guides."
🧹 Nitpick comments (1)
docs/INDEX.md (1)
186-186: ADR-013 no longer exists as a separate file (merged into ADR-006)—consider linking or removing the row for consistency.ADR-013 was merged into ADR-006 and has no corresponding file. The row is correctly unlinked, but for consistency with the table format and to aid navigation, either:
- Remove the row entirely, or
- Convert it to:
| ADR-013: Status Taxonomy | [Superseded — merged into ADR-006](decisions/ADR-006-status-taxonomy.md) |
Comment on lines
+42
to
+52
| │ ├── models/ # Data models | ||
| │ │ ├── service/ # Service wrapper classes | ||
| │ │ │ ├── model_service_compute.py | ||
| │ │ │ ├── model_service_effect.py | ||
| │ │ │ ├── model_service_reducer.py | ||
| │ │ │ └── model_service_orchestrator.py | ||
| │ │ ├── events/ # Event models | ||
| │ │ ├── primitives/ # Primitive type models | ||
| │ │ └── ... (other model categories) | ||
| │ │ └── ... | ||
| │ ├── nodes/ # Base node classes | ||
| │ ├── types/ # Type definitions (93 files) | ||
| │ ├── types/ # Type definitions |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify directory structure
fd -t d 'service' omnibase_core | head -20
echo "---"
# Check references in documentation
rg 'models/service|models/services' docs/architecture/ECOSYSTEM_DIRECTORY_STRUCTURE.md -nRepository: OmniNode-ai/omnibase_core
Length of output: 259
🏁 Script executed:
#!/bin/bash
# Also check the actual file structure around models
fd -t f -a 'model_service' omnibase_core | head -20
echo "---"
# Check for mixin references
rg 'mixin.*[Ii]ntrospection|IntrospectionMixin' docs/architecture/ECOSYSTEM_DIRECTORY_STRUCTURE.md -n -A 2 -B 2Repository: OmniNode-ai/omnibase_core
Length of output: 388
🏁 Script executed:
#!/bin/bash
# Get more context about the file structure
cat -n docs/architecture/ECOSYSTEM_DIRECTORY_STRUCTURE.md | sed -n '30,60p'
echo "---"
cat -n docs/architecture/ECOSYSTEM_DIRECTORY_STRUCTURE.md | sed -n '205,220p'
echo "---"
cat -n docs/architecture/ECOSYSTEM_DIRECTORY_STRUCTURE.md | sed -n '235,245p'Repository: OmniNode-ai/omnibase_core
Length of output: 2536
🏁 Script executed:
#!/bin/bash
# Check actual directory structure
find . -type d -name "*service*" 2>/dev/null | grep -E 'models/service' | head -20Repository: OmniNode-ai/omnibase_core
Length of output: 133
🏁 Script executed:
#!/bin/bash
# Check the repo structure to understand the correct path
git ls-files | grep -E 'models.*service' | head -20Repository: OmniNode-ai/omnibase_core
Length of output: 1366
Fix tree diagram to use plural models/services/ instead of singular models/service/.
The actual directory is models/services/ (plural), but the tree at line 43 shows models/service/ (singular). This contradicts the correct references at lines 210 and 239, which use plural services. Update the tree diagram to match the actual directory structure.
[Also applies to lines 210, 239]
🤖 Prompt for AI Agents
In `@docs/architecture/ECOSYSTEM_DIRECTORY_STRUCTURE.md` around lines 42 - 52,
Update the tree diagram to use the plural directory name "models/services/"
instead of "models/service/": find the entry currently showing "models/service/"
(and its child files like model_service_compute.py, model_service_effect.py,
model_service_reducer.py, model_service_orchestrator.py) and change it to
"models/services/"; also make the same plural correction where the diagram
repeats the path later (the occurrences referenced near the other mentions of
"services" at the lower sections) so the diagram matches the actual directory
structure and the references at lines ~210 and ~239.
| # model_error_handling_subcontract.py | ||
| from pydantic import BaseModel, Field | ||
| from typing import List, Dict, Any | ||
| from typing import Any |
Contributor
There was a problem hiding this comment.
Remove unused import.
Any is imported but never used in this example. Remove it to keep the imports clean.
🧹 Proposed fix
-from typing import Any
from enum import Enum📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| from typing import Any | |
| from enum import Enum |
🤖 Prompt for AI Agents
In `@docs/guides/mixin-development/01_CREATING_MIXINS.md` at line 366, Remove the
unused type import by deleting the line "from typing import Any" in the example;
ensure no other code in this snippet references Any so the import can be safely
removed and the example's imports remain clean.
| from pydantic import ValidationError | ||
| from omnibase_core.model.subcontracts import ModelErrorHandlingSubcontract | ||
| # Tutorial example - you create this class as part of the exercise above | ||
| from model_error_handling_subcontract import ModelErrorHandlingSubcontract |
Contributor
There was a problem hiding this comment.
Fix import path for ModelErrorHandlingSubcontract.
The import uses a bare module name which will fail. This should use the full package path consistent with line 306.
🔧 Proposed fix
-# Tutorial example - you create this class as part of the exercise above
-from model_error_handling_subcontract import ModelErrorHandlingSubcontract
+# Tutorial example - import the class you created in Step 4
+from omnibase_core.models.contracts.subcontracts.model_error_handling_subcontract import ModelErrorHandlingSubcontract📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| from model_error_handling_subcontract import ModelErrorHandlingSubcontract | |
| # Tutorial example - import the class you created in Step 4 | |
| from omnibase_core.models.contracts.subcontracts.model_error_handling_subcontract import ModelErrorHandlingSubcontract |
🤖 Prompt for AI Agents
In `@docs/guides/mixin-development/01_CREATING_MIXINS.md` at line 427, The import
for ModelErrorHandlingSubcontract is using a bare module name; update the import
to use the full package path consistent with the other imports (the same package
path used on line 306) so the symbol ModelErrorHandlingSubcontract is resolved
correctly at runtime—replace the bare import with the fully qualified package
import referencing ModelErrorHandlingSubcontract.
Comment on lines
471
to
+472
| from omnibase_core.nodes import NodeCompute | ||
| from omnibase_core.model.contracts import ModelContractCompute | ||
| from omnibase_core.model.subcontracts import ModelErrorHandlingSubcontract | ||
| from omnibase_core.models.contracts.model_contract_compute import ModelContractCompute |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the correct import paths for NodeCompute and ModelContractCompute
# Check for NodeCompute location
echo "=== Searching for NodeCompute class definition ==="
ast-grep --pattern 'class NodeCompute'
echo ""
echo "=== Searching for ModelContractCompute class definition ==="
ast-grep --pattern 'class ModelContractCompute'
echo ""
echo "=== Checking __init__.py exports ==="
echo "--- nodes/__init__.py ---"
find . -path "*/omnibase_core/nodes/__init__.py" -exec cat {} \;
echo ""
echo "--- models/contracts/__init__.py ---"
find . -path "*/omnibase_core/models/contracts/__init__.py" -exec cat {} \;Repository: OmniNode-ai/omnibase_core
Length of output: 50382
🏁 Script executed:
#!/bin/bash
# Find ModelContractCompute class definition
echo "=== Searching for ModelContractCompute class definition ==="
ast-grep --pattern 'class ModelContractCompute'
echo ""
echo "=== Checking nodes/__init__.py for NodeCompute export ==="
find . -path "*/omnibase_core/nodes/__init__.py" -type f -exec sh -c 'echo "File: {}"; cat "{}" 2>/dev/null | head -50' \;
echo ""
echo "=== Checking models/contracts/__init__.py for ModelContractCompute export ==="
find . -path "*/omnibase_core/models/contracts/__init__.py" -type f -exec sh -c 'echo "File: {}"; cat "{}" 2>/dev/null | head -50' \;
echo ""
echo "=== Searching for model_contract_compute module ==="
find . -name "*model_contract_compute*" -type fRepository: OmniNode-ai/omnibase_core
Length of output: 50381
🏁 Script executed:
#!/bin/bash
# Check if ModelContractCompute is exported from contracts/__init__.py
echo "=== Checking if ModelContractCompute is in contracts/__init__.py ==="
rg -n "ModelContractCompute" ./src/omnibase_core/models/contracts/__init__.pyRepository: OmniNode-ai/omnibase_core
Length of output: 309
Use consistent import patterns for both classes.
Both import paths are valid, but line 472 should follow the same package-level import pattern as line 471 for consistency. Change:
from omnibase_core.models.contracts.model_contract_compute import ModelContractComputeto:
from omnibase_core.models.contracts import ModelContractComputeSince ModelContractCompute is properly exported from omnibase_core.models.contracts.__init__, using the package import aligns with the import style on line 471 and follows the codebase convention.
🤖 Prompt for AI Agents
In `@docs/guides/mixin-development/01_CREATING_MIXINS.md` around lines 471 - 472,
The import style is inconsistent: keep package-level imports like the existing
NodeCompute line by changing the ModelContractCompute import; update the import
statement that references ModelContractCompute so it uses "from
omnibase_core.models.contracts import ModelContractCompute" (matching the
NodeCompute pattern) to rely on the contracts package export and maintain
consistent import conventions for ModelContractCompute alongside NodeCompute.
Comment on lines
+473
to
+474
| # Tutorial example - you create this class as part of the exercise above | ||
| from model_error_handling_subcontract import ModelErrorHandlingSubcontract |
Contributor
There was a problem hiding this comment.
Fix import path (same issue as line 427).
The bare module import will fail. Use the full package path.
🔧 Proposed fix
-# Tutorial example - you create this class as part of the exercise above
-from model_error_handling_subcontract import ModelErrorHandlingSubcontract
+# Tutorial example - import the class you created in Step 4
+from omnibase_core.models.contracts.subcontracts.model_error_handling_subcontract import ModelErrorHandlingSubcontract🤖 Prompt for AI Agents
In `@docs/guides/mixin-development/01_CREATING_MIXINS.md` around lines 473 - 474,
The import "from model_error_handling_subcontract import
ModelErrorHandlingSubcontract" uses a bare module name and will fail; change it
to the full package path (e.g., replace the bare import with the
package-qualified import for ModelErrorHandlingSubcontract) so the module is
resolved correctly — update the import line that references
ModelErrorHandlingSubcontract to use the full package dotted path.
Comment on lines
+557
to
+558
| # Tutorial example - you create this class as part of the exercise above | ||
| from model_error_handling_subcontract import ModelErrorHandlingSubcontract |
Contributor
There was a problem hiding this comment.
Fix import path (same issue as lines 427 and 474).
Use the full package path instead of bare module name.
🔧 Proposed fix
-# Tutorial example - you create this class as part of the exercise above
-from model_error_handling_subcontract import ModelErrorHandlingSubcontract
+# Tutorial example - import the class you created in Step 4
+from omnibase_core.models.contracts.subcontracts.model_error_handling_subcontract import ModelErrorHandlingSubcontract🤖 Prompt for AI Agents
In `@docs/guides/mixin-development/01_CREATING_MIXINS.md` around lines 557 - 558,
The import uses a bare module name; change "from
model_error_handling_subcontract import ModelErrorHandlingSubcontract" to use
the full package path (e.g. from
your_package.submodule.model_error_handling_subcontract import
ModelErrorHandlingSubcontract) so the module resolves consistently (same fix
applied previously for the imports around lines with other occurrences).
| | Standards | Complete | 4 normative specs | | ||
| | Testing | Complete | 6 testing guides | | ||
| | Standards | Complete | 3 normative specs | | ||
| | Testing | Complete | 7 testing guides | |
Contributor
There was a problem hiding this comment.
Testing guide count doesn’t match the list.
The Testing & CI section lists 6 items, but the status row says “7 testing guides.” Please align the count or add the missing guide.
✏️ Suggested tweak
-| Testing | Complete | 7 testing guides |
+| Testing | Complete | 6 testing guides |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| | Testing | Complete | 7 testing guides | | |
| | Testing | Complete | 6 testing guides | |
🤖 Prompt for AI Agents
In `@docs/INDEX.md` at line 349, The table row "| Testing | Complete | 7 testing
guides |" has a mismatched count vs the "Testing & CI" list; either update that
row to "6 testing guides" or add the missing guide to the "Testing & CI" section
so the counts match. Locate the table entry string and the "Testing & CI" list
(the table row text and the "Testing & CI" section heading) and keep them
consistent by changing the number in the table to 6 if no new guide is added, or
add the absent guide to the list and ensure the table remains "7 testing
guides."
added 3 commits
February 15, 2026 14:00
Line numbers shifted due to documentation edits. Removed stale entry for deleted EFFECT_NODE_TEMPLATE.md secret finding.
Resolve 13 file conflicts from Poetry-to-uv migration. Keep docs audit content (substantive edits), adopt uv run commands from main.
Fix code fence language hint in audit report. Replace absolute local paths with generic placeholders in research report.
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
source_nodeprefix inmixin_intent_publisher.py(was stampingomninode_bridge, now correctlyomnibase_core)ProtocolKafkaClientto own file for single-class-per-file complianceTest plan
test_cte_hidden_table_rejected) confirmed unrelated — fails on main toodatabase_ref: omninode_bridgeintentionally kept in YAML/tests — refers to actual PostgreSQL database name, not repoSummary by CodeRabbit
New Features
Documentation
Chores