Skip to content

Commit 71d89d8

Browse files
Flow diagnostic fields proposal done (#96)
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: mahyarmlk <mahyarmlk@users.noreply.github.com>
1 parent 808b7c7 commit 71d89d8

1 file changed

Lines changed: 188 additions & 0 deletions

File tree

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
# GraphDiagnostic Flow Fields — Design Proposal
2+
3+
**Status:** Proposal
4+
**Date:** 2026-06-26
5+
**Author:** Big Pickle
6+
**Affected package:** `@intent-framework/core`
7+
**Related docs:** `docs/proposals/Reachability-Diagnostics.md`
8+
9+
---
10+
11+
## Problem
12+
13+
The `GraphDiagnostic` type (defined in `packages/core/src/graph.ts:7-13`) currently carries `nodeId` and `semanticNodeId` to identify which ask or act a diagnostic refers to. There is no way to associate a diagnostic with a specific flow.
14+
15+
Future flow-scoped diagnostics — such as `flow-step-not-surfaced` and `orphaned-flow` (designed in `Reachability-Diagnostics.md:68-106`) — need to report *which flow* is affected, not just which node. Without flow fields, a consumer of `inspectScreen()` cannot group diagnostics by flow or display flow context to the author.
16+
17+
---
18+
19+
## Goal
20+
21+
Design the smallest additive extension to `GraphDiagnostic` that enables flow-scoped diagnostics, without breaking existing consumers.
22+
23+
---
24+
25+
## Proposed API Shape
26+
27+
### One new type: `FlowDiagnosticMeta`
28+
29+
```ts
30+
export type FlowDiagnosticMeta = {
31+
flowNodeId: string
32+
flowSemanticNodeId?: string
33+
}
34+
```
35+
36+
### Make `GraphDiagnostic` carry optional flow meta
37+
38+
```ts
39+
export type GraphDiagnostic = {
40+
severity: DiagnosticSeverity
41+
code: string
42+
message: string
43+
nodeId?: string
44+
semanticNodeId?: string
45+
flow?: FlowDiagnosticMeta
46+
}
47+
```
48+
49+
### Rationale for a nested `flow` object (vs top-level fields)
50+
51+
- **Namespacing**: `flowNodeId` and `flowSemanticNodeId` are only meaningful together. A flat `flowNodeId` without `flowSemanticNodeId` is ambiguous; grouping them into an optional object makes the relationship explicit.
52+
- **Forward compatibility**: If later we add `flowStepIndex` or `flowStepNodeId`, they naturally live inside `flow` without polluting the top-level type.
53+
- **Backward compatibility**: The `flow` field is optional (`flow?: FlowDiagnosticMeta`). Existing consumers that destructure only `severity`, `code`, `message`, `nodeId`, `semanticNodeId` continue to work unchanged.
54+
- **Type narrowing**: A consumer checking `if (d.flow)` immediately has access to the complete flow context, rather than checking two separate optional fields.
55+
56+
### Example diagnostic using the new shape
57+
58+
```ts
59+
{
60+
severity: "warning",
61+
code: "flow-step-not-surfaced",
62+
message: 'Flow "signup" step "Secret" is not included in any surface.',
63+
nodeId: "ask_secret",
64+
semanticNodeId: "ask:secret",
65+
flow: {
66+
flowNodeId: "flow_signup",
67+
flowSemanticNodeId: "flow:signup"
68+
}
69+
}
70+
```
71+
72+
---
73+
74+
## Alternative: Flat top-level fields
75+
76+
The `Reachability-Diagnostics.md:186-203` proposal originally suggested:
77+
78+
```ts
79+
export type GraphDiagnostic = {
80+
severity: DiagnosticSeverity
81+
code: string
82+
message: string
83+
nodeId?: string
84+
semanticNodeId?: string
85+
flowNodeId?: string
86+
flowSemanticNodeId?: string // new — optional
87+
}
88+
```
89+
90+
**Rejected** because:
91+
92+
1. Two new optional top-level fields with no structural relationship to each other. A consumer must remember to check both and understand they covary.
93+
2. Adding more top-level optional fields increases cognitive overhead for consumers that ignore flow diagnostics entirely.
94+
95+
---
96+
97+
## Alternative: Overload `nodeId` to reference the flow
98+
99+
```ts
100+
// For flow-scoped diagnostics:
101+
{
102+
nodeId: "flow_signup", // nodeId now refers to the flow
103+
semanticNodeId: "flow:signup",
104+
stepNodeId: "ask_secret" // the affected step
105+
}
106+
```
107+
108+
**Rejected** because:
109+
110+
1. Polymorphic `nodeId` breaks existing consumers that assume it always refers to an ask or act node.
111+
2. Introduces a new `stepNodeId` field anyway, so no net field reduction.
112+
3. Violates the principle of least surprise.
113+
114+
---
115+
116+
## Impact on Existing Code
117+
118+
### Changes to `packages/core/src/graph.ts`
119+
120+
- Add `FlowDiagnosticMeta` type (2 lines).
121+
- Change `GraphDiagnostic` to include `flow?: FlowDiagnosticMeta` (add 1 line).
122+
123+
### No changes needed to
124+
125+
- `computeDiagnostics()` — existing diagnostics do not set `flow`, so no existing logic needs updating.
126+
- `inspectScreen()` — the augmentation step (`diagnostics.map(...)`) already spreads each diagnostic; the `flow` field passes through transparently.
127+
- `InspectedScreen` — no change needed; its `diagnostics: GraphDiagnostic[]` field is generic.
128+
- Any existing consumer that reads `audited.code` — new field is optional and ignored by destructuring.
129+
130+
### Public API surface impact
131+
132+
`FlowDiagnosticMeta` and the amended `GraphDiagnostic` are exported from `packages/core/src/index.ts`. This is additive only — no existing exports are removed or renamed.
133+
134+
---
135+
136+
## Interaction with Future Diagnostics
137+
138+
### `flow-step-not-surfaced` (planned)
139+
140+
```ts
141+
{
142+
severity: "warning",
143+
code: "flow-step-not-surfaced",
144+
message: 'Flow "onboarding" step "VerifyEmail" is not in any surface.',
145+
nodeId: "act_verifyEmail",
146+
semanticNodeId: "act:verify-email",
147+
flow: {
148+
flowNodeId: "flow_onboarding",
149+
flowSemanticNodeId: "flow:onboarding"
150+
}
151+
}
152+
```
153+
154+
### `orphaned-flow` (planned)
155+
156+
```ts
157+
{
158+
severity: "warning",
159+
code: "orphaned-flow",
160+
message: 'Flow "legacy" has no surfaced steps.',
161+
flow: {
162+
flowNodeId: "flow_legacy",
163+
flowSemanticNodeId: "flow:legacy"
164+
}
165+
// No nodeId — the diagnostic is about the flow itself, not a specific step node
166+
}
167+
```
168+
169+
---
170+
171+
## Migration
172+
173+
No migration needed. The new field is optional and additive. Consumers that ignore `flow` continue to see all existing fields unchanged.
174+
175+
---
176+
177+
## Open Questions
178+
179+
1. **Should `FlowDiagnosticMeta` be its own exported type or inline in `GraphDiagnostic`?** This proposal exports it separately for documentation value and reuse potential. Inline is simpler but loses the named type in API docs.
180+
2. **Should we add `stepIndex` to `FlowDiagnosticMeta` now?** Could be useful for `flow-step-order-gap`, but this proposal prefers to add fields only when a diagnostic needs them (YAGNI).
181+
182+
---
183+
184+
## Related
185+
186+
- `packages/core/src/graph.ts:7-13` — current `GraphDiagnostic` type
187+
- `docs/proposals/Reachability-Diagnostics.md` — reachability diagnostics design (defines the diagnostics that will use these fields)
188+
- `packages/core/src/index.ts:4` — public export of `GraphDiagnostic`

0 commit comments

Comments
 (0)