SRD-089.A — BPMN import: the parser spine and the converter's own defects¶

Field	Value
Status	Accepted
Date	2026-08-10
Owner	Ruslan Gabitov
Implements	ADR-024 v.4 §2.9 (the element dispositions), and the spine every later stage of slice 2 is built on
Upstream	ADR-019 v.1 §2 (imported `id` is the version key — the reason ids are never auto-generated)
Related	SRD-051 (the frozen one-shot that landed the converter this rebuilds), BPMN converter coverage audit 2026-08-10 (the evidence for §1), and the four SRDs that follow — languages and dialect, flow nodes, typed events and composites, data and document
Tracking	#284 (converter element coverage beyond the MVP subset — the epic this SRD set implements; stage 1 of 5, so `Part of`, not `Closes`). Downstream: #256 (example diagrams, blocked on the converter), #287 (the server's §2.3.2 claim). #261 is closed and was the model half of lanes (SRD-076); their converter half is a #284 deliverable, landing with stage .D

The converter's fence is nine elements, and inside those nine it answers several questions wrongly. ADR-024 v.4 moves the fence; this first stage does not move it at all. It builds the spine the other four stages need — element dispatch as a table, forward references resolved by one mechanism instead of one-off maps — and fixes what the coverage audit found inside the current subset, so the later stages extend correct code rather than propagate defects into thirty more elements.

§1 Background (verified)¶

Every claim here is a run or a grep, per the audit that produced them.

The dispatch is control flow, in six places. parseFlowElement (pkg/convert/bpmn/importer.go:278), handleDefinitionsChild (:189), consumeNodeChild (:706), parseSequenceFlowChild (:651), parseInterfaceChild (:418) and parseOperationChild (:480) each carry their own switch over local names, each with its own idea of what to skip and what to refuse. Adding an element means editing a switch in the right one of six — and the six do not agree today: consumeNodeChild refuses an unknown in-namespace child, while parseOperationChild skips one.

Forward references are one-off. Flow endpoints resolve through asm.byID in linkFlow (:763); exclusive-gateway defaults need a second pass over a second map, asm.gwDefaults → flowByID in applyGatewayDefaults (:824), because a default names a flow that does not exist during pass 1. That is two mechanisms for one problem, and the element set this slice adds brings at least four more references of exactly the same shape — attachedToRef, calledElement, link pairing, and data associations.

Five defects inside the current subset (audit §6; each reproduced by a probe run against 10467bb5):

	Defect	Evidence
F1	Export element order is non-deterministic	`Process.Nodes()`/`Flows()` range over maps (`pkg/model/process/process.go:38,39,205,243`); the shipped `examples/bpmn-convert` printed two different orderings in five runs
F2	An unnamed `task`/`manualTask`/`userTask` cannot be imported	BPMN `name` is 0..1; `parseNode` (`importer.go:319,330-343`) passes the raw name to constructors that reject an empty one, while the same file falls back to the id for `<process>` (`:227-230`) and `serviceTask` (`:522-524`)
F4	`serviceTask@implementation` is written by export, never read by import	`exporter.go:323`; probe: XML said `##WebService`, the model reported `##unspecified`
F5	Export can emit a schema-invalid `parallelGateway`	`UpdateDefaultFlow` is on the base `Gateway` (`pkg/model/gateways/gateway.go:174`) and `setGatewayAttrs` (`exporter.go:341`) writes `default` for every kind; probe emitted `<bpmn:parallelGateway id="g" default="f2">`
F9	`<bpmn:documentation>` is lost both ways	skipped on import (`bpmn.go:48`), never written on export

Three element families fail for carrying a comment. textAnnotation, group and category are refused (probe: unsupported element "textAnnotation", with no spec § pinned, because sectionFor has no entry). The vendored extract lists them out of scope as "pure visual" (conformance.md), which is the same ground that earned documentation and extensionElements their silent-skip carve-out.

§2 Requirements¶

Functional¶

