From 907c33650fc7a1a48a7ff0a51f6d895eb706cd0d Mon Sep 17 00:00:00 2001 From: "glm-5.1" Date: Thu, 21 May 2026 19:40:45 +0000 Subject: [PATCH] fix: architecture review - address 5 critical issues, 6 warnings, 3 suggestions Critical fixes: - C1: Create standalone ADR-006 file (edge type consistency), extract from open-questions.md inline content - C2: Convert CallResult from plain interface to TypeBox schema, aligning with 'TypeBox as single source of truth' constraint - C3: Add fromJSON() cycle detection specification - enforce ADR-002 DAG invariant even on deserialized input - C4: Rewrite consumer-integration.md Phase 4 to use ADR-005 event-append pattern instead of direct signal mutation - C5: Fix operator precedence bug in consumer-integration.md (missing parentheses around OR condition) Warnings addressed: - W1: Fix immutability claim - operation graph is 'conventionally immutable', not prevented by API - W2: Add EventLogProjection to reactive exports map - W3: Add CallResult/CallResultSchema to schema exports map - W4: Fix reactive-execution.md Level 1 error handling to use event-append pattern instead of direct signal mutation - W5: Remove duplicate dataFlow inference description in schema.md - W6: Clarify ADR-006 project context (flowgraph vs taskgraph) Suggestions implemented: - S1: Add 'reviewed' document lifecycle status between draft/stable, update all docs to reviewed status - S2: Add carve-out note for analysis result types in schema.md constraints (they are ephemeral, not serialized) - S3: Add isComplete() and getAggregateStatus() convenience methods to WorkflowReactiveRoot specification --- docs/architecture/README.md | 10 +-- docs/architecture/analysis.md | 2 +- docs/architecture/build-distribution.md | 6 +- docs/architecture/call-graph.md | 2 +- docs/architecture/consumer-integration.md | 50 ++++++++++---- .../decisions/006-edge-type-consistency.md | 65 +++++++++++++++++++ docs/architecture/error-handling.md | 2 +- docs/architecture/flowgraph-api.md | 12 ++-- docs/architecture/host-configs.md | 2 +- docs/architecture/open-questions.md | 31 +-------- docs/architecture/operation-graph.md | 2 +- docs/architecture/reactive-execution.md | 47 ++++++++++++-- docs/architecture/schema.md | 41 +++++------- docs/architecture/workflow-templates.md | 2 +- 14 files changed, 189 insertions(+), 85 deletions(-) create mode 100644 docs/architecture/decisions/006-edge-type-consistency.md diff --git a/docs/architecture/README.md b/docs/architecture/README.md index 618235e..17157bc 100644 --- a/docs/architecture/README.md +++ b/docs/architecture/README.md @@ -1,5 +1,5 @@ --- -status: draft +status: reviewed last_updated: 2026-05-21 --- @@ -72,6 +72,7 @@ Flowgraph is in Phase 0/1 (exploration → architecture). No code exists yet. Th | [003](decisions/003-storage-decoupled.md) | Storage is not flowgraph's concern — in-memory graph with export/import boundary | | [004](decisions/004-no-schema-version.md) | No schema version field in serialized format — consumers wrap in their own versioned envelope | | [005](decisions/005-event-log-as-source-of-truth.md) | Execution Event Log as single source of truth — call protocol events as ground truth, status/result/call-graph as projections | +| [006](decisions/006-edge-type-consistency.md) | `edgeType` as universal required attribute on all edges; single shared graph per `FlowGraph` instance | ### Open Questions @@ -178,15 +179,16 @@ Architecture documents use YAML frontmatter with `status` and `last_updated` fie ```yaml --- -status: draft | stable | deprecated +status: draft | reviewed | stable | deprecated last_updated: YYYY-MM-DD --- ``` | Status | Meaning | Transitions | |--------|---------|-------------| -| `draft` | Under active development. Content may change significantly. Implementation should not start until the document reaches `stable`. | → `stable` when implementation is complete and API contract is verified by tests. | -| `stable` | API contracts are locked. Changes require a review cycle and may warrant an ADR if they affect documented decisions. | → `deprecated` when superseded. → `draft` if a fundamental redesign is needed (rare). | +| `draft` | Under active development. Content may change significantly. Implementation should not start until the document reaches `reviewed`. | → `reviewed` when all open questions are resolved and cross-cutting issues are addressed. | +| `reviewed` | Architecture is final and reviewed. Implementation may begin. API contracts are specified but not yet verified by tests. Changes require a review cycle. | → `stable` when implementation is complete and API contracts are verified by tests. → `draft` if a fundamental redesign is needed (rare). | +| `stable` | API contracts are locked and verified by tests. Changes require a review cycle and may warrant an ADR if they affect documented decisions. | → `deprecated` when superseded. → `draft` if a fundamental redesign is needed (rare). | | `deprecated` | Superseded by another document. Kept for reference. Links should point to the replacement. | Removed when no longer referenced. | ADR documents use a separate `Status` field in their body: `Proposed`, `Accepted`, `Deprecated`, or `Superseded`. ADRs never revert from `Accepted`. diff --git a/docs/architecture/analysis.md b/docs/architecture/analysis.md index 4ef4aef..e4e5094 100644 --- a/docs/architecture/analysis.md +++ b/docs/architecture/analysis.md @@ -1,5 +1,5 @@ --- -status: draft +status: reviewed last_updated: 2026-05-22 --- diff --git a/docs/architecture/build-distribution.md b/docs/architecture/build-distribution.md index 1e45ba0..4bd8170 100644 --- a/docs/architecture/build-distribution.md +++ b/docs/architecture/build-distribution.md @@ -1,5 +1,5 @@ --- -status: draft +status: reviewed last_updated: 2026-05-20 --- @@ -236,9 +236,9 @@ Following the taskgraph pattern, each module has a sub-path export: | `@alkdev/flowgraph` | Barrel export (everything) | Full import | | `@alkdev/flowgraph/component` | ``, ``, ``, ``, `` | Template authoring | | `@alkdev/flowgraph/host` | `GraphologyHostConfig`, `ReactiveHostConfig` | ujsx HostConfig implementations | -| `@alkdev/flowgraph/schema` | TypeBox schemas, enums, types | Schema-only import (no graph dependency) | +| `@alkdev/flowgraph/schema` | TypeBox schemas, enums, types (including `CallResultSchema`, `CallResult`) | Schema-only import (no graph dependency) | | `@alkdev/flowgraph/graph` | `FlowGraph` class, construction, mutation, queries | Core graph operations | -| `@alkdev/flowgraph/reactive` | `WorkflowReactiveRoot`, signal-based execution | Runtime execution | +| `@alkdev/flowgraph/reactive` | `WorkflowReactiveRoot`, `EventLogProjection`, signal-based execution | Runtime execution | | `@alkdev/flowgraph/analysis` | `typeCompat`, `validateTemplate`, ordering functions | Analysis and validation | | `@alkdev/flowgraph/error` | Error classes | Error handling | diff --git a/docs/architecture/call-graph.md b/docs/architecture/call-graph.md index 2e102cc..3ffa9b0 100644 --- a/docs/architecture/call-graph.md +++ b/docs/architecture/call-graph.md @@ -1,5 +1,5 @@ --- -status: draft +status: reviewed last_updated: 2026-05-22 --- diff --git a/docs/architecture/consumer-integration.md b/docs/architecture/consumer-integration.md index 4cffff2..183f4a7 100644 --- a/docs/architecture/consumer-integration.md +++ b/docs/architecture/consumer-integration.md @@ -1,5 +1,5 @@ --- -status: draft +status: reviewed last_updated: 2026-05-19 --- @@ -152,22 +152,50 @@ const reactiveHost = new ReactiveHostConfig(registry, workflowRoot); const reactiveRoot = createRoot(reactiveHost, {}); reactiveRoot.render(template); -// 4. Subscribe to status changes and effect-driven execution +// 4. Drive execution via the event log (ADR-005 pattern) +// The hub coordinator appends call protocol events; the projections derive state. for (const [nodeId, node] of workflowRoot.nodes) { // Start the call when preconditions are met effect(() => { - if (node.preconditions.value && node.status.value === "idle" || node.status.value === "waiting") { - node.status.value = "running"; - // getInput resolves the node's input from predecessor outputs and static config - // For Operation nodes, input comes from the template props or aggregated predecessor results - const input = resolveInput(nodeId, workflowRoot); - registry.execute(node.operationId, input, { parentRequestId: parentCallId }) - .then(result => { node.status.value = "completed"; node.output.value = result; }) - .catch(error => { node.status.value = "failed"; }); + if (node.preconditions.value && (node.status.value === "idle" || node.status.value === "waiting")) { + // All preconditions satisfied — start the call by appending to the event log + const operationId = dag.getNodeAttributes(nodeId).name; + const requestId = crypto.randomUUID(); + workflowRoot.nodeKeyToRequestId.set(nodeId, requestId); + + // Append call.requested event — the status projection derives "running" from this + workflowRoot.append({ + type: "call.requested", + requestId, + operationId, + input: getInput(nodeId, workflowRoot), + timestamp: new Date().toISOString(), + }); + + // Start the actual call — when it completes, append the result event + registry.execute(operationId, getInput(nodeId, workflowRoot), { parentRequestId }) + .then(result => { + // Append call.responded event — the status projection derives "completed" from this + workflowRoot.append({ + type: "call.responded", + requestId, + output: result, + timestamp: new Date().toISOString(), + }); + }) + .catch(error => { + // Append call.error event — the status projection derives "failed" from this + workflowRoot.append({ + type: "call.error", + requestId, + error: { code: error.code, message: error.message }, + timestamp: new Date().toISOString(), + }); + }); } }); - // Track failures + // Track failures for logging effect(() => { if (node.status.value === "failed") { console.error(`Node ${nodeId} failed`); diff --git a/docs/architecture/decisions/006-edge-type-consistency.md b/docs/architecture/decisions/006-edge-type-consistency.md new file mode 100644 index 0000000..b90a778 --- /dev/null +++ b/docs/architecture/decisions/006-edge-type-consistency.md @@ -0,0 +1,65 @@ +# ADR-006: Edge Type Consistency and Single-Graph Architecture + +## Status + +Accepted + +## Context + +Two related questions affect how edge types are represented in flowgraph: + +1. **Should `edgeType` be a required attribute on all edges, or only on edges where it varies?** Operation graphs only have `typed` edges, call graphs have `triggered` and `depends_on`, and template DAGs have `sequential` and `conditional`. Making `edgeType` required on every edge adds redundancy for operation graphs (where it's always `"typed"`) but ensures consistent serialization. + +2. **Should `GraphologyHostConfig` produce separate graphs per edge type, or a single shared graph?** Separate graphs per edge type would enable type-specific queries and avoid attribute namespace collisions, but add complexity for cross-graph traversal and cache coherence. + +These decisions affect the schema (schema.md), the serialized format, the call graph mutation API, and the host config implementations. + +## Decision + +1. **`edgeType` is a required universal attribute on every edge**, stored alongside (not inside) the mode-specific attribute schemas. This means the stored edge attributes are always `{ edgeType, ...modeSpecificAttrs }` at the graphology level. + +2. **All edge types share a single graphology `DirectedGraph` instance per `FlowGraph`.** No separate graphs per edge type. + +3. **Mode-specific attribute schemas (`OperationEdgeAttrs`, `TriggeredEdgeAttrs`, `DependencyEdgeAttrs`) do NOT include `edgeType`.** It's stored separately at the graphology level alongside the mode-specific attributes. + +4. **`TemplateEdgeAttrs` includes `edgeType` as a constrained union (`"sequential" | "conditional"`)** because template edges need to distinguish their type for rendering. + +## Rationale + +### Why `edgeType` on every edge + +- **Consistent serialization/deserialization** — graphology's native JSON format requires edge attributes. Having `edgeType` always present simplifies the format: every edge object has an `edgeType` field. No conditional logic to determine whether `edgeType` is present. + +- **Uniform graphology queries** — filtering edges by type is always `edge.attributes.edgeType === "triggered"`, regardless of graph mode. No need to check whether the attribute exists first. + +- **The redundancy for operation graphs is a minor cost** — `edgeType: "typed"` on every operation edge is two extra bytes per edge. The consistency benefit far outweighs the storage cost. + +### Why a single shared graph + +- **Cross-graph traversal is unnecessary at current scale** — the expected graph sizes (tens to hundreds of nodes) make separate graphs per edge type an over-engineering. Filtering by `edgeType` is O(n) on edges and negligible. + +- **Single graph simplifies the API** — `addEdge`, `removeEdge`, `getEdgeAttributes` work on a single graph. No need for `addTriggeredEdge`, `addDependencyEdge`, `getTypedEdge` etc. + +- **Separate graphs add complexity** — maintaining multiple graphology instances, keeping node sets synchronized across graphs, and handling cross-graph queries would add significant implementation and testing burden. + +- **Future optimization is possible without API changes** — if a concrete performance issue arises with very large graphs, an internal `Map>` edge index can be added as an optimization without changing the public API. + +## Consequences + +- **All `FlowGraph` instances store edges with `{ edgeType, ...modeSpecificAttrs }` at the graphology level.** The TypeBox schemas for mode-specific attributes (`OperationEdgeAttrs`, `TriggeredEdgeAttrs`, `DependencyEdgeAttrs`) define only what's unique to each mode; `edgeType` is added separately during edge creation. + +- **Edge-type filtering is done via standard graphology attribute queries.** Example: `graph.filterEdges((edge, attrs) => attrs.edgeType === "triggered")`. + +- **The `CallEdgeAttrs` union type is discriminated by `edgeType` at runtime** (not by TypeBox schema validation, since `TriggeredEdgeAttrs` and `DependencyEdgeAttrs` are both empty objects). When validating serialized call graph edges, the two-step process applies: (1) check that `edgeType` is present and matches the expected value for the graph mode, (2) validate remaining attributes against the mode-specific schema. + +- **Edge key format uses composite keys for multi-type scenarios.** The `triggered` edge type gets the simple `${source}->${target}` key format; `depends_on` always gets the composite `${source}->${target}:depends_on` format. This ensures deterministic keys even when multiple edge types connect the same node pair. + +- **Serialization validation is a two-step process:** (1) check that `edgeType` is present and matches the expected value for the graph mode, (2) validate remaining attributes against the mode-specific schema. + +## References + +- Schema: [schema.md](../schema.md) — Edge attribute schemas, Edge Key Convention +- Call graph: [call-graph.md](../call-graph.md) — `triggered` and `depends_on` edges +- Operation graph: [operation-graph.md](../operation-graph.md) — `typed` edges +- Host configs: [host-configs.md](../host-configs.md) — Single vs. separate graph decision +- Open questions: [open-questions.md](../open-questions.md) — OQ-004, OQ-029 \ No newline at end of file diff --git a/docs/architecture/error-handling.md b/docs/architecture/error-handling.md index 0cabedd..5d8c1fb 100644 --- a/docs/architecture/error-handling.md +++ b/docs/architecture/error-handling.md @@ -1,5 +1,5 @@ --- -status: draft +status: reviewed last_updated: 2026-05-20 --- diff --git a/docs/architecture/flowgraph-api.md b/docs/architecture/flowgraph-api.md index 612cafb..802931a 100644 --- a/docs/architecture/flowgraph-api.md +++ b/docs/architecture/flowgraph-api.md @@ -1,5 +1,5 @@ --- -status: draft +status: reviewed last_updated: 2026-05-22 --- @@ -87,6 +87,8 @@ static fromJSON(data: FlowGraphSerialized): FlowGraph${target}` format. No user-specified edge keys. -- **Operation graph is immutable after construction** — no mutation methods are exposed after `fromSpecs()`. If the registry changes, rebuild the graph. +- **Operation graph is conventionally immutable after construction** — `fromSpecs()` produces a graph that consumers should treat as immutable. Mutation methods (`addOperation`, `addTypedEdge`) are available for incremental construction but should not be used on factory-produced graphs. If the registry changes, rebuild the graph. - **Call graph supports incremental mutation** — `addCall`, `updateStatus`, `addDependency` are the primary mutation paths. ## Open Questions diff --git a/docs/architecture/host-configs.md b/docs/architecture/host-configs.md index d47287a..2092f7c 100644 --- a/docs/architecture/host-configs.md +++ b/docs/architecture/host-configs.md @@ -1,5 +1,5 @@ --- -status: draft +status: reviewed last_updated: 2026-05-22 --- diff --git a/docs/architecture/open-questions.md b/docs/architecture/open-questions.md index 719794b..9b12460 100644 --- a/docs/architecture/open-questions.md +++ b/docs/architecture/open-questions.md @@ -1,5 +1,5 @@ --- -status: draft +status: reviewed last_updated: 2026-05-22 --- @@ -303,34 +303,9 @@ Cross-cutting compilation of all unresolved questions across the flowgraph archi --- -## ADR-006: Edge Type Consistency and Single-Graph Architecture +### ADR-006: Edge Type Consistency and Single-Graph Architecture -**Status**: Accepted - -**Context**: Two related questions (OQ-04, OQ-29) affect how edge types are represented in flowgraph: -- Should `edgeType` be a required attribute on all edges, or only on edges where it varies? -- Should `GraphologyHostConfig` produce separate graphs per edge type, or a single shared graph? - -**Decision**: -1. `edgeType` is a required universal attribute on every edge, stored alongside (not inside) mode-specific attribute schemas. -2. All edge types share a single graphology `DirectedGraph` instance per `FlowGraph`. -3. Mode-specific attribute schemas (`OperationEdgeAttrs`, `TriggeredEdgeAttrs`, `DependencyEdgeAttrs`) do **not** include `edgeType` — it's stored separately at the graphology level. -4. `TemplateEdgeAttrs` includes `edgeType` as a constrained union (`"sequential" | "conditional"`) because template edges need to distinguish their type for rendering. - -**Rationale**: -- Consistent serialization/deserialization (graphology native JSON format requires edge attributes) -- Uniform graphology queries and edge-type filtering across all graph modes -- The redundancy for operation graphs (`edgeType` is always `"typed"`) is a minor cost for significant consistency gains -- Separate graphs per edge type would add complexity (cross-graph traversal, cache coherence, multi-graph queries) without benefit at current scale -- Single-graph filtering by `edgeType` is O(n) on edges — negligible for expected graph sizes - -**Consequences**: -- All `FlowGraph` instances store edges with `{ edgeType, ...modeSpecificAttrs }` at the graphology level -- Edge-type filtering is done via standard graphology attribute queries -- The `CallEdgeAttrs` union type is discriminated by `edgeType` at runtime (not by TypeBox schema validation, since both variants are empty objects) -- Serialization validation is a two-step process: (1) check that `edgeType` is present and matches the expected value for the graph mode, (2) validate remaining attributes against the mode-specific schema -- The `triggered` edge type gets the simple `${source}->${target}` key format; `depends_on` always gets the composite `${source}->${target}:depends_on` format (see schema.md Edge Key Convention) -- Future optimization (if needed) could add an internal `Map>` index without changing the public API +See [decisions/006-edge-type-consistency.md](decisions/006-edge-type-consistency.md) for the full decision record. --- diff --git a/docs/architecture/operation-graph.md b/docs/architecture/operation-graph.md index cb81bab..906559b 100644 --- a/docs/architecture/operation-graph.md +++ b/docs/architecture/operation-graph.md @@ -1,5 +1,5 @@ --- -status: draft +status: reviewed last_updated: 2026-05-22 --- diff --git a/docs/architecture/reactive-execution.md b/docs/architecture/reactive-execution.md index 5bf7d39..7fa7749 100644 --- a/docs/architecture/reactive-execution.md +++ b/docs/architecture/reactive-execution.md @@ -1,5 +1,5 @@ --- -status: draft +status: reviewed last_updated: 2026-05-22 --- @@ -509,6 +509,38 @@ function callStatusToNodeStatus(callStatus: CallStatus): NodeStatus { } ``` +### Aggregate Status + +For consumers that need to check whether a workflow has completed, the `WorkflowReactiveRoot` provides convenience methods: + +```typescript +/** + * Returns true when all nodes have reached a terminal state + * (completed, failed, aborted, or skipped). + * Useful for checking workflow completion without manually + * iterating statusMap. + */ +isComplete(): boolean + +/** + * Returns an aggregate status summary for the workflow. + * Useful for observability and completion tracking. + */ +getAggregateStatus(): { + completed: number; + failed: number; + aborted: number; + skipped: number; + running: number; + waiting: number; + ready: number; + idle: number; + total: number; +} +``` + +These methods derive from the `statusMap` and align with ADR-005's projection model — they read signal values rather than scanning the event log directly, since the signals are already projections of the log. + ## Event-Driven Execution Under ADR-005, the hub coordinator's responsibility shifts from directly setting signal values to **appending events to the log**. The reactive layer drives execution via `effect()`s that watch projections and invoke calls when preconditions are met. @@ -634,13 +666,18 @@ The reactive execution layer has three levels of error handling, each with disti ### Level 1: Signal-level errors (per-node) -When a call fails, the hub coordinator sets the node's status to `"failed"`: +When a call fails, the hub coordinator appends a `call.error` event to the event log: ```typescript -status.value = "failed"; // Individual node failure +workflowRoot.append({ + type: "call.error", + requestId, + error: { code: error.code, message: error.message }, + timestamp: new Date().toISOString(), +}); ``` -This triggers `blockedByFailure` in all downstream dependents, causing them to transition to `"aborted"`. The failure propagates through the signal graph reactively — no manual error handling is needed. +The status projection derives `NodeStatus.failed` from this event. The `blockedByFailure` computed in all downstream dependents automatically re-evaluates, causing them to transition to `"aborted"`. The failure propagates through the signal graph reactively — no manual error handling is needed. ### Level 2: Conditional error boundaries (branch-level) @@ -700,7 +737,7 @@ The `WorkflowErrorBoundary` catches errors that escape the signal graph (e.g., a | Error type | Scope | Mechanism | Recovery | |------------|-------|-----------|----------| -| Call failure | Single node | `status.value = "failed"` | Cascades to dependents via `blockedByFailure` | +| Call failure | Single node | `workflowRoot.append({ type: "call.error", ... })` | Cascades to dependents via `blockedByFailure` | | Caught by Conditional | Branch | `Conditional.test` evaluates against failed status | Redirect to else-branch, downstream sees `completed` | | Uncaught cascade | Downstream chain | `blockedByFailure` effects | Downstream nodes transition to `aborted` | | System failure | Entire workflow | `abortAll()` | All non-terminal nodes to `aborted` | diff --git a/docs/architecture/schema.md b/docs/architecture/schema.md index 3ac10c1..fb76b13 100644 --- a/docs/architecture/schema.md +++ b/docs/architecture/schema.md @@ -1,5 +1,5 @@ --- -status: draft +status: reviewed last_updated: 2026-05-22 --- @@ -101,21 +101,22 @@ type NodeStatus = Static; ### CallResult -The result of a completed call, used by `Conditional.test` and `Map.over` to access predecessor outputs: +The result of a completed call, used by `Conditional.test` and `Map.over` to access predecessor outputs. Following the TypeBox-as-single-source-of-truth principle, `CallResult` is defined as a TypeBox schema with the corresponding type derived via `Static`: ```typescript -interface CallResult { - status: NodeStatus; // Status of the call (completed, failed, aborted, skipped) - output: unknown; // Call output (if completed) - error?: { // Call error (if failed) - code: string; - message: string; - details?: unknown; - }; -} +const CallResultSchema = Type.Object({ + status: NodeStatusEnum, // Status of the call (completed, failed, aborted, skipped) + output: Type.Unknown(), // Call output (if completed) + error: Type.Optional(Type.Object({ // Call error (if failed) + code: Type.String(), + message: Type.String(), + details: Type.Optional(Type.Unknown()), + })), +}); +type CallResult = Static; ``` -`CallResult` is the value in the `results` map passed to `Conditional.test` and `Map.over` functions. It's derived from `CallNodeAttrs` but simplified for template use — it omits `requestId`, `operationId`, `identity`, and timestamps, preserving only what template logic needs. +`CallResult` is the value in the `results` map passed to `Conditional.test` and `Map.over` functions. It's derived from `CallNodeAttrs` but simplified for template use — it omits `requestId`, `operationId`, `identity`, and timestamps, preserving only what template logic needs. The `output` field uses `Type.Unknown()` because call outputs are arbitrary data; the `error` field mirrors the `CallNodeAttrs.error` structure. ### OperationTypeEnum @@ -262,7 +263,7 @@ type OperationEdgeAttrs = Static; Type-compatibility edges carry a boolean `compatible` flag, an optional `detail` string, and optional structured `mismatches`. This allows the operation graph to include both compatible edges (green paths) and incompatible edges (red paths) for diagnostics. The `detail` field provides a human-readable summary, while `mismatches` provides machine-readable field-level diagnostics. The `TypeCompatResult` from `typeCompat()` populates both fields: `detail` for compatible edges and `mismatches` for incompatible ones. -**Edge type storage (OQ-004)**: `edgeType` is a required universal attribute stored on every edge, regardless of graph mode. This applies uniformly: operation graph edges have `edgeType: "typed"`, call graph edges have `edgeType: "triggered"` or `"depends_on"`, and template edges have `edgeType: "sequential"` or `"conditional"`. The `edgeType` field is stored alongside mode-specific attributes in graphology, not inside the mode-specific attribute schemas (`OperationEdgeAttrs`, `TriggeredEdgeAttrs`, etc.). This ensures consistent serialization/deserialization, uniform graphology queries, and straightforward edge-type filtering. See ADR-006 for the full decision record. +**Edge type storage (OQ-004)**: `edgeType` is a required universal attribute stored on every edge, regardless of graph mode. This applies uniformly: operation graph edges have `edgeType: "typed"`, call graph edges have `edgeType: "triggered"` or `"depends_on"`, and template edges have `edgeType: "sequential"` or `"conditional"`. The `edgeType` field is stored alongside mode-specific attributes in graphology, not inside the mode-specific attribute schemas (`OperationEdgeAttrs`, `TriggeredEdgeAttrs`, etc.). This ensures consistent serialization/deserialization, uniform graphology queries, and straightforward edge-type filtering. See [ADR-006](decisions/006-edge-type-consistency.md) (flowgraph) for the full decision record. ```typescript // How operation graph edges are stored in graphology: @@ -347,12 +348,6 @@ Edges where `dataFlow` cannot be determined (e.g., `Operation.input` is an opaqu Over-marking `dataFlow: true` is safe (it just causes an unnecessary type compatibility check), while under-marking is safe (it skips a check that would have passed anyway, but could let a type-incompatible connection through). The conservative strategy errs on the side of under-marking. -The `dataFlow` attribute is **inferred** by the `GraphologyHostConfig` during template rendering, not manually specified by template authors: - -- A `Sequential` edge where the downstream node references `results["upstreamNode"]` in any expression gets `dataFlow: true` -- A `Sequential` edge where no such reference exists gets `dataFlow: false` (the default) -- A `Conditional` edge always gets `dataFlow: true` (the condition always reads a predecessor's result) - This resolves OQ-01 and OQ-02: `typeCompat()` only runs on edges where `dataFlow: true`. Temporal-only edges bypass type checking entirely, since no data flows between the connected nodes. **Note**: `TemplateEdgeAttrs.edgeType` uses a constrained union of `"sequential" | "conditional"` rather than the full `EdgeTypeEnum`. Template DAGs never have `triggered`, `depends_on`, or `typed` edges — those belong to call graphs and operation graphs respectively. @@ -427,7 +422,7 @@ For call graphs, edges can be either `triggered` or `depends_on`, distinguished ## Edge Key Convention -Following taskgraph's ADR-006, edge keys are deterministic: +Following taskgraph's ADR-006 (edge key convention), edge keys are deterministic: ``` ${source}->${target} @@ -453,8 +448,8 @@ This is an exception to the simple `${source}->${target}` pattern, but it's nece ## Constraints -- **TypeBox schemas are the single source of truth** — no hand-written `interface` or `type` definitions for data shapes. All types are derived via `Static`. -- **Edge keys are deterministic** — `${source}->${target}` format, following ADR-006 in taskgraph. +- **TypeBox schemas are the single source of truth** — no hand-written `interface` or `type` definitions for data shapes that participate in graph attributes, serialization, or runtime validation. All such types are derived via `Static`. Exception: analysis result types returned by validation and compatibility functions (e.g., `ValidationError`, `GraphValidationError`, `TypeIncompatError`) are plain interfaces because they are ephemeral result objects, not serialized graph data. They don't need TypeBox schemas because they are never persisted or transmitted — they are consumed locally and discarded. +- **Edge keys are deterministic** — `${source}->${target}` format, following taskgraph's ADR-006 (edge key convention). - **No parallel edges** — `multi: false` in graphology. At most one edge per (source, target) pair. - **No self-loops** — `allowSelfLoops: false`. An operation cannot be its own prerequisite. - **ISO timestamp strings** — Call graph timestamps are ISO 8601 strings, matching the storage schema. @@ -464,7 +459,7 @@ This is an exception to the simple `${source}->${target}` pattern, but it's nece ## Open Questions -1. ~~**Should `edgeType` be a required field on ALL edges, or only on call graph and template edges?**~~ **Resolved (OQ-004)**: `edgeType` is required on all edges, stored as a universal attribute alongside mode-specific attributes. The mode-specific attribute schemas (`OperationEdgeAttrs`, `TriggeredEdgeAttrs`, `DependencyEdgeAttrs`) do NOT include `edgeType` — it's stored separately in graphology at the same level as the mode-specific attributes. This ensures consistent serialization/deserialization, uniform graphology queries, and straightforward edge-type filtering across all graph modes. See ADR-006. +1. ~~**Should `edgeType` be a required field on ALL edges, or only on call graph and template edges?**~~ **Resolved (OQ-004)**: `edgeType` is required on all edges, stored as a universal attribute alongside mode-specific attributes. The mode-specific attribute schemas (`OperationEdgeAttrs`, `TriggeredEdgeAttrs`, `DependencyEdgeAttrs`) do NOT include `edgeType` — it's stored separately in graphology at the same level as the mode-specific attributes. This ensures consistent serialization/deserialization, uniform graphology queries, and straightforward edge-type filtering across all graph modes. See [ADR-006](decisions/006-edge-type-consistency.md) (flowgraph). 2. ~~**Should `CallNodeAttrs.identity` be a `Type.Record` or the structured `Identity` type from operations?**~~ **Resolved (OQ-022)**: Import the `Identity` type structure from `@alkdev/operations` (peer dependency). Since `@alkdev/operations` is already a peer dependency (for `CallEventMapValue`), adding this type import creates minimal additional coupling. The `CallNodeAttrs.identity` field mirrors the `Identity` interface: `{ id, scopes, resources? }`. Version alignment is handled by semver ranges. The TypeBox schema for `identity` is defined inline in `CallNodeAttrs` to match the shape (not imported as a TypeBox schema, since `@alkdev/operations` defines `Identity` as a TypeScript interface), but the field semantics match exactly. diff --git a/docs/architecture/workflow-templates.md b/docs/architecture/workflow-templates.md index ac0d55f..da4c0ec 100644 --- a/docs/architecture/workflow-templates.md +++ b/docs/architecture/workflow-templates.md @@ -1,5 +1,5 @@ --- -status: draft +status: reviewed last_updated: 2026-05-22 ---