feat(grpc): Convert takes the id form, as sysml -id does (#732) - #749
Conversation
ConvertRequest.id_form spells derived element ids when notation is written as a graph: qualified (the default) or uuid, for inline content, a file or a parsed model of several documents, refused as -id is for any other direction or value. Stubs regenerated with the pinned generators. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Devin Review found 1 potential issue.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
| opts, err := convertOptions(req.IdForm, convert.FormatSysML, to) | ||
| if err != nil { | ||
| return nil, true, err | ||
| } |
There was a problem hiding this comment.
🟡 Wrong status for invalid id form
For a multi-document model targeting notation, id_form returns FAILED_PRECONDITION instead of INVALID_ARGUMENT. The target check runs before convertOptions, so clients receive the wrong status for an invalid field.
Learn more
A cached model parsed from several documents takes a separate conversion path. That path rejects a notation target with FAILED_PRECONDITION before validating the new id_form field. The field's API contract requires INVALID_ARGUMENT when it is supplied for a direction other than notation-to-graph. This affects requests specifying both an invalid id_form direction and an unsupported multi-document output.
Example: Parse two documents, then call Convert with their model_hash, to_format: "sysml", and id_form: "uuid". The call returns FAILED_PRECONDITION for the notation target; the id_form contract requires INVALID_ARGUMENT.
Recommended fix: Validate nonempty req.IdForm against the parsed target with convertOptions before the multi-document target restriction in convertModelOfDocuments, while preserving FAILED_PRECONDITION when id_form is empty.
Was this helpful? React with 👍 or 👎 to provide feedback.
Review: a model of several documents converted to notation with an id_form was FAILED_PRECONDITION, where one document is INVALID_ARGUMENT; id_form is now judged first on both paths. CI regenerates the Go stubs under go.mod's Go 1.25.0, whose formatter spells some generated comments differently from a newer toolchain's; the stubs are regenerated with it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Fixes #732.
ConvertRequest.id_formchooses how derived element ids are spelled when notation is written as a graph, assysml -iddoes:id_formpackage P { part def A; part a : A; }→api-json,@idofP::a"qualified"P__a(as before)"uuid"sysml -convert api-json -id uuid"guid", or any value with asysmltargetINVALID_ARGUMENT, as-idrefuses themIt applies to inline content, a file, and a
model_hashparsed from several documents.Change:
sysml.proto:string id_form = 7;onConvertRequest. The stubs are regenerated with the pinned generators (make proto-buf,proto-ts,python-proto,proto-rust): Go, Java, TypeScript, Python and Rust, plus the Rust descriptor. Each diff adds only the field and the re-encoded descriptor.internal/frontend/grpc/export.go:convertOptionsreadsid_formwithexport.ParseIDForm, under the rulescmd/sysml'sconvertOptionsapplies to-id.convert.ConvertWith/ConvertTolerantWith, and the several-document path throughexport.ModelToRDFWith.docs/guide/07-saving-and-rdf.mddocuments the field.Tests:
internal/frontend/grpc/convert_id_form_test.go:"",qualifiedanduuid, the service's output is byte-identical toconvert.ConvertWithunder that form;uuid;INVALID_ARGUMENT.make proto-lintandmake proto-breakingpass (a field added, nothing removed or renumbered).internal/frontend/grpc/...,tests/grpc/...andinternal/translate/convert/...pass.TestOccupiedPortExitsNonzeroincmd/sysml-grpcfails the same way ondeveloplocally.client/node'spackage.jsonandpackage-lock.jsonare out of sync ondevelop(the@openmbee/opensysml-sysml-grpc-*optional dependencies aren't in the lock), sonpm cirefuses. The TypeScript stub was generated with@bufbuild/protoc-gen-es2.14.0, the pinned version, installed apart. The lock is left as it is.🤖 Generated with Claude Code