Skip to content
48 changes: 41 additions & 7 deletions src/extensions/score_draw_uml_funcs/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,40 @@ def draw_module(
return structure_text, linkage_text, proc_impl_interfaces, proc_used_interfaces


def _resolve_component_for_view(
need: dict[str, str], all_needs: dict[str, dict[str, str]]
) -> dict[str, str]:
"""Resolve component architecture views to their owning component."""
if need.get("type") != "comp_arc_sta":
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this compare against "comp"?

return need

Comment on lines +395 to +401
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_resolve_component_for_view()'s type annotations are too narrow: need is annotated as dict[str, str], but the function relies on belongs_to being a list (as produced by sphinx-needs). With the current annotation, basedpyright will treat need.get('belongs_to') as str | None, making the isinstance(component_ref, list) branch effectively dead / incorrectly typed. Consider changing the helper (and potentially the relevant call sites) to accept dict[str, Any] / dict[str, dict[str, Any]] (consistent with the CallableType alias at the bottom of the file), or define a proper TypedDict for needs that allows list-valued links like belongs_to.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

component_ref = need.get("belongs_to")
if isinstance(component_ref, list) and component_ref:
component_id = component_ref[0]
if len(component_ref) > 1:
logger.info(
f"{need}: component static view has multiple belongs_to targets, "
f"using only first component: {component_id}"
)

else:
component_id = component_ref
if not component_id:
logger.info(f"{need}: missing belongs_to for component static view")
return need

component_need = all_needs.get(component_id, {})
if not component_need:
logger.info(f"{need}: belongs_to component {component_id} could not be found")
return need

if component_need.get("type") != "comp":
logger.info(f"{need}: belongs_to {component_id} is not a component")
return need

return component_need


# ╭──────────────────────────────────────────────────────────────────────────────╮
# │ Classes with hashing to enable caching │
# ╰──────────────────────────────────────────────────────────────────────────────╯
Expand Down Expand Up @@ -575,16 +609,16 @@ def __repr__(self):
def __call__(
self, need: dict[str, str], all_needs: dict[str, dict[str, str]]
) -> str:
structure_text, linkage_text, _, _ = draw_comp_incl_impl_int(
need, all_needs, dict(), dict(), True
component_need = _resolve_component_for_view(need, all_needs)
structure_text, linkage_text, proc_impl_interfaces, _ = draw_comp_incl_impl_int(
component_need, all_needs, dict(), dict(), True
)
Comment on lines +612 to 615
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR changes draw_component behavior for comp_arc_sta views, but there doesn't appear to be any existing test that exercises {{ draw_component(...) }} on a comp_arc_sta need (the metamodel RST tests cover draw_feature and draw_module only). Adding a small RST test case that renders a comp_arc_sta and asserts the expected interfaces/subcomponents appear would help prevent regressions in this area.

Copilot uses AI. Check for mistakes.

# Draw all implemented interfaces outside the boxes
local_impl_interfaces = draw_impl_interface(need, all_needs, set())
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now completely missing. Is that on purpose?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Before only local interface are drawn. Now this is extended to any interface. Not sure, why this difference was made in the past, but lead to missing interfaces. Now any impl interface is drawn. See the next line. The loop goes over any implementation interfaces and not only the local ones.


# Add all interfaces which are implemented by component to global list
# and provide implementation
for iface in local_impl_interfaces:
# and provide implementation. Use the interfaces collected during the
# recursive consists_of walk so component drawings also include
# interfaces implemented by nested subcomponents.
for iface in proc_impl_interfaces:
Comment on lines 617 to +621
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new comment says interfaces are collected during a "recursive consists_of walk", but draw_comp_incl_impl_int() only recurses into consists_of when white_box_view is true and then calls itself without passing white_box_view=True further down, so the walk is only one level deep. Either adjust the comment to match the actual behavior, or (if intended) pass white_box_view through to make the traversal fully recursive.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback adjust comment

# check for misspelled implements
if not all_needs.get(iface, []):
logger.info(f"{need}: implements {iface} could not be found")
Expand Down
Loading