From 7c34d2a9fcff9bcd0dba130eaac39ca4ae3ae858 Mon Sep 17 00:00:00 2001 From: Malcolm Nixon Date: Tue, 21 Jul 2026 10:14:22 -0400 Subject: [PATCH] Fix Describe children-count mismatch and label query entries per verb - QueryEngine.Describe now derives its 'Children: N' summary count from the same filtered entries list used to build the Entries table, so the count can never diverge from the number of rows actually shown (previously it counted every raw AST child node, including non-element comments/metadata annotations/imports that have no QualifiedName and are excluded from Entries). - QueryResultRenderer.RenderMarkdown now precedes each verb's entries table (or its 'no entries' fallback) with a verb-specific bold-text label (e.g. **Children** for describe, **Uses** for uses), so an empty result reads as an unremarkable, expected outcome for that verb instead of an ambiguous, possibly-broken one. The label is always plain bold text, never an ATX heading, so the report stays within a single Markdown section regardless of the caller's requested heading depth. - Added/updated unit and integration tests, design docs, reqstream requirements, and verification test-scenario write-ups. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/design/sysml2-tools-core/query.md | 21 ++++- docs/reqstream/sysml2-tools-core/query.yaml | 35 +++++++- docs/verification/sysml2-tools-core/query.md | 30 ++++++- .../Query/QueryEngine.cs | 4 +- .../Query/QueryResultRenderer.cs | 50 +++++++++-- .../Query/QueryStrings.resx | 2 +- .../Query/QueryRenderingTests.cs | 85 ++++++++++++++++++- .../Query/QueryVerbsTests.cs | 40 ++++++++- 8 files changed, 246 insertions(+), 21 deletions(-) diff --git a/docs/design/sysml2-tools-core/query.md b/docs/design/sysml2-tools-core/query.md index 5d7a6637..68d4c53c 100644 --- a/docs/design/sysml2-tools-core/query.md +++ b/docs/design/sysml2-tools-core/query.md @@ -110,7 +110,13 @@ flowchart TD 6. `Impact` performs a breadth-first transitive closure over incoming edges, optionally bounded by `WalkDepth`, with a visited set preventing infinite loops on cyclic graphs. 7. `Describe` reports the target element's own kind and qualified name in `Summary`, then adds - resolved supertypes, typing, annotations, applied metadata annotations, and direct children. + resolved supertypes, typing, annotations, applied metadata annotations, and a `Children: N` + count. `N` is the count of visible, named child entries actually placed in `Entries` (i.e. + direct children with a non-null `QualifiedName` that pass the `IncludeStdlib` visibility + rule), not the raw `element.Children.Count` - so the stated count always matches the number + of rows shown, even though non-element children (comments, metadata annotations, imports) + and, when `IncludeStdlib` is unset, stdlib-seeded children are present in `element.Children` + but excluded from both the count and the table. Metadata values preserve scalar booleans, numbers, and strings directly, while unsupported non-scalar values fall back to raw source text so information is never silently dropped. 8. `Hierarchy` walks specialization relationships recursively. `--direction up` follows @@ -131,9 +137,16 @@ flowchart TD supplied. 14. `QueryResultRenderer` sorts entries exactly once by `QualifiedName` (ordinal) for both Markdown and JSON. All verbs except `dependencies` render Markdown as a heading, optional - summary bullet list, and a shared table. `dependencies` is the one intentional exception: - Markdown is rendered as direction-grouped prose bullets after shortening the subject and - entry names with `QualifiedNameShortener`. `RenderJson` never shortens names and uses + summary bullet list, a verb-specific bold-text label (e.g. `**Children**` for `describe`, + `**Uses**` for `uses`), and then either the shared table or a verb-specific "no entries" + fallback line (e.g. `_No children._`) when there are none. Labeling the (possibly empty) + entries section tells the reader what kind of thing it holds, so a zero-row result reads as + an unremarkable, expected outcome rather than a broken query. The label is always plain bold + text, never an ATX heading, so the whole report stays within the single Markdown section + started by the main heading regardless of the caller's requested heading depth. + `dependencies` is the one intentional exception: Markdown is rendered as direction-grouped + prose bullets after shortening the subject and entry names with `QualifiedNameShortener`, + with no separate label. `RenderJson` never shortens names and uses `QueryResultSerializerContext` so the JSON shape remains fully qualified and AOT-safe. 15. `QueryResultExporter` renders first, then writes the exact Markdown or JSON text to the caller-supplied file path. Markdown is joined with `"\n"`; no parent-directory creation or diff --git a/docs/reqstream/sysml2-tools-core/query.yaml b/docs/reqstream/sysml2-tools-core/query.yaml index 057cd680..3b19993b 100644 --- a/docs/reqstream/sysml2-tools-core/query.yaml +++ b/docs/reqstream/sysml2-tools-core/query.yaml @@ -106,12 +106,17 @@ sections: - id: SysML2Tools-Core-Query-Describe title: >- The Query subsystem shall report a target element's own kind, qualified name, resolved - supertypes, typing, annotations, applied metadata annotations, and direct children - through the `describe` verb. + supertypes, typing, annotations, applied metadata annotations, and a `Children` count + through the `describe` verb, where the reported count always equals the number of + entries actually returned (visible, named children), never the raw count of the + element's underlying AST child nodes. justification: | A single-element fact sheet is the most common starting point for programmatic model exploration, consolidating information otherwise scattered across the raw source and - semantic model. + semantic model. A `Children` count that disagrees with the number of rows shown (as + happens when non-element children, such as comments/metadata/imports, or hidden + stdlib children, are counted but not listed) misleads a reader into thinking rows are + missing or the query is broken. tests: - Describe_ReportsKindSupertypesAnnotationsAndChildren - Describe_MultiLineComment_CollapsesToSingleSummaryLine @@ -121,6 +126,7 @@ sections: - Describe_MetadataWithUnsupportedListAttribute_FallsBackToRawText - Describe_MultipleMetadataAnnotations_ReportsEachIndependently - Describe_FormatJson_IncludesMetadataInSummaryArray + - Describe_ChildIncludesNonElementMetadataAnnotation_ChildrenCountMatchesEntryRows - id: SysML2Tools-Core-Query-Hierarchy title: >- @@ -236,3 +242,26 @@ sections: - WriteJsonAsync_HappyPath_MatchesRendererOutput - WriteMarkdown_MissingParentDirectory_PropagatesIoException - WriteJson_MissingParentDirectory_PropagatesIoException + + - id: SysML2Tools-Core-Query-EntriesLabeling + title: >- + For every verb except `dependencies`, Markdown rendering shall precede the entries + table (or its "no entries" fallback) with a verb-specific bold-text label and + fallback message naming what kind of thing the entries represent (e.g. "Children" for + `describe`, "Uses" for `uses`), falling back to a generic "Entries"/"_No entries._" + label for any verb without a specific mapping. The label shall always render as plain + bold text rather than an ATX heading, so the report never branches into a Markdown + sub-section regardless of the caller's requested heading depth. + justification: | + A bare, unlabeled table (or a bare "_No entries._" line) gives a reader no way to tell + whether zero rows is an unremarkable, expected outcome for that verb or a sign the + query is broken. Naming the entries per verb removes that ambiguity without requiring + the reader to already know each verb's semantics by heart. Using bold text instead of + a sub-heading keeps the whole report within a single Markdown section, which is easier + to read and simpler to embed into other documents. + tests: + - RenderMarkdown_NoEntries_ReportsNoEntries + - RenderMarkdown_EntriesPresent_IncludesVerbSpecificBoldLabel + - RenderMarkdown_UnrecognizedVerb_FallsBackToGenericEntriesLabel + - RenderMarkdown_ListOrFindVerb_UsesMatchingElementsLabel + - RenderMarkdown_MaxHeadingDepth_EntriesLabelStaysBoldTextNotHeading diff --git a/docs/verification/sysml2-tools-core/query.md b/docs/verification/sysml2-tools-core/query.md index d1a4ad17..4c3c519f 100644 --- a/docs/verification/sysml2-tools-core/query.md +++ b/docs/verification/sysml2-tools-core/query.md @@ -54,6 +54,14 @@ scenarios listed under "Test Scenarios (Tool Test Project)" below run via `dotne - `QualifiedNameShortener.Shorten` strips only the longest shared leading `::`-segment prefix across a pool of qualified names, always capped so every name keeps at least its own leaf segment, and rejects `null` pools or `null` pool entries. +- `describe`'s `Children: N` summary line always equals the number of rows in the `Entries` + table (visible, named children only), never the raw count of the element's underlying AST + child nodes. +- Every verb except `dependencies` precedes its entries table (or "no entries" fallback) with a + verb-specific bold-text label and fallback message (e.g. `**Children**` / `_No children._` for + `describe`), falling back to a generic `**Entries**` / `_No entries._` label for any + unrecognized verb. The label is always plain bold text, never an ATX heading, so the report + never branches into a Markdown sub-section regardless of the caller's requested heading depth. ### Test Scenarios @@ -77,7 +85,27 @@ supertypes are reported. `QueryEngine.Describe`, and verifies that annotations appear in the result summary. **`RenderMarkdown_NoEntries_ReportsNoEntries`**: Verifies that a non-`dependencies` result with -no entries renders the shared Markdown `_No entries._` fallback. +no entries is preceded by its verb-specific bold-text label (e.g. `**Uses**`) and renders the +verb-specific "no entries" fallback text (e.g. `_No outgoing references._`) instead of an empty +table. + +**`RenderMarkdown_EntriesPresent_IncludesVerbSpecificBoldLabel`**: Verifies that a +non-`dependencies` result with entries is also preceded by its verb-specific bold-text label +(e.g. `**Children**` for `describe`), not just the "no entries" fallback case, and that the +label is plain bold text rather than an ATX heading. + +**`RenderMarkdown_UnrecognizedVerb_FallsBackToGenericEntriesLabel`**: Verifies that a verb with +no specific entries-label mapping falls back to the generic `**Entries**` label and `_No +entries._` fallback text. + +**`RenderMarkdown_ListOrFindVerb_UsesMatchingElementsLabel`**: Verifies that both `list` +and `find` share the same `**Matching Elements**` label, since `find` is a filtered form +of `list` and their entries mean the same thing. + +**`RenderMarkdown_MaxHeadingDepth_EntriesLabelStaysBoldTextNotHeading`**: Verifies that at the +maximum Markdown heading depth (6), the entries label remains plain bold text (`**Children**`) +rather than becoming (or being mistaken for) an ATX heading, so the report never branches into +a Markdown sub-section at any valid heading depth. **`RenderMarkdown_DependenciesVerb_RendersBulletProseNotTable`**: Verifies that `dependencies` Markdown renders as grouped prose bullets rather than the shared table. diff --git a/src/DemaConsulting.SysML2Tools.Core/Query/QueryEngine.cs b/src/DemaConsulting.SysML2Tools.Core/Query/QueryEngine.cs index 37a7e1a3..c4571326 100644 --- a/src/DemaConsulting.SysML2Tools.Core/Query/QueryEngine.cs +++ b/src/DemaConsulting.SysML2Tools.Core/Query/QueryEngine.cs @@ -313,13 +313,13 @@ public static QueryResult Describe(SysmlWorkspace workspace, SysmlNode element, } } - summary.Add($"Children: {element.Children.Count}"); - var entries = element.Children .Where(c => c.QualifiedName is not null && IsVisible(c.QualifiedName, workspace, options.IncludeStdlib)) .Select(c => new QueryResultEntry { QualifiedName = c.QualifiedName!, Kind = DescribeKind(c) }) .ToList(); + summary.Add($"Children: {entries.Count}"); + return new QueryResult { Verb = "describe", diff --git a/src/DemaConsulting.SysML2Tools.Core/Query/QueryResultRenderer.cs b/src/DemaConsulting.SysML2Tools.Core/Query/QueryResultRenderer.cs index 4b181488..2ce6081f 100644 --- a/src/DemaConsulting.SysML2Tools.Core/Query/QueryResultRenderer.cs +++ b/src/DemaConsulting.SysML2Tools.Core/Query/QueryResultRenderer.cs @@ -18,10 +18,18 @@ public static class QueryResultRenderer { /// /// Renders a as Markdown lines: a heading, an optional summary - /// bullet list, then a compact table of entries sorted by - /// (ordinal). The dependencies verb - /// is the sole exception: after the heading, its entries are rendered as prose bullets - /// (see ) rather than a table. + /// bullet list, then a verb-specific bold-text label (e.g. "Children" for + /// describe, "Uses" for uses) followed by a compact table of entries + /// sorted by (ordinal), or a verb-specific + /// "no entries" fallback line (e.g. "_No children._") when there are none. The label and + /// fallback text tell the reader what kind of thing the entries represent, so an empty + /// result reads as an unremarkable, expected outcome rather than a broken query. The + /// label is always plain bold text rather than its own heading, so the whole report + /// stays within the single Markdown section started by the main heading - it never + /// branches into a sub-section, regardless of . The + /// dependencies verb is the sole exception: after the heading, its entries are + /// rendered as prose bullets (see ) rather than a + /// table, with no separate label. /// /// The result to render. /// @@ -77,9 +85,16 @@ public static IReadOnlyList RenderMarkdown( } var sorted = SortEntries(result.Entries); + var (entriesHeading, noEntriesText) = EntriesLabel(result.Verb); + + // Plain bold text (not an ATX heading) keeps the entries label within the same + // Markdown section as the main heading, regardless of the caller's heading depth. + lines.Add($"**{entriesHeading}**"); + lines.Add(""); + if (sorted.Count == 0) { - lines.Add("_No entries._"); + lines.Add(noEntriesText); return lines; } @@ -93,6 +108,31 @@ public static IReadOnlyList RenderMarkdown( return lines; } + /// + /// Maps a verb name to a human-readable heading for its entries table (or the fallback + /// text shown when it has no entries), so a reader can tell what kind of thing an empty + /// result means without already knowing the verb's semantics (e.g. "_No children._" for + /// describe makes clear that zero rows is an unremarkable, expected outcome for a + /// childless element, not a broken query). + /// + /// The query verb. + /// The heading text and "no entries" fallback text for that verb. + private static (string Heading, string NoEntriesText) EntriesLabel(string verb) => verb switch + { + "uses" => ("Uses", "_No outgoing references._"), + "used-by" => ("Used By", "_No incoming references._"), + "impact" => ("Impacted Elements", "_No impacted elements._"), + "describe" => ("Children", "_No children._"), + "hierarchy" => ("Related Types", "_No related types._"), + "requirements" => ("Requirement Relationships", "_No requirement relationships._"), + "interface" => ("Ports & Typed Features", "_No ports or typed features._"), + "connections" => ("Connection Endpoints", "_No connection endpoints._"), + "states" => ("States & Transitions", "_No states or transitions._"), + "list" or "find" => ("Matching Elements", "_No matching elements._"), + _ => ("Entries", "_No entries._") + }; + + /// /// Renders a as an indented JSON document via the /// source-generated serializer, with entries diff --git a/src/DemaConsulting.SysML2Tools.Tool/Query/QueryStrings.resx b/src/DemaConsulting.SysML2Tools.Tool/Query/QueryStrings.resx index 79283cc1..5beed964 100644 --- a/src/DemaConsulting.SysML2Tools.Tool/Query/QueryStrings.resx +++ b/src/DemaConsulting.SysML2Tools.Tool/Query/QueryStrings.resx @@ -245,7 +245,7 @@ sysml2tools query find --name Wheel test/SysMLModels/OMG/examples/VehicleExample/*.sysml - Markdown output: a '# query <verb>[: <element>]' heading, an optional bullet-list summary, then a '| Qualified Name | Kind | Detail |' table (empty results print '_No entries._'). + Markdown output: a '# query <verb>[: <element>]' heading, an optional bullet-list summary, then a verb-specific bold-text label (e.g. '**Children**') and a '| Qualified Name | Kind | Detail |' table (empty results print a verb-specific fallback, e.g. '_No children._'). JSON output: { "Verb", "Element", "Summary": [...], "Entries": [{ "QualifiedName", "Kind", "Detail", "Notes": [] }] }, entries sorted by QualifiedName. diff --git a/test/DemaConsulting.SysML2Tools.Tests/Query/QueryRenderingTests.cs b/test/DemaConsulting.SysML2Tools.Tests/Query/QueryRenderingTests.cs index 59e1c26b..0a848cfc 100644 --- a/test/DemaConsulting.SysML2Tools.Tests/Query/QueryRenderingTests.cs +++ b/test/DemaConsulting.SysML2Tools.Tests/Query/QueryRenderingTests.cs @@ -16,8 +16,9 @@ namespace DemaConsulting.SysML2Tools.Tests.Query; public class QueryRenderingTests { /// - /// RenderMarkdown on a result with no entries reports "No entries" rather than an empty - /// table. + /// RenderMarkdown on a result with no entries reports a verb-specific "no entries" + /// fallback line (rather than an empty table), and precedes it with a verb-specific + /// bold-text label naming what kind of entry the (empty) table would have held. /// [Fact] public void RenderMarkdown_NoEntries_ReportsNoEntries() @@ -26,7 +27,85 @@ public void RenderMarkdown_NoEntries_ReportsNoEntries() var lines = QueryResultRenderer.RenderMarkdown(result); - Assert.Contains(lines, l => l.Contains("No entries")); + Assert.Contains("**Uses**", lines); + Assert.Contains(lines, l => l.Contains("No outgoing references")); + } + + /// + /// RenderMarkdown adds a verb-specific bold-text label before the entries table (not + /// just before the "no entries" fallback), naming what kind of thing the table's rows + /// represent for that verb. The label is plain bold text, not an ATX heading, so the + /// whole report stays within the single Markdown section started by the main heading. + /// + [Fact] + public void RenderMarkdown_EntriesPresent_IncludesVerbSpecificBoldLabel() + { + var result = new QueryResult + { + Verb = "describe", + Element = "Model::Car", + Entries = [new QueryResultEntry { QualifiedName = "Model::Car::engine" }] + }; + + var lines = QueryResultRenderer.RenderMarkdown(result); + + Assert.Contains("**Children**", lines); + Assert.DoesNotContain(lines, l => l.StartsWith('#') && l.Contains("Children")); + } + + /// + /// RenderMarkdown gives an unrecognized verb the generic "Entries" label and "_No + /// entries._" fallback text, so any future verb added without an explicit label still + /// renders sensibly instead of throwing. + /// + [Fact] + public void RenderMarkdown_UnrecognizedVerb_FallsBackToGenericEntriesLabel() + { + var result = new QueryResult { Verb = "some-future-verb", Element = "Model::Car" }; + + var lines = QueryResultRenderer.RenderMarkdown(result); + + Assert.Contains("**Entries**", lines); + Assert.Contains("_No entries._", lines); + } + + /// + /// RenderMarkdown's entries label is always plain bold text - never an ATX heading - + /// regardless of the caller's requested heading depth, so the report never branches + /// into a Markdown sub-section even at the maximum heading depth (6). + /// + [Fact] + public void RenderMarkdown_MaxHeadingDepth_EntriesLabelStaysBoldTextNotHeading() + { + var result = new QueryResult + { + Verb = "describe", + Element = "Model::Car", + Entries = [new QueryResultEntry { QualifiedName = "Model::Car::engine" }] + }; + + var lines = QueryResultRenderer.RenderMarkdown(result, depth: 6); + + Assert.Equal("###### query describe: Model::Car", lines[0]); + Assert.Contains("**Children**", lines); + Assert.DoesNotContain(lines.Skip(1), l => l.StartsWith('#')); + } + + /// + /// RenderMarkdown gives both "list" and "find" the same "Matching Elements" bold-text + /// label, since "find" is a filtered form of "list" and their entries mean the same + /// thing. + /// + [Theory] + [InlineData("list")] + [InlineData("find")] + public void RenderMarkdown_ListOrFindVerb_UsesMatchingElementsLabel(string verb) + { + var result = new QueryResult { Verb = verb }; + + var lines = QueryResultRenderer.RenderMarkdown(result); + + Assert.Contains("**Matching Elements**", lines); } /// diff --git a/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QueryVerbsTests.cs b/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QueryVerbsTests.cs index c1980752..0409225b 100644 --- a/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QueryVerbsTests.cs +++ b/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QueryVerbsTests.cs @@ -238,6 +238,40 @@ part def Car specializes Vehicle { Assert.Contains("Children: 1", output); } + /// + /// 'describe' reports a Children: N count that always matches the number of rows + /// in the Entries table. A bare metadata annotation (@Critical;) is a non-element + /// child with no qualified name, so it is present in the underlying AST child list but + /// excluded from both the count and the table; only the one real child (engine) + /// is counted and shown. + /// + [Fact] + public async Task Describe_ChildIncludesNonElementMetadataAnnotation_ChildrenCountMatchesEntryRows() + { + const string sysml = """ + package Model { + metadata def Critical; + + part def Car { + @Critical; + part engine; + } + } + """; + + var (output, exitCode) = await QueryTestFixtures.RunQueryAsync( + sysml, "describe", "--element", "Model::Car"); + + Assert.Equal(0, exitCode); + Assert.Contains("Children: 1", output); + Assert.Contains("Model::Car::engine", output); + + var tableRows = output + .Split('\n') + .Count(line => line.TrimStart().StartsWith("| Model::Car::", StringComparison.Ordinal)); + Assert.Equal(1, tableRows); + } + /// /// 'describe' collapses a multi-line comment/documentation annotation into a single /// summary line, so the Markdown output keeps one fact per bullet rather than letting @@ -265,9 +299,11 @@ part def Car { Assert.Contains("Documentation: A multi-line doc comment.", output); // The bullet list must remain one fact per line: no bullet's text should itself - // contain an embedded newline, and no orphan '*' continuation-marker lines exist. + // contain an embedded newline, and no orphan '* ' continuation-marker lines exist + // (checked as "* " with a trailing space, so this doesn't false-positive on + // legitimate "**Bold**" entries-label lines, which have no space after the stars). var lines = output.Split('\n'); - Assert.DoesNotContain(lines, line => line.Trim().StartsWith('*')); + Assert.DoesNotContain(lines, line => line.Trim().StartsWith("* ", StringComparison.Ordinal)); } ///