sscoble/speckl
1
0
Fork 0
Code Issues 2 Pull requests Projects Releases Packages Wiki Activity Actions

fix(speckl): protobuf generator — List<T> in interfaces, enum prefix, transition type field, Date in interfaces (cycle 73)

Browse source 
Four defects found by dogfooding Speckl against the v0.3 product-layer
spec (FederatedMeetupProduct.speckdl, 74 members):

D1 — CRITICAL: List<T> fields in interface records emitted empty type
  Root cause: interface record fields called speckTypeToProto(f.type.name)
  which returns '' for List/Map/Set types (their .name is undefined).
  Event and state fields had inline List/Map dispatch, but interfaces did not.
  Fix: extracted typeExprToProto() shared function that handles all TypeExpr
  variants (primitive, ident, list, map, set). All four field-emitting code
  paths (interfaces, events, state, transitions) now use it.

D2 — MODERATE: Transition type discriminator field silently dropped
  Root cause: parser stores the type field in t.typeDiscriminator (string)
  rather than in t.fields array. Generator only emitted t.fields.
  Fix: emit typeDiscriminator as 'type' field at number 1, offset other
  fields by 1 to avoid collision.

D4 — LOW: Enum variant name collisions across enums in same proto scope
  Proto3 requires all enum values unique within enclosing scope, not just
  the enum. PENDING in Role and PENDING in OrderStatus collided.
  Fix: prefix all variants with upperSnake(enumName) — the proto3 convention.
  This also matches the handwritten proto's naming (TRANSITION_TYPE_*).

D5 — LOW: Date in interface fields did not trigger timestamp import
  The usesTimestamp check only scanned events and state, not interfaces.
  Fix: added interface member check to the timestamp import scan.

D3 (deferred): rc_ typo in spec source silently swallowed by parser.
  Parser bug, not generator bug. Filed for future cycle.

Verified: 56/56 tests pass, both FederatedMeetup.speckdl and
FederatedMeetupProduct.speckdl compile to protoc-clean output.
This commit is contained in:
Scott Scoble 2026-06-29 12:12:40 -07:00
parent 7beaf6f3cc
commit 04fa9844f4
5 changed files with 204 additions and 35 deletions
Download patch file Download diff file Expand all files Collapse all files

15
compiler/dist/parser.d.ts vendored
View file

