feat(graph-engine): make layer runtime state non-null and bound early #30552
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).
Summary of ChangesHello @laipz8200, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the robustness and predictability of the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
laipz8200
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request successfully refactors how self.graph_runtime_state is handled in GraphEngineLayer to eliminate the need for None checks, which is a great improvement for robustness and code clarity. The introduction of the UninitializedReadOnlyGraphRuntimeState placeholder is a clean and effective design pattern for this purpose, providing fail-fast behavior. The change to initialize layers upon registration with the engine is logical. The updates across various layers, tests, and documentation are thorough and consistent with the core change. The addition of new unit tests for the initialization logic further strengthens the quality of this contribution.
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).
laipz8200
requested review from
Mairuis,
QuantumGhost and
Yeuoly
as code owners
dosubot
bot
added
size:L
laipz8200
requested review from
fatelei
and removed request for
Mairuis,
QuantumGhost and
Yeuoly
Important
Fixes #<issue number>.Summary
Fixes #30551.
GraphEngineLayer.graph_runtime_statenon-null while surfacing early access.Screenshots
Checklist
dev/reformat(backend) andcd web && npx lint-staged(frontend) to appease the lint gods