refactor(engine)!: simplify execution architecture - #249
Conversation
📄 Knowledge review✏️ Suggested updates2 page suggestions need review.
|
|
All contributors on this pull request have signed the CLA. |
4c3f8a5 to
a6b49de
Compare
a6b49de to
8c4a904
Compare
Consolidate frame scheduling, event processing, commands, layers, filters, workers, and runtime execution state behind direct module boundaries and canonical public imports. BREAKING CHANGE: Rename the engine, runtime, event, layer, filter, command, and container APIs; remove legacy import paths and compatibility aliases; unify nested event ownership under container_id.
Keep full-graph edge ordinals in scoped frame configs, restore legacy snapshot supersets into only the attached graph, and prevent child traversal events from unblocking root response paths.
Use public DSL edge IDs within each graph and attach frame IDs to traversal events. Migrate RuntimeState v2 and ResponseStreamFilter v1 snapshots at their deserialization boundaries while keeping current snapshots strict.
- propagate inherited loop variable updates without leaking child-local values\n- resolve canonical and legacy container ownership for loop ends and nested DSLs\n- harden lifecycle events, legacy snapshot/filter migration, and stalled resumes
Continue polling when the dispatch queue is temporarily empty instead of inferring terminal idleness from an approximate queue size. Reserve generated edge IDs within each owning graph, treat unattached snapshot state as absent, and validate worker counts before mutating runtime state.
Keep Redis commands readable by pre-refactor consumers during rolling deployments by emitting their marker and variable-update wire shape. Reject non-container graph owners, resolve canonical iteration ownership, and correct the layer metrics example.
c216567 to
dc0be95
Compare
Prioritize aborts and failures over pauses, keep commands observable while pause draining, and reject orphaned resume tasks before workers start. Preserve parent-authoritative Loop updates across child frames and terminal results, and preflight every descendant's ownership, plugin dependencies, and credentials before node construction.
Propagate external variable updates to live descendant frames that already expose the selector while preserving independent variable identities. Reconcile loop state only after successful completion so configured exception fallbacks remain authoritative.
|
Hi @laipz8200 — thank you for driving this refactor. We run exported Dify DSL Finding: fixed polling adds 25–50 ms of dead time per node hopThree fixed sleeps serialize every node transition:
On a sequential chain these compound: 25–50 ms per node hop, regardless of Measured impact (OpenTelemetry span attribution, 110 samples × 2 rounds)Production-shaped Chatflow: ~22 nodes/turn, remote plugin daemon, mock LLM Per-category gap: 407–596 ms across scenario groups; P95 wall 1162 ms. Two-line fix, verified--- a/src/graphon/engine/worker/worker.py
+++ b/src/graphon/engine/worker/worker.py
@@ -129,7 +129,7 @@
if not task_claimed:
- self._stop_event.wait(0.1)
+ self._stop_event.wait(0.0005)
continue
--- a/src/graphon/engine/dispatcher.py
+++ b/src/graphon/engine/dispatcher.py
@@ (both call sites)
- task = self._dispatch_queue.get(timeout=0.1)
+ task = self._dispatch_queue.get(timeout=0.002)Results on the same 110-sample suite (branch coverage unchanged at 55/55):
RequestSince #249 is rewriting these exact modules: could the new scheduler / Happy to contribute the patch against this branch, add an "interactive |
Important
!Related Issue
Closes #248
Summary
This refactor reduces the engine's public vocabulary and collapses overlapping execution responsibilities. It intentionally removes compatibility aliases: downstream integrations must upgrade imports, constructor calls, extension hooks, and event field access together.
Architecture changes
GraphEngineand thegraph_enginepackage withEngineandgraphon.engine.RuntimeState; child frames receive only their container-scoped graph. Child-local variables never escape their frame; Loop Variable Assigner updates to selectors already owned by a parent are written back, while Iteration frames remain isolated.Scheduler, and keep frame creation/restoration inFrameRegistry.GraphEngineConfigare removed.event, command channels/processing intocommand, and use singularcontainer_handler,filter,layer, andworkermodules.Downstream migration
Imports and public names
graphon.graph_enginegraphon.enginegraphon.graph_eventsgraphon.engine_eventsgraphon.filters/ old engine filter pathsgraphon.engine.filterlayers,command_channels,command_processing,container_handlers, andworker_managementpathsgraphon.engineGraphEngineEngineGraphEngineEventEngineEventGraphNodeEventBaseNodeEventNodeEventBaseNodeEventPayloadGraphRuntimeStateRuntimeStateGraphInitParamsInitParamsGraphEngineLayerLayerGraphEventFilterEngineEventFilterGraphEventFilterContextEngineEventFilterContextfilter_graph_events(...)filter_engine_events(...)TaskEventNodeEventTaskConcrete
GraphRun*Event,GraphEdge*Event, andNodeRun*Eventnames remain unchanged. There are no aliases for the old imports or type names.Engine construction and identity
GraphEngineConfig; pass the fixed worker count asworkers=when loading or constructing an engine.workersmust be a positive integer; booleans and floats are rejected before runtime state is attached.workflow_idtoEngine. Set it on the rootRuntimeState; restored and child states share itsGraphExecutionidentity.InMemoryChannel.engine.layer(layer)withengine.add_layer(layer). The method mutates the engine and returnsNone.Events and containers
in_loop_idandin_iteration_idwith onecontainer_idfield on engine events.container_ididentifies the event's direct owning container. Top-level events usecontainer_id == ""; nested events do not expose ancestor container IDs.NodeEventPayload;Node.run()adds execution context and emitsNodeEvent, which is part of theEngineEventstream.container_idis defined by the commonEngineEvent, so GraphRun lifecycle events expose the same field and use""at the top level.loop_idanditeration_idremain readable when their container ancestry identifies the direct owner. Only genuinely ambiguous definitions must add canonicalcontainer_id.NodeExecutionType.CONTAINER; ordinary nodes can no longer own silently skipped child scopes. Downstream custom container types remain supported through their node factory execution type.container_idownership as well as legacyiteration_id.UpdateVariablesCommandvalues processed after child write-back survive later rounds, terminal results, pauses, and resume; integrations must not reapply stale Loop result outputs to those selectors.Layers and filters
DebugLoggingLayerandGraphEngineLayerNotInitializedError; invalid layer use now raises the standard runtime error.ResumableEngineEventFilterandfilter_id; filters operate directly on the engine event stream.ResponseStreamFilterandExecutionLimitsLayeruse their canonical singular-module imports. NonessentialExecutionLimitsLayerhelpers andLimitTypeare no longer public API.Commands and remote control
CommandProcessornow receivesframe_registry=instead ofvariable_pool=. Custom constructors must register the root frame before polling commands so workflow-level updates reach every live inherited scope.GraphEngineManager. Downstream applications now own task IDs, Redis key naming, and command routing policy.GraphEngineCommand/CommandTypebranching with the concrete discriminated command types such asAbortCommand,PauseCommand, andUpdateVariablesCommand.VariableUpdatewrapper;UpdateVariablesCommand.updatesreceives variables directly.:pendingmarker and{ "value": variable }update wrapper. New readers do not require the marker and accept both wrapped and direct update values. Custom Redis pipelines must continue supportingsetin addition to list read/delete, push, and expiry operations. The marker and wrapper can be removed after pre-refactor consumers have been retired for at least one command TTL.Extension APIs
ContainerHandler.prepare_frame_event(...)now observes variable-update events after the source frame pool has been updated. Custom handlers can inspect or propagate the stored value without reapplying the event.ContainerHandlerhooks change as follows:start_await(...)handle_request(...)complete_frame(...)complete_frame_if_ready(...)should_collect(...)should_emit(...)FrameRegistrychanges as follows:materialize_frame(...)create(...)materialize_child_frame(...)create_child(...)materialize_child_frame_from_state(...)restore_child(...)registry.get(frame_id)registry[frame_id]Execution frame fields change as follows:
frame.graph_runtime_stateframe.stateframe.state_manager/frame.edge_processorframe.schedulerframe.error_handlerframe.failure_handlerCustom node factories must implement pure
validate_node(...) -> NodeExecutionTypepreflight, return an isolated scoped copy fromwith_graph_config(...), and continue binding child runtime state throughwith_runtime_state(...).validate_node(...)must resolve the same concrete implementation and schema ascreate_node(...), return that implementation's execution type, and resolve static plugin and credential requirements without constructing a Node, Slim/tool runtime, or other execution collaborator. Factories that ownInitParamsmust copy it with the passedgraph_configbeforecreate_node(...)can invoke node constructors orpost_init(); do not mutate a factory shared with parent or sibling frames. Custom ready queues must consumeReadyTaskvalues and implement serialization through the new ready-queue module;ReadyQueueStateis no longer public.Removed internal boundaries
GraphStateManager,EdgeProcessor,graph_traversal,event_management,orchestration,error_handler, and dynamic worker-management modules.GraphExecutionandNodeExecutiontographon.runtime.execution.WorkflowExecutiontemporarily for downstream compatibility, with a TODO marking its future removal. Replace the event stream's custom read/write lock withthreading.Lock; downstream code must not import or depend on the removedReadWriteLock.Edge identity and persisted state
Edge.idnow preserves the public DSL edge ID and is unique within its owning graph instead of being replaced by a generated runtimeedge_NID. Missing DSL IDs still receive a deterministicedge_Nfallback. Explicit IDs reserve matching fallback names only within the owning graph, so separate frame graphs may reuse the same edge ID.frame_id. Consumers combining events from multiple frames must use(frame_id, edge_id)as the runtime identity; callers constructing traversal events must provideframe_id.RuntimeStatesnapshots advance from2.0to3.0. A removable, validator-gated compatibility path translates legacy full-graphedge_Nstate only after the persisted graph is attached.ResponseStreamFiltersnapshots advance from1.0to2.0. Its isolated compatibility path translates legacy paths when the filter is initialized against the persisted graph. ProgrammaticGraph.new()graphs withoutgraph_configcan restore version 1 filter state only while their complete edge IDs remain the original contiguousedge_Nsequence.Persistence compatibility
RuntimeState, root-only version 1RuntimeState, and version 1ResponseStreamFiltersnapshots remain readable through the bounded compatibility paths described above.ResumeTaskwhose container invocation is missing now fails synchronously before workers start, with ready and deferred tasks returned to their original queues. Downstream resume code should treat this as corrupt or incompatible persisted state rather than a successful or retryable run.GraphRunPausedEvent.InitParams.model_dump()data is unchanged, although its generated schema title changes with the class name.isinstancechecks, schema-title assertions, and pickles that refer to removed class/module names are not compatible.container_id.Dify adaptation scope
Dify should upgrade Graphon and migrate in one change. The main affected areas are workflow engine construction, runtime-state creation and restoration, layer registration, command/stop routing, event filters, persistence and response conversion, custom container handlers, and type annotations. In particular, all
in_loop_id/in_iteration_idaccess in runners, persistence layers, and response converters must use direct-ownercontainer_idsemantics rather than mechanically preserving both ancestor fields.Checklist
!