feat(graph-engine): make layer runtime state non-null and bound early#9
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).
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
2 participants
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.
Benchmark PR from qodo-benchmark#177
Note
Makes layer runtime state reliably available and fail-fast if uninitialized.
GraphEngineLayerNotInitializedErrorand non-nullgraph_runtime_stateproperty;initialize()is idempotentengine.layer()(early), simplifies_initialize_layers()and removes prior None checks in layersPauseStatePersistenceLayer,TriggerPostLayer,DebugLoggingLayer, andWorkflowPersistenceLayerto rely on always-availablegraph_runtime_stateengine.layer(); adds newtest_layer_initializationworkflow/README.mdandlayers/README.mdWritten by Cursor Bugbot for commit 9244e52. Configure here.