resolve architecture review round 2: criticals, warnings, suggestions
- C-05: Add flowgraph-api.md with complete public API surface - C-06: Document <Map> component in workflow-templates.md - C-07: Specify Conditional else-branch behavior - C-08: Add lifecycle/ownership section to reactive-execution.md - C-09: Add consumer-integration.md end-to-end walkthrough - W-02: Add reactive error boundary semantics (3 levels) - W-03: Complete ReactiveContext interface definition - W-04: Add template composition rules (8 rules) - W-05: Document removeChild for both HostConfigs - W-06: Document signal/effect disposal lifecycle - W-07: Add ADR-004 (no schema version field) - W-08: Add type compatibility depth/contract to analysis.md - W-11: Add performance characteristics section - S-01: Getting Started merged into consumer-integration.md - S-02: Add flow diagrams for template rendering pipeline - S-03: Add node status state machine diagram - S-04: Add testing strategy section - S-06: Validate source structure cross-references Review round 2 fixes: - Define TemplateNodeAttrs as alias for OperationNodeAttrs - Document CallEventMapValue and CallResult types in schema.md - Standardize CycleError naming (replace CircularDependencyError) - Add function form to Map.over type definition - Define Map aggregate completion/failure semantics - Fix immutability claim for fromCallEvents - Clarify edgeType storage alongside OperationEdgeAttrs - Clarify WorkflowNode.status === statusMap (same Signal) - Add component-to-tag mapping for WorkflowTag
This commit is contained in:
64
docs/architecture/decisions/004-no-schema-version.md
Normal file
64
docs/architecture/decisions/004-no-schema-version.md
Normal file
@@ -0,0 +1,64 @@
|
||||
# ADR-004: No Schema Version Field in Serialized Format
|
||||
|
||||
## Status
|
||||
|
||||
Accepted
|
||||
|
||||
## Context
|
||||
|
||||
Flowgraph's `FlowGraphSerialized` type follows graphology's native JSON format. The format does not include a `schemaVersion` field. This decision affects:
|
||||
|
||||
1. **Backward compatibility** — how consumers handle format changes across versions
|
||||
2. **Persistence** — whether stored graphs can be reliably migrated
|
||||
3. **Interoperability** — whether different versions of flowgraph can exchange data
|
||||
|
||||
The review (001-architecture-gap-analysis.md, W-07) flagged this as a potential gap: "Consumers that need persistence wrap it in their own versioned envelope."
|
||||
|
||||
## Decision
|
||||
|
||||
Flowgraph's serialized format will NOT include a `schemaVersion` field. This follows the same pattern as taskgraph. Consumers that need versioned persistence must wrap the serialized format in their own envelope that includes version metadata.
|
||||
|
||||
## Rationale
|
||||
|
||||
1. **Graphology format is upstream** — the serialized format is graphology's native JSON format. Adding a `schemaVersion` field modifies a format we don't own. This creates a fork between what graphology produces and what flowgraph expects.
|
||||
|
||||
2. **Flowgraph is not a persistence layer** — the library handles in-memory graph construction, validation, and analysis. Persistence is explicitly the consumer's responsibility (see ADR-003). Adding versioning to the serialized format would blur this boundary.
|
||||
|
||||
3. **Versioning belongs at the envelope level** — a consumer storing graphs in Postgres should wrap the serialized data in a versioned envelope: `{ version: 1, data: FlowGraphSerialized, metadata: {...} }`. This gives the consumer full control over migration logic, which they need anyway for their own schema migrations.
|
||||
|
||||
4. **Breaking changes are breaking changes** — if the serialized format changes incompatibly, no version field will help. The consumer must handle the migration. A version field would give false confidence that "old versions can be read" when in practice the consumer must write migration code.
|
||||
|
||||
5. **Taskgraph precedent** — taskgraph uses the same approach and it has worked well. The pattern is established in the ecosystem.
|
||||
|
||||
## Consequences
|
||||
|
||||
- **Positive**: Simpler serialized format, no dependency on version parsing, no false confidence in backward compatibility.
|
||||
- **Positive**: Clean separation of concerns — flowgraph handles graph operations, consumers handle persistence and versioning.
|
||||
- **Negative**: Consumers must implement their own versioned envelope if they persist graphs. This is a small burden documented in the consumer integration guide.
|
||||
- **Negative**: There's no standard way for two different flowgraph versions to detect incompatibility. The consumer must track this themselves.
|
||||
|
||||
## Mitigation
|
||||
|
||||
The consumer integration guide documents the recommended pattern:
|
||||
|
||||
```typescript
|
||||
// Consumer-side versioned envelope
|
||||
interface PersistedGraph {
|
||||
version: number; // Increment on breaking changes
|
||||
data: FlowGraphSerialized; // Raw graphology format
|
||||
metadata: {
|
||||
createdAt: string;
|
||||
graphType: "operation" | "call";
|
||||
flowgraphVersion: string; // The npm package version for reference
|
||||
};
|
||||
}
|
||||
```
|
||||
|
||||
When restoring a graph, the consumer checks `version` and `flowgraphVersion` to decide whether migration is needed. This is outside flowgraph's responsibility.
|
||||
|
||||
## References
|
||||
|
||||
- Schema: [schema.md](../schema.md) — `FlowGraphSerialized`, `SerializedGraph` factory
|
||||
- Storage decoupled ADR: [003-storage-decoupled.md](003-storage-decoupled.md)
|
||||
- Consumer integration: [consumer-integration.md](../consumer-integration.md)
|
||||
- Review: [001-architecture-gap-analysis.md](../../reviews/001-architecture-gap-analysis.md) — W-07
|
||||
Reference in New Issue
Block a user