From d95a42adf7e288d20c1509ea57f70c77c8c24c8c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=EA=B9=80=EC=9E=AC=ED=98=84?=
 <231584193+kimdzhekhon@users.noreply.github.com>
Date: Sun, 19 Jul 2026 09:42:38 +0900
Subject: [PATCH] fix(manifest_ingest): scope Maven pom.xml dependency parsing
 correctly

_parse_pom() had two accuracy bugs found by code review:

1. root.findall(".//dependencies/dependency") searches the whole tree,
   so it also picked up <dependencyManagement> entries (version pins,
   not real dependencies) and <build><plugins><plugin> dependencies as
   if they were project dependencies, producing false depends_on edges.
   Scoped to root-level <dependencies> plus <profiles><profile>
   dependencies, which are the only legitimate dependency sources.

2. A child module's <groupId>/<version> are commonly omitted and
   inherited from <parent>. The old code left them unset in that case,
   so the node id became a bare artifactId instead of "group:artifact",
   silently breaking depends_on edges from any other pom that references
   the dependency by its full coordinates. Falls back to
   parent/groupId and parent/version when the top-level element is
   absent.

Found via codex-assisted code review, verified against the actual code
before delegating the fix.
---
 graphify/manifest_ingest.py   |  9 ++++---
 tests/test_manifest_ingest.py | 49 +++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/graphify/manifest_ingest.py b/graphify/manifest_ingest.py
index ae3aa61fc..18f39fffd 100644
--- a/graphify/manifest_ingest.py
+++ b/graphify/manifest_ingest.py
@@ -226,17 +226,20 @@ def _parse_pom(text: str) -> dict | None:
     text = re.sub(r'\sxmlns="[^"]*"', '', text, count=1)
     root = ET.fromstring(text)
     aid = root.findtext("artifactId")
-    gid = root.findtext("groupId")
+    gid = root.findtext("groupId") or root.findtext("parent/groupId")
+    version = root.findtext("version") or root.findtext("parent/version")
     if not aid:
         return None
     name = f"{gid}:{aid}" if gid else aid
     deps: list[str] = []
-    for dep in root.findall(".//dependencies/dependency"):
+    project_deps = root.findall("dependencies/dependency")
+    profile_deps = root.findall("profiles/profile/dependencies/dependency")
+    for dep in project_deps + profile_deps:
         da = dep.findtext("artifactId")
         dg = dep.findtext("groupId")
         if da:
             deps.append(f"{dg}:{da}" if dg else da)
-    return {"name": name, "version": root.findtext("version"), "deps": deps}
+    return {"name": name, "version": version, "deps": deps}
 
 
 _PARSERS = {
diff --git a/tests/test_manifest_ingest.py b/tests/test_manifest_ingest.py
index 2b97f5fb9..74181b595 100644
--- a/tests/test_manifest_ingest.py
+++ b/tests/test_manifest_ingest.py
@@ -75,6 +75,55 @@ def test_pom_parses_artifact_and_deps(tmp_path):
     assert any(e["target"] == "pkg_org_lib_core" for e in r["edges"])
 
 
+def test_pom_includes_only_top_level_dependencies(tmp_path):
+    p = _write(tmp_path / "pom.xml",
+               '<project><groupId>com.acme</groupId><artifactId>app</artifactId>'
+               '<dependencies><dependency><groupId>org.lib</groupId>'
+               '<artifactId>runtime</artifactId></dependency></dependencies></project>')
+    r = extract_package_manifest(p)
+    deps = {e["target"] for e in r["edges"] if e["relation"] == "depends_on"}
+    assert deps == {"pkg_org_lib_runtime"}
+
+
+def test_pom_excludes_dependency_management_and_plugin_dependencies(tmp_path):
+    p = _write(tmp_path / "pom.xml",
+               '<project><groupId>com.acme</groupId><artifactId>app</artifactId>'
+               '<dependencyManagement><dependencies><dependency><groupId>org.managed</groupId>'
+               '<artifactId>bom-entry</artifactId></dependency></dependencies></dependencyManagement>'
+               '<build><plugins><plugin><dependencies><dependency><groupId>org.plugin</groupId>'
+               '<artifactId>helper</artifactId></dependency></dependencies></plugin></plugins></build>'
+               '</project>')
+    r = extract_package_manifest(p)
+    deps = {e["target"] for e in r["edges"] if e["relation"] == "depends_on"}
+    assert deps == set()
+
+
+def test_pom_uses_parent_group_id_when_project_group_id_is_missing(tmp_path):
+    p = _write(tmp_path / "pom.xml",
+               '<project><parent><groupId>com.parent</groupId><artifactId>parent</artifactId>'
+               '<version>1.0</version></parent><artifactId>child</artifactId></project>')
+    r = extract_package_manifest(p)
+    assert _pkg_nodes(r)[0]["label"] == "com.parent:child"
+
+
+def test_pom_uses_parent_version_when_project_version_is_missing(tmp_path):
+    p = _write(tmp_path / "pom.xml",
+               '<project><parent><groupId>com.parent</groupId><artifactId>parent</artifactId>'
+               '<version>1.2.3</version></parent><artifactId>child</artifactId></project>')
+    r = extract_package_manifest(p)
+    assert _pkg_nodes(r)[0]["version"] == "1.2.3"
+
+
+def test_pom_includes_profile_dependencies(tmp_path):
+    p = _write(tmp_path / "pom.xml",
+               '<project><groupId>com.acme</groupId><artifactId>app</artifactId><profiles><profile>'
+               '<id>optional-feature</id><dependencies><dependency><groupId>org.profile</groupId>'
+               '<artifactId>feature</artifactId></dependency></dependencies></profile></profiles></project>')
+    r = extract_package_manifest(p)
+    deps = {e["target"] for e in r["edges"] if e["relation"] == "depends_on"}
+    assert deps == {"pkg_org_profile_feature"}
+
+
 # ── #1377: a package referenced by N manifests is ONE node ───────────────────
 
 def test_apm_dependency_collapses_to_single_canonical_node(tmp_path):