refactor(plantuml): replace linker with idmap-based cross-diagram linking by AAmbuj · Pull Request #290 · eclipse-score/tooling

AAmbuj · 2026-06-24T09:35:18Z

Replace the separate linker tool with idmap sidecar files emitted directly by the parser.

Key changes:

Add puml_idmap crate: emits *.idmap.json sidecars with defines/references roles detected from diagram structure (component children, class methods/ variables, sequence participants)
Extend puml_cli with --source-name and --idmap-output-dir args; use the workspace-relative path identity instead of the basename for a stable, path-unique source identifier
Rewrite clickable_plantuml Sphinx extension: load idmaps at builder-inited, resolve references via FQN→alias lookup with a proximity tiebreak and a tie→no-link guard; emit correct relative URLs for svg_obj mode; and percent-encode injected PlantUML URLs
Update architectural_design.bzl: parser now emits 3 outputs (fbs, lobster, idmap); remove linker action and _linker attr
Delete plantuml/linker (replaced by idmap approach)

hoe-jo

Thanks for the PR, looks promising, some minor topics:

Remove basename collision fallback; make source-path matching exact

Problem: extension joins idmap source (workspace short path from --source-name) to the Sphinx plantuml node path; when they differ under Bazel it falls back to idmap_by_basename with first-seen-wins, so two proxy.puml in different packages silently mislink.

Define one canonical key = workspace-relative POSIX path; normalize idmap source and node filename into it. Determine the staging→workspace offset once in on_builder_inited from app.srcdir.

Delete idmap_by_basename, _resolve_idmap fallback, and _ENV_IDMAP_BY_BASENAME.
If two idmaps share a key, raise build error (not warning). If a node can't map to a key, skip + warn.
Acceptance: two same-basename diagrams in different dirs each link only to their own definer; duplicate keys fail the build.

Fix proximity tiebreak path namespace
Problem: _proximity_tiebreak/_common_prefix_length may compare a staging source key against workspace-relative candidates. Once P0-1 lands, all are canonical keys — verify and add an assertion. Acceptance: tiebreak only on canonical keys; tie still → no link + warning.
rust_test for puml_idmap
Add #[cfg(test)] + rust_test in BUILD. Cover: component children/none, class members, sequence participants, Empty, alias fallback, deterministic sort.
pytest for clickable_plantuml
New tests/ + py_test. Cover: index build, FQN-before-alias, self-link skip, single vs tie, svg_obj vs svg URL, percent-encode, anchor.

5: _escape_plantuml_url — # in _SAFE can break [[ ]]; verify one-URL-per-alias dedup matches old contract.
6: confirm DefaultInfo idmap leak into consumer srcs intended; else split provider.
7: README invariant: --source-name must equal srcdir-relative path; unique basenames.

AAmbuj · 2026-07-02T09:21:20Z

What changed

Replaced the old standalone PlantUML linker flow with an idmap-based cross-diagram linking pipeline.
Removed the linker implementation and migrated link-data generation into parser outputs.
Added a new Rust idmap module and wired parser/build targets to consume it.
Refactored clickable_plantuml to resolve links from canonical source-path keys (instead of basename-based matching).
Added path-aware definer resolution (FQN-first, alias fallback), ambiguity tiebreaking, and safer URL escaping/injection behavior.
Updated Bazel rules (architectural_design, unit_design, fmea, dependable_element, sphinx_module) to produce/stage/pass idmap sidecars correctly.
Added new Rust and Python test coverage for idmap generation and clickable link resolution.
Updated rule and architecture docs to reflect the new idmap pipeline and invariants.

Why

Basename-based matching could silently mislink when diagrams shared names across directories.
The old linker phase duplicated responsibility and increased complexity.
Canonical path matching + idmap sidecars provides deterministic cross-diagram linking and better scalability.
The new structure improves maintainability and keeps parser/link semantics in one coherent data pipeline.

AAmbuj · 2026-07-02T09:27:19Z

Test:

Unit tests (Python):

clickable_plantuml index build/loading
duplicate-key handling
FQN-before-alias resolution
self-link skip
single candidate vs tie behavior
svg_obj vs svg URL mode handling
percent-encoding and anchor handling
one-URL-per-alias contract

Unit tests (Rust):

component/class/sequence/empty idmap generation
alias fallback behavior
deterministic ordering/sorting

Bazel targets validated:

bazel test //plantuml/sphinx/clickable_plantuml/tests:test_clickable_plantuml
bazel test //plantuml/parser/puml_idmap:puml_idmap_test
bazel test //plantuml/parser/puml_cli:puml_cli_test

hoe-jo · 2026-07-02T15:49:48Z

Change the per-node "no resolvable source path" logger.warning to logger.debug. Diagrams that aren't idmap-backed are a normal case, not an error.
Optionally add one aggregated logger.info at end of build: "N plantuml nodes had no idmap (not linkable)".
Test: add a case with a plantuml node absent from source_keys and assert no WARNING record is emitted (via caplog).

hoe-jo · 2026-07-02T15:50:49Z

Revert to symlink for all files and instead make _node_source_key robust to symlinks by using os.path.realpath on both the node path and workspace_offset before prefix-matching. normpath doesn't resolve symlinks, which is the actual reason real files were needed — fixing it in Python removes the need to copy anything.

hoe-jo · 2026-07-02T15:59:32Z

+    let output_path = output_dir.join(format!("{file_stem}.idmap.json"));
+
+    let json = serde_json::to_string_pretty(&idmap)
+        .map_err(|e| io::Error::new(io::ErrorKind::Other, e))?;


io::Error::other(e)

hoe-jo · 2026-07-02T16:05:48Z

