From 8b11d0429889eb11e7800d84724f58cd5cb20d9e Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 26 Sep 2026 19:54:13 +0000 Subject: [PATCH 1/9] fix(describe): DESCRIBE FRAGMENT FROM PAGE/SNIPPET found no widget, ever MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The visitor stores DescribeFragmentFromStmt.ContainerType as "PAGE"/"SNIPPET" while describeFragmentFrom switched on "page"/"snippet" with no default, so neither branch ran and every widget was reported missing ("not found in page M.P" — without even naming the widget). Same casing split as ALTER PAGE (#402) and ALTER STYLING (#631). Normalise with strings.ToLower (the convention of the other consumers), make an unrecognised container type an error instead of an empty widget list, and name the widget in the not-found message. The new tests parse the statement and dispatch it through the registry, so they pin the visitor/executor casing contract that a hand-built lowercase AST could not see. Co-Authored-By: Claude Opus 5.5 --- .../fix-issue/findings/mdl-executor.jsonl | 1 + .../describe-fragment-from-container-case.mdl | 32 +++++ mdl/executor/cmd_fragments.go | 13 +- mdl/executor/cmd_fragments_from_test.go | 112 ++++++++++++++++++ 4 files changed, 156 insertions(+), 2 deletions(-) create mode 100644 mdl-examples/bug-tests/describe-fragment-from-container-case.mdl create mode 100644 mdl/executor/cmd_fragments_from_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 8bb82beaa..df18c6f9b 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -713,3 +713,4 @@ {"area": "mdl/executor", "date": "2026-09-25", "symptom": "`textbox t (Attribute: FullName)` at the top of a page (CREATE PAGE/SNIPPET, a plain container, or ALTER PAGE … INSERT at page level) passed plain `mxcli check`, `exec --no-check`/ALTER reported success, and `bson dump` showed `AttributeRef: null` — mxbuild 11.13.0: CE0544 \"This widget can only function inside a data context\" + CE7005 (textbox/textarea/datepicker/checkbox/radiobuttons/dropdown), CE0402 (dynamictext Attribute:), CE0642 (combobox). Qualified `Mod.Ent.Attr` there is stored and fails CE0544/CE2421/CE1365/CE7247 \"Move this widget into a data container\" + CE7006. `Attribute: $P/Attr` / `$currentObject/Attr` dropped even INSIDE a data view.", "cause": "resolveAttributePath returns the bare name when entityContext is \"\", and attributeRefToGen (and widgetobj setAttributeRefField) write nil for any path with < 2 dots, so the binding vanished between builder and writer; refuseBareAttributeRefs never sees it because no Attribute string is emitted. The only refusal (validatePageContextTree) runs in the --references phase for CREATE PAGE/SNIPPET, so plain check, --no-check and ALTER were unguarded. `$x/Attr` parses via the generic property rule as an *ast.DataSourceV3, so GetAttribute() returns \"\" and every builder skipped it.", "file": "mdl/executor/cmd_pages_input_binding_context.go (inputBindingProblem, checkInputBinding, validateInputBindingContext = MDL-WIDGET34), wired in cmd_pages_builder_v3_widgets.go (6 input builders + buildDynamicTextV3), widget_engine.go (primary Attribute mapping), validate_widgets.go (validateWidgetTreeIn); tests cmd_pages_input_binding_context_test.go; bug-tests input-binding-without-context{,.fail}.mdl", "insight": "Reuse the MDL-PAGEARG01 three-state context (pageArgContext known/present) rather than entityContext==\"\" as the 'outside a data container' signal: entityContext is also empty INSIDE a container whose flow cannot be resolved (excluded ShareFeedback_Logo), where DESCRIBE writes qualified names that must keep building — refusing qualified-on-empty-entity would have broken that round trip. So known-absent context refuses bare AND qualified; unknown context (ALTER) refuses only the bare name the writer provably nulls. Two existing unit tests (OnChangeSurvivesBuilder, DynamicTextV3_AttributeBinds) built inputs with NO entity and passed — the second asserted a bare `Title` AttributeRef counted as 'bound', i.e. it pinned the bug: when a fixture has no entity context, ask what the writer does with its output. The `$P/Attr` drop was found only by dumping the control page, not from the report — print the AST value type with a probe test before assuming a spelling reaches the builder. Evidence: 22 mxbuild errors before on the probe matrix; after, every case refused with nothing written, controls (dataview/listview/gallery/datagrid/snippet dataview/ALTER into dataview) 0 errors, 17/17 stock pages + 4/4 snippets describe→exec round trip.", "refs": ["MDL-WIDGET34"], "ce": ["CE0544", "CE7005", "CE0402", "CE0642", "CE2421", "CE1365", "CE7247", "CE7006"]} {"area":"mdl/executor","date":"2026-09-25","symptom":"`alter page FeedbackModule.ShareFeedback_Logo { insert after textBox1 { image zzImg (ImageType: imageUrl, ImageUrl: '{1}', ImageUrlParams: [{1} = ImageB64]) } }` passed `check --references`; exec wrote a bare AttributeRef and `mx check` could not LOAD the project (ArgumentNullException setting 'Attribute'). Same at page top level outside any data container; a text box's `Attribute:` there is silently dropped (CE7005).","cause":"ALTER's entity context comes from the STORED document (nearest enclosing data source, or a flow source's return type via resolveDataSourceFlowEntity). With the flow missing (Feedback v4.0.2 ships no DS_FeedbackForm) or no container at all, entityContext is \"\" and resolveAttributePath returns the bare name. CREATE PAGE refused this at check time (relaxExcludedWidgetRefs/unscopedBindings, #678); ALTER's check never opened the document, so nothing could know the scope.","file":"mdl/executor/validate_alter_unscoped.go, mdl/executor/cmd_alter_page.go (alterEntityContext), mdl/executor/validate.go (bindingsWithoutScope)","insight":"For ALTER, scope is a property of the stored document, not the statement: a check-time question about it must open the document (OpenPageForMutation, never Save; validate_alter_set.go already does this) and ask through the SAME function exec uses, so the INSERT/REPLACE entity resolution was lifted into alterEntityContext rather than restated, and the binding walk lifted out of unscopedBindings (bindingsWithoutScope) rather than copied. Controls that keep it from blocking working scripts: skip a target the stored doc lacks (added earlier in the script), a flow the script declares with an entity return (sc.flowParams), DataGrid2 column and list-view-template paths, and documents the script creates. Reproduce with a Studio Pro-authored page whose flow is genuinely absent.","refs":["#678","#685"]} {"area": "mdl/executor", "date": "2026-09-25", "symptom": "`describe page` on a File Uploader (files mode) emits `DataSource: association …`, and exec of that output fails: \"widget `upFiles` (fileuploader) exposes 2 datasources, so a generic `datasource:` clause is ambiguous — name the one you mean: associatedFiles, associatedImages\"", "cause": "DESCRIBE chose generic vs named by counting CONFIGURED datasources (namedCustomWidgetDataSources drops unset ones, so files mode = 1), while the builder's refuseAmbiguousGenericDataSource counts DECLARED datasource mappings (a generated def maps every top-level datasource = 2). Two sides of one round trip deciding the same question from different evidence.", "file": "mdl/executor/cmd_pages_describe_parse.go", "insight": "When describe and build each decide 'is this ambiguous?', they must count the same set. Fix read the DECLARED count from the stored schema (PropertyTypes with ValueType.Type=DataSource, excluding IsLinked) and excluded widgets with an embedded .def.json — those are hand-written and pick one datasource mapping per mode, so a database-mode ComboBox (two declared) must keep the generic clause. The #956 bug-test script itself authored the refused generic clause on a File Uploader: a bug-test that only runs `mxcli check` without a project can't see an exec-time refusal, so grep bug-tests for the old spelling whenever a builder starts refusing one.", "refs": ["mendixlabs/mxcli#1199", "mendixlabs/mxcli#956", "mendixlabs/mxcli#1109"], "rules": []} +{"area": "mdl/executor", "date": "2026-09-26", "symptom": "`describe fragment from page M.P widget w` (and `from snippet`) fails for every container and every widget — including ones `describe page` prints — with `not found in page M.P`, a message that does not even name the widget", "cause": "Visitor stores DescribeFragmentFromStmt.ContainerType as \"PAGE\"/\"SNIPPET\"; describeFragmentFrom switched on \"page\"/\"snippet\" with no default, so neither branch ran, the widget list stayed empty, and the fall-through reported the widget missing. Third instance of this split: ALTER PAGE (#402), DESCRIBE/ALTER STYLING (#631)", "file": "`mdl/executor/cmd_fragments.go` (`describeFragmentFrom`)", "insight": "The mismatch hid behind a plausible error because a switch on the discriminator had no default: an unmatched container type looked like an empty container, and an empty container looks like a missing widget. Normalise with strings.ToLower where the discriminator is consumed (the house convention — cmd_styling, cmd_alter_page, validate_alter_* all do) AND make the default an error, so the next casing drift fails loudly instead of reporting the wrong thing. The existing mock tests hand-built the AST and so agreed with the handler; only a test that goes visitor.Build → NewRegistry().Dispatch pins the contract between the two layers (cmd_fragments_from_test.go). Verified on Evora: Administration.Account_Edit/textBox6 and AgentCommons.Snippet_Agent_Details/dataView7 now describe", "refs": ["#402", "#631"]} diff --git a/mdl-examples/bug-tests/describe-fragment-from-container-case.mdl b/mdl-examples/bug-tests/describe-fragment-from-container-case.mdl new file mode 100644 index 000000000..eaa9862fe --- /dev/null +++ b/mdl-examples/bug-tests/describe-fragment-from-container-case.mdl @@ -0,0 +1,32 @@ +-- DESCRIBE FRAGMENT FROM PAGE/SNIPPET always failed — found by an +-- agent-orientation audit on a large app (Evora Factory Management). +-- +-- REPORTED SYMPTOM +-- +-- describe fragment from page Administration.Account_Edit widget textBox6 +-- → Error: not found in page Administration.Account_Edit +-- +-- for every page, every snippet and every widget — including widgets that +-- `describe page` prints. The message also omitted the widget's name. +-- +-- CAUSE +-- +-- The visitor stores ContainerType as "PAGE"/"SNIPPET"; the handler in +-- mdl/executor/cmd_fragments.go switched on "page"/"snippet". Nothing +-- matched, the widget list stayed empty, and the fall-through reported the +-- widget as missing. Same casing split as ALTER STYLING (#631) and ALTER +-- PAGE (#402) before it. +-- +-- EXPECTED (run against a project that has these documents; the page is the +-- stock Administration module, so most apps carry it) +-- +-- 1. prints: textbox textBox6 (Label: 'Full name', Attribute: FullName) +-- 2. Error: widget noSuchWidget not found in page Administration.Account_Edit +-- 3. Error: page not found: Administration.NoSuchPage +-- +-- Usage: +-- mxcli exec mdl-examples/bug-tests/describe-fragment-from-container-case.mdl -p app.mpr + +describe fragment from page Administration.Account_Edit widget textBox6; +describe fragment from page Administration.Account_Edit widget noSuchWidget; +describe fragment from page Administration.NoSuchPage widget textBox6; diff --git a/mdl/executor/cmd_fragments.go b/mdl/executor/cmd_fragments.go index 15b5782f0..7f50781fb 100644 --- a/mdl/executor/cmd_fragments.go +++ b/mdl/executor/cmd_fragments.go @@ -81,7 +81,13 @@ func describeFragmentFrom(ctx *ExecContext, s *ast.DescribeFragmentFromStmt) err var rawWidgets []rawWidget - switch s.ContainerType { + // The visitor stores ContainerType uppercase ("PAGE"/"SNIPPET"), as it + // does for DESCRIBE/ALTER STYLING and ALTER PAGE. Normalise here, and make + // an unrecognised value an error: falling through the switch leaves no + // widgets, which then reads as "widget not found" — the disguise this + // casing mismatch wore here and in cmd_styling.go before it. + containerType := strings.ToLower(s.ContainerType) + switch containerType { case "page": allPages, err := ctx.Backend.ListPages() if err != nil { @@ -119,12 +125,15 @@ func describeFragmentFrom(ctx *ExecContext, s *ast.DescribeFragmentFromStmt) err return mdlerrors.NewNotFound("snippet", s.ContainerName.String()) } rawWidgets = getSnippetWidgetsFromRaw(ctx, foundSnippet.ID) + + default: + return mdlerrors.NewUnsupported("describe fragment from: unsupported container type " + s.ContainerType) } // Find the widget by name target := findRawWidgetByName(rawWidgets, s.WidgetName) if target == nil { - return mdlerrors.NewNotFoundMsg("widget", s.WidgetName, fmt.Sprintf("not found in %s %s", strings.ToLower(s.ContainerType), s.ContainerName.String())) + return mdlerrors.NewNotFoundMsg("widget", s.WidgetName, fmt.Sprintf("widget %s not found in %s %s", s.WidgetName, containerType, s.ContainerName.String())) } // Output as MDL diff --git a/mdl/executor/cmd_fragments_from_test.go b/mdl/executor/cmd_fragments_from_test.go new file mode 100644 index 000000000..225a55874 --- /dev/null +++ b/mdl/executor/cmd_fragments_from_test.go @@ -0,0 +1,112 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "bytes" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/visitor" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/pages" +) + +// These tests parse the statement and dispatch it through the registry, +// rather than hand-building a DescribeFragmentFromStmt. The defect they pin +// was a casing contract between two layers: the visitor stores ContainerType +// as "PAGE"/"SNIPPET" and the handler switched on "page"/"snippet", so every +// `describe fragment from ...` fell through the switch with no widgets and +// reported the widget as missing. A test that builds the AST in lowercase +// agrees with the handler and cannot see that. + +func fragmentFromCtx(t *testing.T) (*ExecContext, *bytes.Buffer) { + t.Helper() + mod := mkModule("Shop") + pg := mkPage(mod.ID, "Product_Edit") + sn := mkSnippet(mod.ID, "Product_Card") + + widget := func(name string) map[string]any { + return map[string]any{ + "$Type": "Forms$DivContainer", + "Name": name, + "Widgets": []any{int32(2)}, + } + } + raw := map[model.ID]map[string]any{ + pg.ID: { + "$Type": "Forms$Page", + "FormCall": map[string]any{ + "Arguments": []any{int32(2), map[string]any{ + "Widgets": []any{int32(2), widget("pageBox")}, + }}, + }, + }, + sn.ID: { + "$Type": "Forms$Snippet", + "Widgets": []any{int32(2), widget("snippetBox")}, + }, + } + + ctx, buf := newMockCtx(t, withHierarchy(mkHierarchy(mod))) + mb := ctx.Backend.(*mock.MockBackend) + mb.ListPagesFunc = func() ([]*pages.Page, error) { return []*pages.Page{pg}, nil } + mb.ListSnippetsFunc = func() ([]*pages.Snippet, error) { return []*pages.Snippet{sn}, nil } + mb.GetRawUnitFunc = func(id model.ID) (map[string]any, error) { return raw[id], nil } + return ctx, buf +} + +func runParsed(t *testing.T, ctx *ExecContext, src string) error { + t.Helper() + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("parse errors: %v", errs) + } + if len(prog.Statements) != 1 { + t.Fatalf("want 1 statement, got %d", len(prog.Statements)) + } + return NewRegistry().Dispatch(ctx, prog.Statements[0]) +} + +func TestDescribeFragmentFrom_Parsed(t *testing.T) { + for _, tc := range []struct{ name, src, want string }{ + {"page", `describe fragment from page Shop.Product_Edit widget pageBox;`, "pageBox"}, + {"snippet", `describe fragment from snippet Shop.Product_Card widget snippetBox;`, "snippetBox"}, + {"uppercase keywords", `DESCRIBE FRAGMENT FROM PAGE Shop.Product_Edit WIDGET pageBox;`, "pageBox"}, + } { + t.Run(tc.name, func(t *testing.T) { + ctx, buf := fragmentFromCtx(t) + if err := runParsed(t, ctx, tc.src); err != nil { + t.Fatalf("execute: %v", err) + } + if !strings.Contains(buf.String(), tc.want) { + t.Fatalf("output does not describe widget %q:\n%s", tc.want, buf.String()) + } + }) + } +} + +// A missing widget must name what was looked for — the widget and the +// container — so the reader can tell a typo from a wrong page. +func TestDescribeFragmentFrom_MissingWidgetNamesItAndContainer(t *testing.T) { + ctx, _ := fragmentFromCtx(t) + err := runParsed(t, ctx, `describe fragment from page Shop.Product_Edit widget noSuchBox;`) + if err == nil { + t.Fatal("want an error for a missing widget") + } + for _, want := range []string{"noSuchBox", "page", "Shop.Product_Edit"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q does not mention %q", err, want) + } + } +} + +// A missing container is its own error, not a missing widget. +func TestDescribeFragmentFrom_MissingContainer(t *testing.T) { + ctx, _ := fragmentFromCtx(t) + err := runParsed(t, ctx, `describe fragment from snippet Shop.NoSuchSnippet widget snippetBox;`) + if err == nil || !strings.Contains(err.Error(), "snippet not found") { + t.Fatalf("want snippet-not-found, got %v", err) + } +} From 94367426107957fa577f13dfd58d09531279a87e Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 26 Sep 2026 20:00:15 +0000 Subject: [PATCH 2/9] fix(search): warn when the source index was never built MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `search` needs only a full catalog, which indexes string literals but not MDL source. On a project where `refresh catalog full source` had never run, the source half of every search came back empty with no word — read by agents as "no microflow/page mentions this". search now checks the build mode the catalog records (not the row count, so a built-but-empty index stays silent) and, below "source", prints a warning naming `refresh catalog full source` on a new ExecContext.Diagnostics writer (nil = stderr), keeping --format json stdout pure. The old unconditional "Tip: refresh catalog source" on stdout is replaced by it. Table format now delegates to execSearch up front instead of querying twice. Co-Authored-By: Claude Opus 5.5 --- .../fix-issue/findings/mdl-executor.jsonl | 1 + cmd/mxcli/cmd_query.go | 3 + .../bug-tests/search-missing-source-index.mdl | 31 ++++ mdl/executor/cmd_catalog.go | 42 +++++- mdl/executor/cmd_search_source_index_test.go | 136 ++++++++++++++++++ mdl/executor/exec_context.go | 15 ++ 6 files changed, 224 insertions(+), 4 deletions(-) create mode 100644 mdl-examples/bug-tests/search-missing-source-index.mdl create mode 100644 mdl/executor/cmd_search_source_index_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 8bb82beaa..449b237f4 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -713,3 +713,4 @@ {"area": "mdl/executor", "date": "2026-09-25", "symptom": "`textbox t (Attribute: FullName)` at the top of a page (CREATE PAGE/SNIPPET, a plain container, or ALTER PAGE … INSERT at page level) passed plain `mxcli check`, `exec --no-check`/ALTER reported success, and `bson dump` showed `AttributeRef: null` — mxbuild 11.13.0: CE0544 \"This widget can only function inside a data context\" + CE7005 (textbox/textarea/datepicker/checkbox/radiobuttons/dropdown), CE0402 (dynamictext Attribute:), CE0642 (combobox). Qualified `Mod.Ent.Attr` there is stored and fails CE0544/CE2421/CE1365/CE7247 \"Move this widget into a data container\" + CE7006. `Attribute: $P/Attr` / `$currentObject/Attr` dropped even INSIDE a data view.", "cause": "resolveAttributePath returns the bare name when entityContext is \"\", and attributeRefToGen (and widgetobj setAttributeRefField) write nil for any path with < 2 dots, so the binding vanished between builder and writer; refuseBareAttributeRefs never sees it because no Attribute string is emitted. The only refusal (validatePageContextTree) runs in the --references phase for CREATE PAGE/SNIPPET, so plain check, --no-check and ALTER were unguarded. `$x/Attr` parses via the generic property rule as an *ast.DataSourceV3, so GetAttribute() returns \"\" and every builder skipped it.", "file": "mdl/executor/cmd_pages_input_binding_context.go (inputBindingProblem, checkInputBinding, validateInputBindingContext = MDL-WIDGET34), wired in cmd_pages_builder_v3_widgets.go (6 input builders + buildDynamicTextV3), widget_engine.go (primary Attribute mapping), validate_widgets.go (validateWidgetTreeIn); tests cmd_pages_input_binding_context_test.go; bug-tests input-binding-without-context{,.fail}.mdl", "insight": "Reuse the MDL-PAGEARG01 three-state context (pageArgContext known/present) rather than entityContext==\"\" as the 'outside a data container' signal: entityContext is also empty INSIDE a container whose flow cannot be resolved (excluded ShareFeedback_Logo), where DESCRIBE writes qualified names that must keep building — refusing qualified-on-empty-entity would have broken that round trip. So known-absent context refuses bare AND qualified; unknown context (ALTER) refuses only the bare name the writer provably nulls. Two existing unit tests (OnChangeSurvivesBuilder, DynamicTextV3_AttributeBinds) built inputs with NO entity and passed — the second asserted a bare `Title` AttributeRef counted as 'bound', i.e. it pinned the bug: when a fixture has no entity context, ask what the writer does with its output. The `$P/Attr` drop was found only by dumping the control page, not from the report — print the AST value type with a probe test before assuming a spelling reaches the builder. Evidence: 22 mxbuild errors before on the probe matrix; after, every case refused with nothing written, controls (dataview/listview/gallery/datagrid/snippet dataview/ALTER into dataview) 0 errors, 17/17 stock pages + 4/4 snippets describe→exec round trip.", "refs": ["MDL-WIDGET34"], "ce": ["CE0544", "CE7005", "CE0402", "CE0642", "CE2421", "CE1365", "CE7247", "CE7006"]} {"area":"mdl/executor","date":"2026-09-25","symptom":"`alter page FeedbackModule.ShareFeedback_Logo { insert after textBox1 { image zzImg (ImageType: imageUrl, ImageUrl: '{1}', ImageUrlParams: [{1} = ImageB64]) } }` passed `check --references`; exec wrote a bare AttributeRef and `mx check` could not LOAD the project (ArgumentNullException setting 'Attribute'). Same at page top level outside any data container; a text box's `Attribute:` there is silently dropped (CE7005).","cause":"ALTER's entity context comes from the STORED document (nearest enclosing data source, or a flow source's return type via resolveDataSourceFlowEntity). With the flow missing (Feedback v4.0.2 ships no DS_FeedbackForm) or no container at all, entityContext is \"\" and resolveAttributePath returns the bare name. CREATE PAGE refused this at check time (relaxExcludedWidgetRefs/unscopedBindings, #678); ALTER's check never opened the document, so nothing could know the scope.","file":"mdl/executor/validate_alter_unscoped.go, mdl/executor/cmd_alter_page.go (alterEntityContext), mdl/executor/validate.go (bindingsWithoutScope)","insight":"For ALTER, scope is a property of the stored document, not the statement: a check-time question about it must open the document (OpenPageForMutation, never Save; validate_alter_set.go already does this) and ask through the SAME function exec uses, so the INSERT/REPLACE entity resolution was lifted into alterEntityContext rather than restated, and the binding walk lifted out of unscopedBindings (bindingsWithoutScope) rather than copied. Controls that keep it from blocking working scripts: skip a target the stored doc lacks (added earlier in the script), a flow the script declares with an entity return (sc.flowParams), DataGrid2 column and list-view-template paths, and documents the script creates. Reproduce with a Studio Pro-authored page whose flow is genuinely absent.","refs":["#678","#685"]} {"area": "mdl/executor", "date": "2026-09-25", "symptom": "`describe page` on a File Uploader (files mode) emits `DataSource: association …`, and exec of that output fails: \"widget `upFiles` (fileuploader) exposes 2 datasources, so a generic `datasource:` clause is ambiguous — name the one you mean: associatedFiles, associatedImages\"", "cause": "DESCRIBE chose generic vs named by counting CONFIGURED datasources (namedCustomWidgetDataSources drops unset ones, so files mode = 1), while the builder's refuseAmbiguousGenericDataSource counts DECLARED datasource mappings (a generated def maps every top-level datasource = 2). Two sides of one round trip deciding the same question from different evidence.", "file": "mdl/executor/cmd_pages_describe_parse.go", "insight": "When describe and build each decide 'is this ambiguous?', they must count the same set. Fix read the DECLARED count from the stored schema (PropertyTypes with ValueType.Type=DataSource, excluding IsLinked) and excluded widgets with an embedded .def.json — those are hand-written and pick one datasource mapping per mode, so a database-mode ComboBox (two declared) must keep the generic clause. The #956 bug-test script itself authored the refused generic clause on a File Uploader: a bug-test that only runs `mxcli check` without a project can't see an exec-time refusal, so grep bug-tests for the old spelling whenever a builder starts refusing one.", "refs": ["mendixlabs/mxcli#1199", "mendixlabs/mxcli#956", "mendixlabs/mxcli#1109"], "rules": []} +{"area": "mdl/executor", "date": "2026-09-26", "symptom": "`search 'keyword'` (MDL and `mxcli search`, every format) on a project where `refresh catalog full source` was never run returns string-literal matches only, with no word that MDL source was not searched. An agent orienting on a large app (Evora Factory Management, 786 entities) read the empty source section as \"no microflow/page mentions this\". When nothing matched at all a tip did print — to stdout, and regardless of whether the index existed", "cause": "search calls ensureCatalog(full=true): full mode builds the strings FTS table but not source, and a full build is deliberately not promoted to source (source costs a describe of every document). The source query ran against an empty FTS table and `err == nil && Count > 0` treated \"never built\" and \"nothing matched\" identically", "file": "mdl/executor/cmd_catalog.go", "insight": "An empty result from a table that a cheaper build mode leaves unpopulated is the #1060 GRAPH_CYCLES shape again: the catalog already records its build_mode in catalog_meta (GetCacheInfo().BuildMode), so decide \"was this index built?\" from that marker, never from row count — a source-mode catalog with zero rows is a real answer and must stay silent. warnIfCatalogModeInsufficient did this for `select from CATALOG.SOURCE` but search built its own query and bypassed it; any command that queries a mode-gated table directly needs the same check. Route the warning to a diagnostics writer (ExecContext.Diagnostics, nil = stderr), not ctx.Output, or it corrupts --format json. The test fixture that makes this cheap: write a real catalog cache (SetCacheInfo with the stand-in .mpr's mtime + SaveToFile to /.mxcli/catalog.db) and ensureCatalog loads it with a MockBackend, no project needed", "refs": ["mendixlabs/mxcli#1060"], "rules": []} diff --git a/cmd/mxcli/cmd_query.go b/cmd/mxcli/cmd_query.go index d99b7fbf0..582438bca 100644 --- a/cmd/mxcli/cmd_query.go +++ b/cmd/mxcli/cmd_query.go @@ -212,6 +212,9 @@ var searchCmd = &cobra.Command{ Searches across string literals (captions, labels, messages) and MDL source definitions. Requires at least a FULL catalog build (done automatically). +MDL source is searched only once the source index exists — build it with + mxcli -p app.mpr -c "refresh catalog full source" +Until then a warning on stderr says only string literals were searched. Output Formats: table - Human-readable table (default) diff --git a/mdl-examples/bug-tests/search-missing-source-index.mdl b/mdl-examples/bug-tests/search-missing-source-index.mdl new file mode 100644 index 000000000..a62ffa875 --- /dev/null +++ b/mdl-examples/bug-tests/search-missing-source-index.mdl @@ -0,0 +1,31 @@ +-- ============================================================================ +-- search: say when the source index is missing +-- ============================================================================ +-- +-- Symptom (before fix): +-- `search 'keyword'` (MDL and `mxcli search`) needs only a FULL catalog, +-- which indexes string literals but not MDL source. On any project where +-- `refresh catalog full source` had never been run, CATALOG.SOURCE was +-- empty and the source half of every search came back empty with no word — +-- an agent read "no source matches" as "no microflow mentions this". When +-- strings DID match, there was no hint at all; when nothing matched, the tip +-- printed regardless of whether the index existed. +-- +-- After fix: +-- A catalog whose recorded build mode is below "source" prints, on stderr: +-- Warning: the source index (CATALOG.SOURCE) is not built (catalog mode: full) ... +-- Build it with: refresh catalog full source (CLI: mxcli -p ... -c "...") +-- A source-mode catalog never warns, even with zero matches (keyed off the +-- build mode, not the row count). `--format json` stdout stays pure JSON. +-- +-- Run against any project (needs a connection; the catalog is built on demand): +-- mxcli -p app.mpr -c "refresh catalog full" +-- mxcli -p app.mpr -c "search 'retrieve'" -- expect the warning on stderr +-- mxcli -p app.mpr -c "refresh catalog full source" +-- mxcli -p app.mpr -c "search 'retrieve'" -- expect Source Matches, no warning +-- mxcli search -p app.mpr "zzqq_no_match" --format json -- stdout: [] stderr: nothing +-- ============================================================================ + +refresh catalog full; + +search 'retrieve'; diff --git a/mdl/executor/cmd_catalog.go b/mdl/executor/cmd_catalog.go index 6d9be1256..961a19e41 100644 --- a/mdl/executor/cmd_catalog.go +++ b/mdl/executor/cmd_catalog.go @@ -1052,12 +1052,43 @@ func execSearch(ctx *ExecContext, stmt *ast.SearchStmt) error { if !found { fmt.Fprintln(ctx.Output, "No matches found.") - fmt.Fprintln(ctx.Output, "Tip: Use refresh catalog source to enable source-level search.") } + warnIfSourceIndexMissing(ctx) return nil } +// warnIfSourceIndexMissing says, on the diagnostics channel, that a search did +// not look at MDL source because CATALOG.SOURCE was never built. `search` needs +// only a full catalog, which has the strings index but not the source one, so +// without this the source half of every answer came back empty in silence — +// indistinguishable from "no microflow, page or expression mentions this". +// +// Keyed off the build mode the catalog records, never the row count: a +// source-mode catalog with no matching (or no) rows is a real answer, and +// counting rows would make "built, nothing matched" warn too. +func warnIfSourceIndexMissing(ctx *ExecContext) { + mode := "" + if ctx.Catalog != nil { + if info, err := ctx.Catalog.GetCacheInfo(); err == nil { + mode = info.BuildMode + } + } + if catalogModeRank(mode) >= catalogModeRank("source") { + return + } + if mode == "" { + mode = "unknown" + } + w := ctx.diagnostics() + fmt.Fprintf(w, "Warning: the source index (CATALOG.SOURCE) is not built (catalog mode: %s), so only string literals were searched — MDL source (microflow/nanoflow bodies, expressions, page and entity definitions) was not.\n", mode) + if ctx.MprPath != "" { + fmt.Fprintf(w, "Build it with: refresh catalog full source (CLI: mxcli -p %q -c \"refresh catalog full source\")\n", ctx.MprPath) + } else { + fmt.Fprintln(w, "Build it with: refresh catalog full source") + } +} + // escapeFTSQuery escapes special characters in FTS5 queries. // FTS5 treats characters like '/', '.', '-' as token separators. To make // queries like 'rest/companies' or 'Module.Entity' usable, we replace these @@ -1080,6 +1111,10 @@ func search(ctx *ExecContext, query, format string) error { return mdlerrors.NewNotConnected() } + if format != "names" && format != "json" { // "table" (default) + return execSearch(ctx, &ast.SearchStmt{Query: query}) + } + // Ensure catalog is built (at least full mode for strings table) if err := ensureCatalog(ctx, true); err != nil { return err @@ -1137,10 +1172,10 @@ func search(ctx *ExecContext, query, format string) error { if len(allResults) == 0 { if format != "json" { fmt.Fprintln(ctx.Output, "No matches found.") - fmt.Fprintln(ctx.Output, "Tip: Use refresh catalog source to enable source-level search.") } else { fmt.Fprintln(ctx.Output, "[]") } + warnIfSourceIndexMissing(ctx) return nil } @@ -1160,10 +1195,9 @@ func search(ctx *ExecContext, query, format string) error { return err } fmt.Fprintln(ctx.Output, string(jsonBytes)) - default: // "table" - return execSearch(ctx, &ast.SearchStmt{Query: query}) } + warnIfSourceIndexMissing(ctx) return nil } diff --git a/mdl/executor/cmd_search_source_index_test.go b/mdl/executor/cmd_search_source_index_test.go new file mode 100644 index 000000000..ec171ca67 --- /dev/null +++ b/mdl/executor/cmd_search_source_index_test.go @@ -0,0 +1,136 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "bytes" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/catalog" +) + +// searchCacheFixture writes a real catalog cache next to a stand-in .mpr, at +// the given build mode, so search's ensureCatalog loads it instead of +// building. sourceRows seeds CATALOG.SOURCE; the strings table always holds +// one caption containing "Invoice". +func searchCacheFixture(t *testing.T, mode string, sourceRows [][2]string) (*ExecContext, *bytes.Buffer, *bytes.Buffer) { + t.Helper() + dir := t.TempDir() + mprPath := filepath.Join(dir, "app.mpr") + if err := os.WriteFile(mprPath, []byte("stand-in"), 0o644); err != nil { + t.Fatal(err) + } + fi, err := os.Stat(mprPath) + if err != nil { + t.Fatal(err) + } + + cat, err := catalog.New() + if err != nil { + t.Fatal(err) + } + db := cat.CatalogDB() + if _, err := db.Exec(`INSERT INTO strings (QualifiedName, ObjectType, StringValue, StringContext, ModuleName) + VALUES ('Sales.Invoice_Overview', 'PAGE', 'Invoice overview', 'caption', 'Sales')`); err != nil { + t.Fatalf("seed strings: %v", err) + } + for _, r := range sourceRows { + if _, err := db.Exec(`INSERT INTO source (QualifiedName, ObjectType, SourceText, ModuleName, ElementId) + VALUES (?, 'MICROFLOW', ?, 'Sales', 'id-1')`, r[0], r[1]); err != nil { + t.Fatalf("seed source: %v", err) + } + } + if err := cat.SetCacheInfo(mprPath, fi.ModTime(), "11.0.0", mode, 0); err != nil { + t.Fatal(err) + } + cachePath := filepath.Join(dir, ".mxcli", "catalog.db") + if err := os.MkdirAll(filepath.Dir(cachePath), 0o755); err != nil { + t.Fatal(err) + } + if err := cat.SaveToFile(cachePath); err != nil { + t.Fatalf("save cache: %v", err) + } + cat.Close() + + var diag bytes.Buffer + ctx, out := newMockCtx(t, withQuiet()) + ctx.MprPath = mprPath + ctx.Diagnostics = &diag + t.Cleanup(func() { + if ctx.Catalog != nil { + ctx.Catalog.Close() + } + }) + return ctx, out, &diag +} + +const sourceIndexHint = "refresh catalog full source" + +// A full-mode catalog has no source index. A search there answered only from +// string literals and said nothing — an agent read the empty source section +// as "no MDL mentions this". The warning must name the command that builds it, +// and go to diagnostics so --format json stays parseable. +func TestSearch_WarnsWhenSourceIndexNotBuilt(t *testing.T) { + for _, format := range []string{"json", "names", "table"} { + t.Run(format, func(t *testing.T) { + ctx, out, diag := searchCacheFixture(t, "full", nil) + if err := search(ctx, "Invoice", format); err != nil { + t.Fatalf("search: %v", err) + } + if !strings.Contains(diag.String(), sourceIndexHint) { + t.Errorf("expected a missing-source-index warning naming %q on diagnostics; got %q", sourceIndexHint, diag.String()) + } + if strings.Contains(out.String(), sourceIndexHint) || strings.Contains(out.String(), "Warning") { + t.Errorf("warning leaked into the results payload:\n%s", out.String()) + } + if format == "json" { + var v []map[string]any + if err := json.Unmarshal(out.Bytes(), &v); err != nil { + t.Errorf("json output is not pure JSON: %v\n%s", err, out.String()) + } + } + }) + } +} + +// The warning is about the index, not about the result: a query that matches +// nothing anywhere on a full-mode catalog must still say the source was not +// searched (this is the case where "No matches found." is most misleading). +func TestSearch_WarnsOnNoMatchesWithoutSourceIndex(t *testing.T) { + ctx, _, diag := searchCacheFixture(t, "full", nil) + if err := execSearch(ctx, &ast.SearchStmt{Query: "Nonexistent"}); err != nil { + t.Fatal(err) + } + if !strings.Contains(diag.String(), sourceIndexHint) { + t.Errorf("expected warning on diagnostics; got %q", diag.String()) + } +} + +// An index that was built but matched nothing is a real answer. Keyed off the +// recorded build mode, not the row count: a source-mode catalog with zero +// source rows must not warn. +func TestSearch_NoWarningWhenSourceIndexBuilt(t *testing.T) { + cases := map[string][][2]string{ + "built-empty": nil, + "built-matching": {{"Sales.ACT_Invoice_Create", "create microflow Sales.ACT_Invoice_Create () begin end;"}}, + } + for name, rows := range cases { + t.Run(name, func(t *testing.T) { + ctx, out, diag := searchCacheFixture(t, "source", rows) + if err := search(ctx, "Invoice", "json"); err != nil { + t.Fatal(err) + } + if diag.Len() != 0 { + t.Errorf("source index is built; expected no warning, got %q", diag.String()) + } + if strings.Contains(out.String(), sourceIndexHint) { + t.Errorf("unexpected hint in output:\n%s", out.String()) + } + }) + } +} diff --git a/mdl/executor/exec_context.go b/mdl/executor/exec_context.go index 7a5e97a65..b6ed7d8d0 100644 --- a/mdl/executor/exec_context.go +++ b/mdl/executor/exec_context.go @@ -34,6 +34,12 @@ type ExecContext struct { // Output is the writer for user-visible output (with line-limit guard). Output io.Writer + // Diagnostics receives warnings about the answer rather than the answer + // itself — "this result is incomplete because…" — so they never land in a + // payload a caller parses (search --format json). Nil means os.Stderr; use + // diagnostics() rather than reading the field. + Diagnostics io.Writer + // describeQualifyAttrs is set by DESCRIBE PAGE while it reads the widgets // inside a data container whose flow cannot be resolved: no entity is in // scope there, so an attribute binding keeps its stored Module.Entity.Attr @@ -135,6 +141,15 @@ type ExecContext struct { ScriptDir string } +// diagnostics returns the writer for warnings about a result (see the +// Diagnostics field): the configured one, or os.Stderr. +func (ctx *ExecContext) diagnostics() io.Writer { + if ctx.Diagnostics != nil { + return ctx.Diagnostics + } + return os.Stderr +} + // ResolveScriptRelative turns a path written inside an MDL script into an // absolute one: relative to the script's own directory when that is known, and // to the working directory otherwise. From a7c2377d3e15349bbc12b85e51f77e7442dbdf1f Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 26 Sep 2026 20:02:04 +0000 Subject: [PATCH 3/9] fix(structure): show microflow and nanoflow counts at depth 1 `show structure depth 1` (and its JSON form) filtered the catalog on MicroflowType = 'microflow' / 'nanoflow', while the catalog builder stores 'MICROFLOW' / 'NANOFLOW'. SQLite's `=` is case-sensitive, so both counts were always empty, and the summary omits zero counts, so every module appeared to have no flows at all. The builder's values are now exported constants (catalog.MicroflowTypeMicroflow/Nanoflow/Rule), used by the writer, by the structure query and by the linter's DocumentNoun switch, so reader and writer share one spelling. The test runs the real catalog builder over a MockBackend and reads the counts back through structureDepth1 / structureDepth1JSON, so it detects a casing drift between writer and reader; reverting only the reader to lower case makes it fail with the reported symptom. Co-Authored-By: Claude Opus 5.5 --- .../fix-issue/findings/mdl-executor.jsonl | 1 + .../bug-tests/structure-flow-counts.mdl | 37 +++++++ mdl/catalog/builder_microflows.go | 21 +++- mdl/executor/cmd_structure.go | 16 ++- .../cmd_structure_flow_counts_test.go | 99 +++++++++++++++++++ mdl/linter/context.go | 4 +- 6 files changed, 169 insertions(+), 9 deletions(-) create mode 100644 mdl-examples/bug-tests/structure-flow-counts.mdl create mode 100644 mdl/executor/cmd_structure_flow_counts_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 8bb82beaa..48ab98f39 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -713,3 +713,4 @@ {"area": "mdl/executor", "date": "2026-09-25", "symptom": "`textbox t (Attribute: FullName)` at the top of a page (CREATE PAGE/SNIPPET, a plain container, or ALTER PAGE … INSERT at page level) passed plain `mxcli check`, `exec --no-check`/ALTER reported success, and `bson dump` showed `AttributeRef: null` — mxbuild 11.13.0: CE0544 \"This widget can only function inside a data context\" + CE7005 (textbox/textarea/datepicker/checkbox/radiobuttons/dropdown), CE0402 (dynamictext Attribute:), CE0642 (combobox). Qualified `Mod.Ent.Attr` there is stored and fails CE0544/CE2421/CE1365/CE7247 \"Move this widget into a data container\" + CE7006. `Attribute: $P/Attr` / `$currentObject/Attr` dropped even INSIDE a data view.", "cause": "resolveAttributePath returns the bare name when entityContext is \"\", and attributeRefToGen (and widgetobj setAttributeRefField) write nil for any path with < 2 dots, so the binding vanished between builder and writer; refuseBareAttributeRefs never sees it because no Attribute string is emitted. The only refusal (validatePageContextTree) runs in the --references phase for CREATE PAGE/SNIPPET, so plain check, --no-check and ALTER were unguarded. `$x/Attr` parses via the generic property rule as an *ast.DataSourceV3, so GetAttribute() returns \"\" and every builder skipped it.", "file": "mdl/executor/cmd_pages_input_binding_context.go (inputBindingProblem, checkInputBinding, validateInputBindingContext = MDL-WIDGET34), wired in cmd_pages_builder_v3_widgets.go (6 input builders + buildDynamicTextV3), widget_engine.go (primary Attribute mapping), validate_widgets.go (validateWidgetTreeIn); tests cmd_pages_input_binding_context_test.go; bug-tests input-binding-without-context{,.fail}.mdl", "insight": "Reuse the MDL-PAGEARG01 three-state context (pageArgContext known/present) rather than entityContext==\"\" as the 'outside a data container' signal: entityContext is also empty INSIDE a container whose flow cannot be resolved (excluded ShareFeedback_Logo), where DESCRIBE writes qualified names that must keep building — refusing qualified-on-empty-entity would have broken that round trip. So known-absent context refuses bare AND qualified; unknown context (ALTER) refuses only the bare name the writer provably nulls. Two existing unit tests (OnChangeSurvivesBuilder, DynamicTextV3_AttributeBinds) built inputs with NO entity and passed — the second asserted a bare `Title` AttributeRef counted as 'bound', i.e. it pinned the bug: when a fixture has no entity context, ask what the writer does with its output. The `$P/Attr` drop was found only by dumping the control page, not from the report — print the AST value type with a probe test before assuming a spelling reaches the builder. Evidence: 22 mxbuild errors before on the probe matrix; after, every case refused with nothing written, controls (dataview/listview/gallery/datagrid/snippet dataview/ALTER into dataview) 0 errors, 17/17 stock pages + 4/4 snippets describe→exec round trip.", "refs": ["MDL-WIDGET34"], "ce": ["CE0544", "CE7005", "CE0402", "CE0642", "CE2421", "CE1365", "CE7247", "CE7006"]} {"area":"mdl/executor","date":"2026-09-25","symptom":"`alter page FeedbackModule.ShareFeedback_Logo { insert after textBox1 { image zzImg (ImageType: imageUrl, ImageUrl: '{1}', ImageUrlParams: [{1} = ImageB64]) } }` passed `check --references`; exec wrote a bare AttributeRef and `mx check` could not LOAD the project (ArgumentNullException setting 'Attribute'). Same at page top level outside any data container; a text box's `Attribute:` there is silently dropped (CE7005).","cause":"ALTER's entity context comes from the STORED document (nearest enclosing data source, or a flow source's return type via resolveDataSourceFlowEntity). With the flow missing (Feedback v4.0.2 ships no DS_FeedbackForm) or no container at all, entityContext is \"\" and resolveAttributePath returns the bare name. CREATE PAGE refused this at check time (relaxExcludedWidgetRefs/unscopedBindings, #678); ALTER's check never opened the document, so nothing could know the scope.","file":"mdl/executor/validate_alter_unscoped.go, mdl/executor/cmd_alter_page.go (alterEntityContext), mdl/executor/validate.go (bindingsWithoutScope)","insight":"For ALTER, scope is a property of the stored document, not the statement: a check-time question about it must open the document (OpenPageForMutation, never Save; validate_alter_set.go already does this) and ask through the SAME function exec uses, so the INSERT/REPLACE entity resolution was lifted into alterEntityContext rather than restated, and the binding walk lifted out of unscopedBindings (bindingsWithoutScope) rather than copied. Controls that keep it from blocking working scripts: skip a target the stored doc lacks (added earlier in the script), a flow the script declares with an entity return (sc.flowParams), DataGrid2 column and list-view-template paths, and documents the script creates. Reproduce with a Studio Pro-authored page whose flow is genuinely absent.","refs":["#678","#685"]} {"area": "mdl/executor", "date": "2026-09-25", "symptom": "`describe page` on a File Uploader (files mode) emits `DataSource: association …`, and exec of that output fails: \"widget `upFiles` (fileuploader) exposes 2 datasources, so a generic `datasource:` clause is ambiguous — name the one you mean: associatedFiles, associatedImages\"", "cause": "DESCRIBE chose generic vs named by counting CONFIGURED datasources (namedCustomWidgetDataSources drops unset ones, so files mode = 1), while the builder's refuseAmbiguousGenericDataSource counts DECLARED datasource mappings (a generated def maps every top-level datasource = 2). Two sides of one round trip deciding the same question from different evidence.", "file": "mdl/executor/cmd_pages_describe_parse.go", "insight": "When describe and build each decide 'is this ambiguous?', they must count the same set. Fix read the DECLARED count from the stored schema (PropertyTypes with ValueType.Type=DataSource, excluding IsLinked) and excluded widgets with an embedded .def.json — those are hand-written and pick one datasource mapping per mode, so a database-mode ComboBox (two declared) must keep the generic clause. The #956 bug-test script itself authored the refused generic clause on a File Uploader: a bug-test that only runs `mxcli check` without a project can't see an exec-time refusal, so grep bug-tests for the old spelling whenever a builder starts refusing one.", "refs": ["mendixlabs/mxcli#1199", "mendixlabs/mxcli#956", "mendixlabs/mxcli#1109"], "rules": []} +{"area": "mdl/executor", "date": "2026-09-26", "symptom": "`show structure depth 1` / `mxcli structure -d 1` never lists microflow or nanoflow counts for any module (Evora Factory Management: FactoryManagement shows entities and pages but not its 127 microflows), in both the text and the JSON form (Microflows/Nanoflows always 0).", "cause": "cmd_structure.go filtered the catalog with MicroflowType = 'microflow' / 'nanoflow'; the catalog builder writes 'MICROFLOW' / 'NANOFLOW'. SQLite `=` on TEXT is case-sensitive, so both counts were empty, and the depth-1 formatter omits zero counts, so absence looked like 'this module has no flows'.", "file": "mdl/executor/cmd_structure.go, mdl/catalog/builder_microflows.go", "insight": "Two silences stacked: queryCountByModule swallows errors and empty results alike, and the summary drops zero counts — so a filter matching nothing produces no signal at any layer. The cheap detector is a test that runs the REAL catalog builder (catalog.NewBuilder over a MockBackend; it needs GetProjectSettingsFunc and ListModuleSettingsFunc set, the mock's defaults panic / error) and reads through the consumer's query — a hand-seeded microflows_data row only proves the reader agrees with the test author. The writer's values are now exported constants (catalog.MicroflowTypeMicroflow/Nanoflow/Rule) and every Go reader uses them; grep for `MicroflowType = '` literals found no other lower-case reader. Same shape as the refs.SourceType drift (#1027): any catalog enum column needs a named vocabulary on the writer side.", "refs": ["fix/structure-flow-counts"]} diff --git a/mdl-examples/bug-tests/structure-flow-counts.mdl b/mdl-examples/bug-tests/structure-flow-counts.mdl new file mode 100644 index 000000000..cfe92d608 --- /dev/null +++ b/mdl-examples/bug-tests/structure-flow-counts.mdl @@ -0,0 +1,37 @@ +-- Bug test: `show structure depth 1` (CLI: `mxcli structure -d 1`) never +-- listed microflow or nanoflow counts, on any project. +-- +-- Found by an agent-orientation audit on a large app (Evora Factory +-- Management): every module line read "N entities, M pages, ..." with no +-- flows at all, although FactoryManagement alone has 127 microflows. +-- +-- Cause: the structure summary filtered the catalog on +-- MicroflowType = 'microflow' / 'nanoflow', while the catalog builder stores +-- 'MICROFLOW' / 'NANOFLOW'. SQLite's `=` is case-sensitive, so both counts +-- were empty and the summary omits zero counts - no error, just absence. +-- +-- Expected after the fix, for module StructFlowCounts: +-- 2 microflows, 1 nanoflow +-- (the rule-free module keeps the check simple; rules are a separate doctype +-- and are never counted as microflows). + +create module StructFlowCounts; + +create microflow StructFlowCounts.ACT_One () +begin + log info 'one'; +end; +/ + +create microflow StructFlowCounts.ACT_Two () +begin + log info 'two'; +end; +/ + +create nanoflow StructFlowCounts.NF_One () +begin +end; +/ + +show structure depth 1 in StructFlowCounts; diff --git a/mdl/catalog/builder_microflows.go b/mdl/catalog/builder_microflows.go index 1bbc97022..b38c42416 100644 --- a/mdl/catalog/builder_microflows.go +++ b/mdl/catalog/builder_microflows.go @@ -10,6 +10,21 @@ import ( "github.com/mendixlabs/mxcli/sdk/microflows" ) +// Values of microflows.MicroflowType — which of the three flow flavours sharing +// microflows_data a row is. Upper-case, like every other catalog type +// vocabulary (refs.SourceType, objects.ObjectType). +// +// Named because readers outside this package filter on them: `show structure` +// compared against 'microflow' / 'nanoflow' for as long as the column has held +// upper case, matched nothing, and so never showed a flow count. SQLite's `=` +// is case-sensitive; a literal that disagrees with the writer fails silently +// as an empty result, never as an error. Filter on these, not on a literal. +const ( + MicroflowTypeMicroflow = "MICROFLOW" + MicroflowTypeNanoflow = "NANOFLOW" + MicroflowTypeRule = "RULE" +) + func (b *Builder) buildMicroflows() error { // Get all microflows (cached — avoids re-parsing in later phases) mfs, err := b.cachedMicroflows() @@ -132,7 +147,7 @@ func (b *Builder) buildMicroflows() error { qualifiedName, moduleName, b.hierarchy.buildFolderPath(mf.ContainerID), // real folder path (Bug 12b class) - "MICROFLOW", + MicroflowTypeMicroflow, mf.Documentation, returnType, len(mf.Parameters), @@ -235,7 +250,7 @@ func (b *Builder) buildMicroflows() error { qualifiedName, moduleName, b.hierarchy.buildFolderPath(nf.ContainerID), // real folder path (Bug 12b class) - "NANOFLOW", + MicroflowTypeNanoflow, nf.Documentation, returnType, len(nf.Parameters), @@ -335,7 +350,7 @@ func (b *Builder) buildMicroflows() error { qualifiedName, moduleName, b.hierarchy.buildFolderPath(rule.ContainerID), - "RULE", + MicroflowTypeRule, rule.Documentation, returnType, len(rule.Parameters), diff --git a/mdl/executor/cmd_structure.go b/mdl/executor/cmd_structure.go index 5cf7f605c..4501445d8 100644 --- a/mdl/executor/cmd_structure.go +++ b/mdl/executor/cmd_structure.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/catalog" mdlerrors "github.com/mendixlabs/mxcli/mdl/errors" "github.com/mendixlabs/mxcli/mdl/types" "github.com/mendixlabs/mxcli/model" @@ -66,8 +67,8 @@ func execShowStructure(ctx *ExecContext, s *ast.ShowStmt) error { // and columns for each element type count. func structureDepth1JSON(ctx *ExecContext, modules []structureModule) error { entityCounts := queryCountByModule(ctx, "entities") - mfCounts := queryCountByModule(ctx, "microflows where MicroflowType = 'microflow'") - nfCounts := queryCountByModule(ctx, "microflows where MicroflowType = 'nanoflow'") + mfCounts := queryCountByModule(ctx, flowTypeFilter(catalog.MicroflowTypeMicroflow)) + nfCounts := queryCountByModule(ctx, flowTypeFilter(catalog.MicroflowTypeNanoflow)) pageCounts := queryCountByModule(ctx, "pages") enumCounts := queryCountByModule(ctx, "enumerations") snippetCounts := queryCountByModule(ctx, "snippets") @@ -189,8 +190,8 @@ func asString(v any) string { func structureDepth1(ctx *ExecContext, modules []structureModule) error { // Query counts per module from catalog entityCounts := queryCountByModule(ctx, "entities") - mfCounts := queryCountByModule(ctx, "microflows where MicroflowType = 'microflow'") - nfCounts := queryCountByModule(ctx, "microflows where MicroflowType = 'nanoflow'") + mfCounts := queryCountByModule(ctx, flowTypeFilter(catalog.MicroflowTypeMicroflow)) + nfCounts := queryCountByModule(ctx, flowTypeFilter(catalog.MicroflowTypeNanoflow)) pageCounts := queryCountByModule(ctx, "pages") enumCounts := queryCountByModule(ctx, "enumerations") snippetCounts := queryCountByModule(ctx, "snippets") @@ -285,6 +286,13 @@ func queryCountByModule(ctx *ExecContext, tableAndWhere string) map[string]int { return counts } +// flowTypeFilter selects one flow flavour from the microflows table, spelled +// with the catalog builder's own constant: a hand-typed lower-case literal here +// matched no row and hid every microflow and nanoflow count. +func flowTypeFilter(flowType string) string { + return fmt.Sprintf("microflows where MicroflowType = '%s'", flowType) +} + // countByModuleFromBackend counts elements per module using the backend (for types without catalog tables). func countByModuleFromBackend(ctx *ExecContext, kind string) map[string]int { counts := make(map[string]int) diff --git a/mdl/executor/cmd_structure_flow_counts_test.go b/mdl/executor/cmd_structure_flow_counts_test.go new file mode 100644 index 000000000..155a60161 --- /dev/null +++ b/mdl/executor/cmd_structure_flow_counts_test.go @@ -0,0 +1,99 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/catalog" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// flowCountsCatalog runs the REAL catalog builder over a backend holding two +// microflows, one nanoflow and one rule in module Sales. Seeding microflows_data +// by hand would only prove the reader agrees with whatever value the test typed; +// going through the builder is what detects the writer and the reader spelling +// MicroflowType differently. +func flowCountsCatalog(t *testing.T) *catalog.Catalog { + t.Helper() + const mod = model.ID("mod-sales") + be := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + // The builder dereferences project settings; the mock's nil default + // would panic before any flow is catalogued. + GetProjectSettingsFunc: func() (*model.ProjectSettings, error) { return &model.ProjectSettings{}, nil }, + ListModuleSettingsFunc: func() ([]*types.ModuleSettings, error) { return nil, nil }, + ListModulesFunc: func() ([]*model.Module, error) { + return []*model.Module{{BaseElement: model.BaseElement{ID: mod}, Name: "Sales"}}, nil + }, + ListMicroflowsFunc: func() ([]*microflows.Microflow, error) { + return []*microflows.Microflow{ + {BaseElement: model.BaseElement{ID: "mf-1"}, ContainerID: mod, Name: "ACT_One"}, + {BaseElement: model.BaseElement{ID: "mf-2"}, ContainerID: mod, Name: "ACT_Two"}, + }, nil + }, + ListNanoflowsFunc: func() ([]*microflows.Nanoflow, error) { + return []*microflows.Nanoflow{ + {BaseElement: model.BaseElement{ID: "nf-1"}, ContainerID: mod, Name: "NF_One"}, + }, nil + }, + ListRulesFunc: func() ([]*microflows.Rule, error) { + return []*microflows.Rule{ + {BaseElement: model.BaseElement{ID: "rule-1"}, ContainerID: mod, Name: "RULE_One"}, + }, nil + }, + } + cat, err := catalog.New() + if err != nil { + t.Fatalf("catalog.New: %v", err) + } + t.Cleanup(func() { cat.Close() }) + if err := catalog.NewBuilder(cat, be).Build(nil); err != nil { + t.Fatalf("catalog build: %v", err) + } + return cat +} + +// `show structure depth 1` never listed microflow or nanoflow counts: it +// filtered on MicroflowType = 'microflow' / 'nanoflow' while the catalog +// builder stores 'MICROFLOW' / 'NANOFLOW', so both queries matched nothing and +// the zero counts were silently omitted from the summary line. +func TestStructureDepth1CountsFlowsFromBuiltCatalog(t *testing.T) { + cat := flowCountsCatalog(t) + ctx, buf := newMockCtx(t) + ctx.Catalog = cat + + mods := []structureModule{{Name: "Sales", ID: "mod-sales"}} + if err := structureDepth1(ctx, mods); err != nil { + t.Fatalf("structureDepth1: %v", err) + } + out := buf.String() + // The rule is its own doctype: it must not be counted as a microflow. + for _, want := range []string{"2 microflows", "1 nanoflow"} { + if !strings.Contains(out, want) { + t.Errorf("depth-1 summary lacks %q; got:\n%s", want, out) + } + } +} + +func TestStructureDepth1JSONCountsFlowsFromBuiltCatalog(t *testing.T) { + cat := flowCountsCatalog(t) + ctx, buf := newMockCtx(t) + ctx.Catalog = cat + ctx.Format = FormatJSON + + mods := []structureModule{{Name: "Sales", ID: "mod-sales"}} + if err := structureDepth1JSON(ctx, mods); err != nil { + t.Fatalf("structureDepth1JSON: %v", err) + } + out := buf.String() + for _, want := range []string{`"Microflows": 2`, `"Nanoflows": 1`} { + if !strings.Contains(out, want) { + t.Errorf("JSON structure lacks %s; got:\n%s", want, out) + } + } +} diff --git a/mdl/linter/context.go b/mdl/linter/context.go index e27ef4fbf..13b9d05f6 100644 --- a/mdl/linter/context.go +++ b/mdl/linter/context.go @@ -615,9 +615,9 @@ type Microflow struct { // with an imprecise noun than not at all. func (m Microflow) DocumentNoun() string { switch m.MicroflowType { - case "NANOFLOW": + case catalog.MicroflowTypeNanoflow: return "nanoflow" - case "RULE": + case catalog.MicroflowTypeRule: return "rule" default: return "microflow" From 11ff6f204e02f34454cd2590f7dbe1e9930d9020 Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 26 Sep 2026 20:10:34 +0000 Subject: [PATCH 4/9] fix(catalog): reference edges to attributes, enumerations, workflows and mapped entities The refs graph stopped at documents. `impact Module.Entity.Attr` answered "not referenced" for an attribute a microflow writes and a page displays (Evora: DigitalTwin.Machine.NumberOfIncidents), an enumeration had no inbound edge at all, a workflow started only by a microflow had no caller, a page navigating an association and a mapping mapping an entity were invisible. - A raw-document walk over microflows, nanoflows, rules, pages, snippets, workflows and import/export mappings matches every string value against the names the model declares: whole-string matches are structured references (MemberChange.Attribute, AttributeRef.Attribute, EntityRefStep.Association, EnumerationType.Enumeration, ObjectMappingElement.Entity); inside expressions, association paths and qualified enumeration values. New kinds: member, type, value, mapping. - XPath constraints resolve bare attribute names against their target entity (and its generalizations), association paths, and enum attributes compared to a literal (kind xpath). Page/snippet constraints had no target entity because resolveEntityRefFromBSON read a key no stored EntityRef carries. - Entities -> enumerations from attribute types (kind type). - WorkflowCallAction -> WORKFLOW (kind call). - New types ATTRIBUTE, ENUMERATION, ENUMERATION_VALUE, IMPORT_MAPPING, EXPORT_MAPPING published in the lint-rule vocabulary; members kept off the graph_god_nodes asset side; CatalogSchemaVersion 15. Not covered: a bare member named through a variable in a free-text expression ($Order/Total), whose type is not known to the catalog. Co-Authored-By: Claude Opus 5.5 --- .../skills/mendix/write-lint-rules/SKILL.md | 8 +- mdl/catalog/builder.go | 11 +- mdl/catalog/builder_member_refs.go | 516 ++++++++++++++++++ mdl/catalog/builder_member_refs_test.go | 373 +++++++++++++ mdl/catalog/builder_references.go | 32 ++ mdl/catalog/builder_references_test.go | 7 + mdl/catalog/builder_xpath.go | 49 +- mdl/catalog/builder_xpath_test.go | 38 ++ mdl/catalog/catalogdb.go | 4 + mdl/catalog/lint_rule_doc_vocabulary_test.go | 1 + mdl/catalog/tables.go | 15 +- 11 files changed, 1030 insertions(+), 24 deletions(-) create mode 100644 mdl/catalog/builder_member_refs.go create mode 100644 mdl/catalog/builder_member_refs_test.go diff --git a/.claude/skills/mendix/write-lint-rules/SKILL.md b/.claude/skills/mendix/write-lint-rules/SKILL.md index 4f2457759..dbc6f5c92 100644 --- a/.claude/skills/mendix/write-lint-rules/SKILL.md +++ b/.claude/skills/mendix/write-lint-rules/SKILL.md @@ -515,13 +515,13 @@ Returned by `permissions()` (all types) or `permissions_for()` (entity-specific) ### reference | Property | Type | Example | |----------|------|---------| -| `source_type` | string | The document the edge comes FROM, upper-case: `"MICROFLOW"`, `"NANOFLOW"`, `"RULE"`, `"PAGE"`, `"SNIPPET"`, `"ENTITY"`, `"ASSOCIATION"`, `"WORKFLOW"`, `"NAVIGATION"`, `"SCHEDULED_EVENT"`, `"PUBLISHED_REST_OPERATION"`, `"PROJECT_SETTINGS"` | +| `source_type` | string | The document the edge comes FROM, upper-case: `"MICROFLOW"`, `"NANOFLOW"`, `"RULE"`, `"PAGE"`, `"SNIPPET"`, `"ENTITY"`, `"ASSOCIATION"`, `"WORKFLOW"`, `"NAVIGATION"`, `"SCHEDULED_EVENT"`, `"PUBLISHED_REST_OPERATION"`, `"PROJECT_SETTINGS"`, `"IMPORT_MAPPING"`, `"EXPORT_MAPPING"` | | `source_id` | string | Source UUID | | `source_name` | string | `"Sales.ACT_Customer_Create"` | -| `target_type` | string | What it points AT, upper-case: `"ENTITY"`, `"ASSOCIATION"`, `"MICROFLOW"`, `"NANOFLOW"`, `"RULE"`, `"PAGE"`, `"LAYOUT"`, `"WORKFLOW"`, `"WIDGET"`, `"JAVA_ACTION"`, `"REST_OPERATION"`, `"REGULAR_EXPRESSION"`. `LAYOUT` and `WIDGET` are only ever targets; `SCHEDULED_EVENT` and `PROJECT_SETTINGS` only ever sources | +| `target_type` | string | What it points AT, upper-case: `"ENTITY"`, `"ASSOCIATION"`, `"MICROFLOW"`, `"NANOFLOW"`, `"RULE"`, `"PAGE"`, `"LAYOUT"`, `"WORKFLOW"`, `"WIDGET"`, `"JAVA_ACTION"`, `"REST_OPERATION"`, `"REGULAR_EXPRESSION"`, `"ATTRIBUTE"`, `"ENUMERATION"`, `"ENUMERATION_VALUE"`. `LAYOUT`, `WIDGET`, `ATTRIBUTE`, `ENUMERATION` and `ENUMERATION_VALUE` are only ever targets; `SCHEDULED_EVENT` and `PROJECT_SETTINGS` only ever sources | | `target_id` | string | Target UUID | -| `target_name` | string | `"Sales.Customer"` | -| `ref_kind` | string | How it references: `"call"`, `"create"`, `"retrieve"`, `"change"`, `"delete"`, `"show_page"`, `"datasource"`, `"action"`, `"layout"`, `"parameter"`, `"return"`, `"generalize"`, `"associate"`, `"home_page"`, `"login_page"`, `"menu_item"`, `"calculate"`, `"schedule"`, `"validate"`, `"settings"`, `"widget"`, `"sync"`, `"publish"`, `"event"` — lower-case, unlike the types above | +| `target_name` | string | `"Sales.Customer"`; three-part for an attribute or an enumeration value: `"Sales.Order.Total"`, `"Sales.OrderStatus.Open"` | +| `ref_kind` | string | How it references: `"call"`, `"create"`, `"retrieve"`, `"change"`, `"delete"`, `"show_page"`, `"datasource"`, `"action"`, `"layout"`, `"parameter"`, `"return"`, `"generalize"`, `"associate"`, `"home_page"`, `"login_page"`, `"menu_item"`, `"calculate"`, `"schedule"`, `"validate"`, `"settings"`, `"widget"`, `"sync"`, `"publish"`, `"event"`, `"member"` (binds/reads/writes an attribute or navigates an association), `"xpath"` (an XPath constraint names it), `"type"` (typed as an enumeration), `"value"` (an expression names an enumeration value), `"mapping"` (an import/export mapping maps the entity) — lower-case, unlike the types above. Attribute names used only through a variable in a free-text expression (`$Order/Total`) have no edge | | `module_name` | string | Source module | ### project_security diff --git a/mdl/catalog/builder.go b/mdl/catalog/builder.go index 7e1ee6f6f..a6ff64162 100644 --- a/mdl/catalog/builder.go +++ b/mdl/catalog/builder.go @@ -580,6 +580,12 @@ func (b *Builder) Build(progress ProgressFunc) error { return fmt.Errorf("failed to build export mappings: %w", err) } + // Build XPath expressions table (full mode only). Before buildReferences, + // which resolves the attributes and associations each constraint names. + if err := b.buildXPathExpressions(); err != nil { + return fmt.Errorf("failed to build xpath expressions: %w", err) + } + // Build cross-references (only in full mode) if err := b.buildReferences(); err != nil { return fmt.Errorf("failed to build references: %w", err) @@ -590,11 +596,6 @@ func (b *Builder) Build(progress ProgressFunc) error { return fmt.Errorf("failed to build permissions: %w", err) } - // Build XPath expressions table (full mode only) - if err := b.buildXPathExpressions(); err != nil { - return fmt.Errorf("failed to build xpath expressions: %w", err) - } - // Build strings FTS table (full mode only) if err := b.buildStrings(); err != nil { return fmt.Errorf("failed to build strings: %w", err) diff --git a/mdl/catalog/builder_member_refs.go b/mdl/catalog/builder_member_refs.go new file mode 100644 index 000000000..fb2398af3 --- /dev/null +++ b/mdl/catalog/builder_member_refs.go @@ -0,0 +1,516 @@ +// SPDX-License-Identifier: Apache-2.0 + +package catalog + +import ( + "database/sql" + "sort" + "strings" + + "go.mongodb.org/mongo-driver/v2/bson" +) + +// Member references: the edges from a document to an ATTRIBUTE, an +// ASSOCIATION it navigates, an ENUMERATION it types something as, or an +// ENUMERATION_VALUE it names. +// +// Until these existed the reference graph stopped at documents, so +// `impact Module.Entity.Attr` answered "not referenced" for an attribute that a +// microflow writes and a page displays (Evora Factory Management: +// DigitalTwin.Machine.NumberOfIncidents). A missing edge is a wrong answer, not +// a missing feature — the tool states it confidently and the caller deletes. +// +// The sites are found by walking the RAW document, not the typed readers. A +// typed walk reaches the sites someone wrote a case for — change members, +// retrieve sorting — and silently misses the rest (aggregate-by-attribute, +// list operations, text template parameters, conditional visibility, pluggable +// widget attribute properties, mapping elements …). Every one of those stores +// the member by its fully qualified name, so matching every string value +// against the set of names the model actually declares reaches all of them with +// no per-type code. The match is on the WHOLE string (or on a whole path token +// inside an expression), against names that exist, so an unrelated string +// cannot produce an edge. +// +// What this cannot see: an attribute named only by its bare name through a +// variable in an expression (`$Order/Total`), because resolving `$Order` needs +// the variable's type. XPath is different — its context entity is known — and +// is handled by xpathRefs below. + +// memberRefSourceTypes maps the unit types whose documents are walked to the +// catalog object type recorded as refs.SourceType. It is a closed list: the +// SourceType vocabulary is documented to lint-rule authors, and walking every +// unit would put values there (PAGE_TEMPLATE, BUILDING_BLOCK) that name +// design-time templates rather than anything that runs. Domain models are +// absent on purpose: an entity's own access rules and indexes name its members, +// and those are the entity describing itself, not another document using it. +var memberRefSourceTypes = map[string]string{ + "Microflows$Microflow": RefObjectMicroflow, + "Microflows$Nanoflow": RefObjectNanoflow, + "Microflows$Rule": RefObjectRule, + "Forms$Page": RefObjectPage, + "Forms$Snippet": RefObjectSnippet, + "Workflows$Workflow": RefObjectWorkflow, + "ImportMappings$ImportMapping": RefObjectImportMapping, + "ExportMappings$ExportMapping": RefObjectExportMapping, +} + +// memberRefSkipKeys are string properties that hold prose or a document's own +// name, never a reference. XPath constraints are skipped too: xpathRefs reads +// them with their context entity, which resolves bare attribute names the raw +// walk cannot. +var memberRefSkipKeys = map[string]bool{ + "Name": true, + "Documentation": true, + "XPathConstraint": true, + "XpathConstraint": true, +} + +// memberRefIndex is the set of names the model declares, which is what makes a +// string a reference rather than text. +type memberRefIndex struct { + attributes map[string]string // "Mod.Entity.Attr" -> enumeration QN, or "" when not an enumeration + associations map[string]bool + enumerations map[string]bool + enumValues map[string]bool // "Mod.Enum.Value" + entities map[string]bool + generalization map[string]string // entity -> the entity it specializes +} + +// loadMemberRefIndex reads the name sets from the tables buildEntities, +// buildAssociations and buildEnumerations filled earlier in the transaction. +func (b *Builder) loadMemberRefIndex() *memberRefIndex { + idx := &memberRefIndex{ + attributes: map[string]string{}, + associations: map[string]bool{}, + enumerations: map[string]bool{}, + enumValues: map[string]bool{}, + entities: map[string]bool{}, + generalization: map[string]string{}, + } + scan := func(query string, fn func(a, b string)) { + rows, err := b.tx.Query(query) + if err != nil { + return + } + defer rows.Close() + for rows.Next() { + var a, c sql.NullString + if rows.Scan(&a, &c) == nil && a.String != "" { + fn(a.String, c.String) + } + } + } + scan(`SELECT EntityQualifiedName || '.' || Name, COALESCE(EnumerationQualifiedName, '') FROM attributes_data`, + func(qn, enum string) { idx.attributes[qn] = enum }) + scan(`SELECT QualifiedName, '' FROM associations_data`, + func(qn, _ string) { idx.associations[qn] = true }) + scan(`SELECT QualifiedName, '' FROM enumerations_data`, + func(qn, _ string) { idx.enumerations[qn] = true }) + scan(`SELECT EnumerationQualifiedName || '.' || Name, '' FROM enumeration_values_data`, + func(qn, _ string) { idx.enumValues[qn] = true }) + scan(`SELECT QualifiedName, COALESCE(Generalization, '') FROM entities_data`, + func(qn, gen string) { + idx.entities[qn] = true + if gen != "" { + idx.generalization[qn] = gen + } + }) + return idx +} + +// resolveAttribute finds the attribute `name` on entity, or on the entity it +// specializes: an XPath over a specialization names inherited attributes +// bare, and the attribute's qualified name is the generalization's. +func (idx *memberRefIndex) resolveAttribute(entity, name string) (string, bool) { + seen := map[string]bool{} + for e := entity; e != "" && !seen[e]; e = idx.generalization[e] { + seen[e] = true + if _, ok := idx.attributes[e+"."+name]; ok { + return e + "." + name, true + } + } + return "", false +} + +// refEdge is one outbound edge found in a document. +type refEdge struct { + TargetType, TargetName, RefKind string +} + +// edgeSet collects edges once each, in a deterministic order. +type edgeSet struct { + seen map[refEdge]bool + edges []refEdge +} + +func (s *edgeSet) add(targetType, targetName, kind string) { + e := refEdge{targetType, targetName, kind} + if s.seen == nil { + s.seen = map[refEdge]bool{} + } + if !s.seen[e] { + s.seen[e] = true + s.edges = append(s.edges, e) + } +} + +func (s *edgeSet) sorted() []refEdge { + sort.Slice(s.edges, func(i, j int) bool { + a, b := s.edges[i], s.edges[j] + if a.TargetType != b.TargetType { + return a.TargetType < b.TargetType + } + if a.TargetName != b.TargetName { + return a.TargetName < b.TargetName + } + return a.RefKind < b.RefKind + }) + return s.edges +} + +// memberRefsInUnit returns the member, enumeration and (for mappings) entity +// edges one stored document makes. +func memberRefsInUnit(contents []byte, sourceType string, idx *memberRefIndex) []refEdge { + var doc bson.D + if err := bson.Unmarshal(contents, &doc); err != nil { + return nil + } + isMapping := sourceType == RefObjectImportMapping || sourceType == RefObjectExportMapping + var set edgeSet + walkBSONStrings(doc, "", func(key, v string) { + if memberRefSkipKeys[key] || v == "" { + return + } + // A whole-string match is a structured reference: MemberChange.Attribute, + // AttributeRef.Attribute, EntityRefStep.Association, EnumerationType.Enumeration, + // ObjectMappingElement.Entity. + switch { + case hasKey(idx.attributes, v): + set.add(RefObjectAttribute, v, RefKindMember) + return + case idx.associations[v]: + set.add(RefObjectAssociation, v, RefKindMember) + return + case idx.enumerations[v]: + set.add(RefObjectEnumeration, v, RefKindType) + return + case idx.enumValues[v]: + set.add(RefObjectEnumerationValue, v, RefKindValue) + return + case isMapping && idx.entities[v]: + set.add(RefObjectEntity, v, RefKindMapping) + return + } + // Otherwise it may be an expression: enumeration values are always + // written qualified, and an association path names its members. + if strings.ContainsAny(v, "./") { + scanPaths(v, "", idx, func(targetType, name string) { + kind := RefKindMember + if targetType == RefObjectEnumerationValue { + kind = RefKindValue + } + set.add(targetType, name, kind) + }) + } + }) + return set.sorted() +} + +func hasKey(m map[string]string, k string) bool { + _, ok := m[k] + return ok +} + +// walkBSONStrings calls fn for every string property value in a decoded +// document, with the property's key. Arrays of strings are visited with the +// array's key. +func walkBSONStrings(v any, key string, fn func(key, value string)) { + switch t := v.(type) { + case string: + fn(key, t) + case bson.D: + for _, e := range t { + walkBSONStrings(e.Value, e.Key, fn) + } + case bson.M: + for k, x := range t { + walkBSONStrings(x, k, fn) + } + case map[string]any: + for k, x := range t { + walkBSONStrings(x, k, fn) + } + case bson.A: + for _, x := range t { + walkBSONStrings(x, key, fn) + } + case []any: + for _, x := range t { + walkBSONStrings(x, key, fn) + } + } +} + +// xpathKeywords are XPath/expression words that can stand where an attribute +// name does and must never resolve to one. +var xpathKeywords = map[string]bool{ + "and": true, "or": true, "not": true, "true": true, "false": true, + "empty": true, "div": true, "mod": true, "if": true, "then": true, "else": true, +} + +// scanPaths finds the members an XPath constraint or an expression names. +// +// context is the entity bare names are resolved against — the retrieved entity +// for an XPath, "" for an expression (where a bare name hangs off a variable +// whose type is not known here). Inside a path, an entity segment becomes the +// context for the next one, and a predicate `[...]` after a path is evaluated +// against the path's last entity, so `Mod.Assoc/Mod.Other[Name = 'x']` resolves +// Name on Mod.Other. +// +// An enumeration attribute compared to a string literal (`Status = 'Open'`) +// names that enumeration value: XPath spells values as their bare name in +// quotes, so this is the only place an XPath reference to a value is visible. +func scanPaths(text, context string, idx *memberRefIndex, emit func(targetType, name string)) { + stack := []string{context} + top := func() string { return stack[len(stack)-1] } + lastPathEntity := "" + pendingEnum := "" // enumeration of the attribute just seen, until a comparison literal or anything else + i := 0 + for i < len(text) { + c := text[i] + switch { + case c == '\'': + // String literal with '' escaping. + j := i + 1 + var lit strings.Builder + for j < len(text) { + if text[j] == '\'' { + if j+1 < len(text) && text[j+1] == '\'' { + lit.WriteByte('\'') + j += 2 + continue + } + break + } + lit.WriteByte(text[j]) + j++ + } + if pendingEnum != "" && idx.enumValues[pendingEnum+"."+lit.String()] { + emit(RefObjectEnumerationValue, pendingEnum+"."+lit.String()) + } + pendingEnum = "" + lastPathEntity = "" + i = j + 1 + case c == '%': + // XPath token such as '%CurrentDateTime%' outside quotes. + j := strings.IndexByte(text[i+1:], '%') + if j < 0 { + return + } + pendingEnum = "" + i += j + 2 + case c == '[': + ctx := top() + if lastPathEntity != "" { + ctx = lastPathEntity + } + stack = append(stack, ctx) + lastPathEntity = "" + pendingEnum = "" + i++ + case c == ']': + if len(stack) > 1 { + stack = stack[:len(stack)-1] + } + lastPathEntity = "" + pendingEnum = "" + i++ + case isIdentChar(c) || c == '$' || c == '.' || c == '/': + j := i + for j < len(text) && (isIdentChar(text[j]) || text[j] == '$' || text[j] == '.' || text[j] == '/') { + j++ + } + chunk := text[i:j] + i = j + // A function name (`contains(`, `toString(`) is not a member. + k := j + for k < len(text) && text[k] == ' ' { + k++ + } + if k < len(text) && text[k] == '(' { + pendingEnum = "" + lastPathEntity = "" + continue + } + pendingEnum, lastPathEntity = resolvePath(chunk, top(), idx, emit) + case c == ' ' || c == '\t' || c == '\n' || c == '\r' || c == '=' || c == '!': + i++ + default: + pendingEnum = "" + lastPathEntity = "" + i++ + } + } +} + +// resolvePath resolves one navigation path such as +// `Mod.Assoc/Mod.Entity/Attr`, `$var/Mod.Assoc/Mod.Entity/Attr` or `Attr`, +// emitting the associations, attributes and enumeration values it names. It +// returns the enumeration of an attribute that ends the path (for a following +// `= 'Value'`) and the entity the path ends on (for a following predicate). +func resolvePath(chunk, context string, idx *memberRefIndex, emit func(targetType, name string)) (pendingEnum, endEntity string) { + cur := context + segs := strings.Split(chunk, "/") + for n, seg := range segs { + pendingEnum, endEntity = "", "" + switch { + case seg == "": + cur = "" + case strings.HasPrefix(seg, "$"): + // A variable: its type is not known here, so what hangs off it + // directly cannot be resolved. Qualified segments after it still can. + cur = "" + case idx.associations[seg]: + emit(RefObjectAssociation, seg) + cur = "" + case idx.entities[seg]: + cur = seg + endEntity = seg + case idx.enumValues[seg]: + emit(RefObjectEnumerationValue, seg) + cur = "" + case !strings.Contains(seg, ".") && cur != "" && !xpathKeywords[seg]: + if attr, ok := idx.resolveAttribute(cur, seg); ok { + emit(RefObjectAttribute, attr) + if n == len(segs)-1 { + pendingEnum = idx.attributes[attr] + } + } + cur = "" + default: + cur = "" + } + } + return pendingEnum, endEntity +} + +// extractMemberRefs walks every document of the types in memberRefSourceTypes +// and emits its member, enumeration and mapping-entity edges. +func (b *Builder) extractMemberRefs(stmt *sql.Stmt, idx *memberRefIndex, projectID, snapshotID string) int { + unitTypes := make([]string, 0, len(memberRefSourceTypes)) + for t := range memberRefSourceTypes { + unitTypes = append(unitTypes, t) + } + sort.Strings(unitTypes) + + count := 0 + for _, unitType := range unitTypes { + sourceType := memberRefSourceTypes[unitType] + units, err := b.reader.ListRawUnitsByType(unitType) + if err != nil { + continue + } + for _, u := range units { + // ListRawUnitsByType matches a PREFIX: Forms$Page also returns + // Forms$PageTemplate units, which are not pages. + if u.Type != unitType || len(u.Contents) == 0 { + continue + } + var named struct { + Name string `bson:"Name"` + } + if bson.Unmarshal(u.Contents, &named) != nil || named.Name == "" { + continue + } + moduleName := b.hierarchy.getModuleName(b.hierarchy.findModuleID(u.ContainerID)) + sourceQN := moduleName + "." + named.Name + for _, e := range memberRefsInUnit(u.Contents, sourceType, idx) { + if _, err := stmt.Exec(sourceType, string(u.ID), sourceQN, + e.TargetType, "", e.TargetName, + e.RefKind, moduleName, projectID, snapshotID); err == nil { + count++ + } + } + } + } + return count +} + +// extractEnumerationTypeRefs emits one `type` edge from every entity to each +// enumeration one of its attributes is typed as. Without it an enumeration had +// no inbound edge at all, and `impact` on it answered "not referenced". +func (b *Builder) extractEnumerationTypeRefs(projectID, snapshotID string) int { + res, err := b.tx.Exec( + `INSERT INTO refs (SourceType, SourceId, SourceName, TargetType, TargetId, TargetName, RefKind, ModuleName, ProjectId, SnapshotId) + SELECT DISTINCT ?, COALESCE(EntityId, ''), EntityQualifiedName, ?, '', EnumerationQualifiedName, ?, ModuleName, ?, ? + FROM attributes_data + WHERE EnumerationQualifiedName IS NOT NULL AND EnumerationQualifiedName != ''`, + RefObjectEntity, RefObjectEnumeration, RefKindType, projectID, snapshotID) + if err != nil { + return 0 + } + n, _ := res.RowsAffected() + return int(n) +} + +// xpathSourceTypes maps xpath_expressions_data.DocumentType to refs.SourceType. +// An access rule's constraint is recorded against the domain model with the +// entity as its qualified name; the entity is what depends on the attribute. +var xpathSourceTypes = map[string]string{ + "MICROFLOW": RefObjectMicroflow, + "NANOFLOW": RefObjectNanoflow, + "PAGE": RefObjectPage, + "SNIPPET": RefObjectSnippet, + "DOMAIN_MODEL": RefObjectEntity, +} + +// extractXPathRefs emits `xpath` edges for the attributes, associations and +// enumeration values each recorded XPath constraint names, resolved against the +// constraint's target entity. buildXPathExpressions runs before buildReferences +// so the table is populated. +func (b *Builder) extractXPathRefs(stmt *sql.Stmt, idx *memberRefIndex, projectID, snapshotID string) int { + rows, err := b.tx.Query(`SELECT DocumentType, DocumentId, DocumentQualifiedName, COALESCE(TargetEntity, ''), + XPathExpression, COALESCE(ModuleName, '') FROM xpath_expressions_data ORDER BY Id`) + if err != nil { + return 0 + } + type xp struct{ docType, docID, docQN, target, xpath, module string } + var all []xp + for rows.Next() { + var r xp + if rows.Scan(&r.docType, &r.docID, &r.docQN, &r.target, &r.xpath, &r.module) == nil { + all = append(all, r) + } + } + rows.Close() + + count := 0 + written := map[string]bool{} + for _, r := range all { + sourceType, ok := xpathSourceTypes[r.docType] + if !ok { + continue + } + sourceID := r.docID + if r.docType == "DOMAIN_MODEL" { + sourceID = "" + } + var set edgeSet + scanPaths(r.xpath, r.target, idx, func(targetType, name string) { + set.add(targetType, name, RefKindXPath) + }) + for _, e := range set.sorted() { + key := sourceType + "\x00" + r.docQN + "\x00" + e.TargetType + "\x00" + e.TargetName + if written[key] { + continue + } + written[key] = true + if _, err := stmt.Exec(sourceType, sourceID, r.docQN, + e.TargetType, "", e.TargetName, + e.RefKind, r.module, projectID, snapshotID); err == nil { + count++ + } + } + } + return count +} diff --git a/mdl/catalog/builder_member_refs_test.go b/mdl/catalog/builder_member_refs_test.go new file mode 100644 index 000000000..aa06ffce9 --- /dev/null +++ b/mdl/catalog/builder_member_refs_test.go @@ -0,0 +1,373 @@ +// SPDX-License-Identifier: Apache-2.0 + +package catalog + +import ( + "strings" + "testing" + + "go.mongodb.org/mongo-driver/v2/bson" + + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// The reference graph had no ATTRIBUTE, ENUMERATION or ENUMERATION_VALUE +// targets at all, no microflow -> workflow edge, no page -> association edge and +// no mapping -> entity edge. `impact Module.Entity.Attr` therefore answered +// "(no impact - element is not referenced)" for an attribute a microflow writes +// and a page displays — measured on Evora Factory Management, where +// DigitalTwin.Machine.NumberOfIncidents is set by a change activity and bound on +// DigitalTwin.Machine_Details. An agent acting on that answer deletes a live +// attribute. +// +// The fixture runs the two real passes over documents shaped like the stored +// BSON (key names checked against Evora's units): buildXPathExpressions, then +// buildReferences, and reads what landed in refs. + +const memberRefsModuleID = model.ID("mod-shop") + +func memberRefsFixture(t *testing.T) *Catalog { + t.Helper() + + cat, err := New() + if err != nil { + t.Fatalf("new catalog: %v", err) + } + t.Cleanup(func() { cat.Close() }) + + tx, err := cat.db.Begin() + if err != nil { + t.Fatalf("begin: %v", err) + } + + // The tables buildReferences reads its name sets from. buildEntities, + // buildAssociations and buildEnumerations fill them in a real build. + seed := []string{ + `INSERT INTO entities_data (Id, Name, QualifiedName, ModuleName, Generalization) VALUES + ('e1', 'Order', 'Shop.Order', 'Shop', ''), + ('e2', 'Customer', 'Shop.Customer', 'Shop', ''), + ('e3', 'SpecialOrder', 'Shop.SpecialOrder', 'Shop', 'Shop.Order')`, + `INSERT INTO attributes_data (Id, Name, EntityId, EntityQualifiedName, ModuleName, DataType, EnumerationQualifiedName) VALUES + ('a1', 'Status', 'e1', 'Shop.Order', 'Shop', 'Enumeration', 'Shop.OrderStatus'), + ('a2', 'Total', 'e1', 'Shop.Order', 'Shop', 'Decimal', ''), + ('a3', 'Code', 'e1', 'Shop.Order', 'Shop', 'String', ''), + ('a4', 'Name', 'e2', 'Shop.Customer', 'Shop', 'String', ''), + ('a5', 'Unused', 'e2', 'Shop.Customer', 'Shop', 'String', '')`, + `INSERT INTO associations_data (Id, Name, QualifiedName, ModuleName, FromEntity, ToEntity) VALUES + ('as1', 'Order_Customer', 'Shop.Order_Customer', 'Shop', 'Shop.Order', 'Shop.Customer')`, + `INSERT INTO enumerations_data (Id, Name, QualifiedName, ModuleName) VALUES + ('en1', 'OrderStatus', 'Shop.OrderStatus', 'Shop')`, + `INSERT INTO enumeration_values_data (Id, EnumerationId, EnumerationQualifiedName, ModuleName, Name) VALUES + ('v1', 'en1', 'Shop.OrderStatus', 'Shop', 'Open'), + ('v2', 'en1', 'Shop.OrderStatus', 'Shop', 'Closed')`, + } + for _, s := range seed { + if _, err := tx.Exec(s); err != nil { + t.Fatalf("seed: %v\n%s", err, s) + } + } + + // The typed microflow: a call-workflow activity and two XPath retrieves. + mf := µflows.Microflow{ + BaseElement: model.BaseElement{ID: "mf-1"}, + ContainerID: memberRefsModuleID, + Name: "ACT_Start", + ObjectCollection: µflows.MicroflowObjectCollection{Objects: []microflows.MicroflowObject{ + newAction("act-wf", µflows.WorkflowCallAction{Workflow: "Shop.WF_Approve"}), + newAction("act-r1", µflows.RetrieveAction{Source: µflows.DatabaseRetrieveSource{ + EntityQualifiedName: "Shop.Order", + XPathConstraint: "[Shop.Order_Customer/Shop.Customer/Name = $n and Status = 'Open' and Code != empty]", + }}), + newAction("act-r2", µflows.RetrieveAction{Source: µflows.DatabaseRetrieveSource{ + EntityQualifiedName: "Shop.SpecialOrder", + XPathConstraint: "[Total > 5]", + }}), + }}, + } + + // The same microflow as stored: the member refs are read from the raw + // document, so a site no typed struct models is still covered. + mfRaw := bson.D{ + {Key: "$Type", Value: "Microflows$Microflow"}, + {Key: "Name", Value: "ACT_Start"}, + // Prose is not a reference: a name mentioned here must not become an edge. + {Key: "Documentation", Value: "Shop.Order.Code"}, + {Key: "ObjectCollection", Value: bson.D{ + {Key: "$Type", Value: "Microflows$MicroflowObjectCollection"}, + {Key: "Objects", Value: bson.A{int32(3), + bson.D{ + {Key: "$Type", Value: "Microflows$ActionActivity"}, + {Key: "Action", Value: bson.D{ + {Key: "$Type", Value: "Microflows$ChangeAction"}, + {Key: "Items", Value: bson.A{int32(3), + bson.D{ + {Key: "$Type", Value: "Microflows$MemberChange"}, + {Key: "Association", Value: ""}, + {Key: "Attribute", Value: "Shop.Order.Total"}, + {Key: "Value", Value: "$Total"}, + }, + bson.D{ + {Key: "$Type", Value: "Microflows$MemberChange"}, + {Key: "Association", Value: "Shop.Order_Customer"}, + {Key: "Attribute", Value: ""}, + {Key: "Value", Value: "$Customer"}, + }, + }}, + }}, + }, + bson.D{ + {Key: "$Type", Value: "Microflows$ExclusiveSplit"}, + {Key: "SplitCondition", Value: bson.D{ + {Key: "$Type", Value: "Microflows$ExpressionSplitCondition"}, + {Key: "Expression", Value: "$Order/Status = Shop.OrderStatus.Closed"}, + }}, + }, + bson.D{ + {Key: "$Type", Value: "Microflows$ActionActivity"}, + {Key: "Action", Value: bson.D{ + {Key: "$Type", Value: "Microflows$CreateVariableAction"}, + {Key: "VariableType", Value: bson.D{ + {Key: "$Type", Value: "DataTypes$EnumerationType"}, + {Key: "Enumeration", Value: "Shop.OrderStatus"}, + }}, + }}, + }, + }}, + }}, + } + + // A page binding an attribute in a text template and navigating an + // association in a data source — neither is a widget's own AttributeRef. + pageRaw := bson.D{ + {Key: "$Type", Value: "Forms$Page"}, + {Key: "Name", Value: "Order_Edit"}, + {Key: "Widgets", Value: bson.A{int32(3), + bson.D{ + {Key: "$Type", Value: "Forms$DynamicText"}, + {Key: "Content", Value: bson.D{ + {Key: "$Type", Value: "Forms$ClientTemplate"}, + {Key: "Parameters", Value: bson.A{int32(2), + bson.D{ + {Key: "$Type", Value: "Forms$ClientTemplateParameter"}, + {Key: "AttributeRef", Value: bson.D{ + {Key: "$Type", Value: "DomainModels$AttributeRef"}, + {Key: "Attribute", Value: "Shop.Order.Code"}, + }}, + }, + }}, + }}, + }, + bson.D{ + {Key: "$Type", Value: "Forms$ListView"}, + {Key: "DataSource", Value: bson.D{ + {Key: "$Type", Value: "Forms$AssociationSource"}, + {Key: "EntityRef", Value: bson.D{ + {Key: "$Type", Value: "DomainModels$IndirectEntityRef"}, + {Key: "Steps", Value: bson.A{int32(2), + bson.D{ + {Key: "$Type", Value: "DomainModels$EntityRefStep"}, + {Key: "Association", Value: "Shop.Order_Customer"}, + {Key: "DestinationEntity", Value: "Shop.Customer"}, + }, + }}, + }}, + }}, + }, + }}, + } + + mappingRaw := bson.D{ + {Key: "$Type", Value: "ImportMappings$ImportMapping"}, + {Key: "Name", Value: "IMM_Order"}, + {Key: "Elements", Value: bson.A{int32(2), + bson.D{ + {Key: "$Type", Value: "ImportMappings$ObjectMappingElement"}, + {Key: "Entity", Value: "Shop.Order"}, + {Key: "Children", Value: bson.A{int32(2), + bson.D{ + {Key: "$Type", Value: "ImportMappings$ValueMappingElement"}, + {Key: "Attribute", Value: "Shop.Order.Total"}, + }, + }}, + }, + }}, + } + + rawUnit := func(id, typ string, doc bson.D) *types.RawUnit { + b, err := bson.Marshal(doc) + if err != nil { + t.Fatalf("marshal %s: %v", id, err) + } + return &types.RawUnit{ID: model.ID(id), ContainerID: memberRefsModuleID, Type: typ, Contents: b} + } + units := []*types.RawUnit{ + rawUnit("mf-1", "Microflows$Microflow", mfRaw), + rawUnit("pg-1", "Forms$Page", pageRaw), + rawUnit("im-1", "ImportMappings$ImportMapping", mappingRaw), + } + + b := &Builder{ + catalog: cat, + reader: &mock.MockBackend{ + ListMicroflowsFunc: func() ([]*microflows.Microflow, error) { + return []*microflows.Microflow{mf}, nil + }, + ListRawUnitsByTypeFunc: func(prefix string) ([]*types.RawUnit, error) { + var out []*types.RawUnit + for _, u := range units { + if prefix == "" || u.Type == prefix || len(u.Type) >= len(prefix) && u.Type[:len(prefix)] == prefix { + out = append(out, u) + } + } + return out, nil + }, + GetNavigationFunc: func() (*types.NavigationDocument, error) { + return &types.NavigationDocument{}, nil + }, + }, + snapshot: &Snapshot{ID: "snap-1"}, + hierarchy: &hierarchy{ + moduleIDs: map[model.ID]bool{memberRefsModuleID: true}, + moduleNames: map[model.ID]string{memberRefsModuleID: "Shop"}, + containerParent: map[model.ID]model.ID{}, + folderNames: map[model.ID]string{}, + }, + tx: tx, + fullMode: true, + } + + if err := b.buildXPathExpressions(); err != nil { + t.Fatalf("buildXPathExpressions: %v", err) + } + if err := b.buildReferences(); err != nil { + t.Fatalf("buildReferences: %v", err) + } + if err := tx.Commit(); err != nil { + t.Fatalf("commit: %v", err) + } + return cat +} + +type refRow struct{ sourceType, sourceName, targetType, targetName, kind string } + +func refsTo(t *testing.T, cat *Catalog, target string) map[refRow]int { + t.Helper() + rows, err := cat.db.Query(`SELECT SourceType, SourceName, TargetType, TargetName, RefKind FROM refs WHERE TargetName = ?`, target) + if err != nil { + t.Fatalf("query refs: %v", err) + } + defer rows.Close() + got := map[refRow]int{} + for rows.Next() { + var r refRow + if err := rows.Scan(&r.sourceType, &r.sourceName, &r.targetType, &r.targetName, &r.kind); err != nil { + t.Fatalf("scan: %v", err) + } + got[r]++ + } + return got +} + +func TestReferencesReachMembersEnumerationsWorkflowsAndMappings(t *testing.T) { + cat := memberRefsFixture(t) + + want := []refRow{ + // microflow -> workflow: the call-workflow activity. + {"MICROFLOW", "Shop.ACT_Start", "WORKFLOW", "Shop.WF_Approve", "call"}, + // Attributes, from every kind of site. + {"MICROFLOW", "Shop.ACT_Start", "ATTRIBUTE", "Shop.Order.Total", "member"}, // change activity + {"PAGE", "Shop.Order_Edit", "ATTRIBUTE", "Shop.Order.Code", "member"}, // text template parameter + {"IMPORT_MAPPING", "Shop.IMM_Order", "ATTRIBUTE", "Shop.Order.Total", "member"}, // value mapping element + {"MICROFLOW", "Shop.ACT_Start", "ATTRIBUTE", "Shop.Customer.Name", "xpath"}, // path segment after an entity + {"MICROFLOW", "Shop.ACT_Start", "ATTRIBUTE", "Shop.Order.Status", "xpath"}, // bare name on the retrieved entity + {"MICROFLOW", "Shop.ACT_Start", "ATTRIBUTE", "Shop.Order.Code", "xpath"}, // compared to empty + {"MICROFLOW", "Shop.ACT_Start", "ATTRIBUTE", "Shop.Order.Total", "xpath"}, // inherited: retrieve of the specialization + {"MICROFLOW", "Shop.ACT_Start", "ASSOCIATION", "Shop.Order_Customer", "member"}, // change activity + {"MICROFLOW", "Shop.ACT_Start", "ASSOCIATION", "Shop.Order_Customer", "xpath"}, // xpath path + {"PAGE", "Shop.Order_Edit", "ASSOCIATION", "Shop.Order_Customer", "member"}, // widget -> association + {"IMPORT_MAPPING", "Shop.IMM_Order", "ENTITY", "Shop.Order", "mapping"}, // mapping -> entity + {"ENTITY", "Shop.Order", "ENUMERATION", "Shop.OrderStatus", "type"}, // attribute type + {"MICROFLOW", "Shop.ACT_Start", "ENUMERATION", "Shop.OrderStatus", "type"}, // variable type + {"MICROFLOW", "Shop.ACT_Start", "ENUMERATION_VALUE", "Shop.OrderStatus.Closed", "value"}, // expression + {"MICROFLOW", "Shop.ACT_Start", "ENUMERATION_VALUE", "Shop.OrderStatus.Open", "xpath"}, // enum attribute compared to a literal + } + + for _, w := range want { + got := refsTo(t, cat, w.targetName) + if got[w] == 0 { + t.Errorf("missing edge %s %s -> %s %s (%s); refs to %s: %v", + w.sourceType, w.sourceName, w.targetType, w.targetName, w.kind, w.targetName, got) + } + if got[w] > 1 { + t.Errorf("edge %v emitted %d times; one document using a member twice is one edge", w, got[w]) + } + } + + // Prose in Documentation is not a use, so the only microflow edge to + // Shop.Order.Code is the XPath one. + for r := range refsTo(t, cat, "Shop.Order.Code") { + if r.sourceType == "MICROFLOW" && r.kind != "xpath" { + t.Errorf("documentation text produced an edge: %v", r) + } + } + // An attribute nothing uses stays unreferenced — the control that shows + // the walk matches names rather than emitting every attribute it knows. + if got := refsTo(t, cat, "Shop.Customer.Unused"); len(got) != 0 { + t.Errorf("unused attribute has references: %v", got) + } +} + +// scanPaths is the resolver both XPath constraints and expressions go through. +// Each case pins one way a bare word could be mistaken for a member, or a real +// member missed. +func TestScanPaths(t *testing.T) { + idx := &memberRefIndex{ + attributes: map[string]string{ + "Shop.Order.Status": "Shop.OrderStatus", "Shop.Order.Code": "", + "Shop.Order.empty": "", "Shop.Order.contains": "", "Shop.Customer.Name": "", + }, + associations: map[string]bool{"Shop.Order_Customer": true}, + enumValues: map[string]bool{"Shop.OrderStatus.Open": true}, + entities: map[string]bool{"Shop.Order": true, "Shop.Customer": true}, + generalization: map[string]string{}, + } + cases := []struct { + name, text, context string + want []string + }{ + {"bare attribute on the context entity", "[Code = 'x']", "Shop.Order", []string{"ATTRIBUTE Shop.Order.Code"}}, + {"no context entity resolves nothing bare", "[Code = 'x']", "", nil}, + {"keyword that is also an attribute name", "[Code != empty]", "Shop.Order", []string{"ATTRIBUTE Shop.Order.Code"}}, + {"function name that is also an attribute name", "[contains(Code, 'x')]", "Shop.Order", []string{"ATTRIBUTE Shop.Order.Code"}}, + {"XPath token between percent signs", "[Code = '[%CurrentUser%]' or Code = %CurrentDateTime%]", "Shop.Order", []string{"ATTRIBUTE Shop.Order.Code"}}, + {"predicate after a path is evaluated on the path's entity", + "[Shop.Order_Customer/Shop.Customer[Name = 'x']]", "Shop.Order", + []string{"ASSOCIATION Shop.Order_Customer", "ATTRIBUTE Shop.Customer.Name"}}, + {"enumeration attribute compared to a literal names the value", "[Status = 'Open']", "Shop.Order", + []string{"ATTRIBUTE Shop.Order.Status", "ENUMERATION_VALUE Shop.OrderStatus.Open"}}, + {"a literal not compared to the enum attribute names nothing", "[Status = Code and Code = 'Open']", "Shop.Order", + []string{"ATTRIBUTE Shop.Order.Code", "ATTRIBUTE Shop.Order.Status"}}, + {"expression: association path from a variable", "$o/Shop.Order_Customer/Shop.Customer/Name", "", + []string{"ASSOCIATION Shop.Order_Customer", "ATTRIBUTE Shop.Customer.Name"}}, + {"expression: bare member of a variable is not resolvable", "$o/Code", "", nil}, + {"expression: qualified enumeration value", "if $o/Status = Shop.OrderStatus.Open then 1 else 2", "", + []string{"ENUMERATION_VALUE Shop.OrderStatus.Open"}}, + {"a qualified name inside a string literal is text", "'Shop.OrderStatus.Open'", "", nil}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + var set edgeSet + scanPaths(tc.text, tc.context, idx, func(tt, n string) { set.add(tt, n, "") }) + var got []string + for _, e := range set.sorted() { + got = append(got, e.TargetType+" "+e.TargetName) + } + if strings.Join(got, "|") != strings.Join(tc.want, "|") { + t.Errorf("scanPaths(%q, %q) = %v, want %v", tc.text, tc.context, got, tc.want) + } + }) + } +} diff --git a/mdl/catalog/builder_references.go b/mdl/catalog/builder_references.go index 210ad9a19..2dc17e7cb 100644 --- a/mdl/catalog/builder_references.go +++ b/mdl/catalog/builder_references.go @@ -39,6 +39,11 @@ const ( RefKindSync = "sync" // An offline navigation profile synchronizes an entity RefKindPublish = "publish" // A published REST operation runs a microflow RefKindEvent = "event" // An entity event handler runs a microflow + RefKindMember = "member" // A document binds, reads or writes an attribute, or navigates an association + RefKindXPath = "xpath" // An XPath constraint names an attribute, association or enumeration value + RefKindType = "type" // An attribute, parameter or variable is typed as an enumeration + RefKindValue = "value" // An expression names an enumeration value + RefKindMapping = "mapping" // An import or export mapping maps an entity ) // Object types recorded in refs.SourceType and refs.TargetType — the catalog's @@ -68,6 +73,11 @@ const ( RefObjectRegularExpression = "REGULAR_EXPRESSION" RefObjectScheduledEvent = "SCHEDULED_EVENT" RefObjectProjectSettings = "PROJECT_SETTINGS" + RefObjectAttribute = "ATTRIBUTE" + RefObjectEnumeration = "ENUMERATION" + RefObjectEnumerationValue = "ENUMERATION_VALUE" + RefObjectImportMapping = "IMPORT_MAPPING" + RefObjectExportMapping = "EXPORT_MAPPING" ) // RefSourceObjectTypes is every value that reaches refs.SourceType, and @@ -89,6 +99,8 @@ var ( RefObjectScheduledEvent, RefObjectPublishedRestOperation, RefObjectProjectSettings, + RefObjectImportMapping, + RefObjectExportMapping, } RefTargetObjectTypes = []string{ @@ -104,6 +116,9 @@ var ( RefObjectJavaAction, RefObjectRestOperation, RefObjectRegularExpression, + RefObjectAttribute, + RefObjectEnumeration, + RefObjectEnumerationValue, } ) @@ -183,6 +198,14 @@ func microflowActionRef(action microflows.MicroflowAction) (targetType, targetNa if a.Operation != "" { return RefObjectRestOperation, a.Operation, RefKindCall, true } + case *microflows.WorkflowCallAction: + // A microflow that starts a workflow is that workflow's caller. Without + // this edge a workflow started only from a microflow had no inbound + // reference: `show callers` said "(no callers found)" and `impact` "not + // referenced" (Evora: AltairIntegration.WF_ScheduleTechnicianAppointment). + if a.Workflow != "" { + return RefObjectWorkflow, a.Workflow, RefKindCall, true + } case *microflows.CreateObjectAction: if a.EntityQualifiedName != "" { return RefObjectEntity, a.EntityQualifiedName, RefKindCreate, true @@ -657,6 +680,15 @@ func (b *Builder) buildReferences() error { // by GRAPH_DEAD_ASSETS, whose advice is to delete them (#1126). refCount += b.extractPublishedRestRefs(stmt, projectID, snapshotID) + // Members, enumerations and mapped entities. Without these the graph ended + // at documents, so `impact` on an attribute, an enumeration or a value said + // "not referenced" however much it was used — the answer an agent reads as + // "safe to delete". See builder_member_refs.go. + idx := b.loadMemberRefIndex() + refCount += b.extractMemberRefs(stmt, idx, projectID, snapshotID) + refCount += b.extractXPathRefs(stmt, idx, projectID, snapshotID) + refCount += b.extractEnumerationTypeRefs(projectID, snapshotID) + b.report("References", refCount) return nil } diff --git a/mdl/catalog/builder_references_test.go b/mdl/catalog/builder_references_test.go index 7f6c480ed..f190c2a87 100644 --- a/mdl/catalog/builder_references_test.go +++ b/mdl/catalog/builder_references_test.go @@ -222,6 +222,13 @@ func TestMicroflowActionRef(t *testing.T) { wantOK: true, targetType: "ASSOCIATION", targetName: "M.Order_Customer", refKind: RefKindRetrieve, }, + { + name: "WorkflowCallAction (previously dropped)", + action: µflows.WorkflowCallAction{Workflow: "M.WF_Approve"}, + wantOK: true, + targetType: "WORKFLOW", targetName: "M.WF_Approve", refKind: RefKindCall, + }, + {name: "empty WorkflowCallAction", action: µflows.WorkflowCallAction{}, wantOK: false}, // Actions whose target is a local variable (no resolvable document QN) must // not emit a ref. {name: "ChangeObjectAction has no document ref", action: µflows.ChangeObjectAction{ChangeVariable: "$Order"}, wantOK: false}, diff --git a/mdl/catalog/builder_xpath.go b/mdl/catalog/builder_xpath.go index 07768fdef..2b66cc5db 100644 --- a/mdl/catalog/builder_xpath.go +++ b/mdl/catalog/builder_xpath.go @@ -279,27 +279,50 @@ func scanBSONArray(v any, fn func(map[string]any)) { // resolveEntityRefFromBSON extracts a qualified entity name from a BSON node // that has an EntityRef field (common in data source nodes). +// +// Studio Pro stores a DomainModels$DirectEntityRef with the name under +// `Entity`, and a DomainModels$IndirectEntityRef (a data source over an +// association path) as `Steps` ending on the entity the constraint applies to. +// `QualifiedName` is kept for the synthetic shape older callers pass; no stored +// EntityRef carries it, which is why every page XPath used to be recorded with +// no target entity. func resolveEntityRefFromBSON(raw map[string]any) string { - // Try EntityRef (used by most data sources) - if entityRef, ok := raw["EntityRef"].(map[string]any); ok { - if name, ok := entityRef["QualifiedName"].(string); ok { + ref := bsonAsMap(raw["EntityRef"]) + if ref == nil { + return "" + } + for _, key := range []string{"Entity", "QualifiedName"} { + if name, ok := ref[key].(string); ok && name != "" { return name } } - // Try bson.D format - if entityRef, ok := raw["EntityRef"].(bson.D); ok { - for _, elem := range entityRef { - if elem.Key == "QualifiedName" { - if name, ok := elem.Value.(string); ok { - return name - } - } + last := "" + scanBSONArray(ref["Steps"], func(step map[string]any) { + if dest, ok := step["DestinationEntity"].(string); ok && dest != "" { + last = dest } + }) + return last +} + +// bsonAsMap returns a decoded sub-document as a map, whichever shape the +// decoder produced it in; nil when v is not a document. +func bsonAsMap(v any) map[string]any { + switch d := v.(type) { + case map[string]any: + return d + case bson.M: + return d + case bson.D: + m := make(map[string]any, len(d)) + for _, e := range d { + m[e.Key] = e.Value + } + return m } - return "" + return nil } -// extractBsonIDString extracts a BSON ID as a string from various formats. func extractBsonIDString(v any) string { if v == nil { return "" diff --git a/mdl/catalog/builder_xpath_test.go b/mdl/catalog/builder_xpath_test.go index 49e426e5a..c825007c2 100644 --- a/mdl/catalog/builder_xpath_test.go +++ b/mdl/catalog/builder_xpath_test.go @@ -4,6 +4,8 @@ package catalog import ( "testing" + + "go.mongodb.org/mongo-driver/bson" ) func TestExtractReferencedEntities(t *testing.T) { @@ -93,6 +95,42 @@ func TestResolveEntityRefFromBSON(t *testing.T) { }, "Module.Entity", }, + // The shapes Studio Pro stores, as buildXPathExpressions decodes them + // (the v1 driver, so nested documents are bson.D). Only QualifiedName was read, which no + // stored EntityRef carries, so every page/snippet XPath constraint was + // recorded with an empty TargetEntity (Evora: 52 of 52) and the bare + // attribute names in it could not be resolved. + { + "stored DirectEntityRef", + map[string]any{ + "EntityRef": bson.D{ + {Key: "$Type", Value: "DomainModels$DirectEntityRef"}, + {Key: "Entity", Value: "Module.Entity"}, + }, + }, + "Module.Entity", + }, + { + "stored IndirectEntityRef ends on the last step's entity", + map[string]any{ + "EntityRef": bson.D{ + {Key: "$Type", Value: "DomainModels$IndirectEntityRef"}, + {Key: "Steps", Value: bson.A{int32(2), + bson.D{ + {Key: "$Type", Value: "DomainModels$EntityRefStep"}, + {Key: "Association", Value: "Module.A_B"}, + {Key: "DestinationEntity", Value: "Module.B"}, + }, + bson.D{ + {Key: "$Type", Value: "DomainModels$EntityRefStep"}, + {Key: "Association", Value: "Module.B_C"}, + {Key: "DestinationEntity", Value: "Module.C"}, + }, + }}, + }, + }, + "Module.C", + }, { "no EntityRef", map[string]any{"Name": "test"}, diff --git a/mdl/catalog/catalogdb.go b/mdl/catalog/catalogdb.go index 61f4b74ad..c47a0ff30 100644 --- a/mdl/catalog/catalogdb.go +++ b/mdl/catalog/catalogdb.go @@ -21,6 +21,10 @@ type CatalogTx interface { Prepare(query string) (*sql.Stmt, error) Exec(query string, args ...any) (sql.Result, error) QueryRow(query string, args ...any) *sql.Row + // Query reads a multi-row result inside the transaction — the member-ref + // pass needs the name sets earlier passes wrote (attributes, associations, + // enumerations) and the XPath constraints buildXPathExpressions recorded. + Query(query string, args ...any) (*sql.Rows, error) Commit() error Rollback() error } diff --git a/mdl/catalog/lint_rule_doc_vocabulary_test.go b/mdl/catalog/lint_rule_doc_vocabulary_test.go index 5e68a44e2..81dc23f5f 100644 --- a/mdl/catalog/lint_rule_doc_vocabulary_test.go +++ b/mdl/catalog/lint_rule_doc_vocabulary_test.go @@ -224,6 +224,7 @@ func TestSkillDocumentsRealRefKinds(t *testing.T) { RefKindMenuItem, RefKindChange, RefKindDelete, RefKindCalculate, RefKindReturn, RefKindSchedule, RefKindValidate, RefKindSettings, RefKindWidget, RefKindSync, RefKindPublish, RefKindEvent, + RefKindMember, RefKindXPath, RefKindType, RefKindValue, RefKindMapping, } { real[k] = true } diff --git a/mdl/catalog/tables.go b/mdl/catalog/tables.go index 82bed23f1..275e8af5c 100644 --- a/mdl/catalog/tables.go +++ b/mdl/catalog/tables.go @@ -7,6 +7,12 @@ package catalog // // History: // +// 15 — refs gains ATTRIBUTE, ENUMERATION and ENUMERATION_VALUE targets (kinds +// member / xpath / type / value), IMPORT_MAPPING / EXPORT_MAPPING sources +// (kind mapping), and the microflow -> workflow `call` edge; graph_god_nodes +// keeps members off the asset side. Same reason as 11 and 13: refs are only +// written by REFRESH CATALOG FULL, so a cached catalog would keep answering +// `impact Mod.Entity.Attr` with "not referenced" for a used attribute. // 14 — import_mappings_data / export_mappings_data: Id is the document's ID // (was an AUTOINCREMENT integer) and Excluded is recorded; source gains // ElementId. Two mappings may share a name when one is excluded, and the @@ -46,7 +52,7 @@ package catalog // SnapshotSource / SourceId / SourceBranch / SourceRevision columns // from every row (issue #576). // 1 — initial flat schema with denormalized snapshot columns on every row. -const CatalogSchemaVersion = "14" +const CatalogSchemaVersion = "15" // MetaSchemaVersion is the catalog_meta key that records the schema version // the cache was built against. @@ -1400,8 +1406,13 @@ func (c *Catalog) createTables() error { -- ModuleName is its own name (the ELSE d.Asset fallback below) and -- whose ObjectType is NULL. A page's OUT-degree still counts the -- widgets it uses, which is a real dependency. + -- + -- ATTRIBUTE and ENUMERATION_VALUE targets are excluded for the + -- same reason: they are members of a document, not documents, and + -- listing them as assets would bury the entity or enumeration + -- they belong to under its own attributes. SELECT TargetName AS Asset, COUNT(*) AS InDeg, 0 AS OutDeg - FROM refs WHERE TargetName != '' AND TargetType != 'WIDGET' GROUP BY TargetName + FROM refs WHERE TargetName != '' AND TargetType NOT IN ('WIDGET', 'ATTRIBUTE', 'ENUMERATION_VALUE') GROUP BY TargetName UNION ALL SELECT SourceName AS Asset, 0 AS InDeg, COUNT(*) AS OutDeg FROM refs WHERE SourceName != '' GROUP BY SourceName From 493932f08e77a45fd7c122a0bc3849c7fa0b0cc9 Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 26 Sep 2026 20:19:37 +0000 Subject: [PATCH 5/9] fix(refs): one row per reference, impact counts elements, and an empty answer says what was checked `refs` and `impact` printed a row per edge, so a microflow with two retrieve activities over an entity appeared twice (Evora: ProductionLine_Reset on DigitalTwin.Machine); the impact summary counted those rows (MICROFLOW: 9 over six microflows) and printed the types in map order, different between runs. - select distinct, with a total order; the summary counts distinct elements per type, in type order, and the footer gives both numbers. - impact/refs on an enumeration include the edges to its values, with a Target column naming which value. - "(no impact - element is not referenced)" is gone. For an attribute or an enumeration value the message lists the sites that were checked and the ones that are not resolved (a member named through a variable in an expression; a decision branch on an enum), and says to run search first. Co-Authored-By: Claude Opus 5.5 --- cmd/mxcli/cmd_query.go | 10 +- .../catalog/show-references-impact.md | 26 +++- mdl/executor/cmd_refs_impact_test.go | 143 ++++++++++++++++++ mdl/executor/cmd_search.go | 82 ++++++---- mdl/executor/reference_target.go | 62 ++++++++ 5 files changed, 292 insertions(+), 31 deletions(-) create mode 100644 mdl/executor/cmd_refs_impact_test.go diff --git a/cmd/mxcli/cmd_query.go b/cmd/mxcli/cmd_query.go index d99b7fbf0..e4e0336e8 100644 --- a/cmd/mxcli/cmd_query.go +++ b/cmd/mxcli/cmd_query.go @@ -74,11 +74,14 @@ Examples: var refsCmd = &cobra.Command{ Use: "refs ", Short: "Find references to an element", - Long: `Find all references to the specified element (entity, microflow, page, etc.). + Long: `Find all references to the specified element (entity, microflow, page, etc., +or an attribute Module.Entity.Attribute, an enumeration, or an enumeration +value Module.Enum.Value). Each (source, kind) is listed once. Examples: mxcli refs -p app.mpr Module.Customer mxcli refs -p app.mpr Module.OrderPage + mxcli refs -p app.mpr Module.Customer.Email `, Args: cobra.ExactArgs(1), Run: func(cmd *cobra.Command, args []string) { @@ -98,10 +101,15 @@ var impactCmd = &cobra.Command{ Use: "impact ", Short: "Show impact of changing an element", Long: `Analyze the impact of changing an element by showing all elements that reference it. +The summary counts distinct elements. For an enumeration, the uses of its values are +included. When nothing is found for an attribute or an enumeration value, the message +says which usage sites were checked: one named only through a variable in a free-text +expression ($Order/Total) is not resolved, so run 'mxcli search' before deleting. Examples: mxcli impact -p app.mpr Module.Customer mxcli impact -p app.mpr Module.OrderStatus + mxcli impact -p app.mpr Module.Customer.Email `, Args: cobra.ExactArgs(1), Run: func(cmd *cobra.Command, args []string) { diff --git a/docs-site/src/reference/catalog/show-references-impact.md b/docs-site/src/reference/catalog/show-references-impact.md index 943275788..501dcdc35 100644 --- a/docs-site/src/reference/catalog/show-references-impact.md +++ b/docs-site/src/reference/catalog/show-references-impact.md @@ -18,10 +18,28 @@ These commands provide different views of cross-reference information for a give **SHOW CONTEXT OF** assembles the surrounding context of an element -- its definition, its callers, callees, and related elements -- suitable for providing to an LLM or for understanding an element in its broader project context. The optional `DEPTH` parameter controls how many levels of related elements to include. +### Attributes, enumerations and enumeration values + +The target may also be an attribute (`Module.Entity.Attribute`), an enumeration, or an enumeration value (`Module.Enum.Value`). The graph records: + +| Kind | Meaning | +|------|---------| +| `member` | a microflow, nanoflow, rule, page, snippet, workflow or import/export mapping binds, reads or writes the attribute, or navigates the association | +| `xpath` | an XPath constraint names the attribute or association, or compares an enumeration attribute with the value | +| `type` | an attribute, parameter or variable is typed as the enumeration | +| `value` | an expression names the enumeration value | +| `mapping` | an import or export mapping maps the entity | + +`IMPACT OF` an enumeration includes the uses of each of its values, with a `Target` column naming which one. + +Results list each (source, kind) once, and the `IMPACT` summary counts distinct elements. + +When nothing is found, the message says what was searched. An attribute named only through a variable in a free-text expression (`$Order/Total`), and an enumeration value used only as a decision branch, are not resolved by the catalog, so an empty result for an attribute or a value tells you to run `SEARCH` before treating it as unused. + ## Parameters **qualified_name** -: The fully qualified name of the element to analyze (e.g., `Module.EntityName`, `Module.MicroflowName`). +: The fully qualified name of the element to analyze (e.g., `Module.EntityName`, `Module.MicroflowName`, `Module.Entity.Attribute`, `Module.Enum.Value`). **n** (CONTEXT only) : The number of levels of related elements to include. Defaults to 1 if not specified. Higher values include more surrounding context but produce more output. @@ -41,6 +59,12 @@ SHOW REFERENCES TO Sales.Customer; SHOW IMPACT OF Sales.Customer; ``` +### Check an attribute before dropping it + +```sql +SHOW IMPACT OF Sales.Order.DiscountCode; +``` + ### Gather context for a microflow ```sql diff --git a/mdl/executor/cmd_refs_impact_test.go b/mdl/executor/cmd_refs_impact_test.go new file mode 100644 index 000000000..aa360bb25 --- /dev/null +++ b/mdl/executor/cmd_refs_impact_test.go @@ -0,0 +1,143 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/catalog" +) + +// Measured on Evora Factory Management, `impact DigitalTwin.Machine` printed +// FactoryManagement.ProductionLine_Reset | retrieve twice (the microflow has two +// retrieve activities, one edge each) and a summary of "MICROFLOW: 9" over six +// distinct microflows, in an order that changed between runs. `refs` repeated +// the same row. The fixture reproduces each shape. + +func seedRefsCatalog(t *testing.T) *ExecContext { + t.Helper() + cat, err := catalog.New() + if err != nil { + t.Fatalf("catalog.New: %v", err) + } + t.Cleanup(func() { cat.Close() }) + + db := cat.CatalogDB() + seed := []string{ + `INSERT INTO refs (SourceType, SourceId, SourceName, TargetType, TargetId, TargetName, RefKind, ProjectId, SnapshotId) VALUES + ('MICROFLOW', '', 'Shop.ACT_Reset', 'ENTITY', '', 'Shop.Machine', 'retrieve', 'p', 's'), + ('MICROFLOW', '', 'Shop.ACT_Reset', 'ENTITY', '', 'Shop.Machine', 'retrieve', 'p', 's'), + ('MICROFLOW', '', 'Shop.ACT_Reset', 'ENTITY', '', 'Shop.Machine', 'delete', 'p', 's'), + ('MICROFLOW', '', 'Shop.ACT_Count', 'ENTITY', '', 'Shop.Machine', 'retrieve', 'p', 's'), + ('PAGE', '', 'Shop.Machine_Details', 'ENTITY', '', 'Shop.Machine', 'datasource', 'p', 's'), + ('PAGE', '', 'Shop.Machine_Details', 'ENTITY', '', 'Shop.Machine', 'parameter', 'p', 's'), + ('ASSOCIATION', '', 'Shop.Machine_Line', 'ENTITY', '', 'Shop.Machine', 'associate', 'p', 's'), + ('ENTITY', '', 'Shop.Machine', 'ENUMERATION', '', 'Shop.Status', 'type', 'p', 's'), + ('PAGE', '', 'Shop.Machine_Details', 'ENUMERATION_VALUE', '', 'Shop.Status.Critical', 'value', 'p', 's'), + ('MICROFLOW', '', 'Shop.ACT_Reset', 'ENUMERATION_VALUE', '', 'Shop.Status.Critical', 'value', 'p', 's')`, + `INSERT INTO attributes_data (Id, Name, EntityId, EntityQualifiedName, ModuleName, DataType) VALUES + ('a1', 'Unused', 'e1', 'Shop.Machine', 'Shop', 'String')`, + `INSERT INTO enumerations_data (Id, Name, QualifiedName, ModuleName) VALUES + ('en1', 'Status', 'Shop.Status', 'Shop')`, + `INSERT INTO enumeration_values_data (Id, EnumerationId, EnumerationQualifiedName, ModuleName, Name) VALUES + ('v1', 'en1', 'Shop.Status', 'Shop', 'Critical'), + ('v2', 'en1', 'Shop.Status', 'Shop', 'Retired')`, + } + for _, s := range seed { + if _, err := db.Exec(s); err != nil { + t.Fatalf("seed: %v\n%s", err, s) + } + } + ctx, _ := newMockCtx(t) + ctx.Catalog = cat + return ctx +} + +func runRefsCmd(t *testing.T, fn func(*ExecContext, string) error, target string) string { + t.Helper() + ctx := seedRefsCatalog(t) + if err := fn(ctx, target); err != nil { + t.Fatalf("run: %v", err) + } + return ctx.Output.(interface{ String() string }).String() +} + +func rowsMentioning(out, s string) int { + n := 0 + for _, line := range strings.Split(out, "\n") { + if strings.HasPrefix(strings.TrimSpace(line), "|") && strings.Contains(line, s) { + n++ + } + } + return n +} + +func TestShowReferencesListsEachEdgeOnce(t *testing.T) { + out := runRefsCmd(t, showReferences, "Shop.Machine") + // ACT_Reset: retrieve + delete — two edges, not three rows. + if n := rowsMentioning(out, "Shop.ACT_Reset"); n != 2 { + t.Errorf("Shop.ACT_Reset appears in %d rows, want 2 (retrieve, delete):\n%s", n, out) + } + if !strings.Contains(out, "Found 6 reference(s)") { + t.Errorf("want 6 distinct references:\n%s", out) + } +} + +func TestShowImpactCountsElementsNotRows(t *testing.T) { + out := runRefsCmd(t, showImpact, "Shop.Machine") + for _, want := range []string{ + " ASSOCIATION: 1\n MICROFLOW: 2\n PAGE: 1\n", // distinct elements, types in a fixed order + "Found 4 affected element(s)", + } { + if !strings.Contains(out, want) { + t.Errorf("impact output missing %q:\n%s", want, out) + } + } + if n := rowsMentioning(out, "Shop.ACT_Reset"); n != 2 { + t.Errorf("Shop.ACT_Reset appears in %d rows, want 2:\n%s", n, out) + } +} + +// The summary was built by ranging over a map, so two runs of the same +// command could print the types in different orders. +func TestShowImpactIsDeterministic(t *testing.T) { + first := runRefsCmd(t, showImpact, "Shop.Machine") + for i := 0; i < 20; i++ { + if again := runRefsCmd(t, showImpact, "Shop.Machine"); again != first { + t.Fatalf("run %d differs:\n%s\n---\n%s", i, first, again) + } + } +} + +// An enumeration is used through its values as much as through its type; an +// impact that listed only the attribute typed as it would miss the page and +// microflow that break when a value is removed. +func TestShowImpactOfEnumerationIncludesItsValues(t *testing.T) { + out := runRefsCmd(t, showImpact, "Shop.Status") + for _, want := range []string{"Shop.Machine", "Shop.Machine_Details", "Shop.ACT_Reset", "Shop.Status.Critical"} { + if !strings.Contains(out, want) { + t.Errorf("enumeration impact missing %q:\n%s", want, out) + } + } +} + +// With no edge, the catalog cannot say an attribute is unused — only that none +// of the sites it resolves uses it. Saying "not referenced" is what makes an +// agent delete a live attribute. +func TestShowImpactOfUnreferencedAttributeSaysWhatWasChecked(t *testing.T) { + out := runRefsCmd(t, showImpact, "Shop.Machine.Unused") + if strings.Contains(out, "is not referenced") { + t.Errorf("claims the attribute is not referenced:\n%s", out) + } + for _, want := range []string{"expression", "search 'Unused'"} { + if !strings.Contains(out, want) { + t.Errorf("no-reference message missing %q:\n%s", want, out) + } + } + + refs := runRefsCmd(t, showReferences, "Shop.Status.Retired") + if !strings.Contains(refs, "search 'Retired'") { + t.Errorf("enumeration value no-reference message should name what was not checked:\n%s", refs) + } +} diff --git a/mdl/executor/cmd_search.go b/mdl/executor/cmd_search.go index 557fd6277..8e2debdd6 100644 --- a/mdl/executor/cmd_search.go +++ b/mdl/executor/cmd_search.go @@ -213,7 +213,15 @@ func execShowReferences(ctx *ExecContext, s *ast.ShowStmt) error { return err } - typed := s.Name.String() + return showReferences(ctx, s.Name.String()) +} + +// showReferences prints the references to typed from the loaded catalog. +// +// One row per distinct (source, kind): a microflow with two retrieve +// activities over the same entity is one retrieve reference, not two rows +// that read as two callers. +func showReferences(ctx *ExecContext, typed string) error { fmt.Fprintf(ctx.Output, "\nReferences to %s\n", typed) // A widget's TargetName is stored SHOUTED (COMBOBOX) while MDL keywords are @@ -222,21 +230,20 @@ func execShowReferences(ctx *ExecContext, s *ast.ShowStmt) error { targetName, loose := resolveReferenceTarget(ctx, typed) reportResolvedTarget(ctx, typed, targetName, loose) - // Find all references to this target - query := ` - select SourceType, SourceName, RefKind - from refs - where TargetName = ? - ORDER by RefKind, SourceType, SourceName - ` + where, viaValues := refTargetWhere(ctx, targetName) + cols, order := "SourceType, SourceName, RefKind", "RefKind, SourceType, SourceName" + if viaValues { + cols, order = cols+", TargetName as Target", order+", Target" + } + query := `select distinct ` + cols + ` from refs where ` + where + ` order by ` + order - result, err := ctx.Catalog.Query(strings.Replace(query, "?", "'"+escapeSQLString(targetName)+"'", 1)) + result, err := ctx.Catalog.Query(query) if err != nil { return mdlerrors.NewBackend("query references", err) } if result.Count == 0 { - fmt.Fprintln(ctx.Output, "(no references found)") + fmt.Fprintln(ctx.Output, noReferencesMessage(ctx, targetName)) return nil } @@ -257,47 +264,64 @@ func execShowImpact(ctx *ExecContext, s *ast.ShowStmt) error { return err } - typed := s.Name.String() + return showImpact(ctx, s.Name.String()) +} + +// showImpact prints the elements that reference typed, from the loaded catalog. +// +// The summary counts ELEMENTS: it used to count rows, so a microflow that both +// retrieves and deletes an entity was two "affected" microflows, and the types +// came out in map order, different from run to run. +func showImpact(ctx *ExecContext, typed string) error { fmt.Fprintf(ctx.Output, "\nImpact analysis for %s\n", typed) targetName, loose := resolveReferenceTarget(ctx, typed) reportResolvedTarget(ctx, typed, targetName, loose) - // Find all direct references to this target - directQuery := ` - select SourceType, SourceName, RefKind - from refs - where TargetName = ? - ORDER by SourceType, SourceName - ` + where, viaValues := refTargetWhere(ctx, targetName) + cols, order := "SourceType, SourceName, RefKind", "SourceType, SourceName, RefKind" + if viaValues { + cols, order = cols+", TargetName as Target", order+", Target" + } + directQuery := `select distinct ` + cols + ` from refs where ` + where + ` order by ` + order - result, err := ctx.Catalog.Query(strings.Replace(directQuery, "?", "'"+escapeSQLString(targetName)+"'", 1)) + result, err := ctx.Catalog.Query(directQuery) if err != nil { return mdlerrors.NewBackend("query impact", err) } if result.Count == 0 { - fmt.Fprintln(ctx.Output, "(no impact - element is not referenced)") + fmt.Fprintln(ctx.Output, noReferencesMessage(ctx, targetName)) return nil } - // Group by type for summary - typeCounts := make(map[string]int) + // Distinct elements per type. Rows are ordered by SourceType, so the types + // come out sorted. + var types []string + perType := map[string]map[string]bool{} + elements := 0 for _, row := range result.Rows { - if len(row) > 0 { - if t, ok := row[0].(string); ok { - typeCounts[t]++ - } + if len(row) < 2 { + continue + } + t, name := fmt.Sprint(row[0]), fmt.Sprint(row[1]) + if perType[t] == nil { + perType[t] = map[string]bool{} + types = append(types, t) + } + if !perType[t][name] { + perType[t][name] = true + elements++ } } fmt.Fprintf(ctx.Output, "\nSummary:\n") - for t, count := range typeCounts { - fmt.Fprintf(ctx.Output, " %s: %d\n", t, count) + for _, t := range types { + fmt.Fprintf(ctx.Output, " %s: %d\n", t, len(perType[t])) } fmt.Fprintln(ctx.Output) - fmt.Fprintf(ctx.Output, "Found %d affected element(s)\n", result.Count) + fmt.Fprintf(ctx.Output, "Found %d affected element(s) (%d reference(s))\n", elements, result.Count) outputCatalogResults(ctx, result) return nil diff --git a/mdl/executor/reference_target.go b/mdl/executor/reference_target.go index a458c3f8f..de9d99cac 100644 --- a/mdl/executor/reference_target.go +++ b/mdl/executor/reference_target.go @@ -66,3 +66,65 @@ func reportResolvedTarget(ctx *ExecContext, typed, resolved string, matchedLoose } fmt.Fprintf(ctx.Output, "(matched %s)\n", strings.TrimSpace(resolved)) } + +// refTargetWhere returns the refs WHERE clause for a target. An enumeration is +// used through its values as well as its type — a page comparing against +// Mod.Enum.Value breaks when the value is removed — so for an enumeration the +// clause also takes the edges to each of its values, and viaValues reports that +// the caller should show which target each row reached. +func refTargetWhere(ctx *ExecContext, target string) (where string, viaValues bool) { + esc := escapeSQLString(target) + where = fmt.Sprintf("TargetName = '%s'", esc) + if catalogHas(ctx, fmt.Sprintf(`select 1 from enumerations where QualifiedName = '%s' limit 1`, esc)) { + // The name is a LIKE prefix, so its underscores (ENUM_Status) must not + // act as wildcards. + where = fmt.Sprintf(`(TargetName = '%s' or (TargetType = 'ENUMERATION_VALUE' and TargetName like '%s.%%' escape '\'))`, + esc, escapeSQLString(escapeSQLLike(target))) + return where, true + } + return where, false +} + +// escapeSQLLike escapes LIKE wildcards; paired with `escape '\'`. +func escapeSQLLike(s string) string { + r := strings.NewReplacer(`\`, `\\`, `%`, `\%`, `_`, `\_`) + return r.Replace(s) +} + +func catalogHas(ctx *ExecContext, query string) bool { + if ctx == nil || ctx.Catalog == nil { + return false + } + res, err := ctx.Catalog.Query(query) + return err == nil && res.Count > 0 +} + +// noReferencesMessage is what refs/impact print when no edge reaches target. +// +// It says what was searched rather than "element is not referenced": the +// reference graph is built from the sites the catalog resolves, and for an +// attribute or an enumeration value some sites are free text it does not +// resolve. "Not referenced" was read — by an agent, reasonably — as "safe to +// delete", on attributes that were in use. +func noReferencesMessage(ctx *ExecContext, target string) string { + esc := escapeSQLString(target) + member := target + if i := strings.LastIndex(target, "."); i >= 0 { + member = target[i+1:] + } + switch { + case catalogHas(ctx, fmt.Sprintf(`select 1 from attributes where EntityQualifiedName || '.' || Name = '%s' limit 1`, esc)): + return fmt.Sprintf("(no references found to attribute %s)\n"+ + "Checked: attribute bindings and member changes in microflows, nanoflows, rules, pages,\n"+ + "snippets, workflows and import/export mappings, and XPath constraints.\n"+ + "Not checked: the attribute named through a variable in an expression ($Object/%s).\n"+ + "Run `search '%s'` before treating it as unused.", target, member, member) + case catalogHas(ctx, fmt.Sprintf(`select 1 from enumeration_values where EnumerationQualifiedName || '.' || Name = '%s' limit 1`, esc)): + return fmt.Sprintf("(no references found to enumeration value %s)\n"+ + "Checked: qualified uses in expressions (%s) and XPath comparisons of an enumeration\n"+ + "attribute with '%s'.\n"+ + "Not checked: decision branches on an enumeration, which store the bare value name.\n"+ + "Run `search '%s'` before treating it as unused.", target, target, member, member) + } + return fmt.Sprintf("(no references found: nothing in the catalog's reference graph points at %s)", target) +} From fd677e3463cf099219f40ebbfaca2fc19cffad1f Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 26 Sep 2026 20:19:40 +0000 Subject: [PATCH 6/9] fix(context): related entities come from associations, and callers are listed once `context DigitalTwin.Machine` said "Related Entities: (none found)" for an entity with five associations. The section read refs rows whose SOURCE is an entity, but an association edge's source is the ASSOCIATION, so only generalizations could ever match. It now reads both ends from the associations table (present in a fast catalog too), plus the generalization and the specializations. Also: Direct Callers / Shown By / workflow callers list each source once, and the enumeration context reads the same edge set as impact (type and value uses), grouped into entities, flows and pages. Co-Authored-By: Claude Opus 5.5 --- mdl/executor/cmd_context.go | 81 +++++++++++++----------- mdl/executor/cmd_context_refs_test.go | 91 +++++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 36 deletions(-) create mode 100644 mdl/executor/cmd_context_refs_test.go diff --git a/mdl/executor/cmd_context.go b/mdl/executor/cmd_context.go index a0ee96522..cb72a618c 100644 --- a/mdl/executor/cmd_context.go +++ b/mdl/executor/cmd_context.go @@ -151,7 +151,7 @@ func assembleMicroflowContext(ctx *ExecContext, out *strings.Builder, name strin // Direct callers out.WriteString("### Direct Callers\n\n") result, err = ctx.Catalog.Query(fmt.Sprintf( - `select SourceName from refs + `select distinct SourceName from refs where TargetName = '%s' and RefKind = 'call' ORDER by SourceName limit 10`, name)) if err == nil && result.Count > 0 { @@ -243,15 +243,29 @@ func assembleEntityContext(ctx *ExecContext, out *strings.Builder, name string, } out.WriteString("\n") - // Related entities (via associations or generalization) + // Related entities: the other end of each association, the entity this one + // specializes, and the entities that specialize it. + // + // This read refs rows whose SOURCE was an entity, but an association edge's + // source is the ASSOCIATION, so only generalizations were ever found and an + // entity with five associations reported "(none found)". The associations + // table has both ends directly, and is there in a fast-mode catalog too. out.WriteString("### Related Entities\n\n") + esc := escapeSQLString(name) result, err = ctx.Catalog.Query(fmt.Sprintf( - `select distinct TargetName, RefKind from refs - where SourceName = '%s' and TargetType = 'ENTITY' - union - select distinct SourceName, RefKind from refs - where TargetName = '%s' and SourceType = 'ENTITY' - ORDER by RefKind, TargetName limit 10`, name, name)) + `select Related, Relation from ( + select ToEntity as Related, 'association ' || QualifiedName || ': ' || FromEntity || ' -> ' || ToEntity as Relation + from associations where FromEntity = '%[1]s' + union + select FromEntity, 'association ' || QualifiedName || ': ' || FromEntity || ' -> ' || ToEntity + from associations where ToEntity = '%[1]s' + union + select Generalization, 'generalization' + from entities where QualifiedName = '%[1]s' and Generalization is not null and Generalization != '' + union + select QualifiedName, 'specialization' + from entities where Generalization = '%[1]s' + ) order by Relation, Related`, esc)) if err == nil && result.Count > 0 { for _, row := range result.Rows { out.WriteString(fmt.Sprintf("- %v (%v)\n", row[0], row[1])) @@ -316,7 +330,7 @@ func assemblePageContext(ctx *ExecContext, out *strings.Builder, name string, de // Microflows that show this page out.WriteString("### Shown By\n\n") result, err = ctx.Catalog.Query(fmt.Sprintf( - `select SourceName from refs + `select distinct SourceName from refs where TargetName = '%s' and RefKind = 'show_page' ORDER by SourceName limit 10`, name)) if err == nil && result.Count > 0 { @@ -341,33 +355,28 @@ func assembleEnumerationContext(ctx *ExecContext, out *strings.Builder, name str } out.WriteString("\n") - // Entities with attributes of this enumeration type - out.WriteString("### Used By Entities\n\n") - result, err = ctx.Catalog.Query(fmt.Sprintf( - `select distinct SourceName from refs - where TargetName = '%s' and SourceType = 'ENTITY' - ORDER by SourceName limit 15`, name)) - if err == nil && result.Count > 0 { - for _, row := range result.Rows { - out.WriteString(fmt.Sprintf("- %v\n", row[0])) - } - } else { - out.WriteString("(none found)\n") - } - out.WriteString("\n") - - // Microflows that use this enumeration - out.WriteString("### Used By Microflows\n\n") - result, err = ctx.Catalog.Query(fmt.Sprintf( - `select distinct SourceName from refs - where TargetName = '%s' and SourceType = 'MICROFLOW' - ORDER by SourceName limit 15`, name)) - if err == nil && result.Count > 0 { - for _, row := range result.Rows { - out.WriteString(fmt.Sprintf("- %v\n", row[0])) + // Uses of the enumeration: its type (attributes, parameters, variables) + // and its values (expressions, XPath comparisons). refTargetWhere takes + // both, the same set `impact` reports. + where, _ := refTargetWhere(ctx, name) + for _, sec := range []struct{ title, types string }{ + {"Used By Entities", "'ENTITY'"}, + {"Used By Microflows", "'MICROFLOW', 'NANOFLOW', 'RULE'"}, + {"Used By Pages", "'PAGE', 'SNIPPET'"}, + } { + out.WriteString("### " + sec.title + "\n\n") + result, err = ctx.Catalog.Query(fmt.Sprintf( + `select distinct SourceName from refs + where %s and SourceType in (%s) + ORDER by SourceName limit 15`, where, sec.types)) + if err == nil && result.Count > 0 { + for _, row := range result.Rows { + out.WriteString(fmt.Sprintf("- %v\n", row[0])) + } + } else { + out.WriteString("(none found)\n") } - } else { - out.WriteString("(none found)\n") + out.WriteString("\n") } } @@ -561,7 +570,7 @@ func assembleWorkflowContext(ctx *ExecContext, out *strings.Builder, name string // Direct callers (what calls this workflow) out.WriteString("### Direct Callers\n\n") result, err = ctx.Catalog.Query(fmt.Sprintf( - `select SourceName, SourceType from refs + `select distinct SourceName, SourceType from refs where TargetName = '%s' ORDER by SourceName limit 15`, name)) if err == nil && result.Count > 0 { diff --git a/mdl/executor/cmd_context_refs_test.go b/mdl/executor/cmd_context_refs_test.go new file mode 100644 index 000000000..16a1ea648 --- /dev/null +++ b/mdl/executor/cmd_context_refs_test.go @@ -0,0 +1,91 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/catalog" +) + +// `context` on an entity with associations said "Related Entities: (none +// found)". The section read refs rows whose SOURCE is an entity, but an +// association edge's source is the ASSOCIATION, so it only ever found +// generalizations. Evora: DigitalTwin.Machine, five associations. +func TestAssembleEntityContextListsAssociatedEntities(t *testing.T) { + cat, err := catalog.New() + if err != nil { + t.Fatalf("catalog.New: %v", err) + } + defer cat.Close() + db := cat.CatalogDB() + for _, s := range []string{ + `INSERT INTO entities_data (Id, Name, QualifiedName, ModuleName, EntityType, Generalization) VALUES + ('e1', 'Machine', 'Shop.Machine', 'Shop', 'PERSISTENT', ''), + ('e2', 'Line', 'Shop.Line', 'Shop', 'PERSISTENT', ''), + ('e3', 'Incident', 'Shop.Incident', 'Shop', 'PERSISTENT', ''), + ('e4', 'Robot', 'Shop.Robot', 'Shop', 'PERSISTENT', 'Shop.Machine')`, + `INSERT INTO associations_data (Id, Name, QualifiedName, ModuleName, FromEntity, ToEntity) VALUES + ('as1', 'Machine_Line', 'Shop.Machine_Line', 'Shop', 'Shop.Machine', 'Shop.Line'), + ('as2', 'Incident_Machine', 'Shop.Incident_Machine', 'Shop', 'Shop.Incident', 'Shop.Machine')`, + } { + if _, err := db.Exec(s); err != nil { + t.Fatalf("seed: %v", err) + } + } + ctx, _ := newMockCtx(t) + ctx.Catalog = cat + + var out strings.Builder + assembleEntityContext(ctx, &out, "Shop.Machine", 2) + got := out.String() + related := got[strings.Index(got, "### Related Entities"):] + if strings.Contains(related, "(none found)") { + t.Fatalf("entity with two associations reports no related entities:\n%s", related) + } + for _, want := range []string{"Shop.Line", "Shop.Machine_Line", "Shop.Incident", "Shop.Incident_Machine", "Shop.Robot"} { + if !strings.Contains(related, want) { + t.Errorf("Related Entities missing %q:\n%s", want, related) + } + } +} + +// `context` on a microflow listed a caller once per call activity, so a caller +// with three calls to it read as three callers. +func TestAssembleMicroflowContextListsEachCallerOnce(t *testing.T) { + cat, err := catalog.New() + if err != nil { + t.Fatalf("catalog.New: %v", err) + } + defer cat.Close() + if _, err := cat.CatalogDB().Exec(`INSERT INTO refs (SourceType, SourceId, SourceName, TargetType, TargetId, TargetName, RefKind, ProjectId, SnapshotId) VALUES + ('MICROFLOW', '', 'Shop.Caller', 'MICROFLOW', '', 'Shop.Sub', 'call', 'p', 's'), + ('MICROFLOW', '', 'Shop.Caller', 'MICROFLOW', '', 'Shop.Sub', 'call', 'p', 's'), + ('MICROFLOW', '', 'Shop.Caller', 'MICROFLOW', '', 'Shop.Sub', 'call', 'p', 's')`); err != nil { + t.Fatalf("seed: %v", err) + } + ctx, _ := newMockCtx(t) + ctx.Catalog = cat + + var out strings.Builder + assembleMicroflowContext(ctx, &out, "Shop.Sub", 1) + callers := out.String()[strings.Index(out.String(), "### Direct Callers"):] + if n := strings.Count(callers, "- Shop.Caller"); n != 1 { + t.Errorf("Shop.Caller listed %d times under Direct Callers, want 1:\n%s", n, callers) + } +} + +// The enumeration context read ENTITY and MICROFLOW sources of edges to the +// enumeration's own name, which never existed; its values were not looked at. +func TestAssembleEnumerationContextIncludesValueUses(t *testing.T) { + ctx := seedRefsCatalog(t) + var out strings.Builder + assembleEnumerationContext(ctx, &out, "Shop.Status") + got := out.String() + for _, want := range []string{"- Shop.Machine\n", "- Shop.ACT_Reset\n", "- Shop.Machine_Details\n"} { + if !strings.Contains(got, want) { + t.Errorf("enumeration context missing %q:\n%s", want, got) + } + } +} From 60963003b30bf79658e15aaf4d30efbfa0583584 Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 26 Sep 2026 20:20:39 +0000 Subject: [PATCH 7/9] fix(describe): trim a stored expression's outer whitespace when rendering it Studio Pro stores expressions exactly as typed, and a trailing newline left in the expression editor is common. describe interpolated the stored text verbatim, so `change $X (Status = Mod.Enum.Val` / `);` put the closing paren or semicolon on a line of its own (297 such lines in Evora Factory Management's microflows alone). One helper, describeExpr (TrimSpace; interior newlines kept), now renders every stored expression the describer emits: change/create members, set, change-list values, aggregate/reduce, list operations, call microflow / nanoflow / java / javascript / external action arguments, show page args, log node + template params, show message params, REST/web service/DB query params, while, decision and rule arguments, and page widget Visible/Editable conditions, action arguments, datasource arguments and client template parameters. It replaces the five ad-hoc TrimSuffix/TrimRight calls that each covered one slot. The stored model is untouched. Co-Authored-By: Claude Opus 5.5 --- .../fix-issue/findings/mdl-executor.jsonl | 1 + .../describe-expression-trailing-newline.mdl | 49 ++++++++ ...ows_describe_expression_whitespace_test.go | 108 +++++++++++++++++ mdl/executor/cmd_microflows_format_action.go | 111 +++++++++++------- mdl/executor/cmd_pages_describe_datasource.go | 2 +- mdl/executor/cmd_pages_describe_output.go | 15 +-- 6 files changed, 233 insertions(+), 53 deletions(-) create mode 100644 mdl-examples/bug-tests/describe-expression-trailing-newline.mdl create mode 100644 mdl/executor/cmd_microflows_describe_expression_whitespace_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 8bb82beaa..b797f63d1 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -713,3 +713,4 @@ {"area": "mdl/executor", "date": "2026-09-25", "symptom": "`textbox t (Attribute: FullName)` at the top of a page (CREATE PAGE/SNIPPET, a plain container, or ALTER PAGE … INSERT at page level) passed plain `mxcli check`, `exec --no-check`/ALTER reported success, and `bson dump` showed `AttributeRef: null` — mxbuild 11.13.0: CE0544 \"This widget can only function inside a data context\" + CE7005 (textbox/textarea/datepicker/checkbox/radiobuttons/dropdown), CE0402 (dynamictext Attribute:), CE0642 (combobox). Qualified `Mod.Ent.Attr` there is stored and fails CE0544/CE2421/CE1365/CE7247 \"Move this widget into a data container\" + CE7006. `Attribute: $P/Attr` / `$currentObject/Attr` dropped even INSIDE a data view.", "cause": "resolveAttributePath returns the bare name when entityContext is \"\", and attributeRefToGen (and widgetobj setAttributeRefField) write nil for any path with < 2 dots, so the binding vanished between builder and writer; refuseBareAttributeRefs never sees it because no Attribute string is emitted. The only refusal (validatePageContextTree) runs in the --references phase for CREATE PAGE/SNIPPET, so plain check, --no-check and ALTER were unguarded. `$x/Attr` parses via the generic property rule as an *ast.DataSourceV3, so GetAttribute() returns \"\" and every builder skipped it.", "file": "mdl/executor/cmd_pages_input_binding_context.go (inputBindingProblem, checkInputBinding, validateInputBindingContext = MDL-WIDGET34), wired in cmd_pages_builder_v3_widgets.go (6 input builders + buildDynamicTextV3), widget_engine.go (primary Attribute mapping), validate_widgets.go (validateWidgetTreeIn); tests cmd_pages_input_binding_context_test.go; bug-tests input-binding-without-context{,.fail}.mdl", "insight": "Reuse the MDL-PAGEARG01 three-state context (pageArgContext known/present) rather than entityContext==\"\" as the 'outside a data container' signal: entityContext is also empty INSIDE a container whose flow cannot be resolved (excluded ShareFeedback_Logo), where DESCRIBE writes qualified names that must keep building — refusing qualified-on-empty-entity would have broken that round trip. So known-absent context refuses bare AND qualified; unknown context (ALTER) refuses only the bare name the writer provably nulls. Two existing unit tests (OnChangeSurvivesBuilder, DynamicTextV3_AttributeBinds) built inputs with NO entity and passed — the second asserted a bare `Title` AttributeRef counted as 'bound', i.e. it pinned the bug: when a fixture has no entity context, ask what the writer does with its output. The `$P/Attr` drop was found only by dumping the control page, not from the report — print the AST value type with a probe test before assuming a spelling reaches the builder. Evidence: 22 mxbuild errors before on the probe matrix; after, every case refused with nothing written, controls (dataview/listview/gallery/datagrid/snippet dataview/ALTER into dataview) 0 errors, 17/17 stock pages + 4/4 snippets describe→exec round trip.", "refs": ["MDL-WIDGET34"], "ce": ["CE0544", "CE7005", "CE0402", "CE0642", "CE2421", "CE1365", "CE7247", "CE7006"]} {"area":"mdl/executor","date":"2026-09-25","symptom":"`alter page FeedbackModule.ShareFeedback_Logo { insert after textBox1 { image zzImg (ImageType: imageUrl, ImageUrl: '{1}', ImageUrlParams: [{1} = ImageB64]) } }` passed `check --references`; exec wrote a bare AttributeRef and `mx check` could not LOAD the project (ArgumentNullException setting 'Attribute'). Same at page top level outside any data container; a text box's `Attribute:` there is silently dropped (CE7005).","cause":"ALTER's entity context comes from the STORED document (nearest enclosing data source, or a flow source's return type via resolveDataSourceFlowEntity). With the flow missing (Feedback v4.0.2 ships no DS_FeedbackForm) or no container at all, entityContext is \"\" and resolveAttributePath returns the bare name. CREATE PAGE refused this at check time (relaxExcludedWidgetRefs/unscopedBindings, #678); ALTER's check never opened the document, so nothing could know the scope.","file":"mdl/executor/validate_alter_unscoped.go, mdl/executor/cmd_alter_page.go (alterEntityContext), mdl/executor/validate.go (bindingsWithoutScope)","insight":"For ALTER, scope is a property of the stored document, not the statement: a check-time question about it must open the document (OpenPageForMutation, never Save; validate_alter_set.go already does this) and ask through the SAME function exec uses, so the INSERT/REPLACE entity resolution was lifted into alterEntityContext rather than restated, and the binding walk lifted out of unscopedBindings (bindingsWithoutScope) rather than copied. Controls that keep it from blocking working scripts: skip a target the stored doc lacks (added earlier in the script), a flow the script declares with an entity return (sc.flowParams), DataGrid2 column and list-view-template paths, and documents the script creates. Reproduce with a Studio Pro-authored page whose flow is genuinely absent.","refs":["#678","#685"]} {"area": "mdl/executor", "date": "2026-09-25", "symptom": "`describe page` on a File Uploader (files mode) emits `DataSource: association …`, and exec of that output fails: \"widget `upFiles` (fileuploader) exposes 2 datasources, so a generic `datasource:` clause is ambiguous — name the one you mean: associatedFiles, associatedImages\"", "cause": "DESCRIBE chose generic vs named by counting CONFIGURED datasources (namedCustomWidgetDataSources drops unset ones, so files mode = 1), while the builder's refuseAmbiguousGenericDataSource counts DECLARED datasource mappings (a generated def maps every top-level datasource = 2). Two sides of one round trip deciding the same question from different evidence.", "file": "mdl/executor/cmd_pages_describe_parse.go", "insight": "When describe and build each decide 'is this ambiguous?', they must count the same set. Fix read the DECLARED count from the stored schema (PropertyTypes with ValueType.Type=DataSource, excluding IsLinked) and excluded widgets with an embedded .def.json — those are hand-written and pick one datasource mapping per mode, so a database-mode ComboBox (two declared) must keep the generic clause. The #956 bug-test script itself authored the refused generic clause on a File Uploader: a bug-test that only runs `mxcli check` without a project can't see an exec-time refusal, so grep bug-tests for the old spelling whenever a builder starts refusing one.", "refs": ["mendixlabs/mxcli#1199", "mendixlabs/mxcli#956", "mendixlabs/mxcli#1109"], "rules": []} +{"area":"mdl/executor","date":"2026-09-26","symptom":"`describe microflow` puts the closing `)` or `;` of a statement on a line of its own — `change $X (SyncState = MCPClient.ENUM_SyncState.Syncing` / `);`, or a multi-member `create` rendered one `, Member = …` per line with `) commit;` alone at the end. Parses and executes, but reads as broken and makes every agent-facing describe noisier (Evora Factory Management: 297 lines that are only `);`)","cause":"Studio Pro stores an expression exactly as typed and users leave a trailing newline (or space) in the expression editor. The describer interpolated the stored text verbatim at ~40 sites; five of them had grown their own ad-hoc `TrimSuffix(v, \"\\n\")` / `TrimRight(v, \" \\t\\n\\r\")` (return, declare, decision, rule args, web-service timeout) while every neighbouring slot — change/create members, set, call arguments, list operations, log/message params, REST/DB params, widget Visible/Editable, page action args — kept the newline","file":"`mdl/executor/cmd_microflows_format_action.go` (`describeExpr`), `cmd_pages_describe_output.go`, `cmd_pages_describe_datasource.go`","insight":"**Per-site trims are the tell of a missing helper.** Each earlier fix was right for the slot it was reported against and left the class open; one `describeExpr` (TrimSpace — an expression cannot end inside a string literal, so outer whitespace never carries meaning; interior newlines are kept) closes it. Fixtures typed into tests never carry the trailing newline, so only describing a real Studio Pro-authored app surfaces it — grep describe output for lines starting in column 0 with `)` / `, ` / `;`, which the indenting describer never produces itself. **Idempotence consequence, measured on an Evora copy:** the visitor keeps an argument's trailing whitespace, so the old (ugly) output round-tripped byte-stably, while the trimmed output writes the expression back without the newline. ADR-0008's canon compares strings verbatim, so the first re-exec of a describe of such a flow reports `Replaced` and every later one `Unchanged` — a one-time whitespace-only rewrite, a fixpoint, not churn. Control: two describes of the same state differing only in whitespace; old-exec `Unchanged`, new-exec `Replaced` then `Unchanged`","refs":["Evora Factory Management","docs/13-decisions/0008-identity-and-idempotence.md"]} diff --git a/mdl-examples/bug-tests/describe-expression-trailing-newline.mdl b/mdl-examples/bug-tests/describe-expression-trailing-newline.mdl new file mode 100644 index 000000000..aee131519 --- /dev/null +++ b/mdl-examples/bug-tests/describe-expression-trailing-newline.mdl @@ -0,0 +1,49 @@ +-- describe: a stored expression's trailing newline put `)` / `;` on its own line +-- +-- Studio Pro stores an expression exactly as typed, and a newline left at the +-- end of the expression editor is common (Evora Factory Management: every +-- member of one create, dozens of change/call arguments). `describe microflow` +-- interpolated the stored text verbatim: +-- +-- change $Ticket (TicketState = BugTestNewline.TicketStatus.Open +-- ); +-- +-- The statements below store exactly that shape: the visitor keeps an +-- argument's trailing whitespace, so each value below is written with a +-- trailing "\n", as Studio Pro would have written it. +-- +-- Verify after exec: +-- ./bin/mxcli -p app.mpr -c "describe microflow BugTestNewline.ACT_Newline" +-- every `)` and `;` must follow its expression on the same line, e.g. +-- change $Ticket (TicketState = BugTestNewline.TicketStatus.Open); +-- and the multi-line `find` keeps its interior line break. +-- +-- Re-executing that describe output writes the expressions back WITHOUT the +-- newline, which is a real change to the document: the first re-exec reports +-- `Replaced microflow`, every one after it `Unchanged microflow`. + +create module BugTestNewline; + +create enumeration BugTestNewline.TicketStatus (Open 'Open', Closed 'Closed'); + +create persistent entity BugTestNewline.Ticket ( + Title: string(200), + TicketState: enumeration(BugTestNewline.TicketStatus) +); + +create or modify microflow BugTestNewline.ACT_Newline () +begin + $Ticket = create BugTestNewline.Ticket (Title = 'First' +, TicketState = BugTestNewline.TicketStatus.Closed +); + change $Ticket (TicketState = BugTestNewline.TicketStatus.Open +); + $List = create list of BugTestNewline.Ticket; + add $Ticket to $List; + $Found = find($List, $currentObject/Title = 'First' +and $currentObject/TicketState = BugTestNewline.TicketStatus.Open +); + log info node 'BugTest' 'Title {1}' with ({1} = $Ticket/Title +); + return; +end; diff --git a/mdl/executor/cmd_microflows_describe_expression_whitespace_test.go b/mdl/executor/cmd_microflows_describe_expression_whitespace_test.go new file mode 100644 index 000000000..18f86e4ae --- /dev/null +++ b/mdl/executor/cmd_microflows_describe_expression_whitespace_test.go @@ -0,0 +1,108 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// Studio Pro stores an expression exactly as typed, and users routinely leave +// a trailing newline (or space) in the expression editor. `describe` used to +// emit the stored text verbatim, so the closing `)` or `;` landed on a line of +// its own: +// +// change $IteratorSingleMCPTool (SyncState = MCPClient.ENUM_SyncState.Syncing +// ); +// +// The values below are copied from real documents (Evora Factory Management): +// the leading/trailing whitespace is layout, not meaning, and is dropped; +// interior newlines of a multi-line expression are the author's formatting and +// are kept. +func TestDescribeTrimsStoredExpressionWhitespace(t *testing.T) { + e := newTestExecutor() + cases := []struct { + name string + action microflows.MicroflowAction + want string + }{ + { + name: "change member", + action: µflows.ChangeObjectAction{ + ChangeVariable: "IteratorSingleMCPTool", + Changes: []*microflows.MemberChange{ + {AttributeQualifiedName: "MCPClient.SingleMCPTool.SyncState", Value: "MCPClient.ENUM_SyncState.Syncing\n"}, + }, + }, + want: "change $IteratorSingleMCPTool (SyncState = MCPClient.ENUM_SyncState.Syncing);", + }, + { + name: "create members, trailing newline and trailing space", + action: µflows.CreateObjectAction{ + EntityQualifiedName: "DigitalTwin.TechnicianTicket", + OutputVariable: "TechnicianTicket", + InitialMembers: []*microflows.MemberChange{ + {AttributeQualifiedName: "DigitalTwin.TechnicianTicket.Description", Value: "'Ticket for configuration change:' \n"}, + {AttributeQualifiedName: "DigitalTwin.TechnicianTicket.NeedsApproval", Value: "\ntrue\n"}, + }, + }, + want: "$TechnicianTicket = create DigitalTwin.TechnicianTicket (Description = 'Ticket for configuration change:', NeedsApproval = true);", + }, + { + name: "change variable (member path)", + action: µflows.ChangeVariableAction{ + VariableName: "$Order/Total", + Value: "$Total\r\n", + }, + want: "change $Order (Total = $Total);", + }, + { + name: "set variable", + action: µflows.ChangeVariableAction{VariableName: "Count", Value: "$Count + 1\n"}, + want: "set $Count = $Count + 1;", + }, + { + name: "call microflow argument", + action: µflows.MicroflowCallAction{ + MicroflowCall: µflows.MicroflowCall{ + Microflow: "AgentCommons.Version_Validate_BeforeRun", + ParameterMappings: []*microflows.MicroflowCallParameterMapping{ + {Parameter: "AgentCommons.Version_Validate_BeforeRun.Version", Argument: "$PageHelper/AgentCommons.PageHelper_Version/AgentCommons.Version\n"}, + }, + }, + UseReturnVariable: true, + ResultVariableName: "Valid", + }, + want: "$Valid = call microflow AgentCommons.Version_Validate_BeforeRun(Version = $PageHelper/AgentCommons.PageHelper_Version/AgentCommons.Version);", + }, + { + name: "find keeps interior newlines", + action: µflows.ListOperationAction{ + OutputVariable: "Next", + Operation: µflows.FindOperation{ + ListVariable: "List", + Expression: "$currentObject/IsEnabled\nand $currentObject/IsEdited = false\n", + }, + }, + want: "$Next = find($List, $currentObject/IsEnabled\nand $currentObject/IsEdited = false);", + }, + { + name: "log template parameter", + action: µflows.LogMessageAction{ + LogLevel: "Info", + LogNodeName: "'Node'\n", + TemplateParameters: []string{"$Order/Number\n"}, + }, + want: "log info node 'Node' 'Message' with ({1} = $Order/Number);", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := e.formatAction(tc.action, nil, nil) + if got != tc.want { + t.Errorf("got %q\nwant %q", got, tc.want) + } + }) + } +} diff --git a/mdl/executor/cmd_microflows_format_action.go b/mdl/executor/cmd_microflows_format_action.go index 63ddb44be..e8de99f43 100644 --- a/mdl/executor/cmd_microflows_format_action.go +++ b/mdl/executor/cmd_microflows_format_action.go @@ -51,6 +51,23 @@ func refreshModifier(refresh bool) string { return "" } +// describeExpr renders a stored Mendix expression for describe output. +// +// Studio Pro stores an expression exactly as typed, and a trailing newline (or +// space) left in the expression editor is common in real projects. Emitted +// verbatim it puts the closing `)` or `;` of the MDL statement on a line of its +// own. Leading and trailing whitespace is never part of an expression's meaning +// — an expression cannot end inside a string literal — so it is dropped here, +// in the describer, and never in the stored model. Interior newlines are the +// author's formatting of a multi-line expression and are kept. +// +// Every describe site that interpolates a stored expression goes through this +// helper; the ad-hoc TrimSuffix/TrimRight calls it replaced each covered one +// slot and let the neighbouring ones drift. +func describeExpr(v string) string { + return strings.TrimSpace(v) +} + // escapeExpressionValue escapes raw control characters inside string literals // of a Mendix expression value so it can be safely embedded in MDL output. // The lexer's STRING_LITERAL rule forbids raw \r and \n inside single-quoted @@ -105,7 +122,7 @@ func formatActivity( case *microflows.EndEvent: if activity.ReturnValue != "" { - returnVal := strings.TrimSuffix(activity.ReturnValue, "\n") + returnVal := describeExpr(activity.ReturnValue) // Only add $ prefix for bare identifiers (no operators, quotes, or parens) if !strings.HasPrefix(returnVal, "$") && !isMendixKeyword(returnVal) && !isQualifiedEnumLiteral(returnVal) && !strings.ContainsAny(returnVal, "+'\"()") && !isNumericLiteral(returnVal) { @@ -139,7 +156,7 @@ func formatActivity( case *microflows.LoopedActivity: switch ls := activity.LoopSource.(type) { case *microflows.WhileLoopCondition: - return fmt.Sprintf("while %s", ls.WhileExpression) + return fmt.Sprintf("while %s", describeExpr(ls.WhileExpression)) case *microflows.IterableList: iterVar := "Item" listVar := "List" @@ -232,7 +249,7 @@ func formatAction( if a.DataType != nil { varType = formatMicroflowDataType(ctx, a.DataType, entityNames) } - initialValue := strings.TrimSuffix(a.InitialValue, "\n") + initialValue := describeExpr(a.InitialValue) if initialValue == "" { initialValue = "empty" } @@ -254,13 +271,13 @@ func formatAction( if !strings.HasPrefix(objectName, "$") { objectName = "$" + objectName } - return fmt.Sprintf("change %s (%s = %s);", objectName, attrName, a.Value) + return fmt.Sprintf("change %s (%s = %s);", objectName, attrName, describeExpr(a.Value)) } // Simple variable change if strings.HasPrefix(varName, "$") { - return fmt.Sprintf("set %s = %s;", varName, a.Value) + return fmt.Sprintf("set %s = %s;", varName, describeExpr(a.Value)) } - return fmt.Sprintf("set $%s = %s;", varName, a.Value) + return fmt.Sprintf("set $%s = %s;", varName, describeExpr(a.Value)) case *microflows.CreateObjectAction: // Use EntityQualifiedName (BY_NAME_REFERENCE) or fall back to EntityID lookup @@ -313,7 +330,7 @@ func formatAction( memberName = parts[len(parts)-1] } } - members = append(members, fmt.Sprintf("%s = %s", memberName, escapeExpressionValue(m.Value))) + members = append(members, fmt.Sprintf("%s = %s", memberName, escapeExpressionValue(describeExpr(m.Value)))) } return fmt.Sprintf("$%s = create %s (%s)%s%s;", outputVar, entityName, strings.Join(members, ", "), commitModifier(a.Commit), refreshModifier(a.RefreshInClient)) } @@ -343,7 +360,7 @@ func formatAction( memberName = parts[len(parts)-1] } } - members = append(members, fmt.Sprintf("%s = %s", memberName, escapeExpressionValue(m.Value))) + members = append(members, fmt.Sprintf("%s = %s", memberName, escapeExpressionValue(describeExpr(m.Value)))) } return fmt.Sprintf("change $%s (%s)%s%s;", varName, strings.Join(members, ", "), commitModifier(a.Commit), refreshModifier(a.RefreshInClient)) } @@ -385,13 +402,13 @@ func formatAction( varName := a.ChangeVariable switch a.Type { case microflows.ChangeListTypeAdd: - return fmt.Sprintf("add %s to $%s;", a.Value, varName) + return fmt.Sprintf("add %s to $%s;", describeExpr(a.Value), varName) case microflows.ChangeListTypeRemove: - return fmt.Sprintf("remove %s from $%s;", a.Value, varName) + return fmt.Sprintf("remove %s from $%s;", describeExpr(a.Value), varName) case microflows.ChangeListTypeClear: return fmt.Sprintf("clear $%s;", varName) case microflows.ChangeListTypeSet: - return fmt.Sprintf("set $%s = %s;", varName, a.Value) + return fmt.Sprintf("set $%s = %s;", varName, describeExpr(a.Value)) default: return fmt.Sprintf("change list $%s (%s);", varName, a.Type) } @@ -428,17 +445,17 @@ func formatAction( // are required, so they are rendered even when empty rather than dropped — // a reduce that describes without them cannot be executed back (#1004). if a.Function == microflows.AggregateFunctionReduce { - initial := a.ReduceInitialValue + initial := describeExpr(a.ReduceInitialValue) if initial == "" { initial = "empty" } return fmt.Sprintf("$%s = reduce($%s, %s, initial: %s, returns: %s);", - outputVar, a.InputVariable, a.Expression, initial, + outputVar, a.InputVariable, describeExpr(a.Expression), initial, formatMicroflowDataType(ctx, a.ReduceReturnType, entityNames)) } // Expression-based aggregate: SUM($list, $currentObject/Attr + 1) if a.UseExpression && a.Expression != "" { - return fmt.Sprintf("$%s = %s($%s, %s);", outputVar, fn, a.InputVariable, a.Expression) + return fmt.Sprintf("$%s = %s($%s, %s);", outputVar, fn, a.InputVariable, describeExpr(a.Expression)) } // Attribute-based aggregate: SUM($list.Attr) if attrName != "" && a.Function != microflows.AggregateFunctionCount { @@ -562,7 +579,7 @@ func formatAction( } // Node is an expression in Mendix (e.g., 'TEST' or $variable or 'Prefix' + $var) // Output it as-is since it's already stored as an expression - node := a.LogNodeName + node := describeExpr(a.LogNodeName) if node == "" { node = defaultLogNodeExpression } @@ -576,7 +593,7 @@ func formatAction( if len(a.TemplateParameters) > 0 { var params []string for i, expr := range a.TemplateParameters { - params = append(params, fmt.Sprintf("{%d} = %s", i+1, expr)) + params = append(params, fmt.Sprintf("{%d} = %s", i+1, describeExpr(expr))) } withClause = fmt.Sprintf(" with (%s)", strings.Join(params, ", ")) } @@ -599,7 +616,7 @@ func formatAction( if idx := strings.LastIndex(paramName, "."); idx != -1 { paramName = paramName[idx+1:] } - params = append(params, fmt.Sprintf("%s = %s", paramName, pm.Argument)) + params = append(params, fmt.Sprintf("%s = %s", paramName, describeExpr(pm.Argument))) } } @@ -632,7 +649,7 @@ func formatAction( if idx := strings.LastIndex(paramName, "."); idx != -1 { paramName = paramName[idx+1:] } - params = append(params, fmt.Sprintf("%s = %s", paramName, pm.Argument)) + params = append(params, fmt.Sprintf("%s = %s", paramName, describeExpr(pm.Argument))) } } @@ -667,12 +684,12 @@ func formatAction( valueStr = mdlQuote(v.TypedTemplate.Text) } case *microflows.ExpressionBasedCodeActionParameterValue: - valueStr = v.Expression + valueStr = describeExpr(v.Expression) case *microflows.BasicCodeActionParameterValue: if v.Argument == "" { valueStr = "empty" } else { - valueStr = v.Argument + valueStr = describeExpr(v.Argument) } case *microflows.MicroflowParameterValue: if v.Microflow != "" { @@ -713,7 +730,7 @@ func formatAction( var params []string for _, pm := range a.ParameterMappings { - params = append(params, fmt.Sprintf("%s = %s", pm.ParameterName, pm.Argument)) + params = append(params, fmt.Sprintf("%s = %s", pm.ParameterName, describeExpr(pm.Argument))) } paramStr := "" @@ -752,7 +769,7 @@ func formatAction( // Extract just the parameter name from the qualified name parts := strings.Split(pm.Parameter, ".") paramName := parts[len(parts)-1] - params = append(params, fmt.Sprintf("$%s = %s", paramName, pm.Argument)) + params = append(params, fmt.Sprintf("$%s = %s", paramName, describeExpr(pm.Argument))) } // Build the statement @@ -782,7 +799,11 @@ func formatAction( } result := fmt.Sprintf("show message %s type %s", message, msgType) if len(a.TemplateParameters) > 0 { - result += " objects [" + strings.Join(a.TemplateParameters, ", ") + "]" + objs := make([]string, len(a.TemplateParameters)) + for i, p := range a.TemplateParameters { + objs[i] = describeExpr(p) + } + result += " objects [" + strings.Join(objs, ", ") + "]" } // Without this, a describe -> exec round trip turned a BLOCKING message // box into a non-blocking one. The model carried Blocking on both @@ -943,9 +964,9 @@ func formatAction( valueStr = mdlQuote(v.TypedTemplate.Text) } case *microflows.ExpressionBasedCodeActionParameterValue: - valueStr = v.Expression + valueStr = describeExpr(v.Expression) case *microflows.BasicCodeActionParameterValue: - valueStr = v.Argument + valueStr = describeExpr(v.Argument) case *microflows.EntityTypeCodeActionParameterValue: valueStr = v.Entity } @@ -1038,7 +1059,7 @@ func formatWebServiceCallAction(ctx *ExecContext, a *microflows.WebServiceCallAc parts = append(parts, "receive mapping "+formatWebServiceReference(string(a.ReceiveMappingID))) } if a.TimeoutExpression != "" { - parts = append(parts, "timeout "+strings.TrimRight(a.TimeoutExpression, " \t\n\r")) + parts = append(parts, "timeout "+describeExpr(a.TimeoutExpression)) } return strings.Join(parts, "\n") + ";" } @@ -1059,7 +1080,7 @@ func formatWebServiceArguments(args []microflows.WebServiceArgument) string { if arg.Name == "" { return "" } - parts = append(parts, arg.Name+" = "+arg.Expression) + parts = append(parts, arg.Name+" = "+describeExpr(arg.Expression)) } return strings.Join(parts, ", ") } @@ -1122,9 +1143,9 @@ func formatListOperation(ctx *ExecContext, op microflows.ListOperation, outputVa case *microflows.TailOperation: return fmt.Sprintf("$%s = tail($%s);", outputVar, o.ListVariable) case *microflows.FindOperation: - return fmt.Sprintf("$%s = find($%s, %s);", outputVar, o.ListVariable, o.Expression) + return fmt.Sprintf("$%s = find($%s, %s);", outputVar, o.ListVariable, describeExpr(o.Expression)) case *microflows.FilterOperation: - return fmt.Sprintf("$%s = filter($%s, %s);", outputVar, o.ListVariable, o.Expression) + return fmt.Sprintf("$%s = filter($%s, %s);", outputVar, o.ListVariable, describeExpr(o.Expression)) case *microflows.SortOperation: if len(o.Sorting) > 0 { var sortCols []string @@ -1162,26 +1183,26 @@ func formatListOperation(ctx *ExecContext, op microflows.ListOperation, outputVa case *microflows.FindByAttributeOperation: fieldName := extractFieldName(o.Attribute, o.Association) if fieldName != "" && o.Expression != "" { - return fmt.Sprintf("$%s = find($%s, %s = %s);", outputVar, o.ListVariable, fieldName, o.Expression) + return fmt.Sprintf("$%s = find($%s, %s = %s);", outputVar, o.ListVariable, fieldName, describeExpr(o.Expression)) } else if o.Expression != "" { - return fmt.Sprintf("$%s = find($%s, %s);", outputVar, o.ListVariable, o.Expression) + return fmt.Sprintf("$%s = find($%s, %s);", outputVar, o.ListVariable, describeExpr(o.Expression)) } return fmt.Sprintf("-- $%s = find($%s) — missing attribute/expression", outputVar, o.ListVariable) case *microflows.FilterByAttributeOperation: fieldName := extractFieldName(o.Attribute, o.Association) if fieldName != "" && o.Expression != "" { - return fmt.Sprintf("$%s = filter($%s, %s = %s);", outputVar, o.ListVariable, fieldName, o.Expression) + return fmt.Sprintf("$%s = filter($%s, %s = %s);", outputVar, o.ListVariable, fieldName, describeExpr(o.Expression)) } else if o.Expression != "" { - return fmt.Sprintf("$%s = filter($%s, %s);", outputVar, o.ListVariable, o.Expression) + return fmt.Sprintf("$%s = filter($%s, %s);", outputVar, o.ListVariable, describeExpr(o.Expression)) } return fmt.Sprintf("-- $%s = filter($%s) — missing attribute/expression", outputVar, o.ListVariable) case *microflows.ListRangeOperation: if o.OffsetExpression != "" && o.LimitExpression != "" { - return fmt.Sprintf("$%s = range($%s, %s, %s);", outputVar, o.ListVariable, o.OffsetExpression, o.LimitExpression) + return fmt.Sprintf("$%s = range($%s, %s, %s);", outputVar, o.ListVariable, describeExpr(o.OffsetExpression), describeExpr(o.LimitExpression)) } else if o.OffsetExpression != "" { - return fmt.Sprintf("$%s = range($%s, %s);", outputVar, o.ListVariable, o.OffsetExpression) + return fmt.Sprintf("$%s = range($%s, %s);", outputVar, o.ListVariable, describeExpr(o.OffsetExpression)) } else if o.LimitExpression != "" { - return fmt.Sprintf("$%s = range($%s, 0, %s);", outputVar, o.ListVariable, o.LimitExpression) + return fmt.Sprintf("$%s = range($%s, 0, %s);", outputVar, o.ListVariable, describeExpr(o.LimitExpression)) } return fmt.Sprintf("$%s = range($%s);", outputVar, o.ListVariable) default: @@ -1277,7 +1298,7 @@ func formatRestCallAction(ctx *ExecContext, a *microflows.RestCallAction) string if i > 0 { sb.WriteString(", ") } - sb.WriteString(fmt.Sprintf("{%d} = %s", i+1, param)) + sb.WriteString(fmt.Sprintf("{%d} = %s", i+1, describeExpr(param))) } sb.WriteString(")") } @@ -1288,7 +1309,7 @@ func formatRestCallAction(ctx *ExecContext, a *microflows.RestCallAction) string sb.WriteString("\n header ") sb.WriteString(mdlQuote(h.Name)) sb.WriteString(" = ") - sb.WriteString(h.Value) + sb.WriteString(describeExpr(h.Value)) } } @@ -1314,7 +1335,7 @@ func formatRestCallAction(ctx *ExecContext, a *microflows.RestCallAction) string if i > 0 { sb.WriteString(", ") } - sb.WriteString(fmt.Sprintf("{%d} = %s", i+1, param)) + sb.WriteString(fmt.Sprintf("{%d} = %s", i+1, describeExpr(param))) } sb.WriteString(")") } @@ -1417,14 +1438,14 @@ func formatRestOperationCallAction(ctx *ExecContext, a *microflows.RestOperation if idx := strings.LastIndex(name, "."); idx >= 0 { name = name[idx+1:] } - allParams = append(allParams, struct{ name, value string }{name, pm.Value}) + allParams = append(allParams, struct{ name, value string }{name, describeExpr(pm.Value)}) } for _, qm := range a.QueryParameterMappings { name := qm.Parameter if idx := strings.LastIndex(name, "."); idx >= 0 { name = name[idx+1:] } - allParams = append(allParams, struct{ name, value string }{name, qm.Value}) + allParams = append(allParams, struct{ name, value string }{name, describeExpr(qm.Value)}) } if len(allParams) > 0 { sb.WriteString("\n with (") @@ -1475,7 +1496,7 @@ func formatExecuteDatabaseQueryAction(ctx *ExecContext, a *microflows.ExecuteDat if i > 0 { sb.WriteString(", ") } - sb.WriteString(fmt.Sprintf("%s = %s", pm.ParameterName, pm.Value)) + sb.WriteString(fmt.Sprintf("%s = %s", pm.ParameterName, describeExpr(pm.Value))) } sb.WriteString(")") } @@ -1487,7 +1508,7 @@ func formatExecuteDatabaseQueryAction(ctx *ExecContext, a *microflows.ExecuteDat if i > 0 { sb.WriteString(", ") } - sb.WriteString(fmt.Sprintf("%s = %s", cm.ParameterName, cm.Value)) + sb.WriteString(fmt.Sprintf("%s = %s", cm.ParameterName, describeExpr(cm.Value))) } sb.WriteString(")") } @@ -1846,7 +1867,7 @@ func (e *Executor) formatRestCallAction(a *microflows.RestCallAction) string { func formatSplitCondition(cond microflows.SplitCondition) string { switch c := cond.(type) { case *microflows.ExpressionSplitCondition: - expr := strings.TrimRight(c.Expression, " \t\n\r") + expr := describeExpr(c.Expression) if expr == "" { return "true" } @@ -1862,7 +1883,7 @@ func formatSplitCondition(cond microflows.SplitCondition) string { if idx := strings.LastIndex(paramName, "."); idx >= 0 { paramName = paramName[idx+1:] } - arg := strings.TrimRight(pm.Argument, " \t\n\r") + arg := describeExpr(pm.Argument) if paramName != "" { args = append(args, fmt.Sprintf("%s = %s", paramName, arg)) } else { diff --git a/mdl/executor/cmd_pages_describe_datasource.go b/mdl/executor/cmd_pages_describe_datasource.go index a844e05e4..63bb53f33 100644 --- a/mdl/executor/cmd_pages_describe_datasource.go +++ b/mdl/executor/cmd_pages_describe_datasource.go @@ -489,7 +489,7 @@ func flowSourceArgs(ds map[string]any, settingsKey, flowName string) []rawDataSo if name == "" { continue } - value := extractString(mapping["Expression"]) + value := describeExpr(extractString(mapping["Expression"])) if value == "" { value = pageVariableArgValue(mapping["Variable"]) } diff --git a/mdl/executor/cmd_pages_describe_output.go b/mdl/executor/cmd_pages_describe_output.go index 4ee83ba43..76a2ff26f 100644 --- a/mdl/executor/cmd_pages_describe_output.go +++ b/mdl/executor/cmd_pages_describe_output.go @@ -119,13 +119,13 @@ func appendDataGridPagingProps(props []string, w rawWidget) []string { // appendConditionalProps appends VISIBLE IF and EDITABLE IF if present. func appendConditionalProps(props []string, w rawWidget) []string { if w.VisibleIf != "" { - props = append(props, fmt.Sprintf("Visible: [%s]", w.VisibleIf)) + props = append(props, fmt.Sprintf("Visible: [%s]", describeExpr(w.VisibleIf))) } if prop := visibleWhenProp(w); prop != "" { props = append(props, prop) } if w.EditableIf != "" { - props = append(props, fmt.Sprintf("Editable: [%s]", w.EditableIf)) + props = append(props, fmt.Sprintf("Editable: [%s]", describeExpr(w.EditableIf))) } return props } @@ -187,13 +187,13 @@ func appendAppearanceProps(props []string, w rawWidget) []string { props = append(props, formatDesignPropertiesMDL(w.DesignProperties)) } if w.VisibleIf != "" { - props = append(props, fmt.Sprintf("Visible: [%s]", w.VisibleIf)) + props = append(props, fmt.Sprintf("Visible: [%s]", describeExpr(w.VisibleIf))) } if prop := visibleWhenProp(w); prop != "" { props = append(props, prop) } if w.EditableIf != "" { - props = append(props, fmt.Sprintf("Editable: [%s]", w.EditableIf)) + props = append(props, fmt.Sprintf("Editable: [%s]", describeExpr(w.EditableIf))) } return props } @@ -1632,7 +1632,7 @@ func extractPageParameters(ctx *ExecContext, settings map[string]any) string { // Check for Argument (variable reference or expression stored as string) if value == "" { if arg := extractString(mappingMap["Argument"]); arg != "" { - value = arg // e.g., "$Product" or an expression + value = describeExpr(arg) // e.g., "$Product" or an expression } } @@ -1689,7 +1689,7 @@ func extractMicroflowParameters(ctx *ExecContext, settings map[string]any) strin // Check for Expression (used in Pages$MicroflowParameterMapping) if value == "" { if expr := extractString(mappingMap["Expression"]); expr != "" { - value = expr // e.g., "$Product" or an expression + value = describeExpr(expr) // e.g., "$Product" or an expression } } @@ -1748,7 +1748,7 @@ func extractNanoflowParameters(ctx *ExecContext, action map[string]any) string { // Check for Expression (used in Pages$NanoflowParameterMapping) if value == "" { if expr := extractString(mappingMap["Expression"]); expr != "" { - value = expr // e.g., "$Product" or an expression + value = describeExpr(expr) // e.g., "$Product" or an expression } } @@ -1823,6 +1823,7 @@ func extractClientTemplateParameters(ctx *ExecContext, w map[string]any, fieldNa suffixes = append(suffixes, formatParamFormatSuffix(pMap)) // Check for Expression first (literal value) if expr, ok := pMap["Expression"].(string); ok && expr != "" { + expr = describeExpr(expr) // A non-String attribute binding (Integer/DateTime/…) is written as // `toString($currentObject/Attr)` / `toString($param/Attr)` — see // resolveTemplateAttributePathFull. Emit it back as the bare attribute From b70bcdb4067568f892d317b149b622af03ef481c Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 26 Sep 2026 20:25:09 +0000 Subject: [PATCH 8/9] test(refs): MDL repro for the reference-graph gaps, and findings Co-Authored-By: Claude Opus 5.5 --- .../fix-issue/findings/mdl-executor.jsonl | 1 + .../skills/fix-issue/findings/mdl-other.jsonl | 1 + .../refs-graph-members-enums-workflows.mdl | 117 ++++++++++++++++++ 3 files changed, 119 insertions(+) create mode 100644 mdl-examples/bug-tests/refs-graph-members-enums-workflows.mdl diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 8bb82beaa..5c68efc42 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -713,3 +713,4 @@ {"area": "mdl/executor", "date": "2026-09-25", "symptom": "`textbox t (Attribute: FullName)` at the top of a page (CREATE PAGE/SNIPPET, a plain container, or ALTER PAGE … INSERT at page level) passed plain `mxcli check`, `exec --no-check`/ALTER reported success, and `bson dump` showed `AttributeRef: null` — mxbuild 11.13.0: CE0544 \"This widget can only function inside a data context\" + CE7005 (textbox/textarea/datepicker/checkbox/radiobuttons/dropdown), CE0402 (dynamictext Attribute:), CE0642 (combobox). Qualified `Mod.Ent.Attr` there is stored and fails CE0544/CE2421/CE1365/CE7247 \"Move this widget into a data container\" + CE7006. `Attribute: $P/Attr` / `$currentObject/Attr` dropped even INSIDE a data view.", "cause": "resolveAttributePath returns the bare name when entityContext is \"\", and attributeRefToGen (and widgetobj setAttributeRefField) write nil for any path with < 2 dots, so the binding vanished between builder and writer; refuseBareAttributeRefs never sees it because no Attribute string is emitted. The only refusal (validatePageContextTree) runs in the --references phase for CREATE PAGE/SNIPPET, so plain check, --no-check and ALTER were unguarded. `$x/Attr` parses via the generic property rule as an *ast.DataSourceV3, so GetAttribute() returns \"\" and every builder skipped it.", "file": "mdl/executor/cmd_pages_input_binding_context.go (inputBindingProblem, checkInputBinding, validateInputBindingContext = MDL-WIDGET34), wired in cmd_pages_builder_v3_widgets.go (6 input builders + buildDynamicTextV3), widget_engine.go (primary Attribute mapping), validate_widgets.go (validateWidgetTreeIn); tests cmd_pages_input_binding_context_test.go; bug-tests input-binding-without-context{,.fail}.mdl", "insight": "Reuse the MDL-PAGEARG01 three-state context (pageArgContext known/present) rather than entityContext==\"\" as the 'outside a data container' signal: entityContext is also empty INSIDE a container whose flow cannot be resolved (excluded ShareFeedback_Logo), where DESCRIBE writes qualified names that must keep building — refusing qualified-on-empty-entity would have broken that round trip. So known-absent context refuses bare AND qualified; unknown context (ALTER) refuses only the bare name the writer provably nulls. Two existing unit tests (OnChangeSurvivesBuilder, DynamicTextV3_AttributeBinds) built inputs with NO entity and passed — the second asserted a bare `Title` AttributeRef counted as 'bound', i.e. it pinned the bug: when a fixture has no entity context, ask what the writer does with its output. The `$P/Attr` drop was found only by dumping the control page, not from the report — print the AST value type with a probe test before assuming a spelling reaches the builder. Evidence: 22 mxbuild errors before on the probe matrix; after, every case refused with nothing written, controls (dataview/listview/gallery/datagrid/snippet dataview/ALTER into dataview) 0 errors, 17/17 stock pages + 4/4 snippets describe→exec round trip.", "refs": ["MDL-WIDGET34"], "ce": ["CE0544", "CE7005", "CE0402", "CE0642", "CE2421", "CE1365", "CE7247", "CE7006"]} {"area":"mdl/executor","date":"2026-09-25","symptom":"`alter page FeedbackModule.ShareFeedback_Logo { insert after textBox1 { image zzImg (ImageType: imageUrl, ImageUrl: '{1}', ImageUrlParams: [{1} = ImageB64]) } }` passed `check --references`; exec wrote a bare AttributeRef and `mx check` could not LOAD the project (ArgumentNullException setting 'Attribute'). Same at page top level outside any data container; a text box's `Attribute:` there is silently dropped (CE7005).","cause":"ALTER's entity context comes from the STORED document (nearest enclosing data source, or a flow source's return type via resolveDataSourceFlowEntity). With the flow missing (Feedback v4.0.2 ships no DS_FeedbackForm) or no container at all, entityContext is \"\" and resolveAttributePath returns the bare name. CREATE PAGE refused this at check time (relaxExcludedWidgetRefs/unscopedBindings, #678); ALTER's check never opened the document, so nothing could know the scope.","file":"mdl/executor/validate_alter_unscoped.go, mdl/executor/cmd_alter_page.go (alterEntityContext), mdl/executor/validate.go (bindingsWithoutScope)","insight":"For ALTER, scope is a property of the stored document, not the statement: a check-time question about it must open the document (OpenPageForMutation, never Save; validate_alter_set.go already does this) and ask through the SAME function exec uses, so the INSERT/REPLACE entity resolution was lifted into alterEntityContext rather than restated, and the binding walk lifted out of unscopedBindings (bindingsWithoutScope) rather than copied. Controls that keep it from blocking working scripts: skip a target the stored doc lacks (added earlier in the script), a flow the script declares with an entity return (sc.flowParams), DataGrid2 column and list-view-template paths, and documents the script creates. Reproduce with a Studio Pro-authored page whose flow is genuinely absent.","refs":["#678","#685"]} {"area": "mdl/executor", "date": "2026-09-25", "symptom": "`describe page` on a File Uploader (files mode) emits `DataSource: association …`, and exec of that output fails: \"widget `upFiles` (fileuploader) exposes 2 datasources, so a generic `datasource:` clause is ambiguous — name the one you mean: associatedFiles, associatedImages\"", "cause": "DESCRIBE chose generic vs named by counting CONFIGURED datasources (namedCustomWidgetDataSources drops unset ones, so files mode = 1), while the builder's refuseAmbiguousGenericDataSource counts DECLARED datasource mappings (a generated def maps every top-level datasource = 2). Two sides of one round trip deciding the same question from different evidence.", "file": "mdl/executor/cmd_pages_describe_parse.go", "insight": "When describe and build each decide 'is this ambiguous?', they must count the same set. Fix read the DECLARED count from the stored schema (PropertyTypes with ValueType.Type=DataSource, excluding IsLinked) and excluded widgets with an embedded .def.json — those are hand-written and pick one datasource mapping per mode, so a database-mode ComboBox (two declared) must keep the generic clause. The #956 bug-test script itself authored the refused generic clause on a File Uploader: a bug-test that only runs `mxcli check` without a project can't see an exec-time refusal, so grep bug-tests for the old spelling whenever a builder starts refusing one.", "refs": ["mendixlabs/mxcli#1199", "mendixlabs/mxcli#956", "mendixlabs/mxcli#1109"], "rules": []} +{"area": "mdl/executor", "date": "2026-09-26", "symptom": "`refs`/`impact` printed a row per edge (a microflow with two retrieves of an entity listed twice), the impact summary counted rows (\"MICROFLOW: 9\" over six microflows) in map order that changed between runs, and `context Mod.Entity` said \"Related Entities: (none found)\" for an entity with five associations.", "cause": "No DISTINCT; summary built by ranging over a map of row counts; the context query read refs rows whose SOURCE is an entity, but an association edge's source is the ASSOCIATION, so only generalizations could ever match.", "file": "mdl/executor/cmd_search.go (showReferences, showImpact), reference_target.go (refTargetWhere, noReferencesMessage), cmd_context.go (assembleEntityContext, assembleEnumerationContext)", "insight": "An empty answer from a partial graph must say what was searched, not 'not referenced' \u2014 the wording is part of the correctness of a reference tool, because the caller acts on it by deleting. For attributes and enumeration values the message names the unresolved sites (member via a variable in an expression; decision branch on an enum, which stores the bare value name) and says to run `search`. Impact on an enumeration must include its values (TargetName LIKE 'Enum.%' with ESCAPE, since ENUM_ names contain LIKE's `_` wildcard). For related entities read the associations table, which has both ends and exists in a fast catalog. The exec* functions call ensureCatalog, which needs a connection, so the query/format half is split out (showReferences/showImpact) to be testable against a seeded catalog.New()."} diff --git a/.claude/skills/fix-issue/findings/mdl-other.jsonl b/.claude/skills/fix-issue/findings/mdl-other.jsonl index 80bffaba6..286beca7f 100644 --- a/.claude/skills/fix-issue/findings/mdl-other.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-other.jsonl @@ -73,3 +73,4 @@ {"area": "mdl/catalog", "date": "2026-09-25", "symptom": "Two import mappings with the same name, one excluded and one live (valid: mx check 0 errors), show up in CATALOG.IMPORT_MAPPINGS and CATALOG.SOURCE as two indistinguishable rows — integer Ids 2 and 3, no Excluded column, and both source rows the same `create or modify import mapping …` text with no `@excluded` prefix (mendixlabs/mxcli#1185)", "cause": "Three gaps: describeImportMapping/describeExportMapping never printed @excluded; import_mappings_data/export_mappings_data used an AUTOINCREMENT Id and recorded no Excluded; and buildSource enumerates DOCUMENTS but called the describe callback with only a qualified name, so every describer resolved by name and rendered the preferred twin for both rows. The last one was not mapping-specific — microflow/nanoflow/rule/page twins had identical source rows too, masked because the microflows table itself carries Id + Excluded", "file": "`mdl/catalog/builder_source.go` (`sourceItem.id`, `ElementId`), `mdl/catalog/builder.go` (`DescribeFunc` gains id), `mdl/catalog/tables.go` + `builder_modules.go` (mapping Id/Excluded, schema 14), `mdl/executor/excluded_docs.go` (`pickDescribed`, `describedMapping`), `mdl/executor/cmd_catalog.go` (`ExecContext.describeID`), `mdl/executor/cmd_{import,export}_mappings.go`, `mdl/visitor/visitor_import_export_mapping.go`, `mdl/backend/modelsdk/mapping_write.go` (by-name lookup prefers the live twin)", "insight": "**A collector that walks documents must not hand a NAME to a callback that resolves names** — #914's pickLive is correct for an interactive DESCRIBE and exactly wrong for a per-document sweep, because it maps both twins onto one. Pin by ID (ExecContext.describeID) and fall back to pickLive. **Printing a new annotation in DESCRIBE obliges the write side**: `@excluded` on a mapping was MDL059 until the visitor read it and documentAnnotations listed it; skipping that turns describe->exec into a refusal. **Check CatalogSchemaVersion against its own history comment**: the history recorded 13 (entity event handlers) while the constant said \"12\" — parallel bumps, the merge kept the lower — so caches built at 12 never rebuilt; bumping to 14 carries both. **Making a twin for a real run**: MDL cannot (CREATE refuses a taken name, no RENAME for mappings); `@excluded create` under a second name, then rewrite that unit's BSON Name (bson.D round-trip of the .mxunit) — mx check stays 0 errors, which is the reporter's state. Pre-fix binary on that project: two rows Id 2/3 and identical source text; fixed: document Ids, Excluded 0/1, excluded row starts `@excluded`. Controls: disable the describeID pin (4 executor tests fail, microflow included), drop the @excluded print, pass \"\" as id in buildSource (1 source row, not 2), stash tables.go/builder_modules.go (no such column: Excluded)", "refs": ["mendixlabs/mxcli#1185", "#914"]} {"area": "mdl/linter", "date": "2026-09-25", "symptom": "No way to ask a structural question about ONE document: `lint` scopes by module and by rule but not by document, so validating a microflow after each `exec` meant a project-wide lint (~13 s measured by the reporter) plus a baseline diff to see which findings were new. At that price the gate gets batched to once per session \u2014 three CONV011 violations shipped under a clean mxbuild log, six across three microflows before anyone looked.", "cause": "Feature gap, but the shape of the fix is not obvious: every rule guards its expensive per-document read (`FullMicroflow`) with `IsExcluded(moduleName)`, so module scoping already skipped the costly part for other modules \u2014 the missing granularity was WITHIN a module.", "file": "`mdl/linter/context.go` (`SetIncludedDocuments`, `IsDocumentExcluded`, `documentFilterSQL`; `Microflows`/`Pages`/`Widgets` narrowed in SQL; new `includeActive` flag), `cmd/mxcli/cmd_lint.go` + `main.go` (`-d/--documents`), `cmd/mxcli/lint_document_filter.go` (violation post-filter), tests `mdl/linter/context_document_filter_test.go`", "insight": "**Scope in the ITERATOR, not in each rule** \u2014 one SQL predicate on `Microflows()` covers CONV011, MPR002, CONV010, QUAL003 and every other rule that walks it, with no rule edited and no second copy of the filter to drift. **Two traps, both found by tests I wrote and then tried to break.** (1) An empty inclusion map means 'no filter' to `IsExcluded`, so intersecting `--modules A` with `--documents B.C` \u2014 an empty set \u2014 made lint scan the WHOLE project instead of nothing; distinguishing 'no allowlist' from 'allowlist that matched nothing' needs an explicit bool, not `len(map) > 0`. (2) The narrowing test PASSED against code with the SQL filter reverted, because the shared fixture has one microflow per module and the module implication alone explained the result \u2014 a sibling document in the SAME module is the only fixture that isolates document narrowing from module narrowing. Prove-by-revert is what caught both; the second would otherwise have shipped a test that could never fail. Also: a rule that reports from project settings rather than a document iterator is untouched by SQL narrowing, so a scoped run needs a violation post-filter too \u2014 and it must accept BOTH spellings of Location.DocumentName, since CONV010 sets the qualified name and MPR011 the short one.", "refs": ["ako/mxcli#681", "mendixlabs/mxcli#1186"]} {"area": "mdl/catalog", "date": "2026-09-25", "symptom": "java_actions.ReturnType showed a type-parameter return as the type parameter's own name: 'TypeParameter', 'TypeParEntity', 'FileTypeDocument' depending on what the modeler called it, and a type parameter named `String` (Studio Pro allows it) read back as 'String', identical to the primitive. java_action_parameters.ParameterType had the same ambiguity for both the object parameter and the entity-type selector.", "cause": "The catalog stored TypeString(), which is DESCRIBE's MDL rendering: a bare type-parameter reference IS its name in MDL syntax, so the value carried no marker that it was a type parameter at all.", "file": "mdl/catalog/builder_modules.go (catalogCodeActionType)", "insight": "The report reads like three inconsistent conventions ('TypeParameter' / 'TypeParEntity' / a name) but it is one: every value was the modeler's chosen name, and 'TypeParameter' is merely Studio Pro's default. Fix the encoding in the catalog only (`TypeParameter:`, `EntityTypeParameter:`, the `Kind:Name` shape microflows_data already uses; no primitive contains a colon) \u2014 changing TypeString() would change DESCRIBE output, where the bare name is the syntax. Test with the name colliding with a primitive and a primitive action as control: a unit test on the builder with MockBackend.ListJavaActionsFullFunc, plus an end-to-end catalog query on testdata/expr-checker (copy the whole dir; the .mpr alone is v2 without mprcontents/). MDL itself cannot declare a type parameter named String (`entity ` is a parse error), so the colliding case is only reachable via Studio Pro-authored models.", "refs": ["mendixlabs/mxcli#1183"]} +{"area": "mdl/catalog", "date": "2026-09-26", "symptom": "`impact Module.Entity.Attr` answered \"(no impact - element is not referenced)\" for an attribute a change activity sets and a page displays (Evora Factory Management: DigitalTwin.Machine.NumberOfIncidents); `impact` on an enumeration said the same; `callers` of a workflow started by a microflow said \"(no callers found)\". An agent auditing the app read these as safe-to-delete.", "cause": "The refs graph stopped at documents: no ATTRIBUTE / ENUMERATION / ENUMERATION_VALUE targets at all, and no edge for WorkflowCallAction, for a page navigating an association, or for an import/export mapping mapping an entity. Page/snippet XPath constraints also had no TargetEntity, because resolveEntityRefFromBSON read EntityRef.QualifiedName, a key no stored DirectEntityRef carries (it is `Entity`; an IndirectEntityRef ends on its last step's DestinationEntity).", "file": "mdl/catalog/builder_member_refs.go (memberRefsInUnit, scanPaths, extractXPathRefs, extractEnumerationTypeRefs), builder_references.go (microflowActionRef WorkflowCallAction), builder_xpath.go (resolveEntityRefFromBSON), catalogdb.go (CatalogTx.Query)", "insight": "Member references are found by a RAW-document walk matching every string value against the names the model declares (whole-string = structured ref: MemberChange.Attribute, AttributeRef.Attribute, EntityRefStep.Association, EnumerationType.Enumeration, ObjectMappingElement.Entity; path tokens inside expressions = association paths and qualified enum values). A typed walk would reach only the sites someone wrote a case for; the raw walk reached 4043 attribute bindings on Evora with no per-type code, and exact-set matching means prose cannot produce an edge (Documentation is skipped anyway; the test's control is an unused attribute that must stay unreferenced). XPath is resolved separately because its context entity IS known: bare names resolve against the target entity and its generalizations, predicates after a path switch context, and an enum attribute compared to a literal names the value. What stays invisible is a bare member through a variable in an expression ($Order/Total) \u2014 so the executor must not say 'not referenced' (see the executor finding). New member kinds deliberately stay OUT of graphRefKinds and off graph_god_nodes' asset side: attributes are members, not assets, and reusing change/create/retrieve for them would have pulled every attribute into communities/centrality. Bump CatalogSchemaVersion for any new edge (refs are only written by REFRESH CATALOG FULL). Verified the lint output on Evora is byte-identical in counts before/after, so no rule changed verdicts silently."} diff --git a/mdl-examples/bug-tests/refs-graph-members-enums-workflows.mdl b/mdl-examples/bug-tests/refs-graph-members-enums-workflows.mdl new file mode 100644 index 000000000..00c2c6149 --- /dev/null +++ b/mdl-examples/bug-tests/refs-graph-members-enums-workflows.mdl @@ -0,0 +1,117 @@ +-- ============================================================================ +-- refs / impact / context: gaps in the cross-reference graph +-- ============================================================================ +-- +-- Found by an agent-orientation audit on Evora Factory Management. Before: +-- +-- impact DigitalTwin.Machine.NumberOfIncidents +-- -> (no impact - element is not referenced) +-- but a change activity sets it and a page displays it +-- impact DigitalTwin.ENUM_MachineStatus -> not referenced +-- callers AltairIntegration.WF_ScheduleTechnicianAppointment +-- -> (no callers found), though a microflow calls it +-- impact DigitalTwin.Machine +-- -> ProductionLine_Reset | retrieve printed twice; "MICROFLOW: 9" +-- over six microflows; summary order changed between runs +-- context DigitalTwin.Machine -> "Related Entities: (none found)", +-- with five associations +-- +-- The graph had no ATTRIBUTE / ENUMERATION / ENUMERATION_VALUE targets, no +-- microflow -> workflow edge, no page -> association edge and no mapping -> +-- entity edge; refs/impact printed a row per edge; context read association +-- edges from the wrong end. +-- +-- ---------------------------------------------------------------------------- +-- Running this example +-- +-- mxcli exec refs-graph-members-enums-workflows.mdl -p app.mpr +-- mxcli -p app.mpr -c "refresh catalog full" +-- mxcli impact -p app.mpr RefsGraph.Machine.Incidents +-- # MICROFLOW RefsGraph.ACT_Machine_Reset (member) +-- # PAGE RefsGraph.Machine_Details (member) +-- mxcli impact -p app.mpr RefsGraph.Machine.Status +-- # MICROFLOW RefsGraph.ACT_Machine_Reset (xpath) — the retrieve's constraint +-- mxcli impact -p app.mpr RefsGraph.MachineStatus +-- # ENTITY RefsGraph.Machine (type), MICROFLOW RefsGraph.ACT_Machine_Reset +-- # (value / xpath) — uses of the values are included +-- mxcli callers -p app.mpr RefsGraph.WF_Repair +-- # RefsGraph.ACT_Machine_Repair +-- mxcli impact -p app.mpr RefsGraph.Machine +-- # ACT_Machine_Reset listed once per kind; summary counts elements +-- mxcli context -p app.mpr RefsGraph.Machine +-- # Related Entities: RefsGraph.Line and RefsGraph.Incident, with the +-- # association and its direction +-- mxcli impact -p app.mpr RefsGraph.Machine.Unused +-- # "no references found to attribute ..." plus what was and was not +-- # checked — never "not referenced" +-- ============================================================================ + +create module RefsGraph; + +create enumeration RefsGraph.MachineStatus ( + Active 'Active', + Critical 'Critical' +); + +create persistent entity RefsGraph.Line ( + Name: String(100) +); +/ + +create persistent entity RefsGraph.Machine ( + Name: String(100), + Status: Enumeration(RefsGraph.MachineStatus), + Incidents: Integer, + Unused: String(20) +); +/ + +create persistent entity RefsGraph.Incident ( + Title: String(200) +); +/ + +create association RefsGraph.Machine_Line from RefsGraph.Machine to RefsGraph.Line; +create association RefsGraph.Incident_Machine from RefsGraph.Incident to RefsGraph.Machine; + +-- Two retrieves of the same entity: one `retrieve` reference, not two rows. +create microflow RefsGraph.ACT_Machine_Reset ($Line: RefsGraph.Line) +begin + retrieve $Critical from RefsGraph.Machine where [Status = 'Critical']; + retrieve $All from RefsGraph.Machine where [RefsGraph.Machine_Line = $Line]; + loop $M in $All + begin + if $M/Status = RefsGraph.MachineStatus.Critical then + change $M (Incidents = 0); + end if; + end loop; +end; +/ + +create page RefsGraph.Machine_Details +( + params: { $Machine: RefsGraph.Machine }, + title: 'Machine', + layout: Atlas_Core.Atlas_Default +) +{ + dataview dvMachine (DataSource: $Machine) { + textbox tbName (Label: 'Name', Attribute: Name) + textbox tbIncidents (Label: 'Incidents', Attribute: Incidents) + } +} + +create workflow RefsGraph.WF_Repair + parameter $Context: RefsGraph.Machine +begin + user task Inspect 'Inspect the machine' + page RefsGraph.Machine_Details + outcomes + 'Done' { }; +end workflow; + +create microflow RefsGraph.ACT_Machine_Repair ($Machine: RefsGraph.Machine) +begin + $Wf = call workflow RefsGraph.WF_Repair ($Machine); +end; +/ From 35bc2a470610b4b4d1425f4826ea6555931f8817 Mon Sep 17 00:00:00 2001 From: Ako Date: Sun, 27 Sep 2026 06:59:36 +0000 Subject: [PATCH 9/9] roundtrip: strike putget for SUB_Feedback_PostToAppInsights, fixed by trimming expression whitespace Co-Authored-By: Claude Opus 5.5 --- mdl/roundtrip/allowlist_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mdl/roundtrip/allowlist_test.go b/mdl/roundtrip/allowlist_test.go index 1373f0889..9d1d35b2d 100644 --- a/mdl/roundtrip/allowlist_test.go +++ b/mdl/roundtrip/allowlist_test.go @@ -178,7 +178,7 @@ var knownFailures = map[string]knownFailure{ "microflow FeedbackModule.ConvertBase64String": {laws: []law{lawGetPut, lawPutGet}, issue: "#721", why: "whole-document rebuild: curves, merges, case values (#721 A)"}, "microflow FeedbackModule.ConvertUUIDToURL": {laws: []law{lawGetPut}, issue: "#721", why: "whole-document rebuild: curves, merges, case values (#721 A)"}, "microflow FeedbackModule.PopulateUserAttributes": {laws: []law{lawGetPut}, issue: "#721", why: "whole-document rebuild: curves, merges, case values (#721 A)"}, - "microflow FeedbackModule.SUB_Feedback_PostToAppInsights": {laws: []law{lawGetPut, lawPutGet}, issue: "#721", why: "whole-document rebuild: curves, merges, case values (#721 A)"}, + "microflow FeedbackModule.SUB_Feedback_PostToAppInsights": {laws: []law{lawGetPut}, issue: "#721", why: "whole-document rebuild: curves, merges, case values (#721 A); putget fixed by #718 (expression whitespace)"}, "microflow FeedbackModule.SUB_Feedback_Sanitize": {laws: []law{lawGetPut}, issue: "#721", why: "whole-document rebuild: curves, merges, case values (#721 A)"}, "microflow FeedbackModule.SUB_Feedback_SendToServer": {laws: []law{lawGetPut, lawPutGet}, issue: "#721", why: "whole-document rebuild: curves, merges, case values (#721 A)"}, "microflow FeedbackModule.VAL_Feedback": {laws: []law{lawGetPut, lawPutGet}, issue: "#721", why: "whole-document rebuild: curves, merges, case values (#721 A)"},