FR-1 — Element dispatch is a table, not six switches. Each parse context has a parser table keyed by local name; what no table claims falls to one policy lookup keyed by (context, local name) that answers skipped or refused, with the spec § pinned from a second table. Refusal is the zero value, so an element absent from every table is reported rather than silently accepted. Adding an element in .B–.E is a row.
FR-2 — One deferred-reference mechanism. Any attribute naming an element that may not exist yet is recorded as a pending reference in pass 1 and resolved in pass 2 by a single resolver, which reports an unresolvable reference with the referring element, the attribute and the missing id. asm.gwDefaults and the ad-hoc flowByID map disappear into it.
FR-3 (F1) — Export is byte-identical across runs for one unchanged model.
FR-4 (F2) — A nameless flow element imports, falling back to its id, for every element whose constructor demands a name.
FR-5 (F4) — serviceTask@implementation survives a round-trip.
FR-6 (F5) — default is emitted only on gateway kinds the standard gives one (exclusive, inclusive, complex — §13.4.1 gives the parallel gateway none).

Exercised on the exclusive gateway alone, because it is the only one of the three the exporter emits: export still covers the ADR-024 §2.6 subset, so an inclusive gateway imports and then has no export path (§5 records that asymmetry as the cost of moving the import fence). The rule is stated per kind so it stays right when export catches up. - FR-7 (F9) — <bpmn:documentation> is imported onto the element's Docs() and written back on export. - FR-8 — §2.9's dispositions for out-of-scope families: textAnnotation, group, category, relationship and import are skipped silently; choreography and conversation elements are refused.

Superseded, not retracted. Four of those five skips were re-decided after this stage landed and are mapped today: <import> binds to the <itemDefinition> whose structureRef prefix resolves to it (ADR-024 v.5), and textAnnotation/group/category are parsed into the model-only artifact tier (ADR-039 v.1, ADR-024 v.6) because the §2.3.2 loading obligation needs the model to hold what a diagram states. relationship still skips. The disposition mechanism this FR delivered — a table where refusal is the zero value — is what carried those re-decisions, and is unchanged.

Non-functional¶

NFR-1 — The fence does not move. The set of mapped elements is exactly today's. A file that imports before this SRD imports after it, and a file that is refused is still refused — except the FR-4 and FR-8 classes, which are the point.
NFR-2 — make ci green, including cover-check at COVER_MIN on every touched file.
NFR-3 — No behaviour change in pkg/model beyond the single additive carrier FR-5 needs (§4.3).

§3 Models¶

// parseCtx names the position in the document where an element appeared;
// the same local name can mean different things in two contexts.
type parseCtx uint8

const (
    ctxDefinitions parseCtx = iota
    ctxProcess
    ctxNode
    ctxSequenceFlow
    ctxInterface
    ctxOperation
)

// elementKey identifies one element in one context.
type elementKey struct {
    local string
    ctx   parseCtx
}

// dispositionKind is what the converter does with an element no parser
// claims. refused is the zero value, so absence from every table means
// "reported", never "silently accepted".
type dispositionKind uint8

const (
    refused dispositionKind = iota
    skipped
)

// policy declares every non-default disposition; sections pins the spec §
// an UnsupportedElementError carries.
var (
    policy   = map[elementKey]dispositionKind{ /* … */ }
    sections = map[string]string{ /* … */ }
)

The mapped half is a table per context rather than a column of the one above, because a parser's signature depends on what it is parsing into — a child of <operation> fills an opSpec, a child of <sequenceFlow> fills a flowSpec, a flow element fills the assembly. One func(any) error would buy a single table at the price of a runtime cast in the exact code a later stage is most likely to get wrong.

// One per context; the process table is DERIVED from nodeBuilders so the
// two cannot disagree about which elements are flow nodes.
var (
    definitionsParsers  = map[string]defsParser{ /* … */ }
    processParsers      = /* derived */ map[string]procParser{}
    nodeBuilders        = map[string]nodeBuilder{ /* element → model constructor */ }
    sequenceFlowParsers = map[string]flowChildParser{ /* … */ }
    interfaceParsers    = map[string]ifaceChildParser{ /* … */ }
    operationParsers    = map[string]opChildParser{ /* … */ }
)

