Skip to content

Commit 699abfc

Browse files
Aayush MainiCopilot
andcommitted
Reconcile bare/rich component identities in ComponentsFound
Within a single detector, components registered under bare Ids (no DownloadUrl/SourceUrl) are now merged into their rich counterparts sharing the same BaseId. Changes: - ComponentRecorder.GetDetectedComponents(): group by BaseId, merge bare metadata (licenses, suppliers, containers) into all rich entries - DefaultGraphTranslationService.GatherSetOfDetectedComponentsUnmerged(): extend graph lookup to match on BaseId so rich components absorb graph data (roots, ancestors, devDep, scope, file paths) from bare-Id graphs - Tests for both reconciliation levels Addresses work item #2372676. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent af0cff9 commit 699abfc

File tree

24 files changed

+24489
-42
lines changed

24 files changed

+24489
-42
lines changed

docs/component-identity-merging.md

Lines changed: 293 additions & 0 deletions
Large diffs are not rendered by default.
Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
# Component Identity Reconciliation — Design
2+
3+
## Data Flow Overview
4+
5+
```
6+
Detector A (e.g., NpmComponentDetector)
7+
└─ ComponentRecorder A
8+
├─ SingleFileComponentRecorder (package.json)
9+
│ ├─ detectedComponentsInternal: { componentId → DetectedComponent }
10+
│ └─ DependencyGraph: nodes keyed by componentId
11+
└─ GetDetectedComponents(): merges across SingleFileComponentRecorders, groups by component.Id
12+
13+
Detector B (e.g., NpmComponentDetectorWithRoots)
14+
└─ ComponentRecorder B
15+
├─ SingleFileComponentRecorder (package-lock.json)
16+
│ ├─ detectedComponentsInternal: { componentId → DetectedComponent }
17+
│ └─ DependencyGraph: nodes keyed by componentId
18+
└─ GetDetectedComponents(): merges across SingleFileComponentRecorders, groups by component.Id
19+
20+
DefaultGraphTranslationService.GenerateScanResultFromProcessingResult()
21+
├─ GatherSetOfDetectedComponentsUnmerged(): iterates each detector's recorder,
22+
│ calls componentRecorder.GetDetectedComponents(), enriches with graph data
23+
├─ FlattenAndMergeComponents(): groups by component.Id + detector.Id, merges within groups
24+
│ → produces ComponentsFound
25+
└─ AccumulateAndConvertToContract(): iterates all detectors' graphs by file location
26+
→ produces DependencyGraphs
27+
```
28+
29+
## Three Reconciliation Points
30+
31+
Identity conflicts (bare Id vs rich Id for the same package) can arise at three levels. We fix bottom-up.
32+
33+
### Point 1: SingleFileComponentRecorder::RegisterUsage
34+
35+
**What it reconciles:** Same package within a single file producing different Ids.
36+
37+
**When this happens:** Unlikely for npm (a single lockfile either has all resolved URLs or none), but possible in theory if a file format contains asymmetric information about the same package in different sections.
38+
39+
**Current behavior:** `RegisterUsage` stores in `detectedComponentsInternal.GetOrAdd(componentId, detectedComponent)`. Two entries with different Ids for the same BaseId would be stored separately. Graph edges would reference whichever Id was used at registration time.
40+
41+
**Fix:** Before storing, check if an entry with the same `BaseId` exists. Only bare and rich merge — rich entries never merge with each other. Specifically:
42+
- **Incoming is bare, existing is rich:** Redirect the bare registration to use the existing rich entry's Id. Add graph edges under the rich Id.
43+
- **Incoming is rich, existing is bare:** Re-key the existing bare entry to the rich Id. Update the graph node key and parents' children sets. Enrich the stored component with URL data.
44+
- **Incoming is rich, existing is rich (different Ids):** Keep both as separate entries. No merge.
45+
46+
This requires a `BaseId → Id` lookup (e.g., a secondary dictionary) to find existing entries that share the same `BaseId`.
47+
48+
**Scope:** Defensive. Low priority since single-file conflicts are rare.
49+
50+
### Point 2: ComponentRecorder::GetDetectedComponents + GatherSetOfDetectedComponentsUnmerged
51+
52+
**What it reconciles:** Same package across different files read by one detector.
53+
54+
**When this happens:** A detector reads multiple manifest files. For example, `NpmComponentDetectorWithRoots` processes multiple lockfiles in a monorepo — one lockfile might have a resolved URL for `lodash@4.17.23` and another might not (e.g., an older lockfile format).
55+
56+
#### What already works
57+
58+
The existing code already handles cross-file and cross-graph reconciliation — **under the assumption that the same package always has the same Id:**
59+
60+
- **`ComponentRecorder::GetDetectedComponents()`** merges component-level metadata (LicensesConcluded, Suppliers, ContainerDetailIds) across `SingleFileComponentRecorder`s by grouping on `component.Id`.
61+
- **`GatherSetOfDetectedComponentsUnmerged()`** enriches each component with graph-level metadata (roots, ancestors, locations, devDep, scope) from every graph that contains the component by matching on `component.Component.Id`.
62+
63+
Both of these work correctly when `Id` is the same across files/graphs.
64+
65+
#### What we are fixing
66+
67+
When `DownloadUrl` or `SourceUrl` is set, `TypedComponent.Id` includes these values (via `GetExtendedIdProperties()`). If one file produces a bare Id and another produces a rich Id for the same package, the existing merge logic treats them as different packages because their Ids differ.
68+
69+
**The fix is not new reconciliation logic — it is extending the existing reconciliation to also recognize bare and rich entries as the same package when they share a `BaseId`.**
70+
71+
#### Key semantics of the scan result
72+
73+
`DefaultGraphScanResult.ComponentsFound` is produced by `GatherSetOfDetectedComponentsUnmerged` + `FlattenAndMergeComponents`.
74+
75+
`FlattenAndMergeComponents` groups by `component.Id + detector.Id`. This means the existing design **intentionally** keeps same-package-from-different-detectors as separate entries in `ComponentsFound`. We preserve this semantic — cross-detector reconciliation is **not** needed for `ComponentsFound`.
76+
77+
The reconciliation scope for `ComponentsFound` is **within a single detector** — i.e., within one `ComponentRecorder`. This is handled by `GetDetectedComponents()` and `GatherSetOfDetectedComponentsUnmerged`.
78+
79+
`ComponentRecorder.GetDetectedComponents()` is only used in production by `GatherSetOfDetectedComponentsUnmerged`, so it is safe to change.
80+
81+
#### Plan for ComponentsFound (two-step merge)
82+
83+
**Step 1 — `ComponentRecorder::GetDetectedComponents()`: Component-level metadata merge**
84+
85+
Current behavior groups by `component.Id`. Bare and rich entries for the same package end up in different groups.
86+
87+
New behavior:
88+
89+
1. Group by `component.Component.BaseId` (instead of `component.Id`).
90+
2. Within each BaseId group, separate entries into **rich** (Id != BaseId) and **bare** (Id == BaseId).
91+
3. Rich entries stay separate from each other (they have different Ids — e.g., different DownloadUrls).
92+
4. If rich entries exist, merge bare's component-level metadata into **each** rich entry:
93+
- `LicensesConcluded` (union)
94+
- `Suppliers` (union)
95+
- `ContainerDetailIds` (union)
96+
5. Drop the bare entry from the output.
97+
6. If no rich entries exist, keep the bare entry as-is.
98+
99+
**Step 2 — `GatherSetOfDetectedComponentsUnmerged()`: Graph-level metadata merge**
100+
101+
After Step 1, the bare entry is gone. Each rich component has the bare's component-level metadata. But the graph from the file that originally registered the bare entry stored the bare Id. The enrichment loop:
102+
103+
```csharp
104+
foreach (var graphKvp in dependencyGraphsByLocation.Where(x => x.Value.Contains(component.Component.Id)))
105+
```
106+
107+
...won't match graphs that contain the bare Id for components that are now keyed by a rich Id.
108+
109+
Fix: extend the graph lookup to also match on `BaseId`:
110+
111+
```csharp
112+
foreach (var graphKvp in dependencyGraphsByLocation.Where(x =>
113+
x.Value.Contains(component.Component.Id) || x.Value.Contains(component.Component.BaseId)))
114+
```
115+
116+
This way each rich component picks up graph-level metadata (roots, ancestors, locations, dev dependency, dependency scope) from the bare entry's graphs. Since all rich entries for the same BaseId run through this loop independently, the bare entry's graph data is absorbed into **all** of them — "merge into all" happens naturally.
117+
118+
#### Summary
119+
120+
| Step | Where | What merges | Into what |
121+
|---|---|---|---|
122+
| 1 | `GetDetectedComponents()` | Bare's LicensesConcluded, Suppliers, ContainerDetailIds | Each rich entry with same BaseId |
123+
| 2 | `GatherSetOfDetectedComponentsUnmerged()` | Bare's graph data (roots, ancestors, locations, devDep, scope) | Each rich entry with same BaseId |
124+
125+
Rich entries never merge with each other. Bare merges into all rich. If no rich exists, bare stays.
126+
127+
---
128+
129+
## DependencyGraphs in Scan Result
130+
131+
### Conclusion: No changes needed
132+
133+
After analysis, the `DependencyGraphs` field requires **no identity reconciliation**. Bare Ids remain in the graph output as-is. Downstream consumers must use `ComponentsFound` as the authoritative source for component identity and metadata.
134+
135+
### What DependencyGraphs contains
136+
137+
`DefaultGraphScanResult.DependencyGraphs` is produced by `GraphTranslationUtility.AccumulateAndConvertToContract()`. It iterates each detector's `ComponentRecorder`, walks every graph by file location, and merges them into a dictionary of `DependencyGraphWithMetadata` keyed by file location.
138+
139+
Each `DependencyGraphWithMetadata` contains:
140+
141+
| Field | Type | Content |
142+
|---|---|---|
143+
| `Graph` | `DependencyGraph` | Adjacency list: maps each component Id (string) to the set of Ids it depends on |
144+
| `ExplicitlyReferencedComponentIds` | `HashSet<string>` | Component Ids that are direct/explicit dependencies |
145+
| `DevelopmentDependencies` | `HashSet<string>` | Component Ids classified as dev-only |
146+
| `Dependencies` | `HashSet<string>` | Component Ids classified as production dependencies |
147+
148+
**Every field is purely structural** — string-based Ids and edges between them. There is no component metadata (no name, version, download URL, license, etc.) in the graph output. Component metadata lives exclusively in `ComponentsFound`.
149+
150+
### Why no reconciliation is needed
151+
152+
1. **No component metadata to enrich.** The graph output has no `TypedComponent` objects, no `DetectedComponent` wrappers — just string Ids forming an adjacency structure. There is nothing to "merge" in the metadata sense.
153+
154+
2. **Bare Ids cannot be rewritten to rich Ids.** A bare entry doesn't know which rich Id it should become. Consider: if `lodash@4.17.23` appears as a bare node in one graph and two different lockfiles produce two different rich Ids (with different `DownloadUrl`s), the bare Id has no way to choose which rich Id to rewrite to. Any rewriting would be ambiguous.
155+
156+
3. **Graph structure is still correct.** A bare Id in the graph still correctly represents the dependency relationships. The edges (who depends on whom) and classifications (dev vs prod, explicit vs transitive) are all accurate — they don't depend on whether the Id includes a URL suffix.
157+
158+
4. **Existing merge logic works.** `AccumulateAndConvertToContract` merges graphs across detectors by file location. Within a file location, it unions the graph edges and dependency sets. If one detector registered `lodash@4.17.23` (bare) and another registered `lodash@4.17.23 [DownloadUrl:...]` (rich), both appear in the merged graph. This is correct — they came from different detectors with different levels of information.
159+
160+
### Downstream contract
161+
162+
Consumers of the scan result that need to resolve a component Id from `DependencyGraphs` to its full metadata (name, version, download URL, license, etc.) must look it up in `ComponentsFound`. This was already implicitly the contract — `DependencyGraphs` never carried component metadata. The bare/rich split makes this explicit:
163+
164+
- A bare Id in `DependencyGraphs` can be matched to a rich entry in `ComponentsFound` via `BaseId`.
165+
- A rich Id in `DependencyGraphs` can be matched directly by `Id` in `ComponentsFound`.
166+
167+
This is a **documentation concern**, not a code change. Downstream tools (e.g., CRA, SBOM generators) should be aware that graph Ids may be bare even when richer component data exists in `ComponentsFound`.
168+
169+
## Open Questions
170+
171+
1. **Performance of BaseId lookup in graph search (Step 2):** The extended graph lookup checks both `Id` and `BaseId` for every component × every graph. This is a small constant factor increase. Likely acceptable since it's a linear scan already.
172+
2. **Are there ecosystems where a single file legitimately produces both bare and rich entries for the same package?** If so, Point 1 (RegisterUsage) becomes more than defensive.
173+
3. **Downstream consumer awareness:** Tools consuming `DependencyGraphs` (CRA, SBOM generators) need to know that graph Ids may be bare and must cross-reference `ComponentsFound` for full metadata. This may require documentation or a schema note in the scan result contract.

0 commit comments

Comments
 (0)