feat(query): report the elementId Convert writes (#733) - #747
devin-ai-integration[bot] merged 3 commits into
Conversation
elementId joins the queryable properties (and sysml:elementId in OSLC). It is read from the writer's own identity tables (export.ElementIDs, built once per model), so a declared id, a derived id and a library element's normative id are the ones a conversion of the model writes, and a query result joins the converted graph. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Note
Newer findings are available below. Devin Review posted a newer report on this PR, in addition to the findings presented here.
Devin Review found 3 potential issues.
2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
| libraryFQN, ok := ids.library.libraryElement(sym) | ||
| if !ok { | ||
| return "", false |
There was a problem hiding this comment.
🔴 Distinct scoped elements share one reported ID
When separate identity scopes declare the same qualified name, byName overwrites one element's ID with the other's. Queries for both elements then report the same ID, even when conversion writes different IDs.
Learn more
A model's RDF conversion permits repeated qualified names in different identity scopes by qualifying their subject IRIs; modelToRDF activates this when multiple scopes exist. A map keyed only by qualified name cannot distinguish those subjects, and Of receives only the name rather than the queried symbol. If the scopes declare distinct IDs for the same name, one result necessarily receives the wrong ID.
Example: Two documents each declare package P in different ProjectRef scopes, with declared IDs first-p and second-p. Conversion writes two distinct scoped package subjects, but querying both P symbols returns second-p for both.
Recommended fix: Key the lookup by an identity that distinguishes the declaration, such as the document and node/symbol, and associate each converted subject with its declaration. Pass the queried symbol into Of instead of relying on a qualified name alone.
Was this helpful? React with 👍 or 👎 to provide feedback.
Review: - the ids are read from a conversion of the model made as Convert makes it for the model hash (one document from its text, several from what parse read), so a library file's copy gets its normative ids; - a model the conversion refuses has no elementIds: a query reading one is FAILED_PRECONDITION with the refusal, instead of silently lacking it; - a library element a model annotates with its own id is referenced by name in a conversion, so it reports none, as Convert writes none; - an empty select no longer reports elementId, which converts the model. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review: Convert refuses a model of several documents one of which the parser could not read whole; elementId read ids from the recovered trees instead. It is refused the same way now, and a query reading it fails with the syntax error. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Fixes #733.
elementIdis now a queryable property. It reports theelementIdConvertwrites for the element, from the writer's own identity tables.After
ParseSourcesof both,select: ["@id", "elementId"]:@id(unchanged)elementIdConvertapi-json@idLib::EngineLib::EngineLib__EngineLib__EngineLib::Engine::massLib::Engine::massLib__Engine__massLib__Engine__massApp::eApp::eApp__eApp__eScalarValues::Real(scope: ["ScalarValues::Real"])ScalarValues::Real14c0aa22-5489-59b5-b438-ded26e83ba31On
develop,select: ["elementId"]isINVALID_ARGUMENT(unknown query property "elementId").Change:
query.PropertyElementID(elementId) joins the closed property set, and OSLC mapssysml:elementIdto it.PropertyReader.WithElementIDsupplies it; without one it is absent.selectdoesn't report it (query.DefaultProjection), since reading it converts the model. A query that names it inselectorwheregets it.export.ElementIDsreads the ids from a conversion of the model, made the wayConvertmakes it for thatmodel_hash:NewElementIDsOfDocument, assysmlToRDFWithdoes);NewElementIDs,modelToRDF, whichModelToRDFWithnow wraps).So a library file's copy, a declared
ElementIdand scope qualification come out as written. A standard-library element the model doesn't declare reports the normative id every reference to it carries.A model
Convertrefuses has noelementIds. A query readingelementIdof it isFAILED_PRECONDITIONwith the conversion's reason, and a query not reading it is unaffected.A library element the model gives an id of its own (
metadata … ElementId about ScalarValues::Boolean { id = "custom-boolean"; }) reports noelementId.Convertwrites no id for it: a reference to it is{"@ref": "ScalarValues::Boolean"}.The service builds the ids once per cached model (
CachedModel.elementIDs), in a structured query and in OSLC alike.api.mddocuments the property. In a model of several identity scopes, the API JSON@idcarries the scope qualifier (.one:P__A) whileelementIdis the element's own (P__A), asConvertwrites it.Tests:
internal/frontend/grpc/query_element_id_test.go: every element a whole-model query returns haselementIdequal toConvert'selementIdfor the same qualified name, and equal to its@idwhere no scope qualifies it. The models:ElementId;ScalarValues::Real's normative id, both referenced from a model and in a parsed copy ofScalarValues.kerml;Convertrefuses (two documents declaringpackage P):FAILED_PRECONDITIONwhenelementIdis selected, success otherwise, and noelementIdfor an emptyselect;elementIdfor an annotated library element, alongsideConvert's@reffor it.internal/semantic/query/...,internal/translate/...,internal/frontend/grpc/...,tests/grpc/...andtests/export/...pass.🤖 Generated with Claude Code