// pendingRef is one forward reference recorded in pass 1 (FR-2): an attribute
// naming an element that may appear later in the document.
//
// refSite is the referring end, shared by every kind so an unresolvable
// reference always names both ends and the attribute that joined them.
type pendingRef interface {
    // resolve looks the reference up in the completed index and applies it,
    // or reports why it could not.
    resolve(idx *refIndex) error
}

type refSite struct {
    from   string // the referring element's id
    attr   string // the referring attribute, e.g. "default"
    target string // the id being referred to
}

Landed as an interface rather than as the kind-tagged struct first drafted here. A tag plus apply func(any) error casts at run time, so a later element slice wiring the wrong target learns it as a panic; one implementation per reference kind keeps the target's type through resolution and makes the same mistake a compile error. The two id spaces the drafted refKind separated are still separate — a default naming a node still fails as a type error rather than "not found" — but the separation is carried by the implementations instead of by a tag.

§4 Analysis & decisions¶

§4.1 Export order: walk the graph (FR-3)¶

The model does not record document order — Process.nodes and .flows are map[string]… (process.go:38,39) — so the exporter must impose one:

Option	Verdict
Walk from the start events, following outgoing flows in flow-id order; unreached nodes appended by id	Chosen. Deterministic, and it emits the process the way a token travels it, which is how a reader meets it.
Sort by id	Rejected after first choosing it. Equally deterministic and simpler, but determinism was the requirement and alphabetical is merely the cheapest way to reach it. For the repository's own `linear.bpmn` (`s1`, `t1`, `e1`) it emits a process document that opens with the end event, and an exported file is something people open. The ~30 lines the walk costs are not the open-ended design work this row first claimed.
Preserve document order	Rejected: it needs the model to carry an ordering, a core change for a property that is meaningless for a programmatically built process.

The walk degrades rather than fails: a graph with no start event, or a disconnected fragment, falls back to id order for whatever the walk did not reach — nothing is dropped from the export.

Consequence: the sample export in the converters guide already shows s1 → t1 → e1, which is exactly what the walk produces, so the guide needs no edit. Under the id-sort it would have needed one.

§4.2 A nameless element falls back to its id (FR-4)¶

The alternative is relaxing the model constructors to accept an empty name, and that is wrong: the constructors are a model invariant used by every programmatic caller, and the converter's problem is a converter problem. The id fallback is already this file's own idiom in two places (:227-230, :522-524); FR-4 makes it the rule rather than the exception.

§4.3 `implementation` is a task attribute the model derives from the operation (FR-5)¶

The audit called this "export-only, FIX-track, small". Grounding it changed the answer, and the change is worth recording because it is the only core-touching item in this SRD.

ServiceTask.Implementation() returns a field set from the operation — implementation: operation.Type() (pkg/model/activities/service_task.go:150) — and Type() derives from the operation's Implementor, returning ##unspecified when there is none (pkg/model/service/operation.go:207-214). An imported operation has a nil implementor by design (ADR-024 §2.6: the converter is not an execution engine). So there is nowhere to put the imported hint, and "just read the attribute" does not exist as a fix.

In BPMN, implementation is an own property of the ServiceTask — the type's two own properties are implementation (String, 0..1) and operationRef (Operation ref, 0..1), elements/activities.md:92-93 — not a property of the operation. gobpm conflates the two, which is why the attribute has nowhere to land.

Option	Verdict
A carrier on `ServiceTask`: an optional `implementation` set at construction, defaulting to the operation's type when unset	Chosen. Additive, backwards-compatible (every existing caller keeps today's derived value), and it makes the model match the standard's own placement.
Stop exporting `implementation`	Rejected. It is legitimate interchange content when it says `##WebService`; the fact that it is also how a Go implementor's type leaks out is an argument for the carrier, not for silence.
Synthesize an implementor whose `Type()` returns the hint	Rejected: a fake implementor that cannot execute would satisfy the getter and fail at the first invocation, which is the failure mode this whole ADR exists to move earlier.

