feat(graph-engine): make layer runtime state non-null and bound early#6
Conversation
…non-null state (vibe-kanban 85cdff8f) Parent: 7c1fead6-6f01-4778-9089-3782e9bcc6b1 (Make self.graph\_runtime\_state in GraphEngineLayer not None) Goal: Now that GraphEngineLayer.graph\_runtime\_state is guaranteed non-null, remove conditional checks and align docs/tests. Tasks: - Remove None-guards in built-in layers that currently check graph\_runtime\_state: - api/core/workflow/graph\_engine/layers/debug\_logging.py (remove \`if self.graph\_runtime\_state\` blocks; assume available after init). - api/core/workflow/graph\_engine/layers/persistence.py (remove \`if runtime\_state is None\` branches in \_populate\_completion\_statistics and \_system\_variables; treat runtime\_state as always present). - Update documentation to describe the non-null contract and early binding: - api/core/workflow/graph\_engine/layers/README.md (mention runtime state always available after engine.layer()). - api/core/workflow/README.md if it describes layer usage (optional, only if it mentions None checks or initialization caveats). Acceptance: - No \`if self.graph\_runtime\_state\` checks remain in layers. - Docs reflect that graph\_runtime\_state is always present after registration and before run. - Type checking remains clean (no Optional). Run (targeted if needed): uv run --project api -m pytest api/tests/unit\_tests/core/workflow/graph\_engine/layers
…uninitialized wrapper (vibe-kanban 7858b561) Parent: 7c1fead6-6f01-4778-9089-3782e9bcc6b1 (Make self.graph\_runtime\_state in GraphEngineLayer not None) Goal: Ensure GraphEngineLayer.graph\_runtime\_state is never None while preserving the initialization lifecycle. Provide a typed uninitialized implementation that raises a domain error when accessed too early, and bind a real read-only wrapper as soon as the layer is registered. Implementation details: - Add a domain error class, e.g., GraphEngineLayerNotInitializedError, in api/core/workflow/graph\_engine/layers/base.py or a new api/core/workflow/graph\_engine/layers/errors.py (keep DDD boundaries in mind). - Implement UninitializedReadOnlyGraphRuntimeState that satisfies the ReadOnlyGraphRuntimeState Protocol and raises GraphEngineLayerNotInitializedError for all properties/methods (system\_variable, variable\_pool, start\_at, total\_tokens, llm\_usage, outputs, node\_run\_steps, ready\_queue\_size, exceptions\_count, get\_output, dumps). - Update GraphEngineLayer: - Annotate graph\_runtime\_state as ReadOnlyGraphRuntimeState (no | None). - Default it to UninitializedReadOnlyGraphRuntimeState() in \_\_init\_\_. - Keep command\_channel as-is (optional) unless you decide to give it an uninitialized sentinel. - Update initialize() docstring to note it may be called more than once (early binding + engine run) and should be idempotent. - Update GraphEngine.layer(): - Bind context immediately by calling layer.initialize(ReadOnlyGraphRuntimeStateWrapper(self.\_graph\_runtime\_state), self.\_command\_channel). - Keep on\_graph\_start only in \_initialize\_layers; do not call it here. - If you add a helper like \_bind\_layer\_context(layer) to avoid duplication, use it both here and in \_initialize\_layers. Acceptance: - graph\_runtime\_state is never None at runtime or in types. - Accessing graph\_runtime\_state before binding raises GraphEngineLayerNotInitializedError with a clear message. - After engine.layer(layer), graph\_runtime\_state is readable without None checks. Files likely touched: - api/core/workflow/graph\_engine/layers/base.py - api/core/workflow/graph\_engine/graph\_engine.py - api/core/workflow/runtime/read\_only\_wrappers.py (if you locate UninitializedReadOnlyGraphRuntimeState there)
…zed access (vibe-kanban 0717c38f) Parent: 7c1fead6-6f01-4778-9089-3782e9bcc6b1 (Make self.graph\_runtime\_state in GraphEngineLayer not None) Background: GraphEngineLayer.graph\_runtime\_state must never be None. We will introduce an UninitializedReadOnlyGraphRuntimeState that raises GraphEngineLayerNotInitializedError until GraphEngine binds a real ReadOnlyGraphRuntimeStateWrapper. GraphEngine.layer() will bind early so users can read state after registration without None checks. Work: - Add unit tests for the new non-null contract and uninitialized access behavior. - Prefer a new focused test file: api/tests/unit\_tests/core/workflow/graph\_engine/layers/test\_layer\_initialization.py (or extend api/tests/unit\_tests/core/workflow/graph\_engine/test\_graph\_engine.py if you want to keep related tests together). Test cases (Arrange-Act-Assert): 1) test\_layer\_runtime\_state\_raises\_when\_uninitialized - Create a minimal TestLayer(GraphEngineLayer) with no overrides. - Instantiate it without engine binding. - Assert accessing any property (e.g., layer.graph\_runtime\_state.outputs or layer.graph\_runtime\_state.dumps()) raises GraphEngineLayerNotInitializedError. 2) test\_layer\_runtime\_state\_available\_after\_engine\_layer - Use WorkflowRunner + fixture (e.g., simple\_passthrough\_workflow) to create graph and GraphRuntimeState. - Create GraphEngine with InMemoryChannel. - Register layer via engine.layer(layer). - Assert accessing layer.graph\_runtime\_state.\* does not raise and returns expected defaults (e.g., outputs == {} initially, ready\_queue\_size is int >= 0). Notes: - Import GraphEngineLayerNotInitializedError from the module you add it to. - Ensure types are explicit and no Any. Run (targeted): uv run --project api -m pytest api/tests/unit\_tests/core/workflow/graph\_engine/layers/test\_layer\_initialization.py - fix(app): remove boolean guard on TriggerPostLayer graph_runtime_state access Tests not run (not requested).
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…l with property guard and update layer/tests Tests not run (not requested).
WalkthroughThe changes refactor GraphEngineLayer initialization to guarantee runtime state is bound before layer hooks execute. A new exception Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (11)
💤 Files with no reviewable changes (1)
🧰 Additional context used🧬 Code graph analysis (5)api/tests/unit_tests/core/app/layers/test_pause_state_persist_layer.py (1)
api/tests/unit_tests/core/workflow/graph_engine/layers/test_layer_initialization.py (2)
api/core/workflow/graph_engine/layers/debug_logging.py (2)
api/core/workflow/graph_engine/layers/base.py (1)
api/core/workflow/graph_engine/graph_engine.py (2)
🔇 Additional comments (18)
✏️ Tip: You can disable this entire section by setting Comment |
2 participants
Benchmark PR from qodo-benchmark#177
Summary by CodeRabbit
Documentation
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.