resolve mechanical architecture review issues (C-01,C-02,C-03,W-01,W-09,W-10,W-12)
- C-01: fix broken README link (call-graph-runtime.md → call-graph.md)
- C-02: add CallEdgeAttrs union type alias in schema.md
- C-03/W-12: rename TypedEdgeAttrs → OperationEdgeAttrs for consistent
{GraphType}EdgeAttrs naming pattern, update all references
- W-01: standardize terminology — prerequisites=structural/graph,
preconditions=reactive/computed, rename WorkflowNode.prerequisites
to preconditions, rename computePrerequisites to computePreconditions
- W-09: update ADR-001/002/003 status from Proposed to Accepted
- W-10: clarify call graph mutation API — addCall creates triggered
edges automatically, addDependency creates depends_on edges
- update review checklist with resolved items
This commit is contained in:
@@ -53,7 +53,7 @@ Flowgraph is in Phase 0/1 (exploration → architecture). No code exists yet. Th
|
||||
|----------|---------|
|
||||
| [schema.md](schema.md) | TypeBox Module, TypeScript types, enums (CallStatus, EdgeType, NodeStatus), node/edge attribute schemas, SerializedGraph factory |
|
||||
| [operation-graph.md](operation-graph.md) | Static graph from OperationSpecs, type-compatibility edges, construction paths, validation |
|
||||
| [call-graph-runtime.md](call-graph-runtime.md) | Dynamic graph from call events, node lifecycle, abort cascading, fromCallEvents construction |
|
||||
| [call-graph.md](call-graph.md) | Dynamic graph from call events, node lifecycle, abort cascading, fromCallEvents construction |
|
||||
| [workflow-templates.md](workflow-templates.md) | ujsx components (`<Operation>`, `<Sequential>`, `<Parallel>`, `<Conditional>`), template→DAG hydration, serialization |
|
||||
| [host-configs.md](host-configs.md) | Graphology HostConfig (template→DAG analysis), Reactive HostConfig (template→execution engine), Instance types |
|
||||
| [reactive-execution.md](reactive-execution.md) | Signal-driven status propagation, computed preconditions, abort cascade via signals, ReactiveRoot integration |
|
||||
@@ -107,7 +107,7 @@ src/
|
||||
schema/
|
||||
enums.ts # CallStatus, NodeStatus, EdgeType, OperationType
|
||||
node.ts # OperationNodeAttributes, CallNodeAttributes
|
||||
edge.ts # TypeCompatEdgeAttrs, ParentChildEdgeAttrs, DependencyEdgeAttrs
|
||||
edge.ts # OperationEdgeAttrs, CallEdgeAttrs, TemplateEdgeAttrs
|
||||
graph.ts # FlowGraphSerialized (graphology export format)
|
||||
index.ts
|
||||
graph/
|
||||
|
||||
@@ -25,7 +25,7 @@ Package structure, exports map, dependencies, and platform targets.
|
||||
│ ├── schema/
|
||||
│ │ ├── enums.ts # CallStatus, EdgeType, NodeCategory, NodeStatus
|
||||
│ │ ├── node.ts # OperationNodeAttrs, CallNodeAttrs
|
||||
│ │ ├── edge.ts # TypedEdgeAttrs, CallEdgeAttrs, TemplateEdgeAttrs
|
||||
│ │ ├── edge.ts # OperationEdgeAttrs, CallEdgeAttrs, TemplateEdgeAttrs
|
||||
│ │ ├── graph.ts # SerializedGraph, FlowGraphSerialized
|
||||
│ │ └── index.ts
|
||||
│ ├── graph/
|
||||
|
||||
@@ -209,12 +209,14 @@ Payload fields (`input`, `output`, `error`) are stored as-is in the graph. The h
|
||||
|
||||
```typescript
|
||||
// Add a call node (from call.requested event)
|
||||
// If attrs.parentRequestId is set, also creates a triggered edge from parent to child
|
||||
addCall(attrs: CallNodeAttrs): void
|
||||
|
||||
// Update call status (from call.responded/error/aborted/completed event)
|
||||
updateStatus(requestId: string, status: CallStatus, extra?: Partial<CallNodeAttrs>): void
|
||||
|
||||
// Add a dependency edge (explicit, not auto-populated)
|
||||
// Add a dependency edge (explicit, not auto-populated by call protocol)
|
||||
// Creates an edge with edgeType: "depends_on"
|
||||
addDependency(source: string, target: string): void
|
||||
|
||||
// Remove a call node and its edges
|
||||
@@ -224,7 +226,7 @@ removeCall(requestId: string): void
|
||||
updateCall(requestId: string, attrs: Partial<CallNodeAttrs>): void
|
||||
```
|
||||
|
||||
`updateStatus` validates the transition. `addDependency` validates that both endpoints exist. `removeCall` removes the node and all attached edges (graphology cascade).
|
||||
`addCall` is the primary entry point for populating the call graph from call events. When `attrs.parentRequestId` is present, it automatically creates a `triggered` edge from the parent to the new node. `addDependency` creates explicit `depends_on` edges that represent data dependencies not captured by the parent-child hierarchy. `updateStatus` validates the transition. `addDependency` validates that both endpoints exist and that the edge would not create a cycle. `removeCall` removes the node and all attached edges (graphology cascade).
|
||||
|
||||
## Constraints
|
||||
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
## Status
|
||||
|
||||
Proposed
|
||||
Accepted
|
||||
|
||||
## Context
|
||||
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
## Status
|
||||
|
||||
Proposed
|
||||
Accepted
|
||||
|
||||
## Context
|
||||
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
## Status
|
||||
|
||||
Proposed
|
||||
Accepted
|
||||
|
||||
## Context
|
||||
|
||||
|
||||
@@ -172,7 +172,7 @@ interface WorkflowNode {
|
||||
key: string; // Operation name or structural container ID
|
||||
type: "operation" | "sequential" | "parallel" | "conditional" | "map";
|
||||
status: Signal<NodeStatus>; // Reactive status signal
|
||||
prerequisites: Computed<boolean>; // Computed: true when all prerequisites are met
|
||||
preconditions: Computed<boolean>; // Computed: true when all preconditions are met
|
||||
operationId?: string; // For operation nodes: the fully qualified ID
|
||||
output?: Signal<unknown>; // For operation nodes: the call result (when completed)
|
||||
children: WorkflowNode[]; // Child nodes (structural containers have children)
|
||||
@@ -181,7 +181,7 @@ interface WorkflowNode {
|
||||
|
||||
Each `WorkflowNode` holds:
|
||||
- A `signal<NodeStatus>` that tracks the call's lifecycle (`idle` → `waiting` → `ready` → `running` → `completed`/`failed`/`aborted`/`skipped`)
|
||||
- A `computed` that derives `prerequisites` from parent nodes' statuses
|
||||
- A `computed` that derives `preconditions` from parent nodes' statuses
|
||||
- An optional `output` signal that holds the call result when completed
|
||||
|
||||
### ReactiveContext
|
||||
@@ -204,7 +204,7 @@ createInstance(tag: WorkflowTag, props, ctx: ReactiveContext, parent?: WorkflowN
|
||||
key,
|
||||
type: tag,
|
||||
status,
|
||||
prerequisites: computed(() => computePrerequisites(node, ctx)),
|
||||
preconditions: computed(() => computePreconditions(node, ctx)),
|
||||
children: [],
|
||||
};
|
||||
|
||||
@@ -222,14 +222,14 @@ createInstance(tag: WorkflowTag, props, ctx: ReactiveContext, parent?: WorkflowN
|
||||
|
||||
### Prerequisite Computation
|
||||
|
||||
The `prerequisites` computed signal for each node derives from its structural context:
|
||||
The `preconditions` computed signal for each node derives from its structural context:
|
||||
|
||||
- **Sequential child**: prerequisites = previous sibling is `completed`
|
||||
- **Parallel child**: prerequisites = parent's prerequisites are met
|
||||
- **Conditional child**: prerequisites = parent's prerequisites are met AND condition evaluates to true
|
||||
- **Sequential child**: preconditions = previous sibling is `completed`
|
||||
- **Parallel child**: preconditions = parent's preconditions are met
|
||||
- **Conditional child**: preconditions = parent's preconditions are met AND condition evaluates to true
|
||||
|
||||
```typescript
|
||||
function computePrerequisites(node: WorkflowNode, ctx: ReactiveContext): boolean {
|
||||
function computePreconditions(node: WorkflowNode, ctx: ReactiveContext): boolean {
|
||||
// Sequential: previous sibling must be completed
|
||||
// Parallel: parent must be ready
|
||||
// Conditional: condition must evaluate to true
|
||||
@@ -243,11 +243,11 @@ function computePrerequisites(node: WorkflowNode, ctx: ReactiveContext): boolean
|
||||
|
||||
### Status Propagation
|
||||
|
||||
When a node's `status` signal changes, its dependents' `prerequisites` computed automatically re-evaluate. If prerequisites are met, the node transitions to `ready`:
|
||||
When a node's `status` signal changes, its dependents' `preconditions` computed automatically re-evaluate. If preconditions are met, the node transitions to `ready`:
|
||||
|
||||
```typescript
|
||||
effect(() => {
|
||||
if (node.prerequisites.value) {
|
||||
if (node.preconditions.value) {
|
||||
node.status.value = "ready";
|
||||
}
|
||||
});
|
||||
@@ -314,7 +314,7 @@ For flowgraph, this is acceptable in v1 because:
|
||||
|
||||
The current design where `Sequential`, `Parallel`, and `Conditional` don't create graph nodes is clean for the DAG, but creates complexity for the reactive engine — the "previous sibling" precondition depends on understanding the structural context, which isn't stored on the node itself.
|
||||
|
||||
Alternative: Create "virtual" nodes for structural containers that hold `signal<NodeStatus>` but don't correspond to graph nodes. This makes the reactive engine simpler (every node has a status and prerequisites) at the cost of a slightly larger node tree.
|
||||
Alternative: Create "virtual" nodes for structural containers that hold `signal<NodeStatus>` but don't correspond to graph nodes. This makes the reactive engine simpler (every node has a status and preconditions) at the cost of a slightly larger node tree.
|
||||
|
||||
### Conditional Test Evaluation
|
||||
|
||||
@@ -331,7 +331,7 @@ The `Conditional.test` prop can be a function or a string. At the template level
|
||||
|
||||
## Open Questions
|
||||
|
||||
1. **Should structural containers create "virtual" nodes in the reactive engine?** This would simplify prerequisite computation (every node has a status) but adds nodes that don't correspond to calls or operations.
|
||||
1. **Should structural containers create "virtual" nodes in the reactive engine?** This would simplify precondition computation (every node has a status) but adds nodes that don't correspond to calls or operations.
|
||||
|
||||
2. **Should the GraphologyHostConfig produce a separate graph for edge types?** Currently all edge types (`sequential`, `conditional`, `typed`) share the same graph. An alternative is a separate graph per edge type, enabling type-specific queries without filtering.
|
||||
|
||||
|
||||
@@ -179,7 +179,7 @@ See [analysis.md](analysis.md) for the full type-compatibility algorithm.
|
||||
|
||||
## References
|
||||
|
||||
- Schema: [schema.md](schema.md) — `OperationNodeAttrs`, `TypedEdgeAttrs`, `CallStatus`, `EdgeType`
|
||||
- Schema: [schema.md](schema.md) — `OperationNodeAttrs`, `OperationEdgeAttrs`, `CallStatus`, `EdgeType`
|
||||
- Type compatibility: [analysis.md](analysis.md)
|
||||
- Call graph: [call-graph.md](call-graph.md)
|
||||
- Operation types: `@alkdev/operations/src/types.ts`
|
||||
|
||||
@@ -182,18 +182,20 @@ The node key is `requestId`. This matches the call protocol's correlation mechan
|
||||
|
||||
## Edge Attribute Schemas
|
||||
|
||||
### TypedEdgeAttrs (Operation Graph)
|
||||
### OperationEdgeAttrs (Operation Graph)
|
||||
|
||||
```typescript
|
||||
const TypedEdgeAttrs = Type.Object({
|
||||
const OperationEdgeAttrs = Type.Object({
|
||||
compatible: Type.Boolean({ description: "Whether the source output schema is compatible with the target input schema" }),
|
||||
compatibilityDetail: Type.Optional(Type.String({ description: "Human-readable description of compatibility or mismatch" })),
|
||||
});
|
||||
type TypedEdgeAttrs = Static<typeof TypedEdgeAttrs>;
|
||||
type OperationEdgeAttrs = Static<typeof OperationEdgeAttrs>;
|
||||
```
|
||||
|
||||
Type-compatibility edges carry a boolean `compatible` flag and optional detail. This allows the operation graph to include both compatible edges (green paths) and incompatible edges (red paths) for diagnostics.
|
||||
|
||||
**Naming note**: Previously named `TypedEdgeAttrs`. Renamed to follow the `{GraphType}EdgeAttrs` pattern used by `CallEdgeAttrs` and `TemplateEdgeAttrs`.
|
||||
|
||||
### TriggeredEdgeAttrs (Call Graph)
|
||||
|
||||
```typescript
|
||||
@@ -212,6 +214,14 @@ type DependencyEdgeAttrs = Static<typeof DependencyEdgeAttrs>;
|
||||
|
||||
Data dependency edges also carry no additional attributes. Future extensions may include `dataPath` (which field of the output feeds which field of the input).
|
||||
|
||||
### CallEdgeAttrs (Call Graph Union)
|
||||
|
||||
```typescript
|
||||
type CallEdgeAttrs = TriggeredEdgeAttrs | DependencyEdgeAttrs;
|
||||
```
|
||||
|
||||
A union type used as the edge attribute type parameter for call graphs (`FlowGraph<CallNodeAttrs, CallEdgeAttrs>`). Call graph edges can be either `triggered` (parent-child) or `depends_on` (data dependency), distinguished by their edge type. The union type follows the `{GraphType}EdgeAttrs` naming pattern consistent with `OperationEdgeAttrs` and `TemplateEdgeAttrs`.
|
||||
|
||||
### TemplateEdgeAttrs (Workflow Templates)
|
||||
|
||||
```typescript
|
||||
@@ -267,13 +277,13 @@ Two specialized serialization types, one for each graph type:
|
||||
```typescript
|
||||
const OperationGraphSerialized = SerializedGraph(
|
||||
OperationNodeAttrs,
|
||||
TypedEdgeAttrs,
|
||||
OperationEdgeAttrs,
|
||||
Type.Object({}), // No graph-level attributes
|
||||
);
|
||||
|
||||
const CallGraphSerialized = SerializedGraph(
|
||||
CallNodeAttrs,
|
||||
Type.Union([TriggeredEdgeAttrs, DependencyEdgeAttrs]),
|
||||
CallEdgeAttrs,
|
||||
Type.Object({}), // No graph-level attributes
|
||||
);
|
||||
```
|
||||
|
||||
@@ -165,7 +165,7 @@ The HostConfig maps ujsx component types to graphology operations:
|
||||
### Edge creation rules
|
||||
|
||||
- **Sequential**: For children C1, C2, ..., Cn, edges C1→C2, C2→C3, ..., C(n-1)→Cn are added. Within a sequential group, children have implicit `depends_on` edges.
|
||||
- **Parallel**: No edges between children. All children have the same prerequisites as the parallel group itself.
|
||||
- **Parallel**: No edges between children. All children have the same preconditions as the parallel group itself.
|
||||
- **Conditional**: Edge from the conditional node's prerequisite to the first child of the branch, with `edgeType: "conditional"` and `condition` attribute.
|
||||
- **Nested**: A `Sequential` inside a `Parallel` has its own internal edges. A `Parallel` inside a `Sequential` creates a subgraph where all parallel children share the same predecessor.
|
||||
|
||||
@@ -265,7 +265,7 @@ Validation returns an array of `ValidationError` objects (never throws). See [an
|
||||
- **`Operation` props are workflow metadata** — `name`, `input`, `retries`, `timeout` are NOT passed to the HostConfig's `createInstance`. They're workflow-level configuration that the reactive execution engine uses to configure the call.
|
||||
- **Function props are not serializable** — `Conditional.test` with a function cannot be round-tripped through JSON. Use string references for stored templates.
|
||||
- **Sequential ordering is structural, not temporal** — a `Sequential` group means "these operations should complete in order", not "start the next only after the previous completes" (though the reactive engine implements this via preconditions).
|
||||
- **Parallel has no structural edges** — a `Parallel` group produces no DAG edges between its children. The execution engine starts them concurrently when the group's prerequisites are met.
|
||||
- **Parallel has no structural edges** — a `Parallel` group produces no DAG edges between its children. The execution engine starts them concurrently when the group's preconditions are met.
|
||||
- **Conditional branches are either/or** — a `Conditional` node renders to one branch or the `else` branch. There's no "both" evaluation.
|
||||
|
||||
## Open Questions
|
||||
|
||||
@@ -270,16 +270,16 @@ Strong separation of concerns argument. The consequence "The hub must keep its s
|
||||
|
||||
When addressing these issues, use this checklist to track progress:
|
||||
|
||||
- [ ] C-01: Fix README cross-reference link
|
||||
- [ ] C-02: Add `CallEdgeAttrs` type alias to schema.md
|
||||
- [ ] C-03: Resolve `OperationEdgeAttrs` vs `TypedEdgeAttrs` naming
|
||||
- [x] C-01: Fix README cross-reference link
|
||||
- [x] C-02: Add `CallEdgeAttrs` type alias to schema.md
|
||||
- [x] C-03: Resolve `OperationEdgeAttrs` vs `TypedEdgeAttrs` naming (renamed `TypedEdgeAttrs` → `OperationEdgeAttrs`)
|
||||
- [ ] C-04: Specify failure propagation semantics in reactive-execution.md
|
||||
- [ ] C-05: Create FlowGraph public API document
|
||||
- [ ] C-06: Document `<Map>` component in workflow-templates.md
|
||||
- [ ] C-07: Specify `Conditional` else-branch behavior
|
||||
- [ ] C-08: Specify `WorkflowReactiveRoot` ↔ `ReactiveHostConfig` ownership
|
||||
- [ ] C-09: Create consumer integration guide
|
||||
- [ ] W-01: Standardize `prerequisites` vs `preconditions` terminology
|
||||
- [x] W-01: Standardize `prerequisites` vs `preconditions` terminology (prerequisites=structural/graph, preconditions=reactive/computed)
|
||||
- [ ] W-02: Add reactive error boundary semantics
|
||||
- [ ] W-03: Complete `ReactiveContext` interface definition
|
||||
- [ ] W-04: Add template composition rules
|
||||
@@ -287,10 +287,10 @@ When addressing these issues, use this checklist to track progress:
|
||||
- [ ] W-06: Document signal/effect disposal lifecycle
|
||||
- [ ] W-07: Consider ADR-004 for "no schema version"
|
||||
- [ ] W-08: Specify type compatibility depth
|
||||
- [ ] W-09: Update ADR statuses to Accepted
|
||||
- [ ] W-10: Clarify call graph mutation API (`addDependencyEdge`)
|
||||
- [x] W-09: Update ADR statuses to Accepted
|
||||
- [x] W-10: Clarify call graph mutation API (clarified `addCall` creates `triggered` edges automatically, `addDependency` creates `depends_on` edges)
|
||||
- [ ] W-11: Add performance characteristics section
|
||||
- [ ] W-12: Standardize edge attribute naming pattern
|
||||
- [x] W-12: Standardize edge attribute naming pattern (now `{GraphType}EdgeAttrs`: `OperationEdgeAttrs`, `CallEdgeAttrs`, `TemplateEdgeAttrs`)
|
||||
- [ ] S-01: Getting Started walkthrough document
|
||||
- [ ] S-02: Flow diagrams for template rendering pipeline
|
||||
- [ ] S-03: Node status state machine diagram
|
||||
|
||||
Reference in New Issue
Block a user