make staging identity-preserving:
Stage .puml/.idmap.json at srcdir/<short_path> (i.e. source_prefix="", no strip_prefix for these), so staged-relative == short_path == idmap source.
Then keep symlinks, drop the sed rewrite, and drop execroot surgery

The linker binary parsed FlatBuffers output to produce plantuml_links.json. It is superseded by the idmap-based approach where each diagram emits its own .idmap.json sidecar, making a separate link-resolution step unnecessary.

Replace all references to plantuml_links.json / standalone linker with the idmap sidecar flow. tooling_chain.puml updated to reflect the new pipeline.

Emits a per-source .idmap.json with defines[] / references[] entries used by the clickable_plantuml Sphinx extension to resolve cross-diagram links. Supports Component, Class, Sequence, Activity (empty), and FTA diagrams.

Calls write_idmap_to_file() for every resolved diagram. Passes short_path as source_name so both idmap and lobster outputs use the stable workspace-relative path rather than the sandbox-local filesystem path.

- _load_idmap_files() indexes all .idmap.json sidecars at build-finished time - _resolve_definer() uses FQN-first + proximity tiebreak to find the defining source - _node_source_key() maps each plantuml node back to its idmap source key - Removed: _load_link_mappings(), _collect_link_data() (linker-era code)

- architectural_design.bzl, unit_design.bzl: _run_puml_parser now returns (fbs, lobster, idmap); all callers updated - fmea.bzl: idmap sidecars collected from root_causes; add duplicate-basename guard (fail-fast with actionable error message) - sphinx_module.bzl: identity-preserving symlink staging for .puml and .idmap.json so source keys remain stable under any strip_prefix config

Covers: _load_idmap_files (index building, key conflicts), _node_source_key (symlink resolution, suffix matching, tie-breaking), _resolve_definer (FQN priority, alias fallback, self-link skip, warning on tie), _build_target_url, _escape_plantuml_url, _inject_links, and on_doctree_resolved end-to-end integration.

AAmbuj requested review from LittleHuba, castler, hoe-jo, limdor and ramceb as code owners June 24, 2026 09:35

github-project-automation Bot added this to INF - Infrastructure Community Jun 24, 2026

AAmbuj force-pushed the amsh_plantuml_linker_review_refactor branch from dbb35d4 to 6b7cf91 Compare June 24, 2026 09:59

AAmbuj self-assigned this Jun 24, 2026

AAmbuj force-pushed the amsh_plantuml_linker_review_refactor branch from 6b7cf91 to 024b9b9 Compare June 24, 2026 10:06

hoe-jo reviewed Jun 29, 2026

View reviewed changes

hoe-jo mentioned this pull request Jun 30, 2026

fixed cross-diagram linking and extend to class/sequence diagrams #242

Closed

AAmbuj force-pushed the amsh_plantuml_linker_review_refactor branch 2 times, most recently from 7662a39 to 2ee3857 Compare July 1, 2026 06:42

AAmbuj marked this pull request as draft July 1, 2026 07:23

AAmbuj force-pushed the amsh_plantuml_linker_review_refactor branch 8 times, most recently from 19a3445 to 8cae034 Compare July 2, 2026 09:06

AAmbuj marked this pull request as ready for review July 2, 2026 09:29

AAmbuj requested a review from hoe-jo July 2, 2026 09:29

hoe-jo reviewed Jul 2, 2026

View reviewed changes

AAmbuj force-pushed the amsh_plantuml_linker_review_refactor branch 3 times, most recently from a707cb0 to 5b7aa6a Compare July 3, 2026 07:27

AAmbuj force-pushed the amsh_plantuml_linker_review_refactor branch from 5b7aa6a to 91d64c7 Compare July 3, 2026 07:29

AAmbuj added 7 commits July 3, 2026 13:15

docs(plantuml): update architecture docs for idmap-based linking

08dd5f7

Replace all references to plantuml_links.json / standalone linker with the idmap sidecar flow. tooling_chain.puml updated to reflect the new pipeline.

feat(puml_idmap): add idmap Rust library for all diagram types

a8fe1a2

Emits a per-source .idmap.json with defines[] / references[] entries used by the clickable_plantuml Sphinx extension to resolve cross-diagram links. Supports Component, Class, Sequence, Activity (empty), and FTA diagrams.

feat(puml_cli): wire --idmap-output-dir and --source-name into CLI

c2c64cb

Calls write_idmap_to_file() for every resolved diagram. Passes short_path as source_name so both idmap and lobster outputs use the stable workspace-relative path rather than the sandbox-local filesystem path.

AAmbuj force-pushed the amsh_plantuml_linker_review_refactor branch from 91d64c7 to 8f4f83b Compare July 3, 2026 07:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor(plantuml): replace linker with idmap-based cross-diagram linking#290

refactor(plantuml): replace linker with idmap-based cross-diagram linking#290
AAmbuj wants to merge 7 commits into
eclipse-score:mainfrom
AAmbuj:amsh_plantuml_linker_review_refactor

AAmbuj commented Jun 24, 2026

Uh oh!

hoe-jo left a comment

Uh oh!

AAmbuj commented Jul 2, 2026 •

edited

Loading

Uh oh!

AAmbuj commented Jul 2, 2026

Uh oh!

hoe-jo Jul 2, 2026

Uh oh!

hoe-jo Jul 2, 2026

Uh oh!

hoe-jo Jul 2, 2026

Uh oh!

hoe-jo Jul 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

AAmbuj commented Jun 24, 2026

Uh oh!

hoe-jo left a comment

Choose a reason for hiding this comment

Uh oh!

AAmbuj commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AAmbuj commented Jul 2, 2026

Uh oh!

hoe-jo Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

hoe-jo Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

hoe-jo Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

hoe-jo Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

AAmbuj commented Jul 2, 2026 •

edited

Loading