refactor: expose bundle target metadata by AlexanderLanin · Pull Request #841 · eclipse-score/docs-as-code

AlexanderLanin · 2026-09-20T00:13:07Z

What this achieves

Bundle metadata and direct code targets are now carried from Bazel through the
composition manifest into Python. Python can therefore receive the information
about which bundle declared a physical entry and which implementation targets
belong directly to it.

This PR only transports and parses that information. It does not yet use the
new metadata for additional runtime behavior.

Why

Bazel already knows the declaring bundle and its direct code targets while it
composes nested and cross-repository bundles. The existing manifest discarded
that information and exposed only physical mount data. Reconstructing the
logical ownership later from paths would be unreliable, especially after
rebasing bundles into a larger documentation tree.

The metadata must therefore cross the Bazel/Python boundary before Python
features can consume it.

Changes

Add bundle label, bundle name, root-bundle status, and direct code-target
metadata to the composition manifest.
Parse the new manifest fields into Python data classes.
Generate the manifest from the actual root bundle, while keeping the root
entry out of the Sphinx mount list.
Document the remaining data-based compatibility paths slated for removal.

github-actions · 2026-09-20T01:09:36Z

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

Copilot

Copilot review overview

🟢 Approval recommended

The producer, parser, runtime filtering, documentation, and regression fixtures consistently implement the expanded manifest contract.

Review effort: Balanced
Findings: 1 · 1

Open (2)

Update manifest fixtures to the new producer-shaped contract
Skip root bundle entries when building mounted watch directories

AlexanderLanin · 2026-09-20T01:16:25Z

those 2 are old and already fixed. copilot doesnt notice it :/

a-zw · 2026-09-21T09:32:44Z

 * ``entry_doc`` — the canonical entry document declared by the source bundle.
-* ``external`` — whether the directory belongs to another Bazel module.
+* ``external`` — whether the directory belongs to another Bazel module;
+* ``bundle`` — the declaring bundle's Bazel label, name, and direct targets;


What is the bundle metadata useful for? I can only imagine misuse, i.e. documentation logic coupled more tightly to Bazel.

I would prefer that this bundle attribute is not provided/necessary to Python code.

goal is to link e.g. component needs to bazel targets to source code. Visible effects will take some additional PRs.

Specifically currently there is a heuristic for how to apply code coverage to component needs. This approach will make the path deterministic / safe.

Automated response: The metadata is needed to connect a document or Need to the Bazel bundle that supplied it and to that bundle's direct implementation targets. The current code uses a path-based heuristic for applying code coverage to component Needs; carrying this association from Bazel into Python makes that lookup explicit and deterministic. Python receives the already-resolved metadata and does not reconstruct Bazel labels or call Bazel APIs. This PR only transports the information; later processing will consume it.

a-zw · 2026-09-21T09:35:10Z

+<!-- ----------------------------------------------------------------------------
+  Copyright (c) 2026 Contributors to the Eclipse Foundation
+
+  See the NOTICE file(s) distributed with this work for additional
+  information regarding copyright ownership.
+
+  This program and the accompanying materials are made available under the
+  terms of the Apache License Version 2.0 which is available at
+  https://www.apache.org/licenses/LICENSE-2.0
+
+  SPDX-License-Identifier: Apache-2.0
+----------------------------------------------------------------------------- -->
+


Suggested change



We don't prefix Markdown files with this copyright header, do we?

we do... hopefully this is even enforced
https://github.com/eclipse-score/tools/blob/main/cr_checker/tool/templates.ini#L79

We do, since changes '& improvements in the cr_checker

Automated response: The file was removed from this PR, so the Markdown copyright-header question no longer applies.

a-zw · 2026-09-21T09:36:33Z

+
+# Deprecated behavior to remove
+
+This file tracks compatibility paths that should be removed after callers have


A markdown file is not a good place to track deprecations. Rather, in Starlark we should print deprecation warnings.

That's true, but its a different goal. What I wanted here is a list for us of stuff that we should remove on next major release. Let me just remove it from this PR and handle it separately.

I agree.

THough I think we should have some kind of marker though that will show what is gonna be deprecated next time.
At least in comments somewhere.

Automated response: Agreed for this PR. deprecations.md was removed. The compatibility code keeps a local TODO next to the behavior so the future cleanup remains visible, but this PR does not introduce a separate deprecation-warning mechanism.

MaximilianSoerenPollak

Got some questions.

MaximilianSoerenPollak · 2026-09-21T11:14:55Z

+    # This is the child bundle's own root before it is placed in the parent.
+    # It is distinct from ``root_bundle``, which describes ownership by the
+    # root bundle of the complete composition.
+    is_unplaced_bundle_root = not entry.mount_at
+    if is_unplaced_bundle_root:


I don't quiet understand this.

In what scenario does this occur? Is If it's not used as a bundle but instead standalone correct?
If that is the case I find the name a bit confusing, but also can't think of anything better atm.

If the bundle has not been mounted anywhere, it gets is_unplaced_bundle_root. So this is basically a very dynamic attribute which can change later on.