§4.4 Visual artifacts are skipped, not refused (FR-8)¶

textAnnotation/group/category carry no semantics (conformance.md, "pure visual"), so dropping them leaves the imported definition meaning the same thing — the test ADR-024 §2.9 states. association is not in this group and stays refused here: the extract keeps it in scope "because it carries compensation semantics", so it is mapped work for .D, not a comment to skip.

The means-the-same test was later found not to be the only test. ADR-039 v.1 added a second obligation cutting across it: §2.3.2 loading and the §2.8 round-trip can only re-emit what the model holds, so the three artifacts are carried rather than skipped, while execution still ignores them. The reasoning above is sound and incomplete — it weighed semantics and not representation.

§5 API deltas¶

Surface	Delta
`activities.NewServiceTask`	a new option carrying the BPMN `implementation` hint (§4.3). Additive; unset preserves today's derived value.
`pkg/convert/bpmn`	internal only — the table and the resolver are unexported. No change to `convert.Importer`/`Exporter`; `DocumentImporter` arrives in .E.

§6 Test scenarios¶

#	Scenario	Asserts
T-1	Export the same model 20 times	byte-identical output (FR-3)
T-1a	A process with an unreachable node, and one with no start event	nothing is dropped; the remainder falls back to id order (§4.1)
T-2	Import a file whose `task`, `manualTask` and `userTask` carry no `name`	all three import; each `Name()` equals its id (FR-4)
T-3	Round-trip a `serviceTask` with `implementation="##WebService"`	the attribute survives import→export (FR-5)
T-4	Export a parallel gateway carrying a default flow	no `default` attribute emitted; the exclusive case still emits it (FR-6)
T-5	Round-trip an element with `<bpmn:documentation>`	text present on `Docs()` after import and in the re-exported XML (FR-7)
T-6	Import a file with `textAnnotation`, `group`, `category`, `relationship`, `import`	imports; the flow graph is unchanged (FR-8). The asserted outcome still holds; the premise does not — all but `relationship` are mapped today, and the test that grew from this row is named `TestVisualArtifactsAreCarried`.
T-7	Import a `choreography` / `conversation` element	refused, classified (FR-8)
T-8	A `default` naming an id that does not exist	error names the referring gateway, the attribute and the missing id (FR-2)
T-9	A `default` naming a node instead of a flow	refused as a type mismatch, not "not found" (FR-2)
T-10	Every fixture under `testdata/valid` and `testdata/invalid`	unchanged verdicts — the fence did not move (NFR-1)

testdata/ grows one fixture per new scenario; the existing eight are the NFR-1 regression corpus and are not edited.

Held for this stage. Two of the eight were later edited by other slices — unsupported-element.bpmn swapped its refused element when scriptTask became importable, and exclusive-branch.bpmn gained an explicit language= when .B made an unmarked condition refusable. The NFR-1 claim is about this landing, not a freeze on the corpus.

§7 Milestones¶

M	Content	Commit
M1	The dispatch table + all six switches routed through it; no behaviour change	one
M2	The deferred-reference resolver; `gwDefaults`/`flowByID` removed	one
M3	F1 (the export walk), F5 (parallel gateway default)	one
M4	F2, F9 (import + export)	one
M5	F4 — the `ServiceTask` carrier and both directions	one
M6	FR-8 dispositions	one

§8 Cross-doc¶

Implements ADR-024 v.4 §2.9. The ADR is Draft: it is accepted on this SRD-set's landing, so this pin tracks a document that may still move.
ADR-019 v.1 §2 — ids are never auto-generated; FR-4's fallback fills the name, never the id.
SRD-051 is a frozen one-shot and is not edited by this work, including where it names MustRegisterImporter.

§9 Definition of Done¶