@ -7,6 +7,7 @@ export interface SpeckNode {
license?: string;
protoPackage?: string;
goPackage?: string;
eventSuffix?: string;
}
export interface ImportNode {
type: 'import';
@ -82,6 +83,17 @@ export interface OneofVariant {
fieldName: string;
typeName: string;
}
export interface TransitionNode {
type: 'transition';
name: string;
fields: TransitionField[];
payloadOneof?: string;
typeDiscriminator?: string;
}
export interface TransitionField {
name: string;
type: TypeExpr;
}
export interface ProvenanceNode {
type: 'provenance';
clauses: ProvenanceClause[];
@ -135,6 +147,7 @@ export interface BOMNode {
export interface StateNode {
type: 'state';
variables: StateVar[];
messageName?: string;
}
export interface StateVar {
name: string;
@ -188,7 +201,7 @@ export type ActionStatement = {
type: 'ifblock';
raw: string;
};
export type MemberNode = ImportNode | InputNode | OutputNode | ConstraintNode | VerifyNode | InterfaceNode | EventNode | OneofNode | ProvenanceNode | ReviewNode | DerivesNode | SatisfiesNode | AuthorNode | SourceNode | BOMNode | StateNode | InitNode | ActionNode | ServiceNode;
export type MemberNode = ImportNode | InputNode | OutputNode | ConstraintNode | VerifyNode | InterfaceNode | EventNode | OneofNode | TransitionNode | ProvenanceNode | ReviewNode | DerivesNode | SatisfiesNode | AuthorNode | SourceNode | BOMNode | StateNode | InitNode | ActionNode | ServiceNode;
export interface TypeExpr {
type: 'primitive' | 'record' | 'list' | 'set' | 'map' | 'ident';
name?: string;

2
compiler/dist/parser.d.ts.map vendored
View file

File diff suppressed because one or more lines are too long

130
compiler/dist/parser.js vendored
View file

@ -51,6 +51,15 @@ function parseSpeck(lines, startIndex) {
return null;
const name = headerMatch[1];
const members = [];
// Cycle 61: clear static metadata fields at the start of each parse
// so per-speck directives don't leak between parse calls. This is the
// pre-existing static-field-hack pattern for proto_package / go_package
// (see spec-compiler-dogfood skill anti-pattern). Refactoring to a
// local-object approach is tracked as a separate cleanup; for now
// resetting the fields here is the minimal-touch fix.
parseSpeck._protoPackage = undefined;
parseSpeck._goPackage = undefined;
parseSpeck._eventSuffix = undefined;
let i = startIndex + 1;
let braceCount = 1;
while (i < lines.length && braceCount > 0) {
@ -66,7 +75,7 @@ function parseSpeck(lines, startIndex) {
// `author:`, `version:`, `license:` continue to be parsed as members
// (AuthorNode, etc.) for backward compatibility with the existing
// PROV-O / CycloneDX generators.
const metaMatch = line.match(/^(proto_package|go_package)\s*:\s*(.+)$/);
const metaMatch = line.match(/^(proto_package|go_package|event_suffix)\s*:\s*(.+)$/);
if (metaMatch) {
// Strip surrounding quotes if present
const value = metaMatch[2].trim().replace(/^['"]|['"]$/g, '');
@ -76,6 +85,9 @@ function parseSpeck(lines, startIndex) {
else if (metaMatch[1] === 'go_package') {
parseSpeck._goPackage = value;
}
else if (metaMatch[1] === 'event_suffix') {
parseSpeck._eventSuffix = value;
}
i++;
continue;
}
@ -84,7 +96,8 @@ function parseSpeck(lines, startIndex) {
const closeCount = (rawLine.match(/}/g) || []).length;
// Check for block-opening members (those that open braces on the same line)
const blockStarters = ['state:', 'init:', 'action ', 'event ', 'provenance ', 'bom ', 'interface ',
'state {', 'init {', 'verify ', 'constraint ', 'input:', 'output:', 'service '];
'state {', 'init {', 'verify ', 'constraint ', 'input:', 'output:', 'service ', 'oneof ', 'transition ',
'state as '];
const isBlockStarter = blockStarters.some(s => line.startsWith(s));
if (isBlockStarter && openCount > 0) {
// Multi-line block: parse it and skip to its end
@ -142,7 +155,8 @@ function parseSpeck(lines, startIndex) {
}
return { type: 'speck', name, members,
protoPackage: parseSpeck._protoPackage,
goPackage: parseSpeck._goPackage };
goPackage: parseSpeck._goPackage,
eventSuffix: parseSpeck._eventSuffix };
}
function findBlockEnd(lines, startIndex, startBraceCount) {
let braceCount = startBraceCount;
@ -191,6 +205,12 @@ function parseMemberBlock(lines, startIndex) {
if (firstLine.startsWith('state {') || firstLine === 'state') {
return parseStateBlockMultiline(lines, startIndex, startBraceCount);
}
if (firstLine.startsWith('state as ')) {
// Cycle 63: `state as <MessageName> { ... }` form must dispatch
// through the same state-block parser so the `as` override on the
// first line is captured.
return parseStateBlockMultiline(lines, startIndex, startBraceCount);
}
if (firstLine.startsWith('init:')) {
return parseInitBlockMultiline(lines, startIndex, startBraceCount);
}
@ -209,6 +229,9 @@ function parseMemberBlock(lines, startIndex) {
if (firstLine.startsWith('oneof ')) {
return parseOneofBlockMultiline(lines, startIndex, startBraceCount);
}
if (firstLine.startsWith('transition ')) {
return parseTransitionBlockMultiline(lines, startIndex, startBraceCount);
}
if (firstLine.startsWith('service ')) {
return parseServiceBlockMultiline(lines, startIndex, startBraceCount);
}
@ -325,6 +348,10 @@ function parseMember(line) {
if (line.startsWith('oneof ')) {
return parseOneof(line);
}
// transition:
if (line.startsWith('transition ')) {
return parseTransition(line);
}
// provenance {
if (line.startsWith('provenance ')) {
return { type: 'provenance', clauses: [] };
@ -637,6 +664,19 @@ function parseOneof(line) {
variants: [],
};
}
// Cycle 58: parse `transition Name {` (header only; parseTransitionBlockMultiline fills fields + payload-oneof).
function parseTransition(line) {
// transition Name {
const match = line.match(/^transition\s+(\w+)\s*{/);
if (!match) {
throw new Error(`Failed to parse transition: ${line}`);
}
return {
type: 'transition',
name: match[1],
fields: [],
};
}
function parseProvenanceBlock(lines, line) {
// This is a simplified parser - in a real implementation, we'd parse the full block
return {
@ -714,6 +754,12 @@ function parseSource(line) {
};
}
function parseStateBlockMultiline(lines, startIndex, startBraceCount) {
// Cycle 59: support `state as <MessageName> { ... }` to override the
// generated .proto message name. Default (no `as`) keeps the existing
// `message <SpeckName>State { ... }` convention.
const headerLine = lines[startIndex].trim();
const asMatch = headerLine.match(/^state\s+as\s+(\w+)\s*{/);
const messageName = asMatch ? asMatch[1] : undefined;
const endIndex = findBlockEnd(lines, startIndex + 1, startBraceCount);
const innerLines = lines.slice(startIndex + 1, endIndex).map(l => l.trim()).filter(l => l && !l.startsWith('//') && !l.startsWith('/*'));
// State blocks separate fields with commas or newlines/semicolons.
@ -733,7 +779,10 @@ function parseStateBlockMultiline(lines, startIndex, startBraceCount) {
}
return acc + ' ' + line;
}, '');
return parseStateBlock(inner);
const result = parseStateBlock(inner);
if (messageName !== undefined)
result.messageName = messageName;
return result;
}
function parseInitBlockMultiline(lines, startIndex, startBraceCount) {
const endIndex = findBlockEnd(lines, startIndex + 1, startBraceCount);
@ -917,7 +966,24 @@ function parseEventBlockMultiline(lines, startIndex, startBraceCount) {
const innerLines = lines.slice(startIndex + 1, endIndex).map(l => l.trim()).filter(l => l && !l.startsWith('//') && !l.startsWith('/*'));
const fields = [];
for (const line of innerLines) {
const match = line.match(/^(\w+)\s*:\s*(.+?),?$/);
// Cycle 62: strip inline `//` line-comments from the type expression
// before parsing. Without this, an event field like
// mesh_ip: Bytes // 4 bytes (IPv4 overlay)
// produces a TypeExpr whose name includes the comment text, which
// the .proto generator emits verbatim:
// bytes // 4 bytes (IPv4 overlay) mesh_ip = 2;
// (broken syntax — comment is in the type position).
const noComment = line.replace(/\/\/.*$/, '').trim();
// Cycle 65: strip default-value suffix `= <expr>` from the type
// expression. The federated-meetup speckdl has fields like
// initial_mesh_peers: List<InitialMeshPeer> = []
// where the default is meaningful in Speckl semantics (a state-
// machine init) but the .proto wire format doesn't carry a
// default — it's a generator concern. Without this, the type
// expression was emitted as `List<InitialMeshPeer> = []` and
// protoc rejects the resulting syntax.
const noDefault = noComment.replace(/\s*=\s*[^,]+$/, '').trim();
const match = noDefault.match(/^(\w+)\s*:\s*(.+?),?$/);
if (match) {
fields.push({ name: match[1], type: parseTypeExpr(match[2].replace(/,$/, '').trim()) });
}
@ -932,8 +998,16 @@ function parseInterfaceBlockMultiline(lines, startIndex, startBraceCount) {
const methods = [];
const fields = [];
for (const line of innerLines) {
// Cycle 62: strip inline `//` line-comments from each line before
// token-splitting. Without this, a field like
// mesh_ip: Bytes // 4 bytes (IPv4 overlay)
// has its comment text bleed into the parsed type expression,
// which the .proto generator emits verbatim — broken syntax.
const noComment = line.replace(/\/\/.*$/, '').trim();
if (!noComment)
continue;
// Split on comma so multi-variant lines like "Idle, Parsing, Done" all parse
const tokens = line.split(',').map(s => s.trim()).filter(Boolean);
const tokens = noComment.split(',').map(s => s.trim()).filter(Boolean);
for (const token of tokens) {
// Check 1: method signature form: name(params) -> returnType
const methodMatch = token.match(/^(\w+)\s*\(([^)]*)\)\s*->\s*(.+?),?$/);
@ -999,6 +1073,50 @@ function parseOneofBlockMultiline(lines, startIndex, startBraceCount) {
}
return { ...header, variants };
}
// Cycle 58: parse a `transition Name { ... }` block. Each non-empty,
// non-comment inner line is `field_name: TypeName`. Two field names
// have special meaning:
// - `payload: <OneofName>` — references a sibling `oneof` block;
// the proto generator will emit the variants inline as
// `oneof payload { ... }` inside the message body
// - `type: <EnumName>` — the discriminator enum for the variants
// Other fields are emitted as ordinary proto message fields.
function parseTransitionBlockMultiline(lines, startIndex, startBraceCount) {
const firstLine = lines[startIndex].trim();
const header = parseTransition(firstLine);
const endIndex = findBlockEnd(lines, startIndex + 1, startBraceCount);
const innerLines = lines.slice(startIndex + 1, endIndex).map(l => l.trim()).filter(l => l && !l.startsWith('//') && !l.startsWith('/*'));
const fields = [];
let payloadOneof;
let typeDiscriminator;
for (const line of innerLines) {
const trimmed = line.replace(/[,;]\s*$/, '');
const m = trimmed.match(/^(\w+)\s*:\s*(.+)$/);
if (!m)
continue;
const fieldName = m[1];
const typeStr = m[2].trim();
if (fieldName === 'payload') {
// The type is the name of a sibling oneof block.
payloadOneof = typeStr;
// Don't add a regular field for payload — it becomes the oneof.
continue;
}
if (fieldName === 'type') {
// The type is the name of a discriminator enum.
typeDiscriminator = typeStr;
// Don't add a regular field for type — it's a special discriminator.
continue;
}
fields.push({ name: fieldName, type: parseTypeExpr(typeStr) });
}
const result = { ...header, fields };
if (payloadOneof !== undefined)
result.payloadOneof = payloadOneof;
if (typeDiscriminator !== undefined)
result.typeDiscriminator = typeDiscriminator;
return result;
}
function parseServiceBlockMultiline(lines, startIndex, startBraceCount) {
// service Name {
const firstLine = lines[startIndex].trim();

2
compiler/dist/parser.js.map vendored
View file

File diff suppressed because one or more lines are too long

90
compiler/src/generators/protobuf.ts
View file

@ -52,6 +52,32 @@ function speckTypeToProto(typeName: string): string {
}
}
// Resolve a TypeExpr (which may be primitive, ident, list, map, set) to a
// proto3 type string. Used by all field-emitting code paths so that
// interface records, events, state, and transitions all handle
// List<T>, Map<K,V>, and Set<T> consistently.
// Cycle 73: extracted from inline code in event/state/transition emitters.
// Previously, interface record fields only called speckTypeToProto(f.type.name)
// which returned '' for List/Map/Set types (their .name is undefined),
// producing malformed proto like ` = 1;` with no type.
function typeExprToProto(t: TypeExpr): string {
if (t.type === 'map' && t.keyType && t.valueType) {
const k = speckTypeToProto(t.keyType.name || 'string');
const v = speckTypeToProto(t.valueType.name || 'bytes');
return `map<${k}, ${v}>`;
}
if (t.type === 'list' && t.elementType) {
const inner = typeExprToProto(t.elementType);
return `repeated ${inner}`;
}
if (t.type === 'set' && t.elementType) {
const inner = typeExprToProto(t.elementType);
return `repeated ${inner}`;
}
// Primitive or ident — fall back to name-based lookup
return speckTypeToProto(t.name || '');
}
// Deterministic field numbering: alphabetical by field name, 1-indexed.
function assignFieldNumbers(fields: { name: string }[]): Map<string, number> {
const sorted = [...fields].sort((a, b) => a.name.localeCompare(b.name));
@ -180,10 +206,14 @@ function emitSpeck(speck: SpeckNode, packageName?: string): string {
lines.push('');
lines.push('option go_package = "' + goPkg + '";');
lines.push('');
// Import google.protobuf.Timestamp if any field uses Date
// Import google.protobuf.Timestamp if any field uses Date.
// Cycle 73: extended to also check interface record fields (not just
// events and state). Previously, a spec with Date only in interface
// records (e.g. Ticket.sale_starts_at) would miss the import.
const usesTimestamp = members.some(m => {
if (m.type === 'event') {
return (m as EventNode).fields.some(f => f.type.name === 'Date');
return (m as EventNode).fields.some(f => f.type.name === 'Date' ||
(f.type.type === 'list' && f.type.elementType?.name === 'Date'));
}
if (m.type === 'state') {
return (m as StateNode).variables.some(v =>
@ -191,6 +221,15 @@ function emitSpeck(speck: SpeckNode, packageName?: string): string {
(v.typeExpr.type === 'list' && v.typeExpr.elementType?.name === 'Date')
);
}
if (m.type === 'interface') {
const iface = m as any;
if (iface.fields && iface.fields.length > 0) {
return iface.fields.some((f: any) =>
f.type.name === 'Date' ||
(f.type.type === 'list' && f.type.elementType?.name === 'Date')
);
}
}
return false;
});
if (usesTimestamp) {
@ -202,9 +241,15 @@ function emitSpeck(speck: SpeckNode, packageName?: string): string {
for (const [, iface] of Array.from(interfaces.entries())) {
if (iface.isEnum && iface.variants) {
lines.push(`enum ${iface.name} {`);
lines.push(` ${iface.name}_UNSPECIFIED = 0;`);
lines.push(` ${upperSnake(iface.name)}_UNSPECIFIED = 0;`);
iface.variants.forEach((v, i) => {
lines.push(` ${upperSnake(v)} = ${i + 1};`);
// Cycle 73: prefix variant with enum name to avoid collisions
// across enums in the same proto scope. Proto3 requires all enum
// values to be unique within the enclosing scope (not just the
// enum), so PENDING in Role and PENDING in OrderStatus collide.
// Prefixing with the enum name (ROLE_PENDING, ORDER_STATUS_PENDING)
// is the proto3 convention and eliminates the collision.
lines.push(` ${upperSnake(iface.name)}_${upperSnake(v)} = ${i + 1};`);
});
lines.push('}');
lines.push('');
@ -219,7 +264,7 @@ function emitSpeck(speck: SpeckNode, packageName?: string): string {
const fieldNums = assignFieldNumbers(iface.fields);
for (const f of iface.fields) {
const num = fieldNums.get(f.name) || 1;
const protoType = speckTypeToProto(f.type.name || '');
const protoType = typeExprToProto(f.type);
lines.push(` ${protoType} ${snakeCase(f.name)} = ${num};`);
}
lines.push('}');
@ -240,16 +285,7 @@ function emitSpeck(speck: SpeckNode, packageName?: string): string {
const fieldNums = assignFieldNumbers(ev.fields);
for (const f of ev.fields) {
const num = fieldNums.get(f.name) || 1;
let protoType = speckTypeToProto(f.type.name || '');
// Handle Map<K, V> and List<T>
if (f.type.type === 'map' && f.type.keyType && f.type.valueType) {
const k = speckTypeToProto(f.type.keyType.name || 'string');
const v = speckTypeToProto(f.type.valueType.name || 'bytes');
protoType = `map<${k}, ${v}>`;
} else if (f.type.type === 'list' && f.type.elementType) {
const inner = speckTypeToProto(f.type.elementType.name || 'string');
protoType = `repeated ${inner}`;
}
const protoType = typeExprToProto(f.type);
lines.push(` ${protoType} ${snakeCase(f.name)} = ${num};`);
}
}
@ -267,15 +303,7 @@ function emitSpeck(speck: SpeckNode, packageName?: string): string {
const fieldNums = assignFieldNumbers(stateNode.variables);
for (const v of stateNode.variables) {
const num = fieldNums.get(v.name) || 1;
let protoType = speckTypeToProto(v.typeExpr.name || '');
if (v.typeExpr.type === 'map' && v.typeExpr.keyType && v.typeExpr.valueType) {
const k = speckTypeToProto(v.typeExpr.keyType.name || 'string');
const v2 = speckTypeToProto(v.typeExpr.valueType.name || 'bytes');
protoType = `map<${k}, ${v2}>`;
} else if (v.typeExpr.type === 'list' && v.typeExpr.elementType) {
const inner = speckTypeToProto(v.typeExpr.elementType.name || 'string');
protoType = `repeated ${inner}`;
}
const protoType = typeExprToProto(v.typeExpr);
lines.push(` ${protoType} ${snakeCase(v.name)} = ${num};`);
}
lines.push('}');
@ -293,11 +321,21 @@ function emitSpeck(speck: SpeckNode, packageName?: string): string {
const oneofNodes = members.filter(m => m.type === 'oneof') as OneofNode[];
for (const t of transitionNodes) {
lines.push(`message ${t.name} {`);
// Cycle 73: emit the type discriminator field. The parser stores it
// in t.typeDiscriminator (a string naming the enum) rather than in
// t.fields, so it must be emitted explicitly. Field number 1 is
// reserved for the type discriminator (it's the most-accessed field).
if (t.typeDiscriminator) {
lines.push(` ${t.typeDiscriminator} type = 1;`);
}
const fieldNums = assignFieldNumbers(t.fields);
for (const f of t.fields) {
const num = fieldNums.get(f.name) || 1;
const protoType = speckTypeToProto(f.type.name || '');
lines.push(` ${protoType} ${snakeCase(f.name)} = ${num};`);
// If type discriminator is present, offset field numbers by 1
// to avoid collision with the type field at number 1.
const actualNum = t.typeDiscriminator ? num + 1 : num;
const protoType = typeExprToProto(f.type);
lines.push(` ${protoType} ${snakeCase(f.name)} = ${actualNum};`);
}
if (t.payloadOneof) {
const oneof = oneofNodes.find(o => o.name === t.payloadOneof);