Automated response: The original variable was misleading and has been removed. The code now checks if not entry.mount_at directly and explains both branches. This path runs when a parent rebases entries from a child bundle; it is not specifically a standalone-versus-bundled mode.

MaximilianSoerenPollak · 2026-09-21T11:16:25Z

+        bundle_label = entry.bundle_label,
+        bundle_name = entry.bundle_name,
+        code_targets = entry.code_targets,
+        root_bundle = False,


What is the difference between root_gundle and is_unplaced_bundle_root ?

So e.g. a child mount has is_unplaced_bundle_root = True, until it is finally mounted somewhere.

Automated response: root_bundle is the persisted flag on the final manifest entry. It says whether the entry belongs to the root bundle of the current composition. The old is_unplaced_bundle_root expression was only a temporary condition while rebasing and has now been removed. A bundle can therefore be a root in a standalone composition and a child when embedded.

MaximilianSoerenPollak · 2026-09-21T11:18:49Z

@@ -427,6 +466,9 @@ _docs_bundle = rule(
        "bundle_mount_ats": attr.string_list(),
        "bundle_attach_tos": attr.string_list(),
        "data": attr.label_list(allow_files = True),


So this already has the other things? root_bundle label & name etc. ? Just Code targets was missing?

Automated response: These fields are introduced as one manifest contract. The previous manifest did not carry the bundle label, bundle name, root status, or direct code targets. code_targets is the new payload, while label, name, and root_bundle identify which bundle the targets belong to and how the entry participates in the current composition.

MaximilianSoerenPollak · 2026-09-21T11:19:33Z

            data = own_data,
+            bundle_label = own_bundle_label,
+            bundle_name = own_bundle_name,
+            root_bundle = True,


How do you know that this is a root_bundle at this point?

All bundles start with the assumption that they are roots, until they are placed somewhere. And _rebase_bundle_entry sets it to false.

Automated response: At this point _docs_bundle_impl is creating an entry directly from the current bundle's own source or data. Direct entries are root entries of the current composition and are therefore marked true. If a parent embeds the bundle, _rebase_bundle_entry creates the composed entry and sets root_bundle to false. Root status is composition-specific, not a permanent property of the bundle.

MaximilianSoerenPollak · 2026-09-21T11:19:49Z

            data = own_data,
+            bundle_label = own_bundle_label,
+            bundle_name = own_bundle_name,
+            root_bundle = True,


Same question as above

Automated response: The same explanation applies here: this is the current bundle's direct explicit-source entry, so it starts as a root entry of the current composition. When a parent embeds it, _rebase_bundle_entry marks the resulting child entry as false.

MaximilianSoerenPollak · 2026-09-21T11:20:53Z


-def _mounts_manifest_impl(ctx):
-    """Generate the canonical Sphinx mount manifest."""
+def _sorted_code_targets(targets):
+    """Return target metadata in deterministic label/type order.
+
+    Bundle declarations are ordered for source composition, but metadata
+    consumers should not observe incidental declaration ordering. Encoding the
+    pair before sorting keeps each label associated with its rule type.
+    """
+    encoded = sorted([
+        target.label + "\n" + target.type
+        for target in targets
+    ])
+    return [
+        {
+            "label": value.split("\n")[0],
+            "type": value.split("\n", 1)[1],
+        }
+        for value in encoded
+    ]


Why do we need to sort it?

no real reason, just for stable json output which makes it easier to use it in golden samples. will check again whether we really need it.

Automated response: No, sorting was not required. The sorting helper has been removed. code_targets are now emitted in the producer's declaration order; consumers treat them as a collection and do not assign meaning to that order.

MaximilianSoerenPollak · 2026-09-21T11:21:37Z

+    return [
+        {
+            "label": value.split("\n")[0],
+            "type": value.split("\n", 1)[1],


You are only splitting the first occurence here, is this on purpose?

Automated response: This is no longer relevant because the sorting helper and its artificial label/type newline encoding were removed. The current code does not split the combined string.

MaximilianSoerenPollak · 2026-09-21T11:24:37Z

+def _read_bundle_metadata(entry: dict[str, object]) -> BundleMetadata:
+    """Map the producer-owned bundle fields to their runtime dataclass."""
+    bundle = cast("dict[str, object]", entry["bundle"])
+    targets = cast("list[dict[str, str]]", bundle["code_targets"])
+    return BundleMetadata(
+        label=cast("str", bundle["label"]),
+        name=cast("str", bundle["name"]),
+        code_targets=tuple(
+            BazelTarget(label=target["label"], type=target["type"])
+            for target in targets
+        ),
+    )
+
+
+def _read_mount_spec(entry: dict[str, object]) -> MountSpec:
+    """Map one producer-owned manifest entry to a runtime mount spec."""
+    attach_to = cast("str", entry["attach_to"])
+    return MountSpec(
+        src_root=cast("str", entry["src_root"]),
+        runtime_path=cast("str", entry["runtime_path"]),
+        mount_at=cast("str", entry["mount_at"]),
+        attach_to=attach_to or None,
+        entry_doc=cast("str", entry["entry_doc"]),
+        external=cast("bool", entry["external"]),
+        repository=cast("str", entry["repository"]),
+        generated=cast("bool", entry["generated"]),
+        files=cast("list[str]", entry.get("files", [])),
+        data=cast("list[str]", entry["data"]),
+        root_bundle=cast("bool", entry["root_bundle"]),
+        bundle=_read_bundle_metadata(entry),
+    )


Should these not be methods of the class? Seems kinda llogical to me.

Automated response: Addressed. Parsing now lives in from_manifest_entry class methods on BazelTarget, BundleMetadata, and MountSpec. load_mounts_manifest delegates to MountSpec.from_manifest_entry, which composes the nested metadata objects.

MaximilianSoerenPollak · 2026-09-21T11:25:28Z

+<!-- ----------------------------------------------------------------------------
+  Copyright (c) 2026 Contributors to the Eclipse Foundation
+
+  See the NOTICE file(s) distributed with this work for additional
+  information regarding copyright ownership.
+
+  This program and the accompanying materials are made available under the
+  terms of the Apache License Version 2.0 which is available at
+  https://www.apache.org/licenses/LICENSE-2.0
+
+  SPDX-License-Identifier: Apache-2.0
+----------------------------------------------------------------------------- -->
+


We do, since changes '& improvements in the cr_checker

MaximilianSoerenPollak · 2026-09-21T11:26:08Z

+
+# Deprecated behavior to remove
+
+This file tracks compatibility paths that should be removed after callers have


I agree.

THough I think we should have some kind of marker though that will show what is gonna be deprecated next time.
At least in comments somewhere.

antonkri

I think, we should talk about the overall concept first. What is a bundle? How does it correspond to the s-core metamodel? Is it a unit, a component or a module? Can we have compositions of bundles? What is the overall goal? Which use-cases we cover with this?

AlexanderLanin · 2026-09-22T06:56:57Z

will call and in short: a bundle has no defined scope

MaximilianSoerenPollak

From my side all questions are answered and make sense.

as discussed in the meeting

feat: expose bundle target metadata

df5acb0

AlexanderLanin requested review from MaximilianSoerenPollak, a-zw, dcalavrezo-qorix and nradakovic as code owners September 20, 2026 00:13

github-project-automation Bot added this to INF - Infrastructure Community Sep 20, 2026

AlexanderLanin requested a balanced review from Copilot September 20, 2026 00:25

Copilot started reviewing on behalf of AlexanderLanin September 20, 2026 00:25 View session

This comment was marked as resolved.

Sign in to view

fix: address bundle metadata review feedback

87b2480

AlexanderLanin requested a balanced review from Copilot September 20, 2026 00:55

Copilot started reviewing on behalf of AlexanderLanin September 20, 2026 00:56 View session

This comment was marked as resolved.

Sign in to view

AlexanderLanin requested a balanced review from Copilot September 20, 2026 01:12

Copilot started reviewing on behalf of AlexanderLanin September 20, 2026 01:12 View session

Copilot AI reviewed Sep 20, 2026

View reviewed changes

AlexanderLanin changed the title ~~feat: expose bundle target metadata~~ refactor: expose bundle target metadata Sep 20, 2026

a-zw previously requested changes Sep 21, 2026

View reviewed changes

AlexanderLanin mentioned this pull request Sep 21, 2026

refactor: retain bundle metadata for documents #845

Closed

MaximilianSoerenPollak reviewed Sep 21, 2026

View reviewed changes

refactor: clarify bundle metadata contract

6fdb805

AlexanderLanin force-pushed the feature/bundle-target-need-metadata branch from c3369bb to 6fdb805 Compare September 21, 2026 13:03

This was referenced Sep 21, 2026

feat: support toctree_index parameter #844

Merged

feat: scope verification reports to a report_version via valid_from #843

Merged

antonkri self-requested a review September 22, 2026 06:52

antonkri previously requested changes Sep 22, 2026

View reviewed changes

MaximilianSoerenPollak approved these changes Sep 22, 2026

View reviewed changes

antonkri self-requested a review September 22, 2026 09:13

AlexanderLanin merged commit e0e4d3a into eclipse-score:main Sep 22, 2026
22 checks passed

AlexanderLanin deleted the feature/bundle-target-need-metadata branch September 22, 2026 09:18

github-project-automation Bot moved this to Done in INF - Infrastructure Community Sep 22, 2026


		# Deprecated behavior to remove

		This file tracks compatibility paths that should be removed after callers have

Conversation

AlexanderLanin commented Sep 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this achieves

Why

Changes

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

github-actions Bot commented Sep 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

AlexanderLanin commented Sep 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MaximilianSoerenPollak left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

AlexanderLanin commented Sep 20, 2026 •

edited

Loading

antonkri left a comment •

edited

Loading

AlexanderLanin commented Sep 22, 2026 •

edited

Loading