diff --git a/apps/playground/src/server/generation/service.server.ts b/apps/playground/src/server/generation/service.server.ts index af1b638e..5f6b3a82 100644 --- a/apps/playground/src/server/generation/service.server.ts +++ b/apps/playground/src/server/generation/service.server.ts @@ -1,10 +1,6 @@ import "@tanstack/react-start/server-only"; -import { - SKETCHI_DIAGRAM_STYLE, - type FlowchartDiagram, - type MindmapDiagram, -} from "@sketchi/diagram-core"; +import type { FlowchartDiagram, MindmapDiagram, SequenceDiagram } from "@sketchi/diagram-core"; import { CloudflareAiGatewayBinding, CloudflareGoogleAiStudioClientLive, @@ -16,7 +12,6 @@ import { type DiagramGenerationCandidate, type DiagramGenerationError, type DiagramGenerationType, - type GeneratedSequenceDiagram, DiagramGenerationPolicyLive, } from "@sketchi/diagram-generation"; import { Context, Effect, Layer } from "effect"; @@ -106,16 +101,27 @@ export function mindmapDocumentInput(diagram: MindmapDiagram): unknown { }; } -/** Convert a validated sequence candidate into the native Code Mode contract. */ -export function sequenceDocumentInput(diagram: GeneratedSequenceDiagram): unknown { +/** Convert a validated sequence candidate into a canonical document input. */ +export function sequenceDocumentInput(diagram: SequenceDiagram): unknown { return { type: "sequence", spec: { id: diagram.id, title: diagram.title, - participants: diagram.participants, - messages: diagram.messages, - style: diagram.style ?? SKETCHI_DIAGRAM_STYLE, + participants: diagram.participants.map((participant) => ({ + id: participant.id, + label: participant.label, + ...(participant.kind ? { kind: participant.kind } : {}), + })), + messages: diagram.messages.map((message) => ({ + id: message.id, + source: message.source, + target: message.target, + label: message.label, + ...(message.type ? { type: message.type } : {}), + ...(message.style ? { style: message.style } : {}), + })), + style: diagram.style, }, }; } diff --git a/packages/diagram/agent/src/lib/code-mode/contract.ts b/packages/diagram/agent/src/lib/code-mode/contract.ts index 67f6ccd4..a5bdbc17 100644 --- a/packages/diagram/agent/src/lib/code-mode/contract.ts +++ b/packages/diagram/agent/src/lib/code-mode/contract.ts @@ -4,6 +4,8 @@ import { CANVAS_LIMITS, CANVAS_NODE_ICON, CANVAS_SPEC_VERSION, + SEQUENCE_MESSAGE_STYLES, + SEQUENCE_MESSAGE_TYPES, SKETCHI_DIAGRAM_STYLE, } from "@sketchi/diagram-core"; @@ -598,6 +600,11 @@ export const FlowchartSpecEdgeSchema = FlowchartSpecEdge; export const FlowchartSpecLayoutSchema = FlowchartSpecLayout; export const FlowchartSpecStyleSchema = FlowchartSpecStyle; +/** + * The authoring form of diagram-core's canonical SequenceDiagram: message ids + * are optional and style is defaulted. buildSequenceDiagram normalizes it into + * the core contract, which owns every sequence invariant. + */ export class SequenceParticipantSpec extends Schema.Class( "SequenceParticipantSpec", )( @@ -615,8 +622,8 @@ export class SequenceMessageSpec extends Schema.Class("Sequ source: RequiredNonEmptyString, target: RequiredNonEmptyString, label: RequiredNonEmptyString, - type: optionalContract(literals(["message", "return"])), - style: optionalContract(literals(["solid", "dashed"])), + type: optionalContract(literals(SEQUENCE_MESSAGE_TYPES)), + style: optionalContract(literals(SEQUENCE_MESSAGE_STYLES)), }, { identifier: undefined }, ) {} diff --git a/packages/diagram/agent/src/lib/code-mode/runtime.test.ts b/packages/diagram/agent/src/lib/code-mode/runtime.test.ts index 4c6e5c36..4a5bf325 100644 --- a/packages/diagram/agent/src/lib/code-mode/runtime.test.ts +++ b/packages/diagram/agent/src/lib/code-mode/runtime.test.ts @@ -1991,7 +1991,10 @@ describe("Code Mode runtime", () => { issues: expect.arrayContaining([ expect.objectContaining({ code: "missing_edge_source", - hint: expect.stringContaining("participant"), + ref: { kind: "request", path: "spec.messages.[0].source" }, + message: 'Message source "missing" is not a participant.', + // Published Code Mode wording names the request field. + hint: "Use the id of a participant declared in spec.participants.", }), expect.objectContaining({ code: "self_loop", diff --git a/packages/diagram/agent/src/lib/code-mode/runtime.ts b/packages/diagram/agent/src/lib/code-mode/runtime.ts index 39915b38..95690788 100644 --- a/packages/diagram/agent/src/lib/code-mode/runtime.ts +++ b/packages/diagram/agent/src/lib/code-mode/runtime.ts @@ -6,14 +6,15 @@ import { compileCanvasSpec, getCanvasValidationIssues, getFlowchartValidationIssues, + getSequenceValidationIssues, parseMindmapDiagram, - parseSequenceDiagram, - sequenceLifelineId, validateFlowchartDiagram, type FlowchartDiagram, type FlowchartValidationIssueRef, type MindmapDiagram, type CanvasValidationIssue, + type SequenceDiagram, + type SequenceValidationIssueCode, } from "@sketchi/diagram-core"; import { convertSceneToExcalidraw, @@ -442,96 +443,46 @@ function normalizeSequenceDiagramSpec( }; } -function validateNormalizedSequenceDiagram(spec: NormalizedSequenceDiagramSpec): CodeModeIssue[] { - const issues: CodeModeIssue[] = []; - const participantIds = new Set(); - spec.participants.forEach((participant, index) => { - if (participantIds.has(participant.id)) { - issues.push( - issue({ - code: "duplicate_node_id", - stage: "input", - ref: { kind: "request", path: `spec.participants.[${index}].id` }, - message: `Participant id "${participant.id}" is duplicated.`, - hint: "Give every participant a unique stable id and update message references.", - }), - ); - } - participantIds.add(participant.id); - }); +function toSequenceDiagram(spec: NormalizedSequenceDiagramSpec): SequenceDiagram { + return { + id: spec.id, + title: spec.title, + type: "sequence", + participants: spec.participants, + messages: spec.messages, + style: spec.style, + }; +} - const participantIndexById = new Map( - spec.participants.map((participant, index) => [participant.id, index]), - ); - spec.participants.forEach((participant) => { - const generatedLifelineId = sequenceLifelineId(participant.id); - const collisionIndex = participantIndexById.get(generatedLifelineId); - if (collisionIndex === undefined) { - return; - } - issues.push( - issue({ - code: "duplicate_node_id", - stage: "input", - ref: { - kind: "request", - path: `spec.participants.[${collisionIndex}].id`, - }, - message: `Participant id "${generatedLifelineId}" collides with the generated lifeline for "${participant.id}".`, - hint: "Rename the participant so its id does not equal another participant id followed by :lifeline.", - }), - ); - }); +/** Code Mode's published issue codes for diagram-core's sequence invariants. */ +const SEQUENCE_ISSUE_CODES = { + duplicate_participant_id: "duplicate_node_id", + lifeline_id_collision: "duplicate_node_id", + duplicate_message_id: "duplicate_edge_id", + missing_message_source: "missing_edge_source", + missing_message_target: "missing_edge_target", + self_message: "self_loop", +} as const satisfies Record; + +/** + * Code Mode requests nest the diagram under `spec`, so hints that name a field + * keep the published request path rather than diagram-core's. + */ +const SEQUENCE_ISSUE_HINTS: Partial> = { + missing_message_source: "Use the id of a participant declared in spec.participants.", + missing_message_target: "Use the id of a participant declared in spec.participants.", +}; - const messageIds = new Set(); - spec.messages.forEach((message, index) => { - if (messageIds.has(message.id)) { - issues.push( - issue({ - code: "duplicate_edge_id", - stage: "input", - ref: { kind: "request", path: `spec.messages.[${index}].id` }, - message: `Message id "${message.id}" is duplicated.`, - hint: "Give every message a unique id or omit message ids to generate them deterministically.", - }), - ); - } - messageIds.add(message.id); - if (!participantIds.has(message.source)) { - issues.push( - issue({ - code: "missing_edge_source", - stage: "input", - ref: { kind: "request", path: `spec.messages.[${index}].source` }, - message: `Message source "${message.source}" is not a participant.`, - hint: "Use the id of a participant declared in spec.participants.", - }), - ); - } - if (!participantIds.has(message.target)) { - issues.push( - issue({ - code: "missing_edge_target", - stage: "input", - ref: { kind: "request", path: `spec.messages.[${index}].target` }, - message: `Message target "${message.target}" is not a participant.`, - hint: "Use the id of a participant declared in spec.participants.", - }), - ); - } - if (message.source === message.target) { - issues.push( - issue({ - code: "self_loop", - stage: "input", - ref: { kind: "request", path: `spec.messages.[${index}]` }, - message: `Message "${message.id}" is self-referential.`, - hint: "Choose a different target participant; self messages are not supported.", - }), - ); - } - }); - return issues; +function validateNormalizedSequenceDiagram(spec: NormalizedSequenceDiagramSpec): CodeModeIssue[] { + return getSequenceValidationIssues(toSequenceDiagram(spec)).map((entry) => + issue({ + code: SEQUENCE_ISSUE_CODES[entry.code], + stage: "input", + ref: { kind: "request", path: `spec.${entry.path}` }, + message: entry.message, + hint: SEQUENCE_ISSUE_HINTS[entry.code] ?? entry.hint, + }), + ); } function sequenceQuality(spec: NormalizedSequenceDiagramSpec, threshold: number): QualityReport { @@ -2273,7 +2224,7 @@ const buildSequenceDiagramWorkflow = Effect.fn("codeMode.buildSequenceDiagram.wo } const scene = yield* Effect.try({ - try: () => renderSequenceDiagram(parseSequenceDiagram({ ...normalizedSpec, type: "sequence" })), + try: () => renderSequenceDiagram(toSequenceDiagram(normalizedSpec)), catch: (cause) => new BuildSequenceDiagramFailure({ status: "render_failed", diff --git a/packages/diagram/generation/src/lib/candidates.ts b/packages/diagram/generation/src/lib/candidates.ts index c20e47dc..e3dcea3b 100644 --- a/packages/diagram/generation/src/lib/candidates.ts +++ b/packages/diagram/generation/src/lib/candidates.ts @@ -1,14 +1,18 @@ import { + type CanonicalDiagram, + CanonicalDiagramSchema, DiagramValidationError, isDiagramIconSlug, type FlowchartDiagram, FlowchartDiagramSchema, FlowchartValidationError, - type MindmapDiagram, MindmapDiagramSchema, SKETCHI_DIAGRAM_STYLE, + SequenceDiagramSchema, + SequenceValidationError, validateFlowchartDiagram, validateMindmapDiagram, + validateSequenceDiagram, } from "@sketchi/diagram-core"; import { Result, Schema, SchemaIssue } from "effect"; @@ -35,44 +39,6 @@ export type DiagramGenerationProviderId = typeof DiagramGenerationProviderIdSche export const DiagramGenerationCacheModeSchema = Schema.Literals(["default", "fresh"]); export type DiagramGenerationCacheMode = typeof DiagramGenerationCacheModeSchema.Type; -export class GeneratedSequenceParticipant extends Schema.Class( - "GeneratedSequenceParticipant", -)({ - id: Schema.NonEmptyString, - label: Schema.NonEmptyString, - kind: Schema.optionalKey(Schema.NonEmptyString), -}) {} - -export class GeneratedSequenceMessage extends Schema.Class( - "GeneratedSequenceMessage", -)({ - id: Schema.NonEmptyString, - source: Schema.NonEmptyString, - target: Schema.NonEmptyString, - label: Schema.NonEmptyString, - type: Schema.optionalKey(Schema.Literals(["message", "return"])), - style: Schema.optionalKey(Schema.Literals(["solid", "dashed"])), -}) {} - -export class GeneratedSequenceDiagram extends Schema.Class( - "GeneratedSequenceDiagram", -)({ - id: Schema.NonEmptyString, - title: Schema.NonEmptyString, - type: Schema.Literal("sequence"), - participants: Schema.Array(GeneratedSequenceParticipant).pipe( - Schema.mutable, - Schema.check(Schema.isMinLength(1)), - ), - messages: Schema.Array(GeneratedSequenceMessage).pipe(Schema.mutable), - style: Schema.optionalKey( - Schema.Struct({ - accentColor: Schema.String, - backgroundColor: Schema.String, - }), - ), -}) {} - export class DiagramGenerationUsage extends Schema.Class( "DiagramGenerationUsage", )({ @@ -86,9 +52,7 @@ export class DiagramGenerationCandidate extends Schema.Class(); - for (const participant of decoded.participants) { - if (participantIds.has(participant.id)) { - throw new DiagramValidationError( - `Duplicate sequence participant id "${participant.id}" is not allowed.`, - ); - } - participantIds.add(participant.id); - } - const messageIds = new Set(); - for (const message of decoded.messages) { - if (messageIds.has(message.id)) { - throw new DiagramValidationError( - `Duplicate sequence message id "${message.id}" is not allowed.`, - ); - } - messageIds.add(message.id); - if (!participantIds.has(message.source) || !participantIds.has(message.target)) { - throw new DiagramValidationError( - `Sequence message "${message.id}" references an unknown participant.`, - ); - } - if (message.source === message.target) { - throw new DiagramValidationError( - `Sequence message "${message.id}" cannot target its source participant.`, - ); - } - } - return decoded; -} - const schemaIssueFormatter = SchemaIssue.makeFormatterStandardSchemaV1(); function schemaIssueDiagnostic(issue: { @@ -298,10 +230,11 @@ function schemaIssueDiagnostic(issue: { } function diagramValidationFailure(error: unknown): CandidateParseFailure { - if (error instanceof FlowchartValidationError) { + if (error instanceof FlowchartValidationError || error instanceof SequenceValidationError) { + const family = error instanceof FlowchartValidationError ? "flowchart" : "sequence"; return { diagnostics: error.issues.map( - (entry) => `flowchart.${entry.code}: ${entry.message} Hint: ${entry.hint}`, + (entry) => `${family}.${entry.code}: ${entry.message} Hint: ${entry.hint}`, ), error: error.message, success: false, @@ -416,11 +349,10 @@ function parseCandidateDiagram(text: string): CandidateParseResult { ), sequence: () => decodeAndValidate( - GeneratedSequenceDiagram, - titledDiagram, - parseGeneratedSequence, + SequenceDiagramSchema, + withSketchiDiagramStyle(titledDiagram), + validateSequenceDiagram, intent, - "Generated sequence diagram schema validation failed.", ), }; return parsers[intent.nativeKind](); diff --git a/packages/diagram/generation/src/lib/public-api.test.ts b/packages/diagram/generation/src/lib/public-api.test.ts index 26ee939d..1c01a7d6 100644 --- a/packages/diagram/generation/src/lib/public-api.test.ts +++ b/packages/diagram/generation/src/lib/public-api.test.ts @@ -6,6 +6,7 @@ import { type TelemetryMetricEvent, type TelemetrySpanEvent, } from "@sketchi/observability"; +import { SKETCHI_DIAGRAM_STYLE } from "@sketchi/diagram-core"; import { Cause, Effect, Exit, Fiber, Layer, Schema } from "effect"; import { TestClock } from "effect/testing"; @@ -345,9 +346,38 @@ describe("pure candidate behavior", () => { expect(candidate.diagram?.type).toBe("sequence"); if (candidate.diagram?.type === "sequence") { expect(candidate.diagram.messages).toHaveLength(1); + expect(candidate.diagram.style).toEqual(SKETCHI_DIAGRAM_STYLE); } }); + it("reports sequence invariants with the core contract's issue codes and hints", () => { + const candidate = candidateFromText({ + model: "fixture", + provider: "fixture", + text: generationResponseText({ + id: "retry-sequence", + title: "Retry sequence", + type: "sequence", + participants: [ + { id: "worker", label: "Worker" }, + { id: "queue", label: "Queue" }, + ], + messages: [ + { id: "poll", source: "worker", target: "queue", label: "Poll" }, + { id: "retry", source: "worker", target: "worker", label: "Retry" }, + { id: "ack", source: "queue", target: "billing", label: "Ack" }, + ], + style: { accentColor: "#0f766e", backgroundColor: "#ffffff" }, + }), + }); + + expect(candidate.diagram).toBeUndefined(); + expect(candidate.diagnostics).toEqual([ + 'sequence.self_message: Message "retry" is self-referential. Hint: Choose a different target participant; self messages are not supported.', + 'sequence.missing_message_target: Message target "billing" is not a participant. Hint: Use the id of a participant declared in participants.', + ]); + }); + it("requires the typed response envelope instead of accepting legacy raw IR", () => { const candidate = candidateFromText({ model: "fixture",