Skip to content

fix: improve generic Mermaid and Need template rendering - #832

Merged
AlexanderLanin merged 5 commits into
eclipse-score:mainfrom
etas-contrib:tool-verification-report-immediate-fixes
Sep 16, 2026
Merged

AlexanderLanin merged 5 commits into
eclipse-score:mainfrom
etas-contrib:tool-verification-report-immediate-fixes

Conversation

@AlexanderLanin

Copy link
Copy Markdown
Member

Why

This PR makes the reusable documentation rendering path reliable for generated metamodel diagrams and graph-driven Sphinx-Needs templates. Without these fixes, hyphenated type names can produce invalid or ambiguous Mermaid references, nested Needs can be missed by the file-based checks, and post-template rereads can lose the Needs needed to resolve links and backlinks.

What changed

  • Mermaid diagrams now use safe internal identifiers while retaining the original type names as displayed labels. Hyphenated names receive a short stable digest so normalization does not collapse distinct names.
  • The Mermaid runtime is aligned to version 11.17.2, and the corresponding golden HTML sample is updated.
  • Sphinx-Needs template state and helper naming now describe the build environment and temporarily removed Needs explicitly. Link traversal supports outgoing links and backlink fields, and type selection is documented accurately.
  • Post-template rereads preserve the temporarily removed Needs while rebuilding the document, then clear the temporary snapshot.
  • File-based Need counting recognizes indented nested directives.
  • Comments and extension documentation explain the non-obvious lifecycle and traversal behavior.

This PR contains reusable rendering and test-infrastructure fixes; it does not introduce the tool-verification report metamodel content.

This comment was marked as outdated.

@github-actions

Copy link
Copy Markdown
Contributor

Documentation preview for this pull request is available at:
pr-832: https://eclipse-score.github.io/docs-as-code/pr-832/


# Relative paths of all rst files in RST_DIR
RST_FILES = [str(f.relative_to(RST_DIR)) for f in Path(RST_DIR).rglob("*.rst")]
_NEED_DIRECTIVE_PATTERN = re.compile(r"^\s*\.\.\s+([A-Za-z][\w-]*)::")

@MaximilianSoerenPollak MaximilianSoerenPollak Sep 16, 2026

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.

No way this does only what it should.

But I don't think it matters here, as this is just a display helper, and if the regex is wrong it wont break something important.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this is our tests. so it can only break if we change the test-rst files to something incompatible.

And since this matches too much as you mentioned, there is a check for match.group(1) in need_directive_names

Comment thread src/extensions/score_sphinx_needs_templates/__init__.py
Comment on lines +108 to +134
if source is not None:
# Prefer Sphinx-Needs' backlink index when the source Need is
# present. Keep these results first, but do not assume that a
# non-empty index is complete: links injected later in the build
# may only be visible on the outgoing Need fields.
for link in source.get_backlinks(link_type, as_str=False):
target = _find_need(needs, link.to_link_string())
if target is not None and target["id"] not in linked_ids:
linked.append(target)
linked_ids.add(target["id"])

# During a post-template reread, Sphinx-Needs may not have rebuilt
# backlink caches yet. The current Need may also be temporarily absent
# from the live environment while its document is reread. Derive the
# reverse relation from outgoing links in all cases and merge it with
# the indexed results above. This catches new links while preserving
# the index order and avoids duplicate Needs.
source_id = _base_need_id(need_id)
for candidate in needs.values():
points_to_source = any(
_base_need_id(link.to_link_string()) == source_id
for link in candidate.get_links(link_type, as_str=False)
)
if points_to_source and candidate["id"] not in linked_ids:
linked.append(candidate)
linked_ids.add(candidate["id"])
return linked

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.

I wonder if you can get around this double checking if you wait for the needs_locked event?
Cause as far as I know, then the needs are actually locked and everything is evaluated (so the need should be fully complete)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I looked into the event timing. In our Sphinx-Needs version the event is called needs-before-sealing (there is no needs_locked event). It runs after link/backlink resolution, just before the Need data is locked.

That is useful for inspecting finalized links, but this extension needs to reread report documents, which removes and recreates the page’s Needs. Doing that after link resolution would add the rebuilt Needs too late for that resolution pass (and is outside the intended read phase for adding/removing Needs).

So the current env-updated reread is intentional: it runs after the merged Needs and injected testcase links are available, while the collection is still in its read/mutable phase. Since backlinks are not finalized yet at that point, the helper derives reverse links from outgoing links as a fallback.

Comment on lines +237 to +241
finally:
# The exception, if any, must still propagate. Clear the process-local
# snapshot first so stale Needs cannot affect later rereads or builds
# that continue in the same Python process.
_temporarily_removed_needs = {}

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.

Finally will always run no?
so this happens always, not just in an exception like the comment states.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yeah... to be fair the comment says "if any". I can improve wording!

@MaximilianSoerenPollak MaximilianSoerenPollak left a comment

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.

Some questions

@MaximilianSoerenPollak MaximilianSoerenPollak left a comment

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.

Questions answered. Looks good to me now.

@AlexanderLanin
AlexanderLanin merged commit 76a851a into eclipse-score:main Sep 16, 2026
20 checks passed
@AlexanderLanin
AlexanderLanin deleted the tool-verification-report-immediate-fixes branch September 16, 2026 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

3 participants