FR-1…FR-8 wired, each with its §6 test.
make ci green — including cover-check at COVER_MIN on every touched file, measured after each milestone commit (the gate is HEAD-based).
examples/bpmn-convert runs to exit 0 and prints a stable document.
The converters guide's sample export matches the code (§4.1 — it already does).
/check-srd PASS, then /pr-review — the SRD set opens one PR, at .E.

§10 Implementation summary¶

M	Commit	Content
M1	`4b179b56`	The dispatch tables; six switches and `sectionFor`'s seventh removed
M2	`d9f6d493`	`pendingRef`/`refIndex`; `gwDefaults` + the ad-hoc `flowByID` gone
M3	`956c7d43`	FR-3 (the export walk) + FR-6 (no `default` on a parallel gateway)
M4	`be9ad9ba`	FR-7 (documentation, both ways) + FR-4 (the id fallback); body-before-build
M5	`7a2a1142`	FR-5 (`WithImplementation` on `ServiceTask`)
M6	`cabcc7bf`	FR-8 (the dispositions)
M6a	`23147517`	The gate-red fix — see item 6
M7	`3d24d5f7`	The independent review's agreed findings — see item 7

Files. New: dispatch.go, refs.go, order.go (the first and third with their own tests; the deferred-reference tests live in dispatch_test.go), documentation_test.go, implementation_test.go, dispositions_test.go, pkg/model/activities/service_task_implementation_test.go. Changed: importer.go (−184 lines net at M1 alone), exporter.go, bpmn.go, pkg/model/activities/service_task{,_options}.go.

What the work found that the SRD did not predict, recorded because the later stages inherit it:

F4 was not a small fix. implementation is derived from the Operation's Implementor, which an imported operation deliberately lacks, so the attribute had nowhere to land. Resolved by §4.3's carrier — the only core-touching change in this stage.
F9 forced body-before-build. BaseElement has no doc setter, so documentation must arrive as a construction option, so a node's children must be read before it is built. .C and .D need exactly this: an event definition decides what to construct.
A <process> cannot use that order (its children are every flow element), so it is built lazily on its first flow element. Exact, not lucky: BaseElement's properties serialize ahead of Process's own flowElements, so a schema-valid file always presents documentation first. A file that does not is refused, naming the rule.
Two defects were introduced and caught during the work, both by tests written for something else: successors sorted before filtering nils, so its nil guard was unreachable; and obeying govet/fieldalignment on xmlSequenceFlow moved documentation after the condition in the emitted document, because encoding/xml writes children in field order. Both are now pinned by tests.
<import>'s skip is time-limited. It has no home only because no itemDefinition is mapped yet; the data stage must revisit the row.
The gate caught what package-scoped runs could not. make ci went red at test-core on internal/lintcfg's TestNoLiteralAttrKeys, which scans every string literal in non-test pkg//internal/ code for a canonical observability key — and AttrImplementation spells the same as the BPMN serviceTask attribute M5 started reading. The attribute name now comes from the constant, with a test pinning the two spellings together: they are different things that collide, and a vocabulary rename would otherwise send the parser looking for an attribute no document carries, silently, with every other test green. Recorded because it is a standing hazard for .B–.E, which will read many more attribute names.
The independent review found a real defect in FR-3's own machinery. /pr-review (agy / gemini-3.1-pro-high, three doc-blind lenses, 5/5 chunks each) returned 6 notes: 3 agreed, 3 refuted. The agreed headline — orderFlows answered 0 for every comparison involving a nil, which is not an ordering but an assertion that nil equals everything, so one nil made unrelated flows transitively equal and slices.SortFunc returned an unsorted slice. Measured: e ahead of a b c d. The function carried 100% statement coverage and a passing nil-guard test throughout, because two elements cannot expose an intransitive comparator. Neither /check-srd, nor /check-style, nor the coverage gate is shaped to ask that question. Two test weaknesses landed with it (a count-only assertion that also passed vacuously; the unpinned import-side behaviour of parallelGateway@default). The three refutations are recorded in the PR description; all three lenses shared one blind spot — an early return they each read past — which is why agreement among same-family reviewers is one signal, not three.

Open questions¶

None.